Build failure when linked against static OpenSSL libcrypto with SSL/TLS compression

antecrescent commented 12 months ago

Problem/Question:

Do you support OpenSSL built with support for SSL/TLS compression? Your install scripts for OpenSSL build the library without it (no-zlib) but there is no mention of it in the build documentation.

Currently, the build fails when the test binaries are linked against a static OpenSSL libcrypto with support for SSL/TLS compression:

1. Install a static OpenSSL libcrypto with support for SSL/TLS compression.
2.  cmake . -Bbuild \
    -DCMAKE_BUILD_TYPE=Release \
    -DCMAKE_INSTALL_PREFIX=./s2n-tls-install \
    -DBUILD_TESTING=ON \
    -DBUILD_SHARED_LIBS=OFF
3. cmake --build build -j $(nproc)

FAILED: bin/s2n_3des_test
: && /usr/bin/x86_64-pc-linux-gnu-gcc -march=native -O2 -pipe -Wl,-O1 -Wl,--as-needed   -rdynamic CMakeFiles/s2n_3des_test.dir/tests/unit/s2n_3des_test.c.o -o bin/s2n_3des_test  lib/libtestss2n.a  lib/libs2n.a  /usr/lib64/libcrypto.a  -ldl  -lrt  -lm && :
/usr/lib/gcc/x86_64-pc-linux-gnu/12/../../../../x86_64-pc-linux-gnu/bin/ld: /usr/lib64/libcrypto.a(libcrypto-lib-c_zlib.o): in function `zlib_stateful_expand_block':
(.text+0x49): undefined reference to `inflate'
...

Solution:

Account for the dependency on zlib, possibly like so:

diff --git a/cmake/modules/Findcrypto.cmake b/cmake/modules/Findcrypto.cmake
index dac74350..21cf71f4 100644
--- a/cmake/modules/Findcrypto.cmake
+++ b/cmake/modules/Findcrypto.cmake
@@ -107,6 +107,14 @@ else()
                     IMPORTED_LINK_INTERFACE_LANGUAGES "C"
                     IMPORTED_LOCATION "${crypto_LIBRARY}")
             add_dependencies(AWS::crypto Threads::Threads)
+       if (crypto_LIBRARY STREQUAL crypto_STATIC_LIBRARY)
+           set(OPENSSL_USE_STATIC_LIBS TRUE)
+           find_package(OpenSSL QUIET)
+           if (OpenSSL_FOUND AND ZLIB_FOUND)
+               set_target_properties(AWS::crypto PROPERTIES
+                   INTERFACE_LINK_LIBRARIES ZLIB::ZLIB)
+           endif()
+       endif()
         endif()
     endif()

Requirements / Acceptance Criteria:

What must a solution address in order to solve the problem? Either

mention in the build documentation that OpenSSL with support for TLS/SSL compression is not supported or
add zlib to AWS::crypto's dependencies when it imports the static OpenSSL libcrypto with support for SSL/TLS compression.

Out of scope:

The solution does not address the possibility that zlib may be used in the project but not be a dependency of OpenSSL.

camshaft commented 12 months ago

I'm not sure what the ideal solution is here. Unconditionally linking zlib if static libs are used doesn't seem like the right thing to do. Note that this problem doesn't only apply to zlib, but any library that openssl might optionally depend on (zstd, brotli, libxml, etc).

Ideally openssl would indicate what libraries it needs and would link those itself. Maybe we need to switch to use autotools instead?

antecrescent commented 12 months ago

FindOpenSSL.cmake exposes the found OpenSSL crypto library and its dependencies via the OPENSSL_CRYPTO_LIBRARIES variable. Since cmake >= v3.26.0 it detects libdl, libz and the thread library on Linux systems using pkg-config. Other optional dependencies would still be an issue..

I've only ever used autotools as an end user. Unfortunately, I cannot give any input on whether a switch would be beneficial, sorry.

aws / s2n-tls