[SSSD] [PATCH] nss: return ldap_user_extra_attrs attributes in origbyname request

Sumit Bose sbose at redhat.com
Fri Oct 24 19:46:12 UTC 2014


Hi,

this two patches allow to have extra user attributes, like e.g. a
telephone number, for AD users of trusted domains on the IPA clients.

This first makes it possible that the attributes given in
ldap_user_extra_attrs can be read from AD in ipa-server-mode. The second
extends the list of attributes returned by sss_nss_getorigbyname().

bye,
Sumit

P.S. in the first patch I used sdap_extend_map_with_list() as it is used
in other places as well where the maps in the 4th and 6th argument are
the same. If I see it correctly we are leaking the old version of the
map this way. Since this currently happen only once during init it does
not do much harm, but maybe a ticket to fix this might be useful.
-------------- next part --------------
From dedfb80621b379982ac0200a771d0d8b4fa78332 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 24 Oct 2014 15:41:04 +0200
Subject: [PATCH 1/2] IPA: inherit ldap_user_extra_attrs to AD subdomains

Currently the component of the IPA provider which reads the AD user and
group attributes in ipa-server-mode uses  default settings for the LDAP
related attributes. As a result even if ldap_user_extra_attrs is defined
in sssd.conf no extra attributes are read from AD.

With the patch the value if ldap_user_extra_attrs is inherited to the AD
subdomains to allow them to read extra attributes as well.

Related to https://fedorahosted.org/sssd/ticket/2464
---
 src/providers/ipa/ipa_subdomains.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
index c61c1c6..9281aab 100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -109,6 +109,7 @@ ipa_ad_ctx_new(struct be_ctx *be_ctx,
     char *ad_domain;
     struct sdap_domain *sdom;
     errno_t ret;
+    const char *extra_attrs;
 
     ad_options = ad_create_default_options(id_ctx, id_ctx->server_mode->realm,
                                            id_ctx->server_mode->hostname);
@@ -135,6 +136,36 @@ ipa_ad_ctx_new(struct be_ctx *be_ctx,
         return ret;
     }
 
+    extra_attrs = dp_opt_get_string(id_ctx->sdap_id_ctx->opts->basic,
+                            SDAP_USER_EXTRA_ATTRS);
+    if (extra_attrs != NULL) {
+        DEBUG(SSSDBG_TRACE_ALL,
+              "Setting extra attrs for subdomain [%s] to [%s].\n", ad_domain,
+                                                                   extra_attrs);
+
+        ret = dp_opt_set_string(ad_options->id->basic, SDAP_USER_EXTRA_ATTRS,
+                                extra_attrs);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, "dp_opt_get_string failed.\n");
+            talloc_free(ad_options);
+            return ret;
+        }
+
+        ret = sdap_extend_map_with_list(ad_options->id, ad_options->id,
+                                        SDAP_USER_EXTRA_ATTRS,
+                                        ad_options->id->user_map,
+                                        SDAP_OPTS_USER,
+                                        &ad_options->id->user_map,
+                                        &ad_options->id->user_map_cnt);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, "sdap_extend_map_with_list failed.\n");
+            talloc_free(ad_options);
+            return ret;
+        }
+    } else {
+        DEBUG(SSSDBG_TRACE_ALL, "No extra attrs set.\n");
+    }
+
     gc_service_name = talloc_asprintf(ad_options, "%s%s", "gc_", subdom->name);
     if (gc_service_name == NULL) {
         talloc_free(ad_options);
-- 
1.8.3.1

-------------- next part --------------
From 704e0e0d1d9088e3bf4d4c29ac7f604396402244 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 24 Oct 2014 11:30:33 +0200
Subject: [PATCH 2/2] nss: return ldap_user_extra_attrs attributes in
 origbyname request

To allow IPA clients to offer special attributes of AD users form
trusted domain the extdom plugin on the IPA server must send them to the
clients. The extdom plugin already uses sss_nss_getorigbyname() to get
attributes like the SID and the user principal name. This patch adds the
attributes given the the ldap_user_extra_attrs option to the list of
attributes returned by sss_nss_getorigbyname().

Fixes https://fedorahosted.org/sssd/ticket/2464
---
 src/responder/nss/nsssrv_cmd.c  | 170 ++++++++++++++++++++++++++++++++++------
 src/tests/cmocka/test_nss_srv.c | 130 ++++++++++++++++++++++++++++++
 2 files changed, 278 insertions(+), 22 deletions(-)

diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 9fca644..7481d49 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -4131,6 +4131,7 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx)
                            SYSDB_AD_ACCOUNT_EXPIRES,
                            SYSDB_AD_USER_ACCOUNT_CONTROL,
                            SYSDB_DEFAULT_ATTRS, NULL};
+    const char *all_attrs[] = { "*", NULL};
     bool user_found = false;
     bool group_found = false;
     struct ldb_message *msg = NULL;
@@ -4281,8 +4282,9 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx)
             }
         } else {
             ret = sysdb_search_user_by_name(cmdctx, dom,
-                                            sysdb_name ? sysdb_name : name,
-                                            attrs, &msg);
+                        sysdb_name ? sysdb_name : name,
+                        cmdctx->cmd == SSS_NSS_GETORIGBYNAME ? all_attrs :attrs,
+                        &msg);
             if (ret != EOK && ret != ENOENT) {
                 DEBUG(SSSDBG_CRIT_FAILURE,
                       "Failed to make request to our cache!\n");
@@ -4295,8 +4297,9 @@ static errno_t nss_cmd_getsidby_search(struct nss_dom_ctx *dctx)
             } else {
                 talloc_free(msg);
                 ret = sysdb_search_group_by_name(cmdctx, dom,
-                                                 sysdb_name ? sysdb_name : name,
-                                                 attrs, &msg);
+                       sysdb_name ? sysdb_name : name,
+                       cmdctx->cmd == SSS_NSS_GETORIGBYNAME ? all_attrs : attrs,
+                       &msg);
                 if (ret != EOK && ret != ENOENT) {
                     DEBUG(SSSDBG_CRIT_FAILURE,
                           "Failed to make request to our cache!\n");
@@ -4563,17 +4566,100 @@ static errno_t fill_sid(struct sss_packet *packet,
     return EOK;
 }
 
+static errno_t get_extra_attribute_list(TALLOC_CTX *mem_ctx,
+                                        struct confdb_ctx *cdb,
+                                        struct sss_domain_info *domain,
+                                        size_t *new_attrs_count,
+                                        char ***new_attrs_list)
+{
+    int ret;
+    TALLOC_CTX *tmp_ctx;
+    char *confdb_path;
+    char *extra_attrs;
+    char **extra_attrs_list;
+    size_t c;
+    int count;
+    char *sep;
+    struct sss_domain_info *domain_head;
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n");
+        return ENOMEM;
+    }
+
+    domain_head = get_domains_head(domain);
+    confdb_path = talloc_asprintf(tmp_ctx, CONFDB_DOMAIN_PATH_TMPL,
+                                  domain_head->name);
+    if (confdb_path == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, "talloc_asprintf failed.\n");
+        ret = ENOMEM;
+        goto done;
+    }
+
+    ret = confdb_get_string(cdb, tmp_ctx, confdb_path, "ldap_user_extra_attrs",
+                            NULL, &extra_attrs);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, "confdb_get_string failed.\n");
+        goto done;
+    }
+
+    if (extra_attrs == NULL) {
+        DEBUG(SSSDBG_TRACE_FUNC,
+        "ldap_user_extra_attrs not set, nothing to do.\n");
+        ret = ENOENT;
+        goto done;
+    }
+
+    ret = split_on_separator(tmp_ctx, extra_attrs, ',', true, true,
+                             &extra_attrs_list, &count);
+    if (ret != EOK || count < 0) {
+        DEBUG(SSSDBG_OP_FAILURE, "Failed to parse extra attributes!\n");
+        goto done;
+    }
+
+    for (c = 0; extra_attrs_list[c]; c++) {
+        sep = strchr(extra_attrs_list[c], ':');
+        if (sep != NULL) {
+            if (sep == extra_attrs_list[c] || *(sep + 1) == '\0') {
+                DEBUG(SSSDBG_CRIT_FAILURE,
+                      "Illegal ldap_user_extra_attrs entry found [%s].\n",
+                      extra_attrs_list[c]);
+                ret = EINVAL;
+                goto done;
+            }
+
+            *sep = '\0';
+        }
+    }
+
+    *new_attrs_count = count;
+    *new_attrs_list = talloc_steal(mem_ctx, extra_attrs_list);
+    ret = EOK;
+
+done:
+    talloc_free(tmp_ctx);
+
+    return ret;
+}
+
 static errno_t fill_orig(struct sss_packet *packet,
+                         struct resp_ctx *rctx,
+                         struct sss_domain_info *domain,
                          enum sss_id_type id_type,
                          struct ldb_message *msg)
 {
     int ret;
+    TALLOC_CTX *tmp_ctx;
     const char *tmp_str;
     uint8_t *body;
     size_t blen;
     size_t pctr = 0;
     size_t c;
     size_t sum;
+    size_t found;
+    size_t extra_attrs_count = 0;
+    char **extra_attrs_list = NULL;
     const char *orig_attr_list[] = {SYSDB_SID_STR,
                                     ORIGINALAD_PREFIX SYSDB_NAME,
                                     ORIGINALAD_PREFIX SYSDB_UIDNUM,
@@ -4586,42 +4672,81 @@ static errno_t fill_orig(struct sss_packet *packet,
                                     SYSDB_AD_ACCOUNT_EXPIRES,
                                     SYSDB_AD_USER_ACCOUNT_CONTROL,
                                     NULL};
-    struct sized_string keys[sizeof(orig_attr_list)];
-    struct sized_string vals[sizeof(orig_attr_list)];
+    struct sized_string *keys;
+    struct sized_string *vals;
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n");
+        return ENOMEM;
+    }
+
+    ret = get_extra_attribute_list(tmp_ctx, rctx->cdb, domain,
+                                   &extra_attrs_count, &extra_attrs_list);
+    if (ret != EOK && ret != ENOENT) {
+        DEBUG(SSSDBG_OP_FAILURE, "get_extra_attribute_list failed.\n");
+        goto done;
+    }
+
+    keys = talloc_array(tmp_ctx, struct sized_string,
+                        sizeof(orig_attr_list) + extra_attrs_count);
+    vals = talloc_array(tmp_ctx, struct sized_string,
+                        sizeof(orig_attr_list) + extra_attrs_count);
+    if (keys == NULL || vals == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, "talloc_array failed.\n");
+        ret = ENOMEM;
+        goto done;
+    }
 
     sum = 0;
+    found = 0;
     for (c = 0; orig_attr_list[c] != NULL; c++) {
         tmp_str = ldb_msg_find_attr_as_string(msg, orig_attr_list[c], NULL);
         if (tmp_str != NULL) {
-            to_sized_string(&keys[c], orig_attr_list[c]);
-            sum += keys[c].len;
-            to_sized_string(&vals[c], tmp_str);
-            sum += vals[c].len;
-        } else {
-            vals[c].len = 0;
+            to_sized_string(&keys[found], orig_attr_list[c]);
+            sum += keys[found].len;
+            to_sized_string(&vals[found], tmp_str);
+            sum += vals[found].len;
+
+            found++;
+        }
+    }
+
+    for (c = 0; c < extra_attrs_count; c++) {
+        tmp_str = ldb_msg_find_attr_as_string(msg, extra_attrs_list[c], NULL);
+        if (tmp_str != NULL) {
+            to_sized_string(&keys[found], extra_attrs_list[c]);
+            sum += keys[found].len;
+            to_sized_string(&vals[found], tmp_str);
+            sum += vals[found].len;
+
+            found++;
         }
     }
 
     ret = sss_packet_grow(packet, sum +  3 * sizeof(uint32_t));
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "sss_packet_grow failed.\n");
-        return ret;
+        goto done;
     }
 
     sss_packet_get_body(packet, &body, &blen);
     SAFEALIGN_SETMEM_UINT32(body, 1, &pctr); /* Num results */
     SAFEALIGN_SETMEM_UINT32(body + pctr, 0, &pctr); /* reserved */
     SAFEALIGN_COPY_UINT32(body + pctr, &id_type, &pctr);
-    for (c = 0; orig_attr_list[c] != NULL; c++) {
-        if (vals[c].len != 0) {
-            memcpy(&body[pctr], keys[c].str, keys[c].len);
-            pctr+= keys[c].len;
-            memcpy(&body[pctr], vals[c].str, vals[c].len);
-            pctr+= vals[c].len;
-        }
+    for (c = 0; c < found; c++) {
+        memcpy(&body[pctr], keys[c].str, keys[c].len);
+        pctr+= keys[c].len;
+        memcpy(&body[pctr], vals[c].str, vals[c].len);
+        pctr+= vals[c].len;
     }
 
-    return EOK;
+    ret = EOK;
+
+done:
+    talloc_free(tmp_ctx);
+
+    return ret;
 }
 
 static errno_t fill_name(struct sss_packet *packet,
@@ -4789,7 +4914,8 @@ static errno_t nss_cmd_getbysid_send_reply(struct nss_dom_ctx *dctx)
         ret = fill_sid(cctx->creq->out, id_type, dctx->res->msgs[0]);
         break;
     case SSS_NSS_GETORIGBYNAME:
-        ret = fill_orig(cctx->creq->out, id_type, dctx->res->msgs[0]);
+        ret = fill_orig(cctx->creq->out, cctx->rctx, dctx->domain, id_type,
+                        dctx->res->msgs[0]);
         break;
     default:
         DEBUG(SSSDBG_CRIT_FAILURE, "Unsupported request type.\n");
diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c
index 65298cd..d303858 100644
--- a/src/tests/cmocka/test_nss_srv.c
+++ b/src/tests/cmocka/test_nss_srv.c
@@ -1000,6 +1000,7 @@ void test_nss_setup(struct sss_test_conf_param params[],
     nss_test_ctx->rctx = mock_rctx(nss_test_ctx, nss_test_ctx->tctx->ev,
                                    nss_test_ctx->tctx->dom, nss_test_ctx->nctx);
     assert_non_null(nss_test_ctx->rctx);
+    nss_test_ctx->rctx->cdb = nss_test_ctx->tctx->confdb;
     nss_test_ctx->nctx->rctx = nss_test_ctx->rctx;
 
     /* Create client context */
@@ -1827,6 +1828,122 @@ void test_nss_getorigbyname(void **state)
     assert_int_equal(ret, EOK);
 }
 
+static int test_nss_getorigbyname_extra_check(uint32_t status, uint8_t *body,
+                                              size_t blen)
+{
+    const char *s;
+    enum sss_id_type id_type;
+    size_t rp = 2 * sizeof(uint32_t);
+
+    assert_int_equal(status, EOK);
+
+    SAFEALIGN_COPY_UINT32(&id_type, body+rp, &rp);
+    assert_int_equal(id_type, SSS_ID_TYPE_UID);
+
+    /* Sequence of null terminated strings */
+    s = (char *) body+rp;
+    assert_string_equal(s, SYSDB_SID_STR);
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, "S-1-2-3-4");
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, ORIGINALAD_PREFIX SYSDB_NAME);
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, "orig_name");
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, ORIGINALAD_PREFIX SYSDB_UIDNUM);
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, "1234");
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, "phone");
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, "+12-34 56 78");
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, "mobile");
+    rp += strlen(s) + 1;
+    assert_true(rp < blen);
+
+    s = (char *) body+rp;
+    assert_string_equal(s, "+98-76 54 32");
+    rp += strlen(s) + 1;
+    assert_int_equal(rp, blen);
+
+    return EOK;
+}
+
+void test_nss_getorigbyname_extra_attrs(void **state)
+{
+    errno_t ret;
+    struct sysdb_attrs *attrs;
+
+    attrs = sysdb_new_attrs(nss_test_ctx);
+    assert_non_null(attrs);
+
+    ret = sysdb_attrs_add_string(attrs, SYSDB_SID_STR, "S-1-2-3-4");
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_attrs_add_string(attrs, ORIGINALAD_PREFIX SYSDB_NAME,
+                                 "orig_name");
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_attrs_add_uint32(attrs, ORIGINALAD_PREFIX SYSDB_UIDNUM, 1234);
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_attrs_add_string(attrs, "phone", "+12-34 56 78");
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_attrs_add_string(attrs, "mobile", "+98-76 54 32");
+    assert_int_equal(ret, EOK);
+
+    ret = sysdb_attrs_add_string(attrs, "not_extra", "abc");
+    assert_int_equal(ret, EOK);
+
+    /* Prime the cache with a valid user */
+    ret = sysdb_add_user(nss_test_ctx->tctx->dom,
+                         "testuserorigextra", 2345, 6789,
+                         "test user orig extra",
+                         "/home/testuserorigextra", "/bin/sh", NULL,
+                         attrs, 300, 0);
+    assert_int_equal(ret, EOK);
+
+    mock_input_user_or_group("testuserorigextra");
+    will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETORIGBYNAME);
+    will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
+
+    /* Query for that user, call a callback when command finishes */
+    set_cmd_cb(test_nss_getorigbyname_extra_check);
+    ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETORIGBYNAME,
+                          nss_test_ctx->nss_cmds);
+    assert_int_equal(ret, EOK);
+
+    /* Wait until the test finishes with EOK */
+    ret = test_ev_loop(nss_test_ctx->tctx);
+    assert_int_equal(ret, EOK);
+}
+
 void nss_test_setup(void **state)
 {
     struct sss_test_conf_param params[] = {
@@ -1848,6 +1965,17 @@ void nss_fqdn_test_setup(void **state)
     test_nss_setup(params, state);
 }
 
+void nss_test_setup_extra_attr(void **state)
+{
+    struct sss_test_conf_param params[] = {
+        { "enumerate", "false" },
+        { "ldap_user_extra_attrs", "phone:telephoneNumber, mobile" },
+        { NULL, NULL },             /* Sentinel */
+    };
+
+    test_nss_setup(params, state);
+}
+
 void nss_subdom_test_setup(void **state)
 {
     const char *const testdom[4] = { TEST_SUBDOM_NAME, "TEST.SUB", "test", "S-3" };
@@ -1963,6 +2091,8 @@ int main(int argc, const char *argv[])
                                  nss_test_setup, nss_test_teardown),
         unit_test_setup_teardown(test_nss_getorigbyname,
                                  nss_test_setup, nss_test_teardown),
+        unit_test_setup_teardown(test_nss_getorigbyname_extra_attrs,
+                                 nss_test_setup_extra_attr, nss_test_teardown),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */
-- 
1.8.3.1



More information about the sssd-devel mailing list