dovecot/sme7/dovecot-1.0-CVE-2008-1199.patch

diff --git a/dovecot-example.conf b/dovecot-example.conf
index 4dccda8..55c9be0 100644
--- a/dovecot-example.conf
+++ b/dovecot-example.conf
@@ -252,9 +252,17 @@
    #hidden = yes
 #}
 
-# Grant access to these extra groups for mail processes. Typical use would be
-# to give "mail" group write access to /var/mail to be able to create dotlocks.
-#mail_extra_groups =
+# Group to enable temporarily for privileged operations. Currently this is
+# used only for creating mbox dotlock files when creation fails for INBOX.
+# Typically this is set to "mail" to give access to /var/mail.
+#mail_privileged_group =
+
+# Grant access to these supplementary groups for mail processes. Typically
+# these are used to set up access to shared mailboxes. Note that it may be
+# dangerous to set these if users can create symlinks (e.g. if "mail" group is
+# set here, ln -s /var/mail ~/mail/var could allow a user to delete others'
+# mailboxes, or ln -s /secret/shared/box ~/mail/mybox would allow reading it).
+#mail_access_groups =
 
 # Allow full filesystem access to clients. There's no access checks other than
 # what the operating system does for the active UID/GID. It works with both
diff --git a/src/lib-storage/index/mbox/mbox-lock.c b/src/lib-storage/index/mbox/mbox-lock.c
index 1950b38..9e66f5f 100644
--- a/src/lib-storage/index/mbox/mbox-lock.c
+++ b/src/lib-storage/index/mbox/mbox-lock.c
@@ -1,6 +1,7 @@
 /* Copyright (C) 2002 Timo Sirainen */
 
 #include "lib.h"
+#include "restrict-access.h"
 #include "mail-index-private.h"
 #include "mbox-storage.h"
 #include "mbox-file.h"
@@ -36,6 +37,12 @@ enum mbox_lock_type {
        MBOX_LOCK_COUNT
 };
 
+enum mbox_dotlock_op {
+       MBOX_DOTLOCK_OP_LOCK,
+       MBOX_DOTLOCK_OP_UNLOCK,
+       MBOX_DOTLOCK_OP_TOUCH
+};
+
 struct mbox_lock_context {
        struct mbox_mailbox *mbox;
        int lock_status[MBOX_LOCK_COUNT];
@@ -43,6 +50,7 @@ struct mbox_lock_context {
 
        int lock_type;
        bool dotlock_last_stale;
+       bool using_privileges;
 };
 
 struct mbox_lock_data {
@@ -190,6 +198,9 @@ static bool dotlock_callback(unsigned int secs_left, bool stale, void *context)
        enum mbox_lock_type *lock_types;
        int i;
 
+       if (ctx->using_privileges)
+               restrict_access_drop_priv_gid();
+
        if (stale && !ctx->dotlock_last_stale) {
                /* get next index we wish to try locking. it's the one after
                   dotlocking. */
@@ -221,9 +232,92 @@ static bool dotlock_callback(unsigned int secs_left, bool stale, void *context)
                                  MAILBOX_LOCK_NOTIFY_MAILBOX_OVERRIDE :
                                  MAILBOX_LOCK_NOTIFY_MAILBOX_ABORT,
                                  secs_left);
+       if (ctx->using_privileges) {
+               if (restrict_access_use_priv_gid() < 0) {
+                       /* shouldn't get here */
+                       return FALSE;
+               }
+       }
        return TRUE;
 }
 
+static int mbox_dotlock_privileged_op(struct mbox_mailbox *mbox,
+                                     struct dotlock_settings *set,
+                                     enum mbox_dotlock_op op)
+{
+       const char *dir, *fname;
+       int ret = -1, orig_dir_fd;
+
+       orig_dir_fd = open(".", O_RDONLY);
+       if (orig_dir_fd == -1) {
+               i_error("open(.) failed: %m");
+               return -1;
+       }
+
+       /* allow dotlocks to be created only for files we can read while we're
+          unprivileged. to make sure there are no race conditions we first
+          have to chdir to the mbox file's directory and then use relative
+          paths. unless this is done, users could:
+           - create *.lock files to any directory writable by the
+             privileged group
+           - DoS other users by dotlocking their mailboxes infinitely
+       */
+       fname = strrchr(mbox->path, '/');
+       if (fname == NULL) {
+               /* already relative */
+               fname = mbox->path;
+       } else {
+               dir = t_strdup_until(mbox->path, fname);
+               if (chdir(dir) < 0) {
+                       i_error("chdir(%s) failed: %m", dir);
+                       (void)close(orig_dir_fd);
+                       return -1;
+               }
+               fname++;
+       }
+       if (op == MBOX_DOTLOCK_OP_LOCK) {
+               if (access(fname, R_OK) < 0) {
+                       i_error("access(%s) failed: %m", mbox->path);
+                       return -1;
+               }
+       }
+
+       if (restrict_access_use_priv_gid() < 0) {
+               (void)close(orig_dir_fd);
+               return -1;
+       }
+
+       switch (op) {
+       case MBOX_DOTLOCK_OP_LOCK:
+               /* we're now privileged - avoid doing as much as possible */
+               ret = file_dotlock_create(set, fname, 0, &mbox->mbox_dotlock);
+               if (ret > 0)
+                       mbox->mbox_used_privileges = TRUE;
+               break;
+       case MBOX_DOTLOCK_OP_UNLOCK:
+               /* we're now privileged - avoid doing as much as possible */
+               ret = file_dotlock_delete(&mbox->mbox_dotlock);
+               mbox->mbox_used_privileges = FALSE;
+               break;
+       case MBOX_DOTLOCK_OP_TOUCH:
+               if (!file_dotlock_is_locked(mbox->mbox_dotlock)) {
+                       file_dotlock_delete(&mbox->mbox_dotlock);
+                       mbox->mbox_used_privileges = TRUE;
+                       ret = -1;
+               } else {
+                       ret = file_dotlock_touch(mbox->mbox_dotlock);
+               }
+               break;
+       }
+
+       restrict_access_drop_priv_gid();
+
+       if (fchdir(orig_dir_fd) < 0)
+               i_error("fchdir() failed: %m");
+       (void)close(orig_dir_fd);
+       return ret;
+}
+
 static int mbox_lock_dotlock(struct mbox_lock_context *ctx, int lock_type,
                             time_t max_wait_time __attr_unused__)
 {
@@ -235,7 +329,15 @@ static int mbox_lock_dotlock(struct mbox_lock_context *ctx, int lock_type,
                if (!mbox->mbox_dotlocked)
                        return 1;
 
-               if (file_dotlock_delete(&mbox->mbox_dotlock) <= 0) {
+               if (!mbox->mbox_used_privileges)
+                       ret = file_dotlock_delete(&mbox->mbox_dotlock);
+               else {
+                       ctx->using_privileges = TRUE;
+                       ret = mbox_dotlock_privileged_op(mbox, NULL,
+                                                       MBOX_DOTLOCK_OP_UNLOCK);
+                       ctx->using_privileges = FALSE;
+               }
+               if (ret <= 0) {
                        mbox_set_syscall_error(mbox, "file_dotlock_delete()");
                        ret = -1;
                }
@@ -257,6 +359,13 @@ static int mbox_lock_dotlock(struct mbox_lock_context *ctx, int lock_type,
        set.context = ctx;
 
        ret = file_dotlock_create(&set, mbox->path, 0, &mbox->mbox_dotlock);
+       if (ret < 0 && errno == EACCES && restrict_access_have_priv_gid() &&
+           mbox->mbox_privileged_locking) {
+               /* try again, this time with extra privileges */
+               ret = mbox_dotlock_privileged_op(mbox, &set,
+                                                MBOX_DOTLOCK_OP_LOCK);
+       }
+
        if (ret < 0) {
                mbox_set_syscall_error(mbox, "file_lock_dotlock()");
                return -1;
@@ -601,3 +710,16 @@ int mbox_unlock(struct mbox_mailbox *mbox, unsigned int lock_id)
 
        return mbox_unlock_files(&ctx);
 }
+
+void mbox_dotlock_touch(struct mbox_mailbox *mbox)
+{
+       if (mbox->mbox_dotlock == NULL)
+               return;
+
+       if (!mbox->mbox_used_privileges)
+               (void)file_dotlock_touch(mbox->mbox_dotlock);
+       else {
+               (void)mbox_dotlock_privileged_op(mbox, NULL,
+                                                MBOX_DOTLOCK_OP_TOUCH);
+       }
+}
diff --git a/src/lib-storage/index/mbox/mbox-lock.h b/src/lib-storage/index/mbox/mbox-lock.h
index 19c51dd..baa1843 100644
--- a/src/lib-storage/index/mbox/mbox-lock.h
+++ b/src/lib-storage/index/mbox/mbox-lock.h
@@ -7,4 +7,6 @@ int mbox_lock(struct mbox_mailbox *mbox, int lock_type,
              unsigned int *lock_id_r);
 int mbox_unlock(struct mbox_mailbox *mbox, unsigned int lock_id);
 
+void mbox_dotlock_touch(struct mbox_mailbox *mbox);
+
 #endif
diff --git a/src/lib-storage/index/mbox/mbox-storage.c b/src/lib-storage/index/mbox/mbox-storage.c
index 330d0c1..cdfd2e0 100644
--- a/src/lib-storage/index/mbox/mbox-storage.c
+++ b/src/lib-storage/index/mbox/mbox-storage.c
@@ -443,6 +443,13 @@ bool mbox_is_valid_mask(struct mail_storage *storage, const char *mask)
        return TRUE;
 }
 
+static bool mbox_name_is_dotlock(const char *name)
+{
+       unsigned int len = strlen(name);
+
+       return len >= 5 && strcmp(name + len - 5, ".lock") == 0;
+}
+
 static bool mbox_is_valid_create_name(struct mail_storage *storage,
                                      const char *name)
 {
@@ -458,7 +465,7 @@ static bool mbox_is_valid_create_name(struct mail_storage *storage,
                    return FALSE;
        }
 
-       return mbox_is_valid_mask(storage, name);
+       return mbox_is_valid_mask(storage, name) && !mbox_name_is_dotlock(name);
 }
 
 static bool mbox_is_valid_existing_name(struct mail_storage *storage,
@@ -470,7 +477,7 @@ static bool mbox_is_valid_existing_name(struct mail_storage *storage,
        if (name[0] == '\0' || name[len-1] == '/')
                return FALSE;
 
-       return mbox_is_valid_mask(storage, name);
+       return mbox_is_valid_mask(storage, name) && !mbox_name_is_dotlock(name);
 }
 
 static const char *mbox_get_index_dir(struct index_storage *storage,
@@ -597,7 +604,7 @@ static void mbox_lock_touch_timeout(void *context)
 {
        struct mbox_mailbox *mbox = context;
 
-       (void)file_dotlock_touch(mbox->mbox_dotlock);
+       mbox_dotlock_touch(mbox);
 }
 
 static struct mbox_mailbox *
@@ -697,6 +704,12 @@ mbox_open(struct mbox_storage *storage, const char *name,
                }
        }
 
+       if (strcmp(name, "INBOX") == 0) {
+               /* if INBOX isn't under the root directory, it's probably in
+                  /var/mail and we want to allow privileged dotlocking */
+               if (strncmp(path, istorage->dir, strlen(istorage->dir)) != 0)
+                       mbox->mbox_privileged_locking = TRUE;
+       }
        return &mbox->ibox.box;
 }
 
diff --git a/src/lib-storage/index/mbox/mbox-storage.h b/src/lib-storage/index/mbox/mbox-storage.h
index fe1dc4c..127df6a 100644
--- a/src/lib-storage/index/mbox/mbox-storage.h
+++ b/src/lib-storage/index/mbox/mbox-storage.h
@@ -48,6 +48,8 @@ struct mbox_mailbox {
        unsigned int mbox_very_dirty_syncs:1;
        unsigned int mbox_save_md5:1;
        unsigned int mbox_dotlocked:1;
+       unsigned int mbox_used_privileges:1;
+       unsigned int mbox_privileged_locking:1;
 };
 
 struct mbox_transaction_context {
diff --git a/src/lib/file-dotlock.c b/src/lib/file-dotlock.c
index 1ae16ab..26e762d 100644
--- a/src/lib/file-dotlock.c
+++ b/src/lib/file-dotlock.c
@@ -262,7 +262,8 @@ static int create_temp_file(string_t *path, bool write_pid)
                        break;
 
                if (errno != EEXIST) {
-                       i_error("open(%s) failed: %m", str_c(path));
+                       if (errno != EACCES)
+                               i_error("open(%s) failed: %m", str_c(path));
                        return -1;
                }
        }
@@ -319,8 +320,10 @@ static int try_create_lock_hardlink(struct lock_info *lock_info, bool write_pid,
                if (errno == EEXIST)
                        return 0;
 
-               i_error("link(%s, %s) failed: %m",
-                       lock_info->temp_path, lock_info->lock_path);
+               if (errno != EACCES) {
+                       i_error("link(%s, %s) failed: %m",
+                               lock_info->temp_path, lock_info->lock_path);
+               }
                return -1;
        }
 
@@ -342,7 +345,8 @@ static int try_create_lock_excl(struct lock_info *lock_info, bool write_pid)
                if (errno == EEXIST)
                        return 0;
 
-               i_error("open(%s) failed: %m", lock_info->lock_path);
+               if (errno != EACCES)
+                       i_error("open(%s) failed: %m", lock_info->lock_path);
                return -1;
        }
 
@@ -633,7 +637,6 @@ int file_dotlock_replace(struct dotlock **dotlock_p,
                         enum dotlock_replace_flags flags)
 {
        struct dotlock *dotlock;
-       struct stat st, st2;
        const char *lock_path;
        int fd;
 
@@ -645,28 +648,14 @@ int file_dotlock_replace(struct dotlock **dotlock_p,
                dotlock->fd = -1;
 
        lock_path = file_dotlock_get_lock_path(dotlock);
-       if ((flags & DOTLOCK_REPLACE_FLAG_VERIFY_OWNER) != 0) {
-               if (fstat(fd, &st) < 0) {
-                       i_error("fstat(%s) failed: %m", lock_path);
-                       file_dotlock_free(dotlock);
-                       return -1;
-               }
-
-               if (lstat(lock_path, &st2) < 0) {
-                       i_error("lstat(%s) failed: %m", lock_path);
-                       file_dotlock_free(dotlock);
-                       return -1;
-               }
-
-               if (st.st_ino != st2.st_ino ||
-                   !CMP_DEV_T(st.st_dev, st2.st_dev)) {
-                       i_warning("Our dotlock file %s was overridden "
-                                 "(kept it %d secs)", lock_path,
-                                 (int)(time(NULL) - dotlock->lock_time));
-                       errno = EEXIST;
-                       file_dotlock_free(dotlock);
-                       return 0;
-               }
+       if ((flags & DOTLOCK_REPLACE_FLAG_VERIFY_OWNER) != 0 &&
+           !file_dotlock_is_locked(dotlock)) {
+               i_warning("Our dotlock file %s was overridden "
+                         "(kept it %d secs)", lock_path,
+                         (int)(time(NULL) - dotlock->lock_time));
+               errno = EEXIST;
+               file_dotlock_free(dotlock);
+               return 0;
        }
 
        if (rename(lock_path, dotlock->path) < 0) {
@@ -701,6 +690,24 @@ int file_dotlock_touch(struct dotlock *dotlock)
        return ret;
 }
 
+bool file_dotlock_is_locked(struct dotlock *dotlock)
+{
+       struct stat st, st2;
+       const char *lock_path;
+
+       lock_path = file_dotlock_get_lock_path(dotlock);
+       if (fstat(dotlock->fd, &st) < 0) {
+               i_error("fstat(%s) failed: %m", lock_path);
+               return FALSE;
+       }
+
+       if (lstat(lock_path, &st2) < 0) {
+               i_error("lstat(%s) failed: %m", lock_path);
+               return FALSE;
+       }
+       return st.st_ino == st2.st_ino && CMP_DEV_T(st.st_dev, st2.st_dev);
+}
+
 const char *file_dotlock_get_lock_path(struct dotlock *dotlock)
 {
        if (dotlock->lock_path == NULL) {
diff --git a/src/lib/file-dotlock.h b/src/lib/file-dotlock.h
index f29a5ac..7e88c45 100644
--- a/src/lib/file-dotlock.h
+++ b/src/lib/file-dotlock.h
@@ -70,6 +70,8 @@ int file_dotlock_replace(struct dotlock **dotlock,
    it's a good idea to update it once in a while so others won't override it.
    If the timestamp is less than a second old, it's not updated. */
 int file_dotlock_touch(struct dotlock *dotlock);
+/* Returns TRUE if the lock is still ok, FALSE if it's been overridden. */
+bool file_dotlock_is_locked(struct dotlock *dotlock);
 
 /* Returns the lock file path. */
 const char *file_dotlock_get_lock_path(struct dotlock *dotlock);
diff --git a/src/lib/restrict-access.c b/src/lib/restrict-access.c
index 5d6692d..f668f97 100644
--- a/src/lib/restrict-access.c
+++ b/src/lib/restrict-access.c
@@ -1,15 +1,22 @@
-/* Copyright (c) 2002-2004 Timo Sirainen */
+/* Copyright (c) 2002-2008 Dovecot authors, see the included COPYING file */
+
+#define _GNU_SOURCE /* setresgid() */
+#include <sys/types.h>
+#include <unistd.h>
 
 #include "lib.h"
 #include "restrict-access.h"
 #include "env-util.h"
 
 #include <stdlib.h>
-#include <unistd.h>
 #include <time.h>
 #include <grp.h>
 
-void restrict_access_set_env(const char *user, uid_t uid, gid_t gid,
+static gid_t primary_gid = (gid_t)-1, privileged_gid = (gid_t)-1;
+static bool using_priv_gid = FALSE;
+
+void restrict_access_set_env(const char *user, uid_t uid,
+                            gid_t gid, gid_t privileged_gid,
                             const char *chroot_dir,
                             gid_t first_valid_gid, gid_t last_valid_gid,
                             const char *extra_groups)
@@ -21,6 +28,10 @@ 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 (privileged_gid != (gid_t)-1) {
+               env_put(t_strdup_printf("RESTRICT_SETGID_PRIV=%s",
+                                       dec2str(privileged_gid)));
+       }
        if (extra_groups != NULL && *extra_groups != '\0') {
                env_put(t_strconcat("RESTRICT_SETEXTRAGROUPS=",
                                    extra_groups, NULL));
@@ -36,7 +47,54 @@ void restrict_access_set_env(const char *user, uid_t uid, gid_t gid,
        }
 }
 
-static gid_t *get_groups_list(int *gid_count_r)
+static void restrict_init_groups(gid_t primary_gid, gid_t privileged_gid)
+{
+       if (privileged_gid == (gid_t)-1) {
+               if (primary_gid == getgid() && primary_gid == getegid()) {
+                       /* everything is already set */
+                       return;
+               }
+
+               if (setgid(primary_gid) != 0) {
+                       i_fatal("setgid(%s) failed with euid=%s, "
+                               "gid=%s, egid=%s: %m",
+                               dec2str(primary_gid), dec2str(geteuid()),
+                               dec2str(getgid()), dec2str(getegid()));
+               }
+               return;
+       }
+
+       if (getegid() != 0 && primary_gid == getgid() &&
+           primary_gid == getegid()) {
+               /* privileged_gid is hopefully in saved ID. if not,
+                  there's nothing we can do about it. */
+               return;
+       }
+
+#ifdef HAVE_SETRESGID
+       if (setresgid(primary_gid, primary_gid, privileged_gid) != 0) {
+               i_fatal("setresgid(%s,%s,%s) failed with euid=%s: %m",
+                       dec2str(primary_gid), dec2str(primary_gid),
+                       dec2str(privileged_gid), dec2str(geteuid()));
+       }
+#else
+       /* real: primary_gid
+          effective: privileged_gid
+          saved: privileged_gid */
+       if (setregid(primary_gid, privileged_gid) != 0) {
+               i_fatal("setregid(%s,%s) failed with euid=%s: %m",
+                       dec2str(primary_gid), dec2str(privileged_gid),
+                       dec2str(geteuid()));
+       }
+       /* effective: privileged_gid -> primary_gid */
+       if (setegid(privileged_gid) != 0) {
+               i_fatal("setegid(%s) failed with euid=%s: %m",
+                       dec2str(privileged_gid), dec2str(geteuid()));
+       }
+#endif
+}
+
+static gid_t *get_groups_list(unsigned int *gid_count_r)
 {
        gid_t *gid_list;
        int ret, gid_count;
@@ -53,39 +111,28 @@ static gid_t *get_groups_list(int *gid_count_r)
        return gid_list;
 }
 
-static void drop_restricted_groups(bool *have_root_group)
+static void drop_restricted_groups(gid_t *gid_list, unsigned int *gid_count,
+                                  bool *have_root_group)
 {
        /* @UNSAFE */
+       gid_t first_valid, last_valid;
        const char *env;
-       gid_t *gid_list, first_valid_gid, last_valid_gid;
-       int i, used, gid_count;
+       unsigned int i, used;
 
        env = getenv("RESTRICT_GID_FIRST");
-       first_valid_gid = env == NULL ? 0 : (gid_t)strtoul(env, NULL, 10);
+       first_valid = env == NULL ? 0 : (gid_t)strtoul(env, NULL, 10);
        env = getenv("RESTRICT_GID_LAST");
-       last_valid_gid = env == NULL ? 0 : (gid_t)strtoul(env, NULL, 10);
-
-       if (first_valid_gid == 0 && last_valid_gid == 0)
-               return;
+       last_valid = env == NULL ? (gid_t)-1 : (gid_t)strtoul(env, NULL, 10);
 
-       t_push();
-       gid_list = get_groups_list(&gid_count);
-
-       for (i = 0, used = 0; i < gid_count; i++) {
-               if (gid_list[i] >= first_valid_gid &&
-                   (last_valid_gid == 0 || gid_list[i] <= last_valid_gid)) {
+       for (i = 0, used = 0; i < *gid_count; i++) {
+               if (gid_list[i] >= first_valid &&
+                   (last_valid == (gid_t)-1 || gid_list[i] <= last_valid)) {
                        if (gid_list[i] == 0)
                                *have_root_group = TRUE;
                        gid_list[used++] = gid_list[i];
                }
        }
-
-       if (used != gid_count) {
-               /* it did contain restricted groups, remove it */
-               if (setgroups(used, gid_list) < 0)
-                       i_fatal("setgroups() failed: %m");
-       }
-       t_pop();
+       *gid_count = used;
 }
 
 static gid_t get_group_id(const char *name)
@@ -101,68 +148,115 @@ static gid_t get_group_id(const char *name)
        return group->gr_gid;
 }
 
-static void grant_extra_groups(const char *groups)
+static void fix_groups_list(const char *extra_groups,
+                           bool preserve_existing, bool *have_root_group)
 {
-       const char *const *tmp;
-       gid_t *gid_list;
-       int gid_count;
-
-       t_push();
-       tmp = t_strsplit(groups, ", ");
-       gid_list = get_groups_list(&gid_count);
-       for (; *tmp != NULL; tmp++) {
-               if (**tmp == '\0')
-                       continue;
-
-               if (!t_try_realloc(gid_list, (gid_count+1) * sizeof(gid_t)))
-                       i_unreached();
-               gid_list[gid_count++] = get_group_id(*tmp);
+       gid_t gid, *gid_list, *gid_list2;
+       const char *const *tmp, *empty = NULL;
+       unsigned int i, gid_count;
+       bool add_primary_gid;
+
+       /* if we're using a privileged GID, we can temporarily drop our
+          effective GID. we still want to be able to use its privileges,
+          so add it to supplementary groups. */
+       add_primary_gid = privileged_gid != (gid_t)-1;
+
+       tmp = extra_groups == NULL ? &empty :
+               t_strsplit_spaces(extra_groups, ", ");
+
+       if (preserve_existing) {
+               gid_list = get_groups_list(&gid_count);
+               drop_restricted_groups(gid_list, &gid_count,
+                                      have_root_group);
+               /* see if the list already contains the primary GID */
+               for (i = 0; i < gid_count; i++) {
+                       if (gid_list[i] == primary_gid) {
+                               add_primary_gid = FALSE;
+                               break;
+                       }
+               }
+       } else {
+               gid_list = NULL;
+               gid_count = 0;
+       }
+       if (gid_count == 0) {
+               /* Some OSes don't like an empty groups list,
+                  so use the primary GID as the only one. */
+               gid_list = t_new(gid_t, 2);
+               gid_list[0] = primary_gid;
+               gid_count = 1;
+               add_primary_gid = FALSE;
        }
 
-       if (setgroups(gid_count, gid_list) < 0)
-               i_fatal("setgroups() failed: %m");
+       if (*tmp != NULL || add_primary_gid) {
+               /* @UNSAFE: add extra groups and/or primary GID to gids list */
+               gid_list2 = t_new(gid_t, gid_count + strarray_length(tmp) + 1);
+               memcpy(gid_list2, gid_list, gid_count * sizeof(gid_t));
+               for (; *tmp != NULL; tmp++) {
+                       gid = get_group_id(*tmp);
+                       if (gid != primary_gid)
+                               gid_list2[gid_count++] = gid;
+               }
+               if (add_primary_gid)
+                       gid_list2[gid_count++] = primary_gid;
+               gid_list = gid_list2;
+       }
 
-       t_pop();
+       if (setgroups(gid_count, gid_list) < 0) {
+               if (errno == EINVAL) {
+                       i_fatal("setgroups(%s) failed: Too many extra groups",
+                               extra_groups == NULL ? "" : extra_groups);
+               } else {
+                       i_fatal("setgroups() failed: %m");
+               }
+       }
 }
 
 void restrict_access_by_env(bool disallow_root)
 {
        const char *env;
-       gid_t gid;
        uid_t uid;
-       bool have_root_group;
+       bool is_root, have_root_group, preserve_groups = FALSE;
+       bool allow_root_gid;
+
+       is_root = geteuid() == 0;
 
-       /* 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. Do this
-          before chrooting so initgroups() actually works. */
+       /* set the primary/privileged group */
        env = getenv("RESTRICT_SETGID");
-       gid = env == NULL ? 0 : (gid_t)strtoul(env, NULL, 10);
-       have_root_group = gid == 0;
-       if (gid != 0 && (gid != getgid() || gid != getegid())) {
-               if (setgid(gid) != 0)
-                       i_fatal("setgid(%s) failed: %m", dec2str(gid));
-
-               env = getenv("RESTRICT_USER");
-               if (env == NULL) {
-                       /* user not known, use only this one group */
-                       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));
-                       }
+       primary_gid = env == NULL || *env == '\0' ? (gid_t)-1 :
+               (gid_t)strtoul(env, NULL, 10);
+       env = getenv("RESTRICT_SETGID_PRIV");
+       privileged_gid = env == NULL || *env == '\0' ? (gid_t)-1 :
+               (gid_t)strtoul(env, NULL, 10);
+
+       have_root_group = primary_gid == 0;
+       if (primary_gid != (gid_t)-1 || privileged_gid != (gid_t)-1) {
+               if (primary_gid == (gid_t)-1)
+                       primary_gid = getegid();
+               restrict_init_groups(primary_gid, privileged_gid);
+       } else {
+               if (primary_gid == (gid_t)-1)
+                       primary_gid = getegid();
+       }
 
-                        drop_restricted_groups(&have_root_group);
+       /* set system user's groups */
+       env = getenv("RESTRICT_USER");
+       if (env != NULL && *env != '\0' && is_root) {
+               if (initgroups(env, primary_gid) < 0) {
+                       i_fatal("initgroups(%s, %s) failed: %m",
+                               env, dec2str(primary_gid));
                }
+               preserve_groups = TRUE;
        }
 
-       /* grant additional groups to process */
+       /* add extra groups. if we set system user's groups, drop the
+          restricted groups at the same time. */
        env = getenv("RESTRICT_SETEXTRAGROUPS");
-       if (env != NULL && *env != '\0')
-               grant_extra_groups(env);
+       if (is_root) {
+               t_push();
+               fix_groups_list(env, preserve_groups, &have_root_group);
+               t_pop();
+       }
 
        /* chrooting */
        env = getenv("RESTRICT_CHROOT");
@@ -190,7 +284,7 @@ void restrict_access_by_env(bool disallow_root)
 
        /* uid last */
        env = getenv("RESTRICT_SETUID");
-       uid = env == NULL ? 0 : (uid_t)strtoul(env, NULL, 10);
+       uid = env == NULL || *env == '\0' ? 0 : (uid_t)strtoul(env, NULL, 10);
        if (uid != 0) {
                if (setuid(uid) != 0)
                        i_fatal("setuid(%s) failed: %m", dec2str(uid));
@@ -206,12 +300,20 @@ void restrict_access_by_env(bool disallow_root)
        }
 
        env = getenv("RESTRICT_GID_FIRST");
-       if ((!have_root_group || (env != NULL && atoi(env) != 0)) && uid != 0) {
+       if (env != NULL && atoi(env) != 0)
+               allow_root_gid = FALSE;
+       else if (primary_gid == 0 || privileged_gid == 0)
+               allow_root_gid = TRUE;
+       else
+               allow_root_gid = FALSE;
+
+       if (!allow_root_gid && uid != 0) {
                if (getgid() == 0 || getegid() == 0 || setgid(0) == 0) {
-                       if (gid == 0)
+                       if (primary_gid == 0)
                                i_fatal("GID 0 isn't permitted");
                        i_fatal("We couldn't drop root group privileges "
-                               "(wanted=%s, gid=%s, egid=%s)", dec2str(gid),
+                               "(wanted=%s, gid=%s, egid=%s)",
+                               dec2str(primary_gid),
                                dec2str(getgid()), dec2str(getegid()));
                }
        }
@@ -220,8 +322,43 @@ void restrict_access_by_env(bool disallow_root)
        env_put("RESTRICT_USER=");
        env_put("RESTRICT_CHROOT=");
        env_put("RESTRICT_SETUID=");
-       env_put("RESTRICT_SETGID=");
+       if (privileged_gid == (gid_t)-1) {
+               /* if we're dropping privileges before executing and
+                  a privileged group is set, the groups must be fixed
+                  after exec */
+               env_put("RESTRICT_SETGID=");
+               env_put("RESTRICT_SETGID_PRIV=");
+       }
        env_put("RESTRICT_SETEXTRAGROUPS=");
        env_put("RESTRICT_GID_FIRST=");
        env_put("RESTRICT_GID_LAST=");
 }
+
+int restrict_access_use_priv_gid(void)
+{
+       i_assert(!using_priv_gid);
+
+       if (privileged_gid == (gid_t)-1)
+               return 0;
+       if (setegid(privileged_gid) < 0) {
+               i_error("setegid(privileged) failed: %m");
+               return -1;
+       }
+       using_priv_gid = TRUE;
+       return 0;
+}
+
+void restrict_access_drop_priv_gid(void)
+{
+       if (!using_priv_gid)
+               return;
+
+       if (setegid(primary_gid) < 0)
+               i_fatal("setegid(primary) failed: %m");
+       using_priv_gid = FALSE;
+}
+
+bool restrict_access_have_priv_gid(void)
+{
+       return privileged_gid != (gid_t)-1;
+}
diff --git a/src/lib/restrict-access.h b/src/lib/restrict-access.h
index 69103a8..a72ff90 100644
--- a/src/lib/restrict-access.h
+++ b/src/lib/restrict-access.h
@@ -2,8 +2,10 @@
 #define __RESTRICT_ACCESS_H
 
 /* 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,
+   restrict_access_by_env(). If privileged_gid != (gid_t)-1,
+   the privileged GID can be temporarily enabled/disabled. */
+void restrict_access_set_env(const char *user, uid_t uid,
+                            gid_t gid, gid_t privileged_gid,
                             const char *chroot_dir,
                             gid_t first_valid_gid, gid_t last_valid_gid,
                             const char *extra_groups);
@@ -13,4 +15,11 @@ void restrict_access_set_env(const char *user, uid_t uid, gid_t gid,
    environment settings and we have root uid or gid. */
 void restrict_access_by_env(bool disallow_root);
 
+/* If privileged_gid was set, these functions can be used to temporarily
+   gain access to the group. */
+int restrict_access_use_priv_gid(void);
+void restrict_access_drop_priv_gid(void);
+/* Returns TRUE if privileged GID exists for this process. */
+bool restrict_access_have_priv_gid(void);
+
 #endif
diff --git a/src/master/auth-process.c b/src/master/auth-process.c
index 56f8499..4d00dfc 100644
--- a/src/master/auth-process.c
+++ b/src/master/auth-process.c
@@ -413,8 +413,8 @@ static void auth_set_environment(struct auth_settings *set)
        int i;
 
        /* setup access environment */
-       restrict_access_set_env(set->user, set->uid, set->gid, set->chroot,
-                               0, 0, NULL);
+       restrict_access_set_env(set->user, set->uid, set->gid,
+                               (gid_t)-1, set->chroot, 0, 0, NULL);
 
        /* set other environment */
        env_put("DOVECOT_MASTER=1");
diff --git a/src/master/login-process.c b/src/master/login-process.c
index 7e4f5ea..b616d0a 100644
--- a/src/master/login-process.c
+++ b/src/master/login-process.c
@@ -519,7 +519,7 @@ static void login_process_init_env(struct login_group *group, pid_t pid)
           parameter since we don't want to call initgroups() for login
           processes. */
        restrict_access_set_env(NULL, set->login_uid,
-                               set->server->login_gid,
+                               set->server->login_gid, (gid_t)-1,
                                set->login_chroot ? set->login_dir : NULL,
                                0, 0, NULL);
 
diff --git a/src/master/mail-process.c b/src/master/mail-process.c
index 38c48e8..5948b5b 100644
--- a/src/master/mail-process.c
+++ b/src/master/mail-process.c
@@ -589,9 +589,10 @@ bool create_mail_process(enum process_type process_type, struct settings *set,
 
        /* setup environment - set the most important environment first
           (paranoia about filling up environment without noticing) */
-       restrict_access_set_env(system_user, uid, gid, chroot_dir,
+       restrict_access_set_env(system_user, uid, gid, set->mail_priv_gid_t,
+                               chroot_dir,
                                set->first_valid_gid, set->last_valid_gid,
-                               set->mail_extra_groups);
+                               set->mail_access_groups);
 
        restrict_process_size(set->mail_process_size, (unsigned int)-1);
 
@@ -699,8 +700,13 @@ bool create_mail_process(enum process_type process_type, struct settings *set,
           any errors above will be logged */
        closelog();
 
-       if (set->mail_drop_priv_before_exec)
+       if (set->mail_drop_priv_before_exec) {
                restrict_access_by_env(TRUE);
+               /* privileged GID is now only in saved-GID. if we want to
+                  preserve it accross exec, it needs to be temporarily
+                  in effective gid */
+               restrict_access_use_priv_gid();
+       }
 
        client_process_exec(set->mail_executable, title);
        i_fatal_status(FATAL_EXEC, "execv(%s) failed: %m",
diff --git a/src/master/master-settings-defs.c b/src/master/master-settings-defs.c
index 5f051fb..56d0c12 100644
--- a/src/master/master-settings-defs.c
+++ b/src/master/master-settings-defs.c
@@ -58,6 +58,8 @@ static struct setting_def setting_defs[] = {
        DEF(SET_INT, first_valid_gid),
        DEF(SET_INT, last_valid_gid),
        DEF(SET_STR, mail_extra_groups),
+       DEF(SET_STR, mail_access_groups),
+       DEF(SET_STR, mail_privileged_group),
 
        DEF(SET_STR, default_mail_env),
        DEF(SET_STR, mail_location),
diff --git a/src/master/master-settings.c b/src/master/master-settings.c
index 08b2b70..c3bff8c 100644
--- a/src/master/master-settings.c
+++ b/src/master/master-settings.c
@@ -21,6 +21,7 @@
 #include <sys/stat.h>
 #include <sys/wait.h>
 #include <pwd.h>
+#include <grp.h>
 
 enum settings_type {
        SETTINGS_TYPE_ROOT,
@@ -207,6 +208,8 @@ struct settings default_settings = {
        MEMBER(first_valid_gid) 1,
        MEMBER(last_valid_gid) 0,
        MEMBER(mail_extra_groups) "",
+       MEMBER(mail_access_groups) "",
+       MEMBER(mail_privileged_group) "",
 
        MEMBER(default_mail_env) "",
        MEMBER(mail_location) "",
@@ -365,6 +368,25 @@ static bool get_login_uid(struct settings *set)
        return TRUE;
 }
 
+static bool parse_gid(const char *str, gid_t *gid_r)
+{
+       struct group *gr;
+       char *p;
+
+       if (*str >= '0' && *str <= '9') {
+               *gid_r = (gid_t)strtoul(str, &p, 10);
+               if (*p == '\0')
+                       return TRUE;
+       }
+
+       gr = getgrnam(str);
+       if (gr == NULL)
+               return FALSE;
+
+       *gid_r = gr->gr_gid;
+       return TRUE;
+}
+
 static bool auth_settings_verify(struct auth_settings *auth)
 {
        struct passwd *pw;
@@ -628,9 +650,35 @@ static bool settings_verify(struct settings *set)
        const char *dir;
        int facility;
 
+       set->mail_priv_gid_t = (gid_t)-1;
+
        if (!get_login_uid(set))
                return FALSE;
 
+       if (*set->mail_privileged_group != '\0') {
+               if (!parse_gid(set->mail_privileged_group,
+                              &set->mail_priv_gid_t)) {
+                       i_error("Non-existing mail_privileged_group: %s",
+                               set->mail_privileged_group);
+                       return FALSE;
+               }
+       }
+       if (*set->mail_extra_groups != '\0') {
+               if (*set->mail_access_groups != '\0') {
+                       i_error("Can't set both mail_extra_groups "
+                               "and mail_access_groups");
+                       return FALSE;
+               }
+               if (!set->server->warned_mail_extra_groups) {
+                       set->server->warned_mail_extra_groups = TRUE;
+                       i_warning("mail_extra_groups setting was often used "
+                                 "insecurely so it is now deprecated, "
+                                 "use mail_access_groups or "
+                                 "mail_privileged_group instead");
+               }
+               set->mail_access_groups = set->mail_extra_groups;
+       }
+
        if (set->protocol == MAIL_PROTOCOL_POP3 &&
            *set->pop3_uidl_format == '\0') {
                i_error("POP3 enabled but pop3_uidl_format not set");
diff --git a/src/master/master-settings.h b/src/master/master-settings.h
index a539035..e7be6e1 100644
--- a/src/master/master-settings.h
+++ b/src/master/master-settings.h
@@ -66,6 +66,8 @@ struct settings {
        unsigned int first_valid_uid, last_valid_uid;
        unsigned int first_valid_gid, last_valid_gid;
        const char *mail_extra_groups;
+       const char *mail_access_groups;
+       const char *mail_privileged_group;
 
        const char *default_mail_env;
        const char *mail_location;
@@ -125,6 +127,7 @@ struct settings {
        int listen_fd, ssl_listen_fd;
 
        uid_t login_uid;
+       gid_t mail_priv_gid_t;
 
        struct ip_addr listen_ip, ssl_listen_ip;
        unsigned int listen_port, ssl_listen_port;
@@ -235,6 +238,7 @@ struct server_settings {
        array_t ARRAY_DEFINE(dicts, const char *);
 
        gid_t login_gid;
+       unsigned int warned_mail_extra_groups:1;
 };
 
 extern struct server_settings *settings_root;