[PR #2682] [MERGED] resolver: never use truncated UDP response #3231

New issue

Closed

opened 2026-03-16 11:32:31 +03:00 by kerem · 0 comments

kerem commented

2026-03-16 11:32:31 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/hickory-dns/hickory-dns/pull/2682
Author: @divergentdave
Created: 12/16/2024
Status: ✅ Merged
Merged: 12/18/2024
Merged by: @marcus0x62

Base: main ← Head: david/resolver-truncation-fix

📝 Commits (1)

0002736 resolver: never use truncated UDP response

📊 Changes

3 files changed (+5 additions, -20 deletions)

View changed files

📝 conformance/packages/conformance-tests/src/resolver/dns/rfc1035/truncation.rs (+0 -1)
📝 crates/resolver/src/name_server/name_server_pool.rs (+2 -18)
📝 crates/resolver/src/resolver.rs (+3 -1)

📄 Description

This changes NameServerPool to only return data from a TCP response if the UDP response was truncated. This fixes #2608, and fixes NSEC/NSEC3 validation in some cases. I think we should not return truncated responses in this context, even if that's the only response available, because it is presently the cause of multiple correctness issues, and because this PR's behavior aligns with other implementations, like BIND and Unbound's recursive resolvers. There are some cases where it would be possible to implement optimizations making use of truncated responses, such as eagerly connecting to nameservers during iterative recursion before receiving complete referral responses, but I think we'd need to pierce existing encapsulation, and make more of the codebase truncation-aware. Plus, ProtoError makes reasoning about this behavior harder.

When handling a truncated UDP response for a query of an empty RRset, ProtoError::from_response() returns Ok(response), because of a check for the truncation bit. NameServerPool also checks the truncation bit on the response, and sends a TCP query. Subsequently, ProtoError::from_response() turns the TCP response into Err(ProtoErrorKind::NoRecordsFound { .. }.into()). Between these two choices, the existing code will return the Ok(...) containing a truncated response over the complete NoRecordsFound error, which leads to later validation reporting that the NSEC/NSEC3/RRSIG records are bogus, since it is working from an incomplete RRset.

_{🔄 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/2682 **Author:** [@divergentdave](https://github.com/divergentdave) **Created:** 12/16/2024 **Status:** ✅ Merged **Merged:** 12/18/2024 **Merged by:** [@marcus0x62](https://github.com/marcus0x62) **Base:** `main` ← **Head:** `david/resolver-truncation-fix` --- ### 📝 Commits (1) - [`0002736`](https://github.com/hickory-dns/hickory-dns/commit/0002736ec5d62c0b22e9fcf8f12b0d5c355d32e2) resolver: never use truncated UDP response ### 📊 Changes **3 files changed** (+5 additions, -20 deletions) <details> <summary>View changed files</summary> 📝 `conformance/packages/conformance-tests/src/resolver/dns/rfc1035/truncation.rs` (+0 -1) 📝 `crates/resolver/src/name_server/name_server_pool.rs` (+2 -18) 📝 `crates/resolver/src/resolver.rs` (+3 -1) </details> ### 📄 Description This changes `NameServerPool` to only return data from a TCP response if the UDP response was truncated. This fixes #2608, and fixes NSEC/NSEC3 validation in some cases. I think we should not return truncated responses in this context, even if that's the only response available, because it is presently the cause of multiple correctness issues, and because this PR's behavior aligns with other implementations, like BIND and Unbound's recursive resolvers. There are some cases where it would be possible to implement optimizations making use of truncated responses, such as eagerly connecting to nameservers during iterative recursion before receiving complete referral responses, but I think we'd need to pierce existing encapsulation, and make more of the codebase truncation-aware. Plus, `ProtoError` makes reasoning about this behavior harder. When handling a truncated UDP response for a query of an empty RRset, `ProtoError::from_response()` returns `Ok(response)`, because of a check for the truncation bit. `NameServerPool` also checks the truncation bit on the response, and sends a TCP query. Subsequently, `ProtoError::from_response()` turns the TCP response into `Err(ProtoErrorKind::NoRecordsFound { .. }.into())`. Between these two choices, the existing code will return the `Ok(...)` containing a truncated response over the complete `NoRecordsFound` error, which leads to later validation reporting that the NSEC/NSEC3/RRSIG records are bogus, since it is working from an incomplete RRset. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>