Skip to content
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

Build embedded BoringSSL with symbol prefix #702

Merged
merged 4 commits into from
Aug 18, 2020

Conversation

ilammy
Copy link
Collaborator

@ilammy ilammy commented Aug 14, 2020

Since PR #681, when building with ENGINE=boringssl, Soter embeds BoringSSL into its libraries, including static ones. This enables the users to link only against libthemis.a and libsoter.a, without having to salvage the BoringSSL binaries from the build directory (which are not included into packages).

What is the issue here?

However, embedding BoringSSL has a side effect. Due to the way static linkage works, all BoringSSL symbols from libsoter.a are indistinguishable from Soter symbols for exporting purposes. That is, any binary linked against libsoter.a will by default include and export all BoringSSL symbols. This is problematic because it can cause symbol conflicts. Dynamic libraries are not affected by this because we control what symbols are exported. That way BoringSSL stays inside Soter library and does not cause conflicts.

In order to avoid conflicts, start using BoringSSL's symbol renaming facility. EVP_sha256() will be renamed into SOTER_0_14_0_EVP_sha256() and all call sites from Soter will refer to this new symbol instead. This makes Soter's BoringSSL copy independent of any WhateverSSL is there in the binary.

How renaming works

Bad news here is that renaming requires building BoringSSL twice. First we build it to enumerate the symbols to rename, then it is built again, now with renamed symbols. All symbols need to be renamed, not only those used by Soter, because the entire BoringSSL library is embedded into Soter. We cannot simply enumerate and hardcode what we use.

After building BoringSSL with renamed symbols, Soter needs to use them in renamed form. For that it needs to be built with BORINGSSL_PREFIX define and <boringssl_prefix_symbols.h> in its include path. That header will perform actual renaming using C preprocessor.

Other build details

Previously Soter did not care about build order, but now BoringSSL needs to be built (possibly renamed) before Soter can be built.

Note that extracting symbols with BoringSSL tool requires Go. BoringSSL build itself requires Go so this is not a new dependency.

Disable BoringSSL renaming for certain systems

Enable BoringSSL symbol renaming by default for Linux and macOS desktop builds and disable it for WebAssembly builds.

AndroidThemis also uses BoringSSL, but it does not use Themis Makefile for that. It does not use renaming either.

Renaming is important only for the case of static libraries, and only desktop systems provide them in packages. Mobile systems use dynamic linkage for the most part and avoid symbol conflicts.

The default can be overridden from the command line:

make ENGINE=boringssl RENAME_BORINGSSL_SYMBOLS=no

or "yes" to enable it.

Checklist

  • Change is covered by automated tests
  • The coding guidelines are followed
  • Public API has proper documentation
  • Changelog is updated

When building with ENGINE=boringssl, Soter embeds BoringSSL into its
libraries, including static ones. This enables the users to link only
against libthemis.a and libsoter.a, without having to salvage the
BoringSSL binaries from the build directory (which are *not* included
into packages).

What is the issue here?
-----------------------

However, embedding BoringSSL has a side effect. Due to the way static
linkage works, all BoringSSL symbols from "libsoter.a" are
indistinguishable from Soter symbols for exporting purposes. That is,
any binary linked against "libsoter.a" will by default include and
export all BoringSSL symbols. This is problematic because it can cause
symbol conflicts. Dynamic libraries are not affected by this because
we control what symbols are exported. That way BoringSSL stays inside
Soter library and does not cause conflicts.

In order to avoid conflicts, start using BoringSSL's symbol renaming
facility. EVP_sha256() will be renamed into SOTER_0_14_0_EVP_sha256()
and all call sites from Soter will refer to this new symbol instead.
This makes Soter's BoringSSL copy independent of any WhateverSSL is
there in the binary.

How renaming works
------------------

Bad news here is that renaming requires building BoringSSL twice. First
we build it to enumerate the symbols to rename, then it is built again,
now with renamed symbols. All symbols need to be renamed, not only those
used by Soter, because the entire BoringSSL library is embedded into
Soter. We cannot simply enumerate and hardcode what we use.

After building BoringSSL with renamed symbols, Soter needs to use them
in renamed form. For that it needs to be built with BORINGSSL_PREFIX
define and <boringssl_prefix_symbols.h> in its include path. That header
will perform actual renaming using C preprocessor.

Other build details
-------------------

Previously Soter did not care about build order, but now BoringSSL needs
to be built (possibly renamed) before Soter can be built.

Note that extracting symbols with BoringSSL tool requires Go. BoringSSL
build itself requires Go so this is not a new dependency.
Enable BoringSSL symbol renaming by default for Linux and macOS desktop
builds and disable it for WebAssembly builds.

AndroidThemis also uses BoringSSL, but it does not use Themis Makefile
for that. It does not use renaming either.

Renaming is important only for the case of static libraries, and only
desktop systems provide them in packages. Mobile systems use dynamic
linkage for the most part and avoid symbol conflicts.

The default can be overridden from the command line:

    make ENGINE=boringssl RENAME_BORINGSSL_SYMBOLS=no

or "yes" to enable it.
@ilammy ilammy added core Themis Core written in C, its packages O-macOS 💻 Operating system: macOS infrastructure Automated building and packaging C-BoringSSL Crypto provider: BoringSSL O-Linux 🐧 Operating system: Linux labels Aug 14, 2020
@ilammy ilammy requested a review from Lagovas as a code owner August 14, 2020 09:09
Copy link
Collaborator

@Lagovas Lagovas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow, cool. never thought about that and have not seen such approaches )

@ilammy
Copy link
Collaborator Author

ilammy commented Aug 17, 2020

The build currently fails in CircleCI when building BoringSSL because of outdated Go version. CircleCI uses Go 1.9 while BoringSSL recommends using the latest stable version (Go 1.15 as of now). I have checked that using the latest version fixes the issue.

Well, given #709, I guess I'll just sit here and wait while the corpse of my enemy floats past me and then refresh this PR.

@shadinua, take a note for Buildbot on this.

Copy link
Contributor

@vixentael vixentael left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good idea! Let's see what we'll get!

Comment on lines +60 to +85
@mkdir -p $(BIN_PATH)/boringssl/stage-1
@cd $(BIN_PATH)/boringssl/stage-1 && \
$(CMAKE) $(SOTER_ENGINE_CMAKE_FLAGS) $(abspath third_party/boringssl/src)
@$(MAKE) -C $(BIN_PATH)/boringssl/stage-1 crypto decrepit
@mkdir -p $(BIN_PATH)/boringssl/crypto $(BIN_PATH)/boringssl/decrepit
@cp $(BIN_PATH)/boringssl/stage-1/crypto/libcrypto.a $(BIN_PATH)/boringssl/crypto/libcrypto.a
@cp $(BIN_PATH)/boringssl/stage-1/decrepit/libdecrepit.a $(BIN_PATH)/boringssl/decrepit/libdecrepit.a
ifeq ($(RENAME_BORINGSSL_SYMBOLS),yes)
@cd third_party/boringssl/src && \
$(GO) run util/read_symbols.go -out $(abspath $(BIN_PATH)/boringssl/symbols.txt) \
$(abspath $(BIN_PATH)/boringssl/stage-1/crypto/libcrypto.a) \
$(abspath $(BIN_PATH)/boringssl/stage-1/decrepit/libdecrepit.a)
@# Path to symbols must be a relative one (relative to the build directory)
@# because absolute paths confuse BoringSSL's make.
@echo "building embedded BoringSSL again with renamed symbols..."
@mkdir -p $(BIN_PATH)/boringssl/stage-2
@cd $(BIN_PATH)/boringssl/stage-2 && \
$(CMAKE) $(SOTER_ENGINE_CMAKE_FLAGS) \
-DBORINGSSL_PREFIX=$(SOTER_BORINGSSL_PREFIX) \
-DBORINGSSL_PREFIX_SYMBOLS=../symbols.txt \
$(abspath third_party/boringssl/src)
@$(MAKE) -C $(BIN_PATH)/boringssl/stage-2 crypto decrepit
@mkdir -p $(BIN_PATH)/boringssl/crypto $(BIN_PATH)/boringssl/decrepit
@cp $(BIN_PATH)/boringssl/stage-2/crypto/libcrypto.a $(BIN_PATH)/boringssl/crypto/libcrypto.a
@cp $(BIN_PATH)/boringssl/stage-2/decrepit/libdecrepit.a $(BIN_PATH)/boringssl/decrepit/libdecrepit.a
endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my guts tell me that this code might have unexpected sideeffects, but i've read it multiple times and can't find a problem 🤔

@ilammy ilammy merged commit 4ae325d into cossacklabs:master Aug 18, 2020
@ilammy ilammy deleted the boringssl-renaming branch August 18, 2020 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-BoringSSL Crypto provider: BoringSSL core Themis Core written in C, its packages infrastructure Automated building and packaging O-Linux 🐧 Operating system: Linux O-macOS 💻 Operating system: macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants