Fix confdb issues.

Avoid uninitialized memory messages in valgrind (in _btreemap_get_keys).
Do not free memory we just stored in the btree (in confdb_get_domains_list).
Streamline confdb_get_domains() and remove extra calls when we already have
all the information handy.
Do not store basedn in domain info, the base dn is always calculated out of
the domain name.
Remove the "provider" attribute, it was really used only to distinguish between
LOCAL and other domains, directly check for LOCAL as a special case instead.

5 files changed, 67 insertions, 110 deletions

diff --git a/server/confdb/confdb.c b/server/confdb/confdb.c
index ca335c58..e93a4f45 100644
--- a/server/confdb/confdb.c
+++ b/server/confdb/confdb.c
@@ -625,14 +625,13 @@ int confdb_get_domains(struct confdb_ctx *cdb,
     TALLOC_CTX *tmp_ctx;
     struct ldb_dn *dn;
     struct ldb_result *res;
-    struct ldb_message_element *el;
-    int ret, i;
-    const char *attrs[] = {CONFDB_DOMAIN_ATTR, NULL};
-    char *path;
     struct btreemap *domain_map;
     struct sss_domain_info *domain;
+    const char *tmp;
+    int ret, i;
 
     tmp_ctx = talloc_new(mem_ctx);
+    if (!tmp_ctx) return ENOMEM;
 
     dn = ldb_dn_new(tmp_ctx,cdb->ldb, CONFDB_DOMAIN_BASEDN);
     if (!dn) {
@@ -641,128 +640,90 @@ int confdb_get_domains(struct confdb_ctx *cdb,
     }
 
     ret = ldb_search(cdb->ldb, tmp_ctx, &res, dn,
-                     LDB_SCOPE_ONELEVEL, attrs, NULL);
+                     LDB_SCOPE_ONELEVEL, NULL, NULL);
     if (ret != LDB_SUCCESS) {
         ret = EIO;
         goto done;
     }
 
     domain_map = NULL;
-    i = 0;
-    while (i < res->count) {
+    for(i = 0; i < res->count; i++) {
         /* allocate the domain on the tmp_ctx. It will be stolen
          * by btreemap_set_value
          */
-        domain = talloc_zero(tmp_ctx, struct sss_domain_info);
-        el = ldb_msg_find_element(res->msgs[i], CONFDB_DOMAIN_ATTR);
-        if (el && el->num_values > 0) {
-            if (el->num_values > 1) {
-                DEBUG(0, ("Error, domains should not have multivalued cn\n"));
-                ret = EINVAL;
-                goto done;
-            }
+        domain = talloc_zero(mem_ctx, struct sss_domain_info);
 
-            /* should always be strings so this should be safe */
-            struct ldb_val v = el->values[0];
-            domain->name = talloc_strndup(domain, (char *)v.data, v.length);
-            if (!domain->name) {
-                ret = ENOMEM;
-                talloc_free(domain_map);
-                goto done;
-            }
-
-            /* Create the confdb path for this domain */
-            path = talloc_asprintf(tmp_ctx, "config/domains/%s", domain->name);
-            if (!path) {
-                ret = ENOMEM;
-                goto done;
-            }
-
-            /* Build the BaseDN for this domain */
-            domain->basedn = talloc_asprintf(domain, SYSDB_DOM_BASE, domain->name);
-            if (domain->basedn == NULL) {
-                ret = ENOMEM;
-                goto done;
-            }
-            DEBUG(3, ("BaseDN: %s\n", domain->basedn));
-
-            /* Determine if this domain can be enumerated */
-            ret = confdb_get_int(cdb, domain, path,
-                                 "enumerate", false, &(domain->enumerate));
-            if (ret != EOK) {
-                DEBUG(0, ("Failed to fetch enumerate for [%s]!\n", domain->name));
-                goto done;
-            }
+        tmp = ldb_msg_find_attr_as_string(res->msgs[i], "cn", NULL);
+        if (!tmp) {
+            DEBUG(0, ("Invalid configuration entry, fatal error!\n"));
+            ret = EINVAL;
+            goto done;
+        }
+        domain->name = talloc_strdup(domain, tmp);
+        if (!domain->name) {
+            ret = ENOMEM;
+            goto done;
+        }
 
-            /* Determine if this is a legacy domain */
-            ret = confdb_get_bool(cdb, domain, path,
-                                  "legacy", false, &(domain->legacy));
-            if (ret != EOK) {
-                DEBUG(0, ("Failed to fetch legacy for [%s]!\n", domain->name));
-                goto done;
-            }
+        domain->timeout = ldb_msg_find_attr_as_int(res->msgs[i],
+                                                   "timeout", 0);
 
-            /* Determine if this domain is managed by a backend provider */
-            ret = confdb_get_string(cdb, domain, path, "provider",
-                                    NULL, &domain->provider);
-            if (ret != EOK) {
-                DEBUG(0, ("Failed to fetch provider for [%s]!\n", domain->name));
-                goto done;
-            }
-            if (domain->provider) domain->has_provider = true;
+        /* Determine if this domain can be enumerated */
+        domain->enumerate = ldb_msg_find_attr_as_int(res->msgs[i],
+                                                     "enumerate", 0);
+        if (domain->enumerate == 0) {
+            DEBUG(0, ("No enumeration for [%s]!\n", domain->name));
+        }
 
-            ret = btreemap_set_value(mem_ctx, &domain_map,
-                                     domain->name, domain,
-                                     _domain_comparator);
-            if (ret != EOK) {
-                DEBUG(1, ("Failed to store domain info for [%s]!\n", domain->name));
-                goto done;
-            }
+        /* Determine if this is a legacy domain */
+        if (ldb_msg_find_attr_as_bool(res->msgs[i], "legacy", 0)) {
+            domain->legacy = true;
+        }
 
-            talloc_free(path);
+        ret = btreemap_set_value(mem_ctx, &domain_map,
+                                 domain->name, domain,
+                                 _domain_comparator);
+        if (ret != EOK) {
+            DEBUG(1, ("Failed to store domain info for [%s]!\n", domain->name));
+            talloc_free(domain_map);
+            goto done;
         }
-        i++;
+    }
+
+    if (domain_map == NULL) {
+        DEBUG(0, ("No domains configured, fatal error!\n"));
+        ret = EINVAL;
     }
 
     *domains = domain_map;
 
 done:
     talloc_free(tmp_ctx);
-    if (ret != EOK) {
-        talloc_free(domain_map);
-        *domains = NULL;
-    }
     return ret;
 }
 
 int confdb_get_domains_list(struct confdb_ctx *cdb,
                             TALLOC_CTX *mem_ctx,
+                            struct btreemap **domain_map,
                             const char ***domain_names,
                             int *count)
 {
+    const void **names;
+    int num;
     int ret;
-    struct btreemap *domain_map;
-    TALLOC_CTX *tmp_ctx;
 
-    tmp_ctx = talloc_new(mem_ctx);
-    if(tmp_ctx == NULL) {
-        return ENOMEM;
+    if (*domain_map == NULL) {
+        ret = confdb_get_domains(cdb, mem_ctx, domain_map);
+        if (ret != EOK) return ret;
     }
 
-    ret = confdb_get_domains(cdb, tmp_ctx, &domain_map);
-    if (ret != EOK || domain_map == NULL) {
-        DEBUG(0, ("Error, no domains were configured\n"));
-        *domain_names = NULL;
-        count = 0;
-        goto done;
-    }
-
-    ret = btreemap_get_keys(mem_ctx, domain_map, (const void ***)domain_names, count);
+    ret = btreemap_get_keys(mem_ctx, *domain_map, &names, &num);
     if (ret != EOK) {
         DEBUG(0, ("Couldn't get domain list\n"));
+        return ret;
     }
 
-done:
-    talloc_free(tmp_ctx);
-    return ret;
+    *domain_names = (const char **)names;
+    *count = num;
+    return EOK;
 }
diff --git a/server/confdb/confdb.h b/server/confdb/confdb.h
index 3bd0d038..de679035 100644
--- a/server/confdb/confdb.h
+++ b/server/confdb/confdb.h
@@ -31,10 +31,8 @@
 
 struct sss_domain_info {
     char *name;
-    char *basedn;
+    int timeout;
     int enumerate;
-    bool has_provider;
-    char *provider;
     bool legacy;
 };
 
@@ -76,6 +74,7 @@ int confdb_get_domains(struct confdb_ctx *cdb,
 
 int confdb_get_domains_list(struct confdb_ctx *cdb,
                             TALLOC_CTX *mem_ctx,
+                            struct btreemap **domain_map,
                             const char ***domain_names,
                             int *count);
 
diff --git a/server/monitor/monitor.c b/server/monitor/monitor.c
index a07178f8..fb5b9b91 100644
--- a/server/monitor/monitor.c
+++ b/server/monitor/monitor.c
@@ -63,6 +63,7 @@ struct mt_svc {
 struct mt_ctx {
     struct tevent_context *ev;
     struct confdb_ctx *cdb;
+    struct btreemap *dom_map;
     char **services;
     struct mt_svc *svc_list;
     struct sbus_srv_ctx *sbus_srv;
@@ -364,7 +365,7 @@ int monitor_process_init(TALLOC_CTX *mem_ctx,
 {
     struct mt_ctx *ctx;
     struct mt_svc *svc;
-    char **doms;
+    const char **doms;
     int dom_count;
     char *path;
     int ret, i;
@@ -435,7 +436,8 @@ int monitor_process_init(TALLOC_CTX *mem_ctx,
     }
 
     /* now start the data providers */
-    ret = confdb_get_domains_list(cdb, ctx, (const char ***)&doms, &dom_count);
+    ret = confdb_get_domains_list(cdb, ctx,
+                                  &(ctx->dom_map), &doms, &dom_count);
     if (ret != EOK) {
         DEBUG(2, ("No domains configured. LOCAL should always exist!\n"));
         return ret;
diff --git a/server/responder/nss/nsssrv_cmd.c b/server/responder/nss/nsssrv_cmd.c
index 76da6e06..f14724b9 100644
--- a/server/responder/nss/nsssrv_cmd.c
+++ b/server/responder/nss/nsssrv_cmd.c
@@ -121,7 +121,7 @@ static int nss_parse_name(struct nss_dom_ctx *dctx, const char *fullname)
         return EINVAL;
     }
 
-    dctx->check_provider = info->has_provider;
+    dctx->check_provider = strcasecmp(domain, "LOCAL");
     dctx->legacy = info->legacy;
 
     dctx->domain = talloc_strdup(dctx, domain);
@@ -660,7 +660,7 @@ static int nss_cmd_getpwuid(struct cli_ctx *cctx)
         dctx->cmdctx = cmdctx;
         dctx->domain = talloc_strdup(dctx, domains[i]);
         if (!dctx->domain) return ENOMEM;
-        dctx->check_provider = info->has_provider;
+        dctx->check_provider = strcasecmp(domains[i], "LOCAL");
         dctx->legacy = info->legacy;
 
 
@@ -858,7 +858,7 @@ static int nss_cmd_setpwent_ext(struct cli_ctx *cctx, bool immediate)
         dctx->cmdctx = cmdctx;
         dctx->domain = talloc_strdup(dctx, domains[i]);
         if (!dctx->domain) return ENOMEM;
-        dctx->check_provider = info->has_provider;
+        dctx->check_provider = strcasecmp(domains[i], "LOCAL");
         dctx->legacy = info->legacy;
 
         if (dctx->check_provider) {
@@ -1564,7 +1564,7 @@ static int nss_cmd_getgrgid(struct cli_ctx *cctx)
         dctx->cmdctx = cmdctx;
         dctx->domain = talloc_strdup(dctx, domains[i]);
         if (!dctx->domain) return ENOMEM;
-        dctx->check_provider = info->has_provider;
+        dctx->check_provider = strcasecmp(domains[i], "LOCAL");
         dctx->legacy = info->legacy;
 
         DEBUG(4, ("Requesting info for [%lu@%s]\n",
@@ -1760,7 +1760,7 @@ static int nss_cmd_setgrent_ext(struct cli_ctx *cctx, bool immediate)
         dctx->cmdctx = cmdctx;
         dctx->domain = talloc_strdup(dctx, domains[i]);
         if (!dctx->domain) return ENOMEM;
-        dctx->check_provider = info->has_provider;
+        dctx->check_provider = strcasecmp(domains[i], "LOCAL");
         dctx->legacy = info->legacy;
 
         if (dctx->check_provider) {
diff --git a/server/util/btreemap.c b/server/util/btreemap.c
index 7bda0570..2d9b1761 100644
--- a/server/util/btreemap.c
+++ b/server/util/btreemap.c
@@ -166,19 +166,13 @@ int btreemap_set_value(TALLOC_CTX *mem_ctx,
     return EOK;
 }
 
-static int _btreemap_get_keys(TALLOC_CTX *mem_ctx, struct btreemap *map, const void ***array, int *count, int depth)
+static int _btreemap_get_keys(TALLOC_CTX *mem_ctx, struct btreemap *map,
+                              const void ***array, int *count, int depth)
 {
     int ret;
     const void **tmp_array;
 
-    if (map == NULL) {
-        if (depth == 0) {
-            /* This is the top-level */
-            count = 0;
-            *array = NULL;
-        }
-        return EOK;
-    }
+    if (map == NULL) return EOK;
 
     /* Left Node */
     ret = _btreemap_get_keys(mem_ctx, map->left, array, count, depth+1);
@@ -212,5 +206,6 @@ static int _btreemap_get_keys(TALLOC_CTX *mem_ctx, struct btreemap *map, const v
 int btreemap_get_keys(TALLOC_CTX *mem_ctx, struct btreemap *map, const void ***array, int *count)
 {
     *array = NULL;
+    *count = 0;
     return _btreemap_get_keys(mem_ctx, map, array, count, 0);
 }