Skip to content

THRIFT-5706: lib/cpp Fix the Security tests on openssl 1.1 and 3.0#2940

Merged
emmenlau merged 2 commits intoapache:masterfrom
thomasbruggink:fix-lib-cpp-security-tests
Aug 7, 2024
Merged

THRIFT-5706: lib/cpp Fix the Security tests on openssl 1.1 and 3.0#2940
emmenlau merged 2 commits intoapache:masterfrom
thomasbruggink:fix-lib-cpp-security-tests

Conversation

@thomasbruggink
Copy link
Copy Markdown
Contributor

This PR fixes the Security tests to build on a clean install of ubuntu
20.04 and ubuntu 22.04 without modifications to the systems openssl
configuration.

* Enable TLS 1.0 and TLS 1.1 on OpenSSL 1.1 with the seclevel=0 flag
* Disable TLS 1.0 and TLS 1.1 on OpenSSL 3.0

While its technically possible to enable it on OpenSSL 3 I think because
of all the issues with these old TLS versions dropping support for it is
better.

This PR builds forth on the work done here: #2811

Tested with the ubuntu 20.04 (OpenSSL 1.1) and 22.04 (OpenSSL 3.0) docker containers.
All lib/cpp tests succeed in both.

This PR fixes the Security tests to build on a clean install of ubuntu
20.04 and ubuntu 22.04 without modifications to the systems openssl
configuration.

 * Enable TLS 1.0 and TLS 1.1 on OpenSSL 1.1 with the seclevel=0 flag
 * Disable TLS 1.0 and TLS 1.1 on OpenSSL 3.0

While its technically possible to enable it on OpenSSL 3 I think because
of all the issues with these old TLS versions dropping support for it is
better.

This PR builds forth on the work done here: apache#2811

Tested with the ubuntu 20.04 (OpenSSL 1.1) and 22.04 (OpenSSL 3.0) docker containers.
All lib/cpp tests succeed in both.
@thomasbruggink thomasbruggink force-pushed the fix-lib-cpp-security-tests branch from 8269092 to 8148f2f Compare February 26, 2024 13:32
@Jens-G Jens-G added the c++ label Mar 4, 2024
@Jens-G Jens-G requested a review from emmenlau May 11, 2024 10:41
@thomasbruggink
Copy link
Copy Markdown
Contributor Author

@emmenlau any update on the review of this?
cpp test suite is still broken without this patch on modern systems.

@emmenlau
Copy link
Copy Markdown
Member

emmenlau commented Aug 7, 2024

Sorry for the long delay! Looks good to me, lets bring this in!

Copy link
Copy Markdown
Member

@emmenlau emmenlau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sufficiently deep in the details of OpenSSL to know why these changes are correct. But they look thorough and to quite some extent self-explanatory. Also, I understand that tests have increased rather than decreased by the changes (see the changed check of openssl1 vs openssl3 at the end. Therefore: Looks good to me.

@emmenlau emmenlau merged commit a4ebb75 into apache:master Aug 7, 2024
afuaide pushed a commit to afuaide/thrift that referenced this pull request Aug 13, 2024
…ty-tests

THRIFT-5706: lib/cpp Fix the Security tests on openssl 1.1 and 3.0
afuaide pushed a commit to afuaide/thrift that referenced this pull request Aug 13, 2024
…ty-tests

THRIFT-5706: lib/cpp Fix the Security tests on openssl 1.1 and 3.0
@thomasbruggink thomasbruggink deleted the fix-lib-cpp-security-tests branch September 26, 2024 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants