[PR #2807] [MERGED] Additional NSEC3 tests and improvements #3339

New issue

Closed

opened 2026-03-16 11:38:12 +03:00 by kerem · 0 comments

kerem commented

2026-03-16 11:38:12 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/hickory-dns/hickory-dns/pull/2807
Author: @divergentdave
Created: 2/26/2025
Status: ✅ Merged
Merged: 2/28/2025
Merged by: @divergentdave

Base: main ← Head: david/nsec3-tests

📝 Commits (7)

7b08733 Delete old ignored/commented out tests
5f77e1e Add NXDOMAIN scenario tests
56e4997 Fix Display format of NSEC3 records
9b629c0 Remove FIXME for NSEC3 collision detection
238ee70 Exclude non-authoritative records from NSEC3 chain
49a973a Add support for signing Opt-Out zones
b0d4049 Add tests for NSEC3 validation w/ omitted records

📊 Changes

14 files changed (+965 additions, -84 deletions)

View changed files

📝 Cargo.lock (+1 -0)
📝 conformance/packages/conformance-tests/src/resolver/dnssec/fixtures.rs (+2 -6)
📝 conformance/packages/conformance-tests/src/resolver/dnssec/rfc4035/section_4/section_4_5.rs (+1 -1)
📝 conformance/packages/conformance-tests/src/resolver/dnssec/rfc4035/section_4/section_4_6.rs (+2 -1)
📝 conformance/packages/conformance-tests/src/resolver/dnssec/scenarios/secure.rs (+73 -9)
📝 crates/proto/src/dnssec/rdata/nsec3.rs (+1 -1)
📝 crates/server/src/authority/catalog.rs (+2 -0)
📝 crates/server/src/lib.rs (+3 -0)
📝 crates/server/src/store/in_memory/authority.rs (+36 -13)
📝 tests/integration-tests/Cargo.toml (+1 -0)
📝 tests/integration-tests/tests/integration/catalog_tests.rs (+1 -0)
📝 tests/integration-tests/tests/integration/client_tests.rs (+0 -53)
➕ tests/integration-tests/tests/integration/invalid_nsec3_tests.rs (+841 -0)
📝 tests/integration-tests/tests/integration/main.rs (+1 -0)

📄 Description

This makes the following improvements:

Fix the presentation format of NSEC3 records to use the correct base32 encoding alphabet for the next hashed owner name.
Exclude non-authoritative records from the NSEC3 chain.
Add support for signing zones with NSEC3 Opt-Out records. (this is needed to align with the example zone in RFC 5155 appendix B, see below)

Two conformance tests are added to fulfill TODOs. A new set of integration tests check that NSEC3 validation fails when any one required NSEC3 record is missing. This is based on the example zone and queries from the appendixes of RFC 5155. I didn't find any NSEC3 validation issues with this, but half of the tests did not produce the correct response before omitting NSEC3 records due to three different issues.

I also cleaned up some commented-out and ignored tests that have since been made redundant, and removed a FIXME regarding NSEC3 collision detection. The probability of a collision between NSEC3 records is astronomically unlikely, so it's not necessary to implement a countermeasure like changing the salt.

_{🔄 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/2807 **Author:** [@divergentdave](https://github.com/divergentdave) **Created:** 2/26/2025 **Status:** ✅ Merged **Merged:** 2/28/2025 **Merged by:** [@divergentdave](https://github.com/divergentdave) **Base:** `main` ← **Head:** `david/nsec3-tests` --- ### 📝 Commits (7) - [`7b08733`](https://github.com/hickory-dns/hickory-dns/commit/7b0873355b0f115832b9d25e0ac4a124b0d33dbd) Delete old ignored/commented out tests - [`5f77e1e`](https://github.com/hickory-dns/hickory-dns/commit/5f77e1e76b56350c1e421fc137db04e496e1e963) Add NXDOMAIN scenario tests - [`56e4997`](https://github.com/hickory-dns/hickory-dns/commit/56e4997cb2d482412ffb0807c423b37be5a70c83) Fix Display format of NSEC3 records - [`9b629c0`](https://github.com/hickory-dns/hickory-dns/commit/9b629c01b89c316814a618feebfbbcd6219ba4a8) Remove FIXME for NSEC3 collision detection - [`238ee70`](https://github.com/hickory-dns/hickory-dns/commit/238ee70e8a9a200dd9ff848baf1fb86bdbad271b) Exclude non-authoritative records from NSEC3 chain - [`49a973a`](https://github.com/hickory-dns/hickory-dns/commit/49a973a00adb4a00c01be1505cf9a8e3cf1fa08e) Add support for signing Opt-Out zones - [`b0d4049`](https://github.com/hickory-dns/hickory-dns/commit/b0d40494cd61c6e7541c2d609b986634deb75f7d) Add tests for NSEC3 validation w/ omitted records ### 📊 Changes **14 files changed** (+965 additions, -84 deletions) <details> <summary>View changed files</summary> 📝 `Cargo.lock` (+1 -0) 📝 `conformance/packages/conformance-tests/src/resolver/dnssec/fixtures.rs` (+2 -6) 📝 `conformance/packages/conformance-tests/src/resolver/dnssec/rfc4035/section_4/section_4_5.rs` (+1 -1) 📝 `conformance/packages/conformance-tests/src/resolver/dnssec/rfc4035/section_4/section_4_6.rs` (+2 -1) 📝 `conformance/packages/conformance-tests/src/resolver/dnssec/scenarios/secure.rs` (+73 -9) 📝 `crates/proto/src/dnssec/rdata/nsec3.rs` (+1 -1) 📝 `crates/server/src/authority/catalog.rs` (+2 -0) 📝 `crates/server/src/lib.rs` (+3 -0) 📝 `crates/server/src/store/in_memory/authority.rs` (+36 -13) 📝 `tests/integration-tests/Cargo.toml` (+1 -0) 📝 `tests/integration-tests/tests/integration/catalog_tests.rs` (+1 -0) 📝 `tests/integration-tests/tests/integration/client_tests.rs` (+0 -53) ➕ `tests/integration-tests/tests/integration/invalid_nsec3_tests.rs` (+841 -0) 📝 `tests/integration-tests/tests/integration/main.rs` (+1 -0) </details> ### 📄 Description This makes the following improvements: * Fix the presentation format of NSEC3 records to use the correct base32 encoding alphabet for the next hashed owner name. * Exclude non-authoritative records from the NSEC3 chain. * Add support for signing zones with NSEC3 Opt-Out records. (this is needed to align with the example zone in RFC 5155 appendix B, see below) Two conformance tests are added to fulfill TODOs. A new set of integration tests check that NSEC3 validation fails when any one required NSEC3 record is missing. This is based on the example zone and queries from the appendixes of RFC 5155. I didn't find any NSEC3 validation issues with this, but half of the tests did not produce the correct response before omitting NSEC3 records due to three different issues. I also cleaned up some commented-out and ignored tests that have since been made redundant, and removed a FIXME regarding NSEC3 collision detection. The probability of a collision between NSEC3 records is astronomically unlikely, so it's not necessary to implement a countermeasure like changing the salt. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>