URL: https://github.com/SSSD/sssd/pull/5580
Author: sumit-bose
 Title: #5580: ipa subdomains: do not fail completely if one step fails
Action: opened

PR body:
"""
Currently while updating server side data stored on an IPA server during a
subdomains request the whole request will fail if a single step fails. As a
result the remaining server side data which would have been looked up after
the failed attempt are missing.

With this patch a failure in a single lookup is not considered fatal and
SSSD will try to read the remaining data after an error occurred.

Resolves: https://github.com/SSSD/sssd/issues/5571
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5580/head:pr5580
git checkout pr5580
From 3c308e0d84a29777e0e4fc5fc3f4487b7bbc5efc Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Thu, 8 Apr 2021 10:54:42 +0200
Subject: [PATCH 1/3] ipa: skip id-range of unknown type

If a new range type is added in the IPA serve SSSD currently considers
this as an error and stops processing and further server side options.

With this patch unknown range types are just skipped and no error is
returned.

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

:fixes: unknown IPA id-range types are not considered as an error
---
 src/providers/ipa/ipa_idmap.c      |  2 +-
 src/providers/ipa/ipa_subdomains.c | 12 ++++++++----
 src/tests/cmocka/test_ipa_idmap.c  | 26 ++++++++++++++++----------
 src/util/util_errors.c             |  1 +
 src/util/util_errors.h             |  1 +
 5 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/src/providers/ipa/ipa_idmap.c b/src/providers/ipa/ipa_idmap.c
index 4e68310822..3d8592e7c6 100644
--- a/src/providers/ipa/ipa_idmap.c
+++ b/src/providers/ipa/ipa_idmap.c
@@ -204,7 +204,7 @@ errno_t get_idmap_data_from_range(struct range_info *r, char *domain_name,
             DEBUG(SSSDBG_MINOR_FAILURE, "Range type [%s] of id range " \
                                         "[%s] not supported.\n", \
                                         r->range_type, r->name);
-            return EINVAL;
+            return ERR_UNSUPPORTED_RANGE_TYPE;
         }
     }
 
diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
index 82dd1a1f43..d67d2c2b79 100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -173,6 +173,7 @@ static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
     struct range_info *r;
     const char *value;
     size_t c;
+    size_t rc = 0;
     size_t d;
     int ret;
     enum idmap_error_code err;
@@ -283,11 +284,14 @@ static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
 
         ret = get_idmap_data_from_range(r, domain_name, &name1, &sid1, &rid1,
                                         &range1, &mapping1);
-        if (ret != EOK) {
+        if (ret == ERR_UNSUPPORTED_RANGE_TYPE) {
+            talloc_free(r);
+            continue;
+        } else if (ret != EOK) {
             DEBUG(SSSDBG_OP_FAILURE, "get_idmap_data_from_range failed.\n");
             goto done;
         }
-        for (d = 0; d < c; d++) {
+        for (d = 0; d < rc; d++) {
             ret = get_idmap_data_from_range(range_list[d], domain_name, &name2,
                                             &sid2, &rid2, &range2, &mapping2);
             if (ret != EOK) {
@@ -309,10 +313,10 @@ static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
             }
         }
 
-        range_list[c] = r;
+        range_list[rc++] = r;
     }
 
-    range_list[c] = NULL;
+    range_list[rc] = NULL;
 
     *_range_list = range_list;
     ret = EOK;
diff --git a/src/tests/cmocka/test_ipa_idmap.c b/src/tests/cmocka/test_ipa_idmap.c
index 6b1a15642d..0409792b21 100644
--- a/src/tests/cmocka/test_ipa_idmap.c
+++ b/src/tests/cmocka/test_ipa_idmap.c
@@ -81,23 +81,29 @@ void test_get_idmap_data_from_range(void **state)
         {{RANGE_NAME, BASE_ID, RANGE_SIZE, BASE_RID, 0, DOMAIN_SID,
           discard_const(IPA_RANGE_AD_TRUST_POSIX)},
          EOK, DOMAIN_SID, DOMAIN_SID, 0, {BASE_ID, RANGE_MAX}, true},
+        /* IPA_RANGE_AD_TRUST range  with unsupported type */
+        {{RANGE_NAME, BASE_ID, RANGE_SIZE, BASE_RID, 0, DOMAIN_SID,
+          discard_const("unsupported-range")},
+         ERR_UNSUPPORTED_RANGE_TYPE, NULL, NULL, 0, {0, 0}, false},
         {{0}, 0, NULL, NULL, 0, {0, 0}, false}
     };
 
-    for (c = 0; d[c].exp_dom_name != NULL; c++) {
+    for (c = 0; d[c].exp_dom_name != NULL || d[c].exp_ret != 0; c++) {
         ret = get_idmap_data_from_range(&d[c].r, DOMAIN_NAME, &dom_name, &sid,
                                         &rid, &range, &external_mapping);
         assert_int_equal(ret, d[c].exp_ret);
-        assert_string_equal(dom_name, d[c].exp_dom_name);
-        if (d[c].exp_sid == NULL) {
-            assert_null(sid);
-        } else {
-            assert_string_equal(sid, d[c].exp_sid);
+        if (ret == 0) {
+            assert_string_equal(dom_name, d[c].exp_dom_name);
+            if (d[c].exp_sid == NULL) {
+                assert_null(sid);
+            } else {
+                assert_string_equal(sid, d[c].exp_sid);
+            }
+            assert_int_equal(rid, d[c].exp_rid);
+            assert_int_equal(range.min, d[c].exp_range.min);
+            assert_int_equal(range.max, d[c].exp_range.max);
+            assert_true(external_mapping == d[c].exp_external_mapping);
         }
-        assert_int_equal(rid, d[c].exp_rid);
-        assert_int_equal(range.min, d[c].exp_range.min);
-        assert_int_equal(range.max, d[c].exp_range.max);
-        assert_true(external_mapping == d[c].exp_external_mapping);
     }
 }
 
diff --git a/src/util/util_errors.c b/src/util/util_errors.c
index b5c7419a94..c2d52f73b8 100644
--- a/src/util/util_errors.c
+++ b/src/util/util_errors.c
@@ -124,6 +124,7 @@ struct err_string error_to_str[] = {
     { "ID is outside the allowed range" }, /* ERR_ID_OUTSIDE_RANGE */
     { "Group ID is duplicated" }, /* ERR_GID_DUPLICATED */
     { "Multiple objects were found when only one was expected" }, /* ERR_MULTIPLE_ENTRIES */
+    { "Unsupported range type" }, /* ERR_UNSUPPORTED_RANGE_TYPE */
 
     /* DBUS Errors */
     { "Connection was killed on demand" }, /* ERR_SBUS_KILL_CONNECTION */
diff --git a/src/util/util_errors.h b/src/util/util_errors.h
index 75c46286af..9ff1ac26b4 100644
--- a/src/util/util_errors.h
+++ b/src/util/util_errors.h
@@ -145,6 +145,7 @@ enum sssd_errors {
     ERR_ID_OUTSIDE_RANGE,
     ERR_GID_DUPLICATED,
     ERR_MULTIPLE_ENTRIES,
+    ERR_UNSUPPORTED_RANGE_TYPE,
 
     /* DBUS Errors */
     ERR_SBUS_KILL_CONNECTION,

From 979ccaca7deff5cb9593f40192b1bd51863c2886 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Thu, 8 Apr 2021 10:56:32 +0200
Subject: [PATCH 2/3] ipa: add unit test for ipa_ranges_parse_results

A unit test is added to check if unknown range types are properly
skipped. For this ipa_ranges_parse_results() is made public and moved to
a source file which is already used in a unit test to avoid the
inclusion of additional dependencies.

Resolves: https://github.com/SSSD/sssd/issues/5571
---
 src/providers/ipa/ipa_common.h     |  14 +++
 src/providers/ipa/ipa_idmap.c      | 166 +++++++++++++++++++++++++++
 src/providers/ipa/ipa_subdomains.c | 173 -----------------------------
 src/tests/cmocka/test_ipa_idmap.c  |  97 ++++++++++++++++
 4 files changed, 277 insertions(+), 173 deletions(-)

diff --git a/src/providers/ipa/ipa_common.h b/src/providers/ipa/ipa_common.h
index 6bb1739efb..db382ab8ba 100644
--- a/src/providers/ipa/ipa_common.h
+++ b/src/providers/ipa/ipa_common.h
@@ -29,6 +29,14 @@
 #include "providers/ad/ad_common.h"
 #include "providers/ad/ad_srv.h"
 
+#define IPA_CN "cn"
+#define IPA_TRUSTED_DOMAIN_SID "ipaNTTrustedDomainSID"
+#define IPA_RANGE_TYPE "ipaRangeType"
+#define IPA_BASE_ID "ipaBaseID"
+#define IPA_ID_RANGE_SIZE "ipaIDRangeSize"
+#define IPA_BASE_RID "ipaBaseRID"
+#define IPA_SECONDARY_BASE_RID "ipaSecondaryBaseRID"
+
 struct ipa_service {
     struct sdap_service *sdap;
     struct krb5_service *krb5_service;
@@ -284,6 +292,12 @@ errno_t get_idmap_data_from_range(struct range_info *r, char *domain_name,
                                   struct sss_idmap_range *_range,
                                   bool *_external_mapping);
 
+errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
+                                 char *domain_name,
+                                 size_t count,
+                                 struct sysdb_attrs **reply,
+                                 struct range_info ***_range_list);
+
 errno_t ipa_idmap_get_ranges_from_sysdb(struct sdap_idmap_ctx *idmap_ctx,
                                         const char *dom_name,
                                         const char *dom_sid_str,
diff --git a/src/providers/ipa/ipa_idmap.c b/src/providers/ipa/ipa_idmap.c
index 3d8592e7c6..7e65ed02a4 100644
--- a/src/providers/ipa/ipa_idmap.c
+++ b/src/providers/ipa/ipa_idmap.c
@@ -214,6 +214,172 @@ errno_t get_idmap_data_from_range(struct range_info *r, char *domain_name,
     return EOK;
 }
 
+errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
+                                 char *domain_name,
+                                 size_t count,
+                                 struct sysdb_attrs **reply,
+                                 struct range_info ***_range_list)
+{
+    struct range_info **range_list = NULL;
+    struct range_info *r;
+    const char *value;
+    size_t c;
+    size_t rc = 0;
+    size_t d;
+    int ret;
+    enum idmap_error_code err;
+    char *name1;
+    char *name2;
+    char *sid1;
+    char *sid2;
+    uint32_t rid1;
+    uint32_t rid2;
+    struct sss_idmap_range range1;
+    struct sss_idmap_range range2;
+    bool mapping1;
+    bool mapping2;
+
+    range_list = talloc_array(mem_ctx, struct range_info *, count + 1);
+    if (range_list == NULL) {
+        DEBUG(SSSDBG_OP_FAILURE, "talloc_array failed.\n");
+        return ENOMEM;
+    }
+
+    for (c = 0; c < count; c++) {
+        r = talloc_zero(range_list, struct range_info);
+        if (r == NULL) {
+            DEBUG(SSSDBG_OP_FAILURE, "talloc_zero failed.\n");
+            ret = ENOMEM;
+            goto done;
+        }
+
+        ret = sysdb_attrs_get_string(reply[c], IPA_CN, &value);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
+            goto done;
+        }
+
+        r->name = talloc_strdup(r, value);
+        if (r->name == NULL) {
+            DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
+            ret = ENOMEM;
+            goto done;
+        }
+
+        ret = sysdb_attrs_get_string(reply[c], IPA_TRUSTED_DOMAIN_SID, &value);
+        if (ret == EOK) {
+            r->trusted_dom_sid = talloc_strdup(r, value);
+            if (r->trusted_dom_sid == NULL) {
+                DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
+                ret = ENOMEM;
+                goto done;
+            }
+        } else if (ret != ENOENT) {
+            DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
+            goto done;
+        }
+
+        ret = sysdb_attrs_get_uint32_t(reply[c], IPA_BASE_ID,
+                                       &r->base_id);
+        if (ret != EOK && ret != ENOENT) {
+            DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
+            goto done;
+        }
+
+        ret = sysdb_attrs_get_uint32_t(reply[c], IPA_ID_RANGE_SIZE,
+                                       &r->id_range_size);
+        if (ret != EOK && ret != ENOENT) {
+            DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
+            goto done;
+        }
+
+        ret = sysdb_attrs_get_uint32_t(reply[c], IPA_BASE_RID,
+                                       &r->base_rid);
+        if (ret != EOK && ret != ENOENT) {
+            DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
+            goto done;
+        }
+
+        ret = sysdb_attrs_get_uint32_t(reply[c], IPA_SECONDARY_BASE_RID,
+                                       &r->secondary_base_rid);
+        if (ret != EOK && ret != ENOENT) {
+            DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
+            goto done;
+        }
+
+        ret = sysdb_attrs_get_string(reply[c], IPA_RANGE_TYPE, &value);
+        if (ret == EOK) {
+            r->range_type = talloc_strdup(r, value);
+            if (r->range_type == NULL) {
+                DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
+                ret = ENOMEM;
+                goto done;
+            }
+        } else if (ret == ENOENT) {
+            /* Older IPA servers might not have the range_type attribute, but
+             * only support local ranges and trusts with algorithmic mapping. */
+            if (r->trusted_dom_sid == NULL) {
+                r->range_type = talloc_strdup(r, IPA_RANGE_LOCAL);
+            } else {
+                r->range_type = talloc_strdup(r, IPA_RANGE_AD_TRUST);
+            }
+        } else {
+            DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
+            goto done;
+        }
+        if (r->range_type == NULL) {
+            DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
+            ret = ENOMEM;
+            goto done;
+        }
+
+        ret = get_idmap_data_from_range(r, domain_name, &name1, &sid1, &rid1,
+                                        &range1, &mapping1);
+        if (ret == ERR_UNSUPPORTED_RANGE_TYPE) {
+            talloc_free(r);
+            continue;
+        } else if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, "get_idmap_data_from_range failed.\n");
+            goto done;
+        }
+        for (d = 0; d < rc; d++) {
+            ret = get_idmap_data_from_range(range_list[d], domain_name, &name2,
+                                            &sid2, &rid2, &range2, &mapping2);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_OP_FAILURE,
+                      "get_idmap_data_from_range failed.\n");
+                goto done;
+            }
+
+            err = sss_idmap_check_collision_ex(name1, sid1, &range1, rid1,
+                                               r->name, mapping1,
+                                               name2, sid2, &range2, rid2,
+                                               range_list[d]->name, mapping2);
+            if (err != IDMAP_SUCCESS) {
+                DEBUG(SSSDBG_CRIT_FAILURE,
+                      "Collision of ranges [%s] and [%s] detected.\n",
+                      r->name, range_list[d]->name);
+                ret = EINVAL;
+                goto done;
+            }
+        }
+
+        range_list[rc++] = r;
+    }
+
+    range_list[rc] = NULL;
+
+    *_range_list = range_list;
+    ret = EOK;
+
+done:
+    if (ret != EOK) {
+        talloc_free(range_list);
+    }
+
+    return ret;
+}
+
 errno_t ipa_idmap_get_ranges_from_sysdb(struct sdap_idmap_ctx *idmap_ctx,
                                         const char *dom_name,
                                         const char *dom_sid_str,
diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
index d67d2c2b79..17a1876798 100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -37,18 +37,11 @@
 #define MASTER_DOMAIN_FILTER "objectclass=ipaNTDomainAttrs"
 #define RANGE_FILTER "objectclass=ipaIDRange"
 
-#define IPA_CN "cn"
 #define IPA_FLATNAME "ipaNTFlatName"
 #define IPA_SID "ipaNTSecurityIdentifier"
-#define IPA_TRUSTED_DOMAIN_SID "ipaNTTrustedDomainSID"
-#define IPA_RANGE_TYPE "ipaRangeType"
 #define IPA_ADDITIONAL_SUFFIXES "ipaNTAdditionalSuffixes"
 #define IPA_SID_BLACKLIST_INCOMING "ipaNTSIDBlacklistIncoming"
 
-#define IPA_BASE_ID "ipaBaseID"
-#define IPA_ID_RANGE_SIZE "ipaIDRangeSize"
-#define IPA_BASE_RID "ipaBaseRID"
-#define IPA_SECONDARY_BASE_RID "ipaSecondaryBaseRID"
 #define OBJECTCLASS "objectClass"
 
 #define IPA_ASSIGNED_ID_VIEW "ipaAssignedIDView"
@@ -163,172 +156,6 @@ ipa_subdom_reinit(struct ipa_subdomains_ctx *ctx)
     return EOK;
 }
 
-static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
-                                        char *domain_name,
-                                        size_t count,
-                                        struct sysdb_attrs **reply,
-                                        struct range_info ***_range_list)
-{
-    struct range_info **range_list = NULL;
-    struct range_info *r;
-    const char *value;
-    size_t c;
-    size_t rc = 0;
-    size_t d;
-    int ret;
-    enum idmap_error_code err;
-    char *name1;
-    char *name2;
-    char *sid1;
-    char *sid2;
-    uint32_t rid1;
-    uint32_t rid2;
-    struct sss_idmap_range range1;
-    struct sss_idmap_range range2;
-    bool mapping1;
-    bool mapping2;
-
-    range_list = talloc_array(mem_ctx, struct range_info *, count + 1);
-    if (range_list == NULL) {
-        DEBUG(SSSDBG_OP_FAILURE, "talloc_array failed.\n");
-        return ENOMEM;
-    }
-
-    for (c = 0; c < count; c++) {
-        r = talloc_zero(range_list, struct range_info);
-        if (r == NULL) {
-            DEBUG(SSSDBG_OP_FAILURE, "talloc_zero failed.\n");
-            ret = ENOMEM;
-            goto done;
-        }
-
-        ret = sysdb_attrs_get_string(reply[c], IPA_CN, &value);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
-            goto done;
-        }
-
-        r->name = talloc_strdup(r, value);
-        if (r->name == NULL) {
-            DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
-            ret = ENOMEM;
-            goto done;
-        }
-
-        ret = sysdb_attrs_get_string(reply[c], IPA_TRUSTED_DOMAIN_SID, &value);
-        if (ret == EOK) {
-            r->trusted_dom_sid = talloc_strdup(r, value);
-            if (r->trusted_dom_sid == NULL) {
-                DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
-                ret = ENOMEM;
-                goto done;
-            }
-        } else if (ret != ENOENT) {
-            DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
-            goto done;
-        }
-
-        ret = sysdb_attrs_get_uint32_t(reply[c], IPA_BASE_ID,
-                                       &r->base_id);
-        if (ret != EOK && ret != ENOENT) {
-            DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
-            goto done;
-        }
-
-        ret = sysdb_attrs_get_uint32_t(reply[c], IPA_ID_RANGE_SIZE,
-                                       &r->id_range_size);
-        if (ret != EOK && ret != ENOENT) {
-            DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
-            goto done;
-        }
-
-        ret = sysdb_attrs_get_uint32_t(reply[c], IPA_BASE_RID,
-                                       &r->base_rid);
-        if (ret != EOK && ret != ENOENT) {
-            DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
-            goto done;
-        }
-
-        ret = sysdb_attrs_get_uint32_t(reply[c], IPA_SECONDARY_BASE_RID,
-                                       &r->secondary_base_rid);
-        if (ret != EOK && ret != ENOENT) {
-            DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
-            goto done;
-        }
-
-        ret = sysdb_attrs_get_string(reply[c], IPA_RANGE_TYPE, &value);
-        if (ret == EOK) {
-            r->range_type = talloc_strdup(r, value);
-            if (r->range_type == NULL) {
-                DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
-                ret = ENOMEM;
-                goto done;
-            }
-        } else if (ret == ENOENT) {
-            /* Older IPA servers might not have the range_type attribute, but
-             * only support local ranges and trusts with algorithmic mapping. */
-            if (r->trusted_dom_sid == NULL) {
-                r->range_type = talloc_strdup(r, IPA_RANGE_LOCAL);
-            } else {
-                r->range_type = talloc_strdup(r, IPA_RANGE_AD_TRUST);
-            }
-        } else {
-            DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
-            goto done;
-        }
-        if (r->range_type == NULL) {
-            DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
-            ret = ENOMEM;
-            goto done;
-        }
-
-        ret = get_idmap_data_from_range(r, domain_name, &name1, &sid1, &rid1,
-                                        &range1, &mapping1);
-        if (ret == ERR_UNSUPPORTED_RANGE_TYPE) {
-            talloc_free(r);
-            continue;
-        } else if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, "get_idmap_data_from_range failed.\n");
-            goto done;
-        }
-        for (d = 0; d < rc; d++) {
-            ret = get_idmap_data_from_range(range_list[d], domain_name, &name2,
-                                            &sid2, &rid2, &range2, &mapping2);
-            if (ret != EOK) {
-                DEBUG(SSSDBG_OP_FAILURE,
-                      "get_idmap_data_from_range failed.\n");
-                goto done;
-            }
-
-            err = sss_idmap_check_collision_ex(name1, sid1, &range1, rid1,
-                                               r->name, mapping1,
-                                               name2, sid2, &range2, rid2,
-                                               range_list[d]->name, mapping2);
-            if (err != IDMAP_SUCCESS) {
-                DEBUG(SSSDBG_CRIT_FAILURE,
-                      "Collision of ranges [%s] and [%s] detected.\n",
-                      r->name, range_list[d]->name);
-                ret = EINVAL;
-                goto done;
-            }
-        }
-
-        range_list[rc++] = r;
-    }
-
-    range_list[rc] = NULL;
-
-    *_range_list = range_list;
-    ret = EOK;
-
-done:
-    if (ret != EOK) {
-        talloc_free(range_list);
-    }
-
-    return ret;
-}
-
 struct priv_sss_debug {
     int level;
 };
diff --git a/src/tests/cmocka/test_ipa_idmap.c b/src/tests/cmocka/test_ipa_idmap.c
index 0409792b21..dd15b5b132 100644
--- a/src/tests/cmocka/test_ipa_idmap.c
+++ b/src/tests/cmocka/test_ipa_idmap.c
@@ -218,6 +218,101 @@ void test_ipa_idmap_get_ranges_from_sysdb(void **state)
     assert_int_equal(ret, EIO);
 }
 
+struct sysdb_attrs *create_range_attrs(TALLOC_CTX *mem_ctx,
+                                       struct range_info *r)
+{
+    int ret;
+    struct sysdb_attrs *a = NULL;
+
+    a = sysdb_new_attrs(mem_ctx);
+    assert_non_null(a);
+
+    ret = sysdb_attrs_add_string(a, IPA_CN, r->name);
+
+    if (ret == 0) {
+        ret = sysdb_attrs_add_string(a, IPA_TRUSTED_DOMAIN_SID,
+                                     r->trusted_dom_sid);
+    }
+
+    if (ret == 0) {
+        ret = sysdb_attrs_add_uint32(a, IPA_BASE_ID, r->base_id);
+    }
+
+    if (ret == 0) {
+        ret = sysdb_attrs_add_uint32(a, IPA_ID_RANGE_SIZE, r->id_range_size);
+    }
+
+    if (ret == 0) {
+        ret = sysdb_attrs_add_uint32(a, IPA_BASE_RID, r->base_rid);
+    }
+
+    if (ret == 0) {
+        ret = sysdb_attrs_add_uint32(a, IPA_SECONDARY_BASE_RID,
+                                     r->secondary_base_rid);
+    }
+
+    if (ret == 0) {
+        ret = sysdb_attrs_add_string(a, IPA_RANGE_TYPE, r->range_type);
+    }
+
+    if (ret != 0) {
+        talloc_zfree(a);
+    }
+
+    return a;
+
+}
+
+
+void test_ipa_ranges_parse_results(void **state)
+{
+    struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
+    assert_non_null(test_ctx);
+    int ret;
+    size_t count = 5;
+    size_t c;
+    size_t d;
+    struct sysdb_attrs *reply[5];
+    struct range_info **range_list;
+    struct range_info r[5] = {
+        { discard_const("range1"), 1000, 500, 0, 1000, discard_const("S-1-2-1"), discard_const(IPA_RANGE_AD_TRUST) },
+        { discard_const("range2"), 2000, 500, 0, 2000, discard_const("S-1-2-2"), discard_const(IPA_RANGE_AD_TRUST) },
+        { discard_const("range3"), 3000, 500, 0, 3000, discard_const("S-1-2-3"), discard_const("unsupported-type") },
+        { discard_const("range4"), 4000, 500, 0, 4000, discard_const("S-1-2-4"), discard_const(IPA_RANGE_AD_TRUST) },
+        { discard_const("range5"), 5000, 500, 0, 5000, discard_const("S-1-2-5"), discard_const(IPA_RANGE_AD_TRUST) }
+    };
+
+    for (c = 0; c < count; c++) {
+        reply[c] = create_range_attrs(test_ctx, &r[c]);
+        assert_non_null(reply[c]);
+    }
+
+    ret = ipa_ranges_parse_results(test_ctx, discard_const("mydom"),
+                                   count, reply, &range_list);
+    for (c = 0; c < count; c++) {
+        talloc_free(reply[c]);
+    }
+    assert_int_equal(ret, EOK);
+    d = 0;
+    for (c = 0; c < count; c++) {
+        if (strcmp(r[c].range_type, "unsupported-type") == 0) {
+            continue;
+        }
+        assert_string_equal(r[c].name, range_list[d]->name);
+        assert_string_equal(r[c].trusted_dom_sid,
+                            range_list[d]->trusted_dom_sid);
+        assert_string_equal(r[c].range_type, range_list[d]->range_type);
+        assert_int_equal(r[c].base_id, range_list[d]->base_id);
+        assert_int_equal(r[c].id_range_size, range_list[d]->id_range_size);
+        assert_int_equal(r[c].base_rid, range_list[d]->base_rid);
+        assert_int_equal(r[c].secondary_base_rid,
+                         range_list[d]->secondary_base_rid);
+        d++;
+    }
+
+    talloc_free(range_list);
+}
+
 int main(int argc, const char *argv[])
 {
     poptContext pc;
@@ -232,6 +327,8 @@ int main(int argc, const char *argv[])
         cmocka_unit_test(test_get_idmap_data_from_range),
         cmocka_unit_test_setup_teardown(test_ipa_idmap_get_ranges_from_sysdb,
                                         setup_idmap_ctx, teardown_idmap_ctx),
+        cmocka_unit_test_setup_teardown(test_ipa_ranges_parse_results,
+                                        setup_idmap_ctx, teardown_idmap_ctx),
     };
 
     /* Set debug level to invalid value so we can decide if -d 0 was used. */

From 735e891c05ab692fceb5094cd2b0d3cb197055d1 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Thu, 8 Apr 2021 18:11:02 +0200
Subject: [PATCH 3/3] ipa subdomains: do not fail completely if one step fails

Currently while updating server side data stored on an IPA server
during a subdomains request the whole request will fail if a single step
fails. As a result the remaining server side data which would have been
looked up after the failed attempt are missing.

With this patch a failure in a single lookup is not considered fatal and
SSSD will try to read the remaining data after an error occurred.

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

:fixes: During the IPA subdomains request a failure in reading a single
    specific configuration option is not considered fatal and the
    request will continue
---
 src/providers/ipa/ipa_subdomains.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
index 17a1876798..252c25efc9 100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -2714,8 +2714,7 @@ static void ipa_subdomains_refresh_ranges_done(struct tevent_req *subreq)
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, "Unable to get IPA ranges "
               "[%d]: %s\n", ret, sss_strerror(ret));
-        tevent_req_error(req, ret);
-        return;
+        /* Not good, but let's try to continue with other server side options */
     }
 
     subreq = ipa_subdomains_certmap_send(state, state->ev, state->sd_ctx,
@@ -2743,8 +2742,7 @@ static void ipa_subdomains_refresh_certmap_done(struct tevent_req *subreq)
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, "Failed to read certificate mapping rules "
               "[%d]: %s\n", ret, sss_strerror(ret));
-        tevent_req_error(req, ret);
-        return;
+        /* Not good, but let's try to continue with other server side options */
     }
 
     subreq = ipa_subdomains_master_send(state, state->ev, state->sd_ctx,
@@ -2772,8 +2770,7 @@ static void ipa_subdomains_refresh_master_done(struct tevent_req *subreq)
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, "Unable to get master domain "
               "[%d]: %s\n", ret, sss_strerror(ret));
-        tevent_req_error(req, ret);
-        return;
+        /* Not good, but let's try to continue with other server side options */
     }
 
     subreq = ipa_subdomains_slave_send(state, state->ev, state->sd_ctx,
@@ -2801,8 +2798,7 @@ static void ipa_subdomains_refresh_slave_done(struct tevent_req *subreq)
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, "Unable to get subdomains "
               "[%d]: %s\n", ret, sss_strerror(ret));
-        tevent_req_error(req, ret);
-        return;
+        /* Not good, but let's try to continue with other server side options */
     }
 
     subreq = ipa_subdomains_view_name_send(state, state->ev, state->sd_ctx,
@@ -2832,8 +2828,7 @@ static void ipa_subdomains_refresh_view_name_done(struct tevent_req *subreq)
         DEBUG(SSSDBG_CRIT_FAILURE,
               "Unable to get view name [%d]: %s\n",
               ret, sss_strerror(ret));
-        tevent_req_error(req, ret);
-        return;
+        /* Not good, but let's try to continue with other server side options */
     }
 
     subreq = ipa_subdomains_view_domain_resolution_order_send(
@@ -2867,8 +2862,7 @@ ipa_subdomains_refresh_view_domain_resolution_order_done(struct tevent_req *subr
         DEBUG(SSSDBG_CRIT_FAILURE,
               "Unable to get view domain_resolution order [%d]: %s\n",
               ret, sss_strerror(ret));
-        tevent_req_error(req, ret);
-        return;
+        /* Not good, but let's try to continue with other server side options */
     }
 
     subreq = ipa_domain_resolution_order_send(state, state->ev, state->sd_ctx,
@@ -2900,8 +2894,7 @@ ipa_domain_refresh_resolution_order_done(struct tevent_req *subreq)
         DEBUG(SSSDBG_MINOR_FAILURE,
               "Unable to get the domains order resolution [%d]: %s\n",
               ret, sss_strerror(ret));
-        tevent_req_error(req, ret);
-        return;
+        /* Not good, but let's try to continue with other server side options */
     }
 
     ret = sdap_id_op_done(state->sdap_op, ret, &dp_error);
_______________________________________________
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