[PR #3175] [MERGED] Fix NSEC validation #3611

New issue

Closed

opened 2026-03-16 11:53:09 +03:00 by kerem · 0 comments

kerem commented

2026-03-16 11:53:09 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/hickory-dns/hickory-dns/pull/3175
Author: @divergentdave
Created: 8/1/2025
Status: ✅ Merged
Merged: 8/15/2025
Merged by: @divergentdave

Base: main ← Head: david/nsec-validation

📝 Commits (7)

4c76c7d Remove verify_nsec() from public API
d996290 Rewrite verify_nsec()
adde14a Simplify NSEC3 validation with Name::prepend_label
51b463e Check response code
6671b7a Extract closure for handling NSEC record match
094d0ad Change proof_log_yield() to take the whole query
ec1a0ed Add wrappers around proof_log_yield()

📊 Changes

6 files changed (+212 additions, -205 deletions)

View changed files

📝 bin/tests/integration/authority_battery/dnssec.rs (+41 -65)
📝 conformance/packages/conformance-tests/src/resolver/dnssec/scenarios/bogus.rs (+0 -1)
📝 conformance/packages/conformance-tests/src/resolver/nsec.rs (+0 -2)
📝 crates/proto/src/dnssec/dnssec_dns_handle/mod.rs (+126 -62)
📝 crates/proto/src/dnssec/dnssec_dns_handle/nsec3_validation.rs (+45 -73)
📝 crates/proto/src/dnssec/mod.rs (+0 -2)

📄 Description

This closes #3143. I rewrote verify_nsec() to address the problems identified in that issue. I also made the function private, and rewrote an integration test that was depending on it.

The general flow of this function is now to check for an exact match, check for a covering record, identify the next closest enclosing name from the ancestors of the names in the covering NSEC record, and finally check for any NSEC record matching or covering the wildcard name below that next closest enclosing name. There isn't any verification algorithm suggested in RFC 4035, but my thinking is that this will identify where the RFC 1034 algorithm would have failed to find a name and checked for a wildcard. Code paths that return Proof::Secure are now properly guarded by checks of record types, when dealing with a direct match (including CNAME), and checks for appropriate response codes.

This fixes three conformance tests, added in #3172 and #3147.

_{🔄 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/3175 **Author:** [@divergentdave](https://github.com/divergentdave) **Created:** 8/1/2025 **Status:** ✅ Merged **Merged:** 8/15/2025 **Merged by:** [@divergentdave](https://github.com/divergentdave) **Base:** `main` ← **Head:** `david/nsec-validation` --- ### 📝 Commits (7) - [`4c76c7d`](https://github.com/hickory-dns/hickory-dns/commit/4c76c7d6f070cfd6efeec57792a8f5d9e7be7937) Remove verify_nsec() from public API - [`d996290`](https://github.com/hickory-dns/hickory-dns/commit/d99629092067f44cbf41a7722fb3c454342a9441) Rewrite verify_nsec() - [`adde14a`](https://github.com/hickory-dns/hickory-dns/commit/adde14a578cacec84baf4f7334079b813e72173e) Simplify NSEC3 validation with Name::prepend_label - [`51b463e`](https://github.com/hickory-dns/hickory-dns/commit/51b463e5f4b4feb47db215ade4b933adf874238a) Check response code - [`6671b7a`](https://github.com/hickory-dns/hickory-dns/commit/6671b7a145a13636ab81521309fa6617ed75a66a) Extract closure for handling NSEC record match - [`094d0ad`](https://github.com/hickory-dns/hickory-dns/commit/094d0ad4a3aefbf3d6703296b7fe5ac57f80975d) Change proof_log_yield() to take the whole query - [`ec1a0ed`](https://github.com/hickory-dns/hickory-dns/commit/ec1a0ed0b7ee2b29aa3be2f51adec9403009beab) Add wrappers around proof_log_yield() ### 📊 Changes **6 files changed** (+212 additions, -205 deletions) <details> <summary>View changed files</summary> 📝 `bin/tests/integration/authority_battery/dnssec.rs` (+41 -65) 📝 `conformance/packages/conformance-tests/src/resolver/dnssec/scenarios/bogus.rs` (+0 -1) 📝 `conformance/packages/conformance-tests/src/resolver/nsec.rs` (+0 -2) 📝 `crates/proto/src/dnssec/dnssec_dns_handle/mod.rs` (+126 -62) 📝 `crates/proto/src/dnssec/dnssec_dns_handle/nsec3_validation.rs` (+45 -73) 📝 `crates/proto/src/dnssec/mod.rs` (+0 -2) </details> ### 📄 Description This closes #3143. I rewrote `verify_nsec()` to address the problems identified in that issue. I also made the function private, and rewrote an integration test that was depending on it. The general flow of this function is now to check for an exact match, check for a covering record, identify the next closest enclosing name from the ancestors of the names in the covering NSEC record, and finally check for any NSEC record matching or covering the wildcard name below that next closest enclosing name. There isn't any verification algorithm suggested in RFC 4035, but my thinking is that this will identify where the RFC 1034 algorithm would have failed to find a name and checked for a wildcard. Code paths that return `Proof::Secure` are now properly guarded by checks of record types, when dealing with a direct match (including CNAME), and checks for appropriate response codes. This fixes three conformance tests, added in #3172 and #3147. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>