[SSSD] [PATCH] KRB5: guess UPN for subdomain users

Jakub Hrozek jhrozek at redhat.com
Thu Jun 27 14:37:09 UTC 2013


On Thu, Jun 27, 2013 at 04:00:28PM +0200, Sumit Bose wrote:
> On Thu, Jun 27, 2013 at 01:27:28PM +0200, Jakub Hrozek wrote:
> > Hi,
> > 
> > during testing I found out that we mishandle UPNs for subdomain users
> > when using Kerberos authentication.
> > 
> > If there is no userPrincipal attribute we guess based on username at REALM.
> > But for subdomain users the username is already qualified, so so you end
> > up with username at DOMAIN@REALM. Currently first login works fine because
> > krb5 auth code treats the result as an enterprise principal. But if you
> > are checking existing ccache then the krb5 code errors out because one of
> > the krb5_cc_* functions treats username at DOMAIN@REALM as invalid principal.
> > 
> > The attached patch checks if the username is already qualified and
> > replaces the domain name with realm name when guessing the UPN. I really
> > don't like the result because parsing out is inherently fragile. I think
> > we should store the plain username in an additional sysdb attribute,
> > too.
> 
> ...
> 
> >  }
> >  
> >  errno_t krb5_get_simple_upn(TALLOC_CTX *mem_ctx, struct krb5_ctx *krb5_ctx,
> > -                            const char *domain_name, const char *username,
> > +                            struct sss_domain_info *dom, const char *username,
> >                              const char *user_dom, char **_upn)
> >  {
> >      const char *realm = NULL;
> >      char *uc_dom = NULL;
> >      char *upn;
> > +    char *name;
> > +    char *domname;
> > +    TALLOC_CTX *tmp_ctx = NULL;
> 
> I'm not sure a tmp_ctx is really needed here.

I would prefer tmp_ctx because there's quite a couple of allocations and
using tmp_ctx is simpler than keeping track of individual allocated
strings.

I just forgot to use it in some (most) places in the function. New patch
is attached that allocates on top of tmp_ctx and then only steals the
result.
-------------- next part --------------
>From 43e5e1e4f1d182e6cd9db74326ce78df2bf9ea17 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Thu, 27 Jun 2013 12:02:34 +0200
Subject: [PATCH] KRB5: guess UPN for subdomain users

---
 src/providers/krb5/krb5_access.c    |  2 +-
 src/providers/krb5/krb5_auth.c      |  2 +-
 src/providers/krb5/krb5_common.c    | 47 ++++++++++++++++++++++++++++---------
 src/providers/krb5/krb5_common.h    |  2 +-
 src/providers/krb5/krb5_renew_tgt.c |  2 +-
 src/providers/krb5/krb5_utils.c     |  4 ++--
 src/providers/krb5/krb5_utils.h     |  2 +-
 7 files changed, 43 insertions(+), 18 deletions(-)

diff --git a/src/providers/krb5/krb5_access.c b/src/providers/krb5/krb5_access.c
index c4ee672f80b457a7c70c8bc836a34a14a7f14560..8caed7c69aa0083bee2cc42c9c9ff03def8f4c1b 100644
--- a/src/providers/krb5/krb5_access.c
+++ b/src/providers/krb5/krb5_access.c
@@ -103,7 +103,7 @@ struct tevent_req *krb5_access_send(TALLOC_CTX *mem_ctx,
         break;
     case 1:
         ret = find_or_guess_upn(state, res->msgs[0], krb5_ctx,
-                                be_ctx->domain->name, pd->user, pd->domain,
+                                be_ctx->domain, pd->user, pd->domain,
                                 &state->kr->upn);
         if (ret != EOK) {
             DEBUG(SSSDBG_OP_FAILURE, ("find_or_guess_upn failed.\n"));
diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index dfd22f7a3b88c67fa28d21418da25a20dce2db20..22495f57079d979b6c3484acf916ecc0736292d1 100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -594,7 +594,7 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx,
 
     case 1:
         ret = find_or_guess_upn(state, res->msgs[0], krb5_ctx,
-                                be_ctx->domain->name, pd->user, pd->domain,
+                                be_ctx->domain, pd->user, pd->domain,
                                 &kr->upn);
         if (ret != EOK) {
             DEBUG(SSSDBG_OP_FAILURE, ("find_or_guess_upn failed.\n"));
diff --git a/src/providers/krb5/krb5_common.c b/src/providers/krb5/krb5_common.c
index 9db14b8a668af42cdcab8d889aa795c3ef7702b7..7b4831d7a7fc8bb7d164dc04ee6d887b0e6dc89a 100644
--- a/src/providers/krb5/krb5_common.c
+++ b/src/providers/krb5/krb5_common.c
@@ -884,41 +884,66 @@ errno_t krb5_install_sigterm_handler(struct tevent_context *ev,
 }
 
 errno_t krb5_get_simple_upn(TALLOC_CTX *mem_ctx, struct krb5_ctx *krb5_ctx,
-                            const char *domain_name, const char *username,
+                            struct sss_domain_info *dom, const char *username,
                             const char *user_dom, char **_upn)
 {
     const char *realm = NULL;
     char *uc_dom = NULL;
     char *upn;
+    char *name;
+    char *domname;
+    TALLOC_CTX *tmp_ctx = NULL;
+    errno_t ret;
 
-    if (user_dom != NULL && domain_name != NULL &&
-        strcasecmp(domain_name,user_dom) != 0) {
-        uc_dom = get_uppercase_realm(mem_ctx, user_dom);
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_new failed.\n"));
+        return ENOMEM;
+    }
+
+    if (user_dom != NULL && dom->name != NULL &&
+        strcasecmp(dom->name, user_dom) != 0) {
+        uc_dom = get_uppercase_realm(tmp_ctx, user_dom);
         if (uc_dom == NULL) {
             DEBUG(SSSDBG_OP_FAILURE, ("get_uppercase_realm failed.\n"));
-            return ENOMEM;
+            ret = ENOMEM;
+            goto done;
         }
     } else {
         realm = dp_opt_get_cstring(krb5_ctx->opts, KRB5_REALM);
         if (realm == NULL) {
             DEBUG(SSSDBG_OP_FAILURE, ("Missing Kerberos realm.\n"));
-            return ENOENT;
+            ret = ENOMEM;
+            goto done;
         }
     }
 
+    /* Subdomains already have a fully qualified name, which contains
+     * the domain name. We need to replace it with the realm name
+     */
+    ret = sss_parse_name(tmp_ctx, dom->names, username, &domname, &name);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Could not parse %s into name and " \
+                                    "domain components\n"));
+        goto done;
+    }
+
     /* NOTE: this is a hack, works only in some environments */
-    upn = talloc_asprintf(mem_ctx, "%s@%s",  username,
+    upn = talloc_asprintf(tmp_ctx, "%s@%s",  name,
                                              realm != NULL ? realm : uc_dom);
-    talloc_free(uc_dom);
     if (upn == NULL) {
         DEBUG(1, ("talloc_asprintf failed.\n"));
-        return ENOMEM;
+        ret = ENOMEM;
+        goto done;
     }
 
     DEBUG(9, ("Using simple UPN [%s].\n", upn));
 
-    *_upn = upn;
-    return EOK;
+    *_upn = talloc_steal(mem_ctx, upn);
+    ret = EOK;
+done:
+    talloc_free(tmp_ctx);
+    return ret;
 }
 
 errno_t compare_principal_realm(const char *upn, const char *realm,
diff --git a/src/providers/krb5/krb5_common.h b/src/providers/krb5/krb5_common.h
index ac188b56d0f80b775c8b626ed7d8ce5fe521556b..27089ab969c9695d0d92c10a540387376e305467 100644
--- a/src/providers/krb5/krb5_common.h
+++ b/src/providers/krb5/krb5_common.h
@@ -182,7 +182,7 @@ errno_t write_krb5info_file(const char *realm, const char *kdc,
 errno_t remove_krb5_info_files(TALLOC_CTX *mem_ctx, const char *realm);
 
 errno_t krb5_get_simple_upn(TALLOC_CTX *mem_ctx, struct krb5_ctx *krb5_ctx,
-                            const char *domain_name, const char *username,
+                            struct sss_domain_info *dom, const char *username,
                             const char *user_dom, char **_upn);
 
 errno_t compare_principal_realm(const char *upn, const char *realm,
diff --git a/src/providers/krb5/krb5_renew_tgt.c b/src/providers/krb5/krb5_renew_tgt.c
index 0b1f26fd3a646e29a2f39ae6699714ca5318b66f..d6cdff8f8c0efc98445363c0a12a7ec19d12477f 100644
--- a/src/providers/krb5/krb5_renew_tgt.c
+++ b/src/providers/krb5/krb5_renew_tgt.c
@@ -442,7 +442,7 @@ static errno_t check_ccache_files(struct renew_tgt_ctx *renew_tgt_ctx)
         }
 
         ret = find_or_guess_upn(tmp_ctx, msgs[c], renew_tgt_ctx->krb5_ctx,
-                                renew_tgt_ctx->be_ctx->domain->name,
+                                renew_tgt_ctx->be_ctx->domain,
                                 user_name, user_dom, &upn);
         if (ret != EOK) {
             DEBUG(SSSDBG_OP_FAILURE, ("find_or_guess_upn failed.\n"));
diff --git a/src/providers/krb5/krb5_utils.c b/src/providers/krb5/krb5_utils.c
index 3f16faa7fb238bbb9801029beed66293cd873c15..1f7ed07452d8814e813692f84423c74c1c8ecb65 100644
--- a/src/providers/krb5/krb5_utils.c
+++ b/src/providers/krb5/krb5_utils.c
@@ -32,7 +32,7 @@
 
 errno_t find_or_guess_upn(TALLOC_CTX *mem_ctx, struct ldb_message *msg,
                           struct krb5_ctx *krb5_ctx,
-                          const char *domain_name, const char *user,
+                          struct sss_domain_info *dom, const char *user,
                           const char *user_dom, char **_upn)
 {
     const char *upn;
@@ -40,7 +40,7 @@ errno_t find_or_guess_upn(TALLOC_CTX *mem_ctx, struct ldb_message *msg,
 
     upn = ldb_msg_find_attr_as_string(msg, SYSDB_UPN, NULL);
     if (upn == NULL) {
-        ret = krb5_get_simple_upn(mem_ctx, krb5_ctx, domain_name, user,
+        ret = krb5_get_simple_upn(mem_ctx, krb5_ctx, dom, user,
                                   user_dom, _upn);
         if (ret != EOK) {
             DEBUG(SSSDBG_OP_FAILURE, ("krb5_get_simple_upn failed.\n"));
diff --git a/src/providers/krb5/krb5_utils.h b/src/providers/krb5/krb5_utils.h
index aad2770d4d3beb28c3ccb06278ee4cfc88562c85..2e1bec717ac7c3ac1c9bb87392d011eb854f21be 100644
--- a/src/providers/krb5/krb5_utils.h
+++ b/src/providers/krb5/krb5_utils.h
@@ -34,7 +34,7 @@
 
 errno_t find_or_guess_upn(TALLOC_CTX *mem_ctx, struct ldb_message *msg,
                           struct krb5_ctx *krb5_ctx,
-                          const char *domain_name, const char *user,
+                          struct sss_domain_info *dom, const char *user,
                           const char *user_dom, char **_upn);
 
 errno_t check_if_cached_upn_needs_update(struct sysdb_ctx *sysdb,
-- 
1.8.3.1



More information about the sssd-devel mailing list