Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cmake looks for static version of OpenSSL when building AWS static #1926

Closed
HFTrader opened this issue May 10, 2022 · 9 comments
Closed

Cmake looks for static version of OpenSSL when building AWS static #1926

HFTrader opened this issue May 10, 2022 · 9 comments
Labels
bug This issue is a bug.

Comments

@HFTrader
Copy link
Contributor

Describe the bug

The build fails on CentOS 7/8 with crypto_LIBRARY not found if -DBUILD_SHARED_LIBS=OFF is passed.

Expected Behavior

The build should use the dynamic version of the OpenSSL library if the static version is not available.

Current Behavior

When defining -DBUILD_SHARED_LIBS=OFF the build searches for a respective static version of OpenSSL which is not available in some distributions like Redhat/CentOS.

Reproduction Steps

On Redhat 7/8, checkout AWS as usual then issue

cd aws-sdk-cpp
mkdir build && cd build
cmake -DBUILD_SHARED_LIBS=OFF -DBUILD_ONLY=s3 ..

it should fail with

CMake Error at /usr/local/share/cmake-3.22/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find crypto (missing: crypto_LIBRARY)

Possible Solution

A simple solution is just to replace the use of the crypto static library with the shared one if the static is not found.

diff -Naur aws-sdk-cpp/cmake/Findcrypto.cmake aws-sdk-cpp.new/cmake/Findcrypto.cmake
--- aws-sdk-cpp/cmake/Findcrypto.cmake  2022-05-09 19:38:30.000000000 -0500
+++ aws-sdk-cpp.new/cmake/Findcrypto.cmake      2022-05-10 11:02:48.982319574 -0500
@@ -59,7 +59,11 @@
         if (BUILD_SHARED_LIBS)
             set(crypto_LIBRARY ${crypto_SHARED_LIBRARY})
         else()
-            set(crypto_LIBRARY ${crypto_STATIC_LIBRARY})
+            if (crypto_STATIC_LIBRARY)
+               set(crypto_LIBRARY ${crypto_STATIC_LIBRARY})
+            else()
+               set(crypto_LIBRARY ${crypto_SHARED_LIBRARY})
+            endif()
         endif()
     endif()

Additional Information/Context

I am not totally acquainted on why the reason to pick up the static openssl library version when building static but it does sound out of place to me.

AWS CPP SDK version used

1.9.253

Compiler and Version used

gcc (GCC) 9.2.1 20191120 (Red Hat 9.2.1-2)

Operating System and version

CentOS-8 from docker

@HFTrader HFTrader added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels May 10, 2022
HFTrader added a commit to HFTrader/aws-sdk-cpp that referenced this issue May 10, 2022
@SergeyRyabinin
Copy link
Contributor

Hi Henrique,
Thank you a lot for reporting this issue.

The short answer is "It's complicated"... We have a dependency on openssl because we have a dependency on curl inside the SDK, at the same time, we have a dependency on AWS CRT package which itself has a dependency on aws-lc and s2n that have a dependency on aws-lc or openssl aka libcrypto.

We could definitely improve our documentation about BUILD_SHARED_LIBS, it sounds ambiguous (if just a shared version of SDK is requested and/or shared/static linking with dependencies is requested).
We are also working on improving our dependency model and will improve crypto module lookup too.

In the meantime, CentOS/RedHat yum-based distros have a package openssl-static that should be available in the repo and provide you a static version of openssl. I hope this could be a solution for you for now.

Best regards,
Sergey

@HFTrader
Copy link
Contributor Author

Sergey, openssl-static is available only on Centos-7, not Centos-8.
I found some posts with 2-year old requests to add it to Centos-8 but that's the last.

@HFTrader
Copy link
Contributor Author

HFTrader commented May 10, 2022

I'm using this patch with S3 on Redhat 7/8 and Ubuntu 18.04/20.04/21.04/22.04 and it all compiles, builds and runs great both the AWS lib and the application, no issues.

@kkarbowiak
Copy link
Contributor

I think, as already mentioned, the static/shared library issue is indeed complicated.

On my box (running current Manjaro) I have no static version of libcrypto library, but the SDK itself builds fine with both BUILD_SHARED_LIBS=ON and BUILD_SHARED_LIBS=OFF. Alas, in both cases the application that uses the SDK fails to build due to static version of libcrypto missing.

I'm wondering whether this might be a side effect of some other dependencies (e.g. aws-lambda-cpp which I also use). However, when I build my project without using the SDK, all builds fine.

@HFTrader
Copy link
Contributor Author

Care to share a screenshot of the first lines after the cmake command?
You should see something like this:
image

@SergeyRyabinin
Copy link
Contributor

Henrique, thanks a lot for your insights and PR and confirmation that it works for you.
We have run our internal checks on the PR and going to merge it with a next release, thank you!

@SergeyRyabinin
Copy link
Contributor

Krzysiek,

Alas, in both cases the application that uses the SDK fails to build due to static version of libcrypto missing.

It sounds like there is somewhere an undeclared interface dependency in the SDK/CRT. We have a test that builds a small test app to catch this type of issues but it seems it did not work this time.
If possible, could you please run SDK cmake configuration with a dependency graph generation, such as

<your regular cmake command to configure CPP SDK> --graphviz=sdk_deps.dot
dot -Tpng sdk_deps.dot -o sdk_deps.png  # render dot graph file to a png, or just upload .dot file if you don't have graphviz/dot available

@jmklix jmklix removed the needs-triage This issue or PR still needs to be triaged. label May 16, 2022
@SergeyRyabinin
Copy link
Contributor

The original issue is solved by the submitter's PR.

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug.
Projects
None yet
Development

No branches or pull requests

4 participants