[Freeipa-devel] [PATCHES] 0568-0570 Convert User default permissions to managed

[Martin Kosek]

On 06/09/2014 02:20 PM, Petr Viktorin wrote:
> 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: fbar at 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?

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.

> 
>> 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 member&memberuser&memberhost unless we want
to again produce multiple LDAP searches for checking direct/indirect membership.


2) Installation produces update errors:

ipa.ipaserver.install.ldapupdate.LDAPUpdate: ERROR    Add failure missing
required attribute "objectclass"
ipa.ipaserver.install.ldapupdate.LDAPUpdate: ERROR    Add 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.

Martin