mirror of
https://github.com/isc-projects/bind9.git
synced 2026-05-27 20:25:55 -04:00
Fix GSS-API context leak in TKEY negotiation
Reject multi-round GSS-API negotiation (GSS_S_CONTINUE_NEEDED) in dst_gssapi_acceptctx(). Each call to gss_accept_sec_context() allocates a context inside the GSS library; without this fix, the context handle was passed back to process_gsstkey() which did not store it persistently, leaking it on every incomplete negotiation. An unauthenticated attacker could exhaust server memory by sending repeated TKEY queries with GSSAPI tokens, each leaking one GSS context. The leaked memory is allocated by the GSS library via malloc(), bypassing BIND's memory accounting. In practice, Kerberos/SPNEGO (the only mechanism used with BIND) completes in a single round, so rejecting continuation does not affect real-world deployments. See RFC 3645 Section 4.1.3. (cherry picked from commit 3d8e0d068f08694282c5ecd3bd6c332de6c75485)
This commit is contained in:
Ondřej Surý
Michał Kępień
parent
f6d193147a
commit
9bdace3fa3
3 changed files with 66 additions and 73 deletions
|
|
@ -607,7 +607,14 @@ dst_gssapi_initctx(const dns_name_t *name, isc_buffer_t *intoken,
|
|||
GSS_SPNEGO_MECHANISM, flags, 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",
|
||||
|
|
@ -632,12 +639,6 @@ dst_gssapi_initctx(const dns_name_t *name, isc_buffer_t *intoken,
|
|||
CHECK(isc_buffer_copyregion(outtoken, &r));
|
||||
}
|
||||
|
||||
if (gret == GSS_S_COMPLETE) {
|
||||
result = ISC_R_SUCCESS;
|
||||
} else {
|
||||
result = DNS_R_CONTINUE;
|
||||
}
|
||||
|
||||
cleanup:
|
||||
if (gouttoken.length != 0U) {
|
||||
(void)gss_release_buffer(&minor, &gouttoken);
|
||||
|
|
@ -662,15 +663,10 @@ dst_gssapi_acceptctx(dns_gss_cred_id_t cred, const char *gssapi_keytab,
|
|||
char buf[1024];
|
||||
|
||||
REQUIRE(outtoken != NULL && *outtoken == 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 HAVE_GSSAPI_GSSAPI_KRB5_H || HAVE_GSSAPI_KRB5_H
|
||||
gret = gsskrb5_register_acceptor_identity(gssapi_keytab);
|
||||
|
|
@ -715,8 +711,15 @@ dst_gssapi_acceptctx(dns_gss_cred_id_t cred, const char *gssapi_keytab,
|
|||
|
||||
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:
|
||||
|
|
@ -729,7 +732,7 @@ dst_gssapi_acceptctx(dns_gss_cred_id_t cred, const char *gssapi_keytab,
|
|||
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)));
|
||||
|
|
@ -747,43 +750,44 @@ dst_gssapi_acceptctx(dns_gss_cred_id_t cred, const char *gssapi_keytab,
|
|||
(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)));
|
||||
CHECK(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--;
|
||||
}
|
||||
|
||||
gss_log(3, "gss-api source name (accept) is %.*s",
|
||||
(int)gnamebuf.length, (char *)gnamebuf.value);
|
||||
|
||||
GBUFFER_TO_REGION(gnamebuf, r);
|
||||
isc_buffer_init(&namebuf, r.base, r.length);
|
||||
isc_buffer_add(&namebuf, r.length);
|
||||
|
||||
CHECK(dns_name_fromtext(principal, &namebuf, dns_rootname, 0,
|
||||
NULL));
|
||||
} else {
|
||||
result = DNS_R_CONTINUE;
|
||||
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 cleanup;
|
||||
}
|
||||
|
||||
/*
|
||||
* 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--;
|
||||
}
|
||||
|
||||
gss_log(3, "gss-api source name (accept) is %.*s", (int)gnamebuf.length,
|
||||
(char *)gnamebuf.value);
|
||||
|
||||
GBUFFER_TO_REGION(gnamebuf, r);
|
||||
isc_buffer_init(&namebuf, r.base, r.length);
|
||||
isc_buffer_add(&namebuf, r.length);
|
||||
|
||||
CHECK(dns_name_fromtext(principal, &namebuf, dns_rootname, 0, NULL));
|
||||
|
||||
*ctxout = context;
|
||||
|
||||
cleanup:
|
||||
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) {
|
||||
|
|
|
|||
|
|
@ -113,20 +113,17 @@ dst_gssapi_acceptctx(dns_gss_cred_id_t cred, const char *gssapi_keytab,
|
|||
* 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 dns_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
|
||||
|
|
|
|||
|
|
@ -190,15 +190,6 @@ 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
|
||||
*/
|
||||
result = dns_tsigkey_find(&tsigkey, name, &tkeyin->algorithm, ring);
|
||||
if (result == ISC_R_SUCCESS) {
|
||||
gss_ctx = dst_key_getgssctx(tsigkey->key);
|
||||
}
|
||||
|
||||
/*
|
||||
* Note that tctx->gsscred may be NULL if tctx->gssapi_keytab is set
|
||||
*/
|
||||
|
|
@ -206,7 +197,7 @@ 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 (result != ISC_R_SUCCESS) {
|
||||
if (tsigkey != NULL) {
|
||||
dns_tsigkey_detach(&tsigkey);
|
||||
}
|
||||
|
|
@ -214,12 +205,11 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin,
|
|||
tkey_log("process_gsstkey(): dns_tsigerror_badkey");
|
||||
return ISC_R_SUCCESS;
|
||||
}
|
||||
if (result != DNS_R_CONTINUE && result != ISC_R_SUCCESS) {
|
||||
CHECK(result);
|
||||
}
|
||||
|
||||
/*
|
||||
* 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.
|
||||
*/
|
||||
if (dns_name_countlabels(principal) == 0U) {
|
||||
if (tsigkey != NULL) {
|
||||
|
|
@ -285,6 +275,9 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin,
|
|||
return ISC_R_SUCCESS;
|
||||
|
||||
cleanup:
|
||||
if (dstkey == NULL && gss_ctx != NULL) {
|
||||
dst_gssapi_deletectx(tctx->mctx, &gss_ctx);
|
||||
}
|
||||
if (tsigkey != NULL) {
|
||||
dns_tsigkey_detach(&tsigkey);
|
||||
}
|
||||
|
|
@ -689,9 +682,8 @@ dns_tkey_gssnegotiate(dns_message_t *qmsg, dns_message_t *rmsg,
|
|||
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.
|
||||
*/
|
||||
CHECK(dns_tsigkey_createfromkey(tkeyname, DST_ALG_GSSAPI, dstkey, true,
|
||||
false, NULL, rtkey.inception,
|
||||
|
|
|
|||
Loading…
Reference in a new issue