Clean up sending supported sig algs (#4254)

tests/unit/s2n_client_signature_algorithms_extension_test.c

@@ -64,8 +64,7 @@ int main(int argc, char **argv)
         EXPECT_SUCCESS(s2n_client_signature_algorithms_extension.recv(server_conn, &io));
         EXPECT_EQUAL(s2n_stuffer_data_available(&io), 0);
 
-        EXPECT_EQUAL(server_conn->handshake_params.client_sig_hash_algs.len,
-                s2n_supported_sig_schemes_count(client_conn));
+        EXPECT_TRUE(server_conn->handshake_params.client_sig_hash_algs.len > 0);
 
         s2n_stuffer_free(&io);
         s2n_connection_free(client_conn);

tests/unit/s2n_server_signature_algorithms_extension_test.c

@@ -44,7 +44,7 @@ int main(int argc, char **argv)
         EXPECT_SUCCESS(s2n_server_signature_algorithms_extension.recv(client_conn, &io));
         EXPECT_EQUAL(s2n_stuffer_data_available(&io), 0);
 
-        EXPECT_EQUAL(client_conn->handshake_params.server_sig_hash_algs.len, s2n_supported_sig_schemes_count(server_conn));
+        EXPECT_TRUE(client_conn->handshake_params.server_sig_hash_algs.len > 0);
 
         s2n_stuffer_free(&io);
         EXPECT_SUCCESS(s2n_connection_free(server_conn));

tests/unit/s2n_signature_algorithms_test.c

@@ -61,149 +61,105 @@ int main(int argc, char **argv)
 
     struct s2n_cert_chain_and_key *certs[] = { ecdsa_cert_chain, rsa_cert_chain };
 
-    /* s2n_supported_sig_schemes_count & s2n_supported_sig_scheme_list_size */
+    /* s2n_signature_algorithms_supported_list_send */
     {
-        struct s2n_config *config = s2n_config_new();
-
-        struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT);
-        s2n_connection_set_config(conn, config);
-
-        const struct s2n_security_policy *security_policy = NULL;
-        EXPECT_SUCCESS(s2n_connection_get_security_policy(conn, &security_policy));
-        EXPECT_NOT_NULL(security_policy);
-
-        struct s2n_security_policy test_security_policy = {
-            .minimum_protocol_version = security_policy->minimum_protocol_version,
-            .cipher_preferences = security_policy->cipher_preferences,
-            .kem_preferences = security_policy->kem_preferences,
-            .signature_preferences = &test_preferences,
-            .ecc_preferences = security_policy->ecc_preferences,
-        };
-
-        config->security_policy = &test_security_policy;
-
-        /* Test: if all signatures supported, count all signatures */
-        {
-            conn->actual_protocol_version = S2N_TLS13;
-            EXPECT_EQUAL(s2n_supported_sig_schemes_count(conn), LENGTH);
-            EXPECT_EQUAL(s2n_supported_sig_scheme_list_size(conn), LENGTH * TLS_SIGNATURE_SCHEME_LEN);
-        };
-
-        /* Test: if some signatures are not supported, exclude them from the count */
-        {
-            conn->actual_protocol_version = S2N_TLS10;
-            /* Do not include s2n_ecdsa_secp384r1_sha384, which has a minimum version of tls13 */
-            EXPECT_EQUAL(s2n_supported_sig_schemes_count(conn), LENGTH - 1);
-            EXPECT_EQUAL(s2n_supported_sig_scheme_list_size(conn), (LENGTH - 1) * TLS_SIGNATURE_SCHEME_LEN);
-        };
-
-        s2n_connection_free(conn);
-        s2n_config_free(config);
-    };
-
-    /* s2n_send_supported_sig_scheme_list */
-    {
-        struct s2n_config *config = s2n_config_new();
-
-        struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT);
-        s2n_connection_set_config(conn, config);
-
-        const struct s2n_security_policy *security_policy = NULL;
-        EXPECT_SUCCESS(s2n_connection_get_security_policy(conn, &security_policy));
-        EXPECT_NOT_NULL(security_policy);
-
-        struct s2n_security_policy test_security_policy = {
-            .minimum_protocol_version = security_policy->minimum_protocol_version,
-            .cipher_preferences = security_policy->cipher_preferences,
-            .kem_preferences = security_policy->kem_preferences,
-            .signature_preferences = &test_preferences,
-            .ecc_preferences = security_policy->ecc_preferences,
-        };
-
-        config->security_policy = &test_security_policy;
-
-        struct s2n_stuffer result = { 0 };
-        s2n_stuffer_growable_alloc(&result, STUFFER_SIZE);
-
-        uint16_t size, iana_value;
+        struct s2n_security_policy test_security_policy = *s2n_fetch_default_config()->security_policy;
+        test_security_policy.signature_preferences = &test_preferences;
 
         /* Test: if all signatures supported, send all signatures */
         {
-            s2n_stuffer_wipe(&result);
-
+            DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT),
+                    s2n_connection_ptr_free);
+            EXPECT_NOT_NULL(conn);
+            conn->security_policy_override = &test_security_policy;
             conn->actual_protocol_version = S2N_TLS13;
-            EXPECT_SUCCESS(s2n_send_supported_sig_scheme_list(conn, &result));
 
+            DEFER_CLEANUP(struct s2n_stuffer result = { 0 }, s2n_stuffer_free);
+            EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&result, 0));
+            EXPECT_OK(s2n_signature_algorithms_supported_list_send(conn, &result));
+
+            uint16_t size = 0;
             EXPECT_SUCCESS(s2n_stuffer_read_uint16(&result, &size));
-            EXPECT_EQUAL(size, s2n_supported_sig_scheme_list_size(conn));
+            EXPECT_EQUAL(size, s2n_stuffer_data_available(&result));
 
-            for (int i = 0; i < LENGTH; i++) {
+            for (size_t i = 0; i < s2n_array_len(test_signature_schemes); i++) {
+                uint16_t iana_value = 0;
                 EXPECT_SUCCESS(s2n_stuffer_read_uint16(&result, &iana_value));
                 EXPECT_EQUAL(iana_value, test_signature_schemes[i]->iana_value);
             }
-
             EXPECT_EQUAL(s2n_stuffer_data_available(&result), 0);
         };
 
         /* Test: do not send unsupported signatures */
         {
-            s2n_stuffer_wipe(&result);
-
+            DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT),
+                    s2n_connection_ptr_free);
+            EXPECT_NOT_NULL(conn);
+            conn->security_policy_override = &test_security_policy;
             conn->actual_protocol_version = S2N_TLS12;
-            EXPECT_SUCCESS(s2n_send_supported_sig_scheme_list(conn, &result));
 
+            DEFER_CLEANUP(struct s2n_stuffer result = { 0 }, s2n_stuffer_free);
+            EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&result, 0));
+            EXPECT_OK(s2n_signature_algorithms_supported_list_send(conn, &result));
+
+            uint16_t size = 0;
             EXPECT_SUCCESS(s2n_stuffer_read_uint16(&result, &size));
-            EXPECT_EQUAL(size, s2n_supported_sig_scheme_list_size(conn));
+            EXPECT_EQUAL(size, s2n_stuffer_data_available(&result));
 
-            for (int i = 0; i < LENGTH; i++) {
+            for (size_t i = 0; i < s2n_array_len(test_signature_schemes); i++) {
                 if (test_signature_schemes[i] != &s2n_ecdsa_secp384r1_sha384) {
+                    uint16_t iana_value = 0;
                     EXPECT_SUCCESS(s2n_stuffer_read_uint16(&result, &iana_value));
                     EXPECT_EQUAL(iana_value, test_signature_schemes[i]->iana_value);
                 }
             }
-
             EXPECT_EQUAL(s2n_stuffer_data_available(&result), 0);
-        }
+        };
 
         /* Test: written signatures readable */
         {
-            s2n_stuffer_wipe(&result);
+            DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT),
+                    s2n_connection_ptr_free);
+            EXPECT_NOT_NULL(conn);
+            conn->security_policy_override = &test_security_policy;
             conn->actual_protocol_version = S2N_TLS13;
 
-            struct s2n_sig_scheme_list signatures;
+            DEFER_CLEANUP(struct s2n_stuffer result = { 0 }, s2n_stuffer_free);
+            EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&result, 0));
+            EXPECT_OK(s2n_signature_algorithms_supported_list_send(conn, &result));
 
-            EXPECT_SUCCESS(s2n_send_supported_sig_scheme_list(conn, &result));
+            struct s2n_sig_scheme_list signatures = { 0 };
             EXPECT_SUCCESS(s2n_recv_supported_sig_scheme_list(&result, &signatures));
             EXPECT_EQUAL(s2n_stuffer_data_available(&result), 0);
 
-            EXPECT_EQUAL(signatures.len, LENGTH);
-            for (int i = 0; i < LENGTH; i++) {
+            EXPECT_EQUAL(signatures.len, s2n_array_len(test_signature_schemes));
+            for (size_t i = 0; i < s2n_array_len(test_signature_schemes); i++) {
                 EXPECT_EQUAL(signatures.iana_list[i], test_signature_schemes[i]->iana_value);
             }
         };
 
         /* Test: do not send TLS1.2 signature schemes if QUIC enabled */
         {
-            s2n_stuffer_wipe(&result);
-            config->quic_enabled = true;
-
+            DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT),
+                    s2n_connection_ptr_free);
+            EXPECT_NOT_NULL(conn);
+            conn->security_policy_override = &test_security_policy;
             conn->actual_protocol_version = S2N_TLS13;
-            EXPECT_SUCCESS(s2n_send_supported_sig_scheme_list(conn, &result));
+            conn->quic_enabled = true;
+
+            DEFER_CLEANUP(struct s2n_stuffer result = { 0 }, s2n_stuffer_free);
+            EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&result, 0));
+            EXPECT_OK(s2n_signature_algorithms_supported_list_send(conn, &result));
 
+            uint16_t size = 0;
             EXPECT_SUCCESS(s2n_stuffer_read_uint16(&result, &size));
-            EXPECT_TRUE(size > 0);
-            EXPECT_EQUAL(size, s2n_supported_sig_scheme_list_size(conn));
+            EXPECT_EQUAL(size, s2n_stuffer_data_available(&result));
 
+            uint16_t iana_value = 0;
             EXPECT_SUCCESS(s2n_stuffer_read_uint16(&result, &iana_value));
             EXPECT_EQUAL(iana_value, s2n_ecdsa_secp384r1_sha384.iana_value);
             EXPECT_EQUAL(s2n_stuffer_data_available(&result), 0);
-
-            config->quic_enabled = false;
-        }
-
-        s2n_connection_free(conn);
-        s2n_config_free(config);
-        s2n_stuffer_free(&result);
+        };
     };
 
     /* s2n_get_and_validate_negotiated_signature_scheme */
@@ -689,41 +645,33 @@ int main(int argc, char **argv)
     };
 
     /* Test: send and receive default signature preferences */
-    {
-        struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT);
-
-        struct s2n_stuffer result = { 0 };
-        s2n_stuffer_growable_alloc(&result, STUFFER_SIZE);
-
-        struct s2n_sig_scheme_list signatures;
-
-        for (int i = S2N_TLS10; i < S2N_TLS13; i++) {
-            s2n_stuffer_wipe(&result);
-            conn->actual_protocol_version = i;
-
-            EXPECT_SUCCESS(s2n_send_supported_sig_scheme_list(conn, &result));
-            EXPECT_SUCCESS(s2n_recv_supported_sig_scheme_list(&result, &signatures));
-            EXPECT_EQUAL(s2n_stuffer_data_available(&result), 0);
-
-            EXPECT_EQUAL(signatures.len, s2n_supported_sig_schemes_count(conn));
-
-            /* Verify no duplicates - some preferences contain duplicates, but only
-             * one should be valid at a time. */
-            uint16_t iana, other_iana;
-            for (int a = 0; a < signatures.len; a++) {
-                iana = signatures.iana_list[a];
-                for (int b = 0; b < signatures.len; b++) {
-                    if (a == b) {
-                        continue;
-                    }
-                    other_iana = signatures.iana_list[b];
-                    EXPECT_NOT_EQUAL(iana, other_iana);
+    for (size_t i = S2N_TLS10; i < S2N_TLS13; i++) {
+        DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT),
+                s2n_connection_ptr_free);
+        EXPECT_NOT_NULL(conn);
+        conn->actual_protocol_version = i;
+
+        DEFER_CLEANUP(struct s2n_stuffer result = { 0 }, s2n_stuffer_free);
+        EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&result, 0));
+        EXPECT_OK(s2n_signature_algorithms_supported_list_send(conn, &result));
+
+        struct s2n_sig_scheme_list signatures = { 0 };
+        EXPECT_SUCCESS(s2n_recv_supported_sig_scheme_list(&result, &signatures));
+        EXPECT_EQUAL(s2n_stuffer_data_available(&result), 0);
+
+        /* Verify no duplicates - some preferences contain duplicates, but only
+         * one should be valid at a time. */
+        uint16_t iana = 0, other_iana = 0;
+        for (size_t a = 0; a < signatures.len; a++) {
+            iana = signatures.iana_list[a];
+            for (int b = 0; b < signatures.len; b++) {
+                if (a == b) {
+                    continue;
                 }
+                other_iana = signatures.iana_list[b];
+                EXPECT_NOT_EQUAL(iana, other_iana);
             }
         }
-
-        s2n_connection_free(conn);
-        s2n_stuffer_free(&result);
     };
 
     /* Test: libcrypto may not support PSS signatures */
@@ -761,25 +709,22 @@ int main(int argc, char **argv)
         config->security_policy = &test_security_policy;
 
         /* Do not offer PSS signatures schemes if unsupported:
-         * s2n_send_supported_sig_scheme_list + PSS */
+         * s2n_signature_algorithms_supported_list_send + PSS */
         {
-            struct s2n_stuffer result = { 0 };
-            s2n_stuffer_growable_alloc(&result, STUFFER_SIZE);
+            DEFER_CLEANUP(struct s2n_stuffer result = { 0 }, s2n_stuffer_free);
+            EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&result, 0));
+            EXPECT_OK(s2n_signature_algorithms_supported_list_send(conn, &result));
 
-            EXPECT_SUCCESS(s2n_send_supported_sig_scheme_list(conn, &result));
-
-            uint16_t size;
+            uint16_t size = 0;
             EXPECT_SUCCESS(s2n_stuffer_read_uint16(&result, &size));
-            EXPECT_EQUAL(size, s2n_supported_sig_scheme_list_size(conn));
+            EXPECT_EQUAL(size, s2n_stuffer_data_available(&result));
             if (s2n_is_rsa_pss_certs_supported()) {
                 EXPECT_EQUAL(size, 2 * sizeof(uint16_t));
             } else if (s2n_is_rsa_pss_signing_supported()) {
                 EXPECT_EQUAL(size, 1 * sizeof(uint16_t));
             } else {
                 EXPECT_EQUAL(size, 0);
             }
-
-            s2n_stuffer_free(&result);
         };
 
         /* Do not accept a PSS signature scheme if unsupported:

tests/unit/s2n_tls13_cert_request_test.c

@@ -69,7 +69,7 @@ int main(int argc, char **argv)
         EXPECT_TRUE(s2n_stuffer_data_available(&client_conn->handshake.io) > 0);
         EXPECT_SUCCESS(s2n_tls13_cert_req_recv(client_conn));
 
-        EXPECT_EQUAL(client_conn->handshake_params.server_sig_hash_algs.len, s2n_supported_sig_schemes_count(server_conn));
+        EXPECT_TRUE(client_conn->handshake_params.server_sig_hash_algs.len > 0);
 
         EXPECT_SUCCESS(s2n_connection_free(client_conn));
         EXPECT_SUCCESS(s2n_connection_free(server_conn));

tls/extensions/s2n_client_signature_algorithms.c

@@ -24,12 +24,13 @@
 #include "utils/s2n_safety.h"
 
 static bool s2n_client_signature_algorithms_should_send(struct s2n_connection *conn);
+static int s2n_client_signature_algorithms_send(struct s2n_connection *conn, struct s2n_stuffer *extension);
 static int s2n_client_signature_algorithms_recv(struct s2n_connection *conn, struct s2n_stuffer *extension);
 
 const s2n_extension_type s2n_client_signature_algorithms_extension = {
     .iana_value = TLS_EXTENSION_SIGNATURE_ALGORITHMS,
     .is_response = false,
-    .send = s2n_send_supported_sig_scheme_list,
+    .send = s2n_client_signature_algorithms_send,
     .recv = s2n_client_signature_algorithms_recv,
     .should_send = s2n_client_signature_algorithms_should_send,
     .if_missing = s2n_extension_noop_if_missing,
@@ -40,6 +41,12 @@ static bool s2n_client_signature_algorithms_should_send(struct s2n_connection *c
     return s2n_connection_get_protocol_version(conn) >= S2N_TLS12;
 }
 
+static int s2n_client_signature_algorithms_send(struct s2n_connection *conn, struct s2n_stuffer *extension)
+{
+    POSIX_GUARD_RESULT(s2n_signature_algorithms_supported_list_send(conn, extension));
+    return S2N_SUCCESS;
+}
+
 static int s2n_client_signature_algorithms_recv(struct s2n_connection *conn, struct s2n_stuffer *extension)
 {
     return s2n_recv_supported_sig_scheme_list(extension, &conn->handshake_params.client_sig_hash_algs);

tls/extensions/s2n_server_signature_algorithms.c

@@ -24,17 +24,24 @@
 #include "tls/s2n_tls_parameters.h"
 #include "utils/s2n_safety.h"
 
+static int s2n_signature_algorithms_send(struct s2n_connection *conn, struct s2n_stuffer *extension);
 static int s2n_signature_algorithms_recv(struct s2n_connection *conn, struct s2n_stuffer *extension);
 
 const s2n_extension_type s2n_server_signature_algorithms_extension = {
     .iana_value = TLS_EXTENSION_SIGNATURE_ALGORITHMS,
     .is_response = false,
-    .send = s2n_send_supported_sig_scheme_list,
+    .send = s2n_signature_algorithms_send,
     .recv = s2n_signature_algorithms_recv,
     .should_send = s2n_extension_always_send,
     .if_missing = s2n_extension_error_if_missing,
 };
 
+static int s2n_signature_algorithms_send(struct s2n_connection *conn, struct s2n_stuffer *extension)
+{
+    POSIX_GUARD_RESULT(s2n_signature_algorithms_supported_list_send(conn, extension));
+    return S2N_SUCCESS;
+}
+
 static int s2n_signature_algorithms_recv(struct s2n_connection *conn, struct s2n_stuffer *extension)
 {
     return s2n_recv_supported_sig_scheme_list(extension, &conn->handshake_params.server_sig_hash_algs);

tls/s2n_server_cert_request.c

@@ -172,7 +172,7 @@ int s2n_cert_req_send(struct s2n_connection *conn)
     }
 
     if (conn->actual_protocol_version == S2N_TLS12) {
-        POSIX_GUARD(s2n_send_supported_sig_scheme_list(conn, out));
+        POSIX_GUARD_RESULT(s2n_signature_algorithms_supported_list_send(conn, out));
     }
 
     /* RFC 5246 7.4.4 - If the certificate_authorities list is empty, then the