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@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.
...
}
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.
+ 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);
if you call talloc_free(name) here you should also call
talloc_free(domname) after calling sss_parse_name(). And the you do not
need the tmp_ctx.
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);
You used mem_ctx to allocate upn, so not stealing is needed here.
bye,
Sumit
+ 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(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel