You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2020/12/02 03:03:47 UTC
[GitHub] [cassandra] dcapwell commented on a change in pull request #770: CASSANDRA-13325 configure which TLS protocols should be accepted
dcapwell commented on a change in pull request #770:
URL: https://github.com/apache/cassandra/pull/770#discussion_r533864563
##########
File path: src/java/org/apache/cassandra/security/SSLFactory.java
##########
@@ -397,41 +392,121 @@ public static synchronized void initHotReloading(EncryptionOptions.ServerEncrypt
isHotReloadingInitialized = true;
}
-
- /**
- * Sanity checks all certificates to ensure we can actually load them
+ // Non-logging
+ /*
+ * This class will filter all requested ciphers out that are not supported by the current {@link SSLEngine},
+ * logging messages for all dropped ciphers, and throws an exception if no ciphers are supported
*/
- public static void validateSslCerts(EncryptionOptions.ServerEncryptionOptions serverOpts, EncryptionOptions clientOpts) throws IOException
+ public static final class LoggingCipherSuiteFilter implements CipherSuiteFilter
{
- try
+ // Version without logging the ciphers, make sure same filtering logic is used
+ // all the time, regardless of user output.
+ public static final CipherSuiteFilter QUIET_FILTER = new LoggingCipherSuiteFilter();
+ final String settingDescription;
+
+ private LoggingCipherSuiteFilter()
{
- // Ensure we're able to create both server & client SslContexts if they might ever be needed
- if (serverOpts != null && serverOpts.tlsEncryptionPolicy() != EncryptionOptions.TlsEncryptionPolicy.UNENCRYPTED)
- {
- createNettySslContext(serverOpts, true, SocketType.SERVER, openSslIsAvailable());
- createNettySslContext(serverOpts, true, SocketType.CLIENT, openSslIsAvailable());
- }
+ this.settingDescription = null;
}
- catch (Exception e)
+
+ public LoggingCipherSuiteFilter(String settingDescription)
{
- throw new IOException("Failed to create SSL context using server_encryption_options!", e);
+ this.settingDescription = settingDescription;
}
- try
+
+ @Override
+ public String[] filterCipherSuites(Iterable<String> ciphers, List<String> defaultCiphers,
+ Set<String> supportedCiphers)
{
- // Ensure we're able to create both server & client SslContexts if they might ever be needed
- if (clientOpts != null && clientOpts.tlsEncryptionPolicy() != EncryptionOptions.TlsEncryptionPolicy.UNENCRYPTED)
+ Objects.requireNonNull(defaultCiphers, "defaultCiphers");
+ Objects.requireNonNull(supportedCiphers, "supportedCiphers");
+
+ final List<String> newCiphers;
+ if (ciphers == null)
+ {
+ newCiphers = new ArrayList<>(defaultCiphers.size());
+ ciphers = defaultCiphers;
+ }
+ else
+ {
+ newCiphers = new ArrayList<>(supportedCiphers.size());
+ }
+ for (String c : ciphers)
{
- createNettySslContext(clientOpts, clientOpts.require_client_auth, SocketType.SERVER, openSslIsAvailable());
- createNettySslContext(clientOpts, clientOpts.require_client_auth, SocketType.CLIENT, openSslIsAvailable());
+ if (c == null)
+ {
+ break;
+ }
+ if (supportedCiphers.contains(c))
+ {
+ newCiphers.add(c);
+ }
+ else
+ {
+ if (settingDescription != null)
+ {
+ logger.warn("Dropping unsupported cipher_suite {} from {} configuration",
+ c, settingDescription.toLowerCase());
+ }
+ }
}
+ if (newCiphers.isEmpty())
+ {
+ throw new IllegalStateException("No ciphers left after filtering supported cipher suite");
+ }
+
+ return newCiphers.toArray(new String[0]);
}
- catch (Exception e)
+ }
+
+ public static void validateSslContext(String contextDescription, EncryptionOptions options, boolean buildTrustStore, boolean logProtocolAndCiphers) throws IOException
+ {
+ if (options != null && options.tlsEncryptionPolicy() != EncryptionOptions.TlsEncryptionPolicy.UNENCRYPTED)
{
- throw new IOException("Failed to create SSL context using client_encryption_options!", e);
+ try
+ {
+ CipherSuiteFilter loggingCipherSuiteFilter = logProtocolAndCiphers ? new LoggingCipherSuiteFilter(contextDescription)
+ : LoggingCipherSuiteFilter.QUIET_FILTER;
+ SslContext sslContext = createNettySslContext(options, buildTrustStore, SocketType.SERVER, openSslIsAvailable(), loggingCipherSuiteFilter);
+ SSLEngine engine = sslContext.newEngine(ByteBufAllocator.DEFAULT);
Review comment:
don't we need to release as you don't actually use this? its ref counted and `io.netty.handler.ssl.ReferenceCountedOpenSslEngine` looks like it should be cleaned up if allocated.
##########
File path: src/java/org/apache/cassandra/config/EncryptionOptions.java
##########
@@ -209,6 +214,83 @@ public void setOptional(boolean optional) {
this.optional = optional;
}
+ /**
+ * Sets accepted TLS protocol for this channel. Note that this should only be called by
+ * the configuration parser or tests. It is public only for that purpose, mutating protocol state
+ * is probably a bad idea.
+ * @param protocol value to set
+ */
+ @VisibleForTesting
+ public void setProtocol(String protocol) {
+ this.protocol = protocol;
+ }
+
+ /**
+ * Sets accepted TLS protocols for this channel. Note that this should only be called by
+ * the configuration parser or tests. It is public only for that purpose, mutating protocol state
+ * is probably a bad idea. The function casing is required for snakeyaml to find this setter for the protected field.
+ * @param accepted_protocols value to set
+ */
+ public void setaccepted_protocols(List<String> accepted_protocols) {
+ this.accepted_protocols = accepted_protocols == null ? null : ImmutableList.copyOf(accepted_protocols);
+ }
+
+ /* This list is substituted in configurations that have explicitly specified the original "TLS" default,
+ * it is not a 'default' list or 'support protocol versions' list. It is just an attempt to preserve the
+ * original intent for the user configuration
+ */
+ private final List<String> tlsProtocolSubstitution = ImmutableList.of("TLSv1.3", "TLSv1.2", "TLSv1.1", "TLSv1");
Review comment:
nit: should be static and named `TLS_PROTOCOL_SUBSTITUTION`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org