]> granicus.if.org Git - sudo/commitdiff
Do group setup in policy_init_session() before calling out to the
authorTodd C. Miller <Todd.Miller@courtesan.com>
Thu, 28 Apr 2016 17:01:58 +0000 (11:01 -0600)
committerTodd C. Miller <Todd.Miller@courtesan.com>
Thu, 28 Apr 2016 17:01:58 +0000 (11:01 -0600)
plugin.  This makes it possible for the pam_group module to change
the group in pam_setcred().  It's a bit bogus since pam_setcred()
is documented as not changing the group or user ID, but pam_group
is shipped with stock Linux-PAM so we need to support it.

src/sudo.c

index 46e5ce44f8c3000747b3dcc28dd95e4d8ac941d8..dedafd5698d7993a8292878c8f529fbf0b145b0f 100644 (file)
@@ -939,7 +939,8 @@ restore_nproc(void)
 }
 
 /*
- * Setup the execution environment immediately prior to the call to execve()
+ * Setup the execution environment immediately prior to the call to execve().
+ * Group setup is performed by policy_init_session(), called earlier.
  * Returns true on success and false on failure.
  */
 bool
@@ -1018,30 +1019,6 @@ exec_setup(struct command_details *details, const char *ptyname, int ptyfd)
 #endif /* HAVE_LOGIN_CAP_H */
     }
 
-    /*
-     * Set groups, including supplementary group vector.
-     */
-    if (!ISSET(details->flags, CD_PRESERVE_GROUPS)) {
-       if (details->ngroups >= 0) {
-           if (sudo_setgroups(details->ngroups, details->groups) < 0) {
-               sudo_warn(U_("unable to set supplementary group IDs"));
-               goto done;
-           }
-       }
-    }
-#ifdef HAVE_SETEUID
-    if (ISSET(details->flags, CD_SET_EGID) && setegid(details->egid)) {
-       sudo_warn(U_("unable to set effective gid to runas gid %u"),
-           (unsigned int)details->egid);
-       goto done;
-    }
-#endif
-    if (ISSET(details->flags, CD_SET_GID) && setgid(details->gid)) {
-       sudo_warn(U_("unable to set gid to runas gid %u"),
-           (unsigned int)details->gid);
-       goto done;
-    }
-
     if (ISSET(details->flags, CD_SET_PRIORITY)) {
        if (setpriority(PRIO_PROCESS, 0, details->priority) != 0) {
            sudo_warn(U_("unable to set process priority"));
@@ -1365,6 +1342,35 @@ policy_init_session(struct command_details *details)
     int rval = true;
     debug_decl(policy_init_session, SUDO_DEBUG_PCOMM)
 
+    /*
+     * We set groups, including supplementary group vector,
+     * as part of the session setup.  This allows for dynamic
+     * groups to be set via pam_group(8) in pam_setcred(3).
+     */
+    if (!ISSET(details->flags, CD_PRESERVE_GROUPS)) {
+       if (details->ngroups >= 0) {
+           if (sudo_setgroups(details->ngroups, details->groups) < 0) {
+               sudo_warn(U_("unable to set supplementary group IDs"));
+               rval = -1;
+               goto done;
+           }
+       }
+    }
+#ifdef HAVE_SETEUID
+    if (ISSET(details->flags, CD_SET_EGID) && setegid(details->egid)) {
+       sudo_warn(U_("unable to set effective gid to runas gid %u"),
+           (unsigned int)details->egid);
+       rval = -1;
+       goto done;
+    }
+#endif
+    if (ISSET(details->flags, CD_SET_GID) && setgid(details->gid)) {
+       sudo_warn(U_("unable to set gid to runas gid %u"),
+           (unsigned int)details->gid);
+       rval = -1;
+       goto done;
+    }
+
     if (policy_plugin.u.policy->init_session) {
        /*
         * Backwards compatibility for older API versions
@@ -1381,6 +1387,7 @@ policy_init_session(struct command_details *details)
        }
        sudo_debug_set_active_instance(sudo_debug_instance);
     }
+done:
     debug_return_int(rval);
 }