Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On 03/07/2013 06:32 PM, Sumit Bose wrote: On Wed, Mar 06, 2013 at 05:33:43PM +0100, Sumit Bose wrote: On Wed, Mar 06, 2013 at 08:51:47AM -0500, Simo Sorce wrote: On Wed, 2013-03-06 at 14:49 +0100, Martin Kosek wrote: On 03/06/2013 10:41 AM, Sumit Bose wrote: On Tue, Mar 05, 2013 at 05:13:58PM +0100, Martin Kosek wrote: On 03/04/2013 04:22 PM, Sumit Bose wrote: On Fri, Mar 01, 2013 at 08:58:34AM -0500, Simo Sorce wrote: On Fri, 2013-03-01 at 10:08 +0100, Martin Kosek wrote: On 03/01/2013 09:20 AM, Sumit Bose wrote: On Fri, Mar 01, 2013 at 08:33:51AM +0100, Martin Kosek wrote: On 02/28/2013 03:28 PM, Simo Sorce wrote: On Thu, 2013-02-28 at 13:02 +0100, Martin Kosek wrote: On 02/28/2013 12:42 PM, Sumit Bose wrote: On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote: On 02/27/2013 06:48 PM, Sumit Bose wrote: Hi Sumit, This looks like a good idea and would prevent the magic default PAC type, yes. Though I would not add this service-specific setting to global IPA config object. I would rather like to see that in the service tree, for example as a configuration option of the service root which could be controlled with serviceconfig-* commands (we already have dnsconfig, trustconfig), e.g: # ipa serviceconfig-add-pacmap --service=nfs --pac-type=NONE # ipa serviceconfig-add-pacmap --service=cifs --pac-type=PAD # ipa serviceconfig-show Default PAC Map: nfs:NONE, cifs:PAD Are you thinking of having this in addition to the for-all-services default values in cn=ipaConfig,cn=etc or shall those be dropped? I don't like the first case because then three different objects needs to be consulted to find out which is the right type. This wouldn't be an issue for the plugin, but I think it is hard for the user/admin to follow. Hm, you are right. If the current defaults shall be dropped I think this is a major change because it will require changes in the current CLI and WebUI which will be visible to the users. I'm not against this change, I'm just wondering if it is worth the effort for the next release? Maybe an argument to keep this is in global default is that the settings are used for the host/*.* services as well which are in a different sub-tree of the cn=accounts container. Additionally in future we might want apply those setting to the user TGTs as well? Yeah, that was actually my point. That we are mixing service-specific PAC "rules" to the global setting. Which may be shared with host/*.* principals and user principals. This automatic PAC rules may require some designing so that is is generally usable. I think putting everything in the general config is more understandable and discoverable. These per-service defaults are basically exceptions to the general rule so it make sense to keep everything together. Simo. Ok, if these are really just an exceptions to the general rule (and there will not be too many of them), I think we can leave it in config entry. But if we expect to have exceptions for other types of entries (hosts, users), I think we should rather use something like "service:nfs:NONE" do distinguish this exception. Question is, do we want to implement the interface and processing for that in current Sumit's patches or do we use that is they are? I would like to update the patches so that they can handle the service:TYPE style entry and replace the current update code with just adding nfs:NONE to the global options. I will update the design page accordingly, too. Ok. If the update procedure shrinks just to adding service:nfs:NONE then it'd be great. If we need to distinguish between service principals and user principals I would prefer rather use a special keyword for upns service: is redundant and I do not want here to be able to say upn:martin:NONE because per principal options are available on the principal object. I actually really do not see the need for changing the default just for user principals. If we are worried that one day we might want to really have upn:NONE, then let's use nfs/:NONE, host/:NONE etc... so one day we might add upn:NONE and the lack of / will tell us this is not a service named upn/foo.bar.baz but rather it means user principal names. However I do not see us ever really needing upn:NONE I would prefer if the enhancements needed for the CLI and WebUI can be covered by other/new tickets, but I'm happy to add the needed information to the design page too. bye, Sumit I am OK with adding the interface for this special exception later. In that case, a ticket + note in the design as you mentioned would be enough. Ack. Simo. Please find attached a new version of the patches. 0095 i(updating) is renamed and much simpler now. I opened https://fedorahosted.org/freeipa/ticket/3484 to added the needed change for 'service:TYPE' to CLI and WebUI. For the time being I've added patch 0108 which simply allows 'nfs:NONE' as a type to make sure that it is not deleted accidentally when e.g. using the WebUI. If you do not like
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On Wed, Mar 06, 2013 at 05:33:43PM +0100, Sumit Bose wrote: > On Wed, Mar 06, 2013 at 08:51:47AM -0500, Simo Sorce wrote: > > On Wed, 2013-03-06 at 14:49 +0100, Martin Kosek wrote: > > > On 03/06/2013 10:41 AM, Sumit Bose wrote: > > > > On Tue, Mar 05, 2013 at 05:13:58PM +0100, Martin Kosek wrote: > > > >> On 03/04/2013 04:22 PM, Sumit Bose wrote: > > > >>> On Fri, Mar 01, 2013 at 08:58:34AM -0500, Simo Sorce wrote: > > > On Fri, 2013-03-01 at 10:08 +0100, Martin Kosek wrote: > > > > On 03/01/2013 09:20 AM, Sumit Bose wrote: > > > >> On Fri, Mar 01, 2013 at 08:33:51AM +0100, Martin Kosek wrote: > > > >>> On 02/28/2013 03:28 PM, Simo Sorce wrote: > > > On Thu, 2013-02-28 at 13:02 +0100, Martin Kosek wrote: > > > > On 02/28/2013 12:42 PM, Sumit Bose wrote: > > > >> On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote: > > > >>> On 02/27/2013 06:48 PM, Sumit Bose wrote: > > > > > > >>> Hi Sumit, > > > >>> > > > >>> This looks like a good idea and would prevent the magic > > > >>> default PAC type, yes. > > > >>> Though I would not add this service-specific setting to > > > >>> global IPA config object. > > > >>> > > > >>> I would rather like to see that in the service tree, for > > > >>> example as a > > > >>> configuration option of the service root which could be > > > >>> controlled with > > > >>> serviceconfig-* commands (we already have dnsconfig, > > > >>> trustconfig), e.g: > > > >>> > > > >>> # ipa serviceconfig-add-pacmap --service=nfs --pac-type=NONE > > > >>> # ipa serviceconfig-add-pacmap --service=cifs --pac-type=PAD > > > >>> # ipa serviceconfig-show > > > >>> Default PAC Map: nfs:NONE, cifs:PAD > > > >> > > > >> Are you thinking of having this in addition to the > > > >> for-all-services > > > >> default values in cn=ipaConfig,cn=etc or shall those be > > > >> dropped? I don't > > > >> like the first case because then three different objects needs > > > >> to be > > > >> consulted to find out which is the right type. This wouldn't > > > >> be an issue > > > >> for the plugin, but I think it is hard for the user/admin to > > > >> follow. > > > > > > > > Hm, you are right. > > > > > > > >> > > > >> If the current defaults shall be dropped I think this is a > > > >> major change > > > >> because it will require changes in the current CLI and WebUI > > > >> which will > > > >> be visible to the users. I'm not against this change, I'm just > > > >> wondering > > > >> if it is worth the effort for the next release? > > > >> > > > >> Maybe an argument to keep this is in global default is that > > > >> the settings > > > >> are used for the host/*.* services as well which are in a > > > >> different > > > >> sub-tree of the cn=accounts container. Additionally in future > > > >> we might > > > >> want apply those setting to the user TGTs as well? > > > > > > > > Yeah, that was actually my point. That we are mixing > > > > service-specific PAC > > > > "rules" to the global setting. Which may be shared with > > > > host/*.* principals and > > > > user principals. This automatic PAC rules may require some > > > > designing so that is > > > > is generally usable. > > > > > > I think putting everything in the general config is more > > > understandable > > > and discoverable. These per-service defaults are basically > > > exceptions to > > > the general rule so it make sense to keep everything together. > > > > > > Simo. > > > > > > >>> > > > >>> Ok, if these are really just an exceptions to the general rule > > > >>> (and there will > > > >>> not be too many of them), I think we can leave it in config > > > >>> entry. But if we > > > >>> expect to have exceptions for other types of entries (hosts, > > > >>> users), I think we > > > >>> should rather use something like "service:nfs:NONE" do > > > >>> distinguish this exception. > > > >>> > > > >>> Question is, do we want to implement the interface and processing > > > >>> for that in > > > >>> current Sumit's patches or do we use that is they are? > > > >> > > > >> I would like to update the patches so that they can handle the > > > >> service:TYPE style entry and replace the current update code with > > > >> just > > > >> adding nfs:NONE to the global options. I will update the design > > > >> page > > > >> accordingly, too. > > > > > > > > Ok. If the updat
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On Wed, Mar 06, 2013 at 08:51:47AM -0500, Simo Sorce wrote: > On Wed, 2013-03-06 at 14:49 +0100, Martin Kosek wrote: > > On 03/06/2013 10:41 AM, Sumit Bose wrote: > > > On Tue, Mar 05, 2013 at 05:13:58PM +0100, Martin Kosek wrote: > > >> On 03/04/2013 04:22 PM, Sumit Bose wrote: > > >>> On Fri, Mar 01, 2013 at 08:58:34AM -0500, Simo Sorce wrote: > > On Fri, 2013-03-01 at 10:08 +0100, Martin Kosek wrote: > > > On 03/01/2013 09:20 AM, Sumit Bose wrote: > > >> On Fri, Mar 01, 2013 at 08:33:51AM +0100, Martin Kosek wrote: > > >>> On 02/28/2013 03:28 PM, Simo Sorce wrote: > > On Thu, 2013-02-28 at 13:02 +0100, Martin Kosek wrote: > > > On 02/28/2013 12:42 PM, Sumit Bose wrote: > > >> On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote: > > >>> On 02/27/2013 06:48 PM, Sumit Bose wrote: > > > > >>> Hi Sumit, > > >>> > > >>> This looks like a good idea and would prevent the magic default > > >>> PAC type, yes. > > >>> Though I would not add this service-specific setting to global > > >>> IPA config object. > > >>> > > >>> I would rather like to see that in the service tree, for > > >>> example as a > > >>> configuration option of the service root which could be > > >>> controlled with > > >>> serviceconfig-* commands (we already have dnsconfig, > > >>> trustconfig), e.g: > > >>> > > >>> # ipa serviceconfig-add-pacmap --service=nfs --pac-type=NONE > > >>> # ipa serviceconfig-add-pacmap --service=cifs --pac-type=PAD > > >>> # ipa serviceconfig-show > > >>> Default PAC Map: nfs:NONE, cifs:PAD > > >> > > >> Are you thinking of having this in addition to the > > >> for-all-services > > >> default values in cn=ipaConfig,cn=etc or shall those be dropped? > > >> I don't > > >> like the first case because then three different objects needs > > >> to be > > >> consulted to find out which is the right type. This wouldn't be > > >> an issue > > >> for the plugin, but I think it is hard for the user/admin to > > >> follow. > > > > > > Hm, you are right. > > > > > >> > > >> If the current defaults shall be dropped I think this is a major > > >> change > > >> because it will require changes in the current CLI and WebUI > > >> which will > > >> be visible to the users. I'm not against this change, I'm just > > >> wondering > > >> if it is worth the effort for the next release? > > >> > > >> Maybe an argument to keep this is in global default is that the > > >> settings > > >> are used for the host/*.* services as well which are in a > > >> different > > >> sub-tree of the cn=accounts container. Additionally in future we > > >> might > > >> want apply those setting to the user TGTs as well? > > > > > > Yeah, that was actually my point. That we are mixing > > > service-specific PAC > > > "rules" to the global setting. Which may be shared with host/*.* > > > principals and > > > user principals. This automatic PAC rules may require some > > > designing so that is > > > is generally usable. > > > > I think putting everything in the general config is more > > understandable > > and discoverable. These per-service defaults are basically > > exceptions to > > the general rule so it make sense to keep everything together. > > > > Simo. > > > > >>> > > >>> Ok, if these are really just an exceptions to the general rule (and > > >>> there will > > >>> not be too many of them), I think we can leave it in config entry. > > >>> But if we > > >>> expect to have exceptions for other types of entries (hosts, > > >>> users), I think we > > >>> should rather use something like "service:nfs:NONE" do distinguish > > >>> this exception. > > >>> > > >>> Question is, do we want to implement the interface and processing > > >>> for that in > > >>> current Sumit's patches or do we use that is they are? > > >> > > >> I would like to update the patches so that they can handle the > > >> service:TYPE style entry and replace the current update code with > > >> just > > >> adding nfs:NONE to the global options. I will update the design page > > >> accordingly, too. > > > > > > Ok. If the update procedure shrinks just to adding service:nfs:NONE > > > then it'd > > > be great. > > > > If we need to distinguish between service principals and user > > principals > > I would prefer rather use a special keyword for upns > > > > se
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On Wed, 2013-03-06 at 14:49 +0100, Martin Kosek wrote: > On 03/06/2013 10:41 AM, Sumit Bose wrote: > > On Tue, Mar 05, 2013 at 05:13:58PM +0100, Martin Kosek wrote: > >> On 03/04/2013 04:22 PM, Sumit Bose wrote: > >>> On Fri, Mar 01, 2013 at 08:58:34AM -0500, Simo Sorce wrote: > On Fri, 2013-03-01 at 10:08 +0100, Martin Kosek wrote: > > On 03/01/2013 09:20 AM, Sumit Bose wrote: > >> On Fri, Mar 01, 2013 at 08:33:51AM +0100, Martin Kosek wrote: > >>> On 02/28/2013 03:28 PM, Simo Sorce wrote: > On Thu, 2013-02-28 at 13:02 +0100, Martin Kosek wrote: > > On 02/28/2013 12:42 PM, Sumit Bose wrote: > >> On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote: > >>> On 02/27/2013 06:48 PM, Sumit Bose wrote: > > >>> Hi Sumit, > >>> > >>> This looks like a good idea and would prevent the magic default > >>> PAC type, yes. > >>> Though I would not add this service-specific setting to global > >>> IPA config object. > >>> > >>> I would rather like to see that in the service tree, for example > >>> as a > >>> configuration option of the service root which could be > >>> controlled with > >>> serviceconfig-* commands (we already have dnsconfig, > >>> trustconfig), e.g: > >>> > >>> # ipa serviceconfig-add-pacmap --service=nfs --pac-type=NONE > >>> # ipa serviceconfig-add-pacmap --service=cifs --pac-type=PAD > >>> # ipa serviceconfig-show > >>> Default PAC Map: nfs:NONE, cifs:PAD > >> > >> Are you thinking of having this in addition to the for-all-services > >> default values in cn=ipaConfig,cn=etc or shall those be dropped? I > >> don't > >> like the first case because then three different objects needs to > >> be > >> consulted to find out which is the right type. This wouldn't be an > >> issue > >> for the plugin, but I think it is hard for the user/admin to > >> follow. > > > > Hm, you are right. > > > >> > >> If the current defaults shall be dropped I think this is a major > >> change > >> because it will require changes in the current CLI and WebUI which > >> will > >> be visible to the users. I'm not against this change, I'm just > >> wondering > >> if it is worth the effort for the next release? > >> > >> Maybe an argument to keep this is in global default is that the > >> settings > >> are used for the host/*.* services as well which are in a different > >> sub-tree of the cn=accounts container. Additionally in future we > >> might > >> want apply those setting to the user TGTs as well? > > > > Yeah, that was actually my point. That we are mixing > > service-specific PAC > > "rules" to the global setting. Which may be shared with host/*.* > > principals and > > user principals. This automatic PAC rules may require some > > designing so that is > > is generally usable. > > I think putting everything in the general config is more > understandable > and discoverable. These per-service defaults are basically > exceptions to > the general rule so it make sense to keep everything together. > > Simo. > > >>> > >>> Ok, if these are really just an exceptions to the general rule (and > >>> there will > >>> not be too many of them), I think we can leave it in config entry. > >>> But if we > >>> expect to have exceptions for other types of entries (hosts, users), > >>> I think we > >>> should rather use something like "service:nfs:NONE" do distinguish > >>> this exception. > >>> > >>> Question is, do we want to implement the interface and processing for > >>> that in > >>> current Sumit's patches or do we use that is they are? > >> > >> I would like to update the patches so that they can handle the > >> service:TYPE style entry and replace the current update code with just > >> adding nfs:NONE to the global options. I will update the design page > >> accordingly, too. > > > > Ok. If the update procedure shrinks just to adding service:nfs:NONE > > then it'd > > be great. > > If we need to distinguish between service principals and user principals > I would prefer rather use a special keyword for upns > > service: is redundant and I do not want here to be able to say > upn:martin:NONE because per principal options are available on the > principal object. > > I actually really do not see the need for changing the default just for > user principals. If we are worried that one day we might want to real
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On 03/06/2013 10:41 AM, Sumit Bose wrote: > On Tue, Mar 05, 2013 at 05:13:58PM +0100, Martin Kosek wrote: >> On 03/04/2013 04:22 PM, Sumit Bose wrote: >>> On Fri, Mar 01, 2013 at 08:58:34AM -0500, Simo Sorce wrote: On Fri, 2013-03-01 at 10:08 +0100, Martin Kosek wrote: > On 03/01/2013 09:20 AM, Sumit Bose wrote: >> On Fri, Mar 01, 2013 at 08:33:51AM +0100, Martin Kosek wrote: >>> On 02/28/2013 03:28 PM, Simo Sorce wrote: On Thu, 2013-02-28 at 13:02 +0100, Martin Kosek wrote: > On 02/28/2013 12:42 PM, Sumit Bose wrote: >> On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote: >>> On 02/27/2013 06:48 PM, Sumit Bose wrote: >>> Hi Sumit, >>> >>> This looks like a good idea and would prevent the magic default PAC >>> type, yes. >>> Though I would not add this service-specific setting to global IPA >>> config object. >>> >>> I would rather like to see that in the service tree, for example as >>> a >>> configuration option of the service root which could be controlled >>> with >>> serviceconfig-* commands (we already have dnsconfig, trustconfig), >>> e.g: >>> >>> # ipa serviceconfig-add-pacmap --service=nfs --pac-type=NONE >>> # ipa serviceconfig-add-pacmap --service=cifs --pac-type=PAD >>> # ipa serviceconfig-show >>> Default PAC Map: nfs:NONE, cifs:PAD >> >> Are you thinking of having this in addition to the for-all-services >> default values in cn=ipaConfig,cn=etc or shall those be dropped? I >> don't >> like the first case because then three different objects needs to be >> consulted to find out which is the right type. This wouldn't be an >> issue >> for the plugin, but I think it is hard for the user/admin to follow. > > Hm, you are right. > >> >> If the current defaults shall be dropped I think this is a major >> change >> because it will require changes in the current CLI and WebUI which >> will >> be visible to the users. I'm not against this change, I'm just >> wondering >> if it is worth the effort for the next release? >> >> Maybe an argument to keep this is in global default is that the >> settings >> are used for the host/*.* services as well which are in a different >> sub-tree of the cn=accounts container. Additionally in future we >> might >> want apply those setting to the user TGTs as well? > > Yeah, that was actually my point. That we are mixing service-specific > PAC > "rules" to the global setting. Which may be shared with host/*.* > principals and > user principals. This automatic PAC rules may require some designing > so that is > is generally usable. I think putting everything in the general config is more understandable and discoverable. These per-service defaults are basically exceptions to the general rule so it make sense to keep everything together. Simo. >>> >>> Ok, if these are really just an exceptions to the general rule (and >>> there will >>> not be too many of them), I think we can leave it in config entry. But >>> if we >>> expect to have exceptions for other types of entries (hosts, users), I >>> think we >>> should rather use something like "service:nfs:NONE" do distinguish this >>> exception. >>> >>> Question is, do we want to implement the interface and processing for >>> that in >>> current Sumit's patches or do we use that is they are? >> >> I would like to update the patches so that they can handle the >> service:TYPE style entry and replace the current update code with just >> adding nfs:NONE to the global options. I will update the design page >> accordingly, too. > > Ok. If the update procedure shrinks just to adding service:nfs:NONE then > it'd > be great. If we need to distinguish between service principals and user principals I would prefer rather use a special keyword for upns service: is redundant and I do not want here to be able to say upn:martin:NONE because per principal options are available on the principal object. I actually really do not see the need for changing the default just for user principals. If we are worried that one day we might want to really have upn:NONE, then let's use nfs/:NONE, host/:NONE etc... so one day we might add upn:NONE and the lack of / will tell us this is not a service named upn/foo.bar.baz but rather it means user principal names. However I do not see us ever really needing upn:NONE >> I w
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On Tue, Mar 05, 2013 at 05:13:58PM +0100, Martin Kosek wrote: > On 03/04/2013 04:22 PM, Sumit Bose wrote: > > On Fri, Mar 01, 2013 at 08:58:34AM -0500, Simo Sorce wrote: > >> On Fri, 2013-03-01 at 10:08 +0100, Martin Kosek wrote: > >>> On 03/01/2013 09:20 AM, Sumit Bose wrote: > On Fri, Mar 01, 2013 at 08:33:51AM +0100, Martin Kosek wrote: > > On 02/28/2013 03:28 PM, Simo Sorce wrote: > >> On Thu, 2013-02-28 at 13:02 +0100, Martin Kosek wrote: > >>> On 02/28/2013 12:42 PM, Sumit Bose wrote: > On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote: > > On 02/27/2013 06:48 PM, Sumit Bose wrote: > >> > > Hi Sumit, > > > > This looks like a good idea and would prevent the magic default PAC > > type, yes. > > Though I would not add this service-specific setting to global IPA > > config object. > > > > I would rather like to see that in the service tree, for example as > > a > > configuration option of the service root which could be controlled > > with > > serviceconfig-* commands (we already have dnsconfig, trustconfig), > > e.g: > > > > # ipa serviceconfig-add-pacmap --service=nfs --pac-type=NONE > > # ipa serviceconfig-add-pacmap --service=cifs --pac-type=PAD > > # ipa serviceconfig-show > > Default PAC Map: nfs:NONE, cifs:PAD > > Are you thinking of having this in addition to the for-all-services > default values in cn=ipaConfig,cn=etc or shall those be dropped? I > don't > like the first case because then three different objects needs to be > consulted to find out which is the right type. This wouldn't be an > issue > for the plugin, but I think it is hard for the user/admin to follow. > >>> > >>> Hm, you are right. > >>> > > If the current defaults shall be dropped I think this is a major > change > because it will require changes in the current CLI and WebUI which > will > be visible to the users. I'm not against this change, I'm just > wondering > if it is worth the effort for the next release? > > Maybe an argument to keep this is in global default is that the > settings > are used for the host/*.* services as well which are in a different > sub-tree of the cn=accounts container. Additionally in future we > might > want apply those setting to the user TGTs as well? > >>> > >>> Yeah, that was actually my point. That we are mixing service-specific > >>> PAC > >>> "rules" to the global setting. Which may be shared with host/*.* > >>> principals and > >>> user principals. This automatic PAC rules may require some designing > >>> so that is > >>> is generally usable. > >> > >> I think putting everything in the general config is more understandable > >> and discoverable. These per-service defaults are basically exceptions > >> to > >> the general rule so it make sense to keep everything together. > >> > >> Simo. > >> > > > > Ok, if these are really just an exceptions to the general rule (and > > there will > > not be too many of them), I think we can leave it in config entry. But > > if we > > expect to have exceptions for other types of entries (hosts, users), I > > think we > > should rather use something like "service:nfs:NONE" do distinguish this > > exception. > > > > Question is, do we want to implement the interface and processing for > > that in > > current Sumit's patches or do we use that is they are? > > I would like to update the patches so that they can handle the > service:TYPE style entry and replace the current update code with just > adding nfs:NONE to the global options. I will update the design page > accordingly, too. > >>> > >>> Ok. If the update procedure shrinks just to adding service:nfs:NONE then > >>> it'd > >>> be great. > >> > >> If we need to distinguish between service principals and user principals > >> I would prefer rather use a special keyword for upns > >> > >> service: is redundant and I do not want here to be able to say > >> upn:martin:NONE because per principal options are available on the > >> principal object. > >> > >> I actually really do not see the need for changing the default just for > >> user principals. If we are worried that one day we might want to really > >> have upn:NONE, then let's use nfs/:NONE, host/:NONE etc... so one day we > >> might add upn:NONE and the lack of / will tell us this is not a service > >> named upn/foo.bar.baz but rather it means user principal names. > >> > >> However I do not see us ever really needing upn:NONE > >> > I would prefer if the enhancements needed for th
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On 03/04/2013 04:22 PM, Sumit Bose wrote: > On Fri, Mar 01, 2013 at 08:58:34AM -0500, Simo Sorce wrote: >> On Fri, 2013-03-01 at 10:08 +0100, Martin Kosek wrote: >>> On 03/01/2013 09:20 AM, Sumit Bose wrote: On Fri, Mar 01, 2013 at 08:33:51AM +0100, Martin Kosek wrote: > On 02/28/2013 03:28 PM, Simo Sorce wrote: >> On Thu, 2013-02-28 at 13:02 +0100, Martin Kosek wrote: >>> On 02/28/2013 12:42 PM, Sumit Bose wrote: On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote: > On 02/27/2013 06:48 PM, Sumit Bose wrote: >> > Hi Sumit, > > This looks like a good idea and would prevent the magic default PAC > type, yes. > Though I would not add this service-specific setting to global IPA > config object. > > I would rather like to see that in the service tree, for example as a > configuration option of the service root which could be controlled > with > serviceconfig-* commands (we already have dnsconfig, trustconfig), > e.g: > > # ipa serviceconfig-add-pacmap --service=nfs --pac-type=NONE > # ipa serviceconfig-add-pacmap --service=cifs --pac-type=PAD > # ipa serviceconfig-show > Default PAC Map: nfs:NONE, cifs:PAD Are you thinking of having this in addition to the for-all-services default values in cn=ipaConfig,cn=etc or shall those be dropped? I don't like the first case because then three different objects needs to be consulted to find out which is the right type. This wouldn't be an issue for the plugin, but I think it is hard for the user/admin to follow. >>> >>> Hm, you are right. >>> If the current defaults shall be dropped I think this is a major change because it will require changes in the current CLI and WebUI which will be visible to the users. I'm not against this change, I'm just wondering if it is worth the effort for the next release? Maybe an argument to keep this is in global default is that the settings are used for the host/*.* services as well which are in a different sub-tree of the cn=accounts container. Additionally in future we might want apply those setting to the user TGTs as well? >>> >>> Yeah, that was actually my point. That we are mixing service-specific >>> PAC >>> "rules" to the global setting. Which may be shared with host/*.* >>> principals and >>> user principals. This automatic PAC rules may require some designing so >>> that is >>> is generally usable. >> >> I think putting everything in the general config is more understandable >> and discoverable. These per-service defaults are basically exceptions to >> the general rule so it make sense to keep everything together. >> >> Simo. >> > > Ok, if these are really just an exceptions to the general rule (and there > will > not be too many of them), I think we can leave it in config entry. But if > we > expect to have exceptions for other types of entries (hosts, users), I > think we > should rather use something like "service:nfs:NONE" do distinguish this > exception. > > Question is, do we want to implement the interface and processing for > that in > current Sumit's patches or do we use that is they are? I would like to update the patches so that they can handle the service:TYPE style entry and replace the current update code with just adding nfs:NONE to the global options. I will update the design page accordingly, too. >>> >>> Ok. If the update procedure shrinks just to adding service:nfs:NONE then >>> it'd >>> be great. >> >> If we need to distinguish between service principals and user principals >> I would prefer rather use a special keyword for upns >> >> service: is redundant and I do not want here to be able to say >> upn:martin:NONE because per principal options are available on the >> principal object. >> >> I actually really do not see the need for changing the default just for >> user principals. If we are worried that one day we might want to really >> have upn:NONE, then let's use nfs/:NONE, host/:NONE etc... so one day we >> might add upn:NONE and the lack of / will tell us this is not a service >> named upn/foo.bar.baz but rather it means user principal names. >> >> However I do not see us ever really needing upn:NONE >> I would prefer if the enhancements needed for the CLI and WebUI can be covered by other/new tickets, but I'm happy to add the needed information to the design page too. bye, Sumit >>> >>> I am OK with adding the interface for this special exception later. In that >>> case, a ticket + note in the design as you mentioned would be enough. >> >> Ack. >> >>
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On Fri, Mar 01, 2013 at 08:58:34AM -0500, Simo Sorce wrote: > On Fri, 2013-03-01 at 10:08 +0100, Martin Kosek wrote: > > On 03/01/2013 09:20 AM, Sumit Bose wrote: > > > On Fri, Mar 01, 2013 at 08:33:51AM +0100, Martin Kosek wrote: > > >> On 02/28/2013 03:28 PM, Simo Sorce wrote: > > >>> On Thu, 2013-02-28 at 13:02 +0100, Martin Kosek wrote: > > On 02/28/2013 12:42 PM, Sumit Bose wrote: > > > On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote: > > >> On 02/27/2013 06:48 PM, Sumit Bose wrote: > > >>> > > >> Hi Sumit, > > >> > > >> This looks like a good idea and would prevent the magic default PAC > > >> type, yes. > > >> Though I would not add this service-specific setting to global IPA > > >> config object. > > >> > > >> I would rather like to see that in the service tree, for example as a > > >> configuration option of the service root which could be controlled > > >> with > > >> serviceconfig-* commands (we already have dnsconfig, trustconfig), > > >> e.g: > > >> > > >> # ipa serviceconfig-add-pacmap --service=nfs --pac-type=NONE > > >> # ipa serviceconfig-add-pacmap --service=cifs --pac-type=PAD > > >> # ipa serviceconfig-show > > >> Default PAC Map: nfs:NONE, cifs:PAD > > > > > > Are you thinking of having this in addition to the for-all-services > > > default values in cn=ipaConfig,cn=etc or shall those be dropped? I > > > don't > > > like the first case because then three different objects needs to be > > > consulted to find out which is the right type. This wouldn't be an > > > issue > > > for the plugin, but I think it is hard for the user/admin to follow. > > > > Hm, you are right. > > > > > > > > If the current defaults shall be dropped I think this is a major > > > change > > > because it will require changes in the current CLI and WebUI which > > > will > > > be visible to the users. I'm not against this change, I'm just > > > wondering > > > if it is worth the effort for the next release? > > > > > > Maybe an argument to keep this is in global default is that the > > > settings > > > are used for the host/*.* services as well which are in a different > > > sub-tree of the cn=accounts container. Additionally in future we might > > > want apply those setting to the user TGTs as well? > > > > Yeah, that was actually my point. That we are mixing service-specific > > PAC > > "rules" to the global setting. Which may be shared with host/*.* > > principals and > > user principals. This automatic PAC rules may require some designing > > so that is > > is generally usable. > > >>> > > >>> I think putting everything in the general config is more understandable > > >>> and discoverable. These per-service defaults are basically exceptions to > > >>> the general rule so it make sense to keep everything together. > > >>> > > >>> Simo. > > >>> > > >> > > >> Ok, if these are really just an exceptions to the general rule (and > > >> there will > > >> not be too many of them), I think we can leave it in config entry. But > > >> if we > > >> expect to have exceptions for other types of entries (hosts, users), I > > >> think we > > >> should rather use something like "service:nfs:NONE" do distinguish this > > >> exception. > > >> > > >> Question is, do we want to implement the interface and processing for > > >> that in > > >> current Sumit's patches or do we use that is they are? > > > > > > I would like to update the patches so that they can handle the > > > service:TYPE style entry and replace the current update code with just > > > adding nfs:NONE to the global options. I will update the design page > > > accordingly, too. > > > > Ok. If the update procedure shrinks just to adding service:nfs:NONE then > > it'd > > be great. > > If we need to distinguish between service principals and user principals > I would prefer rather use a special keyword for upns > > service: is redundant and I do not want here to be able to say > upn:martin:NONE because per principal options are available on the > principal object. > > I actually really do not see the need for changing the default just for > user principals. If we are worried that one day we might want to really > have upn:NONE, then let's use nfs/:NONE, host/:NONE etc... so one day we > might add upn:NONE and the lack of / will tell us this is not a service > named upn/foo.bar.baz but rather it means user principal names. > > However I do not see us ever really needing upn:NONE > > > > I would prefer if the enhancements needed for the CLI and WebUI can be > > > covered by other/new tickets, but I'm happy to add the needed > > > information to the design page too. > > > > > > bye, > > > Sumit > > > > I am OK with adding the interface for this special exception later. In that > > case, a ticket + note in the d
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On Fri, 2013-03-01 at 10:08 +0100, Martin Kosek wrote: > On 03/01/2013 09:20 AM, Sumit Bose wrote: > > On Fri, Mar 01, 2013 at 08:33:51AM +0100, Martin Kosek wrote: > >> On 02/28/2013 03:28 PM, Simo Sorce wrote: > >>> On Thu, 2013-02-28 at 13:02 +0100, Martin Kosek wrote: > On 02/28/2013 12:42 PM, Sumit Bose wrote: > > On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote: > >> On 02/27/2013 06:48 PM, Sumit Bose wrote: > >>> > >> Hi Sumit, > >> > >> This looks like a good idea and would prevent the magic default PAC > >> type, yes. > >> Though I would not add this service-specific setting to global IPA > >> config object. > >> > >> I would rather like to see that in the service tree, for example as a > >> configuration option of the service root which could be controlled with > >> serviceconfig-* commands (we already have dnsconfig, trustconfig), e.g: > >> > >> # ipa serviceconfig-add-pacmap --service=nfs --pac-type=NONE > >> # ipa serviceconfig-add-pacmap --service=cifs --pac-type=PAD > >> # ipa serviceconfig-show > >> Default PAC Map: nfs:NONE, cifs:PAD > > > > Are you thinking of having this in addition to the for-all-services > > default values in cn=ipaConfig,cn=etc or shall those be dropped? I don't > > like the first case because then three different objects needs to be > > consulted to find out which is the right type. This wouldn't be an issue > > for the plugin, but I think it is hard for the user/admin to follow. > > Hm, you are right. > > > > > If the current defaults shall be dropped I think this is a major change > > because it will require changes in the current CLI and WebUI which will > > be visible to the users. I'm not against this change, I'm just wondering > > if it is worth the effort for the next release? > > > > Maybe an argument to keep this is in global default is that the settings > > are used for the host/*.* services as well which are in a different > > sub-tree of the cn=accounts container. Additionally in future we might > > want apply those setting to the user TGTs as well? > > Yeah, that was actually my point. That we are mixing service-specific PAC > "rules" to the global setting. Which may be shared with host/*.* > principals and > user principals. This automatic PAC rules may require some designing so > that is > is generally usable. > >>> > >>> I think putting everything in the general config is more understandable > >>> and discoverable. These per-service defaults are basically exceptions to > >>> the general rule so it make sense to keep everything together. > >>> > >>> Simo. > >>> > >> > >> Ok, if these are really just an exceptions to the general rule (and there > >> will > >> not be too many of them), I think we can leave it in config entry. But if > >> we > >> expect to have exceptions for other types of entries (hosts, users), I > >> think we > >> should rather use something like "service:nfs:NONE" do distinguish this > >> exception. > >> > >> Question is, do we want to implement the interface and processing for that > >> in > >> current Sumit's patches or do we use that is they are? > > > > I would like to update the patches so that they can handle the > > service:TYPE style entry and replace the current update code with just > > adding nfs:NONE to the global options. I will update the design page > > accordingly, too. > > Ok. If the update procedure shrinks just to adding service:nfs:NONE then it'd > be great. If we need to distinguish between service principals and user principals I would prefer rather use a special keyword for upns service: is redundant and I do not want here to be able to say upn:martin:NONE because per principal options are available on the principal object. I actually really do not see the need for changing the default just for user principals. If we are worried that one day we might want to really have upn:NONE, then let's use nfs/:NONE, host/:NONE etc... so one day we might add upn:NONE and the lack of / will tell us this is not a service named upn/foo.bar.baz but rather it means user principal names. However I do not see us ever really needing upn:NONE > > I would prefer if the enhancements needed for the CLI and WebUI can be > > covered by other/new tickets, but I'm happy to add the needed > > information to the design page too. > > > > bye, > > Sumit > > I am OK with adding the interface for this special exception later. In that > case, a ticket + note in the design as you mentioned would be enough. Ack. 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] [PATCHES] 94-99 Read and use per-service PAC type
On 03/01/2013 09:20 AM, Sumit Bose wrote: > On Fri, Mar 01, 2013 at 08:33:51AM +0100, Martin Kosek wrote: >> On 02/28/2013 03:28 PM, Simo Sorce wrote: >>> On Thu, 2013-02-28 at 13:02 +0100, Martin Kosek wrote: On 02/28/2013 12:42 PM, Sumit Bose wrote: > On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote: >> On 02/27/2013 06:48 PM, Sumit Bose wrote: >>> >> Hi Sumit, >> >> This looks like a good idea and would prevent the magic default PAC >> type, yes. >> Though I would not add this service-specific setting to global IPA >> config object. >> >> I would rather like to see that in the service tree, for example as a >> configuration option of the service root which could be controlled with >> serviceconfig-* commands (we already have dnsconfig, trustconfig), e.g: >> >> # ipa serviceconfig-add-pacmap --service=nfs --pac-type=NONE >> # ipa serviceconfig-add-pacmap --service=cifs --pac-type=PAD >> # ipa serviceconfig-show >> Default PAC Map: nfs:NONE, cifs:PAD > > Are you thinking of having this in addition to the for-all-services > default values in cn=ipaConfig,cn=etc or shall those be dropped? I don't > like the first case because then three different objects needs to be > consulted to find out which is the right type. This wouldn't be an issue > for the plugin, but I think it is hard for the user/admin to follow. Hm, you are right. > > If the current defaults shall be dropped I think this is a major change > because it will require changes in the current CLI and WebUI which will > be visible to the users. I'm not against this change, I'm just wondering > if it is worth the effort for the next release? > > Maybe an argument to keep this is in global default is that the settings > are used for the host/*.* services as well which are in a different > sub-tree of the cn=accounts container. Additionally in future we might > want apply those setting to the user TGTs as well? Yeah, that was actually my point. That we are mixing service-specific PAC "rules" to the global setting. Which may be shared with host/*.* principals and user principals. This automatic PAC rules may require some designing so that is is generally usable. >>> >>> I think putting everything in the general config is more understandable >>> and discoverable. These per-service defaults are basically exceptions to >>> the general rule so it make sense to keep everything together. >>> >>> Simo. >>> >> >> Ok, if these are really just an exceptions to the general rule (and there >> will >> not be too many of them), I think we can leave it in config entry. But if we >> expect to have exceptions for other types of entries (hosts, users), I think >> we >> should rather use something like "service:nfs:NONE" do distinguish this >> exception. >> >> Question is, do we want to implement the interface and processing for that in >> current Sumit's patches or do we use that is they are? > > I would like to update the patches so that they can handle the > service:TYPE style entry and replace the current update code with just > adding nfs:NONE to the global options. I will update the design page > accordingly, too. Ok. If the update procedure shrinks just to adding service:nfs:NONE then it'd be great. > > I would prefer if the enhancements needed for the CLI and WebUI can be > covered by other/new tickets, but I'm happy to add the needed > information to the design page too. > > bye, > Sumit I am OK with adding the interface for this special exception later. In that case, a ticket + note in the design as you mentioned would be enough. Thanks, Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On Fri, Mar 01, 2013 at 08:33:51AM +0100, Martin Kosek wrote: > On 02/28/2013 03:28 PM, Simo Sorce wrote: > > On Thu, 2013-02-28 at 13:02 +0100, Martin Kosek wrote: > >> On 02/28/2013 12:42 PM, Sumit Bose wrote: > >>> On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote: > On 02/27/2013 06:48 PM, Sumit Bose wrote: > > > Hi Sumit, > > This looks like a good idea and would prevent the magic default PAC > type, yes. > Though I would not add this service-specific setting to global IPA > config object. > > I would rather like to see that in the service tree, for example as a > configuration option of the service root which could be controlled with > serviceconfig-* commands (we already have dnsconfig, trustconfig), e.g: > > # ipa serviceconfig-add-pacmap --service=nfs --pac-type=NONE > # ipa serviceconfig-add-pacmap --service=cifs --pac-type=PAD > # ipa serviceconfig-show > Default PAC Map: nfs:NONE, cifs:PAD > >>> > >>> Are you thinking of having this in addition to the for-all-services > >>> default values in cn=ipaConfig,cn=etc or shall those be dropped? I don't > >>> like the first case because then three different objects needs to be > >>> consulted to find out which is the right type. This wouldn't be an issue > >>> for the plugin, but I think it is hard for the user/admin to follow. > >> > >> Hm, you are right. > >> > >>> > >>> If the current defaults shall be dropped I think this is a major change > >>> because it will require changes in the current CLI and WebUI which will > >>> be visible to the users. I'm not against this change, I'm just wondering > >>> if it is worth the effort for the next release? > >>> > >>> Maybe an argument to keep this is in global default is that the settings > >>> are used for the host/*.* services as well which are in a different > >>> sub-tree of the cn=accounts container. Additionally in future we might > >>> want apply those setting to the user TGTs as well? > >> > >> Yeah, that was actually my point. That we are mixing service-specific PAC > >> "rules" to the global setting. Which may be shared with host/*.* > >> principals and > >> user principals. This automatic PAC rules may require some designing so > >> that is > >> is generally usable. > > > > I think putting everything in the general config is more understandable > > and discoverable. These per-service defaults are basically exceptions to > > the general rule so it make sense to keep everything together. > > > > Simo. > > > > Ok, if these are really just an exceptions to the general rule (and there will > not be too many of them), I think we can leave it in config entry. But if we > expect to have exceptions for other types of entries (hosts, users), I think > we > should rather use something like "service:nfs:NONE" do distinguish this > exception. > > Question is, do we want to implement the interface and processing for that in > current Sumit's patches or do we use that is they are? I would like to update the patches so that they can handle the service:TYPE style entry and replace the current update code with just adding nfs:NONE to the global options. I will update the design page accordingly, too. I would prefer if the enhancements needed for the CLI and WebUI can be covered by other/new tickets, but I'm happy to add the needed information to the design page too. bye, Sumit > > Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On 02/28/2013 03:28 PM, Simo Sorce wrote: > On Thu, 2013-02-28 at 13:02 +0100, Martin Kosek wrote: >> On 02/28/2013 12:42 PM, Sumit Bose wrote: >>> On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote: On 02/27/2013 06:48 PM, Sumit Bose wrote: > Hi Sumit, This looks like a good idea and would prevent the magic default PAC type, yes. Though I would not add this service-specific setting to global IPA config object. I would rather like to see that in the service tree, for example as a configuration option of the service root which could be controlled with serviceconfig-* commands (we already have dnsconfig, trustconfig), e.g: # ipa serviceconfig-add-pacmap --service=nfs --pac-type=NONE # ipa serviceconfig-add-pacmap --service=cifs --pac-type=PAD # ipa serviceconfig-show Default PAC Map: nfs:NONE, cifs:PAD >>> >>> Are you thinking of having this in addition to the for-all-services >>> default values in cn=ipaConfig,cn=etc or shall those be dropped? I don't >>> like the first case because then three different objects needs to be >>> consulted to find out which is the right type. This wouldn't be an issue >>> for the plugin, but I think it is hard for the user/admin to follow. >> >> Hm, you are right. >> >>> >>> If the current defaults shall be dropped I think this is a major change >>> because it will require changes in the current CLI and WebUI which will >>> be visible to the users. I'm not against this change, I'm just wondering >>> if it is worth the effort for the next release? >>> >>> Maybe an argument to keep this is in global default is that the settings >>> are used for the host/*.* services as well which are in a different >>> sub-tree of the cn=accounts container. Additionally in future we might >>> want apply those setting to the user TGTs as well? >> >> Yeah, that was actually my point. That we are mixing service-specific PAC >> "rules" to the global setting. Which may be shared with host/*.* principals >> and >> user principals. This automatic PAC rules may require some designing so that >> is >> is generally usable. > > I think putting everything in the general config is more understandable > and discoverable. These per-service defaults are basically exceptions to > the general rule so it make sense to keep everything together. > > Simo. > Ok, if these are really just an exceptions to the general rule (and there will not be too many of them), I think we can leave it in config entry. But if we expect to have exceptions for other types of entries (hosts, users), I think we should rather use something like "service:nfs:NONE" do distinguish this exception. Question is, do we want to implement the interface and processing for that in current Sumit's patches or do we use that is they are? Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On Thu, 2013-02-28 at 13:02 +0100, Martin Kosek wrote: > On 02/28/2013 12:42 PM, Sumit Bose wrote: > > On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote: > >> On 02/27/2013 06:48 PM, Sumit Bose wrote: > >> Hi Sumit, > >> > >> This looks like a good idea and would prevent the magic default PAC type, > >> yes. > >> Though I would not add this service-specific setting to global IPA config > >> object. > >> > >> I would rather like to see that in the service tree, for example as a > >> configuration option of the service root which could be controlled with > >> serviceconfig-* commands (we already have dnsconfig, trustconfig), e.g: > >> > >> # ipa serviceconfig-add-pacmap --service=nfs --pac-type=NONE > >> # ipa serviceconfig-add-pacmap --service=cifs --pac-type=PAD > >> # ipa serviceconfig-show > >> Default PAC Map: nfs:NONE, cifs:PAD > > > > Are you thinking of having this in addition to the for-all-services > > default values in cn=ipaConfig,cn=etc or shall those be dropped? I don't > > like the first case because then three different objects needs to be > > consulted to find out which is the right type. This wouldn't be an issue > > for the plugin, but I think it is hard for the user/admin to follow. > > Hm, you are right. > > > > > If the current defaults shall be dropped I think this is a major change > > because it will require changes in the current CLI and WebUI which will > > be visible to the users. I'm not against this change, I'm just wondering > > if it is worth the effort for the next release? > > > > Maybe an argument to keep this is in global default is that the settings > > are used for the host/*.* services as well which are in a different > > sub-tree of the cn=accounts container. Additionally in future we might > > want apply those setting to the user TGTs as well? > > Yeah, that was actually my point. That we are mixing service-specific PAC > "rules" to the global setting. Which may be shared with host/*.* principals > and > user principals. This automatic PAC rules may require some designing so that > is > is generally usable. I think putting everything in the general config is more understandable and discoverable. These per-service defaults are basically exceptions to the general rule so it make sense to keep everything together. 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] [PATCHES] 94-99 Read and use per-service PAC type
On 02/28/2013 12:42 PM, Sumit Bose wrote: > On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote: >> On 02/27/2013 06:48 PM, Sumit Bose wrote: >>> On Wed, Feb 27, 2013 at 08:37:18AM -0500, Simo Sorce wrote: On Wed, 2013-02-27 at 11:58 +0100, Sumit Bose wrote: > On Mon, Feb 25, 2013 at 04:35:20PM +0100, Martin Kosek wrote: >> On 02/21/2013 04:24 PM, Sumit Bose wrote: >>> Hi, >>> >>> this series of patches fix https://fedorahosted.org/freeipa/ticket/2960 >>> The related design page is >>> http://freeipa.org/page/V3/Read_and_use_per_service_pac_type . >>> >>> It was agreed while discussing the design page that the currently >>> hardcoded value for the NFS service will be dropped completely, hence >>> the first patch reverts to patch which added the hardcoded value. >>> >>> The second patch adds the needed attribute to compensate the now missing >>> hardcoded value. >>> >>> In the following three patches the PAC type is read and used >>> accordingly. The last patch adds a unit test for a new function. >>> >>> bye, >>> Sumit >> >> I did only sanity testing to the C part of the RFE so far, but by >> reading it it >> looks ok. It may be beneficial if Simo or Alexander checked if the >> ipa-kdb part >> is OK. > > Alexander asked in irc to change strcmp() to strcasecmp() so that > options like 'Ms-Pac' or 'none' are accepted as well. Is there a good reason to allow insensitive matching ? in LDAP you can't use true or false either for booleans, you have to use TRUE and FALSE, so I am not so thrilled to allow insensitive matching for this option as well. >>> >>> I'm fine with this. Alexander, any comments. >>> >> I will comment on the Python parts: >> >> 1) Your update check testing if there is any NFS service with >> ipakrbauthzdata >> looks like a good heuristics to prevent admin frustration. Though an >> ideal >> solution would require >> https://fedorahosted.org/freeipa/ticket/2961 >> to prevent multiple updates when admin purposefully removes this >> attribute. We >> may want to reference the ticket in a comment in the update plugin... > > I added a comment in the code and in #2961. > >> >> >> 2) The upgrade plugin may get into infinite loop if ldap.find_entries >> returns >> truncated result. As you do not update entries directly but only return >> update >> instructions when you have no truncated results, you will loop. >> >> To simulate this, I just created 2 NFS principals and set size_limit=1 >> in your >> find_entries call. > > Thanks for catching this, I removed the truncated logic and added a > messages if truncated was set. > >> >> >> 3) Ok, we set ipakrbauthzdata to NONE on upgrades. But what about new NFS >> service principals added by service-add? Shouldn't we set >> ipakrbauthzdata for >> new services too? As a default when no user ipakrhbauthzdata is set... >> Otherwise admins could be surprised why their new NFS services do not >> work >> while the others do. >> >> Also, I think we should have this NFS culprit documented somewhere (i.e. >> why we >> set ipakrbauthzdata to NONE by default). At least as a small section in >> the >> online help for services plugin... > > I added comments to the help and the doc string in patch #100. If you > think it is necessary to implicitly set PAC type to 'NONE' if NFS > services are added and no type is given explicitly, I would prefer if > you can open a new tickets so that it can be discussed independently. > > Thank you for the review. New versions attached. I do not think we should set the policy to NONE by default, OTOH I see the problem with upgrades. Maybe we should have a way to express additional defaults, per service type. Ie add additional values like: ipakrbAuthsData: nfs:NONE This would be a default but only for services that have the form nfs/*@* This would allow us to avoid magical special casing in the code and allow admins to change the overall default for specific services as needed. Maybe we should add a new ticket for this ? >>> >>> This sounds like a good compromise and will make updating much easier. >>> If we agree to do it this way I can add ipakrbAuthsData: nfs:NONE and >>> fix the code with this ticket. But we would need tickets for the CLI and >>> WebUI to handle this new case. >>> >>> For the WebUI there already is >>> https://fedorahosted.org/freeipa/ticket/3404 , maybe it can be extended >>> to handle this new case as well? >>> >>> As for design documents I think I can added the needed information to >>> http://freeipa.org/page/V3/Read_and_use_per_service_pac_type . >>> >>> bye, >>
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote: > On 02/27/2013 06:48 PM, Sumit Bose wrote: > > On Wed, Feb 27, 2013 at 08:37:18AM -0500, Simo Sorce wrote: > >> On Wed, 2013-02-27 at 11:58 +0100, Sumit Bose wrote: > >>> On Mon, Feb 25, 2013 at 04:35:20PM +0100, Martin Kosek wrote: > On 02/21/2013 04:24 PM, Sumit Bose wrote: > > Hi, > > > > this series of patches fix https://fedorahosted.org/freeipa/ticket/2960 > > The related design page is > > http://freeipa.org/page/V3/Read_and_use_per_service_pac_type . > > > > It was agreed while discussing the design page that the currently > > hardcoded value for the NFS service will be dropped completely, hence > > the first patch reverts to patch which added the hardcoded value. > > > > The second patch adds the needed attribute to compensate the now missing > > hardcoded value. > > > > In the following three patches the PAC type is read and used > > accordingly. The last patch adds a unit test for a new function. > > > > bye, > > Sumit > > I did only sanity testing to the C part of the RFE so far, but by > reading it it > looks ok. It may be beneficial if Simo or Alexander checked if the > ipa-kdb part > is OK. > >>> > >>> Alexander asked in irc to change strcmp() to strcasecmp() so that > >>> options like 'Ms-Pac' or 'none' are accepted as well. > >> > >> Is there a good reason to allow insensitive matching ? > >> > >> in LDAP you can't use true or false either for booleans, you have to use > >> TRUE and FALSE, so I am not so thrilled to allow insensitive matching > >> for this option as well. > > > > I'm fine with this. Alexander, any comments. > > > >> > I will comment on the Python parts: > > 1) Your update check testing if there is any NFS service with > ipakrbauthzdata > looks like a good heuristics to prevent admin frustration. Though an > ideal > solution would require > https://fedorahosted.org/freeipa/ticket/2961 > to prevent multiple updates when admin purposefully removes this > attribute. We > may want to reference the ticket in a comment in the update plugin... > >>> > >>> I added a comment in the code and in #2961. > >>> > > > 2) The upgrade plugin may get into infinite loop if ldap.find_entries > returns > truncated result. As you do not update entries directly but only return > update > instructions when you have no truncated results, you will loop. > > To simulate this, I just created 2 NFS principals and set size_limit=1 > in your > find_entries call. > >>> > >>> Thanks for catching this, I removed the truncated logic and added a > >>> messages if truncated was set. > >>> > > > 3) Ok, we set ipakrbauthzdata to NONE on upgrades. But what about new NFS > service principals added by service-add? Shouldn't we set > ipakrbauthzdata for > new services too? As a default when no user ipakrhbauthzdata is set... > Otherwise admins could be surprised why their new NFS services do not > work > while the others do. > > Also, I think we should have this NFS culprit documented somewhere (i.e. > why we > set ipakrbauthzdata to NONE by default). At least as a small section in > the > online help for services plugin... > >>> > >>> I added comments to the help and the doc string in patch #100. If you > >>> think it is necessary to implicitly set PAC type to 'NONE' if NFS > >>> services are added and no type is given explicitly, I would prefer if > >>> you can open a new tickets so that it can be discussed independently. > >>> > >>> Thank you for the review. New versions attached. > >> > >> I do not think we should set the policy to NONE by default, OTOH I see > >> the problem with upgrades. Maybe we should have a way to express > >> additional defaults, per service type. > >> > >> Ie add additional values like: > >> > >> ipakrbAuthsData: nfs:NONE > >> > >> This would be a default but only for services that have the form nfs/*@* > >> > >> This would allow us to avoid magical special casing in the code and > >> allow admins to change the overall default for specific services as > >> needed. > >> > >> Maybe we should add a new ticket for this ? > > > > This sounds like a good compromise and will make updating much easier. > > If we agree to do it this way I can add ipakrbAuthsData: nfs:NONE and > > fix the code with this ticket. But we would need tickets for the CLI and > > WebUI to handle this new case. > > > > For the WebUI there already is > > https://fedorahosted.org/freeipa/ticket/3404 , maybe it can be extended > > to handle this new case as well? > > > > As for design documents I think I can added the needed information to > > http://freeipa.org/page/V3/Read_and_use_per_service_pac_type . > > > > bye, > > Sumit > > Hi Sumit, > > This looks
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On 02/27/2013 06:48 PM, Sumit Bose wrote: > On Wed, Feb 27, 2013 at 08:37:18AM -0500, Simo Sorce wrote: >> On Wed, 2013-02-27 at 11:58 +0100, Sumit Bose wrote: >>> On Mon, Feb 25, 2013 at 04:35:20PM +0100, Martin Kosek wrote: On 02/21/2013 04:24 PM, Sumit Bose wrote: > Hi, > > this series of patches fix https://fedorahosted.org/freeipa/ticket/2960 > The related design page is > http://freeipa.org/page/V3/Read_and_use_per_service_pac_type . > > It was agreed while discussing the design page that the currently > hardcoded value for the NFS service will be dropped completely, hence > the first patch reverts to patch which added the hardcoded value. > > The second patch adds the needed attribute to compensate the now missing > hardcoded value. > > In the following three patches the PAC type is read and used > accordingly. The last patch adds a unit test for a new function. > > bye, > Sumit I did only sanity testing to the C part of the RFE so far, but by reading it it looks ok. It may be beneficial if Simo or Alexander checked if the ipa-kdb part is OK. >>> >>> Alexander asked in irc to change strcmp() to strcasecmp() so that >>> options like 'Ms-Pac' or 'none' are accepted as well. >> >> Is there a good reason to allow insensitive matching ? >> >> in LDAP you can't use true or false either for booleans, you have to use >> TRUE and FALSE, so I am not so thrilled to allow insensitive matching >> for this option as well. > > I'm fine with this. Alexander, any comments. > >> I will comment on the Python parts: 1) Your update check testing if there is any NFS service with ipakrbauthzdata looks like a good heuristics to prevent admin frustration. Though an ideal solution would require https://fedorahosted.org/freeipa/ticket/2961 to prevent multiple updates when admin purposefully removes this attribute. We may want to reference the ticket in a comment in the update plugin... >>> >>> I added a comment in the code and in #2961. >>> 2) The upgrade plugin may get into infinite loop if ldap.find_entries returns truncated result. As you do not update entries directly but only return update instructions when you have no truncated results, you will loop. To simulate this, I just created 2 NFS principals and set size_limit=1 in your find_entries call. >>> >>> Thanks for catching this, I removed the truncated logic and added a >>> messages if truncated was set. >>> 3) Ok, we set ipakrbauthzdata to NONE on upgrades. But what about new NFS service principals added by service-add? Shouldn't we set ipakrbauthzdata for new services too? As a default when no user ipakrhbauthzdata is set... Otherwise admins could be surprised why their new NFS services do not work while the others do. Also, I think we should have this NFS culprit documented somewhere (i.e. why we set ipakrbauthzdata to NONE by default). At least as a small section in the online help for services plugin... >>> >>> I added comments to the help and the doc string in patch #100. If you >>> think it is necessary to implicitly set PAC type to 'NONE' if NFS >>> services are added and no type is given explicitly, I would prefer if >>> you can open a new tickets so that it can be discussed independently. >>> >>> Thank you for the review. New versions attached. >> >> I do not think we should set the policy to NONE by default, OTOH I see >> the problem with upgrades. Maybe we should have a way to express >> additional defaults, per service type. >> >> Ie add additional values like: >> >> ipakrbAuthsData: nfs:NONE >> >> This would be a default but only for services that have the form nfs/*@* >> >> This would allow us to avoid magical special casing in the code and >> allow admins to change the overall default for specific services as >> needed. >> >> Maybe we should add a new ticket for this ? > > This sounds like a good compromise and will make updating much easier. > If we agree to do it this way I can add ipakrbAuthsData: nfs:NONE and > fix the code with this ticket. But we would need tickets for the CLI and > WebUI to handle this new case. > > For the WebUI there already is > https://fedorahosted.org/freeipa/ticket/3404 , maybe it can be extended > to handle this new case as well? > > As for design documents I think I can added the needed information to > http://freeipa.org/page/V3/Read_and_use_per_service_pac_type . > > bye, > Sumit Hi Sumit, This looks like a good idea and would prevent the magic default PAC type, yes. Though I would not add this service-specific setting to global IPA config object. I would rather like to see that in the service tree, for example as a configuration option of the service root which could be controlled with serviceconfig-* commands (we already have
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On Wed, Feb 27, 2013 at 06:48:27PM +0100, Sumit Bose wrote: > On Wed, Feb 27, 2013 at 08:37:18AM -0500, Simo Sorce wrote: > > On Wed, 2013-02-27 at 11:58 +0100, Sumit Bose wrote: > > > On Mon, Feb 25, 2013 at 04:35:20PM +0100, Martin Kosek wrote: > > > > On 02/21/2013 04:24 PM, Sumit Bose wrote: > > > > > Hi, > > > > > > > > > > this series of patches fix > > > > > https://fedorahosted.org/freeipa/ticket/2960 > > > > > The related design page is > > > > > http://freeipa.org/page/V3/Read_and_use_per_service_pac_type . > > > > > > > > > > It was agreed while discussing the design page that the currently > > > > > hardcoded value for the NFS service will be dropped completely, hence > > > > > the first patch reverts to patch which added the hardcoded value. > > > > > > > > > > The second patch adds the needed attribute to compensate the now > > > > > missing > > > > > hardcoded value. > > > > > > > > > > In the following three patches the PAC type is read and used > > > > > accordingly. The last patch adds a unit test for a new function. > > > > > > > > > > bye, > > > > > Sumit > > > > > > > > I did only sanity testing to the C part of the RFE so far, but by > > > > reading it it > > > > looks ok. It may be beneficial if Simo or Alexander checked if the > > > > ipa-kdb part > > > > is OK. > > > > > > Alexander asked in irc to change strcmp() to strcasecmp() so that > > > options like 'Ms-Pac' or 'none' are accepted as well. > > > > Is there a good reason to allow insensitive matching ? > > > > in LDAP you can't use true or false either for booleans, you have to use > > TRUE and FALSE, so I am not so thrilled to allow insensitive matching > > for this option as well. > > I'm fine with this. Alexander, any comments. Alexander said on irc that he is fine with the strict case-sensitive handling of the values, as long as the user input is automatically upper-cased by the framework as it is currently done for booleans. Since the WebUI does not allow user entered values this will only affect the CLI. bye, Sumit > > > > > > > I will comment on the Python parts: > > > > > > > > 1) Your update check testing if there is any NFS service with > > > > ipakrbauthzdata > > > > looks like a good heuristics to prevent admin frustration. Though an > > > > ideal > > > > solution would require > > > > https://fedorahosted.org/freeipa/ticket/2961 > > > > to prevent multiple updates when admin purposefully removes this > > > > attribute. We > > > > may want to reference the ticket in a comment in the update plugin... > > > > > > I added a comment in the code and in #2961. > > > > > > > > > > > > > > > 2) The upgrade plugin may get into infinite loop if ldap.find_entries > > > > returns > > > > truncated result. As you do not update entries directly but only return > > > > update > > > > instructions when you have no truncated results, you will loop. > > > > > > > > To simulate this, I just created 2 NFS principals and set size_limit=1 > > > > in your > > > > find_entries call. > > > > > > Thanks for catching this, I removed the truncated logic and added a > > > messages if truncated was set. > > > > > > > > > > > > > > > 3) Ok, we set ipakrbauthzdata to NONE on upgrades. But what about new > > > > NFS > > > > service principals added by service-add? Shouldn't we set > > > > ipakrbauthzdata for > > > > new services too? As a default when no user ipakrhbauthzdata is set... > > > > Otherwise admins could be surprised why their new NFS services do not > > > > work > > > > while the others do. > > > > > > > > Also, I think we should have this NFS culprit documented somewhere > > > > (i.e. why we > > > > set ipakrbauthzdata to NONE by default). At least as a small section in > > > > the > > > > online help for services plugin... > > > > > > I added comments to the help and the doc string in patch #100. If you > > > think it is necessary to implicitly set PAC type to 'NONE' if NFS > > > services are added and no type is given explicitly, I would prefer if > > > you can open a new tickets so that it can be discussed independently. > > > > > > Thank you for the review. New versions attached. > > > > I do not think we should set the policy to NONE by default, OTOH I see > > the problem with upgrades. Maybe we should have a way to express > > additional defaults, per service type. > > > > Ie add additional values like: > > > > ipakrbAuthsData: nfs:NONE > > > > This would be a default but only for services that have the form nfs/*@* > > > > This would allow us to avoid magical special casing in the code and > > allow admins to change the overall default for specific services as > > needed. > > > > Maybe we should add a new ticket for this ? > > This sounds like a good compromise and will make updating much easier. > If we agree to do it this way I can add ipakrbAuthsData: nfs:NONE and > fix the code with this ticket. But we would need tickets for the CLI and > WebUI to handle this new case. >
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On Wed, Feb 27, 2013 at 08:37:18AM -0500, Simo Sorce wrote: > On Wed, 2013-02-27 at 11:58 +0100, Sumit Bose wrote: > > On Mon, Feb 25, 2013 at 04:35:20PM +0100, Martin Kosek wrote: > > > On 02/21/2013 04:24 PM, Sumit Bose wrote: > > > > Hi, > > > > > > > > this series of patches fix https://fedorahosted.org/freeipa/ticket/2960 > > > > The related design page is > > > > http://freeipa.org/page/V3/Read_and_use_per_service_pac_type . > > > > > > > > It was agreed while discussing the design page that the currently > > > > hardcoded value for the NFS service will be dropped completely, hence > > > > the first patch reverts to patch which added the hardcoded value. > > > > > > > > The second patch adds the needed attribute to compensate the now missing > > > > hardcoded value. > > > > > > > > In the following three patches the PAC type is read and used > > > > accordingly. The last patch adds a unit test for a new function. > > > > > > > > bye, > > > > Sumit > > > > > > I did only sanity testing to the C part of the RFE so far, but by reading > > > it it > > > looks ok. It may be beneficial if Simo or Alexander checked if the > > > ipa-kdb part > > > is OK. > > > > Alexander asked in irc to change strcmp() to strcasecmp() so that > > options like 'Ms-Pac' or 'none' are accepted as well. > > Is there a good reason to allow insensitive matching ? > > in LDAP you can't use true or false either for booleans, you have to use > TRUE and FALSE, so I am not so thrilled to allow insensitive matching > for this option as well. I'm fine with this. Alexander, any comments. > > > > I will comment on the Python parts: > > > > > > 1) Your update check testing if there is any NFS service with > > > ipakrbauthzdata > > > looks like a good heuristics to prevent admin frustration. Though an ideal > > > solution would require > > > https://fedorahosted.org/freeipa/ticket/2961 > > > to prevent multiple updates when admin purposefully removes this > > > attribute. We > > > may want to reference the ticket in a comment in the update plugin... > > > > I added a comment in the code and in #2961. > > > > > > > > > > > 2) The upgrade plugin may get into infinite loop if ldap.find_entries > > > returns > > > truncated result. As you do not update entries directly but only return > > > update > > > instructions when you have no truncated results, you will loop. > > > > > > To simulate this, I just created 2 NFS principals and set size_limit=1 in > > > your > > > find_entries call. > > > > Thanks for catching this, I removed the truncated logic and added a > > messages if truncated was set. > > > > > > > > > > > 3) Ok, we set ipakrbauthzdata to NONE on upgrades. But what about new NFS > > > service principals added by service-add? Shouldn't we set ipakrbauthzdata > > > for > > > new services too? As a default when no user ipakrhbauthzdata is set... > > > Otherwise admins could be surprised why their new NFS services do not work > > > while the others do. > > > > > > Also, I think we should have this NFS culprit documented somewhere (i.e. > > > why we > > > set ipakrbauthzdata to NONE by default). At least as a small section in > > > the > > > online help for services plugin... > > > > I added comments to the help and the doc string in patch #100. If you > > think it is necessary to implicitly set PAC type to 'NONE' if NFS > > services are added and no type is given explicitly, I would prefer if > > you can open a new tickets so that it can be discussed independently. > > > > Thank you for the review. New versions attached. > > I do not think we should set the policy to NONE by default, OTOH I see > the problem with upgrades. Maybe we should have a way to express > additional defaults, per service type. > > Ie add additional values like: > > ipakrbAuthsData: nfs:NONE > > This would be a default but only for services that have the form nfs/*@* > > This would allow us to avoid magical special casing in the code and > allow admins to change the overall default for specific services as > needed. > > Maybe we should add a new ticket for this ? This sounds like a good compromise and will make updating much easier. If we agree to do it this way I can add ipakrbAuthsData: nfs:NONE and fix the code with this ticket. But we would need tickets for the CLI and WebUI to handle this new case. For the WebUI there already is https://fedorahosted.org/freeipa/ticket/3404 , maybe it can be extended to handle this new case as well? As for design documents I think I can added the needed information to http://freeipa.org/page/V3/Read_and_use_per_service_pac_type . bye, Sumit > > 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] [PATCHES] 94-99 Read and use per-service PAC type
On Wed, 2013-02-27 at 11:58 +0100, Sumit Bose wrote: > On Mon, Feb 25, 2013 at 04:35:20PM +0100, Martin Kosek wrote: > > On 02/21/2013 04:24 PM, Sumit Bose wrote: > > > Hi, > > > > > > this series of patches fix https://fedorahosted.org/freeipa/ticket/2960 > > > The related design page is > > > http://freeipa.org/page/V3/Read_and_use_per_service_pac_type . > > > > > > It was agreed while discussing the design page that the currently > > > hardcoded value for the NFS service will be dropped completely, hence > > > the first patch reverts to patch which added the hardcoded value. > > > > > > The second patch adds the needed attribute to compensate the now missing > > > hardcoded value. > > > > > > In the following three patches the PAC type is read and used > > > accordingly. The last patch adds a unit test for a new function. > > > > > > bye, > > > Sumit > > > > I did only sanity testing to the C part of the RFE so far, but by reading > > it it > > looks ok. It may be beneficial if Simo or Alexander checked if the ipa-kdb > > part > > is OK. > > Alexander asked in irc to change strcmp() to strcasecmp() so that > options like 'Ms-Pac' or 'none' are accepted as well. Is there a good reason to allow insensitive matching ? in LDAP you can't use true or false either for booleans, you have to use TRUE and FALSE, so I am not so thrilled to allow insensitive matching for this option as well. > > I will comment on the Python parts: > > > > 1) Your update check testing if there is any NFS service with > > ipakrbauthzdata > > looks like a good heuristics to prevent admin frustration. Though an ideal > > solution would require > > https://fedorahosted.org/freeipa/ticket/2961 > > to prevent multiple updates when admin purposefully removes this attribute. > > We > > may want to reference the ticket in a comment in the update plugin... > > I added a comment in the code and in #2961. > > > > > > > 2) The upgrade plugin may get into infinite loop if ldap.find_entries > > returns > > truncated result. As you do not update entries directly but only return > > update > > instructions when you have no truncated results, you will loop. > > > > To simulate this, I just created 2 NFS principals and set size_limit=1 in > > your > > find_entries call. > > Thanks for catching this, I removed the truncated logic and added a > messages if truncated was set. > > > > > > > 3) Ok, we set ipakrbauthzdata to NONE on upgrades. But what about new NFS > > service principals added by service-add? Shouldn't we set ipakrbauthzdata > > for > > new services too? As a default when no user ipakrhbauthzdata is set... > > Otherwise admins could be surprised why their new NFS services do not work > > while the others do. > > > > Also, I think we should have this NFS culprit documented somewhere (i.e. > > why we > > set ipakrbauthzdata to NONE by default). At least as a small section in the > > online help for services plugin... > > I added comments to the help and the doc string in patch #100. If you > think it is necessary to implicitly set PAC type to 'NONE' if NFS > services are added and no type is given explicitly, I would prefer if > you can open a new tickets so that it can be discussed independently. > > Thank you for the review. New versions attached. I do not think we should set the policy to NONE by default, OTOH I see the problem with upgrades. Maybe we should have a way to express additional defaults, per service type. Ie add additional values like: ipakrbAuthsData: nfs:NONE This would be a default but only for services that have the form nfs/*@* This would allow us to avoid magical special casing in the code and allow admins to change the overall default for specific services as needed. Maybe we should add a new ticket for this ? 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] [PATCHES] 94-99 Read and use per-service PAC type
On Mon, Feb 25, 2013 at 04:35:20PM +0100, Martin Kosek wrote:
> On 02/21/2013 04:24 PM, Sumit Bose wrote:
> > Hi,
> >
> > this series of patches fix https://fedorahosted.org/freeipa/ticket/2960
> > The related design page is
> > http://freeipa.org/page/V3/Read_and_use_per_service_pac_type .
> >
> > It was agreed while discussing the design page that the currently
> > hardcoded value for the NFS service will be dropped completely, hence
> > the first patch reverts to patch which added the hardcoded value.
> >
> > The second patch adds the needed attribute to compensate the now missing
> > hardcoded value.
> >
> > In the following three patches the PAC type is read and used
> > accordingly. The last patch adds a unit test for a new function.
> >
> > bye,
> > Sumit
>
> I did only sanity testing to the C part of the RFE so far, but by reading it
> it
> looks ok. It may be beneficial if Simo or Alexander checked if the ipa-kdb
> part
> is OK.
Alexander asked in irc to change strcmp() to strcasecmp() so that
options like 'Ms-Pac' or 'none' are accepted as well.
>
> I will comment on the Python parts:
>
> 1) Your update check testing if there is any NFS service with ipakrbauthzdata
> looks like a good heuristics to prevent admin frustration. Though an ideal
> solution would require
> https://fedorahosted.org/freeipa/ticket/2961
> to prevent multiple updates when admin purposefully removes this attribute. We
> may want to reference the ticket in a comment in the update plugin...
I added a comment in the code and in #2961.
>
>
> 2) The upgrade plugin may get into infinite loop if ldap.find_entries returns
> truncated result. As you do not update entries directly but only return update
> instructions when you have no truncated results, you will loop.
>
> To simulate this, I just created 2 NFS principals and set size_limit=1 in your
> find_entries call.
Thanks for catching this, I removed the truncated logic and added a
messages if truncated was set.
>
>
> 3) Ok, we set ipakrbauthzdata to NONE on upgrades. But what about new NFS
> service principals added by service-add? Shouldn't we set ipakrbauthzdata for
> new services too? As a default when no user ipakrhbauthzdata is set...
> Otherwise admins could be surprised why their new NFS services do not work
> while the others do.
>
> Also, I think we should have this NFS culprit documented somewhere (i.e. why
> we
> set ipakrbauthzdata to NONE by default). At least as a small section in the
> online help for services plugin...
I added comments to the help and the doc string in patch #100. If you
think it is necessary to implicitly set PAC type to 'NONE' if NFS
services are added and no type is given explicitly, I would prefer if
you can open a new tickets so that it can be discussed independently.
Thank you for the review. New versions attached.
bye,
Sumit
>
> Martin
From 8eb34dd3190853bd3fa400434be4f7c720bf1354 Mon Sep 17 00:00:00 2001
From: Sumit Bose
Date: Tue, 12 Feb 2013 09:59:00 +0100
Subject: [PATCH 094/100] Revert "MS-PAC: Special case NFS services"
This reverts commit 5269458f552380759c86018cd1f30b64761be92e.
With the implementation of https://fedorahosted.org/freeipa/ticket/2960
a special hardcoded handling of NFS service tickets is not needed
anymore.
---
daemons/ipa-kdb/ipa_kdb_mspac.c | 36 +---
1 file changed, 1 insertion(+), 35 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index
95349702038e1d55bd257816f7f61e9557a5..f05c552fc18eb9b15ee3e39b513184589bc8c468
100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -743,24 +743,6 @@ static bool is_cross_realm_krbtgt(krb5_const_principal
princ)
return true;
}
-static bool is_service_of_type(krb5_const_principal princ, const char *type)
-{
-size_t len;
-
-if (princ->length < 2) {
-return false;
-}
-
-len = strlen(type);
-
-if ((princ->data[0].length == len) ||
-(strncasecmp(princ->data[0].data, type, len) == 0)) {
-return true;
-}
-
-return false;
-}
-
static char *gen_sid_string(TALLOC_CTX *memctx, struct dom_sid *dom_sid,
uint32_t rid)
{
@@ -1555,7 +1537,6 @@ krb5_error_code ipadb_sign_authdata(krb5_context context,
krb5_error_code kerr;
krb5_pac pac = NULL;
krb5_data pac_data;
-bool is_nfs = false;
/* When using s4u2proxy client_princ actually refers to the proxied user
* while client->princ to the proxy service asking for the TGS on behalf
@@ -1566,32 +1547,17 @@ krb5_error_code ipadb_sign_authdata(krb5_context
context,
ks_client_princ = client->princ;
}
-/* NFS Server on Linux is limited and will choke on big tickets.
- * So avoid attachnig the PAC to nfs/ tickets for now.
- * FIXME: remove this when we have interface to support disabling
- * PACs on arbitrary services */
-if (is_service_of_type(ks_clien
Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type
On 02/21/2013 04:24 PM, Sumit Bose wrote: > Hi, > > this series of patches fix https://fedorahosted.org/freeipa/ticket/2960 > The related design page is > http://freeipa.org/page/V3/Read_and_use_per_service_pac_type . > > It was agreed while discussing the design page that the currently > hardcoded value for the NFS service will be dropped completely, hence > the first patch reverts to patch which added the hardcoded value. > > The second patch adds the needed attribute to compensate the now missing > hardcoded value. > > In the following three patches the PAC type is read and used > accordingly. The last patch adds a unit test for a new function. > > bye, > Sumit I did only sanity testing to the C part of the RFE so far, but by reading it it looks ok. It may be beneficial if Simo or Alexander checked if the ipa-kdb part is OK. I will comment on the Python parts: 1) Your update check testing if there is any NFS service with ipakrbauthzdata looks like a good heuristics to prevent admin frustration. Though an ideal solution would require https://fedorahosted.org/freeipa/ticket/2961 to prevent multiple updates when admin purposefully removes this attribute. We may want to reference the ticket in a comment in the update plugin... 2) The upgrade plugin may get into infinite loop if ldap.find_entries returns truncated result. As you do not update entries directly but only return update instructions when you have no truncated results, you will loop. To simulate this, I just created 2 NFS principals and set size_limit=1 in your find_entries call. 3) Ok, we set ipakrbauthzdata to NONE on upgrades. But what about new NFS service principals added by service-add? Shouldn't we set ipakrbauthzdata for new services too? As a default when no user ipakrhbauthzdata is set... Otherwise admins could be surprised why their new NFS services do not work while the others do. Also, I think we should have this NFS culprit documented somewhere (i.e. why we set ipakrbauthzdata to NONE by default). At least as a small section in the online help for services plugin... Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
