Hello list,

there is patch set attached resolving:
https://fedorahosted.org/sssd/ticket/2828

You can also see:
https://github.com/celestian/sssd/commits/subdomains_v2


This patch set is without tests yet.
I will send tests after Pavel B. refactor of AD provider.


I would like to remark couple of things:

1) Option ad_enabled_domains disabled by default.

2) We use labels master domain and root domain in code. Master domain belongs to one that SSSD is connected to. Root domain means AD forest root.

3) There is a lot of different environment setup. So this option has slightly different meaning in each of them.

4) There is secret feature if you forget fill in master domain to the option it will be added automatically.


What is this option good for?
Let's imagine environment like:

A (forest root)
|
|-- S1 (SSSD client)
|
|-- B (AD client)
|   |
|   |-- S2 (SSSD client)
|
|-- C (AD client)
|
|-- D (AD client)

A ... forest root
B, C, D ... AD clients
S1 ... SSSD client connected to A
S2 ... SSSD client connected to B

We could write:

* on S1: ad_enabled_domain = A, B, C
and then S1 will not attempt to connect D.

* on S1: ad_enabled_domain = B, C
the same as above because A is automagically
filled in.

* on S2: ad_enabled_domain = A, C
and then B is filled in automatically
and S2 will not attempt to connect D.

* on S2: ad_enabled_domain = C
and then B is filled in automatically
and S2 will not attempt to connect D and A.
(I am wondering that it is possible but it
happened during manual testing.)


Regards

--
Petr^4 Čech
>From 69def6fdecd9983701909a26b4a5027b7f90ecf3 Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@redhat.com>
Date: Fri, 13 May 2016 05:21:07 -0400
Subject: [PATCH 1/5] AD_PROVIDER: Add ad_enabled_domains option

Resolves:
https://fedorahosted.org/sssd/ticket/2828
---
 src/config/SSSDConfig/__init__.py.in   |  1 +
 src/config/etc/sssd.api.d/sssd-ad.conf |  1 +
 src/man/sssd-ad.5.xml                  | 22 ++++++++++++++++++++++
 src/providers/ad/ad_common.h           |  1 +
 src/providers/ad/ad_opts.c             |  1 +
 5 files changed, 26 insertions(+)

diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index abbf5dca8b2c155b92d2aefdcd80e09a6681c39d..d2d70ef1bbe87a6803ff9b2d7c90aa60525f754c 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -188,6 +188,7 @@ option_strings = {
 
     # [provider/ad]
     'ad_domain' : _('Active Directory domain'),
+    'ad_enabled_domains' : _('Enabled Active Directory domains'),
     'ad_server' : _('Active Directory server address'),
     'ad_backup_server' : _('Active Directory backup server address'),
     'ad_hostname' : _('Active Directory client hostname'),
diff --git a/src/config/etc/sssd.api.d/sssd-ad.conf b/src/config/etc/sssd.api.d/sssd-ad.conf
index 23006d26ca6fe7ca2b912ef091b4c73d5d23bee1..0d16387aafbd4f1f9f46654d31f403ad465f8422 100644
--- a/src/config/etc/sssd.api.d/sssd-ad.conf
+++ b/src/config/etc/sssd.api.d/sssd-ad.conf
@@ -1,5 +1,6 @@
 [provider/ad]
 ad_domain = str, None, false
+ad_enabled_domains = str, None, false
 ad_server = str, None, false
 ad_backup_server = str, None, false
 ad_hostname = str, None, false
diff --git a/src/man/sssd-ad.5.xml b/src/man/sssd-ad.5.xml
index ef27976dd62e164cfb91359efc69bd54e1aa9711..1e3309fe0b9719adb7451e5491cca3366df3531b 100644
--- a/src/man/sssd-ad.5.xml
+++ b/src/man/sssd-ad.5.xml
@@ -114,6 +114,28 @@ ldap_id_mapping = False
                 </varlistentry>
 
                 <varlistentry>
+                    <term>ad_enabled_domains (string)</term>
+                    <listitem>
+                        <para>
+                            The comma-separated list of the enabled Active
+                            Directory domains. This is optional. If provided,
+                            SSSD will not contact domains not listed in this
+                            option. If not provided, all domains from AD forest
+                            are enabled.
+                        </para>
+                        <para>
+                            For proper operation, this option should be
+                            specified as the lower-case version of the long
+                            version of the Active Directory domain.
+                        </para>
+                        <para>
+                            The short domain name (also known as the NetBIOS
+                            or the flat name) is autodetected by the SSSD.
+                        </para>
+                    </listitem>
+                </varlistentry>
+
+                <varlistentry>
                     <term>ad_server, ad_backup_server (string)</term>
                     <listitem>
                         <para>
diff --git a/src/providers/ad/ad_common.h b/src/providers/ad/ad_common.h
index 37178d611da5c23fe8fd1f8b5033a3cd90301a71..0511cbf0a482aef890d08e6681dd193f2739e2a1 100644
--- a/src/providers/ad/ad_common.h
+++ b/src/providers/ad/ad_common.h
@@ -42,6 +42,7 @@ struct ad_options;
 
 enum ad_basic_opt {
     AD_DOMAIN = 0,
+    AD_ENABLED_DOMAINS,
     AD_SERVER,
     AD_BACKUP_SERVER,
     AD_HOSTNAME,
diff --git a/src/providers/ad/ad_opts.c b/src/providers/ad/ad_opts.c
index 57dfcca6b998083c7cf9ac0bcb142ff7736cc8b9..8e02fbeb4d580ac31775917ed787e5d7ff3c9271 100644
--- a/src/providers/ad/ad_opts.c
+++ b/src/providers/ad/ad_opts.c
@@ -28,6 +28,7 @@
 
 struct dp_option ad_basic_opts[] = {
     { "ad_domain", DP_OPT_STRING, NULL_STRING, NULL_STRING },
+    { "ad_enabled_domains", DP_OPT_STRING, NULL_STRING, NULL_STRING },
     { "ad_server", DP_OPT_STRING, NULL_STRING, NULL_STRING },
     { "ad_backup_server", DP_OPT_STRING, NULL_STRING, NULL_STRING },
     { "ad_hostname", DP_OPT_STRING, NULL_STRING, NULL_STRING },
-- 
2.5.5

>From 79bf37920f20f327e40b638c4d0c2fe892bae589 Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@redhat.com>
Date: Fri, 13 May 2016 08:49:43 -0400
Subject: [PATCH 2/5] AD_PROVIDER: Initializing of ad_enabled_domains

We add ad_enabled_domains into ad_subdomains_ctx.

Resolves:
https://fedorahosted.org/sssd/ticket/2828
---
 src/providers/ad/ad_init.c       | 93 +++++++++++++++++++++++++++++++++++++++-
 src/providers/ad/ad_subdomains.c |  3 ++
 src/providers/ad/ad_subdomains.h |  1 +
 3 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/src/providers/ad/ad_init.c b/src/providers/ad/ad_init.c
index b17a9f50a3a0e8788196691055faf956a5f6a464..5a64c2afde8f8d7c53af60948d210cd175a62ff1 100644
--- a/src/providers/ad/ad_init.c
+++ b/src/providers/ad/ad_init.c
@@ -533,6 +533,88 @@ ad_shutdown(struct be_req *req)
     sdap_handler_done(req, DP_ERR_OK, EOK, NULL);
 }
 
+static int get_enabled_domains(TALLOC_CTX *mem_ctx, const char *ad_domain,
+                               char ***_ad_enabled_domains)
+{
+    int ret;
+    char *str;
+    char **domains = NULL;
+    char **list = NULL;
+    int count;
+    bool is_ad_in_domains;
+    TALLOC_CTX *tmp_ctx = NULL;
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        return ENOMEM;
+    }
+
+    str = dp_opt_get_string(ad_options->basic, AD_ENABLED_DOMAINS);
+    if (str == NULL) {
+        _ad_enabled_domains = NULL;
+        ret = EOK;
+        goto done;
+    }
+
+    count = 0;
+    ret = split_on_separator(tmp_ctx, str, ',', true, true, &domains, &count);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, "Failed to parse option [%s], [%i] [%s]!\n",
+                                 ad_options->basic[AD_ENABLED_DOMAINS].opt_name,
+                                 ret, sss_strerror(ret));
+        ret = EINVAL;
+        goto done;
+    }
+
+    list = talloc_array_size(tmp_ctx, sizeof(char*), count);
+    if (list == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    is_ad_in_domains = false;
+    for (int i = 0; i < count; i++) {
+        list[i] = talloc_strdup(list, domains[i]);
+        if (list[i] == NULL) {
+            ret = ENOMEM;
+            goto done;
+        }
+
+        is_ad_in_domains += strcmp(ad_domain, domains[i]) == 0 ? true : false;
+    }
+
+    if (is_ad_in_domains == false) {
+        list = talloc_realloc(tmp_ctx, list, char*, count + 1);
+        if (list == NULL) {
+            ret = ENOMEM;
+            goto done;
+        }
+
+        list[count] = talloc_strdup(list, ad_domain);
+        if (list[count] == NULL) {
+            ret = ENOMEM;
+            goto done;
+        }
+
+        count++;
+    }
+
+    list = talloc_realloc(tmp_ctx, list, char*, count + 1);
+    if (list == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
+    list[count] = NULL;
+    count++;
+
+    *_ad_enabled_domains = talloc_steal(mem_ctx, list);
+    ret = EOK;
+
+done:
+    talloc_free(tmp_ctx);
+    return ret;
+}
+
 int sssm_ad_subdomains_init(struct be_ctx *bectx,
                             struct bet_ops **ops,
                             void **pvt_data)
@@ -540,6 +622,7 @@ int sssm_ad_subdomains_init(struct be_ctx *bectx,
     int ret;
     struct ad_id_ctx *id_ctx;
     const char *ad_domain;
+    char **ad_enabled_domains = NULL;
 
     ret = sssm_ad_id_init(bectx, ops, (void **) &id_ctx);
     if (ret != EOK) {
@@ -554,7 +637,15 @@ int sssm_ad_subdomains_init(struct be_ctx *bectx,
 
     ad_domain = dp_opt_get_cstring(ad_options->basic, AD_DOMAIN);
 
-    ret = ad_subdom_init(bectx, id_ctx, ad_domain, ops, pvt_data);
+    ret = get_enabled_domains(bectx, ad_domain, &ad_enabled_domains);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "Failed to parse option [%s]!\n",
+                                ad_options->basic[AD_ENABLED_DOMAINS].opt_name);
+        return EINVAL;
+    }
+
+    ret = ad_subdom_init(bectx, id_ctx, ad_domain, ad_enabled_domains,
+                         ops, pvt_data);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, "ad_subdom_init failed.\n");
         return ret;
diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
index 4bdd2a7adbf104354a33fd382eb175d9c315d356..a30199912db52d97da8fb94d42cb323c3950c957 100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -63,6 +63,7 @@ struct ad_subdomains_ctx {
     struct sdap_id_conn_ctx *ldap_ctx;
     struct sss_idmap_ctx *idmap_ctx;
     char *domain_name;
+    char **ad_enabled_domains;
 
     time_t last_refreshed;
     struct tevent_timer *timer_event;
@@ -1179,6 +1180,7 @@ struct bet_ops ad_subdomains_ops = {
 int ad_subdom_init(struct be_ctx *be_ctx,
                    struct ad_id_ctx *id_ctx,
                    const char *ad_domain,
+                   char **ad_enabled_domains,
                    struct bet_ops **ops,
                    void **pvt_data)
 {
@@ -1201,6 +1203,7 @@ int ad_subdom_init(struct be_ctx *be_ctx,
         DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
         return ENOMEM;
     }
+    ctx->ad_enabled_domains = ad_enabled_domains;
     ctx->ad_id_ctx = id_ctx;
     *ops = &ad_subdomains_ops;
     *pvt_data = ctx;
diff --git a/src/providers/ad/ad_subdomains.h b/src/providers/ad/ad_subdomains.h
index da93af37943bd27313b69cd35b4242f07822608f..8922bd6859e49441f2ca06bc3e6fd0949348d729 100644
--- a/src/providers/ad/ad_subdomains.h
+++ b/src/providers/ad/ad_subdomains.h
@@ -31,6 +31,7 @@
 int ad_subdom_init(struct be_ctx *be_ctx,
                    struct ad_id_ctx *id_ctx,
                    const char *ad_domain,
+                   char **ad_enabled_domains,
                    struct bet_ops **ops,
                    void **pvt_data);
 
-- 
2.5.5

>From a96b80ffb3968afd2c182ad9e5dd5a9da7798743 Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@redhat.com>
Date: Thu, 9 Jun 2016 02:43:01 -0400
Subject: [PATCH 3/5] AD_PROVIDER: ad_enabled_domains - only master

We can skip looking up other domains if option ad_enabled_domains
contains only master domain.

Resolves:
https://fedorahosted.org/sssd/ticket/2828
---
 src/providers/ad/ad_subdomains.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
index a30199912db52d97da8fb94d42cb323c3950c957..381143bf945a419cb3a3163bb7fdc1acc8d0107b 100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -577,6 +577,7 @@ static void ad_subdomains_get_conn_done(struct tevent_req *req)
         goto fail;
     }
 
+    /* connect to the DC we are a member of */
     req = ad_master_domain_send(ctx, ctx->sd_ctx->be_ctx->ev,
                                 ctx->sd_ctx->ldap_ctx,
                                 ctx->sdap_op,
@@ -627,6 +628,21 @@ static void ad_subdomains_master_dom_done(struct tevent_req *req)
         goto done;
     }
 
+    /*
+     * If ad_enabled_domains contains only master domain
+     * we shouldn't lookup other domains.
+     */
+    if (ctx->sd_ctx->ad_enabled_domains != NULL) {
+        if (talloc_array_length(ctx->sd_ctx->ad_enabled_domains) == 2) {
+            if (strcmp(ctx->sd_ctx->ad_enabled_domains[0],
+                       ctx->sd_ctx->be_ctx->domain->name) == 0) {
+                DEBUG(SSSDBG_TRACE_FUNC,
+                      "No other enabled domain than master.\n");
+                goto done;
+            }
+        }
+    }
+
     if (ctx->forest == NULL ||
           strcasecmp(ctx->sd_ctx->be_ctx->domain->name, ctx->forest) != 0) {
         DEBUG(SSSDBG_TRACE_FUNC,
-- 
2.5.5

>From ffb0b820394a7a0326d98dcf780e7fbf0aa73e71 Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@redhat.com>
Date: Tue, 14 Jun 2016 12:58:29 +0200
Subject: [PATCH 4/5] AD_PROVIDER: ad_enabled_domains - other then master

We can skip looking up other domains if
option ad_enabled_domains doesn't contain them.

Resolves:
https://fedorahosted.org/sssd/ticket/2828
---
 src/providers/ad/ad_subdomains.c | 80 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 79 insertions(+), 1 deletion(-)

diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
index 381143bf945a419cb3a3163bb7fdc1acc8d0107b..4c62d4569d116832e240b15eb012fda11cd5ca7f 100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -1010,6 +1010,68 @@ fail:
     return ret;
 }
 
+static errno_t filter_enabled_domains(TALLOC_CTX *mem_ctx,
+                                      char **enabled_domains_list,
+                                      struct sysdb_attrs **subdoms,
+                                      size_t subdoms_count,
+                                      struct sysdb_attrs ***_filtered_subdoms,
+                                      size_t *_filtered_subdoms_count)
+{
+    errno_t ret;
+    const char *subdom_name;
+    bool is_enabled;
+    struct sysdb_attrs **filtered_subdoms = NULL;
+    size_t fsd_count = 0;
+    TALLOC_CTX *tmp_ctx = NULL;
+
+    if (enabled_domains_list == NULL) {
+        return EINVAL;
+    }
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        return ENOMEM;
+    }
+
+    for (size_t i = 0; i < subdoms_count; i++) {
+
+        ret = sysdb_attrs_get_string(subdoms[i], AD_AT_TRUST_PARTNER,
+                                     &subdom_name);
+        if (ret != EOK) {
+            ret = ENOENT;
+            goto done;
+        }
+
+        is_enabled = false;
+        for (size_t j = 0; enabled_domains_list[j] != NULL; j++) {
+            if (strcmp(subdom_name, enabled_domains_list[j]) == 0) {
+                is_enabled = true;
+                break;
+            }
+        }
+
+        if (is_enabled) {
+            filtered_subdoms = talloc_realloc(tmp_ctx, filtered_subdoms,
+                                              struct sysdb_attrs *,
+                                              fsd_count + 1);
+            if (filtered_subdoms == NULL) {
+                ret = ENOMEM;
+                goto done;
+            }
+            filtered_subdoms[fsd_count] = talloc_steal(mem_ctx, subdoms[i]);
+            fsd_count++;
+        }
+    }
+
+    *_filtered_subdoms = talloc_steal(mem_ctx, filtered_subdoms);
+    *_filtered_subdoms_count = fsd_count;
+    ret = EOK;
+
+done:
+    talloc_free(tmp_ctx);
+    return ret;
+}
+
 static void ad_subdomains_get_slave_domain_done(struct tevent_req *req)
 {
     int ret;
@@ -1020,6 +1082,9 @@ static void ad_subdomains_get_slave_domain_done(struct tevent_req *req)
     bool refresh_has_changes = false;
     size_t nsubdoms;
     struct sysdb_attrs **subdoms;
+    struct sysdb_attrs **filtered_subdoms = NULL;
+    size_t nfsubdoms = 0;
+
 
     ctx = tevent_req_callback_data(req, struct ad_subdomains_req_ctx);
 
@@ -1077,8 +1142,21 @@ static void ad_subdomains_get_slave_domain_done(struct tevent_req *req)
         return;
     }
 
+    ret = filter_enabled_domains(ctx,
+                                 ctx->sd_ctx->ad_enabled_domains,
+                                 subdoms, nsubdoms,
+                                 &filtered_subdoms, &nfsubdoms);
+    if (ret == EINVAL) {
+        filtered_subdoms = subdoms;
+        nfsubdoms = nsubdoms;
+    } else if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("Cannot filter enabled subdomains\n"));
+        tevent_req_error(req, ret);
+        return;
+    }
+
     /* Got all the subdomains, let's process them */
-    ret = ad_subdomains_refresh(ctx->sd_ctx, nsubdoms, false, subdoms,
+    ret = ad_subdomains_refresh(ctx->sd_ctx, nfsubdoms, false, filtered_subdoms,
                                 &refresh_has_changes);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "Failed to refresh subdomains.\n");
-- 
2.5.5

>From e2ca6b2d5ed5e73e314b7e0fff4b55fa1eca0b74 Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@redhat.com>
Date: Tue, 14 Jun 2016 16:43:33 +0200
Subject: [PATCH 5/5] AD_PROVIDER: Enhanced debug message

In case we are not able to connect forest root AD
we could write its name.
---
 src/providers/ad/ad_subdomains.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
index 4c62d4569d116832e240b15eb012fda11cd5ca7f..c53ba4d79deec050f71306a7722f677355a0f646 100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -898,8 +898,8 @@ static void ad_subdomains_root_conn_done(struct tevent_req *req)
         be_mark_dom_offline(ctx->root_domain, be_req_get_be_ctx(ctx->be_req));
 
         DEBUG(SSSDBG_OP_FAILURE,
-              "Failed to connect to AD server: [%d](%s)\n",
-              ret, strerror(ret));
+              "Failed to connect to AD server '%s': [%d](%s)\n",
+              ctx->root_domain->name, ret, strerror(ret));
         goto fail;
     }
 
-- 
2.5.5

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

Reply via email to