From 4827d4b06c2aaec913536143e4a26a0904d1fc58 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 7 Jul 2017 23:19:05 +1000 Subject: [PATCH] 4647. [bug] Change 4643 broke verification of TSIG signed TCP message sequences where not all the messages contain TSIG records. These may be used in AXFR and IXFR responses. [RT #45509] (cherry picked from commit 58f0fb325bbd9258d06431281eb8fdea2b126305) --- lib/dns/tests/Makefile.in | 9 +- lib/dns/tests/tsig_test.c | 489 ++++++++++++++++++++++++++++++++++++++++++++++ lib/dns/tsig.c | 10 +- 3 files changed, 504 insertions(+), 4 deletions(-) create mode 100644 lib/dns/tests/tsig_test.c diff --git a/lib/dns/tests/Makefile.in b/lib/dns/tests/Makefile.in index 8d1b83e..023e60c 100644 --- a/lib/dns/tests/Makefile.in +++ b/lib/dns/tests/Makefile.in @@ -39,13 +39,13 @@ LIBS = @LIBS@ @ATFLIBS@ OBJS = dnstest.@O@ SRCS = dnstest.c gost_test.c master_test.c dbiterator_test.c time_test.c \ - private_test.c update_test.c zonemgr_test.c zt_test.c \ + private_test.c tsig_test.c update_test.c zonemgr_test.c zt_test.c \ dbdiff_test.c geoip_test.c dispatch_test.c nsec3_test.c \ rdataset_test.c rdata_test.c SUBDIRS = TARGETS = gost_test@EXEEXT@ master_test@EXEEXT@ dbiterator_test@EXEEXT@ time_test@EXEEXT@ \ - private_test@EXEEXT@ update_test@EXEEXT@ zonemgr_test@EXEEXT@ \ + private_test@EXEEXT@ tsig_test@EXEEXT@ update_test@EXEEXT@ zonemgr_test@EXEEXT@ \ zt_test@EXEEXT@ dbversion_test@EXEEXT@ dbdiff_test@EXEEXT@ geoip_test@EXEEXT@ \ dispatch_test@EXEEXT@ nsec3_test@EXEEXT@ \ rdataset_test@EXEEXT@ rdata_test@EXEEXT@ @@ -134,6 +134,11 @@ geoip_test@EXEEXT@: geoip_test.@O@ ${ISCDEPLIBS} ${DNSDEPLIBS} geoip_test.@O@ dnstest.@O@ ${DNSLIBS} \ ${ISCLIBS} ${LIBS} +tsig_test@EXEEXT@: tsig_test.@O@ dnstest.@O@ ${ISCDEPLIBS} ${DNSDEPLIBS} + ${LIBTOOL_MODE_LINK} ${PURIFY} ${CC} ${CFLAGS} ${LDFLAGS} -o $@ \ + tsig_test.@O@ dnstest.@O@ ${DNSLIBS} \ + ${ISCLIBS} ${LIBS} + unit:: sh ${top_srcdir}/unit/unittest.sh diff --git a/lib/dns/tests/tsig_test.c b/lib/dns/tests/tsig_test.c new file mode 100644 index 0000000..956e4a0 --- /dev/null +++ b/lib/dns/tests/tsig_test.c @@ -0,0 +1,489 @@ +/* + * Copyright (C) 2017 Internet Systems Consortium, Inc. ("ISC") + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +/* ! \file */ + +#include +#include +#include + +#include +#include +#include + +#include "dnstest.h" + +#ifdef HAVE_INTTYPES_H +#include /* uintptr_t */ +#endif + +static int debug = 0; + +static isc_result_t +add_mac(dst_context_t *tsigctx, isc_buffer_t *buf) { + dns_rdata_any_tsig_t tsig; + dns_rdata_t rdata = DNS_RDATA_INIT; + isc_buffer_t databuf; + isc_region_t r; + isc_result_t result; + unsigned char tsigbuf[1024]; + + isc_buffer_usedregion(buf, &r); + dns_rdata_fromregion(&rdata, dns_rdataclass_any, + dns_rdatatype_tsig, &r); + isc_buffer_init(&databuf, tsigbuf, sizeof(tsigbuf)); + CHECK(dns_rdata_tostruct(&rdata, &tsig, NULL)); + isc_buffer_putuint16(&databuf, tsig.siglen); + isc_buffer_putmem(&databuf, tsig.signature, tsig.siglen); + isc_buffer_usedregion(&databuf, &r); + result = dst_context_adddata(tsigctx, &r); + dns_rdata_freestruct(&tsig); + cleanup: + return (result); +} + +static isc_result_t +add_tsig(dst_context_t *tsigctx, dns_tsigkey_t *key, isc_buffer_t *target) { + dns_compress_t cctx; + dns_rdata_any_tsig_t tsig; + dns_rdata_t rdata = DNS_RDATA_INIT; + dns_rdatalist_t rdatalist; + dns_rdataset_t rdataset; + isc_buffer_t *dynbuf = NULL; + isc_buffer_t databuf; + isc_buffer_t sigbuf; + isc_region_t r; + isc_result_t result = ISC_R_SUCCESS; + isc_stdtime_t now; + unsigned char tsigbuf[1024]; + unsigned int count; + unsigned int sigsize; + isc_boolean_t invalidate_ctx = ISC_FALSE; + + CHECK(dns_compress_init(&cctx, -1, mctx)); + invalidate_ctx = ISC_TRUE; + + memset(&tsig, 0, sizeof(tsig)); + tsig.common.rdclass = dns_rdataclass_any; + tsig.common.rdtype = dns_rdatatype_tsig; + ISC_LINK_INIT(&tsig.common, link); + dns_name_init(&tsig.algorithm, NULL); + dns_name_clone(key->algorithm, &tsig.algorithm); + + isc_stdtime_get(&now); + tsig.timesigned = now; + tsig.fudge = DNS_TSIG_FUDGE; + tsig.originalid = 50; + tsig.error = dns_rcode_noerror; + tsig.otherlen = 0; + tsig.other = NULL; + + isc_buffer_init(&databuf, tsigbuf, sizeof(tsigbuf)); + isc_buffer_putuint48(&databuf, tsig.timesigned); + isc_buffer_putuint16(&databuf, tsig.fudge); + isc_buffer_usedregion(&databuf, &r); + CHECK(dst_context_adddata(tsigctx, &r)); + + CHECK(dst_key_sigsize(key->key, &sigsize)); + tsig.signature = (unsigned char *) isc_mem_get(mctx, sigsize); + if (tsig.signature == NULL) + CHECK(ISC_R_NOMEMORY); + isc_buffer_init(&sigbuf, tsig.signature, sigsize); + CHECK(dst_context_sign(tsigctx, &sigbuf)); + tsig.siglen = isc_buffer_usedlength(&sigbuf); + + CHECK(isc_buffer_allocate(mctx, &dynbuf, 512)); + CHECK(dns_rdata_fromstruct(&rdata, dns_rdataclass_any, + dns_rdatatype_tsig, &tsig, dynbuf)); + dns_rdatalist_init(&rdatalist); + rdatalist.rdclass = dns_rdataclass_any; + rdatalist.type = dns_rdatatype_tsig; + ISC_LIST_APPEND(rdatalist.rdata, &rdata, link); + dns_rdataset_init(&rdataset); + CHECK(dns_rdatalist_tordataset(&rdatalist, &rdataset)); + CHECK(dns_rdataset_towire(&rdataset, &key->name, &cctx, + target, 0, &count)); + + /* + * Fixup additional record count. + */ + ((unsigned char*)target->base)[11]++; + if (((unsigned char*)target->base)[11] == 0) + ((unsigned char*)target->base)[10]++; + cleanup: + if (tsig.signature != NULL) + isc_mem_put(mctx, tsig.signature, sigsize); + if (dynbuf != NULL) + isc_buffer_free(&dynbuf); + if (invalidate_ctx) + dns_compress_invalidate(&cctx); + + return (result); +} + +static void +printmessage(dns_message_t *msg) { + isc_buffer_t b; + char *buf = NULL; + int len = 1024; + isc_result_t result = ISC_R_SUCCESS; + + if (!debug) + return; + + do { + buf = isc_mem_get(mctx, len); + if (buf == NULL) { + result = ISC_R_NOMEMORY; + break; + } + + isc_buffer_init(&b, buf, len); + result = dns_message_totext(msg, &dns_master_style_debug, + 0, &b); + if (result == ISC_R_NOSPACE) { + isc_mem_put(mctx, buf, len); + len *= 2; + } else if (result == ISC_R_SUCCESS) + printf("%.*s\n", (int) isc_buffer_usedlength(&b), buf); + } while (result == ISC_R_NOSPACE); + + if (buf != NULL) + isc_mem_put(mctx, buf, len); +} + +static void +render(isc_buffer_t *buf, unsigned flags, dns_tsigkey_t *key, + isc_buffer_t **tsigin, isc_buffer_t **tsigout, + dst_context_t *tsigctx) +{ + dns_message_t *msg = NULL; + dns_compress_t cctx; + isc_result_t result; + + result = dns_message_create(mctx, DNS_MESSAGE_INTENTRENDER, &msg); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_create: %s", + dns_result_totext(result)); + + msg->id = 50; + msg->rcode = dns_rcode_noerror; + msg->flags = flags; + + if (tsigin == tsigout) + msg->tcp_continuation = 1; + + if (tsigctx == NULL) { + result = dns_message_settsigkey(msg, key); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_settsigkey: %s", + dns_result_totext(result)); + + result = dns_message_setquerytsig(msg, *tsigin); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_setquerytsig: %s", + dns_result_totext(result)); + } + + result = dns_compress_init(&cctx, -1, mctx); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_compress_init: %s", + dns_result_totext(result)); + + result = dns_message_renderbegin(msg, &cctx, buf); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_renderbegin: %s", + dns_result_totext(result)); + + result = dns_message_renderend(msg); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_renderend: %s", + dns_result_totext(result)); + + if (tsigctx != NULL) { + isc_region_t r; + + isc_buffer_usedregion(buf, &r); + result = dst_context_adddata(tsigctx, &r); + } else { + if (tsigin == tsigout && *tsigin != NULL) + isc_buffer_free(tsigin); + + result = dns_message_getquerytsig(msg, mctx, tsigout); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_getquerytsig: %s", + dns_result_totext(result)); + } + + dns_compress_invalidate(&cctx); + dns_message_destroy(&msg); +} + +/* + * Check that a simulated three message TCP sequence where the first + * and last messages contain TSIGs but the intermediate message doesn't + * correctly verifies. + */ +ATF_TC(tsig_tcp); +ATF_TC_HEAD(tsig_tcp, tc) { + atf_tc_set_md_var(tc, "descr", "test tsig tcp-continuation validation"); +} +ATF_TC_BODY(tsig_tcp, tc) { + dns_name_t *tsigowner = NULL; + dns_fixedname_t fkeyname; + dns_message_t *msg = NULL; + dns_name_t *keyname; + dns_tsig_keyring_t *ring = NULL; + dns_tsigkey_t *key = NULL; + isc_buffer_t *buf = NULL; + isc_buffer_t *querytsig = NULL; + isc_buffer_t *tsigin = NULL; + isc_buffer_t *tsigout = NULL; + isc_result_t result; + unsigned char secret[16] = { 0 }; + dst_context_t *tsigctx = NULL; + dst_context_t *outctx = NULL; + + UNUSED(tc); + + result = dns_test_begin(stderr, ISC_FALSE); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + /* isc_log_setdebuglevel(lctx, 99); */ + + dns_fixedname_init(&fkeyname); + keyname = dns_fixedname_name(&fkeyname); + result = dns_name_fromstring(keyname, "test", 0, NULL); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + result = dns_tsigkeyring_create(mctx, &ring); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + result = dns_tsigkey_create(keyname, dns_tsig_hmacsha256_name, + secret, sizeof(secret), ISC_FALSE, + NULL, 0, 0, mctx, ring, &key); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + /* + * Create request. + */ + result = isc_buffer_allocate(mctx, &buf, 65535); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + render(buf, 0, key, &tsigout, &querytsig, NULL); + isc_buffer_free(&buf); + + /* + * Create response message 1. + */ + result = isc_buffer_allocate(mctx, &buf, 65535); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + render(buf, DNS_MESSAGEFLAG_QR, key, &querytsig, &tsigout, NULL); + + /* + * Process response message 1. + */ + result = dns_message_create(mctx, DNS_MESSAGE_INTENTPARSE, &msg); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_create: %s", + dns_result_totext(result)); + + result = dns_message_settsigkey(msg, key); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_settsigkey: %s", + dns_result_totext(result)); + + result = dns_message_parse(msg, buf, 0); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_parse: %s", + dns_result_totext(result)); + + printmessage(msg); + + result = dns_message_setquerytsig(msg, querytsig); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_setquerytsig: %s", + dns_result_totext(result)); + + result = dns_tsig_verify(buf, msg, NULL, NULL); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_tsig_verify: %s", + dns_result_totext(result)); + ATF_CHECK_EQ(msg->verified_sig, 1); + ATF_CHECK_EQ(msg->tsigstatus, dns_rcode_noerror); + + /* + * Check that we have a TSIG in the first message. + */ + ATF_REQUIRE(dns_message_gettsig(msg, &tsigowner) != NULL); + + result = dns_message_getquerytsig(msg, mctx, &tsigin); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_getquerytsig: %s", + dns_result_totext(result)); + + tsigctx = msg->tsigctx; + msg->tsigctx = NULL; + isc_buffer_free(&buf); + dns_message_destroy(&msg); + + result = dst_context_create2(key->key, mctx, DNS_LOGCATEGORY_DNSSEC, + &outctx); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + /* + * Start digesting. + */ + result = add_mac(outctx, tsigout); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + /* + * Create response message 2. + */ + result = isc_buffer_allocate(mctx, &buf, 65535); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + render(buf, DNS_MESSAGEFLAG_QR, key, &tsigout, &tsigout, outctx); + + /* + * Process response message 2. + */ + result = dns_message_create(mctx, DNS_MESSAGE_INTENTPARSE, &msg); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_create: %s", + dns_result_totext(result)); + + msg->tcp_continuation = 1; + msg->tsigctx = tsigctx; + tsigctx = NULL; + + result = dns_message_settsigkey(msg, key); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_settsigkey: %s", + dns_result_totext(result)); + + result = dns_message_parse(msg, buf, 0); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_parse: %s", + dns_result_totext(result)); + + printmessage(msg); + + result = dns_message_setquerytsig(msg, tsigin); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_setquerytsig: %s", + dns_result_totext(result)); + + result = dns_tsig_verify(buf, msg, NULL, NULL); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_tsig_verify: %s", + dns_result_totext(result)); + ATF_CHECK_EQ(msg->verified_sig, 1); + ATF_CHECK_EQ(msg->tsigstatus, dns_rcode_noerror); + + /* + * Check that we don't have a TSIG in the second message. + */ + tsigowner = NULL; + ATF_REQUIRE(dns_message_gettsig(msg, &tsigowner) == NULL); + + tsigctx = msg->tsigctx; + msg->tsigctx = NULL; + isc_buffer_free(&buf); + dns_message_destroy(&msg); + + /* + * Create response message 3. + */ + result = isc_buffer_allocate(mctx, &buf, 65535); + ATF_REQUIRE_EQ(result, ISC_R_SUCCESS); + render(buf, DNS_MESSAGEFLAG_QR, key, &tsigout, &tsigout, outctx); + + result = add_tsig(outctx, key, buf); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "add_tsig: %s", + dns_result_totext(result)); + + /* + * Process response message 3. + */ + result = dns_message_create(mctx, DNS_MESSAGE_INTENTPARSE, &msg); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_create: %s", + dns_result_totext(result)); + + msg->tcp_continuation = 1; + msg->tsigctx = tsigctx; + tsigctx = NULL; + + result = dns_message_settsigkey(msg, key); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_settsigkey: %s", + dns_result_totext(result)); + + result = dns_message_parse(msg, buf, 0); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_parse: %s", + dns_result_totext(result)); + + printmessage(msg); + + /* + * Check that we had a TSIG in the third message. + */ + ATF_REQUIRE(dns_message_gettsig(msg, &tsigowner) != NULL); + + result = dns_message_setquerytsig(msg, tsigin); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_setquerytsig: %s", + dns_result_totext(result)); + + result = dns_tsig_verify(buf, msg, NULL, NULL); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_tsig_verify: %s", + dns_result_totext(result)); + ATF_CHECK_EQ(msg->verified_sig, 1); + ATF_CHECK_EQ(msg->tsigstatus, dns_rcode_noerror); + + if (tsigin != NULL) + isc_buffer_free(&tsigin); + + result = dns_message_getquerytsig(msg, mctx, &tsigin); + ATF_CHECK_EQ_MSG(result, ISC_R_SUCCESS, + "dns_message_getquerytsig: %s", + dns_result_totext(result)); + + isc_buffer_free(&buf); + dns_message_destroy(&msg); + + if (outctx != NULL) + dst_context_destroy(&outctx); + if (querytsig != NULL) + isc_buffer_free(&querytsig); + if (tsigin != NULL) + isc_buffer_free(&tsigin); + if (tsigout != NULL) + isc_buffer_free(&tsigout); + if (buf != NULL) + isc_buffer_free(&buf); + if (msg != NULL) + dns_message_destroy(&msg); + if (key != NULL) + dns_tsigkey_detach(&key); + if (ring != NULL) + dns_tsigkeyring_detach(&ring); + dns_test_end(); +} + +/* + * Main + */ +ATF_TP_ADD_TCS(tp) { + ATF_TP_ADD_TC(tp, tsig_tcp); + return (atf_no_error()); +} diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c index 7b91d1e..325c901 100644 --- a/lib/dns/tsig.c +++ b/lib/dns/tsig.c @@ -1535,7 +1535,7 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, msg->verified_sig = 1; ret = ISC_R_SUCCESS; -cleanup_context: + cleanup_context: if (ctx != NULL) dst_context_destroy(&ctx); @@ -1859,8 +1859,14 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { ret = ISC_R_SUCCESS; cleanup_context: - if (msg->tsigctx != NULL) + /* + * Except in error conditions, don't destroy the DST context + * for unsigned messages; it is a running sum till the next + * TSIG signed message. + */ + if ((ret != ISC_R_SUCCESS || has_tsig) && msg->tsigctx != NULL) { dst_context_destroy(&msg->tsigctx); + } cleanup_querystruct: dns_rdata_freestruct(&querytsig); -- 2.9.4