[SSSD] [PATCH] Back end patches for running SSSD as a non-root user

Jakub Hrozek jhrozek at redhat.com
Fri Oct 24 20:52:47 UTC 2014


On Mon, Oct 20, 2014 at 06:35:13PM +0200, Michal Židek wrote:
> On 10/20/2014 04:13 PM, Jakub Hrozek wrote:
> >Hi,
> >
> >I'll send patches that help SSSD run as a non-root user to this thread.
> >I'm still chasing some bugs in the krb5_child changes, but the attached
> >patches are ready for review.
> >
> >If other developers don't like the idea of a Python-based unit test that
> >spawns a KDC, I'm equally fine with keeping those patches in my tree --
> >they served their purpose, after all.
> >
> >
> >0001-BUILD-Install-ldap_child-and-krb5_child-as-setuid-if.patch
> >
> 
> Ack.
> 
> >
> >0002-LDAP-Move-sss_krb5_verify_keytab_ex-to-ldap_child.patch
> >
> 
> Ack.
> 
> >
> >0003-LDAP-read-the-correct-data-type-from-ldap_child-s-in.patch
> >
> >      /* ticket lifetime */
> >-    SAFEALIGN_COPY_INT32_CHECK(&ibuf->lifetime, buf + p, size, &p);
> >+    SAFEALIGN_COPY_UINT32_CHECK(&ibuf->lifetime, buf + p, size, &p);
> >      DEBUG(SSSDBG_TRACE_LIBS, "lifetime: %d\n", ibuf->lifetime);
>                                             ^^^
> Should be %u. Otherwise Ack.

Fixed.

> 
> >
> >0004-LDAP-Drop-privileges-after-kinit-in-ldap_child.patch
> >
> 
> Ack.
> 

Thank you for the review.

> >
> >0005-TESTS-Fix-krb5_child-test.patch
> >0006-TESTS-Add-a-cwrap-enabled-test-for-krb5_child.patch

I'm withdrawing these two patches for now. The krb5_child needs more
work and should be submitted with the krb5_child changes anyway.

The ldap_child patches are attached.
-------------- next part --------------
>From 11c1db91ba9d036f73167b2d74012d3f0ada60fd Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Sat, 11 Oct 2014 20:22:42 +0200
Subject: [PATCH 1/4] BUILD: Install ldap_child and as setuid if running under
 non-privileged user

The ldap_child permissions should be 4750, owned by root.sssd,
to make sure only root and sssd can execute the child and if executed by
sssd, the child will run as root.
---
 Makefile.am | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 61bf5cf957d4024b67f48cf42f5735b5fa368945..a913cc9c8206b8e95c6c16ff0beb09acefa1a606 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2825,6 +2825,11 @@ else
 	$(MKDIR_P) $(DESTDIR)$(initdir)
 endif
 
+if SSSD_USER
+	chgrp $(SSSD_USER) $(sssdlibexecdir)/ldap_child
+	chmod 4750 $(sssdlibexecdir)/ldap_child
+endif
+
 install-data-hook:
 	rm $(DESTDIR)/$(nsslibdir)/libnss_sss.so.2 \
        $(DESTDIR)/$(nsslibdir)/libnss_sss.so
-- 
1.9.3

-------------- next part --------------
>From 154a897b09136c4499fc024c9110096d34d2b6d5 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Sat, 11 Oct 2014 17:39:21 +0200
Subject: [PATCH 2/4] LDAP: Move sss_krb5_verify_keytab_ex to ldap_child

The function was called from one place only, so it makes no sense to
keep it in a shared module. Moreover, the function should only be
called from code that runs as root.
---
 src/providers/ldap/ldap_child.c | 80 ++++++++++++++++++++++++++++++++++++++++-
 src/util/sss_krb5.c             | 76 ---------------------------------------
 src/util/sss_krb5.h             |  3 --
 3 files changed, 79 insertions(+), 80 deletions(-)

diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c
index e5779b70906d90ab855677f04a154e179f2163c6..faa6f027ba52f55985c63e8055292e616b5bb6bf 100644
--- a/src/providers/ldap/ldap_child.c
+++ b/src/providers/ldap/ldap_child.c
@@ -160,6 +160,84 @@ set_child_debugging(krb5_context ctx)
     return EOK;
 }
 
+static int lc_verify_keytab_ex(const char *principal,
+                               const char *keytab_name,
+                               krb5_context context,
+                               krb5_keytab keytab)
+{
+    bool found;
+    char *kt_principal;
+    krb5_error_code krberr;
+    krb5_kt_cursor cursor;
+    krb5_keytab_entry entry;
+
+    krberr = krb5_kt_start_seq_get(context, keytab, &cursor);
+    if (krberr) {
+        DEBUG(SSSDBG_FATAL_FAILURE,
+              "Cannot read keytab [%s].\n", KEYTAB_CLEAN_NAME);
+
+        sss_log(SSS_LOG_ERR, "Error reading keytab file [%s]: [%d][%s]. "
+                             "Unable to create GSSAPI-encrypted LDAP "
+                             "connection.",
+                             KEYTAB_CLEAN_NAME, krberr,
+                             sss_krb5_get_error_message(context, krberr));
+
+        return EIO;
+    }
+
+    found = false;
+    while((krb5_kt_next_entry(context, keytab, &entry, &cursor)) == 0){
+        krberr = krb5_unparse_name(context, entry.principal, &kt_principal);
+        if (krberr) {
+            DEBUG(SSSDBG_FATAL_FAILURE,
+                  "Could not parse keytab entry\n");
+            sss_log(SSS_LOG_ERR, "Could not parse keytab entry\n");
+            return EIO;
+        }
+
+        if (strcmp(principal, kt_principal) == 0) {
+            found = true;
+        }
+        free(kt_principal);
+        krberr = sss_krb5_free_keytab_entry_contents(context, &entry);
+        if (krberr) {
+            /* This should never happen. The API docs for this function
+             * specify only success for this function
+             */
+            DEBUG(SSSDBG_CRIT_FAILURE,"Could not free keytab entry contents\n");
+            /* This is non-fatal, so we'll continue here */
+        }
+
+        if (found) {
+            break;
+        }
+    }
+
+    krberr = krb5_kt_end_seq_get(context, keytab, &cursor);
+    if (krberr) {
+        DEBUG(SSSDBG_FATAL_FAILURE, "Could not close keytab.\n");
+        sss_log(SSS_LOG_ERR, "Could not close keytab file [%s].",
+                             KEYTAB_CLEAN_NAME);
+        return EIO;
+    }
+
+    if (!found) {
+        DEBUG(SSSDBG_FATAL_FAILURE,
+              "Principal [%s] not found in keytab [%s]\n",
+               principal,
+               KEYTAB_CLEAN_NAME);
+        sss_log(SSS_LOG_ERR, "Error processing keytab file [%s]: "
+                             "Principal [%s] was not found. "
+                             "Unable to create GSSAPI-encrypted LDAP connection.",
+                             KEYTAB_CLEAN_NAME, principal);
+
+        return EFAULT;
+    }
+
+    return EOK;
+}
+
+
 static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx,
                                                const char *realm_str,
                                                const char *princ_str,
@@ -287,7 +365,7 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx,
     }
 
     /* Verify the keytab */
-    ret = sss_krb5_verify_keytab_ex(full_princ, keytab_name, context, keytab);
+    ret = lc_verify_keytab_ex(full_princ, keytab_name, context, keytab);
     if (ret) {
         DEBUG(SSSDBG_OP_FAILURE,
                 "Unable to verify principal is present in the keytab\n");
diff --git a/src/util/sss_krb5.c b/src/util/sss_krb5.c
index d9b076ebeccbcd571f14e1b642cade86b21fd5d7..377d61e47254133a6c1b6ed2f20dc0ef1a572aa3 100644
--- a/src/util/sss_krb5.c
+++ b/src/util/sss_krb5.c
@@ -249,82 +249,6 @@ done:
     return ret;
 }
 
-int sss_krb5_verify_keytab_ex(const char *principal, const char *keytab_name,
-                              krb5_context context, krb5_keytab keytab)
-{
-    bool found;
-    char *kt_principal;
-    krb5_error_code krberr;
-    krb5_kt_cursor cursor;
-    krb5_keytab_entry entry;
-
-    krberr = krb5_kt_start_seq_get(context, keytab, &cursor);
-    if (krberr) {
-        DEBUG(SSSDBG_FATAL_FAILURE,
-              "Cannot read keytab [%s].\n", KEYTAB_CLEAN_NAME);
-
-        sss_log(SSS_LOG_ERR, "Error reading keytab file [%s]: [%d][%s]. "
-                             "Unable to create GSSAPI-encrypted LDAP "
-                             "connection.",
-                             KEYTAB_CLEAN_NAME, krberr,
-                             sss_krb5_get_error_message(context, krberr));
-
-        return EIO;
-    }
-
-    found = false;
-    while((krb5_kt_next_entry(context, keytab, &entry, &cursor)) == 0){
-        krberr = krb5_unparse_name(context, entry.principal, &kt_principal);
-        if (krberr) {
-            DEBUG(SSSDBG_FATAL_FAILURE,
-                  "Could not parse keytab entry\n");
-            sss_log(SSS_LOG_ERR, "Could not parse keytab entry\n");
-            return EIO;
-        }
-
-        if (strcmp(principal, kt_principal) == 0) {
-            found = true;
-        }
-        free(kt_principal);
-        krberr = sss_krb5_free_keytab_entry_contents(context, &entry);
-        if (krberr) {
-            /* This should never happen. The API docs for this function
-             * specify only success for this function
-             */
-            DEBUG(SSSDBG_CRIT_FAILURE,"Could not free keytab entry contents\n");
-            /* This is non-fatal, so we'll continue here */
-        }
-
-        if (found) {
-            break;
-        }
-    }
-
-    krberr = krb5_kt_end_seq_get(context, keytab, &cursor);
-    if (krberr) {
-        DEBUG(SSSDBG_FATAL_FAILURE, "Could not close keytab.\n");
-        sss_log(SSS_LOG_ERR, "Could not close keytab file [%s].",
-                             KEYTAB_CLEAN_NAME);
-        return EIO;
-    }
-
-    if (!found) {
-        DEBUG(SSSDBG_FATAL_FAILURE,
-              "Principal [%s] not found in keytab [%s]\n",
-               principal,
-               KEYTAB_CLEAN_NAME);
-        sss_log(SSS_LOG_ERR, "Error processing keytab file [%s]: "
-                             "Principal [%s] was not found. "
-                             "Unable to create GSSAPI-encrypted LDAP connection.",
-                             KEYTAB_CLEAN_NAME, principal);
-
-        return EFAULT;
-    }
-
-    return EOK;
-}
-
-
 enum matching_mode {MODE_NORMAL, MODE_PREFIX, MODE_POSTFIX};
 /**
  * We only have primary and instances stored separately, we need to
diff --git a/src/util/sss_krb5.h b/src/util/sss_krb5.h
index 83c72097594dc24de1f8ac93d5394b6766a449f4..afa0d1943d8a23ae1543ae3874b5abbfbb4b3372 100644
--- a/src/util/sss_krb5.h
+++ b/src/util/sss_krb5.h
@@ -70,9 +70,6 @@ void KRB5_CALLCONV sss_krb5_get_init_creds_opt_free (krb5_context context,
 
 void KRB5_CALLCONV sss_krb5_free_unparsed_name(krb5_context context, char *name);
 
-int sss_krb5_verify_keytab_ex(const char *principal, const char *keytab_name,
-                              krb5_context context, krb5_keytab keytab);
-
 krb5_error_code find_principal_in_keytab(krb5_context ctx,
                                          krb5_keytab keytab,
                                          const char *pattern_primary,
-- 
1.9.3

-------------- next part --------------
>From 363ff88add3eaed24808fe2eb9e38a066dd9a5f5 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Sun, 19 Oct 2014 19:20:28 +0200
Subject: [PATCH 3/4] LDAP: read the correct data type from ldap_child's input
 buffer

The back end wrote uint32_t, the ldap_child process would read int32_t.
---
 src/providers/ldap/ldap_child.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c
index faa6f027ba52f55985c63e8055292e616b5bb6bf..db38cad59fe9eb1f28f78d27c2a2c260075060d1 100644
--- a/src/providers/ldap/ldap_child.c
+++ b/src/providers/ldap/ldap_child.c
@@ -96,8 +96,8 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size,
     }
 
     /* ticket lifetime */
-    SAFEALIGN_COPY_INT32_CHECK(&ibuf->lifetime, buf + p, size, &p);
-    DEBUG(SSSDBG_TRACE_LIBS, "lifetime: %d\n", ibuf->lifetime);
+    SAFEALIGN_COPY_UINT32_CHECK(&ibuf->lifetime, buf + p, size, &p);
+    DEBUG(SSSDBG_TRACE_LIBS, "lifetime: %u\n", ibuf->lifetime);
 
     return EOK;
 }
-- 
1.9.3

-------------- next part --------------
>From 6aef235e22bd7c7609e20b1293344ecf505e8eb0 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
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         | 79 ++++++++++++++++++++-------------
 src/providers/ldap/sdap_child_helpers.c |  8 +++-
 3 files changed, 59 insertions(+), 32 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index a913cc9c8206b8e95c6c16ff0beb09acefa1a606..f0a427254a6051c29588cd74b6622bb3e3e6cae1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2498,7 +2498,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 db38cad59fe9eb1f28f78d27c2a2c260075060d1..6f8162e38066d6d1a1262c54ec441d4679f63206 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;
 }
 
@@ -243,6 +251,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)
 {
@@ -373,6 +383,44 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx,
         goto done;
     }
 
+    memset(&my_creds, 0, sizeof(my_creds));
+    memset(&options, 0, sizeof(options));
+
+    krb5_get_init_creds_opt_set_address_list(&options, NULL);
+    krb5_get_init_creds_opt_set_forwardable(&options, 0);
+    krb5_get_init_creds_opt_set_proxiable(&options, 0);
+    krb5_get_init_creds_opt_set_tkt_life(&options, lifetime);
+
+    tmp_str = getenv("KRB5_CANONICALIZE");
+    if (tmp_str != NULL && strcasecmp(tmp_str, "true") == 0) {
+        DEBUG(SSSDBG_CONF_SETTINGS, "Will canonicalize principals\n");
+        canonicalize = 1;
+    }
+    sss_krb5_get_init_creds_opt_set_canonicalize(&options, canonicalize);
+
+    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",
+               sss_krb5_get_error_message(context, krberr));
+        sss_log(SSS_LOG_ERR,
+                "Failed to initialize credentials using keytab [%s]: %s. "
+                "Unable to create GSSAPI-encrypted LDAP connection.",
+                KEYTAB_CLEAN_NAME,
+                sss_krb5_get_error_message(context, krberr));
+        goto done;
+    }
+    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_file_dummy = talloc_asprintf(tmp_ctx, "%s/ccache_%s_XXXXXX",
                                         DB_PATH, realm_name);
     ccname_file = talloc_asprintf(tmp_ctx, "%s/ccache_%s",
@@ -409,36 +457,6 @@ static krb5_error_code ldap_child_get_tgt_sync(TALLOC_CTX *memctx,
         goto done;
     }
 
-    memset(&my_creds, 0, sizeof(my_creds));
-    memset(&options, 0, sizeof(options));
-
-    krb5_get_init_creds_opt_set_address_list(&options, NULL);
-    krb5_get_init_creds_opt_set_forwardable(&options, 0);
-    krb5_get_init_creds_opt_set_proxiable(&options, 0);
-    krb5_get_init_creds_opt_set_tkt_life(&options, lifetime);
-
-    tmp_str = getenv("KRB5_CANONICALIZE");
-    if (tmp_str != NULL && strcasecmp(tmp_str, "true") == 0) {
-        DEBUG(SSSDBG_CONF_SETTINGS, "Will canonicalize principals\n");
-        canonicalize = 1;
-    }
-    sss_krb5_get_init_creds_opt_set_canonicalize(&options, canonicalize);
-
-    krberr = krb5_get_init_creds_keytab(context, &my_creds, kprinc,
-                                        keytab, 0, NULL, &options);
-    if (krberr) {
-        DEBUG(SSSDBG_FATAL_FAILURE,
-              "Failed to init credentials: %s\n",
-               sss_krb5_get_error_message(context, krberr));
-        sss_log(SSS_LOG_ERR,
-                "Failed to initialize credentials using keytab [%s]: %s. "
-                "Unable to create GSSAPI-encrypted LDAP connection.",
-                KEYTAB_CLEAN_NAME,
-                sss_krb5_get_error_message(context, krberr));
-        goto done;
-    }
-    DEBUG(SSSDBG_TRACE_INTERNAL, "credentials initialized\n");
-
     /* Use updated principal if changed due to canonicalization. */
     krberr = krb5_cc_initialize(context, ccache, my_creds.client);
     if (krberr) {
@@ -644,6 +662,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



More information about the sssd-devel mailing list