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

Jakub Hrozek jhrozek at redhat.com
Mon Jul 22 14:11:01 UTC 2013


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. 
-------------- next part --------------
>From 83f70c7501a65a008b2ad27f76c0557deb47a011 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 19 Jul 2013 08:36:28 +0300
Subject: [PATCH] KRB5: Do not send PAC in server mode

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.
---
 src/providers/ad/ad_init.c              |  1 +
 src/providers/ipa/ipa_init.c            |  8 +++++++-
 src/providers/krb5/krb5_child_handler.c | 16 +++++++++++-----
 src/providers/krb5/krb5_common.h        |  8 +++++++-
 src/providers/krb5/krb5_init.c          |  1 +
 5 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/src/providers/ad/ad_init.c b/src/providers/ad/ad_init.c
index c5d3fac235dd2473ce98c425adfc9aef0e7b9219..3923076781e71b738c08e3db66489f6a0107317a 100644
--- a/src/providers/ad/ad_init.c
+++ b/src/providers/ad/ad_init.c
@@ -254,6 +254,7 @@ sssm_ad_auth_init(struct be_ctx *bectx,
         goto done;
     }
 
+    krb5_auth_ctx->config_type = K5C_GENERIC;
     krb5_auth_ctx->service = ad_options->service->krb5_service;
 
     ret = ad_get_auth_options(krb5_auth_ctx, ad_options, bectx,
diff --git a/src/providers/ipa/ipa_init.c b/src/providers/ipa/ipa_init.c
index fe13b18722d7a2506ea52850e17a4cf20073bc56..407ab166918c5ff5599382c8281502380aa179fe 100644
--- a/src/providers/ipa/ipa_init.c
+++ b/src/providers/ipa/ipa_init.c
@@ -300,7 +300,13 @@ int sssm_ipa_auth_init(struct be_ctx *bectx,
         goto done;
     }
     krb5_auth_ctx->service = ipa_options->service->krb5_service;
-    krb5_auth_ctx->is_ipa = true;
+
+    if (dp_opt_get_bool(id_ctx->ipa_options->basic,
+                        IPA_SERVER_MODE) == true) {
+        krb5_auth_ctx->config_type = K5C_IPA_SERVER;
+    } else {
+        krb5_auth_ctx->config_type = K5C_IPA_CLIENT;
+    }
     ipa_options->auth_ctx->krb5_auth_ctx = krb5_auth_ctx;
 
     ret = ipa_get_auth_options(ipa_options, bectx->cdb, bectx->conf_path,
diff --git a/src/providers/krb5/krb5_child_handler.c b/src/providers/krb5/krb5_child_handler.c
index 044c27195a08b423cab2459915bf3488ba6ca885..8117d1e5db01cfe0684ee4d7f3e4acf8ff1e4768 100644
--- a/src/providers/krb5/krb5_child_handler.c
+++ b/src/providers/krb5/krb5_child_handler.c
@@ -142,11 +142,17 @@ static errno_t create_send_buffer(struct krb5child_req *kr,
 
     validate = dp_opt_get_bool(kr->krb5_ctx->opts, KRB5_VALIDATE) ? 1 : 0;
 
-    /* Always send PAC except for local IPA users */
-    if (kr->krb5_ctx->is_ipa) {
-        send_pac = kr->upn_from_different_realm ? 1 : 0;
-    } else {
-        send_pac = 1;
+    /* 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;
     }
 
     if (kr->pd->cmd == SSS_CMD_RENEW) {
diff --git a/src/providers/krb5/krb5_common.h b/src/providers/krb5/krb5_common.h
index 27089ab969c9695d0d92c10a540387376e305467..e56bd496e04a7a5c8776a34648aabe0f2d73a3b9 100644
--- a/src/providers/krb5/krb5_common.h
+++ b/src/providers/krb5/krb5_common.h
@@ -91,6 +91,12 @@ struct deferred_auth_ctx;
 struct renew_tgt_ctx;
 struct sss_krb5_cc_be;
 
+enum krb5_config_type {
+    K5C_GENERIC,
+    K5C_IPA_CLIENT,
+    K5C_IPA_SERVER
+};
+
 struct krb5_ctx {
     /* opts taken from kinit */
     /* in seconds */
@@ -130,7 +136,7 @@ struct krb5_ctx {
 
     hash_table_t *wait_queue_hash;
 
-    bool is_ipa;
+    enum krb5_config_type config_type;
 };
 
 struct remove_info_files_ctx {
diff --git a/src/providers/krb5/krb5_init.c b/src/providers/krb5/krb5_init.c
index c6ec496e507e646da06645707fbf93a8b83ad51f..91f701a188ce281b90d7157439faa5a06b99ef47 100644
--- a/src/providers/krb5/krb5_init.c
+++ b/src/providers/krb5/krb5_init.c
@@ -98,6 +98,7 @@ int sssm_krb5_auth_init(struct be_ctx *bectx,
 
     ctx->action = INIT_PW;
     ctx->opts = krb5_options->opts;
+    ctx->config_type = K5C_GENERIC;
 
     krb5_servers = dp_opt_get_string(ctx->opts, KRB5_KDC);
     krb5_backup_servers = dp_opt_get_string(ctx->opts, KRB5_BACKUP_KDC);
-- 
1.8.3.1



More information about the sssd-devel mailing list