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@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.
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.