Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolver: err for dns-over-rustls w/o roots #2179

Merged
merged 1 commit into from Apr 14, 2024

Conversation

cpu
Copy link
Contributor

@cpu cpu commented Apr 13, 2024

If we're setting up dns-over-rustls and find that we've constructed a Rustls root cert store that has no trust anchors, return an early error. This makes the problem obvious and avoids surfacing some other less specific error cause when we first try to validate a peer certificate with an empty root store.

In order for our new early error to be surfaced correctly the name_sever_pool.rs parallel_conn_loop fn needs its error handling adjusted. Previously it would always compare the new error produced by trying to build the TLS config against the default error it starts its loop with, ProtoErrorKind::NoConnections. Since the error being returned is another ProtoErrorKind that isn't NoRecordsFound, Io, or Timeout the error specificity comparison considers the two errors equivalent in the general case, and the default error was always returned and the new error thrown away.

There is still a small mystery around why a more specific error from tokio-rustls isn't returned when peer validation inevitably fails. In my testing upstream we get a nice io::Error like err: Custom { kind: InvalidData, error: InvalidCertificate(UnknownIssuer) }. I can't figure out where or why, but something in the Hickory DNS stack is reducing this down to just a simple io error with type InvalidData - no nested source error or message. See my comment here I think this is a side-effect of the Clone impl on ProtoErrorKind.

Updates #2066

@cpu
Copy link
Contributor Author

cpu commented Apr 13, 2024

There is still a small mystery around why a more specific error from tokio-rustls isn't returned when peer validation inevitably fails.

Now addressed separately in #2181

If we find that we've constructed a Rustls root cert store that has no
trust anchors, return an early error. This makes the problem obvious
and avoids surfacing some other less specific error cause when we first
try to validate a peer certificate with an empty root store.

In order for our new early error to be surfaced correctly the
`name_sever_pool.rs` `parallel_conn_loop` fn needs its error handling
adjusted. Previously it would always compare the new error produced by
trying to build the TLS config against the default error it starts its
loop with, `ProtoErrorKind::NoConnections`. Since the error being
returned is another `ProtoErrorKind`, and the error specificity
comparison considers two `ProtoErrorKinds` equivalent in the general
case, the default error was always returned and the new error thrown
away.
Copy link
Collaborator

@djc djc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@djc djc merged commit 5aeb1d0 into hickory-dns:main Apr 14, 2024
18 checks passed
@cpu cpu deleted the cpu-err-no-rustls-roots branch April 14, 2024 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants