ComputerSurge/bind9
3
0
Fork 0
Code Pull Requests 1 Activity

[9.20] fix: usr: Valid TSIG signatures with invalid time cause crash

Browse Source 
An assertion failure triggers when the TSIG has valid cryptographic signature, but the time is invalid. This can happen when the times between the primary and secondary servers are not synchronised.

Closes #4811

Backport of MR !9234

Merge branch 'backport-4811-fix-isc_buffer_putuint48-buffer-size-requirement-9.20' into 'bind-9.20'

See merge request isc-projects/bind9!9245
This commit is contained in:
Ondřej Surý
2024-08-05 11:11:47 +00:00
parent 4f369af51e ac170e8c5b
commit 2438db2eae
3 changed files with 126 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

63
bin/tests/system/tsig/tests_badtime.py Normal file
View File

@@ -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)

29
lib/isc/include/isc/buffer.h
View File

@@ -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;

64
tests/dns/tsig_test.c
View File

@@ -24,6 +24,8 @@
#include <cmocka.h>
#include <isc/mem.h>
#include <isc/random.h>
#include <isc/result.h>
#include <isc/util.h>
#include <dns/rdatalist.h>
@@ -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