From a24bd6c6a554415ab0cb173e0a3ffb96510f7a1c Mon Sep 17 00:00:00 2001 From: Stefan Podkowinski Date: Tue, 16 Feb 2016 11:46:11 +0100 Subject: [PATCH] Preserve order for preferred SSL cipher suites Patch by Stefan Podkowinski and Tom Petracca; reviewed by Stefania for CASSANDRA-11164 --- CHANGES.txt | 2 + .../apache/cassandra/security/SSLFactory.java | 41 ++++++---- .../thrift/CustomTThreadPoolServer.java | 4 +- .../apache/cassandra/transport/Server.java | 3 +- .../cassandra/transport/SimpleClient.java | 3 +- test/conf/keystore.jks | Bin 0 -> 2191 bytes .../cassandra/security/SSLFactoryTest.java | 75 ++++++++++++++++++ 7 files changed, 109 insertions(+), 19 deletions(-) create mode 100644 test/conf/keystore.jks create mode 100644 test/unit/org/apache/cassandra/security/SSLFactoryTest.java diff --git a/CHANGES.txt b/CHANGES.txt index aa3adf5656..103ac16d11 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 2.2.6 + * Preserve order for preferred SSL cipher suites (CASSANDRA-11164) * Range.compareTo() violates the contract of Comparable (CASSANDRA-11216) * Avoid NPE when serializing ErrorMessage with null message (CASSANDRA-11167) * Replacing an aggregate with a new version doesn't reset INITCOND (CASSANDRA-10840) @@ -33,6 +34,7 @@ Merged from 2.1: * (cqlsh) Support timezone conversion using pytz (CASSANDRA-10397) * cqlsh: change default encoding to UTF-8 (CASSANDRA-11124) + 2.2.5 * maxPurgeableTimestamp needs to check memtables too (CASSANDRA-9949) * Apply change to compaction throughput in real time (CASSANDRA-10025) diff --git a/src/java/org/apache/cassandra/security/SSLFactory.java b/src/java/org/apache/cassandra/security/SSLFactory.java index e9aa07d637..a327de9f1a 100644 --- a/src/java/org/apache/cassandra/security/SSLFactory.java +++ b/src/java/org/apache/cassandra/security/SSLFactory.java @@ -24,9 +24,10 @@ import java.net.InetAddress; import java.net.InetSocketAddress; import java.security.KeyStore; import java.security.cert.X509Certificate; +import java.util.Arrays; import java.util.Date; import java.util.Enumeration; -import java.util.Set; +import java.util.List; import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLContext; @@ -37,10 +38,12 @@ import javax.net.ssl.TrustManagerFactory; import org.apache.cassandra.config.EncryptionOptions; import org.apache.cassandra.io.util.FileUtils; -import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.base.Predicates; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.common.collect.Sets; /** @@ -58,8 +61,8 @@ public final class SSLFactory SSLContext ctx = createSSLContext(options, true); SSLServerSocket serverSocket = (SSLServerSocket)ctx.getServerSocketFactory().createServerSocket(); serverSocket.setReuseAddress(true); - String[] suits = filterCipherSuites(serverSocket.getSupportedCipherSuites(), options.cipher_suites); - serverSocket.setEnabledCipherSuites(suits); + String[] suites = filterCipherSuites(serverSocket.getSupportedCipherSuites(), options.cipher_suites); + serverSocket.setEnabledCipherSuites(suites); serverSocket.setNeedClientAuth(options.require_client_auth); serverSocket.setEnabledProtocols(ACCEPTED_PROTOCOLS); serverSocket.bind(new InetSocketAddress(address, port), 500); @@ -71,8 +74,8 @@ public final class SSLFactory { SSLContext ctx = createSSLContext(options, true); SSLSocket socket = (SSLSocket) ctx.getSocketFactory().createSocket(address, port, localAddress, localPort); - String[] suits = filterCipherSuites(socket.getSupportedCipherSuites(), options.cipher_suites); - socket.setEnabledCipherSuites(suits); + String[] suites = filterCipherSuites(socket.getSupportedCipherSuites(), options.cipher_suites); + socket.setEnabledCipherSuites(suites); socket.setEnabledProtocols(ACCEPTED_PROTOCOLS); return socket; } @@ -82,8 +85,8 @@ public final class SSLFactory { SSLContext ctx = createSSLContext(options, true); SSLSocket socket = (SSLSocket) ctx.getSocketFactory().createSocket(address, port); - String[] suits = filterCipherSuites(socket.getSupportedCipherSuites(), options.cipher_suites); - socket.setEnabledCipherSuites(suits); + String[] suites = filterCipherSuites(socket.getSupportedCipherSuites(), options.cipher_suites); + socket.setEnabledCipherSuites(suites); socket.setEnabledProtocols(ACCEPTED_PROTOCOLS); return socket; } @@ -93,8 +96,8 @@ public final class SSLFactory { SSLContext ctx = createSSLContext(options, true); SSLSocket socket = (SSLSocket) ctx.getSocketFactory().createSocket(); - String[] suits = filterCipherSuites(socket.getSupportedCipherSuites(), options.cipher_suites); - socket.setEnabledCipherSuites(suits); + String[] suites = filterCipherSuites(socket.getSupportedCipherSuites(), options.cipher_suites); + socket.setEnabledCipherSuites(suites); socket.setEnabledProtocols(ACCEPTED_PROTOCOLS); return socket; } @@ -155,12 +158,18 @@ public final class SSLFactory return ctx; } - private static String[] filterCipherSuites(String[] supported, String[] desired) + public static String[] filterCipherSuites(String[] supported, String[] desired) { - Set des = Sets.newHashSet(desired); - Set toReturn = Sets.intersection(Sets.newHashSet(supported), des); - if (des.size() > toReturn.size()) - logger.warn("Filtering out {} as it isnt supported by the socket", StringUtils.join(Sets.difference(des, toReturn), ",")); - return toReturn.toArray(new String[toReturn.size()]); + if (Arrays.equals(supported, desired)) + return desired; + List ldesired = Arrays.asList(desired); + ImmutableSet ssupported = ImmutableSet.copyOf(supported); + String[] ret = Iterables.toArray(Iterables.filter(ldesired, Predicates.in(ssupported)), String.class); + if (desired.length > ret.length && logger.isWarnEnabled()) + { + Iterable missing = Iterables.filter(ldesired, Predicates.not(Predicates.in(Sets.newHashSet(ret)))); + logger.warn("Filtering out {} as it isn't supported by the socket", Iterables.toString(missing)); + } + return ret; } } diff --git a/src/java/org/apache/cassandra/thrift/CustomTThreadPoolServer.java b/src/java/org/apache/cassandra/thrift/CustomTThreadPoolServer.java index bde5310535..c5f34aef13 100644 --- a/src/java/org/apache/cassandra/thrift/CustomTThreadPoolServer.java +++ b/src/java/org/apache/cassandra/thrift/CustomTThreadPoolServer.java @@ -245,7 +245,7 @@ public class CustomTThreadPoolServer extends TServer if (clientEnc.enabled) { logger.info("enabling encrypted thrift connections between client and server"); - TSSLTransportParameters params = new TSSLTransportParameters(clientEnc.protocol, clientEnc.cipher_suites); + TSSLTransportParameters params = new TSSLTransportParameters(clientEnc.protocol, new String[0]); params.setKeyStore(clientEnc.keystore, clientEnc.keystore_password); if (clientEnc.require_client_auth) { @@ -254,6 +254,8 @@ public class CustomTThreadPoolServer extends TServer } TServerSocket sslServer = TSSLTransportFactory.getServerSocket(addr.getPort(), 0, addr.getAddress(), params); SSLServerSocket sslServerSocket = (SSLServerSocket) sslServer.getServerSocket(); + String[] suites = SSLFactory.filterCipherSuites(sslServerSocket.getSupportedCipherSuites(), clientEnc.cipher_suites); + sslServerSocket.setEnabledCipherSuites(suites); sslServerSocket.setEnabledProtocols(SSLFactory.ACCEPTED_PROTOCOLS); serverTransport = new TCustomServerSocket(sslServer.getServerSocket(), args.keepAlive, args.sendBufferSize, args.recvBufferSize); } diff --git a/src/java/org/apache/cassandra/transport/Server.java b/src/java/org/apache/cassandra/transport/Server.java index c56564cacf..43d07fc532 100644 --- a/src/java/org/apache/cassandra/transport/Server.java +++ b/src/java/org/apache/cassandra/transport/Server.java @@ -331,7 +331,8 @@ public class Server implements CassandraDaemon.Server protected final SslHandler createSslHandler() { SSLEngine sslEngine = sslContext.createSSLEngine(); sslEngine.setUseClientMode(false); - sslEngine.setEnabledCipherSuites(encryptionOptions.cipher_suites); + String[] suites = SSLFactory.filterCipherSuites(sslEngine.getSupportedCipherSuites(), encryptionOptions.cipher_suites); + sslEngine.setEnabledCipherSuites(suites); sslEngine.setNeedClientAuth(encryptionOptions.require_client_auth); sslEngine.setEnabledProtocols(SSLFactory.ACCEPTED_PROTOCOLS); return new SslHandler(sslEngine); diff --git a/src/java/org/apache/cassandra/transport/SimpleClient.java b/src/java/org/apache/cassandra/transport/SimpleClient.java index 701a24cd9c..4759c2ae34 100644 --- a/src/java/org/apache/cassandra/transport/SimpleClient.java +++ b/src/java/org/apache/cassandra/transport/SimpleClient.java @@ -291,7 +291,8 @@ public class SimpleClient implements Closeable super.initChannel(channel); SSLEngine sslEngine = sslContext.createSSLEngine(); sslEngine.setUseClientMode(true); - sslEngine.setEnabledCipherSuites(encryptionOptions.cipher_suites); + String[] suites = SSLFactory.filterCipherSuites(sslEngine.getSupportedCipherSuites(), encryptionOptions.cipher_suites); + sslEngine.setEnabledCipherSuites(suites); sslEngine.setEnabledProtocols(SSLFactory.ACCEPTED_PROTOCOLS); channel.pipeline().addFirst("ssl", new SslHandler(sslEngine)); } diff --git a/test/conf/keystore.jks b/test/conf/keystore.jks new file mode 100644 index 0000000000000000000000000000000000000000..334025d6b97f0cf042853be75da6284263d83c13 GIT binary patch literal 2191 zcmcJP`8(8$7sqEcw!z?LYwS^D`zYI3vhP&FWN8?oxJ{xkj)mT-1FM_{-ao zB6#j@Ra0Mq!jLs7Zd~7$j}uvrxP$=PA;&8)Nj4EB?Bqu}EkMSKY zUr6=|7@T&o>%U_C_}CPR)cWo{=_`4WjYfrcT+0*<>=jq;j=>h1kUP4HRQyr&D_7{j z<{Dlz?|^(l#-3aRpX-_9p03x1>4|J!)@yuOsL?7PPe#7pG`_Ix*wS?|9fZT$Vc6+d zcr&UwM}K3HVQwX}ej!9hNh{$KU(K^FYktRpG1((rhi z{w`aaucXid)M1jdx4U485|-|2I7f_Y6sb4$vc471Ga>3e5Y0hI!b9z}@(9h~50?dAshazAJ<{eD}+{>{k#oX%ko^`Bc57cPTH+ld562G)CK zJTRcB5lA6`1T7;yhBkZ6?ym0gGJ6Ai6F+EVw$s=0Zqn_U>3PA=CT3+F>%$=t!Cs8t z?Vr-+ncXu@7u1yHrgNo(n_BS0@0a~)n=BjLs#=<>eLU|knQRK7Ta#Vx)S@Pu*)aVw zfAISEr@zyF+8~q9QXH#*?rhzz;RHiS9(Oy^KX<(9msq(ELs~c$-xr~`^W?YV=m8^1 zoO^5fYD4Is>q{hz2FlNzRPlIw`BT@VZ#qVGGkc$B{BiZQcd&YjnAIQSdP!GpSrs){ zA064o6O=wmc#E^?XR<(*JuaR3ye{3&7-d0A(8diWDx>|Dkc;k0U4o^#2VZpDFbqUs z{wmofKCCv)+~y!>NH%2MM>_gYti$(wF)Bm#)y{>7A&)Y(^3H|nPWjx;5I<#y)eU&o z%0E=BVO&1HWMmlIhmMVABE!@3c5`uyYn(A@e3d2*Hgk_1`EB9g1_X#oe5n?3FuiZll^f=Dq^5o{V`sMX4rB zL|ugmr3A0f@IKweTThlGbFt+!--5^tLwA{nf~kEQ2_Y?~-qb`F*3>$T`qyMFPHJvW zmiYHqyms@3hK)XfCTb|I55p-hrFm#+jHz{2(u$gV&H^QxMbnKCj=4N9ikzfQfTR+# zrm%f+Van%woE|4tG-?j=_t>>cSLV;f(Bs1VuSmT1@E|Ijd;?JVI}b5VcUDyc^-2@> zl*T@iw4z=pG`!Iqoz`BqW4}pNeK!^*eM0ymJxVE7r8M)@fX_ZAbe2e)HM8Q>x%hIa zb^*t$FZ<30ju|3L)Iw5Jui2uOX4=epnV8`BzUQC$$R~<=BEOS_PGSTkuDX-o2hkZg^b11p^TA|cvZ(q_D>{euS=m}w?@Tnc^G7(sT~svzEC>WO z1*lL%fC^E}hC;v)2u$dX_#A-b;KD>%MQtO&U>-OKxS);%csQWWFb*NuIZ}Y{e-QC^ z&dC z!Q2ok7z|38`f_q1GMUj^cp5AHVL)41aoWW>)vI^zDz$jOf6Mhzvq5C$uyyMLg_ZhH zr{Vs9ylf+zZ!S~BZwoN-C}4vCs`n4}sH=UYR8+O~yl!pzMSLNLkBdXoAEhTk7Qk5r z%T)w~@j};wD1a;hGRLMp*C>^m%r-fm8n_)J-XUq2M#?&|$&`pj;5tO?L;DFF8LU+e zB|ohNCQ=;b=<1Kx{qw9_l&2myip=zzx2m3Ow{%YdUZycEj24P&a zVNI-3uy$R0(>VU8;xn_4dT+^(;-AlW2Xth`^Ncaj6L4wH_D}Z%x2?tb=2A^p9EhDn z@AC1{d7sD;_b0=qz&LASCU?25B`W#gT(xAO)dtcF$vi0QK16ICbWlN6Kxd!eG-0ee;Y8hG)_4!n@U5IszmFb9E zsNZJsXj(P8&tiIXaJEdrXI}0s=}wYgV5#<=f3