Refactor the way subdomain accounts are saved

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.

2 files changed, 42 insertions, 25 deletions

diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c
index 5721d926..202765a5 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 c9551c99..53113fb0 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"));