refactor: replace s2n_raw_write and add large client hello test

Author: Boquan Fang

crypto/s2n_ecc_evp.c

@@ -412,22 +412,18 @@ int s2n_ecc_evp_write_params_point(struct s2n_ecc_evp_params *ecc_evp_params, st
     POSIX_ENSURE_REF(out);
 
 #if EVP_APIS_SUPPORTED
-    struct s2n_blob point_blob = { 0 };
     uint8_t *encoded_point = NULL;
 
     size_t size = EVP_PKEY_get1_tls_encodedpoint(ecc_evp_params->evp_pkey, &encoded_point);
     if (size != ecc_evp_params->negotiated_curve->share_size) {
         OPENSSL_free(encoded_point);
         POSIX_BAIL(S2N_ERR_ECDHE_SERIALIZING);
     } else {
-        point_blob.data = s2n_stuffer_raw_write(out, ecc_evp_params->negotiated_curve->share_size);
-        POSIX_ENSURE_REF(point_blob.data);
-        POSIX_CHECKED_MEMCPY(point_blob.data, encoded_point, size);
+        POSIX_GUARD(s2n_stuffer_write_bytes(out, encoded_point, size));
         OPENSSL_free(encoded_point);
     }
 #else
-    uint8_t point_len;
-    struct s2n_blob point_blob = { 0 };
+    uint8_t point_len = 0;
 
     DEFER_CLEANUP(EC_KEY *ec_key = EVP_PKEY_get1_EC_KEY(ecc_evp_params->evp_pkey), EC_KEY_free_pointer);
     S2N_ERROR_IF(ec_key == NULL, S2N_ERR_ECDHE_UNSUPPORTED_CURVE);
@@ -437,11 +433,19 @@ int s2n_ecc_evp_write_params_point(struct s2n_ecc_evp_params *ecc_evp_params, st
 
     POSIX_GUARD(s2n_ecc_evp_calculate_point_length(point, group, &point_len));
     S2N_ERROR_IF(point_len != ecc_evp_params->negotiated_curve->share_size, S2N_ERR_ECDHE_SERIALIZING);
-    point_blob.data = s2n_stuffer_raw_write(out, point_len);
-    POSIX_ENSURE_REF(point_blob.data);
-    point_blob.size = point_len;
 
-    POSIX_GUARD(s2n_ecc_evp_write_point_data_snug(point, group, &point_blob));
+    /* A safer way to do s2n_stuffer_raw_write without tainted the out stuffer */
+    POSIX_GUARD(s2n_stuffer_reserve_space(out, point_len));
+    struct s2n_stuffer copy = *out;
+    {
+        struct s2n_blob point_blob = { 0 };
+        point_blob.data = s2n_stuffer_raw_write(&copy, point_len);
+        POSIX_ENSURE_REF(point_blob.data);
+        point_blob.size = point_len;
+
+        POSIX_GUARD(s2n_ecc_evp_write_point_data_snug(point, group, &point_blob));
+    }
+    out->write_cursor += point_len;
 #endif
     return 0;
 }
@@ -456,8 +460,14 @@ int s2n_ecc_evp_write_params(struct s2n_ecc_evp_params *ecc_evp_params, struct s
     POSIX_ENSURE_REF(written);
 
     uint8_t key_share_size = ecc_evp_params->negotiated_curve->share_size;
-    /* Remember where the written data starts */
-    written->data = s2n_stuffer_raw_write(out, 0);
+    /**
+     * Remember where the written data starts with 
+     * a safer way to do s2n_stuffer_raw_write without tainted the out stuffer.
+     */
+    struct s2n_stuffer copy = *out;
+    {
+        written->data = s2n_stuffer_raw_write(&copy, 0);
+    }
     POSIX_ENSURE_REF(written->data);
 
     POSIX_GUARD(s2n_stuffer_write_uint8(out, TLS_EC_CURVE_TYPE_NAMED));

tests/unit/s2n_client_hello_test.c

@@ -44,6 +44,14 @@
 #define COMPRESSION_METHODS     0x00, 0x01, 0x02, 0x03, 0x04
 #define COMPRESSION_METHODS_LEN 0x05
 
+#define CIPHER_SUITES_MAX_LENGTH     (UINT16_MAX - 2)
+#define NUM_OF_CIPHER_SUITES_TO_DROP 150
+#define MAXIMUM_NUM_OF_CIPHER_SUITES (CIPHER_SUITES_MAX_LENGTH / S2N_TLS_CIPHER_SUITE_LEN)
+/* Drop 150 cipher suites from max, so that the total handshake message length won't exceed 64KB */
+#define REDUCED_CIPHER_SUITE_COUNT (MAXIMUM_NUM_OF_CIPHER_SUITES - NUM_OF_CIPHER_SUITES_TO_DROP)
+/* Reducing cipher suites by 150 creates approximately 300 bytes margin below maximum handshake length */
+#define ESTMATIED_MAX_HANDSHAKE_LENGTH_MARGIN (NUM_OF_CIPHER_SUITES_TO_DROP * S2N_TLS_CIPHER_SUITE_LEN)
+
 int s2n_parse_client_hello(struct s2n_connection *conn);
 S2N_RESULT s2n_client_hello_get_raw_extension(uint16_t extension_iana,
         struct s2n_blob *raw_extensions, struct s2n_blob *extension);
@@ -1953,6 +1961,138 @@ int main(int argc, char **argv)
         };
     };
 
+    /* Test: Client Hello is slightly less than 64KB */
+    {
+        DEFER_CLEANUP(struct s2n_config *client_config = s2n_config_new(), s2n_config_ptr_free);
+        EXPECT_NOT_NULL(client_config);
+
+        DEFER_CLEANUP(struct s2n_config *server_config = s2n_config_new(), s2n_config_ptr_free);
+        EXPECT_NOT_NULL(server_config);
+
+        EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(server_config, chain_and_key));
+
+        struct s2n_cipher_suite *test_cipher_suites[REDUCED_CIPHER_SUITE_COUNT] = { 0 };
+
+        for (int i = 0; i < REDUCED_CIPHER_SUITE_COUNT; i++) {
+            test_cipher_suites[i] = &s2n_rsa_with_aes_128_gcm_sha256;
+        }
+
+        const struct s2n_cipher_preferences test_cipher_suites_preferences = {
+            .count = s2n_array_len(test_cipher_suites),
+            .suites = test_cipher_suites,
+        };
+
+        struct s2n_security_policy test_security_policy = {
+            .minimum_protocol_version = S2N_TLS12,
+            .cipher_preferences = &test_cipher_suites_preferences,
+            .kem_preferences = &kem_preferences_null,
+            .signature_preferences = &s2n_signature_preferences_20240501,
+            .ecc_preferences = &s2n_ecc_preferences_20240501,
+            .rules = {
+                    [S2N_PERFECT_FORWARD_SECRECY] = true,
+            },
+        };
+
+        EXPECT_SUCCESS(s2n_config_disable_x509_verification(client_config));
+
+        DEFER_CLEANUP(struct s2n_connection *client = s2n_connection_new(S2N_CLIENT),
+                s2n_connection_ptr_free);
+        EXPECT_NOT_NULL(client);
+        EXPECT_SUCCESS(s2n_connection_set_config(client, client_config));
+        EXPECT_SUCCESS(s2n_connection_set_blinding(client, S2N_SELF_SERVICE_BLINDING));
+        client->security_policy_override = &test_security_policy;
+
+        DEFER_CLEANUP(struct s2n_connection *server = s2n_connection_new(S2N_SERVER),
+                s2n_connection_ptr_free);
+        EXPECT_NOT_NULL(server);
+        EXPECT_SUCCESS(s2n_connection_set_config(server, server_config));
+        EXPECT_SUCCESS(s2n_connection_set_blinding(server, S2N_SELF_SERVICE_BLINDING));
+        server->security_policy_override = &test_security_policy;
+
+        DEFER_CLEANUP(struct s2n_test_io_stuffer_pair io_pair = { 0 }, s2n_io_stuffer_pair_free);
+        EXPECT_OK(s2n_io_stuffer_pair_init(&io_pair));
+        EXPECT_OK(s2n_connections_set_io_stuffer_pair(client, server, &io_pair));
+
+        EXPECT_OK(s2n_negotiate_test_server_and_client_until_message(server, client, SERVER_HELLO));
+
+        /* handshake.io shouldn't be tainted after sending and receiving large client hello */
+        EXPECT_TRUE(client->handshake.io.tainted == 0);
+        EXPECT_TRUE(server->handshake.io.tainted == 0);
+
+        struct s2n_client_hello *client_hello = s2n_connection_get_client_hello(server);
+        EXPECT_NOT_NULL(client_hello);
+        uint32_t handshake_max_len_margin = S2N_MAXIMUM_HANDSHAKE_MESSAGE_LENGTH - s2n_client_hello_get_raw_message_length(client_hello);
+        /* Size of Client Hello is less than 300 bytes from the maximum handshake length */
+        EXPECT_TRUE(handshake_max_len_margin < ESTMATIED_MAX_HANDSHAKE_LENGTH_MARGIN);
+    }
+
+    /* Test: Client Hello is larger than 64KB */
+    {
+        DEFER_CLEANUP(struct s2n_config *client_config = s2n_config_new(), s2n_config_ptr_free);
+        EXPECT_NOT_NULL(client_config);
+
+        DEFER_CLEANUP(struct s2n_config *server_config = s2n_config_new(), s2n_config_ptr_free);
+        EXPECT_NOT_NULL(server_config);
+
+        EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(server_config, chain_and_key));
+
+        struct s2n_cipher_suite *test_cipher_suites[MAXIMUM_NUM_OF_CIPHER_SUITES] = { 0 };
+
+        for (int i = 0; i < MAXIMUM_NUM_OF_CIPHER_SUITES; i++) {
+            test_cipher_suites[i] = &s2n_rsa_with_aes_128_gcm_sha256;
+        }
+
+        /* We append the maximun number of cipher suites, so that the Client Hello size grows beyond 64KB */
+        const struct s2n_cipher_preferences test_cipher_suites_preferences = {
+            .count = s2n_array_len(test_cipher_suites),
+            .suites = test_cipher_suites,
+        };
+
+        struct s2n_security_policy test_security_policy = {
+            .minimum_protocol_version = S2N_TLS12,
+            .cipher_preferences = &test_cipher_suites_preferences,
+            .kem_preferences = &kem_preferences_null,
+            .signature_preferences = &s2n_signature_preferences_20240501,
+            .ecc_preferences = &s2n_ecc_preferences_20240501,
+            .rules = {
+                    [S2N_PERFECT_FORWARD_SECRECY] = true,
+            },
+        };
+
+        EXPECT_SUCCESS(s2n_config_disable_x509_verification(client_config));
+
+        DEFER_CLEANUP(struct s2n_connection *client = s2n_connection_new(S2N_CLIENT),
+                s2n_connection_ptr_free);
+        EXPECT_NOT_NULL(client);
+        EXPECT_SUCCESS(s2n_connection_set_config(client, client_config));
+        EXPECT_SUCCESS(s2n_connection_set_blinding(client, S2N_SELF_SERVICE_BLINDING));
+        client->security_policy_override = &test_security_policy;
+
+        DEFER_CLEANUP(struct s2n_connection *server = s2n_connection_new(S2N_SERVER),
+                s2n_connection_ptr_free);
+        EXPECT_NOT_NULL(server);
+        EXPECT_SUCCESS(s2n_connection_set_config(server, server_config));
+        EXPECT_SUCCESS(s2n_connection_set_blinding(server, S2N_SELF_SERVICE_BLINDING));
+        server->security_policy_override = &test_security_policy;
+
+        DEFER_CLEANUP(struct s2n_test_io_stuffer_pair io_pair = { 0 }, s2n_io_stuffer_pair_free);
+        EXPECT_OK(s2n_io_stuffer_pair_init(&io_pair));
+        EXPECT_OK(s2n_connections_set_io_stuffer_pair(client, server, &io_pair));
+
+        s2n_blocked_status blocked = S2N_NOT_BLOCKED;
+
+        /* Write Client Hello into io_pair.server_in */
+        s2n_negotiate(client, &blocked);
+
+        /* The size of Client Hello exceeds S2N_MAXIMUM_HANDSHAKE_MESSAGE_LENGTH */
+        EXPECT_TRUE(io_pair.server_in.write_cursor > S2N_MAXIMUM_HANDSHAKE_MESSAGE_LENGTH);
+        EXPECT_ERROR_WITH_ERRNO(s2n_negotiate_test_server_and_client_until_message(server, client, SERVER_HELLO), S2N_ERR_BAD_MESSAGE);
+
+        /* handshake.io shouldn't be tainted after sending and receiving large client hello */
+        EXPECT_TRUE(client->handshake.io.tainted == 0);
+        EXPECT_TRUE(server->handshake.io.tainted == 0);
+    }
+
     EXPECT_SUCCESS(s2n_cert_chain_and_key_free(chain_and_key));
     EXPECT_SUCCESS(s2n_cert_chain_and_key_free(ecdsa_chain_and_key));
     END_TEST();

tls/extensions/s2n_client_early_data_indication.c

@@ -71,7 +71,7 @@ static S2N_RESULT s2n_early_data_config_is_possible(struct s2n_connection *conn)
 
     /* Early data must require a supported cipher */
     bool match = false;
-    for (uint8_t i = 0; i < cipher_preferences->count; i++) {
+    for (size_t i = 0; i < cipher_preferences->count; i++) {
         if (cipher_preferences->suites[i] == early_data_config->cipher_suite) {
             match = true;
             break;

tls/s2n_cipher_preferences.h

@@ -22,7 +22,7 @@
 #include "tls/s2n_tls13.h"
 
 struct s2n_cipher_preferences {
-    uint8_t count;
+    uint16_t count;
     struct s2n_cipher_suite **suites;
     bool allow_chacha20_boosting;
 };

tls/s2n_security_policies.c

@@ -1509,7 +1509,7 @@ bool s2n_ecc_is_extension_required(const struct s2n_security_policy *security_po
     if (cipher_preferences == NULL) {
         return false;
     }
-    for (uint8_t i = 0; i < cipher_preferences->count; i++) {
+    for (size_t i = 0; i < cipher_preferences->count; i++) {
         if (s2n_cipher_suite_requires_ecc_extension(cipher_preferences->suites[i])) {
             return true;
         }
@@ -1540,7 +1540,7 @@ bool s2n_pq_kem_is_extension_required(const struct s2n_security_policy *security
     if (cipher_preferences == NULL) {
         return false;
     }
-    for (uint8_t i = 0; i < cipher_preferences->count; i++) {
+    for (size_t i = 0; i < cipher_preferences->count; i++) {
         if (s2n_cipher_suite_requires_pq_extension(cipher_preferences->suites[i])) {
             return true;
         }
@@ -1569,7 +1569,7 @@ bool s2n_security_policy_supports_tls13(const struct s2n_security_policy *securi
         return false;
     }
 
-    for (uint8_t i = 0; i < cipher_preferences->count; i++) {
+    for (size_t i = 0; i < cipher_preferences->count; i++) {
         if (cipher_preferences->suites[i]->minimum_required_tls_version >= S2N_TLS13) {
             return true;
         }