Re: [Freeipa-devel] [PATCHES] 0289-0302 Managed Read permissions
On 11/04/2013 04:48 PM, Petr Viktorin wrote: On 10/21/2013 03:57 PM, Martin Kosek wrote: On 10/18/2013 04:28 PM, Petr Viktorin wrote: [...] Alright, I'm crafting an updated design page with the above in mind. Here are the main differences. New permissions won't (necessarily) be in $SUFFIX, so old IPA servers will not be able to modify them. Extra attribute types needed in addition to ipaPerm*Attributes would be: - ipaPermBindType (anonymous/any authenticated user/normal permission) - ipaPermDN (container DN where the ACI is stored) And objectclasses to group them: 'ipaPermissionV2' SUP ipaPermission AUXILIARY MUST ( ipaPermBindType $ ipaPermDN ) 'ipaManagedPermission' SUP ipaPermissionV2 AUXILIARY MAY ( ipaPermDefaultAttrs $ ipaPermAllowedAttrs $ ipaPermExcludedAttrs ) As for 'ipaPermissionV2', all non-SYSTEM permissions should be updated to it. Maybe a better name is needed. Another idea I had is to store all variable parts of the ACI in the permission entry. This would mean we'd not need to parse ACIs to read, search, or update them, which should make these operations faster and the code could be simplified. Doing this would require these new attribute types: - ipaPermRight (add, update, read, delete, etc.) - ipaPermObjectType - ipaPermMemberof - ipaPermFilter - ipaPermSubtree - ipaPermTargetgroup Would that make sense? The more I think about this, the more I want to go this way after all. It partially makes sense - it would speed up permission-find commands. However, it would also duplicate information and sets it in 2 places. Which smells like a bucket of potential bugs to me. True. However, this has to be weighed against the status quo. The current code is complicated. Converting ACIs to dicts and back, calling IPA commands from within IPA commands, incorrect error handling, entry_attrs vs. options in callbacks -- all these details come together to make the code very hard to change, or even verify it works as it should. I fear that a bucket of real bugs is already hiding in the code, and that incremental changes are bound to create more. What if somebody changes ipaPermObjectType, but ACI update fails or is interrupted for some reason? It would create inconsistency between permission entry and the ACI itself. Which should prevail? Obviously the DS would only take the ACI into consideration. Conceptually the permission would prevail, the ACI would be rewritten the next time the permission is updated. This is an error state, comparable to e.g. an UPG not being created for a user, or the memberOf plugin failing to update membership info. (In an ideal world the ACI updates would be done in a DS plugin that can leverage transactions.) The existing implementation has this problem with renames - if a permission is renamed by the ACI is for some reason not updated, an old ACI will stay behind, and it will be pretty hard to find. (We should have an audit tool that checks out-of-sync ACIs -- it would be helpful even if the status quo stays.) Unless permission-find performance is not a problem (yet?), I would not add these new attributes and only add ipaPermDN as this information is required. Performance is only part of the problem. Code simplicity is another -- simple code usually has less bugs, and is easier to work on/review, etc. Makes sense to me, please continue investigaing this way then. The audit tool may indeed be very useful, if admin want's to check if ACIs did not get out of sync. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0289-0302 Managed Read permissions
On 10/18/2013 04:28 PM, Petr Viktorin wrote: On 10/03/2013 12:42 PM, Martin Kosek wrote: On 10/02/2013 01:26 PM, Petr Viktorin wrote: On 10/02/2013 01:07 PM, Simo Sorce wrote: ... To sum it up, I would rather not build our permission system on this group. I think we need top base our ACIs on LDAP bind targets ldap:///all and ldap:///anyone to avoid performance issues and issues with ipausers not being complete: https://access.redhat.com/site/documentation/en-US/Red_Hat_Directory_Server/8.2/html/Administration_Guide/Managing_Access_Control-Bind_Rules.html I rather think we want to base the permissions on binddn. In the beginning, there would be 3 types of permissions based on binddn: * groupdn based: standard permissions that can be assigned privileges * ldap:///all permissions for all authenticated users. Cannot be assigned to privileges (would not make sense) * ldap:///anyone permissions for all, including anonymous users. Cannot be assigned to privileges (would not make sense) Just few examples: Read users - ldap:///anyone Read groups - ldap:///anyone Read sudo - ldap:///all Read hbac - ldap:///all ... Basing the permissions on these would allow us to get rid of all the awful deny permissions. To make the permission Bind part more user friendly, there should be an option in permission-find and a switch in Web UI to e.g. list permissions by bind type, i.e.: - anonymous permissions - authenticated users permissions - group based permissions If use would want to e.g restrict sudo only to specified group, I would see this workflow: 1) Change bind type from authenticated users to group based 2) Bind permission to chosen privilege 3) ... And the opposite, if he wants to make reading more open: 1) Unbind permission from privilege 2) Change bind type to authenticated users or anonymous Makes sense? Yes. I agree with Martin's comments too. Simo. We use privileges to group related permissions. For example an Automount Reader privilege would contain read automount keys and read automount maps permissions. Wouldn't it make more sense (from the user's point of view at least) to have this setting at the privilege level? The selfservice plugin does pretty much the same thing. Should we (aim to) move selfservice functionality to this system? selfservice is not involved in privileges, neither is delegation, they are just handling raw ACIs in a custom manner. With ldap:///all and ldap:///anyone we could theoretically add new permission types and commands, like anonymousrbac-add or authenticatedrbac-add, but I think it would be best to keep them with standard permissions for several reasons: 1) Can use the new cool features you are adding to permission entries 2) With these ACIs, it makes sense to convert them to group-based permission and assign to a privilege (does not make that much sense with selfservice ACIs). AFAIU, we all agree on that and the only part is how the anonymous/authenticated-user permissions should be grouped. I still think that grouping them in privileges or roles does not make much sense - anonymous/authenticated-user ACIs do not need to be grouped anywhere, setting binddn is enough. It would be just our custom wiring in framework without much benefit to real functionality in LDAP. If you have a privilege Automount Reader with setting Access: Anonymous: or similar, what if you add more non-anonymous permission to the privilege? What would happen then? Or what if you remove permission from the privilege, should it automagically become group-based ACI? Going with the workflow I described above seems to me as the most direct approach with keeping the anonymous/authenticated users/group based information in the single authoritative source - ACI/permission. And, since these permissions are longer be compatible with old versions, I could move them out of $SUFFIX and onto the relevant containers. That should also help performance. +1 Martin Alright, I'm crafting an updated design page with the above in mind. Here are the main differences. New permissions won't (necessarily) be in $SUFFIX, so old IPA servers will not be able to modify them. Extra attribute types needed in addition to ipaPerm*Attributes would be: - ipaPermBindType (anonymous/any authenticated user/normal permission) - ipaPermDN (container DN where the ACI is stored) And objectclasses to group them: 'ipaPermissionV2' SUP ipaPermission AUXILIARY MUST ( ipaPermBindType $ ipaPermDN ) 'ipaManagedPermission' SUP ipaPermissionV2 AUXILIARY MAY ( ipaPermDefaultAttrs $ ipaPermAllowedAttrs $ ipaPermExcludedAttrs ) As for 'ipaPermissionV2', all non-SYSTEM permissions should be updated to it. Maybe a better name is needed. Another idea I had is to store all variable parts of the ACI in the permission entry. This would mean we'd not need to parse ACIs to read, search, or update them, which
Re: [Freeipa-devel] [PATCHES] 0289-0302 Managed Read permissions
On 10/21/2013 03:57 PM, Martin Kosek wrote: On 10/18/2013 04:28 PM, Petr Viktorin wrote: On 10/03/2013 12:42 PM, Martin Kosek wrote: On 10/02/2013 01:26 PM, Petr Viktorin wrote: On 10/02/2013 01:07 PM, Simo Sorce wrote: ... To sum it up, I would rather not build our permission system on this group. I think we need top base our ACIs on LDAP bind targets ldap:///all and ldap:///anyone to avoid performance issues and issues with ipausers not being complete: https://access.redhat.com/site/documentation/en-US/Red_Hat_Directory_Server/8.2/html/Administration_Guide/Managing_Access_Control-Bind_Rules.html I rather think we want to base the permissions on binddn. In the beginning, there would be 3 types of permissions based on binddn: * groupdn based: standard permissions that can be assigned privileges * ldap:///all permissions for all authenticated users. Cannot be assigned to privileges (would not make sense) * ldap:///anyone permissions for all, including anonymous users. Cannot be assigned to privileges (would not make sense) Just few examples: Read users - ldap:///anyone Read groups - ldap:///anyone Read sudo - ldap:///all Read hbac - ldap:///all ... Basing the permissions on these would allow us to get rid of all the awful deny permissions. To make the permission Bind part more user friendly, there should be an option in permission-find and a switch in Web UI to e.g. list permissions by bind type, i.e.: - anonymous permissions - authenticated users permissions - group based permissions If use would want to e.g restrict sudo only to specified group, I would see this workflow: 1) Change bind type from authenticated users to group based 2) Bind permission to chosen privilege 3) ... And the opposite, if he wants to make reading more open: 1) Unbind permission from privilege 2) Change bind type to authenticated users or anonymous Makes sense? Yes. I agree with Martin's comments too. Simo. We use privileges to group related permissions. For example an Automount Reader privilege would contain read automount keys and read automount maps permissions. Wouldn't it make more sense (from the user's point of view at least) to have this setting at the privilege level? The selfservice plugin does pretty much the same thing. Should we (aim to) move selfservice functionality to this system? selfservice is not involved in privileges, neither is delegation, they are just handling raw ACIs in a custom manner. With ldap:///all and ldap:///anyone we could theoretically add new permission types and commands, like anonymousrbac-add or authenticatedrbac-add, but I think it would be best to keep them with standard permissions for several reasons: 1) Can use the new cool features you are adding to permission entries 2) With these ACIs, it makes sense to convert them to group-based permission and assign to a privilege (does not make that much sense with selfservice ACIs). AFAIU, we all agree on that and the only part is how the anonymous/authenticated-user permissions should be grouped. I still think that grouping them in privileges or roles does not make much sense - anonymous/authenticated-user ACIs do not need to be grouped anywhere, setting binddn is enough. It would be just our custom wiring in framework without much benefit to real functionality in LDAP. If you have a privilege Automount Reader with setting Access: Anonymous: or similar, what if you add more non-anonymous permission to the privilege? What would happen then? Or what if you remove permission from the privilege, should it automagically become group-based ACI? Going with the workflow I described above seems to me as the most direct approach with keeping the anonymous/authenticated users/group based information in the single authoritative source - ACI/permission. And, since these permissions are longer be compatible with old versions, I could move them out of $SUFFIX and onto the relevant containers. That should also help performance. +1 Martin Alright, I'm crafting an updated design page with the above in mind. Here are the main differences. New permissions won't (necessarily) be in $SUFFIX, so old IPA servers will not be able to modify them. Extra attribute types needed in addition to ipaPerm*Attributes would be: - ipaPermBindType (anonymous/any authenticated user/normal permission) - ipaPermDN (container DN where the ACI is stored) And objectclasses to group them: 'ipaPermissionV2' SUP ipaPermission AUXILIARY MUST ( ipaPermBindType $ ipaPermDN ) 'ipaManagedPermission' SUP ipaPermissionV2 AUXILIARY MAY ( ipaPermDefaultAttrs $ ipaPermAllowedAttrs $ ipaPermExcludedAttrs ) As for 'ipaPermissionV2', all non-SYSTEM permissions should be updated to it. Maybe a better name is needed. Another idea I had is to store all variable parts of the ACI in the permission entry. This would mean we'd not need to parse ACIs to read, search, or update them, which should make these operations faster and the code could be
Re: [Freeipa-devel] [PATCHES] 0289-0302 Managed Read permissions
On 10/03/2013 12:42 PM, Martin Kosek wrote: On 10/02/2013 01:26 PM, Petr Viktorin wrote: On 10/02/2013 01:07 PM, Simo Sorce wrote: ... To sum it up, I would rather not build our permission system on this group. I think we need top base our ACIs on LDAP bind targets ldap:///all and ldap:///anyone to avoid performance issues and issues with ipausers not being complete: https://access.redhat.com/site/documentation/en-US/Red_Hat_Directory_Server/8.2/html/Administration_Guide/Managing_Access_Control-Bind_Rules.html I rather think we want to base the permissions on binddn. In the beginning, there would be 3 types of permissions based on binddn: * groupdn based: standard permissions that can be assigned privileges * ldap:///all permissions for all authenticated users. Cannot be assigned to privileges (would not make sense) * ldap:///anyone permissions for all, including anonymous users. Cannot be assigned to privileges (would not make sense) Just few examples: Read users - ldap:///anyone Read groups - ldap:///anyone Read sudo - ldap:///all Read hbac - ldap:///all ... Basing the permissions on these would allow us to get rid of all the awful deny permissions. To make the permission Bind part more user friendly, there should be an option in permission-find and a switch in Web UI to e.g. list permissions by bind type, i.e.: - anonymous permissions - authenticated users permissions - group based permissions If use would want to e.g restrict sudo only to specified group, I would see this workflow: 1) Change bind type from authenticated users to group based 2) Bind permission to chosen privilege 3) ... And the opposite, if he wants to make reading more open: 1) Unbind permission from privilege 2) Change bind type to authenticated users or anonymous Makes sense? Yes. I agree with Martin's comments too. Simo. We use privileges to group related permissions. For example an Automount Reader privilege would contain read automount keys and read automount maps permissions. Wouldn't it make more sense (from the user's point of view at least) to have this setting at the privilege level? The selfservice plugin does pretty much the same thing. Should we (aim to) move selfservice functionality to this system? selfservice is not involved in privileges, neither is delegation, they are just handling raw ACIs in a custom manner. With ldap:///all and ldap:///anyone we could theoretically add new permission types and commands, like anonymousrbac-add or authenticatedrbac-add, but I think it would be best to keep them with standard permissions for several reasons: 1) Can use the new cool features you are adding to permission entries 2) With these ACIs, it makes sense to convert them to group-based permission and assign to a privilege (does not make that much sense with selfservice ACIs). AFAIU, we all agree on that and the only part is how the anonymous/authenticated-user permissions should be grouped. I still think that grouping them in privileges or roles does not make much sense - anonymous/authenticated-user ACIs do not need to be grouped anywhere, setting binddn is enough. It would be just our custom wiring in framework without much benefit to real functionality in LDAP. If you have a privilege Automount Reader with setting Access: Anonymous: or similar, what if you add more non-anonymous permission to the privilege? What would happen then? Or what if you remove permission from the privilege, should it automagically become group-based ACI? Going with the workflow I described above seems to me as the most direct approach with keeping the anonymous/authenticated users/group based information in the single authoritative source - ACI/permission. And, since these permissions are longer be compatible with old versions, I could move them out of $SUFFIX and onto the relevant containers. That should also help performance. +1 Martin Alright, I'm crafting an updated design page with the above in mind. Here are the main differences. New permissions won't (necessarily) be in $SUFFIX, so old IPA servers will not be able to modify them. Extra attribute types needed in addition to ipaPerm*Attributes would be: - ipaPermBindType (anonymous/any authenticated user/normal permission) - ipaPermDN (container DN where the ACI is stored) And objectclasses to group them: 'ipaPermissionV2' SUP ipaPermission AUXILIARY MUST ( ipaPermBindType $ ipaPermDN ) 'ipaManagedPermission' SUP ipaPermissionV2 AUXILIARY MAY ( ipaPermDefaultAttrs $ ipaPermAllowedAttrs $ ipaPermExcludedAttrs ) As for 'ipaPermissionV2', all non-SYSTEM permissions should be updated to it. Maybe a better name is needed. Another idea I had is to store all variable parts of the ACI in the permission entry. This would mean we'd not need to parse ACIs to read, search, or update them, which should make these operations faster and the code could be simplified. Doing this would require these new attribute types: - ipaPermRight (add,
Re: [Freeipa-devel] [PATCHES] 0289-0302 Managed Read permissions
On 10/02/2013 01:26 PM, Petr Viktorin wrote: On 10/02/2013 01:07 PM, Simo Sorce wrote: ... To sum it up, I would rather not build our permission system on this group. I think we need top base our ACIs on LDAP bind targets ldap:///all and ldap:///anyone to avoid performance issues and issues with ipausers not being complete: https://access.redhat.com/site/documentation/en-US/Red_Hat_Directory_Server/8.2/html/Administration_Guide/Managing_Access_Control-Bind_Rules.html I rather think we want to base the permissions on binddn. In the beginning, there would be 3 types of permissions based on binddn: * groupdn based: standard permissions that can be assigned privileges * ldap:///all permissions for all authenticated users. Cannot be assigned to privileges (would not make sense) * ldap:///anyone permissions for all, including anonymous users. Cannot be assigned to privileges (would not make sense) Just few examples: Read users - ldap:///anyone Read groups - ldap:///anyone Read sudo - ldap:///all Read hbac - ldap:///all ... Basing the permissions on these would allow us to get rid of all the awful deny permissions. To make the permission Bind part more user friendly, there should be an option in permission-find and a switch in Web UI to e.g. list permissions by bind type, i.e.: - anonymous permissions - authenticated users permissions - group based permissions If use would want to e.g restrict sudo only to specified group, I would see this workflow: 1) Change bind type from authenticated users to group based 2) Bind permission to chosen privilege 3) ... And the opposite, if he wants to make reading more open: 1) Unbind permission from privilege 2) Change bind type to authenticated users or anonymous Makes sense? Yes. I agree with Martin's comments too. Simo. We use privileges to group related permissions. For example an Automount Reader privilege would contain read automount keys and read automount maps permissions. Wouldn't it make more sense (from the user's point of view at least) to have this setting at the privilege level? The selfservice plugin does pretty much the same thing. Should we (aim to) move selfservice functionality to this system? selfservice is not involved in privileges, neither is delegation, they are just handling raw ACIs in a custom manner. With ldap:///all and ldap:///anyone we could theoretically add new permission types and commands, like anonymousrbac-add or authenticatedrbac-add, but I think it would be best to keep them with standard permissions for several reasons: 1) Can use the new cool features you are adding to permission entries 2) With these ACIs, it makes sense to convert them to group-based permission and assign to a privilege (does not make that much sense with selfservice ACIs). AFAIU, we all agree on that and the only part is how the anonymous/authenticated-user permissions should be grouped. I still think that grouping them in privileges or roles does not make much sense - anonymous/authenticated-user ACIs do not need to be grouped anywhere, setting binddn is enough. It would be just our custom wiring in framework without much benefit to real functionality in LDAP. If you have a privilege Automount Reader with setting Access: Anonymous: or similar, what if you add more non-anonymous permission to the privilege? What would happen then? Or what if you remove permission from the privilege, should it automagically become group-based ACI? Going with the workflow I described above seems to me as the most direct approach with keeping the anonymous/authenticated users/group based information in the single authoritative source - ACI/permission. And, since these permissions are longer be compatible with old versions, I could move them out of $SUFFIX and onto the relevant containers. That should also help performance. +1 Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0289-0302 Managed Read permissions
On 10/01/2013 09:50 PM, Simo Sorce wrote: - Original Message - On 10/01/2013 10:56 AM, Petr Viktorin wrote: Hello, These patches implement the framework for https://fedorahosted.org/freeipa/ticket/3566 Design is at http://www.freeipa.org/page/V3/Managed_Read_permissions. As you can see from the TODOs it's not yet complete; I'll need a few more discussions about some details and future work. Simo, I believe you're in charge of OIDs? Can you register the schema? Done. Thank you! attributeTypes: 2.16.840.1.113730.3.8.11.42 ipaPermissionDefaultAttribute 2.16.840.1.113730.3.8.11.43 ipaPermissionAllowedAttribute 2.16.840.1.113730.3.8.11.44 ipaPermissionExcludedAttribute Would it make sense to shorten these to something like ipaPermXXXAttr ? Long names are harder to read. Sure. I'll make the change in the next iteration of the patches. objectClasses: 2.16.840.1.113730.3.8.12.20 ipaManagedPermission The schema is added in patch 0293. Ok, Simo. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0289-0302 Managed Read permissions
On 10/01/2013 10:56 AM, Petr Viktorin wrote:
Hello,
These patches implement the framework for
https://fedorahosted.org/freeipa/ticket/3566
Design is at http://www.freeipa.org/page/V3/Managed_Read_permissions.
As you can see from the TODOs it's not yet complete; I'll need a few more
discussions about some details and future work.
The patches only add read permissions for User objects, and the global
anonymous read ACI is not removed. I'll add the other objects after the
overall
structure is ACKed.
To test, remove the ACI (cheatsheet: http://fpaste.org/43296/13806142/) and
verify that anonymous read is disabled and normal users can't read anything
but
user info.
These depend on some of my earlier patches:
- 0258-0265, 0275 - LDIF-based schema updater
- 0276-0277 - Split large doc strings for translation
- 0288 - user template in tests
I needed to test both server and client plugins. Since we only have one API
object (#3090) and can't unload plugins, I needed to fix some issues when they
are loaded at the same time.
Terminology note: currently IPA calls the read/search/write/delete
part
of an ACI a permission, which is confusing since our ACI wrapper objects are
also permissions.
Wherever I can, I use the term rights for these.
Rights is also used in ACI docs:
https://access.redhat.com/site/documentation/en-US/Red_Hat_Directory_Server/8.2/html/Administration_Guide/Managing_Access_Control-Creating_ACIs_Manually.html#id3349243
/Enter patches.
Act I.
0289: Might as well update to new API since I'll be making extensive changes
here
0290: My linting tools were complaining heavily about the tabs, so I fixed the
indentation here.
0291: Fix a crash when ldap2 and a client RPC backend are connected at the
same
time. (This happens in tests that I'll add later)
Act II.
0292: See the Permission flags section of the design.
0293: Add schema. (The OIDs aren't registered yet.)
0294: This makes the test in the next patch possible.
0295: See the MANAGED Permissions section of the design.
0296: See the Read rights section of the design.
Act III.
0297: See Marking Attributes in Plugins and Adding permissions for default
read permissions in the design.
0298: Make the help plugin not fail when server plugins are loaded. This will
happen in later tests.
0299: Tests for 0297
0300! Fix a TODO from 0295
0301: See Adding privileges and role for default read permissions in the
design
0302: Tests for 0301
Thanks for all the patches and carrying on with this major effort! I swiftly
went through the patches and have few comments:
1) Patch 293 requires your schema updater patches, right? That means patches
258-265, 275
There may be more dependencies though:
...
Applying: Add Reader role and user read privilege
Applying: Add tests for the new Reader role
error: patch failed: ipatests/test_xmlrpc/test_user_plugin.py:106
error: ipatests/test_xmlrpc/test_user_plugin.py: patch does not apply
Patch failed at 0014 Add tests for the new Reader role
2) 297: in update_managed_permissions, handling of
ipapermissionallowedattribute and ipapermissionexcludedattribute should be
handled by
+try:
+api.Command.permission_mod(name)
+except errors.EmptyModlist:
+self.log.debug('Attributes unchanged')
right?
3) 301: I did not like the Reader role User Readers privilege very much. I
see several issues related to that:
a) performance reasons - read operation on LDAP must be as fast as possible. DS
checking membership in ipausers where may be 10 of users does not look right
b) ipausers group may be changed to other group by setting
ipaDefaultPrimaryGroup. I also think we are guaranteed that it really contains
all users. There have been also thoughts about removing it.
To sum it up, I would rather not build our permission system on this group.
I think we need top base our ACIs on LDAP bind targets ldap:///all and
ldap:///anyone to avoid performance issues and issues with ipausers not being
complete:
https://access.redhat.com/site/documentation/en-US/Red_Hat_Directory_Server/8.2/html/Administration_Guide/Managing_Access_Control-Bind_Rules.html
I rather think we want to base the permissions on binddn. In the beginning,
there would be 3 types of permissions based on binddn:
* groupdn based: standard permissions that can be assigned privileges
* ldap:///all permissions for all authenticated users. Cannot be assigned to
privileges (would not make sense)
* ldap:///anyone permissions for all, including anonymous users. Cannot be
assigned to privileges (would not make sense)
Just few examples:
Read users - ldap:///anyone
Read groups - ldap:///anyone
Read sudo - ldap:///all
Read hbac - ldap:///all
...
Basing the permissions on these would allow us to get rid of all the awful deny
permissions.
To make the permission Bind part more user friendly, there should be an option
in
Re: [Freeipa-devel] [PATCHES] 0289-0302 Managed Read permissions
- Original Message -
On 10/01/2013 10:56 AM, Petr Viktorin wrote:
Hello,
These patches implement the framework for
https://fedorahosted.org/freeipa/ticket/3566
Design is at http://www.freeipa.org/page/V3/Managed_Read_permissions.
As you can see from the TODOs it's not yet complete; I'll need a few more
discussions about some details and future work.
The patches only add read permissions for User objects, and the global
anonymous read ACI is not removed. I'll add the other objects after the
overall
structure is ACKed.
To test, remove the ACI (cheatsheet: http://fpaste.org/43296/13806142/) and
verify that anonymous read is disabled and normal users can't read anything
but
user info.
These depend on some of my earlier patches:
- 0258-0265, 0275 - LDIF-based schema updater
- 0276-0277 - Split large doc strings for translation
- 0288 - user template in tests
I needed to test both server and client plugins. Since we only have one API
object (#3090) and can't unload plugins, I needed to fix some issues when
they
are loaded at the same time.
Terminology note: currently IPA calls the read/search/write/delete
part
of an ACI a permission, which is confusing since our ACI wrapper objects
are
also permissions.
Wherever I can, I use the term rights for these.
Rights is also used in ACI docs:
https://access.redhat.com/site/documentation/en-US/Red_Hat_Directory_Server/8.2/html/Administration_Guide/Managing_Access_Control-Creating_ACIs_Manually.html#id3349243
/Enter patches.
Act I.
0289: Might as well update to new API since I'll be making extensive
changes here
0290: My linting tools were complaining heavily about the tabs, so I fixed
the
indentation here.
0291: Fix a crash when ldap2 and a client RPC backend are connected at the
same
time. (This happens in tests that I'll add later)
Act II.
0292: See the Permission flags section of the design.
0293: Add schema. (The OIDs aren't registered yet.)
0294: This makes the test in the next patch possible.
0295: See the MANAGED Permissions section of the design.
0296: See the Read rights section of the design.
Act III.
0297: See Marking Attributes in Plugins and Adding permissions for
default
read permissions in the design.
0298: Make the help plugin not fail when server plugins are loaded. This
will
happen in later tests.
0299: Tests for 0297
0300! Fix a TODO from 0295
0301: See Adding privileges and role for default read permissions in the
design
0302: Tests for 0301
Thanks for all the patches and carrying on with this major effort! I swiftly
went through the patches and have few comments:
1) Patch 293 requires your schema updater patches, right? That means patches
258-265, 275
There may be more dependencies though:
...
Applying: Add Reader role and user read privilege
Applying: Add tests for the new Reader role
error: patch failed: ipatests/test_xmlrpc/test_user_plugin.py:106
error: ipatests/test_xmlrpc/test_user_plugin.py: patch does not apply
Patch failed at 0014 Add tests for the new Reader role
2) 297: in update_managed_permissions, handling of
ipapermissionallowedattribute and ipapermissionexcludedattribute should be
handled by
+try:
+api.Command.permission_mod(name)
+except errors.EmptyModlist:
+self.log.debug('Attributes unchanged')
right?
3) 301: I did not like the Reader role User Readers privilege very much. I
see several issues related to that:
a) performance reasons - read operation on LDAP must be as fast as possible.
DS
checking membership in ipausers where may be 10 of users does not look
right
b) ipausers group may be changed to other group by setting
ipaDefaultPrimaryGroup. I also think we are guaranteed that it really
contains
all users. There have been also thoughts about removing it.
To sum it up, I would rather not build our permission system on this group.
I think we need top base our ACIs on LDAP bind targets ldap:///all and
ldap:///anyone to avoid performance issues and issues with ipausers not being
complete:
https://access.redhat.com/site/documentation/en-US/Red_Hat_Directory_Server/8.2/html/Administration_Guide/Managing_Access_Control-Bind_Rules.html
I rather think we want to base the permissions on binddn. In the beginning,
there would be 3 types of permissions based on binddn:
* groupdn based: standard permissions that can be assigned privileges
* ldap:///all permissions for all authenticated users. Cannot be assigned to
privileges (would not make sense)
* ldap:///anyone permissions for all, including anonymous users. Cannot be
assigned to privileges (would not make sense)
Just few examples:
Read users - ldap:///anyone
Read groups - ldap:///anyone
Read sudo - ldap:///all
Read hbac - ldap:///all
...
Re: [Freeipa-devel] [PATCHES] 0289-0302 Managed Read permissions
On 10/02/2013 01:07 PM, Simo Sorce wrote:
- Original Message -
On 10/01/2013 10:56 AM, Petr Viktorin wrote:
Hello,
These patches implement the framework for
https://fedorahosted.org/freeipa/ticket/3566
Design is at http://www.freeipa.org/page/V3/Managed_Read_permissions.
As you can see from the TODOs it's not yet complete; I'll need a few more
discussions about some details and future work.
The patches only add read permissions for User objects, and the global
anonymous read ACI is not removed. I'll add the other objects after the
overall
structure is ACKed.
To test, remove the ACI (cheatsheet: http://fpaste.org/43296/13806142/) and
verify that anonymous read is disabled and normal users can't read anything
but
user info.
These depend on some of my earlier patches:
- 0258-0265, 0275 - LDIF-based schema updater
- 0276-0277 - Split large doc strings for translation
- 0288 - user template in tests
These are the dependencies ^
I needed to test both server and client plugins. Since we only have one API
object (#3090) and can't unload plugins, I needed to fix some issues when
they
are loaded at the same time.
Terminology note: currently IPA calls the read/search/write/delete
part
of an ACI a permission, which is confusing since our ACI wrapper objects
are
also permissions.
Wherever I can, I use the term rights for these.
Rights is also used in ACI docs:
https://access.redhat.com/site/documentation/en-US/Red_Hat_Directory_Server/8.2/html/Administration_Guide/Managing_Access_Control-Creating_ACIs_Manually.html#id3349243
/Enter patches.
Act I.
0289: Might as well update to new API since I'll be making extensive
changes here
0290: My linting tools were complaining heavily about the tabs, so I fixed
the
indentation here.
0291: Fix a crash when ldap2 and a client RPC backend are connected at the
same
time. (This happens in tests that I'll add later)
Act II.
0292: See the Permission flags section of the design.
0293: Add schema. (The OIDs aren't registered yet.)
0294: This makes the test in the next patch possible.
0295: See the MANAGED Permissions section of the design.
0296: See the Read rights section of the design.
Act III.
0297: See Marking Attributes in Plugins and Adding permissions for
default
read permissions in the design.
0298: Make the help plugin not fail when server plugins are loaded. This
will
happen in later tests.
0299: Tests for 0297
0300! Fix a TODO from 0295
0301: See Adding privileges and role for default read permissions in the
design
0302: Tests for 0301
Thanks for all the patches and carrying on with this major effort! I swiftly
went through the patches and have few comments:
1) Patch 293 requires your schema updater patches, right? That means patches
258-265, 275
There may be more dependencies though:
...
Applying: Add Reader role and user read privilege
Applying: Add tests for the new Reader role
error: patch failed: ipatests/test_xmlrpc/test_user_plugin.py:106
error: ipatests/test_xmlrpc/test_user_plugin.py: patch does not apply
Patch failed at 0014 Add tests for the new Reader role
See the list of deps earlier in the mail :)
2) 297: in update_managed_permissions, handling of
ipapermissionallowedattribute and ipapermissionexcludedattribute should be
handled by
+try:
+api.Command.permission_mod(name)
+except errors.EmptyModlist:
+self.log.debug('Attributes unchanged')
right?
Yes. ipapermdefaultattrs is added in ldap.update_entry(entry), and then
from that and allowed+excluded, attributes is recomputed in permission_mod.
This way the algorithm is kept in one place.
3) 301: I did not like the Reader role User Readers privilege very much. I
see several issues related to that:
a) performance reasons - read operation on LDAP must be as fast as possible.
DS
checking membership in ipausers where may be 10 of users does not look
right
b) ipausers group may be changed to other group by setting
ipaDefaultPrimaryGroup. I also think we are guaranteed that it really
contains
all users. There have been also thoughts about removing it.
Having ipausers in Readers is just a default, the admin can change it to
match their setting. But your point is still valid.
To sum it up, I would rather not build our permission system on this group.
I think we need top base our ACIs on LDAP bind targets ldap:///all and
ldap:///anyone to avoid performance issues and issues with ipausers not being
complete:
https://access.redhat.com/site/documentation/en-US/Red_Hat_Directory_Server/8.2/html/Administration_Guide/Managing_Access_Control-Bind_Rules.html
I rather think we want to base the permissions on binddn. In the beginning,
there would be 3 types of permissions based on binddn:
* groupdn based: standard permissions that can be assigned privileges
* ldap:///all permissions for all authenticated users. Cannot be assigned to
privileges (would not make sense)
* ldap:///anyone permissions for all,
Re: [Freeipa-devel] [PATCHES] 0289-0302 Managed Read permissions
On 10/01/2013 10:56 AM, Petr Viktorin wrote: Hello, These patches implement the framework for https://fedorahosted.org/freeipa/ticket/3566 Design is at http://www.freeipa.org/page/V3/Managed_Read_permissions. As you can see from the TODOs it's not yet complete; I'll need a few more discussions about some details and future work. Simo, I believe you're in charge of OIDs? Can you register the schema? attributeTypes: 2.16.840.1.113730.3.8.11.42 ipaPermissionDefaultAttribute 2.16.840.1.113730.3.8.11.43 ipaPermissionAllowedAttribute 2.16.840.1.113730.3.8.11.44 ipaPermissionExcludedAttribute objectClasses: 2.16.840.1.113730.3.8.12.20 ipaManagedPermission The schema is added in patch 0293. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0289-0302 Managed Read permissions
- Original Message - On 10/01/2013 10:56 AM, Petr Viktorin wrote: Hello, These patches implement the framework for https://fedorahosted.org/freeipa/ticket/3566 Design is at http://www.freeipa.org/page/V3/Managed_Read_permissions. As you can see from the TODOs it's not yet complete; I'll need a few more discussions about some details and future work. Simo, I believe you're in charge of OIDs? Can you register the schema? Done. attributeTypes: 2.16.840.1.113730.3.8.11.42 ipaPermissionDefaultAttribute 2.16.840.1.113730.3.8.11.43 ipaPermissionAllowedAttribute 2.16.840.1.113730.3.8.11.44 ipaPermissionExcludedAttribute Would it make sense to shorten these to something like ipaPermXXXAttr ? Long names are harder to read. objectClasses: 2.16.840.1.113730.3.8.12.20 ipaManagedPermission The schema is added in patch 0293. Ok, Simo. -- Simo Sorce * Red Hat, Inc. * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
