Skip to content

Conversation

@goatgoose
Copy link
Contributor

@goatgoose goatgoose commented Oct 23, 2025

Release Summary:

  • A new CMake option was added to enforce that the libcrypto feature probes are able to properly probe the libcrypto for feature support (-DS2N_ENFORCE_PROPER_LIBCRYPTO_FEATURE_PROBE).

Resolved issues:

Related to #5078

Description of changes:

It was indicated in #5078 that there could be ways to build s2n-tls such that the libcrypto feature probes are unable to successfully link to the libcrypto, but s2n-tls itself is during the actual build. This would lead to a state where none of our feature probes are enabled, but the libcrypto does actually support some of these features. This isn't great, since our feature probes determine some rather important stuff such as TLS 1.3 support.

This PR adds a new CMake option to enforce that the feature probes were actually able to link to the libcrypto and correctly determine feature support, and fail the build otherwise.

Call-outs:

We should consider enabling this option by default after users who may not want this behavior opt out (the CRT maybe?).

Testing:

I built s2n-tls on ubuntu25 with this CMake option enabled (without #5572), and with this change the build failed at configuration time rather than build time:

Build log

CMake Deprecation Warning at CMakeLists.txt:1 (cmake_minimum_required):
  Compatibility with CMake < 3.10 will be removed from a future version of
  CMake.

Update the VERSION argument value. Or, use the ... syntax
to tell CMake that the project requires at least but has been updated
to work with policies introduced by or earlier.

-- The C compiler identification is GNU 14.2.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detected CMAKE_SYSTEM_PROCESSOR as x86_64
-- Detected 64-Bit system
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- Found crypto: /usr/lib/x86_64-linux-gnu/libcrypto.a
-- LibCrypto Include Dir: /usr/include
-- LibCrypto Shared Lib: /usr/lib/x86_64-linux-gnu/libcrypto.so
-- LibCrypto Static Lib: /usr/lib/x86_64-linux-gnu/libcrypto.a
-- Using libcrypto from the cmake path
-- CMAKE_AR found: /usr/bin/ar
-- CMAKE_RANLIB found: /usr/bin/ranlib
-- CMAKE_OBJCOPY found: /usr/bin/objcopy
-- feature S2N_ATOMIC_SUPPORTED: TRUE
-- feature S2N_CLOEXEC_SUPPORTED: TRUE
-- feature S2N_CLOEXEC_XOPEN_SUPPORTED: TRUE
-- feature S2N_CLONE_SUPPORTED: TRUE
-- feature S2N_COMPILER_SUPPORTS_BRANCH_ALIGN: TRUE
-- feature S2N_CPUID_AVAILABLE: TRUE
-- feature S2N_DIAGNOSTICS_POP_SUPPORTED: TRUE
-- feature S2N_DIAGNOSTICS_PUSH_SUPPORTED: TRUE
-- feature S2N_EXECINFO_AVAILABLE: TRUE
-- feature S2N_FALL_THROUGH_SUPPORTED: TRUE
-- feature S2N_FEATURES_AVAILABLE: TRUE
-- feature S2N_KTLS_SUPPORTED: TRUE
-- feature S2N_LIBCRYPTO_SANITY_PROBE: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_CUSTOM_OID: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_EC_KEY_CHECK_FIPS: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_ENGINE: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_EVP_AEAD_TLS: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_EVP_KEM: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_EVP_MD_CTX_SET_PKEY_CTX: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_EVP_RC4: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_FLAG_NO_CHECK_TIME: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_HKDF: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_MLDSA: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_MLKEM: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_PRIVATE_RAND: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_PROVIDERS: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_RSA_PSS_SIGNING: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_SHAKE: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_X509_STORE_LIST: FALSE
-- feature S2N_LINUX_SENDFILE: TRUE
-- feature S2N_MADVISE_SUPPORTED: TRUE
-- feature S2N_MINHERIT_SUPPORTED: FALSE
CMake Error at CMakeLists.txt:373 (message):
A sanity-check libcrypto feature probe failed, which indicates that other

  feature probes were likely unable to probe the libcrypto for its supported features:
  Change Dir: '/home/ubuntu/s2n-tls-fork/build/CMakeFiles/CMakeTmp'

Run Build Command(s): /usr/bin/cmake -E env VERBOSE=1 /usr/bin/gmake -f
Makefile cmTC_3c4e4/fast

/usr/bin/gmake -f CMakeFiles/cmTC_3c4e4.dir/build.make
CMakeFiles/cmTC_3c4e4.dir/build

gmake[1]: Entering directory
'/home/ubuntu/s2n-tls-fork/build/CMakeFiles/CMakeTmp'

Building C object CMakeFiles/cmTC_3c4e4.dir/S2N_LIBCRYPTO_SANITY_PROBE.c.o

/usr/bin/cc -I /home/ubuntu/s2n-tls-fork -include
/home/ubuntu/s2n-tls-fork/utils/s2n_prelude.h -c
-Werror-implicit-function-declaration -Wno-unused-variable -o
CMakeFiles/cmTC_3c4e4.dir/S2N_LIBCRYPTO_SANITY_PROBE.c.o -c
/home/ubuntu/s2n-tls-fork/tests/features/S2N_LIBCRYPTO_SANITY_PROBE.c

Linking C executable cmTC_3c4e4

/usr/bin/cmake -E cmake_link_script CMakeFiles/cmTC_3c4e4.dir/link.txt
--verbose=1

/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libcrypto.a(libcrypto-lib-c_zlib.o):
in function `zlib_stateful_expand_block':

(.text+0x89): undefined reference to `inflate'
...

The flag was also set when building the unit tests, which ensures that setting this flag doesn't cause the build to fail under normal circumstances.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Oct 23, 2025
@goatgoose goatgoose force-pushed the enforce-correct-probing branch from c621063 to 069efec Compare October 23, 2025 23:30
@goatgoose goatgoose marked this pull request as ready for review October 27, 2025 21:44
@goatgoose goatgoose requested a review from dougch as a code owner October 27, 2025 21:44
@goatgoose goatgoose requested a review from jmayclin October 27, 2025 21:45
Copy link
Contributor

@dougch dougch left a comment

Choose a reason for hiding this comment

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

Played with this a bit, looks good minus the nix helper function.

-DCMAKE_PREFIX_PATH=$LIBCRYPTO_ROOT \
-DBUILD_SHARED_LIBS=on
-DBUILD_SHARED_LIBS=on \
-DS2N_ENFORCE_PROPER_LIBCRYPTO_FEATURE_PROBE=1
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I added this flag to a CI job was mostly just to make sure it works when you enable it (and the demonstrated failure for ubuntu25 wasn't because enabling the flag just always causes the build to fail, for example). Ideally we should just enable the flag by default when/if it's safe to do so and then all of our CI would get it. But it doesn't hurt to add it early to nix as well.

@goatgoose goatgoose requested a review from dougch November 1, 2025 03:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants