Skip to content

Commit ee58f34

Browse files
authored
feat: Add additional EC key validation for FIPS (#4452)
1 parent 50a3f78 commit ee58f34

File tree

8 files changed

+159
-10
lines changed

8 files changed

+159
-10
lines changed

crypto/s2n_ecc_evp.c

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323

2424
#include <stdint.h>
2525

26+
#include "crypto/s2n_fips.h"
27+
#include "crypto/s2n_libcrypto.h"
2628
#include "tls/s2n_connection.h"
2729
#include "tls/s2n_ecc_preferences.h"
2830
#include "tls/s2n_tls_parameters.h"
@@ -118,6 +120,15 @@ int s2n_is_evp_apis_supported()
118120
return EVP_APIS_SUPPORTED;
119121
}
120122

123+
bool s2n_ecc_evp_supports_fips_check()
124+
{
125+
#ifdef S2N_LIBCRYPTO_SUPPORTS_EC_KEY_CHECK_FIPS
126+
return true;
127+
#else
128+
return false;
129+
#endif
130+
}
131+
121132
#if EVP_APIS_SUPPORTED
122133
static int s2n_ecc_evp_generate_key_x25519(const struct s2n_ecc_named_curve *named_curve, EVP_PKEY **evp_pkey)
123134
{
@@ -163,24 +174,47 @@ static int s2n_ecc_evp_generate_own_key(const struct s2n_ecc_named_curve *named_
163174
return named_curve->generate_key(named_curve, evp_pkey);
164175
}
165176

177+
static S2N_RESULT s2n_ecc_check_key(EC_KEY *ec_key)
178+
{
179+
RESULT_ENSURE_REF(ec_key);
180+
181+
#ifdef S2N_LIBCRYPTO_SUPPORTS_EC_KEY_CHECK_FIPS
182+
if (s2n_is_in_fips_mode()) {
183+
RESULT_GUARD_OSSL(EC_KEY_check_fips(ec_key), S2N_ERR_ECDHE_INVALID_PUBLIC_KEY_FIPS);
184+
return S2N_RESULT_OK;
185+
}
186+
#endif
187+
188+
RESULT_GUARD_OSSL(EC_KEY_check_key(ec_key), S2N_ERR_ECDHE_INVALID_PUBLIC_KEY);
189+
190+
return S2N_RESULT_OK;
191+
}
192+
166193
static int s2n_ecc_evp_compute_shared_secret(EVP_PKEY *own_key, EVP_PKEY *peer_public, uint16_t iana_id, struct s2n_blob *shared_secret)
167194
{
168195
POSIX_ENSURE_REF(peer_public);
169196
POSIX_ENSURE_REF(own_key);
170197

171-
/* From RFC 8446(TLS1.3) Section 4.2.8.2: For the curves secp256r1, secp384r1, and secp521r1, peers MUST validate
172-
* each other's public value Q by ensuring that the point is a valid point on the elliptic curve.
173-
* For the curve x25519 and x448 the peer public-key validation check doesn't apply.
174-
* From RFC 8422(TLS1.2) Section 5.11: With the NIST curves, each party MUST validate the public key sent by its peer
175-
* in the ClientKeyExchange and ServerKeyExchange messages. A receiving party MUST check that the x and y parameters from
176-
* the peer's public value satisfy the curve equation, y^2 = x^3 + ax + b mod p.
177-
* 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
178-
* this is an additional validation step that increases security but decreases performance.
198+
/**
199+
*= https://tools.ietf.org/rfc/rfc8446#section-4.2.8.2
200+
*# For the curves secp256r1, secp384r1, and secp521r1, peers MUST
201+
*# validate each other's public value Q by ensuring that the point is a
202+
*# valid point on the elliptic curve.
203+
*
204+
*= https://tools.ietf.org/rfc/rfc8422#section-5.11
205+
*# With the NIST curves, each party MUST validate the public key sent by
206+
*# its peer in the ClientKeyExchange and ServerKeyExchange messages. A
207+
*# receiving party MUST check that the x and y parameters from the
208+
*# peer's public value satisfy the curve equation, y^2 = x^3 + ax + b
209+
*# mod p.
210+
*
211+
* The validation requirement for the public key value only applies to NIST curves. The
212+
* validation is skipped with non-NIST curves for increased performance.
179213
*/
180214
if (iana_id != TLS_EC_CURVE_ECDH_X25519 && iana_id != TLS_EC_CURVE_ECDH_X448) {
181215
DEFER_CLEANUP(EC_KEY *ec_key = EVP_PKEY_get1_EC_KEY(peer_public), EC_KEY_free_pointer);
182-
S2N_ERROR_IF(ec_key == NULL, S2N_ERR_ECDHE_UNSUPPORTED_CURVE);
183-
POSIX_GUARD_OSSL(EC_KEY_check_key(ec_key), S2N_ERR_ECDHE_SHARED_SECRET);
216+
POSIX_ENSURE(ec_key, S2N_ERR_ECDHE_UNSUPPORTED_CURVE);
217+
POSIX_GUARD_RESULT(s2n_ecc_check_key(ec_key));
184218
}
185219

186220
size_t shared_secret_size = 0;

crypto/s2n_ecc_evp.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,4 @@ int s2n_ecc_evp_parse_params(struct s2n_connection *conn,
8585
int s2n_ecc_evp_find_supported_curve(struct s2n_connection *conn, struct s2n_blob *iana_ids, const struct s2n_ecc_named_curve **found);
8686
int s2n_ecc_evp_params_free(struct s2n_ecc_evp_params *ecc_evp_params);
8787
int s2n_is_evp_apis_supported();
88+
bool s2n_ecc_evp_supports_fips_check();

error/s2n_errno.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ static const char *no_such_error = "Internal s2n error";
8888
ERR_ENTRY(S2N_ERR_ECDHE_GEN_KEY, "Failed to generate an ECDHE key") \
8989
ERR_ENTRY(S2N_ERR_ECDHE_SHARED_SECRET, "Error computing ECDHE shared secret") \
9090
ERR_ENTRY(S2N_ERR_ECDHE_UNSUPPORTED_CURVE, "Unsupported EC curve was presented during an ECDHE handshake") \
91+
ERR_ENTRY(S2N_ERR_ECDHE_INVALID_PUBLIC_KEY, "Failed to validate the peer's point on the elliptic curve") \
92+
ERR_ENTRY(S2N_ERR_ECDHE_INVALID_PUBLIC_KEY_FIPS, "Failed to validate the peer's point on the elliptic curve, per FIPS requirements") \
9193
ERR_ENTRY(S2N_ERR_ECDSA_UNSUPPORTED_CURVE, "Unsupported EC curve was presented during an ECDSA SignatureScheme handshake") \
9294
ERR_ENTRY(S2N_ERR_ECDHE_SERIALIZING, "Error serializing ECDHE public") \
9395
ERR_ENTRY(S2N_ERR_KEM_UNSUPPORTED_PARAMS, "Unsupported KEM params was presented during a handshake that uses a KEM") \

error/s2n_errno.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ typedef enum {
103103
S2N_ERR_ECDHE_GEN_KEY,
104104
S2N_ERR_ECDHE_SHARED_SECRET,
105105
S2N_ERR_ECDHE_UNSUPPORTED_CURVE,
106+
S2N_ERR_ECDHE_INVALID_PUBLIC_KEY,
107+
S2N_ERR_ECDHE_INVALID_PUBLIC_KEY_FIPS,
106108
S2N_ERR_ECDSA_UNSUPPORTED_CURVE,
107109
S2N_ERR_ECDHE_SERIALIZING,
108110
S2N_ERR_KEM_UNSUPPORTED_PARAMS,
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
#include <openssl/ec.h>
17+
18+
int main()
19+
{
20+
EC_KEY *ec_key = NULL;
21+
EC_KEY_check_fips(ec_key);
22+
return 0;
23+
}

tests/features/S2N_LIBCRYPTO_SUPPORTS_EC_KEY_CHECK_FIPS.flags

Whitespace-only changes.

tests/unit/s2n_ecc_evp_test.c

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
#include "crypto/s2n_ecc_evp.h"
1717

1818
#include "api/s2n.h"
19+
#include "crypto/s2n_fips.h"
20+
#include "crypto/s2n_libcrypto.h"
1921
#include "s2n_test.h"
2022
#include "stuffer/s2n_stuffer.h"
2123
#include "testlib/s2n_testlib.h"
@@ -27,10 +29,19 @@
2729

2830
extern const struct s2n_ecc_named_curve s2n_unsupported_curve;
2931

32+
DEFINE_POINTER_CLEANUP_FUNC(EC_KEY*, EC_KEY_free);
33+
DEFINE_POINTER_CLEANUP_FUNC(EC_POINT*, EC_POINT_free);
34+
3035
int main(int argc, char** argv)
3136
{
3237
BEGIN_TEST();
3338
EXPECT_SUCCESS(s2n_disable_tls13_in_test());
39+
40+
/* Test the EC_KEY_CHECK_FIPS feature probe. AWS-LC is a libcrypto known to support this feature. */
41+
if (s2n_libcrypto_is_awslc()) {
42+
EXPECT_TRUE(s2n_ecc_evp_supports_fips_check());
43+
}
44+
3445
{
3546
/* Test generate ephemeral keys for all supported curves */
3647
for (size_t i = 0; i < s2n_all_supported_curves_list_len; i++) {
@@ -405,5 +416,79 @@ int main(int argc, char** argv)
405416
EXPECT_SUCCESS(s2n_ecc_evp_params_free(&client_params));
406417
}
407418
};
419+
420+
/**
421+
*= https://tools.ietf.org/rfc/rfc8446#section-4.2.8.2
422+
*= type=test
423+
*# For the curves secp256r1, secp384r1, and secp521r1, peers MUST
424+
*# validate each other's public value Q by ensuring that the point is a
425+
*# valid point on the elliptic curve. The appropriate validation
426+
*# procedures are defined in Section 4.3.7 of [ECDSA] and alternatively
427+
*# in Section 5.6.2.3 of [KEYAGREEMENT]. This process consists of three
428+
*# steps: (1) verify that Q is not the point at infinity (O), (2) verify
429+
*# that for Q = (x, y) both integers x and y are in the correct
430+
*# interval, and (3) ensure that (x, y) is a correct solution to the
431+
*# elliptic curve equation. For these curves, implementors do not need
432+
*# to verify membership in the correct subgroup.
433+
*
434+
* s2n-tls performs this validation by invoking the libcrypto APIs: EC_KEY_check_key, and
435+
* EC_KEY_check_fips. To ensure that these APIs are properly called, step (1) is invalidated.
436+
*/
437+
{
438+
const struct s2n_ecc_named_curve* const nist_curves[] = {
439+
&s2n_ecc_curve_secp256r1,
440+
&s2n_ecc_curve_secp384r1,
441+
&s2n_ecc_curve_secp521r1,
442+
};
443+
444+
for (size_t i = 0; i < s2n_array_len(nist_curves); i++) {
445+
const struct s2n_ecc_named_curve* curve = nist_curves[i];
446+
447+
DEFER_CLEANUP(struct s2n_ecc_evp_params server_params = { 0 }, s2n_ecc_evp_params_free);
448+
DEFER_CLEANUP(struct s2n_ecc_evp_params client_params = { 0 }, s2n_ecc_evp_params_free);
449+
DEFER_CLEANUP(struct s2n_blob shared_key = { 0 }, s2n_free);
450+
451+
/* Create a server key. */
452+
server_params.negotiated_curve = curve;
453+
EXPECT_SUCCESS(s2n_ecc_evp_generate_ephemeral_key(&server_params));
454+
EXPECT_NOT_NULL(server_params.evp_pkey);
455+
456+
/* Create a client key. */
457+
client_params.negotiated_curve = curve;
458+
EXPECT_SUCCESS(s2n_ecc_evp_generate_ephemeral_key(&client_params));
459+
EXPECT_NOT_NULL(client_params.evp_pkey);
460+
461+
/* Retrieve the existing client public key. */
462+
DEFER_CLEANUP(EC_KEY* ec_key = EVP_PKEY_get1_EC_KEY(client_params.evp_pkey),
463+
EC_KEY_free_pointer);
464+
EXPECT_NOT_NULL(ec_key);
465+
const EC_GROUP* group = EC_KEY_get0_group(ec_key);
466+
EXPECT_NOT_NULL(group);
467+
const EC_POINT* public_key = EC_KEY_get0_public_key(ec_key);
468+
EXPECT_NOT_NULL(public_key);
469+
470+
/* Invalidate the public key by setting the coordinate to infinity. */
471+
DEFER_CLEANUP(EC_POINT* invalid_public_key = EC_POINT_dup(public_key, group),
472+
EC_POINT_free_pointer);
473+
EXPECT_NOT_NULL(invalid_public_key);
474+
EXPECT_EQUAL(EC_POINT_set_to_infinity(group, invalid_public_key), 1);
475+
EXPECT_EQUAL(EC_KEY_set_public_key(ec_key, invalid_public_key), 1);
476+
EXPECT_EQUAL(EVP_PKEY_set1_EC_KEY(client_params.evp_pkey, ec_key), 1);
477+
478+
/* Compute the server's shared secret. */
479+
int ret = s2n_ecc_evp_compute_shared_secret_from_params(&server_params,
480+
&client_params, &shared_key);
481+
482+
/* If s2n-tls is in FIPS mode and the libcrypto supports the EC_KEY_check_fips API,
483+
* ensure that this API is called by checking for the correct error.
484+
*/
485+
if (s2n_is_in_fips_mode() && s2n_ecc_evp_supports_fips_check()) {
486+
EXPECT_FAILURE_WITH_ERRNO(ret, S2N_ERR_ECDHE_INVALID_PUBLIC_KEY_FIPS);
487+
} else {
488+
EXPECT_FAILURE_WITH_ERRNO(ret, S2N_ERR_ECDHE_INVALID_PUBLIC_KEY);
489+
}
490+
}
491+
}
492+
408493
END_TEST();
409494
}

tls/s2n_alerts.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ static S2N_RESULT s2n_translate_protocol_error_to_alert(int error_code, uint8_t
9797
S2N_NO_ALERT(S2N_ERR_ECDHE_GEN_KEY);
9898
S2N_NO_ALERT(S2N_ERR_ECDHE_SHARED_SECRET);
9999
S2N_NO_ALERT(S2N_ERR_ECDHE_UNSUPPORTED_CURVE);
100+
S2N_NO_ALERT(S2N_ERR_ECDHE_INVALID_PUBLIC_KEY);
101+
S2N_NO_ALERT(S2N_ERR_ECDHE_INVALID_PUBLIC_KEY_FIPS);
100102
S2N_NO_ALERT(S2N_ERR_ECDSA_UNSUPPORTED_CURVE);
101103
S2N_NO_ALERT(S2N_ERR_ECDHE_SERIALIZING);
102104
S2N_NO_ALERT(S2N_ERR_KEM_UNSUPPORTED_PARAMS);

0 commit comments

Comments
 (0)