-
Notifications
You must be signed in to change notification settings - Fork 747
Description
Problem:
While refactoring the dynamic record sizing Rust integration test in #5614 to support both TLS 1.2 and TLS 1.3, I found that the harness's handshake-completion logic (handshake_type().contains("NEGOTIATED")) caused TLS 1.2 handshakes to terminate one loop too early. This left the server’s final TLS 1.2 Finished message unread, which then arrived together with application-data bytes and was incorrectly routed through the TLS 1.3 post-handshake path (s2n_post_handshake_recv()), triggering S2N_ERR_BAD_MESSAGE (see related issue #5624).
I added a temporary handshake_completed field in the Rust harness (based on poll_negotiate()) to determine when to exit the handshake loop. This works but is brittle and specific to this harness.
More broadly, all external integrations currently infer handshake completion from indirect signals (error codes, IO blocked statuses, or handshake “type” strings). This can lead to:
- exiting the handshake loop too early,
- mishandling leftover TLS 1.2 handshake bytes, and
- tightly coupling application logic to internal s2n-tls behavior.
A simple, explicit “is the handshake complete?” API would prevent this entire class of issues and provide a more robust and future-proof integration surface.
Need By Date:
N/A
Solution:
Expose the existing internal handshake-completion macro through a public API:
/* Returns 1 if the TLS handshake is complete, 0 otherwise, or < 0 on error. */
int s2n_connection_handshake_complete(struct s2n_connection* conn)
{
return is_handshake_complete(conn);
}
This would:
- Allow external integrations to avoid prematurely exiting the handshake loop (especially for TLS 1.2) and prevent misclassification of leftover handshake fragments.
- Decouple integrations from internal handshake state machine details and reduce reliance on heuristics that may change over time.
Requirements / Acceptance Criteria:
A complete solution should:
- Add a public API (e.g., s2n_connection_handshake_complete) that reports whether the handshake is fully complete.
- Document behavior across TLS 1.2, TLS 1.3, renegotiation, and post-handshake messages (e.g., NewSessionTicket, KeyUpdate).
- Ensure callers can reliably drive handshake loops until all handshake bytes are consumed.
- Be used in at least one real integration (e.g., update the Rust integration tests to use this API instead of poll_negotiate()).
- Include tests verifying that handshake completion is only reported once all handshake messages, including the TLS 1.2 Finished, are processed, covering both TLS 1.2 and TLS 1.3 flows.
Additional context:
- RFC links: N/A
- Related Issues: Similar request in Add s2n_is_handshake_complete() API #854. OpenSSL has SSL_is_init_finished, Rustls has is_handshaking.
- Will the Usage Guide or other documentation need to be updated? Update the Usage Guide section on driving the handshake: https://aws.github.io/s2n-tls/usage-guide/ch07-io.html#performing-the-tls-handshake
- Will this change trigger SAW changes? None expected.
- Should this change be fuzz tested? N/A
Out of scope:
Is there anything the solution will intentionally NOT address?