tls: remove runtime guard ssl_transport_failure_reason_format (envo…

…yproxy#35389) Risk Level: low Testing: Docs Changes: Release Notes: Platform Specific Features: Fixes:envoyproxy#35375 --------- Signed-off-by: Boteng Yao <[email protected]> Signed-off-by: asingh-g <[email protected]>
asingh-g · Aug 20, 2024 · a0e61de · a0e61de
1 parent 1c64783
commit a0e61de
Show file tree

Hide file tree

Showing 4 changed files with 6 additions and 35 deletions.
diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -31,6 +31,9 @@ removed_config_or_runtime:
 - area: DNS
   change: |
     Removed ``envoy.reloadable_features.dns_cache_set_first_resolve_complete`` runtime flag and legacy code paths.
+- area: tls
+  change: |
+    Removed runtime flag ``envoy.reloadable_features.ssl_transport_failure_reason_format``.
 
 new_features:
 - area: tls

diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc
@@ -90,7 +90,6 @@ RUNTIME_GUARD(envoy_reloadable_features_sanitize_te);
 RUNTIME_GUARD(envoy_reloadable_features_send_header_raw_value);
 RUNTIME_GUARD(envoy_reloadable_features_send_local_reply_when_no_buffer_and_upstream_request);
 RUNTIME_GUARD(envoy_reloadable_features_skip_dns_lookup_for_proxied_requests);
-RUNTIME_GUARD(envoy_reloadable_features_ssl_transport_failure_reason_format);
 RUNTIME_GUARD(envoy_reloadable_features_stateful_session_encode_ttl_in_cookie);
 RUNTIME_GUARD(envoy_reloadable_features_strict_duration_validation);
 RUNTIME_GUARD(envoy_reloadable_features_tcp_tunneling_send_downstream_fin_on_upstream_trailers);

diff --git a/source/common/tls/ssl_socket.cc b/source/common/tls/ssl_socket.cc
@@ -6,7 +6,6 @@
 #include "source/common/common/empty_string.h"
 #include "source/common/common/hex.h"
 #include "source/common/http/headers.h"
-#include "source/common/runtime/runtime_features.h"
 #include "source/common/tls/io_handle_bio.h"
 #include "source/common/tls/ssl_handshaker.h"
 #include "source/common/tls/utility.h"
@@ -210,8 +209,6 @@ PostIoAction SslSocket::doHandshake() { return info_->doHandshake(); }
 void SslSocket::drainErrorQueue() {
   bool saw_error = false;
   bool saw_counted_error = false;
-  bool new_ssl_failure_format = Runtime::runtimeFeatureEnabled(
-      "envoy.reloadable_features.ssl_transport_failure_reason_format");
   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) {
@@ -229,10 +226,10 @@ void SslSocket::drainErrorQueue() {
     saw_error = true;
 
     if (failure_reason_.empty()) {
-      failure_reason_ = new_ssl_failure_format ? "TLS_error:" : "TLS error:";
+      failure_reason_ = "TLS_error:";
     }
 
-    absl::StrAppend(&failure_reason_, new_ssl_failure_format ? "|" : " ", err, ":",
+    absl::StrAppend(&failure_reason_, "|", err, ":",
                     absl::NullSafeStringView(ERR_lib_error_string(err)), ":",
                     absl::NullSafeStringView(ERR_func_error_string(err)), ":",
                     absl::NullSafeStringView(ERR_reason_error_string(err)));
@@ -243,9 +240,7 @@ void SslSocket::drainErrorQueue() {
   }
 
   if (!failure_reason_.empty()) {
-    if (new_ssl_failure_format) {
-      absl::StrAppend(&failure_reason_, ":TLS_error_end");
-    }
+    absl::StrAppend(&failure_reason_, ":TLS_error_end");
     ENVOY_CONN_LOG(debug, "remote address:{},{}", callbacks_->connection(),
                    callbacks_->connection().connectionInfoProvider().remoteAddress()->asString(),
                    failure_reason_);

diff --git a/test/common/tls/integration/ssl_integration_test.cc b/test/common/tls/integration/ssl_integration_test.cc
@@ -790,8 +790,6 @@ TEST_P(SslCertficateIntegrationTest, ServerEcdsaClientRsaOnly) {
 // Test the access log.
 TEST_P(SslCertficateIntegrationTest, ServerEcdsaClientRsaOnlyWithAccessLog) {
   TestScopedRuntime scoped_runtime;
-  scoped_runtime.mergeValues(
-      {{"envoy.reloadable_features.ssl_transport_failure_reason_format", "true"}});
   useListenerAccessLog("DOWNSTREAM_TRANSPORT_FAILURE_REASON=%DOWNSTREAM_TRANSPORT_FAILURE_REASON% "
                        "FILTER_CHAIN_NAME=%FILTER_CHAIN_NAME%");
   server_rsa_cert_ = false;
@@ -813,30 +811,6 @@ TEST_P(SslCertficateIntegrationTest, ServerEcdsaClientRsaOnlyWithAccessLog) {
   }
 }
 
-// Server has only an ECDSA certificate, client is only RSA capable, leads to a connection fail.
-TEST_P(SslCertficateIntegrationTest, ServerEcdsaClientRsaOnlyWithAccessLogOriginalFormat) {
-  TestScopedRuntime scoped_runtime;
-  scoped_runtime.mergeValues(
-      {{"envoy.reloadable_features.ssl_transport_failure_reason_format", "false"}});
-  useListenerAccessLog("DOWNSTREAM_TRANSPORT_FAILURE_REASON=%DOWNSTREAM_TRANSPORT_FAILURE_REASON% "
-                       "FILTER_CHAIN_NAME=%FILTER_CHAIN_NAME%");
-  server_rsa_cert_ = false;
-  server_ecdsa_cert_ = true;
-  initialize();
-  auto codec_client =
-      makeRawHttpConnection(makeSslClientConnection(rsaOnlyClientOptions()), absl::nullopt);
-  EXPECT_FALSE(codec_client->connected());
-
-  auto log_result = waitForAccessLog(listener_access_log_name_);
-  if (tls_version_ == envoy::extensions::transport_sockets::tls::v3::TlsParameters::TLSv1_3) {
-    EXPECT_EQ(log_result, "DOWNSTREAM_TRANSPORT_FAILURE_REASON=TLS_error:_268435709:SSL_routines:"
-                          "OPENSSL_internal:NO_COMMON_SIGNATURE_ALGORITHMS FILTER_CHAIN_NAME=-");
-  } else {
-    EXPECT_EQ(log_result, "DOWNSTREAM_TRANSPORT_FAILURE_REASON=TLS_error:_268435640:"
-                          "SSL_routines:OPENSSL_internal:NO_SHARED_CIPHER FILTER_CHAIN_NAME=-");
-  }
-}
-
 // Server with RSA/ECDSA certificates and a client with only RSA cipher suites works.
 // Test empty access log with successful connection.
 TEST_P(SslCertficateIntegrationTest, ServerRsaEcdsaClientRsaOnlyWithAccessLog) {