diff --git a/bin/tests/system/tsig/tests_badtime.py b/bin/tests/system/tsig/tests_badtime.py new file mode 100644 index 0000000000..2f6d251151 --- /dev/null +++ b/bin/tests/system/tsig/tests_badtime.py @@ -0,0 +1,63 @@ +#!/usr/bin/python3 + +# Copyright (C) Internet Systems Consortium, Inc. ("ISC") +# +# SPDX-License-Identifier: MPL-2.0 +# +# 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 https://mozilla.org/MPL/2.0/. +# +# See the COPYRIGHT file distributed with this work for additional +# information regarding copyright ownership. + +# pylint: disable=unused-variable + +import socket +import time + +import pytest + +pytest.importorskip("dns", minversion="2.0.0") +import dns.message +import dns.query +import dns.tsigkeyring + +TIMEOUT = 10 + + +def create_msg(qname, qtype, edns=-1): + msg = dns.message.make_query(qname, qtype, use_edns=edns) + return msg + + +def timeout(): + return time.time() + TIMEOUT + + +def create_socket(host, port): + sock = socket.create_connection((host, port), timeout=10) + sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, True) + return sock + + +def test_tsig_badtime(named_port): + with create_socket("10.53.0.1", named_port) as sock: + msg = create_msg("a.example.", "A") + + keyring = dns.tsigkeyring.from_text( + { + "sha256": "R16NojROxtxH/xbDl//ehDsHm5DjWTQ2YXV+hGC2iBY=", + } + ) + + msg.use_tsig(keyring, keyname="sha256", fudge=0) + + wire = msg.to_wire() + assert len(wire) > 0 + + time.sleep(3) + + (sbytes, stime) = dns.query.send_tcp(sock, wire, timeout()) + with pytest.raises(dns.tsig.PeerBadTime): + (response, rtime) = dns.query.receive_tcp(sock, timeout(), keyring=keyring) diff --git a/lib/isc/include/isc/buffer.h b/lib/isc/include/isc/buffer.h index 8a2beac8f1..9c4a27d1d3 100644 --- a/lib/isc/include/isc/buffer.h +++ b/lib/isc/include/isc/buffer.h @@ -857,22 +857,21 @@ isc_buffer_getuint8(isc_buffer_t *restrict b) { return (val); } -#define ISC_BUFFER_PUT_RESERVE(b, v) \ - { \ - REQUIRE(ISC_BUFFER_VALID(b)); \ - \ - if (b->mctx) { \ - isc_result_t result = isc_buffer_reserve(b, \ - sizeof(val)); \ - ENSURE(result == ISC_R_SUCCESS); \ - } \ - \ - REQUIRE(isc_buffer_availablelength(b) >= sizeof(val)); \ +#define ISC_BUFFER_PUT_RESERVE(b, v, s) \ + { \ + REQUIRE(ISC_BUFFER_VALID(b)); \ + \ + if (b->mctx) { \ + isc_result_t result = isc_buffer_reserve(b, s); \ + ENSURE(result == ISC_R_SUCCESS); \ + } \ + \ + REQUIRE(isc_buffer_availablelength(b) >= s); \ } static inline void isc_buffer_putuint8(isc_buffer_t *restrict b, const uint8_t val) { - ISC_BUFFER_PUT_RESERVE(b, val); + ISC_BUFFER_PUT_RESERVE(b, val, sizeof(val)); uint8_t *cp = isc_buffer_used(b); b->used += sizeof(val); @@ -900,7 +899,7 @@ isc_buffer_getuint16(isc_buffer_t *restrict b) { static inline void isc_buffer_putuint16(isc_buffer_t *restrict b, const uint16_t val) { - ISC_BUFFER_PUT_RESERVE(b, val); + ISC_BUFFER_PUT_RESERVE(b, val, sizeof(val)); uint8_t *cp = isc_buffer_used(b); b->used += sizeof(val); @@ -928,7 +927,7 @@ isc_buffer_getuint32(isc_buffer_t *restrict b) { static inline void isc_buffer_putuint32(isc_buffer_t *restrict b, const uint32_t val) { - ISC_BUFFER_PUT_RESERVE(b, val); + ISC_BUFFER_PUT_RESERVE(b, val, sizeof(val)); uint8_t *cp = isc_buffer_used(b); b->used += sizeof(val); @@ -957,7 +956,7 @@ isc_buffer_getuint48(isc_buffer_t *restrict b) { static inline void isc_buffer_putuint48(isc_buffer_t *restrict b, const uint64_t val) { - ISC_BUFFER_PUT_RESERVE(b, val); + ISC_BUFFER_PUT_RESERVE(b, val, 6); /* 48-bits */ uint8_t *cp = isc_buffer_used(b); b->used += 6; diff --git a/tests/dns/tsig_test.c b/tests/dns/tsig_test.c index b2efb5407b..95e2c0b52d 100644 --- a/tests/dns/tsig_test.c +++ b/tests/dns/tsig_test.c @@ -24,6 +24,8 @@ #include #include +#include +#include #include #include @@ -94,7 +96,8 @@ cleanup: } static isc_result_t -add_tsig(dst_context_t *tsigctx, dns_tsigkey_t *key, isc_buffer_t *target) { +add_tsig(dst_context_t *tsigctx, dns_tsigkey_t *key, isc_buffer_t *target, + isc_stdtime_t now, bool mangle_sig) { dns_compress_t cctx; dns_rdata_any_tsig_t tsig; dns_rdata_t rdata = DNS_RDATA_INIT; @@ -119,7 +122,7 @@ add_tsig(dst_context_t *tsigctx, dns_tsigkey_t *key, isc_buffer_t *target) { dns_name_init(&tsig.algorithm, NULL); dns_name_clone(key->algorithm, &tsig.algorithm); - tsig.timesigned = isc_stdtime_now(); + tsig.timesigned = now; tsig.fudge = DNS_TSIG_FUDGE; tsig.originalid = 50; tsig.error = dns_rcode_noerror; @@ -138,6 +141,9 @@ add_tsig(dst_context_t *tsigctx, dns_tsigkey_t *key, isc_buffer_t *target) { CHECK(dst_context_sign(tsigctx, &sigbuf)); tsig.siglen = isc_buffer_usedlength(&sigbuf); assert_int_equal(sigsize, tsig.siglen); + if (mangle_sig) { + isc_random_buf(tsig.signature, tsig.siglen); + } isc_buffer_allocate(mctx, &dynbuf, 512); CHECK(dns_rdata_fromstruct(&rdata, dns_rdataclass_any, @@ -260,13 +266,8 @@ render(isc_buffer_t *buf, unsigned int flags, dns_tsigkey_t *key, dns_message_detach(&msg); } -/* - * Test tsig tcp-continuation validation: - * Check that a simulated three message TCP sequence where the first - * and last messages contain TSIGs but the intermediate message doesn't - * correctly verifies. - */ -ISC_RUN_TEST_IMPL(tsig_tcp) { +static void +tsig_tcp(isc_stdtime_t now, isc_result_t expected_result, bool mangle_sig) { const dns_name_t *tsigowner = NULL; dns_fixedname_t fkeyname; dns_message_t *msg = NULL; @@ -282,8 +283,6 @@ ISC_RUN_TEST_IMPL(tsig_tcp) { dst_context_t *tsigctx = NULL; dst_context_t *outctx = NULL; - UNUSED(state); - /* isc_log_setdebuglevel(lctx, 99); */ keyname = dns_fixedname_initname(&fkeyname); @@ -409,7 +408,7 @@ ISC_RUN_TEST_IMPL(tsig_tcp) { isc_buffer_allocate(mctx, &buf, 65535); render(buf, DNS_MESSAGEFLAG_QR, key, &tsigout, &tsigout, outctx); - result = add_tsig(outctx, key, buf); + result = add_tsig(outctx, key, buf, now, mangle_sig); assert_int_equal(result, ISC_R_SUCCESS); /* @@ -438,9 +437,20 @@ ISC_RUN_TEST_IMPL(tsig_tcp) { dns_message_setquerytsig(msg, tsigin); result = dns_tsig_verify(buf, msg, NULL, NULL); - assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(msg->verified_sig, 1); - assert_int_equal(msg->tsigstatus, dns_rcode_noerror); + switch (expected_result) { + case ISC_R_SUCCESS: + assert_int_equal(result, ISC_R_SUCCESS); + assert_int_equal(msg->verified_sig, 1); + assert_int_equal(msg->tsigstatus, dns_rcode_noerror); + break; + case DNS_R_CLOCKSKEW: + assert_int_equal(result, DNS_R_CLOCKSKEW); + assert_int_equal(msg->verified_sig, 1); + assert_int_equal(msg->tsigstatus, dns_tsigerror_badtime); + break; + default: + UNREACHABLE(); + } if (tsigin != NULL) { isc_buffer_free(&tsigin); @@ -470,6 +480,29 @@ ISC_RUN_TEST_IMPL(tsig_tcp) { } } +/* + * Test tsig tcp-continuation validation: + * Check that a simulated three message TCP sequence where the first + * and last messages contain TSIGs but the intermediate message doesn't + * correctly verifies. + */ +ISC_RUN_TEST_IMPL(tsig_tcp) { + /* Run with correct current time */ + tsig_tcp(isc_stdtime_now(), ISC_R_SUCCESS, false); +} + +ISC_RUN_TEST_IMPL(tsig_badtime) { + /* Run with time outside of the fudge */ + tsig_tcp(isc_stdtime_now() - 2 * DNS_TSIG_FUDGE, DNS_R_CLOCKSKEW, + false); + tsig_tcp(isc_stdtime_now() + 2 * DNS_TSIG_FUDGE, DNS_R_CLOCKSKEW, + false); +} + +ISC_RUN_TEST_IMPL(tsig_badsig) { + tsig_tcp(isc_stdtime_now(), DNS_R_TSIGERRORSET, true); +} + /* Tests the dns__tsig_algvalid function */ ISC_RUN_TEST_IMPL(algvalid) { UNUSED(state); @@ -487,6 +520,7 @@ ISC_RUN_TEST_IMPL(algvalid) { ISC_TEST_LIST_START ISC_TEST_ENTRY_CUSTOM(tsig_tcp, setup_test, teardown_test) +ISC_TEST_ENTRY_CUSTOM(tsig_badtime, setup_test, teardown_test) ISC_TEST_ENTRY(algvalid) ISC_TEST_LIST_END