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

ci: add default provider to openssl-3.0-fips #5114

Merged
merged 1 commit into from
Feb 13, 2025

Conversation

lrstewart
Copy link
Contributor

@lrstewart lrstewart commented Feb 13, 2025

Release Summary:

Resolved issues:

Related to getting #5112 to pass the CI

Description of changes:

I thought I could work around the limitations of the fips provider, but I was wrong. Unless we want to rewrite like every test and a bunch of the pre-protocol-selection logic.

Instead, just also require a provider that supports non-fips algorithms. Here we add the default provider to our openssl-3.0-fips build.

Call-outs:

The specific problem I couldn't otherwise work around was that EVP_PKEY_CTX_set_signature_md is required for RSA with PKCS1 padding. Otherwise, the padding on the signature calculated by EVP_PKEY_sign will be wrong. See https://docs.openssl.org/3.1/man3/EVP_PKEY_CTX_ctrl/#rsa-parameters:

Two RSA padding modes behave differently if EVP_PKEY_CTX_set_signature_md() is used. If this function is called for PKCS#1 padding the plaintext buffer is an actual digest value and is encapsulated in a DigestInfo structure according to PKCS#1 when signing and this structure is expected (and stripped off) when verifying. If this control is not used with RSA and PKCS#1 padding then the supplied data is used directly and not encapsulated.
It's an annoyingly subtle problem. I didn't notice until running the test on openssl-3.0 (non-fips), which is capable of running both the EVP and legacy signing methods so could compare the results.

I couldn't figure out any way to work around that problem, and I'm skeptical a way exists. PKCS1 padding needs to know the hash algorithm, and openssl-3.0-fips will refuse to accept some hash algorithms. As far as I can tell, the problem algorithms are MD5 (of course) and SHA1, but SHA1 is only rejected when signing? Verifying is fine?

We could someday try to remove the requirement for MD5 and SHA1, since they're not allowed by FIPS, but when I attempted that like ALL the tests failed. A lot of our code just assumes SHA1 is available.

Testing:

Locally, I could get the s2n_evp_signing_test to pass with this update + some additional code changes to use SHA1 from a provider other than FIPS.

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 Feb 13, 2025
Comment on lines 30 to +31
"kTLS us-west-2 no-batch"
"Openssl3fipsWIP us-west-2 no-batch"
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 also noticed that I'd forgotten to turn the Openssl3fipsWIP job on 😅 It's on now so I have to add it to this list.

@lrstewart lrstewart marked this pull request as ready for review February 13, 2025 20:02
@lrstewart lrstewart requested a review from dougch as a code owner February 13, 2025 20:02
@lrstewart lrstewart requested a review from jouho February 13, 2025 20:02
@lrstewart lrstewart enabled auto-merge February 13, 2025 20:38
@lrstewart lrstewart added this pull request to the merge queue Feb 13, 2025
Merged via the queue into aws:main with commit 8201205 Feb 13, 2025
46 checks passed
@lrstewart lrstewart deleted the openssl3fips_cli branch February 13, 2025 22:48
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.

3 participants