[GH-ISSUE #618] Try to avoid cloning Rustls ClientConfig on creating TlsStreams #251

New issue

Closed

opened 2026-03-07 23:01:51 +03:00 by kerem · 3 comments

kerem commented

2026-03-07 23:01:51 +03:00

Owner

Originally created by @sticnarf on GitHub (Nov 16, 2018).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/618

Is your feature request related to a problem? Please describe.

There are two ways of adding CAs if I use DNS over Rustls:

Creating a ClientConfig with CAs first and pass it to TlsStreamBuilder
Add CAs to TlsStreamBuilder using the add_ca function

The problem is that if I want to create TlsStreams repeatly with the same CAs, I can come up with no way to avoid cloning the ClientConfig or adding the CAs every time I build a TlsStream.

Describe the solution you'd like
The essence of the problem is that TlsStreamBuilder owns ClientConfig and the build function consumes the TlsStreamBuilder. There are approaches, but they break the API and I cannot decide which one is better.

One approach is letting TlsStreamBuilder give up the ownership of ClientConfig. TlsStreamBuilder can own an Arc<ClientConfig> but will have to remove the add_ca function.

Another one is preventing the build function from consuming the builder and still retain the add_ca function. This might introduce more work. The add_ca function modifies the ClientConfig, but it is possible that the config is made into an Arc and passed out. So, after build is called, calling add_ca implies a clone to the ClientConfig. However, it is unacceptable that every time adding a CA clones the config. So we must remember if build is called. It looks ugly but brings less API breakage.

Of course, I am happy to see if there is a better solution.

Originally created by @sticnarf on GitHub (Nov 16, 2018). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/618 **Is your feature request related to a problem? Please describe.** There are two ways of adding CAs if I use DNS over Rustls: * Creating a ClientConfig with CAs first and pass it to TlsStreamBuilder * Add CAs to TlsStreamBuilder using the `add_ca` function The problem is that if I want to create `TlsStream`s repeatly with the same CAs, I can come up with no way to avoid cloning the ClientConfig or adding the CAs every time I build a TlsStream. **Describe the solution you'd like** The essence of the problem is that TlsStreamBuilder owns ClientConfig and the `build` function consumes the TlsStreamBuilder. There are approaches, but they break the API and I cannot decide which one is better. One approach is letting TlsStreamBuilder give up the ownership of ClientConfig. TlsStreamBuilder can own an `Arc<ClientConfig>` but will have to remove the `add_ca` function. Another one is preventing the `build` function from consuming the builder and still retain the `add_ca` function. This might introduce more work. The `add_ca` function modifies the `ClientConfig`, but it is possible that the config is made into an Arc and passed out. So, after `build` is called, calling `add_ca` implies a clone to the ClientConfig. However, it is unacceptable that every time adding a CA clones the config. So we must remember if `build` is called. It looks ugly but brings less API breakage. Of course, I am happy to see if there is a better solution.

kerem closed this issue

2026-03-07 23:01:56 +03:00

kerem commented

2026-03-07 23:02:02 +03:00

Author

Owner

@bluejekyll commented on GitHub (Nov 17, 2018):

I’m on a trip, so I can’t look at the code right now. But based on your observations, probably changing to Arc in the builder would be fine.

On removing the ad_ca method on the builder, the reason this is there is to have a more consistent interface with how OpenSSL is supported. I’m not sure how important that is, and have been considering dropping OpenSSL support all together.

I’m open to either option. One thing that I’d like to understand, this is just to be able to reuse the same ClientConfig, right? And not to change it in between usage, as that would require a clone anyway I believe.

@bluejekyll commented on GitHub (Nov 17, 2018): I’m on a trip, so I can’t look at the code right now. But based on your observations, probably changing to `Arc` in the builder would be fine. On removing the `ad_ca` method on the builder, the reason this is there is to have a more consistent interface with how OpenSSL is supported. I’m not sure how important that is, and have been considering dropping OpenSSL support all together. I’m open to either option. One thing that I’d like to understand, this is just to be able to reuse the same `ClientConfig`, right? And not to change it in between usage, as that would require a clone anyway I believe.

kerem commented

2026-03-07 23:02:07 +03:00

Author

Owner

@sticnarf commented on GitHub (Nov 18, 2018):

Exactly. I think it is a common case that we do resolving more than once. Lots of TlsStreams are opened, and we rarely change the CA list.

I took a look at the code in the resolver crate. It just adds all CAs every time before a connection is established. It feels bad to me if we need to do a lot of resolving...

By the way, I do not think we can actually drop OpenSSL support. Rustls depends on ring, which is currently not very portable. For example, ring does not support mips architecture yet.

@sticnarf commented on GitHub (Nov 18, 2018): Exactly. I think it is a common case that we do resolving more than once. Lots of `TlsStream`s are opened, and we rarely change the CA list. I took a look at the code in the resolver crate. It just adds all CAs every time before a connection is established. It feels bad to me if we need to do a lot of resolving... By the way, I do not think we can actually drop OpenSSL support. Rustls depends on ring, which is currently not very portable. For example, ring does not support mips architecture yet.

kerem commented

2026-03-07 23:02:12 +03:00

Author

Owner

@bluejekyll commented on GitHub (Nov 18, 2018):

I agree with the reason to do this. If you’re comfortable submitting a PR, That would be great.

As to mips, I’m unaware of anyone using this with mips, and don’t know if that’s possible with the current library. Theoretically it is a valuable reason to support OpenSSL, among others.

I was just giving background on these design decisions.

@bluejekyll commented on GitHub (Nov 18, 2018): I agree with the reason to do this. If you’re comfortable submitting a PR, That would be great. As to mips, I’m unaware of anyone using this with mips, and don’t know if that’s possible with the current library. Theoretically it is a valuable reason to support OpenSSL, among others. I was just giving background on these design decisions.

kerem referenced this issue

2026-03-15 22:15:17 +03:00

[GH-ISSUE #190] resolver lookup_ip should checkout /etc/hosts/ before dns requests? #387

kerem referenced this issue

2026-03-15 22:24:01 +03:00

[GH-ISSUE #251] Support user-specified location for /etc/hosts file #417