s2n-tls icon indicating copy to clipboard operation
s2n-tls copied to clipboard
verified publisher icon aws

Cmake file issue when: User openssl + s2n cmake

Open Hadatko opened this issue 3 years ago • 3 comments

Security issue notifications

If you discover a potential security issue in s2n we ask that you notify AWS Security via our vulnerability reporting page. Please do not create a public github issue.

Problem:

A short description of what the problem is and why we need to fix it. Add reproduction steps if necessary.

Hello, user may have build aws libraries with custom libssl using cmake flag USE_OPENSSL. This is working well. But when i am linking final application s2n cmake module file is forcing aws crypto instead of using openssl crypto.

Code from aws-c-cal where is if condition for which crypto to use:

include(CMakeFindDependencyMacro)

find_dependency(aws-c-common)

if (BUILD_SHARED_LIBS)
    include(${CMAKE_CURRENT_LIST_DIR}/shared/aws-c-cal-targets.cmake)
else()
    include(${CMAKE_CURRENT_LIST_DIR}/static/aws-c-cal-targets.cmake)
endif()

if (NOT BYO_CRYPTO AND NOT WIN32 AND NOT APPLE)
    get_target_property(AWS_C_CAL_DEPS AWS::aws-c-cal INTERFACE_LINK_LIBRARIES)
    # pre-cmake 3.3 IN_LIST search approach
    list (FIND AWS_C_CAL_DEPS "OpenSSL::Crypto" _index)
    if (${_index} GREATER -1) # if USE_OPENSSL AND NOT ANDROID
        # aws-c-cal has been built with a dependency on OpenSSL::Crypto,
        # therefore consumers of this library have a dependency on OpenSSL and must have it found
        find_dependency(OpenSSL REQUIRED)
        find_dependency(Threads REQUIRED)
    else()
        list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/modules")
        find_dependency(crypto)
    endif()
endif()

Code from s2n where it is missing:

include(CMakeFindDependencyMacro)

if (NOT MSVC)
    set(THREADS_PREFER_PTHREAD_FLAG ON)
    find_package(Threads REQUIRED)
endif()

list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/modules")
find_dependency(crypto)

if (BUILD_SHARED_LIBS)
    include(${CMAKE_CURRENT_LIST_DIR}/shared/s2n-targets.cmake)
else()
    include(${CMAKE_CURRENT_LIST_DIR}/static/s2n-targets.cmake)
endif()

I changed that to this temporary which is hidding aws crypto and searching for openssl crypto:

include(CMakeFindDependencyMacro)

if (NOT MSVC)
    set(THREADS_PREFER_PTHREAD_FLAG ON)
    find_package(Threads REQUIRED)
endif()

if (NOT BYO_CRYPTO AND NOT WIN32 AND NOT APPLE)
        # aws-c-cal has been built with a dependency on OpenSSL::Crypto,
        # therefore consumers of this library have a dependency on OpenSSL and must have it found
        find_dependency(OpenSSL REQUIRED)
        find_dependency(Threads REQUIRED)
endif()

if (BUILD_SHARED_LIBS)
    include(${CMAKE_CURRENT_LIST_DIR}/shared/s2n-targets.cmake)
else()
    include(${CMAKE_CURRENT_LIST_DIR}/static/s2n-targets.cmake)
endif()

Also i had to change one value from other cmake file from INTERFACE_LINK_LIBRARIES "AWS::crypto;Threads::Threads;dl;rt;m" to INTERFACE_LINK_LIBRARIES "OpenSSL::Crypto;Threads::Threads;dl;rt;m"

Solution:

A description of the possible solution in terms of S2N architecture. Highlight and explain any potentially controversial design decisions taken. Described above.

  • Does this change what S2N sends over the wire? If yes, explain.
  • NO
  • Does this change any public APIs? If yes, explain.
  • NO
  • Which versions of TLS will this impact?
  • NO

Requirements / Acceptance Criteria:

What must a solution address in order to solve the problem? How do we know the solution is complete?

  • RFC links: Links to relevant RFC(s)
  • Related Issues: Link any relevant issues
  • Will the Usage Guide or other documentation need to be updated?
  • Testing: How will this change be tested? Call out new integration tests, functional tests, or particularly interesting/important unit tests.
    • Will this change trigger SAW changes? Changes to the state machine, the s2n_handshake_io code that controls state transitions, the DRBG, or the corking/uncorking logic could trigger SAW failures.
    • Should this change be fuzz tested? Will it handle untrusted input? Create a separate issue to track the fuzzing work.

Out of scope:

Is there anything the solution will intentionally NOT address?

Hadatko avatar Sep 15 '22 10:09 Hadatko

Actually i tried clean and build and i cannot build this library. I think you should align with https://github.com/awslabs/aws-c-cal/blob/cc43764ddef1c23c6c5ae16badecbc989e4e45c8/CMakeLists.txt and USE_OPENSSL parameter. Or am i doing wrong?

current issue:

...
- madvise() support detected
-- clone() support detected
-- Found crypto: /home/acrios_cervennka/kseftiky/aeler/atlas-pilot-fw/Firmware/containers/aws-iotsdk/awsbuild/../dependencies/openssl/libcrypto.a  
-- LibCrypto Include Dir: /home/acrios_cervennka/kseftiky/aeler/atlas-pilot-fw/Firmware/containers/aws-iotsdk/dependencies/openssl/include
-- LibCrypto Shared Lib:  crypto_SHARED_LIBRARY-NOTFOUND
-- LibCrypto Static Lib:  crypto_STATIC_LIBRARY-NOTFOUND
Using libcrypto from the cmake path.
CMake Error at /home/acrios_cervennka/kseftiky/aeler/atlas-pilot-fw/Firmware/containers/aws-iotsdk/awsbuild/CMakeFiles/CMakeTmp/CMakeLists.txt:18 (add_executable):
  Target "cmTC_726ae" links to target "AWS::crypto" but the target was not
  found.  Perhaps a find_package() call is missing for an IMPORTED target, or
  an ALIAS target is missing?


CMake Error at crt/aws-crt-cpp/crt/s2n/CMakeLists.txt:496 (try_compile):
  Failed to generate test project build system.


-- Configuring incomplete, errors occurred!
See also "/home/acrios_cervennka/kseftiky/aeler/atlas-pilot-fw/Firmware/containers/aws-iotsdk/awsbuild/CMakeFiles/CMakeOutput.log".
See also "/home/acrios_cervennka/kseftiky/aeler/atlas-pilot-fw/Firmware/containers/aws-iotsdk/awsbuild/CMakeFiles/CMakeError.log".

My command

AWS_IOT_SDK_FOLDERNAME="aws-iot-device-sdk-cpp"
AWS_IOT_SDK_URL="https://github.com/aws/aws-iot-device-sdk-cpp-v2.git"
AWS_IOT_SDK_VER="1.18.6"

rm -rf awsbuild &&
    git clone --recursive ${AWS_IOT_SDK_URL} --branch v${AWS_IOT_SDK_VER} ${AWS_IOT_SDK_FOLDERNAME} &&
    mkdir awsbuild &&
    cd awsbuild &&
    cmake -DUSE_OPENSSL=ON \
        -DCMAKE_INSTALL_PREFIX="$(pwd)/../dependencies/aws-iot-device-sdk" \
        -Dcrypto_LIBRARY="$(pwd)/../dependencies/openssl/libcrypto.a" \
        -Dcrypto_INCLUDE_DIR="$(pwd)/../dependencies/openssl/include" \
        -DOPENSSL_CRYPTO_LIBRARY="$(pwd)/../dependencies/openssl/libcrypto.a" \
        -DOPENSSL_INCLUDE_DIR="$(pwd)/../dependencies/openssl/include" \
        -DCMAKE_BUILD_TYPE="Release" ../${AWS_IOT_SDK_FOLDERNAME} &&
    cmake --build . --target install

Hadatko avatar Sep 15 '22 12:09 Hadatko

Hi @Hadatko not sure I fully understand the issue and how to repro it.

Are you building aws-c-cal (with USE_OPENSSL) and also manually building s2n-tls? Or are you only building aws-c-cal? What behavior do you expect to see?

By "aws crypto" do you mean aws-lc?

Which platform are you building on and whats the default libcrypto for that platform?

From the commands you included it seems like you are trying to build aws-iot-device-sdk-cpp; so they would be the best folks to help you decipher their build error. It might be worth reaching out to them if you havn't already.

toidiu avatar Sep 20 '22 22:09 toidiu

Hi @toidiu , i will try to explain deeper. I am building https://github.com/aws/aws-iot-device-sdk-cpp-v2 on my laptop with kubuntu OS. Here is described scenario with default value of USE_OPENSSL with https://github.com/awslabs/aws-crt-cpp/issues/407 which i think you may not be interested in.

Second scenario is build sdk with USE_OPENSSL=ON. This way i am trying to use my openssl which i build internaly instead of aws-lc. This way s2n cannot be built as it looks like its cmake files doesn't have support for it. aws-c-cal repository was shown as example how they did the support.

I made second issue here: https://github.com/awslabs/aws-crt-cpp/issues/406. But there is described another build issue i got with this repository.

Hadatko avatar Sep 21 '22 08:09 Hadatko