On Tue, Jul 23, 2013 at 10:17:37AM +0200, Pavel Březina wrote:
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.
Thanks, pushed to master.