feat: add s2n_connection_handshake_complete() public API by FreezB11 · Pull Request #5906 · aws/s2n-tls

FreezB11 · 2026-06-04T21:38:45Z

Goal

Add s2n_connection_handshake_complete() as a public API so external integrations
can reliably determine when the TLS handshake is fully complete, without relying on
indirect signals like error codes, blocked statuses, or handshake type strings.

Why

While refactoring the dynamic record sizing Rust integration test (#5614) to support
both TLS 1.2 and TLS 1.3, the harness's handshake-completion check
(handshake_type().contains("NEGOTIATED")) caused TLS 1.2 handshakes to exit the
loop one iteration too early. The server's final Finished message was left unread,
then arrived together with application-data bytes and was misrouted through
s2n_post_handshake_recv(), triggering S2N_ERR_BAD_MESSAGE.

More broadly, all external integrations currently infer handshake completion from
indirect signals, which can lead to exiting the loop too early, mishandling leftover
TLS 1.2 handshake bytes, and tight coupling to internal s2n-tls state machine details.

How

Expose the existing internal s2n_handshake_is_complete() function (which checks
ACTIVE_STATE(conn).writer == 'B') as a null-safe public API. This is the same
check that drives s2n_negotiate_impl()'s own loop, making it the authoritative
completion signal in the codebase.

/* Returns 1 if complete, 0 if in progress, -1 on error (e.g. NULL conn) */
S2N_API extern int s2n_connection_handshake_complete(struct s2n_connection *conn);

Analogous to OpenSSL's SSL_is_init_finished and Rustls' is_handshaking.

Callouts

The Rust binding uses .into_result() rather than the private Error::capture().

Testing

All 281 unit tests pass including the new s2n_handshake_complete_test, which covers
NULL safety, fresh connection, TLS 1.2, TLS 1.3, and the return value contract.

Exposes the internal s2n_handshake_is_complete() check as a public API so external integrations can reliably determine when the TLS handshake is fully complete without relying on indirect signals like error codes, blocked statuses, or handshake type strings. Fixes a TLS 1.2 regression where checking handshake_type() for 'NEGOTIATED' caused the handshake loop to exit one iteration too early, leaving the server's Finished message unread. That stray record then arrived with application data and was misrouted through s2n_post_handshake_recv(), triggering S2N_ERR_BAD_MESSAGE. Changes: - api/s2n.h: declare s2n_connection_handshake_complete() with full docs - tls/s2n_handshake_io.c: implement as a null-safe wrapper around s2n_handshake_is_complete() - tests/unit/s2n_handshake_complete_test.c: tests covering NULL safety, fresh connection, TLS 1.2 and TLS 1.3 completion, return value contract - bindings/rust/extended: add safe handshake_complete() wrapper - bindings/rust/standard: remove brittle handshake_done workaround Closes aws#5625 Related: aws#5624, aws#854

FreezB11 · 2026-06-05T09:54:34Z

@kaukabrizvi @jmayclin, hey there, is it possible for review of this PR

FreezB11 · 2026-06-11T16:31:40Z

+        match res {
+            1 => Ok(true),
+            0 => Ok(false),
+            _ => Err(Error::capture()),


@note -> error here

pub fn handshake_complete(&self) -> Result<bool, Error> {
unsafe {
s2n_connection_handshake_complete(self.connection.as_ptr())
.into_result()
.map(|res| res != 0)
}
}

@kaukabrizvi do you think will work i didnt see the rust implementation correctly, i hope this will work shall i patch this in

This suggestion looks good, and is in line with other FFI calls in the file. I'd patch it in.

I have patched that also there was fmt error which is also fixed.
Could you please look at the rust side of code before pushing it to actions

jmayclin

Could you please adopt the PR template in the repo?

jmayclin · 2026-06-08T16:04:40Z

+ * @returns 1 if the handshake is complete, 0 if still in progress,
+ *          or -1 on error (e.g. NULL conn).
+ */
+S2N_API extern int s2n_connection_handshake_complete(struct s2n_connection *conn);


I'd prefer to return a bool here.

int was to represent the state, bool can either be 0,1

I agree that this is functionally correct, but I would argue that returning a bool is more ergonomic and easier for consumers.

I'd also like to make this function infallible. Outside of the null case, it seems odd to make this function failable. And for the null cause I'd just pick a default value to return (e.g. just always return false if connection is null)

Co-authored-by: James Mayclin <maycj@amazon.com>

found the issue form the actions report

FreezB11 · 2026-06-11T16:56:07Z

i have update the PR description, My bad i didnt look into the .github folder, i have updated the connection.rs also,

struct literal needed to be one line

FreezB11 · 2026-06-11T18:36:14Z

yup finally no errorsssss

kaukabrizvi · 2026-06-11T18:20:34Z

+ * "Complete" means all handshake messages have been sent and received,
+ * including the TLS 1.2 server Finished message. Once this returns 1,
+ * the connection is ready for application data.
+ *


Suggested change

*

What is the behavior of this function when negotiation fails? I think we'd want the API to return true, negotiation is complete (and it failed). To double check this could you add some assertions in e.g. https://github.com/aws/s2n-tls/blob/main/bindings/rust/standard/integration/src/handshake/handshake_failure_errors.rs?

kaukabrizvi · 2026-06-11T18:26:53Z

@@ -131,10 +131,6 @@ pub struct S2NConnection {
    io: Pin<Box<ViewIO>>,
    connection: Connection,
    // We have to store the result of s2n_negotiate to know when the handshake is complete.


This comment is stale.

kaukabrizvi · 2026-06-11T20:02:36Z

+        EXPECT_EQUAL(s2n_connection_handshake_complete(server_conn), 0);
+
+        /* Drive the handshake fully to completion */
+        EXPECT_SUCCESS(s2n_negotiate_test_server_and_client(server_conn, client_conn));


Since you call negotiate end-to-end, the final assertion would also pass against the old handshake_type contains "NEGOTIATED" logic, both sides finish either way. To actually exercise the path the comment and issue describe, you'd want to drive the handshake until the server has sent Finished but the client hasn't read it yet, and assert completion is false there (where the old logic would have returned true). Without that step, this doesn't add coverage beyond handshake_type.

kaukabrizvi · 2026-06-11T20:21:36Z

+        EXPECT_SUCCESS(s2n_config_free(config));
+    };
+
+    /* ── TLS 1.3: complete after initial handshake exchange ─────────────


Again, this doesn't really test the claimed behavior. The claim is that post-handshake messages should not reset the completion flag but the test does not send any post-handshake messages.

Drive the handshake step-by-step so we can assert that s2n_connection_handshake_complete() returns 0 on the client side after the server has sent its Finished message but before the client has consumed it. The old handshake_type/NEGOTIATED-bit logic would have returned 1 at that point; the new s2n_handshake_is_complete() path must return 0. Without this intermediate assertion the test offered no coverage beyond the existing handshake_type check.

…tion checks

FreezB11 · 2026-06-12T07:20:45Z

the unit test has been corrected.

FreezB11 · 2026-07-06T20:48:56Z

@jmayclin @kaukabrizvi @bdonlan @glguy could someone help review and push this

FreezB11 and others added 3 commits June 5, 2026 03:06

Merge branch 'main' into feature/handshake-complete-api

50cf982

Merge branch 'main' into feature/handshake-complete-api

59e3fed

Merge branch 'main' into feature/handshake-complete-api

a4f2cfc

kaukabrizvi self-requested a review June 11, 2026 16:23

Merge branch 'main' into feature/handshake-complete-api

d3d1dea

FreezB11 commented Jun 11, 2026

View reviewed changes

jmayclin reviewed Jun 11, 2026

View reviewed changes

FreezB11 and others added 3 commits June 11, 2026 22:09

Update tests/unit/s2n_handshake_complete_test.c

7b6834f

Co-authored-by: James Mayclin <maycj@amazon.com>

Update api/s2n.h

02f4091

Co-authored-by: James Mayclin <maycj@amazon.com>

rust error fix

a2d0e1e

found the issue form the actions report

rust format fix

98ddde9

struct literal needed to be one line

kaukabrizvi reviewed Jun 11, 2026

View reviewed changes

FreezB11 and others added 4 commits June 12, 2026 12:19

Merge branch 'main' into feature/handshake-complete-api

f3f1eda

stale comment - removed

1eed131

tests: step-by-step handshake driving for both TLS 1.2 and 1.3 comple…

dd2f6f6

…tion checks

FreezB11 requested a review from kaukabrizvi June 12, 2026 18:36

FreezB11 added 5 commits June 13, 2026 00:57

Merge branch 'main' into feature/handshake-complete-api

a196d20

Merge branch 'main' into feature/handshake-complete-api

433ce3f

Merge branch 'main' into feature/handshake-complete-api

2ed7b70

Merge branch 'main' into feature/handshake-complete-api

1845987

Merge branch 'main' into feature/handshake-complete-api

18fbfb6

FreezB11 requested a review from jmayclin June 24, 2026 21:27

FreezB11 added 2 commits June 29, 2026 04:17

Merge branch 'main' into feature/handshake-complete-api

87b2fe0

Merge branch 'main' into feature/handshake-complete-api

c9fea5e

Uh oh!

Conversation

FreezB11 commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Goal

Why

How

Callouts

Testing

Related

Uh oh!

FreezB11 commented Jun 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FreezB11 Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmayclin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FreezB11 Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

FreezB11 commented Jun 11, 2026

Uh oh!

FreezB11 commented Jun 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FreezB11 commented Jun 12, 2026

Uh oh!

FreezB11 commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

FreezB11 commented Jun 4, 2026 •

edited

Loading

FreezB11 Jun 11, 2026 •

edited

Loading

FreezB11 Jun 11, 2026 •

edited

Loading