summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJakub Hrozek <jhrozek@redhat.com>2014-10-19 12:28:13 +0200
committerJakub Hrozek <jhrozek@redhat.com>2014-11-18 20:33:46 +0100
commit35b4b217fa2b91bfc8d58c47024faf41c95fc807 (patch)
tree6517adfb61aedb2f721855c3f6b38887b2f1c117
parent2745b0156f12df7a7eb93d57716233243658e4d9 (diff)
downloadsssd-35b4b217fa2b91bfc8d58c47024faf41c95fc807.tar.gz
sssd-35b4b217fa2b91bfc8d58c47024faf41c95fc807.tar.xz
sssd-35b4b217fa2b91bfc8d58c47024faf41c95fc807.zip
KRB5: Do not switch_creds() if already the specified user
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 <sbose@redhat.com> Reviewed-by: Lukáš Slebodník <lslebodn@redhat.com>
-rw-r--r--src/tests/cwrap/test_become_user.c7
-rw-r--r--src/util/become_user.c29
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 06d3ad425..7ecea5aac 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 b5f94f993..7dd2c752b 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,