Re: [PATCH] Fix crdential sourcing with new setuid behavior in rpc.gssd

[Jeff Layton <jlayton@redhat.com>]

On Wed, 15 Jan 2014 16:41:34 -0500
Simo Sorce <simo@redhat.com> wrote:

> From 421f66b1cd0b031ef843f7680f463027904b93ca Mon Sep 17 00:00:00 2001
> From: Simo Sorce <simo@redhat.com>
> Date: Wed, 15 Jan 2014 16:01:49 -0500
> Subject: [PATCH] Improve first attempt at acquiring GSS credentials
> 
> Since now rpc.gssd is swithing uid before attempting to acquire
> credentials, we do not need to pass in the special uid-as-a-string name
> to gssapi, because the process is already running under the user's
> credentials.
> 
> By making this optional we can fix a class of false negatives where the
> user name does not match the actual ccache credentials and the ccache
> type used is not one of the only 2 supported explicitly by rpc.gssd by the
> fallback trolling done later.
> 
> Signed-off-by: Simo Sorce <simo@redhat.com>
> ---
>  utils/gssd/krb5_util.c | 32 ++++++++++++++++++--------------
>  1 file changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
> index 697d1d2e79db0cc38160ea4772d3af3a9b7d6c21..7db5baf4e4bea75ed7beebd2103afbc291efb641 100644
> --- a/utils/gssd/krb5_util.c
> +++ b/utils/gssd/krb5_util.c
> @@ -1383,24 +1383,28 @@ gssd_acquire_user_cred(uid_t uid, gss_cred_id_t *gss_cred)
>  {
>  	OM_uint32 maj_stat, min_stat;
>  	gss_buffer_desc name_buf;
> -	gss_name_t name;
> +	gss_name_t name = GSS_C_NO_NAME;
>  	char buf[11];
>  	int ret;
>  
> -	ret = snprintf(buf, 11, "%u", uid);
> -	if (ret < 1 || ret > 10) {
> -		return -1;
> -	}
> -	name_buf.value = buf;
> -	name_buf.length = ret + 1;
> +	/* the follwing is useful only if change_identity() in
> +	 * process_krb5_upcall() failed to change uids */
> +	if (getuid() == 0) {
> +		ret = snprintf(buf, 11, "%u", uid);
> +		if (ret < 1 || ret > 10) {
> +			return -1;
> +		}
> +		name_buf.value = buf;
> +		name_buf.length = ret + 1;
>  

If change_identity() fails, then process_krb5_upcall should just give
up and do an error downcall, so falling back to using
GSS_C_NT_STRING_UID_NAME in that case seems unnecessary.

Also, we can end up in here legitimately with uid == 0 if
root_uses_machine_creds == 0. So I wonder if we even need the stuff
inside this "if (getuid() == 0)" block at all...

> -	maj_stat = gss_import_name(&min_stat, &name_buf,
> -				   GSS_C_NT_STRING_UID_NAME, &name);
> -	if (maj_stat != GSS_S_COMPLETE) {
> -		if (get_verbosity() > 0)
> -			pgsserr("gss_import_name",
> -				maj_stat, min_stat, &krb5oid);
> -		return -1;
> +		maj_stat = gss_import_name(&min_stat, &name_buf,
> +					   GSS_C_NT_STRING_UID_NAME, &name);
> +		if (maj_stat != GSS_S_COMPLETE) {
> +			if (get_verbosity() > 0)
> +				pgsserr("gss_import_name",
> +					maj_stat, min_stat, &krb5oid);
> +			return -1;
> +		}
>  	}
>  
>  	ret = gssd_acquire_krb5_cred(name, gss_cred);


Other than that, I'm fine with ripping that junk out.

-- 
Jeff Layton <jlayton@redhat.com>