[Freeipa-devel] [PATCHES] 0572-0575 Add ACI.txt + default bind rule type

Tue Jun 10 13:22:05 UTC 2014

On 06/10/2014 01:30 PM, Martin Kosek wrote:
> On 06/10/2014 10:05 AM, Petr Viktorin wrote:
>> On 06/09/2014 08:08 PM, Petr Viktorin wrote:
>>> Having another verification tool should help reviewing the permission
>>> patches.
>>>
>>>
>>> To avoid conflicts, apply on top of my patches 0568-0570 (Write User
>>> permissions).
>>>
>>>
>>> 0572: I tried to make the ACIs generated by the permission plugin as
>>> predictable as possible, but I missed one place it's affected by
>>> dict/set iteration order (which is undefined). Here's a fix.
>>>
>>> 0573: Minor refactoring to make the next patch easier.
>>>
>>> 0574: Add ACI.txt & makeaci. Due to the predictable ACIs, all this needs
>>> to do is generate the file; comparing can be done bit-by-bit.
>>> I do run the validation results through difflib, but frankly it's easier
>>> just to use Git.
>>
>> On my way home yesterday I remembered I left out an important piece of
>> information - the DN where the ACI is. Attaching updated patch 0574.
>>
>>> 0575: Make 'permission' the default bind rule type for managed
>>> permissions. Rationale in the commit message.
>>> Run makeaci to verify this doesn't change the result.
>
> This will create some additional burden for you when converting ACIs, but the
> idea is good.

Any ACI.txt conflicts are easily resolved by running makeaci, and I 
think the additional verification is worth it.

> The script worked for me, can we just create some more friendly error message
> than an assertion traceback?
>
> # ./makeaci --validate
> ...
> Traceback (most recent call last):
>    File "./makeaci", line 116, in <module>
>      main(options)
>    File "./makeaci", line 108, in main
>      raise AssertionError('validation failed')
> AssertionError: validation failed

The tool is meant for developers and packagers; tracebacks are designed 
to be friendly for this target group.

But, OK, I've made it exit() instead, and added some instructions.

 From the thread on user permissions:
On 06/10/2014 10:13 AM, Martin Kosek wrote:
> 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.

I've amended the read permissions for containerish objects, and added a 
check to makeaci to enforce this in the future.

I insist on printing a traceback on error here. This check is meant for 
developers, tracebacks are perfect for them.

-- 
Petr³

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0572.3-permission-plugin-Sort-rights-when-writing-the-ACI.patch
Type: text/x-patch
Size: 973 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140610/1333d67f/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0573.3-Add-method-to-enumerate-managed-permission-templates.patch
Type: text/x-patch
Size: 3373 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140610/1333d67f/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0574.3-Add-ACI.txt.patch
Type: text/x-patch
Size: 31216 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140610/1333d67f/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0575.3-Make-permission-the-default-bind-type-for-managed-pe.patch
Type: text/x-patch
Size: 12492 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140610/1333d67f/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0577.3-Make-sure-member-attrs-are-always-granted-together-i.patch
Type: text/x-patch
Size: 25145 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140610/1333d67f/attachment-0004.bin>