[SSSD] [PATCH 1/3] Refactor the way subdomain accounts are saved

Simo Sorce simo at redhat.com
Fri Nov 16 18:34:04 UTC 2012


The original sysdb code had a strong assumption that only users from one
domain are saved in the databse, with the subdomain feature, we have
changed reality, but have not adjusted all the code arund the sysdb calls
to not rely on the original assumption.

One of the side effects of this incongrunece is that currently group
memberships do not return fully qualified names for subdomain users as they
should.

In oreder to fix this and other potential issues surrounding the violation
of the original assumption, we need to fully qualify subdomain user names.
By savin them fully qualified we do not risk aliasing local users and have
group memberhips or other name based matching code mistake a domain user
with subdomain usr or vice versa.
---
 src/db/sysdb.h                   |    9 ++++++
 src/db/sysdb_search.c            |    4 ++-
 src/db/sysdb_subdomains.c        |   41 ++++++++++++++++++++++++++++
 src/providers/data_provider_be.c |   11 +++++++
 src/providers/ipa/ipa_s2n_exop.c |   54 +++++++++++++++++++++++++++++++++++---
 src/responder/nss/nsssrv_cmd.c   |   10 +++++--
 src/responder/pac/pacsrv_cmd.c   |   15 +++++++++-
 src/responder/pac/pacsrv_utils.c |   52 ++++++++++++++++++++----------------
 src/responder/pam/pamsrv_cmd.c   |    5 +++-
 src/util/domain_info_utils.c     |    2 +-
 10 files changed, 168 insertions(+), 35 deletions(-)

diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index 39e0f9f5245c67f3798055823e98c2db0fbdebba..7bbb5ba59dd89406b21184914167708489996714 100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -422,6 +422,15 @@ errno_t sysdb_store_domgroup(struct sss_domain_info *domain,
 errno_t sysdb_delete_domgroup(struct sss_domain_info *domain,
                               const char *name, gid_t gid);
 
+int sysdb_subdom_getpwnam(TALLOC_CTX *mem_ctx,
+                          struct sysdb_ctx *sysdb,
+                          const char *name,
+                          struct ldb_result **_res);
+int sysdb_subdom_getgrnam(TALLOC_CTX *mem_ctx,
+                          struct sysdb_ctx *sysdb,
+                          const char *name,
+                          struct ldb_result **_res);
+
 errno_t sysdb_get_ranges(TALLOC_CTX *mem_ctx, struct sysdb_ctx *sysdb,
                              size_t *range_count,
                              struct range_info ***range_list);
diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c
index 1ab94770003a205485488c879c9deba856183291..49f628bfd3d15aba77b169b163c6ba15a7813744 100644
--- a/src/db/sysdb_search.c
+++ b/src/db/sysdb_search.c
@@ -365,7 +365,9 @@ int sysdb_initgroups(TALLOC_CTX *mem_ctx,
         return ENOMEM;
     }
 
-    ret = sysdb_getpwnam(tmp_ctx, sysdb, name, &res);
+    /* if this is a subdomain we need to search for the fully qualified
+     * name in the database */
+    ret = sysdb_subdom_getpwnam(tmp_ctx, sysdb, name, &res);
     if (ret != EOK) {
         DEBUG(1, ("sysdb_getpwnam failed: [%d][%s]\n",
                   ret, strerror(ret)));
diff --git a/src/db/sysdb_subdomains.c b/src/db/sysdb_subdomains.c
index 2e0170f4d37f04468e9a56dbca9b8176c017c9ff..2efa05728e5bf92c4df325aff7783334856e5696 100644
--- a/src/db/sysdb_subdomains.c
+++ b/src/db/sysdb_subdomains.c
@@ -668,3 +668,44 @@ errno_t sysdb_delete_domgroup(struct sss_domain_info *domain,
 
     return sysdb_delete_group(domain->sysdb, name, gid);
 }
+
+int sysdb_subdom_getpwnam(TALLOC_CTX *mem_ctx,
+                          struct sysdb_ctx *sysdb,
+                          const char *name,
+                          struct ldb_result **res)
+{
+    const char *src_name = NULL;
+    int ret;
+
+    if (sysdb->domain->parent) {
+        src_name = talloc_asprintf(mem_ctx, sysdb->domain->names->fq_fmt,
+                                   name, sysdb->domain->name);
+        if (!src_name) return ENOMEM;
+    }
+
+    ret = sysdb_getpwnam(mem_ctx, sysdb, src_name ? src_name : name, &res);
+    talloc_zfree(src_name);
+
+    return ret;
+}
+
+int sysdb_subdom_getgrnam(TALLOC_CTX *mem_ctx,
+                          struct sysdb_ctx *sysdb,
+                          const char *name,
+                          struct ldb_result **res)
+{
+    const char *src_name = NULL;
+    int ret;
+
+    if (sysdb->domain->parent) {
+        src_name = talloc_asprintf(mem_ctx, sysdb->domain->names->fq_fmt,
+                                   name, sysdb->domain->name);
+        if (!src_name) return ENOMEM;
+    }
+
+    ret = sysdb_getgrnam(mem_ctx, sysdb, src_name ? src_name : name, &res);
+    talloc_zfree(src_name);
+
+    return ret;
+}
+
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index 253dc46fb3010b2270ef454bb97e47243a18ceed..c43f32d5186f89d952e7ddf233040abb11c016e9 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -2188,6 +2188,17 @@ int be_process_init(TALLOC_CTX *mem_ctx,
         goto fail;
     }
 
+    /* We need this for subdomains support, as they have to store fully
+     * qualified user and group names for now */
+    ret = sss_names_init(ctx->domain, cdb,
+                         ctx->domain->name, &ctx->domain->names);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_FATAL_FAILURE,
+              ("fatal error setting fully qualified name format for %s\n",
+              ctx->domain->name));
+        goto fail;
+    }
+
     ret = be_srv_init(ctx);
     if (ret != EOK) {
         DEBUG(SSSDBG_FATAL_FAILURE, ("fatal error setting up server bus\n"));
diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c
index 1a81c860901dc18a3d443266889701a8980fe031..8fc22819bf72eb194563da1bb3ded2678ac494e6 100644
--- a/src/providers/ipa/ipa_s2n_exop.c
+++ b/src/providers/ipa/ipa_s2n_exop.c
@@ -591,6 +591,9 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq)
     uint64_t timeout = 10*60*60; /* FIXME: find a better timeout ! */
     const char *homedir = NULL;
     struct sysdb_attrs *user_attrs = NULL;
+    char *name;
+    char *realm;
+    char *upn;
 
     ret = ipa_s2n_exop_recv(subreq, state, &result, &retoid, &retdata);
     talloc_zfree(subreq);
@@ -640,21 +643,64 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq)
                 goto done;
             }
 
-            ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS,
-                                         attrs->a.user.pw_name);
+            /* we always use the fully qualified name for subdomain users */
+            name = talloc_asprintf(state, state->dom->names->fq_fmt,
+                                   attrs->a.user.pw_name, state->dom->name);
+            if (!name) {
+                DEBUG(SSSDBG_OP_FAILURE, ("failed to format user name.\n"));
+                ret = ENOMEM;
+                goto done;
+            }
+
+            ret = sysdb_attrs_add_string(user_attrs, SYSDB_NAME_ALIAS, name);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n"));
+                goto done;
+            }
+
+            /* We also have to store a fake UPN here, because otherwise the
+             * krb5 child later won't be able to properly construct one as
+             * the username is fully qualified but the child doesn't have
+             * access to the regex to deconstruct it */
+            /* FIXME: The real UPN is available from the PAC, we should get
+             * it from there. */
+            realm = get_uppercase_realm(state, state->dom->name);
+            if (!realm) {
+                DEBUG(SSSDBG_OP_FAILURE, ("failed to get realm.\n"));
+                ret = ENOMEM;
+                goto done;
+            }
+            upn = talloc_asprintf(state, "%s@%s",
+                                  attrs->a.user.pw_name, realm);
+            if (!upn) {
+                DEBUG(SSSDBG_OP_FAILURE, ("failed to format UPN.\n"));
+                ret = ENOMEM;
+                goto done;
+            }
+
+            ret = sysdb_attrs_add_string(user_attrs, SYSDB_UPN, upn);
             if (ret != EOK) {
                 DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n"));
                 goto done;
             }
 
-            ret = sysdb_store_domuser(state->dom, attrs->a.user.pw_name, NULL,
+            ret = sysdb_store_domuser(state->dom, name, NULL,
                                       attrs->a.user.pw_uid,
                                       0, NULL, /* gecos */
                                       homedir, NULL,
                                       user_attrs, NULL, timeout, now);
             break;
         case RESP_GROUP:
-            ret = sysdb_store_domgroup(state->dom, attrs->a.group.gr_name,
+            /* we always use the fully qualified name for subdomain users */
+            name = talloc_asprintf(state, state->dom->names->fq_fmt,
+                                   attrs->a.group.gr_name, state->dom->name);
+            if (!name) {
+                DEBUG(SSSDBG_OP_FAILURE, ("failed to format user name,\n"));
+                ret = ENOMEM;
+                goto done;
+            }
+
+            ret = sysdb_store_domgroup(state->dom, name,
                                        attrs->a.group.gr_gid, NULL, timeout,
                                        now);
             break;
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 036e88f4c0428dd592b8067d7ec341728ef38ddb..c667a3f90f849e8b9af3806d91157a7ff37117a8 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -635,7 +635,7 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx)
         dctx->domain = dom;
 
         talloc_free(name);
-        name = sss_get_cased_name(dctx, cmdctx->name, dom->case_sensitive);
+        name = sss_get_cased_name(cmdctx, cmdctx->name, dom->case_sensitive);
         if (!name) return ENOMEM;
 
         /* verify this user has not yet been negatively cached,
@@ -666,7 +666,9 @@ static int nss_cmd_getpwnam_search(struct nss_dom_ctx *dctx)
             return EIO;
         }
 
-        ret = sysdb_getpwnam(cmdctx, sysdb, name, &dctx->res);
+        /* if this is a subdomain we need to search for the fully qualified
+         * name in the database */
+        ret = sysdb_subdom_getpwnam(cmdctx, sysdb, name, &dctx->res);
         if (ret != EOK) {
             DEBUG(1, ("Failed to make request to our cache!\n"));
             return EIO;
@@ -2201,7 +2203,9 @@ static int nss_cmd_getgrnam_search(struct nss_dom_ctx *dctx)
             return EIO;
         }
 
-        ret = sysdb_getgrnam(cmdctx, sysdb, name, &dctx->res);
+        /* if this is a subdomain we need to search for the fully qualified
+         * name in the database */
+        ret = sysdb_subdom_getgrnam(cmdctx, sysdb, name, &dctx->res);
         if (ret != EOK) {
             DEBUG(1, ("Failed to make request to our cache!\n"));
             return EIO;
diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index 5721d9262f88387e95b926ae4c3fe4d2033d15de..202765a594aab1748b9a071b98e5175c1d3876ae 100644
--- a/src/responder/pac/pacsrv_cmd.c
+++ b/src/responder/pac/pacsrv_cmd.c
@@ -53,6 +53,7 @@ struct pac_req_ctx {
     struct pac_ctx *pac_ctx;
     const char *domain_name;
     const char *user_name;
+    char *fq_name;
     struct sss_domain_info *dom;
 
     struct PAC_LOGON_INFO *logon_info;
@@ -201,6 +202,16 @@ static errno_t pac_add_user_next(struct pac_req_ctx *pr_ctx)
     struct dom_sid *my_dom_sid;
     struct local_mapping_ranges *my_range_map;
 
+    /* this is a subdomain so we need to search for the fully qualified
+     * name in the database */
+    pr_ctx->fq_name = talloc_asprintf(pr_ctx, pr_ctx->dom->names->fq_fmt,
+                                      pr_ctx->user_name, pr_ctx->dom->name);
+    if (!pr_ctx->fq_name) {
+        ret = ENOMEM;
+        DEBUG(SSSDBG_OP_FAILURE, ("talloc_sprintf failed.\n"));
+        goto done;
+    }
+
     ret = save_pac_user(pr_ctx);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, ("save_pac_user failed.\n"));
@@ -365,7 +376,7 @@ static errno_t save_pac_user(struct pac_req_ctx *pr_ctx)
         goto done;
     }
 
-    ret = sysdb_search_user_by_name(tmp_ctx, sysdb, pr_ctx->user_name, attrs,
+    ret = sysdb_search_user_by_name(tmp_ctx, sysdb, pr_ctx->fq_name, attrs,
                                     &msg);
     if (ret == EOK) {
         /* TODO: check id uid and gid are equal. */
@@ -423,7 +434,7 @@ struct tevent_req *pac_save_memberships_send(struct pac_req_ctx *pr_ctx)
     }
 
     state->gid_iter = 0;
-    state->user_dn = sysdb_user_dn(dom->sysdb, state, pr_ctx->user_name);
+    state->user_dn = sysdb_user_dn(dom->sysdb, state, pr_ctx->fq_name);
     if (state->user_dn == NULL) {
         ret = ENOMEM;
         goto done;
diff --git a/src/responder/pac/pacsrv_utils.c b/src/responder/pac/pacsrv_utils.c
index c9551c998332b053833aaa1d08e435bcfec0ec21..53113fb0da18cb235c0c049b14bd836556f647ec 100644
--- a/src/responder/pac/pacsrv_utils.c
+++ b/src/responder/pac/pacsrv_utils.c
@@ -502,6 +502,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
     struct sysdb_attrs *attrs = NULL;
     struct netr_SamBaseInfo *base_info;
     int ret;
+    char *lname;
     char *uc_realm;
     char *upn;
 
@@ -513,36 +514,41 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
 
     base_info = &logon_info->info3.base;
 
-    if (base_info->account_name.size != 0) {
-        /* To be compatible with winbind based lookups we have to use lower
-         * case names only, effectively making the domain case-insenvitive. */
-        pwd->pw_name = sss_tc_utf8_str_tolower(pwd,
-                                               base_info->account_name.string);
-        if (pwd->pw_name == NULL) {
-            DEBUG(SSSDBG_OP_FAILURE, ("sss_tc_utf8_str_tolower failed.\n"));
-            ret = ENOMEM;
-            goto done;
-        }
-    } else {
+    if (base_info->account_name.size == 0) {
         DEBUG(SSSDBG_OP_FAILURE, ("Missing account name in PAC.\n"));
         ret = EINVAL;
         goto done;
     }
-
-    if (base_info->rid > 0) {
-        ret = domsid_rid_to_uid(pac_ctx, dom->sysdb, dom->name,
-                                base_info->domain_sid,
-                                base_info->rid, &pwd->pw_uid);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, ("domsid_rid_to_uid failed.\n"));
-            goto done;
-        }
-    } else {
+    if (base_info->rid == 0) {
         DEBUG(SSSDBG_OP_FAILURE, ("Missing user RID in PAC.\n"));
         ret = EINVAL;
         goto done;
     }
 
+    /* To be compatible with winbind based lookups we have to use lower
+     * case names only, effectively making the domain case-insenvitive. */
+    lname = sss_tc_utf8_str_tolower(pwd, base_info->account_name.string);
+    if (lname == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, ("sss_tc_utf8_str_tolower failed.\n"));
+        ret = ENOMEM;
+        goto done;
+    }
+    pwd->pw_name = talloc_asprintf(pwd, dom->names->fq_fmt,
+                                   lname, dom->name);
+    if (!pwd->pw_name) {
+        DEBUG(SSSDBG_OP_FAILURE, ("talloc_sprintf failed.\n"));
+        ret = ENOMEM;
+        goto done;
+    }
+
+    ret = domsid_rid_to_uid(pac_ctx, dom->sysdb, dom->name,
+                            base_info->domain_sid,
+                            base_info->rid, &pwd->pw_uid);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("domsid_rid_to_uid failed.\n"));
+        goto done;
+    }
+
     pwd->pw_gid = 0; /* We use MPGs for sub-domains */
 
     if (base_info->full_name.size != 0) {
@@ -559,7 +565,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
 
     if (dom->subdomain_homedir) {
         pwd->pw_dir = expand_homedir_template(pwd, dom->subdomain_homedir,
-                                              pwd->pw_name, pwd->pw_uid,
+                                              lname, pwd->pw_uid,
                                               dom->name);
         if (pwd->pw_dir == NULL) {
             ret = ENOMEM;
@@ -583,7 +589,7 @@ errno_t get_pwd_from_pac(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    upn = talloc_asprintf(mem_ctx, "%s@%s", pwd->pw_name, uc_realm);
+    upn = talloc_asprintf(mem_ctx, "%s@%s", lname, uc_realm);
     talloc_free(uc_realm);
     if (upn == NULL) {
         DEBUG(SSSDBG_OP_FAILURE, ("talloc_asprintf failed.\n"));
diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
index 1702a0e91413387e457dc2352cc9d4e693119212..4269642206cc0295c0046de4e59a3ad8f1044d1a 100644
--- a/src/responder/pam/pamsrv_cmd.c
+++ b/src/responder/pam/pamsrv_cmd.c
@@ -1217,7 +1217,10 @@ static int pam_check_user_search(struct pam_auth_req *preq)
             preq->pd->pam_status = PAM_SYSTEM_ERR;
             return EFAULT;
         }
-        ret = sysdb_getpwnam(preq, sysdb, name, &preq->res);
+
+        /* if this is a subdomain we need to search for the fully qualified
+         * name in the database */
+        ret = sysdb_subdom_getpwnam(preq, sysdb, name, &preq->res);
         if (ret != EOK) {
             DEBUG(1, ("Failed to make request to our cache!\n"));
             return EIO;
diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c
index 6ee3545520f52171e04bdc096d84fc67dbf82f2e..a6aa5c73327456d2323898af786cb43e0b69d7da 100644
--- a/src/util/domain_info_utils.c
+++ b/src/util/domain_info_utils.c
@@ -73,7 +73,7 @@ struct sss_domain_info *new_subdomain(TALLOC_CTX *mem_ctx,
     }
 
     dom->enumerate = false;
-    dom->fqnames = true;
+    dom->fqnames = false;
     /* FIXME: get ranges from the server */
     dom->id_min = 0;
     dom->id_max = 0xffffffff;
-- 
1.7.1




More information about the sssd-devel mailing list