s2n-tls icon indicating copy to clipboard operation
s2n-tls copied to clipboard

CI: Remove libcrypto install directories from PATH before updating CMake

Open goatgoose opened this issue 2 months ago • 2 comments

Problem:

We set the CMAKE_FIND_PACKAGE_PREFER_CONFIG variable in our CMakeLists.txt, which tells CMake to prefer the config mode of find_package rather than the module mode. However, this variable didn't exist before CMake 3.15, and the ubuntu18 codebuild image used in part of the CI currently has CMake 3.10 installed:

root@ff2c05776ead:/# cmake --version
cmake version 3.10.2

So, in the ubuntu18 image, our CMake build still prefers our custom Findcrypto module over AWS-LC's crypto-config.cmake when linking to a libcrypto.

This means that s2n-tls will search for libcrypto artifacts in a series of search steps, starting with user paths such as CMAKE_PREFIX_PATH and ending in system paths. We specify the libcrypto we want to test with by defining the CMAKE_PREFIX_PATH variable, which allows our intended libcrypto to be discovered before other libcryptos, such as the system libcrypto.

After updating CMake, the CMAKE_FIND_PACKAGE_PREFER_CONFIG variable will become meaningful, and s2n-tls will begin searching for AWS-LC's crypto-config.cmake file for linking instructions rather than manually searching for libcrypto artifacts. The search for AWS-LC's config goes through a similar series of search steps, starting with user paths and ending with system paths.

One search step in this process, however, is to look for configs in the PATH environment variable:

https://cmake.org/cmake/help/latest/command/find_package.html#config-mode-search-procedure 5. Search the standard system environment variables. (...) Path entries ending in /bin or /sbin are automatically converted to their parent directories

This is a problem for us, since we add libcrypto bin directories to our PATH so the integration tests can have access to the TLS library executables (i.e. bssl, openssl s_client). So, s2n-tls will discover AWS-LC's CMake config by looking in the PATH, regardless of what libcrypto we set in the CMAKE_PREFIX_PATH, which will result in us testing with the wrong libcrypto.

Note that we have a test to ensure we're testing with the correct libcrypto, so we will catch this when we attempt to update CMake (at least for our unit tests). It just may be confusing without this context.

Solution:

The simplest solution would be to remove the libcrypto install directories from our PATH, so CMake can't discover them. Maybe by renaming them with symlinks to remove the trailing /bin that CMake removes during the search?

Another option to consider is if we should modify the libcrypto search process to restrict its search for AWS-LC's CMake config to only user paths, allowing the manual search to be attempted before broadening the config search to system paths. So, if the user provided any libcrypto with some user path like CMAKE_PREFIX_PATH, we try to link to that first.

goatgoose avatar Oct 23 '25 16:10 goatgoose

The nix devShell isn't affected by this issue... with cmake 3.30.0, aws-lc's *.cmake files are making it into the CMAKE_PREFIX_PATH as well as PATH, presumably being matched before PATH?:

[nix awslc] $ cmake --version
cmake version 3.30.5

CMake suite maintained and supported by Kitware (kitware.com/cmake).
[nix awslc] $ find /nix/store/wh9m8q8dkdj5mbkr48ialivzwb3yyc76-aws-lc/ -name \*crypto\*cmake
/nix/store/wh9m8q8dkdj5mbkr48ialivzwb3yyc76-aws-lc/lib/crypto/cmake/crypto-config.cmake
/nix/store/wh9m8q8dkdj5mbkr48ialivzwb3yyc76-aws-lc/lib/crypto/cmake/shared/crypto-targets.cmake
/nix/store/wh9m8q8dkdj5mbkr48ialivzwb3yyc76-aws-lc/lib/crypto/cmake/shared/crypto-targets-relwithdebinfo.cmake
...
[nix awslc] $ echo $NIXPKGS_CMAKE_PREFIX_PATH|grep -c 'aws-lc'
1
[nix awslc] $ echo $PATH|grep -c 'aws-lc'
1
[nix awslc] $ clean;configure|tail -35
...
-- FOUND AWS-LC CRYPTO cmake config - shared  #This is from lc's crypto-config.cmake
-- Using libcrypto from the cmake path
...

It might be helpful to compare the cmake behavior in the nix shell vs. an upgraded cmake on older Ubuntu.

dougch avatar Oct 23 '25 19:10 dougch

We could also consider having our tests set a specific variable to provide the libcrypto install directory rather than updating CMake paths and hoping it gets discovered first: https://github.com/aws/s2n-tls/issues/5577

goatgoose avatar Oct 23 '25 21:10 goatgoose