[GH-ISSUE #2812] Change semantics of Proof for empty DS RRsets #1065

New issue

Closed

opened 2026-03-16 01:30:18 +03:00 by kerem · 0 comments

kerem commented

2026-03-16 01:30:18 +03:00

Owner

Originally created by @divergentdave on GitHub (Feb 27, 2025).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/2812

Describe the bug
verify_nsec() and nsec3_validation::validate_nodata_response() each have a special case to return Proof::Insecure instead of Proof::Secure when they authenticate that a DS RRset is empty. Both verify_dnskey_rrset() and find_ds_records() expect this, and use ? to propagate the encapsulating ProofError, so that any responses in unsigned child zones correctly get treated as insecure, not bogus. However, this design wrinkle leaks out when handling queries for DS records, since users of DnssecClient also see this Proof::Insecure.

To Reproduce
See integration test invalid_nsec3_tests::ds_child_zone_no_data_error.

Expected behavior
We should defer this special handling for empty DS RRsets, and return Proof::Secure for the DS query itself if it is successfully validated. This would also mean that we could consolidate all cases that result in Proof::Insecure in one place. (secure empty DS RRset, secure non-empty DS RRset with no supported signature/digest algorithms, or insecure parent zone)

Version:
Crate: hickory-proto
Version: main

Additional context
See #2807.

Originally created by @divergentdave on GitHub (Feb 27, 2025). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/2812 **Describe the bug** `verify_nsec()` and `nsec3_validation::validate_nodata_response()` each have a special case to return `Proof::Insecure` instead of `Proof::Secure` when they authenticate that a DS RRset is empty. Both `verify_dnskey_rrset()` and `find_ds_records()` expect this, and use `?` to propagate the encapsulating `ProofError`, so that any responses in unsigned child zones correctly get treated as insecure, not bogus. However, this design wrinkle leaks out when handling queries for DS records, since users of `DnssecClient` also see this `Proof::Insecure`. **To Reproduce** See integration test `invalid_nsec3_tests::ds_child_zone_no_data_error`. **Expected behavior** We should defer this special handling for empty DS RRsets, and return `Proof::Secure` for the DS query itself if it is successfully validated. This would also mean that we could consolidate all cases that result in `Proof::Insecure` in one place. (secure empty DS RRset, secure non-empty DS RRset with no supported signature/digest algorithms, or insecure parent zone) **Version:** Crate: hickory-proto Version: main **Additional context** See #2807.