Re: krb5_aname_to_localname() and krb5_kuserok().

Roland,

krb5_aname_to_localname() calls krb5_kuserok(), this will not cause a recursion ?

You patch don't check return value of krb5_aname_to_localname()

Love


19 nov 2011 kl. 18:06 skrev Roland C. Dowdeswell:

> I just noticed that krb5_kuserok() does not consult krb5_aname_to_lname()
> when making authorisation decisions but rather duplicates some of
> the code.  I attach a simple patch that changes the behaviour and
> also describes what I mean.
> 
> (I noticed this when I added CDB support to krb5_aname_to_lname().)
> 
> When reading the patch, we can note that krb5_aname_to_lname() will
> sometimes call krb5_kuserok() but this does not cause loops in the
> code as presented as the krb5_kuserok() below only calls
> krb5_aname_to_lname() on single component princs whereas
> krb5_aname_to_lname() only cals krb5_kuserok() on two component
> princs where the second component is ``root''.
> 
> I'm not convinced that I like the restriction of krb5_kuserok()
> only matching single component principals against krb5_aname_to_lname()
> and krb5_aname_to_lname() only dealing with single component
> principals as this seems a little arbitrary but I left it intact
> as it solves the abovementioned problem.  In the future, however,
> I think that we should consider relaxing this restriction.
> 
> --
>    Roland Dowdeswell                      http://Imrryr.ORG/~elric/
> 
> diff --git a/lib/krb5/kuserok.c b/lib/krb5/kuserok.c
> index b21ab9b..2b19a4e 100644
> --- a/lib/krb5/kuserok.c
> +++ b/lib/krb5/kuserok.c
> @@ -168,33 +168,25 @@ check_directory(krb5_context context,
> #endif  /* !_WIN32 */
> 
> static krb5_boolean
> -match_local_principals(krb5_context context,
> -		       krb5_principal principal,
> -		       const char *luser)
> +check_an2ln(krb5_context context,
> +	    krb5_principal principal,
> +	    const char *luser)
> {
>     krb5_error_code ret;
>     krb5_realm *realms, *r;
>     krb5_boolean result = FALSE;
> +    char *lname;
> 
>     /* multi-component principals can never match */
>     if(krb5_principal_get_comp_string(context, principal, 1) != NULL)
> 	return FALSE;
> 
> -    ret = krb5_get_default_realms (context, &realms);
> -    if (ret)
> -	return FALSE;
> +    lname = malloc(strlen(luser) + 1);
> +    krb5_aname_to_localname(context, principal, strlen(luser)+1, lname);
> +    if (!strncmp(strlen(luser)+1, luser, lname))
> +	result = TRUE;
> 
> -    for (r = realms; *r != NULL; ++r) {
> -	if(strcmp(krb5_principal_get_realm(context, principal),
> -		  *r) != 0)
> -	    continue;
> -	if(strcmp(krb5_principal_get_comp_string(context, principal, 0),
> -		  luser) == 0) {
> -	    result = TRUE;
> -	    break;
> -	}
> -    }
> -    krb5_free_host_realm (context, realms);
> +    free(lname);
>     return result;
> }
> 
> @@ -291,13 +283,13 @@ krb5_kuserok (krb5_context context,
>     /* finally if no files exist, allow all principals matching
>        <localuser>@<LOCALREALM> */
>     if(found_file == FALSE)
> -	return match_local_principals(context, principal, luser);
> +	return check_an2ln(context, principal, luser);
> 
>     return FALSE;
> #else
>     /* The .k5login file may be on a remote profile and we don't have
>        access to the profile until we have a token handle for the
>        user's credentials. */
> -    return match_local_principals(context, principal, luser);
> +    return check_an2ln(context, principal, luser);
> #endif
> }