commit 6f948210822233be867392d80f859b30417fdacb Author: Ondřej Surý Date: Fri May 1 08:41:52 2026 +0200 [9.11] [CVE-2026-3039] sec: usr: Fix GSS-API resource leak Fixed a memory leak where each GSS-API TKEY negotiation leaked a security context inside the GSS library. An unauthenticated attacker could exhaust server memory by sending repeated TKEY queries to a server with tkey-gssapi-keytab configured. The leaked memory was allocated by the GSS library, bypassing BIND's memory accounting. Multi-round GSS-API negotiation (GSS_S_CONTINUE_NEEDED) is now rejected, as BIND never supported it correctly and Kerberos/SPNEGO completes in a single round. Also implemented missing RFC 3645 requirement: the client now verifies that mutual authentication and integrity flags are granted by the GSS-API mechanism (Section 3.1.1). Closes: https://gitlab.isc.org/isc-projects/bind9/-/issues/5752 Backport of !965 Merge branch 'backport-5752-fix-memory-leak-in-TKEY-negotiation-9.11' into 'security-bind-9.11' See merge request isc-private/bind9!977 diff --git a/lib/dns/gssapictx.c b/lib/dns/gssapictx.c index e21c7de21e..e41ea24f46 100644 --- a/lib/dns/gssapictx.c +++ b/lib/dns/gssapictx.c @@ -612,7 +612,14 @@ dst_gssapi_initctx(dns_name_t *name, isc_buffer_t *intoken, 0, NULL, gintokenp, NULL, &gouttoken, &ret_flags, NULL); - if (gret != GSS_S_COMPLETE && gret != GSS_S_CONTINUE_NEEDED) { + switch (gret) { + case GSS_S_COMPLETE: + result = ISC_R_SUCCESS; + break; + case GSS_S_CONTINUE_NEEDED: + result = DNS_R_CONTINUE; + break; + default: gss_err_message(mctx, gret, minor, err_message); if (err_message != NULL && *err_message != NULL) gss_log(3, "Failure initiating security context: %s", @@ -637,14 +644,10 @@ dst_gssapi_initctx(dns_name_t *name, isc_buffer_t *intoken, RETERR(isc_buffer_copyregion(outtoken, &r)); } - if (gret == GSS_S_COMPLETE) - result = ISC_R_SUCCESS; - else - result = DNS_R_CONTINUE; - - out: - if (gouttoken.length != 0U) +out: + if (gouttoken.length != 0U) { (void)gss_release_buffer(&minor, &gouttoken); + } (void)gss_release_name(&minor, &gname); return (result); #else @@ -662,7 +665,7 @@ dst_gssapi_initctx(dns_name_t *name, isc_buffer_t *intoken, isc_result_t dst_gssapi_acceptctx(gss_cred_id_t cred, const char *gssapi_keytab, - isc_region_t *intoken, isc_buffer_t **outtoken, + isc_region_t *intoken, isc_buffer_t **outtokenp, gss_ctx_id_t *ctxout, dns_name_t *principal, isc_mem_t *mctx) { @@ -677,15 +680,11 @@ dst_gssapi_acceptctx(gss_cred_id_t cred, isc_result_t result; char buf[1024]; - REQUIRE(outtoken != NULL && *outtoken == NULL); + REQUIRE(outtokenp != NULL && *outtokenp == NULL); + REQUIRE(*ctxout == NULL); REGION_TO_GBUFFER(*intoken, gintoken); - if (*ctxout == NULL) - context = GSS_C_NO_CONTEXT; - else - context = *ctxout; - if (gssapi_keytab != NULL) { #if defined(ISC_PLATFORM_GSSAPI_KRB5_HEADER) || defined(WIN32) gret = gsskrb5_register_acceptor_identity(gssapi_keytab); @@ -727,8 +726,15 @@ dst_gssapi_acceptctx(gss_cred_id_t cred, switch (gret) { case GSS_S_COMPLETE: - case GSS_S_CONTINUE_NEEDED: break; + /* + * RFC 3645 4.1.3: we don't handle GSS_S_CONTINUE_NEEDED + * Multi-round GSS-API negotiation is not supported. + */ + case GSS_S_CONTINUE_NEEDED: + gss_log(3, "multi-round GSS-API negotiation not supported"); + (void)gss_delete_sec_context(&minor, &context, NULL); + /* FALLTHROUGH */ case GSS_S_DEFECTIVE_TOKEN: case GSS_S_DEFECTIVE_CREDENTIAL: case GSS_S_BAD_SIG: @@ -741,7 +747,7 @@ dst_gssapi_acceptctx(gss_cred_id_t cred, case GSS_S_BAD_MECH: case GSS_S_FAILURE: result = DNS_R_INVALIDTKEY; - /* fall through */ + /* FALLTHROUGH */ default: gss_log(3, "failed gss_accept_sec_context: %s", gss_error_tostring(gret, minor, buf, sizeof(buf))); @@ -749,55 +755,70 @@ dst_gssapi_acceptctx(gss_cred_id_t cred, } if (gouttoken.length > 0U) { - RETERR(isc_buffer_allocate(mctx, outtoken, + RETERR(isc_buffer_allocate(mctx, outtokenp, (unsigned int)gouttoken.length)); GBUFFER_TO_REGION(gouttoken, r); - RETERR(isc_buffer_copyregion(*outtoken, &r)); + result = isc_buffer_copyregion(*outtokenp, &r); + if (result != ISC_R_SUCCESS) { + goto out; + } (void)gss_release_buffer(&minor, &gouttoken); } - if (gret == GSS_S_COMPLETE) { - gret = gss_display_name(&minor, gname, &gnamebuf, NULL); - if (gret != GSS_S_COMPLETE) { - gss_log(3, "failed gss_display_name: %s", - gss_error_tostring(gret, minor, - buf, sizeof(buf))); - RETERR(ISC_R_FAILURE); - } + INSIST(gret == GSS_S_COMPLETE); - /* - * Compensate for a bug in Solaris8's implementation - * of gss_display_name(). Should be harmless in any - * case, since principal names really should not - * contain null characters. - */ - if (gnamebuf.length > 0U && - ((char *)gnamebuf.value)[gnamebuf.length - 1] == '\0') - gnamebuf.length--; + gret = gss_display_name(&minor, gname, &gnamebuf, NULL); + if (gret != GSS_S_COMPLETE) { + gss_log(3, "failed gss_display_name: %s", + gss_error_tostring(gret, minor, buf, sizeof(buf))); + result = ISC_R_FAILURE; + goto out; + } - gss_log(3, "gss-api source name (accept) is %.*s", - (int)gnamebuf.length, (char *)gnamebuf.value); + /* + * Compensate for a bug in Solaris8's implementation + * of gss_display_name(). Should be harmless in any + * case, since principal names really should not + * contain null characters. + */ + if (gnamebuf.length > 0U && + ((char *)gnamebuf.value)[gnamebuf.length - 1] == '\0') + { + gnamebuf.length--; + } - GBUFFER_TO_REGION(gnamebuf, r); - isc_buffer_init(&namebuf, r.base, r.length); - isc_buffer_add(&namebuf, r.length); + gss_log(3, "gss-api source name (accept) is %.*s", (int)gnamebuf.length, + (char *)gnamebuf.value); - RETERR(dns_name_fromtext(principal, &namebuf, dns_rootname, - 0, NULL)); + GBUFFER_TO_REGION(gnamebuf, r); + isc_buffer_init(&namebuf, r.base, r.length); + isc_buffer_add(&namebuf, r.length); - if (gnamebuf.length != 0U) { - gret = gss_release_buffer(&minor, &gnamebuf); - if (gret != GSS_S_COMPLETE) - gss_log(3, "failed gss_release_buffer: %s", - gss_error_tostring(gret, minor, buf, - sizeof(buf))); - } - } else - result = DNS_R_CONTINUE; + result = dns_name_fromtext(principal, &namebuf, dns_rootname, 0, NULL); + if (result != ISC_R_SUCCESS) { + goto out; + } *ctxout = context; - out: +out: + if (result != ISC_R_SUCCESS && *outtokenp != NULL) { + isc_buffer_free(outtokenp); + } + + if (result != ISC_R_SUCCESS && context != GSS_C_NO_CONTEXT) { + (void)gss_delete_sec_context(&minor, &context, NULL); + } + + if (gnamebuf.length != 0U) { + gret = gss_release_buffer(&minor, &gnamebuf); + if (gret != GSS_S_COMPLETE) { + gss_log(3, "failed gss_release_buffer: %s", + gss_error_tostring(gret, minor, buf, + sizeof(buf))); + } + } + if (gname != NULL) { gret = gss_release_name(&minor, &gname); if (gret != GSS_S_COMPLETE) diff --git a/lib/dns/include/dst/gssapi.h b/lib/dns/include/dst/gssapi.h index 17a9f548a2..3f32acba36 100644 --- a/lib/dns/include/dst/gssapi.h +++ b/lib/dns/include/dst/gssapi.h @@ -129,20 +129,17 @@ dst_gssapi_acceptctx(gss_cred_id_t cred, * generated by gss_accept_sec_context() to be sent to the * initiator * 'context' is a valid pointer to receive the generated context handle. - * On the initial call, it should be a pointer to NULL, which - * will be allocated as a gss_ctx_id_t. Subsequent calls - * should pass in the handle generated on the first call. - * Call dst_gssapi_releasecred to delete the context and free - * the memory. * * Requires: - * 'outtoken' to != NULL && *outtoken == NULL. + * 'outtoken' != NULL && *outtoken == NULL. + * 'context' != NULL && *context == NULL. * * Returns: - * ISC_R_SUCCESS msg was successfully updated to include the - * query to be sent - * DNS_R_CONTINUE transaction still in progress - * other an error occurred while building the message + * ISC_R_SUCCESS msg was successfully updated to include + * the query to be sent + * DNS_R_INVALIDTKEY an error occurred while accepting the + * context + * ISC_R_FAILURE other error occurred */ isc_result_t diff --git a/lib/dns/tkey.c b/lib/dns/tkey.c index 3fa794fdc3..34ba44350e 100644 --- a/lib/dns/tkey.c +++ b/lib/dns/tkey.c @@ -494,18 +494,9 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin, return (ISC_R_SUCCESS); } - /* - * XXXDCL need to check for key expiry per 4.1.1 - * XXXDCL need a way to check fully established, perhaps w/key_flags - */ - intoken.base = tkeyin->key; intoken.length = tkeyin->keylen; - result = dns_tsigkey_find(&tsigkey, name, &tkeyin->algorithm, ring); - if (result == ISC_R_SUCCESS) - gss_ctx = dst_key_getgssctx(tsigkey->key); - principal = dns_fixedname_initname(&fixed); /* @@ -514,25 +505,27 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin, result = dst_gssapi_acceptctx(tctx->gsscred, tctx->gssapi_keytab, &intoken, &outtoken, &gss_ctx, principal, tctx->mctx); - if (result == DNS_R_INVALIDTKEY) { - if (tsigkey != NULL) - dns_tsigkey_detach(&tsigkey); + if (result != ISC_R_SUCCESS) { tkeyout->error = dns_tsigerror_badkey; - tkey_log("process_gsstkey(): dns_tsigerror_badkey"); /* XXXSRA */ - return (ISC_R_SUCCESS); - } - if (result != DNS_R_CONTINUE && result != ISC_R_SUCCESS) + tkey_log("process_gsstkey(): dns_tsigerror_badkey"); + result = ISC_R_SUCCESS; goto failure; + } + /* - * XXXDCL Section 4.1.3: Limit GSS_S_CONTINUE_NEEDED to 10 times. + * Multi-round GSS-API negotiation (GSS_S_CONTINUE_NEEDED) is + * rejected in dst_gssapi_acceptctx(), so if we reach here the + * negotiation is complete and the principal must be set. */ isc_stdtime_get(&now); if (dns_name_countlabels(principal) == 0U) { - if (tsigkey != NULL) { - dns_tsigkey_detach(&tsigkey); - } + tkeyout->error = dns_tsigerror_badkey; + tkey_log("process_gsstkey(): " + "completed context with empty principal"); + result = ISC_R_SUCCESS; + goto failure; } else if (tsigkey == NULL) { #ifdef GSSAPI OM_uint32 gret, minor, lifetime; @@ -603,6 +596,10 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin, return (ISC_R_SUCCESS); failure: + if (dstkey == NULL && gss_ctx != NULL) { + dst_gssapi_deletectx(tctx->mctx, &gss_ctx); + } + if (tsigkey != NULL) dns_tsigkey_detach(&tsigkey); @@ -612,10 +609,10 @@ failure: if (outtoken != NULL) isc_buffer_free(&outtoken); - tkey_log("process_gsstkey(): %s", - isc_result_totext(result)); /* XXXSRA */ - - return (result); + if (result != ISC_R_SUCCESS) { + tkey_log("process_gsstkey(): %s", isc_result_totext(result)); + } + return result; } static isc_result_t @@ -1512,9 +1509,8 @@ dns_tkey_gssnegotiate(dns_message_t *qmsg, dns_message_t *rmsg, &dstkey, NULL)); /* - * XXXSRA This seems confused. If we got CONTINUE from initctx, - * the GSS negotiation hasn't completed yet, so we can't sign - * anything yet. + * GSS negotiation is complete (CONTINUE returned earlier). + * Create the TSIG key from the established context. */ RETERR(dns_tsigkey_createfromkey(tkeyname,