diff --git a/crypto/s2n_ecc_evp.c b/crypto/s2n_ecc_evp.c index 4bc1f98a5ce..e140f5450b5 100644 --- a/crypto/s2n_ecc_evp.c +++ b/crypto/s2n_ecc_evp.c @@ -180,12 +180,12 @@ static S2N_RESULT s2n_ecc_check_key(EC_KEY *ec_key) #ifdef S2N_LIBCRYPTO_SUPPORTS_EC_KEY_CHECK_FIPS if (s2n_is_in_fips_mode()) { - RESULT_GUARD_OSSL(EC_KEY_check_fips(ec_key), S2N_ERR_ECDHE_SHARED_SECRET); + RESULT_GUARD_OSSL(EC_KEY_check_fips(ec_key), S2N_ERR_ECDHE_INVALID_PUBLIC_KEY_FIPS); return S2N_RESULT_OK; } #endif - RESULT_GUARD_OSSL(EC_KEY_check_key(ec_key), S2N_ERR_ECDHE_SHARED_SECRET); + RESULT_GUARD_OSSL(EC_KEY_check_key(ec_key), S2N_ERR_ECDHE_INVALID_PUBLIC_KEY); return S2N_RESULT_OK; } @@ -195,18 +195,25 @@ static int s2n_ecc_evp_compute_shared_secret(EVP_PKEY *own_key, EVP_PKEY *peer_p POSIX_ENSURE_REF(peer_public); POSIX_ENSURE_REF(own_key); - /* From RFC 8446(TLS1.3) Section 4.2.8.2: For the curves secp256r1, secp384r1, and secp521r1, peers MUST validate - * each other's public value Q by ensuring that the point is a valid point on the elliptic curve. - * For the curve x25519 and x448 the peer public-key validation check doesn't apply. - * From RFC 8422(TLS1.2) Section 5.11: With the NIST curves, each party MUST validate the public key sent by its peer - * in the ClientKeyExchange and ServerKeyExchange messages. A receiving party MUST check that the x and y parameters from - * the peer's public value satisfy the curve equation, y^2 = x^3 + ax + b mod p. - * Note that the `EC_KEY_check_key` validation is a MUST for only NIST curves, if a non-NIST curve is added to s2n-tls + /** + *= https://tools.ietf.org/rfc/rfc8446#section-4.2.8.2 + *# For the curves secp256r1, secp384r1, and secp521r1, peers MUST + *# validate each other's public value Q by ensuring that the point is a + *# valid point on the elliptic curve. + * + *= https://tools.ietf.org/rfc/rfc8422#section-5.11 + *# With the NIST curves, each party MUST validate the public key sent by + *# its peer in the ClientKeyExchange and ServerKeyExchange messages. A + *# receiving party MUST check that the x and y parameters from the + *# peer's public value satisfy the curve equation, y^2 = x^3 + ax + b + *# mod p. + * + * Note that the `EC_KEY_check_key` validation is a MUST for only NIST curves, if a non-NIST curve is added to s2n-tls * this is an additional validation step that increases security but decreases performance. */ if (iana_id != TLS_EC_CURVE_ECDH_X25519 && iana_id != TLS_EC_CURVE_ECDH_X448) { DEFER_CLEANUP(EC_KEY *ec_key = EVP_PKEY_get1_EC_KEY(peer_public), EC_KEY_free_pointer); - S2N_ERROR_IF(ec_key == NULL, S2N_ERR_ECDHE_UNSUPPORTED_CURVE); + POSIX_ENSURE(ec_key, S2N_ERR_ECDHE_UNSUPPORTED_CURVE); POSIX_GUARD_RESULT(s2n_ecc_check_key(ec_key)); } diff --git a/error/s2n_errno.c b/error/s2n_errno.c index 411d4bf70c2..7b770681758 100644 --- a/error/s2n_errno.c +++ b/error/s2n_errno.c @@ -88,6 +88,8 @@ static const char *no_such_error = "Internal s2n error"; ERR_ENTRY(S2N_ERR_ECDHE_GEN_KEY, "Failed to generate an ECDHE key") \ ERR_ENTRY(S2N_ERR_ECDHE_SHARED_SECRET, "Error computing ECDHE shared secret") \ ERR_ENTRY(S2N_ERR_ECDHE_UNSUPPORTED_CURVE, "Unsupported EC curve was presented during an ECDHE handshake") \ + ERR_ENTRY(S2N_ERR_ECDHE_INVALID_PUBLIC_KEY, "Failed to validate the peer's point on the elliptic curve") \ + ERR_ENTRY(S2N_ERR_ECDHE_INVALID_PUBLIC_KEY_FIPS, "Failed to validate the peer's point on the elliptic curve, per FIPS requirements") \ ERR_ENTRY(S2N_ERR_ECDSA_UNSUPPORTED_CURVE, "Unsupported EC curve was presented during an ECDSA SignatureScheme handshake") \ ERR_ENTRY(S2N_ERR_ECDHE_SERIALIZING, "Error serializing ECDHE public") \ ERR_ENTRY(S2N_ERR_KEM_UNSUPPORTED_PARAMS, "Unsupported KEM params was presented during a handshake that uses a KEM") \ diff --git a/error/s2n_errno.h b/error/s2n_errno.h index f2efeffcc1e..fa16387839d 100644 --- a/error/s2n_errno.h +++ b/error/s2n_errno.h @@ -103,6 +103,8 @@ typedef enum { S2N_ERR_ECDHE_GEN_KEY, S2N_ERR_ECDHE_SHARED_SECRET, S2N_ERR_ECDHE_UNSUPPORTED_CURVE, + S2N_ERR_ECDHE_INVALID_PUBLIC_KEY, + S2N_ERR_ECDHE_INVALID_PUBLIC_KEY_FIPS, S2N_ERR_ECDSA_UNSUPPORTED_CURVE, S2N_ERR_ECDHE_SERIALIZING, S2N_ERR_KEM_UNSUPPORTED_PARAMS, diff --git a/tests/unit/s2n_ecc_evp_test.c b/tests/unit/s2n_ecc_evp_test.c index 82b757d8cba..9f6e63c40df 100644 --- a/tests/unit/s2n_ecc_evp_test.c +++ b/tests/unit/s2n_ecc_evp_test.c @@ -16,6 +16,7 @@ #include "crypto/s2n_ecc_evp.h" #include "api/s2n.h" +#include "crypto/s2n_fips.h" #include "crypto/s2n_libcrypto.h" #include "s2n_test.h" #include "stuffer/s2n_stuffer.h" @@ -412,5 +413,77 @@ int main(int argc, char** argv) EXPECT_SUCCESS(s2n_ecc_evp_params_free(&client_params)); } }; + + /** + *= https://tools.ietf.org/rfc/rfc8446#section-4.2.8.2 + *= type=test + *# For the curves secp256r1, secp384r1, and secp521r1, peers MUST + *# validate each other's public value Q by ensuring that the point is a + *# valid point on the elliptic curve. The appropriate validation + *# procedures are defined in Section 4.3.7 of [ECDSA] and alternatively + *# in Section 5.6.2.3 of [KEYAGREEMENT]. This process consists of three + *# steps: (1) verify that Q is not the point at infinity (O), (2) verify + *# that for Q = (x, y) both integers x and y are in the correct + *# interval, and (3) ensure that (x, y) is a correct solution to the + *# elliptic curve equation. For these curves, implementors do not need + *# to verify membership in the correct subgroup. + * + * s2n-tls performs this validation by invoking the libcrypto APIs: EC_KEY_check_key, and + * EC_KEY_check_fips. To ensure that these APIs are properly called, step (1) is invalidated. + */ + { + const struct s2n_ecc_named_curve* const nist_curves[] = { + &s2n_ecc_curve_secp256r1, + &s2n_ecc_curve_secp384r1, + &s2n_ecc_curve_secp521r1, + }; + + for (size_t i = 0; i < s2n_array_len(nist_curves); i++) { + const struct s2n_ecc_named_curve* curve = nist_curves[i]; + + DEFER_CLEANUP(struct s2n_ecc_evp_params server_params = { 0 }, s2n_ecc_evp_params_free); + DEFER_CLEANUP(struct s2n_ecc_evp_params client_params = { 0 }, s2n_ecc_evp_params_free); + DEFER_CLEANUP(struct s2n_blob shared_key = { 0 }, s2n_free); + + /* Create a server key. */ + server_params.negotiated_curve = curve; + EXPECT_SUCCESS(s2n_ecc_evp_generate_ephemeral_key(&server_params)); + EXPECT_NOT_NULL(server_params.evp_pkey); + + /* Create a client key. */ + client_params.negotiated_curve = curve; + EXPECT_SUCCESS(s2n_ecc_evp_generate_ephemeral_key(&client_params)); + EXPECT_NOT_NULL(client_params.evp_pkey); + + /* Retrieve the existing client public key. */ + EC_KEY* ec_key = EVP_PKEY_get1_EC_KEY(client_params.evp_pkey); + EXPECT_NOT_NULL(ec_key); + const EC_GROUP* group = EC_KEY_get0_group(ec_key); + EXPECT_NOT_NULL(group); + const EC_POINT* public_key = EC_KEY_get0_public_key(ec_key); + EXPECT_NOT_NULL(public_key); + + /* Invalidate the public key by setting the coordinate to infinity. */ + EC_POINT* invalid_public_key = EC_POINT_dup(public_key, group); + EXPECT_NOT_NULL(invalid_public_key); + EXPECT_EQUAL(EC_POINT_set_to_infinity(group, invalid_public_key), 1); + EXPECT_EQUAL(EC_KEY_set_public_key(ec_key, invalid_public_key), 1); + EXPECT_EQUAL(EVP_PKEY_set1_EC_KEY(client_params.evp_pkey, ec_key), 1); + + /* Compute the server's shared secret. */ + int ret = s2n_ecc_evp_compute_shared_secret_from_params(&server_params, + &client_params, &shared_key); + + /* If s2n-tls is in FIPS mode and the libcrypto supports the EC_KEY_check_fips API, + * ensure that this API is called by checking for the correct error. + */ + if (s2n_is_in_fips_mode() && s2n_libcrypto_supports_ec_key_check_fips()) { + EXPECT_FAILURE_WITH_ERRNO(ret, S2N_ERR_ECDHE_INVALID_PUBLIC_KEY_FIPS); + } else { + EXPECT_FAILURE_WITH_ERRNO(ret, S2N_ERR_ECDHE_INVALID_PUBLIC_KEY); + } + } + } + END_TEST(); } diff --git a/tls/s2n_alerts.c b/tls/s2n_alerts.c index ca0e2039664..88752b16d25 100644 --- a/tls/s2n_alerts.c +++ b/tls/s2n_alerts.c @@ -97,6 +97,8 @@ static S2N_RESULT s2n_translate_protocol_error_to_alert(int error_code, uint8_t S2N_NO_ALERT(S2N_ERR_ECDHE_GEN_KEY); S2N_NO_ALERT(S2N_ERR_ECDHE_SHARED_SECRET); S2N_NO_ALERT(S2N_ERR_ECDHE_UNSUPPORTED_CURVE); + S2N_NO_ALERT(S2N_ERR_ECDHE_INVALID_PUBLIC_KEY); + S2N_NO_ALERT(S2N_ERR_ECDHE_INVALID_PUBLIC_KEY_FIPS); S2N_NO_ALERT(S2N_ERR_ECDSA_UNSUPPORTED_CURVE); S2N_NO_ALERT(S2N_ERR_ECDHE_SERIALIZING); S2N_NO_ALERT(S2N_ERR_KEM_UNSUPPORTED_PARAMS);