Cmake find_package crypto picking aws-lc over boringssl

dougch commented 2 months ago

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:

We had a CodeBuild job failure on the boringssl unit test, where cmake was trying to use aws-lc as the libcrypto.

(Incomplete) repro steps: using an Ubuntu22 Codebuild image, run the test similar to CI with:

 BUILD_S2N=true S2N_LIBCRYPTO=boringssl GCC_VERSION=9 TESTS=unit ./codebuild/bin/s2n_codebuild.sh

The Cmake banner shows we're not picking up boringssl, and the build fails:

running with libcrypto: boringssl, gcc_version: 9
-- The C compiler identification is GNU 9.5.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/gcc-9 - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detected CMAKE_SYSTEM_PROCESSOR as x86_64
-- Detected 64-Bit system
-- Looking for pthread.h
-- Looking for pthread.h - found
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE  
-- FOUND AWS-LC CRYPTO cmake config - shared
-- Using libcrypto from the cmake path
...
In function ‘s2n_kyber_evp_decapsulate’:
/codebuild/output/src1580115204/src/git-codecommit.us-west-2.amazonaws.com/v1/repos/s2n_replica/crypto/s2n_kyber_evp.c:83:30: error: cast discards ‘const’ qualifier from pointer target type [-Werror=cast-qual]
   83 |                              (uint8_t *) ciphertext, kem->ciphertext_length),
      |                              ^

Solution:

Remove aws-lc or rework the way cmake's path search is configured.

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?

dougch commented 2 months ago

worth noting Ubuntu22 is using Cmake3.22, and there have been changes to how Cmake looks for modules https://cmake.org/cmake/help/latest/command/find_package.html#search-procedure

dougch commented 1 month ago

Confirmed. In our CI image, both boringssl and awslc are available in /usr/local and running cmake with --trace turned on shows it picking awslc, because of the awslc/lib/crypto/cmake/crypto-config.cmake file(s). Removing the *cmake files from the awslc install causes cmake to fall back to the PREFIX_PATH and builds against boringssl. So finessing the cmake search paths is one solution, the other is to just remove the awslc configs.

aws / s2n-tls