Remove unused s2n_config_client_hello_cb_enable_poll (#3850)

bindings/rust/s2n-tls-sys/src/internal.rs

@@ -18,6 +18,3 @@ extern "C" {
         config: *mut *mut s2n_config,
     ) -> ::libc::c_int;
 }
-extern "C" {
-    pub fn s2n_config_client_hello_cb_enable_poll(config: *mut s2n_config) -> ::libc::c_int;
-}

tests/unit/s2n_self_talk_client_hello_cb_test.c

@@ -37,7 +37,6 @@ struct client_hello_context {
      * this flag tests the previous behavior from blocking callbacks
      */
     int legacy_rc_for_server_name_used;
-    bool mark_done;
 };
 
 int mock_client(struct s2n_test_io_pair *io_pair, int expect_failure, int expect_server_name_used)
@@ -183,24 +182,6 @@ int client_hello_fail_handshake(struct s2n_connection *conn, void *ctx)
     return -1;
 }
 
-int s2n_client_hello_poll_cb(struct s2n_connection *conn, void *ctx)
-{
-    struct client_hello_context *client_hello_ctx;
-    if (ctx == NULL) {
-        return -1;
-    }
-    client_hello_ctx = ctx;
-    /* Increment counter to ensure that callback was invoked */
-    client_hello_ctx->invoked++;
-
-    if (client_hello_ctx->mark_done) {
-        EXPECT_SUCCESS(s2n_client_hello_cb_done(conn));
-        return S2N_SUCCESS;
-    }
-
-    return S2N_SUCCESS;
-}
-
 int s2n_negotiate_nonblocking_ch_cb(struct s2n_connection *conn,
         struct client_hello_context *ch_ctx, bool server_name_used)
 {
@@ -229,34 +210,6 @@ int s2n_negotiate_nonblocking_ch_cb(struct s2n_connection *conn,
     return s2n_negotiate(conn, &blocked);
 }
 
-int s2n_negotiate_nonblocking_poll(struct s2n_connection *conn,
-        struct client_hello_context *ch_ctx)
-{
-    EXPECT_NOT_NULL(conn);
-    EXPECT_NOT_NULL(ch_ctx);
-    int invoked = 0;
-    s2n_blocked_status blocked = S2N_NOT_BLOCKED;
-
-    EXPECT_EQUAL(ch_ctx->invoked, 0);
-
-    do {
-        /* negotiate handshake, we should pause after the nonblocking callback is invoked */
-        EXPECT_FAILURE_WITH_ERRNO(s2n_negotiate(conn, &blocked), S2N_ERR_ASYNC_BLOCKED);
-        EXPECT_EQUAL(blocked, S2N_BLOCKED_ON_APPLICATION_INPUT);
-        invoked++;
-        EXPECT_EQUAL(ch_ctx->invoked, invoked);
-    } while (invoked < 10);
-    EXPECT_EQUAL(ch_ctx->invoked, invoked);
-
-    ch_ctx->mark_done = true;
-
-    /* Expect the callback to complete after 2nd iteration */
-    EXPECT_SUCCESS(s2n_negotiate(conn, &blocked));
-    EXPECT_EQUAL(ch_ctx->invoked, invoked + 1);
-
-    return S2N_SUCCESS;
-}
-
 int s2n_negotiate_blocking_ch_cb(struct s2n_connection *conn, struct client_hello_context *ch_ctx)
 {
     s2n_blocked_status blocked;
@@ -477,37 +430,6 @@ int run_test_reject_handshake_ch_cb(s2n_client_hello_cb_mode cb_mode,
     return S2N_SUCCESS;
 }
 
-int run_test_poll_ch_cb(s2n_client_hello_cb_mode cb_mode,
-        struct s2n_cert_chain_and_key *chain_and_key,
-        struct client_hello_context *ch_ctx)
-{
-    struct s2n_test_io_pair io_pair = { 0 };
-    struct s2n_config *config = s2n_config_new();
-    EXPECT_NOT_NULL(config);
-    struct s2n_connection *conn;
-    pid_t pid = 0;
-
-    EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, chain_and_key));
-
-    /* Setup ClientHello callback */
-    EXPECT_SUCCESS(s2n_config_set_client_hello_cb(config, s2n_client_hello_poll_cb, ch_ctx));
-    EXPECT_SUCCESS(s2n_config_set_client_hello_cb_mode(config, cb_mode));
-
-    /* Enable callback polling mode */
-    EXPECT_SUCCESS(s2n_config_client_hello_cb_enable_poll(config));
-
-    EXPECT_SUCCESS(start_client_conn(&io_pair, &pid, 0, 0));
-    EXPECT_SUCCESS(init_server_conn(&conn, &io_pair, config));
-
-    /* negotiate and make assertions */
-    EXPECT_SUCCESS(s2n_negotiate_nonblocking_poll(conn, ch_ctx));
-
-    EXPECT_SUCCESS(server_recv(conn));
-
-    EXPECT_SUCCESS(test_case_clean(conn, pid, config, &io_pair, ch_ctx));
-    return S2N_SUCCESS;
-}
-
 int main(int argc, char **argv)
 {
     struct client_hello_context client_hello_ctx = { 0 };
@@ -561,9 +483,6 @@ int main(int argc, char **argv)
     EXPECT_SUCCESS(run_test_reject_handshake_ch_cb(S2N_CLIENT_HELLO_CB_NONBLOCKING,
             chain_and_key, &client_hello_ctx));
 
-    EXPECT_SUCCESS(run_test_poll_ch_cb(S2N_CLIENT_HELLO_CB_NONBLOCKING,
-            chain_and_key, &client_hello_ctx));
-
     EXPECT_SUCCESS(s2n_cert_chain_and_key_free(chain_and_key));
     free(cert_chain_pem);
     free(private_key_pem);

tests/unit/s2n_server_hello_retry_test.c

@@ -37,7 +37,6 @@ struct client_hello_context {
     int invocations;
     s2n_client_hello_cb_mode mode;
     bool mark_done;
-    bool enable_poll;
 };
 
 int s2n_negotiate_poll_hello_retry(struct s2n_connection *server_conn,
@@ -47,20 +46,6 @@ int s2n_negotiate_poll_hello_retry(struct s2n_connection *server_conn,
     s2n_blocked_status blocked = S2N_NOT_BLOCKED;
     EXPECT_FAILURE_WITH_ERRNO(s2n_negotiate(client_conn, &blocked), S2N_ERR_IO_BLOCKED);
 
-    int expected_invocation = 0;
-
-    /* if polling is enabled then confirm that the callback is incremented each time */
-    if (client_hello_ctx->enable_poll) {
-        do {
-            /* invocation should increase each time s2n_negotiate is called */
-            EXPECT_FAILURE_WITH_ERRNO(s2n_negotiate(server_conn, &blocked), S2N_ERR_ASYNC_BLOCKED);
-            EXPECT_EQUAL(blocked, S2N_BLOCKED_ON_APPLICATION_INPUT);
-            expected_invocation++;
-            EXPECT_EQUAL(client_hello_ctx->invocations, expected_invocation);
-        } while (expected_invocation < 10);
-    }
-    EXPECT_EQUAL(client_hello_ctx->invocations, expected_invocation);
-
     /* complete the callback on the next call */
     client_hello_ctx->mark_done = true;
     EXPECT_SUCCESS(s2n_negotiate_test_server_and_client(server_conn, client_conn));
@@ -69,8 +54,7 @@ int s2n_negotiate_poll_hello_retry(struct s2n_connection *server_conn,
      * hello retry will invoke the s2n_negotiate twice but the callback should
      * be called once regardless of polling
      */
-    expected_invocation++;
-    EXPECT_EQUAL(client_hello_ctx->invocations, expected_invocation);
+    EXPECT_EQUAL(client_hello_ctx->invocations, 1);
 
     return S2N_SUCCESS;
 }
@@ -110,7 +94,7 @@ int s2n_client_hello_poll_cb(struct s2n_connection *conn, void *ctx)
     return S2N_SUCCESS;
 }
 
-S2N_RESULT hello_retry_client_hello_cb_test(bool enable_poll)
+S2N_RESULT hello_retry_client_hello_cb_test()
 {
     struct s2n_cert_chain_and_key *tls13_chain_and_key = NULL;
     EXPECT_SUCCESS(s2n_test_cert_chain_and_key_new(&tls13_chain_and_key,
@@ -142,18 +126,12 @@ S2N_RESULT hello_retry_client_hello_cb_test(bool enable_poll)
     /* setup the client hello callback */
     struct client_hello_context client_hello_ctx = { .invocations = 0,
         .mode = S2N_CLIENT_HELLO_CB_NONBLOCKING,
-        .mark_done = false,
-        .enable_poll = enable_poll };
+        .mark_done = false };
     EXPECT_SUCCESS(s2n_config_set_client_hello_cb(config,
             s2n_client_hello_poll_cb, &client_hello_ctx));
     EXPECT_SUCCESS(s2n_config_set_client_hello_cb_mode(config,
             S2N_CLIENT_HELLO_CB_NONBLOCKING));
 
-    if (enable_poll) {
-        /* Enable callback polling mode */
-        EXPECT_SUCCESS(s2n_config_client_hello_cb_enable_poll(config));
-    }
-
     /* negotiate and make assertions */
     EXPECT_SUCCESS(s2n_negotiate_poll_hello_retry(server_conn, client_conn, &client_hello_ctx));
 
@@ -542,10 +520,7 @@ int main(int argc, char **argv)
 
     /* Hello Retry Request + (poll and no-poll) client hello callback */
     {
-        /* enable polling */
-        EXPECT_OK(hello_retry_client_hello_cb_test(true));
-        /* disable polling */
-        EXPECT_OK(hello_retry_client_hello_cb_test(false));
+        EXPECT_OK(hello_retry_client_hello_cb_test());
     };
 
     /* Test s2n_set_hello_retry_required correctly sets the handshake type to HELLO_RETRY_REQUEST,

tls/s2n_client_hello.c

@@ -505,31 +505,24 @@ static S2N_RESULT s2n_client_hello_process_cb_response(struct s2n_connection *co
     RESULT_BAIL(S2N_ERR_CANCELLED);
 }
 
-bool s2n_client_hello_invoke_callback(struct s2n_connection *conn)
-{
-    /* Invoke only if the callback has not been called or if polling mode is enabled */
-    bool invoke = !conn->client_hello.callback_invoked || conn->config->client_hello_cb_enable_poll;
-    /*
-     * The callback should not be called if this client_hello is in response to a hello retry.
-     */
-    return invoke && !IS_HELLO_RETRY_HANDSHAKE(conn);
-}
-
 int s2n_client_hello_recv(struct s2n_connection *conn)
 {
-    if (conn->config->client_hello_cb_enable_poll == 0) {
-        POSIX_ENSURE(conn->client_hello.callback_async_blocked == 0, S2N_ERR_ASYNC_BLOCKED);
-    }
+    POSIX_ENSURE(!conn->client_hello.callback_async_blocked, S2N_ERR_ASYNC_BLOCKED);
 
-    if (conn->client_hello.parsed == 0) {
-        /* Parse client hello */
+    /* Only parse the ClientHello once */
+    if (!conn->client_hello.parsed) {
         POSIX_GUARD(s2n_parse_client_hello(conn));
-        conn->client_hello.parsed = 1;
+        conn->client_hello.parsed = true;
     }
-    /* Call the client_hello_cb once unless polling is enabled. */
-    if (s2n_client_hello_invoke_callback(conn)) {
+
+    /* Only invoke the ClientHello callback once.
+     * This means that we do NOT invoke the callback again on the second ClientHello
+     * in a TLS1.3 retry handshake. We explicitly check for a retry because the
+     * callback state may have been cleared while parsing the second ClientHello.
+     */
+    if (!conn->client_hello.callback_invoked && !IS_HELLO_RETRY_HANDSHAKE(conn)) {
         /* Mark the collected client hello as available when parsing is done and before the client hello callback */
-        conn->client_hello.callback_invoked = 1;
+        conn->client_hello.callback_invoked = true;
 
         /* Call client_hello_cb if exists, letting application to modify s2n_connection or swap s2n_config */
         if (conn->config->client_hello_cb) {

tls/s2n_config.c

@@ -990,20 +990,6 @@ int s2n_config_get_ctx(struct s2n_config *config, void **ctx)
     return S2N_SUCCESS;
 }
 
-/*
- * Set the client_hello callback behavior to polling.
- *
- * Polling means that the callback function can be called multiple times.
- */
-int s2n_config_client_hello_cb_enable_poll(struct s2n_config *config)
-{
-    POSIX_ENSURE_REF(config);
-
-    config->client_hello_cb_enable_poll = 1;
-
-    return S2N_SUCCESS;
-}
-
 int s2n_config_set_send_buffer_size(struct s2n_config *config, uint32_t size)
 {
     POSIX_ENSURE_REF(config);

tls/s2n_config.h

@@ -79,12 +79,6 @@ struct s2n_config {
      * but async signing must be enabled. Use this flag to enforce that restriction.
      */
     unsigned no_signing_key : 1;
-    /*
-     * This option exists to allow for polling the client_hello callback.
-     *
-     * Note: This defaults to false to ensure backwards compatibility.
-     */
-    unsigned client_hello_cb_enable_poll : 1;
     /*
      * Whether to verify signatures locally before sending them over the wire.
      * See s2n_config_set_verify_after_sign.

tls/s2n_internal.h

@@ -44,11 +44,3 @@ struct s2n_connection;
  * modified after it has been built. Doing so is undefined behavior.
  */
 S2N_PRIVATE_API int s2n_connection_get_config(struct s2n_connection *conn, struct s2n_config **config);
-
-/*
- * Enable polling the async client_hello callback to make progress.
- *
- * `s2n_negotiate` must be called multiple times to poll the callback function
- * and make progress.
- */
-S2N_PRIVATE_API int s2n_config_client_hello_cb_enable_poll(struct s2n_config *config);