Add symbol prefixing feature for BoringSSL - #103
Conversation
|
Thanks for fixing this issue. It seems there’s a merge conflict — could you help resolve it? |
|
Additionally, GitHub Actions provides macOS and Windows environments, which makes testing on those platforms possible. I think you should also add a GitHub Actions workflow to ensure everything works correctly across different platforms. |
I created this branch about a month ago and overlooked opening the PR. I’ll resolve the merge conflict now 👍 |
3bb25f7 to
f2bded7
Compare
I'll look into it 👍
I’m not fully confident with this yet, but I'll give it a try and see how far I can get. |
661ed10 to
6db074a
Compare
|
done? |
|
Not yet. As I mentioned upstream (in cloudflare/boring), I have only managed to add GitHub Actions for Linux and Android so far. For now, I have added a check to ensure that no one can use this feature on macOS/iOS/Windows (the build will panic). I am still struggling with macOS and Windows:
If you are okay with merging this with only Linux and Android support, I will continue working on Windows and macOS support in a follow-up PR. |
|
We can initially support Linux/Android, but at least we should provide compilation errors for non-supported platforms. #[cfg(feature = "prefix-symbols")]
compile_error!("not supports...."); |
|
I've added this in build/main.rs (main function) if config.features.prefix_symbols && ["macos", "ios", "windows"].contains(&config.target_os.as_str())
{
panic!("The `prefix_symbols` feature is not supported on macOS/iOS or windows targets.");
} |
|
Could you also fix Clippy at the same time? It is suggested to place the "prefix-symbols ci job" directly at the end of the ci.yml file. This will make the testing process more convenient. Additionally, on Windows, the CMake version needs to be downgraded to 0.1.54 in order to fix the testing issue. |
|
The Clippy issues are not in the code I added, but I can fix them 👍 Same for the Windows CI not passing, I didn't modify the Windows CI, but I can look into it
Sure, I'll do that 👍 (I've made it a separated file so that it would be easier to merge upstream) |
6db074a to
13b8864
Compare
|
ci.yml and clippy: done ✔️
cmake already appears to be set to version "0.1.54" in the workspace's Cargo.toml, do I need to strictly pin it to "=0.1.54" ? |
to ci: https://github.com/cloudflare/quiche/pull/2285/changes |
|
Done ✔️ |
|
It seems that the 32-bit operating system still has some issues. My suggestion is to abandon supporting 32-bit? Some tests still haven't passed: https://github.com/0x676e67/boring2/actions/runs/20210077959/job/58014693403?pr=103 |
|
Weird, it's working on my github actions https://github.com/BarbossHack/boring2/actions/runs/20194237006 Maybe moving it to ci.yml changed something... I could fix it, but yeah I think we can drop 32 bit support, especially since i686-linux-android has long standing issues when building openssl (with -latomic), which are unrelated to the boringssl symbol prefixing |
|
Check if there are any areas that need to be cleaned. If not, we can combine them. |
|
You mean combining all the job steps in 1 step? |
Goal
Add a prefix to all symbols in libcrypto and libssl to prevent conflicts with other OpenSSL or BoringSSL versions that might be linked in the same process.
Why?
When statically linking both OpenSSL and BoringSSL in the same project, we encountered duplicate symbols, for example:
How?
The first approach was to use the BoringSSL-provided Go script
make_prefix_headers.go. It mostly worked, but:read_symbols.go, then to generateboringssl_prefix_symbols.hwithmake_prefix_headers.go, and finally rebuild again the static libraries with this header. (We could have prebuilt it like gRPC does, but I preferred a fully automated workflow.)boringssl_prefix_symbols.h), which I ended up fixing with the Binutilsobjcopytool, not very straightforward.Therefore, I decided to use only the Binutils tools
nmandobjcopyto fully automate the symbol prefixing.Steps (build.rs):
libssl.aandlibcrypto.a).nmto list all exported symbols in these static libraries.objcopyto prefix all symbols.bindings.rs) with#[link_name]where necessary.Cross-compilation
Currently, this workflow is tested for Linux and Android only.
For Android, the target-specific
nmandobjcopyfrom the NDK are used.macOS/iOS and Windows toolchains are not tested. I currently don’t have access to a macOS environment and have limited experience with Windows toolchains.
Usage
I've added an opt-in feature
prefix-symbols, as it is not tested on macOS/iOS/Windows yet.