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 it it can simply be dropped, everything is working fine without it. > >>> > >>> bye, > >>> Sumit > >>> > >> > >> Patch 0098: > >> > >> If this part does not match (and it will not for all non-nfs service > >> principals): > >> > >> + if (service_type->length == (sep - authz_data_list[c]) && > >> + strncmp(authz_data_list[c], service_type->data, > >> + service_type->length) == 0) { > >> + authz_data_type = sep + 1; > >> + } > >> > >> krb5kdc.log will contain an error: > >> > >> Mar 05 10:48:50 ipa.linux.ad.test krb5kdc[26703](Error): Ignoring > >> unsupported > >> authorization data type [nfs:NONE]. > >> Mar 05 10:48:50 ipa.linux.ad.test krb5kdc[26703](info): TGS_REQ (4 etypes > >> {18 > >> 17 16 23}) 10.16.78.33: ISSUE: authtime 1362482261, etypes {rep=18 tkt=18 > >> ses=18}, HTTP/ipa.linux.ad.t...@linux.ad.test for > >> ldap/ipa.linux.ad.t...@linux.ad.test > >> > >> I think we should just "continue" in this case. > > > > good catch, fixed > > Ok, thanks. > > > > >> > >> Otherwise, this looks and works fine. > > > > Thank you for the review, new version attached. > > > > I have an additional question about processing the service specific > > defaults. Using 'service:NONE' is unambiguous because NONE trumps all. > > But what do we expect if e.g. the defaults are MS-PAC and ldap:PAD for a > > LDAP service ticket. Shall it contain PAC and PAD or only a PAD? I think > > the first one where all defaults which apply to a service are added up > > is more clear, and this is also the way the current code works. But I > > wouldn't mind to change the logic if you think it makes more sense the > > other way round, i.e. if there is a service specific default matching > > the requested service only the service specific default are accounted. > > Hmm, that's a good question. I understand service:PACTYPE as an exclusion to > the general setting, so for me it would make sense to override the global > config - i.e. tje second case. > > Thus, if global config is MS-PAC, ldap:PAD, I think that ldap should have just > PAD. If one also want MS-PAC, it should be set like "MS-PAC, ldap:PAD, > ldap:MS-PAC". Otherwise you would not be able to configure that you want > MS-PAC > for all services and _only_ PAD for ldap... > > Does it make sense? We should also make sure that we update the RFE page to > whatever we decide to do.
+1 to Martin's choice. Simo. -- Simo Sorce * Red Hat, Inc * New York _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
