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.