diff --git a/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc b/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc index 06fa51331293..0585939c4173 100644 --- a/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc +++ b/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc @@ -215,8 +215,15 @@ int DefaultCertValidator::doVerifyCertChain( } } - return allow_untrusted_certificate_ ? 1 - : (validated != Envoy::Ssl::ClientValidationStatus::Failed); + // If `trusted_ca` exists, it is already verified in the code above. Thus, we just need to make + // sure the verification for other validation context configurations doesn't fail (i.e. either + // `NotValidated` or `Validated`). If `trusted_ca` doesn't exist, we will need to make sure other + // configurations are verified and the verification succeed. + int validation_status = verify_trusted_ca_ + ? validated != Envoy::Ssl::ClientValidationStatus::Failed + : validated == Envoy::Ssl::ClientValidationStatus::Validated; + + return allow_untrusted_certificate_ ? 1 : validation_status; } Envoy::Ssl::ClientValidationStatus DefaultCertValidator::verifyCertificate( diff --git a/source/extensions/transport_sockets/tls/context_impl.cc b/source/extensions/transport_sockets/tls/context_impl.cc index 378097a1aca7..abb59d986111 100644 --- a/source/extensions/transport_sockets/tls/context_impl.cc +++ b/source/extensions/transport_sockets/tls/context_impl.cc @@ -1168,16 +1168,20 @@ bool TlsContext::isCipherEnabled(uint16_t cipher_id, uint16_t client_version) { bool ContextImpl::verifyCertChain(X509& leaf_cert, STACK_OF(X509) & intermediates, std::string& error_details) { bssl::UniquePtr ctx(X509_STORE_CTX_new()); + + ASSERT(!tls_contexts_.empty()); // It doesn't matter which SSL context is used, because they share the same // cert validation config. - X509_STORE* store = SSL_CTX_get_cert_store(tls_contexts_[0].ssl_ctx_.get()); + 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)) { error_details = "Failed to verify certificate chain: X509_STORE_CTX_init"; return false; } int res = cert_validator_->doVerifyCertChain(ctx.get(), nullptr, leaf_cert, nullptr); - if (res <= 0) { + // If |SSL_VERIFY_NONE|, the error is non-fatal, but we keep the error details. + if (res <= 0 && SSL_CTX_get_verify_mode(ssl_ctx) != SSL_VERIFY_NONE) { const int n = X509_STORE_CTX_get_error(ctx.get()); const int depth = X509_STORE_CTX_get_error_depth(ctx.get()); error_details = absl::StrCat("X509_verify_cert: certificate verification error at depth ", diff --git a/test/extensions/transport_sockets/tls/cert_validator/default_validator_test.cc b/test/extensions/transport_sockets/tls/cert_validator/default_validator_test.cc index ec6c3753d9b2..fee175872bd6 100644 --- a/test/extensions/transport_sockets/tls/cert_validator/default_validator_test.cc +++ b/test/extensions/transport_sockets/tls/cert_validator/default_validator_test.cc @@ -146,6 +146,25 @@ TEST(DefaultCertValidatorTest, TestCertificateVerificationWithSANMatcher) { EXPECT_EQ(stats.fail_verify_san_.value(), 1); } +TEST(DefaultCertValidatorTest, TestCertificateVerificationWithNoValidationContext) { + Stats::TestUtil::TestStore test_store; + SslStats stats = generateSslStats(test_store); + // Create the default validator object. + auto default_validator = + std::make_unique( + /*CertificateValidationContextConfig=*/nullptr, stats, + Event::GlobalTimeSystem().timeSystem()); + + EXPECT_EQ(default_validator->verifyCertificate(/*cert=*/nullptr, /*verify_san_list=*/{}, + /*subject_alt_name_matchers=*/{}), + Envoy::Ssl::ClientValidationStatus::NotValidated); + X509 cert = {}; + EXPECT_EQ(default_validator->doVerifyCertChain(/*store_ctx=*/nullptr, + /*ssl_extended_info=*/nullptr, /*leaf_cert=*/cert, + /*transport_socket_options=*/nullptr), + 0); +} + } // namespace Tls } // namespace TransportSockets } // namespace Extensions