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

[Tomas Babej]

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.

Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0019-4-Forbid-overlapping-primary-and-secondary-rid-ranges.patch
Type: text/x-patch
Size: 14570 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20121018/f2f4ac0b/attachment.bin>