Re: [Freeipa-devel] [PATCHES] 0568-0570 Convert User default permissions to managed
On 06/10/2014 10:13 AM, Martin Kosek wrote:
On 06/09/2014 02:20 PM, Petr Viktorin wrote:
On 06/06/2014 11:38 AM, Martin Kosek wrote:
[...]
4) When running user unit tests, I found couple issues:
a) Some attributes we may still miss in the permissions:
- krbPrincipalExpiration
- userclass
- ipaUserAuthType
- preferredLanguage
I am thinking we could base Modify Users permission on the read one and add
regular attributes there
I put in userclass and preferredLanguage.
I'm not sure about the other two; should regular user admins be able to change
these?
Good question. I think User Administrators should be able to set
krbPrincipalExpiration or ipaUserAuthType, though it is true it may not be part
of regular Modify Users permission and we may want to create more permissions.
Simo, does that sound OK?
I can't think of a good name. Manage User authentication?
Note that this can go in a later patch.
b) Read membership ACIs for users and groups miss member attribute and thus
indirect/direct processing goes wrong.
Added.
1) Hm, I think was not clear enough. memberOf should not be added to users, as
no object should be member of user. Please revert this change. I originally
thought it is missing in Read Group Membership permissions, but it isn't.
We are again hitting the problem of a search operation when we are not allowed
to search on all attributes (the CVE fix). This is the search produced by ipa
user-show fbar:
[10/Jun/2014:09:59:28 +0200] conn=155 op=5 SRCH base=dc=example,dc=com
scope=2
filter=(|(member=uid=fbar,cn=users,cn=accounts,dc=example,dc=com)(memberUser=uid=fbar,cn=users,cn=accounts,dc=example,dc=com)(memberHost=uid=fbar,cn=users,cn=accounts,dc=example,dc=com))
attrs=
[10/Jun/2014:09:59:28 +0200] conn=155 op=5 RESULT err=0 tag=101 nentries=0
etime=0
It returns zero results, until I also allow memberUser and memberHost:
# ipa permission-mod 'System: Read Group Membership'
--attrs={member,memberof,memberuid,memberuser,memberhost}
Now I get the results:
[10/Jun/2014:10:04:25 +0200] conn=160 op=5 SRCH base=dc=example,dc=com
scope=2
filter=(|(member=uid=fbar,cn=users,cn=accounts,dc=example,dc=com)(memberUser=uid=fbar,cn=users,cn=accounts,dc=example,dc=com)(memberHost=uid=fbar,cn=users,cn=accounts,dc=example,dc=com))
attrs=
[10/Jun/2014:10:04:25 +0200] conn=160 op=5 RESULT err=0 tag=101 nentries=1
etime=0
ipa user-show fbar
...
Member of groups: ipausers
Indirect Member of role: test
...
User still cannot see if he is direct or indirect member of role, but this may
not be a problem.
The easiest approach solution may be to update all Read * Membership
permissions/ACIs to always contain membermemberusermemberhost unless we want
to again produce multiple LDAP searches for checking direct/indirect membership.
Ah, now I see what you mean.
So: a read permission that includes any of these 3 should include all of
them.
It would make sense for makeaci to enforce this, so I'll include it in
the other patchset.
2) Installation produces update errors:
ipa.ipaserver.install.ldapupdate.LDAPUpdate: ERRORAdd failure missing
required attribute objectclass
ipa.ipaserver.install.ldapupdate.LDAPUpdate: ERRORAdd failure missing
required attribute objectclass
Problem is in install/updates/45-roles.update, permissions you converted still
have member update here.
dn: cn=Change a user password,cn=permissions,cn=pbac,$SUFFIX
add:member: 'cn=Modify Users and Reset passwords,cn=privileges,cn=pbac,$SUFFIX'
dn: cn=Modify Users,cn=permissions,cn=pbac,$SUFFIX
add:member: 'cn=Modify Users and Reset passwords,cn=privileges,cn=pbac,$SUFFIX'
Speaking if which, this privilege also needs to be added to default privileges
of the managed permissions.
Thanks for the catch.
--
Petr³
From 8b44bca76cb73aa0b0bc14e165c4462885b9b2a7 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Wed, 4 Jun 2014 16:11:30 +0200
Subject: [PATCH] managed perm updater: Handle case where we changed default
ACIs in the past
This handles the case where IPA's default ACIs changed in something else
than just attribute lists.
In this case we can narrow the set of ACIs we think the user might be
upgrading from.
Part of the work for: https://fedorahosted.org/freeipa/ticket/4346
---
.../install/plugins/update_managed_permissions.py| 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/ipaserver/install/plugins/update_managed_permissions.py b/ipaserver/install/plugins/update_managed_permissions.py
index 13433d353cd09de77029fd76f7070bf79a432774..e6f852c09d1a732109da5d56320192d4e617ab38 100644
--- a/ipaserver/install/plugins/update_managed_permissions.py
+++ b/ipaserver/install/plugins/update_managed_permissions.py
@@ -408,11 +408,20 @@ def get_upgrade_attr_lists(self, current_acistring, default_acistrings):
An attribute will be included if the user has it in LDAP but it does
not appear in *any* historic ACI.
It will be excluded
Re: [Freeipa-devel] [PATCHES] 0568-0570 Convert User default permissions to managed
On 06/06/2014 11:38 AM, Martin Kosek wrote: On 06/04/2014 06:43 PM, Petr Viktorin wrote: Hello, I try to think about any kind of data the user might have in LDAP, but in the spirit of YAGNI, I'll deal with the various corner cases in IPA's historic default permissions as I go along. Patch 0568 adds support for the case where the default permissions changed in something else than attribute lists. Needed for the 'Change User password' permission. Patch 0569 converts user permissions to managed. Patch 0570 fixes https://fedorahosted.org/freeipa/ticket/3697 1) Add aci has targetfilter part - is that intentional? Yes. From the permission plugin''s point of view, it's part of the definition of --type user (i.e. this applies to users). Regardless I think it should be there. # ipa permission-show 'System: Add Users' --all --raw ... aci: (targetfilter = (objectclass=posixaccount))(version 3.0;acl permission:System: Add Users;allow (add) groupdn = ldap:///cn=System: Add Users,cn=permissions,cn=pbac,dc=mkosek-fedora20,dc=test;) This part IS effective though, so it may not be a bad thing at all, to keep it in the ACI: # ldapadd -Y GSSAPI SASL/GSSAPI authentication started SASL username: f...@mkosek-fedora20.test SASL SSF: 56 SASL data security layer installed. dn: cn=foo,cn=users,cn=accounts,dc=mkosek-fedora20,dc=test objectclass: top objectclass: nscontainer cn: foo adding new entry cn=foo,cn=users,cn=accounts,dc=mkosek-fedora20,dc=test ldap_add: Insufficient access (50) additional info: Insufficient 'add' privilege to add the entry 'cn=foo,cn=users,cn=accounts,dc=mkosek-fedora20,dc=test'. # ipa user-add --first=Foo --last Bar fbar2 -- Added user fbar2 -- User login: fbar2 First name: Foo ... 2) System: Add User to default group I was wondering whether we should keep the AluCI in cn=groups container or directly with the group, but I think the group itself is a good idea. (Unless someone deletes and recreates it). Hm, this is a good point. If the ipausers group is deleted, there'll be an permission with a missing ACI that can't be created. That could be quite annoying. I put the ACI it in the container. 3) System: Change User password I hit some nasty DS error which prevented authorized user to update password. ACI log attached. Ludwig, does that ring any bell? The ACI itself looks OK though as after I restarted DS, it started to work. Maybe DS did not cache the ACIs properly after upgrade? Which DS version are you using? 4) When running user unit tests, I found couple issues: a) Some attributes we may still miss in the permissions: - krbPrincipalExpiration - userclass - ipaUserAuthType - preferredLanguage I am thinking we could base Modify Users permission on the read one and add regular attributes there I put in userclass and preferredLanguage. I'm not sure about the other two; should regular user admins be able to change these? b) Read membership ACIs for users and groups miss member attribute and thus indirect/direct processing goes wrong. Added. -- Petr³ From cbf9d15a6c0a3bb02d3c84594e8f299dd40f831e Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Wed, 4 Jun 2014 16:11:30 +0200 Subject: [PATCH] managed perm updater: Handle case where we changed default ACIs in the past This handles the case where IPA's default ACIs changed in something else than just attribute lists. In this case we can narrow the set of ACIs we think the user might be upgrading from. Part of the work for: https://fedorahosted.org/freeipa/ticket/4346 --- .../install/plugins/update_managed_permissions.py| 20 ++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/ipaserver/install/plugins/update_managed_permissions.py b/ipaserver/install/plugins/update_managed_permissions.py index 13433d353cd09de77029fd76f7070bf79a432774..e6f852c09d1a732109da5d56320192d4e617ab38 100644 --- a/ipaserver/install/plugins/update_managed_permissions.py +++ b/ipaserver/install/plugins/update_managed_permissions.py @@ -408,11 +408,20 @@ def get_upgrade_attr_lists(self, current_acistring, default_acistrings): An attribute will be included if the user has it in LDAP but it does not appear in *any* historic ACI. It will be excluded if it is in *all* historic ACIs but not in LDAP. +Rationale: When we don't know which version of an ACI the user is +upgrading from, we only consider attributes where all the versions +agree. For other attrs we'll use the default from the new managed perm. If the ACIs differ in something else than the list of attributes, raise IncompatibleACIModification. This means manual action is needed (either delete the old permission or change it to resemble the default -again, then re-run ipa-ldap-updater) +again, then re-run ipa-ldap-updater). + +In case there are multiple historic default ACIs, and
Re: [Freeipa-devel] [PATCHES] 0568-0570 Convert User default permissions to managed
On 06/04/2014 06:43 PM, Petr Viktorin wrote: Hello, I try to think about any kind of data the user might have in LDAP, but in the spirit of YAGNI, I'll deal with the various corner cases in IPA's historic default permissions as I go along. Patch 0568 adds support for the case where the default permissions changed in something else than attribute lists. Needed for the 'Change User password' permission. Patch 0569 converts user permissions to managed. Patch 0570 fixes https://fedorahosted.org/freeipa/ticket/3697 1) Add aci has targetfilter part - is that intentional? # ipa permission-show 'System: Add Users' --all --raw ... aci: (targetfilter = (objectclass=posixaccount))(version 3.0;acl permission:System: Add Users;allow (add) groupdn = ldap:///cn=System: Add Users,cn=permissions,cn=pbac,dc=mkosek-fedora20,dc=test;) This part IS effective though, so it may not be a bad thing at all, to keep it in the ACI: # ldapadd -Y GSSAPI SASL/GSSAPI authentication started SASL username: f...@mkosek-fedora20.test SASL SSF: 56 SASL data security layer installed. dn: cn=foo,cn=users,cn=accounts,dc=mkosek-fedora20,dc=test objectclass: top objectclass: nscontainer cn: foo adding new entry cn=foo,cn=users,cn=accounts,dc=mkosek-fedora20,dc=test ldap_add: Insufficient access (50) additional info: Insufficient 'add' privilege to add the entry 'cn=foo,cn=users,cn=accounts,dc=mkosek-fedora20,dc=test'. # ipa user-add --first=Foo --last Bar fbar2 -- Added user fbar2 -- User login: fbar2 First name: Foo ... 2) System: Add User to default group I was wondering whether we should keep the ACI in cn=groups container or directly with the group, but I think the group itself is a good idea. (Unless someone deletes and recreates it). 3) System: Change User password I hit some nasty DS error which prevented authorized user to update password. ACI log attached. Ludwig, does that ring any bell? The ACI itself looks OK though as after I restarted DS, it started to work. Maybe DS did not cache the ACIs properly after upgrade? 4) When running user unit tests, I found couple issues: a) Some attributes we may still miss in the permissions: - krbPrincipalExpiration - userclass - ipaUserAuthType - preferredLanguage I am thinking we could base Modify Users permission on the read one and add regular attributes there b) Read membership ACIs for users and groups miss member attribute and thus indirect/direct processing goes wrong. This is all I could find, patches are looking good, otherwise. Martin [06/Jun/2014:11:17:16 +0200] NSACLPlugin - conn=4 op=742 (main): Allow search on entry(krbprincipalname=krbtgt/mkosek-fedora20.t...@mkosek-fedora20.test,cn=mkosek-fedora20.test,cn=kerberos,dc=mkosek-fedora20,dc=test): root user [06/Jun/2014:11:17:16 +0200] NSACLPlugin - conn=4 op=742 (main): Allow search on entry(krbprincipalname=krbtgt/mkosek-fedora20.t...@mkosek-fedora20.test,cn=mkosek-fedora20.test,cn=kerberos,dc=mkosek-fedora20,dc=test): root user [06/Jun/2014:11:17:16 +0200] NSACLPlugin - conn=4 op=742 (main): Allow search on entry(krbprincipalname=krbtgt/mkosek-fedora20.t...@mkosek-fedora20.test,cn=mkosek-fedora20.test,cn=kerberos,dc=mkosek-fedora20,dc=test): root user [06/Jun/2014:11:17:16 +0200] NSACLPlugin - Root access (read) allowed on entry(krbprincipalname=krbtgt/mkosek-fedora20.t...@mkosek-fedora20.test,cn=mkosek-fedora20.test,cn=kerberos,dc=mkosek-fedora20,dc=test) [06/Jun/2014:11:17:16 +0200] NSACLPlugin - Root access (read) allowed on entry(krbprincipalname=krbtgt/mkosek-fedora20.t...@mkosek-fedora20.test,cn=mkosek-fedora20.test,cn=kerberos,dc=mkosek-fedora20,dc=test) [06/Jun/2014:11:17:16 +0200] NSACLPlugin - Root access (read) allowed on entry(krbprincipalname=krbtgt/mkosek-fedora20.t...@mkosek-fedora20.test,cn=mkosek-fedora20.test,cn=kerberos,dc=mkosek-fedora20,dc=test) [06/Jun/2014:11:17:16 +0200] NSACLPlugin - Root access (read) allowed on entry(krbprincipalname=krbtgt/mkosek-fedora20.t...@mkosek-fedora20.test,cn=mkosek-fedora20.test,cn=kerberos,dc=mkosek-fedora20,dc=test) [06/Jun/2014:11:17:16 +0200] NSACLPlugin - Root access (read) allowed on entry(krbprincipalname=krbtgt/mkosek-fedora20.t...@mkosek-fedora20.test,cn=mkosek-fedora20.test,cn=kerberos,dc=mkosek-fedora20,dc=test) [06/Jun/2014:11:17:16 +0200] NSACLPlugin - Root access (read) allowed on entry(krbprincipalname=krbtgt/mkosek-fedora20.t...@mkosek-fedora20.test,cn=mkosek-fedora20.test,cn=kerberos,dc=mkosek-fedora20,dc=test) [06/Jun/2014:11:17:16 +0200] NSACLPlugin - Root access (read) allowed on entry(krbprincipalname=krbtgt/mkosek-fedora20.t...@mkosek-fedora20.test,cn=mkosek-fedora20.test,cn=kerberos,dc=mkosek-fedora20,dc=test) [06/Jun/2014:11:17:16 +0200] NSACLPlugin - Root access (read) allowed on entry(krbprincipalname=krbtgt/mkosek-fedora20.t...@mkosek-fedora20.test,cn=mkosek-fedora20.test,cn=kerberos,dc=mkosek-fedora20,dc=test) [06/Jun/2014:11:17:16 +0200] NSACLPlugin - Root
[Freeipa-devel] [PATCHES] 0568-0570 Convert User default permissions to managed
Hello,
I try to think about any kind of data the user might have in LDAP, but
in the spirit of YAGNI, I'll deal with the various corner cases in IPA's
historic default permissions as I go along.
Patch 0568 adds support for the case where the default permissions
changed in something else than attribute lists. Needed for the 'Change
User password' permission.
Patch 0569 converts user permissions to managed.
Patch 0570 fixes https://fedorahosted.org/freeipa/ticket/3697
--
Petr³
From a09d3e24805f29a828f67ee4cab3a6f8bbc0e693 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Wed, 4 Jun 2014 16:11:30 +0200
Subject: [PATCH] managed perm updater: Handle case where we changed default
ACIs in the past
This handles the case where IPA's default ACIs changed in something else
than just attribute lists.
In this case we can narrow the set of ACIs we think the user might be
upgrading from.
Part of the work for: https://fedorahosted.org/freeipa/ticket/4346
---
.../install/plugins/update_managed_permissions.py| 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/ipaserver/install/plugins/update_managed_permissions.py b/ipaserver/install/plugins/update_managed_permissions.py
index 13433d353cd09de77029fd76f7070bf79a432774..e6f852c09d1a732109da5d56320192d4e617ab38 100644
--- a/ipaserver/install/plugins/update_managed_permissions.py
+++ b/ipaserver/install/plugins/update_managed_permissions.py
@@ -408,11 +408,20 @@ def get_upgrade_attr_lists(self, current_acistring, default_acistrings):
An attribute will be included if the user has it in LDAP but it does
not appear in *any* historic ACI.
It will be excluded if it is in *all* historic ACIs but not in LDAP.
+Rationale: When we don't know which version of an ACI the user is
+upgrading from, we only consider attributes where all the versions
+agree. For other attrs we'll use the default from the new managed perm.
If the ACIs differ in something else than the list of attributes,
raise IncompatibleACIModification. This means manual action is needed
(either delete the old permission or change it to resemble the default
-again, then re-run ipa-ldap-updater)
+again, then re-run ipa-ldap-updater).
+
+In case there are multiple historic default ACIs, and some of them
+are compatible with the current but other ones aren't, we deduce that
+the user is upgrading from one of the compatible ones.
+The incompatible ones are removed from consideration, both for
+compatibility and attribute lists.
assert default_acistrings
@@ -434,6 +443,7 @@ def _pop_targetattr(aci):
attrs_in_all_defaults = None
attrs_in_any_defaults = set()
+all_incompatible = True
for default_acistring in default_acistrings:
default_aci = ACI(default_acistring)
default_attrs = _pop_targetattr(default_aci)
@@ -442,7 +452,9 @@ def _pop_targetattr(aci):
if current_aci != default_aci:
self.log.debug('ACIs not compatible')
-raise(IncompatibleACIModification())
+continue
+else:
+all_incompatible = False
if attrs_in_all_defaults is None:
attrs_in_all_defaults = set(default_attrs)
@@ -450,6 +462,10 @@ def _pop_targetattr(aci):
attrs_in_all_defaults = attrs_in_all_defaults
attrs_in_any_defaults |= default_attrs
+if all_incompatible:
+self.log.debug('All old default ACIs are incompatible')
+raise(IncompatibleACIModification())
+
included = current_attrs - attrs_in_any_defaults
excluded = attrs_in_all_defaults - current_attrs
--
1.9.0
From fb01e2a0c9e84ff618b5de01c1373bded154e5d9 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Wed, 4 Jun 2014 15:21:26 +0200
Subject: [PATCH] Convert User default permissions to managed
Part of the work for: https://fedorahosted.org/freeipa/ticket/4346
---
install/share/delegation.ldif| 72 ---
install/updates/40-delegation.update | 16 ---
ipalib/plugins/user.py | 84
3 files changed, 84 insertions(+), 88 deletions(-)
diff --git a/install/share/delegation.ldif b/install/share/delegation.ldif
index 43d13974ffd63ea6ee554c815b911715609149b8..2c712bfc174b3151a5bf69e37ebfe58f2fff96f4 100644
--- a/install/share/delegation.ldif
+++ b/install/share/delegation.ldif
@@ -133,65 +133,6 @@ dn: cn=Host Enrollment,cn=privileges,cn=pbac,$SUFFIX
# Default permissions.
-# User administration
-
-dn: cn=Add Users,cn=permissions,cn=pbac,$SUFFIX
-changetype: add
-objectClass: top
-objectClass: groupofnames
-objectClass: ipapermission
-cn: Add Users
-member: cn=User
