Skip to content

Commit ab62d9e

Browse files
authored
tls: expose detailed certificate validation failure reasons in access logs (envoyproxy#42420)
Propagate detailed certificate validation errors (e.g., "certificate has expired", "unable to get local issuer certificate", "SAN matcher failed") from DefaultCertValidator through SslExtendedSocketInfo to the transport failure reason string in access logs. Previously, when TLS certificate validation failed, access logs only showed the generic CERTIFICATE_VERIFY_FAILED error without actionable details. This made debugging TLS issues difficult in production where debug logging is not feasible. The detailed error is now visible in %UPSTREAM_TRANSPORT_FAILURE_REASON% and %DOWNSTREAM_TRANSPORT_FAILURE_REASON% access log formatters. Fixes envoyproxy#42266 Signed-off-by: Prashanth Josyula <prashanth.chaitanya@salesforce.com>
1 parent c47dccc commit ab62d9e

10 files changed

Lines changed: 202 additions & 2 deletions

File tree

changelogs/current.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,14 @@ new_features:
424424
- area: tls_inspector
425425
change: |
426426
Added configuration parameter to TLS inspector for maximum acceptable client hello size.
427+
- area: tls
428+
change: |
429+
Enhanced TLS certificate validation failure messages in access logs to include detailed error information.
430+
The ``%DOWNSTREAM_TRANSPORT_FAILURE_REASON%`` and ``%UPSTREAM_TRANSPORT_FAILURE_REASON%`` access log
431+
formatters now include specific validation failure reasons such as ``verify cert failed: SAN matcher``,
432+
``verify cert failed: cert hash and spki``, or the OpenSSL verification error string (e.g., certificate
433+
has expired, unable to get local issuer certificate). This provides better visibility into TLS handshake
434+
failures without requiring debug-level logging.
427435
- area: ext_proc
428436
change: |
429437
Added support for forwarding cluster metadata to ext_proc server.

envoy/ssl/ssl_socket_extended_info.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
#include "envoy/event/dispatcher.h"
1010
#include "envoy/ssl/handshaker.h"
1111

12+
#include "absl/strings/string_view.h"
13+
1214
namespace Envoy {
1315
namespace Ssl {
1416

@@ -112,6 +114,17 @@ class SslExtendedSocketInfo {
112114
* @return CertificateSelectionStatus the cert selection status.
113115
*/
114116
virtual CertificateSelectionStatus certificateSelectionResult() const PURE;
117+
118+
/**
119+
* Set detailed certificate validation error information.
120+
* @param error_details the detailed error message from certificate validation.
121+
*/
122+
virtual void setCertificateValidationError(absl::string_view error_details) PURE;
123+
124+
/**
125+
* @return the detailed certificate validation error message, or empty if none.
126+
*/
127+
virtual absl::string_view certificateValidationError() const PURE;
115128
};
116129

117130
} // namespace Ssl

source/common/tls/context_impl.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,10 @@ enum ssl_verify_result_t ContextImpl::customVerifyCallback(SSL* ssl, uint8_t* ou
487487
if (result.tls_alert.has_value() && out_alert) {
488488
*out_alert = result.tls_alert.value();
489489
}
490+
// Store detailed error information for access log reporting.
491+
if (result.error_details.has_value()) {
492+
extended_socket_info->setCertificateValidationError(result.error_details.value());
493+
}
490494
return ssl_verify_invalid;
491495
}
492496
}

source/common/tls/ssl_handshaker.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,16 @@ void ValidateResultCallbackImpl::onSslHandshakeCancelled() { extended_socket_inf
2020

2121
void ValidateResultCallbackImpl::onCertValidationResult(bool succeeded,
2222
Ssl::ClientValidationStatus detailed_status,
23-
const std::string& /*error_details*/,
23+
const std::string& error_details,
2424
uint8_t tls_alert) {
2525
if (!extended_socket_info_.has_value()) {
2626
return;
2727
}
2828
extended_socket_info_->setCertificateValidationStatus(detailed_status);
2929
extended_socket_info_->setCertificateValidationAlert(tls_alert);
30+
if (!error_details.empty()) {
31+
extended_socket_info_->setCertificateValidationError(error_details);
32+
}
3033
extended_socket_info_->onCertificateValidationCompleted(succeeded, true);
3134
}
3235

source/common/tls/ssl_handshaker.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,11 @@ class SslExtendedSocketInfoImpl : public Envoy::Ssl::SslExtendedSocketInfo {
9393
return cert_selection_result_;
9494
}
9595

96+
void setCertificateValidationError(absl::string_view error_details) override {
97+
cert_validation_error_ = std::string(error_details);
98+
}
99+
absl::string_view certificateValidationError() const override { return cert_validation_error_; }
100+
96101
private:
97102
Envoy::Ssl::ClientValidationStatus certificate_validation_status_{
98103
Envoy::Ssl::ClientValidationStatus::NotValidated};
@@ -112,6 +117,8 @@ class SslExtendedSocketInfoImpl : public Envoy::Ssl::SslExtendedSocketInfo {
112117
// NotStarted if no cert selection has ever been kicked off.
113118
Ssl::CertificateSelectionStatus cert_selection_result_{
114119
Ssl::CertificateSelectionStatus::NotStarted};
120+
// Stores the detailed certificate validation error message.
121+
std::string cert_validation_error_;
115122
};
116123

117124
class SslHandshakerImpl : public ConnectionInfoImplBase,

source/common/tls/ssl_socket.cc

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,13 +211,17 @@ PostIoAction SslSocket::doHandshake() { return info_->doHandshake(); }
211211
void SslSocket::drainErrorQueue() {
212212
bool saw_error = false;
213213
bool saw_counted_error = false;
214+
bool saw_cert_verify_failed = false;
215+
bool saw_no_client_cert = false;
214216
while (uint64_t err = ERR_get_error()) {
215217
if (ERR_GET_LIB(err) == ERR_LIB_SSL) {
216218
if (ERR_GET_REASON(err) == SSL_R_PEER_DID_NOT_RETURN_A_CERTIFICATE) {
217219
ctx_->stats().fail_verify_no_cert_.inc();
218220
saw_counted_error = true;
221+
saw_no_client_cert = true;
219222
} else if (ERR_GET_REASON(err) == SSL_R_CERTIFICATE_VERIFY_FAILED) {
220223
saw_counted_error = true;
224+
saw_cert_verify_failed = true;
221225
}
222226
} else if (ERR_GET_LIB(err) == ERR_LIB_SYS) {
223227
// Any syscall errors that result in connection closure are already tracked in other
@@ -241,6 +245,23 @@ void SslSocket::drainErrorQueue() {
241245
return;
242246
}
243247

248+
// Append detailed error info for certificate-related failures.
249+
bool added_detail = false;
250+
if (saw_cert_verify_failed) {
251+
auto* extended_socket_info = reinterpret_cast<Envoy::Ssl::SslExtendedSocketInfo*>(
252+
SSL_get_ex_data(rawSsl(), ContextImpl::sslExtendedSocketInfoIndex()));
253+
if (extended_socket_info != nullptr) {
254+
absl::string_view cert_validation_error = extended_socket_info->certificateValidationError();
255+
if (!cert_validation_error.empty()) {
256+
absl::StrAppend(&failure_reason_, ":", cert_validation_error);
257+
added_detail = true;
258+
}
259+
}
260+
}
261+
if (!added_detail && saw_no_client_cert) {
262+
absl::StrAppend(&failure_reason_, ":peer did not provide required client certificate");
263+
}
264+
244265
if (!failure_reason_.empty()) {
245266
absl::StrAppend(&failure_reason_, ":TLS_error_end");
246267
ENVOY_CONN_LOG(debug, "remote address:{},{}", callbacks_->connection(),

test/common/tls/cert_validator/default_validator_test.cc

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,79 @@ TEST(DefaultCertValidatorTest, DefaultValidatorCaExpirationStats) {
817817
EXPECT_EQ(gauge_opt->get().value(), std::chrono::seconds::max().count());
818818
}
819819

820+
// Test that ValidationResults contains detailed error information when SAN validation fails.
821+
TEST(DefaultCertValidatorTest, TestCertificateValidationErrorDetailsForSanFailure) {
822+
NiceMock<Server::Configuration::MockServerFactoryContext> context;
823+
Stats::TestUtil::TestStore test_store;
824+
SslStats stats = generateSslStats(*test_store.rootScope());
825+
826+
// Load a certificate with DNS SANs
827+
bssl::UniquePtr<X509> cert = readCertFromFile(
828+
TestEnvironment::substitute("{{ test_rundir }}/test/common/tls/test_data/san_dns_cert.pem"));
829+
830+
// Create SAN matchers that won't match the certificate (using regex as DNS requires it)
831+
envoy::type::matcher::v3::StringMatcher matcher;
832+
matcher.MergeFrom(TestUtility::createRegexMatcher(R"raw(nomatch\.example\.com)raw"));
833+
std::vector<SanMatcherPtr> subject_alt_name_matchers;
834+
subject_alt_name_matchers.push_back(
835+
SanMatcherPtr{std::make_unique<StringSanMatcher>(GEN_DNS, matcher, context)});
836+
837+
// Create the validator with no config (so verify_trusted_ca_ is false)
838+
auto default_validator =
839+
std::make_unique<Extensions::TransportSockets::Tls::DefaultCertValidator>(
840+
/*CertificateValidationContextConfig=*/nullptr, stats, context);
841+
842+
// Call verifyCertificate directly which populates error_details on SAN mismatch
843+
std::string error_details;
844+
uint8_t tls_alert;
845+
Ssl::ClientValidationStatus status = default_validator->verifyCertificate(
846+
cert.get(), /*verify_san_list=*/{}, subject_alt_name_matchers, /*stream_info=*/{},
847+
&error_details, &tls_alert);
848+
849+
// Validation should fail because the SAN doesn't match
850+
EXPECT_EQ(Ssl::ClientValidationStatus::Failed, status);
851+
852+
// The error_details should be populated with a meaningful error message
853+
EXPECT_EQ(error_details, "verify cert failed: SAN matcher");
854+
}
855+
856+
// Test that TestSslExtendedSocketInfo properly stores and retrieves certificate validation errors.
857+
TEST(DefaultCertValidatorTest, TestSslExtendedSocketInfoCertValidationError) {
858+
TestSslExtendedSocketInfo extended_socket_info;
859+
860+
// Initially the error should be empty
861+
EXPECT_TRUE(extended_socket_info.certificateValidationError().empty());
862+
863+
// Set an error
864+
const std::string error_msg = "verify cert failed: X509_verify_cert: certificate has expired";
865+
extended_socket_info.setCertificateValidationError(error_msg);
866+
867+
// Verify the error is stored and retrievable
868+
EXPECT_EQ(error_msg, extended_socket_info.certificateValidationError());
869+
}
870+
871+
// Test that empty cert chain returns appropriate error details.
872+
TEST(DefaultCertValidatorTest, TestEmptyCertChainErrorDetails) {
873+
NiceMock<Server::Configuration::MockServerFactoryContext> context;
874+
Stats::TestUtil::TestStore test_store;
875+
SslStats stats = generateSslStats(*test_store.rootScope());
876+
877+
auto default_validator =
878+
std::make_unique<Extensions::TransportSockets::Tls::DefaultCertValidator>(
879+
/*CertificateValidationContextConfig=*/nullptr, stats, context);
880+
881+
SSLContextPtr ssl_ctx = SSL_CTX_new(TLS_method());
882+
bssl::UniquePtr<STACK_OF(X509)> cert_chain(sk_X509_new_null());
883+
884+
ValidationResults results = default_validator->doVerifyCertChain(
885+
*cert_chain, /*callback=*/nullptr,
886+
/*transport_socket_options=*/nullptr, *ssl_ctx, {}, false, "");
887+
888+
EXPECT_EQ(ValidationResults::ValidationStatus::Failed, results.status);
889+
EXPECT_TRUE(results.error_details.has_value());
890+
EXPECT_EQ(results.error_details.value(), "verify cert failed: empty cert chain");
891+
}
892+
820893
} // namespace Tls
821894
} // namespace TransportSockets
822895
} // namespace Extensions

test/common/tls/cert_validator/test_common.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,17 @@ class TestSslExtendedSocketInfo : public Envoy::Ssl::SslExtendedSocketInfo {
4747
return cert_selection_result_;
4848
}
4949

50+
void setCertificateValidationError(absl::string_view error_details) override {
51+
cert_validation_error_ = std::string(error_details);
52+
}
53+
absl::string_view certificateValidationError() const override { return cert_validation_error_; }
54+
5055
private:
5156
Envoy::Ssl::ClientValidationStatus status_;
5257
Ssl::ValidateStatus validate_result_{Ssl::ValidateStatus::NotStarted};
5358
Ssl::CertificateSelectionStatus cert_selection_result_{
5459
Ssl::CertificateSelectionStatus::NotStarted};
60+
std::string cert_validation_error_;
5561
};
5662

5763
class TestCertificateValidationContextConfig

test/common/tls/handshaker_test.cc

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,71 @@ TEST_F(HandshakerTest, NormalOperationWithSslHandshakerImplForTest) {
244244
EXPECT_EQ(post_io_action, Network::PostIoAction::Close);
245245
}
246246

247+
// Test that SslExtendedSocketInfoImpl properly stores and retrieves certificate validation errors.
248+
TEST_F(HandshakerTest, SslExtendedSocketInfoCertValidationError) {
249+
NiceMock<Network::MockConnection> mock_connection;
250+
ON_CALL(mock_connection, state).WillByDefault(Return(Network::Connection::State::Closed));
251+
252+
NiceMock<MockHandshakeCallbacks> handshake_callbacks;
253+
ON_CALL(handshake_callbacks, connection()).WillByDefault(ReturnRef(mock_connection));
254+
255+
SslHandshakerImpl handshaker(std::move(server_ssl_), 0, &handshake_callbacks);
256+
257+
// Get the extended socket info through the SSL ex_data mechanism
258+
auto* extended_info =
259+
reinterpret_cast<Ssl::SslExtendedSocketInfo*>(SSL_get_ex_data(handshaker.ssl(), 0));
260+
ASSERT_NE(extended_info, nullptr);
261+
262+
// Initially, the certificate validation error should be empty.
263+
EXPECT_TRUE(extended_info->certificateValidationError().empty());
264+
265+
// Set the certificate validation error.
266+
const std::string error_msg =
267+
"verify cert failed: X509_verify_cert: certificate verification error at depth 0: "
268+
"certificate has expired";
269+
extended_info->setCertificateValidationError(error_msg);
270+
271+
// Verify the error is properly stored and retrievable.
272+
EXPECT_EQ(error_msg, extended_info->certificateValidationError());
273+
274+
// Test that we can overwrite the error.
275+
const std::string new_error_msg = "verify cert failed: SAN matcher";
276+
extended_info->setCertificateValidationError(new_error_msg);
277+
EXPECT_EQ(new_error_msg, extended_info->certificateValidationError());
278+
}
279+
280+
// Test that ValidateResultCallbackImpl properly stores error details.
281+
TEST_F(HandshakerTest, ValidateResultCallbackStoresErrorDetails) {
282+
NiceMock<Network::MockConnection> mock_connection;
283+
ON_CALL(mock_connection, state).WillByDefault(Return(Network::Connection::State::Open));
284+
285+
NiceMock<MockHandshakeCallbacks> handshake_callbacks;
286+
ON_CALL(handshake_callbacks, connection()).WillByDefault(ReturnRef(mock_connection));
287+
288+
SslHandshakerImpl handshaker(std::move(server_ssl_), 0, &handshake_callbacks);
289+
290+
// Get the extended socket info
291+
auto* extended_info =
292+
reinterpret_cast<Ssl::SslExtendedSocketInfo*>(SSL_get_ex_data(handshaker.ssl(), 0));
293+
ASSERT_NE(extended_info, nullptr);
294+
295+
// Create a validation callback (simulating async validation)
296+
auto callback = extended_info->createValidateResultCallback();
297+
ASSERT_NE(callback, nullptr);
298+
299+
// Simulate validation failure with error details
300+
const std::string error_details =
301+
"verify cert failed: X509_verify_cert: certificate verification error at depth 1: "
302+
"unable to get local issuer certificate";
303+
callback->onCertValidationResult(false, Ssl::ClientValidationStatus::Failed, error_details,
304+
SSL_AD_UNKNOWN_CA);
305+
306+
// Verify the error was stored in extended socket info
307+
EXPECT_EQ(error_details, extended_info->certificateValidationError());
308+
EXPECT_EQ(Ssl::ClientValidationStatus::Failed, extended_info->certificateValidationStatus());
309+
EXPECT_EQ(SSL_AD_UNKNOWN_CA, extended_info->certificateValidationAlert());
310+
}
311+
247312
} // namespace
248313
} // namespace Tls
249314
} // namespace TransportSockets

test/extensions/access_loggers/grpc/tcp_grpc_access_log_integration_test.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,7 @@ TEST_P(TcpGrpcAccessLogIntegrationTest, TlsHandshakeFailure_VerifyFailed) {
833833
downstream_local_address:
834834
socket_address:
835835
address: {0}
836-
downstream_transport_failure_reason: "TLS_error:|268435581:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED:TLS_error_end"
836+
downstream_transport_failure_reason: "TLS_error:|268435581:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED:verify cert failed: cert hash and spki:TLS_error_end"
837837
access_log_type: NotSet
838838
downstream_direct_remote_address:
839839
socket_address:

0 commit comments

Comments
 (0)