build and link s2n-tls with openssl3

[toidiu]

This PR is a part of a sequence of PRs to integrate openssl3 with s2n-tls

Description of changes:

This PR installs openssl3 and build/links s2n-tls with openssl3.

The second commit is necessary at the moment if you want to build and link locally since there are other errors introduced by openssl 3 integration. These will be handled in a future PR.

d3da874 (-DUNSAFE_TREAT_WARNINGS_AS_ERRORS=OFF ) ignores warnings as errors, which is needed until all errors are fixed.

Testing:

I am able to build locally. The following is an output showing that s2n links to openssl-3

$ ldd build/bin/s2nd
        ...
        libcrypto.so.3 => /home/ubuntu/projects/rsync/s2n-tls/test-deps/openssl-3/lib64/libcrypto.so.3 (0x00007f2d14679000)
        ...

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

[lrstewart]

Why limit this to specific tests? Don't you need it to build s2n-tls regardless of what tests are being run?

[dougch]

sort of a historical pattern; really there should be only two cases: explicitly include this library with S2N_LIBCRYPTO and ALL. Our current approach where we want to break out individual tests with specific libcryptos can be sped up by not installing everything, but hopefully the custom docker container work will move all of the install steps to container build time.

[toidiu]

I took the code bit from openssl 1.1.1 example. But if i am understanding this correctly, the default is openssl1.1.1 and for all other libcrypto we rebuild?

I am actually not sure why we gate around certain test for openssl 1.1.1 above(unit test seems to be missing).

[toidiu]

i think we can do the following for now since openssl1.1.1 seems like the default case here

if [[ "$S2N_LIBCRYPTO" == "openssl-3" && ! -d "$OPENSSL_3_INSTALL_DIR" ]]; then

[lrstewart]

sort of a historical pattern; really there should be only two cases: explicitly include this library with S2N_LIBCRYPTO and ALL. Our current approach where we want to break out individual tests with specific libcryptos can be sped up by not installing everything, but hopefully the custom docker container work will move all of the install steps to container build time.

Isn't that kind of an accident tho? It just happens that we only test multiple different libcryptos in our integration test CIs. We SHOULDN'T be running any other test in the CI with S2N_LIBCRYPTO set to something else. But if we don't download the right libcrypto if we do end up doing that, it seems like a bug. And people could use this locally too.

[lrstewart]

If nothing else, it seems like the build should fail if an unexpected libcrypto is requested. We don't just silently fall back to building with the system libcrypto or openssl-1.1.1, right?

[toidiu]

Not super familiar with the entire build process but it does seem odd and error-prone that we install openssl-1.1.1 even though its not specified.

From local testing the linker seems to do eager search and does link to the a different openssl version if the original one is not there. While this may not be an issue, it might be safer to only build an openssl version if we are testing with it.. custom docker images with only one version available sounds like the better option.

[lrstewart]

That sounds like we could definitely end up incorrectly thinking we're testing against libcryptos we're not actually testing against :\ Can you create an issue for that?

[toidiu]

#3455

[dougch]

Only callout, might be helpful to put minor version in the directory name, e.g. openssl-3.0

[goatgoose]

We may also want to add an openssl3 build preset so we can run the integration tests with it: https://github.com/aws/s2n-tls/blob/main/codebuild/bin/s2n_set_build_preset.sh

[toidiu]

Only callout, might be helpful to put minor version in the directory name, e.g. openssl-3.0

We need to decide what we will be testing 3.x, 3.0.x? (currently 3.0.5) I specifically left out the minor version because the install script can be updated to specify a new version.

I think what we want is 3.x and so i didnt want to specify a number there in preparation for when we bump versions. (3.0, 3.1.. etc)

[lrstewart]

Only callout, might be helpful to put minor version in the directory name, e.g. openssl-3.0

We need to decide what we will be testing 3.x, 3.0.x? (currently 3.0.5) I specifically left out the minor version because the install script can be updated to specify a new version.

I think what we want is 3.x and so i didnt want to specify a number there in preparation for when we bump versions. (3.0, 3.1.. etc)

What's the LTS version? I understood it was 3.0.x.

[camshaft]

I think what we want is 3.x and so i didnt want to specify a number there in preparation for when we bump versions. (3.0, 3.1.. etc)

OpenSSL has a history of making breaking changes between minor versions. Look at 1.0 vs 1.1. I think it's likely to be the case moving forward.

[lrstewart]

You only made whitespace changes to this file-- why include it?

[toidiu]

Didnt mean to specifically make any changes to that file. Codebuild was failing to resolve the correct github hash so i just rebased and push a new commit.

The change was meant to be a noop but my ide does the tab to space thing and trailing space thing.