From 8581b5d6caca498d23aaa7d0f9df3916ec2ddaa9 Mon Sep 17 00:00:00 2001 From: adityaanikam Date: Sun, 16 Aug 2026 01:18:27 +0530 Subject: [PATCH] Verify TSIG MAC before checking timestamp so BadSignature is not preempted by BadTime --- dns/tsig.py | 4 ++-- tests/test_tsig.py | 42 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/dns/tsig.py b/dns/tsig.py index ce736f1ea..68e9fecdd 100644 --- a/dns/tsig.py +++ b/dns/tsig.py @@ -303,14 +303,14 @@ def validate( raise PeerBadTruncation else: raise PeerError(f"unknown TSIG error code {rdata.error}") - if abs(rdata.time_signed - now) > rdata.fudge: - raise BadTime if key.name != owner: raise BadKey if key.algorithm != rdata.algorithm: raise BadAlgorithm ctx = _digest(new_wire, key, rdata, None, request_mac, ctx, multi) ctx.verify(rdata.mac) + if abs(rdata.time_signed - now) > rdata.fudge: + raise BadTime return _maybe_start_digest(key, rdata.mac, multi) diff --git a/tests/test_tsig.py b/tests/test_tsig.py index c8c1efe53..9c0c2cdbf 100644 --- a/tests/test_tsig.py +++ b/tests/test_tsig.py @@ -3,7 +3,7 @@ import base64 import time import unittest -from unittest.mock import Mock +from unittest.mock import Mock, patch import dns.message import dns.rcode @@ -48,23 +48,42 @@ def test_verify_mac_for_context(self): with self.assertRaises(dns.tsig.BadSignature): ctx.verify(bad_expected) + def _tsig_start(self, wire, kr): + # dns.tsig.validate() needs the exact byte offset of the TSIG + # record, but nothing in the public API exposes it directly. + # dns.message.from_wire() computes it correctly during parsing, + # so capture the value it uses via the same call. + captured = {} + real_validate = dns.tsig.validate + + def spy(*args, **kwargs): + captured["tsig_start"] = args[6] + return real_validate(*args, **kwargs) + + with patch("dns.tsig.validate", side_effect=spy): + dns.message.from_wire(wire, kr) + return captured["tsig_start"] + def test_validate(self): # make message and grab the TSIG m = dns.message.make_query("example", "a") m.use_tsig(keyring, keyname, algorithm=dns.tsig.HMAC_SHA256) w = m.to_wire() tsig = m.tsig[0] + tsig_start = self._tsig_start(w, keyring) - # get the time and create a key with matching characteristics + # get the time and use the correct key so only the timestamp is wrong now = int(time.time()) - key = dns.tsig.Key("foo.com", "abcd", "hmac-sha256") + correct_key = dns.tsig.Key(keyname, keyring[keyname], dns.tsig.HMAC_SHA256) # add enough to the time to take it over the fudge amount with self.assertRaises(dns.tsig.BadTime): dns.tsig.validate( - w, key, dns.name.from_text("foo.com"), tsig, now + 1000, b"", 0 + w, correct_key, keyname, tsig, now + 1000, b"", tsig_start ) + key = dns.tsig.Key("foo.com", "abcd", "hmac-sha256") + # change the key name with self.assertRaises(dns.tsig.BadKey): dns.tsig.validate(w, key, dns.name.from_text("bar.com"), tsig, now, b"", 0) @@ -74,6 +93,21 @@ def test_validate(self): with self.assertRaises(dns.tsig.BadAlgorithm): dns.tsig.validate(w, key, dns.name.from_text("foo.com"), tsig, now, b"", 0) + def test_validate_bad_signature_takes_precedence_over_bad_time(self): + # https://github.com/rthalley/dnspython/issues/1287 + # RFC 8945 5.2: MAC validity must be established before other + # errors like a bad timestamp are reported. + m = dns.message.make_query("example", "a") + m.use_tsig(keyring, keyname, algorithm=dns.tsig.HMAC_SHA256) + w = m.to_wire() + tsig = m.tsig[0] + tsig_start = self._tsig_start(w, keyring) + + now = int(time.time()) + wrong_key = dns.tsig.Key(keyname, "abcd", "hmac-sha256") + with self.assertRaises(dns.tsig.BadSignature): + dns.tsig.validate(w, wrong_key, keyname, tsig, now + 1000, b"", tsig_start) + def test_gssapi_context(self): def verify_signature(data, mac): if data == b"throw":