-
Notifications
You must be signed in to change notification settings - Fork 121
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
Fix build with "fips-link-precompiled" feature #147
Conversation
.expect("`fips-link-precompiled` requires `BORING_SSL_PRECOMPILED_BCM_O` env variable to be specified"); | ||
|
||
let libcrypto_path = PathBuf::from(bssl_dir) | ||
.join("build/crypto/libcrypto.a") | ||
.join("build/libcrypto.a") |
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 forgot to mention this in the commit mesesage, but this seems to be necessary for the version of boringSSL we're building.
3139a3b
to
26acb64
Compare
26acb64
to
a69cbb9
Compare
When the "fips-link-precompiled" feature is used, the build script for boringSSL (`boring-sys/build.sh`) adds a precompiled `bcm.o` module provided by the user. The module is renamed to `bcm-fips.o` and inserted into `libcrypto.a` just before the `bcm.o` built by the script. The intent is to "shadow" the module so that, for any symbols that are provided by both, the linker picks the implementations provided by `bcm-fips.o`. At the same time, any sybmols in `bcm.o` that are not in `bcm-fips.o` can be used. This configuration requires special flags in order to tell the linker how to resolve duplicate symbols (RUSTFLAGS="-Clink-args=-Wl,-zmuldefs" is sufficient). However even with these flags thare are certain symbols that don't resolve. In particular `bcm-fips.o` expects `bcm.o` to provide `RAND_need_entropy`. Rather than attempt to cobble together a working version of this "shadow" build of `libcrypto.a`, we modify the build script so that it "replaces" `bcm.o` with the precompiled module provided by the user. Based on internal conversations, this appears to be sufficient for every use case for these bindings. If the shadow build is required, then the user will need to provide their own version of `libcrypto.a` (This is not supported as of this commit.) One more change is required in order to build with "fips-link-precompiled". Building fails because the FFI exports a different API than the bindings expects. To fix this, it is sufficient to change the features so that "fips-link-precompiled" does not imply "fips".
a69cbb9
to
67d4cb5
Compare
.canonicalize() | ||
.unwrap() | ||
.display() | ||
.to_string(); | ||
|
||
let bcm_o_dst_path = PathBuf::from(bssl_dir).join("build/bcm-fips.o"); |
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 is the critical line. The "shadow" build gives the precompiled module a distinctive name so that bcm-fips.o
and bcm.o
are both included in libcrypto.a
. In this PR, we give the precompiled module the same name so that it replaces bcm.o
.
lgtm |
We expect `fips::enabled()` to be set when the "fips-link-precompiled" feature is used.
I've updated test |
Closing this PR since we know it doesn't fix the "fips-link-precompiled" build on its own. |
When the "fips-link-precompiled" feature is used, the build script boringSSL (
boring-sys/build.sh
) adds a precompiledbcm.o
module provided by the user. The module is renamed tobcm-fips.o
and inserted intolibcrypto.a
just before thebcm.o
built by the script. The intent is to "shadow" the module so that, for any symbols that are provided by both, the linker picks the implementations provided bybcm-fips.o
. At the same time, any sybmols inbcm.o
that are not inbcm-fips.o
can be used.This configuration requires special flags in order to tell the linker how to resolve duplicate symbols (RUSTFLAGS="-Clink-args=-Wl,-zmuldefs" is sufficient). However even with these flags thare are certain symbols that don't resolve. In particular
bcm-fips.o
expectsbcm.o
to provideRAND_need_entropy
.Rather than attempt to cobble together a working version of this "shadow" build of
libcrypto.a
, we modify the build script so that it "replaces"bcm.o
with the precompiled module provided by the user. Based on internal conversations, this appears to be sufficient for every use case for these bindings. If the shadow build is required, then the user will need to provide their own version oflibcrypto.a
(This is not supported as of this commit.)One more change is required in order to build with "fips-link-precompiled". Building fails because the FFI exports a different API than the bindings expects. To fix this, it is sufficient to change the features so that "fips-link-precompiled" does not imply "fips".