[GH-ISSUE #2072] trust-dns-client/dns-over-https-openssl pulling in rustls in v0.23+ #872

Closed
opened 2026-03-16 00:42:10 +03:00 by kerem · 11 comments
Owner

Originally created by @decathorpe on GitHub (Oct 20, 2023).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/2072

It appears that with the release of trust-dns v0.23 (and still present in git main), there was some reshuffling of crate features that causes the "dns-over-https-openssl" feature of trust-dns-client to pull in "trust-dns-proto/dns-over-rustls", which seems to be unintentional:

trust-dns-client/dns-over-https-openssl -> trust-dns-client/dns-over-https
trust-dns-client/dns-over-https -> trust-dns-proto/dns-over-https
trust-dns-proto/dns-over-https -> trust-dns-proto/dns-over-rustls

To me it appears that the generic "trust-dns-proto/dns-over-https" feature should actually be called "dns-over-https-rustls"? There is also no "dns-over-https-openssl" feature in trust-dns-proto, but there is "dns-over-https-rustls".

To match the structure of the other trust-dns-* crates, I assume there would be a generic (backend-agnostic) "dns-over-https" feature, and two features for "dns-over-https-rustls" and "dns-over-https-openssl", which both depend on "dns-over-https", and with rustls-backend specific dependencies only in "dns-over-https-rustls", not in "dns-over-https".

Originally created by @decathorpe on GitHub (Oct 20, 2023). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/2072 It appears that with the release of trust-dns v0.23 (and still present in git main), there was some reshuffling of crate features that causes the "dns-over-https-openssl" feature of trust-dns-client to pull in "trust-dns-proto/dns-over-rustls", which seems to be unintentional: trust-dns-client/dns-over-https-openssl -> trust-dns-client/dns-over-https trust-dns-client/dns-over-https -> trust-dns-proto/dns-over-https trust-dns-proto/dns-over-https -> trust-dns-proto/dns-over-rustls To me it appears that the generic "trust-dns-proto/dns-over-https" feature should actually be called "dns-over-https-rustls"? There is also no "dns-over-https-openssl" feature in trust-dns-proto, but there is "dns-over-https-rustls". To match the structure of the other trust-dns-* crates, I assume there would be a generic (backend-agnostic) "dns-over-https" feature, and two features for "dns-over-https-rustls" and "dns-over-https-openssl", which both depend on "dns-over-https", and with rustls-backend specific dependencies only in "dns-over-https-rustls", not in "dns-over-https".
kerem closed this issue 2026-03-16 00:42:16 +03:00
Author
Owner

@decathorpe commented on GitHub (Oct 20, 2023):

To clarify what I mean, I would replace these lines from the trust-dns-proto/Cargo.toml features table:

dns-over-https = [
    "bytes",
    "h2",
    "http",
    "dns-over-rustls",
    "webpki-roots",
    "tokio-runtime",
]
dns-over-https-rustls = ["dns-over-https"]

With this:

dns-over-https = [
    "bytes",
    "h2",
    "http",
    "tokio-runtime",
]
dns-over-https-rustls = [
    "dns-over-https",
    "dns-over-rustls",
    "webpki-roots",
]

This might not be 100% correct (I'm not sure whether the dependencies in dns-over-https are only dependencies in case the rustls backend is selected or if they're also required for the openssl / native-tls backends), but it ensures that rustls is no longer pulled in when using the OpenSSL or native-tls backends in other trust-dns-* crates.

Applying this patch appears to work as expected when building the trust-dns-* crates for Fedora Linux (where I am responsible for packaging them as dependencies of reqwest / awc / etc.). Depending on RusTLS in is not possible in our environment, since it doesn't support all CPU architectures that we need to support, so rustls (accidentally / unnecessarily?) getting pulled in by dns-over-https-openssl is not good.

<!-- gh-comment-id:1772601172 --> @decathorpe commented on GitHub (Oct 20, 2023): To clarify what I mean, I would replace these lines from the `trust-dns-proto/Cargo.toml` features table: ```toml dns-over-https = [ "bytes", "h2", "http", "dns-over-rustls", "webpki-roots", "tokio-runtime", ] dns-over-https-rustls = ["dns-over-https"] ``` With this: ```toml dns-over-https = [ "bytes", "h2", "http", "tokio-runtime", ] dns-over-https-rustls = [ "dns-over-https", "dns-over-rustls", "webpki-roots", ] ``` This might not be 100% correct (I'm not sure whether the dependencies in `dns-over-https` are only dependencies in case the rustls backend is selected or if they're also required for the openssl / native-tls backends), but it ensures that rustls is no longer pulled in when using the OpenSSL or native-tls backends in other trust-dns-* crates. Applying this patch appears to work as expected when building the trust-dns-* crates for Fedora Linux (where I am responsible for packaging them as dependencies of reqwest / awc / etc.). Depending on RusTLS in is not possible in our environment, since it doesn't support all CPU architectures that we need to support, so rustls (accidentally / unnecessarily?) getting pulled in by `dns-over-https-openssl` is not good.
Author
Owner

@bluejekyll commented on GitHub (Oct 20, 2023):

Yes, there might have been a bit of an oversight on some reviews in this area. Can you look at the new release in 0.24 (requires the change to the new branding, hickory). I think the dependencies and features might have been fixed in this area?

<!-- gh-comment-id:1773043004 --> @bluejekyll commented on GitHub (Oct 20, 2023): Yes, there might have been a bit of an oversight on some reviews in this area. Can you look at the new release in `0.24` (requires the change to the new branding, hickory). I think the dependencies and features might have been fixed in this area?
Author
Owner

@decathorpe commented on GitHub (Oct 20, 2023):

Thanks for looking into it!

This is still the case in git main: https://github.com/hickory-dns/hickory-dns/blob/main/crates/proto/Cargo.toml#L50


EDIT: This is what it looks like in v0.22:

https://github.com/hickory-dns/hickory-dns/blob/v0.22.1/crates/proto/Cargo.toml#L39-L45

So it appears that there was some change between v0.22 and v0.23 that messed up the feature dependencies for the dns-over-https and dns-over-rustls features.

I can work on a PR to sort that out, if that would be helpful?

<!-- gh-comment-id:1773073891 --> @decathorpe commented on GitHub (Oct 20, 2023): Thanks for looking into it! This is still the case in git main: https://github.com/hickory-dns/hickory-dns/blob/main/crates/proto/Cargo.toml#L50 --- EDIT: This is what it looks like in v0.22: https://github.com/hickory-dns/hickory-dns/blob/v0.22.1/crates/proto/Cargo.toml#L39-L45 So it appears that there was some change between v0.22 and v0.23 that messed up the feature dependencies for the `dns-over-https` and `dns-over-rustls` features. I can work on a PR to sort that out, if that would be helpful?
Author
Owner

@djc commented on GitHub (Oct 20, 2023):

Yes please!

<!-- gh-comment-id:1773239339 --> @djc commented on GitHub (Oct 20, 2023): Yes please!
Author
Owner

@bluejekyll commented on GitHub (Oct 21, 2023):

also, we have a feature called dns-over-https-openssl but I'm pretty sure we don't actually have https support for openssl and h2 or h3 at this point. Is OpenSSL something you're interested in? (we should remove that as an option in the crates I think, if you look at the h2 impls, there's no OpenSSL support there.

We'v been discussing removing OpenSSL from most runtime options in favor or Rustls and ring.

<!-- gh-comment-id:1773569944 --> @bluejekyll commented on GitHub (Oct 21, 2023): also, we have a feature called `dns-over-https-openssl` but I'm pretty sure we don't actually have https support for openssl and h2 or h3 at this point. Is OpenSSL something you're interested in? (we should remove that as an option in the crates I think, if you look at the `h2` impls, there's no OpenSSL support there. We'v been discussing removing OpenSSL from most runtime options in favor or Rustls and ring.
Author
Owner

@decathorpe commented on GitHub (Oct 21, 2023):

Removing support for OpenSSL would be unfortunate, since that's by far the best supported option on most Linux distributions. Once the Rustls/ring combination supports all the CPU architectures we need (probably with rustls v0.22 and ring v0.17 from what I can tell?) that will be better, but OpenSSL would still be preferred in most cases.

For example, the OpenSSL package in Fedora Linux is maintained directly by Red Hat, with timely fixes for security vulnerabilities. On RHEL itself, OpenSSL is FIPS certified, while other crypto libraries (like ring) are not. We also have guidelines for using system-provided crypto libraries if possible, so depending on "non-standard" crypto like Rustls / ring is a no-go for core system components. Additionally, cryptography libraries like OpenSSL are configured to follow a system-wide security / crypto policy (for example, which algorithms are allowed), which Rustls / ring does not (yet) support at all.

PS: I know that h3 doesn't yet support crypto backends other than Rustls / ring, which is also one of the reasons why h3 is not packaged for Fedora Linux (and the HTTP/3 / QUIC support in other crates like reqwest is disabled).

<!-- gh-comment-id:1773742598 --> @decathorpe commented on GitHub (Oct 21, 2023): Removing support for OpenSSL would be unfortunate, since that's *by far* the best supported option on most Linux distributions. Once the Rustls/ring combination supports all the CPU architectures we need (probably with rustls v0.22 and ring v0.17 from what I can tell?) that will be better, but OpenSSL would still be preferred in most cases. For example, the OpenSSL package in Fedora Linux is maintained directly by Red Hat, with timely fixes for security vulnerabilities. On RHEL itself, OpenSSL is FIPS certified, while other crypto libraries (like ring) are not. We also have guidelines for using system-provided crypto libraries if possible, so depending on "non-standard" crypto like Rustls / ring is a no-go for core system components. Additionally, cryptography libraries like OpenSSL are configured to follow a system-wide security / crypto policy (for example, which algorithms are allowed), which Rustls / ring does not (yet) support at all. PS: I [know](https://github.com/quinn-rs/quinn/issues/1389) that h3 doesn't yet support crypto backends other than Rustls / ring, which is also one of the reasons why h3 is not packaged for Fedora Linux (and the HTTP/3 / QUIC support in other crates like reqwest is disabled).
Author
Owner

@decathorpe commented on GitHub (Oct 21, 2023):

I've opened PRs against main and the release/0.23 branch. If the changes are accepted, it would be great to get a v0.23.2 release after the PR for v0.23 is merged.

<!-- gh-comment-id:1773746691 --> @decathorpe commented on GitHub (Oct 21, 2023): I've opened PRs against main and the release/0.23 branch. If the changes are accepted, it would be great to get a v0.23.2 release after the PR for v0.23 is merged.
Author
Owner

@djc commented on GitHub (Oct 21, 2023):

I actually think the current state of the features better reflects what's actually needed? As in, proto/src/https/https_client_stream.rs does unconditionally require rustls (and already did so in 0.22). So maybe the feature names are misleading, but it looks to me like the current dependency tree for them is accurate relative to the code implementing the DoH feature.

Why are you packaging trust-dns-proto/hickory-dns-proto? Presumably it's for some application that needs it? Are those applications part of the core system components? Maybe it would be useful to file an issue for what it would take for rustls to apply the system-wide security/crypto policy (I had not heard of this requirement before).

Support for a FIPS-approved rustls CryptoProvider is being tracked in https://github.com/rustls/rustls/issues/1540. ring is working on FIPS approval, and we're looking into aws-lc-rs which should become FIPS approved in the future, too.

<!-- gh-comment-id:1773779595 --> @djc commented on GitHub (Oct 21, 2023): I actually think the current state of the features better reflects what's actually needed? As in, `proto/src/https/https_client_stream.rs` does unconditionally require rustls (and already did so in 0.22). So maybe the feature names are misleading, but it looks to me like the current dependency tree for them is accurate relative to the code implementing the DoH feature. Why are you packaging trust-dns-proto/hickory-dns-proto? Presumably it's for some application that needs it? Are those applications part of the core system components? Maybe it would be useful to file an issue for what it would take for rustls to apply the system-wide security/crypto policy (I had not heard of this requirement before). Support for a FIPS-approved rustls `CryptoProvider` is being tracked in https://github.com/rustls/rustls/issues/1540. *ring* is working on FIPS approval, and we're looking into aws-lc-rs which should become FIPS approved in the future, too.
Author
Owner

@decathorpe commented on GitHub (Oct 21, 2023):

I had not realized that, thanks for checking. For contect, the three crates in Fedora that depend on trust-dns-* are:

It looks like none of them use any of the dns-over-https features, so I might be able to disable those features entirely in our packages. I usually try not to do patch in changes like that (to try to make the difference between crates as shipped on crates.io and available for package builds in Fedora as small as possible), but in this case, that shounds like the "cleanest" solution?

<!-- gh-comment-id:1773786585 --> @decathorpe commented on GitHub (Oct 21, 2023): I had not realized that, thanks for checking. For contect, the three crates in Fedora that depend on trust-dns-* are: - https://crates.io/crates/awc - https://crates.io/crates/reqwest - https://crates.io/crates/sequoia-net It looks like none of them use any of the `dns-over-https` features, so I might be able to disable those features entirely in our packages. I usually try not to do patch in changes like that (to try to make the difference between crates as shipped on crates.io and available for package builds in Fedora as small as possible), but in this case, that shounds like the "cleanest" solution?
Author
Owner

@djc commented on GitHub (Oct 23, 2023):

It's interesting because all three of those crate seem to depend on trust-dns-resolver with mostly default features, which (at least on main) doesn't seem to enable the rustls features from -proto?

<!-- gh-comment-id:1775018940 --> @djc commented on GitHub (Oct 23, 2023): It's interesting because all three of those crate seem to depend on trust-dns-resolver with mostly default features, which (at least on main) doesn't seem to enable the rustls features from -proto?
Author
Owner

@decathorpe commented on GitHub (Oct 23, 2023):

They don't, but due to the way Rust packaging works on Fedora (and I assume on debian as well), dependencies for non-defsult features need to be satisfiable as well, or explicitly disabled - which I will likely do in the future for the features of trust-dns / hickory-dns that depend on rustls/ring. Once ring/rustls support the architectures that we need, I can always revert those changes.

That said, it seems like the dependency on rustls is actually intentional, so I'll close this ticket and the PRs I opened.

<!-- gh-comment-id:1775043455 --> @decathorpe commented on GitHub (Oct 23, 2023): They don't, but due to the way Rust packaging works on Fedora (and I assume on debian as well), dependencies for non-defsult features need to be satisfiable as well, or explicitly disabled - which I will likely do in the future for the features of trust-dns / hickory-dns that depend on rustls/ring. Once ring/rustls support the architectures that we need, I can always revert those changes. That said, it seems like the dependency on rustls is actually intentional, so I'll close this ticket and the PRs I opened.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/hickory-dns#872
No description provided.