allows cmake to force crypto linkage.

[sbiscigl]

Description of changes:

Describe s2n’s current behavior and how your code changes that behavior. If there are no issues this PR is resolving, explain why this change is necessary.

This mirrors the AWS C++ SDK's PR that uses the same FindCrypto module. There is a user that wants to be able to control static linkage outside of the BUILD_SHARED_LIBS variable and force using a dynamic libcrypto while setting BUILD_SHARED_LIBS to OFF this will check two new variables FORCE_SHARED_CRYPTO and FORCE_STATIC_CRYPTO to force the usage of the corresponding library.

Call-outs:

Address any potentially confusing code. Is there code added that needs to be cleaned up later? Is there code that is missing because it’s still in development?

N/A

Testing:

How is this change tested (unit tests, fuzz tests, etc.)? Are there any testing steps to be verified by the reviewer?

I tested locally to verify that when using the cpp sdk that the correct crypto was used in linking.

Is this a refactor change? If so, how have you proved that the intended behavior hasn't changed?

No

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[jmayclin]

Can you document this option, similar to https://github.com/aws/aws-sdk-cpp/pull/2829/files#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aR69

[sbiscigl]

yep sounds good, can add it here, since it is your option, but we are using it, are you fine with the name AWS_CRYPTO_BUILD_SHARED_LIBS or would you like S2N_CRYPTO_BUILD_SHARED_LIBS?

from a SDK perspective we want it this way so that a user only has to specify one cmake option, but if you feel differently, im ok with that.

[jmayclin]

I'd prefer S2N_BUILD_SHARED_LIBS so that future combinations of static/dynamic linking don't require any more options to be added.

Also, thoughts on matching the current behavior? E.g.

if (BUILD_SHARED_LIBS || S2N_BUILD_SHARED_LIBS)
...

That feels conceptually simpler to me.

[sbiscigl]

yeah thats makes sense to me, updated

[jmayclin]

Discussed offline that we should use ${*} syntax: https://cmake.org/cmake/help/latest/command/if.html#variable-expansion

[goatgoose]

It seems like cmake doesn't like the syntax here:

CMake Error at /tmp/tmp.xcMT5KADjW/s2n-install-shared/lib/s2n/cmake/modules/Findcrypto.cmake:59 (if):
  if given arguments:

    "BUILD_SHARED_LIBS" "OR"

  Unknown arguments specified

Would something like this maybe work?

Suggested change

	if (BUILD_SHARED_LIBS OR ${S2N_USE_CRYPTO_SHARED_LIBS})
	if (${BUILD_SHARED_LIBS} OR ${S2N_USE_CRYPTO_SHARED_LIBS})

[sbiscigl]

what cmake version are you using? its working as expected on my machine? got a image i can use to reproduce this?

[sbiscigl]

this works on a fresh al2023 image

FROM public.ecr.aws/amazonlinux/amazonlinux:2023

# Install dev tools
RUN yum groupinstall -y "Development Tools"

# Install cmake and ninja
RUN yum install -y cmake3 ninja-build

# Install static and dynamic lc
RUN git clone --depth 1 -b fips-2022-11-02 https://github.com/aws/aws-lc && \
    cd aws-lc && \
    mkdir build && \
    cd build && \
    cmake -G Ninja -DCMAKE_INSTALL_LIBDIR=lib -DCMAKE_INSTALL_PREFIX=/lc-install .. && \
    cmake --build . && \
    cmake --install . && \
    rm -rf ./* && \
    cmake -G Ninja -DBUILD_SHARED_LIBS=ON -DCMAKE_INSTALL_LIBDIR=lib -DCMAKE_INSTALL_PREFIX=/lc-install .. && \
    cmake --build . && \
    cmake --install .

RUN git clone https://github.com/sbiscigl/s2n-tls.git # && \
    cd s2n-tls && \
    mkdir build && \
    cd build && \
    cmake -G Ninja -DCMAKE_PREFIX_PATH=/lc-install .. && \
    cmake --build .