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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ _Code:_

- **Core**

- Include embedded BoringSSL into Soter for convenience ([#681](https://github.com/cossacklabs/themis/pull/681)).
- Include embedded BoringSSL into Soter for convenience ([#681](https://github.com/cossacklabs/themis/pull/681), [#702](https://github.com/cossacklabs/themis/pull/702)).
- `make deb` and `make rpm` with `ENGINE=boringssl` will now produce `libthemis-boringssl` packages with embedded BoringSSL ([#683](https://github.com/cossacklabs/themis/pull/683), [#686](https://github.com/cossacklabs/themis/pull/686)).
- `secure_session_create()` now allows only EC keys, returning an error for RSA ([#693](https://github.com/cossacklabs/themis/pull/693)).

Expand All @@ -34,6 +34,7 @@ _Infrastructure:_
- Optimized dependencies of `libthemis` DEB and RPM packages ([#682](https://github.com/cossacklabs/themis/pull/682), [#686](https://github.com/cossacklabs/themis/pull/686)).
- `make deb` and `make rpm` with `ENGINE=boringssl` will now produce `libthemis-boringssl` packages with embedded BoringSSL ([#683](https://github.com/cossacklabs/themis/pull/683), [#686](https://github.com/cossacklabs/themis/pull/686)).
- Build system and tests now respect the `PATH` settings ([#685](https://github.com/cossacklabs/themis/pull/685)).
- Rename embedded BoringSSL symbols by default to avoid conflicts with system OpenSSL ([#702](https://github.com/cossacklabs/themis/pull/702)).
- Started phasing out CircleCI in favour of GitHub Actions ([#709](https://github.com/cossacklabs/themis/pull/709)).

## [0.13.2](https://github.com/cossacklabs/themis/releases/tag/0.13.2), August 14th 2020
Expand Down
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ LIBRARY_SO_VERSION = 0
#----- Toolchain ---------------------------------------------------------------

CMAKE ?= cmake
GO ?= go

CLANG_FORMAT ?= clang-format
CLANG_TIDY ?= clang-tidy
Expand Down
44 changes: 41 additions & 3 deletions src/soter/boringssl/soter.mk
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,50 @@ CRYPTO_ENGINE_LDFLAGS += -lcrypto -ldecrepit -lpthread
SOTER_ENGINE_CMAKE_FLAGS += -DCMAKE_BUILD_TYPE=Release
SOTER_ENGINE_CMAKE_FLAGS += -DCMAKE_C_FLAGS="-fpic"

ifdef IS_LINUX
RENAME_BORINGSSL_SYMBOLS = yes
endif
ifdef IS_MACOS
RENAME_BORINGSSL_SYMBOLS = yes
endif
ifdef IS_EMSCRIPTEN
RENAME_BORINGSSL_SYMBOLS = no
SOTER_ENGINE_CMAKE_FLAGS += -DOPENSSL_NO_ASM=1
endif

ifeq ($(RENAME_BORINGSSL_SYMBOLS),yes)
# The prefix must be a valid C identifier. Replace invalid characters.
SOTER_BORINGSSL_PREFIX := $(shell echo "SOTER_$(VERSION)" | tr '.-' '_')

SOTER_CRYPTO_ENGINE_CFLAGS += -DBORINGSSL_PREFIX=$(SOTER_BORINGSSL_PREFIX)
SOTER_CRYPTO_ENGINE_CFLAGS += -I$(BIN_PATH)/boringssl/stage-2/symbol_prefix_include
endif

$(BIN_PATH)/boringssl/crypto/libcrypto.a $(BIN_PATH)/boringssl/decrepit/libdecrepit.a:
@echo "building embedded BoringSSL..."
@mkdir -p $(BIN_PATH)/boringssl
@cd $(BIN_PATH)/boringssl && $(CMAKE) $(SOTER_ENGINE_CMAKE_FLAGS) $(abspath third_party/boringssl/src)
@$(MAKE) -C $(BIN_PATH)/boringssl crypto decrepit
@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
Comment on lines +60 to +85
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 🤔

6 changes: 6 additions & 0 deletions src/soter/soter.mk
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ FMT_CHECK += $(patsubst %,$(OBJ_PATH)/%.fmt_check, $(SOTER_FMT_SRC))
SOTER_STATIC = $(BIN_PATH)/$(LIBSOTER_A)

$(SOTER_OBJ): CFLAGS += -DSOTER_EXPORT
$(SOTER_OBJ): CFLAGS += $(SOTER_CRYPTO_ENGINE_CFLAGS)

# First build Soter library, then merge embedded crypto engine libs into it.
# On macOS this may cause warnings about files with no symbols in BoringSSL,
Expand All @@ -66,6 +67,11 @@ $(BIN_PATH)/$(LIBSOTER_A): CMD = $(AR) rcs $@ $(filter %.o, $^) \
&& scripts/merge-static-libs.sh $@ $(filter %.a, $^) \
$(if $(IS_MACOS),> >(grep -v 'has no symbols$$'))

# Make sure to build dependencies before objects. This is important in case
# of embedded BoringSSL with renamed symbols: they need to be renamed before
# Soter's objects are built against them.
$(SOTER_OBJ): $(SOTER_ENGINE_DEPS)

$(BIN_PATH)/$(LIBSOTER_A): $(SOTER_OBJ) $(SOTER_ENGINE_DEPS)
@mkdir -p $(@D)
@echo -n "link "
Expand Down