[PR #3349] [MERGED] Better NSEC and NSEC3 wildcard expansion validation #3765

New issue

Closed

opened 2026-03-16 12:01:35 +03:00 by kerem · 0 comments

kerem commented

2026-03-16 12:01:35 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/hickory-dns/hickory-dns/pull/3349
Author: @marcus0x62
Created: 11/14/2025
Status: ✅ Merged
Merged: 11/25/2025
Merged by: @marcus0x62

Base: main ← Head: nsec-nsec3-wildcard-validation

📝 Commits (10+)

2ede286 server: return covering NSEC records for wildcard requests
62e94cb server: expand rrsig name for wildcard expanded records
d03c604 proto: return Option<&Name> in find_soa_name
811ed4e proto: make SOA name optional in verify_nsec
7c59271 proto: unit tests for verify_nsec
ab54f26 proto: NSEC verification for wildcard expansion responses
f2b1b29 tests: only look for NSEC records in authorities section
d76963c tests: apply invalid nsec test
e4c52d2 proto: closer wildcard verification for NSEC responses
9e1a93b proto: make SOA argument optional in NSEC3 verification

📊 Changes

6 files changed (+1618 additions, -393 deletions)

View changed files

📝 crates/proto/src/dnssec/handle.rs (+831 -83)
📝 crates/proto/src/dnssec/nsec3.rs (+741 -277)
📝 crates/server/src/store/in_memory/inner.rs (+5 -1)
📝 crates/server/src/zone_handler/catalog.rs (+39 -29)
📝 tests/integration-tests/tests/integration/invalid_nsec3_tests.rs (+0 -1)
📝 tests/integration-tests/tests/integration/invalid_nsec_tests.rs (+2 -2)

📄 Description

Addresses #2882 - insufficient validation of wildcard expanded positive responses. So, high level, this required the following changes:

Return NSEC records for wildcard-expanded responses from the server (necessary for our tests.) This was done for NSEC3 already.
Make the SOA record optional in verify_nsec and verify_nsec_3 - the SOA record isn't returned in all cases that need NSEC/NSEC3 verification
Changes to verify wildcard expanded responses in the NSEC code
Fix a bug in (one of the) NSEC3 closest encloser proof functions that looked for a covering record the the closest encloser name (instead of a matching record.)
Type map verification in NSEC3 wildcard no data responses
Fix a bug in the nsec3 find_covering_record function where it would accept matching records.

Not strictly necessary, but:

I found two separate implementations of the closest encloser proof in the NSEC3 code, each with a commingled wildcard proof (one for NXDomain responses, one for no data responses.) I simplified that into one implementation of the closest encloser proof and broke out the wildcard proof into a separate function.
Around 1,100 lines of this PR is unit tests, mostly for verify_nsec and verify_nsec3. I think testing those directly makes sense given the security impact of these functions, and the fact that conformance level tests directed at these functions could pass or fail for a lot of reasons not directly related to the correctness of the NSEC/NSEC3 implementations...

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/hickory-dns/hickory-dns/pull/3349 **Author:** [@marcus0x62](https://github.com/marcus0x62) **Created:** 11/14/2025 **Status:** ✅ Merged **Merged:** 11/25/2025 **Merged by:** [@marcus0x62](https://github.com/marcus0x62) **Base:** `main` ← **Head:** `nsec-nsec3-wildcard-validation` --- ### 📝 Commits (10+) - [`2ede286`](https://github.com/hickory-dns/hickory-dns/commit/2ede2864232d4d97490fb343e330b39460ebedda) server: return covering NSEC records for wildcard requests - [`62e94cb`](https://github.com/hickory-dns/hickory-dns/commit/62e94cb4c01f845182e8b227435d42e0bfbcb580) server: expand rrsig name for wildcard expanded records - [`d03c604`](https://github.com/hickory-dns/hickory-dns/commit/d03c604813bbb624c577ee7980214402c78903ff) proto: return Option<&Name> in find_soa_name - [`811ed4e`](https://github.com/hickory-dns/hickory-dns/commit/811ed4e088a026e6e6013d4baf0799848c7d01a4) proto: make SOA name optional in verify_nsec - [`7c59271`](https://github.com/hickory-dns/hickory-dns/commit/7c592714d5a277c9b1d78a5e843372adeec21262) proto: unit tests for verify_nsec - [`ab54f26`](https://github.com/hickory-dns/hickory-dns/commit/ab54f2652abcfc33386e964e33ca0ef3fecba724) proto: NSEC verification for wildcard expansion responses - [`f2b1b29`](https://github.com/hickory-dns/hickory-dns/commit/f2b1b294477de916b516b211dc6d6faa1da843e8) tests: only look for NSEC records in authorities section - [`d76963c`](https://github.com/hickory-dns/hickory-dns/commit/d76963ca3435ef510242f03fcb2d23b17c4c6d1b) tests: apply invalid nsec test - [`e4c52d2`](https://github.com/hickory-dns/hickory-dns/commit/e4c52d2253d99369ebfffa50512c6a824be9c2d0) proto: closer wildcard verification for NSEC responses - [`9e1a93b`](https://github.com/hickory-dns/hickory-dns/commit/9e1a93bf686c7230e316fa7b3dd6643abb2cc0af) proto: make SOA argument optional in NSEC3 verification ### 📊 Changes **6 files changed** (+1618 additions, -393 deletions) <details> <summary>View changed files</summary> 📝 `crates/proto/src/dnssec/handle.rs` (+831 -83) 📝 `crates/proto/src/dnssec/nsec3.rs` (+741 -277) 📝 `crates/server/src/store/in_memory/inner.rs` (+5 -1) 📝 `crates/server/src/zone_handler/catalog.rs` (+39 -29) 📝 `tests/integration-tests/tests/integration/invalid_nsec3_tests.rs` (+0 -1) 📝 `tests/integration-tests/tests/integration/invalid_nsec_tests.rs` (+2 -2) </details> ### 📄 Description Addresses #2882 - insufficient validation of wildcard expanded positive responses. So, high level, this required the following changes: * Return NSEC records for wildcard-expanded responses from the server (necessary for our tests.) This was done for NSEC3 already. * Make the SOA record optional in verify_nsec and verify_nsec_3 - the SOA record isn't returned in all cases that need NSEC/NSEC3 verification * Changes to verify wildcard expanded responses in the NSEC code * Fix a bug in (one of the) NSEC3 closest encloser proof functions that looked for a covering record the the closest encloser name (instead of a matching record.) * Type map verification in NSEC3 wildcard no data responses * Fix a bug in the nsec3 find_covering_record function where it would accept matching records. Not strictly necessary, but: * I found two separate implementations of the closest encloser proof in the NSEC3 code, each with a commingled wildcard proof (one for NXDomain responses, one for no data responses.) I simplified that into one implementation of the closest encloser proof and broke out the wildcard proof into a separate function. * Around 1,100 lines of this PR is unit tests, mostly for verify_nsec and verify_nsec3. I think testing those directly makes sense given the security impact of these functions, and the fact that conformance level tests directed at these functions could pass or fail for a lot of reasons not directly related to the correctness of the NSEC/NSEC3 implementations... --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>