CDRIVER-4730 TLS Backend Cleanup #1408

vector-of-bool · 2023-09-20T16:30:57Z

This PR is a continuation of work from #1405, and addresses the ENABLE_SSL setting and related logic.

Changed

The FindLibreSSL.cmake module has been added, and comes from the LibreSSL project itself, plus a few bugfixes. Newer LibreSSL ships CMake config-file packages, so maybe we should rely on those instead? It depends on whether our LibreSSL userbase is all on the newer version (very unlikely, as that's a new feature).
The Earthfile has been expanded to include parameters for the tls build option, which controls which packages are installed and sets the appropriate CMake build settings.
Earthfile parameters have been renamed and reformatted for just being "prettier" in general.
More EVG tasks were added for the additional configurations. The earthly.py file has been expanded to express these additional complexities, since it now needs to exclude tasks on certain platforms that do not (yet) have a LibreSSL.

Task Goofiness

I wasn't particularly happy with the way EVG tasks are named, as they're quite hard on the eyes. Evergreen does not allow spaces in task names, forward-slash breaks things, and commas are "not supported", so I broke out the unicode to create new exciting names, such as:

check : sasl:Cyrus • tls:LibreSSL • test_mongocxx_ref:master

which uses U+00a0 "No-break space" and U+2022 "bullet". I'm not married to this trick, so feel free to veto this aside change.

Adds a --tls build parameter to switch TLS backends. This PR also adds a "str" utility for doing more concise string manipulation in POSIX sh.

This refactors earthly.py for extensibility and to match the parameterization used in Earthfile. Task names now use NBSP for whitespace.

kevinAlbs · 2023-09-22T15:03:27Z

src/libmongoc/CMakeLists.txt

+set(SecureTransport/pkg_config_LIBS -framework Corefoundation -framework Security)
+set(SecureChannel/LINK_LIBRARIES secur32.lib crypt32.lib Bcrypt.lib)
+set(SecureChannel/pkg_config_LIBS ${SecureChannel/LINK_LIBRARIES})
+set(LibreSSL/LINK_LIBRARIES LibreSSL::TLS LibreSSL::SSL LibreSSL::Crypto)


Suggested change

set(LibreSSL/LINK_LIBRARIES LibreSSL::TLS LibreSSL::SSL LibreSSL::Crypto)

set(LibreSSL/LINK_LIBRARIES LibreSSL::TLS LibreSSL::Crypto)

I expect linking to LibreSSL's libssl (OpenSSL compat library) is not needed.

Current C driver config includes: set (SSL_LIBRARIES -ltls -lcrypto).

Change was tested with:

./tools/earthly.sh +build --env=alpine3.18 --tls=LibreSSL --sasl=Cyrus

kevinAlbs · 2023-09-22T15:08:11Z

src/libmongoc/src/mongoc-config.cmake

+if(_tls_package)
+  # XXX: Some platforms (e.g. Arch) install LibreSSL in a qualified path to not collide
+  #      with OpenSSL. Adding those as "roots" here will cause FindLibreSSL to search
+  #      thos directories as well.


Suggested change

# thos directories as well.

# those directories as well.

tools/str.sh

.evergreen/config_generator/components/earthly.py

kevinAlbs · 2023-09-22T17:02:40Z

CMakeLists.txt

@@ -623,4 +618,4 @@ if (CMAKE_GENERATOR STREQUAL "Ninja Multi-Config" AND PROJECT_IS_TOP_LEVEL)
   set (CMAKE_CROSS_CONFIGS "all")
 endif ()

-feature_summary(WHAT ENABLED_FEATURES DISABLED_FEATURES)
+feature_summary(WHAT ALL)


Suggested change

feature_summary(WHAT ALL)

feature_summary(WHAT ENABLED_FEATURES DISABLED_FEATURES)

Revert?

Testing configuring with:

cmake \ -G "Ninja" \ -DENABLE_SSL=DARWIN \ -S$(pwd) -B$(pwd)/cmake-build

Before these changes shows:

-- The following features have been enabled: * TLS, for secure network communication (SecureTransport) * Cryptography, cryptographic primitives (SecureTransport) * AWS Authentication, authenticate with MongoDB servers using credentials from AWS instance metadata * SASL Authentication, authenticate with MongoDB servers using SASL: “Simple Authentication and Security Layer” (Cyrus) -- Configuring done (0.8s)

After these changes:

-- The following features have been enabled: * TLS, for secure network communication (SecureTransport) * Cryptography, cryptographic primitives (SecureTransport) * AWS Authentication, authenticate with MongoDB servers using credentials from AWS instance metadata * SASL Authentication, authenticate with MongoDB servers using SASL: “Simple Authentication and Security Layer” (Cyrus) -- The following OPTIONAL packages have been found: * Python * Python3 * PkgConfig * mongocrypt -- The following REQUIRED packages have been found: * SASL2 (required version >= 2.0), Cyrus implementation of SASL Provides libmongoc SASL authentication support * Threads * Sphinx -- The following REQUIRED packages have not been found: * Utf8Proc -- Configuring done (0.7s)

The listing of Utf8Proc as "REQUIRED" and "not found" seems unexpected. That may be surfacing a separate issue. Until that is addressed, I suggest excluding from the feature summary.

I forgot about the misleading Utf8Proc report, which is why I didn't use ALL in the prior PR. I can see the cause and opened a ticket for this one: CDRIVER-4732

kevinAlbs · 2023-09-22T17:41:26Z

src/libmongoc/CMakeLists.txt

-      endif ()
-   endif ()
+# Per-backend link libs/options:
+set(SecureTransport/LINK_OPTIONS "SHELL:-framework CoreFoundation" "SHELL:-framework Security")


Remove SHELL:?

On local install (with macOS), this produced a package config file with SHELL: in Libs:

# pkg-config .pc file generated by CMake 3.27.4 for libmongoc-1.11.1. DO NOT EDIT! prefix=/Users/kevin.albertson/review/mongo-c-driver-1408/.install exec_prefix=${prefix} libdir=${exec_prefix}/lib Name: libmongoc Description: The MongoDB C Database Driver library Version: 1.11.1 Requires: libbson-static-1.0 Libs: -L${libdir} -lmongoc-static-1.0 -lresolv -lsasl2 -framework Corefoundation -framework Security -L/opt/homebrew/Cellar/zstd/1.5.5/lib -lzstd -L/Users/kevin.albertson/code/c-bootstrap/install/libmongocrypt-1.8.0-alpha1/lib -lmongocrypt -fsanitize=address,leak SHELL:-framework CoreFoundation SHELL:-framework Security Cflags: -fPIC -fsanitize=address,leak -DMONGOC_STATIC -DBSON_STATIC -I${prefix}/include/libmongoc-1.0

Attempting to use resulted in an error:

% export PKG_CONFIG_PATH=/Users/kevin.albertson/review/mongo-c-driver-1408/.install/lib/pkgconfig % /opt/homebrew/opt/llvm/bin/clang -o test.c $(pkg-config --libs --cflags libmongoc-static-1.0) clang-16: error: no such file or directory: 'SHELL:-framework' clang-16: error: no such file or directory: 'CoreFoundation' clang-16: error: no such file or directory: 'SHELL:-framework' clang-16: error: no such file or directory: 'Security'

kevinAlbs · 2023-09-22T17:43:01Z

which uses U+00a0 "No-break space" and U+2022 "bullet". I'm not married to this trick, so feel free to veto this aside change.

I prefer avoiding the "No-break space". I often use the evergreen CLI to run individual tasks (with the -v and -t arguments). I expect the "No-break space" may be a surprise when copy-pasting a task name. I suggest removing the spaces: check:sasl:Cyrus•tls:LibreSSL•test_mongocxx_ref:master or replacing with a non-space Unicode character.

Needs CDRIVER-4732 fixed to support `WHAT ALL`

kevinAlbs · 2023-09-26T14:48:27Z

.evergreen/config_generator/components/earthly.py

+
+def task_filter(env: EnvKey, conf: Configuration) -> bool:
+    """
+    Control which tasks are actually defined by matching on the


Unfinished comment?

* Clean up of TLS detection and import * Earthly TLS controls and cleaner build envs Adds a --tls build parameter to switch TLS backends. This PR also adds a "str" utility for doing more concise string manipulation in POSIX sh. * EVG Earthly tasks for more build configurations This refactors earthly.py for extensibility and to match the parameterization used in Earthfile. * Support "auto" in Earthfile * Tweak name spacing * More feature and package summary information * Unused LibreSSL compat

vector-of-bool added 9 commits September 19, 2023 21:13

Clean up of TLS detection and import

da20b8c

Earthly TLS controls and cleaner build envs

7b34dc6

Adds a --tls build parameter to switch TLS backends. This PR also adds a "str" utility for doing more concise string manipulation in POSIX sh.

EVG Earthly tasks for more build configurations

dc007e5

This refactors earthly.py for extensibility and to match the parameterization used in Earthfile. Task names now use NBSP for whitespace.

Support "auto" in Earthfile

7fd00a1

Tweak name spacing

45f79b6

Fix broken FindLibreSSL module

487050b

More feature and package summary information

e00eac2

More ENABLE_SSL logic cleanup

cde3a29

Relocate FindLibreSSL fixes

4c1371a

vector-of-bool requested review from kevinAlbs, rcsanchez97 and kkloberdanz and removed request for rcsanchez97 September 20, 2023 20:53

vector-of-bool marked this pull request as ready for review September 21, 2023 06:02

vector-of-bool requested review from adriandole and removed request for kkloberdanz September 22, 2023 00:43

kevinAlbs changed the title ~~TLS Backend Cleanup~~ CDRIVER-4730 TLS Backend Cleanup Sep 22, 2023

kevinAlbs reviewed Sep 22, 2023

View reviewed changes

vector-of-bool added 6 commits September 22, 2023 12:09

Revert from using SHELL:

ce45920

Tweak specials chars in task naming

0010083

Unused LibreSSL compat

4e05e7e

Undo: FeatureSummary report changes are misleading

3c54cac

Needs CDRIVER-4732 fixed to support `WHAT ALL`

I forget that we have ✨ pattern matching ✨

78e7cfd

Merge branch 'master' into tls-cleanup.2

133b8ca

vector-of-bool requested a review from kevinAlbs September 26, 2023 00:29

kevinAlbs approved these changes Sep 26, 2023

View reviewed changes

Unfinished sentence

ddc7a27

adriandole approved these changes Sep 27, 2023

View reviewed changes

vector-of-bool merged commit 8603726 into mongodb:master Sep 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

CDRIVER-4730 TLS Backend Cleanup #1408

CDRIVER-4730 TLS Backend Cleanup #1408

Uh oh!

vector-of-bool commented Sep 20, 2023

Uh oh!

kevinAlbs Sep 22, 2023

Uh oh!

kevinAlbs Sep 22, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kevinAlbs Sep 22, 2023

Uh oh!

vector-of-bool Sep 22, 2023

Uh oh!

kevinAlbs Sep 22, 2023

Uh oh!

vector-of-bool Sep 25, 2023

Uh oh!

kevinAlbs commented Sep 22, 2023

Uh oh!

kevinAlbs Sep 26, 2023

Uh oh!

Uh oh!

	set(LibreSSL/LINK_LIBRARIES LibreSSL::TLS LibreSSL::SSL LibreSSL::Crypto)
	set(LibreSSL/LINK_LIBRARIES LibreSSL::TLS LibreSSL::Crypto)

	feature_summary(WHAT ALL)
	feature_summary(WHAT ENABLED_FEATURES DISABLED_FEATURES)

CDRIVER-4730 TLS Backend Cleanup #1408

CDRIVER-4730 TLS Backend Cleanup #1408

Uh oh!

Conversation

vector-of-bool commented Sep 20, 2023

Changed

Task Goofiness

Uh oh!

kevinAlbs Sep 22, 2023

Choose a reason for hiding this comment

Uh oh!

kevinAlbs Sep 22, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kevinAlbs Sep 22, 2023

Choose a reason for hiding this comment

Uh oh!

vector-of-bool Sep 22, 2023

Choose a reason for hiding this comment

Uh oh!

kevinAlbs Sep 22, 2023

Choose a reason for hiding this comment

Uh oh!

vector-of-bool Sep 25, 2023

Choose a reason for hiding this comment

Uh oh!

kevinAlbs commented Sep 22, 2023

Uh oh!

kevinAlbs Sep 26, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!