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

Jakub Hrozek jhrozek at redhat.com
Tue Jul 23 12:23:28 UTC 2013


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.



More information about the sssd-devel mailing list