Introduce a new set of env variables specific to FIPS builds #170
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
nox
force-pushed
the
env
branch
3 times, most recently
from
ee99d80 to
723be81
Compare
Using a struct improves navigation of the build script, as we can rely on rust-analyzer to help us check how a feature flag or an environment variable is used, as opposed to grepping for multiple env::var calls or #[cfg] attributes. This commit also removes some obsolete blocks of code related to the now defunct ndk-old-gcc and fuzzing features.
The current directory from a build script executed by cargo is always the manifest dir, so we may as well only use the manifest dir.
It's actually unused.
This means BORING_SSL_PRECOMPILED_BCM_O is now BORING_BSSL_PRECOMPILED_BCM_O. Prefix BORING_BSSL_ has been chosen because that's the one that is used the most among all the variables the build script uses.
Builds using feature fips or fips-link-precompiled now read variables prefixed by BORING_BSSL_FIPS_ instead of BORING_BSSL_. This helps complex builds where build dependencies also use boring, where we may not want to use fips there. Without those separate variables, the boring build for the build dependencies end up relying on e.g. BORING_BSSL_PATH, causing errors if this path is a boring checkout intended for fips builds, while the fips feature isn't enabled for the build dependency.
ghedo
approved these changes
Oct 23, 2023
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.
BoringSSL checkouts for FIPS-validated builds and for normal builds can be quite different, enough for them to not build with their expected
fipsorfips-link-precompiledfeature settings, for example when they are both used as runtime dependencies and build dependencies. When using cargo's resolver 2, the user then needs to make sure that boring is compiled with the same FIPS-related features for both kind of dependencies, which can sometimes be difficult.For this reason, this PR introduces a new set of variables prefixed by
BORING_BSSL_FIPS_instead ofBORING_BSSL_:BORING_BSSL_FIPS_PATHBORING_BSSL_FIPS_INCLUDE_PATHBORING_BSSL_FIPS_SOURCE_PATHBORING_BSSL_FIPS_PRECOMPILED_BCM_OWhile at it, the prefix were normalized, as
BORING_BSSL_FIPS_PRECOMPILED_BCM_Oused to beBORING_SSL_PRECOMPILED_BCM_O(note how it was prefixed withBORING_SSL_instead ofBORING_BSSL_).To support this work, the build script was refactored and a helper struct
Configwas introduced to help navigating how the env variables and the various feature flags were used. Now they are all struct fields that can be tracked easily with rust-analyzer.