Skip to content

Conversation

@ColtonWilley
Copy link

Goal

Refactor s2n_ecc_check_key() to use EVP APIs instead of direct EC_KEY APIs for public key validation.

Why

Existing methods to check EC public key are not provider enabled, breaking attempts to use an alternate crypto provider.

How

-Added new EVP-based s2n_ecc_check_key() implementation gated by EVP_APIS_SUPPORTED
-Uses EVP_PKEY_public_check() for key validation
-Returns FIPS-specific error code when s2n_is_in_fips_mode() is true
-Preserved EC_KEY fallback for older libcryptos

Testing

All existing ECC tests pass.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ColtonWilley ColtonWilley marked this pull request as draft December 31, 2025 20:16
@ColtonWilley ColtonWilley marked this pull request as ready for review December 31, 2025 21:03
@ColtonWilley ColtonWilley changed the title Update ECC public key check to use proper openssl3+ APIs Update ECC public key check to use openssl3+ APIs Dec 31, 2025
jouho and others added 2 commits January 5, 2026 15:06
EVP_PKEY_public_check() is an OpenSSL 3.0 API, not 1.1.1.
Using S2N_OPENSSL_VERSION_AT_LEAST(1,1,1) caused build failures
with AWS-LC since it reports as OpenSSL 1.1.1 compatible but
doesn't have this function.
@ColtonWilley
Copy link
Author

@jouho I think the flag gating is finally correct. Please let me know if you need anything else on this PR.


#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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants