Re: [Freeipa-devel] [PATCH] 0022 Use ipauniqueid for the RDN of sudo commands (rebased)
Earlier, someone (I think Alexander?) mentioned off-list that since the sudocmd attribute is case-sensitive, it should be compared as case-sensitive when used in the DN, so this is a directory server bug. I found now that ipalib.dn.AVA.__eq__'s docstring says: The value comparison is also case insensitive because the all [sic] attribute types used in a DN are derived from the 'name' atribute type (OID 2.5.4.41) whose EQUALITY MATCH RULE is caseIgnoreMatch. We do case-insensitive compares on DNs, and there's no easy way to change this (the DN code knows nothing about a particular schema, including case-sensitivity of its attributes). So however DS is supposed to work (I don't have a manual handy), we're pretty much committed to case-insensitive attributes in DNs. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0022 Use ipauniqueid for the RDN of sudo commands (rebased)
On Tue, 2012-03-13 at 18:17 +0100, Petr Viktorin wrote:
> 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
> > ---
Re: [Freeipa-devel] [PATCH] 0022 Use ipauniqueid for the RDN of sudo commands (rebased)
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-11e
Re: [Freeipa-devel] [PATCH] 0022 Use ipauniqueid for the RDN of sudo commands (rebased)
On Tue, 2012-03-13 at 13:43 +0100, Petr Viktorin wrote:
> 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 exis
Re: [Freeipa-devel] [PATCH] 0022 Use ipauniqueid for the RDN of sudo commands (rebased)
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³
From ad5d4716de5bd294d576785b035e9b2e581a0240 Mon Sep 17 00:00:00 2001
From: Petr Viktorin
Date: Tue, 13 Mar 2012 08:37:24 -0400
Subject: [PATCH] squash! Use ipauniqueid for the RDN of sudo commands
Prevent a sudo command from being deleted if it is a member of any
sudo rules.
When getting a primary key from a DN, if the entry doesn't exist
and the primary key is not in the DN
Re: [Freeipa-devel] [PATCH] 0022 Use ipauniqueid for the RDN of sudo commands (rebased)
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
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redha
Re: [Freeipa-devel] [PATCH] 0022 Use ipauniqueid for the RDN of sudo commands (rebased)
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?
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?
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0022 Use ipauniqueid for the RDN of sudo commands (rebased)
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
Re: [Freeipa-devel] [PATCH] 0022 Use ipauniqueid for the RDN of sudo commands (rebased)
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
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0022 Use ipauniqueid for the RDN of sudo commands (rebased)
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.
--
Petr³
From bad3dc3722720369ec95f3c6b7029db72c223c3f Mon Sep 17 00:00:00 2001
From: Petr Viktorin
Date: Thu, 8 Mar 2012 07:55:00 -0500
Subject: [PATCH] Use ipauniqueid for the RDN of sudo commands
Since sudo commands are case-sensitive, we can't use the CN
as the RDN.
Tests for case-sensitive behavior included
https://fedorahosted.org/freeipa/ticket/2482
---
ipalib/plugins/sudocmd.py |1 +
tests/test_xmlrpc/test_sudocmd_plugin.py | 82
tests/test_xmlrpc/test_sudocmdgroup_plugin.py | 86 +
tests/test_xmlrpc/xmlrpc_test.py | 13 +++-
4 files changed, 151 insertions(+), 31 deletions(-)
diff --git a/ipalib/plugins/sudocmd.py b/ipalib/plugins/sudocmd.py
index 42068edea3c51804be9ee5919934462afbee578f..f27a58cadd6e6abc16611621387f26125737bf78 100644
--- a/ipalib/plugins/sudocmd.py
+++ b/ipalib/plugins/sudocmd.py
@@ -62,6 +62,7 @@ class sudocmd(LDAPObject):
'memberof': ['sudocmdgroup'],
}
uuid_attribute = 'ipauniqueid'
+rdn_attribute = 'ipauniqueid'
label = _('Sudo Commands')
label_singular = _('Sudo Command')
diff --git a/tests/test_xmlrpc/test_sudocmd_plugin.py b/tests/test_xmlrpc/test_sudocmd_plugin.py
index cbbd26cd7061ee73c44f691692072daaf6f84f50..a9f118460432e9c33f665db05df0340d73da885d 100644
--- a/tests/test_xmlrpc/test_sudocmd_plugin.py
+++ b/tests/test_xmlrpc/test_sudocmd_plugin.py
@@ -22,17 +22,20 @@ Test the `ipalib/plugins/sudocmd.py` module.
"""
from ipalib import api, errors
-from tests.test_xmlrpc.xmlrpc_test import Declarative, fuzzy_uuid
+from tests.test_xmlrpc.xmlrpc_test import (Declarative, fuzzy_sudocmddn,
+fuzzy_uuid)
from tests.test_xmlrpc import objectclasses
from ipalib.dn import *
sudocmd1 = u'/usr/bin/sudotestcmd1'
+sudocmd1_camelcase = u'/usr/bin/sudoTestCmd1'
class test_sudocmd(Declarative):
cleanup_commands = [
('sudocmd_del', [sudocmd1], {}),
+('sudocmd_del', [sudocmd1_camelcase], {}),
]
tests = [
@@ -69,9 +72,7 @@ class test_sudocmd(Declarative):
value=sudocmd1,
summary=u'Added Sudo Command "%s"' % sudocmd1,
result=dict(
-dn=lambda x: DN(x) == \
-DN(('sudocmd',sudocmd1),('cn','sudocmds'),('cn','sudo'),
- api.env.basedn),
+dn=fuzzy_sudocmddn,
sudocmd=[sudocmd1],
description=[u'Test sudo command 1'],
objectclass=objectclasses.sudocmd,
@@ -80,6 +81,26 @@ class test_sudocmd(Declarative):
),
),
+dict(
+desc='Create %r' % sudocmd1_camelcase,
+command=('sudocmd_add', [sudocmd1_camelcase],
+dict(
+description=u'Test sudo command 2',
+),
+),
+expected=dict(
+value=sudocmd1_camelcase,
+summary=u'Added Sudo Command "%s"' % sudocmd1_camelcase,
+result=dict(
+dn=fuzzy_sudocmddn,
+sudocmd=[sudocmd1_camelcase],
+description=[u'Test sudo command 2'],
+objectclass=objectclasses.sudocmd,
+ipauniqueid=[fuzzy_uuid],
+),
+),
+),
+
dict(
desc='Try to create duplicate %r' % sudocmd1,
@@ -91,6 +112,16 @@ class test_sudocmd(Declarative):
expected=errors.DuplicateEntry(),
),
+dict(
+desc='Try to create duplicate %r' % sudocmd1_camelcase,
+command=('sudocmd_add', [sudocmd1_camelcase],
+dict(
+description=u'Test sudo command 2',
+
