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 2022/11/12 01:11:29 UTC
[GitHub] [cassandra] yifan-c commented on a diff in pull request #1995: CASSANDRA-18034 Adding endpoint verification option to client_encryption_options
yifan-c commented on code in PR #1995:
URL: https://github.com/apache/cassandra/pull/1995#discussion_r1020630533
##########
test/distributed/org/apache/cassandra/distributed/test/NativeTransportEncryptionOptionsTest.java:
##########
@@ -219,4 +234,118 @@ public void nodeMustNotStartWithNonExistantCiphersTest() throws Throwable
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
+ 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", false)
+ .build());
+ }).start())
+ {
+ InetAddress address = cluster.get(1).config().broadcastAddress().getAddress();
+ SslContext sslContext = SslContextBuilder.forClient()
+ .keyManager(createKeyManagerFactory("test/conf/cassandra_ssl_test_outbound.keystore", "cassandra"))
+ .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();
+ driverCluster.connect();
+ }
+ }
+
+ @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
+ 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", true)
+ .build());
+ }).start())
+ {
+ InetAddress address = cluster.get(1).config().broadcastAddress().getAddress();
+ SslContext sslContext = SslContextBuilder.forClient()
+ .keyManager(createKeyManagerFactory("test/conf/cassandra_ssl_test_outbound.keystore", "cassandra"))
+ .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();
+ expectedException.expect(NoHostAvailableException.class);
+ driverCluster.connect();
+ }
+ }
+
+ @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
+ 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", true)
+ .build());
+ }).start())
+ {
+ InetAddress address = cluster.get(1).config().broadcastAddress().getAddress();
+ SslContext sslContext = SslContextBuilder.forClient()
+ .keyManager(createKeyManagerFactory("test/conf/cassandra_ssl_test_endpoint_verify.keystore", "cassandra"))
+ .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();
+ driverCluster.connect();
+ }
+ }
Review Comment:
nit: there are fair amount of code duplication in the 3 new tests. Can you refactor to reduce the dups? For example,
```java
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();
}
}
```
--
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: pr-unsubscribe@cassandra.apache.org
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