Fix contrib/cryptomb/private_key_providers/test:speed_test build in FIPS mode #38027
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mode Some of the tests in that file rely on EC_group_p256 function that was first introduced in google/boringssl@417069f. Evnidently the version of BoringSSL that Envoy uses in FIPS mode does not contain this change. Because of that build of speed_test.cc fails. To fix the issue I'm replacing calls to EC_group_p256 with EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1). To the best of my knowledge functionally it should be exactly the same thing. NOTE: Probably a more long term solution would be to update the version of BoringSSL we are using to newer one, as the current one is more than 2 years old at this point. However, it's a bit more work than I can do right now, thuse this temporary fix. I will return to the question of updating BoringSSL though affter I close the ongoing bugs. Signed-off-by: Mikhail Krinkin <[email protected]>
|
Note: this code has no maintainer support, and @giantcroc is the only person listed in the owners file. After approval from @giantcroc this can be assigned to a maintainer for final approval. |
|
@krinkinmu we are currently using the latest FIPS version of boringssl. They take a long time to be approved. |
ggreenway
approved these changes
Jan 22, 2025
@ggreenway thank you for pointing it out, it will save me some time on figuring out how to update FIPS version of Boring SSL! |
bazmurphy
pushed a commit
to bazmurphy/envoy
that referenced
this pull request
Jan 29, 2025
…IPS mode (envoyproxy#38027) Some of the tests in that file rely on EC_group_p256 function that was first introduced in google/boringssl@417069f. Evnidently the version of BoringSSL that Envoy uses in FIPS mode does not contain this change. Because of that build of speed_test.cc fails. To fix the issue I'm replacing calls to EC_group_p256 with EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1). To the best of my knowledge functionally it should be exactly the same thing. Signed-off-by: Mikhail Krinkin <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit Message:
Some of the tests in that file rely on EC_group_p256 function that was first introduced in
google/boringssl@417069f. Evnidently the version of BoringSSL that Envoy uses in FIPS mode does not contain this change. Because of that build of speed_test.cc fails.
To fix the issue I'm replacing calls to EC_group_p256 with EC_GROUP_new_by_curve_name(NID_X9_62_prime256v1). To the best of my knowledge functionally it should be exactly the same thing.
NOTE: Probably a more long term solution would be to update the version of BoringSSL we are using to newer one, as the current one is more than 2 years old at this point. However, it's a bit more work than I can do right now, thus this temporary fix. I will return to the question of updating BoringSSL though affter I close the ongoing bugs.
Additional Description: n/a
Risk Level: low
Testing: tested that builds work now and that all tests in //contrib/cryptomb/private_key_providers/test are still passing.
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a