| Message ID | 20240228222253.1080880-3-smayhew@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | gssd: improve interoperability with NFS servers that don't have support for the newest encryption types | expand |
Re-adding the list. On Mon, 04 Mar 2024, Scott Mayhew wrote: > On Mon, 04 Mar 2024, Olga Kornievskaia wrote: > > > On Wed, Feb 28, 2024 at 5:23 PM Scott Mayhew <smayhew@redhat.com> 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. > > > > This doesn't seem like a good idea because it opens up a security > > problem that a connection reset (by an attacker) during context > > establishment would force the client to downgrade its security. > > I'm willing to drop this patch if that's the consensus. Or we could > still it behind yet another config option. > > I'd still like to see the first one get merged though. > > -Scott > > > > > > > > Signed-off-by: Scott Mayhew <smayhew@redhat.com> > > > --- > > > 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 > > > > > > -- > > > 2.43.0 > > > > > > > >
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 >
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
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> --- 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(-)