From f46a81932fd30ba19598682ddb0f3b9fa0e9b1dd Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 29 Jun 2023 09:15:38 +1000 Subject: [PATCH 1/2] Address race in dns_tsigkey_find() Restart the process with a write lock if we discover an expired key while holding the read lock. (cherry picked from commit d2ba96488ea30eaeb225c12ea77e71de399b6175) --- lib/dns/tsig.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c index 857ec4cfcd..8f96008e31 100644 --- a/lib/dns/tsig.c +++ b/lib/dns/tsig.c @@ -1757,8 +1757,9 @@ isc_result_t dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, const dns_name_t *algorithm, dns_tsig_keyring_t *ring) { dns_tsigkey_t *key; - isc_stdtime_t now; isc_result_t result; + isc_rwlocktype_t locktype = isc_rwlocktype_read; + isc_stdtime_t now; REQUIRE(tsigkey != NULL); REQUIRE(*tsigkey == NULL); @@ -1770,25 +1771,30 @@ dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, RWUNLOCK(&ring->lock, isc_rwlocktype_write); isc_stdtime_get(&now); - RWLOCK(&ring->lock, isc_rwlocktype_read); + +again: + RWLOCK(&ring->lock, locktype); key = NULL; result = dns_rbt_findname(ring->keys, name, 0, NULL, (void *)&key); if (result == DNS_R_PARTIALMATCH || result == ISC_R_NOTFOUND) { - RWUNLOCK(&ring->lock, isc_rwlocktype_read); + RWUNLOCK(&ring->lock, locktype); return (ISC_R_NOTFOUND); } if (algorithm != NULL && !dns_name_equal(key->algorithm, algorithm)) { - RWUNLOCK(&ring->lock, isc_rwlocktype_read); + RWUNLOCK(&ring->lock, locktype); return (ISC_R_NOTFOUND); } if (key->inception != key->expire && isc_serial_lt(key->expire, now)) { /* * The key has expired. */ - RWUNLOCK(&ring->lock, isc_rwlocktype_read); - RWLOCK(&ring->lock, isc_rwlocktype_write); + if (locktype == isc_rwlocktype_read) { + RWUNLOCK(&ring->lock, locktype); + locktype = isc_rwlocktype_write; + goto again; + } remove_fromring(key); - RWUNLOCK(&ring->lock, isc_rwlocktype_write); + RWUNLOCK(&ring->lock, locktype); return (ISC_R_NOTFOUND); } #if 0 @@ -1803,7 +1809,7 @@ dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, } #endif /* if 0 */ isc_refcount_increment(&key->refs); - RWUNLOCK(&ring->lock, isc_rwlocktype_read); + RWUNLOCK(&ring->lock, locktype); adjust_lru(key); *tsigkey = key; return (ISC_R_SUCCESS); From 0a8367e17b28a772bdff39922e2e19747263b654 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 29 Jun 2023 09:27:29 +1000 Subject: [PATCH 2/2] Add CHANGES note for [GL #4182] (cherry picked from commit a62cda787ffdced3fc2ae31d8da86cba17a3f95f) --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index 6faa12370c..d209c80678 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6314. [bug] Address race conditions in dns_tsigkey_find(). + [GL #4182] + 6312. [bug] Conversion from NSEC3 signed to NSEC signed could temporarily put the zone into a state where it was treated as unsigned until the NSEC chain was built.