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

Send zero-length TLS1.2 NewSessionTicket message if necessary #4528

Closed
lrstewart opened this issue Apr 26, 2024 · 0 comments · Fixed by #4532
Closed

Send zero-length TLS1.2 NewSessionTicket message if necessary #4528

lrstewart opened this issue Apr 26, 2024 · 0 comments · Fixed by #4532
Assignees
Labels

Comments

@lrstewart
Copy link
Contributor

lrstewart commented Apr 26, 2024

Problem:

Currently, it appears that we error if there are no valid session ticket keys when we go to write the NewSessionTicket message. We GUARD s2n_encrypt_session_ticket, which fails if there is no key available.

The RFC says:

This message MUST be sent if the
server included a SessionTicket extension in the ServerHello.
...
If the server determines that it does not want to include a
ticket after it has included the SessionTicket extension in the
ServerHello, then it sends a zero-length ticket in the
NewSessionTicket handshake message.

We only send a zero-length ticket if the configuration changes, which doesn't seem likely:

/* When server changes it's mind mid handshake send lifetime hint and session ticket length as zero */
if (!conn->config->use_tickets) {
POSIX_GUARD(s2n_stuffer_write_uint32(&conn->handshake.io, 0));
POSIX_GUARD(s2n_stuffer_write_uint16(&conn->handshake.io, 0));
return 0;
}
However, we don't do the same if the session ticket key expires, which seems like a more likely scenario.

I don't think this is an issue in TLS1.3, because the NewSessionTicket message is optional / post-handshake, so it looks like we just bail and don't send it if no key is available: https://github.com/aws/s2n-tls/blob/main/tls/s2n_server_new_session_ticket.c#L170-L172 Might still be worth a test if one doesn't already exist? I didn't check.

Solution:

Verify that this is a missed case, and that the handshake does fail if the session ticket key expires in the (admittedly VERY brief) time between the SERVER_HELLO and the SERVER_NEW_SESSION_TICKET message. We probably need a test, which might be tricky because both messages are sent in the same flight. I think s2n_negotiate_until_message / s2n_negotiate_test_server_and_client_until_message will still work.

Write an empty NewSessionTicket message if s2n_encrypt_session_ticket fails, rather than triggering a handshake failure.

  • Does this change what S2N sends over the wire? If yes, explain.
  • Does this change any public APIs? If yes, explain.
  • Which versions of TLS will this impact?

Requirements / Acceptance Criteria:

What must a solution address in order to solve the problem? How do we know the solution is complete?

  • RFC links: Links to relevant RFC(s)
  • Related Issues: Link any relevant issues
  • Will the Usage Guide or other documentation need to be updated?
  • Testing: How will this change be tested? Call out new integration tests, functional tests, or particularly interesting/important unit tests.
    • Will this change trigger SAW changes? Changes to the state machine, the s2n_handshake_io code that controls state transitions, the DRBG, or the corking/uncorking logic could trigger SAW failures.
    • Should this change be fuzz tested? Will it handle untrusted input? Create a separate issue to track the fuzzing work.

Out of scope:

Is there anything the solution will intentionally NOT address?

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 a pull request may close this issue.

3 participants