[SSSD] [PATCH] IPA: check ranges for collisions before saving them

Sumit Bose sbose at redhat.com
Tue Feb 25 16:15:18 UTC 2014


On Mon, Feb 24, 2014 at 07:12:00PM +0100, Jakub Hrozek wrote:
> On Thu, Feb 20, 2014 at 10:34:44AM +0100, Sumit Bose wrote:
> > On Wed, Feb 19, 2014 at 04:23:55PM +0100, Jakub Hrozek wrote:
> > > On Wed, Feb 12, 2014 at 03:43:51PM +0100, Sumit Bose wrote:
> > > > Hi,
> > > > 
> > > > currently we trust the idrange data we get from the IPA server. But it
> > > > turned out that some checks are missing on the server
> > > > (https://fedorahosted.org/freeipa/ticket/4137) so SSSD should check the
> > > > input before saving the data to the cache.
> > > > 
> > > > The first two patches make some code which already exists available for
> > > > other callers. The third contains the actual functionality.
> > > > 
> > > > bye,
> > > > Sumit
> > > 
> > > Hi, I rebased your patches on top of master, can you check if they look
> > > OK to you and resend if they are fine? I'll continue testing with my
> > > version in the meantime.
> > 
> > sorry I forgot to send a version for master myself. The patches are
> > exactly the same as in my master tree.
> > 
> > Thanks
> > 
> > bye,
> > Sumit
> 

...

> >          err = sss_idmap_add_domain_ex(idmap_ctx->map, name, sid, &range,
> > -                                      r->name, rid, external_mapping);
> > -        if (err != IDMAP_SUCCESS && err != IDMAP_COLLISION) {
> > +                                      range_list[c]->name, rid,
> > +                                      external_mapping);
> > +        if (err != IDMAP_SUCCESS
> > +                && allow_collisions && err != IDMAP_COLLISION) {
> 
> I don't think this condition is right, if allow_collisions is false,
> then it never matches.

good catch, thank you, fixed.

> 
> >              DEBUG(SSSDBG_CRIT_FAILURE, "Could not add range [%s] to ID map\n",
> > -                                        r->name);
> > +                                       range_list[c]->name);
> >              ret = EIO;
> >              goto done;
> >          }
> >      }
> 
> [snip]
> 
> > diff --git a/src/tests/cmocka/test_ipa_idmap.c b/src/tests/cmocka/test_ipa_idmap.c
> 
> [snip]
> 
> > +int main(int argc, const char *argv[])
> > +{
> > +    poptContext pc;
> > +    int opt;
> > +    struct poptOption long_options[] = {
> > +        POPT_AUTOHELP
> > +        SSSD_DEBUG_OPTS
> > +        POPT_TABLEEND
> > +    };
> > +
> > +    const UnitTest tests[] = {
> > +        //unit_test_setup_teardown(test_add_domain,
> > +        //                         test_sss_idmap_setup, test_sss_idmap_teardown),
> 
> It seems that no further patch includes test_add_domain either, did you
> want to remove the commented out code?
> 

yes, I removed it and added a new test to make sure the allow_collisions
check above is working as expected.

New version for master and 1.11 are attached.

bye,
Sumit
-------------- next part --------------
From d7aaf467f0b30be679117bfe7b00decac8f6d67a Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Mon, 3 Feb 2014 13:30:35 +0100
Subject: [PATCH 1/3] IDMAP: add sss_idmap_check_collision(_ex)

---
 src/lib/idmap/sss_idmap.c         | 123 ++++++++++++++++++++++++++------------
 src/lib/idmap/sss_idmap.h         |  65 ++++++++++++++++++++
 src/tests/cmocka/test_sss_idmap.c |  93 ++++++++++++++++++++++++++++
 3 files changed, 244 insertions(+), 37 deletions(-)

diff --git a/src/lib/idmap/sss_idmap.c b/src/lib/idmap/sss_idmap.c
index 3f1e7a5..4c45312 100644
--- a/src/lib/idmap/sss_idmap.c
+++ b/src/lib/idmap/sss_idmap.c
@@ -380,55 +380,104 @@ enum idmap_error_code sss_idmap_calculate_range(struct sss_idmap_ctx *ctx,
     return IDMAP_SUCCESS;
 }
 
-static enum idmap_error_code dom_check_collision(
-                                             struct idmap_domain_info *dom_list,
-                                             struct idmap_domain_info *new_dom)
+enum idmap_error_code sss_idmap_check_collision_ex(const char *o_name,
+                                                const char *o_sid,
+                                                struct sss_idmap_range *o_range,
+                                                uint32_t o_first_rid,
+                                                const char *o_range_id,
+                                                bool o_external_mapping,
+                                                const char *n_name,
+                                                const char *n_sid,
+                                                struct sss_idmap_range *n_range,
+                                                uint32_t n_first_rid,
+                                                const char *n_range_id,
+                                                bool n_external_mapping)
 {
-    struct idmap_domain_info *dom;
     bool names_equal;
     bool sids_equal;
 
-    for (dom = dom_list; dom != NULL; dom = dom->next) {
+    /* TODO: if both ranges have the same ID check if an update is
+     * needed. */
+
+    /* Check if ID ranges overlap.
+     * ID ranges with external mapping may overlap. */
+    if ((!n_external_mapping && !o_external_mapping)
+        && ((n_range->min >= o_range->min
+                && n_range->min <= o_range->max)
+            || (n_range->max >= o_range->min
+                && n_range->max <= o_range->max))) {
+        return IDMAP_COLLISION;
+    }
 
-        /* TODO: if both ranges have the same ID check if an update is
-         * needed. */
-
-        /* Check if ID ranges overlap.
-         * ID ranges with external mapping may overlap. */
-        if ((!new_dom->external_mapping && !dom->external_mapping)
-            && ((new_dom->range->min >= dom->range->min
-                    && new_dom->range->min <= dom->range->max)
-                || (new_dom->range->max >= dom->range->min
-                    && new_dom->range->max <= dom->range->max))) {
-            return IDMAP_COLLISION;
-        }
+    names_equal = (strcasecmp(n_name, o_name) == 0);
+    sids_equal = ((n_sid == NULL && o_sid == NULL)
+                    || (n_sid != NULL && o_sid != NULL
+                        && strcasecmp(n_sid, o_sid) == 0));
+
+    /* check if domain name and SID are consistent */
+    if ((names_equal && !sids_equal) || (!names_equal && sids_equal)) {
+        return IDMAP_COLLISION;
+    }
 
-        names_equal = (strcasecmp(new_dom->name, dom->name) == 0);
-        sids_equal = ((new_dom->sid == NULL && dom->sid == NULL)
-                        || (new_dom->sid != NULL && dom->sid != NULL
-                            && strcasecmp(new_dom->sid, dom->sid) == 0));
+    /* check if external_mapping is consistent */
+    if (names_equal && sids_equal
+            && n_external_mapping != o_external_mapping) {
+        return IDMAP_COLLISION;
+    }
 
-        /* check if domain name and SID are consistent */
-        if ((names_equal && !sids_equal) || (!names_equal && sids_equal)) {
-            return IDMAP_COLLISION;
-        }
+    /* check if RID ranges overlap */
+    if (names_equal && sids_equal
+            && n_external_mapping == false
+            && n_first_rid >= o_first_rid
+            && n_first_rid <= o_first_rid + (o_range->max - o_range->min)) {
+        return IDMAP_COLLISION;
+    }
 
-        /* check if external_mapping is consistent */
-        if (names_equal && sids_equal
-                && new_dom->external_mapping != dom->external_mapping) {
-            return IDMAP_COLLISION;
-        }
+    return IDMAP_SUCCESS;
+}
 
-        /* check if RID ranges overlap */
-        if (names_equal && sids_equal
-                && new_dom->external_mapping == false
-                && new_dom->first_rid >= dom->first_rid
-                && new_dom->first_rid <=
-                    dom->first_rid + (dom->range->max - dom->range->min)) {
-            return IDMAP_COLLISION;
+enum idmap_error_code sss_idmap_check_collision(struct sss_idmap_ctx *ctx,
+                                                char *n_name, char *n_sid,
+                                                struct sss_idmap_range *n_range,
+                                                uint32_t n_first_rid,
+                                                char *n_range_id,
+                                                bool n_external_mapping)
+{
+    struct idmap_domain_info *dom;
+    enum idmap_error_code err;
+
+    for (dom = ctx->idmap_domain_info; dom != NULL; dom = dom->next) {
+        err = sss_idmap_check_collision_ex(dom->name, dom->sid, dom->range,
+                                           dom->first_rid, dom->range_id,
+                                           dom->external_mapping,
+                                           n_name, n_sid, n_range, n_first_rid,
+                                           n_range_id, n_external_mapping);
+        if (err != IDMAP_SUCCESS) {
+            return err;
         }
     }
+    return IDMAP_SUCCESS;
+}
 
+static enum idmap_error_code dom_check_collision(
+                                             struct idmap_domain_info *dom_list,
+                                             struct idmap_domain_info *new_dom)
+{
+    struct idmap_domain_info *dom;
+    enum idmap_error_code err;
+
+    for (dom = dom_list; dom != NULL; dom = dom->next) {
+        err = sss_idmap_check_collision_ex(dom->name, dom->sid, dom->range,
+                                           dom->first_rid, dom->range_id,
+                                           dom->external_mapping,
+                                           new_dom->name, new_dom->sid,
+                                           new_dom->range, new_dom->first_rid,
+                                           new_dom->range_id,
+                                           new_dom->external_mapping);
+        if (err != IDMAP_SUCCESS) {
+            return err;
+        }
+    }
     return IDMAP_SUCCESS;
 }
 
diff --git a/src/lib/idmap/sss_idmap.h b/src/lib/idmap/sss_idmap.h
index fe1e3c1..0797083 100644
--- a/src/lib/idmap/sss_idmap.h
+++ b/src/lib/idmap/sss_idmap.h
@@ -289,6 +289,71 @@ enum idmap_error_code sss_idmap_add_domain_ex(struct sss_idmap_ctx *ctx,
                                               const char *range_id,
                                               uint32_t rid,
                                               bool external_mapping);
+
+/**
+ * @brief Check if a new range would collide with any existing one
+ *
+ * @param[in] ctx         Idmap context
+ * @param[in] n_name      Zero-terminated string with the domain name the new
+ *                        range should belong to
+ * @param[in] n_sid       Zero-terminated string representation of the domain
+ *                        SID (S-1-15-.....) the new range sould belong to
+ * @param[in] n_range     The new id range
+ * @param[in] n_range_id  unique identifier of the new range, it is needed
+ *                        to allow updates at runtime, may be NULL
+ * @param[in] n_first_rid The RID that should be mapped to the first ID of the
+ *                        new range.
+ * @param[in] n_external_mapping Mapping type of the new range
+ *
+ * @return
+ *  - #IDMAP_COLLISION:     New range collides with existing one
+ */
+enum idmap_error_code sss_idmap_check_collision(struct sss_idmap_ctx *ctx,
+                                                char *n_name, char *n_sid,
+                                                struct sss_idmap_range *n_range,
+                                                uint32_t n_first_rid,
+                                                char *n_range_id,
+                                                bool n_external_mapping);
+
+/**
+ * @brief Check if two ranges would collide
+ *
+ * @param[in] o_name      Zero-terminated string with the domain name the
+ *                        first range should belong to
+ * @param[in] o_sid       Zero-terminated string representation of the domain
+ *                        SID (S-1-15-.....) the first range sould belong to
+ * @param[in] o_range     The first id range
+ * @param[in] o_range_id  unique identifier of the first range, it is needed
+ *                        to allow updates at runtime, may be NULL
+ * @param[in] o_first_rid The RID that should be mapped to the first ID of the
+ *                        first range.
+ * @param[in] o_external_mapping Mapping type of the first range
+ * @param[in] n_name      Zero-terminated string with the domain name the
+ *                        second range should belong to
+ * @param[in] n_sid       Zero-terminated string representation of the domain
+ *                        SID (S-1-15-.....) the second range sould belong to
+ * @param[in] n_range     The second id range
+ * @param[in] n_range_id  unique identifier of the second range, it is needed
+ *                        to allow updates at runtime, may be NULL
+ * @param[in] n_first_rid The RID that should be mapped to the first ID of the
+ *                        second range.
+ * @param[in] n_external_mapping Mapping type of the second range
+ *
+ * @return
+ *  - #IDMAP_COLLISION:     New range collides with existing one
+ */
+enum idmap_error_code sss_idmap_check_collision_ex(const char *o_name,
+                                                const char *o_sid,
+                                                struct sss_idmap_range *o_range,
+                                                uint32_t o_first_rid,
+                                                const char *o_range_id,
+                                                bool o_external_mapping,
+                                                const char *n_name,
+                                                const char *n_sid,
+                                                struct sss_idmap_range *n_range,
+                                                uint32_t n_first_rid,
+                                                const char *n_range_id,
+                                                bool n_external_mapping);
 /**
  * @brief Translate SID to a unix UID or GID
  *
diff --git a/src/tests/cmocka/test_sss_idmap.c b/src/tests/cmocka/test_sss_idmap.c
index 019b461..ff93321 100644
--- a/src/tests/cmocka/test_sss_idmap.c
+++ b/src/tests/cmocka/test_sss_idmap.c
@@ -30,11 +30,15 @@
 #define TEST_RANGE_MAX 399999
 #define TEST_DOM_NAME "test.dom"
 #define TEST_DOM_SID "S-1-5-21-123-456-789"
+#define TEST_FIRST_RID 0
+#define TEST_EXT_MAPPING true
 
 #define TEST_2_RANGE_MIN 600000
 #define TEST_2_RANGE_MAX 799999
 #define TEST_2_DOM_NAME "test2.dom"
 #define TEST_2_DOM_SID "S-1-5-21-987-654-321"
+#define TEST_2_FIRST_RID 1000000
+#define TEST_2_EXT_MAPPING true
 
 #define TEST_OFFSET 1000000
 #define TEST_OFFSET_STR "1000000"
@@ -408,6 +412,94 @@ void test_has_algorithmic_by_name(void **state)
     assert_false(use_id_mapping);
 }
 
+void test_sss_idmap_check_collision_ex(void **state)
+{
+    enum idmap_error_code err;
+    struct sss_idmap_range r1 = {TEST_RANGE_MIN, TEST_RANGE_MAX};
+    struct sss_idmap_range r2 = {TEST_2_RANGE_MIN, TEST_2_RANGE_MAX};
+
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       TEST_EXT_MAPPING,
+                                       TEST_2_DOM_NAME, TEST_2_DOM_SID, &r2,
+                                       TEST_2_FIRST_RID, NULL,
+                                       TEST_2_EXT_MAPPING);
+    assert_int_equal(err, IDMAP_SUCCESS);
+
+    /* Same name, different SID */
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       TEST_EXT_MAPPING,
+                                       TEST_DOM_NAME, TEST_2_DOM_SID, &r2,
+                                       TEST_2_FIRST_RID, NULL,
+                                       TEST_2_EXT_MAPPING);
+    assert_int_equal(err, IDMAP_COLLISION);
+
+    /* Same SID, different name */
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       TEST_EXT_MAPPING,
+                                       TEST_2_DOM_NAME, TEST_DOM_SID, &r2,
+                                       TEST_2_FIRST_RID, NULL,
+                                       TEST_2_EXT_MAPPING);
+    assert_int_equal(err, IDMAP_COLLISION);
+
+    /* Same SID and name, no overlaps */
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       TEST_EXT_MAPPING,
+                                       TEST_DOM_NAME, TEST_DOM_SID, &r2,
+                                       TEST_2_FIRST_RID, NULL,
+                                       TEST_2_EXT_MAPPING);
+    assert_int_equal(err, IDMAP_SUCCESS);
+
+    /* Same SID and name, different mappings */
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       TEST_EXT_MAPPING,
+                                       TEST_DOM_NAME, TEST_DOM_SID, &r2,
+                                       TEST_2_FIRST_RID, NULL,
+                                       !TEST_EXT_MAPPING);
+    assert_int_equal(err, IDMAP_COLLISION);
+
+    /* Same SID and name, Overlapping RID range */
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       false,
+                                       TEST_DOM_NAME, TEST_DOM_SID, &r2,
+                                       TEST_FIRST_RID, NULL,
+                                       false);
+    assert_int_equal(err, IDMAP_COLLISION);
+
+    /* Different SID and name, Overlapping RID range */
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       false,
+                                       TEST_2_DOM_NAME, TEST_2_DOM_SID, &r2,
+                                       TEST_FIRST_RID, NULL,
+                                       false);
+    assert_int_equal(err, IDMAP_SUCCESS);
+
+
+    /* Overlapping ranges with no external mapping */
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       false,
+                                       TEST_2_DOM_NAME, TEST_2_DOM_SID, &r1,
+                                       TEST_2_FIRST_RID, NULL,
+                                       false);
+    assert_int_equal(err, IDMAP_COLLISION);
+
+    /* Overlapping ranges with external mapping */
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       true,
+                                       TEST_2_DOM_NAME, TEST_2_DOM_SID, &r1,
+                                       TEST_2_FIRST_RID, NULL,
+                                       true);
+    assert_int_equal(err, IDMAP_SUCCESS);
+}
+
 int main(int argc, const char *argv[])
 {
     poptContext pc;
@@ -439,6 +531,7 @@ int main(int argc, const char *argv[])
         unit_test_setup_teardown(test_has_algorithmic_by_name,
                                  test_sss_idmap_setup_with_both,
                                  test_sss_idmap_teardown),
+        unit_test(test_sss_idmap_check_collision_ex),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */
-- 
1.8.3.1

-------------- next part --------------
From 8da888bbd832e48620b6e6e1e5edf04de07618fd Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 7 Feb 2014 15:54:30 +0100
Subject: [PATCH 2/3] IPA: refactor idmap code and add test

---
 Makefile.am                       |  15 +++
 src/providers/ipa/ipa_common.h    |  10 ++
 src/providers/ipa/ipa_idmap.c     | 245 +++++++++++++++----------------------
 src/tests/cmocka/test_ipa_idmap.c | 249 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 372 insertions(+), 147 deletions(-)
 create mode 100644 src/tests/cmocka/test_ipa_idmap.c

diff --git a/Makefile.am b/Makefile.am
index e51003c..36312fe 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -162,6 +162,7 @@ if HAVE_CMOCKA
         fqnames-tests \
         nestedgroups-tests \
         test_sss_idmap \
+        test_ipa_idmap \
         test_utils \
         ad_access_filter_tests \
         ad_common_tests \
@@ -1505,6 +1506,20 @@ test_sss_idmap_LDADD = \
     $(SSSD_INTERNAL_LTLIBS) \
     libsss_test_common.la
 
+test_ipa_idmap_SOURCES = \
+    src/tests/cmocka/test_ipa_idmap.c \
+    src/providers/ipa/ipa_idmap.c
+test_ipa_idmap_CFLAGS = \
+    $(AM_CFLAGS)
+test_ipa_idmap_LDFLAGS = \
+    -Wl,-wrap,sysdb_get_ranges
+test_ipa_idmap_LDADD = \
+    $(CMOCKA_LIBS) \
+    $(POPT_LIBS) \
+    libsss_idmap.la \
+    $(SSSD_INTERNAL_LTLIBS) \
+    libsss_test_common.la
+
 test_utils_SOURCES = \
     src/tests/cmocka/test_utils.c
 test_utils_CFLAGS = \
diff --git a/src/providers/ipa/ipa_common.h b/src/providers/ipa/ipa_common.h
index 02f0baf..0b8a17c 100644
--- a/src/providers/ipa/ipa_common.h
+++ b/src/providers/ipa/ipa_common.h
@@ -195,6 +195,16 @@ int ipa_sudo_init(struct be_ctx *be_ctx,
                   struct bet_ops **ops,
                   void **pvt_data);
 
+errno_t get_idmap_data_from_range(struct range_info *r, char *domain_name,
+                                  char **_name, char **_sid, uint32_t *_rid,
+                                  struct sss_idmap_range *_range,
+                                  bool *_external_mapping);
+
+errno_t ipa_idmap_get_ranges_from_sysdb(struct sdap_idmap_ctx *idmap_ctx,
+                                        const char *dom_name,
+                                        const char *dom_sid_str,
+                                        bool allow_collisions);
+
 errno_t ipa_idmap_init(TALLOC_CTX *mem_ctx,
                        struct sdap_id_ctx *id_ctx,
                        struct sdap_idmap_ctx **_idmap_ctx);
diff --git a/src/providers/ipa/ipa_idmap.c b/src/providers/ipa/ipa_idmap.c
index ee7a6fd..ae202d6 100644
--- a/src/providers/ipa/ipa_idmap.c
+++ b/src/providers/ipa/ipa_idmap.c
@@ -156,9 +156,68 @@ done:
     return ret;
 }
 
-errno_t ipa_idmap_find_new_domain(struct sdap_idmap_ctx *idmap_ctx,
-                                  const char *dom_name,
-                                  const char *dom_sid_str)
+errno_t get_idmap_data_from_range(struct range_info *r, char *domain_name,
+                                  char **_name, char **_sid, uint32_t *_rid,
+                                  struct sss_idmap_range *_range,
+                                  bool *_external_mapping)
+{
+    if (r->range_type == NULL) {
+        /* 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->secondary_base_rid != 0) {
+            /* local IPA domain */
+            *_rid = 0;
+            *_external_mapping = true;
+            *_name = domain_name;
+            *_sid = NULL;
+        } else if (r->trusted_dom_sid != NULL
+                && r->secondary_base_rid == 0) {
+            /* trusted domain */
+            *_rid = r->base_rid;
+            *_external_mapping = false;
+            *_name = r->trusted_dom_sid;
+            *_sid = r->trusted_dom_sid;
+        } else {
+            DEBUG(SSSDBG_MINOR_FAILURE, "Cannot determine range type, " \
+                                        "for id range [%s].\n",
+                                        r->name);
+            return EINVAL;
+        }
+    } else {
+        if (strcmp(r->range_type, IPA_RANGE_LOCAL) == 0) {
+            *_rid = 0;
+            *_external_mapping = true;
+            *_name = domain_name;
+            *_sid = NULL;
+        } else if (strcmp(r->range_type, IPA_RANGE_AD_TRUST_POSIX) == 0) {
+            *_rid = 0;
+            *_external_mapping = true;
+            *_name = r->trusted_dom_sid;
+            *_sid = r->trusted_dom_sid;
+        } else if (strcmp(r->range_type, IPA_RANGE_AD_TRUST) == 0) {
+            *_rid = r->base_rid;
+            *_external_mapping = false;
+            *_name = r->trusted_dom_sid;
+            *_sid = r->trusted_dom_sid;
+        } else {
+            DEBUG(SSSDBG_MINOR_FAILURE, "Range type [%s] of id range " \
+                                        "[%s] not supported.\n", \
+                                        r->range_type, r->name);
+            return EINVAL;
+        }
+    }
+
+    _range->min = r->base_id;
+    _range->max = r->base_id + r->id_range_size -1;
+
+    return EOK;
+}
+
+errno_t ipa_idmap_get_ranges_from_sysdb(struct sdap_idmap_ctx *idmap_ctx,
+                                        const char *dom_name,
+                                        const char *dom_sid_str,
+                                        bool allow_collisions)
 {
     int ret;
     size_t range_count;
@@ -166,7 +225,6 @@ errno_t ipa_idmap_find_new_domain(struct sdap_idmap_ctx *idmap_ctx,
     TALLOC_CTX *tmp_ctx;
     size_t c;
     enum idmap_error_code err;
-    struct range_info *r;
     struct sss_idmap_range range;
     uint32_t rid;
     bool external_mapping;
@@ -187,72 +245,37 @@ errno_t ipa_idmap_find_new_domain(struct sdap_idmap_ctx *idmap_ctx,
     }
 
     for (c = 0; c < range_count; c++) {
-        r = range_list[c];
-
-        if (r->range_type == NULL) {
-            /* 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->secondary_base_rid != 0) {
-                /* local IPA domain */
-                rid = 0;
-                external_mapping = true;
-                name = idmap_ctx->id_ctx->be->domain->name;
-                sid = NULL;
-            } else if (r->trusted_dom_sid != NULL
-                    && r->secondary_base_rid == 0) {
-                /* trusted domain */
-                rid = r->base_rid;
-                external_mapping = false;
-                name = r->trusted_dom_sid;
-                sid = r->trusted_dom_sid;
-            } else {
-                DEBUG(SSSDBG_MINOR_FAILURE, "Cannot determine range type, " \
-                                             "skipping id ange [%s].\n",
-                                             r->name);
-                continue;
-            }
-        } else {
-            if (strcmp(r->range_type, IPA_RANGE_LOCAL) == 0) {
-                rid = 0;
-                external_mapping = true;
-                name = idmap_ctx->id_ctx->be->domain->name;
-                sid = NULL;
-            } else if (strcmp(r->range_type, IPA_RANGE_AD_TRUST_POSIX) == 0) {
-                rid = 0;
-                external_mapping = true;
-                name = r->trusted_dom_sid;
-                sid = r->trusted_dom_sid;
-            } else if (strcmp(r->range_type, IPA_RANGE_AD_TRUST) == 0) {
-                rid = r->base_rid;
-                external_mapping = false;
-                name = r->trusted_dom_sid;
-                sid = r->trusted_dom_sid;
-            } else {
-                DEBUG(SSSDBG_MINOR_FAILURE, "Range type [%s] not supported, " \
-                                             "skipping id range [%s].\n",
-                                             r->range_type, r->name);
-                continue;
-            }
+        ret = get_idmap_data_from_range(range_list[c],
+                                        idmap_ctx->id_ctx->be->domain->name,
+                                        &name, &sid, &rid, &range,
+                                        &external_mapping);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, "get_idmap_data_from_range failed for " \
+                                     "id range [%s], skipping.\n",
+                                     range_list[c]->name);
+            continue;
         }
 
-        range.min = r->base_id;
-        range.max = r->base_id + r->id_range_size -1;
         err = sss_idmap_add_domain_ex(idmap_ctx->map, name, sid, &range,
-                                      r->name, rid, external_mapping);
-        if (err != IDMAP_SUCCESS && err != IDMAP_COLLISION) {
-            DEBUG(SSSDBG_CRIT_FAILURE, "Could not add range [%s] to ID map\n",
-                                        r->name);
-            ret = EIO;
-            goto done;
+                                      range_list[c]->name, rid,
+                                      external_mapping);
+        if (err != IDMAP_SUCCESS) {
+            if (!allow_collisions || err != IDMAP_COLLISION) {
+                DEBUG(SSSDBG_CRIT_FAILURE, "Could not add range [%s] to ID map\n",
+                                           range_list[c]->name);
+                ret = EIO;
+                goto done;
+            }
         }
     }
 
-    ret = ipa_idmap_check_posix_child(idmap_ctx, dom_name, dom_sid_str,
-                                      range_count, range_list);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, "ipa_idmap_check_posix_child failed.\n");
-        goto done;
+    if (dom_name != NULL || dom_sid_str != NULL) {
+        ret = ipa_idmap_check_posix_child(idmap_ctx, dom_name, dom_sid_str,
+                                          range_count, range_list);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, "ipa_idmap_check_posix_child failed.\n");
+            goto done;
+        }
     }
 
     ret = EOK;
@@ -263,6 +286,14 @@ done:
     return ret;
 }
 
+errno_t ipa_idmap_find_new_domain(struct sdap_idmap_ctx *idmap_ctx,
+                                  const char *dom_name,
+                                  const char *dom_sid_str)
+{
+    return ipa_idmap_get_ranges_from_sysdb(idmap_ctx, dom_name, dom_sid_str,
+                                           true);
+}
+
 errno_t ipa_idmap_init(TALLOC_CTX *mem_ctx,
                        struct sdap_id_ctx *id_ctx,
                        struct sdap_idmap_ctx **_idmap_ctx)
@@ -270,17 +301,7 @@ errno_t ipa_idmap_init(TALLOC_CTX *mem_ctx,
     errno_t ret;
     TALLOC_CTX *tmp_ctx;
     enum idmap_error_code err;
-    size_t c;
     struct sdap_idmap_ctx *idmap_ctx = NULL;
-    struct sysdb_ctx *sysdb = id_ctx->be->domain->sysdb;
-    size_t range_count;
-    struct range_info **range_list;
-    struct range_info *r;
-    struct sss_idmap_range range;
-    uint32_t rid;
-    bool external_mapping;
-    char *name;
-    char *sid;
 
     tmp_ctx = talloc_new(NULL);
     if (!tmp_ctx) return ENOMEM;
@@ -309,82 +330,12 @@ errno_t ipa_idmap_init(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-
-    /* Read in any existing mappings from the cache */
-    ret = sysdb_get_ranges(tmp_ctx, sysdb, &range_count, &range_list);
-    if (ret != EOK && ret != ENOENT) {
-        DEBUG(SSSDBG_FATAL_FAILURE,
-              "Could not read ranges from the cache: [%s]\n",
-               strerror(ret));
+    ret = ipa_idmap_get_ranges_from_sysdb(idmap_ctx, NULL, NULL, false);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, "ipa_idmap_get_ranges_from_sysdb failed.\n");
         goto done;
     }
 
-    DEBUG(SSSDBG_CONF_SETTINGS,
-          "Initializing [%zu] domains for ID-mapping\n", range_count);
-
-    for (c = 0; c < range_count; c++) {
-
-        r = range_list[c];
-
-        if (r->range_type == NULL) {
-            /* 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->secondary_base_rid != 0) {
-                /* local IPA domain */
-                rid = 0;
-                external_mapping = true;
-                sid = NULL;
-                name = id_ctx->be->domain->name;
-            } else if (r->trusted_dom_sid != NULL
-                    && r->secondary_base_rid == 0) {
-                /* trusted domain */
-                rid = r->base_rid;
-                external_mapping = false;
-                sid = r->trusted_dom_sid;
-                name = sid;
-            } else {
-                DEBUG(SSSDBG_MINOR_FAILURE, "Cannot determine range type, " \
-                                             "skipping id ange [%s].\n",
-                                             r->name);
-                continue;
-            }
-        } else {
-            if (strcmp(r->range_type, IPA_RANGE_LOCAL) == 0) {
-                rid = 0;
-                external_mapping = true;
-                sid = NULL;
-                name = id_ctx->be->domain->name;
-            } else if (strcmp(r->range_type, IPA_RANGE_AD_TRUST_POSIX) == 0) {
-                rid = 0;
-                external_mapping = true;
-                sid = r->trusted_dom_sid;
-                name = sid;
-            } else if (strcmp(r->range_type, IPA_RANGE_AD_TRUST) == 0) {
-                rid = r->base_rid;
-                external_mapping = false;
-                sid = r->trusted_dom_sid;
-                name = sid;
-            } else {
-                DEBUG(SSSDBG_MINOR_FAILURE, "Range type [%s] not supported, " \
-                                             "skipping id range [%s].\n",
-                                             r->range_type, r->name);
-                continue;
-            }
-        }
-
-        range.min = r->base_id;
-        range.max = r->base_id + r->id_range_size -1;
-        err = sss_idmap_add_domain_ex(idmap_ctx->map, name, sid, &range,
-                                      r->name, rid, external_mapping);
-        if (err != IDMAP_SUCCESS) {
-            DEBUG(SSSDBG_CRIT_FAILURE, "Could not add range [%s] to ID map\n",
-                                        r->name);
-            ret = EIO;
-            goto done;
-        }
-    }
-
     *_idmap_ctx = talloc_steal(mem_ctx, idmap_ctx);
     ret = EOK;
 
diff --git a/src/tests/cmocka/test_ipa_idmap.c b/src/tests/cmocka/test_ipa_idmap.c
new file mode 100644
index 0000000..2fb2cde
--- /dev/null
+++ b/src/tests/cmocka/test_ipa_idmap.c
@@ -0,0 +1,249 @@
+/*
+    Authors:
+        Sumit Bose <sbose at redhat.com>
+
+    Copyright (C) 2014 Red Hat
+
+    SSSD tests: Unit tests for id-mapping in the IPA provider
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <popt.h>
+
+#include "tests/cmocka/common_mock.h"
+#include "lib/idmap/sss_idmap.h"
+#include "providers/ipa/ipa_common.h"
+#include "providers/ldap/sdap_idmap.h"
+
+#define RANGE_NAME discard_const("range1")
+#define DOMAIN_SID discard_const("S-1-5-21-2-3-4")
+#define DOMAIN_NAME discard_const("dom.test")
+#define BASE_RID 111
+#define SECONDARY_BASE_RID 11223344
+#define BASE_ID 123456
+#define RANGE_SIZE 222222
+#define RANGE_MAX (BASE_ID + RANGE_SIZE - 1)
+
+void test_get_idmap_data_from_range(void **state)
+{
+    char *dom_name;
+    char *sid;
+    uint32_t rid;
+    struct sss_idmap_range range;
+    bool external_mapping;
+    size_t c;
+    errno_t ret;
+
+    struct test_data {
+        struct range_info r;
+        errno_t exp_ret;
+        char *exp_dom_name;
+        char *exp_sid;
+        uint32_t exp_rid;
+        struct sss_idmap_range exp_range;
+        bool exp_external_mapping;
+    } d[] = {
+        /* working IPA_RANGE_LOCAL range */
+        {{RANGE_NAME, BASE_ID, RANGE_SIZE, BASE_RID, SECONDARY_BASE_RID,
+          NULL, discard_const(IPA_RANGE_LOCAL)},
+         EOK, DOMAIN_NAME, NULL, 0, {BASE_ID, RANGE_MAX}, true},
+        /* working old-style IPA_RANGE_LOCAL range without range type */
+        {{RANGE_NAME, BASE_ID, RANGE_SIZE, BASE_RID, SECONDARY_BASE_RID,
+          NULL, NULL},
+         EOK, DOMAIN_NAME, NULL, 0, {BASE_ID, RANGE_MAX}, true},
+        /* old-style IPA_RANGE_LOCAL without SID and secondary base rid */
+        {{RANGE_NAME, BASE_ID, RANGE_SIZE, BASE_RID, 0, NULL, NULL},
+         EINVAL, NULL, NULL, 0, {0, 0}, false},
+        /* old-style range with SID and secondary base rid */
+        {{RANGE_NAME, BASE_ID, RANGE_SIZE, BASE_RID, SECONDARY_BASE_RID,
+          DOMAIN_SID, NULL},
+         EINVAL, NULL, NULL, 0, {0, 0}, false},
+        /* working IPA_RANGE_AD_TRUST range */
+        {{RANGE_NAME, BASE_ID, RANGE_SIZE, BASE_RID, 0, DOMAIN_SID,
+          discard_const(IPA_RANGE_AD_TRUST)},
+         EOK, DOMAIN_SID, DOMAIN_SID, BASE_RID, {BASE_ID, RANGE_MAX}, false},
+        /* working old-style IPA_RANGE_AD_TRUST range without range type */
+        {{RANGE_NAME, BASE_ID, RANGE_SIZE, BASE_RID, 0, DOMAIN_SID, NULL},
+         EOK, DOMAIN_SID, DOMAIN_SID, BASE_RID, {BASE_ID, RANGE_MAX}, false},
+        /* working IPA_RANGE_AD_TRUST_POSIX range */
+        {{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},
+        {{0}, 0, NULL, NULL, 0, {0, 0}, false}
+    };
+
+    for (c = 0; d[c].exp_dom_name != NULL; 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);
+        }
+        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);
+    }
+}
+
+errno_t __wrap_sysdb_get_ranges(TALLOC_CTX *mem_ctx, struct sysdb_ctx *sysdb,
+                                size_t *range_count,
+                                struct range_info ***range_list)
+{
+
+    *range_count = sss_mock_type(size_t);
+    *range_list = talloc_steal(mem_ctx,
+                               sss_mock_ptr_type(struct range_info **));
+    return EOK;
+}
+
+struct test_ctx {
+    struct sdap_idmap_ctx *idmap_ctx;
+    struct sdap_id_ctx *sdap_id_ctx;
+};
+
+static struct range_info **get_range_list(TALLOC_CTX *mem_ctx)
+{
+    struct range_info **range_list;
+
+    range_list = talloc_array(mem_ctx, struct range_info *, 2);
+    assert_non_null(range_list);
+
+    range_list[0] = talloc_zero(range_list, struct range_info);
+    assert_non_null(range_list[0]);
+
+    range_list[0]->name = talloc_strdup(range_list[0], RANGE_NAME);
+    assert_non_null( range_list[0]->name);
+    range_list[0]->base_id = BASE_ID;
+    range_list[0]->id_range_size = RANGE_SIZE;
+    range_list[0]->base_rid = BASE_RID;
+    range_list[0]->secondary_base_rid = 0;
+    range_list[0]->trusted_dom_sid = talloc_strdup(range_list[0], DOMAIN_SID);
+    assert_non_null(range_list[0]->trusted_dom_sid);
+    range_list[0]->range_type = talloc_strdup(range_list[0],
+                                              IPA_RANGE_AD_TRUST);
+    assert_non_null(range_list[0]->range_type);
+
+    return range_list;
+}
+
+void setup_idmap_ctx(void **state)
+{
+    int ret;
+    struct test_ctx *test_ctx;
+
+    assert_true(leak_check_setup());
+
+    test_ctx = talloc_zero(global_talloc_context, struct test_ctx);
+    assert_non_null(test_ctx);
+
+    test_ctx->sdap_id_ctx = talloc_zero(test_ctx,
+                                        struct sdap_id_ctx);
+    assert_non_null(test_ctx->sdap_id_ctx);
+
+    test_ctx->sdap_id_ctx->be = talloc_zero(test_ctx->sdap_id_ctx,
+                                            struct be_ctx);
+    assert_non_null(test_ctx->sdap_id_ctx->be);
+
+    test_ctx->sdap_id_ctx->be->domain = talloc_zero(test_ctx->sdap_id_ctx->be,
+                                                    struct sss_domain_info);
+    assert_non_null(test_ctx->sdap_id_ctx->be->domain);
+
+    test_ctx->sdap_id_ctx->be->domain->name =
+                  talloc_strdup(test_ctx->sdap_id_ctx->be->domain, DOMAIN_NAME);
+    assert_non_null(test_ctx->sdap_id_ctx->be->domain->name);
+
+    will_return(__wrap_sysdb_get_ranges, 1);
+    will_return(__wrap_sysdb_get_ranges, get_range_list(global_talloc_context));
+
+    ret = ipa_idmap_init(test_ctx, test_ctx->sdap_id_ctx,
+                         &test_ctx->idmap_ctx);
+    assert_int_equal(ret, EOK);
+
+    check_leaks_push(test_ctx);
+    *state = test_ctx;
+}
+
+void teardown_idmap_ctx(void **state)
+{
+    struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
+
+    assert_non_null(test_ctx);
+
+    assert_true(check_leaks_pop(test_ctx) == true);
+
+    talloc_free(test_ctx);
+    assert_true(leak_check_teardown());
+}
+
+void test_ipa_idmap_get_ranges_from_sysdb(void **state)
+{
+    int ret;
+    struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
+    assert_non_null(test_ctx);
+
+    will_return(__wrap_sysdb_get_ranges, 1);
+    will_return(__wrap_sysdb_get_ranges, get_range_list(test_ctx->idmap_ctx));
+    ret = ipa_idmap_get_ranges_from_sysdb(test_ctx->idmap_ctx,
+                                          DOMAIN_NAME, DOMAIN_SID, true);
+    assert_int_equal(ret, EOK);
+
+    will_return(__wrap_sysdb_get_ranges, 1);
+    will_return(__wrap_sysdb_get_ranges, get_range_list(global_talloc_context));
+    ret = ipa_idmap_get_ranges_from_sysdb(test_ctx->idmap_ctx,
+                                          DOMAIN_NAME, DOMAIN_SID, false);
+    assert_int_equal(ret, EIO);
+}
+
+int main(int argc, const char *argv[])
+{
+    poptContext pc;
+    int opt;
+    struct poptOption long_options[] = {
+        POPT_AUTOHELP
+        SSSD_DEBUG_OPTS
+        POPT_TABLEEND
+    };
+
+    const UnitTest tests[] = {
+        unit_test(test_get_idmap_data_from_range),
+        unit_test_setup_teardown(test_ipa_idmap_get_ranges_from_sysdb,
+                                 setup_idmap_ctx, teardown_idmap_ctx),
+    };
+
+    /* Set debug level to invalid value so we can deside if -d 0 was used. */
+    debug_level = SSSDBG_INVALID;
+
+    pc = poptGetContext(argv[0], argc, argv, long_options, 0);
+    while((opt = poptGetNextOpt(pc)) != -1) {
+        switch(opt) {
+        default:
+            fprintf(stderr, "\nInvalid option %s: %s\n\n",
+                    poptBadOption(pc, 0), poptStrerror(opt));
+            poptPrintUsage(pc, stderr, 0);
+            return 1;
+        }
+    }
+    poptFreeContext(pc);
+
+    DEBUG_INIT(debug_level);
+
+    tests_set_cwd();
+
+    return run_tests(tests);
+}
-- 
1.8.3.1

-------------- next part --------------
From bba63e883dcc14d328e176731245b122f29c690a Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 7 Feb 2014 18:17:09 +0100
Subject: [PATCH 3/3] IPA: check ranges for collisions before saving them

Fixes https://fedorahosted.org/sssd/ticket/2253
---
 src/providers/ipa/ipa_subdomains.c | 84 +++++++++++++++++++++++++++++---------
 1 file changed, 64 insertions(+), 20 deletions(-)

diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
index b9d9ceb..7103ba2 100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -351,14 +351,28 @@ const char *get_flat_name_from_subdomain_name(struct be_ctx *be_ctx,
 }
 
 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 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) {
@@ -367,8 +381,8 @@ static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
     }
 
     for (c = 0; c < count; c++) {
-        range_list[c] = talloc_zero(range_list, struct range_info);
-        if (range_list[c] == NULL) {
+        r = talloc_zero(range_list, struct range_info);
+        if (r == NULL) {
             DEBUG(SSSDBG_OP_FAILURE, "talloc_zero failed.\n");
             ret = ENOMEM;
             goto done;
@@ -379,8 +393,9 @@ static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
             DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
             goto done;
         }
-        range_list[c]->name = talloc_strdup(range_list[c], value);
-        if (range_list[c]->name == NULL) {
+
+        r->name = talloc_strdup(r, value);
+        if (r->name == NULL) {
             DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
             ret = ENOMEM;
             goto done;
@@ -388,9 +403,8 @@ static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
 
         ret = sysdb_attrs_get_string(reply[c], IPA_TRUSTED_DOMAIN_SID, &value);
         if (ret == EOK) {
-            range_list[c]->trusted_dom_sid = talloc_strdup(range_list[c],
-                                                           value);
-            if (range_list[c]->trusted_dom_sid == NULL) {
+            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;
@@ -401,28 +415,28 @@ static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
         }
 
         ret = sysdb_attrs_get_uint32_t(reply[c], IPA_BASE_ID,
-                                       &range_list[c]->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,
-                                       &range_list[c]->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,
-                                       &range_list[c]->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,
-                                       &range_list[c]->secondary_base_rid);
+                                       &r->secondary_base_rid);
         if (ret != EOK && ret != ENOENT) {
             DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
             goto done;
@@ -430,8 +444,8 @@ static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
 
         ret = sysdb_attrs_get_string(reply[c], IPA_RANGE_TYPE, &value);
         if (ret == EOK) {
-            range_list[c]->range_type = talloc_strdup(range_list[c], value);
-            if (range_list[c]->range_type == NULL) {
+            r->range_type = talloc_strdup(r, value);
+            if (r->range_type == NULL) {
                 DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
                 ret = ENOMEM;
                 goto done;
@@ -439,23 +453,52 @@ static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
         } 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 (range_list[c]->trusted_dom_sid == NULL) {
-                range_list[c]->range_type = talloc_strdup(range_list[c],
-                                                          IPA_RANGE_LOCAL);
+            if (r->trusted_dom_sid == NULL) {
+                r->range_type = talloc_strdup(r, IPA_RANGE_LOCAL);
             } else {
-                range_list[c]->range_type = talloc_strdup(range_list[c],
-                                                          IPA_RANGE_AD_TRUST);
+                r->range_type = talloc_strdup(r, IPA_RANGE_AD_TRUST);
             }
         } else {
             DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_get_string failed.\n");
             goto done;
         }
-        if (range_list[c]->range_type == NULL) {
+        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 != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, ("get_idmap_data_from_range failed.\n"));
+            goto done;
+        }
+        for (d = 0; d < c; 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[c] = r;
     }
+
     range_list[c] = NULL;
 
     *_range_list = range_list;
@@ -1013,7 +1056,8 @@ static void ipa_subdomains_handler_ranges_done(struct tevent_req *req)
         goto done;
     }
 
-    ret = ipa_ranges_parse_results(ctx, reply_count, reply, &range_list);
+    ret = ipa_ranges_parse_results(ctx, domain->name,
+                                   reply_count, reply, &range_list);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE,
               "ipa_ranges_parse_results request failed.\n");
-- 
1.8.3.1

-------------- next part --------------
From a5188d7ec8b9b4f1bd685e9e09ff643864b8c1d3 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Mon, 3 Feb 2014 13:30:35 +0100
Subject: [PATCH 1/3] IDMAP: add sss_idmap_check_collision(_ex)

---
 src/lib/idmap/sss_idmap.c         | 123 ++++++++++++++++++++++++++------------
 src/lib/idmap/sss_idmap.h         |  65 ++++++++++++++++++++
 src/tests/cmocka/test_sss_idmap.c |  93 ++++++++++++++++++++++++++++
 3 files changed, 244 insertions(+), 37 deletions(-)

diff --git a/src/lib/idmap/sss_idmap.c b/src/lib/idmap/sss_idmap.c
index 3f1e7a5..4c45312 100644
--- a/src/lib/idmap/sss_idmap.c
+++ b/src/lib/idmap/sss_idmap.c
@@ -380,55 +380,104 @@ enum idmap_error_code sss_idmap_calculate_range(struct sss_idmap_ctx *ctx,
     return IDMAP_SUCCESS;
 }
 
-static enum idmap_error_code dom_check_collision(
-                                             struct idmap_domain_info *dom_list,
-                                             struct idmap_domain_info *new_dom)
+enum idmap_error_code sss_idmap_check_collision_ex(const char *o_name,
+                                                const char *o_sid,
+                                                struct sss_idmap_range *o_range,
+                                                uint32_t o_first_rid,
+                                                const char *o_range_id,
+                                                bool o_external_mapping,
+                                                const char *n_name,
+                                                const char *n_sid,
+                                                struct sss_idmap_range *n_range,
+                                                uint32_t n_first_rid,
+                                                const char *n_range_id,
+                                                bool n_external_mapping)
 {
-    struct idmap_domain_info *dom;
     bool names_equal;
     bool sids_equal;
 
-    for (dom = dom_list; dom != NULL; dom = dom->next) {
+    /* TODO: if both ranges have the same ID check if an update is
+     * needed. */
+
+    /* Check if ID ranges overlap.
+     * ID ranges with external mapping may overlap. */
+    if ((!n_external_mapping && !o_external_mapping)
+        && ((n_range->min >= o_range->min
+                && n_range->min <= o_range->max)
+            || (n_range->max >= o_range->min
+                && n_range->max <= o_range->max))) {
+        return IDMAP_COLLISION;
+    }
 
-        /* TODO: if both ranges have the same ID check if an update is
-         * needed. */
-
-        /* Check if ID ranges overlap.
-         * ID ranges with external mapping may overlap. */
-        if ((!new_dom->external_mapping && !dom->external_mapping)
-            && ((new_dom->range->min >= dom->range->min
-                    && new_dom->range->min <= dom->range->max)
-                || (new_dom->range->max >= dom->range->min
-                    && new_dom->range->max <= dom->range->max))) {
-            return IDMAP_COLLISION;
-        }
+    names_equal = (strcasecmp(n_name, o_name) == 0);
+    sids_equal = ((n_sid == NULL && o_sid == NULL)
+                    || (n_sid != NULL && o_sid != NULL
+                        && strcasecmp(n_sid, o_sid) == 0));
+
+    /* check if domain name and SID are consistent */
+    if ((names_equal && !sids_equal) || (!names_equal && sids_equal)) {
+        return IDMAP_COLLISION;
+    }
 
-        names_equal = (strcasecmp(new_dom->name, dom->name) == 0);
-        sids_equal = ((new_dom->sid == NULL && dom->sid == NULL)
-                        || (new_dom->sid != NULL && dom->sid != NULL
-                            && strcasecmp(new_dom->sid, dom->sid) == 0));
+    /* check if external_mapping is consistent */
+    if (names_equal && sids_equal
+            && n_external_mapping != o_external_mapping) {
+        return IDMAP_COLLISION;
+    }
 
-        /* check if domain name and SID are consistent */
-        if ((names_equal && !sids_equal) || (!names_equal && sids_equal)) {
-            return IDMAP_COLLISION;
-        }
+    /* check if RID ranges overlap */
+    if (names_equal && sids_equal
+            && n_external_mapping == false
+            && n_first_rid >= o_first_rid
+            && n_first_rid <= o_first_rid + (o_range->max - o_range->min)) {
+        return IDMAP_COLLISION;
+    }
 
-        /* check if external_mapping is consistent */
-        if (names_equal && sids_equal
-                && new_dom->external_mapping != dom->external_mapping) {
-            return IDMAP_COLLISION;
-        }
+    return IDMAP_SUCCESS;
+}
 
-        /* check if RID ranges overlap */
-        if (names_equal && sids_equal
-                && new_dom->external_mapping == false
-                && new_dom->first_rid >= dom->first_rid
-                && new_dom->first_rid <=
-                    dom->first_rid + (dom->range->max - dom->range->min)) {
-            return IDMAP_COLLISION;
+enum idmap_error_code sss_idmap_check_collision(struct sss_idmap_ctx *ctx,
+                                                char *n_name, char *n_sid,
+                                                struct sss_idmap_range *n_range,
+                                                uint32_t n_first_rid,
+                                                char *n_range_id,
+                                                bool n_external_mapping)
+{
+    struct idmap_domain_info *dom;
+    enum idmap_error_code err;
+
+    for (dom = ctx->idmap_domain_info; dom != NULL; dom = dom->next) {
+        err = sss_idmap_check_collision_ex(dom->name, dom->sid, dom->range,
+                                           dom->first_rid, dom->range_id,
+                                           dom->external_mapping,
+                                           n_name, n_sid, n_range, n_first_rid,
+                                           n_range_id, n_external_mapping);
+        if (err != IDMAP_SUCCESS) {
+            return err;
         }
     }
+    return IDMAP_SUCCESS;
+}
 
+static enum idmap_error_code dom_check_collision(
+                                             struct idmap_domain_info *dom_list,
+                                             struct idmap_domain_info *new_dom)
+{
+    struct idmap_domain_info *dom;
+    enum idmap_error_code err;
+
+    for (dom = dom_list; dom != NULL; dom = dom->next) {
+        err = sss_idmap_check_collision_ex(dom->name, dom->sid, dom->range,
+                                           dom->first_rid, dom->range_id,
+                                           dom->external_mapping,
+                                           new_dom->name, new_dom->sid,
+                                           new_dom->range, new_dom->first_rid,
+                                           new_dom->range_id,
+                                           new_dom->external_mapping);
+        if (err != IDMAP_SUCCESS) {
+            return err;
+        }
+    }
     return IDMAP_SUCCESS;
 }
 
diff --git a/src/lib/idmap/sss_idmap.h b/src/lib/idmap/sss_idmap.h
index 1e1c9a5..ccc63f7 100644
--- a/src/lib/idmap/sss_idmap.h
+++ b/src/lib/idmap/sss_idmap.h
@@ -289,6 +289,71 @@ enum idmap_error_code sss_idmap_add_domain_ex(struct sss_idmap_ctx *ctx,
                                               const char *range_id,
                                               uint32_t rid,
                                               bool external_mapping);
+
+/**
+ * @brief Check if a new range would collide with any existing one
+ *
+ * @param[in] ctx         Idmap context
+ * @param[in] n_name      Zero-terminated string with the domain name the new
+ *                        range should belong to
+ * @param[in] n_sid       Zero-terminated string representation of the domain
+ *                        SID (S-1-15-.....) the new range sould belong to
+ * @param[in] n_range     The new id range
+ * @param[in] n_range_id  unique identifier of the new range, it is needed
+ *                        to allow updates at runtime, may be NULL
+ * @param[in] n_first_rid The RID that should be mapped to the first ID of the
+ *                        new range.
+ * @param[in] n_external_mapping Mapping type of the new range
+ *
+ * @return
+ *  - #IDMAP_COLLISION:     New range collides with existing one
+ */
+enum idmap_error_code sss_idmap_check_collision(struct sss_idmap_ctx *ctx,
+                                                char *n_name, char *n_sid,
+                                                struct sss_idmap_range *n_range,
+                                                uint32_t n_first_rid,
+                                                char *n_range_id,
+                                                bool n_external_mapping);
+
+/**
+ * @brief Check if two ranges would collide
+ *
+ * @param[in] o_name      Zero-terminated string with the domain name the
+ *                        first range should belong to
+ * @param[in] o_sid       Zero-terminated string representation of the domain
+ *                        SID (S-1-15-.....) the first range sould belong to
+ * @param[in] o_range     The first id range
+ * @param[in] o_range_id  unique identifier of the first range, it is needed
+ *                        to allow updates at runtime, may be NULL
+ * @param[in] o_first_rid The RID that should be mapped to the first ID of the
+ *                        first range.
+ * @param[in] o_external_mapping Mapping type of the first range
+ * @param[in] n_name      Zero-terminated string with the domain name the
+ *                        second range should belong to
+ * @param[in] n_sid       Zero-terminated string representation of the domain
+ *                        SID (S-1-15-.....) the second range sould belong to
+ * @param[in] n_range     The second id range
+ * @param[in] n_range_id  unique identifier of the second range, it is needed
+ *                        to allow updates at runtime, may be NULL
+ * @param[in] n_first_rid The RID that should be mapped to the first ID of the
+ *                        second range.
+ * @param[in] n_external_mapping Mapping type of the second range
+ *
+ * @return
+ *  - #IDMAP_COLLISION:     New range collides with existing one
+ */
+enum idmap_error_code sss_idmap_check_collision_ex(const char *o_name,
+                                                const char *o_sid,
+                                                struct sss_idmap_range *o_range,
+                                                uint32_t o_first_rid,
+                                                const char *o_range_id,
+                                                bool o_external_mapping,
+                                                const char *n_name,
+                                                const char *n_sid,
+                                                struct sss_idmap_range *n_range,
+                                                uint32_t n_first_rid,
+                                                const char *n_range_id,
+                                                bool n_external_mapping);
 /**
  * @brief Translate SID to a unix UID or GID
  *
diff --git a/src/tests/cmocka/test_sss_idmap.c b/src/tests/cmocka/test_sss_idmap.c
index 019b461..ff93321 100644
--- a/src/tests/cmocka/test_sss_idmap.c
+++ b/src/tests/cmocka/test_sss_idmap.c
@@ -30,11 +30,15 @@
 #define TEST_RANGE_MAX 399999
 #define TEST_DOM_NAME "test.dom"
 #define TEST_DOM_SID "S-1-5-21-123-456-789"
+#define TEST_FIRST_RID 0
+#define TEST_EXT_MAPPING true
 
 #define TEST_2_RANGE_MIN 600000
 #define TEST_2_RANGE_MAX 799999
 #define TEST_2_DOM_NAME "test2.dom"
 #define TEST_2_DOM_SID "S-1-5-21-987-654-321"
+#define TEST_2_FIRST_RID 1000000
+#define TEST_2_EXT_MAPPING true
 
 #define TEST_OFFSET 1000000
 #define TEST_OFFSET_STR "1000000"
@@ -408,6 +412,94 @@ void test_has_algorithmic_by_name(void **state)
     assert_false(use_id_mapping);
 }
 
+void test_sss_idmap_check_collision_ex(void **state)
+{
+    enum idmap_error_code err;
+    struct sss_idmap_range r1 = {TEST_RANGE_MIN, TEST_RANGE_MAX};
+    struct sss_idmap_range r2 = {TEST_2_RANGE_MIN, TEST_2_RANGE_MAX};
+
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       TEST_EXT_MAPPING,
+                                       TEST_2_DOM_NAME, TEST_2_DOM_SID, &r2,
+                                       TEST_2_FIRST_RID, NULL,
+                                       TEST_2_EXT_MAPPING);
+    assert_int_equal(err, IDMAP_SUCCESS);
+
+    /* Same name, different SID */
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       TEST_EXT_MAPPING,
+                                       TEST_DOM_NAME, TEST_2_DOM_SID, &r2,
+                                       TEST_2_FIRST_RID, NULL,
+                                       TEST_2_EXT_MAPPING);
+    assert_int_equal(err, IDMAP_COLLISION);
+
+    /* Same SID, different name */
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       TEST_EXT_MAPPING,
+                                       TEST_2_DOM_NAME, TEST_DOM_SID, &r2,
+                                       TEST_2_FIRST_RID, NULL,
+                                       TEST_2_EXT_MAPPING);
+    assert_int_equal(err, IDMAP_COLLISION);
+
+    /* Same SID and name, no overlaps */
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       TEST_EXT_MAPPING,
+                                       TEST_DOM_NAME, TEST_DOM_SID, &r2,
+                                       TEST_2_FIRST_RID, NULL,
+                                       TEST_2_EXT_MAPPING);
+    assert_int_equal(err, IDMAP_SUCCESS);
+
+    /* Same SID and name, different mappings */
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       TEST_EXT_MAPPING,
+                                       TEST_DOM_NAME, TEST_DOM_SID, &r2,
+                                       TEST_2_FIRST_RID, NULL,
+                                       !TEST_EXT_MAPPING);
+    assert_int_equal(err, IDMAP_COLLISION);
+
+    /* Same SID and name, Overlapping RID range */
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       false,
+                                       TEST_DOM_NAME, TEST_DOM_SID, &r2,
+                                       TEST_FIRST_RID, NULL,
+                                       false);
+    assert_int_equal(err, IDMAP_COLLISION);
+
+    /* Different SID and name, Overlapping RID range */
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       false,
+                                       TEST_2_DOM_NAME, TEST_2_DOM_SID, &r2,
+                                       TEST_FIRST_RID, NULL,
+                                       false);
+    assert_int_equal(err, IDMAP_SUCCESS);
+
+
+    /* Overlapping ranges with no external mapping */
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       false,
+                                       TEST_2_DOM_NAME, TEST_2_DOM_SID, &r1,
+                                       TEST_2_FIRST_RID, NULL,
+                                       false);
+    assert_int_equal(err, IDMAP_COLLISION);
+
+    /* Overlapping ranges with external mapping */
+    err = sss_idmap_check_collision_ex(TEST_DOM_NAME, TEST_DOM_SID, &r1,
+                                       TEST_FIRST_RID, NULL,
+                                       true,
+                                       TEST_2_DOM_NAME, TEST_2_DOM_SID, &r1,
+                                       TEST_2_FIRST_RID, NULL,
+                                       true);
+    assert_int_equal(err, IDMAP_SUCCESS);
+}
+
 int main(int argc, const char *argv[])
 {
     poptContext pc;
@@ -439,6 +531,7 @@ int main(int argc, const char *argv[])
         unit_test_setup_teardown(test_has_algorithmic_by_name,
                                  test_sss_idmap_setup_with_both,
                                  test_sss_idmap_teardown),
+        unit_test(test_sss_idmap_check_collision_ex),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */
-- 
1.8.3.1

-------------- next part --------------
From 64e4a168fdbf6f963ce5934e394e6eff6e5ecdd6 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 7 Feb 2014 15:54:30 +0100
Subject: [PATCH 2/3] IPA: refactor idmap code and add test

---
 Makefile.am                       |  15 +++
 src/providers/ipa/ipa_common.h    |  10 ++
 src/providers/ipa/ipa_idmap.c     | 246 +++++++++++++++----------------------
 src/tests/cmocka/test_ipa_idmap.c | 249 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 373 insertions(+), 147 deletions(-)
 create mode 100644 src/tests/cmocka/test_ipa_idmap.c

diff --git a/Makefile.am b/Makefile.am
index 101401f..f490bc3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -150,6 +150,7 @@ if HAVE_CMOCKA
         dyndns-tests \
         fqnames-tests \
         test_sss_idmap \
+        test_ipa_idmap \
         test_utils \
         ad_access_filter_tests \
         ad_common_tests \
@@ -1356,6 +1357,20 @@ test_sss_idmap_LDADD = \
     $(SSSD_INTERNAL_LTLIBS) \
     libsss_test_common.la
 
+test_ipa_idmap_SOURCES = \
+    src/tests/cmocka/test_ipa_idmap.c \
+    src/providers/ipa/ipa_idmap.c
+test_ipa_idmap_CFLAGS = \
+    $(AM_CFLAGS)
+test_ipa_idmap_LDFLAGS = \
+    -Wl,-wrap,sysdb_get_ranges
+test_ipa_idmap_LDADD = \
+    $(CMOCKA_LIBS) \
+    $(POPT_LIBS) \
+    libsss_idmap.la \
+    $(SSSD_INTERNAL_LTLIBS) \
+    libsss_test_common.la
+
 test_utils_SOURCES = \
     src/tests/cmocka/test_utils.c
 test_utils_CFLAGS = \
diff --git a/src/providers/ipa/ipa_common.h b/src/providers/ipa/ipa_common.h
index 02f0baf..0b8a17c 100644
--- a/src/providers/ipa/ipa_common.h
+++ b/src/providers/ipa/ipa_common.h
@@ -195,6 +195,16 @@ int ipa_sudo_init(struct be_ctx *be_ctx,
                   struct bet_ops **ops,
                   void **pvt_data);
 
+errno_t get_idmap_data_from_range(struct range_info *r, char *domain_name,
+                                  char **_name, char **_sid, uint32_t *_rid,
+                                  struct sss_idmap_range *_range,
+                                  bool *_external_mapping);
+
+errno_t ipa_idmap_get_ranges_from_sysdb(struct sdap_idmap_ctx *idmap_ctx,
+                                        const char *dom_name,
+                                        const char *dom_sid_str,
+                                        bool allow_collisions);
+
 errno_t ipa_idmap_init(TALLOC_CTX *mem_ctx,
                        struct sdap_id_ctx *id_ctx,
                        struct sdap_idmap_ctx **_idmap_ctx);
diff --git a/src/providers/ipa/ipa_idmap.c b/src/providers/ipa/ipa_idmap.c
index eaca0ed..a65086a 100644
--- a/src/providers/ipa/ipa_idmap.c
+++ b/src/providers/ipa/ipa_idmap.c
@@ -156,9 +156,68 @@ done:
     return ret;
 }
 
-errno_t ipa_idmap_find_new_domain(struct sdap_idmap_ctx *idmap_ctx,
-                                  const char *dom_name,
-                                  const char *dom_sid_str)
+errno_t get_idmap_data_from_range(struct range_info *r, char *domain_name,
+                                  char **_name, char **_sid, uint32_t *_rid,
+                                  struct sss_idmap_range *_range,
+                                  bool *_external_mapping)
+{
+    if (r->range_type == NULL) {
+        /* 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->secondary_base_rid != 0) {
+            /* local IPA domain */
+            *_rid = 0;
+            *_external_mapping = true;
+            *_name = domain_name;
+            *_sid = NULL;
+        } else if (r->trusted_dom_sid != NULL
+                && r->secondary_base_rid == 0) {
+            /* trusted domain */
+            *_rid = r->base_rid;
+            *_external_mapping = false;
+            *_name = r->trusted_dom_sid;
+            *_sid = r->trusted_dom_sid;
+        } else {
+            DEBUG(SSSDBG_MINOR_FAILURE, ("Cannot determine range type, " \
+                                         "for id range [%s].\n",
+                                         r->name));
+            return EINVAL;
+        }
+    } else {
+        if (strcmp(r->range_type, IPA_RANGE_LOCAL) == 0) {
+            *_rid = 0;
+            *_external_mapping = true;
+            *_name = domain_name;
+            *_sid = NULL;
+        } else if (strcmp(r->range_type, IPA_RANGE_AD_TRUST_POSIX) == 0) {
+            *_rid = 0;
+            *_external_mapping = true;
+            *_name = r->trusted_dom_sid;
+            *_sid = r->trusted_dom_sid;
+        } else if (strcmp(r->range_type, IPA_RANGE_AD_TRUST) == 0) {
+            *_rid = r->base_rid;
+            *_external_mapping = false;
+            *_name = r->trusted_dom_sid;
+            *_sid = r->trusted_dom_sid;
+        } else {
+            DEBUG(SSSDBG_MINOR_FAILURE, ("Range type [%s] of id range " \
+                                         "[%s] not supported.\n", \
+                                         r->range_type, r->name));
+            return EINVAL;
+        }
+    }
+
+    _range->min = r->base_id;
+    _range->max = r->base_id + r->id_range_size -1;
+
+    return EOK;
+}
+
+errno_t ipa_idmap_get_ranges_from_sysdb(struct sdap_idmap_ctx *idmap_ctx,
+                                        const char *dom_name,
+                                        const char *dom_sid_str,
+                                        bool allow_collisions)
 {
     int ret;
     size_t range_count;
@@ -166,7 +225,6 @@ errno_t ipa_idmap_find_new_domain(struct sdap_idmap_ctx *idmap_ctx,
     TALLOC_CTX *tmp_ctx;
     size_t c;
     enum idmap_error_code err;
-    struct range_info *r;
     struct sss_idmap_range range;
     uint32_t rid;
     bool external_mapping;
@@ -187,72 +245,37 @@ errno_t ipa_idmap_find_new_domain(struct sdap_idmap_ctx *idmap_ctx,
     }
 
     for (c = 0; c < range_count; c++) {
-        r = range_list[c];
-
-        if (r->range_type == NULL) {
-            /* 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->secondary_base_rid != 0) {
-                /* local IPA domain */
-                rid = 0;
-                external_mapping = true;
-                name = idmap_ctx->id_ctx->be->domain->name;
-                sid = NULL;
-            } else if (r->trusted_dom_sid != NULL
-                    && r->secondary_base_rid == 0) {
-                /* trusted domain */
-                rid = r->base_rid;
-                external_mapping = false;
-                name = r->trusted_dom_sid;
-                sid = r->trusted_dom_sid;
-            } else {
-                DEBUG(SSSDBG_MINOR_FAILURE, ("Cannot determine range type, " \
-                                             "skipping id ange [%s].\n",
-                                             r->name));
-                continue;
-            }
-        } else {
-            if (strcmp(r->range_type, IPA_RANGE_LOCAL) == 0) {
-                rid = 0;
-                external_mapping = true;
-                name = idmap_ctx->id_ctx->be->domain->name;
-                sid = NULL;
-            } else if (strcmp(r->range_type, IPA_RANGE_AD_TRUST_POSIX) == 0) {
-                rid = 0;
-                external_mapping = true;
-                name = r->trusted_dom_sid;
-                sid = r->trusted_dom_sid;
-            } else if (strcmp(r->range_type, IPA_RANGE_AD_TRUST) == 0) {
-                rid = r->base_rid;
-                external_mapping = false;
-                name = r->trusted_dom_sid;
-                sid = r->trusted_dom_sid;
-            } else {
-                DEBUG(SSSDBG_MINOR_FAILURE, ("Range type [%s] not supported, " \
-                                             "skipping id range [%s].\n",
-                                             r->range_type, r->name));
-                continue;
-            }
+        ret = get_idmap_data_from_range(range_list[c],
+                                        idmap_ctx->id_ctx->be->domain->name,
+                                        &name, &sid, &rid, &range,
+                                        &external_mapping);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, ("get_idmap_data_from_range failed for " \
+                                      "id range [%s], skipping.\n",
+                                      range_list[c]->name));
+            continue;
         }
 
-        range.min = r->base_id;
-        range.max = r->base_id + r->id_range_size -1;
         err = sss_idmap_add_domain_ex(idmap_ctx->map, name, sid, &range,
-                                      r->name, rid, external_mapping);
-        if (err != IDMAP_SUCCESS && err != IDMAP_COLLISION) {
-            DEBUG(SSSDBG_CRIT_FAILURE, ("Could not add range [%s] to ID map\n",
-                                        r->name));
-            ret = EIO;
-            goto done;
+                                      range_list[c]->name, rid,
+                                      external_mapping);
+        if (err != IDMAP_SUCCESS) {
+            if (!allow_collisions || err != IDMAP_COLLISION) {
+                DEBUG(SSSDBG_CRIT_FAILURE, ("Could not add range [%s] to ID map\n",
+                                            range_list[c]->name));
+                ret = EIO;
+                goto done;
+            }
         }
     }
 
-    ret = ipa_idmap_check_posix_child(idmap_ctx, dom_name, dom_sid_str,
-                                      range_count, range_list);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, ("ipa_idmap_check_posix_child failed.\n"));
-        goto done;
+    if (dom_name != NULL || dom_sid_str != NULL) {
+        ret = ipa_idmap_check_posix_child(idmap_ctx, dom_name, dom_sid_str,
+                                          range_count, range_list);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, ("ipa_idmap_check_posix_child failed.\n"));
+            goto done;
+        }
     }
 
     ret = EOK;
@@ -263,6 +286,14 @@ done:
     return ret;
 }
 
+errno_t ipa_idmap_find_new_domain(struct sdap_idmap_ctx *idmap_ctx,
+                                  const char *dom_name,
+                                  const char *dom_sid_str)
+{
+    return ipa_idmap_get_ranges_from_sysdb(idmap_ctx, dom_name, dom_sid_str,
+                                           true);
+}
+
 errno_t ipa_idmap_init(TALLOC_CTX *mem_ctx,
                        struct sdap_id_ctx *id_ctx,
                        struct sdap_idmap_ctx **_idmap_ctx)
@@ -270,17 +301,7 @@ errno_t ipa_idmap_init(TALLOC_CTX *mem_ctx,
     errno_t ret;
     TALLOC_CTX *tmp_ctx;
     enum idmap_error_code err;
-    size_t c;
     struct sdap_idmap_ctx *idmap_ctx = NULL;
-    struct sysdb_ctx *sysdb = id_ctx->be->domain->sysdb;
-    size_t range_count;
-    struct range_info **range_list;
-    struct range_info *r;
-    struct sss_idmap_range range;
-    uint32_t rid;
-    bool external_mapping;
-    char *name;
-    char *sid;
 
     tmp_ctx = talloc_new(NULL);
     if (!tmp_ctx) return ENOMEM;
@@ -309,82 +330,13 @@ errno_t ipa_idmap_init(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-
-    /* Read in any existing mappings from the cache */
-    ret = sysdb_get_ranges(tmp_ctx, sysdb, &range_count, &range_list);
-    if (ret != EOK && ret != ENOENT) {
-        DEBUG(SSSDBG_FATAL_FAILURE,
-              ("Could not read ranges from the cache: [%s]\n",
-               strerror(ret)));
+    ret = ipa_idmap_get_ranges_from_sysdb(idmap_ctx, NULL, NULL, false);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE,
+              ("ipa_idmap_get_ranges_from_sysdb failed.\n"));
         goto done;
     }
 
-    DEBUG(SSSDBG_CONF_SETTINGS,
-          ("Initializing [%zu] domains for ID-mapping\n", range_count));
-
-    for (c = 0; c < range_count; c++) {
-
-        r = range_list[c];
-
-        if (r->range_type == NULL) {
-            /* 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->secondary_base_rid != 0) {
-                /* local IPA domain */
-                rid = 0;
-                external_mapping = true;
-                sid = NULL;
-                name = id_ctx->be->domain->name;
-            } else if (r->trusted_dom_sid != NULL
-                    && r->secondary_base_rid == 0) {
-                /* trusted domain */
-                rid = r->base_rid;
-                external_mapping = false;
-                sid = r->trusted_dom_sid;
-                name = sid;
-            } else {
-                DEBUG(SSSDBG_MINOR_FAILURE, ("Cannot determine range type, " \
-                                             "skipping id ange [%s].\n",
-                                             r->name));
-                continue;
-            }
-        } else {
-            if (strcmp(r->range_type, IPA_RANGE_LOCAL) == 0) {
-                rid = 0;
-                external_mapping = true;
-                sid = NULL;
-                name = id_ctx->be->domain->name;
-            } else if (strcmp(r->range_type, IPA_RANGE_AD_TRUST_POSIX) == 0) {
-                rid = 0;
-                external_mapping = true;
-                sid = r->trusted_dom_sid;
-                name = sid;
-            } else if (strcmp(r->range_type, IPA_RANGE_AD_TRUST) == 0) {
-                rid = r->base_rid;
-                external_mapping = false;
-                sid = r->trusted_dom_sid;
-                name = sid;
-            } else {
-                DEBUG(SSSDBG_MINOR_FAILURE, ("Range type [%s] not supported, " \
-                                             "skipping id range [%s].\n",
-                                             r->range_type, r->name));
-                continue;
-            }
-        }
-
-        range.min = r->base_id;
-        range.max = r->base_id + r->id_range_size -1;
-        err = sss_idmap_add_domain_ex(idmap_ctx->map, name, sid, &range,
-                                      r->name, rid, external_mapping);
-        if (err != IDMAP_SUCCESS) {
-            DEBUG(SSSDBG_CRIT_FAILURE, ("Could not add range [%s] to ID map\n",
-                                        r->name));
-            ret = EIO;
-            goto done;
-        }
-    }
-
     *_idmap_ctx = talloc_steal(mem_ctx, idmap_ctx);
     ret = EOK;
 
diff --git a/src/tests/cmocka/test_ipa_idmap.c b/src/tests/cmocka/test_ipa_idmap.c
new file mode 100644
index 0000000..2fb2cde
--- /dev/null
+++ b/src/tests/cmocka/test_ipa_idmap.c
@@ -0,0 +1,249 @@
+/*
+    Authors:
+        Sumit Bose <sbose at redhat.com>
+
+    Copyright (C) 2014 Red Hat
+
+    SSSD tests: Unit tests for id-mapping in the IPA provider
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <popt.h>
+
+#include "tests/cmocka/common_mock.h"
+#include "lib/idmap/sss_idmap.h"
+#include "providers/ipa/ipa_common.h"
+#include "providers/ldap/sdap_idmap.h"
+
+#define RANGE_NAME discard_const("range1")
+#define DOMAIN_SID discard_const("S-1-5-21-2-3-4")
+#define DOMAIN_NAME discard_const("dom.test")
+#define BASE_RID 111
+#define SECONDARY_BASE_RID 11223344
+#define BASE_ID 123456
+#define RANGE_SIZE 222222
+#define RANGE_MAX (BASE_ID + RANGE_SIZE - 1)
+
+void test_get_idmap_data_from_range(void **state)
+{
+    char *dom_name;
+    char *sid;
+    uint32_t rid;
+    struct sss_idmap_range range;
+    bool external_mapping;
+    size_t c;
+    errno_t ret;
+
+    struct test_data {
+        struct range_info r;
+        errno_t exp_ret;
+        char *exp_dom_name;
+        char *exp_sid;
+        uint32_t exp_rid;
+        struct sss_idmap_range exp_range;
+        bool exp_external_mapping;
+    } d[] = {
+        /* working IPA_RANGE_LOCAL range */
+        {{RANGE_NAME, BASE_ID, RANGE_SIZE, BASE_RID, SECONDARY_BASE_RID,
+          NULL, discard_const(IPA_RANGE_LOCAL)},
+         EOK, DOMAIN_NAME, NULL, 0, {BASE_ID, RANGE_MAX}, true},
+        /* working old-style IPA_RANGE_LOCAL range without range type */
+        {{RANGE_NAME, BASE_ID, RANGE_SIZE, BASE_RID, SECONDARY_BASE_RID,
+          NULL, NULL},
+         EOK, DOMAIN_NAME, NULL, 0, {BASE_ID, RANGE_MAX}, true},
+        /* old-style IPA_RANGE_LOCAL without SID and secondary base rid */
+        {{RANGE_NAME, BASE_ID, RANGE_SIZE, BASE_RID, 0, NULL, NULL},
+         EINVAL, NULL, NULL, 0, {0, 0}, false},
+        /* old-style range with SID and secondary base rid */
+        {{RANGE_NAME, BASE_ID, RANGE_SIZE, BASE_RID, SECONDARY_BASE_RID,
+          DOMAIN_SID, NULL},
+         EINVAL, NULL, NULL, 0, {0, 0}, false},
+        /* working IPA_RANGE_AD_TRUST range */
+        {{RANGE_NAME, BASE_ID, RANGE_SIZE, BASE_RID, 0, DOMAIN_SID,
+          discard_const(IPA_RANGE_AD_TRUST)},
+         EOK, DOMAIN_SID, DOMAIN_SID, BASE_RID, {BASE_ID, RANGE_MAX}, false},
+        /* working old-style IPA_RANGE_AD_TRUST range without range type */
+        {{RANGE_NAME, BASE_ID, RANGE_SIZE, BASE_RID, 0, DOMAIN_SID, NULL},
+         EOK, DOMAIN_SID, DOMAIN_SID, BASE_RID, {BASE_ID, RANGE_MAX}, false},
+        /* working IPA_RANGE_AD_TRUST_POSIX range */
+        {{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},
+        {{0}, 0, NULL, NULL, 0, {0, 0}, false}
+    };
+
+    for (c = 0; d[c].exp_dom_name != NULL; 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);
+        }
+        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);
+    }
+}
+
+errno_t __wrap_sysdb_get_ranges(TALLOC_CTX *mem_ctx, struct sysdb_ctx *sysdb,
+                                size_t *range_count,
+                                struct range_info ***range_list)
+{
+
+    *range_count = sss_mock_type(size_t);
+    *range_list = talloc_steal(mem_ctx,
+                               sss_mock_ptr_type(struct range_info **));
+    return EOK;
+}
+
+struct test_ctx {
+    struct sdap_idmap_ctx *idmap_ctx;
+    struct sdap_id_ctx *sdap_id_ctx;
+};
+
+static struct range_info **get_range_list(TALLOC_CTX *mem_ctx)
+{
+    struct range_info **range_list;
+
+    range_list = talloc_array(mem_ctx, struct range_info *, 2);
+    assert_non_null(range_list);
+
+    range_list[0] = talloc_zero(range_list, struct range_info);
+    assert_non_null(range_list[0]);
+
+    range_list[0]->name = talloc_strdup(range_list[0], RANGE_NAME);
+    assert_non_null( range_list[0]->name);
+    range_list[0]->base_id = BASE_ID;
+    range_list[0]->id_range_size = RANGE_SIZE;
+    range_list[0]->base_rid = BASE_RID;
+    range_list[0]->secondary_base_rid = 0;
+    range_list[0]->trusted_dom_sid = talloc_strdup(range_list[0], DOMAIN_SID);
+    assert_non_null(range_list[0]->trusted_dom_sid);
+    range_list[0]->range_type = talloc_strdup(range_list[0],
+                                              IPA_RANGE_AD_TRUST);
+    assert_non_null(range_list[0]->range_type);
+
+    return range_list;
+}
+
+void setup_idmap_ctx(void **state)
+{
+    int ret;
+    struct test_ctx *test_ctx;
+
+    assert_true(leak_check_setup());
+
+    test_ctx = talloc_zero(global_talloc_context, struct test_ctx);
+    assert_non_null(test_ctx);
+
+    test_ctx->sdap_id_ctx = talloc_zero(test_ctx,
+                                        struct sdap_id_ctx);
+    assert_non_null(test_ctx->sdap_id_ctx);
+
+    test_ctx->sdap_id_ctx->be = talloc_zero(test_ctx->sdap_id_ctx,
+                                            struct be_ctx);
+    assert_non_null(test_ctx->sdap_id_ctx->be);
+
+    test_ctx->sdap_id_ctx->be->domain = talloc_zero(test_ctx->sdap_id_ctx->be,
+                                                    struct sss_domain_info);
+    assert_non_null(test_ctx->sdap_id_ctx->be->domain);
+
+    test_ctx->sdap_id_ctx->be->domain->name =
+                  talloc_strdup(test_ctx->sdap_id_ctx->be->domain, DOMAIN_NAME);
+    assert_non_null(test_ctx->sdap_id_ctx->be->domain->name);
+
+    will_return(__wrap_sysdb_get_ranges, 1);
+    will_return(__wrap_sysdb_get_ranges, get_range_list(global_talloc_context));
+
+    ret = ipa_idmap_init(test_ctx, test_ctx->sdap_id_ctx,
+                         &test_ctx->idmap_ctx);
+    assert_int_equal(ret, EOK);
+
+    check_leaks_push(test_ctx);
+    *state = test_ctx;
+}
+
+void teardown_idmap_ctx(void **state)
+{
+    struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
+
+    assert_non_null(test_ctx);
+
+    assert_true(check_leaks_pop(test_ctx) == true);
+
+    talloc_free(test_ctx);
+    assert_true(leak_check_teardown());
+}
+
+void test_ipa_idmap_get_ranges_from_sysdb(void **state)
+{
+    int ret;
+    struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
+    assert_non_null(test_ctx);
+
+    will_return(__wrap_sysdb_get_ranges, 1);
+    will_return(__wrap_sysdb_get_ranges, get_range_list(test_ctx->idmap_ctx));
+    ret = ipa_idmap_get_ranges_from_sysdb(test_ctx->idmap_ctx,
+                                          DOMAIN_NAME, DOMAIN_SID, true);
+    assert_int_equal(ret, EOK);
+
+    will_return(__wrap_sysdb_get_ranges, 1);
+    will_return(__wrap_sysdb_get_ranges, get_range_list(global_talloc_context));
+    ret = ipa_idmap_get_ranges_from_sysdb(test_ctx->idmap_ctx,
+                                          DOMAIN_NAME, DOMAIN_SID, false);
+    assert_int_equal(ret, EIO);
+}
+
+int main(int argc, const char *argv[])
+{
+    poptContext pc;
+    int opt;
+    struct poptOption long_options[] = {
+        POPT_AUTOHELP
+        SSSD_DEBUG_OPTS
+        POPT_TABLEEND
+    };
+
+    const UnitTest tests[] = {
+        unit_test(test_get_idmap_data_from_range),
+        unit_test_setup_teardown(test_ipa_idmap_get_ranges_from_sysdb,
+                                 setup_idmap_ctx, teardown_idmap_ctx),
+    };
+
+    /* Set debug level to invalid value so we can deside if -d 0 was used. */
+    debug_level = SSSDBG_INVALID;
+
+    pc = poptGetContext(argv[0], argc, argv, long_options, 0);
+    while((opt = poptGetNextOpt(pc)) != -1) {
+        switch(opt) {
+        default:
+            fprintf(stderr, "\nInvalid option %s: %s\n\n",
+                    poptBadOption(pc, 0), poptStrerror(opt));
+            poptPrintUsage(pc, stderr, 0);
+            return 1;
+        }
+    }
+    poptFreeContext(pc);
+
+    DEBUG_INIT(debug_level);
+
+    tests_set_cwd();
+
+    return run_tests(tests);
+}
-- 
1.8.3.1

-------------- next part --------------
From e69fb1953acc79f063c80bf7c6f43a21a50b8597 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 7 Feb 2014 18:17:09 +0100
Subject: [PATCH 3/3] IPA: check ranges for collisions before saving them

Fixes https://fedorahosted.org/sssd/ticket/2253
---
 src/providers/ipa/ipa_subdomains.c | 83 +++++++++++++++++++++++++++++---------
 1 file changed, 63 insertions(+), 20 deletions(-)

diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
index 88b6ba5..07ae03b 100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -351,14 +351,28 @@ const char *get_flat_name_from_subdomain_name(struct be_ctx *be_ctx,
 }
 
 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 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) {
@@ -367,8 +381,8 @@ static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
     }
 
     for (c = 0; c < count; c++) {
-        range_list[c] = talloc_zero(range_list, struct range_info);
-        if (range_list[c] == NULL) {
+        r = talloc_zero(range_list, struct range_info);
+        if (r == NULL) {
             DEBUG(SSSDBG_OP_FAILURE, ("talloc_zero failed.\n"));
             ret = ENOMEM;
             goto done;
@@ -379,8 +393,8 @@ static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
             DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_get_string failed.\n"));
             goto done;
         }
-        range_list[c]->name = talloc_strdup(range_list[c], value);
-        if (range_list[c]->name == NULL) {
+        r->name = talloc_strdup(r, value);
+        if (r->name == NULL) {
             DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed.\n"));
             ret = ENOMEM;
             goto done;
@@ -388,9 +402,8 @@ static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
 
         ret = sysdb_attrs_get_string(reply[c], IPA_TRUSTED_DOMAIN_SID, &value);
         if (ret == EOK) {
-            range_list[c]->trusted_dom_sid = talloc_strdup(range_list[c],
-                                                           value);
-            if (range_list[c]->trusted_dom_sid == NULL) {
+            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;
@@ -401,28 +414,28 @@ static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
         }
 
         ret = sysdb_attrs_get_uint32_t(reply[c], IPA_BASE_ID,
-                                       &range_list[c]->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,
-                                       &range_list[c]->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,
-                                       &range_list[c]->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,
-                                       &range_list[c]->secondary_base_rid);
+                                       &r->secondary_base_rid);
         if (ret != EOK && ret != ENOENT) {
             DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_get_string failed.\n"));
             goto done;
@@ -430,8 +443,8 @@ static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
 
         ret = sysdb_attrs_get_string(reply[c], IPA_RANGE_TYPE, &value);
         if (ret == EOK) {
-            range_list[c]->range_type = talloc_strdup(range_list[c], value);
-            if (range_list[c]->range_type == NULL) {
+            r->range_type = talloc_strdup(r, value);
+            if (r->range_type == NULL) {
                 DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed.\n"));
                 ret = ENOMEM;
                 goto done;
@@ -439,23 +452,52 @@ static errno_t ipa_ranges_parse_results(TALLOC_CTX *mem_ctx,
         } 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 (range_list[c]->trusted_dom_sid == NULL) {
-                range_list[c]->range_type = talloc_strdup(range_list[c],
-                                                          IPA_RANGE_LOCAL);
+            if (r->trusted_dom_sid == NULL) {
+                r->range_type = talloc_strdup(r, IPA_RANGE_LOCAL);
             } else {
-                range_list[c]->range_type = talloc_strdup(range_list[c],
-                                                          IPA_RANGE_AD_TRUST);
+                r->range_type = talloc_strdup(r, IPA_RANGE_AD_TRUST);
             }
         } else {
             DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_get_string failed.\n"));
             goto done;
         }
-        if (range_list[c]->range_type == NULL) {
+        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 != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, ("get_idmap_data_from_range failed.\n"));
+            goto done;
+        }
+        for (d = 0; d < c; 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[c] = r;
     }
+
     range_list[c] = NULL;
 
     *_range_list = range_list;
@@ -1013,7 +1055,8 @@ static void ipa_subdomains_handler_ranges_done(struct tevent_req *req)
         goto done;
     }
 
-    ret = ipa_ranges_parse_results(ctx, reply_count, reply, &range_list);
+    ret = ipa_ranges_parse_results(ctx, domain->name,
+                                   reply_count, reply, &range_list);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE,
               ("ipa_ranges_parse_results request failed.\n"));
-- 
1.8.3.1



More information about the sssd-devel mailing list