CASSANDRASC-82: Expose additional SSL configuration options for the Sidecar Service

Patch by Francisco Guerrero; Reviewed by Doug Rohrer, Yifan Cai for CASSANDRASC-82
diff --git a/CHANGES.txt b/CHANGES.txt
index 8e38818..a457c5d 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,5 +1,6 @@
 1.0.0
 -----
+ * Expose additional SSL configuration options for the Sidecar Service (CASSANDRASC-82)
  * Expose additional node settings (CASSANDRASC-84)
  * Sidecar does not handle keyspaces and table names with mixed case (CASSANDRASC-76)
  * Require gossip to be enabled for ring and token ranges mapping endpoints (CASSANDRASC-83)
diff --git a/src/main/dist/conf/sidecar.yaml b/src/main/dist/conf/sidecar.yaml
index 884e183..6104f69 100644
--- a/src/main/dist/conf/sidecar.yaml
+++ b/src/main/dist/conf/sidecar.yaml
@@ -113,7 +113,12 @@
 #    use_openssl: true
 #    handshake_timeout_sec: 10
 #    client_auth: NONE # valid options are NONE, REQUEST, REQUIRED
+#    accepted_protocols:
+#     - TLSv1.2
+#     - TLSv1.3
+#    cipher_suites: []
 #    keystore:
+#      type: PKCS12
 #      path: "path/to/keystore.p12"
 #      password: password
 #      check_interval_sec: 300
diff --git a/src/main/java/org/apache/cassandra/sidecar/config/SslConfiguration.java b/src/main/java/org/apache/cassandra/sidecar/config/SslConfiguration.java
index 768d197..2205e35 100644
--- a/src/main/java/org/apache/cassandra/sidecar/config/SslConfiguration.java
+++ b/src/main/java/org/apache/cassandra/sidecar/config/SslConfiguration.java
@@ -18,6 +18,8 @@
 
 package org.apache.cassandra.sidecar.config;
 
+import java.util.List;
+
 /**
  * Encapsulates SSL Configuration
  */
@@ -54,6 +56,22 @@
     String clientAuth();
 
     /**
+     * Return a list of the enabled cipher suites. The list of cipher suites must be provided in the
+     * desired order for its intended use.
+     *
+     * @return the enabled cipher suites
+     */
+    List<String> cipherSuites();
+
+    /**
+     * Returns a list of enabled SSL/TLS protocols. The list of accepted protocols must be provided in the
+     * desired order of use.
+     *
+     * @return the enabled SSL/TLS protocols
+     */
+    List<String> secureTransportProtocols();
+
+    /**
      * @return {@code true} if the keystore is configured, and the {@link KeyStoreConfiguration#path()} and
      * {@link KeyStoreConfiguration#password()} parameters are provided
      */
diff --git a/src/main/java/org/apache/cassandra/sidecar/config/yaml/SslConfigurationImpl.java b/src/main/java/org/apache/cassandra/sidecar/config/yaml/SslConfigurationImpl.java
index fbb7677..0239121 100644
--- a/src/main/java/org/apache/cassandra/sidecar/config/yaml/SslConfigurationImpl.java
+++ b/src/main/java/org/apache/cassandra/sidecar/config/yaml/SslConfigurationImpl.java
@@ -18,7 +18,10 @@
 
 package org.apache.cassandra.sidecar.config.yaml;
 
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
 import java.util.stream.Collectors;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
@@ -36,6 +39,8 @@
     public static final boolean DEFAULT_USE_OPEN_SSL = true;
     public static final long DEFAULT_HANDSHAKE_TIMEOUT_SECONDS = 10L;
     public static final String DEFAULT_CLIENT_AUTH = "NONE";
+    public static final List<String> DEFAULT_SECURE_TRANSPORT_PROTOCOLS
+    = Collections.unmodifiableList(Arrays.asList("TLSv1.2", "TLSv1.3"));
 
 
     @JsonProperty("enabled")
@@ -49,6 +54,12 @@
 
     protected String clientAuth;
 
+    @JsonProperty(value = "cipher_suites")
+    protected final List<String> cipherSuites;
+
+    @JsonProperty(value = "accepted_protocols")
+    protected final List<String> secureTransportProtocols;
+
     @JsonProperty("keystore")
     protected final KeyStoreConfiguration keystore;
 
@@ -68,6 +79,8 @@
         setClientAuth(builder.clientAuth);
         keystore = builder.keystore;
         truststore = builder.truststore;
+        cipherSuites = builder.cipherSuites;
+        secureTransportProtocols = builder.secureTransportProtocols;
     }
 
     /**
@@ -100,6 +113,9 @@
         return handshakeTimeoutInSeconds;
     }
 
+    /**
+     * {@inheritDoc}
+     */
     @Override
     @JsonProperty(value = "client_auth", defaultValue = "NONE")
     public String clientAuth()
@@ -131,6 +147,26 @@
      * {@inheritDoc}
      */
     @Override
+    @JsonProperty(value = "cipher_suites")
+    public List<String> cipherSuites()
+    {
+        return cipherSuites;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    @JsonProperty(value = "accepted_protocols")
+    public List<String> secureTransportProtocols()
+    {
+        return secureTransportProtocols;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
     @JsonProperty("keystore")
     public KeyStoreConfiguration keystore()
     {
@@ -166,12 +202,14 @@
      */
     public static class Builder implements DataObjectBuilder<Builder, SslConfigurationImpl>
     {
-        private boolean enabled = DEFAULT_SSL_ENABLED;
-        private boolean useOpenSsl = DEFAULT_USE_OPEN_SSL;
-        private long handshakeTimeoutInSeconds = DEFAULT_HANDSHAKE_TIMEOUT_SECONDS;
-        private String clientAuth = DEFAULT_CLIENT_AUTH;
-        private KeyStoreConfiguration keystore = null;
-        private KeyStoreConfiguration truststore = null;
+        protected boolean enabled = DEFAULT_SSL_ENABLED;
+        protected boolean useOpenSsl = DEFAULT_USE_OPEN_SSL;
+        protected long handshakeTimeoutInSeconds = DEFAULT_HANDSHAKE_TIMEOUT_SECONDS;
+        protected String clientAuth = DEFAULT_CLIENT_AUTH;
+        protected List<String> cipherSuites = Collections.emptyList();
+        protected List<String> secureTransportProtocols = DEFAULT_SECURE_TRANSPORT_PROTOCOLS;
+        protected KeyStoreConfiguration keystore = null;
+        protected KeyStoreConfiguration truststore = null;
 
         protected Builder()
         {
@@ -191,8 +229,7 @@
          */
         public Builder enabled(boolean enabled)
         {
-            this.enabled = enabled;
-            return this;
+            return update(b -> b.enabled = enabled);
         }
 
         /**
@@ -203,8 +240,7 @@
          */
         public Builder useOpenSsl(boolean useOpenSsl)
         {
-            this.useOpenSsl = useOpenSsl;
-            return this;
+            return update(b -> b.useOpenSsl = useOpenSsl);
         }
 
         /**
@@ -215,8 +251,7 @@
          */
         public Builder handshakeTimeoutInSeconds(long handshakeTimeoutInSeconds)
         {
-            this.handshakeTimeoutInSeconds = handshakeTimeoutInSeconds;
-            return this;
+            return update(b -> b.handshakeTimeoutInSeconds = handshakeTimeoutInSeconds);
         }
 
         /**
@@ -227,8 +262,29 @@
          */
         public Builder clientAuth(String clientAuth)
         {
-            this.clientAuth = clientAuth;
-            return this;
+            return update(b -> b.clientAuth = clientAuth);
+        }
+
+        /**
+         * Sets the {@code cipherSuites} and returns a reference to this Builder enabling method chaining.
+         *
+         * @param cipherSuites the {@code cipherSuites} to set
+         * @return a reference to this Builder
+         */
+        public Builder cipherSuites(List<String> cipherSuites)
+        {
+            return update(b -> b.cipherSuites = new ArrayList<>(cipherSuites));
+        }
+
+        /**
+         * Sets the {@code secureTransportProtocols} and returns a reference to this Builder enabling method chaining.
+         *
+         * @param secureTransportProtocols the {@code secureTransportProtocols} to set
+         * @return a reference to this Builder
+         */
+        public Builder secureTransportProtocols(List<String> secureTransportProtocols)
+        {
+            return update(b -> b.secureTransportProtocols = new ArrayList<>(secureTransportProtocols));
         }
 
         /**
@@ -239,8 +295,7 @@
          */
         public Builder keystore(KeyStoreConfiguration keystore)
         {
-            this.keystore = keystore;
-            return this;
+            return update(b -> b.keystore = keystore);
         }
 
         /**
@@ -251,8 +306,7 @@
          */
         public Builder truststore(KeyStoreConfiguration truststore)
         {
-            this.truststore = truststore;
-            return this;
+            return update(b -> b.truststore = truststore);
         }
 
         /**
diff --git a/src/main/java/org/apache/cassandra/sidecar/server/HttpServerOptionsProvider.java b/src/main/java/org/apache/cassandra/sidecar/server/HttpServerOptionsProvider.java
index 6ee3e65..fad0b81 100644
--- a/src/main/java/org/apache/cassandra/sidecar/server/HttpServerOptionsProvider.java
+++ b/src/main/java/org/apache/cassandra/sidecar/server/HttpServerOptionsProvider.java
@@ -18,6 +18,7 @@
 
 package org.apache.cassandra.sidecar.server;
 
+import java.util.LinkedHashSet;
 import java.util.function.Function;
 
 import org.apache.commons.lang3.SystemUtils;
@@ -71,7 +72,14 @@
         if (ssl != null && ssl.enabled())
         {
             options.setClientAuth(ClientAuth.valueOf(ssl.clientAuth()))
-                   .setSsl(true);
+                   .setSsl(true)
+                   // Use LinkedHashSet to preserve input order
+                   .setEnabledSecureTransportProtocols(new LinkedHashSet<>(ssl.secureTransportProtocols()));
+
+            for (String cipherSuite : ssl.cipherSuites())
+            {
+                options.addEnabledCipherSuite(cipherSuite);
+            }
 
             if (ssl.preferOpenSSL() && OpenSSLEngineOptions.isAvailable())
             {
diff --git a/src/test/integration/org/apache/cassandra/sidecar/IntegrationTestBase.java b/src/test/integration/org/apache/cassandra/sidecar/IntegrationTestBase.java
index 01459d3..32dd6a8 100644
--- a/src/test/integration/org/apache/cassandra/sidecar/IntegrationTestBase.java
+++ b/src/test/integration/org/apache/cassandra/sidecar/IntegrationTestBase.java
@@ -92,7 +92,7 @@
         integrationTestModule.setCassandraTestContext(sidecarTestContext);
 
         server = injector.getInstance(Server.class);
-
+        client = WebClient.create(vertx);
         VertxTestContext context = new VertxTestContext();
 
         if (sidecarTestContext.isClusterBuilt())
diff --git a/src/test/java/org/apache/cassandra/sidecar/config/SidecarConfigurationTest.java b/src/test/java/org/apache/cassandra/sidecar/config/SidecarConfigurationTest.java
index b0378a7..23d7001 100644
--- a/src/test/java/org/apache/cassandra/sidecar/config/SidecarConfigurationTest.java
+++ b/src/test/java/org/apache/cassandra/sidecar/config/SidecarConfigurationTest.java
@@ -193,16 +193,16 @@
         assertThat(i1.jmxRolePassword()).isEqualTo("controlPassword");
 
         // service configuration
-        validateDefaultServiceConfiguration(config.serviceConfiguration());
+        validateServiceConfigurationFromYaml(config.serviceConfiguration());
 
         // ssl configuration
         assertThat(config.sslConfiguration()).isNull();
 
         // health check configuration
-        validateHealthCheckConfiguration(config.healthCheckConfiguration());
+        validateHealthCheckConfigurationFromYaml(config.healthCheckConfiguration());
 
         // cassandra input validation configuration
-        validateDefaultCassandraInputValidationConfiguration(config.cassandraInputValidationConfiguration());
+        validateCassandraInputValidationConfigurationFromYaml(config.cassandraInputValidationConfiguration());
     }
 
     void validateMultipleInstancesSidecarConfiguration(SidecarConfiguration config, boolean withSslConfiguration)
@@ -251,12 +251,12 @@
         assertThat(i3.jmxSslEnabled()).isFalse();
 
         // service configuration
-        validateDefaultServiceConfiguration(config.serviceConfiguration());
+        validateServiceConfigurationFromYaml(config.serviceConfiguration());
 
         // ssl configuration
         if (withSslConfiguration)
         {
-            validateDefaultSslConfiguration(config.sslConfiguration());
+            validateSslConfigurationFromYaml(config.sslConfiguration());
         }
         else
         {
@@ -264,13 +264,13 @@
         }
 
         // health check configuration
-        validateHealthCheckConfiguration(config.healthCheckConfiguration());
+        validateHealthCheckConfigurationFromYaml(config.healthCheckConfiguration());
 
         // cassandra input validation configuration
-        validateDefaultCassandraInputValidationConfiguration(config.cassandraInputValidationConfiguration());
+        validateCassandraInputValidationConfigurationFromYaml(config.cassandraInputValidationConfiguration());
     }
 
-    void validateDefaultServiceConfiguration(ServiceConfiguration serviceConfiguration)
+    void validateServiceConfigurationFromYaml(ServiceConfiguration serviceConfiguration)
     {
         assertThat(serviceConfiguration).isNotNull();
         assertThat(serviceConfiguration.host()).isEqualTo("0.0.0.0");
@@ -299,14 +299,14 @@
         assertThat(trafficShaping.checkIntervalForStatsMillis()).isEqualTo(3000L);
     }
 
-    private void validateHealthCheckConfiguration(HealthCheckConfiguration config)
+    private void validateHealthCheckConfigurationFromYaml(HealthCheckConfiguration config)
     {
         assertThat(config).isNotNull();
         assertThat(config.initialDelayMillis()).isEqualTo(100);
         assertThat(config.checkIntervalMillis()).isEqualTo(30_000);
     }
 
-    void validateDefaultCassandraInputValidationConfiguration(CassandraInputValidationConfiguration config)
+    void validateCassandraInputValidationConfigurationFromYaml(CassandraInputValidationConfiguration config)
     {
         assertThat(config).isNotNull();
         assertThat(config.forbiddenKeyspaces()).containsExactlyInAnyOrder("system_schema",
@@ -323,7 +323,7 @@
         assertThat(config.allowedPatternForRestrictedComponentName()).isEqualTo("[a-zA-Z0-9_-]+(.db|TOC.txt)");
     }
 
-    void validateDefaultSslConfiguration(SslConfiguration config)
+    void validateSslConfigurationFromYaml(SslConfiguration config)
     {
         assertThat(config).isNotNull();
         assertThat(config.enabled()).isTrue();
@@ -331,6 +331,7 @@
         assertThat(config.handshakeTimeoutInSeconds()).isEqualTo(25L);
         assertThat(config.clientAuth()).isEqualTo("REQUEST");
         assertThat(config.keystore()).isNotNull();
+        assertThat(config.keystore().type()).isEqualTo("PKCS12");
         assertThat(config.keystore().path()).isEqualTo("path/to/keystore.p12");
         assertThat(config.keystore().password()).isEqualTo("password");
         assertThat(config.keystore().reloadStore()).isTrue();
@@ -340,6 +341,15 @@
         assertThat(config.truststore().password()).isEqualTo("password");
         assertThat(config.truststore().reloadStore()).isFalse();
         assertThat(config.truststore().checkIntervalInSeconds()).isEqualTo(-1);
+        assertThat(config.secureTransportProtocols()).containsExactly("TLSv1.3");
+        assertThat(config.cipherSuites()).contains("TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
+                                                   "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+                                                   "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
+                                                   "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
+                                                   "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
+                                                   "TLS_RSA_WITH_AES_128_GCM_SHA256",
+                                                   "TLS_RSA_WITH_AES_128_CBC_SHA",
+                                                   "TLS_RSA_WITH_AES_256_CBC_SHA");
     }
 
     private Path yaml(String resourceName)
diff --git a/src/test/java/org/apache/cassandra/sidecar/config/yaml/SslConfigurationImplTest.java b/src/test/java/org/apache/cassandra/sidecar/config/yaml/SslConfigurationImplTest.java
new file mode 100644
index 0000000..d38bc5a
--- /dev/null
+++ b/src/test/java/org/apache/cassandra/sidecar/config/yaml/SslConfigurationImplTest.java
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.sidecar.config.yaml;
+
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Unit tests for {@link SslConfigurationImpl}
+ */
+class SslConfigurationImplTest
+{
+    @Test
+    void testDefaults()
+    {
+        SslConfigurationImpl sslConfiguration = new SslConfigurationImpl();
+        assertThat(sslConfiguration).isNotNull();
+        assertThat(sslConfiguration.enabled()).isFalse();
+        assertThat(sslConfiguration.preferOpenSSL()).isTrue();
+        assertThat(sslConfiguration.handshakeTimeoutInSeconds()).isEqualTo(10);
+        assertThat(sslConfiguration.clientAuth()).isEqualTo("NONE");
+        assertThat(sslConfiguration.cipherSuites()).isEmpty();
+        assertThat(sslConfiguration.secureTransportProtocols()).containsExactly("TLSv1.2", "TLSv1.3");
+        assertThat(sslConfiguration.keystore()).isNull();
+        assertThat(sslConfiguration.isTrustStoreConfigured()).isFalse();
+        assertThat(sslConfiguration.truststore()).isNull();
+    }
+}
diff --git a/src/test/java/org/apache/cassandra/sidecar/server/ServerSSLTest.java b/src/test/java/org/apache/cassandra/sidecar/server/ServerSSLTest.java
index 82cc746..f5001af 100644
--- a/src/test/java/org/apache/cassandra/sidecar/server/ServerSSLTest.java
+++ b/src/test/java/org/apache/cassandra/sidecar/server/ServerSSLTest.java
@@ -342,6 +342,43 @@
     }
 
     @Test
+    void failsOnClientUsingUnacceptableProtocol(VertxTestContext context)
+    {
+        SslConfigurationImpl ssl =
+        SslConfigurationImpl.builder()
+                            .enabled(true)
+                            .useOpenSsl(true)
+                            .keystore(p12KeyStore)
+                            .truststore(p12TrustStore)
+                            .build();
+
+        builder.sslConfiguration(ssl)
+               .serviceConfiguration(ServiceConfigurationImpl.builder()
+                                                             .host("127.0.0.1")
+                                                             .port(9043)
+                                                             .build());
+
+        server = server();
+
+        // Remove default protocols enabled for the server and only add an unsupported protocol to the client
+        WebClientOptions options = new WebClientOptions().setSsl(true)
+                                                         .addEnabledSecureTransportProtocol("TLSv1.1")
+                                                         .removeEnabledSecureTransportProtocol("TLSv1.2")
+                                                         .removeEnabledSecureTransportProtocol("TLSv1.3");
+        server.start()
+              .compose(s -> validateHealthEndpoint(clientWithP12Keystore(options, true, false)))
+              .onComplete(context.failing(throwable -> {
+                  assertThat(throwable).isNotNull()
+                                       .isInstanceOf(SSLHandshakeException.class)
+                                       .hasMessageContaining("Failed to create SSL connection")
+                                       .hasCauseInstanceOf(SSLHandshakeException.class)
+                                       .hasRootCauseMessage("No appropriate protocol (protocol is disabled " +
+                                                            "or cipher suites are inappropriate)");
+                  context.completeNow();
+              }));
+    }
+
+    @Test
     void testHotReloadOfServerCertificates(VertxTestContext context)
     {
         KeyStoreConfigurationImpl expiredP12KeyStore =
@@ -437,6 +474,11 @@
     WebClient clientWithP12Keystore(boolean includeTrustStore, boolean includeClientKeyStore)
     {
         WebClientOptions options = new WebClientOptions().setSsl(true);
+        return clientWithP12Keystore(options, includeTrustStore, includeClientKeyStore);
+    }
+
+    WebClient clientWithP12Keystore(WebClientOptions options, boolean includeTrustStore, boolean includeClientKeyStore)
+    {
         if (includeTrustStore)
         {
             options.setTrustOptions(new PfxOptions().setPath(trustStoreP12Path.toString())
diff --git a/src/test/resources/config/sidecar_multiple_instances.yaml b/src/test/resources/config/sidecar_multiple_instances.yaml
index 72fc1df..62c8903 100644
--- a/src/test/resources/config/sidecar_multiple_instances.yaml
+++ b/src/test/resources/config/sidecar_multiple_instances.yaml
@@ -112,7 +112,12 @@
 #    use_openssl: true
 #    handshake_timeout_sec: 10
 #    client_auth: NONE # valid options are NONE, REQUEST, REQUIRED
+#    accepted_protocols:
+#     - TLSv1.2
+#     - TLSv1.3
+#    cipher_suites: []
 #    keystore:
+#      type: PKCS12
 #      path: "path/to/keystore.p12"
 #      password: password
 #      check_interval_sec: 300
diff --git a/src/test/resources/config/sidecar_single_instance.yaml b/src/test/resources/config/sidecar_single_instance.yaml
index 64c9bfd..c2d3a85 100644
--- a/src/test/resources/config/sidecar_single_instance.yaml
+++ b/src/test/resources/config/sidecar_single_instance.yaml
@@ -65,7 +65,12 @@
 #    use_openssl: true
 #    handshake_timeout_sec: 10
 #    client_auth: NONE # valid options are NONE, REQUEST, REQUIRED
+#    accepted_protocols:
+#     - TLSv1.2
+#     - TLSv1.3
+#    cipher_suites: []
 #    keystore:
+#      type: PKCS12
 #      path: "path/to/keystore.p12"
 #      password: password
 #      check_interval_sec: 300
diff --git a/src/test/resources/config/sidecar_ssl.yaml b/src/test/resources/config/sidecar_ssl.yaml
index afe7731..bec5ffd 100644
--- a/src/test/resources/config/sidecar_ssl.yaml
+++ b/src/test/resources/config/sidecar_ssl.yaml
@@ -112,7 +112,19 @@
   use_openssl: false
   handshake_timeout_sec: 25
   client_auth: REQUEST # valid options are NONE, REQUEST, REQUIRED
+  accepted_protocols:
+   - TLSv1.3
+  cipher_suites:
+   - TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
+   - TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
+   - TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
+   - TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA
+   - TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA
+   - TLS_RSA_WITH_AES_128_GCM_SHA256
+   - TLS_RSA_WITH_AES_128_CBC_SHA
+   - TLS_RSA_WITH_AES_256_CBC_SHA
   keystore:
+    type: PKCS12
     path: "path/to/keystore.p12"
     password: password
     check_interval_sec: 300