Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 17.10.2014 13:52, Jan Cholasta wrote: Thanks, ACK. rebased due to version change and pushed to: master: * 59ee6314afc7f0f7735ab1349caa970f0f00d78a keytab manipulation permission management * b69a8dad2ebd98516d36b1470fa27c0819b8a985 tests: management of keytab permissions ipa-4-1: * 9cfcb03c704309b972e4737e0a7a7a245dfa7923 keytab manipulation permission management * 7313ed4f9eb55aa58daf42b2605fd5c238b83ac4 tests: management of keytab permissions -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
Dne 17.10.2014 v 13:48 Petr Vobornik napsal(a): On 17.10.2014 11:06, Jan Cholasta wrote: Dne 16.10.2014 v 20:28 Martin Kosek napsal(a): On 10/16/2014 07:03 PM, Petr Vobornik wrote: On 16.10.2014 11:53, Jan Cholasta wrote: Dne 16.10.2014 v 11:24 Petr Vobornik napsal(a): On 16.10.2014 09:54, Jan Cholasta wrote: Dne 13.10.2014 v 12:42 Petr Vobornik napsal(a): On 8.10.2014 18:51, Petr Vobornik wrote: On 1.10.2014 18:15, Petr Vobornik wrote: Hello list, Patch for: https://fedorahosted.org/freeipa/ticket/4419 New revisions of 761 and 763 with updated API and ACIs: Given: Given the implementation, I see you can't remove it from snip OK, you are obviously not responsible for this mess, so let's go with it. snip ugly hacks though.)> snip I'm not particularly happy about the '_subtype' option bussiness, but at least it's not invasive, so I guess it's OK. Note that I still think this API sucks and we should instead go with the generic member-like attribute approach, or take our time to design it properly so that it fits in the framework (no time in 4.1) instead of making it a hacky Franken-API like it is now. and a discussion with Honza I've attached alternative versions of this patch - based on 761-1 with API as follows: ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-create-keytab HOSTNAME --users=STR --groups STR ipa service-allow-retrieve-keytab PRINCIPAL --users=STR --groups STR ipa service-disallow-retrieve-keytab PRINCIPAL --users=STR --groups STR ipa service-allow-create-keytab PRINCIPAL --users=STR --groups STR ipa service-disallow-create-keytab PRINCIPAL --users=STR --groups STR and updated ACIs Both approaches have their own drawbacks. Given the discussion we had, I think I can live with this version too, especially if it makes the API or the code less hackier than with the API version I proposed. So if Honza ACKs the code, I am fine with this API version. Patch 761: ACK on the approach. The commands do not show failed members in CLI, to fix this, add: Str('ipaallowedtoperform_read_keys', label=_('Failed allowed to retrieve keytab'), ), Str('ipaallowedtoperform_write_keys', label=_('Failed allowed to create keytab'), ), to the global output param lists in service and host plugins. (Feel free to fix the label to your liking.) Added Thanks, ACK. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 17.10.2014 11:06, Jan Cholasta wrote: Dne 16.10.2014 v 20:28 Martin Kosek napsal(a): On 10/16/2014 07:03 PM, Petr Vobornik wrote: On 16.10.2014 11:53, Jan Cholasta wrote: Dne 16.10.2014 v 11:24 Petr Vobornik napsal(a): On 16.10.2014 09:54, Jan Cholasta wrote: Dne 13.10.2014 v 12:42 Petr Vobornik napsal(a): On 8.10.2014 18:51, Petr Vobornik wrote: On 1.10.2014 18:15, Petr Vobornik wrote: Hello list, Patch for: https://fedorahosted.org/freeipa/ticket/4419 New revisions of 761 and 763 with updated API and ACIs: Given: Given the implementation, I see you can't remove it from snip OK, you are obviously not responsible for this mess, so let's go with it. snip ugly hacks though.)> snip I'm not particularly happy about the '_subtype' option bussiness, but at least it's not invasive, so I guess it's OK. Note that I still think this API sucks and we should instead go with the generic member-like attribute approach, or take our time to design it properly so that it fits in the framework (no time in 4.1) instead of making it a hacky Franken-API like it is now. and a discussion with Honza I've attached alternative versions of this patch - based on 761-1 with API as follows: ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-create-keytab HOSTNAME --users=STR --groups STR ipa service-allow-retrieve-keytab PRINCIPAL --users=STR --groups STR ipa service-disallow-retrieve-keytab PRINCIPAL --users=STR --groups STR ipa service-allow-create-keytab PRINCIPAL --users=STR --groups STR ipa service-disallow-create-keytab PRINCIPAL --users=STR --groups STR and updated ACIs Both approaches have their own drawbacks. Given the discussion we had, I think I can live with this version too, especially if it makes the API or the code less hackier than with the API version I proposed. So if Honza ACKs the code, I am fine with this API version. Patch 761: ACK on the approach. The commands do not show failed members in CLI, to fix this, add: Str('ipaallowedtoperform_read_keys', label=_('Failed allowed to retrieve keytab'), ), Str('ipaallowedtoperform_write_keys', label=_('Failed allowed to create keytab'), ), to the global output param lists in service and host plugins. (Feel free to fix the label to your liking.) Added Patch 763: ACK. -- Petr Vobornik From f665e51f1822db41998987de516b577a530df9dd Mon Sep 17 00:00:00 2001 From: Petr Vobornik Date: Thu, 2 Oct 2014 16:57:08 +0200 Subject: [PATCH] keytab manipulation permission management Adds new API: ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-create-keytab HOSTNAME --users=STR --groups STR ipa service-allow-retrieve-keytab PRINCIPAL --users=STR --groups STR ipa service-disallow-retrieve-keytab PRINCIPAL --users=STR --groups STR ipa service-allow-create-keytab PRINCIPAL --users=STR --groups STR ipa service-disallow-create-keytab PRINCIPAL --users=STR --groups STR these methods add or remove user or group DNs in `ipaallowedtoperform` attr with `read_keys` and `write_keys` subtypes. service|host-mod|show outputs these attrs only with --all option as: Users allowed to retrieve keytab: user1 Groups allowed to retrieve keytab: group1 Users allowed to create keytab: user1 Groups allowed to create keytab: group1 Adding of object class is implemented as a reusable method since this code is used on many places and most likely will be also used in new features. Older code may be refactored later. https://fedorahosted.org/freeipa/ticket/4419 --- ACI.txt| 4 ++ API.txt| 96 VERSION| 4 +- ipalib/plugins/baseldap.py | 17 ++ ipalib/plugins/host.py | 116 +-- ipalib/plugins/service.py | 134 +++-- 6 files changed, 360 insertions(+), 11 deletions(-) diff --git a/ACI.txt b/ACI.txt index bc031ff1f24b9e9f08fe9ba78e5a262162f32cef..6fc88233e340d6eabe4ad819176e3cfa98d8c9ce 100644 --- a/ACI.txt +++ b/ACI.txt @@ -95,6 +95,8 @@ aci: (targetattr = "userpassword")(targetfilter = "(objectclass=ipahost)")(versi dn: cn=computers,cn=accounts,dc=ipa,dc=example aci: (targetattr = "krblastpwdchange || krbprincipalkey")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Manage Host Keytab";allow (write) groupdn = "ldap:///cn=System: Manage Host Keytab,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=computers,cn=accounts,dc=ipa,dc=example +aci: (targetattr = "createtimestamp || entryusn || ipaallowedtoperform;read_keys || ipaallowedtoperform;
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
Dne 16.10.2014 v 20:28 Martin Kosek napsal(a): On 10/16/2014 07:03 PM, Petr Vobornik wrote: On 16.10.2014 11:53, Jan Cholasta wrote: Dne 16.10.2014 v 11:24 Petr Vobornik napsal(a): On 16.10.2014 09:54, Jan Cholasta wrote: Dne 13.10.2014 v 12:42 Petr Vobornik napsal(a): On 8.10.2014 18:51, Petr Vobornik wrote: On 1.10.2014 18:15, Petr Vobornik wrote: Hello list, Patch for: https://fedorahosted.org/freeipa/ticket/4419 New revisions of 761 and 763 with updated API and ACIs: Given: Given the implementation, I see you can't remove it from snip OK, you are obviously not responsible for this mess, so let's go with it. snip ugly hacks though.)> snip I'm not particularly happy about the '_subtype' option bussiness, but at least it's not invasive, so I guess it's OK. Note that I still think this API sucks and we should instead go with the generic member-like attribute approach, or take our time to design it properly so that it fits in the framework (no time in 4.1) instead of making it a hacky Franken-API like it is now. and a discussion with Honza I've attached alternative versions of this patch - based on 761-1 with API as follows: ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-create-keytab HOSTNAME --users=STR --groups STR ipa service-allow-retrieve-keytab PRINCIPAL --users=STR --groups STR ipa service-disallow-retrieve-keytab PRINCIPAL --users=STR --groups STR ipa service-allow-create-keytab PRINCIPAL --users=STR --groups STR ipa service-disallow-create-keytab PRINCIPAL --users=STR --groups STR and updated ACIs Both approaches have their own drawbacks. Given the discussion we had, I think I can live with this version too, especially if it makes the API or the code less hackier than with the API version I proposed. So if Honza ACKs the code, I am fine with this API version. Patch 761: ACK on the approach. The commands do not show failed members in CLI, to fix this, add: Str('ipaallowedtoperform_read_keys', label=_('Failed allowed to retrieve keytab'), ), Str('ipaallowedtoperform_write_keys', label=_('Failed allowed to create keytab'), ), to the global output param lists in service and host plugins. (Feel free to fix the label to your liking.) Patch 763: ACK. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 10/16/2014 07:03 PM, Petr Vobornik wrote: On 16.10.2014 11:53, Jan Cholasta wrote: Dne 16.10.2014 v 11:24 Petr Vobornik napsal(a): On 16.10.2014 09:54, Jan Cholasta wrote: Dne 13.10.2014 v 12:42 Petr Vobornik napsal(a): On 8.10.2014 18:51, Petr Vobornik wrote: On 1.10.2014 18:15, Petr Vobornik wrote: Hello list, Patch for: https://fedorahosted.org/freeipa/ticket/4419 New revisions of 761 and 763 with updated API and ACIs: Given: Given the implementation, I see you can't remove it from snip OK, you are obviously not responsible for this mess, so let's go with it. snip ugly hacks though.)> snip I'm not particularly happy about the '_subtype' option bussiness, but at least it's not invasive, so I guess it's OK. Note that I still think this API sucks and we should instead go with the generic member-like attribute approach, or take our time to design it properly so that it fits in the framework (no time in 4.1) instead of making it a hacky Franken-API like it is now. and a discussion with Honza I've attached alternative versions of this patch - based on 761-1 with API as follows: ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-create-keytab HOSTNAME --users=STR --groups STR ipa service-allow-retrieve-keytab PRINCIPAL --users=STR --groups STR ipa service-disallow-retrieve-keytab PRINCIPAL --users=STR --groups STR ipa service-allow-create-keytab PRINCIPAL --users=STR --groups STR ipa service-disallow-create-keytab PRINCIPAL --users=STR --groups STR and updated ACIs Both approaches have their own drawbacks. Given the discussion we had, I think I can live with this version too, especially if it makes the API or the code less hackier than with the API version I proposed. So if Honza ACKs the code, I am fine with this API version. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 16.10.2014 11:53, Jan Cholasta wrote: Dne 16.10.2014 v 11:24 Petr Vobornik napsal(a): On 16.10.2014 09:54, Jan Cholasta wrote: Dne 13.10.2014 v 12:42 Petr Vobornik napsal(a): On 8.10.2014 18:51, Petr Vobornik wrote: On 1.10.2014 18:15, Petr Vobornik wrote: Hello list, Patch for: https://fedorahosted.org/freeipa/ticket/4419 New revisions of 761 and 763 with updated API and ACIs: Given: Given the implementation, I see you can't remove it from snip OK, you are obviously not responsible for this mess, so let's go with it. snip ugly hacks though.)> snip I'm not particularly happy about the '_subtype' option bussiness, but at least it's not invasive, so I guess it's OK. Note that I still think this API sucks and we should instead go with the generic member-like attribute approach, or take our time to design it properly so that it fits in the framework (no time in 4.1) instead of making it a hacky Franken-API like it is now. and a discussion with Honza I've attached alternative versions of this patch - based on 761-1 with API as follows: ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-create-keytab HOSTNAME --users=STR --groups STR ipa service-allow-retrieve-keytab PRINCIPAL --users=STR --groups STR ipa service-disallow-retrieve-keytab PRINCIPAL --users=STR --groups STR ipa service-allow-create-keytab PRINCIPAL --users=STR --groups STR ipa service-disallow-create-keytab PRINCIPAL --users=STR --groups STR and updated ACIs Both approaches have their own drawbacks. -- Petr Vobornik From af96457db14bc8d9232e7cce3fdb9aab852ba107 Mon Sep 17 00:00:00 2001 From: Petr Vobornik Date: Thu, 2 Oct 2014 16:57:08 +0200 Subject: [PATCH] keytab manipulation permission management Adds new API: ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-create-keytab HOSTNAME --users=STR --groups STR ipa service-allow-retrieve-keytab PRINCIPAL --users=STR --groups STR ipa service-disallow-retrieve-keytab PRINCIPAL --users=STR --groups STR ipa service-allow-create-keytab PRINCIPAL --users=STR --groups STR ipa service-disallow-create-keytab PRINCIPAL --users=STR --groups STR these methods add or remove user or group DNs in `ipaallowedtoperform` attr with `read_keys` and `write_keys` subtypes. service|host-mod|show outputs these attrs only with --all option as: Users allowed to retrieve keytab: user1 Groups allowed to retrieve keytab: group1 Users allowed to create keytab: user1 Groups allowed to create keytab: group1 Adding of object class is implemented as a reusable method since this code is used on many places and most likely will be also used in new features. Older code may be refactored later. https://fedorahosted.org/freeipa/ticket/4419 --- ACI.txt| 4 ++ API.txt| 96 ++ VERSION| 4 +- ipalib/plugins/baseldap.py | 17 ++ ipalib/plugins/host.py | 110 -- ipalib/plugins/service.py | 128 +++-- 6 files changed, 348 insertions(+), 11 deletions(-) diff --git a/ACI.txt b/ACI.txt index bc031ff1f24b9e9f08fe9ba78e5a262162f32cef..6fc88233e340d6eabe4ad819176e3cfa98d8c9ce 100644 --- a/ACI.txt +++ b/ACI.txt @@ -95,6 +95,8 @@ aci: (targetattr = "userpassword")(targetfilter = "(objectclass=ipahost)")(versi dn: cn=computers,cn=accounts,dc=ipa,dc=example aci: (targetattr = "krblastpwdchange || krbprincipalkey")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Manage Host Keytab";allow (write) groupdn = "ldap:///cn=System: Manage Host Keytab,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=computers,cn=accounts,dc=ipa,dc=example +aci: (targetattr = "createtimestamp || entryusn || ipaallowedtoperform;read_keys || ipaallowedtoperform;write_keys || modifytimestamp || objectclass")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Manage Host Keytab Permissions";allow (compare,read,search,write) groupdn = "ldap:///cn=System: Manage Host Keytab Permissions,cn=permissions,cn=pbac,dc=ipa,dc=example";) +dn: cn=computers,cn=accounts,dc=ipa,dc=example aci: (targetattr = "ipasshpubkey")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Manage Host SSH Public Keys";allow (write) groupdn = "ldap:///cn=System: Manage Host SSH Public Keys,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=computers,cn=accounts,dc=ipa,dc=example aci: (targetattr = "description || ipaassignedidview || l || macaddress || nshardwareplatform || nshostlocation || nsosversion || userclass")(targetfilter = "(objectclass=ip
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 16.10.2014 11:53, Jan Cholasta wrote: Dne 16.10.2014 v 11:24 Petr Vobornik napsal(a): On 16.10.2014 09:54, Jan Cholasta wrote: Dne 13.10.2014 v 12:42 Petr Vobornik napsal(a): On 8.10.2014 18:51, Petr Vobornik wrote: On 1.10.2014 18:15, Petr Vobornik wrote: Hello list, Patch for: https://fedorahosted.org/freeipa/ticket/4419 New revisions of 761 and 763 with updated API and ACIs: ipa host-allow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-allow-operation HOSTNAME create-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME create-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL create-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL create-keytab --users=STR --groups STR ACIs are targeted to specific operations by including subtypes. patch #761 rebased because of VERSION bump Since we are apparently not going to treat ipaAllowedToPerform as a member attribute, you should remove reference to it from attribute_members and relationships attributes of service and host. I still think of it a as member attribute, at least internally. Given the implementation, I see you can't remove it from attribute_members, but it should be removed from relationships nonetheless. affected parts of relationships removed What's up with ipaallowedtoperform_subtypes_map? Why rename the operations? It's not renaming. It's a change of internal raw LDAP value into self-describing name consistent with terminology of ipa-getkeytab OK, you are obviously not responsible for this mess, so let's go with it. You probably don't want to hardcode 'ipaallowedtoperform_read_keys' in rename_ipaallowedtoperform_to_ldap(). Fixed, but it doesn't make much difference given that the method has only one purpose. Sorry, I missed the comment at the beginning of service_allow_operation, it indeed does not make any difference. (I can't say I'm a fan of such ugly hacks though.) Why do you override get_args() in service_{,dis}allow_operation instead of overriding takes_args? Fixed I'm not particularly happy about the '_subtype' option bussiness, but at least it's not invasive, so I guess it's OK. Note that I still think this API sucks and we should instead go with the generic member-like attribute approach, or take our time to design it properly so that it fits in the framework (no time in 4.1) instead of making it a hacky Franken-API like it is now. -- Petr Vobornik From 7c3aed88ffd9e03158eb1c67d5b56c969c0ebee5 Mon Sep 17 00:00:00 2001 From: Petr Vobornik Date: Thu, 2 Oct 2014 16:57:08 +0200 Subject: [PATCH] keytab manipulation permission management Adds new API: ipa host-allow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-allow-operation HOSTNAME create-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME create-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL create-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL create-keytab --users=STR --groups STR these methods add or remove user or group DNs in `ipaallowedtoperform` attr with `read_keys` and `write_keys` subtypes. service|host-mod|show outputs these attrs as: Users allowed to retrieve keytab: user1 Groups allowed to retrieve keytab: group1 Users allowed to create keytab: user1 Groups allowed to create keytab: group1 Adding of object class is implemented as a reusable method since this code is used on many places and most likely will be also used in new features. Older code may be refactored later. https://fedorahosted.org/freeipa/ticket/4419 --- ACI.txt| 4 ++ API.txt| 52 +++ VERSION| 4 +- ipalib/plugins/baseldap.py | 17 ++ ipalib/plugins/host.py | 49 -- ipalib/plugins/service.py | 125 +++-- 6 files changed, 240 insertions(+), 11 deletions(-) diff --git a/ACI.txt b/ACI.txt index bc031ff1f24b9e9f08fe9ba78e5a262162f32cef..6fc88233e340d6eabe4ad819176e3cfa98d8c9ce 100644 --- a/ACI.txt +++ b/ACI.txt @@ -95,6 +95,8 @@ aci: (targetattr = "userpassword")(targetfilter = "(objectclass=ipahost)")(versi dn: cn=computers,cn=accounts,dc=ipa,dc=example aci: (targetattr = "krblastpwdchange || krbprincipalkey")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Manage Host Keytab";allow (wri
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
Dne 16.10.2014 v 11:24 Petr Vobornik napsal(a): On 16.10.2014 09:54, Jan Cholasta wrote: Dne 13.10.2014 v 12:42 Petr Vobornik napsal(a): On 8.10.2014 18:51, Petr Vobornik wrote: On 1.10.2014 18:15, Petr Vobornik wrote: Hello list, Patch for: https://fedorahosted.org/freeipa/ticket/4419 New revisions of 761 and 763 with updated API and ACIs: ipa host-allow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-allow-operation HOSTNAME create-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME create-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL create-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL create-keytab --users=STR --groups STR ACIs are targeted to specific operations by including subtypes. patch #761 rebased because of VERSION bump Since we are apparently not going to treat ipaAllowedToPerform as a member attribute, you should remove reference to it from attribute_members and relationships attributes of service and host. I still think of it a as member attribute, at least internally. Given the implementation, I see you can't remove it from attribute_members, but it should be removed from relationships nonetheless. What's up with ipaallowedtoperform_subtypes_map? Why rename the operations? It's not renaming. It's a change of internal raw LDAP value into self-describing name consistent with terminology of ipa-getkeytab OK, you are obviously not responsible for this mess, so let's go with it. You probably don't want to hardcode 'ipaallowedtoperform_read_keys' in rename_ipaallowedtoperform_to_ldap(). Fixed, but it doesn't make much difference given that the method has only one purpose. Sorry, I missed the comment at the beginning of service_allow_operation, it indeed does not make any difference. (I can't say I'm a fan of such ugly hacks though.) Why do you override get_args() in service_{,dis}allow_operation instead of overriding takes_args? Fixed I'm not particularly happy about the '_subtype' option bussiness, but at least it's not invasive, so I guess it's OK. Note that I still think this API sucks and we should instead go with the generic member-like attribute approach, or take our time to design it properly so that it fits in the framework (no time in 4.1) instead of making it a hacky Franken-API like it is now. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 16.10.2014 09:54, Jan Cholasta wrote: Dne 13.10.2014 v 12:42 Petr Vobornik napsal(a): On 8.10.2014 18:51, Petr Vobornik wrote: On 1.10.2014 18:15, Petr Vobornik wrote: Hello list, Patch for: https://fedorahosted.org/freeipa/ticket/4419 New revisions of 761 and 763 with updated API and ACIs: ipa host-allow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-allow-operation HOSTNAME create-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME create-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL create-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL create-keytab --users=STR --groups STR ACIs are targeted to specific operations by including subtypes. patch #761 rebased because of VERSION bump Since we are apparently not going to treat ipaAllowedToPerform as a member attribute, you should remove reference to it from attribute_members and relationships attributes of service and host. I still think of it a as member attribute, at least internally. What's up with ipaallowedtoperform_subtypes_map? Why rename the operations? It's not renaming. It's a change of internal raw LDAP value into self-describing name consistent with terminology of ipa-getkeytab You probably don't want to hardcode 'ipaallowedtoperform_read_keys' in rename_ipaallowedtoperform_to_ldap(). Fixed, but it doesn't make much difference given that the method has only one purpose. Why do you override get_args() in service_{,dis}allow_operation instead of overriding takes_args? Fixed I'm not particularly happy about the '_subtype' option bussiness, but at least it's not invasive, so I guess it's OK. Note that I still think this API sucks and we should instead go with the generic member-like attribute approach, or take our time to design it properly so that it fits in the framework (no time in 4.1) instead of making it a hacky Franken-API like it is now. -- Petr Vobornik From 1025b588794921f1c00ff3a64068e31f95be35c0 Mon Sep 17 00:00:00 2001 From: Petr Vobornik Date: Thu, 2 Oct 2014 16:57:08 +0200 Subject: [PATCH] keytab manipulation permission management Adds new API: ipa host-allow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-allow-operation HOSTNAME create-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME create-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL create-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL create-keytab --users=STR --groups STR these methods add or remove user or group DNs in `ipaallowedtoperform` attr with `read_keys` and `write_keys` subtypes. service|host-mod|show outputs these attrs as: Users allowed to retrieve keytab: user1 Groups allowed to retrieve keytab: group1 Users allowed to create keytab: user1 Groups allowed to create keytab: group1 Adding of object class is implemented as a reusable method since this code is used on many places and most likely will be also used in new features. Older code may be refactored later. https://fedorahosted.org/freeipa/ticket/4419 --- ACI.txt| 4 ++ API.txt| 52 +++ VERSION| 4 +- ipalib/plugins/baseldap.py | 17 ++ ipalib/plugins/host.py | 51 -- ipalib/plugins/service.py | 127 +++-- 6 files changed, 244 insertions(+), 11 deletions(-) diff --git a/ACI.txt b/ACI.txt index bc031ff1f24b9e9f08fe9ba78e5a262162f32cef..6fc88233e340d6eabe4ad819176e3cfa98d8c9ce 100644 --- a/ACI.txt +++ b/ACI.txt @@ -95,6 +95,8 @@ aci: (targetattr = "userpassword")(targetfilter = "(objectclass=ipahost)")(versi dn: cn=computers,cn=accounts,dc=ipa,dc=example aci: (targetattr = "krblastpwdchange || krbprincipalkey")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Manage Host Keytab";allow (write) groupdn = "ldap:///cn=System: Manage Host Keytab,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=computers,cn=accounts,dc=ipa,dc=example +aci: (targetattr = "createtimestamp || entryusn || ipaallowedtoperform;read_keys || ipaallowedtoperform;write_keys || modifytimestamp || objectclass")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Manage Host Keytab Permissions";allow (compare,read,search,write) groupdn = "ldap:///cn=System: Manage Host Keytab Permissions,cn=permissions,cn=
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
Dne 13.10.2014 v 12:42 Petr Vobornik napsal(a): On 8.10.2014 18:51, Petr Vobornik wrote: On 1.10.2014 18:15, Petr Vobornik wrote: Hello list, Patch for: https://fedorahosted.org/freeipa/ticket/4419 New revisions of 761 and 763 with updated API and ACIs: ipa host-allow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-allow-operation HOSTNAME create-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME create-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL create-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL create-keytab --users=STR --groups STR ACIs are targeted to specific operations by including subtypes. patch #761 rebased because of VERSION bump Since we are apparently not going to treat ipaAllowedToPerform as a member attribute, you should remove reference to it from attribute_members and relationships attributes of service and host. What's up with ipaallowedtoperform_subtypes_map? Why rename the operations? You probably don't want to hardcode 'ipaallowedtoperform_read_keys' in rename_ipaallowedtoperform_to_ldap(). Why do you override get_args() in service_{,dis}allow_operation instead of overriding takes_args? I'm not particularly happy about the '_subtype' option bussiness, but at least it's not invasive, so I guess it's OK. Note that I still think this API sucks and we should instead go with the generic member-like attribute approach, or take our time to design it properly so that it fits in the framework (no time in 4.1) instead of making it a hacky Franken-API like it is now. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 8.10.2014 18:51, Petr Vobornik wrote: On 1.10.2014 18:15, Petr Vobornik wrote: Hello list, Patch for: https://fedorahosted.org/freeipa/ticket/4419 New revisions of 761 and 763 with updated API and ACIs: ipa host-allow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-allow-operation HOSTNAME create-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME create-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL create-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL create-keytab --users=STR --groups STR ACIs are targeted to specific operations by including subtypes. patch #761 rebased because of VERSION bump -- Petr Vobornik From 224f09d92b64d1658a13b760a85bc562729af8ed Mon Sep 17 00:00:00 2001 From: Petr Vobornik Date: Thu, 2 Oct 2014 16:57:08 +0200 Subject: [PATCH] keytab manipulation permission management Adds new API: ipa host-allow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-allow-operation HOSTNAME create-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME create-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL create-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL create-keytab --users=STR --groups STR these methods add or remove user or group DNs in `ipaallowedtoperform` attr with `read_keys` and `write_keys` subtypes. service|host-mod|show outputs these attrs as: Users allowed to retrieve keytab: user1 Groups allowed to retrieve keytab: group1 Users allowed to create keytab: user1 Groups allowed to create keytab: group1 Adding of object class is implemented as a reusable method since this code is used on many places and most likely will be also used in new features. Older code may be refactored later. https://fedorahosted.org/freeipa/ticket/4419 --- ACI.txt| 4 ++ API.txt| 52 +++ VERSION| 4 +- ipalib/plugins/baseldap.py | 17 ++ ipalib/plugins/host.py | 51 -- ipalib/plugins/service.py | 127 +++-- 6 files changed, 244 insertions(+), 11 deletions(-) diff --git a/ACI.txt b/ACI.txt index bc031ff1f24b9e9f08fe9ba78e5a262162f32cef..6fc88233e340d6eabe4ad819176e3cfa98d8c9ce 100644 --- a/ACI.txt +++ b/ACI.txt @@ -95,6 +95,8 @@ aci: (targetattr = "userpassword")(targetfilter = "(objectclass=ipahost)")(versi dn: cn=computers,cn=accounts,dc=ipa,dc=example aci: (targetattr = "krblastpwdchange || krbprincipalkey")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Manage Host Keytab";allow (write) groupdn = "ldap:///cn=System: Manage Host Keytab,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=computers,cn=accounts,dc=ipa,dc=example +aci: (targetattr = "createtimestamp || entryusn || ipaallowedtoperform;read_keys || ipaallowedtoperform;write_keys || modifytimestamp || objectclass")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Manage Host Keytab Permissions";allow (compare,read,search,write) groupdn = "ldap:///cn=System: Manage Host Keytab Permissions,cn=permissions,cn=pbac,dc=ipa,dc=example";) +dn: cn=computers,cn=accounts,dc=ipa,dc=example aci: (targetattr = "ipasshpubkey")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Manage Host SSH Public Keys";allow (write) groupdn = "ldap:///cn=System: Manage Host SSH Public Keys,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=computers,cn=accounts,dc=ipa,dc=example aci: (targetattr = "description || ipaassignedidview || l || macaddress || nshardwareplatform || nshostlocation || nsosversion || userclass")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Modify Hosts";allow (write) groupdn = "ldap:///cn=System: Modify Hosts,cn=permissions,cn=pbac,dc=ipa,dc=example";) @@ -193,6 +195,8 @@ aci: (targetfilter = "(objectclass=ipaservice)")(version 3.0;acl "permission:Sys dn: cn=services,cn=accounts,dc=ipa,dc=example aci: (targetattr = "krblastpwdchange || krbprincipalkey")(targetfilter = "(objectclass=ipaservice)")(version 3.0;acl "permission:System: Manage Service Keytab";allow (write) groupdn = "ldap:///cn=System: Manage Service Keytab,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=services,cn=accounts,dc=ipa,dc=example +aci: (targetattr = "createtimestamp || entryusn || ipaallowedtoperform;read_keys
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 1.10.2014 18:15, Petr Vobornik wrote: Hello list, Patch for: https://fedorahosted.org/freeipa/ticket/4419 New revisions of 761 and 763 with updated API and ACIs: ipa host-allow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-allow-operation HOSTNAME create-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME create-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL create-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL create-keytab --users=STR --groups STR ACIs are targeted to specific operations by including subtypes. -- Petr Vobornik From e44e27ca63ab333b50f4cf465ea61115c9c83840 Mon Sep 17 00:00:00 2001 From: Petr Vobornik Date: Thu, 2 Oct 2014 16:57:08 +0200 Subject: [PATCH] keytab manipulation permission management Adds new API: ipa host-allow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME retrieve-keytab --users=STR --groups STR ipa host-allow-operation HOSTNAME create-keytab --users=STR --groups STR ipa host-disallow-operation HOSTNAME create-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL retrieve-keytab --users=STR --groups STR ipa service-allow-operation PRINCIPAL create-keytab --users=STR --groups STR ipa service-disallow-operation PRINCIPAL create-keytab --users=STR --groups STR these methods add or remove user or group DNs in `ipaallowedtoperform` attr with `read_keys` and `write_keys` subtypes. service|host-mod|show outputs these attrs as: Users allowed to retrieve keytab: user1 Groups allowed to retrieve keytab: group1 Users allowed to create keytab: user1 Groups allowed to create keytab: group1 Adding of object class is implemented as a reusable method since this code is used on many places and most likely will be also used in new features. Older code may be refactored later. https://fedorahosted.org/freeipa/ticket/4419 --- ACI.txt| 4 ++ API.txt| 52 +++ VERSION| 4 +- ipalib/plugins/baseldap.py | 17 ++ ipalib/plugins/host.py | 51 -- ipalib/plugins/service.py | 127 +++-- 6 files changed, 244 insertions(+), 11 deletions(-) diff --git a/ACI.txt b/ACI.txt index cebdc2ccec45db1dbf0d5ea0c7f2b1a3a7feeb6e..312e51719d9906f8d6f262330d2bdafe1e59d88a 100644 --- a/ACI.txt +++ b/ACI.txt @@ -95,6 +95,8 @@ aci: (targetattr = "userpassword")(targetfilter = "(objectclass=ipahost)")(versi dn: cn=computers,cn=accounts,dc=ipa,dc=example aci: (targetattr = "krblastpwdchange || krbprincipalkey")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Manage Host Keytab";allow (write) groupdn = "ldap:///cn=System: Manage Host Keytab,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=computers,cn=accounts,dc=ipa,dc=example +aci: (targetattr = "createtimestamp || entryusn || ipaallowedtoperform;read_keys || ipaallowedtoperform;write_keys || modifytimestamp || objectclass")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Manage Host Keytab Permissions";allow (compare,read,search,write) groupdn = "ldap:///cn=System: Manage Host Keytab Permissions,cn=permissions,cn=pbac,dc=ipa,dc=example";) +dn: cn=computers,cn=accounts,dc=ipa,dc=example aci: (targetattr = "ipasshpubkey")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Manage Host SSH Public Keys";allow (write) groupdn = "ldap:///cn=System: Manage Host SSH Public Keys,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=computers,cn=accounts,dc=ipa,dc=example aci: (targetattr = "description || ipaassignedidview || l || macaddress || nshardwareplatform || nshostlocation || nsosversion || userclass")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Modify Hosts";allow (write) groupdn = "ldap:///cn=System: Modify Hosts,cn=permissions,cn=pbac,dc=ipa,dc=example";) @@ -193,6 +195,8 @@ aci: (targetfilter = "(objectclass=ipaservice)")(version 3.0;acl "permission:Sys dn: cn=services,cn=accounts,dc=ipa,dc=example aci: (targetattr = "krblastpwdchange || krbprincipalkey")(targetfilter = "(objectclass=ipaservice)")(version 3.0;acl "permission:System: Manage Service Keytab";allow (write) groupdn = "ldap:///cn=System: Manage Service Keytab,cn=permissions,cn=pbac,dc=ipa,dc=example";) dn: cn=services,cn=accounts,dc=ipa,dc=example +aci: (targetattr = "createtimestamp || entryusn || ipaallowedtoperform;read_keys || ipaallowedtoperform;write_keys || modifytimestamp || objectclass")(targetfilter = "
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 6.10.2014 16:17, Martin Kosek wrote: On 10/06/2014 04:15 PM, Petr Vobornik wrote: On 6.10.2014 15:49, Martin Kosek wrote: On 10/06/2014 03:01 PM, Simo Sorce wrote: On Mon, 06 Oct 2014 12:53:57 +0200 Martin Kosek wrote: On 10/06/2014 10:33 AM, Jan Cholasta wrote: Dne 3.10.2014 v 17:02 Martin Kosek napsal(a): On 10/03/2014 04:59 PM, Jan Cholasta wrote: Dne 3.10.2014 v 16:47 Petr Vobornik napsal(a): On 3.10.2014 16:24, Martin Kosek wrote: NACK. I will not comment on mechanics, if you get an ACK from Honza, it is good enough. I just do not like the API. It is hard to guess what "host-add-retrieve-keytab" means. That word does not even make much sense. Can we use something more readable? For example: ipa host-add-allowed-operation HOSTNAME --operation read_keys --users=STR --groups STR ipa host-add-allowed-operation HOSTNAME --operation write_keys --users=STR --groups STR and ipa host-remove-allowed-operation HOSTNAME --operation read_keys --users=STR --groups STR ipa host-remove-allowed-operation HOSTNAME --operation write_keys --users=STR --groups STR Same with services. At least to me, it looks more readable. Thanks, Martin Seems to me as adding of allowed operation. Not allowing an operation. +1 What about: ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-create-keytab HOSTNAME --users=STR --groups STR I like these the best. Maybe with a -to or -by suffix. or if we expect more operations in a future: ipa host-allow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-allow-operation HOSTNAME --operation write-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation write-keys --users=STR --groups STR or if we want to keep 'add' and 'remove' in command names: ipa host-add-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-add-create-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-create-keytab-right HOSTNAME --users=STR --groups=STR personally I'm not a fan o the --operation switch, but could be persuaded by a 'future' usage. Not a fan either, because it is not consistent with the rest of the framework. Also, non-optional options are not really options. Right. Though mandatory options is a concept already existing in FreeIPA framework in many places. That does not make it right. Right :-) What I see as a deal breaker is that with --operation switch, we are ready for dozens of potential future operations. With operation hardcoded in command name, we are not. I don't see dozens of operations coming in the near future, there's no need for a premature optimization like this. My point was that it will be difficult to switch from having per-operation commands to one general command for all operations later, however far the future is. Given there is no clear agreement on the API (ipa host-allow-operation vs. host-allow-read-keytab+host-allow-write-keytab) yet, I would like to ask Rob or Simo for their opinion/vote here too so that we can select an approach and go with it. I am not even sure why we are tying this to hosts to be honest. Isn't the use case to for example allow several load balancing nodes host/serverX.example.com get a keytab for host/server.example.com? The allow-operation plugin is generic, and we should probably have a command that reflect that like: ipa operations-add/mod/del and options to say what the operation does and what it applies to. Of course the naming needs more thought, but I do not think having a command for this specific narrowed down operations is wise. Actually it may even fit right into the permissions commands (Add a --operation switch ?), as these operations are just a particular type of ACIs/Permissions that apply to abstract operations rather than LDAP operations, so it is a natural extension of our permissions. Simo. Ok, now that's a 180° turn from what we were discussing until now... I really do not think we can go the permission route ATM, the permission plugin is now very bound to how ACIs work and how they are used in FreeIPA tree. Changing the backend from ACI to ipaAllowedOperation would be something completely different. The super-general operation-* command is interesting idea, though it decouples these operations from their targets. I think there is a benefit in seeing an command allowing writing/reading a keytab right in the list of host commands or in host page Web UI. Given the short time before 4.1 release, I think it will be more straightforward to go with ipa host-allow-operation or ipa host-allow-read-keytab/host-allow-write-keytab route. Martin It also works a bit differently. Permissions defined by
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 10/06/2014 04:15 PM, Petr Vobornik wrote: > On 6.10.2014 15:49, Martin Kosek wrote: >> On 10/06/2014 03:01 PM, Simo Sorce wrote: >>> On Mon, 06 Oct 2014 12:53:57 +0200 >>> Martin Kosek wrote: >>> On 10/06/2014 10:33 AM, Jan Cholasta wrote: > Dne 3.10.2014 v 17:02 Martin Kosek napsal(a): >> On 10/03/2014 04:59 PM, Jan Cholasta wrote: >>> Dne 3.10.2014 v 16:47 Petr Vobornik napsal(a): On 3.10.2014 16:24, Martin Kosek wrote: > NACK. I will not comment on mechanics, if you get an ACK from > Honza, it is good enough. I just do not like the API. It is > hard to guess what "host-add-retrieve-keytab" means. That word > does not even make much sense. > > Can we use something more readable? For example: > > ipa host-add-allowed-operation HOSTNAME --operation read_keys > --users=STR --groups STR > ipa host-add-allowed-operation HOSTNAME --operation write_keys > --users=STR --groups STR > > and > > ipa host-remove-allowed-operation HOSTNAME --operation read_keys > --users=STR --groups STR > ipa host-remove-allowed-operation HOSTNAME --operation > write_keys --users=STR --groups STR > > Same with services. At least to me, it looks more readable. > > Thanks, > Martin > Seems to me as adding of allowed operation. Not allowing an operation. >>> >>> +1 >>> What about: ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-create-keytab HOSTNAME --users=STR --groups STR >>> >>> I like these the best. Maybe with a -to or -by suffix. >>> or if we expect more operations in a future: ipa host-allow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-allow-operation HOSTNAME --operation write-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation write-keys --users=STR --groups STR or if we want to keep 'add' and 'remove' in command names: ipa host-add-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-add-create-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-create-keytab-right HOSTNAME --users=STR --groups=STR personally I'm not a fan o the --operation switch, but could be persuaded by a 'future' usage. >>> >>> Not a fan either, because it is not consistent with the rest of >>> the framework. >>> Also, non-optional options are not really options. >> >> Right. Though mandatory options is a concept already existing in >> FreeIPA framework in many places. > > That does not make it right. Right :-) >> What I see as a deal breaker is that with >> --operation switch, we are ready for dozens of potential future >> operations. With operation hardcoded in command name, we are not. > > I don't see dozens of operations coming in the near future, there's > no need for a premature optimization like this. My point was that it will be difficult to switch from having per-operation commands to one general command for all operations later, however far the future is. Given there is no clear agreement on the API (ipa host-allow-operation vs. host-allow-read-keytab+host-allow-write-keytab) yet, I would like to ask Rob or Simo for their opinion/vote here too so that we can select an approach and go with it. >>> >>> I am not even sure why we are tying this to hosts to be honest. >> >> Isn't the use case to for example allow several load balancing nodes >> host/serverX.example.com get a keytab for host/server.example.com? >> >>> The allow-operation plugin is generic, and we should probably have a >>> command that reflect that like: >>> ipa operations-add/mod/del and options to say what the >>> operation does and what it applies to. >>> >>> Of course the naming needs more thought, but I do not think having a >>> command for this specific narrowed down operations is wise. >>> >>> Actually it may even fit right into the permissions commands (Add a >>> --operation switch ?), as these operations are just a particular type >>> of ACIs/Permissions that apply to abstract operations rather than >>> LDAP operations, so it is a natural extension of our permissions. >>> >>> Simo. >> >> Ok, now that's a 180° turn f
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 6.10.2014 15:49, Martin Kosek wrote: On 10/06/2014 03:01 PM, Simo Sorce wrote: On Mon, 06 Oct 2014 12:53:57 +0200 Martin Kosek wrote: On 10/06/2014 10:33 AM, Jan Cholasta wrote: Dne 3.10.2014 v 17:02 Martin Kosek napsal(a): On 10/03/2014 04:59 PM, Jan Cholasta wrote: Dne 3.10.2014 v 16:47 Petr Vobornik napsal(a): On 3.10.2014 16:24, Martin Kosek wrote: NACK. I will not comment on mechanics, if you get an ACK from Honza, it is good enough. I just do not like the API. It is hard to guess what "host-add-retrieve-keytab" means. That word does not even make much sense. Can we use something more readable? For example: ipa host-add-allowed-operation HOSTNAME --operation read_keys --users=STR --groups STR ipa host-add-allowed-operation HOSTNAME --operation write_keys --users=STR --groups STR and ipa host-remove-allowed-operation HOSTNAME --operation read_keys --users=STR --groups STR ipa host-remove-allowed-operation HOSTNAME --operation write_keys --users=STR --groups STR Same with services. At least to me, it looks more readable. Thanks, Martin Seems to me as adding of allowed operation. Not allowing an operation. +1 What about: ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-create-keytab HOSTNAME --users=STR --groups STR I like these the best. Maybe with a -to or -by suffix. or if we expect more operations in a future: ipa host-allow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-allow-operation HOSTNAME --operation write-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation write-keys --users=STR --groups STR or if we want to keep 'add' and 'remove' in command names: ipa host-add-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-add-create-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-create-keytab-right HOSTNAME --users=STR --groups=STR personally I'm not a fan o the --operation switch, but could be persuaded by a 'future' usage. Not a fan either, because it is not consistent with the rest of the framework. Also, non-optional options are not really options. Right. Though mandatory options is a concept already existing in FreeIPA framework in many places. That does not make it right. Right :-) What I see as a deal breaker is that with --operation switch, we are ready for dozens of potential future operations. With operation hardcoded in command name, we are not. I don't see dozens of operations coming in the near future, there's no need for a premature optimization like this. My point was that it will be difficult to switch from having per-operation commands to one general command for all operations later, however far the future is. Given there is no clear agreement on the API (ipa host-allow-operation vs. host-allow-read-keytab+host-allow-write-keytab) yet, I would like to ask Rob or Simo for their opinion/vote here too so that we can select an approach and go with it. I am not even sure why we are tying this to hosts to be honest. Isn't the use case to for example allow several load balancing nodes host/serverX.example.com get a keytab for host/server.example.com? The allow-operation plugin is generic, and we should probably have a command that reflect that like: ipa operations-add/mod/del and options to say what the operation does and what it applies to. Of course the naming needs more thought, but I do not think having a command for this specific narrowed down operations is wise. Actually it may even fit right into the permissions commands (Add a --operation switch ?), as these operations are just a particular type of ACIs/Permissions that apply to abstract operations rather than LDAP operations, so it is a natural extension of our permissions. Simo. Ok, now that's a 180° turn from what we were discussing until now... I really do not think we can go the permission route ATM, the permission plugin is now very bound to how ACIs work and how they are used in FreeIPA tree. Changing the backend from ACI to ipaAllowedOperation would be something completely different. The super-general operation-* command is interesting idea, though it decouples these operations from their targets. I think there is a benefit in seeing an command allowing writing/reading a keytab right in the list of host commands or in host page Web UI. Given the short time before 4.1 release, I think it will be more straightforward to go with ipa host-allow-operation or ipa host-allow-read-keytab/host-allow-write-keytab route. Martin It also works a bit differently. Permissions defined by admin, using a permission plugin, are part of RBAC. Allowed operations aren't. Binding
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On Mon, 06 Oct 2014 15:49:09 +0200 Martin Kosek wrote: > On 10/06/2014 03:01 PM, Simo Sorce wrote: > > On Mon, 06 Oct 2014 12:53:57 +0200 > > Martin Kosek wrote: > > > >> On 10/06/2014 10:33 AM, Jan Cholasta wrote: > >>> Dne 3.10.2014 v 17:02 Martin Kosek napsal(a): > On 10/03/2014 04:59 PM, Jan Cholasta wrote: > > Dne 3.10.2014 v 16:47 Petr Vobornik napsal(a): > >> On 3.10.2014 16:24, Martin Kosek wrote: > >>> NACK. I will not comment on mechanics, if you get an ACK from > >>> Honza, it is good enough. I just do not like the API. It is > >>> hard to guess what "host-add-retrieve-keytab" means. That word > >>> does not even make much sense. > >>> > >>> Can we use something more readable? For example: > >>> > >>> ipa host-add-allowed-operation HOSTNAME --operation read_keys > >>> --users=STR --groups STR > >>> ipa host-add-allowed-operation HOSTNAME --operation write_keys > >>> --users=STR --groups STR > >>> > >>> and > >>> > >>> ipa host-remove-allowed-operation HOSTNAME --operation > >>> read_keys --users=STR --groups STR > >>> ipa host-remove-allowed-operation HOSTNAME --operation > >>> write_keys --users=STR --groups STR > >>> > >>> Same with services. At least to me, it looks more readable. > >>> > >>> Thanks, > >>> Martin > >>> > >> > >> Seems to me as adding of allowed operation. Not allowing an > >> operation. > > > > +1 > > > >> > >> What about: > >> > >> ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups > >> STR ipa host-disallow-retrieve-keytab HOSTNAME --users=STR > >> --groups STR ipa host-allow-create-keytab HOSTNAME --users=STR > >> --groups STR ipa host-disallow-create-keytab HOSTNAME > >> --users=STR --groups STR > > > > I like these the best. Maybe with a -to or -by suffix. > > > >> > >> or if we expect more operations in a future: > >> > >> ipa host-allow-operation HOSTNAME --operation read-keys > >> --users=STR --groups STR > >> ipa host-disallow-operation HOSTNAME --operation read-keys > >> --users=STR --groups STR > >> ipa host-allow-operation HOSTNAME --operation write-keys > >> --users=STR --groups STR > >> ipa host-disallow-operation HOSTNAME --operation write-keys > >> --users=STR --groups STR > >> > >> or if we want to keep 'add' and 'remove' in command names: > >> > >> ipa host-add-retrieve-keytab-right HOSTNAME --users=STR > >> --groups=STR ipa host-add-create-keytab-right HOSTNAME > >> --users=STR --groups=STR ipa host-remove-retrieve-keytab-right > >> HOSTNAME --users=STR --groups=STR ipa > >> host-remove-create-keytab-right HOSTNAME --users=STR > >> --groups=STR > >> > >> > >> personally I'm not a fan o the --operation switch, but could be > >> persuaded by a 'future' usage. > > > > Not a fan either, because it is not consistent with the rest of > > the framework. > > Also, non-optional options are not really options. > > Right. Though mandatory options is a concept already existing in > FreeIPA framework in many places. > >>> > >>> That does not make it right. > >> > >> Right :-) > >> > What I see as a deal breaker is that with > --operation switch, we are ready for dozens of potential future > operations. With operation hardcoded in command name, we are not. > >>> > >>> I don't see dozens of operations coming in the near future, > >>> there's no need for a premature optimization like this. > >> > >> My point was that it will be difficult to switch from having > >> per-operation commands to one general command for all operations > >> later, however far the future is. > >> > >> Given there is no clear agreement on the API (ipa > >> host-allow-operation vs. > >> host-allow-read-keytab+host-allow-write-keytab) yet, I would like > >> to ask Rob or Simo for their opinion/vote here too so that we can > >> select an approach and go with it. > > > > I am not even sure why we are tying this to hosts to be honest. > > Isn't the use case to for example allow several load balancing nodes > host/serverX.example.com get a keytab for host/server.example.com? > > > The allow-operation plugin is generic, and we should probably have a > > command that reflect that like: > > ipa operations-add/mod/del and options to say what the > > operation does and what it applies to. > > > > Of course the naming needs more thought, but I do not think having a > > command for this specific narrowed down operations is wise. > > > > Actually it may even fit right into the permissions commands (Add a > > --operation switch ?), as these operations are just a particular > > type of ACIs/Permissions that apply to abstract operations rather > > than LDAP operations, so it is a natural extension of our > > permissions. > > > > Simo. > > Ok, now that's a 180° turn from what we were
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 10/06/2014 03:01 PM, Simo Sorce wrote: > On Mon, 06 Oct 2014 12:53:57 +0200 > Martin Kosek wrote: > >> On 10/06/2014 10:33 AM, Jan Cholasta wrote: >>> Dne 3.10.2014 v 17:02 Martin Kosek napsal(a): On 10/03/2014 04:59 PM, Jan Cholasta wrote: > Dne 3.10.2014 v 16:47 Petr Vobornik napsal(a): >> On 3.10.2014 16:24, Martin Kosek wrote: >>> NACK. I will not comment on mechanics, if you get an ACK from >>> Honza, it is good enough. I just do not like the API. It is >>> hard to guess what "host-add-retrieve-keytab" means. That word >>> does not even make much sense. >>> >>> Can we use something more readable? For example: >>> >>> ipa host-add-allowed-operation HOSTNAME --operation read_keys >>> --users=STR --groups STR >>> ipa host-add-allowed-operation HOSTNAME --operation write_keys >>> --users=STR --groups STR >>> >>> and >>> >>> ipa host-remove-allowed-operation HOSTNAME --operation read_keys >>> --users=STR --groups STR >>> ipa host-remove-allowed-operation HOSTNAME --operation >>> write_keys --users=STR --groups STR >>> >>> Same with services. At least to me, it looks more readable. >>> >>> Thanks, >>> Martin >>> >> >> Seems to me as adding of allowed operation. Not allowing an >> operation. > > +1 > >> >> What about: >> >> ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR >> ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups >> STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups >> STR ipa host-disallow-create-keytab HOSTNAME --users=STR >> --groups STR > > I like these the best. Maybe with a -to or -by suffix. > >> >> or if we expect more operations in a future: >> >> ipa host-allow-operation HOSTNAME --operation read-keys >> --users=STR --groups STR >> ipa host-disallow-operation HOSTNAME --operation read-keys >> --users=STR --groups STR >> ipa host-allow-operation HOSTNAME --operation write-keys >> --users=STR --groups STR >> ipa host-disallow-operation HOSTNAME --operation write-keys >> --users=STR --groups STR >> >> or if we want to keep 'add' and 'remove' in command names: >> >> ipa host-add-retrieve-keytab-right HOSTNAME --users=STR >> --groups=STR ipa host-add-create-keytab-right HOSTNAME >> --users=STR --groups=STR ipa host-remove-retrieve-keytab-right >> HOSTNAME --users=STR --groups=STR ipa >> host-remove-create-keytab-right HOSTNAME --users=STR --groups=STR >> >> >> personally I'm not a fan o the --operation switch, but could be >> persuaded by a 'future' usage. > > Not a fan either, because it is not consistent with the rest of > the framework. > Also, non-optional options are not really options. Right. Though mandatory options is a concept already existing in FreeIPA framework in many places. >>> >>> That does not make it right. >> >> Right :-) >> What I see as a deal breaker is that with --operation switch, we are ready for dozens of potential future operations. With operation hardcoded in command name, we are not. >>> >>> I don't see dozens of operations coming in the near future, there's >>> no need for a premature optimization like this. >> >> My point was that it will be difficult to switch from having >> per-operation commands to one general command for all operations >> later, however far the future is. >> >> Given there is no clear agreement on the API (ipa >> host-allow-operation vs. >> host-allow-read-keytab+host-allow-write-keytab) yet, I would like to >> ask Rob or Simo for their opinion/vote here too so that we can select >> an approach and go with it. > > I am not even sure why we are tying this to hosts to be honest. Isn't the use case to for example allow several load balancing nodes host/serverX.example.com get a keytab for host/server.example.com? > The allow-operation plugin is generic, and we should probably have a > command that reflect that like: > ipa operations-add/mod/del and options to say what the > operation does and what it applies to. > > Of course the naming needs more thought, but I do not think having a > command for this specific narrowed down operations is wise. > > Actually it may even fit right into the permissions commands (Add a > --operation switch ?), as these operations are just a particular type > of ACIs/Permissions that apply to abstract operations rather than > LDAP operations, so it is a natural extension of our permissions. > > Simo. Ok, now that's a 180° turn from what we were discussing until now... I really do not think we can go the permission route ATM, the permission plugin is now very bound to how ACIs work and how they are used in FreeIPA tree. Changing the backend from ACI to ipaAllowedOperation would be something completely different. The super-general operation-* command is i
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On Mon, 06 Oct 2014 12:53:57 +0200 Martin Kosek wrote: > On 10/06/2014 10:33 AM, Jan Cholasta wrote: > > Dne 3.10.2014 v 17:02 Martin Kosek napsal(a): > >> On 10/03/2014 04:59 PM, Jan Cholasta wrote: > >>> Dne 3.10.2014 v 16:47 Petr Vobornik napsal(a): > On 3.10.2014 16:24, Martin Kosek wrote: > > NACK. I will not comment on mechanics, if you get an ACK from > > Honza, it is good enough. I just do not like the API. It is > > hard to guess what "host-add-retrieve-keytab" means. That word > > does not even make much sense. > > > > Can we use something more readable? For example: > > > > ipa host-add-allowed-operation HOSTNAME --operation read_keys > > --users=STR --groups STR > > ipa host-add-allowed-operation HOSTNAME --operation write_keys > > --users=STR --groups STR > > > > and > > > > ipa host-remove-allowed-operation HOSTNAME --operation read_keys > > --users=STR --groups STR > > ipa host-remove-allowed-operation HOSTNAME --operation > > write_keys --users=STR --groups STR > > > > Same with services. At least to me, it looks more readable. > > > > Thanks, > > Martin > > > > Seems to me as adding of allowed operation. Not allowing an > operation. > >>> > >>> +1 > >>> > > What about: > > ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR > ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups > STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups > STR ipa host-disallow-create-keytab HOSTNAME --users=STR > --groups STR > >>> > >>> I like these the best. Maybe with a -to or -by suffix. > >>> > > or if we expect more operations in a future: > > ipa host-allow-operation HOSTNAME --operation read-keys > --users=STR --groups STR > ipa host-disallow-operation HOSTNAME --operation read-keys > --users=STR --groups STR > ipa host-allow-operation HOSTNAME --operation write-keys > --users=STR --groups STR > ipa host-disallow-operation HOSTNAME --operation write-keys > --users=STR --groups STR > > or if we want to keep 'add' and 'remove' in command names: > > ipa host-add-retrieve-keytab-right HOSTNAME --users=STR > --groups=STR ipa host-add-create-keytab-right HOSTNAME > --users=STR --groups=STR ipa host-remove-retrieve-keytab-right > HOSTNAME --users=STR --groups=STR ipa > host-remove-create-keytab-right HOSTNAME --users=STR --groups=STR > > > personally I'm not a fan o the --operation switch, but could be > persuaded by a 'future' usage. > >>> > >>> Not a fan either, because it is not consistent with the rest of > >>> the framework. > >>> Also, non-optional options are not really options. > >> > >> Right. Though mandatory options is a concept already existing in > >> FreeIPA framework in many places. > > > > That does not make it right. > > Right :-) > > >> What I see as a deal breaker is that with > >> --operation switch, we are ready for dozens of potential future > >> operations. With operation hardcoded in command name, we are not. > > > > I don't see dozens of operations coming in the near future, there's > > no need for a premature optimization like this. > > My point was that it will be difficult to switch from having > per-operation commands to one general command for all operations > later, however far the future is. > > Given there is no clear agreement on the API (ipa > host-allow-operation vs. > host-allow-read-keytab+host-allow-write-keytab) yet, I would like to > ask Rob or Simo for their opinion/vote here too so that we can select > an approach and go with it. I am not even sure why we are tying this to hosts to be honest. The allow-operation plugin is generic, and we should probably have a command that reflect that like: ipa operations-add/mod/del and options to say what the operation does and what it applies to. Of course the naming needs more thought, but I do not think having a command for this specific narrowed down operations is wise. Actually it may even fit right into the permissions commands (Add a --operation switch ?), as these operations are just a particular type of ACIs/Permissions that apply to abstract operations rather than LDAP operations, so it is a natural extension of our permissions. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 10/06/2014 01:31 PM, Petr Viktorin wrote: > On 10/03/2014 05:02 PM, Martin Kosek wrote: > [...] >>> I like these the best. Maybe with a -to or -by suffix. >>> or if we expect more operations in a future: ipa host-allow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-allow-operation HOSTNAME --operation write-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation write-keys --users=STR --groups STR or if we want to keep 'add' and 'remove' in command names: ipa host-add-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-add-create-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-create-keytab-right HOSTNAME --users=STR --groups=STR personally I'm not a fan o the --operation switch, but could be persuaded by a 'future' usage. >>> >>> Not a fan either, because it is not consistent with the rest of the >>> framework. >>> Also, non-optional options are not really options. > > To quote optparse docs: >> If there is a piece of information that your program absolutely requires >> in order to run successfully, that’s what positional arguments are for. > > How about something like: > > ipa host-allow-operation HOSTNAME read-keys --users=STR --groups STR > ipa host-disallow-operation HOSTNAME read-keys --users=STR --groups STR > ipa host-allow-operation HOSTNAME write-keys --users=STR --groups STR > ipa host-disallow-operation HOSTNAME write-keys --users=STR --groups STR Hmm, works for me - from CLI perspective. Not sure though how easy it is to implement in the current framework. But at least with "takes_args" attribute it should not be difficult to specify the arguments. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 10/03/2014 05:02 PM, Martin Kosek wrote: [...] I like these the best. Maybe with a -to or -by suffix. or if we expect more operations in a future: ipa host-allow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-allow-operation HOSTNAME --operation write-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation write-keys --users=STR --groups STR or if we want to keep 'add' and 'remove' in command names: ipa host-add-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-add-create-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-create-keytab-right HOSTNAME --users=STR --groups=STR personally I'm not a fan o the --operation switch, but could be persuaded by a 'future' usage. Not a fan either, because it is not consistent with the rest of the framework. Also, non-optional options are not really options. To quote optparse docs: If there is a piece of information that your program absolutely requires in order to run successfully, that’s what positional arguments are for. How about something like: ipa host-allow-operation HOSTNAME read-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME read-keys --users=STR --groups STR ipa host-allow-operation HOSTNAME write-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME write-keys --users=STR --groups STR Right. Though mandatory options is a concept already existing in FreeIPA framework in many places. What I see as a deal breaker is that with --operation switch, we are ready for dozens of potential future operations. With operation hardcoded in command name, we are not. Positional arguments (multiple *keys) also have their precedents, in DNS or automount plugins. Also note that framework internals can be changed more easily (to achieve more consistency) than API. Martin -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 10/06/2014 10:33 AM, Jan Cholasta wrote: > Dne 3.10.2014 v 17:02 Martin Kosek napsal(a): >> On 10/03/2014 04:59 PM, Jan Cholasta wrote: >>> Dne 3.10.2014 v 16:47 Petr Vobornik napsal(a): On 3.10.2014 16:24, Martin Kosek wrote: > NACK. I will not comment on mechanics, if you get an ACK from Honza, it > is good enough. I just do not like the API. It is hard to guess what > "host-add-retrieve-keytab" means. That word does not even make much > sense. > > Can we use something more readable? For example: > > ipa host-add-allowed-operation HOSTNAME --operation read_keys > --users=STR --groups STR > ipa host-add-allowed-operation HOSTNAME --operation write_keys > --users=STR --groups STR > > and > > ipa host-remove-allowed-operation HOSTNAME --operation read_keys > --users=STR --groups STR > ipa host-remove-allowed-operation HOSTNAME --operation write_keys > --users=STR --groups STR > > Same with services. At least to me, it looks more readable. > > Thanks, > Martin > Seems to me as adding of allowed operation. Not allowing an operation. >>> >>> +1 >>> What about: ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-create-keytab HOSTNAME --users=STR --groups STR >>> >>> I like these the best. Maybe with a -to or -by suffix. >>> or if we expect more operations in a future: ipa host-allow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-allow-operation HOSTNAME --operation write-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation write-keys --users=STR --groups STR or if we want to keep 'add' and 'remove' in command names: ipa host-add-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-add-create-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-create-keytab-right HOSTNAME --users=STR --groups=STR personally I'm not a fan o the --operation switch, but could be persuaded by a 'future' usage. >>> >>> Not a fan either, because it is not consistent with the rest of the >>> framework. >>> Also, non-optional options are not really options. >> >> Right. Though mandatory options is a concept already existing in FreeIPA >> framework in many places. > > That does not make it right. Right :-) >> What I see as a deal breaker is that with >> --operation switch, we are ready for dozens of potential future >> operations. With operation hardcoded in command name, we are not. > > I don't see dozens of operations coming in the near future, there's no need > for > a premature optimization like this. My point was that it will be difficult to switch from having per-operation commands to one general command for all operations later, however far the future is. Given there is no clear agreement on the API (ipa host-allow-operation vs. host-allow-read-keytab+host-allow-write-keytab) yet, I would like to ask Rob or Simo for their opinion/vote here too so that we can select an approach and go with it. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 10/03/2014 05:03 PM, Petr Vobornik wrote: > On 3.10.2014 16:46, Simo Sorce wrote: I did not do any ACI work in the patch yet. I assume that we would like to add the attr into 'System: Read Host|Service' permission. But I think that write right should have it's own permission. >>> >>> I have added 2 new permissions. Simo, are they OK? >>> >>> for services: >>> 'System: Manage Service Keytab Permissions': { >>> 'ipapermright': {'read', 'search', 'compare', 'write'}, >>> 'ipapermdefaultattr': {'ipaallowedtoperform', 'objectclass'}, >>> 'default_privileges': {'Service Administrators', 'Host >>> Administrators'}, >>> }, >>> >>> for hosts: >>> 'System: Manage Host Keytab Permissions': { >>> 'ipapermright': {'read', 'search', 'compare', 'write'}, >>> 'ipapermdefaultattr': {'ipaallowedtoperform', 'objectclass'}, >>> 'default_privileges': {'Host Administrators'}, >>> }, >>> >>> I'm not sure about the write right for 'objectclass' but it's required >>> in order to add 'ipaallowedoperations' oc. >> >> As long as it allows only to add/remove the specific value it should be fine. >> >> Can you please send the raw ACIs ? >> I still find it difficult to reason on the security of the result withouth >> looking at the lower level. >> > > in cn=computers,cn=accounts,dc=example,dc=com: > > (targetattr = "createtimestamp || entryusn || ipaallowedtoperform || > modifytimestamp || objectclass")(targetfilter = > "(objectclass=ipahost)")(version 3.0;acl "permission:System: Manage Host > Keytab > Permissions";allow (compare,read,search,write) groupdn = "ldap:///cn=System: > Manage Host Keytab Permissions,cn=permissions,cn=pbac,dc=example,dc=com";) > > in cn=services,cn=accounts,dc=idm,dc=example,dc=com: > > (targetattr = "createtimestamp || entryusn || ipaallowedtoperform || > modifytimestamp || objectclass")(targetfilter = > "(objectclass=ipaservice)")(version 3.0;acl "permission:System: Manage Service > Keytab Permissions";allow (compare,read,search,write) groupdn = > "ldap:///cn=System: Manage Service Keytab > Permissions,cn=permissions,cn=pbac,dc=example,dc=com";) I do not think this is what Simo wanted to see. IMO, by "allowing only a specific value" he wanted to allow holders of a permission to only add/update/remove only selected operations, for example ipaallowedtoperform;read_key. The targetattr part should then only target ipaallowedtoperform;read_key and not whole ipaallowedtoperform. (If that is really supported in DS ACI). Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
Dne 3.10.2014 v 17:02 Martin Kosek napsal(a): On 10/03/2014 04:59 PM, Jan Cholasta wrote: Dne 3.10.2014 v 16:47 Petr Vobornik napsal(a): On 3.10.2014 16:24, Martin Kosek wrote: NACK. I will not comment on mechanics, if you get an ACK from Honza, it is good enough. I just do not like the API. It is hard to guess what "host-add-retrieve-keytab" means. That word does not even make much sense. Can we use something more readable? For example: ipa host-add-allowed-operation HOSTNAME --operation read_keys --users=STR --groups STR ipa host-add-allowed-operation HOSTNAME --operation write_keys --users=STR --groups STR and ipa host-remove-allowed-operation HOSTNAME --operation read_keys --users=STR --groups STR ipa host-remove-allowed-operation HOSTNAME --operation write_keys --users=STR --groups STR Same with services. At least to me, it looks more readable. Thanks, Martin Seems to me as adding of allowed operation. Not allowing an operation. +1 What about: ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-create-keytab HOSTNAME --users=STR --groups STR I like these the best. Maybe with a -to or -by suffix. or if we expect more operations in a future: ipa host-allow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-allow-operation HOSTNAME --operation write-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation write-keys --users=STR --groups STR or if we want to keep 'add' and 'remove' in command names: ipa host-add-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-add-create-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-create-keytab-right HOSTNAME --users=STR --groups=STR personally I'm not a fan o the --operation switch, but could be persuaded by a 'future' usage. Not a fan either, because it is not consistent with the rest of the framework. Also, non-optional options are not really options. Right. Though mandatory options is a concept already existing in FreeIPA framework in many places. That does not make it right. What I see as a deal breaker is that with --operation switch, we are ready for dozens of potential future operations. With operation hardcoded in command name, we are not. I don't see dozens of operations coming in the near future, there's no need for a premature optimization like this. Also note that framework internals can be changed more easily (to achieve more consistency) than API. I would rather avoid ad-hoc "enhancements" that add more complexity and untransparentness (if that's a word) to the framework, we already have enough of these IMHO. Martin -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 3.10.2014 16:46, Simo Sorce wrote: I did not do any ACI work in the patch yet. I assume that we would like to add the attr into 'System: Read Host|Service' permission. But I think that write right should have it's own permission. I have added 2 new permissions. Simo, are they OK? for services: 'System: Manage Service Keytab Permissions': { 'ipapermright': {'read', 'search', 'compare', 'write'}, 'ipapermdefaultattr': {'ipaallowedtoperform', 'objectclass'}, 'default_privileges': {'Service Administrators', 'Host Administrators'}, }, for hosts: 'System: Manage Host Keytab Permissions': { 'ipapermright': {'read', 'search', 'compare', 'write'}, 'ipapermdefaultattr': {'ipaallowedtoperform', 'objectclass'}, 'default_privileges': {'Host Administrators'}, }, I'm not sure about the write right for 'objectclass' but it's required in order to add 'ipaallowedoperations' oc. As long as it allows only to add/remove the specific value it should be fine. Can you please send the raw ACIs ? I still find it difficult to reason on the security of the result withouth looking at the lower level. in cn=computers,cn=accounts,dc=example,dc=com: (targetattr = "createtimestamp || entryusn || ipaallowedtoperform || modifytimestamp || objectclass")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Manage Host Keytab Permissions";allow (compare,read,search,write) groupdn = "ldap:///cn=System: Manage Host Keytab Permissions,cn=permissions,cn=pbac,dc=example,dc=com";) in cn=services,cn=accounts,dc=idm,dc=example,dc=com: (targetattr = "createtimestamp || entryusn || ipaallowedtoperform || modifytimestamp || objectclass")(targetfilter = "(objectclass=ipaservice)")(version 3.0;acl "permission:System: Manage Service Keytab Permissions";allow (compare,read,search,write) groupdn = "ldap:///cn=System: Manage Service Keytab Permissions,cn=permissions,cn=pbac,dc=example,dc=com";) Simo. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 10/03/2014 04:59 PM, Jan Cholasta wrote: Dne 3.10.2014 v 16:47 Petr Vobornik napsal(a): On 3.10.2014 16:24, Martin Kosek wrote: NACK. I will not comment on mechanics, if you get an ACK from Honza, it is good enough. I just do not like the API. It is hard to guess what "host-add-retrieve-keytab" means. That word does not even make much sense. Can we use something more readable? For example: ipa host-add-allowed-operation HOSTNAME --operation read_keys --users=STR --groups STR ipa host-add-allowed-operation HOSTNAME --operation write_keys --users=STR --groups STR and ipa host-remove-allowed-operation HOSTNAME --operation read_keys --users=STR --groups STR ipa host-remove-allowed-operation HOSTNAME --operation write_keys --users=STR --groups STR Same with services. At least to me, it looks more readable. Thanks, Martin Seems to me as adding of allowed operation. Not allowing an operation. +1 What about: ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-create-keytab HOSTNAME --users=STR --groups STR I like these the best. Maybe with a -to or -by suffix. or if we expect more operations in a future: ipa host-allow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-allow-operation HOSTNAME --operation write-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation write-keys --users=STR --groups STR or if we want to keep 'add' and 'remove' in command names: ipa host-add-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-add-create-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-create-keytab-right HOSTNAME --users=STR --groups=STR personally I'm not a fan o the --operation switch, but could be persuaded by a 'future' usage. Not a fan either, because it is not consistent with the rest of the framework. Also, non-optional options are not really options. Right. Though mandatory options is a concept already existing in FreeIPA framework in many places. What I see as a deal breaker is that with --operation switch, we are ready for dozens of potential future operations. With operation hardcoded in command name, we are not. Also note that framework internals can be changed more easily (to achieve more consistency) than API. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
Dne 3.10.2014 v 16:47 Petr Vobornik napsal(a): On 3.10.2014 16:24, Martin Kosek wrote: NACK. I will not comment on mechanics, if you get an ACK from Honza, it is good enough. I just do not like the API. It is hard to guess what "host-add-retrieve-keytab" means. That word does not even make much sense. Can we use something more readable? For example: ipa host-add-allowed-operation HOSTNAME --operation read_keys --users=STR --groups STR ipa host-add-allowed-operation HOSTNAME --operation write_keys --users=STR --groups STR and ipa host-remove-allowed-operation HOSTNAME --operation read_keys --users=STR --groups STR ipa host-remove-allowed-operation HOSTNAME --operation write_keys --users=STR --groups STR Same with services. At least to me, it looks more readable. Thanks, Martin Seems to me as adding of allowed operation. Not allowing an operation. +1 What about: ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-create-keytab HOSTNAME --users=STR --groups STR I like these the best. Maybe with a -to or -by suffix. or if we expect more operations in a future: ipa host-allow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-allow-operation HOSTNAME --operation write-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation write-keys --users=STR --groups STR or if we want to keep 'add' and 'remove' in command names: ipa host-add-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-add-create-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-create-keytab-right HOSTNAME --users=STR --groups=STR personally I'm not a fan o the --operation switch, but could be persuaded by a 'future' usage. Not a fan either, because it is not consistent with the rest of the framework. Also, non-optional options are not really options. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 10/03/2014 04:47 PM, Petr Vobornik wrote: On 3.10.2014 16:24, Martin Kosek wrote: NACK. I will not comment on mechanics, if you get an ACK from Honza, it is good enough. I just do not like the API. It is hard to guess what "host-add-retrieve-keytab" means. That word does not even make much sense. Can we use something more readable? For example: ipa host-add-allowed-operation HOSTNAME --operation read_keys --users=STR --groups STR ipa host-add-allowed-operation HOSTNAME --operation write_keys --users=STR --groups STR and ipa host-remove-allowed-operation HOSTNAME --operation read_keys --users=STR --groups STR ipa host-remove-allowed-operation HOSTNAME --operation write_keys --users=STR --groups STR Same with services. At least to me, it looks more readable. Thanks, Martin Seems to me as adding of allowed operation. Not allowing an operation. What about: ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-create-keytab HOSTNAME --users=STR --groups STR or if we expect more operations in a future: ipa host-allow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-allow-operation HOSTNAME --operation write-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation write-keys --users=STR --groups STR or if we want to keep 'add' and 'remove' in command names: ipa host-add-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-add-create-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-create-keytab-right HOSTNAME --users=STR --groups=STR personally I'm not a fan o the --operation switch, but could be persuaded by a 'future' usage. ipa host-allow-operation HOSTNAME --operation read-keys --users=STR --groups STR and friends looks the best to me. Given the way the ipaAllowedOperation attribute is designed (countless possible sub types), new future operations can be expected. Simo or Rob, any opinion on this API? Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
On 3.10.2014 16:24, Martin Kosek wrote: NACK. I will not comment on mechanics, if you get an ACK from Honza, it is good enough. I just do not like the API. It is hard to guess what "host-add-retrieve-keytab" means. That word does not even make much sense. Can we use something more readable? For example: ipa host-add-allowed-operation HOSTNAME --operation read_keys --users=STR --groups STR ipa host-add-allowed-operation HOSTNAME --operation write_keys --users=STR --groups STR and ipa host-remove-allowed-operation HOSTNAME --operation read_keys --users=STR --groups STR ipa host-remove-allowed-operation HOSTNAME --operation write_keys --users=STR --groups STR Same with services. At least to me, it looks more readable. Thanks, Martin Seems to me as adding of allowed operation. Not allowing an operation. What about: ipa host-allow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-retrieve-keytab HOSTNAME --users=STR --groups STR ipa host-allow-create-keytab HOSTNAME --users=STR --groups STR ipa host-disallow-create-keytab HOSTNAME --users=STR --groups STR or if we expect more operations in a future: ipa host-allow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation read-keys --users=STR --groups STR ipa host-allow-operation HOSTNAME --operation write-keys --users=STR --groups STR ipa host-disallow-operation HOSTNAME --operation write-keys --users=STR --groups STR or if we want to keep 'add' and 'remove' in command names: ipa host-add-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-add-create-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-retrieve-keytab-right HOSTNAME --users=STR --groups=STR ipa host-remove-create-keytab-right HOSTNAME --users=STR --groups=STR personally I'm not a fan o the --operation switch, but could be persuaded by a 'future' usage. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
- Original Message - > From: "Petr Vobornik" > To: "freeipa-devel" , "jch >> Jan Cholasta" > , "simo Sorce" > > Sent: Friday, October 3, 2014 10:08:53 AM > Subject: Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission > management > > New revision according to Honza's recommendations. Comments inline. > > On 1.10.2014 18:15, Petr Vobornik wrote: > > Hello list, > > > > Patch for: https://fedorahosted.org/freeipa/ticket/4419 > > > > Before I start any work on Web UI and tests I would like to gather > > feedback on: > > - the new API > > - member attributes with subtypes management approach > > - ACI > > > > I did not do any ACI work in the patch yet. I assume that we would like > > to add the attr into 'System: Read Host|Service' permission. But I > > think that write right should have it's own permission. > > I have added 2 new permissions. Simo, are they OK? > > for services: > 'System: Manage Service Keytab Permissions': { > 'ipapermright': {'read', 'search', 'compare', 'write'}, > 'ipapermdefaultattr': {'ipaallowedtoperform', 'objectclass'}, > 'default_privileges': {'Service Administrators', 'Host > Administrators'}, > }, > > for hosts: > 'System: Manage Host Keytab Permissions': { > 'ipapermright': {'read', 'search', 'compare', 'write'}, > 'ipapermdefaultattr': {'ipaallowedtoperform', 'objectclass'}, > 'default_privileges': {'Host Administrators'}, > }, > > I'm not sure about the write right for 'objectclass' but it's required > in order to add 'ipaallowedoperations' oc. As long as it allows only to add/remove the specific value it should be fine. Can you please send the raw ACIs ? I still find it difficult to reason on the security of the result withouth looking at the lower level. > > Patch info: > > Adds new API: > >ipa host-add-retrieve-keytab HOSTNAME --users=STR --groups=STR > >ipa host-add-write-keytab HOSTNAME --users=STR --groups=STR > >ipa host-remove-retrieve-keytab HOSTNAME --users=STR --groups=STR > >ipa host-remove-write-keytab HOSTNAME --users=STR --groups=STR > > > >ipa service-add-retrieve-keytab PRINCIPAL --users=STR --groups=STR > >ipa service-add-write-keytab PRINCIPAL --users=STR --groups=STR > >ipa service-remove-retrieve-keytab PRINCIPAL --users=STR --groups=STR > >ipa service-remove-write-keytab PRINCIPAL --users=STR --groups=STR > > *-write-keytab commands were changed to *-create-keytab to be consistent > with descriptions Uhmm these names are a bit difficult to grok, maybe somehting like: ipa service-modify --allowed_operations retrieve_keytab/create_keytab would be easier. Simo. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
NACK. I will not comment on mechanics, if you get an ACK from Honza, it is good enough. I just do not like the API. It is hard to guess what "host-add-retrieve-keytab" means. That word does not even make much sense. Can we use something more readable? For example: ipa host-add-allowed-operation HOSTNAME --operation read_keys --users=STR --groups STR ipa host-add-allowed-operation HOSTNAME --operation write_keys --users=STR --groups STR and ipa host-remove-allowed-operation HOSTNAME --operation read_keys --users=STR --groups STR ipa host-remove-allowed-operation HOSTNAME --operation write_keys --users=STR --groups STR Same with services. At least to me, it looks more readable. Thanks, Martin On 10/03/2014 04:08 PM, Petr Vobornik wrote: New revision according to Honza's recommendations. Comments inline. On 1.10.2014 18:15, Petr Vobornik wrote: Hello list, Patch for: https://fedorahosted.org/freeipa/ticket/4419 Before I start any work on Web UI and tests I would like to gather feedback on: - the new API - member attributes with subtypes management approach - ACI I did not do any ACI work in the patch yet. I assume that we would like to add the attr into 'System: Read Host|Service' permission. But I think that write right should have it's own permission. I have added 2 new permissions. Simo, are they OK? for services: 'System: Manage Service Keytab Permissions': { 'ipapermright': {'read', 'search', 'compare', 'write'}, 'ipapermdefaultattr': {'ipaallowedtoperform', 'objectclass'}, 'default_privileges': {'Service Administrators', 'Host Administrators'}, }, for hosts: 'System: Manage Host Keytab Permissions': { 'ipapermright': {'read', 'search', 'compare', 'write'}, 'ipapermdefaultattr': {'ipaallowedtoperform', 'objectclass'}, 'default_privileges': {'Host Administrators'}, }, I'm not sure about the write right for 'objectclass' but it's required in order to add 'ipaallowedoperations' oc. Patch info: Adds new API: ipa host-add-retrieve-keytab HOSTNAME --users=STR --groups=STR ipa host-add-write-keytab HOSTNAME --users=STR --groups=STR ipa host-remove-retrieve-keytab HOSTNAME --users=STR --groups=STR ipa host-remove-write-keytab HOSTNAME --users=STR --groups=STR ipa service-add-retrieve-keytab PRINCIPAL --users=STR --groups=STR ipa service-add-write-keytab PRINCIPAL --users=STR --groups=STR ipa service-remove-retrieve-keytab PRINCIPAL --users=STR --groups=STR ipa service-remove-write-keytab PRINCIPAL --users=STR --groups=STR *-write-keytab commands were changed to *-create-keytab to be consistent with descriptions these methods add or remove user or group DNs in `ipaallowedtoperform` attr with `read_keys` and `write_keys` subtypes. service|host-mod|show outputs these attrs only with --all option as: --all is no longer required Users allowed to retrieve keytab: user1 Groups allowed to retrieve keytab: group1 Users allowed to write keytab: user1 Groups allowed to write keytab: group1 1) This patch implements subtypes support for attributes members. It's done to be relatively reusable but it's confined within the RFE boundaries. I.e. it does not contain support for standard attributes or is not integrated into LDAPAddMember or LDAPRemoveMember commands. It's rather as separate opt-ins. One of the reasons was also not to disturb existing code at the end of 4-1 milestone. Was replaced by more specific methods more local to a service and a host plugins. 3) Adding of object class is implemented as a reusable method since this code is used on many places and most likely will be also used in new features. Older code may be refactored later. Thanks RPC tests added in patch #763. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 761 keytab manipulation permission management
New revision according to Honza's recommendations. Comments inline. On 1.10.2014 18:15, Petr Vobornik wrote: Hello list, Patch for: https://fedorahosted.org/freeipa/ticket/4419 Before I start any work on Web UI and tests I would like to gather feedback on: - the new API - member attributes with subtypes management approach - ACI I did not do any ACI work in the patch yet. I assume that we would like to add the attr into 'System: Read Host|Service' permission. But I think that write right should have it's own permission. I have added 2 new permissions. Simo, are they OK? for services: 'System: Manage Service Keytab Permissions': { 'ipapermright': {'read', 'search', 'compare', 'write'}, 'ipapermdefaultattr': {'ipaallowedtoperform', 'objectclass'}, 'default_privileges': {'Service Administrators', 'Host Administrators'}, }, for hosts: 'System: Manage Host Keytab Permissions': { 'ipapermright': {'read', 'search', 'compare', 'write'}, 'ipapermdefaultattr': {'ipaallowedtoperform', 'objectclass'}, 'default_privileges': {'Host Administrators'}, }, I'm not sure about the write right for 'objectclass' but it's required in order to add 'ipaallowedoperations' oc. Patch info: Adds new API: ipa host-add-retrieve-keytab HOSTNAME --users=STR --groups=STR ipa host-add-write-keytab HOSTNAME --users=STR --groups=STR ipa host-remove-retrieve-keytab HOSTNAME --users=STR --groups=STR ipa host-remove-write-keytab HOSTNAME --users=STR --groups=STR ipa service-add-retrieve-keytab PRINCIPAL --users=STR --groups=STR ipa service-add-write-keytab PRINCIPAL --users=STR --groups=STR ipa service-remove-retrieve-keytab PRINCIPAL --users=STR --groups=STR ipa service-remove-write-keytab PRINCIPAL --users=STR --groups=STR *-write-keytab commands were changed to *-create-keytab to be consistent with descriptions these methods add or remove user or group DNs in `ipaallowedtoperform` attr with `read_keys` and `write_keys` subtypes. service|host-mod|show outputs these attrs only with --all option as: --all is no longer required Users allowed to retrieve keytab: user1 Groups allowed to retrieve keytab: group1 Users allowed to write keytab: user1 Groups allowed to write keytab: group1 1) This patch implements subtypes support for attributes members. It's done to be relatively reusable but it's confined within the RFE boundaries. I.e. it does not contain support for standard attributes or is not integrated into LDAPAddMember or LDAPRemoveMember commands. It's rather as separate opt-ins. One of the reasons was also not to disturb existing code at the end of 4-1 milestone. Was replaced by more specific methods more local to a service and a host plugins. 3) Adding of object class is implemented as a reusable method since this code is used on many places and most likely will be also used in new features. Older code may be refactored later. Thanks RPC tests added in patch #763. -- Petr Vobornik From ffc26abbb3f46376369c27c3f6b9dec56ef0c390 Mon Sep 17 00:00:00 2001 From: Petr Vobornik Date: Thu, 2 Oct 2014 16:57:08 +0200 Subject: [PATCH] keytab manipulation permission management Adds new API: ipa host-add-retrieve-keytab HOSTNAME --users=STR --groups=STR ipa host-add-create-keytab HOSTNAME --users=STR --groups=STR ipa host-remove-retrieve-keytab HOSTNAME --users=STR --groups=STR ipa host-remove-create-keytab HOSTNAME --users=STR --groups=STR ipa service-add-retrieve-keytab PRINCIPAL --users=STR --groups=STR ipa service-add-create-keytab PRINCIPAL --users=STR --groups=STR ipa service-remove-retrieve-keytab PRINCIPAL --users=STR --groups=STR ipa service-remove-create-keytab PRINCIPAL --users=STR --groups=STR these methods add or remove user or group DNs in `ipaallowedtoperform` attr with `read_keys` and `write_keys` subtypes. service|host-mod|show outputs these attrs only with --all option as: Users allowed to retrieve keytab: user1 Groups allowed to retrieve keytab: group1 Users allowed to create keytab: user1 Groups allowed to create keytab: group1 Adding of object class is implemented as a reusable method since this code is used on many places and most likely will be also used in new features. Older code may be refactored later. https://fedorahosted.org/freeipa/ticket/4419 --- ACI.txt| 4 ++ API.txt| 96 ++ VERSION| 4 +- ipalib/plugins/baseldap.py | 17 ++ ipalib/plugins/host.py | 107 -- ipalib/plugins/service.py | 125 +++-- 6 files changed, 342 insertions(+), 11 deletions(-) diff --git a/ACI.txt b/ACI.txt index cebdc2ccec45db1dbf0d5ea0c7f2b1a3a7feeb6e..c5510dba009afff583e836704a9e03b78f5e28b5 100644 --- a/ACI.txt +++ b/ACI.txt @@ -95,6 +95,8 @@ aci: (targetattr = "userpassword")(targetfilter = "(objectclass=ipahost)")(versi dn: cn=com
[Freeipa-devel] [PATCH] 761 keytab manipulation permission management
Hello list, Patch for: https://fedorahosted.org/freeipa/ticket/4419 Before I start any work on Web UI and tests I would like to gather feedback on: - the new API - member attributes with subtypes management approach - ACI I did not do any ACI work in the patch yet. I assume that we would like to add the attr into 'System: Read Host|Service' permission. But I think that write right should have it's own permission. Patch info: Adds new API: ipa host-add-retrieve-keytab HOSTNAME --users=STR --groups=STR ipa host-add-write-keytab HOSTNAME --users=STR --groups=STR ipa host-remove-retrieve-keytab HOSTNAME --users=STR --groups=STR ipa host-remove-write-keytab HOSTNAME --users=STR --groups=STR ipa service-add-retrieve-keytab PRINCIPAL --users=STR --groups=STR ipa service-add-write-keytab PRINCIPAL --users=STR --groups=STR ipa service-remove-retrieve-keytab PRINCIPAL --users=STR --groups=STR ipa service-remove-write-keytab PRINCIPAL --users=STR --groups=STR these methods add or remove user or group DNs in `ipaallowedtoperform` attr with `read_keys` and `write_keys` subtypes. service|host-mod|show outputs these attrs only with --all option as: Users allowed to retrieve keytab: user1 Groups allowed to retrieve keytab: group1 Users allowed to write keytab: user1 Groups allowed to write keytab: group1 1) This patch implements subtypes support for attributes members. It's done to be relatively reusable but it's confined within the RFE boundaries. I.e. it does not contain support for standard attributes or is not integrated into LDAPAddMember or LDAPRemoveMember commands. It's rather as separate opt-ins. One of the reasons was also not to disturb existing code at the end of 4-1 milestone. 2) I tried to keep the command names or attr label short, but they are still long like a novel. Any shorter recommendations are welcome. 3) Adding of object class is implemented as a reusable method since this code is used on many places and most likely will be also used in new features. Older code may be refactored later. Thanks -- Petr Vobornik From a39f57373649abd9e9ac16a2ba3ab685ed4d9bad Mon Sep 17 00:00:00 2001 From: Petr Vobornik Date: Wed, 24 Sep 2014 16:21:47 +0200 Subject: [PATCH] keytab manipulation permission management Adds new API: ipa host-add-retrieve-keytab HOSTNAME --users=STR --groups=STR ipa host-add-write-keytab HOSTNAME --users=STR --groups=STR ipa host-remove-retrieve-keytab HOSTNAME --users=STR --groups=STR ipa host-remove-write-keytab HOSTNAME --users=STR --groups=STR ipa service-add-retrieve-keytab PRINCIPAL --users=STR --groups=STR ipa service-add-write-keytab PRINCIPAL --users=STR --groups=STR ipa service-remove-retrieve-keytab PRINCIPAL --users=STR --groups=STR ipa service-remove-write-keytab PRINCIPAL --users=STR --groups=STR these methods add or remove user or group DNs in `ipaallowedtoperform` attr with `read_keys` and `write_keys` subtypes. service|host-mod|show outputs these attrs only with --all option as: Users allowed to retrieve keytab: user1 Groups allowed to retrieve keytab: group1 Users allowed to write keytab: user1 Groups allowed to write keytab: group1 1) This patch implements subtypes support for attributes members. It's done to be relatively reusable but it's confined within the RFE boundaries. I.e. it does not contain support for standard attributes or is not integrated into LDAPAddMember or LDAPRemoveMember commands. It's rather as separate opt-ins. One of the reasons was also not to disturb existing code at the end of 4-1 milestone. 2) I tried to keep the command names or attr label short, but they are still long like a novel. Any shorter recommendations are welcome. 3) Adding of object class is implemented as a reusable method since this code is used on many places and most likely will be also used in new features. Older code may be refactored later. https://fedorahosted.org/freeipa/ticket/4419 --- API.txt| 96 ++ VERSION| 4 +- ipalib/plugins/baseldap.py | 88 ++ ipalib/plugins/host.py | 71 -- ipalib/plugins/service.py | 72 +++--- 5 files changed, 321 insertions(+), 10 deletions(-) diff --git a/API.txt b/API.txt index c5e76c759ec2cb66af2fecd451ca9fa5d1ec9959..54bfc691160709aa27e9bf4b5832e39cdf8d4a8a 100644 --- a/API.txt +++ b/API.txt @@ -1819,6 +1819,30 @@ option: Str('version?', exclude='webui') output: Output('completed', , None) output: Output('failed', , None) output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None)) +command: host_add_retrieve_keytab +args: 1,6,3 +arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=False, primary_key=True, query=True, required=True) +option: Flag('all', autofill=True, cli_name='all', default=False, exclude