internal-dns: NXDOMAIN/SERVFAIL where appropriate #1054
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently when the internal DNS server encounters an error, or has no records to server it will just return a DNS response with no resource records. This causes issues for resolvers that expect: - A SERVFAIL for records the DNS server cannot resolve. - An NXDOMAIN for records the DNS server asserts do not exist. This patch updates the internal DNS server to conform to those expectations. This required adding a zone configuration option to the internal DNS server that identifies what domain it's presiding over. Any records that do not fall within that domain will result in a SERVFAIL, causing resolvers to look for other DNS servers for answers. This particular failure mode falls under the SERVFAIL DNS Error Code 7 [1] "No Reachable Authority" as the internal DNS server does not take responsibility for recursive resolution. [1] https://tools.ietf.org/id/draft-ietf-dnsop-extended-error-05.html#rfc.section.4.2.7
smklein
approved these changes
May 10, 2022
| match socket.send_to(&resp_data, &src).await { | ||
| Ok(_) => {} | ||
| Err(e) => { | ||
| error!(log, "NXDOMAIN send: {}", e); |
There was a problem hiding this comment.
Do we not need to nack here? I see we're doing it on most other error paths
There was a problem hiding this comment.
I also noticed it missing with the deserialize record "error!" call below, fwiw
There was a problem hiding this comment.
Good catch. There should be nack on the deserialize error. For the socket.send_to errors there's not much we can do as the nack also needs to call socket.send_to to get the nack to the client.
leftwo
pushed a commit
that referenced
this pull request
Jan 10, 2024
Propolis changes since the last update: Gripe when using non-raw block device Update zerocopy dependency nvme: Wire up GetFeatures command Make Viona more robust in the face of errors bump softnpu (#577) Modernize 16550 UART Crucible changes since the last update: Don't check ROP if the scrub is done (#1093) Allow crutest cli to be quiet on generic test (#1070) Offload write encryption (#1066) Simplify handling of BlockReq at program exit (#1085) Update Rust crate byte-unit to v5 (#1054) Remove unused fields in match statements, downstairs edition (#1084) Remove unused fields in match statements and consolidate (#1083) Add logger to Guest (#1082) Drive hash / decrypt tests from Upstairs::apply Wait to reconnect if auto_promote is false Change guest work id from u64 -> GuestWorkId remove BlockOp::Commit (#1072) Various clippy fixes (#1071) Don't panic if tasks are destroyed out of order Update Rust crate reedline to 0.27.1 (#1074) Update Rust crate async-trait to 0.1.75 (#1073) Buffer should destructure to Vec when single-referenced Don't fail to make unencrypted regions (#1067) Fix shadowing in downstairs (#1063) Single-task refactoring (#1058) Update Rust crate tokio to 1.35 (#1052) Update Rust crate openapiv3 to 2.0.0 (#1050) Update Rust crate libc to 0.2.151 (#1049) Update Rust crate rusqlite to 0.30 (#1035)
leftwo
added a commit
that referenced
this pull request
Jan 11, 2024
Propolis changes since the last update: Gripe when using non-raw block device Update zerocopy dependency nvme: Wire up GetFeatures command Make Viona more robust in the face of errors bump softnpu (#577) Modernize 16550 UART Crucible changes since the last update: Don't check ROP if the scrub is done (#1093) Allow crutest cli to be quiet on generic test (#1070) Offload write encryption (#1066) Simplify handling of BlockReq at program exit (#1085) Update Rust crate byte-unit to v5 (#1054) Remove unused fields in match statements, downstairs edition (#1084) Remove unused fields in match statements and consolidate (#1083) Add logger to Guest (#1082) Drive hash / decrypt tests from Upstairs::apply Wait to reconnect if auto_promote is false Change guest work id from u64 -> GuestWorkId remove BlockOp::Commit (#1072) Various clippy fixes (#1071) Don't panic if tasks are destroyed out of order Update Rust crate reedline to 0.27.1 (#1074) Update Rust crate async-trait to 0.1.75 (#1073) Buffer should destructure to Vec when single-referenced Don't fail to make unencrypted regions (#1067) Fix shadowing in downstairs (#1063) Single-task refactoring (#1058) Update Rust crate tokio to 1.35 (#1052) Update Rust crate openapiv3 to 2.0.0 (#1050) Update Rust crate libc to 0.2.151 (#1049) Update Rust crate rusqlite to 0.30 (#1035) --------- Co-authored-by: Alan Hanson <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently when the internal DNS server encounters an error, or has no
records to serve, it will just return a DNS response with no resource
records. This causes issues for resolvers that expect:
This patch updates the internal DNS server to conform to those
expectations.
This required adding a zone configuration option to the internal DNS
server that identifies what domain it's presiding over. Any records that
do not fall within that domain will result in a SERVFAIL, causing
resolvers to look for other DNS servers for answers. This particular
failure mode falls under the SERVFAIL DNS Error Code 7 [1] "No Reachable
Authority" as the internal DNS server does not take responsibility for
recursive resolution.
[1] https://tools.ietf.org/id/draft-ietf-dnsop-extended-error-05.html#rfc.section.4.2.7