feat: implement nested tls#5
Conversation
ameba23
left a comment
ameba23
left a comment
There was a problem hiding this comment.
💯 The nested TLS implementation is great, as is the performance benchmarking setup.
Im less familiar with the actix related stuff but i can't see any issues.
One possible issue:
Should this library be crypto provider agnostic? I think that by adding rustls / tokio-rustls with default features enabled we implicitly enable aws-lc-rs
| } | ||
|
|
||
| let Some(fut) = this.fut.as_pin_mut() else { | ||
| panic!("unexpected polling state: missing handshake future") |
There was a problem hiding this comment.
This can never happen right?
There was a problem hiding this comment.
unless something is really wrong, no
| #[inline] | ||
| pub fn connect<IO>( | ||
| &self, | ||
| domain: rustls::pki_types::ServerName<'static>, |
There was a problem hiding this comment.
Are there any possible use-cases where you would want a different name for the inner and outer sessions?
I can't think of any.
There was a problem hiding this comment.
i mean this assumes that the hostname we want to connect to is the same on the inner session and the outer session.
For our use case, the outer session would use a domain name and the inner session would be self-signed or private-ca-signed, and we can choose what to set the SAN to. Using the same name for both means that the inner session (attested-tls crate) needs to know the domain name used by the outer session.
I don't think this is an issue - but maybe there are some edge cases where we would need a different hostname for inner and outer session.
There was a problem hiding this comment.
ah, got it. no, I also can not imagine use-case when inner and outer certs would have to have different CNs. some of the SANs might be different, though. for example, the outer, CA-signed cert might have just public FQDN, but the inner certs might have the public CN (same as outer) + SAN for each of the individual VMs serving.
but from the client perspective it would be just FQDN, same for both certs
|
|
||
| fn poll_ready(&self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> { | ||
| <ConnectorService as Service<ConnectInfo<Uri>>>::poll_ready(&self.tcp, cx) | ||
| .map_err(|err| ConnectError::Resolver(Box::new(err))) |
There was a problem hiding this comment.
This error variant indicates the hostname could not be resolved. is that actually the case here or is this just a way to wrap the error?
good point. pushed 0d84afe |
2 participants
This PR implements nested TLS primitives + actix wrappers (accessible via
actixfeature).