Allow first_valid_gid to be 0. Drop any supplementary groups not in valid

gid range.

--HG--
branch : HEAD

diff --git a/dovecot-example.conf b/dovecot-example.conf
index 7a83379d92215819fff33a47bed5c7901e29dd3f..6d9256d667b49603d3269d602e6e109d19884072 100644 (file)
--- a/dovecot-example.conf
+++ b/dovecot-example.conf
@@ -144,13 +144,17 @@ login = pop3
 # Show protocol level SSL errors.
 #verbose_ssl = no
 
-# Valid UID/GID ranges for users, defaults to 500 and above. This is mostly
+# Valid UID range for users, defaults to 500 and above. This is mostly
 # to make sure that users can't log in as daemons or other system users.
-# Note that denying root logins is hardcoded to dovecot-master binary and
-# can't be done even if first_valid_uid is set to 0.
+# Note that denying root logins is hardcoded to dovecot binary and can't
+# be done even if first_valid_uid is set to 0.
 #first_valid_uid = 500
 #last_valid_uid = 0
 
+# Valid GID range for users, defaults to non-root/wheel. Users having
+# non-valid GID as primary group ID aren't allowed to log in. If user
+# belongs to supplementary groups with non-valid GIDs, those groups are
+# not set.
 #first_valid_gid = 1
 #last_valid_gid = 0
 

diff --git a/src/lib/restrict-access.c b/src/lib/restrict-access.c
index 27e209976f1a983de3a8d935ab905464e2aea801..200941842e1df7b6d43fe9278827125ebe039341 100644 (file)
--- a/src/lib/restrict-access.c
+++ b/src/lib/restrict-access.c
@@ -30,8 +30,15 @@
 #include <time.h>
 #include <grp.h>
 
+#define HARD_MAX_GROUPS 10240
+
+#ifndef NGROUPS_MAX
+#  define NGROUPS_MAX 128
+#endif
+
 void restrict_access_set_env(const char *user, uid_t uid, gid_t gid,
-                            const char *chroot_dir)
+                            const char *chroot_dir,
+                            gid_t first_valid_gid, gid_t last_valid_gid)
 {
        if (user != NULL && *user != '\0')
                env_put(t_strconcat("RESTRICT_USER=", user, NULL));
@@ -40,33 +47,68 @@ void restrict_access_set_env(const char *user, uid_t uid, gid_t gid,
 
        env_put(t_strdup_printf("RESTRICT_SETUID=%s", dec2str(uid)));
        env_put(t_strdup_printf("RESTRICT_SETGID=%s", dec2str(gid)));
+
+       if (first_valid_gid != 0) {
+               env_put(t_strdup_printf("RESTRICT_GID_FIRST=%s",
+                                       dec2str(first_valid_gid)));
+       }
+       if (last_valid_gid != 0) {
+               env_put(t_strdup_printf("RESTRICT_GID_LAST=%s",
+                                       dec2str(last_valid_gid)));
+       }
 }
 
-void restrict_access_by_env(int disallow_root)
+static void drop_restricted_groups(void)
 {
+       /* @UNSAFE */
        const char *env;
-       gid_t gid;
-       uid_t uid;
-
-       /* chrooting */
-       env = getenv("RESTRICT_CHROOT");
-       if (env != NULL) {
-               /* kludge: localtime() must be called before chroot(),
-                  or the timezone isn't known */
-               time_t t = 0;
-               (void)localtime(&t);
+       gid_t *gid_list, first_valid_gid, last_valid_gid;
+       int ret, i, gid_count;
+
+       env = getenv("RESTRICT_GID_FIRST");
+       first_valid_gid = env == NULL ? 0 : (gid_t)atol(env);
+       env = getenv("RESTRICT_GID_LAST");
+       last_valid_gid = env == NULL ? 0 : (gid_t)atol(env);
+
+       if (first_valid_gid == 0 && last_valid_gid == 0)
+               return;
+
+       gid_count = NGROUPS_MAX;
+       gid_list = t_buffer_get(sizeof(gid_t) * gid_count);
+       while ((ret = getgroups(gid_count, gid_list)) < 0) {
+               if (errno != EINVAL ||
+                   gid_count < HARD_MAX_GROUPS)
+                       i_fatal("getgroups() failed: %m");
+
+               gid_count *= 2;
+               gid_list = t_buffer_reget(gid_list, sizeof(gid_t) * gid_count);
+       }
 
-               if (chroot(env) != 0)
-                       i_fatal("chroot(%s) failed: %m", env);
+       gid_count = 0;
+       for (i = 0; i < ret; i++) {
+               if (gid_list[i] >= first_valid_gid &&
+                   (last_valid_gid == 0 || gid_list[i] <= last_valid_gid))
+                       gid_list[gid_count++] = gid_list[i];
+       }
 
-               if (chdir("/") != 0)
-                       i_fatal("chdir(/) failed: %m");
+       if (ret != gid_count) {
+               /* it did contain 0, remove it */
+               if (setgroups(gid_count, gid_list) < 0)
+                       i_fatal("setgroups() failed: %m");
        }
+}
+
+void restrict_access_by_env(int disallow_root)
+{
+       const char *env;
+       gid_t gid;
+       uid_t uid;
 
        /* groups - the getgid() checks are just so we don't fail if we're
-          not running as root and try to just use our own GID. */
+          not running as root and try to just use our own GID. Do this
+          before chrooting so initgroups() actually works. */
        env = getenv("RESTRICT_SETGID");
-       gid = env == NULL ? 0 : (gid_t) atol(env);
+       gid = env == NULL ? 0 : (gid_t)atol(env);
        if (gid != 0 && (gid != getgid() || gid != getegid())) {
                if (setgid(gid) != 0)
                        i_fatal("setgid(%s) failed: %m", dec2str(gid));
@@ -74,18 +116,38 @@ void restrict_access_by_env(int disallow_root)
                env = getenv("RESTRICT_USER");
                if (env == NULL) {
                        /* user not known, use only this one group */
-                       (void)setgroups(1, &gid);
+                       if (setgroups(1, &gid) < 0) {
+                               i_fatal("setgroups(%s) failed: %m",
+                                       dec2str(gid));
+                       }
                } else {
                        if (initgroups(env, gid) != 0) {
                                i_fatal("initgroups(%s, %s) failed: %m",
                                        env, dec2str(gid));
                        }
+
+                        drop_restricted_groups();
                }
        }
 
+       /* chrooting */
+       env = getenv("RESTRICT_CHROOT");
+       if (env != NULL) {
+               /* kludge: localtime() must be called before chroot(),
+                  or the timezone isn't known */
+               time_t t = 0;
+               (void)localtime(&t);
+
+               if (chroot(env) != 0)
+                       i_fatal("chroot(%s) failed: %m", env);
+
+               if (chdir("/") != 0)
+                       i_fatal("chdir(/) failed: %m");
+       }
+
        /* uid last */
        env = getenv("RESTRICT_SETUID");
-       uid = env == NULL ? 0 : (uid_t) atol(env);
+       uid = env == NULL ? 0 : (uid_t)atol(env);
        if (uid != 0) {
                if (setuid(uid) != 0)
                        i_fatal("setuid(%s) failed: %m", dec2str(uid));

diff --git a/src/lib/restrict-access.h b/src/lib/restrict-access.h
index 78f8039790fa7615c59c8a0d4ba0e78f026a6207..b3ea5d7b770e97c3cd58f395d01069259ddf75a6 100644 (file)
--- a/src/lib/restrict-access.h
+++ b/src/lib/restrict-access.h
@@ -4,7 +4,8 @@
 /* set environment variables so they can be read with
    restrict_access_by_env() */
 void restrict_access_set_env(const char *user, uid_t uid, gid_t gid,
-                            const char *chroot_dir);
+                            const char *chroot_dir,
+                            gid_t first_valid_gid, gid_t last_valid_gid);
 
 /* chroot, setuid() and setgid() based on environment variables.
    If disallow_roots is TRUE, we'll kill ourself if we didn't have the

diff --git a/src/master/auth-process.c b/src/master/auth-process.c
index fabacc51d01192cb1b4e13110bbcdc89cb6aa2c5..94bb82b67d7bf46c5f9a64271f08441a463c7732 100644 (file)
--- a/src/master/auth-process.c
+++ b/src/master/auth-process.c
@@ -309,7 +309,7 @@ static pid_t create_auth_process(struct auth_process_group *group)
 
        /* setup access environment */
        restrict_access_set_env(group->set->user, pwd->pw_uid, pwd->pw_gid,
-                               group->set->chroot);
+                               group->set->chroot, 0, 0);
 
        /* set other environment */
        env_put(t_strconcat("AUTH_PROCESS=", dec2str(getpid()), NULL));

diff --git a/src/master/login-process.c b/src/master/login-process.c
index 17502d3efe240b5874206c14e94e824e9f2c8eac..d28a556c4c08618955b6022e8dddcf2b1cbb63ea 100644 (file)
--- a/src/master/login-process.c
+++ b/src/master/login-process.c
@@ -390,7 +390,8 @@ static void login_process_init_env(struct login_group *group, pid_t pid)
           clean_child_process() since it clears environment */
        restrict_access_set_env(group->set->user,
                                group->set->uid, set->login_gid,
-                               set->login_chroot ? set->login_dir : NULL);
+                               set->login_chroot ? set->login_dir : NULL,
+                               0, 0);
 
        env_put("DOVECOT_MASTER=1");
 

diff --git a/src/master/mail-process.c b/src/master/mail-process.c
index b9d91353be0d5d0c07320307f3ba7675fbaf1bfd..39a4043fef7af580189b09a76db20ee4c2623511 100644 (file)
--- a/src/master/mail-process.c
+++ b/src/master/mail-process.c
@@ -25,11 +25,6 @@ static int validate_uid_gid(uid_t uid, gid_t gid)
                return FALSE;
        }
 
-       if (uid != 0 && gid == 0) {
-               i_error("mail process isn't allowed to be in group 0");
-               return FALSE;
-       }
-
        if (uid < (uid_t)set->first_valid_uid ||
            (set->last_valid_uid != 0 && uid > (uid_t)set->last_valid_uid)) {
                i_error("mail process isn't allowed to use UID %s "
@@ -40,8 +35,9 @@ static int validate_uid_gid(uid_t uid, gid_t gid)
 
        if (gid < (gid_t)set->first_valid_gid ||
            (set->last_valid_gid != 0 && gid > (gid_t)set->last_valid_gid)) {
-               i_error("mail process isn't allowed to use "
-                       "GID %s (UID is %s)", dec2str(gid), dec2str(uid));
+               i_error("mail process isn't allowed to use primary group ID %s "
+                       "with UID %s (see first_valid_gid in config file).",
+                       dec2str(gid), dec2str(uid));
                return FALSE;
        }
 
@@ -154,7 +150,8 @@ int create_mail_process(int socket, struct ip_addr *ip,
        /* setup environment - set the most important environment first
           (paranoia about filling up environment without noticing) */
        restrict_access_set_env(data + reply->system_user_idx,
-                               reply->uid, reply->gid, chroot_dir);
+                               reply->uid, reply->gid, chroot_dir,
+                               set->first_valid_gid, set->last_valid_gid);
 
        restrict_process_size(process_size, (unsigned int)-1);