From b3cace9fbc524661fdf39331aeac7cdc34533d21 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Tue, 28 Sep 2021 15:20:41 -0400 Subject: [PATCH 01/10] set default on X509_STORE_CTX Signed-off-by: Dan Zhang --- source/extensions/transport_sockets/tls/context_impl.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/source/extensions/transport_sockets/tls/context_impl.cc b/source/extensions/transport_sockets/tls/context_impl.cc index f8870d4dddc88..e94a2a1d70ffc 100644 --- a/source/extensions/transport_sockets/tls/context_impl.cc +++ b/source/extensions/transport_sockets/tls/context_impl.cc @@ -1167,10 +1167,15 @@ bool ContextImpl::verifyCertChain(X509& leaf_cert, STACK_OF(X509) & intermediate // cert validation config. const SSL_CTX* ssl_ctx = tls_contexts_[0].ssl_ctx_.get(); X509_STORE* store = SSL_CTX_get_cert_store(ssl_ctx); - if (!X509_STORE_CTX_init(ctx.get(), store, &leaf_cert, &intermediates)) { + if (!X509_STORE_CTX_init(ctx.get(), store, &leaf_cert, &intermediates) { error_details = "Failed to verify certificate chain: X509_STORE_CTX_init"; return false; } + if (!X509_STORE_CTX_set_default(ctx.get(), + "ssl_client")) { + error_details = "Failed to verify certificate chain: X509_STORE_CTX_set_default"; + return false; + } int res = cert_validator_->doVerifyCertChain(ctx.get(), nullptr, leaf_cert, nullptr); // If |SSL_VERIFY_NONE|, the error is non-fatal, but we keep the error details. From e634d47764639d1ad61ced32e384b16d537ededf Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Wed, 29 Sep 2021 00:22:04 -0400 Subject: [PATCH 02/10] added test Signed-off-by: Dan Zhang --- .../transport_sockets/tls/context_impl.cc | 7 +- .../quic/envoy_quic_proof_verifier_test.cc | 71 ++++++++++++++++++- tools/spelling/spelling_dictionary.txt | 1 + 3 files changed, 75 insertions(+), 4 deletions(-) diff --git a/source/extensions/transport_sockets/tls/context_impl.cc b/source/extensions/transport_sockets/tls/context_impl.cc index e94a2a1d70ffc..fee1767de6073 100644 --- a/source/extensions/transport_sockets/tls/context_impl.cc +++ b/source/extensions/transport_sockets/tls/context_impl.cc @@ -1167,12 +1167,13 @@ bool ContextImpl::verifyCertChain(X509& leaf_cert, STACK_OF(X509) & intermediate // cert validation config. const SSL_CTX* ssl_ctx = tls_contexts_[0].ssl_ctx_.get(); X509_STORE* store = SSL_CTX_get_cert_store(ssl_ctx); - if (!X509_STORE_CTX_init(ctx.get(), store, &leaf_cert, &intermediates) { + if (!X509_STORE_CTX_init(ctx.get(), store, &leaf_cert, &intermediates)) { error_details = "Failed to verify certificate chain: X509_STORE_CTX_init"; return false; } - if (!X509_STORE_CTX_set_default(ctx.get(), - "ssl_client")) { + // Currently only EnvoyQuicProofVerifier, which is used by the client code, calls this method. So + // hard-code "ssl_server" for now. + if (!X509_STORE_CTX_set_default(ctx.get(), "ssl_server")) { error_details = "Failed to verify certificate chain: X509_STORE_CTX_set_default"; return false; } diff --git a/test/common/quic/envoy_quic_proof_verifier_test.cc b/test/common/quic/envoy_quic_proof_verifier_test.cc index 21b7be3136a7e..6311e657d4ac3 100644 --- a/test/common/quic/envoy_quic_proof_verifier_test.cc +++ b/test/common/quic/envoy_quic_proof_verifier_test.cc @@ -78,7 +78,7 @@ class EnvoyQuicProofVerifierTest : public testing::Test { const std::string empty_string_; const std::vector empty_string_list_; const std::string cert_chain_{quic::test::kTestCertificateChainPem}; - const std::string root_ca_cert_; + std::string root_ca_cert_; const std::string leaf_cert_; const absl::optional custom_validator_config_{ absl::nullopt}; @@ -192,5 +192,74 @@ VdGXMAjeXhnOnPvmDi5hUz/uvI+Pg6cNmUoCRwSCnK/DazhA EXPECT_EQ("Invalid leaf cert, only P-256 ECDSA certificates are supported", error_details); } +TEST_F(EnvoyQuicProofVerifierTest, VerifyCertChainFailureNonServerAuthEKU) { + root_ca_cert_ = R"(-----BEGIN CERTIFICATE----- +MIID3TCCAsWgAwIBAgIUdCu/mLip3X/We37vh3BA9u/nxakwDQYJKoZIhvcNAQEL +BQAwdjELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcM +DVNhbiBGcmFuY2lzY28xDTALBgNVBAoMBEx5ZnQxGTAXBgNVBAsMEEx5ZnQgRW5n +aW5lZXJpbmcxEDAOBgNVBAMMB1Rlc3QgQ0EwHhcNMjAwODA1MTkxNjAwWhcNMjIw +ODA1MTkxNjAwWjB2MQswCQYDVQQGEwJVUzETMBEGA1UECAwKQ2FsaWZvcm5pYTEW +MBQGA1UEBwwNU2FuIEZyYW5jaXNjbzENMAsGA1UECgwETHlmdDEZMBcGA1UECwwQ +THlmdCBFbmdpbmVlcmluZzEQMA4GA1UEAwwHVGVzdCBDQTCCASIwDQYJKoZIhvcN +AQEBBQADggEPADCCAQoCggEBALu2Ihi4DmaQG7zySZlWyM9SjxOXCI5840V7Hn0C +XoiI8sQQmKSC2YCzsaphQoJ0lXCi6Y47o5FkooYyLeNDQTGS0nh+IWm5RCyochtO +fnaKPv/hYxhpyFQEwkJkbF1Zt1s6j2rq5MzmbWZx090uXZEE82DNZ9QJaMPu6VWt +iwGoGoS5HF5HNlUVxLNUsklNH0ZfDafR7/LC2ty1vO1c6EJ6yCGiyJZZ7Ilbz27Q +HPAUd8CcDNKCHZDoMWkLSLN3Nj1MvPVZ5HDsHiNHXthP+zV8FQtloAuZ8Srsmlyg +rJREkc7gF3f6HrH5ShNhsRFFc53NUjDbYZuha1u4hiOE8lcCAwEAAaNjMGEwDwYD +VR0TAQH/BAUwAwEB/zAOBgNVHQ8BAf8EBAMCAQYwHQYDVR0OBBYEFJZL2ixTtL6V +xpNz4qekny4NchiHMB8GA1UdIwQYMBaAFJZL2ixTtL6VxpNz4qekny4NchiHMA0G +CSqGSIb3DQEBCwUAA4IBAQAcgG+AaCdrUFEVJDn9UsO7zqzQ3c1VOp+WAtAU8OQK +Oc4vJYVVKpDs8OZFxmukCeqm1gz2zDeH7TfgCs5UnLtkplx1YO1bd9qvserJVHiD +LAK+Yl24ZEbrHPaq0zI1RLchqYUOGWmi51pcXi1gsfc8DQ3GqIXoai6kYJeV3jFJ +jxpQSR32nx6oNN/6kVKlgmBjlWrOy7JyDXGim6Z97TzmS6Clctewmw/5gZ9g+M8e +g0ZdFbFkNUjzSNm44hiDX8nR6yJRn+gLaARaJvp1dnT+MlvofZuER17WYKH4OyMs +ie3qKR3an4KC20CtFbpZfv540BVuTTOCtQ5xqZ/LTE78 +-----END CERTIFICATE-----)"; + configCertVerificationDetails(true); + const std::string ocsp_response; + const std::string cert_sct; + std::string error_details; + // This is a cert same as test/config/integration/certs/servercert.pem but with extKeyUsage: + // clientAuth. + const std::string certs{R"(-----BEGIN CERTIFICATE----- +MIIEYjCCA0qgAwIBAgIUWzmfQSTX8xfzUzdByjCjCJN8E/wwDQYJKoZIhvcNAQEL +BQAwdjELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcM +DVNhbiBGcmFuY2lzY28xDTALBgNVBAoMBEx5ZnQxGTAXBgNVBAsMEEx5ZnQgRW5n +aW5lZXJpbmcxEDAOBgNVBAMMB1Rlc3QgQ0EwHhcNMjEwOTI5MTY0NTM3WhcNMjMw +OTI5MTY0NTM3WjCBpjELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWEx +FjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDTALBgNVBAoMBEx5ZnQxGTAXBgNVBAsM +EEx5ZnQgRW5naW5lZXJpbmcxGjAYBgNVBAMMEVRlc3QgQmFja2VuZCBUZWFtMSQw +IgYJKoZIhvcNAQkBFhViYWNrZW5kLXRlYW1AbHlmdC5jb20wggEiMA0GCSqGSIb3 +DQEBAQUAA4IBDwAwggEKAoIBAQC9JgaI7hxjPM0tsUna/QmivBdKbCrLnLW9Teak +RH/Ebg68ovyvrRIlybDT6XhKi+iVpzVY9kqxhGHgrFDgGLBakVMiYJ5EjIgHfoo4 +UUAHwIYbunJluYCgANzpprBsvTC/yFYDVMqUrjvwHsoYYVm36io994k9+t813b70 +o0l7/PraBsKkz8NcY2V2mrd/yHn/0HAhv3hl6iiJme9yURuDYQrae2ACSrQtsbel +KwdZ/Re71Z1awz0OQmAjMa2HuCop+Q/1QLnqBekT5+DH1qKUzJ3Jkq6NRkERXOpi +87j04rtCBteCogrO67qnuBZ2lH3jYEMb+lQdLkyNMLltBSdLAgMBAAGjgbYwgbMw +DAYDVR0TAQH/BAIwADALBgNVHQ8EBAMCBeAwEwYDVR0lBAwwCgYIKwYBBQUHAwIw +QQYDVR0RBDowOIYec3BpZmZlOi8vbHlmdC5jb20vYmFja2VuZC10ZWFtgghseWZ0 +LmNvbYIMd3d3Lmx5ZnQuY29tMB0GA1UdDgQWBBTZdxNltzTEpl+A1UpK8BsxkkIG +hjAfBgNVHSMEGDAWgBSWS9osU7S+lcaTc+KnpJ8uDXIYhzANBgkqhkiG9w0BAQsF +AAOCAQEAhiXkQJZ53L3uoQMX6xNhAFThomirnLm2RT10kPIbr5mmf3wcR8+EKrWX +dWCj56bk1tSDbQZqx33DSGbhvNaydggbo69Pkie5b7J9O7AWzT21NME6Jis9hHED +VUI63L+7SgJ2oZs0o8xccUaLFeknuNdQL4qUEwhMwCC8kYLz+c6g0qwDwZi1MtdL +YR4qm2S6KveVPGzBHpUjfWf/whSCM3JN5Fm8gWfC6d6XEYz6z1dZrj3lpwmhRgF6 +Wb72f68jzCQ3BFqKRFsJI2xz3EP6PoQ+e6EQjMpjQLomxIhIN/aTsgrKwA5wf6vQ +ZCFbredVxDBZuoVsfrKPSQa407Jj1Q== +-----END CERTIFICATE-----)"}; + std::stringstream pem_stream(certs); + std::vector chain = quic::CertificateView::LoadPemFromStream(&pem_stream); + std::unique_ptr cert_view = + quic::CertificateView::ParseSingleCertificate(chain[0]); + ASSERT(cert_view); + EXPECT_EQ(quic::QUIC_FAILURE, + verifier_->VerifyCertChain("lyft.com", 54321, chain, ocsp_response, cert_sct, nullptr, + &error_details, nullptr, nullptr, nullptr)); + EXPECT_EQ("X509_verify_cert: certificate verification error at depth 0: unsupported certificate " + "purpose", + error_details); +} + } // namespace Quic } // namespace Envoy diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index f9d05beace12e..9f8393280a73e 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -1083,6 +1083,7 @@ sendto serializable serializer serv +servercert setenv setsockopt sig From 691701b02da811c905e1febb8ba5f621b9673353 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Thu, 30 Sep 2021 14:43:29 -0400 Subject: [PATCH 03/10] comment Signed-off-by: Dan Zhang --- test/common/quic/envoy_quic_proof_verifier_test.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/common/quic/envoy_quic_proof_verifier_test.cc b/test/common/quic/envoy_quic_proof_verifier_test.cc index 6311e657d4ac3..2052ed3d50754 100644 --- a/test/common/quic/envoy_quic_proof_verifier_test.cc +++ b/test/common/quic/envoy_quic_proof_verifier_test.cc @@ -193,6 +193,7 @@ VdGXMAjeXhnOnPvmDi5hUz/uvI+Pg6cNmUoCRwSCnK/DazhA } TEST_F(EnvoyQuicProofVerifierTest, VerifyCertChainFailureNonServerAuthEKU) { + // Override the CA cert with test/config/integration/certs/ca.pem root_ca_cert_ = R"(-----BEGIN CERTIFICATE----- MIID3TCCAsWgAwIBAgIUdCu/mLip3X/We37vh3BA9u/nxakwDQYJKoZIhvcNAQEL BQAwdjELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcM @@ -216,7 +217,7 @@ jxpQSR32nx6oNN/6kVKlgmBjlWrOy7JyDXGim6Z97TzmS6Clctewmw/5gZ9g+M8e g0ZdFbFkNUjzSNm44hiDX8nR6yJRn+gLaARaJvp1dnT+MlvofZuER17WYKH4OyMs ie3qKR3an4KC20CtFbpZfv540BVuTTOCtQ5xqZ/LTE78 -----END CERTIFICATE-----)"; - configCertVerificationDetails(true); + configCertVerificationDetails(false); const std::string ocsp_response; const std::string cert_sct; std::string error_details; From 50cda562436d46c6fd8d2f50c33e1a3baa5c04d5 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Thu, 30 Sep 2021 18:55:37 -0400 Subject: [PATCH 04/10] comments Signed-off-by: Dan Zhang --- source/extensions/transport_sockets/tls/context_impl.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source/extensions/transport_sockets/tls/context_impl.cc b/source/extensions/transport_sockets/tls/context_impl.cc index fee1767de6073..7625ba7b8a709 100644 --- a/source/extensions/transport_sockets/tls/context_impl.cc +++ b/source/extensions/transport_sockets/tls/context_impl.cc @@ -1171,8 +1171,7 @@ bool ContextImpl::verifyCertChain(X509& leaf_cert, STACK_OF(X509) & intermediate error_details = "Failed to verify certificate chain: X509_STORE_CTX_init"; return false; } - // Currently only EnvoyQuicProofVerifier, which is used by the client code, calls this method. So - // hard-code "ssl_server" for now. + // Currently this method is only used to verify server certs, so hard-code "ssl_server" for now. if (!X509_STORE_CTX_set_default(ctx.get(), "ssl_server")) { error_details = "Failed to verify certificate chain: X509_STORE_CTX_set_default"; return false; From 6f575ea8c7b1b9ab0fabf549bd8dbd3c72861249 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Fri, 1 Oct 2021 11:51:33 -0400 Subject: [PATCH 05/10] comment Signed-off-by: Dan Zhang --- test/common/quic/envoy_quic_proof_verifier_test.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/common/quic/envoy_quic_proof_verifier_test.cc b/test/common/quic/envoy_quic_proof_verifier_test.cc index 2052ed3d50754..7af92fb11e308 100644 --- a/test/common/quic/envoy_quic_proof_verifier_test.cc +++ b/test/common/quic/envoy_quic_proof_verifier_test.cc @@ -193,7 +193,7 @@ VdGXMAjeXhnOnPvmDi5hUz/uvI+Pg6cNmUoCRwSCnK/DazhA } TEST_F(EnvoyQuicProofVerifierTest, VerifyCertChainFailureNonServerAuthEKU) { - // Override the CA cert with test/config/integration/certs/ca.pem + // Override the CA cert with cert copied from test/config/integration/certs/cacert.pem. root_ca_cert_ = R"(-----BEGIN CERTIFICATE----- MIID3TCCAsWgAwIBAgIUdCu/mLip3X/We37vh3BA9u/nxakwDQYJKoZIhvcNAQEL BQAwdjELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcM @@ -221,8 +221,9 @@ ie3qKR3an4KC20CtFbpZfv540BVuTTOCtQ5xqZ/LTE78 const std::string ocsp_response; const std::string cert_sct; std::string error_details; - // This is a cert same as test/config/integration/certs/servercert.pem but with extKeyUsage: - // clientAuth. + // This is a cert generated with the test/config/integration/certs/certs.sh. And the config that + // used to generate this cert is same as test/config/integration/certs/servercert.cfg but with + // 'extKeyUsage: clientAuth'. const std::string certs{R"(-----BEGIN CERTIFICATE----- MIIEYjCCA0qgAwIBAgIUWzmfQSTX8xfzUzdByjCjCJN8E/wwDQYJKoZIhvcNAQEL BQAwdjELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcM From bf4fb3b2a1e0d0d07c7a856b7d24ccfa60ec45d6 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Fri, 1 Oct 2021 13:12:41 -0400 Subject: [PATCH 06/10] spelling Signed-off-by: Dan Zhang --- tools/spelling/spelling_dictionary.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index 9f8393280a73e..dd99ceb836a2e 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -481,6 +481,7 @@ bursty bytecode bytestream bytestring +cacert cacheable cacheability callee @@ -497,6 +498,7 @@ canonicalizer canonicalizing cardinality casted +cfg charset checkin checksum From 455c88d31a0270a8f2c64971f33172ab97061a34 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Mon, 4 Oct 2021 16:11:47 -0400 Subject: [PATCH 07/10] add test for coverage Signed-off-by: Dan Zhang --- test/common/quic/envoy_quic_proof_verifier_test.cc | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/common/quic/envoy_quic_proof_verifier_test.cc b/test/common/quic/envoy_quic_proof_verifier_test.cc index 7af92fb11e308..304488c49a664 100644 --- a/test/common/quic/envoy_quic_proof_verifier_test.cc +++ b/test/common/quic/envoy_quic_proof_verifier_test.cc @@ -7,6 +7,7 @@ #include "test/mocks/ssl/mocks.h" #include "test/mocks/stats/mocks.h" #include "test/test_common/test_time.h" +#include "test/test_common/utility.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -119,6 +120,12 @@ TEST_F(EnvoyQuicProofVerifierTest, VerifyCertChainFailureFromSsl) { error_details); } +TEST_F(EnvoyQuicProofVerifierTest, VerifyCertChainFailureInvalidCA) { + root_ca_cert_ = "invalid root CA"; + EXPECT_THROW_WITH_REGEX(configCertVerificationDetails(true), EnvoyException, + "Failed to load trusted CA certificates from"); +} + TEST_F(EnvoyQuicProofVerifierTest, VerifyCertChainFailureInvalidLeafCert) { configCertVerificationDetails(true); const std::string ocsp_response; @@ -217,7 +224,7 @@ jxpQSR32nx6oNN/6kVKlgmBjlWrOy7JyDXGim6Z97TzmS6Clctewmw/5gZ9g+M8e g0ZdFbFkNUjzSNm44hiDX8nR6yJRn+gLaARaJvp1dnT+MlvofZuER17WYKH4OyMs ie3qKR3an4KC20CtFbpZfv540BVuTTOCtQ5xqZ/LTE78 -----END CERTIFICATE-----)"; - configCertVerificationDetails(false); + configCertVerificationDetails(true); const std::string ocsp_response; const std::string cert_sct; std::string error_details; From c2961453bbbc5395b95b970c5f6c982d88acf45c Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Tue, 5 Oct 2021 14:48:13 -0400 Subject: [PATCH 08/10] test coverage Signed-off-by: Dan Zhang --- source/extensions/transport_sockets/tls/ssl_socket.cc | 2 +- source/extensions/transport_sockets/tls/ssl_socket.h | 3 --- test/extensions/transport_sockets/tls/ssl_socket_test.cc | 5 ++++- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/source/extensions/transport_sockets/tls/ssl_socket.cc b/source/extensions/transport_sockets/tls/ssl_socket.cc index c5056786ee9a2..31ed7fd52f012 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.cc +++ b/source/extensions/transport_sockets/tls/ssl_socket.cc @@ -164,7 +164,7 @@ Network::IoResult SslSocket::doRead(Buffer::Instance& read_buffer) { } void SslSocket::onPrivateKeyMethodComplete() { - ASSERT(isThreadSafe()); + ASSERT(callbacks_ != nullptr && callbacks_->connection().dispatcher().isThreadSafe()); ASSERT(info_->state() == Ssl::SocketState::HandshakeInProgress); // Resume handshake. diff --git a/source/extensions/transport_sockets/tls/ssl_socket.h b/source/extensions/transport_sockets/tls/ssl_socket.h index 186bebbabc067..a6502cc9548ce 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.h +++ b/source/extensions/transport_sockets/tls/ssl_socket.h @@ -86,9 +86,6 @@ class SslSocket : public Network::TransportSocket, void drainErrorQueue(); void shutdownSsl(); void shutdownBasic(); - bool isThreadSafe() const { - return callbacks_ != nullptr && callbacks_->connection().dispatcher().isThreadSafe(); - } const Network::TransportSocketOptionsConstSharedPtr transport_socket_options_; Network::TransportSocketCallbacks* callbacks_{}; diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index 24b2e1422c7ee..44b57be0834ec 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -658,6 +658,7 @@ void testUtilV2(const TestUtilOptionsV2& options) { ServerSslSocketFactory server_ssl_socket_factory(std::move(server_cfg), manager, server_stats_store, server_names); + EXPECT_FALSE(server_ssl_socket_factory.usesProxyProtocolOptions()); Event::DispatcherPtr dispatcher(server_api->allocateDispatcher("test_thread")); auto socket = std::make_shared( @@ -702,8 +703,10 @@ void testUtilV2(const TestUtilOptionsV2& options) { ? options.transportSocketOptions()->serverNameOverride().value() : options.clientCtxProto().sni(); socket->setRequestedServerName(sni); + Network::TransportSocketPtr transport_socket = server_ssl_socket_factory.createTransportSocket(nullptr); + EXPECT_FALSE(transport_socket->startSecureTransport()); server_connection = dispatcher->createServerConnection( - std::move(socket), server_ssl_socket_factory.createTransportSocket(nullptr), + std::move(socket), std::move(transport_socket), stream_info); server_connection->addConnectionCallbacks(server_connection_callbacks); })); From 9e791bdba3a70576dc3513d266ce31598dd12793 Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Tue, 5 Oct 2021 14:56:36 -0400 Subject: [PATCH 09/10] format Signed-off-by: Dan Zhang --- test/extensions/transport_sockets/tls/ssl_socket_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index 44b57be0834ec..c85ae51bfac66 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -703,11 +703,11 @@ void testUtilV2(const TestUtilOptionsV2& options) { ? options.transportSocketOptions()->serverNameOverride().value() : options.clientCtxProto().sni(); socket->setRequestedServerName(sni); - Network::TransportSocketPtr transport_socket = server_ssl_socket_factory.createTransportSocket(nullptr); + Network::TransportSocketPtr transport_socket = + server_ssl_socket_factory.createTransportSocket(nullptr); EXPECT_FALSE(transport_socket->startSecureTransport()); server_connection = dispatcher->createServerConnection( - std::move(socket), std::move(transport_socket), - stream_info); + std::move(socket), std::move(transport_socket), stream_info); server_connection->addConnectionCallbacks(server_connection_callbacks); })); From 707302927ce5d31bdf50cdcfb9eafd98e12a12fb Mon Sep 17 00:00:00 2001 From: Dan Zhang Date: Thu, 7 Oct 2021 17:26:07 -0400 Subject: [PATCH 10/10] X509_VERIFY_PARAM_set1 Signed-off-by: Dan Zhang --- source/extensions/transport_sockets/tls/context_impl.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/source/extensions/transport_sockets/tls/context_impl.cc b/source/extensions/transport_sockets/tls/context_impl.cc index e1f23c7e214b1..0afc83d5a67ae 100644 --- a/source/extensions/transport_sockets/tls/context_impl.cc +++ b/source/extensions/transport_sockets/tls/context_impl.cc @@ -1172,8 +1172,11 @@ bool ContextImpl::verifyCertChain(X509& leaf_cert, STACK_OF(X509) & intermediate return false; } // Currently this method is only used to verify server certs, so hard-code "ssl_server" for now. - if (!X509_STORE_CTX_set_default(ctx.get(), "ssl_server")) { - error_details = "Failed to verify certificate chain: X509_STORE_CTX_set_default"; + if (!X509_STORE_CTX_set_default(ctx.get(), "ssl_server") || + !X509_VERIFY_PARAM_set1(X509_STORE_CTX_get0_param(ctx.get()), + SSL_CTX_get0_param(const_cast(ssl_ctx)))) { + error_details = + "Failed to verify certificate chain: fail to setup X509_STORE_CTX or its param."; return false; }