-
Notifications
You must be signed in to change notification settings - Fork 236
fix(iroh)!: Update dependencies & fix 0-RTT with newer rustls by pulling the expected NodeId
out of the ServerName
in verifiers
#3290
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
Conversation
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3290/docs/iroh/ Last updated: 2025-05-07T11:37:57Z |
Deltachat only updated to 0.33 ~2 months ago, so I presume that this change would mean that Deltachat cannot update to 0.35 because this would be a wire-level breaking change towards all Deltachat versions which are older than 2 months. AFAIK deltachat so far intended to update to 0.35 as a last update before iroh 1.0. |
This is all incorrect. See the comment below. |
Actually @flub correctly pointed out: It's not actually a wire-breaking change! As I write in the PR review comment, the This is easy enough to test, so I pulled out the I didn't test 0-RTT, since (1) the changes in this PR are actually only tangentially relevant to 0-RTT and are more changes to how we configure rustls' client authentication/server authentication, and (2) because version 0.32 didn't have 0-RTT support. |
It was set to `pub(crate)` needlessly.
NodeId
out of the ServerName
in verifiersNodeId
out of the ServerName
in verifiers
Not updating redb as that'd be MSRV breaking |
NodeId
out of the ServerName
in verifiersNodeId
out of the ServerName
in verifiers
Description
This is code that will both update deps like #3288 but also makes 0-RTT tests pass again.
They were failing due to a change in
rustls
:Essentially what they're doing is checking that the certificate verifiers are
Arc::prt_eq
and depending on that allow or disallow 0-RTT.When running the tests with rustls logging enabled, I confirmed that this is indeed what's happening:
The fix isn't straight-forward.
The reason we're creating new
ServerVerifier
s every time is that we're passing in the expected remote node ID every time.There used to be no other way of grabbing the other node's ID in the verifier, until, coincidentally #3146 landed, which changed the server name that's set when connecting from always being
localhost
to<base32 NodeId>.iroh.invalid
.I'm now pulling in that value from the
ServerName
that's passed to theServerCertificateVerifier
inverify_server_cert
.As far as I understand that value should be a client-set value not a value we get over the network.
The downside of doing this is that we'll not be compatible with iroh versions that don't use the
<base32 NodeId>.iroh.invalid
server names.There's an unrelated change in here that's changing
make_client_config
andmake_server_config
infallible, as was done in #3161, which will definitely also have merge conflicts with this PR.Breaking Changes
Router::spawn
is now a plain function instead of anasync fn
Router::spawn
is now infallible, instead of returninganyhow::Result<()>
Change checklist