[SSSD] [PATCH] KRB5: Do not send PAC in server mode

Pavel Březina pbrezina at redhat.com
Tue Jul 23 08:17:37 UTC 2013


On 07/22/2013 04:11 PM, Jakub Hrozek wrote:
> On Mon, Jul 22, 2013 at 03:27:43PM +0200, Pavel Březina wrote:
>> On 07/22/2013 11:53 AM, Jakub Hrozek wrote:
>>> On Fri, Jul 19, 2013 at 03:04:31PM +0200, Jakub Hrozek wrote:
>>>> On Fri, Jul 19, 2013 at 01:46:02PM +0200, Jakub Hrozek wrote:
>>>>> The krb5 child contacts the PAC responder for any user except for the IPA
>>>>> native users if the PAC is configured. This works fine for the general
>>>>> case but the ipa_server_mode is a special one. The PAC responder is there,
>>>>> but since in the server mode we should be operating as AD provider default,
>>>>> the PAC shouldn't be analyzed either in this case.
>>>>
>>>> I already confused Alexander on IRC, so I should also clarify the path
>>>> -- the PAC is not used only during *password auth*. The PAC responder is
>>>> still up and running and the PAC is used in case of Kerberos auth.
>>>
>>> Sorry, I squashed some fixes into another patch and sent the buggy
>>> version by accident. Attached is a new patch.
>>
>> Nack.
>>
>> The patch works fine, but I have some comments.
>>
>>> @@ -130,7 +136,7 @@ struct krb5_ctx {
>>>
>>>      hash_table_t *wait_queue_hash;
>>>
>>> -    bool is_ipa;
>>> +    enum krb5_config_type config_type;
>>> };
>>>
>>
>> This needs to be set also in AD and KRB5 providers. Even though K5C_GENERIC
>> is 0 so it won't make a difference, it should be set explicitly.
>
> Well, the is_ipa boolean that was present before this patch was not set
> explicitly either and I explicitly set the default value to 0. But I'm
> fine with setting the generic value as default.
>
>>
>>> +    /* Always send PAC except for local IPA users and IPA server mode */
>>> +    switch (kr->krb5_ctx->config_type) {
>>> +        case K5C_IPA_CLIENT:
>>> +            send_pac = kr->upn_from_different_realm ? 1 : 0;
>>> +            break;
>>> +        case K5C_IPA_SERVER:
>>> +            send_pac = 0;
>>> +            break;
>>> +        default:
>>> +            send_pac = 1;
>>> +            break;
>>>      }
>>
>> In general, it is better to list all enum values instead of using default
>> when using switch on enum type. However, I don't really have a strong
>> opinion in this case, so use here whatever you like better.
>
> I like the default better because the flow of the code should be:
>
> if sssd is a client, send PAC for foreign UPNs
> if sssd is a server, never send PAC
> all other configurations always send PAC
>
> I think the "default" statement better aligns with the flow.

OK. Ack.





More information about the sssd-devel mailing list