From 458ab6652130ebea1f305b5d383e263a2fbbcedc Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Sun, 19 Oct 2014 12:28:13 +0200 Subject: [PATCH 88/92] KRB5: Do not switch_creds() if already the specified user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The code didn't have to handle this case previously as sssd_be was always running as root and switching to the ccache as the user logging in. Also handle NULL creds on restore_creds() in case there was no switch. One less if-condition and fewer indentation levels. Related: https://fedorahosted.org/sssd/ticket/2370 Reviewed-by: Sumit Bose Reviewed-by: Lukáš Slebodník --- src/tests/cwrap/test_become_user.c | 7 +++++++ src/util/become_user.c | 29 +++++++++++++++++++++-------- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/tests/cwrap/test_become_user.c b/src/tests/cwrap/test_become_user.c index 06d3ad425c4928e9e9bff661fbb8f7b4536b8896..7ecea5aac34bb73ca81d94ad481f05b338e65ed0 100644 --- a/src/tests/cwrap/test_become_user.c +++ b/src/tests/cwrap/test_become_user.c @@ -76,6 +76,7 @@ void test_switch_user(void **state) struct passwd *sssd; TALLOC_CTX *tmp_ctx; struct sss_creds *saved_creds; + struct sss_creds *saved_creds2 = NULL; check_leaks_push(global_talloc_context); tmp_ctx = talloc_new(global_talloc_context); @@ -102,6 +103,12 @@ void test_switch_user(void **state) assert_int_equal(saved_creds->uid, 0); assert_int_equal(saved_creds->gid, 0); + /* Attempt to restore creds again */ + ret = switch_creds(tmp_ctx, sssd->pw_uid, sssd->pw_gid, + 0, NULL, &saved_creds2); + assert_int_equal(ret, EOK); + assert_null(saved_creds2); + /* restore root */ ret = restore_creds(saved_creds); assert_int_equal(ret, EOK); diff --git a/src/util/become_user.c b/src/util/become_user.c index b5f94f993cd2c23bd3340fc502d36a530aa729fa..7dd2c752b1d0f289e7d82feee6d93e5974823f95 100644 --- a/src/util/become_user.c +++ b/src/util/become_user.c @@ -90,9 +90,14 @@ errno_t switch_creds(TALLOC_CTX *mem_ctx, struct sss_creds *ssc = NULL; int size; int ret; + uid_t myuid; + uid_t mygid; DEBUG(SSSDBG_FUNC_DATA, "Switch user to [%d][%d].\n", uid, gid); + myuid = geteuid(); + mygid = getegid(); + if (saved_creds) { /* save current user credentials */ size = getgroups(0, NULL); @@ -124,8 +129,8 @@ errno_t switch_creds(TALLOC_CTX *mem_ctx, } /* we care only about effective ids */ - ssc->uid = geteuid(); - ssc->gid = getegid(); + ssc->uid = myuid; + ssc->gid = mygid; } /* if we are regaining root set euid first so that we have CAP_SETUID back, @@ -141,7 +146,12 @@ errno_t switch_creds(TALLOC_CTX *mem_ctx, } } - /* TODO: use prctl to get/set capabilities too ? */ + /* TODO: use libcap-ng if we need to get/set capabilities too ? */ + + if (myuid == uid && mygid == gid) { + DEBUG(SSSDBG_FUNC_DATA, "Already user [%"SPRIuid"].\n", uid); + return EOK; + } /* try to setgroups first should always work if CAP_SETUID is set, * otherwise it will always fail, failure is not critical though as @@ -177,11 +187,9 @@ errno_t switch_creds(TALLOC_CTX *mem_ctx, done: if (ret) { - if (ssc) { - /* attempt to restore creds first */ - restore_creds(ssc); - talloc_free(ssc); - } + /* attempt to restore creds first */ + restore_creds(ssc); + talloc_free(ssc); } else if (saved_creds) { *saved_creds = ssc; } @@ -190,6 +198,11 @@ done: errno_t restore_creds(struct sss_creds *saved_creds) { + if (saved_creds == NULL) { + /* In case save_creds was saved with the UID already dropped */ + return EOK; + } + return switch_creds(saved_creds, saved_creds->uid, saved_creds->gid, -- 1.9.3