Received-SPF: pass (google.com: domain of linux-nfs+bounces-2303-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <d0123f6e-4687-41a4-8a28-cbe583a6f6f3@redhat.com>
Date: Fri, 15 Mar 2024 09:13:34 -0400
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [nfs-utils PATCH 2/2] gssd: add a "backoff" feature to
 limit_krb5_enctypes()
Content-Language: en-US
To: Scott Mayhew <smayhew@redhat.com>
Cc: linux-nfs@vger.kernel.org
References: <20240228222253.1080880-1-smayhew@redhat.com>
 <20240228222253.1080880-3-smayhew@redhat.com>
From: Steve Dickson <steved@redhat.com>
In-Reply-To: <20240228222253.1080880-3-smayhew@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


On 2/28/24 5:22 PM, Scott Mayhew wrote:
> If the NFS server reset the connection when we tried to create a GSS
> context with it, then there's a good chance that we used an encryption
> type that it didn't support.
> 
> Add a one time backoff/retry mechanism, where we adjust the list of
> encryption types that we set via gss_set_allowable_enctypes().  We can
> do this easily because the list of encryption types should be ordered
> from highest preference to lowest.  We just need to find the first entry
> that's not one of the newer encryption types, and then use that as the
> start of the list.
> 
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
Not Committed!

steved.
> ---
>   utils/gssd/gssd_proc.c | 15 +++++++++++++--
>   utils/gssd/krb5_util.c | 40 +++++++++++++++++++++++++++++++++++++++-
>   utils/gssd/krb5_util.h |  2 +-
>   3 files changed, 53 insertions(+), 4 deletions(-)
> 
> diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
> index 7629de0b..0da54598 100644
> --- a/utils/gssd/gssd_proc.c
> +++ b/utils/gssd/gssd_proc.c
> @@ -337,6 +337,10 @@ create_auth_rpc_client(struct clnt_info *clp,
>   	rpc_gss_options_req_t	req;
>   	rpc_gss_options_ret_t	ret;
>   	char			mechanism[] = "kerberos_v5";
> +#endif
> +#ifdef HAVE_SET_ALLOWABLE_ENCTYPES
> +	bool			backoff = false;
> +	struct rpc_err		err;
>   #endif
>   	pthread_t tid = pthread_self();
>   
> @@ -354,14 +358,14 @@ create_auth_rpc_client(struct clnt_info *clp,
>   		goto out_fail;
>   	}
>   
> -
>   	if (authtype == AUTHTYPE_KRB5) {
>   #ifdef HAVE_SET_ALLOWABLE_ENCTYPES
> +again:
>   		/*
>   		 * Do this before creating rpc connection since we won't need
>   		 * rpc connection if it fails!
>   		 */
> -		if (limit_krb5_enctypes(&sec)) {
> +		if (limit_krb5_enctypes(&sec, backoff)) {
>   			printerr(1, "WARNING: Failed while limiting krb5 "
>   				    "encryption types for user with uid %d\n",
>   				 uid);
> @@ -445,6 +449,13 @@ create_auth_rpc_client(struct clnt_info *clp,
>   					goto success;
>   			}
>   		}
> +#endif
> +#ifdef HAVE_SET_ALLOWABLE_ENCTYPES
> +		clnt_geterr(rpc_clnt, &err);
> +		if (err.re_errno == ECONNRESET && !backoff) {
> +			backoff = true;
> +			goto again;
> +		}
>   #endif
>   		/* Our caller should print appropriate message */
>   		printerr(2, "WARNING: Failed to create krb5 context for "
> diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
> index 57b3cf8a..5502e74e 100644
> --- a/utils/gssd/krb5_util.c
> +++ b/utils/gssd/krb5_util.c
> @@ -1675,7 +1675,7 @@ out:
>    */
>   
>   int
> -limit_krb5_enctypes(struct rpc_gss_sec *sec)
> +limit_krb5_enctypes(struct rpc_gss_sec *sec, bool backoff)
>   {
>   	u_int maj_stat, min_stat;
>   	krb5_enctype enctypes[] = { ENCTYPE_DES_CBC_CRC,
> @@ -1689,6 +1689,17 @@ limit_krb5_enctypes(struct rpc_gss_sec *sec)
>   	int num_set_enctypes;
>   	krb5_enctype *set_enctypes;
>   	int err = -1;
> +	int i, j;
> +	bool done = false;
> +
> +	if (backoff && sec->cred != GSS_C_NO_CREDENTIAL) {
> +		printerr(2, "%s: backoff: releasing old cred\n", __func__);
> +		maj_stat = gss_release_cred(&min_stat, &sec->cred);
> +		if (maj_stat != GSS_S_COMPLETE) {
> +			printerr(2, "%s: gss_release_cred() failed\n", __func__);
> +			return -1;
> +		}
> +	}
>   
>   	if (sec->cred == GSS_C_NO_CREDENTIAL) {
>   		err = gssd_acquire_krb5_cred(&sec->cred);
> @@ -1718,6 +1729,33 @@ limit_krb5_enctypes(struct rpc_gss_sec *sec)
>   		set_enctypes = krb5_enctypes;
>   	}
>   
> +	if (backoff) {
> +		j = num_set_enctypes;
> +		for (i = 0; i < j && !done; i++) {
> +			switch (*set_enctypes) {
> +			case ENCTYPE_AES128_CTS_HMAC_SHA256_128:
> +			case ENCTYPE_AES256_CTS_HMAC_SHA384_192:
> +			case ENCTYPE_CAMELLIA128_CTS_CMAC:
> +			case ENCTYPE_CAMELLIA256_CTS_CMAC:
> +				printerr(2, "%s: backoff: removing enctype %d\n",
> +					 __func__, *set_enctypes);
> +				set_enctypes++;
> +				num_set_enctypes--;
> +				break;
> +			default:
> +				done = true;
> +				break;
> +			}
> +		}
> +		printerr(2, "%s: backoff: %d remaining enctypes\n",
> +			 __func__, num_set_enctypes);
> +		if (!num_set_enctypes) {
> +			printerr(0, "%s: no remaining enctypes after backoff\n",
> +				 __func__);
> +			return -1;
> +		}
> +	}
> +
>   	maj_stat = gss_set_allowable_enctypes(&min_stat, sec->cred,
>   				&krb5oid, num_set_enctypes, set_enctypes);
>   
> diff --git a/utils/gssd/krb5_util.h b/utils/gssd/krb5_util.h
> index 40ad3233..0be0c500 100644
> --- a/utils/gssd/krb5_util.h
> +++ b/utils/gssd/krb5_util.h
> @@ -26,7 +26,7 @@ int gssd_k5_remove_bad_service_cred(char *srvname);
>   
>   #ifdef HAVE_SET_ALLOWABLE_ENCTYPES
>   extern int limit_to_legacy_enctypes;
> -int limit_krb5_enctypes(struct rpc_gss_sec *sec);
> +int limit_krb5_enctypes(struct rpc_gss_sec *sec, bool backoff);
>   int get_allowed_enctypes(void);
>   #endif
>