[Freeipa-devel] [PATCH] 0022 Use ipauniqueid for the RDN of sudo commands (rebased)

[Petr Viktorin]

On 03/12/2012 06:10 PM, Martin Kosek wrote:
> On Mon, 2012-03-12 at 17:12 +0100, Petr Viktorin wrote:
>> On 03/12/2012 04:01 PM, Martin Kosek wrote:
>>> On Mon, 2012-03-12 at 14:38 +0100, Petr Viktorin wrote:
>>>> On 03/12/2012 01:26 PM, Martin Kosek wrote:
>>>>> On Thu, 2012-03-08 at 16:57 +0100, Petr Viktorin wrote:
>>>>>> Since sudo commands are case-sensitive, we can't use the CN as the RDN.
>>>>>> With this patch, the UUID is used instead.
>>>>>> It seems like a too easy fix. What am I missing?
>>>>>>
>>>>>> As far as I understand, the fact that the DN has a different structure
>>>>>> now shouldn't cause problems, even if there still are commands created
>>>>>> by old IPA versions.
>>>>>> For testing, use an unpatched version to create a few of these.
>>>>>>
>>>>>> The sudo commands are no longer sorted in sudocmd-find output. Doing
>>>>>> that would require the ability to use an arbitrary attribute as sort
>>>>>> key. Should I file an issue for that?
>>>>>
>>>>> I don't think that's necessary. We sort by LDAP object's primary key and
>>>>> since new SUDO commands still have sudocmd as its primary key, the
>>>>> sorting should just work (at least it does for me).
>>>>
>>>> Right, sorry for the noise.
>>>>
>>>>>>
>>>>>> Tests for the case sensitivity are included.
>>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/2482
>>>>>
>>>>> This works pretty fine. Both my old client tests and sudoers compat tree
>>>>> tests looks good. So, cautious ACK from me.
>>>>>
>>>>> Martin
>>>>>
>>>>
>>>> The attached version is rebased against my patch 20.
>>>>
>>>
>>> Ah, I found an issue with the changed RDN attribute. We crash when I
>>> delete sudocmd that sudorule has enrolled as a member:
>>>
>>> # ipa sudocmd-add bar1
>>> # ipa sudocmd-add bar2
>>> # ipa sudorule-add foo
>>> # ipa sudorule-add-allow-command foo --sudocmds=bar1,bar2
>>> # ipa sudocmd-del bar2
>>> # ipa sudorule-find
>>> ipa: ERROR: an internal error has occurred
>>>
>>> /var/log/httpd/error_log:
>>> [Mon Mar 12 10:41:24 2012] [error] Traceback (most recent call last):
>>> [Mon Mar 12 10:41:24 2012] [error]   File
>>> "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py",   line 315,
>>> in wsgi_execute
>>> [Mon Mar 12 10:41:24 2012] [error]     result =
>>> self.Command[name](*args, **options)
>>> [Mon Mar 12 10:41:24 2012] [error]   File
>>> "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line  438, in
>>> __call__
>>> [Mon Mar 12 10:41:24 2012] [error]     ret = self.run(*args, **options)
>>> [Mon Mar 12 10:41:24 2012] [error]   File
>>> "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line  696, in run
>>> [Mon Mar 12 10:41:24 2012] [error]     return self.execute(*args,
>>> **options)
>>> [Mon Mar 12 10:41:24 2012] [error]   File
>>> "/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.   py", line
>>> 1866, in execute
>>> [Mon Mar 12 10:41:24 2012] [error]
>>> self.obj.convert_attribute_members(e[1], *args, **options)
>>> [Mon Mar 12 10:41:24 2012] [error]   File
>>> "/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.   py", line
>>> 518, in convert_attribute_members
>>> [Mon Mar 12 10:41:24 2012] [error]
>>> ldap_obj.get_primary_key_from_dn(member)
>>> [Mon Mar 12 10:41:24 2012] [error]   File
>>> "/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.   py", line
>>> 490, in get_primary_key_from_dn
>>> [Mon Mar 12 10:41:24 2012] [error]     return dn[self.primary_key.name]
>>> [Mon Mar 12 10:41:24 2012] [error]   File
>>> "/usr/lib/python2.7/site-packages/ipalib/dn.py", line 1137,  in
>>> __getitem__
>>> [Mon Mar 12 10:41:24 2012] [error]     raise KeyError("\\"%s\\" not
>>> found in %s" % (key, self.         __str__()))
>>>
>>>
>>> The problem is in this function:
>>>       def get_primary_key_from_dn(self, dn):
>>>           try:
>>>               if self.rdn_attribute:
>>>                   (dn, entry_attrs) = self.backend.get_entry(
>>>                       dn, [self.primary_key.name]
>>>                   )
>>>                   try:
>>>                       return entry_attrs[self.primary_key.name][0]
>>>                   except (KeyError, IndexError):
>>>                       return ''
>>>           except errors.NotFound:
>>>               pass
>>>           # DN object assures we're returning a decoded (unescaped) value
>>>           dn = DN(dn)
>>>           return dn[self.primary_key.name]
>>>
>>> Martin
>>>
>>
>> Should sudocmd-del should also delete the command from any rules the
>> command is in?
>
> I would rather prevent deleting it when it is any sudorule to avoid
> unpleasant user surprises.
>
>>
>> That function needs to be fixed either way. But I'm not sure what it
>> should do if the entry doesn't exist. Return the full DN instead?
>>
>
> Probably, as this is the only value we know at the moment - it certainly
> should not crash. Although DN with ipaUniqueID won't be very helpful. A
> precaution I suggested above should prevent that.
>
> Martin
>

Here is a partial patch to do that, I'll squash after addressing the ACI 
problem. If you have time I'd welcome comments on this.

-- 
Petr³
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0022-partial.patch
Type: text/x-patch
Size: 4094 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120313/07813102/attachment.bin>