On 06/10/2014 01:16 PM, Petr Viktorin wrote:
> 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 member&memberuser&memberhost 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.

Ok.

>> 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.
> 
> Thanks for the catch.

I could not find anything else, we can deal with additional fixes in other 
patches.

ACK to all 3 patches, pushed to master: 46faed0b4bb2169fd2be79809b9a65b77a72ee14

Martin

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

Reply via email to