From 6791fbc530ad64d5764882f4c866ee2a42a3f1fc Mon Sep 17 00:00:00 2001 From: Sebastien Buisson Date: Tue, 3 Dec 2024 12:10:26 +0100 Subject: [PATCH] LU-18497 gss: carry out creds prepare as user Instead of forking a child process that switches to user identity to get the credentials cache name, just carry out the whole credentials preparation as the user, and switch back to original uid/gid in order to proceed to ioctls, eliminating the need to map memory between processes. 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 Change-Id: I7807263d71cd3fc8a9934cc4c4da8b497b845e6f Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/57192 Reviewed-by: Aurelien Degremont Reviewed-by: Andreas Dilger Reviewed-by: Oleg Drokin Tested-by: jenkins Tested-by: Maloo --- lustre/utils/gss/lgss_keyring.c | 41 +++++++----- lustre/utils/gss/lgss_krb5_utils.c | 126 ++++++++++--------------------------- lustre/utils/gss/lgss_utils.c | 20 +++--- 3 files changed, 69 insertions(+), 118 deletions(-) diff --git a/lustre/utils/gss/lgss_keyring.c b/lustre/utils/gss/lgss_keyring.c index c9bd249..f661b42 100644 --- a/lustre/utils/gss/lgss_keyring.c +++ b/lustre/utils/gss/lgss_keyring.c @@ -993,8 +993,7 @@ static int prepare_and_instantiate(struct lgss_cred *cred, key_serial_t keyid, { key_serial_t inst_keyring; bool prepared = true; - pid_t child = 1; - int rc = 0; + int rc; if (lgss_prepare_cred(cred)) { logmsg(LL_ERR, "key %08x: failed to prepare credentials " @@ -1018,19 +1017,13 @@ static int prepare_and_instantiate(struct lgss_cred *cred, key_serial_t keyid, if (cred->lc_root_flags) { inst_keyring = 0; } else { - inst_keyring = KEY_SPEC_USER_KEYRING; - /* fork to not change identity in main process */ - rc = fork_and_switch_id(uid, &child); - if (rc || child) - goto out; - } - - /* if dealing with a user key, grant user write permission, - * it will be required for key update - */ - if (child == 0) { key_perm_t perm; + inst_keyring = KEY_SPEC_USER_KEYRING; + + /* when dealing with a user key, grant user write permission, + * it will be required for key update + */ perm = KEY_POS_VIEW | KEY_POS_WRITE | KEY_POS_SEARCH | KEY_POS_LINK | KEY_POS_SETATTR | KEY_USR_VIEW | KEY_USR_WRITE; @@ -1050,10 +1043,6 @@ static int prepare_and_instantiate(struct lgss_cred *cred, key_serial_t keyid, keyid, inst_keyring); } -out: - if (child == 0) - /* job done for child */ - exit(rc); return prepared ? rc : 1; } @@ -1376,12 +1365,30 @@ 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 */ diff --git a/lustre/utils/gss/lgss_krb5_utils.c b/lustre/utils/gss/lgss_krb5_utils.c index 5af8e6f..0a974a9 100644 --- a/lustre/utils/gss/lgss_krb5_utils.c +++ b/lustre/utils/gss/lgss_krb5_utils.c @@ -487,83 +487,6 @@ end_find: } /** - * Compose the TGT cc name for user, needs to fork process and switch identity. - * For that reason, ccname buffer passed in must be mmapped with MAP_SHARED. - */ -static int get_user_tgt_ccname(struct lgss_cred *cred, krb5_context ctx, - char *ccname, int size) -{ - pid_t child; - int status, rc = 0; - - /* fork to not change identity in main process, it needs to stay root - * in order to proceed to ioctls - */ - child = fork(); - if (child == -1) { - logmsg(LL_ERR, "cannot fork child for user %u: %s\n", - cred->lc_uid, strerror(errno)); - rc = -errno; - } else if (child == 0) { - /* switch identity */ - rc = switch_identity(cred->lc_uid); - if (rc) - exit(1); - - /* getting default ccname requires impersonating user */ - rc = lgss_krb5_get_default_ccache_name(ctx, ccname, size); - if (rc) { - logmsg(LL_ERR, - "cannot get default ccname for user %u\n", - cred->lc_uid); - exit(1); - } - - /* job done for child */ - exit(0); - } else { - logmsg(LL_TRACE, "forked child %d\n", child); - if (wait(&status) < 0) { - logmsg(LL_ERR, "wait child %d failed: %s\n", - child, strerror(errno)); - return -errno; - } - if (!WIFEXITED(status)) { - logmsg(LL_ERR, "child %d terminated with %d\n", - child, status); - return status; - } - } - - /* try ccname as fetched by child */ - rc = acquire_user_cred_and_check(ccname); - if (!rc) - /* user's creds found in default ccache */ - goto end_ccache; - - /* fallback: look at every file matching - * - /tmp/ *krb5cc* - * - /run/user// *krb5cc* - */ - rc = find_existing_krb5_ccache(cred->lc_uid, LGSS_DEFAULT_CRED_DIR, - ccname, size); - if (!rc) - /* user's creds found in LGSS_DEFAULT_CRED_DIR */ - goto end_ccache; - - rc = find_existing_krb5_ccache(cred->lc_uid, LGSS_USER_CRED_DIR, - ccname, size); - if (!rc) - /* user's creds found in LGSS_USER_CRED_DIR */ - goto end_ccache; - - rc = -ENODATA; - -end_ccache: - return rc; -} - -/** * find out whether current TGT cache is valid or not */ static int lkrb5_check_root_tgt_cc(krb5_context ctx, unsigned int flag, @@ -886,7 +809,7 @@ static int lkrb5_prepare_user_cred(struct lgss_cred *cred) { krb5_context ctx; krb5_error_code code; - int size = PATH_MAX; + int size = PATH_MAX + 1; void *ccname; int rc; @@ -899,30 +822,49 @@ static int lkrb5_prepare_user_cred(struct lgss_cred *cred) return -1; } - /* buffer passed to get_user_tgt_ccname() must be mmapped with - * MAP_SHARED because it is accessed read/write from a child process - */ - ccname = mmap(NULL, size, PROT_READ | PROT_WRITE, - MAP_SHARED | MAP_ANONYMOUS, -1, 0); - if (ccname == MAP_FAILED) { - logmsg(LL_ERR, "cannot mmap memory for user %u: %s\n", - cred->lc_uid, strerror(errno)); + ccname = calloc(1, size); + if (!ccname) { rc = -errno; goto free; } - rc = get_user_tgt_ccname(cred, ctx, ccname, size); + /* getting default ccname requires impersonating user */ + rc = lgss_krb5_get_default_ccache_name(ctx, ccname, size); + if (rc) + goto end_ccache; + + /* try ccname as returned by gss */ + rc = acquire_user_cred_and_check(ccname); + if (!rc) + /* user's creds found in default ccache */ + goto end_ccache; + + /* fallback: look at every file matching + * - /tmp/ *krb5cc* + * - /run/user// *krb5cc* + */ + rc = find_existing_krb5_ccache(cred->lc_uid, LGSS_DEFAULT_CRED_DIR, + ccname, size); + if (!rc) + /* user's creds found in LGSS_DEFAULT_CRED_DIR */ + goto end_ccache; + + rc = find_existing_krb5_ccache(cred->lc_uid, LGSS_USER_CRED_DIR, + ccname, size); + if (!rc) + /* user's creds found in LGSS_USER_CRED_DIR */ + goto end_ccache; + + rc = -ENODATA; + +end_ccache: if (rc) logmsg(LL_ERR, "cannot get user %u ccname: %s\n", cred->lc_uid, strerror(-rc)); else logmsg(LL_INFO, "using krb5 cache name: %s\n", (char *)ccname); - if (munmap(ccname, size) == -1) { - logmsg(LL_ERR, "cannot munmap memory for user %u: %s\n", - cred->lc_uid, strerror(errno)); - rc = rc ? rc : -errno; - } + free(ccname); free: krb5_free_context(ctx); diff --git a/lustre/utils/gss/lgss_utils.c b/lustre/utils/gss/lgss_utils.c index b3439b6..5e02536 100644 --- a/lustre/utils/gss/lgss_utils.c +++ b/lustre/utils/gss/lgss_utils.c @@ -543,13 +543,15 @@ int switch_identity(uid_t uid) struct passwd *pw; int rc; - /* drop list of supp groups */ - rc = setgroups(0, NULL); - if (rc == -1) { - logmsg(LL_ERR, "cannot drop list of supp groups: %s\n", - strerror(errno)); - rc = -errno; - goto end_switch; + /* drop list of supp groups for regular user */ + if (uid) { + rc = setgroups(0, NULL); + if (rc == -1) { + logmsg(LL_ERR, "cannot drop list of supp groups: %s\n", + strerror(errno)); + rc = -errno; + goto end_switch; + } } pw = getpwuid(uid); @@ -560,7 +562,7 @@ int switch_identity(uid_t uid) goto end_switch; } - rc = setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid); + rc = setresgid(pw->pw_gid, pw->pw_gid, 0); if (rc == -1) { logmsg(LL_ERR, "cannot set real gid to %u: %s\n", pw->pw_gid, strerror(errno)); @@ -568,7 +570,7 @@ int switch_identity(uid_t uid) goto end_switch; } - rc = setresuid(uid, uid, uid); + rc = setresuid(uid, uid, 0); if (rc == -1) { logmsg(LL_ERR, "cannot set real uid to %u: %s\n", uid, strerror(errno)); -- 1.8.3.1