check for duplicate key share before sending

aws · Jul 1, 2022 · 88dc849 · 88dc849
1 parent 18f74e7
commit 88dc849
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 8 deletions.
diff --git a/tests/unit/s2n_client_key_share_extension_test.c b/tests/unit/s2n_client_key_share_extension_test.c
@@ -315,7 +315,7 @@ int main(int argc, char **argv)
             EXPECT_SUCCESS(s2n_connection_free(conn));
         }
 
-        /* For HelloRetryRequests, verify that we can resend an existing share to reject early data. */
+        /* For HelloRetryRequests, verify that we cannot resend an existing share. */
         {
             struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT);
             EXPECT_NOT_NULL(conn);
@@ -341,13 +341,8 @@ int main(int argc, char **argv)
             conn->early_data_state = S2N_EARLY_DATA_REJECTED;
             conn->kex_params.server_ecc_evp_params.negotiated_curve = curve;
 
-            EXPECT_SUCCESS(s2n_client_key_share_extension.send(conn, &second_extension));
-            EXPECT_EQUAL(conn->kex_params.client_ecc_evp_params.negotiated_curve, curve);
-            EXPECT_NOT_NULL(conn->kex_params.client_ecc_evp_params.evp_pkey);
-
-            /* Same shares (same bytes) are written both times */
-            EXPECT_EQUAL(first_extension.write_cursor, second_extension.write_cursor);
-            EXPECT_BYTEARRAY_EQUAL(first_extension.blob.data, second_extension.blob.data, first_extension.write_cursor);
+            EXPECT_FAILURE_WITH_ERRNO(s2n_client_key_share_extension.send(conn, &second_extension),
+                    S2N_ERR_BAD_KEY_SHARE);
 
             EXPECT_SUCCESS(s2n_stuffer_free(&first_extension));
             EXPECT_SUCCESS(s2n_stuffer_free(&second_extension));

diff --git a/tls/extensions/s2n_client_key_share.c b/tls/extensions/s2n_client_key_share.c
@@ -189,6 +189,16 @@ static int s2n_generate_default_pq_hybrid_key_share(struct s2n_connection *conn,
 
 static int s2n_client_key_share_send(struct s2n_connection *conn, struct s2n_stuffer *out)
 {
+    if (s2n_is_hello_retry_handshake(conn)) {
+        const struct s2n_ecc_named_curve *server_curve = conn->kex_params.server_ecc_evp_params.negotiated_curve;
+        const struct s2n_ecc_named_curve *client_curve = conn->kex_params.client_ecc_evp_params.negotiated_curve;
+        const struct s2n_kem_group *server_group = conn->kex_params.server_kem_group_params.kem_group;
+        const struct s2n_kem_group *client_group = conn->kex_params.client_kem_group_params.kem_group;
+
+        /* Ensure a new key share will be sent after a hello retry request */
+        POSIX_ENSURE(server_curve != client_curve || server_group != client_group, S2N_ERR_BAD_KEY_SHARE);
+    }
+
     struct s2n_stuffer_reservation shares_size = {0};
     POSIX_GUARD(s2n_stuffer_reserve_uint16(out, &shares_size));
     POSIX_GUARD(s2n_generate_default_pq_hybrid_key_share(conn, out));