>From 0931969ee8ab41b7fb6cf68834c57adb33ac17e0 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Sun, 19 Oct 2014 19:15:52 +0200 Subject: [PATCH 4/4] LDAP: Drop privileges after kinit in ldap_child After ldap_child initializes privileges using root-owned keytab, it drops privileges to the SSSD user, minimizing the amount of code that runs as root. --- Makefile.am | 4 +- src/providers/ldap/ldap_child.c | 96 ++++++++++++++++++++------------- src/providers/ldap/sdap_child_helpers.c | 8 ++- 3 files changed, 70 insertions(+), 38 deletions(-) diff --git a/Makefile.am b/Makefile.am index 405c5b281849d9a626c47da8e4f387093ffe68f7..4182a09ea078ba874dc708a57bd6bf06475bdf34 100644 --- a/Makefile.am +++ b/Makefile.am @@ -2512,7 +2512,9 @@ ldap_child_SOURCES = \ src/util/atomic_io.c \ src/util/authtok.c \ src/util/util.c \ - src/util/signal.c + src/util/signal.c \ + src/util/become_user.c \ + $(NULL) ldap_child_CFLAGS = \ $(AM_CFLAGS) \ $(POPT_CFLAGS) \ diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c index e1abc9fd73f2ae95f0a0c28159589ebd36d2cf06..a922b181715c5e89301e9f50bdb81723d1ff2a6a 100644 --- a/src/providers/ldap/ldap_child.c +++ b/src/providers/ldap/ldap_child.c @@ -49,6 +49,8 @@ struct input_buffer { const char *princ_str; const char *keytab_name; krb5_deltat lifetime; + uid_t uid; + gid_t gid; }; static errno_t unpack_buffer(uint8_t *buf, size_t size, @@ -99,6 +101,12 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, SAFEALIGN_COPY_UINT32_CHECK(&ibuf->lifetime, buf + p, size, &p); DEBUG(SSSDBG_TRACE_LIBS, "lifetime: %u\n", ibuf->lifetime); + /* UID and GID to run as */ + SAFEALIGN_COPY_UINT32_CHECK(&ibuf->uid, buf + p, size, &p); + SAFEALIGN_COPY_UINT32_CHECK(&ibuf->gid, buf + p, size, &p); + DEBUG(SSSDBG_FUNC_DATA, + "Will run as [%"SPRIuid"][%"SPRIgid"].\n", ibuf->uid, ibuf->gid); + return EOK; } @@ -242,6 +250,8 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx, const char *princ_str, const char *keytab_name, const krb5_deltat lifetime, + uid_t uid, + gid_t gid, const char **ccname_out, time_t *expire_time_out) { @@ -372,42 +382,6 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx, goto done; } - ccname_file_dummy = talloc_asprintf(tmp_ctx, "%s/ccache_%s_XXXXXX", - DB_PATH, realm_name); - ccname_file = talloc_asprintf(tmp_ctx, "%s/ccache_%s", - DB_PATH, realm_name); - if (ccname_file_dummy == NULL || ccname_file == NULL) { - ret = ENOMEM; - goto done; - } - - old_umask = umask(077); - fd = mkstemp(ccname_file_dummy); - umask(old_umask); - if (fd == -1) { - ret = errno; - goto done; - } - /* We only care about creating a unique file name here, we don't - * need the fd - */ - close(fd); - - ccname_dummy = talloc_asprintf(tmp_ctx, "FILE:%s", ccname_file_dummy); - ccname = talloc_asprintf(tmp_ctx, "FILE:%s", ccname_file); - if (ccname_dummy == NULL || ccname == NULL) { - krberr = ENOMEM; - goto done; - } - DEBUG(SSSDBG_TRACE_INTERNAL, "keytab ccname: [%s]\n", ccname_dummy); - - krberr = krb5_cc_resolve(context, ccname_dummy, &ccache); - if (krberr) { - DEBUG(SSSDBG_OP_FAILURE, "Failed to set cache name: %s\n", - sss_krb5_get_error_message(context, krberr)); - goto done; - } - memset(&my_creds, 0, sizeof(my_creds)); memset(&options, 0, sizeof(options)); @@ -423,8 +397,36 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx, } sss_krb5_get_init_creds_opt_set_canonicalize(&options, canonicalize); + ccname_file = talloc_asprintf(tmp_ctx, "%s/ccache_%s", + DB_PATH, realm_name); + if (ccname_file == NULL) { + ret = ENOMEM; + goto done; + } + + ccname_file_dummy = talloc_asprintf(tmp_ctx, "%s/ccache_%s_XXXXXX", + DB_PATH, realm_name); + if (ccname_file_dummy == NULL) { + ret = ENOMEM; + goto done; + } + + old_umask = umask(077); + fd = mkstemp(ccname_file_dummy); + umask(old_umask); + if (fd == -1) { + ret = errno; + goto done; + } + /* We only care about creating a unique file name here, we don't + * need the fd + */ + close(fd); + krberr = krb5_get_init_creds_keytab(context, &my_creds, kprinc, keytab, 0, NULL, &options); + krb5_kt_close(context, keytab); + keytab = NULL; if (krberr) { DEBUG(SSSDBG_FATAL_FAILURE, "Failed to init credentials: %s\n", @@ -438,6 +440,27 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx, } DEBUG(SSSDBG_TRACE_INTERNAL, "credentials initialized\n"); + krberr = become_user(uid, gid); + if (krberr != 0) { + DEBUG(SSSDBG_CRIT_FAILURE, "become_user failed.\n"); + goto done; + } + + ccname_dummy = talloc_asprintf(tmp_ctx, "FILE:%s", ccname_file_dummy); + ccname = talloc_asprintf(tmp_ctx, "FILE:%s", ccname_file); + if (ccname_dummy == NULL || ccname == NULL) { + krberr = ENOMEM; + goto done; + } + DEBUG(SSSDBG_TRACE_INTERNAL, "keytab ccname: [%s]\n", ccname_dummy); + + krberr = krb5_cc_resolve(context, ccname_dummy, &ccache); + if (krberr) { + DEBUG(SSSDBG_OP_FAILURE, "Failed to set cache name: %s\n", + sss_krb5_get_error_message(context, krberr)); + goto done; + } + /* Use updated principal if changed due to canonicalization. */ krberr = krb5_cc_initialize(context, ccache, my_creds.client); if (krberr) { @@ -643,6 +666,7 @@ int main(int argc, const char *argv[]) kerr = ldap_child_get_tgt_sync(main_ctx, ibuf->realm_str, ibuf->princ_str, ibuf->keytab_name, ibuf->lifetime, + ibuf->uid, ibuf->gid, &ccname, &expire_time); if (kerr != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "ldap_child_get_tgt_sync failed.\n"); diff --git a/src/providers/ldap/sdap_child_helpers.c b/src/providers/ldap/sdap_child_helpers.c index 448c5af10e9da8949bcaa39d8a3fa05ec309e16f..e5d46b9b756cd50fadb212da72ad1cc9bdd93330 100644 --- a/src/providers/ldap/sdap_child_helpers.c +++ b/src/providers/ldap/sdap_child_helpers.c @@ -152,7 +152,7 @@ static errno_t create_tgt_req_send_buffer(TALLOC_CTX *mem_ctx, return ENOMEM; } - buf->size = 4 * sizeof(uint32_t); + buf->size = 6 * sizeof(uint32_t); if (realm_str) { buf->size += strlen(realm_str); } @@ -201,6 +201,12 @@ static errno_t create_tgt_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; } -- 1.9.3