LU-18633 gss: carry out creds negotiation as user

Instead of switching back to root id right after we did credentials
preparation, just do the whole credentials negotiation as the user.
We just need root privileges to proceed to ioctls that exchange data
with kernel space.

sanity-krb5 test_11 is added to exercise this.

Fixes: 6791fbc530 ("LU-18497 gss: carry out creds prepare as user")
Test-Parameters: trivial
Test-Parameters: testgroup=review-dne-selinux-ssk-part-1
Test-Parameters: testgroup=review-dne-selinux-ssk-part-2
Test-Parameters: kerberos=true testlist=sanity-krb5
Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
Change-Id: I93dd367abadca077e61c1910638337a2d80996c8
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/57752
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Aurelien Degremont <adegremont@nvidia.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/tests/sanity-krb5.sh b/lustre/tests/sanity-krb5.sh
index bd313d5..8489c24 100755 (executable)
--- a/lustre/tests/sanity-krb5.sh
+++ b/lustre/tests/sanity-krb5.sh
@@ -529,6 +529,61 @@ test_10() {
 }
 run_test 10 "Support revoked session keyring"
 
+exit_11() {
+       zconf_umount $HOSTNAME $MOUNT
+
+       zconf_mount $HOSTNAME $MOUNT
+       if [ "$MOUNT_2" ]; then
+               zconf_mount $HOSTNAME $MOUNT2
+       fi
+
+       restore_krb5_cred
+}
+
+test_11() {
+       local count
+
+       $LFS mkdir -i 0 -c $MDSCOUNT $DIR/$tdir ||
+               error "mkdir $DIR/$tdir failed"
+       chmod 0777 $DIR/$tdir || error "chmod $DIR/$tdir failed"
+       $RUNAS ls -ld $DIR/$tdir || error "ls -ld $DIR/$tdir failed"
+       $RUNAS grep lgssc /proc/keys
+       $RUNAS klist
+
+       # get rid of gss context and credentials for user
+       $RUNAS $LFS flushctx -k -r $MOUNT || error "can't flush context (1)"
+       $RUNAS grep lgssc /proc/keys
+       $RUNAS klist
+
+       stack_trap exit_11 EXIT
+       zconf_umount $HOSTNAME $MOUNT || error "umount $MOUNT failed"
+       if [ "$MOUNT_2" ]; then
+               zconf_umount $HOSTNAME $MOUNT2 ||
+                       error "umount $MOUNT2 failed"
+       fi
+       kdestroy
+       klist
+
+       # we want KCM ccache
+       cp /etc/krb5.conf /etc/krb5.conf.bkp
+       stack_trap "/bin/mv /etc/krb5.conf.bkp /etc/krb5.conf" EXIT
+       sed -i '1i default_ccache_name = KCM:' /etc/krb5.conf
+       sed -i '1i [libdefaults]' /etc/krb5.conf
+       zconf_mount $HOSTNAME $MOUNT || error "remount $MOUNT failed"
+       klist
+
+       $RUNAS touch $DIR/$tdir/$tfile && error "write $tfile should fail"
+       restore_krb5_cred
+       $RUNAS klist
+       $RUNAS touch $DIR/$tdir/$tfile || error "write $tfile failed"
+       $RUNAS klist
+       $RUNAS klist | grep -q lustre_mds || error "mds ticket not present"
+
+       $RUNAS $LFS flushctx -k -r $MOUNT || error "can't flush context (2)"
+       kdestroy
+}
+run_test 11 "KCM ccache"
+
 #
 # following tests will manipulate flavors and may end with any flavor set,
 # so each test should not assume any start flavor.

diff --git a/lustre/utils/gss/lgss_keyring.c b/lustre/utils/gss/lgss_keyring.c
index f661b42..278aaa7 100644 (file)
--- a/lustre/utils/gss/lgss_keyring.c
+++ b/lustre/utils/gss/lgss_keyring.c
@@ -167,6 +167,12 @@ static int gss_do_ioctl(struct lgssd_ioctl_param *param)
        glob_t path;
        int rc;
 
+       /* switch to root in order to proceed to ioctls */
+       if (param->uid && switch_identity(0)) {
+               rc = -EACCES;
+               goto out_params;
+       }
+
        rc = cfs_get_param_paths(&path, "sptlrpc/gss/init_channel");
        if (rc != 0)
                return rc;
@@ -191,6 +197,11 @@ static int gss_do_ioctl(struct lgssd_ioctl_param *param)
 
 out_params:
        cfs_free_param_data(&path);
+
+       /* switch back to user */
+       if (param->uid && switch_identity(param->uid))
+               rc = -EACCES;
+
        return rc;
 }
 
@@ -554,35 +565,6 @@ static void lgssc_fini_nego_data(struct lgss_nego_data *lnd)
         }
 }
 
-static int fork_and_switch_id(int uid, pid_t *child)
-{
-       int status, rc = 0;
-
-       *child = fork();
-       if (*child == -1) {
-               logmsg(LL_ERR, "cannot fork child for user %u: %s\n",
-                      uid, strerror(errno));
-               rc = errno;
-       } else if (*child == 0) {
-               /* switch identity */
-               rc = switch_identity(uid);
-               if (rc)
-                       rc = errno;
-       } else {
-               if (wait(&status) < 0) {
-                       rc = errno;
-                       logmsg(LL_ERR, "child %d failed: %s\n",
-                              *child, strerror(rc));
-               } else {
-                       rc = WEXITSTATUS(status);
-                       if (rc)
-                               logmsg(LL_ERR, "child %d terminated with %d\n",
-                                      *child, rc);
-               }
-       }
-       return rc;
-}
-
 static int do_keyctl_update(char *reason, key_serial_t keyid,
                            const void *payload, size_t plen)
 {
@@ -605,8 +587,7 @@ static int do_keyctl_update(char *reason, key_serial_t keyid,
 static int error_kernel_key(key_serial_t keyid, int rpc_error, int gss_error,
                            uid_t uid)
 {
-       key_serial_t inst_keyring = KEY_SPEC_SESSION_KEYRING;
-       pid_t child = 1;
+       key_serial_t inst_keyring;
        int seqwin = 0;
        char *p, *end;
        char buf[32];
@@ -614,16 +595,10 @@ static int error_kernel_key(key_serial_t keyid, int rpc_error, int gss_error,
 
        logmsg(LL_TRACE, "revoking kernel key %08x\n", keyid);
 
-       /* Only possessor and uid can update the key. So for a user key that is
-        * linked to the user keyring, switch uid/gid in a subprocess to not
-        * change identity in main process.
-        */
-       if (uid) {
-               rc = fork_and_switch_id(uid, &child);
-               if (rc || child)
-                       goto out;
+       if (uid)
                inst_keyring = KEY_SPEC_USER_KEYRING;
-       }
+       else
+               inst_keyring = KEY_SPEC_SESSION_KEYRING;
 
        p = buf;
        end = buf + sizeof(buf);
@@ -645,10 +620,6 @@ static int error_kernel_key(key_serial_t keyid, int rpc_error, int gss_error,
                       keyid, inst_keyring);
        }
 
-out:
-       if (child == 0)
-               /* job done for child */
-               exit(rc);
        return rc;
 }
 
@@ -658,22 +629,10 @@ static int update_kernel_key(key_serial_t keyid,
 {
        char *buf = NULL, *p = NULL, *end = NULL;
        unsigned int buf_size = 0;
-       pid_t child = 1;
-       int uid, rc = 0;
+       int rc = 0;
 
        logmsg(LL_TRACE, "updating kernel key %08x\n", keyid);
 
-       /* Only possessor and uid can update the key. So for a user key that is
-        * linked to the user keyring, switch uid/gid in a subprocess to not
-        * change identity in main process.
-        */
-       uid = lnd->lnd_uid;
-       if (uid) {
-               rc = fork_and_switch_id(uid, &child);
-               if (rc || child)
-                       goto out;
-       }
-
        buf_size = sizeof(lnd->lnd_seq_win) +
                sizeof(lnd->lnd_rmt_ctx.length) + lnd->lnd_rmt_ctx.length +
                sizeof(ctx_token->length) + ctx_token->length;
@@ -700,9 +659,6 @@ static int update_kernel_key(key_serial_t keyid,
 
 out:
        free(buf);
-       if (child == 0)
-               /* job done for child */
-               exit(rc);
        return rc;
 }
 
@@ -716,8 +672,8 @@ static int lgssc_kr_negotiate_krb(key_serial_t keyid, struct lgss_cred *cred,
        bool redo = true;
 
        if (lgss_get_service_str(&g_service, kup->kup_svc, kup->kup_nid)) {
-               logmsg(LL_ERR, "key %08x: failed to construct service "
-                      "string\n", keyid);
+               logmsg(LL_ERR, "key %08x: failed to construct service string\n",
+                      keyid);
                error_kernel_key(keyid, -EACCES, 0, cred->lc_uid);
                goto out_cred;
        }
@@ -731,8 +687,9 @@ static int lgssc_kr_negotiate_krb(key_serial_t keyid, struct lgss_cred *cred,
 retry_nego:
        memset(&lnd, 0, sizeof(lnd));
        if (lgssc_init_nego_data(&lnd, kup, cred->lc_mech->lmt_mech_n)) {
-               logmsg(LL_ERR, "key %08x: failed to initialize "
-                      "negotiation data\n", keyid);
+               logmsg(LL_ERR,
+                      "key %08x: failed to initialize negotiation data\n",
+                      keyid);
                error_kernel_key(keyid, lnd.lnd_rpc_err, lnd.lnd_gss_err,
                                 cred->lc_uid);
                goto out_cred;
@@ -787,8 +744,8 @@ static int lgssc_kr_negotiate_manual(key_serial_t keyid, struct lgss_cred *cred,
 
        rc = lgss_get_service_str(&g_service, kup->kup_svc, kup->kup_nid);
        if (rc) {
-               logmsg(LL_ERR, "key %08x: failed to construct service "
-                      "string\n", keyid);
+               logmsg(LL_ERR, "key %08x: failed to construct service string\n",
+                      keyid);
                error_kernel_key(keyid, -EACCES, 0, 0);
                goto out_cred;
        }
@@ -804,8 +761,9 @@ retry:
        memset(&lnd, 0, sizeof(lnd));
        rc = lgssc_init_nego_data(&lnd, kup, cred->lc_mech->lmt_mech_n);
        if (rc) {
-               logmsg(LL_ERR, "key %08x: failed to initialize "
-                      "negotiation data\n", keyid);
+               logmsg(LL_ERR,
+                      "key %08x: failed to initialize negotiation data\n",
+                      keyid);
                error_kernel_key(keyid, lnd.lnd_rpc_err, lnd.lnd_gss_err, 0);
                goto out_cred;
        }
@@ -1208,6 +1166,13 @@ int main(int argc, char *argv[])
                }
        }
 
+       if (!cred->lc_root_flags) {
+               /* switch to user id for creds handling */
+               rc = switch_identity(uparam.kup_uid);
+               if (rc)
+                       return rc;
+       }
+
        /*
         * if caller's namespace is different, fork a child and associate it
         * with caller's namespace to do credentials preparation
@@ -1365,30 +1330,12 @@ out_pipe:
                else
                        logmsg(LL_TRACE, "stick with current namespace\n");
 
-               if (!cred->lc_root_flags) {
-                       /* switch to user id for creds prepare */
-                       rc = switch_identity(uparam.kup_uid);
-                       if (rc)
-                               goto out_reg;
-               }
-
                /* In case of prepare error, a key will be instantiated
                 * all the same. But then we will have to error this key
                 * instead of doing normal gss negotiation.
                 */
                rc = prepare_and_instantiate(cred, keyid, uparam.kup_uid);
 
-               if (!cred->lc_root_flags) {
-                       int rc2;
-
-                       /* switch back to root in order to proceed to ioctls */
-                       rc2 = switch_identity(0);
-                       if (rc2) {
-                               rc = rc2;
-                               goto out_reg;
-                       }
-               }
-
                /*
                 * fork a child to do the real gss negotiation
                 */