On 10/17/2012 02:34 PM, Sumit Bose wrote:
On Wed, Oct 17, 2012 at 12:59:52PM +0200, Tomas Babej wrote:
On 10/17/2012 11:14 AM, Sumit Bose wrote:
On Tue, Oct 16, 2012 at 02:26:24PM +0200, Tomas Babej wrote:
Hi,

commands ipa idrange-add / idrange-mod no longer allows the user
to enter primary or secondary rid range such that has non-zero
intersection with primary or secondary rid range of another
existing id range, as this could cause collision.

Unit tests added to test_range_plugin.py

https://fedorahosted.org/freeipa/ticket/3086

Tomas
Thank you for the patch, comments are in-line.

bye,
Sumit

....
Thank you for your suggestions. New version of the patch attached.

Tomas
Thank you for addressing my comments. I just realized that the check is
too strict.

The ranges of the Posix IDs [base_id - base_id+id_range_size) may not
overlap for any existing range because those IDs belong to the single
Posix ID namespace of the IPA domain. I.e each user, local or from a
trusted domain, must have a unique Posix ID.

The RID ranges [base_rid, base_rid+id_range_size) and
[secondary_base_rid, secondary_base_rid+id_range_size) may not overlap
with RID ranges from the same domain. So the RID ranges for the local
domain may not overlap and the RID ranges for any specific trusted
domain may not overlap. It is allowed that there is a range form the
local domain may have base_rid=1000 and a range from a trusted domain as
well. This is ok because the RID is only part of the identifier, each
domain has a unique domain SID which is used together with the RID to
identify e.g. a user.

I would suggest to look for the ipaNTTrustedDomainSID attribute in
slapi_entry_to_range_info() too and add it to struct range_info. In
ranges_overlap() you can then check the Posix ID range for all ranges
but do the RID checks only when the domain identifiers are either both
NULL (local IPA domain) or are the same strings.

Sorry for not seeing this earlier.

bye,
Sumit

Thanks for catching this issue. It is solved in the newest revision
of the patch.

Tomas
>From dab63f5d42e53218a0611c82a1cb0768ad4be17f Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Mon, 15 Oct 2012 06:28:16 -0400
Subject: [PATCH] Forbid overlapping primary and secondary rid ranges

Commands ipa idrange-add / idrange-mod no longer allows the user
to enter primary or secondary rid range such that has non-zero
intersection with primary or secondary rid range of another
existing id range, as this could cause collision.

Unit tests added to test_range_plugin.py

https://fedorahosted.org/freeipa/ticket/3086
---
 .../ipa-range-check/ipa_range_check.c              | 114 +++++++++++++++++---
 tests/test_xmlrpc/test_range_plugin.py             | 120 +++++++++++++++++++--
 2 files changed, 212 insertions(+), 22 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
index 499e54a9c4a4c9134a231c0cd09e700390565a14..b866259134658da77aff3760b872acfe4ed5a5fe 100644
--- a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
+++ b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
@@ -49,6 +49,7 @@
 #define IPA_ID_RANGE_SIZE "ipaIDRangeSize"
 #define IPA_BASE_RID "ipaBaseRID"
 #define IPA_SECONDARY_BASE_RID "ipaSecondaryBaseRID"
+#define IPA_DOMAIN_ID "ipaNTTrustedDomainSID"
 #define RANGES_FILTER "objectclass=ipaIDRange"
 
 #define IPA_PLUGIN_NAME "ipa-range-check"
@@ -70,6 +71,7 @@ struct ipa_range_check_ctx {
 
 struct range_info {
     char *name;
+    char *domain_id;
     uint32_t base_id;
     uint32_t id_range_size;
     uint32_t base_rid;
@@ -93,6 +95,8 @@ static int slapi_entry_to_range_info(struct slapi_entry *entry,
         return EINVAL;
     }
 
+    range->domain_id = slapi_entry_attr_get_charptr(entry, IPA_DOMAIN_ID);
+
     ul_val = slapi_entry_attr_get_ulong(entry, IPA_BASE_ID);
     if (ul_val == 0 || ul_val >= UINT32_MAX) {
         ret = ERANGE;
@@ -132,24 +136,81 @@ done:
     return ret;
 }
 
-#define IN_RANGE(x,base,size) ( (x) >= (base) && ((x) - (base)) < (size) )
-static bool ranges_overlap(struct range_info *r1, struct range_info *r2)
+#define IN_RANGE(x,base,size) ( (x) >= (base) && ((x) - (base) < (size)) )
+static bool intervals_overlap(uint32_t x, uint32_t base, uint32_t x_size, uint32_t base_size)
 {
-    if (r1->name != NULL && r2->name != NULL &&
-        strcasecmp(r1->name, r2->name) == 0) {
-        return false;
-    }
-
-    if (IN_RANGE(r1->base_id, r2->base_id, r2->id_range_size) ||
-        IN_RANGE((r1->base_id + r1->id_range_size - 1), r2->base_id, r2->id_range_size) ||
-        IN_RANGE(r2->base_id, r1->base_id, r1->id_range_size) ||
-        IN_RANGE((r2->base_id + r2->id_range_size - 1), r1->base_id, r1->id_range_size)) {
+    if (IN_RANGE(x, base, base_size) ||
+        IN_RANGE((x + x_size - 1), base, base_size) ||
+        IN_RANGE(base, x, x_size) ||
+        IN_RANGE((base + base_size - 1), x, x_size)) {
         return true;
     }
 
     return false;
 }
 
+/**
+ * returns 0 if there is no overlap
+ *
+ * connected ranges must not overlap:
+ * existing range:  base  rid  sec_rid
+ *                   |     |  \  / |
+ *                   |     |   \/  |
+ *                   |     |   /\  |
+ *                   |     |  /  \ |
+ * new range:       base  rid  sec_rid
+ **/
+static int ranges_overlap(struct range_info *r1, struct range_info *r2)
+{
+    if (r1->name != NULL && r2->name != NULL &&
+        strcasecmp(r1->name, r2->name) == 0) {
+        return 0;
+    }
+
+    /* check if base range overlaps with existing base range */
+    if (intervals_overlap(r1->base_id, r2->base_id,
+        r1->id_range_size, r2->id_range_size)){
+        return 1;
+    }
+
+    /* if both base_rid and secondary_base_rid = 0, the rid range is not set */
+    bool rid_ranges_set = (r1->base_rid != 0 || r1->secondary_base_rid != 0) &&
+                          (r2->base_rid != 0 || r2->secondary_base_rid != 0);
+
+    bool ranges_from_same_domain =
+         (r1->domain_id == NULL && r2->domain_id == NULL) ||
+         (strcasecmp(r1->domain_id, r2->domain_id) == 0);
+
+    /**
+     * in case rid range is not set or ranges belong to different domains
+     * we can skip rid range tests as they are irrelevant
+     */
+    if (rid_ranges_set && ranges_from_same_domain){
+
+        /* check if rid range overlaps with existing rid range */
+        if (intervals_overlap(r1->base_rid, r2->base_rid,
+            r1->id_range_size, r2->id_range_size))
+            return 2;
+
+        /* check if secondary rid range overlaps with existing secondary rid range */
+        if (intervals_overlap(r1->secondary_base_rid, r2->secondary_base_rid,
+            r1->id_range_size, r2->id_range_size))
+            return 3;
+
+        /* check if rid range overlaps with existing secondary rid range */
+        if (intervals_overlap(r1->base_rid, r2->secondary_base_rid,
+            r1->id_range_size, r2->id_range_size))
+            return 4;
+
+        /* check if secondary rid range overlaps with existing rid range */
+        if (intervals_overlap(r1->secondary_base_rid, r2->base_rid,
+            r1->id_range_size, r2->id_range_size))
+            return 5;
+    }
+
+    return 0;
+}
+
 static int ipa_range_check_start(Slapi_PBlock *pb)
 {
     return 0;
@@ -176,7 +237,7 @@ static int ipa_range_check_pre_op(Slapi_PBlock *pb, int modtype)
     int search_result;
     Slapi_Entry **search_entries = NULL;
     size_t c;
-    bool overlap = true;
+    int no_overlap = 0;
     const char *check_attr;
     char *errmsg = NULL;
 
@@ -315,13 +376,34 @@ static int ipa_range_check_pre_op(Slapi_PBlock *pb, int modtype)
             goto done;
         }
 
-        overlap = ranges_overlap(old_range, new_range);
+        no_overlap = ranges_overlap(new_range, old_range);
         free(old_range);
         old_range = NULL;
-        if (overlap) {
-            LOG_FATAL("New range overlaps with existing one.\n");
+        if (no_overlap != 0) {
             ret = LDAP_CONSTRAINT_VIOLATION;
-            errmsg = "New range overlaps with existing one.";
+
+            switch (no_overlap){
+            case 1:
+                errmsg = "New base range overlaps with existing base range.";
+                break;
+            case 2:
+                errmsg = "New primary rid range overlaps with existing primary rid range.";
+                break;
+            case 3:
+                errmsg = "New secondary rid range overlaps with existing secondary rid range.";
+                break;
+            case 4:
+                errmsg = "New primary rid range overlaps with existing secondary rid range.";
+                break;
+            case 5:
+                errmsg = "New secondary rid range overlaps with existing primary rid range.";
+                break;
+            default:
+                errmsg = "New range overlaps with existing one.";
+                break;
+            }
+
+            LOG_FATAL("%s\n",errmsg);
             goto done;
         }
     }
diff --git a/tests/test_xmlrpc/test_range_plugin.py b/tests/test_xmlrpc/test_range_plugin.py
index 00e430bf0458e40f622e731566de2ab760f49bf5..56c6db528b6c9cc4e1ac8cf22232fdfa46953389 100644
--- a/tests/test_xmlrpc/test_range_plugin.py
+++ b/tests/test_xmlrpc/test_range_plugin.py
@@ -27,10 +27,41 @@ from xmlrpc_test import Declarative, fuzzy_digits, fuzzy_uuid
 from tests.test_xmlrpc import objectclasses
 from ipapython.dn import *
 
-testrange1 = u't-range-1'
+testrange1 = u'testrange1'
 testrange1_base_id = 900000
 testrange1_size = 99999
 
+testrange2 = u'testrange2'
+testrange2_base_id = 100
+testrange2_size = 50
+testrange2_base_rid = 100
+testrange2_secondary_base_rid=1000
+
+testrange3 = u'testrange3'
+testrange3_base_id = 200
+testrange3_size = 50
+testrange3_base_rid = 70
+testrange3_secondary_base_rid=1100
+
+testrange4 = u'testrange4'
+testrange4_base_id = 300
+testrange4_size = 50
+testrange4_base_rid = 200
+testrange4_secondary_base_rid=1030
+
+testrange5 = u'testrange5'
+testrange5_base_id = 400
+testrange5_size = 50
+testrange5_base_rid = 1020
+testrange5_secondary_base_rid=1200
+
+testrange6 = u'testrange6'
+testrange6_base_id = 130
+testrange6_size = 50
+testrange6_base_rid = 500
+testrange6_secondary_base_rid=1300
+
+
 user1=u'tuser1'
 user1_uid = 900000
 group1=u'group1'
@@ -38,7 +69,7 @@ group1_gid = 900100
 
 class test_range(Declarative):
     cleanup_commands = [
-        ('idrange_del', [testrange1], {}),
+        ('idrange_del', [testrange1,testrange2,testrange3,testrange4,testrange5,testrange6], {}),
         ('user_del', [user1], {}),
         ('group_del', [group1], {}),
     ]
@@ -48,7 +79,7 @@ class test_range(Declarative):
             desc='Create ID range %r' % (testrange1),
             command=('idrange_add', [testrange1],
                       dict(ipabaseid=testrange1_base_id, ipaidrangesize=testrange1_size,
-                           ipabaserid=1000, ipasecondarybaserid=20000)),
+                           ipabaserid=10000, ipasecondarybaserid=20000)),
             expected=dict(
                 result=dict(
                     dn=DN(('cn',testrange1),('cn','ranges'),('cn','etc'),
@@ -56,7 +87,7 @@ class test_range(Declarative):
                     cn=[testrange1],
                     objectclass=[u'ipaIDrange', u'ipadomainidrange'],
                     ipabaseid=[unicode(testrange1_base_id)],
-                    ipabaserid=[u'1000'],
+                    ipabaserid=[u'10000'],
                     ipasecondarybaserid=[u'20000'],
                     ipaidrangesize=[unicode(testrange1_size)],
                     iparangetype=[u'local domain range'],
@@ -75,7 +106,7 @@ class test_range(Declarative):
                           api.env.basedn),
                     cn=[testrange1],
                     ipabaseid=[unicode(testrange1_base_id)],
-                    ipabaserid=[u'1000'],
+                    ipabaserid=[u'10000'],
                     ipasecondarybaserid=[u'20000'],
                     ipaidrangesize=[unicode(testrange1_size)],
                     iparangetype=[u'local domain range'],
@@ -179,7 +210,7 @@ class test_range(Declarative):
                 result=dict(
                     cn=[testrange1],
                     ipabaseid=[unicode(testrange1_base_id)],
-                    ipabaserid=[u'1000'],
+                    ipabaserid=[u'10000'],
                     ipasecondarybaserid=[u'20000'],
                     ipaidrangesize=[u'90000'],
                     iparangetype=[u'local domain range'],
@@ -231,4 +262,81 @@ class test_range(Declarative):
             ),
         ),
 
+        dict(
+            desc='Create ID range %r' % (testrange2),
+            command=('idrange_add', [testrange2],
+                      dict(ipabaseid=testrange2_base_id,
+                          ipaidrangesize=testrange2_size,
+                          ipabaserid=testrange2_base_rid,
+                          ipasecondarybaserid=testrange2_secondary_base_rid)),
+            expected=dict(
+                result=dict(
+                    dn=DN(('cn',testrange2),('cn','ranges'),('cn','etc'),
+                          api.env.basedn),
+                    cn=[testrange2],
+                    objectclass=[u'ipaIDrange', u'ipadomainidrange'],
+                    ipabaseid=[unicode(testrange2_base_id)],
+                    ipabaserid=[unicode(testrange2_base_rid)],
+                    ipasecondarybaserid=[unicode(testrange2_secondary_base_rid)],
+                    ipaidrangesize=[unicode(testrange2_size)],
+                    iparangetype=[u'local domain range'],
+                ),
+                value=testrange2,
+                summary=u'Added ID range "%s"' % (testrange2),
+            ),
+        ),
+
+        dict(
+            desc='Try to create ID range %r with overlapping rid range' % (testrange3),
+            command=('idrange_add', [testrange3],
+                      dict(ipabaseid=testrange3_base_id,
+                          ipaidrangesize=testrange3_size,
+                          ipabaserid=testrange3_base_rid,
+                          ipasecondarybaserid=testrange3_secondary_base_rid)),
+            expected=errors.DatabaseError(
+                desc='Constraint violation', info='New primary rid range overlaps with existing primary rid range.'),
+        ),
+
+       dict(
+            desc='Try to create ID range %r with overlapping secondary rid range' % (testrange4),
+            command=('idrange_add', [testrange4],
+                      dict(ipabaseid=testrange4_base_id,
+                          ipaidrangesize=testrange4_size,
+                          ipabaserid=testrange4_base_rid,
+                          ipasecondarybaserid=testrange4_secondary_base_rid)),
+            expected=errors.DatabaseError(
+                desc='Constraint violation', info='New secondary rid range overlaps with existing secondary rid range.'),
+        ),
+
+        dict(
+            desc='Try to create ID range %r with primary range overlapping secondary rid range' % (testrange5),
+            command=('idrange_add', [testrange5],
+                      dict(ipabaseid=testrange5_base_id,
+                          ipaidrangesize=testrange5_size,
+                          ipabaserid=testrange5_base_rid,
+                          ipasecondarybaserid=testrange5_secondary_base_rid)),
+            expected=errors.DatabaseError(
+                desc='Constraint violation', info='New primary rid range overlaps with existing secondary rid range.'),
+        ),
+
+        dict(
+            desc='Try to create ID range %r with overlapping id range' % (testrange6),
+            command=('idrange_add', [testrange6],
+                      dict(ipabaseid=testrange6_base_id,
+                          ipaidrangesize=testrange6_size,
+                          ipabaserid=testrange6_base_rid,
+                          ipasecondarybaserid=testrange6_secondary_base_rid)),
+            expected=errors.DatabaseError(
+                desc='Constraint violation', info='New base range overlaps with existing base range.'),
+        ),
+
+        dict(
+            desc='Delete ID range %r' % testrange2,
+            command=('idrange_del', [testrange2], {}),
+            expected=dict(
+                result=dict(failed=u''),
+                value=testrange2,
+                summary=u'Deleted ID range "%s"' % testrange2,
+            ),
+        ),
     ]
-- 
1.7.11.7

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to