[Freeipa-devel] [PATCH] 784 limit what attributes may be modified

[Rob Crittenden]

Martin Kosek wrote:
> On Mon, 2011-05-16 at 17:46 -0400, Rob Crittenden wrote:
>> Add option to limit the attributes allowed in an entry.
>>
>> Kerberos ticket policy can update policy in a user entry. This allowed
>> set/addattr to be used to modify attributes outside of the ticket policy
>> perview, also bypassing all validation/normalization. Likewise the
>> ticket policy was updatable by the user plugin bypassing all validation.
>>
>> Add two new LDAPObject values to control this behavior:
>>
>> limit_object_classes: only attributes in these are allowed
>> disallow_object_classes: attributes in these are disallowed
>>
>> By default both of these lists are empty so are skipped.
>>
>> ticket 744
>>
>> rob
>
> NACK. I have some concerns with this patch. In function
> _check_limit_object_class:
>
> 1) You change input attribute 'attrs' by removing the items from it. If
> user passes the same list of attrs to be checked and the function is run
> twice, the 'attrs' parameter in second run is corrupt.
>
> You can try it by running e.g. `ipa krbtpolicy-mod --maxrenew=24044' and
> checking the value of this parameter in the function.

Good catch, updated patch attached.

>
> 2) The purpose of this statement is not clear to me:
> +    if len(attrs)>  0 and allow_only:
> +        raise errors.ObjectclassViolation(info='attribute "%(attribute)s" not allowed' % dict(attribute=attrs[0]))
> Maybe just the exception text is misleading.

This function has 2 modes: "allow only the attributes in these 
objectclasses" or "specifically deny the attributes in these 
objectclasses". This enforces the first type. If when we've gone through 
all the attributes there are any left over they must not be allowed so 
raise an error. This is documented in the function header.

>
> Otherways it's good, it correctly raised an exception when I tried to
> misuse --setattr option.
>
> Martin
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-rcrit-784-2-krbtpolicy.patch
Type: text/x-diff
Size: 11405 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110527/31f72028/attachment.bin>