UTIL: refactor switch_creds()

to simplify logic, make it more reliable doesn't
matter what SSSD service user is used and rely
only on permitted caps.

Author: alexey-tikhonov

src/util/become_user.c

@@ -33,7 +33,6 @@ errno_t become_user(uid_t uid, gid_t gid, bool keep_set_uid)
     DEBUG(SSSDBG_FUNC_DATA,
           "Trying to become user [%"SPRIuid"][%"SPRIgid"].\n", uid, gid);
 
-    /* skip call if we already are the requested user */
     cuid = geteuid();
     if (uid == cuid) {
         DEBUG(SSSDBG_FUNC_DATA, "Already user [%"SPRIuid"].\n", uid);
@@ -49,7 +48,6 @@ errno_t become_user(uid_t uid, gid_t gid, bool keep_set_uid)
         goto done;
     }
 
-    /* change GID so that root cannot be regained (changes saved GID too) */
     ret = setresgid(gid, gid, gid);
     if (ret == -1) {
         ret = errno;
@@ -58,8 +56,6 @@ errno_t become_user(uid_t uid, gid_t gid, bool keep_set_uid)
         goto done;
     }
 
-    /* change UID so that root cannot be regained (changes saved UID too) */
-    /* this call also takes care of dropping CAP_SETUID, so this is a PNR */
     ret = setresuid(uid, uid, (keep_set_uid ? -1 : uid));
     if (ret == -1) {
         ret = errno;
@@ -69,6 +65,9 @@ errno_t become_user(uid_t uid, gid_t gid, bool keep_set_uid)
     }
 
 done:
+    /* clears permitted set as well, so even if suid allows to set
+     * euid back to 0 this wouldn't regain any capabilities
+     */
     sss_drop_all_caps();
 
     return ret;
@@ -134,55 +133,49 @@ errno_t switch_creds(TALLOC_CTX *mem_ctx,
         ssc->gid = mygid;
     }
 
-    /* if we are regaining root, set EUID first so that we have CAP_SETUID back,
-     * and the other calls work too, otherwise call it last so that we can
-     * change groups before we loose CAP_SETUID */
-    if (uid == 0) {
-        ret = setresuid(0, 0, 0);
-        if (ret == -1) {
-            ret = errno;
-            DEBUG(SSSDBG_CRIT_FAILURE,
-                  "setresuid failed [%d][%s].\n", ret, strerror(ret));
-            goto done;
-        }
-    }
-
-    /* TODO: use libcap-ng if we need to get/set capabilities too? */
-
-
-    /* try to setgroups first should always work if CAP_SETUID is set,
-     * otherwise it will always fail, failure is not critical though as
-     * generally we only really care about UID and at most primary GID */
+    /* 'man capabilities':
+     * - If the effective user ID is changed from 0 to nonzero,
+     *   then all capabilities are cleared from the effective set.
+     *
+     * - If the effective user ID is changed from nonzero to 0,
+     *   then the permitted set is copied to the effective set.
+     *
+     * The above means that behavior depends on if SSSD service user
+     * is 'root' or 'sssd'.
+     * Manage effective capabilities explicitly to avoid euid change
+     * effects.
+     */
+
+    sss_set_cap_effective(CAP_SETGID, true); /* if this fails then next op fails anyway */
     ret = setgroups(num_gids, gids);
     if (ret == -1) {
         ret = errno;
-        DEBUG(SSSDBG_TRACE_FUNC,
+        DEBUG(SSSDBG_OP_FAILURE,
               "setgroups failed [%d][%s].\n", ret, strerror(ret));
+        goto done;
     }
-
-    /* change GID now, (leaves saved GID to current, so we can restore) */
     ret = setresgid(-1, gid, -1);
     if (ret == -1) {
         ret = errno;
-        DEBUG(SSSDBG_CRIT_FAILURE,
+        DEBUG(SSSDBG_OP_FAILURE,
               "setresgid failed [%d][%s].\n", ret, strerror(ret));
         goto done;
     }
 
-    if (uid != 0) {
-        /* change UID, (leaves saved UID to current, so we can restore) */
-        ret = setresuid(-1, uid, -1);
-        if (ret == -1) {
-            ret = errno;
-            DEBUG(SSSDBG_CRIT_FAILURE,
-                  "setresuid failed [%d][%s].\n", ret, strerror(ret));
-            goto done;
-        }
+    sss_set_cap_effective(CAP_SETUID, true);
+    ret = setresuid(-1, uid, -1);
+    if (ret == -1) {
+        ret = errno;
+        DEBUG(SSSDBG_OP_FAILURE,
+              "setresuid failed [%d][%s].\n", ret, strerror(ret));
+        goto done;
     }
 
     ret = 0;
 
 done:
+    sss_set_cap_effective(CAP_SETGID, false);
+    sss_set_cap_effective(CAP_SETUID, false); /* might be no-op but don't bother */
     if (ret) {
         /* attempt to restore creds first */
         restore_creds(ssc);