Coder Social home page Coder Social logo

Comments (8)

goatgoose avatar goatgoose commented on June 12, 2024 1

To resolve this it might make sense for us to just update the default certificate chain to one that doesn't use SHA1.

from s2n-tls.

maddeleine avatar maddeleine commented on June 12, 2024

Thanks for the issue! That is a very interesting set of failures. I don't think we test on any RHEL in our CI 🤔 not sure how we would add that.

from s2n-tls.

dougch avatar dougch commented on June 12, 2024

While not the exact AMI you're using, we've had success with unit tests on AL2023, which is similar to FC34/35, with Openssl 3.0.x. I was unable to repro this error on FC35, using the steps provided.

One thing that might help is changing your build target to -DCMAKE_BUILD_TYPE=RelwithDebInfo or even just Debug. Another thing to consider is that the error might not be 100% accurate as it's the default in this case statement.

from s2n-tls.

wombelix avatar wombelix commented on June 12, 2024

While not the exact AMI you're using, we've had success with unit tests on AL2023, which is similar to FC34/35, with Openssl 3.0.x. I was unable to repro this error on FC35, using the steps provided.

The issue only occurs on RHEL 9, so I'm not exactly sure why you refer to F34/F35 in this context? Because F34 was the base for CentOS 9 Stream which is the base for RHEL 9?

I'm no expert in the s2n-tls code base but also saw the case statement you mentioned (https://github.com/aws/s2n-tls/blob/main/tls/s2n_x509_validator.c#L721). My first thought was, if the tests run against the system wide trusted certs, maybe the tests certs are signed by ca that's not trusted anymore or use a hash that's unsupported in current RHEL 9 releases. Or because of compliance requirements openssl is compiled with different flags, therefore more strict as on other OS versions and fails.

I can test the changed build flag and report the results back.
Any other suggestions how it can be troubleshooted? The default case statement hides it away a bit.

from s2n-tls.

wombelix avatar wombelix commented on June 12, 2024

I did a build with -DCMAKE_BUILD_TYPE=RelwithDebInfo rhel9_relwithdebinfo_builder-live.log.gz and -DCMAKE_BUILD_TYPE=Debug rhel9_debug_builder-live.log.gz on RHEL9. Both times the same tests failed again with the untrusted certification issues.

I had another idea but it's also not the problem or at least not the part of s2n-tls I thought is responsible.
But let me share it so someone with more s2n-tls knowledge can take a look too:

Reflecting on the fact that it is a problem that right now only occurs on RHEL9, the feature flag S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH was drawing my attention:

+ /usr/bin/cmake -S . -B redhat-linux-build -DCMAKE_C_FLAGS_RELEASE:STRING=-DNDEBUG -DCMAKE_CXX_FLAGS_RELEASE:STRING=-DNDEBUG -DCMAKE_Fortran_FLAGS_RELEASE:STRING=-DNDEBUG -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON -DCMAKE_INSTALL_DO_STRIP:BOOL=OFF -DCMAKE_INSTALL_PREFIX:PATH=/usr -DINCLUDE_INSTALL_DIR:PATH=/usr/include -DLIB_INSTALL_DIR:PATH=/usr/lib64 -DSYSCONF_INSTALL_DIR:PATH=/etc -DSHARE_INSTALL_PREFIX:PATH=/usr/share -DLIB_SUFFIX=64 -DBUILD_SHARED_LIBS:BOOL=ON -GNinja -DBUILD_SHARED_LIBS=ON -DCMAKE_BUILD_TYPE=RelwithDebInfo
-- The C compiler identification is GNU 11.4.1
[...]

-- feature S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH: TRUE

[...]

Red Hat has set SHA1 on path to deprecation and started with it in RHEL 9:
https://lists.fedoraproject.org/archives/list/[email protected]/thread/VVLHQAWI3IQ7NRLKMUHJ27JV3V2JAFDP/
https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/9/html/considerations_in_adopting_rhel_9/assembly_security_considerations-in-adopting-rhel-9#ref_considerations-security-crypto_changes-to-security

In Fedora the patch to disable SHA1 in the openssl package still allows SHA1 by default:
https://src.fedoraproject.org/rpms/openssl/blob/rawhide/f/0049-Allow-disabling-of-SHA1-signatures.patch#_131

But in CentOS 9 Stream and therefore RHEL 9 the patch is changed to disable SHA1 by default:
https://gitlab.com/redhat/centos-stream/rpms/openssl/-/blob/c9s/0049-Selectively-disallow-SHA1-signatures.patch?ref_type=heads#L131

This patch leaves EVP_md5_sha1 in openssl/evp.h of the openssl-devel package untouched. My understanding is that therefore https://github.com/aws/s2n-tls/blob/main/tests/features/S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH.c in combination with #3921 and #4074 makes the false assumption that openSSL provides support for SHA1 and enables the feature.

To test my theory I removed the feature by applying this patch:

From 046f65355835d0a186efc71a15b892fd2730f773 Mon Sep 17 00:00:00 2001
From: Dominik Wombacher <[email protected]>
Date: Fri, 17 May 2024 09:07:54 +0000
Subject: [PATCH] fix: Disable feature
 'S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH'

---
 ...S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH.c | 20 -------------------
 ...LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH.flags |  0
 2 files changed, 20 deletions(-)
 delete mode 100644 tests/features/S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH.c
 delete mode 100644 tests/features/S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH.flags

diff --git a/tests/features/S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH.c b/tests/features/S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH.c
deleted file mode 100644
index baa928e1..00000000
--- a/tests/features/S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH.c
+++ /dev/null
@@ -1,20 +0,0 @@
-/*
- * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
- *
- * Licensed under the Apache License, Version 2.0 (the "License").
- * You may not use this file except in compliance with the License.
- * A copy of the License is located at
- *
- *  http://aws.amazon.com/apache2.0
- *
- * or in the "license" file accompanying this file. This file is distributed
- * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
- * express or implied. See the License for the specific language governing
- * permissions and limitations under the License.
- */
-
-#include <openssl/evp.h>
-int main() {
-    EVP_md5_sha1();
-    return 0;
-}
diff --git a/tests/features/S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH.flags b/tests/features/S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH.flags
deleted file mode 100644
index e69de29b..00000000
-- 
2.45.0

But this didn't change anything, the same tests failing and claiming the cert is untrusted:

The following tests FAILED:
	 17 - s2n_cert_validation_callback_test (Failed)
	 55 - s2n_config_test (Failed)
	 63 - s2n_crl_test (Failed)
	130 - s2n_mutual_auth_test (Failed)
	262 - s2n_x509_validator_test (Failed)
	263 - s2n_x509_validator_time_verification_test (Failed)
Errors while running CTest

So maybe it is related to the SHA1 deprecation and I was looking at the wrong part of the code.
Or the failed tests have a different root cause and not related to the SHA1 deprecation that was started in RHEL 9.
I can't tell for sure, too little C programming skills.

from s2n-tls.

maddeleine avatar maddeleine commented on June 12, 2024

Using the build type RelwithDebInfo will build s2n-tls with debug symbols. This allows you to use gdb with s2n-tls and pause the tests in a specific location. In particular, it would be interesting to see what the actual value of ossl_error is here in one of the failing tests, because it's not matching one of our expected cases.

The note about SHA1 in RHEL 9 is intriguing. However, the feature flag you mentioned isn't actually used to turn on/off SHA1 support, which is why removing it does nothing.

from s2n-tls.

maddeleine avatar maddeleine commented on June 12, 2024

Your guess about the issue being a SHA1 signature algorithm is a good one. The failing tests use a certificate which uses sha1WithRSAEncryption as its signature algorithm.

This diff changes the s2n_config_test to use a certificate with a sha256 signature algorithm instead of one with the sha1 signature algorithm. If it passes on RHEL 9, then that further supports the theory that its a sha1 problem.

diff --git a/tests/unit/s2n_config_test.c b/tests/unit/s2n_config_test.c
index d4fe8d7d9..05f461dd2 100644
--- a/tests/unit/s2n_config_test.c
+++ b/tests/unit/s2n_config_test.c
@@ -910,7 +910,7 @@ int main(int argc, char **argv)
             DEFER_CLEANUP(struct s2n_cert_chain_and_key *chain_and_key = NULL,
                     s2n_cert_chain_and_key_ptr_free);
             EXPECT_SUCCESS(s2n_test_cert_chain_and_key_new(&chain_and_key,
-                    S2N_DEFAULT_TEST_CERT_CHAIN, S2N_DEFAULT_TEST_PRIVATE_KEY));
+                    S2N_RSA_2048_PKCS8_CERT_CHAIN, S2N_RSA_2048_PKCS8_KEY));
 
             /* Ensure a handshake succeeds with a minimal server config and no mutual auth */
             {
@@ -927,13 +927,13 @@ int main(int argc, char **argv)
                 DEFER_CLEANUP(struct s2n_config *client_config = s2n_config_new_minimal(), s2n_config_ptr_free);
                 EXPECT_NOT_NULL(client_config);
                 EXPECT_SUCCESS(s2n_config_set_cipher_preferences(client_config, "default"));
-                EXPECT_SUCCESS(s2n_config_set_verification_ca_location(client_config, S2N_DEFAULT_TEST_CERT_CHAIN, NULL));
+                EXPECT_SUCCESS(s2n_config_set_verification_ca_location(client_config, S2N_RSA_2048_PKCS8_CERT_CHAIN, NULL));
 
                 DEFER_CLEANUP(struct s2n_connection *client_conn = s2n_connection_new(S2N_CLIENT),
                         s2n_connection_ptr_free);
                 EXPECT_NOT_NULL(client_conn);
                 EXPECT_SUCCESS(s2n_connection_set_config(client_conn, client_config));
-                EXPECT_SUCCESS(s2n_set_server_name(client_conn, "s2nTestServer"));
+                EXPECT_SUCCESS(s2n_set_server_name(client_conn, "FakeRoot"));
 
                 struct s2n_test_io_pair io_pair = { 0 };
                 EXPECT_SUCCESS(s2n_io_pair_init_non_blocking(&io_pair));

from s2n-tls.

wombelix avatar wombelix commented on June 12, 2024

Your guess about the issue being a SHA1 signature algorithm is a good one. The failing tests use a certificate which uses sha1WithRSAEncryption as its signature algorithm.

This diff changes the s2n_config_test to use a certificate with a sha256 signature algorithm instead of one with the sha1 signature algorithm. If it passes on RHEL 9, then that further supports the theory that its a sha1 problem.

Thanks @maddeleine, I applied the patch and did a new build.
Now we have five instead of six tests failing, which seems like the expected result because 55 - s2n_config_test passed this time.

So I would say we getting closer and it seems indeed relation to the SHA1 deprecation.

Still failing:

	 17 - s2n_cert_validation_callback_test (Failed)
	 63 - s2n_crl_test (Failed)
	130 - s2n_mutual_auth_test (Failed)
	262 - s2n_x509_validator_test (Failed)
	263 - s2n_x509_validator_time_verification_test (Failed)

Details

 16/263 Test  #17: s2n_cert_validation_callback_test ................***Failed    0.06 sec
Running /builddir/build/BUILD/s2n-tls-1.4.14/tests/unit/s2n_cert_validation_callback_test.c ... NOTE: Some details are omitted, run with S2N_PRINT_STACKTRACE=1 for a verbose backtrace.
See https://github.com/aws/s2n-tls/blob/main/docs/usage-guide
FAILED test 152
s2n_result_is_ok(s2n_x509_validator_validate_cert_chain(&validator, conn, chain_data, chain_len, &pkey_type, &public_key_out)) is not true  (/builddir/build/BUILD/s2n-tls-1.4.14/tests/unit/s2n_cert_validation_callback_test.c:239)
Error Message: 'Certificate is untrusted'
 Debug String: 'Error encountered in /builddir/build/BUILD/s2n-tls-1.4.14/tls/s2n_x509_validator.c:721'
 System Error: Success (0)

 62/263 Test  #63: s2n_crl_test .....................................***Failed    0.55 sec
Running /builddir/build/BUILD/s2n-tls-1.4.14/tests/unit/s2n_crl_test.c ... NOTE: Some details are omitted, run with S2N_PRINT_STACKTRACE=1 for a verbose backtrace.
See https://github.com/aws/s2n-tls/blob/main/docs/usage-guide
FAILED test 341
!(((s2n_negotiate_test_server_and_client(server_conn, client_conn))) == (-1)) is not true  (/builddir/build/BUILD/s2n-tls-1.4.14/tests/unit/s2n_crl_test.c:782)
Error Message: 'Certificate is untrusted'
 Debug String: 'Error encountered in /builddir/build/BUILD/s2n-tls-1.4.14/tls/s2n_x509_validator.c:721'
 System Error: Success (0)

152/263 Test #130: s2n_mutual_auth_test .............................***Failed   24.54 sec
Running /builddir/build/BUILD/s2n-tls-1.4.14/tests/unit/s2n_mutual_auth_test.c ... NOTE: Some details are omitted, run with S2N_PRINT_STACKTRACE=1 for a verbose backtrace.
See https://github.com/aws/s2n-tls/blob/main/docs/usage-guide
FAILED test 28
!(((s2n_negotiate_test_server_and_client(server_conn, client_conn))) == (-1)) is not true  (/builddir/build/BUILD/s2n-tls-1.4.14/tests/unit/s2n_mutual_auth_test.c:123)
Error Message: 'Certificate is untrusted'
 Debug String: 'Error encountered in /builddir/build/BUILD/s2n-tls-1.4.14/tls/s2n_x509_validator.c:721'
 System Error: Input/output error (5)

262/263 Test #262: s2n_x509_validator_test ..........................***Failed    0.04 sec
Running /builddir/build/BUILD/s2n-tls-1.4.14/tests/unit/s2n_x509_validator_test.c ... NOTE: Some details are omitted, run with S2N_PRINT_STACKTRACE=1 for a verbose backtrace.
See https://github.com/aws/s2n-tls/blob/main/docs/usage-guide
FAILED test 50
s2n_result_is_ok(s2n_x509_validator_validate_cert_chain(&validator, connection, chain_data, chain_len, &pkey_type, &public_key_out)) is not true  (/builddir/build/BUILD/s2n-tls-1.4.14/tests/unit/s2n_x509_validator_test.c:334)
Error Message: 'Certificate is untrusted'
 Debug String: 'Error encountered in /builddir/build/BUILD/s2n-tls-1.4.14/tls/s2n_x509_validator.c:721'
 System Error: Success (0)

263/263 Test #263: s2n_x509_validator_time_verification_test ........***Failed   26.26 sec
Running /builddir/build/BUILD/s2n-tls-1.4.14/tests/unit/s2n_x509_validator_time_verification_test.c ... NOTE: Some details are omitted, run with S2N_PRINT_STACKTRACE=1 for a verbose backtrace.
See https://github.com/aws/s2n-tls/blob/main/docs/usage-guide
FAILED test 144
(s2n_errno) == (expected_error) is not true  (/builddir/build/BUILD/s2n-tls-1.4.14/tests/unit/s2n_x509_validator_time_verification_test.c:230)
Error Message: 'Certificate is untrusted'
 Debug String: 'Error encountered in /builddir/build/BUILD/s2n-tls-1.4.14/tls/s2n_x509_validator.c:721'
 System Error: Success (0)

from s2n-tls.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.