Skip to content
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.

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

refactor: add libcrypto PRF impl for openssl-3.0-fips #5158

Merged
merged 4 commits into from
Mar 6, 2025

Conversation

lrstewart
Copy link
Contributor

@lrstewart lrstewart commented Mar 2, 2025

Release Summary:

Resolved issues:

resolves #5143

Description of changes:

Add a new libcrypto PRF using openssl-3.0's EVP_KDF methods and "EVP_KDF-TLS1_PRF" algorithm:
https://docs.openssl.org/3.4/man3/EVP_KDF/
https://docs.openssl.org/3.4/man7/EVP_KDF-TLS1_PRF/

I also moved the libcrypto PRFs to a separate file. I think it belongs in the crypto folder?

Testing:

I mostly reused the existing tests, but with some improvements to make them more useful:

  • I added more known-value tests to cover variations on how s2n_prf executes.
  • I made the values used in the s2n_prf tests different. I had some issues figuring out bugs because they were all just identically all-zero.
  • Run the s2n_prf tests before the tests for all the methods that depend on s2n_prf.

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

@github-actions github-actions bot added the s2n-core team label Mar 2, 2025
Comment on lines +23 to +25
#if defined(OPENSSL_IS_AWSLC)

/* The AWSLC TLS PRF API is exported in all AWSLC versions. However, in the AWSLC FIPS branch, this
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't modify any of the AWSLC PRF code. I just moved it.

@lrstewart lrstewart marked this pull request as ready for review March 2, 2025 06:29
@lrstewart lrstewart requested a review from dougch as a code owner March 2, 2025 06:29
@lrstewart lrstewart requested review from goatgoose and jmayclin March 2, 2025 06:30
@lrstewart lrstewart requested a review from goatgoose March 5, 2025 20:47
const char *digest_name = "MD5-SHA1";
const char *fetch_properties = "-fips";

if (conn->actual_protocol_version == S2N_TLS12) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: What do you think about making the version handling explicit? I think is implicitly

if protocol == TLS10 || protocol == TLS11 {
    md5-sha1
} if protocol == TLS12 {
    cipher_specific
} else {
    unreachable(?)
}

I think this is also a nice bit of documentation about where this code is called from (e.g. not TLS 1.3 or SSLv3?)

@lrstewart lrstewart enabled auto-merge March 6, 2025 03:55
@lrstewart lrstewart added this pull request to the merge queue Mar 6, 2025
Merged via the queue into aws:main with commit 2c0f038 Mar 6, 2025
46 checks passed
@lrstewart lrstewart deleted the openssl3fips_prf branch March 6, 2025 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

openssl-3.0-fips support for PRF
4 participants