Use NIST P-256 for key generation when client do not specify curve

tests/unit/s2n_client_hello_recv_test.c

@@ -31,6 +31,7 @@
 #include "tls/s2n_connection.h"
 #include "tls/s2n_client_hello.h"
 
+#include "utils/s2n_blob.h"
 #include "utils/s2n_safety.h"
 
 int main(int argc, char **argv)
@@ -375,6 +376,48 @@ int main(int argc, char **argv)
         s2n_connection_free(server_conn);
     }
 
+    /* Test that curve selection will be NIST P-256 when tls12 client does not sending curve extension. */
+    {
+        S2N_BLOB_FROM_HEX(tls12_client_hello_no_curves,
+            "030307de81928fe1" "7cba77904c2798da" "2521a76b013a16e4" "21ade32208f658d4" "327d000048000400"
+            "05000a0016002f00" "3300350039003c00" "3d0067006b009c00" "9d009e009fc009c0" "0ac011c012c013c0"
+            "14c023c024c027c0" "28c02bc02cc02fc0" "30cca8cca9ccaaff" "04ff0800ff010000" "30000d0016001404"
+            "0105010601030104" "0305030603030302" "010203000b000201" "00fe01000c000a00" "17000d0013000100"
+            "0a");
+
+        /* The above code is generated the following code,
+            disabling s2n_client_supported_groups_extension
+            from client_hello_extensions (s2n_extension_type_lists.c)
+            and exporting the resulting client_conn->handshake.io.blob
+
+        EXPECT_NOT_NULL(client_conn = s2n_connection_new(S2N_CLIENT));
+        EXPECT_SUCCESS(s2n_connection_set_config(client_conn, tls12_config));
+        EXPECT_SUCCESS(s2n_client_hello_send(client_conn));
+        EXPECT_EQUAL(client_conn->client_protocol_version, S2N_TLS12);
+        EXPECT_EQUAL(client_conn->client_hello_version, S2N_TLS12);
+        s2n_connection_free(client_conn);
+        */
+
+        EXPECT_SUCCESS(s2n_enable_tls13());
+
+        EXPECT_NOT_NULL(server_conn = s2n_connection_new(S2N_SERVER));
+        EXPECT_SUCCESS(s2n_connection_set_config(server_conn, tls13_config));
+
+        EXPECT_SUCCESS(s2n_stuffer_write(&server_conn->handshake.io, &tls12_client_hello_no_curves));
+        EXPECT_SUCCESS(s2n_client_hello_recv(server_conn));
+
+        /* ensure negotiated_curve == secp256r1 for maximum client compatability */
+        EXPECT_EQUAL(server_conn->secure.server_ecc_evp_params.negotiated_curve, &s2n_ecc_curve_secp256r1);
+
+        EXPECT_EQUAL(server_conn->server_protocol_version, S2N_TLS13);
+        EXPECT_EQUAL(server_conn->actual_protocol_version, S2N_TLS12);
+        EXPECT_EQUAL(server_conn->client_protocol_version, S2N_TLS12);
+        EXPECT_EQUAL(server_conn->client_hello_version, S2N_TLS12);
+
+        s2n_connection_free(server_conn);
+        EXPECT_SUCCESS(s2n_disable_tls13());
+    }
+
     s2n_config_free(tls12_config);
     s2n_config_free(tls13_config);
     s2n_cert_chain_and_key_free(chain_and_key);

tls/s2n_client_hello.c

@@ -193,8 +193,11 @@ static int s2n_parse_client_hello(struct s2n_connection *conn)
     GUARD(s2n_connection_get_ecc_preferences(conn, &ecc_pref));
     notnull_check(ecc_pref);
 
-    /* This is going to be our default if the client has no preference. */
-    conn->secure.server_ecc_evp_params.negotiated_curve = ecc_pref->ecc_curves[0];
+    /* This is going to be our fallback if the client has no preference. */
+    /* A TLS-compliant application MUST support key exchange with secp256r1 (NIST P-256) */
+    /* and SHOULD support key exchange with X25519 [RFC7748]. */
+    /* - https://tools.ietf.org/html/rfc8446#section-9.1 */
+    conn->secure.server_ecc_evp_params.negotiated_curve = &s2n_ecc_curve_secp256r1;
 
     GUARD(s2n_extension_list_parse(in, &conn->client_hello.extensions));
 

tls/s2n_ecc_preferences.c

@@ -83,3 +83,12 @@ bool s2n_ecc_preferences_includes_curve(const struct s2n_ecc_preferences *ecc_pr
 
     return false;
 }
+
+/* Check that ecc preferences include P-256 */
+S2N_RESULT s2n_ecc_preferences_includes_p256(const struct s2n_ecc_preferences *ecc_preferences) {
+    if (!s2n_ecc_preferences_includes_curve(ecc_preferences, TLS_EC_CURVE_SECP_256_R1)) {
+        return S2N_RESULT_ERROR;
+    }
+
+    return S2N_RESULT_OK;
+}

tls/s2n_ecc_preferences.h

@@ -31,3 +31,4 @@ extern const struct s2n_ecc_preferences s2n_ecc_preferences_null;
 
 int s2n_check_ecc_preferences_curves_list(const struct s2n_ecc_preferences *ecc_preferences);
 bool s2n_ecc_preferences_includes_curve(const struct s2n_ecc_preferences *ecc_preferences, uint16_t query_iana_id);
+S2N_RESULT s2n_ecc_preferences_includes_p256(const struct s2n_ecc_preferences *ecc_preferences);

tls/s2n_security_policies.c

@@ -636,6 +636,11 @@ int s2n_security_policies_init()
         const struct s2n_ecc_preferences *ecc_preference = security_policy->ecc_preferences;
         notnull_check(ecc_preference);
         GUARD(s2n_check_ecc_preferences_curves_list(ecc_preference));
+        if (security_policy != &security_policy_null) {
+            /* catch any offending security policy that does not support P-256 */
+            GUARD_AS_POSIX(s2n_ecc_preferences_includes_p256(ecc_preference));
+        }
+
         for (int j = 0; j < cipher_preference->count; j++) {
             struct s2n_cipher_suite *cipher = cipher_preference->suites[j];
             notnull_check(cipher);