[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