From 4b0eec34ecf3817aaf9e4a6d3c7e065f140d8bf8 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Mon, 20 Jan 2025 14:44:06 -0500 Subject: [PATCH 1/3] librustls: add doc header to CMakeLists.txt At least one person has convinced themselves CMake is the required/preferred way to build librustls despite the README documenting the opposite. Let's try to be even more overt about it. --- librustls/CMakeLists.txt | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/librustls/CMakeLists.txt b/librustls/CMakeLists.txt index 00f8e639..6384bf01 100644 --- a/librustls/CMakeLists.txt +++ b/librustls/CMakeLists.txt @@ -1,3 +1,15 @@ +# !!!!!! Important !!!!!! +# +# CMake is only used for building the **C client/server examples** and for other misc. +# developer tasks. +# +# If you want to build/install librustls, use `cargo capi install` instead. +# See the README[0] for more information. +# +# [0]: https://github.com/rustls/rustls-ffi?tab=readme-ov-file#build-rustls-ffi +# +# !!!!!! Important !!!!!! + cmake_minimum_required(VERSION 3.15) project(rustls-ffi) From 95999ff34c4aaef6fd6e3813daf63fba782b5f55 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Mon, 20 Jan 2025 15:18:04 -0500 Subject: [PATCH 2/3] rm CI dep on rustls-ffi-test for artifact test Previously the artifacts.yaml CI job relied on an external repo that had a simple C/CMake application depending on librustls. When verifying artifacts we cloned this repo and tested a build against the artifact. This commit reworks the main repo's test application code/build to support this use-case. The main tweak required is to support _not_ building librustls from src, but instead finding it with pkg-config. We gate this behind a new `FORCE_SYSTEM_RUSTLS` option. The artifact test CI is updated to use this option. Annoyingly we have to patch the .pc prefix to an absolute dir to get this working nicely, which means handling Windows specially. Similarly, for yet to be determined reasons, on Win32 we have to add some extra target_link_libraries that don't propagate automatically through the pc config. For this we just shift the existing Win32 specific target_link_libraries invocation up and use it for both pkg-config librustls and build-it-ourselves librustls. --- .github/workflows/artifacts.yaml | 66 ++++++++++++-------- librustls/cmake/options.cmake | 7 +++ librustls/cmake/rust.cmake | 56 ++++++++++------- librustls/tests/CMakeLists.txt | 103 ++++++++++++++++++------------- 4 files changed, 143 insertions(+), 89 deletions(-) diff --git a/.github/workflows/artifacts.yaml b/.github/workflows/artifacts.yaml index 23228e11..b99864e4 100644 --- a/.github/workflows/artifacts.yaml +++ b/.github/workflows/artifacts.yaml @@ -163,10 +163,10 @@ jobs: - os: macos-13 artifact: rustls-ffi-x86_64-macos steps: - - name: Checkout rustls-ffi-test sources + - name: Checkout sources uses: actions/checkout@v4 with: - repository: 'cpu/rustls-ffi-test' + persist-credentials: false - name: Download rustls-ffi artifact uses: actions/download-artifact@v4 with: @@ -176,19 +176,29 @@ jobs: # the correct location that we extracted the archive. This seems more reliable # than using `--define-prefix` - it seems to tack an extra 'lib/' subcomponent # onto the include path that breaks the build. - - name: Fix pkg-config prefix - # We use bash shell explicitly to avoid PowerShell on Windows and to ensure we have 'sed'. + - name: Fix pkg-config prefix (UNIX) + if: matrix.os != 'windows-latest' shell: bash # For further fun, sed isn't consistent between macOS and Linux. run: | case "${{ runner.os }}" in "macOS") - sed -i '' "s|prefix=.*|prefix=${{ matrix.artifact }}|" ${{ matrix.artifact }}/lib/pkgconfig/rustls.pc + sed -i '' "s|prefix=.*|prefix=$(pwd)/${{ matrix.artifact }}|" ${{ matrix.artifact }}/lib/pkgconfig/rustls.pc ;; *) - sed -i "s|prefix=.*|prefix=${{ matrix.artifact }}|" ${{ matrix.artifact }}/lib/pkgconfig/rustls.pc + sed -i "s|prefix=.*|prefix=$(pwd)/${{ matrix.artifact }}|" ${{ matrix.artifact }}/lib/pkgconfig/rustls.pc ;; esac + - name: Fix pkg-config prefix (Windows) + if: matrix.os == 'windows-latest' + shell: pwsh + run: | + $prefix = (Get-Location).Path + "/${{ matrix.artifact }}" + $prefix = $prefix -replace '\\', '/' + + $content = Get-Content "${{ matrix.artifact }}\lib\pkgconfig\rustls.pc" + $content = $content -replace "prefix=.*", "prefix=$prefix" + Set-Content "${{ matrix.artifact }}\lib\pkgconfig\rustls.pc" $content # Dump out what pkg-config says about the rustls package. - name: Debug pkg-config run: | @@ -202,7 +212,7 @@ jobs: if: matrix.os != 'windows-latest' env: PKG_CONFIG_PATH: ${{ matrix.artifact }}/lib/pkgconfig - run: cmake -S . -B build -DCMAKE_BUILD_TYPE=Release + run: cmake -S librustls -B build -DCMAKE_BUILD_TYPE=Release -DFORCE_SYSTEM_RUSTLS=ON # Set up the cmake build, overriding PKG_CONFIG_PATH to # point to the extracted rustls-ffi archive. # @@ -212,26 +222,30 @@ jobs: if: matrix.os == 'windows-latest' env: PKG_CONFIG_PATH: ${{ matrix.artifact }}/lib/pkgconfig - run: cmake -DPKG_CONFIG_EXECUTABLE=C:\Strawberry\perl\bin\pkg-config.bat -S . -B build - # Build the rustls-ffi-test binary. - - name: Build rustls-ffi-test (UNIX) + run: cmake -DPKG_CONFIG_EXECUTABLE=C:\Strawberry\perl\bin\pkg-config.bat -DFORCE_SYSTEM_RUSTLS=ON -S librustls -B build + # Build the client and server binaries + - name: Build rustls-ffi client/server (UNIX) if: matrix.os != 'windows-latest' run: cmake --build build -v - # Build the rustls-ffi-test binary. + # Build the client and server binaries # On Windows we need to specify a configuration to avoid a warning about using the default # debug MSCRT runtime with a lib built with the release MSCRT runtime. - - name: Build rustls-ffi-test (Windows) + - name: Build rustls-ffi client/server (Windows) if: matrix.os == 'windows-latest' run: cmake --build build --config Release -v - # Run the rustls-ffi-test binary. - - name: Run rustls-ffi-test (UNIX) + # Run the rustls-ffi client binary. + - name: Run rustls-ffi client (UNIX) if: matrix.os != 'windows-latest' - run: ./build/rustls-ffi-test + env: + RUSTLS_PLATFORM_VERIFIER: 1 + run: ./build/tests/client example.com 443 / 1 # Run the rustls-ffi-test binary. # On Windows it's in a different output location under build. - - name: Run rustls-ffi-test (Windows) + - name: Run rustls-ffi client (Windows) if: matrix.os == 'windows-latest' - run: ./build/Release/rustls-ffi-test.exe + env: + RUSTLS_PLATFORM_VERIFIER: 1 + run: .\build\tests\Release\client.exe example.com 443 / 1 test-deb: name: "Test Linux Deb (${{ matrix.os }})" @@ -241,10 +255,10 @@ jobs: matrix: os: [ ubuntu-latest, ubuntu-20.04 ] steps: - - name: Checkout rustls-ffi-test sources + - name: Checkout sources uses: actions/checkout@v4 with: - repository: 'cpu/rustls-ffi-test' + persist-credentials: false - name: Download rustls-ffi deb artifact uses: actions/download-artifact@v4 with: @@ -258,10 +272,12 @@ jobs: pkg-config --libs rustls # Set up the cmake build, no pkg-config ENV overrides needed. - name: Setup cmake build - run: cmake -S . -B build -DCMAKE_BUILD_TYPE=Release - # Build the rustls-ffi-test binary. - - name: Build rustls-ffi-test + run: cmake -S librustls -B build -DCMAKE_BUILD_TYPE=Release -DFORCE_SYSTEM_RUSTLS=ON + # Build the client and server binaries + - name: Build rustls-ffi client/server run: cmake --build build -v - # Run the rustls-ffi-test binary. - - name: Run rustls-ffi-test - run: ./build/rustls-ffi-test + # Run the rustls-ffi client binary. + - name: Run rustls-ffi client + env: + RUSTLS_PLATFORM_VERIFIER: 1 + run: ./build/tests/client example.com 443 / 1 diff --git a/librustls/cmake/options.cmake b/librustls/cmake/options.cmake index c021fa62..56c1852b 100644 --- a/librustls/cmake/options.cmake +++ b/librustls/cmake/options.cmake @@ -67,3 +67,10 @@ if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES) FORCE ) endif() + +# Useful for testing the client/server examples with a pre-built rustls-ffi. +option( + FORCE_SYSTEM_RUSTLS + "Require system-installed rustls-ffi, never build" + OFF +) diff --git a/librustls/cmake/rust.cmake b/librustls/cmake/rust.cmake index 2f2a08d2..52149020 100644 --- a/librustls/cmake/rust.cmake +++ b/librustls/cmake/rust.cmake @@ -1,27 +1,41 @@ include(ExternalProject) set_directory_properties(PROPERTIES EP_PREFIX ${CMAKE_BINARY_DIR}/rust) -ExternalProject_Add( - rustls-ffi - DOWNLOAD_COMMAND "" - CONFIGURE_COMMAND "" - BUILD_COMMAND - cargo capi build --locked ${CARGO_FEATURES} - "$,--release,-->" - # Rely on cargo checking timestamps, rather than tell CMake where every - # output is. - BUILD_ALWAYS true - INSTALL_COMMAND - cargo capi install --libdir=lib --prefix=${CMAKE_BINARY_DIR}/rust - --locked ${CARGO_FEATURES} "$,--release,--debug>" - # Run cargo test with --quiet because msbuild will treat the presence - # of "error" in stdout as an error, and we have some test functions that - # end in "_error". Quiet mode suppresses test names, so this is a - # sufficient workaround. - TEST_COMMAND - cargo test --locked ${CARGO_FEATURES} - "$,--release,-->" --quiet -) +if(FORCE_SYSTEM_RUSTLS) + find_package(PkgConfig REQUIRED) + pkg_check_modules(RUSTLS_FFI REQUIRED rustls) + + if(NOT RUSTLS_FFI_FOUND) + message(FATAL_ERROR "System rustls-ffi required but not found") + endif() + + message(STATUS "RUSTLS_FFI_INCLUDE_DIRS: ${RUSTLS_FFI_INCLUDE_DIRS}") + message(STATUS "RUSTLS_FFI_LIBRARY_DIRS: ${RUSTLS_FFI_LIBRARY_DIRS}") + message(STATUS "RUSTLS_FFI_LIBRARIES: ${RUSTLS_FFI_LIBRARIES}") +else() + ExternalProject_Add( + rustls-ffi + DOWNLOAD_COMMAND "" + CONFIGURE_COMMAND "" + BUILD_COMMAND + cargo capi build --locked ${CARGO_FEATURES} + "$,--release,-->" + # Rely on cargo checking timestamps, rather than tell CMake where every + # output is. + BUILD_ALWAYS true + INSTALL_COMMAND + cargo capi install --libdir=lib --prefix=${CMAKE_BINARY_DIR}/rust + --locked ${CARGO_FEATURES} + "$,--release,--debug>" + # Run cargo test with --quiet because msbuild will treat the presence + # of "error" in stdout as an error, and we have some test functions that + # end in "_error". Quiet mode suppresses test names, so this is a + # sufficient workaround. + TEST_COMMAND + cargo test --locked ${CARGO_FEATURES} + "$,--release,-->" --quiet + ) +endif() add_custom_target( cbindgen diff --git a/librustls/tests/CMakeLists.txt b/librustls/tests/CMakeLists.txt index f22e1a39..38b8206b 100644 --- a/librustls/tests/CMakeLists.txt +++ b/librustls/tests/CMakeLists.txt @@ -46,62 +46,79 @@ endif() function(test_binary target_name) add_executable(${target_name}) target_sources(${target_name} PRIVATE ${target_name}.c common.c common.h) - add_dependencies(${target_name} rustls-ffi) - - target_include_directories( - ${target_name} - PRIVATE ${CMAKE_BINARY_DIR}/rust/include - ) if(WIN32) - target_compile_options(${target_name} PRIVATE ${sanitizer_flags}) target_link_libraries( ${target_name} - "${CMAKE_BINARY_DIR}/rust/lib/rustls.${lib_extension}" + PRIVATE + advapi32.lib + bcrypt.lib + crypt32.lib + cryptnet.lib + kernel32.lib + ncrypt.lib + bcrypt.lib + advapi32.lib + legacy_stdio_definitions.lib + kernel32.lib + advapi32.lib + kernel32.lib + ntdll.lib + userenv.lib + ws2_32.lib + synchronization.lib + kernel32.lib + ws2_32.lib + kernel32.lib + msvcrt.lib ) - target_link_libraries( + endif() + + if(RUSTLS_FFI_FOUND) + target_include_directories( ${target_name} - advapi32.lib - bcrypt.lib - crypt32.lib - cryptnet.lib - kernel32.lib - ncrypt.lib - bcrypt.lib - advapi32.lib - legacy_stdio_definitions.lib - kernel32.lib - advapi32.lib - kernel32.lib - ntdll.lib - userenv.lib - ws2_32.lib - synchronization.lib - kernel32.lib - ws2_32.lib - kernel32.lib - msvcrt.lib + PRIVATE ${RUSTLS_FFI_INCLUDE_DIRS} ) - set_property( - TARGET ${target_name} - PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreadedDLL" + target_link_directories( + ${target_name} + PRIVATE ${RUSTLS_FFI_LIBRARY_DIRS} ) - elseif(UNIX) - target_compile_options(${target_name} PRIVATE ${sanitizer_flags}) - target_link_options(${target_name} PRIVATE ${sanitizer_flags}) - target_link_libraries( + target_link_libraries(${target_name} PRIVATE ${RUSTLS_FFI_LIBRARIES}) + else() + add_dependencies(${target_name} rustls-ffi) + + target_include_directories( ${target_name} - "${CMAKE_BINARY_DIR}/rust/lib/librustls.${lib_extension}" + PRIVATE ${CMAKE_BINARY_DIR}/rust/include ) - if(CERT_COMPRESSION) - target_link_libraries(${target_name} m) - endif() - if(APPLE) + + if(WIN32) + target_compile_options(${target_name} PRIVATE ${sanitizer_flags}) + target_link_libraries( + ${target_name} + PRIVATE "${CMAKE_BINARY_DIR}/rust/lib/rustls.${lib_extension}" + ) + set_property( + TARGET ${target_name} + PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreadedDLL" + ) + elseif(UNIX) + target_compile_options(${target_name} PRIVATE ${sanitizer_flags}) + target_link_options(${target_name} PRIVATE ${sanitizer_flags}) target_link_libraries( ${target_name} - "-framework Foundation" - "-framework Security" + "${CMAKE_BINARY_DIR}/rust/lib/librustls.${lib_extension}" ) + if(CERT_COMPRESSION) + target_link_libraries(${target_name} m) + endif() + if(APPLE) + target_link_libraries( + ${target_name} + "-framework Foundation" + "-framework Security" + ) + endif() endif() endif() endfunction() From 1bdd83e9e52a4a2945468e62f51eff08eefb2adb Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Mon, 20 Jan 2025 17:53:50 -0500 Subject: [PATCH 3/3] abort on CMake install w/ clear error This updates the librustls CMakeLists.txt to abort on install. It's only intended to be used for the C examples, which we _don't_ want anyone to install anywhere! Use cargo capi for librustls, not cmake. ``` $ cmake --install build -- Install configuration: "Release" CMake Error at build/cmake_install.cmake:46 (message): librustls installation via CMake is not supported. Use 'cargo capi install' instead. See: https://github.com/rustls/rustls-ffi?tab=readme-ov-file#build-rustls-ffi ``` --- librustls/CMakeLists.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/librustls/CMakeLists.txt b/librustls/CMakeLists.txt index 6384bf01..eaa84806 100644 --- a/librustls/CMakeLists.txt +++ b/librustls/CMakeLists.txt @@ -14,6 +14,13 @@ cmake_minimum_required(VERSION 3.15) project(rustls-ffi) +install( + CODE + "message(FATAL_ERROR + \"librustls installation via CMake is not supported. Use 'cargo capi install' instead.\n\" + \"See: https://github.com/rustls/rustls-ffi?tab=readme-ov-file#build-rustls-ffi\")" +) + # Use `cmake -LH $BUILD_DIR` to see all options/help. # Use `cmake --build $BUILD_DIR --target help` to see all targets.