On 03/12/2012 04:44 PM, Martin Kosek wrote:
On Mon, 2012-03-12 at 16:01 +0100, 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


I found one more issue, this one is more serious (I am glad my hunch was
not malfunctioning). This patch breaks ACIs for sudocmd, user with
appropriate privilege will no longer be able to add/modify/delete sudo
commands:

# kinit fbar
Password for f...@idm.lab.bos.redhat.com:
# ipa sudorule-add fbar
----------------------
Added Sudo Rule "fbar"
----------------------
   Rule name: fbar
   Enabled: TRUE
# ipa sudorule-del fbar
------------------------
Deleted Sudo Rule "fbar"
------------------------
# ipa sudocmd-add fbar1
ipa: ERROR: Insufficient access: Insufficient 'add' privilege to add the
entry
'ipaUniqueID=c68b98ae-6c58-11e1-be75-00163e7228ea,cn=sudocmds,cn=sudo,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com'.
# ipa sudocmd-del sudocmd1
ipa: ERROR: Insufficient access: Insufficient 'delete' privilege to
delete the entry
'ipaUniqueID=0c7659d4-6c50-11e1-9b18-00163e7228ea,cn=sudocmds,cn=sudo,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com'.
# ipa sudocmd-mod sudocmd1 --desc=foo
ipa: ERROR: Insufficient access: Insufficient 'write' privilege to the
'description' attribute of entry
'ipauniqueid=0c7659d4-6c50-11e1-9b18-00163e7228ea,cn=sudocmds,cn=sudo,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com'


Rob, I heard you're the ACI expert around here.

We can change the permission subtree to ldap:///*=*,cn=sudocmds,cn=sudo,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com which would work (*if* an upgrade goes smoothly). Or do a target filter, but if I understand correctly, IPA permissions can't work with those so we'd have to make sure those don't misbehave. Or drop the entire patch for the upcoming release, and leave Sudo commands case insensitive. (There's potentially a security issue here. Sudo commands are not just the names of the binaries, but also their arguments.)

--
PetrĀ³

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to