diff --git a/source/extensions/transport_sockets/tls/context_impl.cc b/source/extensions/transport_sockets/tls/context_impl.cc index 38ff08d9f51b1..0afc83d5a67ae 100644 --- a/source/extensions/transport_sockets/tls/context_impl.cc +++ b/source/extensions/transport_sockets/tls/context_impl.cc @@ -1171,6 +1171,14 @@ bool ContextImpl::verifyCertChain(X509& leaf_cert, STACK_OF(X509) & intermediate error_details = "Failed to verify certificate chain: X509_STORE_CTX_init"; 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") || + !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; + } 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. 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/common/quic/envoy_quic_proof_verifier_test.cc b/test/common/quic/envoy_quic_proof_verifier_test.cc index 21b7be3136a7e..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" @@ -78,7 +79,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}; @@ -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; @@ -192,5 +199,76 @@ VdGXMAjeXhnOnPvmDi5hUz/uvI+Pg6cNmUoCRwSCnK/DazhA EXPECT_EQ("Invalid leaf cert, only P-256 ECDSA certificates are supported", error_details); } +TEST_F(EnvoyQuicProofVerifierTest, VerifyCertChainFailureNonServerAuthEKU) { + // 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 +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 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 +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/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index 24b2e1422c7ee..c85ae51bfac66 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,9 +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); + EXPECT_FALSE(transport_socket->startSecureTransport()); server_connection = dispatcher->createServerConnection( - std::move(socket), server_ssl_socket_factory.createTransportSocket(nullptr), - stream_info); + std::move(socket), std::move(transport_socket), stream_info); server_connection->addConnectionCallbacks(server_connection_callbacks); })); diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index f89846c06b988..377c190894944 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 @@ -1084,6 +1086,7 @@ sendto serializable serializer serv +servercert setenv setsockopt sig