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

Sumit Bose sbose at redhat.com
Wed Feb 12 14:43:51 UTC 2014


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
-------------- next part --------------
From 09a859f8162fd9dff0c92c332700fd58afbf321a 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 f333b85472ed37868bff29a1a71109965fc967ab 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                       |  13 +++
 src/providers/ipa/ipa_common.h    |   5 +
 src/providers/ipa/ipa_idmap.c     | 239 +++++++++++++++-----------------------
 src/tests/cmocka/test_ipa_idmap.c | 139 ++++++++++++++++++++++
 4 files changed, 252 insertions(+), 144 deletions(-)
 create mode 100644 src/tests/cmocka/test_ipa_idmap.c

diff --git a/Makefile.am b/Makefile.am
index ad51fd4..2585eb7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -156,6 +156,7 @@ if HAVE_CMOCKA
         dyndns-tests \
         fqnames-tests \
         test_sss_idmap \
+        test_ipa_idmap \
         test_utils \
         ad_access_filter_tests \
         ad_common_tests \
@@ -1489,6 +1490,18 @@ 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_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..3c160f4 100644
--- a/src/providers/ipa/ipa_common.h
+++ b/src/providers/ipa/ipa_common.h
@@ -195,6 +195,11 @@ 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_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..0f46129 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;
+}
+
+static 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,36 @@ 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) {
+                                      range_list[c]->name, rid,
+                                      external_mapping);
+        if (err != IDMAP_SUCCESS
+                && allow_collisions && err != IDMAP_COLLISION) {
             DEBUG(SSSDBG_CRIT_FAILURE, ("Could not add range [%s] to ID map\n",
-                                        r->name));
+                                        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 +285,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 +300,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 +329,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..5b4e34d
--- /dev/null
+++ b/src/tests/cmocka/test_ipa_idmap.c
@@ -0,0 +1,139 @@
+/*
+    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"
+
+#define RANGE_NAME discard_const("range1")
+#define DOMAIN_SID discard_const("S-1-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);
+    }
+}
+
+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),
+        unit_test(test_get_idmap_data_from_range),
+    };
+
+    /* 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 1ce1e9071c796be26dcda4c605c0eda426f739f4 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

---
 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