tls: expose detailed certificate validation failure reasons in access logs by prashanthjos · Pull Request #42420 · envoyproxy/envoy

prashanthjos · 2025-12-04T19:53:28Z

Commit Message:
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.

Additional Description:
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. This change adds two new methods to SslExtendedSocketInfo (setCertificateValidationError/certificateValidationError) and wires them through:
ContextImpl::customVerifyCallback() stores error_details from ValidationResults
ValidateResultCallbackImpl::onCertValidationResult() stores error_details for async validation
SslSocket::drainErrorQueue() appends the detailed error when SSL_R_CERTIFICATE_VERIFY_FAILED is encountered

The detailed error is now visible in %UPSTREAM_TRANSPORT_FAILURE_REASON% and %DOWNSTREAM_TRANSPORT_FAILURE_REASON% access log formatters.

Risk Level: Low

Testing:
Added unit tests in default_validator_test.cc: TestCertificateValidationErrorDetailsForSanFailure, TestSslExtendedSocketInfoCertValidationError, TestEmptyCertChainErrorDetails

Added unit tests in handshaker_test.cc: SslExtendedSocketInfoCertValidationError, ValidateResultCallbackStoresErrorDetails
All existing TLS tests pass

Docs Changes: Uses existing documented access log formatters

Release Notes: Added detailed certificate validation failure reasons to %UPSTREAM_TRANSPORT_FAILURE_REASON% and %DOWNSTREAM_TRANSPORT_FAILURE_REASON% access log fields. When TLS certificate validation fails, the access log now includes specific error details such as "certificate has expired" or "unable to get local issuer certificate" instead of only the generic CERTIFICATE_VERIFY_FAILED code.

Fixes #42266

… logs Previously, when TLS certificate validation failed, Envoy's access logs only showed the generic "CERTIFICATE_VERIFY_FAILED" error code without actionable details about why the validation failed. This made debugging TLS issues difficult, especially in production environments where debug logging is not feasible. This change propagates the detailed certificate validation error (e.g., "certificate has expired", "unable to get local issuer certificate", "SAN matcher failed") from the DefaultCertValidator through SslExtendedSocketInfo to the transport failure reason string that appears in access logs. The detailed error is appended to the existing failure_reason_ when SSL_R_CERTIFICATE_VERIFY_FAILED is encountered, resulting in access log output like: TLS_error:|268435581:SSL:CERTIFICATE_VERIFY_FAILED:verify cert failed: X509_verify_cert: certificate verification error at depth 0: certificate has expired:TLS_error_end This information is exposed via the existing access log formatters: - %UPSTREAM_TRANSPORT_FAILURE_REASON% (for upstream/cluster connections) - %DOWNSTREAM_TRANSPORT_FAILURE_REASON% (for downstream/listener mTLS) Signed-off-by: Prashanth Josyula <prashanth.chaitanya@salesforce.com>

prashanthjos · 2025-12-04T22:29:10Z

/retest

…gs only showed the generic "CERTIFICATE_VERIFY_FAILED" error code without actionable details about why the validation failed. This made debugging TLS issues difficult, especially in production environments where debug logging is not feasible. This change propagates the detailed certificate validation error (e.g., "certificate has expired", "unable to get local issuer certificate", "SAN matcher failed") from the DefaultCertValidator through SslExtendedSocketInfo to the transport failure reason string that appears in access logs. The detailed error is appended to the existing failure_reason_ when SSL_R_CERTIFICATE_VERIFY_FAILED is encountered, resulting in access log output like: TLS_error:|268435581:SSL:CERTIFICATE_VERIFY_FAILED:verify cert failed: X509_verify_cert: certificate verification error at depth 0: certificate has expired:TLS_error_end This information is exposed via the existing access log formatters: - %UPSTREAM_TRANSPORT_FAILURE_REASON% (for upstream/cluster connections) - %DOWNSTREAM_TRANSPORT_FAILURE_REASON% (for downstream/listener mTLS) Signed-off-by: Prashanth Josyula <prashanth.chaitanya@salesforce.com>

prashanthjos · 2025-12-05T02:05:14Z

/retest

prashanthjos · 2025-12-05T02:25:58Z

@ggreenway @wbpcode can you please take a look when you get sometime.

ggreenway

This is a really nice improvement. Thanks!

/wait

ggreenway · 2025-12-05T21:11:26Z

  while (uint64_t err = ERR_get_error()) {
    if (ERR_GET_LIB(err) == ERR_LIB_SSL) {
      if (ERR_GET_REASON(err) == SSL_R_PEER_DID_NOT_RETURN_A_CERTIFICATE) {
        ctx_->stats().fail_verify_no_cert_.inc();


Is there anything in the details text right now if the peer didn't provide a cert? If not, should we add something here in this case?

ggreenway · 2025-12-05T21:12:33Z

+  EXPECT_THAT(error_details, testing::HasSubstr("verify cert failed"));
+  EXPECT_THAT(error_details, testing::HasSubstr("SAN matcher"));


What is the entire string of error_details here? Can this match either the full string or a longer substring?

…gs only showed the generic "CERTIFICATE_VERIFY_FAILED" error code without actionable details about why the validation failed. This made debugging TLS issues difficult, especially in production environments where debug logging is not feasible. This change propagates the detailed certificate validation error (e.g., "certificate has expired", "unable to get local issuer certificate","SAN matcher failed") from the DefaultCertValidator through SslExtendedSocketInfo to the transport failure reason string that appears in access logs. The detailed error is appended to the existing failure_reason_ when SSL_R_CERTIFICATE_VERIFY_FAILED is encountered, resulting in access log output like: TLS_error:|268435581:SSL:CERTIFICATE_VERIFY_FAILED:verify cert failed: X509_verify_cert: certificate verification error at depth 0: certificate has expired:TLS_error_end This information is exposed via the existing access log formatters: - %UPSTREAM_TRANSPORT_FAILURE_REASON% (for upstream/cluster connections) - %DOWNSTREAM_TRANSPORT_FAILURE_REASON% (for downstream/listener mTLS) Signed-off-by: Prashanth Josyula <prashanth.chaitanya@salesforce.com>

prashanthjos · 2025-12-06T04:02:01Z

/retest

prashanthjos · 2025-12-07T02:11:37Z

@ggreenway please have a relook I have addressed your comments

ggreenway

/wait

Pawan-Bishnoi · 2025-12-11T06:02:58Z

+    } else if (saw_no_client_cert) {
+      absl::StrAppend(&failure_reason_, ":peer did not provide required client certificate");
+    }
+  }


Can this be simplified as below? Anything we miss trying to remove the repetitive if check

// Append detailed error info for certificate-related failures. if (saw_cert_verify_failed || saw_no_client_cert) { auto* extended_socket_info = reinterpret_cast<Envoy::Ssl::SslExtendedSocketInfo*>( SSL_get_ex_data(rawSsl(), ContextImpl::sslExtendedSocketInfoIndex())); if (saw_no_client_cert) { absl::StrAppend(&failure_reason_, ":peer did not provide required client certificate"); } else if (extended_socket_info != nullptr) { absl::string_view cert_validation_error = extended_socket_info->certificateValidationError(); if (!cert_validation_error.empty()) { absl::StrAppend(&failure_reason_, ":", cert_validation_error); } } }

or even

if (saw_no_client_cert) { absl::StrAppend(&failure_reason_, ":peer did not provide required client certificate"); } else if (saw_cert_verify_failed) { // Append detailed error info for certificate-related failures. auto* extended_socket_info = reinterpret_cast<Envoy::Ssl::SslExtendedSocketInfo*>( SSL_get_ex_data(rawSsl(), ContextImpl::sslExtendedSocketInfoIndex())); if (extended_socket_info != nullptr) { absl::string_view cert_validation_error = extended_socket_info->certificateValidationError(); if (!cert_validation_error.empty()) { absl::StrAppend(&failure_reason_, ":", cert_validation_error); } } }

wbpcode

Although I think it would be better to merge the setCertificateValidationError and setCertificateValidationStatus. But current change also fine to me.

wbpcode · 2025-12-11T09:11:36Z

Defer this to @ggreenway for further review/approval.

And hello, @ggreenway, I see you left a require change, but I didn't see any new comment from you. Do I missed something?

ggreenway · 2025-12-11T16:41:41Z

@wbpcode that's weird; github seems to have discarded the comment I left in that review.

ggreenway · 2025-12-11T16:45:21Z

+        absl::StrAppend(&failure_reason_, ":peer did not provide required client certificate");
+      }
+    } else if (saw_no_client_cert) {
+      absl::StrAppend(&failure_reason_, ":peer did not provide required client certificate");


Please refactor the logic so there's only one case that appends this error. Maybe something roughly like this:

bool added_reason = false; if (saw_cert_verify_failed) { // checks for null StrAppend(failure_reason_, "the text"); added_reason = true; } if (!added_reason && saw_no_client_cert) { StrAppend(......); }

… logs Previously, when TLS certificate validation failed, Envoy's access logs only showed the generic "CERTIFICATE_VERIFY_FAILED" error code without actionable details about why the validation failed. This made debugging TLS issues difficult, especially in production environments where debug logging is not feasible. This change propagates the detailed certificate validation error (e.g., "certificate has expired", "unable to get local issuer certificate", "SAN matcher failed") from the DefaultCertValidator through SslExtendedSocketInfo to the transport failure reason string that appears in access logs. The detailed error is appended to the existing failure_reason_ when SSL_R_CERTIFICATE_VERIFY_FAILED is encountered, resulting in access log output like: TLS_error:|268435581:SSL:CERTIFICATE_VERIFY_FAILED:verify cert failed: X509_verify_cert: certificate verification error at depth 0: certificate has expired:TLS_error_end This information is exposed via the existing access log formatters: - %UPSTREAM_TRANSPORT_FAILURE_REASON% (for upstream/cluster connections) - %DOWNSTREAM_TRANSPORT_FAILURE_REASON% (for downstream/listener mTLS) Signed-off-by: Prashanth Josyula <prashanth.16@gmail.com>

ggreenway

LGTM

ggreenway

Please add a release note changelogs/current.yaml

/wait

Signed-off-by: Prashanth Josyula <prashanth.16@gmail.com>

ggreenway · 2025-12-11T23:31:23Z

There's a merge conflict on the changelog. Please merge main into your PR and resolve.

/wait

Signed-off-by: Prashanth Josyula <prashanth.16@gmail.com>

prashanthjos · 2025-12-12T00:44:08Z

/retest

… 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> Signed-off-by: MayorFaj <mayorfaj@gmail.com>

… 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> Signed-off-by: Gustavo <grnmeira@gmail.com>

prashanth-chaitanya added 2 commits December 4, 2025 19:20

ggreenway self-assigned this Dec 4, 2025

prashanthjos requested a review from wbpcode as a code owner December 5, 2025 01:11

ggreenway requested changes Dec 5, 2025

View reviewed changes

repokitteh-read-only Bot added the waiting label Dec 5, 2025

repokitteh-read-only Bot removed the waiting label Dec 6, 2025

wbpcode self-assigned this Dec 8, 2025

ggreenway requested changes Dec 8, 2025

View reviewed changes

repokitteh-read-only Bot added the waiting label Dec 8, 2025

Pawan-Bishnoi reviewed Dec 11, 2025

View reviewed changes

wbpcode previously approved these changes Dec 11, 2025

View reviewed changes

wbpcode removed the waiting label Dec 11, 2025

ggreenway requested changes Dec 11, 2025

View reviewed changes

ggreenway added the waiting label Dec 11, 2025

prashanthjos dismissed wbpcode’s stale review via 444148f December 11, 2025 22:31

repokitteh-read-only Bot removed the waiting label Dec 11, 2025

ggreenway approved these changes Dec 11, 2025

View reviewed changes

ggreenway requested changes Dec 11, 2025

View reviewed changes

repokitteh-read-only Bot added the waiting label Dec 11, 2025

ggreenway enabled auto-merge (squash) December 11, 2025 22:56

Added the changelogs

45add15

Signed-off-by: Prashanth Josyula <prashanth.16@gmail.com>

auto-merge was automatically disabled December 11, 2025 23:12
Head branch was pushed to by a user without write access

repokitteh-read-only Bot removed the waiting label Dec 11, 2025

repokitteh-read-only Bot added the waiting label Dec 11, 2025

Merge branch 'main' into main

d6e3c1e

Signed-off-by: Prashanth Josyula <prashanth.16@gmail.com>

repokitteh-read-only Bot removed the waiting label Dec 11, 2025

ggreenway approved these changes Dec 11, 2025

View reviewed changes

ggreenway merged commit ab62d9e into envoyproxy:main Dec 12, 2025
24 checks passed

zhaohuabing mentioned this pull request Apr 28, 2026

Do not expose detailed TLS transport failure reasons in HTTP responses #44694

Closed

		EXPECT_THAT(error_details, testing::HasSubstr("verify cert failed"));
		EXPECT_THAT(error_details, testing::HasSubstr("SAN matcher"));

Conversation

prashanthjos commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prashanthjos commented Dec 4, 2025

Uh oh!

prashanthjos commented Dec 5, 2025

Uh oh!

prashanthjos commented Dec 5, 2025

Uh oh!

ggreenway left a comment

Choose a reason for hiding this comment

Uh oh!

ggreenway Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

prashanthjos Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

ggreenway Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

prashanthjos Dec 6, 2025

Choose a reason for hiding this comment

Uh oh!

prashanthjos commented Dec 6, 2025

Uh oh!

prashanthjos commented Dec 7, 2025

Uh oh!

ggreenway left a comment

Choose a reason for hiding this comment

Uh oh!

Pawan-Bishnoi Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Pawan-Bishnoi Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wbpcode left a comment

Choose a reason for hiding this comment

Uh oh!

wbpcode commented Dec 11, 2025

Uh oh!

ggreenway commented Dec 11, 2025

Uh oh!

ggreenway Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

ggreenway left a comment

Choose a reason for hiding this comment

Uh oh!

ggreenway left a comment

Choose a reason for hiding this comment

Uh oh!

ggreenway commented Dec 11, 2025

Uh oh!

prashanthjos commented Dec 12, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

prashanthjos commented Dec 4, 2025 •

edited

Loading

Pawan-Bishnoi Dec 11, 2025 •

edited

Loading