On 06/14/2016 05:21 PM, Petr Cech wrote:
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.)

Hello list,

I rebased this patch set due to refactoring of the data provider. New patch set is attached.

I will add tests, I hope soon :-)


Regards

--
Petr^4 Čech
>From c3be8767dc0f284e8d1d8db85b2a2b4d53e2b42f 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/4] 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 06127584f5a4cd7f2412a8f88d2af0c0e527d4ff..45b3669d4d61409a730a796d9955084370b5359a 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -189,6 +189,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 ce363c5a4122aa5e48ca83b0b2bdf63ff4372d91..5eea9e477038913a94b4a61b6d1a211abb951bfe 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 e70f987dbceacc2900fa5608feccf6bf6f971c36 Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@redhat.com>
Date: Tue, 21 Jun 2016 08:34:15 +0200
Subject: [PATCH 2/4] 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_subdomains.c | 94 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
index 5b0bee86665a46f45fde5ec1034a9ccf8700d13a..1a92359f9d78f871fa0fcd3ebb9d3c7cfd37e7ad 100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -57,6 +57,91 @@
 /* do not refresh more often than every 5 seconds for now */
 #define AD_SUBDOMAIN_REFRESH_LIMIT 5
 
+static errno_t ad_get_enabled_domains(TALLOC_CTX *mem_ctx,
+                                      struct ad_id_ctx *ad_id_ctx,
+                                      const char *ad_domain,
+                                      char ***_ad_enabled_domains)
+{
+    int ret;
+    const char *str;
+    const char *option_name;
+    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_cstring(ad_id_ctx->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) {
+        option_name = ad_id_ctx->ad_options->basic[AD_ENABLED_DOMAINS].opt_name;
+        DEBUG(SSSDBG_CRIT_FAILURE, "Failed to parse option [%s], [%i] [%s]!\n",
+                                   option_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;
+}
+
 static errno_t
 ad_subdom_ad_ctx_new(struct be_ctx *be_ctx,
                      struct ad_id_ctx *id_ctx,
@@ -164,6 +249,7 @@ struct ad_subdomains_ctx {
 
     struct sdap_domain *sdom;
     char *domain_name;
+    char **ad_enabled_domains;
 
     time_t last_refreshed;
 };
@@ -1346,6 +1432,7 @@ errno_t ad_subdomains_init(TALLOC_CTX *mem_ctx,
 {
     struct ad_subdomains_ctx *sd_ctx;
     const char *ad_domain;
+    char **ad_enabled_domains;
     time_t period;
     errno_t ret;
 
@@ -1357,6 +1444,12 @@ errno_t ad_subdomains_init(TALLOC_CTX *mem_ctx,
         return ENOMEM;
     }
 
+    ret = ad_get_enabled_domains(sd_ctx, ad_id_ctx, ad_domain,
+                                 &ad_enabled_domains);
+    if (ret != EOK) {
+        return EINVAL;
+    }
+
     sd_ctx->be_ctx = be_ctx;
     sd_ctx->sdom = ad_id_ctx->sdap_id_ctx->opts->sdom;
     sd_ctx->sdap_id_ctx = ad_id_ctx->sdap_id_ctx;
@@ -1365,6 +1458,7 @@ errno_t ad_subdomains_init(TALLOC_CTX *mem_ctx,
         DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
         return ENOMEM;
     }
+    sd_ctx->ad_enabled_domains = ad_enabled_domains;
     sd_ctx->ad_id_ctx = ad_id_ctx;
 
     dp_set_method(dp_methods, DPM_DOMAINS_HANDLER,
-- 
2.5.5

>From 30bb33ba8dac5fc0fef70ff0200af4a574f7aaad Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@redhat.com>
Date: Tue, 21 Jun 2016 09:48:52 +0200
Subject: [PATCH 3/4] 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 1a92359f9d78f871fa0fcd3ebb9d3c7cfd37e7ad..f209f0b02a62416d5f422e7e0e8c3ec9cfe436c0 100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -1170,6 +1170,7 @@ static void ad_subdomains_refresh_connect_done(struct tevent_req *subreq)
         return;
     }
 
+    /* connect to the DC we are a member of */
     subreq = ad_master_domain_send(state, state->ev, state->id_ctx->conn,
                                    state->sdap_op, state->sd_ctx->domain_name);
     if (subreq == NULL) {
@@ -1219,6 +1220,21 @@ static void ad_subdomains_refresh_master_done(struct tevent_req *subreq)
         goto done;
     }
 
+    /*
+     * If ad_enabled_domains contains only master domain
+     * we shouldn't lookup other domains.
+     */
+    if (state->sd_ctx->ad_enabled_domains != NULL) {
+        if (talloc_array_length(state->sd_ctx->ad_enabled_domains) == 2) {
+            if (strcmp(state->sd_ctx->ad_enabled_domains[0],
+                       state->be_ctx->domain->name) == 0) {
+                DEBUG(SSSDBG_TRACE_FUNC,
+                      "No other enabled domain than master.\n");
+                goto done;
+            }
+        }
+    }
+
     subreq = ad_get_root_domain_send(state, state->ev, forest,
                                      sdap_id_op_handle(state->sdap_op),
                                      state->sd_ctx);
-- 
2.5.5

>From bf8e0de21752821cc8b5f3add1d5b721a89681f4 Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@redhat.com>
Date: Tue, 21 Jun 2016 15:04:22 +0200
Subject: [PATCH 4/4] 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 | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
index f209f0b02a62416d5f422e7e0e8c3ec9cfe436c0..8788e7c68317536acaa7a5bfa3cc65b0608a5796 100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -497,6 +497,7 @@ done:
 
 static errno_t ad_subdomains_process(TALLOC_CTX *mem_ctx,
                                      struct sss_domain_info *domain,
+                                     char **enabled_domains_list,
                                      size_t nsd, struct sysdb_attrs **sd,
                                      struct sysdb_attrs *root,
                                      size_t *_nsd_out,
@@ -505,9 +506,11 @@ static errno_t ad_subdomains_process(TALLOC_CTX *mem_ctx,
     size_t i, sdi;
     struct sysdb_attrs **sd_out;
     const char *sd_name;
+    const bool is_sd_filtered = (enabled_domains_list == NULL) ? false : true;
+    bool is_sd_enabled;
     errno_t ret;
 
-    if (root == NULL) {
+    if (root == NULL && is_sd_filtered == false) {
         /* We are connected directly to the root domain. The 'sd'
          * list is complete and we can just use it
          */
@@ -534,6 +537,17 @@ static errno_t ad_subdomains_process(TALLOC_CTX *mem_ctx,
             goto fail;
         }
 
+        is_sd_enabled = true;
+        if (is_sd_filtered == true) {
+            is_sd_enabled = false;
+            for (size_t j = 0; enabled_domains_list[j] != NULL; j++) {
+                if (strcmp(sd_name, enabled_domains_list[j]) == 0) {
+                    is_sd_enabled = true;
+                    break;
+                }
+            }
+        }
+
         if (strcasecmp(sd_name, domain->name) == 0) {
             DEBUG(SSSDBG_TRACE_INTERNAL,
                   "Not including primary domain %s in the subdomain list\n",
@@ -541,14 +555,19 @@ static errno_t ad_subdomains_process(TALLOC_CTX *mem_ctx,
             continue;
         }
 
-        sd_out[sdi] = talloc_steal(sd_out, sd[i]);
-        sdi++;
+        if (is_sd_enabled) {
+            sd_out[sdi] = talloc_steal(sd_out, sd[i]);
+            sdi++;
+        }
     }
 
     /* Now include the root */
-    sd_out[sdi] = talloc_steal(sd_out, root);
+    if (root != NULL) {
+        sd_out[sdi] = talloc_steal(sd_out, root);
+        sdi++;
+    }
 
-    *_nsd_out = sdi+1;
+    *_nsd_out = sdi;
     *_sd_out = sd_out;
     return EOK;
 
@@ -789,6 +808,7 @@ static void ad_get_slave_domain_done(struct tevent_req *subreq)
      * subdomains.
      */
     ret = ad_subdomains_process(state, state->be_ctx->domain,
+                                state->sd_ctx->ad_enabled_domains,
                                 reply_count, reply, state->root_attrs,
                                 &nsubdoms, &subdoms);
     if (ret != EOK) {
-- 
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