From 0ad72b96d253375646556973789a0e466de246a6 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Mon, 7 Aug 2017 18:54:05 -0700 Subject: [PATCH] [master] ensure verified_sig 4670. [cleanup] Ensure that a request MAC is never sent back in an XFR response unless the signature was verified. [RT #45494] --- CHANGES | 4 ++++ bin/named/xfrout.c | 12 +++++++++--- lib/dns/tsig.c | 7 ++++--- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/CHANGES b/CHANGES index 8693dfe68f..2a520d084c 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +4670. [cleanup] Ensure that a request MAC is never sent back + in an XFR response unless the signature was + verified. [RT #45494] + 4669. [func] Iterative query logic in resolver.c has been refactored into smaller functions and commented, for improved readability, maintainability and diff --git a/bin/named/xfrout.c b/bin/named/xfrout.c index 209e615e3f..5c6a6a73ec 100644 --- a/bin/named/xfrout.c +++ b/bin/named/xfrout.c @@ -667,6 +667,7 @@ typedef struct { unsigned int nmsg; /* Number of messages sent */ dns_tsigkey_t *tsigkey; /* Key used to create TSIG */ isc_buffer_t *lasttsig; /* the last TSIG */ + isc_boolean_t verified_tsig; /* verified request MAC */ isc_boolean_t many_answers; int sends; /* Send in progress */ isc_boolean_t shuttingdown; @@ -680,6 +681,7 @@ xfrout_ctx_create(isc_mem_t *mctx, ns_client_t *client, dns_db_t *db, dns_dbversion_t *ver, isc_quota_t *quota, rrstream_t *stream, dns_tsigkey_t *tsigkey, isc_buffer_t *lasttsig, + isc_boolean_t verified_tsig, unsigned int maxtime, unsigned int idletime, isc_boolean_t many_answers, @@ -1031,6 +1033,7 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) { zone, db, ver, quota, stream, dns_message_gettsigkey(request), tsigbuf, + request->verified_sig, 3600, 3600, (format == dns_many_answers) ? @@ -1042,6 +1045,7 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) { zone, db, ver, quota, stream, dns_message_gettsigkey(request), tsigbuf, + request->verified_sig, dns_zone_getmaxxfrout(zone), dns_zone_getidleout(zone), (format == dns_many_answers) ? @@ -1138,9 +1142,9 @@ xfrout_ctx_create(isc_mem_t *mctx, ns_client_t *client, unsigned int id, dns_rdataclass_t qclass, dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, isc_quota_t *quota, rrstream_t *stream, dns_tsigkey_t *tsigkey, - isc_buffer_t *lasttsig, unsigned int maxtime, - unsigned int idletime, isc_boolean_t many_answers, - xfrout_ctx_t **xfrp) + isc_buffer_t *lasttsig, isc_boolean_t verified_tsig, + unsigned int maxtime, unsigned int idletime, + isc_boolean_t many_answers, xfrout_ctx_t **xfrp) { xfrout_ctx_t *xfr; isc_result_t result; @@ -1169,6 +1173,7 @@ xfrout_ctx_create(isc_mem_t *mctx, ns_client_t *client, unsigned int id, xfr->end_of_stream = ISC_FALSE; xfr->tsigkey = tsigkey; xfr->lasttsig = lasttsig; + xfr->verified_tsig = verified_tsig; xfr->txmem = NULL; xfr->txmemlen = 0; xfr->nmsg = 0; @@ -1302,6 +1307,7 @@ sendstream(xfrout_ctx_t *xfr) { CHECK(dns_message_setquerytsig(msg, xfr->lasttsig)); if (xfr->lasttsig != NULL) isc_buffer_free(&xfr->lasttsig); + msg->verified_sig = xfr->verified_tsig; /* * Add a EDNS option to the message? diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c index 6a7a13b160..1f9ed62d65 100644 --- a/lib/dns/tsig.c +++ b/lib/dns/tsig.c @@ -978,7 +978,6 @@ dns_tsig_sign(dns_message_t *msg) { * has validated at this point. This is why we include a * MAC length > 0 in the reply. */ - ret = dst_context_create3(key->key, mctx, DNS_LOGCATEGORY_DNSSEC, ISC_TRUE, &ctx); @@ -991,6 +990,8 @@ dns_tsig_sign(dns_message_t *msg) { if (response) { dns_rdata_t querytsigrdata = DNS_RDATA_INIT; + INSIST(msg->verified_sig); + ret = dns_rdataset_first(msg->querytsig); if (ret != ISC_R_SUCCESS) goto cleanup_context; @@ -1482,6 +1483,7 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, } else if (ret != ISC_R_SUCCESS) { goto cleanup_context; } + msg->verified_sig = 1; } else if (tsig.error != dns_tsigerror_badsig && tsig.error != dns_tsigerror_badkey) { tsig_log(msg->tsigkey, 2, "signature was empty"); @@ -1559,7 +1561,6 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, } msg->tsigstatus = dns_rcode_noerror; - msg->verified_sig = 1; ret = ISC_R_SUCCESS; cleanup_context: @@ -1798,6 +1799,7 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { } else if (ret != ISC_R_SUCCESS) { goto cleanup_context; } + msg->verified_sig = 1; /* * Here at this point, the MAC has been verified. Even @@ -1885,7 +1887,6 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { } msg->tsigstatus = dns_rcode_noerror; - msg->verified_sig = 1; ret = ISC_R_SUCCESS; cleanup_context: