[PR #2031] [CLOSED] Separate default `rustls::ClientConfig` for each protocol (take 2) #2767

New issue

Closed

opened 2026-03-16 11:07:06 +03:00 by kerem · 0 comments

kerem commented

2026-03-16 11:07:06 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/hickory-dns/hickory-dns/pull/2031
Author: @daxpedda
Created: 9/21/2023
Status: ❌ Closed

Base: main ← Head: alpn-config-2

📝 Commits (1)

f705ec7 Separate default rustls::ClientConfig for each protocol

📊 Changes

6 files changed (+225 additions, -107 deletions)

View changed files

📝 crates/resolver/src/config.rs (+15 -0)
📝 crates/resolver/src/https.rs (+21 -24)
📝 crates/resolver/src/name_server/connection_provider.rs (+66 -29)
📝 crates/resolver/src/quic.rs (+109 -23)
📝 crates/resolver/src/tls/dns_over_rustls.rs (+12 -29)
📝 crates/resolver/src/tls/mod.rs (+2 -2)

📄 Description

WIP!

I have to store the configs as in Options to allow Default to work. ~~Probably the simplest solution is to remove the Default implementation and add a ResolverOpts::new()~~.
This is just temporary but currently we don't reuse certificates, for that I would have to either add another field or apply the solution in 1.
Current DoQ takes a ClientConfig instead of a Arc<ClientConfig>, as far as I can see the only reason for that is that it adds the correct ALPN, which is obviously unnecessary when using a default supplied one. I would propose to actually remove this ALPN correction completely.

Originally in #2001 I have added those configs to GenericConnector instead of ResolverOpts, maybe this is also an alternative to consider. I was trying to keep in mind that somewhere we will want to dynamically be able to select between native or WebPKI roots, which will be weird if done in ResolverOpts, because it's holding these configs but it doesn't have a builder or something to select it before constructing those configs.

I hope I'm making sense here, I'm happy to talk this out in realtime on Discord or wherever if it's easier.

This is take two of #2001.
Fixes #1990.

_{🔄 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/2031 **Author:** [@daxpedda](https://github.com/daxpedda) **Created:** 9/21/2023 **Status:** ❌ Closed **Base:** `main` ← **Head:** `alpn-config-2` --- ### 📝 Commits (1) - [`f705ec7`](https://github.com/hickory-dns/hickory-dns/commit/f705ec7245855f2817009d7448cc19ed302013e4) Separate default `rustls::ClientConfig` for each protocol ### 📊 Changes **6 files changed** (+225 additions, -107 deletions) <details> <summary>View changed files</summary> 📝 `crates/resolver/src/config.rs` (+15 -0) 📝 `crates/resolver/src/https.rs` (+21 -24) 📝 `crates/resolver/src/name_server/connection_provider.rs` (+66 -29) 📝 `crates/resolver/src/quic.rs` (+109 -23) 📝 `crates/resolver/src/tls/dns_over_rustls.rs` (+12 -29) 📝 `crates/resolver/src/tls/mod.rs` (+2 -2) </details> ### 📄 Description WIP! 1. I have to store the configs as in `Option`s to allow `Default` to work. ~~Probably the simplest solution is to remove the `Default` implementation and add a `ResolverOpts::new()`~~. 2. This is just temporary but currently we don't reuse certificates, for that I would have to either add another field or apply the solution in 1. 3. Current DoQ takes a `ClientConfig` instead of a `Arc<ClientConfig>`, as far as I can see the only reason for that is that it adds the correct ALPN, which is obviously unnecessary when using a default supplied one. I would propose to actually remove this ALPN correction completely. Originally in #2001 I have added those configs to `GenericConnector` instead of `ResolverOpts`, maybe this is also an alternative to consider. I was trying to keep in mind that somewhere we will want to dynamically be able to select between native or WebPKI roots, which will be weird if done in `ResolverOpts`, because it's holding these configs but it doesn't have a builder or something to select it before constructing those configs. I hope I'm making sense here, I'm happy to talk this out in realtime on Discord or wherever if it's easier. This is take two of #2001. Fixes #1990. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>