Skip to content
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

Specify ciphersuites for TLS 1.2 and TLS 1.3 separately #1659

Open
jsha opened this issue Dec 1, 2023 · 1 comment
Open

Specify ciphersuites for TLS 1.2 and TLS 1.3 separately #1659

jsha opened this issue Dec 1, 2023 · 1 comment

Comments

@jsha
Copy link
Contributor

jsha commented Dec 1, 2023

First discussed here: #564 (comment), and brought up again here: #1628 (review).

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:

  • ClientConfig::builder_with_protocols()
  • ClientConfig::builder().with_protocols()
  • ClientConfig::builder_with_provider().with_protocols()

It would be nice to reduce this to one path:

  • ClientConfig::builder_with_provider(CryptoProvider { tls12_cipher_suites: vec![], ..RING })

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
Copy link
Contributor

sayrer commented Mar 29, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants