[Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type

Sumit Bose sbose at redhat.com
Wed Mar 6 16:33:43 UTC 2013 

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 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.test at LINUX.AD.TEST for
> > >> ldap/ipa.linux.ad.test at 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.

Martin, Simo, thank you for the feedback. I will send an new version of
the patches, add a section in the design page explaining the details and
add some tests for this logic.

bye,
Sumit

> 
> Simo.
> 
> -- 
> Simo Sorce * Red Hat, Inc * New York
>

More information about the Freeipa-devel mailing list