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

Revert "refactor: remove unused evp support for md5+sha1 (#5106)" #5118

Merged
merged 1 commit into from
Feb 18, 2025

Conversation

lrstewart
Copy link
Contributor

@lrstewart lrstewart commented Feb 14, 2025

This reverts commit f3ae011.

Release Summary:

Resolved issues:

related to #5105

Description of changes:

I made this change because my plan for the final state of hash + signing assumed that Legacy signing would still be required for openssl-3.0-fips to use MD5 hashes, so might as well also be kept around for openssl-1.0.2 too. However, I have since found a mistake in my openssl-3.0-fips signing logic and concluded that openssl-3.0-fips will need EVP support for MD5 anyway. With that change, the expected end state of the system has also changed. I updated #5105. In the new version, ALL libcryptos use EVP hashing.

That means that instead of removing the EVP support for MD5+SHA1, I should have removed the Legacy support for MD5+SHA1 (and the Legacy support for all other hash algorithms too).

This PR reverts the old change. We will reuse this openssl-1.0.2-fips logic for both openssl-3.0-fips and openssl-1.0.2.

Testing:

This is a straight revert, so going back to a known good state. Tests should continue to pass.
How do I know it's the RIGHT change? Well my fixed signing logic is working with it locally, but I also thought that before :) Unfortunately unless I push out one giant blob of a PR, out CI might find more mistakes as I publish my incremental PRs and I might have to update previous work again. However, I've given up basically all my "clever" ideas at this point, so I don't actually think anything else can go wrong. We're not doing FIPS 140-3 and we're requiring the Default provider. Any other concessions and openssl-3.0-fips might just be un-supportable.

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 14, 2025
@lrstewart lrstewart marked this pull request as ready for review February 14, 2025 19:54
@lrstewart lrstewart mentioned this pull request Feb 14, 2025
9 tasks
@lrstewart lrstewart added this pull request to the merge queue Feb 17, 2025
Merged via the queue into aws:main with commit 7a4d1e7 Feb 18, 2025
47 checks passed
@lrstewart lrstewart deleted the revert branch February 18, 2025 00:29
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