Add 2024 FIPS and fix build issues on older arm FIPS #1920
Merged
Conversation
This file contains hidden or 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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1920 +/- ##
==========================================
+ Coverage 78.53% 78.54% +0.01%
==========================================
Files 585 585
Lines 99783 99803 +20
Branches 14283 14286 +3
==========================================
+ Hits 78362 78395 +33
+ Misses 20785 20774 -11
+ Partials 636 634 -2 ☔ View full report in Codecov by Sentry. |
andrewhop
approved these changes
Oct 16, 2024
samuel40791765
approved these changes
Oct 16, 2024
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.
Issues:
P162016551
Description of changes:
Firstly, when building multiple versions of
bsslusing themainbranch as the base, builds on Arm will fail when using older versions of AWS-LC e.g. FIPS 2021 from the 2021 FIPS branch. The reason is that an internal header file[...]/cpucap/nternal.his included into the compilation unit by#include "...". The gcc search path will then, irrespective of any user-specified search paths via-I, pick up themainbranch header files. This include identifiers such asARMV8_NEOVERSE_V2. But the definition of this identifier is inarm_arch.h, which is transitively included into the compilation unit via#include <..>directives. The search path for these DO follow the user-specified search path from-I(at least on GCC) and use e.g. the FIPS 2021 include file version ofarm_arch.hthat does not have the identifier defined. The result of this are build errors. For example, from one of the performance canary hosts:The pattern of adding internal header files to a consuming application that is going to be build with multiple old version of the library is not ideal, because one can hit these hidden incompatibilities that. But now it's a bit rusted-in and it's not apparent to me if there is a much easier way to organise it currently. Leaving that as an open question and simply fix by guarding the inclusion for now.
Secondly, the reason this even escaped as a bug, is that the CI doesn't test the special multiple-build of
bsslwith other library sources on Arm. We only test on x86_64. The above is purely an Arm-specific bug because for som reason there is a special Arm header file(?). I added an Arm stanza to the Arm omnibus config file that will execute the relevant test script.However, this needs to be deployed before taking effect.(EDIT: no, actually not...)Thirdly, the
LD_LIBRARY_PATHoption was incorrect foraws-lc-fips-2022, when testing the executable. But, in fact, this doesn't actually matter forbssl, becauseDT_RPATHis set in the resulting executables, e.g.:When resolving the dynamic dependency at load-time,
DT_RPATHtakes precedence overLD_LIBRARY_PATH. As an example, trying to resolve to another library version still resolve toDT_RPATHfrom the executable:Finally, added the new FIPS 2024 prospective branch to the test cases.
Testing:
I tested the changes to the speed tool locally to ensure that build errors are resolved.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.