Address Feedback v2

aws · Oct 7, 2024 · 8e227ab · 8e227ab
1 parent 261a209
commit 8e227ab
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 15 deletions.
diff --git a/tests/unit/s2n_kem_preferences_test.c b/tests/unit/s2n_kem_preferences_test.c
@@ -35,22 +35,11 @@ int main(int argc, char **argv)
     EXPECT_FALSE(s2n_kem_preferences_includes_tls13_kem_group(&kem_preferences_null, TLS_PQ_KEM_GROUP_ID_SECP521R1_KYBER_1024_R3));
 
     {
-        const struct s2n_kem_group *test_kem_groups[] = {
-            &s2n_secp256r1_mlkem_768,
-            &s2n_x25519_mlkem_768,
-            &s2n_secp256r1_kyber_512_r3,
-            &s2n_x25519_kyber_512_r3,
-            &s2n_secp384r1_kyber_768_r3,
-            &s2n_secp256r1_kyber_768_r3,
-            &s2n_x25519_kyber_768_r3,
-            &s2n_secp521r1_kyber_1024_r3,
-        };
-
         const struct s2n_kem_preferences test_prefs = {
             .kem_count = 0,
             .kems = NULL,
-            .tls13_kem_group_count = s2n_array_len(test_kem_groups),
-            .tls13_kem_groups = test_kem_groups,
+            .tls13_kem_group_count = S2N_KEM_GROUPS_COUNT,
+            .tls13_kem_groups = ALL_SUPPORTED_KEM_GROUPS,
         };
 
         EXPECT_TRUE(s2n_kem_preferences_includes_tls13_kem_group(&test_prefs, TLS_PQ_KEM_GROUP_ID_SECP256R1_MLKEM_768));

diff --git a/tests/unit/s2n_tls13_pq_handshake_test.c b/tests/unit/s2n_tls13_pq_handshake_test.c
@@ -450,11 +450,50 @@ int main()
         bool len_prefix_expected;
     };
 
+    /* Self talk test with each TLS 1.3 KemGroup we support */
+    for (size_t i = 0; i < S2N_KEM_GROUPS_COUNT; i++) {
+        const struct s2n_kem_group *kem_group = ALL_SUPPORTED_KEM_GROUPS[i];
+
+        if (kem_group == NULL || !s2n_kem_group_is_available(kem_group)) {
+            continue;
+        }
+
+        const struct s2n_kem_preferences singleton_test_pref = {
+            .kem_count = 0,
+            .kems = NULL,
+            .tls13_kem_group_count = 1,
+            .tls13_kem_groups = &kem_group,
+            .tls13_pq_hybrid_draft_revision = 5
+        };
+
+        const struct s2n_security_policy singleton_test_policy = {
+            .minimum_protocol_version = S2N_TLS13,
+            .cipher_preferences = &cipher_preferences_20190801,
+            .kem_preferences = &singleton_test_pref,
+            .signature_preferences = &s2n_signature_preferences_20200207,
+            .ecc_preferences = &s2n_ecc_preferences_20240603,
+        };
+
+        const struct pq_handshake_test_vector test_vec = {
+            .client_policy = &singleton_test_policy,
+            .server_policy = &singleton_test_policy,
+            .expected_kem_group = kem_group,
+            .expected_curve = NULL,
+            .hrr_expected = false,
+            .len_prefix_expected = false,
+        };
+
+        EXPECT_SUCCESS(s2n_test_tls13_pq_handshake(test_vec.client_policy, test_vec.server_policy,
+                test_vec.expected_kem_group, test_vec.expected_curve, test_vec.hrr_expected, test_vec.len_prefix_expected));
+    }
+
     /* ML-KEM is only available on newer versions of AWS-LC. If it's
      * unavailable, we must downgrade the assertions to Kyber or EC. */
     const struct s2n_kem_group *null_if_no_mlkem = &s2n_x25519_mlkem_768;
+    const struct s2n_ecc_named_curve *ec_if_no_mlkem = NULL;
     if (!s2n_libcrypto_supports_mlkem()) {
         null_if_no_mlkem = NULL;
+        ec_if_no_mlkem = default_curve;
     }
 
     /* Test vectors that expect to negotiate PQ assume that PQ is enabled in s2n.
@@ -670,12 +709,13 @@ int main()
                 .hrr_expected = true,
                 .len_prefix_expected = true,
         },
+
         /* Confirm that MLKEM768 is negotiable */
         {
                 .client_policy = &mlkem768_test_policy,
                 .server_policy = &mlkem768_test_policy,
                 .expected_kem_group = null_if_no_mlkem,
-                .expected_curve = NULL,
+                .expected_curve = ec_if_no_mlkem,
                 .hrr_expected = false,
                 .len_prefix_expected = false,
         }

diff --git a/tls/extensions/s2n_server_key_share.c b/tls/extensions/s2n_server_key_share.c
@@ -36,6 +36,9 @@ const s2n_extension_type s2n_server_key_share_extension = {
 
 static int s2n_server_key_share_send_hybrid_partial_ecc(struct s2n_connection *conn, struct s2n_stuffer *out)
 {
+    POSIX_ENSURE_REF(conn);
+    POSIX_ENSURE_REF(out);
+
     struct s2n_kem_group_params *server_kem_group_params = &conn->kex_params.server_kem_group_params;
     struct s2n_kem_params *client_kem_params = &conn->kex_params.client_kem_group_params.kem_params;
 
@@ -173,9 +176,14 @@ static int s2n_server_key_share_send(struct s2n_connection *conn, struct s2n_stu
 
 static int s2n_server_key_share_recv_hybrid_partial_ecc(struct s2n_connection *conn, struct s2n_stuffer *extension)
 {
+    POSIX_ENSURE_REF(conn);
+    POSIX_ENSURE_REF(extension);
+
     struct s2n_kem_params *client_kem_params = &conn->kex_params.client_kem_group_params.kem_params;
     struct s2n_kem_group_params *server_kem_group_params = &conn->kex_params.server_kem_group_params;
-    uint16_t expected_ecc_share_size = server_kem_group_params->kem_group->curve->share_size;
+    const struct s2n_kem_group *server_kem_group = server_kem_group_params->kem_group;
+    POSIX_ENSURE_REF(server_kem_group);
+    uint16_t expected_ecc_share_size = server_kem_group->curve->share_size;
 
     /* Parse ECC key share */
     if (client_kem_params->len_prefixed) {