Skip to content

Commit

Permalink
EC_KEY_check_key test
Browse files Browse the repository at this point in the history
  • Loading branch information
goatgoose committed Mar 14, 2024
1 parent 62f5cfb commit 746381c
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 10 deletions.
27 changes: 17 additions & 10 deletions crypto/s2n_ecc_evp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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));
}

Expand Down
2 changes: 2 additions & 0 deletions error/s2n_errno.c
Original file line number Diff line number Diff line change
Expand Up @@ -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") \
Expand Down
2 changes: 2 additions & 0 deletions error/s2n_errno.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
73 changes: 73 additions & 0 deletions tests/unit/s2n_ecc_evp_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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();
}
2 changes: 2 additions & 0 deletions tls/s2n_alerts.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 746381c

Please sign in to comment.