-
Notifications
You must be signed in to change notification settings - Fork 717
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 openssl-3.0-fips builds #5037
Conversation
curl --retry 3 -L --output OpenSSL_${RELEASE}.zip \ | ||
https://github.com/openssl/openssl/archive/refs/tags/openssl-${RELEASE}.zip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just reordered the arguments here so I could wrap the line :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for not creating two scripts!
minor Nit: we're asking Configure to do some things it says are deprecated:
***** Deprecated options: no-ripemd, no-ssl2, no-hw
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was trying to avoid committing the entire config file, but I think it's unavoidable.
Turns out my "just prepend the right sections" solution wasn't /actually/ working. The error I was getting from the drbg wasn't a valid fips issue, it was a provider configuration issue. To properly configure this, we have to replace sections instead of just duplicating them ;_; The script to modify an existing config would be pretty complicated and fragile, so I think the only option is to commit a manually modified config and use that one.
I've updated the testing section-- the drbg error is gone, and everything is initializing correctly (with s2n_libcrypto_is_fips updated to account for Openssl3).
# We assume that the configs are in the /ssl directory of $INSTALL_DIR | ||
pushd $INSTALL_DIR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pushd is kind of a duplicate. We do pushd above to make the links, and previously I just moved the popd down below this logic. But I suspect that was confusing Sam based on #5037 (comment), which is a sign it probably would have confused future devs too. Might as well make it very clear this code depends on being in the install directory.
Release Summary:
Resolved issues:
Required for first step of #5036
Description of changes:
In order to add openssl-3.0-fips to our CI, we first need a build script to use to update the codebuild image(s).
Call-outs:
Testing:
For now this is manually tested.
Running successfully builds openssl-3.0.fips using:
I can then successfully build s2n-tls using that version as the libcrypto with:
And confirm that fips mode is enabled when
s2n_libcrypto_is_fips
is updated to call EVP_default_properties_is_fips_enabled for Openssl-3:If the openssl config file isn't touched so fips isn't enabled, we can still successfully use the libcrypto:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.