Skip to content

feat(iroh-relay)!: Allow setting a custom ServerCertVerifier and rename CaRootsConfig to CaTlsConfig#4300

Open
Frando wants to merge 12 commits into
mainfrom
Frando/tls-custom
Open

feat(iroh-relay)!: Allow setting a custom ServerCertVerifier and rename CaRootsConfig to CaTlsConfig#4300
Frando wants to merge 12 commits into
mainfrom
Frando/tls-custom

Conversation

@Frando

@Frando Frando commented Jun 2, 2026

Copy link
Copy Markdown
Member

Description

This adds a new mode to fully customize the rustls::ServerCertVerifier used for non-iroh TLS requests throughout iroh, and renames CaRootsConifg to CaTlsConfig (with a deprecated alias to keep backwards compat). A new mode CaTlsConfig::custom_server_cert_verifier allows to supply a callback that gets passed a CryptoProvider and returns a Arc<dyn ServerCertVerifier>.

It is indended for advanced usecases like those described in #2901.

Fixes #2901
cc @link2xt

Breaking Changes

None.

The following items have been deprecated:

  • iroh_relay::tls::CaRootsConfig, use iroh_relay::tls::CaTlsConfig instead (also re-exported as iroh::tls::CaTlsConfig).
  • iroh_relay::tls::CaRootsConfig::custom, use CaTlsConfig::custom_roots instead
  • iroh::endpoint::Builder::ca_roots_config, useiroh::endpoint::Builder::ca_tls_config instead

Notes & open questions

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.

@Frando Frando changed the title feat(iroh-relay)!: Allow to customize rustls config feat(iroh-relay)!: Allow to customize rustls ClientConfig Jun 2, 2026
@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: 235cd1c

@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/4300/docs/iroh/

Last updated: 2026-06-09T13:33:26Z

@n0bot n0bot Bot added this to iroh Jun 2, 2026
@github-project-automation github-project-automation Bot moved this to 🚑 Needs Triage in iroh Jun 2, 2026

@flub flub left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For the issue you also need to be able to use a custom server cert verifier for the https probes in net-report as well as for the relay connection.

I was thinking of adding a endpoint::Builder::server_cert_verifier(impl rustls::client::danger::ServerCertVerifier + 'static) builder method for this feature.

It is really unfortunate that you're breaking the API in iroh-relay for this. Can that not be avoided? We just need to get used to it, it'll be fine.

@Frando

Frando commented Jun 3, 2026

Copy link
Copy Markdown
Member Author

For the issue you also need to be able to use a custom server cert verifier for the https probes in net-report as well as for the relay connection.

Both the HTTPS probes and the relay connection already use the rustls::ClientConfig created from the CaRootsConfig, so the new CaRootsConfig::custom_client_config_builder applies to both and allows to inject a custom cert verifier.

The config is created and passed through already.

I was thinking of adding a endpoint::Builder::server_cert_verifier(impl rustls::client::danger::ServerCertVerifier + 'static) builder method for this feature.

I think this would then conflict with endpoint::Builder::ca_roots_config, or we would have to define semantics which wins if both are set. So I'd prefer to leave it at a single option on the endpoint builder to configure TLS setup for non-iroh connections. We already use the TLS config created via Builder::ca_roots_config throughout everything.

It is really unfortunate that you're breaking the API in iroh-relay for this. Can that not be avoided? We just need to get used to it, it'll be fine.

It could, but then the custom option can only be able to supply a ServerCertVerifier and not a full ClientConfig, so it e.g. wouldn't be able to adjust the set of allowed protocol versions. I also thought that we are still at the stage where we do minor breaking changes if it cleans up existing APIs, which I think this does: Reduce the surface by removing an item from the public API that we don't really need (CaRootsConfig::server_cert_verifier). The change from CaRootsConfig::custom to CaRootsConfig::custom_roots can surely be removed. I made it because it makes the API clearer now that there are two "custom" constructors. But if we deem this too late, this can surely be avoided as well, resulting in a less nice API though IMO.

@Frando Frando changed the title feat(iroh-relay)!: Allow to customize rustls ClientConfig feat(iroh-relay)!: Allow setting a custom ServerCertVerifier and rename CaRootsConfig to CaTlsConfig Jun 5, 2026
@Frando

Frando commented Jun 5, 2026

Copy link
Copy Markdown
Member Author

I pushed a commit that changes the PR to expose setting a ServerCertVerifier, not a full ClientConfig, after discussion in discord. Also updated the PR description accordingly.

@flub flub left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Implementation looks fine.

I'm once more going to suggest that this is so easy to do in a backwards compatible way. We just leave the old enum variant around as deprecated and leave the old name around as a deprecated type alias. I think it will give people much more confidence that the 1.0 will be serious about not breaking things than still doing breaking changes. Especially given it is so easy to not break.

@Frando

Frando commented Jun 8, 2026

Copy link
Copy Markdown
Member Author

I'm once more going to suggest that this is so easy to do in a backwards compatible way.

It would be rather straightforward, yes: Here's a commit that adds deprecated aliases: Frando/tls-custom...Frando/tls-custom-bwcompat

There's one breaking change that I'd like to leave in and that is the removal of CaTlsConfig::server_cert_verifier. We don't need it in iroh and exposing it restricts what we can potentially do in the future I think. But with the above commit we're down to one removal of an API unused in iroh and very likely not used anywhere else.

So, do we want the deprecated aliases? Happy to add them. Who makes the call, @dignifiedquire ?

@dignifiedquire dignifiedquire moved this from 🚑 Needs Triage to 👀 In review in iroh Jun 8, 2026

@flub flub left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm reverting my opinion on this. The feature is lovely. But we can not do any breaking changes anymore.

@github-project-automation github-project-automation Bot moved this from 👀 In review to 🏗 In progress in iroh Jun 9, 2026
@Frando

Frando commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

I pushed another commit to remove the last breaking change and keeps CaTlsConfig::server_cert_verifier.

(btw: semver check did not catch the breaking change before. it seems it can't see through type aliases. so we can not fully rely on this and still need to check things by hand.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🏗 In progress

Development

Successfully merging this pull request may close these issues.

iroh: provide a way to supply custom TLS connection code for relay

3 participants