IPA: Use internal fqname format instead of parsing NSS names

Parsing the extdom plugin output is an "input" operation from the point
of the IPA provider, so we need to parse the name and conversely,
internally use only the qualified name.

Reviewed-by: Sumit Bose <sbose@redhat.com>

3 files changed, 147 insertions, 67 deletions

diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c
index 828dbf3dc..b28cc415b 100644
--- a/src/providers/ipa/ipa_s2n_exop.c
+++ b/src/providers/ipa/ipa_s2n_exop.c
@@ -543,16 +543,22 @@ static errno_t get_extra_attrs(BerElement *ber, struct resp_attrs *resp_attrs)
     return EOK;
 }
 
-static errno_t add_v1_user_data(BerElement *ber, struct resp_attrs *attrs)
+static errno_t add_v1_user_data(struct sss_domain_info *dom,
+                                BerElement *ber,
+                                struct resp_attrs *attrs)
 {
     ber_tag_t tag;
     ber_len_t ber_len;
     int ret;
     char *gecos = NULL;
     char *homedir = NULL;
+    char *name = NULL;
+    char *domain = NULL;
     char *shell = NULL;
     char **list = NULL;
-    size_t c;
+    size_t c, gc;
+    struct sss_domain_info *parent_domain;
+    struct sss_domain_info *obj_domain;
 
     tag = ber_scanf(ber, "aaa", &gecos, &homedir, &shell);
     if (tag == LBER_ERROR) {
@@ -612,14 +618,35 @@ static errno_t add_v1_user_data(BerElement *ber, struct resp_attrs *attrs)
             goto done;
         }
 
-        for (c = 0; c < attrs->ngroups; c++) {
-            attrs->groups[c] = talloc_strdup(attrs->groups,
-                                             list[c]);
-            if (attrs->groups[c] == NULL) {
+        parent_domain = get_domains_head(dom);
+
+        for (c = 0, gc = 0; c < attrs->ngroups; c++) {
+            ret = sss_parse_name(attrs, dom->names, list[c],
+                                 &domain, &name);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_OP_FAILURE,
+                        "Cannot parse member %s\n", list[c]);
+                continue;
+            }
+
+            if (domain != NULL) {
+                obj_domain = find_domain_by_name(parent_domain, domain, true);
+                if (obj_domain == NULL) {
+                    DEBUG(SSSDBG_OP_FAILURE, "find_domain_by_name failed.\n");
+                    return ENOMEM;
+                }
+            } else {
+                obj_domain = parent_domain;
+            }
+
+            attrs->groups[gc] = sss_create_internal_fqname(attrs->groups,
+                                                           name, obj_domain->name);
+            if (attrs->groups[gc] == NULL) {
                 DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
                 ret = ENOMEM;
                 goto done;
             }
+            gc++;
         }
     }
 
@@ -645,13 +672,17 @@ done:
     return ret;
 }
 
-static errno_t add_v1_group_data(BerElement *ber, struct resp_attrs *attrs)
+static errno_t add_v1_group_data(BerElement *ber,
+                                 struct sss_domain_info *dom,
+                                 struct resp_attrs *attrs)
 {
     ber_tag_t tag;
     ber_len_t ber_len;
     int ret;
     char **list = NULL;
-    size_t c;
+    size_t c, mc;
+    char *name = NULL;
+    char *domain = NULL;
 
     tag = ber_scanf(ber, "{v}", &list);
     if (tag == LBER_ERROR) {
@@ -673,15 +704,28 @@ static errno_t add_v1_group_data(BerElement *ber, struct resp_attrs *attrs)
                 goto done;
             }
 
-            for (c = 0; c < attrs->ngroups; c++) {
-                attrs->a.group.gr_mem[c] =
-                                    talloc_strdup(attrs->a.group.gr_mem,
-                                                  list[c]);
-                if (attrs->a.group.gr_mem[c] == NULL) {
+            for (c = 0, mc=0; c < attrs->ngroups; c++) {
+                ret = sss_parse_name(attrs, dom->names, list[c],
+                                     &domain, &name);
+                if (ret != EOK) {
+                    DEBUG(SSSDBG_OP_FAILURE,
+                          "Cannot parse member %s\n", list[c]);
+                    continue;
+                }
+
+                if (domain == NULL) {
+                    domain = dom->name;
+                }
+
+                attrs->a.group.gr_mem[mc] =
+                            sss_create_internal_fqname(attrs->a.group.gr_mem,
+                                                       name, domain);
+                if (attrs->a.group.gr_mem[mc] == NULL) {
                     DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
                     ret = ENOMEM;
                     goto done;
                 }
+                mc++;
             }
         }
     } else {
@@ -720,6 +764,7 @@ static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom,
                                     bool update_initgr_timeout);
 
 static errno_t s2n_response_to_attrs(TALLOC_CTX *mem_ctx,
+                                     struct sss_domain_info *dom,
                                      char *retoid,
                                      struct berval *retdata,
                                      struct resp_attrs **resp_attrs)
@@ -730,6 +775,7 @@ static errno_t s2n_response_to_attrs(TALLOC_CTX *mem_ctx,
     enum response_types type;
     char *domain_name = NULL;
     char *name = NULL;
+    char *lc_name = NULL;
     uid_t uid;
     gid_t gid;
     struct resp_attrs *attrs = NULL;
@@ -787,7 +833,16 @@ static errno_t s2n_response_to_attrs(TALLOC_CTX *mem_ctx,
              * bug in some version of winbind which might lead to upper case
              * letters in the name. To be on the safe side we explicitly
              * lowercase the name. */
-            attrs->a.user.pw_name = sss_tc_utf8_str_tolower(attrs, name);
+            lc_name = sss_tc_utf8_str_tolower(attrs, name);
+            if (lc_name == NULL) {
+                ret = ENOMEM;
+                goto done;
+            }
+
+            attrs->a.user.pw_name = sss_create_internal_fqname(attrs,
+                                                               lc_name,
+                                                               domain_name);
+            talloc_free(lc_name);
             if (attrs->a.user.pw_name == NULL) {
                 DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
                 ret = ENOMEM;
@@ -798,7 +853,7 @@ static errno_t s2n_response_to_attrs(TALLOC_CTX *mem_ctx,
             attrs->a.user.pw_gid = gid;
 
             if (is_v1 && type == RESP_USER_GROUPLIST) {
-                ret = add_v1_user_data(ber, attrs);
+                ret = add_v1_user_data(dom, ber, attrs);
                 if (ret != EOK) {
                     DEBUG(SSSDBG_OP_FAILURE, "add_v1_user_data failed.\n");
                     goto done;
@@ -827,7 +882,16 @@ static errno_t s2n_response_to_attrs(TALLOC_CTX *mem_ctx,
              * bug in some version of winbind which might lead to upper case
              * letters in the name. To be on the safe side we explicitly
              * lowercase the name. */
-            attrs->a.group.gr_name = sss_tc_utf8_str_tolower(attrs, name);
+            lc_name = sss_tc_utf8_str_tolower(attrs, name);
+            if (lc_name == NULL) {
+                ret = ENOMEM;
+                goto done;
+            }
+
+            attrs->a.group.gr_name = sss_create_internal_fqname(attrs,
+                                                                lc_name,
+                                                                domain_name);
+            talloc_free(lc_name);
             if (attrs->a.group.gr_name == NULL) {
                 DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
                 ret = ENOMEM;
@@ -837,7 +901,7 @@ static errno_t s2n_response_to_attrs(TALLOC_CTX *mem_ctx,
             attrs->a.group.gr_gid = gid;
 
             if (is_v1 && type == RESP_GROUP_MEMBERS) {
-                ret = add_v1_group_data(ber, attrs);
+                ret = add_v1_group_data(ber, dom, attrs);
                 if (ret != EOK) {
                     DEBUG(SSSDBG_OP_FAILURE, "add_v1_group_data failed.\n");
                     goto done;
@@ -1011,8 +1075,7 @@ static errno_t ipa_s2n_get_list_step(struct tevent_req *req)
     switch (state->req_input.type) {
     case REQ_INP_NAME:
 
-        ret = sss_parse_name(state, parent_domain->names,
-                             state->list[state->list_idx],
+        ret = sss_parse_name(state, state->dom->names, state->list[state->list_idx],
                              &domain_name, &short_name);
         if (ret != EOK) {
             DEBUG(SSSDBG_CRIT_FAILURE, "Unable to parse name '%s' [%d]: %s\n",
@@ -1108,7 +1171,8 @@ static void ipa_s2n_get_list_next(struct tevent_req *subreq)
     }
 
     talloc_zfree(state->attrs);
-    ret = s2n_response_to_attrs(state, retoid, retdata, &state->attrs);
+    ret = s2n_response_to_attrs(state, state->dom, retoid, retdata,
+                                &state->attrs);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "s2n_response_to_attrs failed.\n");
         goto fail;
@@ -1558,7 +1622,8 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq)
     switch (state->request_type) {
     case REQ_FULL_WITH_MEMBERS:
     case REQ_FULL:
-        ret = s2n_response_to_attrs(state, retoid, retdata, &attrs);
+        ret = s2n_response_to_attrs(state, state->dom, retoid, retdata,
+                                    &attrs);
         if (ret != EOK) {
             DEBUG(SSSDBG_OP_FAILURE, "s2n_response_to_attrs failed.\n");
             goto done;
@@ -1664,7 +1729,7 @@ static void ipa_s2n_get_user_done(struct tevent_req *subreq)
         return;
 
     case REQ_SIMPLE:
-        ret = s2n_response_to_attrs(state, retoid, retdata,
+        ret = s2n_response_to_attrs(state, state->dom, retoid, retdata,
                                     &state->simple_attrs);
         if (ret != EOK) {
             DEBUG(SSSDBG_OP_FAILURE, "s2n_response_to_attrs failed.\n");
@@ -1849,7 +1914,6 @@ static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom,
     bool in_transaction = false;
     int tret;
     struct sysdb_attrs *gid_override_attrs = NULL;
-    char ** exop_grouplist;
     struct ldb_message *msg;
     struct ldb_message_element *el = NULL;
     const char *missing[] = {NULL, NULL};
@@ -1956,14 +2020,7 @@ static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom,
             }
 
             if (name == NULL) {
-                /* we always use the fully qualified name for subdomain users */
-                name = sss_tc_fqname(tmp_ctx, dom->names, dom,
-                                     attrs->a.user.pw_name);
-                if (!name) {
-                    DEBUG(SSSDBG_OP_FAILURE, "failed to format user name.\n");
-                    ret = ENOMEM;
-                    goto done;
-                }
+                name = attrs->a.user.pw_name;
             }
 
             ret = sysdb_attrs_add_lc_name_alias_safe(attrs->sysdb_attrs, name);
@@ -2162,17 +2219,7 @@ static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom,
                     goto done;
                 }
 
-                /* names returned by extdom exop will be all lower case, since
-                 * we handle domain names case sensitve in the cache we have
-                 * to make sure we use the right case. */
-                ret = fix_domain_in_name_list(tmp_ctx, dom, attrs->groups,
-                                              &exop_grouplist);
-                if (ret != EOK) {
-                    DEBUG(SSSDBG_OP_FAILURE, "fix_domain_name failed.\n");
-                    goto done;
-                }
-
-                ret = diff_string_lists(tmp_ctx, exop_grouplist,
+                ret = diff_string_lists(tmp_ctx, attrs->groups,
                                         sysdb_grouplist, &add_groups,
                                         &del_groups, NULL);
                 if (ret != EOK) {
@@ -2220,15 +2267,6 @@ static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom,
                 name = attrs->a.group.gr_name;
             }
 
-            if (IS_SUBDOMAIN(dom)) {
-                /* we always use the fully qualified name for subdomain users */
-                name = sss_get_domain_name(tmp_ctx, name, dom);
-                if (!name) {
-                    DEBUG(SSSDBG_OP_FAILURE, "failed to format user name,\n");
-                    ret = ENOMEM;
-                    goto done;
-                }
-            }
             DEBUG(SSSDBG_TRACE_FUNC, "Processing group %s\n", name);
 
             ret = sysdb_attrs_add_lc_name_alias_safe(attrs->sysdb_attrs, name);
diff --git a/src/providers/ipa/ipa_subdomains_id.c b/src/providers/ipa/ipa_subdomains_id.c
index 7e53dd8dd..3f083d3cd 100644
--- a/src/providers/ipa/ipa_subdomains_id.c
+++ b/src/providers/ipa/ipa_subdomains_id.c
@@ -440,6 +440,7 @@ static void ipa_get_subdom_acct_connected(struct tevent_req *subreq)
     int ret;
     char *endptr;
     struct req_input *req_input;
+    char *shortname;
 
     ret = sdap_id_op_connect_recv(subreq, &dp_error);
     talloc_zfree(subreq);
@@ -498,7 +499,10 @@ static void ipa_get_subdom_acct_connected(struct tevent_req *subreq)
     switch (state->filter_type) {
         case BE_FILTER_NAME:
             req_input->type = REQ_INP_NAME;
-            req_input->inp.name = talloc_strdup(req_input, state->filter);
+            /* The extdom plugin expects the shortname and domain separately */
+            ret = sss_parse_internal_fqname(req_input, state->filter,
+                                            &shortname, NULL);
+            req_input->inp.name = talloc_steal(req_input, shortname);
             if (req_input->inp.name == NULL) {
                 DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
                 tevent_req_error(req, ENOMEM);
@@ -949,7 +953,6 @@ errno_t get_object_from_cache(TALLOC_CTX *mem_ctx,
                             SYSDB_GHOST,
                             SYSDB_HOMEDIR,
                             NULL };
-    char *name;
 
     if (ar->filter_type == BE_FILTER_SECID) {
         ret = sysdb_search_object_by_sid(mem_ctx, dom, ar->filter_value, attrs,
@@ -1022,24 +1025,19 @@ errno_t get_object_from_cache(TALLOC_CTX *mem_ctx,
             goto done;
         }
     } else if (ar->filter_type == BE_FILTER_NAME) {
-        name = sss_get_domain_name(mem_ctx, ar->filter_value, dom);
-        if (name == NULL) {
-            DEBUG(SSSDBG_OP_FAILURE, "sss_get_domain_name failed\n");
-            ret = ENOMEM;
-            goto done;
-        }
-
         switch (ar->entry_type & BE_REQ_TYPE_MASK) {
         case BE_REQ_GROUP:
-            ret = sysdb_search_group_by_name(mem_ctx, dom, name, attrs, &msg);
+            ret = sysdb_search_group_by_name(mem_ctx, dom, ar->filter_value,
+                                             attrs, &msg);
             break;
         case BE_REQ_INITGROUPS:
         case BE_REQ_USER:
         case BE_REQ_USER_AND_GROUP:
-            ret = sysdb_search_user_by_name(mem_ctx, dom, name, attrs, &msg);
+            ret = sysdb_search_user_by_name(mem_ctx, dom, ar->filter_value,
+                                            attrs, &msg);
             if (ret == ENOENT && (ar->entry_type & BE_REQ_TYPE_MASK)
                                                      == BE_REQ_USER_AND_GROUP) {
-                ret = sysdb_search_group_by_name(mem_ctx, dom, name,
+                ret = sysdb_search_group_by_name(mem_ctx, dom, ar->filter_value,
                                                  attrs, &msg);
             }
             break;
diff --git a/src/providers/ipa/ipa_views.c b/src/providers/ipa/ipa_views.c
index 76528a60c..6aeb443c9 100644
--- a/src/providers/ipa/ipa_views.c
+++ b/src/providers/ipa/ipa_views.c
@@ -38,23 +38,30 @@ static errno_t be_acct_req_to_override_filter(TALLOC_CTX *mem_ctx,
     char *endptr;
     char *cert_filter;
     int ret;
+    char *shortname;
 
     switch (ar->filter_type) {
     case BE_FILTER_NAME:
+        ret = sss_parse_internal_fqname(mem_ctx, ar->filter_value,
+                                        &shortname, NULL);
+        if (ret != EOK) {
+            return ret;
+        }
+
         switch ((ar->entry_type & BE_REQ_TYPE_MASK)) {
         case BE_REQ_USER:
         case BE_REQ_INITGROUPS:
             filter = talloc_asprintf(mem_ctx, "(&(objectClass=%s)(%s=%s))",
                          ipa_opts->override_map[IPA_OC_OVERRIDE_USER].name,
                          ipa_opts->override_map[IPA_AT_OVERRIDE_USER_NAME].name,
-                         ar->filter_value);
+                         shortname);
             break;
 
          case BE_REQ_GROUP:
             filter = talloc_asprintf(mem_ctx, "(&(objectClass=%s)(%s=%s))",
                         ipa_opts->override_map[IPA_OC_OVERRIDE_GROUP].name,
                         ipa_opts->override_map[IPA_AT_OVERRIDE_GROUP_NAME].name,
-                        ar->filter_value);
+                        shortname);
             break;
 
          case BE_REQ_USER_AND_GROUP:
@@ -63,13 +70,15 @@ static errno_t be_acct_req_to_override_filter(TALLOC_CTX *mem_ctx,
                         ipa_opts->override_map[IPA_AT_OVERRIDE_USER_NAME].name,
                         ar->filter_value,
                         ipa_opts->override_map[IPA_AT_OVERRIDE_GROUP_NAME].name,
-                        ar->filter_value);
+                        shortname);
             break;
         default:
             DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected entry type [%d] for name filter.\n",
                                        ar->entry_type);
+            talloc_free(shortname);
             return EINVAL;
         }
+        talloc_free(shortname);
         break;
 
     case BE_FILTER_IDNUM:
@@ -266,6 +275,8 @@ struct ipa_get_ad_override_state {
 };
 
 static void ipa_get_ad_override_connect_done(struct tevent_req *subreq);
+static errno_t ipa_get_ad_override_qualify_name(
+                                struct ipa_get_ad_override_state *state);
 static void ipa_get_ad_override_done(struct tevent_req *subreq);
 
 struct tevent_req *ipa_get_ad_override_send(TALLOC_CTX *mem_ctx,
@@ -448,8 +459,14 @@ static void ipa_get_ad_override_done(struct tevent_req *subreq)
 
     DEBUG(SSSDBG_TRACE_ALL, "Found override for object with filter [%s].\n",
                             state->filter);
-
     state->override_attrs = reply[0];
+
+    ret = ipa_get_ad_override_qualify_name(state);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, "Cannot qualify object name\n");
+        goto fail;
+    }
+
     state->dp_error = DP_ERR_OK;
     tevent_req_done(req);
     return;
@@ -460,6 +477,33 @@ fail:
     return;
 }
 
+static errno_t ipa_get_ad_override_qualify_name(
+                                struct ipa_get_ad_override_state *state)
+{
+    int ret;
+    struct ldb_message_element *name;
+    char *fqdn;
+
+    ret = sysdb_attrs_get_el_ext(state->override_attrs, SYSDB_NAME,
+                                 false, &name);
+    if (ret == ENOENT) {
+        return EOK; /* Does not override name */
+    } else if (ret != EOK && ret != ENOENT) {
+        return ret;
+    }
+
+    fqdn = sss_create_internal_fqname(name->values,
+                                      (const char *) name->values[0].data,
+                                      state->ar->domain);
+    if (fqdn == NULL) {
+        return ENOMEM;
+    }
+
+    name->values[0].data = (uint8_t *) fqdn;
+    name->values[0].length = strlen(fqdn);
+    return EOK;
+}
+
 errno_t ipa_get_ad_override_recv(struct tevent_req *req, int *dp_error_out,
                                  TALLOC_CTX *mem_ctx,
                                  struct sysdb_attrs **override_attrs)