[Freeipa-devel] [PATCHES 0031-0032] Improve HBAC rule handling in selinuxusermap-add/mod/find

[Rob Crittenden]

Tomas Babej wrote:
> On 02/06/2013 07:57 PM, Rob Crittenden wrote:
>> Tomas Babej wrote:
>>> Hi,
>>>
>>> this pair of patches improves HBAC rule handling in selinuxusermap
>>> commands.
>>>
>>> Patch 0031 deals with:
>>> https://fedorahosted.org/freeipa/ticket/3349
>>>
>>> Patch 0032 takes care of:
>>> https://fedorahosted.org/freeipa/ticket/3348
>>>
>>> and is to be applied on top of Patch 0031.
>>>
>>> See commit messages for detailed info.
>>>
>>> Tomas
>>>
>>
>> ACK for patch 0032.
>>
>> For patch 0031 we can't change the data type of an existing attribute.
>> It will break backwards compatibility. Can you test with an older
>> client to see if it cares (it may not care about the name of the
>> type). If older clients will work then this is probably ok.
>>
>> I gather that seealso detected as a DN attribute and converted into a
>> DN class and this is blowing up the Str validator?
>>
> Yes, that was exactly the case.
>> rob
>
> I added a workaround for older client versions, tested it with
> freeipa-client/admintools 2.2, works as expeceted.
> However, this only should be issue if there is older admintools package
> on the client than on the server.
>
> Outline is such as follows: I added a new flag for DNParam seelalso
> attribute, called 'allow_malformed' that allows any string to be passed
> to DNParam. Its value gets wrapped in 'malformed=yes,value=<value>'.
> This allows to parse out the string in selinuxusermap-add/mod
> pre_callback out of the DN and search for the rule with such name so
> that it's DN gets in LDAP instead.
>
> Updated patch attached.
>
> Tomas

I like where you're going with this, just a couple of comments:

1. Should we come up with a more universal name for allow_malformed? Is 
this something that we should allow at a higher level? I was thinking 
allow_raw, or allow_non_dn, or something like that.

2. I think that if a bad dn is passed in as a Str the conversion into a 
DN won't be handled:

+            if 'allow_malformed' in self.flags:
+                dn = DN(('malformed','yes'),('value',value))

Should this be wrapped in a try/except to raise a ConversionError if it 
fails?

rob