Feature probes fail to enable supported libcrypto features due to missing linker flags

[goatgoose]

Problem:

In some environments, additional linker flags are needed in order to successfully link to the libcrypto. Since the libcrypto feature probes do not provide these flags, this causes them to fail to link to the libcrypto and disable all libcrypto features as a result. See #5075 (comment) for example, which shows that the feature probes fail not because the libcrypto doesn't support the features, but because the feature probes fail to link the libcrypto to zlib.

Solution:

We should find a way to properly link the feature probes to the libcrypto in these environments.

[goatgoose]

I'm able to reproduce this issue on AL2023 by building OpenSSL 3.4.0 from source with zlib enabled:

> ./Configure --prefix=<install dir> --openssldir=<openssl diur> enable-zstd

Building s2n-tls with this libcrypto results in all of the libcrypto feature probes being disabled:

> cmake3 . -Bbuild -DCMAKE_PREFIX_PATH=<openssl lib dir> -DS2N_INTERN_LIBCRYPTO=ON
...
-- 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_MLKEM: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_RSA_PSS_SIGNING: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_X509_STORE_LIST: FALSE

Observing the feature probe outputs confirms the zlib linking issue:

-- feature S2N_LIBCRYPTO_SUPPORTS_RSA_PSS_SIGNING: FALSE
-- Output of try_compile: Change Dir: /home/ec2-user/s2n-tls/build/CMakeFiles/CMakeTmp
...
/usr/bin/cmake -E cmake_link_script CMakeFiles/cmTC_616c3.dir/link.txt --verbose=1
/usr/bin/cc CMakeFiles/cmTC_616c3.dir/S2N_LIBCRYPTO_SUPPORTS_X509_STORE_LIST.c.o -o cmTC_616c3  /home/ec2-user/openssl/openssl_install/lib64/libcrypto.a -ldl -lrt 
/usr/bin/ld: /home/ec2-user/openssl/openssl_install/lib64/libcrypto.a(libcrypto-lib-c_zstd.o): in function `zstd_stateful_expand_block':
c_zstd.c:(.text+0x9f): undefined reference to `ZSTD_decompressStream'
/usr/bin/ld: c_zstd.c:(.text+0xaa): undefined reference to `ZSTD_isError'
...

However, I also get similar errors when attempting to actually build s2n-tls:

> cmake3 --build build -j $(nproc)
/usr/bin/ld: lib/libs2n.a(s2n_crypto__libcrypto-lib-c_zstd.o): in function `zstd_stateful_expand_block':
c_zstd.c:(.text+0x9f): undefined reference to `ZSTD_decompressStream'
...

Passing the required linker flags to the s2n-tls build seems to correctly enable the feature probes and successfully build s2n-tls:

> PKG_CONFIG_LIBDIR=<openssl lib dir>/pkgconfig pkg-config --libs --static libcrypto
-L<openssl lib dir> -lcrypto -lzstd -ldl -pthread 

> cmake3 . -Bbuild -DCMAKE_PREFIX_PATH=<openssl lib dir> -DS2N_INTERN_LIBCRYPTO=ON -DCMAKE_EXE_LINKER_FLAGS="-lcrypto -lzstd -ldl -pthread"
...
-- feature S2N_LIBCRYPTO_SUPPORTS_EC_KEY_CHECK_FIPS: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_ENGINE: TRUE
-- feature S2N_LIBCRYPTO_SUPPORTS_EVP_AEAD_TLS: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_EVP_KEM: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH: TRUE
-- feature S2N_LIBCRYPTO_SUPPORTS_EVP_MD_CTX_SET_PKEY_CTX: TRUE
-- feature S2N_LIBCRYPTO_SUPPORTS_EVP_RC4: TRUE
-- feature S2N_LIBCRYPTO_SUPPORTS_FLAG_NO_CHECK_TIME: TRUE
-- feature S2N_LIBCRYPTO_SUPPORTS_HKDF: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_MLKEM: FALSE
-- feature S2N_LIBCRYPTO_SUPPORTS_RSA_PSS_SIGNING: TRUE
-- feature S2N_LIBCRYPTO_SUPPORTS_X509_STORE_LIST: TRUE
...
> cmake3 --build build -j $(nproc)
...
[100%] Built target s2n_x509_validator_time_verification_test
[100%] Linking C executable bin/s2n_x509_validator_test
[100%] Built target s2n_x509_validator_test

It seems like any necessary linker flags provided to s2n-tls via DCMAKE_EXE_LINKER_FLAGS will also be used in the feature probes. If this is the case, there might not be an issue.

[goatgoose]

@kou, does providing the necessary linker flags to s2n-tls with -DCMAKE_EXE_LINKER_FLAGS allow the feature flags to be enabled successfully in your example from #5075 (comment)?

Without specifying the linker flags in your current example, are you able to successfully build s2n-tls after configuring with cmake? It wouldn't be good if it was possible to successfully build s2n-tls in a way that allows the feature flags to be disabled due to linking issues. However, from my testing above it seems like a linking issue that causes the feature probes to fail may also result in the s2n-tls build itself failing.

[kou]

CMAKE_EXE_LINKER_FLAGS didn't work because it adds -lz ... "before" (not "after") libcrypto.a:

/bin/cc  -lz -lzstd -ldl -pthread CMakeFiles/cmTC_0a8dc.dir/S2N_LIBCRYPTO_SUPPORTS_RSA_PSS_SIGNING.c.o -o cmTC_0a8dc  /usr/lib/x86_64-linux-gnu/libcrypto.a  -ldl  -lrt

It may be better that we use find_package(OpenSSL) instead of finding libcrypto by find_path()/find_library() something like:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 23ef74369..90cac9ddb 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -450,6 +450,14 @@ if (S2N_INTERN_LIBCRYPTO)
                 bash -c "${CMAKE_AR} -r lib/libs2n.a s2n_libcrypto/*.o"
             VERBATIM
         )
+        # TODO: Get dependency recursively
+        get_target_property(link_libraries ${LINK_LIB} INTERFACE_LINK_LIBRARIES)
+        if (link_libraries STREQUAL "OpenSSL::Crypto")
+            get_target_property(link_libraries OpenSSL::Crypto INTERFACE_LINK_LIBRARIES)
+        endif()
+        foreach(link_library ${link_libraries})
+            target_link_libraries(${PROJECT_NAME} PUBLIC ${link_library})
+        endforeach()
     endif()
 else()
     # LINK_LIB is set above after checking targets. It handles the find_package craziness.
diff --git a/cmake/modules/Findcrypto.cmake b/cmake/modules/Findcrypto.cmake
index 1ac4d9a61..6028b9e2d 100644
--- a/cmake/modules/Findcrypto.cmake
+++ b/cmake/modules/Findcrypto.cmake
@@ -30,7 +30,34 @@ if (TARGET crypto OR TARGET AWS::crypto)
     message(STATUS "crypto Include Dir: ${crypto_INCLUDE_DIR}")
     set(CRYPTO_FOUND true)
     set(crypto_FOUND true)
+    return()
+endif()
+
+include(FindPackageHandleStandardArgs)
+
+if (BUILD_SHARED_LIBS OR S2N_USE_CRYPTO_SHARED_LIBS)
+    set(OPENSSL_USE_STATIC_LIBS FALSE)
 else()
+    set(OPENSSL_USE_STATIC_LIBS TRUE)
+endif()
+find_package(OpenSSL)
+if (OpenSSL_FOUND)
+    # It's better that we use OpenSSL::Crypto (CMake target) directly...
+    set(crypto_INCLUDE_DIR "$<TARGET_PROPERTY:OpenSSL::Crypto,INTERFACE_INCLUDE_DIRECTORIES>")
+    set(crypto_LIBRARY "$<TARGET_FILE:OpenSSL::Crypto>")
+    if (OPENSSL_USE_STATIC_LIBS)
+        set(crypto_STATIC_LIBRARY "${crypto_LIBRARY}")
+    else()
+        set(crypto_SHARED_LIBRARY "${crypto_LIBRARY}")
+    endif()
+    add_library(AWS::crypto INTERFACE IMPORTED)
+    target_link_libraries(AWS::crypto INTERFACE OpenSSL::Crypto)
+    find_package_handle_standard_args(crypto
+        REQUIRED_VARS crypto_LIBRARY
+        VERSION_VAR OpenSSL_VERSION)
+    return()
+endif()
+
     find_path(crypto_INCLUDE_DIR
         NAMES openssl/crypto.h
         HINTS
@@ -109,5 +136,3 @@ else()
             add_dependencies(AWS::crypto Threads::Threads)
         endif()
     endif()
-
-endif()

If we use find_package(OpenSSL), it finds dependencies such as zlib and Zstandard automatically.

[goatgoose]

Interesting ok. Thanks for the example! We'll look into this solution.

[kou]

Please let me know if I can help this project. :-)

I also have a feature probe related problem, I'll open an issue and PR when I sorted out the problem.

[goatgoose]

related issue: #4900