From: Wayne Davison <wayned@samba.org>
Date: Fri, 20 Feb 2009 07:02:03 +0000 (-0800)
Subject: Added a way for supplementary groups to be specified in the rsyncd.conf
X-Git-Url: https://mattmccutchen.net/rsync/rsync.git/commitdiff_plain/7f367bb1b4348ba1edb863ee13f8281471785e09

Added a way for supplementary groups to be specified in the rsyncd.conf
file.  Also made explicitly-set uid/gid values no longer ignored by a
daemon that was not run by a super-user.
---

diff --git a/NEWS b/NEWS
index 89f79ce9..2309aff0 100644
--- a/NEWS
+++ b/NEWS
@@ -38,6 +38,10 @@ Changes since 3.0.4:
     - Added the "reverse lookup" parameter to the rsync daemon config file to
       allow reverse-DNS lookups to be disabled.
 
+    - Added a way for supplementary groups to be specified in the rsyncd.conf
+      file.  Also made explicitly-set uid/gid values no longer ignored by a
+      daemon that was not run by a super-user.
+
   EXTRAS:
 
     - Added an "instant-rsyncd" script to the support directory, which makes
diff --git a/clientserver.c b/clientserver.c
index 059be75d..49238b1e 100644
--- a/clientserver.c
+++ b/clientserver.c
@@ -62,6 +62,8 @@ extern char *iconv_opt;
 extern iconv_t ic_send, ic_recv;
 #endif
 
+#define MAX_GID_LIST 32
+
 char *auth_user;
 int read_only = 0;
 int module_id = -1;
@@ -81,6 +83,9 @@ static int rl_nulls = 0;
 static struct sigaction sigact;
 #endif
 
+static gid_t gid_list[MAX_GID_LIST];
+static int gid_count = 0;
+
 /* Used when "reverse lookup" is off. */
 const char undetermined_hostname[] = "UNDETERMINED";
 
@@ -408,13 +413,75 @@ static int path_failure(int f_out, const char *dir, BOOL was_chdir)
 	return -1;
 }
 
+static int add_a_group(int f_out, const char *gname)
+{
+	gid_t gid;
+	if (!name_to_gid(gname, &gid)) {
+		if (!isDigit(gname)) {
+			rprintf(FLOG, "Invalid gid %s\n", gname);
+			io_printf(f_out, "@ERROR: invalid gid %s\n", gname);
+			return -1;
+		}
+		gid = atol(gname);
+	}
+	if (gid_count == MAX_GID_LIST) {
+		rprintf(FLOG, "Too many groups specified via gid parameter.\n");
+		io_printf(f_out, "@ERROR: too many groups\n");
+		return -1;
+	}
+	gid_list[gid_count++] = gid;
+	return 0;
+}
+
+static struct passwd *want_all_groups(int f_out, uid_t uid)
+{
+#if defined HAVE_GETGROUPLIST || defined HAVE_INITGROUPS
+	struct passwd *pw;
+	if ((pw = getpwuid(uid)) == NULL) {
+		rsyserr(FLOG, errno, "getpwuid failed");
+		io_printf(f_out, "@ERROR: getpwuid failed\n");
+		return NULL;
+	}
+# ifdef HAVE_GETGROUPLIST
+	/* Get all the process's groups, with the pw_gid group first. */
+	gid_count = MAX_GID_LIST;
+	if (getgrouplist(pw->pw_name, pw->pw_gid, gid_list, &gid_count) < 0) {
+		rsyserr(FLOG, errno, "getgrouplist failed");
+		io_printf(f_out, "@ERROR: getgrouplist failed\n");
+		return NULL;
+	}
+	/* Paranoia: is the default group not first in the list? */
+	if (gid_list[0] != pw->pw_gid) {
+		int j;
+		for (j = 0; j < gid_count; j++) {
+			if (gid_list[j] == pw->pw_gid) {
+				gid_list[j] = gid_list[0];
+				gid_list[0] = pw->pw_gid;
+				break;
+			}
+		}
+	}
+# else
+	/* Start with the default group and initgroups() will add the reset. */
+	gid_count = 1;
+	gid_list[0] = pw->pw_gid;
+# endif
+	return pw;
+#else
+	rprintf(FLOG, "This rsync does not support a gid of \"*\"\n");
+	io_printf(f_out, "@ERROR: invalid gid setting.\n");
+	return NULL;
+#endif
+}
+
 static int rsync_module(int f_in, int f_out, int i, const char *addr, const char *host)
 {
 	int argc;
 	char **argv, **orig_argv, **orig_early_argv, *module_chdir;
 	char line[BIGPATHBUFLEN];
-	uid_t uid = (uid_t)-2;  /* canonically "nobody" */
-	gid_t gid = (gid_t)-2;
+	struct passwd *pw = NULL;
+	uid_t uid;
+	int set_uid;
 	char *p, *err_msg = NULL;
 	char *name = lp_name(i);
 	int use_chroot = lp_use_chroot(i);
@@ -486,37 +553,47 @@ static int rsync_module(int f_in, int f_out, int i, const char *addr, const char
 	if (logfile_format_has_i || log_format_has(logfile_format, 'o'))
 		logfile_format_has_o_or_i = 1;
 
-	am_root = (MY_UID() == 0);
+	uid = MY_UID();
+	am_root = (uid == 0);
 
-	if (am_root) {
-		p = lp_uid(i);
+	p = *lp_uid(i) ? lp_uid(i) : am_root ? NOBODY_USER : NULL;
+	if (p) {
 		if (!name_to_uid(p, &uid)) {
 			if (!isDigit(p)) {
 				rprintf(FLOG, "Invalid uid %s\n", p);
 				io_printf(f_out, "@ERROR: invalid uid %s\n", p);
 				return -1;
 			}
-			uid = atoi(p);
+			uid = atol(p);
 		}
+		set_uid = 1;
+	} else
+		set_uid = 0;
 
-		p = lp_gid(i);
-		if (!name_to_gid(p, &gid)) {
-			if (!isDigit(p)) {
-				rprintf(FLOG, "Invalid gid %s\n", p);
-				io_printf(f_out, "@ERROR: invalid gid %s\n", p);
+	p = *lp_gid(i) ? strtok(lp_gid(i), ", ") : NULL;
+	if (p) {
+		/* The "*" gid must be the first item in the list. */
+		if (strcmp(p, "*") == 0) {
+			if ((pw = want_all_groups(f_out, uid)) == NULL)
+				return -1;
+		} else if (add_a_group(f_out, p) < 0)
+			return -1;
+		while ((p = strtok(NULL, ", ")) != NULL) {
+#if defined HAVE_INITGROUPS && !defined HAVE_GETGROUPLIST
+			if (pw) {
+				rprintf(FLOG, "This rsync cannot add groups after \"*\".\n");
+				io_printf(f_out, "@ERROR: invalid gid setting.\n");
 				return -1;
 			}
-			gid = atoi(p);
+#endif
+			if (add_a_group(f_out, p) < 0)
+				return -1;
 		}
+	} else if (am_root) {
+		if (add_a_group(f_out, NOBODY_GROUP) < 0)
+			return -1;
 	}
 
-	/* TODO: If we're not root, but the configuration requests
-	 * that we change to some uid other than the current one, then
-	 * log a warning. */
-
-	/* TODO: Perhaps take a list of gids, and make them into the
-	 * supplementary groups. */
-
 	module_dir = lp_path(i);
 	if (use_chroot) {
 		if ((p = strstr(module_dir, "/./")) != NULL) {
@@ -705,34 +782,33 @@ static int rsync_module(int f_in, int f_out, int i, const char *addr, const char
 		}
 	}
 
-	if (am_root) {
-		/* XXXX: You could argue that if the daemon is started
-		 * by a non-root user and they explicitly specify a
-		 * gid, then we should try to change to that gid --
-		 * this could be possible if it's already in their
-		 * supplementary groups. */
-
-		/* TODO: Perhaps we need to document that if rsyncd is
-		 * started by somebody other than root it will inherit
-		 * all their supplementary groups. */
-
-		if (setgid(gid)) {
-			rsyserr(FLOG, errno, "setgid %d failed", (int)gid);
+	if (gid_count) {
+		if (setgid(gid_list[0])) {
+			rsyserr(FLOG, errno, "setgid %ld failed", (long)gid_list[0]);
 			io_printf(f_out, "@ERROR: setgid failed\n");
 			return -1;
 		}
 #ifdef HAVE_SETGROUPS
-		/* Get rid of any supplementary groups this process
-		 * might have inheristed. */
-		if (setgroups(1, &gid)) {
+		/* Set the group(s) we want to be active. */
+		if (setgroups(gid_count, gid_list)) {
 			rsyserr(FLOG, errno, "setgroups failed");
 			io_printf(f_out, "@ERROR: setgroups failed\n");
 			return -1;
 		}
 #endif
+#if defined HAVE_INITGROUPS && !defined HAVE_GETGROUPLIST
+		/* pw is set if the user wants all the user's groups. */
+		if (pw && initgroups(pw->pw_name, pw->pw_gid) < 0) {
+			rsyserr(FLOG, errno, "initgroups failed");
+			io_printf(f_out, "@ERROR: initgroups failed\n");
+			return -1;
+		}
+#endif
+	}
 
+	if (set_uid) {
 		if (setuid(uid)) {
-			rsyserr(FLOG, errno, "setuid %d failed", (int)uid);
+			rsyserr(FLOG, errno, "setuid %ld failed", (long)uid);
 			io_printf(f_out, "@ERROR: setuid failed\n");
 			return -1;
 		}
diff --git a/configure.in b/configure.in
index d03bc4ed..bc7d4a7f 100644
--- a/configure.in
+++ b/configure.in
@@ -552,7 +552,8 @@ AC_CHECK_FUNCS(waitpid wait4 getcwd strdup chown chmod lchmod mknod mkfifo \
     strlcat strlcpy strtol mallinfo getgroups setgroups geteuid getegid \
     setlocale setmode open64 lseek64 mkstemp64 mtrace va_copy __va_copy \
     strerror putenv iconv_open locale_charset nl_langinfo getxattr \
-    extattr_get_link sigaction sigprocmask setattrlist)
+    extattr_get_link sigaction sigprocmask setattrlist getgrouplist \
+    initgroups)
 
 dnl cygwin iconv.h defines iconv_open as libiconv_open
 if test x"$ac_cv_func_iconv_open" != x"yes"; then
diff --git a/loadparm.c b/loadparm.c
index e8759ad4..8e48e6d9 100644
--- a/loadparm.c
+++ b/loadparm.c
@@ -185,7 +185,7 @@ static const all_vars Defaults = {
  /* exclude; */			NULL,
  /* exclude_from; */		NULL,
  /* filter; */			NULL,
- /* gid; */			NOBODY_GROUP,
+ /* gid; */			NULL,
  /* hosts_allow; */		NULL,
  /* hosts_deny; */		NULL,
  /* include; */			NULL,
@@ -202,7 +202,7 @@ static const all_vars Defaults = {
  /* refuse_options; */		NULL,
  /* secrets_file; */		NULL,
  /* temp_dir; */ 		NULL,
- /* uid; */			NOBODY_USER,
+ /* uid; */			NULL,
 
  /* max_connections; */		0,
  /* max_verbosity; */		1,
diff --git a/rsyncd.conf.yo b/rsyncd.conf.yo
index b6954baf..d4978cd9 100644
--- a/rsyncd.conf.yo
+++ b/rsyncd.conf.yo
@@ -316,13 +316,19 @@ The default is for modules to be listable.
 dit(bf(uid)) This parameter specifies the user name or user ID that
 file transfers to and from that module should take place as when the daemon
 was run as root. In combination with the "gid" parameter this determines what
-file permissions are available. The default is uid -2, which is normally
-the user "nobody".
-
-dit(bf(gid)) This parameter specifies the group name or group ID that
-file transfers to and from that module should take place as when the daemon
-was run as root. This complements the "uid" parameter. The default is gid -2,
-which is normally the group "nobody".
+file permissions are available. The default when run by a super-user is to
+switch to the system's "nobody" user.  The default for a non-super-user is to
+not try to change the user.  See also the "gid" parameter.
+
+dit(bf(gid)) This parameter specifies one or more group names/IDs that will be
+used when accessing the module.  The first one will be the default group, and
+any extra ones be set as supplemental groups.  You may also specify a "*" as
+the first gid in the list, which will be replaced by all the normal groups for
+the transfer's user (see "uid").  The default when run by a super-user is to
+switch to your OS's "nobody" (or perhaps "nogroup") group with no other
+supplementary groups.  The default for a non-super-user is to not change any
+group attributes (and indeed, your OS may not allow a non-super-user to try to
+change their group settings).
 
 dit(bf(fake super)) Setting "fake super = yes" for a module causes the
 daemon side to behave as if the bf(--fake-user) command-line option had
diff --git a/testsuite/rsync.fns b/testsuite/rsync.fns
index 2947a5f9..b26aee32 100644
--- a/testsuite/rsync.fns
+++ b/testsuite/rsync.fns
@@ -269,8 +269,8 @@ log format = %i %h [%a] %m (%u) %l %f%L
 transfer logging = yes
 exclude = ? foobar.baz
 max verbosity = 4
-uid = 0
-gid = 0
+#uid = 0
+#gid = 0
 
 [test-from]
 	path = $fromdir