tls: fix duplicated ':TLS_error_end' suffix in socket failure reason (e…

…nvoyproxy#34799) This fix also prevents duplicated debug logs, since they are always generated after the end suffix is appended to the failure reason. Signed-off-by: Joe Kralicky <joekralicky@gmail.com>
juanmolle · Jun 27, 2024 · f3ee7c7 · f3ee7c7
1 parent e1c6235
commit f3ee7c7
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 15 deletions.
diff --git a/source/common/tls/ssl_socket.cc b/source/common/tls/ssl_socket.cc
@@ -238,6 +238,10 @@ void SslSocket::drainErrorQueue() {
                     absl::NullSafeStringView(ERR_reason_error_string(err)));
   }
 
+  if (!saw_error) {
+    return;
+  }
+
   if (!failure_reason_.empty()) {
     if (new_ssl_failure_format) {
       absl::StrAppend(&failure_reason_, ":TLS_error_end");
@@ -247,7 +251,7 @@ void SslSocket::drainErrorQueue() {
                    failure_reason_);
   }
 
-  if (saw_error && !saw_counted_error) {
+  if (!saw_counted_error) {
     ctx_->stats().connection_error_.inc();
   }
 }

diff --git a/test/common/tls/integration/ssl_integration_test.cc b/test/common/tls/integration/ssl_integration_test.cc
@@ -765,13 +765,13 @@ TEST_P(SslCertficateIntegrationTest, ServerEcdsaClientRsaOnlyWithAccessLog) {
 
   auto log_result = waitForAccessLog(listener_access_log_name_);
   if (tls_version_ == envoy::extensions::transport_sockets::tls::v3::TlsParameters::TLSv1_3) {
-    EXPECT_THAT(log_result,
-                StartsWith("DOWNSTREAM_TRANSPORT_FAILURE_REASON=TLS_error:|268435709:SSL_routines:"
-                           "OPENSSL_internal:NO_COMMON_SIGNATURE_ALGORITHMS:TLS_error_end"));
+    EXPECT_EQ(log_result,
+              "DOWNSTREAM_TRANSPORT_FAILURE_REASON=TLS_error:|268435709:SSL_routines:"
+              "OPENSSL_internal:NO_COMMON_SIGNATURE_ALGORITHMS:TLS_error_end FILTER_CHAIN_NAME=-");
   } else {
-    EXPECT_THAT(log_result,
-                StartsWith("DOWNSTREAM_TRANSPORT_FAILURE_REASON=TLS_error:|268435640:"
-                           "SSL_routines:OPENSSL_internal:NO_SHARED_CIPHER:TLS_error_end"));
+    EXPECT_EQ(log_result,
+              "DOWNSTREAM_TRANSPORT_FAILURE_REASON=TLS_error:|268435640:"
+              "SSL_routines:OPENSSL_internal:NO_SHARED_CIPHER:TLS_error_end FILTER_CHAIN_NAME=-");
   }
 }
 
@@ -791,12 +791,11 @@ TEST_P(SslCertficateIntegrationTest, ServerEcdsaClientRsaOnlyWithAccessLogOrigin
 
   auto log_result = waitForAccessLog(listener_access_log_name_);
   if (tls_version_ == envoy::extensions::transport_sockets::tls::v3::TlsParameters::TLSv1_3) {
-    EXPECT_THAT(log_result,
-                StartsWith("DOWNSTREAM_TRANSPORT_FAILURE_REASON=TLS_error:_268435709:SSL_routines:"
-                           "OPENSSL_internal:NO_COMMON_SIGNATURE_ALGORITHMS"));
+    EXPECT_EQ(log_result, "DOWNSTREAM_TRANSPORT_FAILURE_REASON=TLS_error:_268435709:SSL_routines:"
+                          "OPENSSL_internal:NO_COMMON_SIGNATURE_ALGORITHMS FILTER_CHAIN_NAME=-");
   } else {
-    EXPECT_THAT(log_result, StartsWith("DOWNSTREAM_TRANSPORT_FAILURE_REASON=TLS_error:_268435640:"
-                                       "SSL_routines:OPENSSL_internal:NO_SHARED_CIPHER"));
+    EXPECT_EQ(log_result, "DOWNSTREAM_TRANSPORT_FAILURE_REASON=TLS_error:_268435640:"
+                          "SSL_routines:OPENSSL_internal:NO_SHARED_CIPHER FILTER_CHAIN_NAME=-");
   }
 }
 

diff --git a/test/extensions/access_loggers/grpc/tcp_grpc_access_log_integration_test.cc b/test/extensions/access_loggers/grpc/tcp_grpc_access_log_integration_test.cc
@@ -833,7 +833,7 @@ TEST_P(TcpGrpcAccessLogIntegrationTest, TlsHandshakeFailure_VerifyFailed) {
       downstream_local_address:
         socket_address:
           address: {0}
-      downstream_transport_failure_reason: "TLS_error:|268435581:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED:TLS_error_end:TLS_error_end"
+      downstream_transport_failure_reason: "TLS_error:|268435581:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED:TLS_error_end"
       access_log_type: NotSet
       downstream_direct_remote_address:
         socket_address:
@@ -895,7 +895,7 @@ TEST_P(TcpGrpcAccessLogIntegrationTest, TlsHandshakeFailure_NoSharedCipher) {
       downstream_local_address:
         socket_address:
           address: {0}
-      downstream_transport_failure_reason: "TLS_error:|268435640:SSL routines:OPENSSL_internal:NO_SHARED_CIPHER:TLS_error_end:TLS_error_end"
+      downstream_transport_failure_reason: "TLS_error:|268435640:SSL routines:OPENSSL_internal:NO_SHARED_CIPHER:TLS_error_end"
       access_log_type: NotSet
       downstream_direct_remote_address:
         socket_address:
@@ -952,7 +952,7 @@ TEST_P(TcpGrpcAccessLogIntegrationTest, SslHandshakeFailure_UnsupportedProtocol)
       downstream_local_address:
         socket_address:
           address: {0}
-      downstream_transport_failure_reason: "TLS_error:|268435696:SSL routines:OPENSSL_internal:UNSUPPORTED_PROTOCOL:TLS_error_end:TLS_error_end"
+      downstream_transport_failure_reason: "TLS_error:|268435696:SSL routines:OPENSSL_internal:UNSUPPORTED_PROTOCOL:TLS_error_end"
       access_log_type: NotSet
       downstream_direct_remote_address:
         socket_address: