From 36e16ee3c911c710129fcf3a69595038c3dbd385 Mon Sep 17 00:00:00 2001 From: Jyothsna Konisa Date: Mon, 14 Nov 2022 14:16:07 -0800 Subject: [PATCH] Adding endpoint verification option to client_encryption_options patch by Jyothsna Konisa; reviewed by Jon Meredith, Yifan Cai for CASSANDRA-18034 --- CHANGES.txt | 1 + conf/cassandra.yaml | 1 + .../apache/cassandra/net/SocketFactory.java | 2 +- .../transport/PipelineConfigurator.java | 8 +- .../cassandra/transport/SimpleClient.java | 4 +- test/conf/cassandra_ssl_test.truststore | Bin 3240 -> 5295 bytes ...assandra_ssl_test_endpoint_verify.keystore | Bin 0 -> 2087 bytes .../NativeTransportEncryptionOptionsTest.java | 101 ++++++++++++++++++ 8 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 test/conf/cassandra_ssl_test_endpoint_verify.keystore diff --git a/CHANGES.txt b/CHANGES.txt index 0ea3c65c46..0c94126de7 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.2 + * Adding endpoint verification option to client_encryption_options (CASSANDRA-18034) * Replace 'wcwidth.py' with pypi module (CASSANDRA-17287) * Add nodetool forcecompact to remove tombstoned or ttl'd data ignoring GC grace for given table and partition keys (CASSANDRA-17711) * Offer IF (NOT) EXISTS in cqlsh completion for CREATE TYPE, DROP TYPE, CREATE ROLE and DROP ROLE (CASSANDRA-16640) diff --git a/conf/cassandra.yaml b/conf/cassandra.yaml index fbfa468c00..ff074bddd7 100644 --- a/conf/cassandra.yaml +++ b/conf/cassandra.yaml @@ -1394,6 +1394,7 @@ client_encryption_options: keystore_password: cassandra # Verify client certificates require_client_auth: false + # require_endpoint_verification: false # Set trustore and truststore_password if require_client_auth is true # truststore: conf/.truststore # truststore_password: cassandra diff --git a/src/java/org/apache/cassandra/net/SocketFactory.java b/src/java/org/apache/cassandra/net/SocketFactory.java index 33fff6b7a0..b135ed5107 100644 --- a/src/java/org/apache/cassandra/net/SocketFactory.java +++ b/src/java/org/apache/cassandra/net/SocketFactory.java @@ -215,7 +215,7 @@ public final class SocketFactory * Creates a new {@link SslHandler} from provided SslContext. * @param peer enables endpoint verification for remote address when not null */ - static SslHandler newSslHandler(Channel channel, SslContext sslContext, @Nullable InetSocketAddress peer) + public static SslHandler newSslHandler(Channel channel, SslContext sslContext, @Nullable InetSocketAddress peer) { if (peer == null) return sslContext.newHandler(channel.alloc()); diff --git a/src/java/org/apache/cassandra/transport/PipelineConfigurator.java b/src/java/org/apache/cassandra/transport/PipelineConfigurator.java index 81ff13605e..bf48eea334 100644 --- a/src/java/org/apache/cassandra/transport/PipelineConfigurator.java +++ b/src/java/org/apache/cassandra/transport/PipelineConfigurator.java @@ -48,6 +48,8 @@ import org.apache.cassandra.security.ISslContextFactory; import org.apache.cassandra.security.SSLFactory; import org.apache.cassandra.transport.messages.StartupMessage; +import static org.apache.cassandra.net.SocketFactory.newSslHandler; + /** * Takes care of intializing a Netty Channel and Pipeline for client protocol connections. * The pipeline is first set up with some common handlers for connection limiting, dropping @@ -181,7 +183,8 @@ public class PipelineConfigurator { // Connection uses SSL/TLS, replace the detection handler with a SslHandler and so use // encryption. - SslHandler sslHandler = sslContext.newHandler(channel.alloc()); + InetSocketAddress peer = encryptionOptions.require_endpoint_verification ? (InetSocketAddress) channel.remoteAddress() : null; + SslHandler sslHandler = newSslHandler(channel, sslContext, peer); channelHandlerContext.pipeline().replace(SSL_HANDLER, SSL_HANDLER, sslHandler); } else @@ -199,7 +202,8 @@ public class PipelineConfigurator SslContext sslContext = SSLFactory.getOrCreateSslContext(encryptionOptions, encryptionOptions.require_client_auth, ISslContextFactory.SocketType.SERVER); - channel.pipeline().addFirst(SSL_HANDLER, sslContext.newHandler(channel.alloc())); + InetSocketAddress peer = encryptionOptions.require_endpoint_verification ? (InetSocketAddress) channel.remoteAddress() : null; + channel.pipeline().addFirst(SSL_HANDLER, newSslHandler(channel, sslContext, peer)); }; default: throw new IllegalStateException("Unrecognized TLS encryption policy: " + this.tlsEncryptionPolicy); diff --git a/src/java/org/apache/cassandra/transport/SimpleClient.java b/src/java/org/apache/cassandra/transport/SimpleClient.java index 43bb8addee..2b57d104e4 100644 --- a/src/java/org/apache/cassandra/transport/SimpleClient.java +++ b/src/java/org/apache/cassandra/transport/SimpleClient.java @@ -52,6 +52,7 @@ import org.apache.cassandra.security.SSLFactory; import org.apache.cassandra.transport.messages.*; import org.apache.cassandra.utils.concurrent.UncheckedInterruptedException; +import static org.apache.cassandra.net.SocketFactory.newSslHandler; import static org.apache.cassandra.transport.CQLMessageHandler.envelopeSize; import static org.apache.cassandra.transport.Flusher.MAX_FRAMED_PAYLOAD_SIZE; import static org.apache.cassandra.utils.concurrent.NonBlockingRateLimiter.NO_OP_LIMITER; @@ -624,7 +625,8 @@ public class SimpleClient implements Closeable super.initChannel(channel); SslContext sslContext = SSLFactory.getOrCreateSslContext(encryptionOptions, encryptionOptions.require_client_auth, ISslContextFactory.SocketType.CLIENT); - channel.pipeline().addFirst("ssl", sslContext.newHandler(channel.alloc())); + InetSocketAddress peer = encryptionOptions.require_endpoint_verification ? new InetSocketAddress(host, port) : null; + channel.pipeline().addFirst("ssl", newSslHandler(channel, sslContext, peer)); } } diff --git a/test/conf/cassandra_ssl_test.truststore b/test/conf/cassandra_ssl_test.truststore index 5ba9a9977c4b0bbfa1baedd56bbe4d715e404784..ab01af30cd7e69c3b437083cb0ae3ce6ec498492 100644 GIT binary patch delta 2070 zcmV+x2k+tCboxg%#uYuoh2qnnwuw;Am)$VZZXUE zX<@FiNTS%0E#pYopxMn}weAn~)Kfn;C5}CVl_mpiF|rU=Vm8m*fqIK*3_rt}FfDP0 zR)@j+1F$Z~_hMk_3>rWr-bS-YLak;fe?~Ts5EjyQ{OMkdq%@&Q@=@Akrho9Q->M%$ zS4wJHJc`U2v1MTVasgzUS{0kO=Oc~YXJIxs;s`f6qTVAXf%UOFP(e1yug zQ&|L5IhByOR1PGMKKRovpKTQp6a#fZ>OIV+XF#B8_)W!SK7zw82e>*Im zdY44o>~!&qdR-*E-Ffu?v|yo=3B5pSt!?4DPctpyl`#N|ph5}91xTYDhL*!JFFf6Hjs4x9TV2)tA5*xdp@Z)?D3poN4Dl) zZE}wpeoepbt?w9@mH!CXxLb1owV>VubRnHVZyaUjj%h$sBsb_NUo*jmam6_TQC;GW z0>}wLQT>oU+|1qB>Ps_yWT|<#1n!-a#@=f%eT);w18tkh4Zx8csI8`WB2q}g1;sOrT4h@#|VzShoEu$S$+ZN zZk2>fyjC=^V8d~*TXuSI?|Q0*NaQ4D>0{lg$Se<}?_^a!%(D_DUbJ^f6 z?DTL8)@L3me??$#eU2|Zokb{_14YL3e6>lyw0GduHSCm&LaqP+00966SS~d%IRF3x z;4p#$+%SRx#GnHL0RjRDFt1+DD+V(;FbxI?Duzgg_YDC73k3i$6fqJo5C#KP1OpQZ zF)}wUFfK4IF)$t%4Kgw@FfuSQF)%bWG+Gx8GBYtSA2Kj9F)%bWG+HneF%mEk1_M@L}R9H{4##P^r}l3#9R{Todue{3QX@H?>Il4dBT z8WYi$WkK`&TrK-tn>$ZS&OVL^CD<$y@@06!mRjR@sM|D71H3~bqHH{#$rL zYp(ltq$kP#czN?jTI7nekbNQe!jhP zaR&XDjEiL!b}a6%hNPms$yL{30@`)6%mc^>6l)q=AwhY0rn`)lwfoW8(~g}Pl_L$U AQ2+n{ delta 47 zcmZ3lxk8fX-`jt085kItKzJk1QRdAzxDRuPw3}-`si^p~Eqjs|Mbsy?s@Jx&$;K`bI)_ndG7Ag?h*t7fpP%+7hIkp9?l^g z7WEVVL<|Dq0>DW4Z}1>u|0x6j2hdP{0N{eaBjMi~%n75B+(DyuG^N=eR9k!g@1gb= zQ2EK)=RM42|UBjN+fTi?fF{7~ZtWvf_g^mK}Zc@!7YynbkVudzB1bz3)L zj0z3c_b)tK>fJ~{-p-IUXsg%edbEI3A$=YUleLs7an#*h>3^j%B>x?7B5)gbw#ua-)hzttY<-cW^RM@U>h}!^qY}EgUG+>b@p+Q-hf0-tne`?k zjz%hDSR%jBTmqZe*?QvALSa$#fp6c-)rN8>ce?7N*jBcp%zHUSY2wnM21LcHj82CsUNuvUEQ;9ICbRRNzGIe^diT+OtG>-76htBZ@Mso_v@V zmxewZbSN9sYf&T*J*x2#R4Bnu^sGlYBn zdNxrWx^6Vn6IlSxhI?>hi8tAEo_7a%i>t^yDf^|6xouj9{fA2!eg8!}he5sMAvDr1;Ss&FKcdeNcnIaYIc z*_plmu_rbC%!;_bLE`Fc$iC81q4P7V2YISpm6xA|leaZhB`0we2PHOL{BtBl=Yvco zo!k%x+68W9vmJ|!3Tx$RkfM5VQKIHBKO0;S>FifA;x#Su!!2iiF2TV+=RxoF<{%fe zoX-iHKH#HuW)tv?Q?`k|xv%Tg{o?6g+(SmE{vu3AY^WJ@B-Di14%nRBX;K2yAJ>$; znU4R|5a~l)HJB%F3@%Ofib=2*>2J$TlzYW3rN(22crM z{Dj9<87r3q(POdOWR_0F@4B4U1U*YRCwvh;hTHR}n7?#<)xUBW|xTB_bUJswe6(Obq>G^+JlPpJRag zuNad29Ep!6wjlM?g;hjA9e~}px-kGhTz;Sh5#)ed<75hCRm=i^BRd{ z2k^$gh43jywxgPI=qJbIGhuq~*TO0d8eRuY^x;@34|Hk&^F=JUAfS-40Ej}sEZ{If9%U>619Czcln_B; zvC1G8}^EKD-0% z>`7Z^Ew+YInvdEm$I*XOmUiKu*JXNQM4{waA?8H=743$Uq8HMl_?wUF?K-G!*V9Ra zEFvRwv-LbBIIHl8TuwN-hM9Vo$iKsjH4UbvLY;R=Htfs10eBW>C{(<7p3?j?eV1bE z<^21Ik=)Wi)KH@%u@wh|-i|MQk*_{+jWU!`Di_gu(y_@B|Bj;8Bj^M-Re<;X(ql zO8K2h=~kZCtwko0VO@frfyp4>f6DxDZV(X*zs-4af2ztk8b?(CB1O2{zB1b`3&zjo z+jZoZCk-STmaXMZ4oA5gUQMU`WA3x^{z>P_DXTM~n_A_h!}Fb{jBnyynB&K4|bzyyAuEa literal 0 HcmV?d00001 diff --git a/test/distributed/org/apache/cassandra/distributed/test/NativeTransportEncryptionOptionsTest.java b/test/distributed/org/apache/cassandra/distributed/test/NativeTransportEncryptionOptionsTest.java index c5a810ca25..5f2caaf695 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/NativeTransportEncryptionOptionsTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/NativeTransportEncryptionOptionsTest.java @@ -18,18 +18,33 @@ package org.apache.cassandra.distributed.test; +import java.io.FileInputStream; +import java.io.InputStream; import java.net.InetAddress; +import java.security.KeyStore; import java.util.Collections; +import javax.net.ssl.KeyManagerFactory; +import javax.net.ssl.TrustManagerFactory; + import com.google.common.collect.ImmutableMap; import org.junit.Assert; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; +import com.datastax.driver.core.SSLOptions; +import com.datastax.driver.core.exceptions.NoHostAvailableException; +import com.datastax.shaded.netty.handler.ssl.SslContext; +import com.datastax.shaded.netty.handler.ssl.SslContextBuilder; import org.apache.cassandra.distributed.Cluster; import org.apache.cassandra.distributed.api.Feature; public class NativeTransportEncryptionOptionsTest extends AbstractEncryptionOptionsImpl { + @Rule + public ExpectedException expectedException = ExpectedException.none(); + @Test public void nodeWillNotStartWithBadKeystore() throws Throwable { @@ -219,4 +234,90 @@ public class NativeTransportEncryptionOptionsTest extends AbstractEncryptionOpti assertCannotStartDueToConfigurationException(cluster); } } + + @Test + public void testEndpointVerificationDisabledIpNotInSAN() throws Throwable + { + // When required_endpoint_verification is set to false, client certificate Ip/hostname should be validated + // The certificate in cassandra_ssl_test_outbound.keystore does not have IP/hostname embeded, so when + // require_endpoint_verification is false, the connection should be established + testEndpointVerification(false, true); + } + + @Test + public void testEndpointVerificationEnabledIpNotInSAN() throws Throwable + { + // When required_endpoint_verification is set to true, client certificate Ip/hostname should be validated + // The certificate in cassandra_ssl_test_outbound.keystore does not have IP/hostname emebeded, so when + // require_endpoint_verification is true, the connection should not be established + testEndpointVerification(true, false); + } + + @Test + public void testEndpointVerificationEnabledWithIPInSan() throws Throwable + { + // When required_endpoint_verification is set to true, client certificate Ip/hostname should be validated + // The certificate in cassandra_ssl_test_outbound.keystore have IP/hostname emebeded, so when + // require_endpoint_verification is true, the connection should be established + testEndpointVerification(true, true); + } + + private void testEndpointVerification(boolean requireEndpointVerification, boolean ipInSAN) throws Throwable + { + try (Cluster cluster = builder().withNodes(1).withConfig(c -> { + c.with(Feature.NATIVE_PROTOCOL); + c.set("client_encryption_options", + ImmutableMap.builder().putAll(validKeystore) + .put("enabled", true) + .put("require_client_auth", true) + .put("require_endpoint_verification", requireEndpointVerification) + .build()); + }).start()) + { + InetAddress address = cluster.get(1).config().broadcastAddress().getAddress(); + SslContextBuilder sslContextBuilder = SslContextBuilder.forClient(); + if (ipInSAN) + sslContextBuilder.keyManager(createKeyManagerFactory("test/conf/cassandra_ssl_test_endpoint_verify.keystore", "cassandra")); + else + sslContextBuilder.keyManager(createKeyManagerFactory("test/conf/cassandra_ssl_test_outbound.keystore", "cassandra")); + + SslContext sslContext = sslContextBuilder.trustManager(createTrustManagerFactory("test/conf/cassandra_ssl_test.truststore", "cassandra")) + .build(); + final SSLOptions sslOptions = socketChannel -> sslContext.newHandler(socketChannel.alloc()); + com.datastax.driver.core.Cluster driverCluster = com.datastax.driver.core.Cluster.builder() + .addContactPoint(address.getHostAddress()) + .withSSL(sslOptions) + .build(); + + if (!ipInSAN) + { + expectedException.expect(NoHostAvailableException.class); + } + + driverCluster.connect(); + } + } + + private KeyManagerFactory createKeyManagerFactory(final String keyStorePath, + final String keyStorePassword) throws Exception + { + final InputStream stream = new FileInputStream(keyStorePath); + final KeyStore ks = KeyStore.getInstance("JKS"); + ks.load(stream, keyStorePassword.toCharArray()); + final KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); + kmf.init(ks, keyStorePassword.toCharArray()); + return kmf; + } + + private TrustManagerFactory createTrustManagerFactory(final String trustStorePath, + final String trustStorePassword) throws Exception + { + final InputStream stream = new FileInputStream(trustStorePath); + final KeyStore ts = KeyStore.getInstance("JKS"); + ts.load(stream, trustStorePassword.toCharArray()); + final TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); + tmf.init(ts); + return tmf; + } + }