From 74be564f1621e5f20d035d327888b2da795ecb90 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Wed, 10 Apr 2024 18:49:54 +0200 Subject: [PATCH 1/2] LDAP_CHILD: replace `become_user()` with `sss_drop_all_caps()` Since e2c26e810c3635124255e7619272591eab143553 'ldap_child' always runs under SSSD_USER and uses file capabilities instead. For this reason it doesn't make sense to call `become_user()` - `sss_drop_all_caps()` is enough. --- src/providers/ldap/ldap_child.c | 16 +--------------- src/providers/ldap/sdap_child_helpers.c | 6 ------ 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c index 470f52e7cfa..26528f4c1c3 100644 --- a/src/providers/ldap/ldap_child.c +++ b/src/providers/ldap/ldap_child.c @@ -59,8 +59,6 @@ struct input_buffer { char *keytab_name; krb5_deltat lifetime; krb5_context context; - uid_t uid; - gid_t gid; }; static inline const char *command_to_str(enum ldap_child_command cmd) @@ -133,14 +131,6 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, ibuf->lifetime = (krb5_deltat)value; DEBUG(SSSDBG_TRACE_LIBS, "lifetime: %u\n", ibuf->lifetime); - /* UID and GID to run as */ - SAFEALIGN_COPY_UINT32_CHECK(&value, buf + p, size, &p); - ibuf->uid = (uid_t)value; - SAFEALIGN_COPY_UINT32_CHECK(&value, buf + p, size, &p); - ibuf->gid = (gid_t)value; - DEBUG(SSSDBG_FUNC_DATA, - "Will run as [%"SPRIuid"][%"SPRIgid"].\n", ibuf->uid, ibuf->gid); - return EOK; } @@ -956,11 +946,7 @@ static errno_t handle_get_tgt(TALLOC_CTX *mem_ctx, DEBUG(SSSDBG_TRACE_INTERNAL, "Kerberos context initialized\n"); - kerr = become_user(ibuf->uid, ibuf->gid); - if (kerr != 0) { - DEBUG(SSSDBG_CRIT_FAILURE, "become_user() failed.\n"); - return kerr; - } + sss_drop_all_caps(); DEBUG(SSSDBG_TRACE_INTERNAL, "Running as [%"SPRIuid"][%"SPRIgid"].\n", geteuid(), getegid()); diff --git a/src/providers/ldap/sdap_child_helpers.c b/src/providers/ldap/sdap_child_helpers.c index 24c03a51959..1e66505c24e 100644 --- a/src/providers/ldap/sdap_child_helpers.c +++ b/src/providers/ldap/sdap_child_helpers.c @@ -211,12 +211,6 @@ static errno_t create_child_req_send_buffer(TALLOC_CTX *mem_ctx, /* lifetime */ SAFEALIGN_SET_UINT32(&buf->data[rp], lifetime, &rp); - /* UID and GID to drop privileges to, if needed. The ldap_child process runs as - * setuid if the back end runs unprivileged as it needs to access the keytab - */ - SAFEALIGN_SET_UINT32(&buf->data[rp], geteuid(), &rp); - SAFEALIGN_SET_UINT32(&buf->data[rp], getegid(), &rp); - *io_buf = buf; return EOK; } From 7bfe4f571c22999ba0f78a156dfdd7d6bf13141e Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Wed, 10 Apr 2024 19:19:46 +0200 Subject: [PATCH 2/2] KRB5_CHILD: keep 'set-user-ID' in `k5c_become_user()` Keep saved set-user-ID in `k5c_become_user()` so that 'sssd_be' running under SSSD_USER could signal it. Resolves: https://github.com/SSSD/sssd/issues/5536 --- src/monitor/monitor_bootstrap.c | 2 +- src/providers/krb5/krb5_child.c | 2 +- src/tests/cwrap/test_become_user.c | 4 ++-- src/util/become_user.c | 19 +++++++++++-------- src/util/util.h | 2 +- 5 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/monitor/monitor_bootstrap.c b/src/monitor/monitor_bootstrap.c index d4c1846abeb..0e28141e207 100644 --- a/src/monitor/monitor_bootstrap.c +++ b/src/monitor/monitor_bootstrap.c @@ -95,7 +95,7 @@ int bootstrap_monitor_process(void) */ sss_log(SSS_LOG_WARNING, "'sssd.conf::"CONFDB_MONITOR_USER_RUNAS"' " "option is deprecated. Run under '"SSSD_USER"' initially instead."); - ret = become_user(target_uid, target_gid); /* drops all caps */ + ret = become_user(target_uid, target_gid, false); /* drops all caps */ if (ret != 0) { sss_log(SSS_LOG_ALERT, "Failed to change uid:gid"); return 1; diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c index cb5a1bea224..5c1850e9c7f 100644 --- a/src/providers/krb5/krb5_child.c +++ b/src/providers/krb5/krb5_child.c @@ -152,7 +152,7 @@ static errno_t k5c_become_user(uid_t uid, gid_t gid, bool is_posix) "Will not drop privileges for a non-POSIX user\n"); return EOK; } - return become_user(uid, gid); + return become_user(uid, gid, true); } static krb5_error_code set_lifetime_options(struct cli_opts *cli_opts, diff --git a/src/tests/cwrap/test_become_user.c b/src/tests/cwrap/test_become_user.c index e63cde9d728..1ffc95917af 100644 --- a/src/tests/cwrap/test_become_user.c +++ b/src/tests/cwrap/test_become_user.c @@ -43,7 +43,7 @@ void test_become_user(void **state) pid = fork(); if (pid == 0) { /* Change the UID in a child */ - ret = become_user(sssd->pw_uid, sssd->pw_gid); + ret = become_user(sssd->pw_uid, sssd->pw_gid, false); assert_int_equal(ret, EOK); /* Make sure we have the requested UID and GID now and there @@ -55,7 +55,7 @@ void test_become_user(void **state) assert_int_equal(getgid(), sssd->pw_gid); /* Another become_user is a no-op */ - ret = become_user(sssd->pw_uid, sssd->pw_gid); + ret = become_user(sssd->pw_uid, sssd->pw_gid, false); assert_int_equal(ret, EOK); assert_int_equal(getgroups(0, NULL), 0); diff --git a/src/util/become_user.c b/src/util/become_user.c index c3f726d1894..d2bd7cfd12c 100644 --- a/src/util/become_user.c +++ b/src/util/become_user.c @@ -25,10 +25,10 @@ #include "util/util.h" #include -errno_t become_user(uid_t uid, gid_t gid) +errno_t become_user(uid_t uid, gid_t gid, bool keep_set_uid) { uid_t cuid; - int ret; + int ret = EOK; DEBUG(SSSDBG_FUNC_DATA, "Trying to become user [%"SPRIuid"][%"SPRIgid"].\n", uid, gid); @@ -37,7 +37,7 @@ errno_t become_user(uid_t uid, gid_t gid) cuid = geteuid(); if (uid == cuid) { DEBUG(SSSDBG_FUNC_DATA, "Already user [%"SPRIuid"].\n", uid); - return EOK; + goto done; } /* drop supplementary groups first */ @@ -46,7 +46,7 @@ errno_t become_user(uid_t uid, gid_t gid) ret = errno; DEBUG(SSSDBG_CRIT_FAILURE, "setgroups failed [%d][%s].\n", ret, strerror(ret)); - return ret; + goto done; } /* change GID so that root cannot be regained (changes saved GID too) */ @@ -55,20 +55,23 @@ errno_t become_user(uid_t uid, gid_t gid) ret = errno; DEBUG(SSSDBG_CRIT_FAILURE, "setresgid failed [%d][%s].\n", ret, strerror(ret)); - return ret; + 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, uid); + ret = setresuid(uid, uid, (keep_set_uid ? -1 : uid)); if (ret == -1) { ret = errno; DEBUG(SSSDBG_CRIT_FAILURE, "setresuid failed [%d][%s].\n", ret, strerror(ret)); - return ret; + goto done; } - return EOK; +done: + sss_drop_all_caps(); + + return ret; } struct sss_creds { diff --git a/src/util/util.h b/src/util/util.h index aad112b5cca..e3c973d77c7 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -744,7 +744,7 @@ errno_t mod_defaults_list(TALLOC_CTX *mem_ctx, const char **defaults_list, char **mod_list, char ***_list); /* from become_user.c */ -errno_t become_user(uid_t uid, gid_t gid); +errno_t become_user(uid_t uid, gid_t gid, bool keep_set_uid); struct sss_creds; errno_t switch_creds(TALLOC_CTX *mem_ctx, uid_t uid, gid_t gid,