From 10be6a75c4638569d6d564b1acb60f9ef2350550 Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Sat, 15 Nov 2025 14:40:00 -0600 Subject: [PATCH] spnego: Restrict when SANON gets negotiated There were cases where we weren't negotiating SANON where we should have. But we really don't want to overdo it. In particular we really never ever want a user with expired or absent Kerberos credentials (say) to accidentally negotiate SANON as that will then lead to authorization errors down the line, and those would be hard to diagnose as they would be masking the real issue (expired or absent credentials). So basically either the user passes GSS_C_ANON_FLAG or (and/or) they call gss_set_neg_mechs() to explicitly request SANON. Partly authored by me, partly authored by Claude with heavy human guidance, and reviewed by me. --- lib/gssapi/spnego/accept_sec_context.c | 47 +++++++++++++++++++++----- lib/gssapi/spnego/compat.c | 46 +++++++++++++++++++++---- lib/gssapi/spnego/context_stubs.c | 2 +- 3 files changed, 79 insertions(+), 16 deletions(-) diff --git a/lib/gssapi/spnego/accept_sec_context.c b/lib/gssapi/spnego/accept_sec_context.c index 7a211900f..9a24278dd 100644 --- a/lib/gssapi/spnego/accept_sec_context.c +++ b/lib/gssapi/spnego/accept_sec_context.c @@ -89,7 +89,8 @@ acceptor_approved(OM_uint32 *minor_status, if (gss_oid_equal(mech, GSS_NEGOEX_MECHANISM)) { size_t i; - ret = _gss_spnego_indicate_mechs(minor_status, &oidset); + ret = _gss_spnego_indicate_mechs(minor_status, GSS_C_ANON_FLAG, + &oidset); if (ret != GSS_S_COMPLETE) return ret; @@ -681,13 +682,43 @@ acceptor_start } } - if (acceptor_cred_handle != GSS_C_NO_CREDENTIAL) - ret = _gss_spnego_inquire_cred_mechs(minor_status, - acceptor_cred_handle, - &supported_mechs, - &canonical_order); - else - ret = _gss_spnego_indicate_mechs(minor_status, &supported_mechs); + /* + * If the initiator proposed NegoEx, include anonymous mechanisms in + * supported_mechs since NegoEx mechanisms like sanon-x25519 may be used. + */ + { + OM_uint32 indicate_flags = 0; + size_t i; + + for (i = 0; i < ni->mechTypes.len; i++) { + size_t oidlen = der_length_oid(&ni->mechTypes.val[i]); + gss_OID_desc oid; + char oidbuf[64]; + + if (oidlen > sizeof(oidbuf)) + continue; + + oid.length = oidlen; + oid.elements = oidbuf; + + if (der_put_oid((unsigned char *)oidbuf + oidlen - 1, + oidlen, &ni->mechTypes.val[i], &oidlen) == 0 && + gss_oid_equal(&oid, GSS_NEGOEX_MECHANISM)) { + indicate_flags |= GSS_C_ANON_FLAG; + break; + } + } + + if (acceptor_cred_handle != GSS_C_NO_CREDENTIAL) + ret = _gss_spnego_inquire_cred_mechs(minor_status, + indicate_flags, + acceptor_cred_handle, + &supported_mechs, + &canonical_order); + else + ret = _gss_spnego_indicate_mechs(minor_status, indicate_flags, + &supported_mechs); + } if (ret != GSS_S_COMPLETE) goto out; diff --git a/lib/gssapi/spnego/compat.c b/lib/gssapi/spnego/compat.c index 6cfe55266..9a16c1fad 100644 --- a/lib/gssapi/spnego/compat.c +++ b/lib/gssapi/spnego/compat.c @@ -403,10 +403,11 @@ _gss_spnego_indicate_mechtypelist (OM_uint32 *minor_status, mechtypelist->val = NULL; if (cred_handle != GSS_C_NO_CREDENTIAL) - ret = _gss_spnego_inquire_cred_mechs(minor_status, cred_handle, + ret = _gss_spnego_inquire_cred_mechs(minor_status, req_flags, cred_handle, &supported_mechs, &canonical_order); else - ret = _gss_spnego_indicate_mechs(minor_status, &supported_mechs); + ret = _gss_spnego_indicate_mechs(minor_status, req_flags, + &supported_mechs); if (ret != GSS_S_COMPLETE) return ret; @@ -595,19 +596,24 @@ _gss_spnego_log_mechTypes(MechTypeList *mechTypes) OM_uint32 _gss_spnego_indicate_mechs(OM_uint32 *minor_status, + OM_uint32 req_flags, gss_OID_set *mechs_p) { - gss_OID_desc oids[3]; + gss_OID_desc oids[5]; gss_OID_set_desc except; + except.elements = oids; *mechs_p = GSS_C_NO_OID_SET; oids[0] = *GSS_C_MA_DEPRECATED; oids[1] = *GSS_C_MA_NOT_DFLT_MECH; oids[2] = *GSS_C_MA_MECH_NEGO; - - except.count = sizeof(oids) / sizeof(oids[0]); - except.elements = oids; + except.count = sizeof(oids) / sizeof(oids[0]) - 2; + if ((req_flags & GSS_C_ANON_FLAG) == 0) { + oids[3] = *GSS_C_MA_AUTH_INIT_ANON; + oids[4] = *GSS_C_MA_AUTH_TARG_ANON; + except.count = sizeof(oids) / sizeof(oids[0]); + } return gss_indicate_mechs_by_attrs(minor_status, GSS_C_NO_OID_SET, @@ -622,6 +628,7 @@ _gss_spnego_indicate_mechs(OM_uint32 *minor_status, OM_uint32 _gss_spnego_inquire_cred_mechs(OM_uint32 *minor_status, + OM_uint32 req_flags, gss_const_cred_id_t cred, gss_OID_set *mechs_p, int *canonical_order) @@ -648,7 +655,32 @@ _gss_spnego_inquire_cred_mechs(OM_uint32 *minor_status, heim_assert(cred_mechs != GSS_C_NO_OID_SET && cred_mechs->count > 0, "gss_inquire_cred succeeded but returned no mechanisms"); - ret = _gss_spnego_indicate_mechs(minor_status, &negotiable_mechs); + /* + * If gss_set_neg_mechs() was called and included an anonymous mechanism, + * add GSS_C_ANON_FLAG so it won't be filtered out by _gss_spnego_indicate_mechs(). + */ + if (*canonical_order && (req_flags & GSS_C_ANON_FLAG) == 0) { + for (i = 0; i < cred_mechs->count; i++) { + gss_OID_set mech_attrs = GSS_C_NO_OID_SET; + int present = 0; + + ret = gss_inquire_attrs_for_mech(minor_status, + &cred_mechs->elements[i], + &mech_attrs, NULL); + if (ret == GSS_S_COMPLETE) { + gss_test_oid_set_member(&junk, GSS_C_MA_AUTH_INIT_ANON, + mech_attrs, &present); + gss_release_oid_set(&junk, &mech_attrs); + } + if (present) { + req_flags |= GSS_C_ANON_FLAG; + break; + } + } + } + + ret = _gss_spnego_indicate_mechs(minor_status, req_flags, + &negotiable_mechs); if (ret != GSS_S_COMPLETE) goto out; diff --git a/lib/gssapi/spnego/context_stubs.c b/lib/gssapi/spnego/context_stubs.c index 638e90d7b..f75bfe926 100644 --- a/lib/gssapi/spnego/context_stubs.c +++ b/lib/gssapi/spnego/context_stubs.c @@ -370,7 +370,7 @@ OM_uint32 GSSAPI_CALLCONV _gss_spnego_inquire_names_for_mech ( *name_types = NULL; - ret = _gss_spnego_indicate_mechs(minor_status, &mechs); + ret = _gss_spnego_indicate_mechs(minor_status, 0, &mechs); if (ret != GSS_S_COMPLETE) return ret;