tls: add enable_client_cipher_preference option (#35106)

Fixes #34604

Signed-off-by: Arul Thileeban Sagayam <[email protected]>

api/envoy/extensions/transport_sockets/tls/v3/tls.proto

@@ -63,7 +63,7 @@ message UpstreamTlsContext {
   google.protobuf.BoolValue enforce_rsa_key_usage = 5;
 }
 
-// [#next-free-field: 11]
+// [#next-free-field: 12]
 message DownstreamTlsContext {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.api.v2.auth.DownstreamTlsContext";
@@ -140,6 +140,11 @@ message DownstreamTlsContext {
   // If the client provides SNI but no such cert matched, it will decide to full scan certificates or not based on this config.
   // Defaults to false. See more details in :ref:`Multiple TLS certificates <arch_overview_ssl_cert_select>`.
   google.protobuf.BoolValue full_scan_certs_on_sni_mismatch = 9;
+
+  // By default, Envoy as a server uses its preferred cipher during the handshake.
+  // Setting this to true would allow the downstream client's preferred cipher to be used instead.
+  // Has no effect when using TLSv1_3.
+  bool prefer_client_ciphers = 11;
 }
 
 // TLS key log configuration.

changelogs/current.yaml

@@ -33,6 +33,11 @@ removed_config_or_runtime:
     Removed ``envoy.reloadable_features.dns_cache_set_first_resolve_complete`` runtime flag and legacy code paths.
 
 new_features:
+- area: tls
+  change: |
+    Added :ref:`prefer_client_ciphers
+    <envoy_v3_api_field_extensions.transport_sockets.tls.v3.DownstreamTlsContext.prefer_client_ciphers>`
+    to support enabling client cipher preference instead of server's for TLS handshakes.
 - area: ext_authz
   change: |
     Added config field

envoy/ssl/context_config.h

@@ -199,6 +199,11 @@ class ServerContextConfig : public virtual ContextConfig {
    * downstream TLS handshake, false otherwise.
    */
   virtual bool fullScanCertsOnSNIMismatch() const PURE;
+
+  /**
+   * @return true if the client cipher preference is enabled, false otherwise.
+   */
+  virtual bool preferClientCiphers() const PURE;
 };
 
 using ServerContextConfigPtr = std::unique_ptr<ServerContextConfig>;

source/common/tls/context_impl.cc

@@ -308,11 +308,6 @@ ContextImpl::ContextImpl(Stats::Scope& scope, const Envoy::Ssl::ContextConfig& c
     }
   }
 
-  // use the server's cipher list preferences
-  for (auto& ctx : tls_contexts_) {
-    SSL_CTX_set_options(ctx.ssl_ctx_.get(), SSL_OP_CIPHER_SERVER_PREFERENCE);
-  }
-
   parsed_alpn_protocols_ = parseAlpnProtocols(config.alpnProtocols(), creation_status);
   SET_AND_RETURN_IF_NOT_OK(creation_status, creation_status);
 

source/common/tls/server_context_config_impl.cc

@@ -123,7 +123,8 @@ ServerContextConfigImpl::ServerContextConfigImpl(
       disable_stateless_session_resumption_(getStatelessSessionResumptionDisabled(config)),
       disable_stateful_session_resumption_(config.disable_stateful_session_resumption()),
       full_scan_certs_on_sni_mismatch_(
-          PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, full_scan_certs_on_sni_mismatch, false)) {
+          PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, full_scan_certs_on_sni_mismatch, false)),
+      prefer_client_ciphers_(config.prefer_client_ciphers()) {
   SET_AND_RETURN_IF_NOT_OK(creation_status, creation_status);
   if (session_ticket_keys_provider_ != nullptr) {
     // Validate tls session ticket keys early to reject bad sds updates.

source/common/tls/server_context_config_impl.h

@@ -40,6 +40,7 @@ class ServerContextConfigImpl : public ContextConfigImpl, public Envoy::Ssl::Ser
   }
 
   bool fullScanCertsOnSNIMismatch() const override { return full_scan_certs_on_sni_mismatch_; }
+  bool preferClientCiphers() const override { return prefer_client_ciphers_; }
 
 private:
   ServerContextConfigImpl(
@@ -71,6 +72,7 @@ class ServerContextConfigImpl : public ContextConfigImpl, public Envoy::Ssl::Ser
   const bool disable_stateless_session_resumption_;
   const bool disable_stateful_session_resumption_;
   bool full_scan_certs_on_sni_mismatch_;
+  const bool prefer_client_ciphers_;
 };
 
 } // namespace Tls

source/common/tls/server_context_impl.cc

@@ -159,6 +159,11 @@ ServerContextImpl::ServerContextImpl(Stats::Scope& scope,
           this);
     }
 
+    if (!config.preferClientCiphers()) {
+      // Use server cipher preference based on config.
+      SSL_CTX_set_options(ctx.ssl_ctx_.get(), SSL_OP_CIPHER_SERVER_PREFERENCE);
+    }
+
     // If the handshaker handles session tickets natively, don't call
     // `SSL_CTX_set_tlsext_ticket_key_cb`.
     if (config.disableStatelessSessionResumption()) {

test/common/tls/BUILD

@@ -145,6 +145,7 @@ envoy_cc_test_library(
         "ssl_test_utility.h",
     ],
     deps = [
+        "//source/common/tls:ssl_socket_lib",
         "//source/common/tls:utility_lib",
         "//test/test_common:environment_lib",
     ],
@@ -197,6 +198,7 @@ envoy_cc_test(
     name = "handshaker_factory_test",
     srcs = ["handshaker_factory_test.cc"],
     deps = [
+        ":ssl_test_utils",
         "//source/common/stream_info:stream_info_lib",
         "//source/common/tls:context_lib",
         "//source/common/tls:server_context_config_lib",

test/common/tls/context_impl_test.cc

@@ -13,6 +13,7 @@
 #include "source/common/tls/context_config_impl.h"
 #include "source/common/tls/context_impl.h"
 #include "source/common/tls/server_context_config_impl.h"
+#include "source/common/tls/server_ssl_socket.h"
 #include "source/common/tls/utility.h"
 
 #include "test/common/tls/ssl_certs_test.h"
@@ -141,6 +142,56 @@ TEST_F(SslContextImplTest, TestCipherSuites) {
             "ciphers were rejected when tried individually: BOGUS1-SHA256, BOGUS2-SHA");
 }
 
+// Envoy's default cipher preference is server's.
+TEST_F(SslContextImplTest, TestServerCipherPreference) {
+  const std::string yaml = R"EOF(
+  common_tls_context:
+    tls_certificates:
+      certificate_chain:
+        filename: "{{ test_rundir }}/test/common/tls/test_data/unittest_cert.pem"
+      private_key:
+        filename: "{{ test_rundir }}/test/common/tls/test_data/unittest_key.pem"
+  )EOF";
+
+  envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context;
+  TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), tls_context);
+  auto cfg = ServerContextConfigImpl::create(tls_context, factory_context_).value();
+  ASSERT_FALSE(cfg.get()->preferClientCiphers());
+
+  auto socket_factory = *Extensions::TransportSockets::Tls::ServerSslSocketFactory::create(
+      std::move(cfg), manager_, *store_.rootScope(), {});
+  std::unique_ptr<Network::TransportSocket> socket =
+      socket_factory->createDownstreamTransportSocket();
+  SSL_CTX* ssl_ctx = extractSslCtx(socket.get());
+
+  EXPECT_TRUE(SSL_CTX_get_options(ssl_ctx) & SSL_OP_CIPHER_SERVER_PREFERENCE);
+}
+
+TEST_F(SslContextImplTest, TestPreferClientCiphers) {
+  const std::string yaml = R"EOF(
+  common_tls_context:
+    tls_certificates:
+      certificate_chain:
+        filename: "{{ test_rundir }}/test/common/tls/test_data/unittest_cert.pem"
+      private_key:
+        filename: "{{ test_rundir }}/test/common/tls/test_data/unittest_key.pem"
+  prefer_client_ciphers: true
+  )EOF";
+
+  envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context;
+  TestUtility::loadFromYaml(TestEnvironment::substitute(yaml), tls_context);
+  auto cfg = ServerContextConfigImpl::create(tls_context, factory_context_).value();
+  ASSERT_TRUE(cfg.get()->preferClientCiphers());
+
+  auto socket_factory = *Extensions::TransportSockets::Tls::ServerSslSocketFactory::create(
+      std::move(cfg), manager_, *store_.rootScope(), {});
+  std::unique_ptr<Network::TransportSocket> socket =
+      socket_factory->createDownstreamTransportSocket();
+  SSL_CTX* ssl_ctx = extractSslCtx(socket.get());
+
+  EXPECT_FALSE(SSL_CTX_get_options(ssl_ctx) & SSL_OP_CIPHER_SERVER_PREFERENCE);
+}
+
 TEST_F(SslContextImplTest, TestExpiringCert) {
   const std::string yaml = R"EOF(
   common_tls_context:

test/common/tls/handshaker_factory_test.cc

@@ -11,6 +11,7 @@
 #include "source/common/tls/ssl_handshaker.h"
 #include "source/server/process_context_impl.h"
 
+#include "test/common/tls/ssl_test_utility.h"
 #include "test/mocks/network/connection.h"
 #include "test/mocks/server/transport_socket_factory_context.h"
 #include "test/test_common/registry.h"
@@ -105,14 +106,6 @@ class HandshakerFactoryTest : public testing::Test {
     custom_handshaker->mutable_typed_config()->PackFrom(ProtobufWkt::StringValue());
   }
 
-  // Helper for downcasting a socket to a test socket so we can examine its
-  // SSL_CTX.
-  SSL_CTX* extractSslCtx(Network::TransportSocket* socket) {
-    SslSocket* ssl_socket = dynamic_cast<SslSocket*>(socket);
-    SSL* ssl = ssl_socket->rawSslForTest();
-    return SSL_get_SSL_CTX(ssl);
-  }
-
   NiceMock<Server::Configuration::MockServerFactoryContext> server_factory_context_;
   Stats::IsolatedStoreImpl stats_store_;
   std::unique_ptr<Extensions::TransportSockets::Tls::ContextManagerImpl> context_manager_;

test/common/tls/integration/ssl_integration_test.cc

@@ -275,6 +275,44 @@ TEST_P(SslIntegrationTest, RouterDownstreamDisconnectBeforeResponseComplete) {
   checkStats();
 }
 
+// Test server preference of cipher suites. Server order is ECDHE-RSA-AES128-GCM-SHA256
+// followed by ECDHE-RSA-AES256-GCM-SHA384. "ECDHE-RSA-AES128-GCM-SHA256" should be used based on
+// server preference.
+TEST_P(SslIntegrationTest, TestServerCipherPreference) {
+  server_ciphers_.push_back("ECDHE-RSA-AES128-GCM-SHA256");
+  server_ciphers_.push_back("ECDHE-RSA-AES256-GCM-SHA384");
+  initialize();
+  codec_client_ = makeHttpConnection(makeSslClientConnection(
+      ClientSslTransportOptions{}
+          .setTlsVersion(envoy::extensions::transport_sockets::tls::v3::TlsParameters::TLSv1_2)
+          .setCipherSuites({"ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256"})));
+  auto response =
+      sendRequestAndWaitForResponse(default_request_headers_, 0, default_response_headers_, 0);
+
+  const std::string counter_name = listenerStatPrefix("ssl.ciphers.ECDHE-RSA-AES128-GCM-SHA256");
+  Stats::CounterSharedPtr counter = test_server_->counter(counter_name);
+  EXPECT_EQ(1, test_server_->counter(counter_name)->value());
+}
+
+// Test client preference of cipher suites. Same server preference is followed as in the previous.
+// "ECDHE-RSA-AES256-GCM-SHA384" should be used based on client preference.
+TEST_P(SslIntegrationTest, ClientCipherPreference) {
+  prefer_client_ciphers_ = true;
+  server_ciphers_.push_back("ECDHE-RSA-AES128-GCM-SHA256");
+  server_ciphers_.push_back("ECDHE-RSA-AES256-GCM-SHA384");
+  initialize();
+  codec_client_ = makeHttpConnection(makeSslClientConnection(
+      ClientSslTransportOptions{}
+          .setTlsVersion(envoy::extensions::transport_sockets::tls::v3::TlsParameters::TLSv1_2)
+          .setCipherSuites({"ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256"})));
+  auto response =
+      sendRequestAndWaitForResponse(default_request_headers_, 0, default_response_headers_, 0);
+
+  const std::string counter_name = listenerStatPrefix("ssl.ciphers.ECDHE-RSA-AES256-GCM-SHA384");
+  Stats::CounterSharedPtr counter = test_server_->counter(counter_name);
+  EXPECT_EQ(1, test_server_->counter(counter_name)->value());
+}
+
 // This test must be here vs integration_admin_test so that it tests a server with loaded certs.
 TEST_P(SslIntegrationTest, AdminCertEndpoint) {
   DISABLE_IF_ADMIN_DISABLED; // Admin functionality.

test/common/tls/integration/ssl_integration_test_base.cc

@@ -10,8 +10,10 @@ void SslIntegrationTestBase::initialize() {
                                   .setEcdsaCert(server_ecdsa_cert_)
                                   .setEcdsaCertOcspStaple(server_ecdsa_cert_ocsp_staple_)
                                   .setOcspStapleRequired(ocsp_staple_required_)
+                                  .setPreferClientCiphers(prefer_client_ciphers_)
                                   .setTlsV13(server_tlsv1_3_)
                                   .setCurves(server_curves_)
+                                  .setCiphers(server_ciphers_)
                                   .setExpectClientEcdsaCert(client_ecdsa_cert_)
                                   .setTlsKeyLogFilter(keylog_local_, keylog_remote_,
                                                       keylog_local_negative_,

test/common/tls/integration/ssl_integration_test_base.h

@@ -31,11 +31,13 @@ class SslIntegrationTestBase : public HttpIntegrationTest {
 protected:
   bool server_tlsv1_3_{false};
   std::vector<std::string> server_curves_;
+  std::vector<std::string> server_ciphers_;
   bool server_rsa_cert_{true};
   bool server_rsa_cert_ocsp_staple_{false};
   bool server_ecdsa_cert_{false};
   bool server_ecdsa_cert_ocsp_staple_{false};
   bool ocsp_staple_required_{false};
+  bool prefer_client_ciphers_{false};
   bool client_ecdsa_cert_{false};
   // Set this true to debug SSL handshake issues with openssl s_client. The
   // verbose trace will be in the logs, openssl must be installed separately.

test/common/tls/ssl_test_utility.h

@@ -3,6 +3,8 @@
 #include <string>
 #include <vector>
 
+#include "source/common/tls/ssl_socket.h"
+
 #include "test/test_common/environment.h"
 
 #include "gtest/gtest.h"
@@ -36,6 +38,14 @@ inline bssl::UniquePtr<STACK_OF(X509)> readCertChainFromFile(const std::string&
   return certChain;
 }
 
+// Helper for downcasting a socket to a test socket so we can examine its
+// SSL_CTX.
+SSL_CTX* extractSslCtx(Network::TransportSocket* socket) {
+  SslSocket* ssl_socket = dynamic_cast<SslSocket*>(socket);
+  SSL* ssl = ssl_socket->rawSslForTest();
+  return SSL_get_SSL_CTX(ssl);
+}
+
 } // namespace Tls
 } // namespace TransportSockets
 } // namespace Extensions

test/config/utility.cc

@@ -1315,6 +1315,7 @@ void ConfigHelper::addSslConfig(const ServerSslOptions& options) {
     tls_context.set_ocsp_staple_policy(
         envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext::MUST_STAPLE);
   }
+  tls_context.set_prefer_client_ciphers(options.prefer_client_ciphers_);
   filter_chain->mutable_transport_socket()->set_name("envoy.transport_sockets.tls");
   filter_chain->mutable_transport_socket()->mutable_typed_config()->PackFrom(tls_context);
 }
@@ -1506,6 +1507,9 @@ void ConfigHelper::initializeTls(
   for (const auto& curve : options.curves_) {
     common_tls_context.mutable_tls_params()->add_ecdh_curves(curve);
   }
+  for (const auto& cipher : options.ciphers_) {
+    common_tls_context.mutable_tls_params()->add_cipher_suites(cipher);
+  }
   if (options.rsa_cert_) {
     auto* tls_certificate = common_tls_context.add_tls_certificates();
     tls_certificate->mutable_certificate_chain()->set_filename(
@@ -1528,6 +1532,7 @@ void ConfigHelper::initializeTls(
           "test/config/integration/certs/server_ecdsa_ocsp_resp.der"));
     }
   }
+
   if (!options.san_matchers_.empty()) {
     *validation_context->mutable_match_typed_subject_alt_names() = {options.san_matchers_.begin(),
                                                                     options.san_matchers_.end()};

test/config/utility.h

@@ -70,6 +70,16 @@ class ConfigHelper {
       return *this;
     }
 
+    ServerSslOptions& setPreferClientCiphers(bool prefer_client_ciphers) {
+      prefer_client_ciphers_ = prefer_client_ciphers;
+      return *this;
+    }
+
+    ServerSslOptions& setCiphers(const std::vector<std::string>& ciphers) {
+      ciphers_ = ciphers;
+      return *this;
+    }
+
     ServerSslOptions& setCurves(const std::vector<std::string>& curves) {
       curves_ = curves;
       return *this;
@@ -127,9 +137,11 @@ class ConfigHelper {
     bool rsa_cert_ocsp_staple_{true};
     bool ecdsa_cert_{false};
     bool ecdsa_cert_ocsp_staple_{false};
+    bool prefer_client_ciphers_{false};
     bool ocsp_staple_required_{false};
     bool tlsv1_3_{false};
     std::vector<std::string> curves_;
+    std::vector<std::string> ciphers_;
     bool expect_client_ecdsa_cert_{false};
     bool keylog_local_filter_{false};
     bool keylog_remote_filter_{false};

test/mocks/ssl/mocks.h

@@ -126,6 +126,7 @@ class MockServerContextConfig : public ServerContextConfig {
   MOCK_METHOD(const std::string&, cipherSuites, (), (const));
   MOCK_METHOD(const std::string&, ecdhCurves, (), (const));
   MOCK_METHOD(const std::string&, signatureAlgorithms, (), (const));
+  MOCK_METHOD(bool, preferClientCiphers, (), (const));
   MOCK_METHOD(std::vector<std::reference_wrapper<const TlsCertificateConfig>>, tlsCertificates, (),
               (const));
   MOCK_METHOD(const CertificateValidationContextConfig*, certificateValidationContext, (), (const));