[Freeipa-devel] [PATCH 0019] Forbid overlapping primary and secondary rid ranges

[Sumit Bose]

On Thu, Oct 18, 2012 at 08:31:50AM +0200, Tomas Babej wrote:
> On 10/17/2012 08:12 PM, Sumit Bose wrote:
> >On Wed, Oct 17, 2012 at 03:29:11PM +0200, Tomas Babej wrote:
> >>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
> >sorry, found another one ...
> >
> >...
> >>+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);
> >>+
> >you have to check that both domain_id are not NULL before calling
> >strcasecmp.
> >
> >bye,
> >Sumit
> Null pointer check added.
> 

Thank you.

ACK

bye,
Sumit
> Tomas