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

Relax HRR consistency requirements for second client hello #4429

Merged
merged 2 commits into from Feb 23, 2024

Conversation

goatgoose
Copy link
Contributor

@goatgoose goatgoose commented Feb 19, 2024

Description of changes:

When a HelloRetryRequest is sent to a client in TLS 1.3, the RFC states that the client must send a second ClientHello that matches the first ClientHello, with a few exceptions for extensions:
https://datatracker.ietf.org/doc/html/rfc8446#section-4.1.2

   The client will also send a
   ClientHello when the server has responded to its ClientHello with a
   HelloRetryRequest.  In that case, the client MUST send the same
   ClientHello without modification, except as follows:

While the RFC doesn't specifically state that the server must enforce this requirement, s2n-tls servers do check the second ClientHello for consistency when doing so doesn't sacrifice compatibility.

However, some noncompliant TLS 1.3 clients have been found to modify the following values in their second ClientHello:

  • legacy_session_id
  • cipher_suites
  • supported_versions extension

This PR removes the consistency validation for each of these fields for increased compatibility with these clients.

Call-outs:

Modifying these fields in a second ClientHello could have unintended consequences on the state of the handshake. For example,the following RFC requirement must be upheld when a client sends a modified cipher suite list:

   Servers MUST ensure that they negotiate the
   same cipher suite when receiving a conformant updated ClientHello

Additionally, new logic was added to ensure a client can't downgrade their connection below TLS 1.3 when sending a modified supported versions extension. Are there any other cases that I'm missing?

Testing:

  • New tests have been added for each of the accepted/rejected ClientHello fields.
  • A new test was added to ensure the negotiated cipher suite can't change due to the client sending a modified cipher suite list.
  • New tests were added to ensure that TLS 1.3 is enforced in the supported versions extension in the second ClientHello.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Feb 19, 2024
@goatgoose goatgoose force-pushed the relax-hrr-validation branch 2 times, most recently from f09c894 to 7c3e637 Compare February 19, 2024 21:41
@goatgoose goatgoose marked this pull request as ready for review February 20, 2024 17:25
@goatgoose goatgoose enabled auto-merge (squash) February 23, 2024 16:06
@goatgoose goatgoose merged commit 7436ea0 into aws:main Feb 23, 2024
30 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants