[PR #2763] [MERGED] DNSSEC validation cleanup #3295

New issue

Closed

opened 2026-03-16 11:36:02 +03:00 by kerem · 0 comments

kerem commented

2026-03-16 11:36:02 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/hickory-dns/hickory-dns/pull/2763
Author: @divergentdave
Created: 2/10/2025
Status: ✅ Merged
Merged: 2/11/2025
Merged by: @divergentdave

Base: main ← Head: david/dnssec-validation-cleanup

📝 Commits (3)

5277c7e Remove dead code
f5cdad0 Replace uses of Indeterminate with Bogus
74d58f4 Don't check if all DNSKEYs use unsupported algorithms

📊 Changes

1 file changed (+25 additions, -77 deletions)

View changed files

📝 crates/proto/src/dnssec/dnssec_dns_handle/mod.rs (+25 -77)

📄 Description

This cleans up the DNSSEC validation logic after recent fixes.

The first commit removes dead code in three places. These branches are never taken because an Rrset is never empty, and find_ds_records() never returns Ok with an empty vector.

The second commit replaces all remaining uses of Proof::Indeterminate with Proof::Bogus. I don't fully understand the RFC 4035 definition of Indeterminate, but the RFC 4033 definition says "There is no trust anchor that woud indicate that a specific portion of the tree is secure. This is the default operation mode." These days, the root zone is signed, so islands of security are much less common. Hickory DNS only has support for specifying a trust anchor for the root zone, so we shouldn't be using this status when operating in a validating mode. This has no impact on verify_dnskey_rrset() which was only checking is_secure() on the proofs. This may change the behavior of verify_default_rrset() if all DNSKEY lookups return an error.

The third commit removes logic checking whether all DNSKEY records use unsupported signature algorithms. We separately check if all DS records use unsupported signature algorithms, and only the DS records are authenticated at this point. Thus, looking at the DNSKEY RRset is redundant and not sound.

_{🔄 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/2763 **Author:** [@divergentdave](https://github.com/divergentdave) **Created:** 2/10/2025 **Status:** ✅ Merged **Merged:** 2/11/2025 **Merged by:** [@divergentdave](https://github.com/divergentdave) **Base:** `main` ← **Head:** `david/dnssec-validation-cleanup` --- ### 📝 Commits (3) - [`5277c7e`](https://github.com/hickory-dns/hickory-dns/commit/5277c7e2593c010ab64330e852df34a101347d2d) Remove dead code - [`f5cdad0`](https://github.com/hickory-dns/hickory-dns/commit/f5cdad0e47a59709d56fb88680dfcf292572cb3b) Replace uses of Indeterminate with Bogus - [`74d58f4`](https://github.com/hickory-dns/hickory-dns/commit/74d58f407b8b890564df7fa962cf68b84f5ec93d) Don't check if all DNSKEYs use unsupported algorithms ### 📊 Changes **1 file changed** (+25 additions, -77 deletions) <details> <summary>View changed files</summary> 📝 `crates/proto/src/dnssec/dnssec_dns_handle/mod.rs` (+25 -77) </details> ### 📄 Description This cleans up the DNSSEC validation logic after recent fixes. The first commit removes dead code in three places. These branches are never taken because an `Rrset` is never empty, and `find_ds_records()` never returns `Ok` with an empty vector. The second commit replaces all remaining uses of `Proof::Indeterminate` with `Proof::Bogus`. I don't fully understand the RFC 4035 definition of Indeterminate, but the RFC 4033 definition says "There is no trust anchor that woud indicate that a specific portion of the tree is secure. This is the default operation mode." These days, the root zone is signed, so islands of security are much less common. Hickory DNS only has support for specifying a trust anchor for the root zone, so we shouldn't be using this status when operating in a validating mode. This has no impact on `verify_dnskey_rrset()` which was only checking `is_secure()` on the proofs. This may change the behavior of `verify_default_rrset()` if all DNSKEY lookups return an error. The third commit removes logic checking whether all DNSKEY records use unsupported signature algorithms. We separately check if all DS records use unsupported signature algorithms, and only the DS records are authenticated at this point. Thus, looking at the DNSKEY RRset is redundant and not sound. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>