Kerberos Ticket without principal?

[Jeremy Allison]

On Fri, Jul 22, 2011 at 07:20:54PM +0200, Christian M Ambach wrote:
> Hi list,
> 
> I have seen a smbd built from v3-6-test crashing today in 
> get_user_from_kerberos_info()
> because princ_name was null and it then tripped over that in the following 
> line:
> strchr_m(princ_name, '@').
> Unfortunately, I couldn't save away a log or core file and I wasn't able 
> to
> recreate it yet :(
> 
> What do you think might have gone wrong here? Did the Windows box send an 
> invalid
> Kerberos ticket or did we fail to parse it somewhere?
> 
> My naive approach to prevent further coredumps would be to add a safeguard 
> before the
> strchr_m line, something like:
> 
> --- a/source3/auth/user_krb5.c
> +++ b/source3/auth/user_krb5.c
> @@ -49,6 +49,11 @@ NTSTATUS get_user_from_kerberos_info(TALLOC_CTX 
> *mem_ctx,
>  
>         DEBUG(3, ("Kerberos ticket principal name is [%s]\n", 
> princ_name));
>  
> +       if (!princ_name) {
> +               DEBUG(3, ("Ticket as empty principal!\n"));
> +               return NT_STATUS_LOGON_FAILURE;
> +       }
> +
>         p = strchr_m(princ_name, '@');
>         if (!p) {
>                 DEBUG(3, ("[%s] Doesn't look like a valid principal\n",
> 
> Would that be good enough without having more debug data?
> Other proposals?

Ok, I've checked into this carefully - and get_user_from_kerberos_info()
is called from the smb and smb2 sessionsetup calls - after ads_verify_ticket()
has been called. princ_name can't be NULL if that call succeeds.

The third place this is called is inside gssapi_server_get_user_info(),
where I recently added a call to gse_get_client_name() that was missing
recently which should ensure the princ_name is initialized if it returns
NT_STATUS_OK. The git ref is ba6f88a6 for that addition.

Does your v3-6-test tree have e5f4b6e7aa1d102db023a491991684118875ee38..ba6f88a6720358bea75b162f193182b724b9411c
in it ? This should ensure princ_name is initialized. When did you last
git pull v3-6-test ?

Jeremy.