-
Notifications
You must be signed in to change notification settings - Fork 215
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
feat(iroh)!: implement support for raw public keys in TLS #2937
base: main
Are you sure you want to change the base?
Conversation
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/2937/docs/iroh/ Last updated: 2025-02-14T11:48:11Z |
For a hot minute I was imagining a feature on rustls to disable all non-RPK paths, imagining that we could get rid of ASN.1/DER parsing code. |
iroh-net/src/tls/resolver.rs
Outdated
use crate::tls::Authentication; | ||
|
||
#[derive(Debug)] | ||
pub(crate) struct AlwaysResolvesCert { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kind of annoying that rustls
has all the implementations for ResolvesClientCert
and ResolvesServerCert
that we need, but they're not all exposed.
There's
AlwaysResolvesClientCert
AlwaysResolvesServerCert
AlwaysResolvesClientRawPublicKeyCert
AlwaysResolvesServerRawPublicKeyCert
I'd rather not have to implement a trait outside the crate where it's defined, but eh.
Also it's super annoying that they split it up into client/server like that.
Going to wait with pushing this forward until we know if rustls/rustls#2258 might happen |
ed3d590
to
e44d964
Compare
9b7bb71
to
dfe0885
Compare
d21dffb
to
5d89fe3
Compare
looks like this is not happening, or at least not soon, so moving forward with this |
@@ -326,6 +329,18 @@ impl Builder { | |||
self | |||
} | |||
|
|||
/// Use libp2p based self signed certificates for TLS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps add a link to the libp2p handshake spec: https://github.com/libp2p/specs/blob/master/tls/tls.md
Also explain that you can only connect to other nodes that have this setting set, and finally explain that iroh versions < 0.33 will have that set as default, and the default switched to raw public keys with version 0.33. (thus the default configuration of 0.33 cannot connect to default-configured 0.32 and earlier endpoints.)
|
||
#[tokio::test] | ||
#[traced_test] | ||
async fn test_two_devices_roundtrip_quinn_rebinding_conn() -> Result<()> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm. Are we sure we want to delete these tests?
I see it's weird to see a test that tests "quinn", but I think what this is actually doing is testing UdpConn
(without depending on iroh being correct).
Instead of deleting these, perhaps they should be renamed & moved to udp_conn.rs
?
let remote_key = SubjectPublicKeyInfoDer::from_pem_slice(pem_key.as_bytes()) | ||
.expect("cannot remote open pub key"); | ||
trusted_spki.push(remote_key); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way of doing this that doesn't require going bytes -> String -> bytes?
There's an impl From<Vec<u8>> for SubjectPublicKeyInfoDer
at least.
This is step 1 to land #2798
this was only used in tests, and those only tested quinn isolation things, which have been removed
98ad8d5
to
bb49ae7
Compare
Description
This is step one of #2798. This introduces the configuration of the TLS authentication method, allowing to enable the usage of raw public keys, which will lead to us being able to remove the hack of using self signed certificates.
TODOs
iroh
Breaking Changes
By default iroh endpoints will now use the new
RawPublicKey
TLS configuration. This means they will not be able to talk to older iroh nodes. If needed, the old mechanism can still be used by configuring the endpoint like thisNotes & open questions
Change checklist