URL: https://github.com/SSSD/sssd/pull/5881
Author: scabrero
 Title: #5881: SDAP: Do not fail ASQ search when parsing a referenced entry 
fails
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5881/head:pr5881
git checkout pr5881
From 0e8af65ff8aa35538f6cd55ffd57576ee5fe08c0 Mon Sep 17 00:00:00 2001
From: Samuel Cabrero <scabr...@suse.de>
Date: Wed, 1 Dec 2021 11:17:23 +0100
Subject: [PATCH 1/2] SDAP: Add 'ldap_ignore_unreadable_references' parameter

When resolving a group using the AD provider it may happen sssd doesn't
have permissions to read the entry referenced in the 'member' attribute,
for example when the entry is located under a restricted LDAP sub-tree
for security reasons.

In this scenario, the sssd behavior is not consistent and depends on the
ldap_deref_threshold parameter, that controls if an attribute scoped
query (ASQ) will be used or if the group members will be searched
individually. If an ASQ operation is issued, the operation will fail
because the referenced entry can't be parsed and this can
lead to missing groups and makes impossible to use the group in simple
access provider. On the other hand, when the group members are looked
up individually sssd just ignores the unreadable entry.

This patch adds a new parameter 'ldap_ignore_unreadable_references' to
control if the current operation will fail when an unreadable entry is
found or the entry will be ignored, regardless if sssd issued an ASQ or
the members are looked up individually.

The issue can be replicated deploying this AD setup:

    CN=users,DC=aforest,DC=ad
      CN=g1,CN=users,DC=aforest,DC=ad
        member: CN=g2,CN=users,DC=aforest,DC=ad
        member: CN=g3,CN=users,DC=aforest,DC=ad
        member: CN=g4,CN=users,DC=aforest,DC=ad
        member: CN=g5,CN=users,DC=aforest,DC=ad
        member: CN=user1,CN=users,DC=aforest,DC=ad
      CN=g2,CN=users,DC=aforest,DC=ad
        member: CN=g3,CN=users,DC=aforest,DC=ad
        member: CN=g4,CN=users,DC=aforest,DC=ad
        member: CN=g5,CN=users,DC=aforest,DC=ad
        member: CN=user2,CN=users,DC=aforest,DC=ad
        memberOf: CN=g1,CN=users,DC=aforest,DC=ad
      CN=g3,CN=users,DC=aforest,DC=ad               <-- Deny access to sssd account
        member: CN=g4,CN=users,DC=aforest,DC=ad
        member: CN=g5,CN=users,DC=aforest,DC=ad
        member: CN=user3,CN=users,DC=aforest,DC=ad
        memberOf: CN=g2,CN=users,DC=aforest,DC=ad
        memberOf: CN=g1,CN=users,DC=aforest,DC=ad
      CN=g4,CN=users,DC=aforest,DC=ad
        member: CN=g5,CN=users,DC=aforest,DC=ad
        member: CN=user5,CN=users,DC=aforest,DC=ad
        memberOf: CN=g3,CN=users,DC=aforest,DC=ad
        memberOf: CN=g2,CN=users,DC=aforest,DC=ad
        memberOf: CN=g1,CN=users,DC=aforest,DC=ad
      CN=g5,CN=users,DC=aforest,DC=ad
        member: CN=user5,CN=users,DC=aforest,DC=ad
        memberOf: CN=g4,CN=users,DC=aforest,DC=ad
        memberOf: CN=g3,CN=users,DC=aforest,DC=ad
        memberOf: CN=g2,CN=users,DC=aforest,DC=ad
        memberOf: CN=g1,CN=users,DC=aforest,DC=ad
      CN=user1,CN=users,DC=aforest,DC=ad
        memberOf: CN=g1,CN=users,DC=aforest,DC=ad
      CN=user2,CN=users,DC=aforest,DC=ad
        memberOf: CN=g2,CN=users,DC=aforest,DC=ad
      CN=user3,CN=users,DC=aforest,DC=ad
        memberOf: CN=g3,CN=users,DC=aforest,DC=ad
      CN=user4,CN=users,DC=aforest,DC=ad
        memberOf: CN=g4,CN=users,DC=aforest,DC=ad
      CN=user5,CN=users,DC=aforest,DC=ad
        memberOf: CN=g5,CN=users,DC=aforest,DC=ad

And using this sssd.conf
-------------------------------------------------------------------------------
[sssd]
    config_file_version = 2
    services = nss, pam
    domains = aforest.ad

[nss]

[pam]

[domain/aforest.ad]
    auth_provider = ad
    id_provider = ad
    access_provider = simple
    simple_allow_groups = g1
    ldap_deref_threshold = 1
    debug_level = 10
-------------------------------------------------------------------------------

In this setup sssd can't resolve group 'g1' because it fails parsing one
of the referenced members, 'g3':

    $> getent group g1
    No output.

    $> id user5
    uid=1862001108(user5) gid=1862000513(domain users) groups=1862000513(domain users),1862001111,18620011

When the group is used to filter access it does not work:

    ...
    [simple_access_check_send] (0x0200): [RID#7] Simple access check for us...@aforest.ad
    ...
    [simple_check_get_groups_send] (0x0400): [RID#7] Need to resolve 3 groups
    [sdap_get_generic_ext_step] (0x0400): [RID#8] calling ldap_search_ext with [(&(objectSID=S-1-5-21-3230
    ...
    [sdap_nested_group_hash_insert] (0x4000): [RID#8] Inserting [CN=g1,CN=Users,DC=aforest,DC=ad] into has
    [sdap_nested_group_process_send] (0x2000): [RID#8] About to process group [CN=g1,CN=Users,DC=aforest,D
    ...
    [sdap_nested_group_process_send] (0x0400): [RID#8] More members were missing than the deref threshold
    [sdap_nested_group_process_send] (0x2000): [RID#8] Looking up 2/5 members of group [CN=g1,CN=Users,DC=
    [sdap_nested_group_process_send] (0x2000): [RID#8] Dereferencing members of group [CN=g1,CN=Users,DC=a
    [sdap_deref_search_send] (0x2000): [RID#8] Server supports ASQ
    [sdap_asq_search_send] (0x0400): [RID#8] Dereferencing entry [CN=g1,CN=Users,DC=aforest,DC=ad] using A
    ...
    [sdap_get_generic_ext_step] (0x0400): [RID#8] calling ldap_search_ext with [no filter][CN=g1,CN=Users,
    ...
    [sdap_process_message] (0x4000): [RID#8] Message type: [LDAP_RES_SEARCH_ENTRY]
    [sdap_asq_search_parse_entry] (0x0040): [RID#8] Unknown entry type, no objectClass found for DN [CN=g3
    [sdap_get_generic_op_finished] (0x0020): [RID#8] reply parsing callback failed.
    [sdap_op_destructor] (0x1000): [RID#8] Abandoning operation 3
    [generic_ext_search_handler] (0x0020): [RID#8] sdap_get_generic_ext_recv request failed: [22]: Invalid
    [sdap_deref_search_done] (0x0040): [RID#8] dereference processing failed [22]: Invalid argument
    [sdap_nested_group_deref_direct_done] (0x0020): [RID#8] Error processing direct membership [22]: Inval
    [sdap_nested_done] (0x0020): [RID#8] Nested group processing failed: [22][Invalid argument]
    ...
    [simple_resolve_group_done] (0x0080): [RID#8] Cannot refresh data from DP: 3,0: Group lookup failed
    ...
    [simple_check_get_groups_next] (0x2000): [RID#9] All groups resolved. Done.
    [simple_access_check_done] (0x0040): [RID#9] Could not collect groups of user us...@aforest.ad
    [simple_access_check_done] (0x0400): [RID#9] But no deny groups were defined so we can continue.
    [simple_check_groups] (0x4000): [RID#9] Checking against allow list group name [g...@aforest.ad].
    [simple_access_check_done] (0x2000): [RID#9] Group check done
    [simple_access_check_recv] (0x1000): [RID#9] Access not granted
    ...

Resolves: https://github.com/SSSD/sssd/issues/4893

Signed-off-by: Samuel Cabrero <scabr...@suse.de>
---
 src/config/SSSDConfig/sssdoptions.py     |  1 +
 src/config/cfg_rules.ini                 |  1 +
 src/config/etc/sssd.api.d/sssd-ldap.conf |  1 +
 src/man/sssd-ldap.5.xml                  | 23 +++++++++++++++++++++++
 src/providers/ad/ad_opts.c               |  1 +
 src/providers/ipa/ipa_opts.c             |  1 +
 src/providers/ldap/ldap_opts.c           |  1 +
 src/providers/ldap/sdap.h                |  1 +
 8 files changed, 30 insertions(+)

diff --git a/src/config/SSSDConfig/sssdoptions.py b/src/config/SSSDConfig/sssdoptions.py
index e7de867a99..6e8e84d250 100644
--- a/src/config/SSSDConfig/sssdoptions.py
+++ b/src/config/SSSDConfig/sssdoptions.py
@@ -367,6 +367,7 @@ def __init__(self):
         'ldap_dns_service_name': _('Service name for DNS service lookups'),
         'ldap_page_size': _('The number of records to retrieve in a single LDAP query'),
         'ldap_deref_threshold': _('The number of members that must be missing to trigger a full deref'),
+        'ldap_ignore_unreadable_references': _('Ignore unreadable LDAP references'),
         'ldap_sasl_canonicalize': _('Whether the LDAP library should perform a reverse lookup to canonicalize the '
                                     'host name during a SASL bind'),
         'ldap_rfc2307_fallback_to_local_users': _('Allows to retain local users as members of an LDAP group for '
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index 39b66ba73a..72d99188f3 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -609,6 +609,7 @@ option = ldap_default_authtok_type
 option = ldap_default_bind_dn
 option = ldap_deref
 option = ldap_deref_threshold
+option = ldap_ignore_unreadable_references
 option = ldap_disable_paging
 option = ldap_disable_range_retrieval
 option = ldap_dns_service_name
diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf b/src/config/etc/sssd.api.d/sssd-ldap.conf
index 1945d65524..8800825c85 100644
--- a/src/config/etc/sssd.api.d/sssd-ldap.conf
+++ b/src/config/etc/sssd.api.d/sssd-ldap.conf
@@ -33,6 +33,7 @@ ldap_dns_service_name = str, None, false
 ldap_deref = str, None, false
 ldap_page_size = int, None, false
 ldap_deref_threshold = int, None, false
+ldap_ignore_unreadable_references = bool, None, false
 ldap_sasl_canonicalize = bool, None, false
 ldap_sasl_minssf = int, None, false
 ldap_sasl_maxssf = int, None, false
diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml
index 436350b1c6..1565341c45 100644
--- a/src/man/sssd-ldap.5.xml
+++ b/src/man/sssd-ldap.5.xml
@@ -686,6 +686,29 @@
                     </listitem>
                 </varlistentry>
 
+                <varlistentry>
+                    <term>ldap_ignore_unreadable_references (bool)</term>
+                    <listitem>
+                        <para>
+                            Ignore unreadable LDAP entries referenced in
+                            group's member attribute. If this parameter is set
+                            to false an error will be returned and the
+                            operation will fail instead of just ignoring the
+                            unreadable entry.
+                        </para>
+                        <para>
+                            This parameter may be useful when using the AD
+                            provider and the computer account that sssd uses
+                            to connect to AD does not have access to a
+                            particular entry or LDAP sub-tree for security
+                            reasons.
+                        </para>
+                        <para>
+                            Default: False
+                        </para>
+                    </listitem>
+                </varlistentry>
+
                 <varlistentry>
                     <term>ldap_tls_reqcert (string)</term>
                     <listitem>
diff --git a/src/providers/ad/ad_opts.c b/src/providers/ad/ad_opts.c
index 2ab45df203..c3b39ec0f6 100644
--- a/src/providers/ad/ad_opts.c
+++ b/src/providers/ad/ad_opts.c
@@ -144,6 +144,7 @@ struct dp_option ad_def_ldap_opts[] = {
     { "ldap_auth_disable_tls_never_use_in_production", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
     { "ldap_page_size", DP_OPT_NUMBER, { .number = 1000 }, NULL_NUMBER },
     { "ldap_deref_threshold", DP_OPT_NUMBER, { .number = 10 }, NULL_NUMBER },
+    { "ldap_ignore_unreadable_references", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
     { "ldap_sasl_canonicalize", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
     { "ldap_connection_expire_timeout", DP_OPT_NUMBER, { .number = 900 }, NULL_NUMBER },
     { "ldap_connection_expire_offset", DP_OPT_NUMBER, { .number = 0 }, NULL_NUMBER },
diff --git a/src/providers/ipa/ipa_opts.c b/src/providers/ipa/ipa_opts.c
index dc0a2201ba..891bb86063 100644
--- a/src/providers/ipa/ipa_opts.c
+++ b/src/providers/ipa/ipa_opts.c
@@ -152,6 +152,7 @@ struct dp_option ipa_def_ldap_opts[] = {
     { "ldap_auth_disable_tls_never_use_in_production", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
     { "ldap_page_size", DP_OPT_NUMBER, { .number = 1000 }, NULL_NUMBER },
     { "ldap_deref_threshold", DP_OPT_NUMBER, { .number = 10 }, NULL_NUMBER },
+    { "ldap_ignore_unreadable_references", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
     { "ldap_sasl_canonicalize", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
     { "ldap_connection_expire_timeout", DP_OPT_NUMBER, { .number = 900 }, NULL_NUMBER },
     { "ldap_connection_expire_offset", DP_OPT_NUMBER, { .number = 0 }, NULL_NUMBER },
diff --git a/src/providers/ldap/ldap_opts.c b/src/providers/ldap/ldap_opts.c
index 46f5ec0262..e2c5cb501e 100644
--- a/src/providers/ldap/ldap_opts.c
+++ b/src/providers/ldap/ldap_opts.c
@@ -111,6 +111,7 @@ struct dp_option default_basic_opts[] = {
     { "ldap_auth_disable_tls_never_use_in_production", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
     { "ldap_page_size", DP_OPT_NUMBER, { .number = 1000 }, NULL_NUMBER },
     { "ldap_deref_threshold", DP_OPT_NUMBER, { .number = 10 }, NULL_NUMBER },
+    { "ldap_ignore_unreadable_references", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
     { "ldap_sasl_canonicalize", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
     { "ldap_connection_expire_timeout", DP_OPT_NUMBER, { .number = 900 }, NULL_NUMBER },
     { "ldap_connection_expire_offset", DP_OPT_NUMBER, { .number = 0 }, NULL_NUMBER },
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
index 6382bec25b..c440a06d61 100644
--- a/src/providers/ldap/sdap.h
+++ b/src/providers/ldap/sdap.h
@@ -224,6 +224,7 @@ enum sdap_basic_opt {
     SDAP_DISABLE_AUTH_TLS,
     SDAP_PAGE_SIZE,
     SDAP_DEREF_THRESHOLD,
+    SDAP_IGNORE_UNREADABLE_REFERENCES,
     SDAP_SASL_CANONICALIZE,
     SDAP_EXPIRE_TIMEOUT,
     SDAP_EXPIRE_OFFSET,

From 5b6d8f84b74da5cfe9a37e051c0d63ddd1969e9a Mon Sep 17 00:00:00 2001
From: Samuel Cabrero <scabr...@suse.de>
Date: Wed, 1 Dec 2021 11:36:01 +0100
Subject: [PATCH 2/2] SDAP: Honor ldap_ignore_unreadable_references parameter

Signed-off-by: Samuel Cabrero <scabr...@suse.de>
---
 src/providers/ldap/sdap_async.c               | 24 ++++++++++++++++++-
 src/providers/ldap/sdap_async_nested_groups.c | 13 +++++++++-
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
index da54705496..c77560ec2e 100644
--- a/src/providers/ldap/sdap_async.c
+++ b/src/providers/ldap/sdap_async.c
@@ -2243,6 +2243,17 @@ static errno_t sdap_x_deref_parse_entry(struct sdap_handle *sh,
 
     ret = EOK;
 done:
+    if (ret != EOK && ret != ENOMEM) {
+        bool ldap_ignore_unreadable_references =
+            dp_opt_get_bool(state->opts->basic,
+                            SDAP_IGNORE_UNREADABLE_REFERENCES);
+
+        if (ldap_ignore_unreadable_references) {
+            DEBUG(SSSDBG_TRACE_FUNC, "Ignoring unreadable reference\n");
+            ret = EOK;
+        }
+    }
+
     talloc_zfree(tmp_ctx);
     ldap_controls_free(ctrls);
     ldap_derefresponse_free(deref_res);
@@ -2613,7 +2624,7 @@ static errno_t sdap_asq_search_parse_entry(struct sdap_handle *sh,
     int num_attrs;
     struct sdap_deref_attrs **res;
     char *tmp;
-    char *dn;
+    char *dn = NULL;
     TALLOC_CTX *tmp_ctx;
     bool disable_range_rtrvl;
 
@@ -2704,6 +2715,17 @@ static errno_t sdap_asq_search_parse_entry(struct sdap_handle *sh,
 
     ret = EOK;
 done:
+    if (ret != EOK && ret != ENOMEM) {
+        bool ldap_ignore_unreadable_references =
+            dp_opt_get_bool(state->opts->basic,
+                            SDAP_IGNORE_UNREADABLE_REFERENCES);
+
+        if (ldap_ignore_unreadable_references) {
+            DEBUG(SSSDBG_TRACE_FUNC, "Ignoring unreadable reference [%s]\n",
+                  dn != NULL ? dn : "(null)");
+            ret = EOK;
+        }
+    }
     talloc_zfree(tmp_ctx);
     return ret;
 }
diff --git a/src/providers/ldap/sdap_async_nested_groups.c b/src/providers/ldap/sdap_async_nested_groups.c
index 2570a00bbd..3c69832ee7 100644
--- a/src/providers/ldap/sdap_async_nested_groups.c
+++ b/src/providers/ldap/sdap_async_nested_groups.c
@@ -1355,6 +1355,7 @@ struct sdap_nested_group_single_state {
 
     struct sysdb_attrs **nested_groups;
     int num_groups;
+    bool ignore_unreadable_references;
 };
 
 static errno_t sdap_nested_group_single_step(struct tevent_req *req);
@@ -1395,6 +1396,8 @@ sdap_nested_group_single_send(TALLOC_CTX *mem_ctx,
         goto immediately;
     }
     state->num_groups = 0; /* we will count exact number of the groups */
+    state->ignore_unreadable_references = dp_opt_get_bool(
+            group_ctx->opts->basic, SDAP_IGNORE_UNREADABLE_REFERENCES);
 
     /* process each member individually */
     ret = sdap_nested_group_single_step(req);
@@ -1573,7 +1576,15 @@ sdap_nested_group_single_step_process(struct tevent_req *subreq)
 
         break;
     case SDAP_NESTED_GROUP_DN_UNKNOWN:
-        /* not found in users nor nested_groups, continue */
+        if (state->ignore_unreadable_references) {
+            DEBUG(SSSDBG_TRACE_FUNC, "Ignoring unreadable reference [%s]\n",
+                  state->current_member->dn);
+        } else {
+            DEBUG(SSSDBG_OP_FAILURE, "Unknown entry type [%s]!\n",
+                  state->current_member->dn);
+            ret = EINVAL;
+            goto done;
+        }
         break;
     }
 
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it: 
https://pagure.io/fedora-infrastructure

Reply via email to