rustls / rustls

A modern TLS library in Rust
Other
5.95k stars 636 forks source link

Specify ciphersuites for TLS 1.2 and TLS 1.3 separately #1659

Open jsha opened 10 months ago

jsha commented 10 months ago

First discussed here: https://github.com/rustls/rustls/pull/564#issuecomment-826049679, and brought up again here: https://github.com/rustls/rustls/pull/1628#pullrequestreview-1756467489.

Right now, we specify cipher suites and protocol versions independently. That introduces a possible error case in config building: since our cipher suites are TLS 1.2-only or TLS 1.3-only, it's possible to specify "Only allow TLS 1.3; only allow TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 (a TLS 1.2 cipher suite)". We check that error at config building time, and it's one of two places during config building that the user has to handle an error.

We also have three paths through config building when specifying protocols:

It would be nice to reduce this to one path:

Also we want to support eliminating the tls12 Cargo feature (#224). In practice that means each SupportedCipherSuite needs to contain a &'static SupportedProtocolVersion, so that the linker only has a path to the TLS 1.2 implementation code if there was any TLS 1.2 cipher suite referenced in the application code.

sayrer commented 6 months ago

Yeah, there is something kind of off here. In my older ECH efforts, I made a builder that could only use TLS 1.3 cypher suites, by adding such a thing to those enums. The idea being that code attempting to offer ECH where TLS 1.2 was a possibility wouldn't even compile. I think that is the desired result, but I don't have a strong opinion on how to do it.