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

Tue Mar 5 16:13:58 UTC 2013

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.

Otherwise, this looks and works fine.

Martin