jonmeredith commented on a change in pull request #1027: URL: https://github.com/apache/cassandra/pull/1027#discussion_r686188603
########## File path: src/java/org/apache/cassandra/security/DefaultSslContextFactoryImpl.java ########## @@ -0,0 +1,369 @@ +/* + * 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.security; + +import java.io.File; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.security.KeyStore; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Date; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import javax.net.ssl.KeyManagerFactory; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLException; +import javax.net.ssl.TrustManager; +import javax.net.ssl.TrustManagerFactory; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.netty.handler.ssl.CipherSuiteFilter; +import io.netty.handler.ssl.ClientAuth; +import io.netty.handler.ssl.SslContext; +import io.netty.handler.ssl.SslContextBuilder; +import io.netty.handler.ssl.SslProvider; + +/** + * Cassandra's default implementation class for the configuration key {@code ssl_context_factory}. It uses + * file based keystores. + */ +public final class DefaultSslContextFactoryImpl implements ISslContextFactory +{ + private static final Logger logger = LoggerFactory.getLogger(DefaultSslContextFactoryImpl.class); + + @VisibleForTesting + volatile boolean checkedExpiry = false; + + /** + * List of files that trigger hot reloading of SSL certificates + */ + private volatile List<HotReloadableFile> hotReloadableFiles = new ArrayList<>(); + + /* This list is substituted in configurations that have explicitly specified the original "TLS" default, + * by extracting it from the default "TLS" SSL Context instance + */ + static private final List<String> TLS_PROTOCOL_SUBSTITUTION = SSLFactory.tlsInstanceProtocolSubstitution(); + + private final Map<String,Object> parameters; + private final String keystore; + private final String keystore_password; + private final String truststore; + private final String truststore_password; + private final List<String> cipher_suites; + private String protocol; + private List<String> accepted_protocols; + private final String algorithm; + private final String store_type; + private final boolean require_client_auth; + private final boolean require_endpoint_verification; + // ServerEncryptionOptions does not use the enabled flag at all instead using the existing + // internode_encryption option. So we force this private and expose through isEnabled + // so users of ServerEncryptionOptions can't accidentally use this when they should use isEnabled + // Long term we need to refactor ClientEncryptionOptions and ServerEncyrptionOptions to be separate + // classes so we can choose appropriate configuration for each. + // See CASSANDRA-15262 and CASSANDRA-15146 + protected Boolean enabled; + protected Boolean optional; + + /* For test only */ + DefaultSslContextFactoryImpl(){ + parameters = new HashMap<>(); + keystore = "conf/.keystore"; + keystore_password = "cassandra"; + truststore = "conf/.truststore"; + truststore_password = "cassandra"; + cipher_suites = null; + protocol = null; + accepted_protocols = null; + algorithm = null; + store_type = "JKS"; + require_client_auth = false; + require_endpoint_verification = false; + enabled = null; + optional = null; + } + + public DefaultSslContextFactoryImpl(Map<String,Object> parameters) { + this.parameters = parameters; + keystore = getString("keystore"); + keystore_password = getString("keystore_password"); + truststore = getString("truststore"); + truststore_password = getString("truststore_password"); + cipher_suites = getStringList("cipher_suites"); + protocol = getString("protocol"); + accepted_protocols = getStringList("accepted_protocols"); + algorithm = getString("algorithm"); + store_type = getString("store_type", "JKS"); + require_client_auth = getBoolean("require_client_auth", false); + require_endpoint_verification = getBoolean("require_endpoint_verification", false); + enabled = getBoolean("enabled"); + this.optional = getBoolean("optional"); + } + + private String getString(String key, String defaultValue) { + return this.parameters.get(key) == null ? defaultValue : (String)this.parameters.get(key); + } + + private String getString(String key) { + return (String)this.parameters.get(key); + } + + private List<String> getStringList(String key) { + return (List<String>)this.parameters.get(key); + } + + private Boolean getBoolean(String key, boolean defaultValue) { + return this.parameters.get(key) == null ? defaultValue : (Boolean)this.parameters.get(key); + } + + private Boolean getBoolean(String key) { + return (Boolean)this.parameters.get(key); + } + + @Override + public SSLContext createJSSESslContext(boolean buildTruststore) throws SSLException + { + TrustManager[] trustManagers = null; + if (buildTruststore) + trustManagers = buildTrustManagerFactory().getTrustManagers(); + + KeyManagerFactory kmf = buildKeyManagerFactory(); + + try + { + SSLContext ctx = SSLContext.getInstance("TLS"); + ctx.init(kmf.getKeyManagers(), trustManagers, null); + return ctx; + } + catch (Exception e) + { + throw new SSLException("Error creating/initializing the SSL Context", e); + } + } + + @Override + public SslContext createNettySslContext(boolean buildTruststore, SocketType socketType, boolean useOpenSsl, + CipherSuiteFilter cipherFilter) throws SSLException + { + /* + There is a case where the netty/openssl combo might not support using KeyManagerFactory. Specifically, + I've seen this with the netty-tcnative dynamic openssl implementation. Using the netty-tcnative + static-boringssl works fine with KeyManagerFactory. If we want to support all of the netty-tcnative + options, we would need to fall back to passing in a file reference for both a x509 and PKCS#8 private + key file in PEM format (see {@link SslContextBuilder#forServer(File, File, String)}). However, we are + not supporting that now to keep the config/yaml API simple. + */ + KeyManagerFactory kmf = buildKeyManagerFactory(); + SslContextBuilder builder; + if (socketType == SocketType.SERVER) + { + builder = SslContextBuilder.forServer(kmf).clientAuth(this.require_client_auth ? ClientAuth.REQUIRE : + ClientAuth.NONE); + } + else + { + builder = SslContextBuilder.forClient().keyManager(kmf); + } + + builder.sslProvider(useOpenSsl ? SslProvider.OPENSSL : SslProvider.JDK).protocols(getAcceptedProtocols()); + + // only set the cipher suites if the opertor has explicity configured values for it; else, use the default Review comment: typo carried over in the move from `SSLFactory`, `opertor` -> `operator` ########## File path: src/java/org/apache/cassandra/config/EncryptionOptions.java ########## @@ -456,6 +502,8 @@ public int hashCode() result += 31 * (cipher_suites == null ? 0 : cipher_suites.hashCode()); result += 31 * Boolean.hashCode(require_client_auth); result += 31 * Boolean.hashCode(require_endpoint_verification); + result += 31 * (ssl_context_factory == null ? 0 : ssl_context_factory.class_name.hashCode()); + result += 31 * (ssl_context_factory == null ? 0 : ssl_context_factory.parameters.hashCode()); Review comment: We should fix `hashCode()` in `ParameterizedClass` instead, following the "Item 11: Always override hashCode when you override equals" advice from Effective Java given that it overrides `equals`. ########## File path: src/java/org/apache/cassandra/security/DefaultSslContextFactoryImpl.java ########## @@ -0,0 +1,369 @@ +/* + * 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.security; + +import java.io.File; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.security.KeyStore; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Date; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import javax.net.ssl.KeyManagerFactory; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLException; +import javax.net.ssl.TrustManager; +import javax.net.ssl.TrustManagerFactory; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.netty.handler.ssl.CipherSuiteFilter; +import io.netty.handler.ssl.ClientAuth; +import io.netty.handler.ssl.SslContext; +import io.netty.handler.ssl.SslContextBuilder; +import io.netty.handler.ssl.SslProvider; + +/** + * Cassandra's default implementation class for the configuration key {@code ssl_context_factory}. It uses + * file based keystores. + */ +public final class DefaultSslContextFactoryImpl implements ISslContextFactory +{ + private static final Logger logger = LoggerFactory.getLogger(DefaultSslContextFactoryImpl.class); + + @VisibleForTesting + volatile boolean checkedExpiry = false; + + /** + * List of files that trigger hot reloading of SSL certificates + */ + private volatile List<HotReloadableFile> hotReloadableFiles = new ArrayList<>(); + + /* This list is substituted in configurations that have explicitly specified the original "TLS" default, + * by extracting it from the default "TLS" SSL Context instance + */ + static private final List<String> TLS_PROTOCOL_SUBSTITUTION = SSLFactory.tlsInstanceProtocolSubstitution(); + + private final Map<String,Object> parameters; + private final String keystore; + private final String keystore_password; + private final String truststore; + private final String truststore_password; + private final List<String> cipher_suites; + private String protocol; + private List<String> accepted_protocols; + private final String algorithm; + private final String store_type; + private final boolean require_client_auth; + private final boolean require_endpoint_verification; + // ServerEncryptionOptions does not use the enabled flag at all instead using the existing + // internode_encryption option. So we force this private and expose through isEnabled + // so users of ServerEncryptionOptions can't accidentally use this when they should use isEnabled + // Long term we need to refactor ClientEncryptionOptions and ServerEncyrptionOptions to be separate + // classes so we can choose appropriate configuration for each. + // See CASSANDRA-15262 and CASSANDRA-15146 + protected Boolean enabled; + protected Boolean optional; + + /* For test only */ + DefaultSslContextFactoryImpl(){ + parameters = new HashMap<>(); + keystore = "conf/.keystore"; + keystore_password = "cassandra"; + truststore = "conf/.truststore"; + truststore_password = "cassandra"; + cipher_suites = null; + protocol = null; + accepted_protocols = null; + algorithm = null; + store_type = "JKS"; + require_client_auth = false; + require_endpoint_verification = false; + enabled = null; + optional = null; + } + + public DefaultSslContextFactoryImpl(Map<String,Object> parameters) { + this.parameters = parameters; + keystore = getString("keystore"); + keystore_password = getString("keystore_password"); + truststore = getString("truststore"); + truststore_password = getString("truststore_password"); + cipher_suites = getStringList("cipher_suites"); + protocol = getString("protocol"); + accepted_protocols = getStringList("accepted_protocols"); + algorithm = getString("algorithm"); + store_type = getString("store_type", "JKS"); + require_client_auth = getBoolean("require_client_auth", false); + require_endpoint_verification = getBoolean("require_endpoint_verification", false); + enabled = getBoolean("enabled"); + this.optional = getBoolean("optional"); + } + + private String getString(String key, String defaultValue) { + return this.parameters.get(key) == null ? defaultValue : (String)this.parameters.get(key); + } + + private String getString(String key) { + return (String)this.parameters.get(key); + } + + private List<String> getStringList(String key) { + return (List<String>)this.parameters.get(key); + } + + private Boolean getBoolean(String key, boolean defaultValue) { + return this.parameters.get(key) == null ? defaultValue : (Boolean)this.parameters.get(key); + } + + private Boolean getBoolean(String key) { + return (Boolean)this.parameters.get(key); + } + + @Override + public SSLContext createJSSESslContext(boolean buildTruststore) throws SSLException + { + TrustManager[] trustManagers = null; + if (buildTruststore) + trustManagers = buildTrustManagerFactory().getTrustManagers(); + + KeyManagerFactory kmf = buildKeyManagerFactory(); + + try + { + SSLContext ctx = SSLContext.getInstance("TLS"); + ctx.init(kmf.getKeyManagers(), trustManagers, null); + return ctx; + } + catch (Exception e) + { + throw new SSLException("Error creating/initializing the SSL Context", e); + } + } + + @Override + public SslContext createNettySslContext(boolean buildTruststore, SocketType socketType, boolean useOpenSsl, + CipherSuiteFilter cipherFilter) throws SSLException + { + /* + There is a case where the netty/openssl combo might not support using KeyManagerFactory. Specifically, + I've seen this with the netty-tcnative dynamic openssl implementation. Using the netty-tcnative + static-boringssl works fine with KeyManagerFactory. If we want to support all of the netty-tcnative + options, we would need to fall back to passing in a file reference for both a x509 and PKCS#8 private + key file in PEM format (see {@link SslContextBuilder#forServer(File, File, String)}). However, we are + not supporting that now to keep the config/yaml API simple. + */ + KeyManagerFactory kmf = buildKeyManagerFactory(); + SslContextBuilder builder; + if (socketType == SocketType.SERVER) + { + builder = SslContextBuilder.forServer(kmf).clientAuth(this.require_client_auth ? ClientAuth.REQUIRE : + ClientAuth.NONE); + } + else + { + builder = SslContextBuilder.forClient().keyManager(kmf); + } + + builder.sslProvider(useOpenSsl ? SslProvider.OPENSSL : SslProvider.JDK).protocols(getAcceptedProtocols()); + + // only set the cipher suites if the opertor has explicity configured values for it; else, use the default + // for each ssl implemention (jdk or openssl) + if (cipher_suites != null && !cipher_suites.isEmpty()) + builder.ciphers(cipher_suites, cipherFilter); + + if (buildTruststore) + builder.trustManager(buildTrustManagerFactory()); + + try + { + return builder.build(); + } + catch (SSLException e) + { + throw new SSLException("failed to build the final SslContext object for secure connections", e); + } + } Review comment: Why wrap the exception? If we would like to improve the UX for misconfiguration/problems perhaps it would be better to catch exceptions in `InboundConnectionInitiator.getSslHandler` and `org.apache.cassandra.net.OutboundConnectionInitiator.Initializer#initChannel` and log an error that we are `"Unable to create SslContext for inbound connection"` to make it clear what activity is causing the problem and close down the netty connection. ########## File path: src/java/org/apache/cassandra/security/SSLFactory.java ########## @@ -351,9 +194,16 @@ public static void checkCertFilesForHotReloading(EncryptionOptions.ServerEncrypt if (!isHotReloadingInitialized) throw new IllegalStateException("Hot reloading functionality has not been initialized."); - logger.debug("Checking whether certificates have been updated {}", hotReloadableFiles); + logger.debug("Checking whether certificates have been updated {} and {}", Review comment: nit: `updated for server {} and client {}` - could look at the source to decipher but this would be more explicit. ########## File path: test/conf/cassandra-sslcontextfactory.yaml ########## @@ -0,0 +1,84 @@ +# +# 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. +# + +# +# Warning! +# Consider the effects on 'o.a.c.i.s.LegacySSTableTest' before changing schemas in this file. +# Review comment: This warning doesn't really hold true here - perhaps replace with a reference to the source yaml file and what is changed for testing the SSL context factory? ########## File path: src/java/org/apache/cassandra/security/ISslContextFactory.java ########## @@ -0,0 +1,118 @@ +/* + * 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.security; + +import java.util.List; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLException; + +import io.netty.handler.ssl.CipherSuiteFilter; +import io.netty.handler.ssl.SslContext; + +/** + * The purpose of this interface is to provide pluggable mechanism for creating custom JSSE and Netty SSLContext + * objects. Please use the Cassandra configuration key {@code ssl_context_factory} as part of {@code + * client_encryption_options}/{@code server_encryption_options} and provide a custom class-name implementing this + * interface with parameters to be used to plugin a your own way to load the SSLContext. + * + * Implementation of this interface must have a constructor with argument of type {@code Map<String,Object>} to allow + * custom parameters, needed by the implementation, to be passed from the yaml configuration. Common SSL + * configurations like {@code protocol, algorithm, cipher_suites, accepted_protocols, require_client_auth, + * require_endpoint_verification, enabled, optional} will also be passed to that map by Cassanddra. + * + * Since on top of Netty, Cassandra is internally using JSSE SSLContext also for certain use-cases- this interface + * has methods for both. + * + * Below is an example of how to configure a custom implementation with parameters + * <pre> + * ssl_context_factory: + * class_name: org.apache.cassandra.security.YourSslContextFactoryImpl + * parameters: + * key1: "value1" + * key2: "value2" + * key3: "value3" + * </pre> + */ +public interface ISslContextFactory +{ + /** + * Creates JSSE SSLContext. + * + * @param buildTruststore {@code true} if the caller requires Truststore; {@code false} otherwise + * @return JSSE's {@link SSLContext} + * @throws SSLException in case the Ssl Context creation fails for some reason + */ + SSLContext createJSSESslContext(boolean buildTruststore) throws SSLException; + + /** + * Creates Netty's SslContext object. + * + * @param buildTruststore {@code true} if the caller requires Truststore; {@code false} otherwise Review comment: same comment as for `createJSSESslContext` ########## File path: src/java/org/apache/cassandra/security/ISslContextFactory.java ########## @@ -0,0 +1,118 @@ +/* + * 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.security; + +import java.util.List; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLException; + +import io.netty.handler.ssl.CipherSuiteFilter; +import io.netty.handler.ssl.SslContext; + +/** + * The purpose of this interface is to provide pluggable mechanism for creating custom JSSE and Netty SSLContext + * objects. Please use the Cassandra configuration key {@code ssl_context_factory} as part of {@code + * client_encryption_options}/{@code server_encryption_options} and provide a custom class-name implementing this + * interface with parameters to be used to plugin a your own way to load the SSLContext. + * + * Implementation of this interface must have a constructor with argument of type {@code Map<String,Object>} to allow + * custom parameters, needed by the implementation, to be passed from the yaml configuration. Common SSL + * configurations like {@code protocol, algorithm, cipher_suites, accepted_protocols, require_client_auth, + * require_endpoint_verification, enabled, optional} will also be passed to that map by Cassanddra. + * + * Since on top of Netty, Cassandra is internally using JSSE SSLContext also for certain use-cases- this interface + * has methods for both. + * + * Below is an example of how to configure a custom implementation with parameters + * <pre> + * ssl_context_factory: + * class_name: org.apache.cassandra.security.YourSslContextFactoryImpl + * parameters: + * key1: "value1" + * key2: "value2" + * key3: "value3" + * </pre> + */ +public interface ISslContextFactory +{ + /** + * Creates JSSE SSLContext. + * + * @param buildTruststore {@code true} if the caller requires Truststore; {@code false} otherwise + * @return JSSE's {@link SSLContext} + * @throws SSLException in case the Ssl Context creation fails for some reason + */ + SSLContext createJSSESslContext(boolean buildTruststore) throws SSLException; Review comment: `buildTruststore` is really a request to validate certificates on receipt - and always set true for servers and only true if client auth is requested. What do you think about making that more explicit by renaming it to something like `verifyTrust` the trust store is just the way of achieving that goal? ########## File path: src/java/org/apache/cassandra/security/DefaultSslContextFactoryImpl.java ########## @@ -0,0 +1,369 @@ +/* + * 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.security; + +import java.io.File; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.security.KeyStore; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Date; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import javax.net.ssl.KeyManagerFactory; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLException; +import javax.net.ssl.TrustManager; +import javax.net.ssl.TrustManagerFactory; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.netty.handler.ssl.CipherSuiteFilter; +import io.netty.handler.ssl.ClientAuth; +import io.netty.handler.ssl.SslContext; +import io.netty.handler.ssl.SslContextBuilder; +import io.netty.handler.ssl.SslProvider; + +/** + * Cassandra's default implementation class for the configuration key {@code ssl_context_factory}. It uses + * file based keystores. + */ +public final class DefaultSslContextFactoryImpl implements ISslContextFactory Review comment: I can see having this as `final` being important to know you're definitely getting an unmodified version of the class, but it means that somebody who wanted to just tweak a few settings has to deal with the maintenance issues of keeping their copy up to date. What do you think about dropping the final from this class and adding a new `public final class DefaultSslContextFactory extends DefaulSslContextFactoryImpl`. If somebody just wanted to tweak some of the Netty builder settings, though they could just override the `buildKeyManagerFactory`/`buildTrustManagerFactory` and if `useOpenSsl` gets moved into this module as a something like `getSslProvider()`. ########## File path: src/java/org/apache/cassandra/security/ISslContextFactory.java ########## @@ -0,0 +1,118 @@ +/* + * 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.security; + +import java.util.List; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLException; + +import io.netty.handler.ssl.CipherSuiteFilter; +import io.netty.handler.ssl.SslContext; + +/** + * The purpose of this interface is to provide pluggable mechanism for creating custom JSSE and Netty SSLContext + * objects. Please use the Cassandra configuration key {@code ssl_context_factory} as part of {@code + * client_encryption_options}/{@code server_encryption_options} and provide a custom class-name implementing this + * interface with parameters to be used to plugin a your own way to load the SSLContext. + * + * Implementation of this interface must have a constructor with argument of type {@code Map<String,Object>} to allow + * custom parameters, needed by the implementation, to be passed from the yaml configuration. Common SSL + * configurations like {@code protocol, algorithm, cipher_suites, accepted_protocols, require_client_auth, + * require_endpoint_verification, enabled, optional} will also be passed to that map by Cassanddra. + * + * Since on top of Netty, Cassandra is internally using JSSE SSLContext also for certain use-cases- this interface + * has methods for both. + * + * Below is an example of how to configure a custom implementation with parameters + * <pre> + * ssl_context_factory: + * class_name: org.apache.cassandra.security.YourSslContextFactoryImpl + * parameters: + * key1: "value1" + * key2: "value2" + * key3: "value3" + * </pre> + */ +public interface ISslContextFactory +{ + /** + * Creates JSSE SSLContext. + * + * @param buildTruststore {@code true} if the caller requires Truststore; {@code false} otherwise + * @return JSSE's {@link SSLContext} + * @throws SSLException in case the Ssl Context creation fails for some reason + */ + SSLContext createJSSESslContext(boolean buildTruststore) throws SSLException; + + /** + * Creates Netty's SslContext object. + * + * @param buildTruststore {@code true} if the caller requires Truststore; {@code false} otherwise + * @param socketType {@link SocketType} for Netty's Inbound or Outbound channels + * @param useOpenSsl {@code true} if openSsl is enabled;{@code false} otherwise Review comment: I think this is an implementation detail and we should move it to the default implementation instead. The setting is usually statically initialized and the override is just to disable native library loading in the in-JVM dtests. If we go ahead with removal, it should also be dropped from the `CacheKey`. ########## File path: test/unit/org/apache/cassandra/security/DefaultSslContextFactoryImplTest.java ########## @@ -0,0 +1,133 @@ +/* + * 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.security; + +import java.io.IOException; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import javax.net.ssl.TrustManagerFactory; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +public class DefaultSslContextFactoryImplTest +{ + private Map<String,Object> commonConfig = new HashMap<>(); + + @Before + public void setup() + { + commonConfig.put("truststore", "test/conf/cassandra_ssl_test.truststore"); + commonConfig.put("truststore_password", "cassandra"); + commonConfig.put("require_client_auth", Boolean.FALSE); + commonConfig.put("cipher_suites", Arrays.asList("TLS_RSA_WITH_AES_128_CBC_SHA")); + } + + private void addKeystoreOptions(Map<String,Object> config) + { + config.put("keystore", "test/conf/cassandra_ssl_test.keystore"); + config.put("keystore_password", "cassandra"); + } + + @Test(expected = IOException.class) + public void buildTrustManagerFactory_NoFile() throws IOException Review comment: nit: copied over mix of snake case and camel case in method names. Probably more sympathetic with the rest of the tests just to use camel. No need to clean it up unless it annoys you. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
