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@REALM. But for subdomain users the username is already qualified, so so you end up with username@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@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.
>From d46ad94101cd84e377501ba28ee76d9c77faa18c Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@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 | 45 +++++++++++++++++++++++++++++-------- 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(+), 16 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..4ce8351907a02472fed50b438b152736e198a798 100644 --- a/src/providers/krb5/krb5_common.c +++ b/src/providers/krb5/krb5_common.c @@ -884,41 +884,68 @@ 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) { + 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(mem_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(mem_ctx, "%s@%s", name, realm != NULL ? realm : uc_dom); + talloc_free(name); 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 501cdef10b0afc746ada7763a4253d4a0e3988b7..9eb602cfba7a032374ff57db53fd3c3638209153 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
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel