Skip to content

Update ECC public key check to use openssl3+ APIs #5691

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ColtonWilley wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Update ECC public key check to use openssl3+ APIs #5691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7c1f55c
Update ECC public key check to use proper openssl3+ APIs
ColtonWilley Dec 31, 2025
744a086
Hook new behavior around 1.1.1 check instead, fix clang format issues
ColtonWilley Dec 31, 2025
ccf0c92
Merge branch 'main' into update_ecc_pubkey_check
jouho Jan 5, 2026
b406a1f
Fix EVP_PKEY_public_check guard to use OpenSSL 3.0+ check
ColtonWilley Jan 6, 2026
e7b03ed
Merge branch 'main' into update_ecc_pubkey_check
jouho Jan 9, 2026
894c2c4
Fix version gating on cleanup func
ColtonWilley Jan 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 31 additions & 7 deletions crypto/s2n_ecc_evp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@

DEFINE_POINTER_CLEANUP_FUNC(EVP_PKEY *, EVP_PKEY_free);
DEFINE_POINTER_CLEANUP_FUNC(EVP_PKEY_CTX *, EVP_PKEY_CTX_free);

#if !S2N_OPENSSL_VERSION_AT_LEAST(3, 0, 0)
DEFINE_POINTER_CLEANUP_FUNC(EC_KEY *, EC_KEY_free);
#endif

#if !EVP_APIS_SUPPORTED
DEFINE_POINTER_CLEANUP_FUNC(EC_POINT *, EC_POINT_free);
Expand Down Expand Up @@ -206,21 +209,44 @@ static int s2n_ecc_evp_generate_own_key(const struct s2n_ecc_named_curve *named_
return named_curve->generate_key(named_curve, evp_pkey);
}

static S2N_RESULT s2n_ecc_check_key(EC_KEY *ec_key)
#if S2N_OPENSSL_VERSION_AT_LEAST(3, 0, 0)
static S2N_RESULT s2n_ecc_check_key(EVP_PKEY *evp_pkey)
{
RESULT_ENSURE_REF(ec_key);
RESULT_ENSURE_REF(evp_pkey);

DEFER_CLEANUP(EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new(evp_pkey, NULL), EVP_PKEY_CTX_free_pointer);
RESULT_ENSURE(pctx != NULL, S2N_ERR_ECDHE_INVALID_PUBLIC_KEY);

int result = EVP_PKEY_public_check(pctx);

#ifdef S2N_LIBCRYPTO_SUPPORTS_EC_KEY_CHECK_FIPS
if (s2n_is_in_fips_mode()) {
RESULT_ENSURE(result == 1, S2N_ERR_ECDHE_INVALID_PUBLIC_KEY_FIPS);
Copy link
Contributor

@maddeleine maddeleine Jan 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little odd, it's not necessary to throw S2N_ERR_ECDHE_INVALID_PUBLIC_KEY_FIPS unless the libcrypto actually supports EC_KEY_check_fips. Like, the error is kind of meaningless unless that API is actually supported. You probably can just leave it out in the S2N_OPENSSL_VERSION_AT_LEAST(3, 0, 0) case since there's no equivalent for the EVP APIs.

return S2N_RESULT_OK;
}

RESULT_ENSURE(result == 1, S2N_ERR_ECDHE_INVALID_PUBLIC_KEY);
return S2N_RESULT_OK;
}
#else
static S2N_RESULT s2n_ecc_check_key(EVP_PKEY *evp_pkey)
{
RESULT_ENSURE_REF(evp_pkey);

DEFER_CLEANUP(EC_KEY *ec_key = EVP_PKEY_get1_EC_KEY(evp_pkey), EC_KEY_free_pointer);
RESULT_ENSURE(ec_key != NULL, S2N_ERR_ECDHE_UNSUPPORTED_CURVE);

#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_INVALID_PUBLIC_KEY_FIPS);
return S2N_RESULT_OK;
}
#endif
#endif

RESULT_GUARD_OSSL(EC_KEY_check_key(ec_key), S2N_ERR_ECDHE_INVALID_PUBLIC_KEY);

return S2N_RESULT_OK;
}
#endif

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)
{
Expand All @@ -244,9 +270,7 @@ static int s2n_ecc_evp_compute_shared_secret(EVP_PKEY *own_key, EVP_PKEY *peer_p
* validation is skipped with non-NIST curves for increased 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);
POSIX_ENSURE(ec_key, S2N_ERR_ECDHE_UNSUPPORTED_CURVE);
POSIX_GUARD_RESULT(s2n_ecc_check_key(ec_key));
POSIX_GUARD_RESULT(s2n_ecc_check_key(peer_public));
}

size_t shared_secret_size = 0;
Expand Down
6 changes: 2 additions & 4 deletions tests/unit/s2n_ecc_evp_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,10 +479,8 @@ int main(int argc, char** argv)
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_ecc_evp_supports_fips_check()) {
/* If s2n-tls is in FIPS mode, ensure that the FIPS-specific error is returned. */
if (s2n_is_in_fips_mode()) {
EXPECT_FAILURE_WITH_ERRNO(ret, S2N_ERR_ECDHE_INVALID_PUBLIC_KEY_FIPS);
} else {
EXPECT_FAILURE_WITH_ERRNO(ret, S2N_ERR_ECDHE_INVALID_PUBLIC_KEY);
Expand Down
Loading