From 465a82aba7fa9a2125d51c48888d6ca4a4508576 Mon Sep 17 00:00:00 2001 From: Alessandro Verzicco Date: Fri, 16 Oct 2026 18:28:48 +0200 Subject: [PATCH] Allow restricting TLS clients to an allowlist of certificate identities With ClientAuth.REQUIRE any certificate signed by the configured CA is accepted, which is too broad when the CA also issues certificates for other services. The new repeatable --tlsallowedclient option (and the MGMT_API_TLS_ALLOWED_CLIENTS env var in the images, comma-separated) additionally requires the client certificate subject CN or one of its DNS/URI/email/IP subject alternative names to match an allowed value. When the option is not set, behaviour is unchanged. Fixes #174 Assisted-By: Claude Code (Claude Opus 5.5) --- CHANGELOG.md | 1 + README.md | 20 ++ cassandra-trunk/files/docker-entrypoint.sh | 15 +- cassandra/scripts/docker-entrypoint-jdk8.sh | 15 +- cassandra/scripts/docker-entrypoint.sh | 15 +- dse/files/entrypoint.sh | 15 +- .../mgmtapi/AllowedClientsTrustManager.java | 204 ++++++++++++++++++ .../main/java/com/datastax/mgmtapi/Cli.java | 68 +++++- .../AllowedClientsTrustManagerTest.java | 110 ++++++++++ .../mgmtapi/NettyTlsClientAuthTest.java | 92 ++++++++ 10 files changed, 541 insertions(+), 14 deletions(-) create mode 100644 management-api-server/src/main/java/com/datastax/mgmtapi/AllowedClientsTrustManager.java create mode 100644 management-api-server/src/test/java/com/datastax/mgmtapi/AllowedClientsTrustManagerTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 57966017..236abaa3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Changelog for Management API, new PRs should update the `main / unreleased` sect ``` ## unreleased +* [FEATURE] [#174](https://github.com/k8ssandra/management-api-for-apache-cassandra/issues/174) Allow restricting TLS client certificates to an allowlist of identities with `--tlsallowedclient` (`MGMT_API_TLS_ALLOWED_CLIENTS`, comma-separated, in the images) ## v0.1.127 [2026-10-09] * [FEATURE] [#793](https://github.com/k8ssandra/management-api-for-apache-cassandra/issues/793) Provide VirtualTable to query the status of node's jobs diff --git a/README.md b/README.md index 83106abb..c0c1e620 100644 --- a/README.md +++ b/README.md @@ -375,6 +375,18 @@ the environment variable `MGMT_API_LISTEN_TCP_PORT` and setting it to your desir The above would run a Cassandra 4.0.15 image with Management API listening on port 9090 (instead of 8080). +### Restricting TLS clients + +When TLS is enabled with `MGMT_API_TLS_CA_CERT_FILE`, `MGMT_API_TLS_CERT_FILE` and `MGMT_API_TLS_KEY_FILE`, any client +certificate signed by the CA is accepted. To only accept specific clients, set `MGMT_API_TLS_ALLOWED_CLIENTS` to a +comma-separated list of identities, each matched against the client certificate subject CN and its DNS/URI/email/IP +subject alternative names (see `--tlsallowedclient` below). For example: + + > docker run -e USE_MGMT_API=true -e MGMT_API_TLS_CA_CERT_FILE=/certs/ca.crt -e MGMT_API_TLS_CERT_FILE=/certs/tls.crt \ + -e MGMT_API_TLS_KEY_FILE=/certs/tls.key -e MGMT_API_TLS_ALLOWED_CLIENTS="cass-operator,Reaper Client" ... + +Spaces inside an identity are kept, so do not put spaces around the commas. + ### Virtual tables (Cassandra 5.0 and newer) The agent registers the read-only `management_api.jobs` and `management_api.job_events` @@ -674,6 +686,7 @@ Once the tag is pushed, the release process will start and build the Docker imag [ {-K | --no-keep-alive} ] [ {-p | --pidfile} ] {-S | --cassandra-socket | --db-socket} + [ --tlsallowedclient ... ] [ --tlscacert ] [ --tlscert ] [ --tlskey ] @@ -715,6 +728,13 @@ Once the tag is pushed, the release process will start and build the Docker imag readable and writable. + --tlsallowedclient + Only accept TLS clients whose certificate subject CN or a + DNS/URI/email/IP subject alternative name matches this value, + can be repeated. DNS names match case-insensitively and IP + addresses in any notation, everything else must match exactly. + If not set, any client certificate signed by the CA is accepted + --tlscacert Path to trust certs signed only by this CA diff --git a/cassandra-trunk/files/docker-entrypoint.sh b/cassandra-trunk/files/docker-entrypoint.sh index bf06e5d2..378ae5b2 100755 --- a/cassandra-trunk/files/docker-entrypoint.sh +++ b/cassandra-trunk/files/docker-entrypoint.sh @@ -182,6 +182,17 @@ if [ "$1" = 'mgmtapi' ]; then MGMT_API_TLS_KEY_FILE="--tlskey $MGMT_API_TLS_KEY_FILE" MGMT_API_ARGS="$MGMT_API_ARGS $MGMT_API_TLS_KEY_FILE" fi + # Kept out of MGMT_API_ARGS because identities may contain spaces or glob characters + MGMT_API_TLS_ALLOWED_CLIENT_ARGS=() + if [ ! -z "$MGMT_API_TLS_ALLOWED_CLIENTS" ]; then + # read only consumes one line, so treat newlines in multi-line values as separators too + IFS=',' read -r -a MGMT_API_TLS_ALLOWED_CLIENT_LIST <<< "${MGMT_API_TLS_ALLOWED_CLIENTS//$'\n'/,}" + for MGMT_API_TLS_ALLOWED_CLIENT in "${MGMT_API_TLS_ALLOWED_CLIENT_LIST[@]}"; do + if [ ! -z "$MGMT_API_TLS_ALLOWED_CLIENT" ]; then + MGMT_API_TLS_ALLOWED_CLIENT_ARGS+=(--tlsallowedclient "$MGMT_API_TLS_ALLOWED_CLIENT") + fi + done + fi if [ ! -z "$MGMT_API_PID_FILE" ]; then MGMT_API_PID_FILE="--pidfile $MGMT_API_PID_FILE" @@ -200,8 +211,8 @@ if [ "$1" = 'mgmtapi' ]; then # use default of 128m heap if env variable not set : "${MGMT_API_HEAP_SIZE:=128m}" - echo "Running" java ${MGMT_API_JAVA_OPTS} -Xms${MGMT_API_HEAP_SIZE} -Xmx${MGMT_API_HEAP_SIZE} -jar "$MGMT_API_JAR" $MGMT_API_ARGS - java ${MGMT_API_JAVA_OPTS} -Xms${MGMT_API_HEAP_SIZE} -Xmx${MGMT_API_HEAP_SIZE} -jar "$MGMT_API_JAR" $MGMT_API_ARGS + echo "Running" java ${MGMT_API_JAVA_OPTS} -Xms${MGMT_API_HEAP_SIZE} -Xmx${MGMT_API_HEAP_SIZE} -jar "$MGMT_API_JAR" $MGMT_API_ARGS "${MGMT_API_TLS_ALLOWED_CLIENT_ARGS[@]}" + java ${MGMT_API_JAVA_OPTS} -Xms${MGMT_API_HEAP_SIZE} -Xmx${MGMT_API_HEAP_SIZE} -jar "$MGMT_API_JAR" $MGMT_API_ARGS "${MGMT_API_TLS_ALLOWED_CLIENT_ARGS[@]}" fi exec "$@" diff --git a/cassandra/scripts/docker-entrypoint-jdk8.sh b/cassandra/scripts/docker-entrypoint-jdk8.sh index a158f082..cbad4a30 100755 --- a/cassandra/scripts/docker-entrypoint-jdk8.sh +++ b/cassandra/scripts/docker-entrypoint-jdk8.sh @@ -177,6 +177,17 @@ if [ "$1" = 'mgmtapi' ]; then MGMT_API_TLS_KEY_FILE="--tlskey $MGMT_API_TLS_KEY_FILE" MGMT_API_ARGS="$MGMT_API_ARGS $MGMT_API_TLS_KEY_FILE" fi + # Kept out of MGMT_API_ARGS because identities may contain spaces or glob characters + MGMT_API_TLS_ALLOWED_CLIENT_ARGS=() + if [ ! -z "$MGMT_API_TLS_ALLOWED_CLIENTS" ]; then + # read only consumes one line, so treat newlines in multi-line values as separators too + IFS=',' read -r -a MGMT_API_TLS_ALLOWED_CLIENT_LIST <<< "${MGMT_API_TLS_ALLOWED_CLIENTS//$'\n'/,}" + for MGMT_API_TLS_ALLOWED_CLIENT in "${MGMT_API_TLS_ALLOWED_CLIENT_LIST[@]}"; do + if [ ! -z "$MGMT_API_TLS_ALLOWED_CLIENT" ]; then + MGMT_API_TLS_ALLOWED_CLIENT_ARGS+=(--tlsallowedclient "$MGMT_API_TLS_ALLOWED_CLIENT") + fi + done + fi if [ ! -z "$MGMT_API_PID_FILE" ]; then MGMT_API_PID_FILE="--pidfile $MGMT_API_PID_FILE" @@ -211,8 +222,8 @@ if [ "$1" = 'mgmtapi' ]; then JAVA11_JAVA=${JAVA11_HOME}/bin/java fi fi - echo "Running" ${JAVA11_JAVA} ${MGMT_API_JAVA_OPTS} -Xms${MGMT_API_HEAP_SIZE} -Xmx${MGMT_API_HEAP_SIZE} -jar "$MGMT_API_JAR" $MGMT_API_ARGS - ${JAVA11_JAVA} ${MGMT_API_JAVA_OPTS} -Xms${MGMT_API_HEAP_SIZE} -Xmx${MGMT_API_HEAP_SIZE} -jar "$MGMT_API_JAR" $MGMT_API_ARGS + echo "Running" ${JAVA11_JAVA} ${MGMT_API_JAVA_OPTS} -Xms${MGMT_API_HEAP_SIZE} -Xmx${MGMT_API_HEAP_SIZE} -jar "$MGMT_API_JAR" $MGMT_API_ARGS "${MGMT_API_TLS_ALLOWED_CLIENT_ARGS[@]}" + ${JAVA11_JAVA} ${MGMT_API_JAVA_OPTS} -Xms${MGMT_API_HEAP_SIZE} -Xmx${MGMT_API_HEAP_SIZE} -jar "$MGMT_API_JAR" $MGMT_API_ARGS "${MGMT_API_TLS_ALLOWED_CLIENT_ARGS[@]}" fi exec "$@" diff --git a/cassandra/scripts/docker-entrypoint.sh b/cassandra/scripts/docker-entrypoint.sh index eb0e1c47..d301da33 100644 --- a/cassandra/scripts/docker-entrypoint.sh +++ b/cassandra/scripts/docker-entrypoint.sh @@ -177,6 +177,17 @@ if [ "$1" = 'mgmtapi' ]; then MGMT_API_TLS_KEY_FILE="--tlskey $MGMT_API_TLS_KEY_FILE" MGMT_API_ARGS="$MGMT_API_ARGS $MGMT_API_TLS_KEY_FILE" fi + # Kept out of MGMT_API_ARGS because identities may contain spaces or glob characters + MGMT_API_TLS_ALLOWED_CLIENT_ARGS=() + if [ ! -z "$MGMT_API_TLS_ALLOWED_CLIENTS" ]; then + # read only consumes one line, so treat newlines in multi-line values as separators too + IFS=',' read -r -a MGMT_API_TLS_ALLOWED_CLIENT_LIST <<< "${MGMT_API_TLS_ALLOWED_CLIENTS//$'\n'/,}" + for MGMT_API_TLS_ALLOWED_CLIENT in "${MGMT_API_TLS_ALLOWED_CLIENT_LIST[@]}"; do + if [ ! -z "$MGMT_API_TLS_ALLOWED_CLIENT" ]; then + MGMT_API_TLS_ALLOWED_CLIENT_ARGS+=(--tlsallowedclient "$MGMT_API_TLS_ALLOWED_CLIENT") + fi + done + fi if [ ! -z "$MGMT_API_PID_FILE" ]; then MGMT_API_PID_FILE="--pidfile $MGMT_API_PID_FILE" @@ -196,8 +207,8 @@ if [ "$1" = 'mgmtapi' ]; then # use default of 128m heap if env variable not set : "${MGMT_API_HEAP_SIZE:=128m}" - echo "Running" java ${MGMT_API_JAVA_OPTS} -Xms${MGMT_API_HEAP_SIZE} -Xmx${MGMT_API_HEAP_SIZE} -jar "$MGMT_API_JAR" $MGMT_API_ARGS - java ${MGMT_API_JAVA_OPTS} -Xms${MGMT_API_HEAP_SIZE} -Xmx${MGMT_API_HEAP_SIZE} -jar "$MGMT_API_JAR" $MGMT_API_ARGS + echo "Running" java ${MGMT_API_JAVA_OPTS} -Xms${MGMT_API_HEAP_SIZE} -Xmx${MGMT_API_HEAP_SIZE} -jar "$MGMT_API_JAR" $MGMT_API_ARGS "${MGMT_API_TLS_ALLOWED_CLIENT_ARGS[@]}" + java ${MGMT_API_JAVA_OPTS} -Xms${MGMT_API_HEAP_SIZE} -Xmx${MGMT_API_HEAP_SIZE} -jar "$MGMT_API_JAR" $MGMT_API_ARGS "${MGMT_API_TLS_ALLOWED_CLIENT_ARGS[@]}" fi exec "$@" diff --git a/dse/files/entrypoint.sh b/dse/files/entrypoint.sh index 6bad67e5..8aafdf20 100755 --- a/dse/files/entrypoint.sh +++ b/dse/files/entrypoint.sh @@ -158,6 +158,17 @@ if [ "$USE_MGMT_API" = "true" ] && [ -d "$MAAC_PATH" ] ; then MGMT_API_TLS_KEY_FILE="--tlskey $MGMT_API_TLS_KEY_FILE" MGMT_API_ARGS="$MGMT_API_ARGS $MGMT_API_TLS_KEY_FILE" fi + # Kept out of MGMT_API_ARGS because identities may contain spaces or glob characters + MGMT_API_TLS_ALLOWED_CLIENT_ARGS=() + if [ ! -z "$MGMT_API_TLS_ALLOWED_CLIENTS" ]; then + # read only consumes one line, so treat newlines in multi-line values as separators too + IFS=',' read -r -a MGMT_API_TLS_ALLOWED_CLIENT_LIST <<< "${MGMT_API_TLS_ALLOWED_CLIENTS//$'\n'/,}" + for MGMT_API_TLS_ALLOWED_CLIENT in "${MGMT_API_TLS_ALLOWED_CLIENT_LIST[@]}"; do + if [ ! -z "$MGMT_API_TLS_ALLOWED_CLIENT" ]; then + MGMT_API_TLS_ALLOWED_CLIENT_ARGS+=(--tlsallowedclient "$MGMT_API_TLS_ALLOWED_CLIENT") + fi + done + fi if [ ! -z "$MGMT_API_PID_FILE" ]; then MGMT_API_PID_FILE="--pidfile $MGMT_API_PID_FILE" @@ -206,8 +217,8 @@ if [ "$USE_MGMT_API" = "true" ] && [ -d "$MAAC_PATH" ] ; then echo "/tmp/dse.sock already exists or is already symlinked" fi - echo "Running" ${JAVA11_JAVA} ${MGMT_API_JAVA_OPTS} -Xms${MGMT_API_HEAP_SIZE} -Xmx${MGMT_API_HEAP_SIZE} -jar "$MGMT_API_JAR" $MGMT_API_ARGS - ${JAVA11_JAVA} ${MGMT_API_JAVA_OPTS} -Xms${MGMT_API_HEAP_SIZE} -Xmx${MGMT_API_HEAP_SIZE} -jar "$MGMT_API_JAR" $MGMT_API_ARGS + echo "Running" ${JAVA11_JAVA} ${MGMT_API_JAVA_OPTS} -Xms${MGMT_API_HEAP_SIZE} -Xmx${MGMT_API_HEAP_SIZE} -jar "$MGMT_API_JAR" $MGMT_API_ARGS "${MGMT_API_TLS_ALLOWED_CLIENT_ARGS[@]}" + ${JAVA11_JAVA} ${MGMT_API_JAVA_OPTS} -Xms${MGMT_API_HEAP_SIZE} -Xmx${MGMT_API_HEAP_SIZE} -jar "$MGMT_API_JAR" $MGMT_API_ARGS "${MGMT_API_TLS_ALLOWED_CLIENT_ARGS[@]}" else echo "Running $@" exec "$@" diff --git a/management-api-server/src/main/java/com/datastax/mgmtapi/AllowedClientsTrustManager.java b/management-api-server/src/main/java/com/datastax/mgmtapi/AllowedClientsTrustManager.java new file mode 100644 index 00000000..181947ec --- /dev/null +++ b/management-api-server/src/main/java/com/datastax/mgmtapi/AllowedClientsTrustManager.java @@ -0,0 +1,204 @@ +/* + * Copyright DataStax, Inc. + * + * Please see the included license file for details. + */ +package com.datastax.mgmtapi; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableSet; +import com.google.common.net.InetAddresses; +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.InetAddress; +import java.net.Socket; +import java.security.GeneralSecurityException; +import java.security.KeyStore; +import java.security.cert.CertificateException; +import java.security.cert.CertificateFactory; +import java.security.cert.CertificateParsingException; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Locale; +import java.util.Set; +import javax.naming.NamingEnumeration; +import javax.naming.NamingException; +import javax.naming.directory.Attribute; +import javax.naming.ldap.LdapName; +import javax.naming.ldap.Rdn; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.TrustManager; +import javax.net.ssl.TrustManagerFactory; +import javax.net.ssl.X509ExtendedTrustManager; + +// Being signed by the CA is not enough when the CA also issues certificates for other services +public class AllowedClientsTrustManager extends X509ExtendedTrustManager { + private static final int SAN_RFC822_NAME = 1; + private static final int SAN_DNS_NAME = 2; + private static final int SAN_URI = 6; + private static final int SAN_IP_ADDRESS = 7; + + private final X509ExtendedTrustManager delegate; + private final Set allowedNames; + // DNS names are case-insensitive (RFC 4343) + private final Set allowedDnsNames; + // The JDK renders IP SANs in a fixed form (IPv6 uncompressed), so compare addresses, not strings + private final Set allowedIpAddresses; + + @VisibleForTesting + AllowedClientsTrustManager(X509ExtendedTrustManager delegate, Collection allowedClients) { + this.delegate = delegate; + this.allowedNames = ImmutableSet.copyOf(allowedClients); + ImmutableSet.Builder dnsNames = ImmutableSet.builder(); + ImmutableSet.Builder ipAddresses = ImmutableSet.builder(); + for (String client : allowedClients) { + dnsNames.add(client.toLowerCase(Locale.ROOT)); + if (InetAddresses.isInetAddress(client)) { + ipAddresses.add(InetAddresses.forString(client)); + } + } + this.allowedDnsNames = dnsNames.build(); + this.allowedIpAddresses = ipAddresses.build(); + } + + public static AllowedClientsTrustManager forCaFile(File caCertFile, List allowedClients) + throws GeneralSecurityException, IOException { + KeyStore trustStore = KeyStore.getInstance(KeyStore.getDefaultType()); + trustStore.load(null, null); + int i = 0; + try (InputStream in = new FileInputStream(caCertFile)) { + for (java.security.cert.Certificate cert : + CertificateFactory.getInstance("X.509").generateCertificates(in)) { + trustStore.setCertificateEntry("ca-" + i++, cert); + } + } + // An empty trust store would reject every client instead of failing at startup like Netty does + if (i == 0) { + throw new CertificateException(caCertFile + " does not contain valid certificates"); + } + + TrustManagerFactory tmf = + TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); + tmf.init(trustStore); + for (TrustManager tm : tmf.getTrustManagers()) { + if (tm instanceof X509ExtendedTrustManager) { + return new AllowedClientsTrustManager((X509ExtendedTrustManager) tm, allowedClients); + } + } + throw new GeneralSecurityException("No X509ExtendedTrustManager available"); + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType) + throws CertificateException { + delegate.checkClientTrusted(chain, authType); + checkAllowed(chain); + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) + throws CertificateException { + delegate.checkClientTrusted(chain, authType, socket); + checkAllowed(chain); + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) + throws CertificateException { + delegate.checkClientTrusted(chain, authType, engine); + checkAllowed(chain); + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType) + throws CertificateException { + delegate.checkServerTrusted(chain, authType); + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) + throws CertificateException { + delegate.checkServerTrusted(chain, authType, socket); + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) + throws CertificateException { + delegate.checkServerTrusted(chain, authType, engine); + } + + @Override + public X509Certificate[] getAcceptedIssuers() { + return delegate.getAcceptedIssuers(); + } + + private void checkAllowed(X509Certificate[] chain) throws CertificateException { + if (chain == null || chain.length == 0) { + throw new CertificateException("No client certificate presented"); + } + List identities = new ArrayList<>(); + if (isAllowed(chain[0], identities)) { + return; + } + throw new CertificateException( + "Client certificate " + identities + " is not in the list of allowed clients"); + } + + @VisibleForTesting + boolean isAllowed(X509Certificate cert, List identities) + throws CertificateParsingException { + try { + for (Rdn rdn : new LdapName(cert.getSubjectX500Principal().getName()).getRdns()) { + // Rdn.getType() only exposes the first attribute of a multi-valued RDN such as + // CN=x+SERIALNUMBER=y + Attribute cns = rdn.toAttributes().get("CN"); + if (cns == null) { + continue; + } + NamingEnumeration values = cns.getAll(); + while (values.hasMore()) { + String cn = values.next().toString(); + identities.add(cn); + if (allowedNames.contains(cn)) { + return true; + } + } + } + } catch (NamingException e) { + throw new CertificateParsingException("Unable to parse client certificate subject", e); + } + + Collection> sans = cert.getSubjectAlternativeNames(); + if (sans == null) { + return false; + } + for (List san : sans) { + int type = (Integer) san.get(0); + if (type != SAN_RFC822_NAME + && type != SAN_DNS_NAME + && type != SAN_URI + && type != SAN_IP_ADDRESS) { + continue; + } + String value = (String) san.get(1); + identities.add(value); + if (type == SAN_DNS_NAME) { + if (allowedDnsNames.contains(value.toLowerCase(Locale.ROOT))) { + return true; + } + } else if (type == SAN_IP_ADDRESS) { + if (InetAddresses.isInetAddress(value) + && allowedIpAddresses.contains(InetAddresses.forString(value))) { + return true; + } + } else if (allowedNames.contains(value)) { + return true; + } + } + return false; + } +} diff --git a/management-api-server/src/main/java/com/datastax/mgmtapi/Cli.java b/management-api-server/src/main/java/com/datastax/mgmtapi/Cli.java index 3ad7a34d..4ca87111 100644 --- a/management-api-server/src/main/java/com/datastax/mgmtapi/Cli.java +++ b/management-api-server/src/main/java/com/datastax/mgmtapi/Cli.java @@ -51,6 +51,7 @@ import java.nio.file.WatchEvent; import java.nio.file.WatchKey; import java.nio.file.WatchService; +import java.security.GeneralSecurityException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -166,6 +167,13 @@ public class Cli implements Runnable { private File tlsKey; + @Option( + name = {"--tlsallowedclient"}, + arity = 1, + description = + "Only accept TLS clients whose certificate subject CN or a DNS/URI/email/IP subject alternative name matches this value, can be repeated. DNS names match case-insensitively and IP addresses in any notation, everything else must match exactly. If not set, any client certificate signed by the CA is accepted") + private List tls_allowed_clients = new ArrayList<>(); + private boolean useTls = false; private File dbUnixSocketFile = null; private File dbHomeDir = null; @@ -199,6 +207,29 @@ public Cli( String caCertFile, String certFile, String certKeyFile) { + this( + listenAddresses, + dbHomeDir, + dbUnixSocketFile, + keepAlive, + dbExtraArgs, + caCertFile, + certFile, + certKeyFile, + Collections.emptyList()); + } + + @VisibleForTesting + public Cli( + List listenAddresses, + String dbHomeDir, + String dbUnixSocketFile, + boolean keepAlive, + Collection dbExtraArgs, + String caCertFile, + String certFile, + String certKeyFile, + List allowedClients) { this.listen_address = listenAddresses; this.db_home = dbHomeDir; this.db_unix_socket_file = dbUnixSocketFile; @@ -207,6 +238,7 @@ public Cli( this.tls_ca_cert_file = caCertFile; this.tls_cert_file = certFile; this.tls_key_file = certKeyFile; + this.tls_allowed_clients = new ArrayList<>(allowedClients); } @Override @@ -457,6 +489,11 @@ void checkTLSDeps() { tlsKey = new File(tls_key_file); } + if (!tls_allowed_clients.isEmpty() && !hasAny) { + logger.error("TLS CA Cert, Cert and Key files are required when allowed clients are set"); + System.exit(17); + } + useTls = hasAny; } @@ -469,12 +506,31 @@ void preflightChecks() { @VisibleForTesting void createSSLContext() throws SSLException { - this.sslContext = - SslContextBuilder.forServer(tlsCert, tlsKey) - .trustManager(tlsCaCert) - .clientAuth(ClientAuth.REQUIRE) - .ciphers(null, IdentityCipherSuiteFilter.INSTANCE) - .build(); + this.sslContext = buildSslContext(tlsCaCert, tlsCert, tlsKey, tls_allowed_clients); + } + + @VisibleForTesting + SslContext getSslContext() { + return sslContext; + } + + @VisibleForTesting + static SslContext buildSslContext(File caCert, File cert, File key, List allowedClients) + throws SSLException { + SslContextBuilder builder = SslContextBuilder.forServer(cert, key); + if (allowedClients.isEmpty()) { + builder.trustManager(caCert); + } else { + try { + builder.trustManager(AllowedClientsTrustManager.forCaFile(caCert, allowedClients)); + } catch (GeneralSecurityException | IOException e) { + throw new SSLException("Unable to load CA certificates from " + caCert, e); + } + } + return builder + .clientAuth(ClientAuth.REQUIRE) + .ciphers(null, IdentityCipherSuiteFilter.INSTANCE) + .build(); } @VisibleForTesting diff --git a/management-api-server/src/test/java/com/datastax/mgmtapi/AllowedClientsTrustManagerTest.java b/management-api-server/src/test/java/com/datastax/mgmtapi/AllowedClientsTrustManagerTest.java new file mode 100644 index 00000000..1cadcdf0 --- /dev/null +++ b/management-api-server/src/test/java/com/datastax/mgmtapi/AllowedClientsTrustManagerTest.java @@ -0,0 +1,110 @@ +/* + * Copyright DataStax, Inc. + * + * Please see the included license file for details. + */ +package com.datastax.mgmtapi; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.google.common.collect.ImmutableList; +import java.io.File; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import javax.security.auth.x500.X500Principal; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +public class AllowedClientsTrustManagerTest { + @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @Test + public void testCommonNameMatchesExactly() throws Exception { + X509Certificate cert = cert("CN=Reaper Client,O=Example", null); + + assertTrue(allows(cert, "Reaper Client")); + assertFalse(allows(cert, "reaper client")); + assertFalse(allows(cert, "Reaper")); + } + + @Test + public void testCommonNameInMultiValuedRdnMatches() throws Exception { + X509Certificate cert = cert("CN=allowed+SERIALNUMBER=123,O=Example", null); + + assertTrue(allows(cert, "allowed")); + } + + @Test + public void testDnsNameMatchesCaseInsensitively() throws Exception { + X509Certificate cert = cert("CN=other", ImmutableList.of(san(2, "Client.Example.com"))); + + assertTrue(allows(cert, "client.example.com")); + assertFalse(allows(cert, "example.com")); + } + + @Test + public void testIpAddressMatchesInAnyNotation() throws Exception { + // The JDK renders IPv6 SANs uncompressed + X509Certificate cert = + cert( + "CN=other", + ImmutableList.of( + san(7, "0:0:0:0:0:0:0:1"), san(7, "fd00:0:0:0:0:0:0:5"), san(7, "10.0.0.1"))); + + assertTrue(allows(cert, "::1")); + assertTrue(allows(cert, "FD00::5")); + assertTrue(allows(cert, "10.0.0.1")); + assertFalse(allows(cert, "10.0.0.2")); + } + + @Test + public void testUriAndEmailMatchExactly() throws Exception { + X509Certificate cert = + cert( + "CN=other", + ImmutableList.of(san(6, "spiffe://cluster/ns/reaper"), san(1, "ops@example.com"))); + + assertTrue(allows(cert, "spiffe://cluster/ns/reaper")); + assertTrue(allows(cert, "ops@example.com")); + assertFalse(allows(cert, "SPIFFE://cluster/ns/reaper")); + } + + @Test + public void testOtherSanTypesAreIgnored() throws Exception { + // A directoryName SAN must not be matched as if it were a CN + X509Certificate cert = cert("CN=other", ImmutableList.of(san(4, "allowed"))); + + assertFalse(allows(cert, "allowed")); + } + + @Test(expected = CertificateException.class) + public void testCaFileWithoutCertificatesIsRejected() throws Exception { + File emptyCa = temporaryFolder.newFile("empty-ca.pem"); + + AllowedClientsTrustManager.forCaFile(emptyCa, ImmutableList.of("client")); + } + + private static boolean allows(X509Certificate cert, String allowedClient) throws Exception { + List identities = new ArrayList<>(); + return new AllowedClientsTrustManager(null, ImmutableList.of(allowedClient)) + .isAllowed(cert, identities); + } + + private static X509Certificate cert(String subject, Collection> sans) throws Exception { + X509Certificate cert = mock(X509Certificate.class); + when(cert.getSubjectX500Principal()).thenReturn(new X500Principal(subject)); + when(cert.getSubjectAlternativeNames()).thenReturn(sans); + return cert; + } + + private static List san(int type, String value) { + return ImmutableList.of(type, value); + } +} diff --git a/management-api-server/src/test/java/com/datastax/mgmtapi/NettyTlsClientAuthTest.java b/management-api-server/src/test/java/com/datastax/mgmtapi/NettyTlsClientAuthTest.java index b40e0e5e..5fe03589 100644 --- a/management-api-server/src/test/java/com/datastax/mgmtapi/NettyTlsClientAuthTest.java +++ b/management-api-server/src/test/java/com/datastax/mgmtapi/NettyTlsClientAuthTest.java @@ -203,6 +203,88 @@ public void testTrustedChainBadClientCert() throws Throwable { clientKeyPassword); } + @Test + public void testAllowedClient() throws Throwable { + File trustCertFile = + IntegrationTestUtils.getFile(getClass(), "mutual_auth_client_cert_chain.pem"); + File serverKeyFile = IntegrationTestUtils.getFile(getClass(), "mutual_auth_server.key"); + File serverCrtFile = IntegrationTestUtils.getFile(getClass(), "mutual_auth_server.crt"); + File clientKeyFile = IntegrationTestUtils.getFile(getClass(), "mutual_auth_client.key"); + File clientCrtFile = IntegrationTestUtils.getFile(getClass(), "mutual_auth_client.crt"); + + SslContext serverSslCtx = + Cli.buildSslContext( + trustCertFile, + serverCrtFile, + serverKeyFile, + Lists.newArrayList("SomeOtherClient", "NettyTestClient")); + + testHTTPS(serverSslCtx, trustCertFile, clientKeyFile, clientCrtFile, null); + } + + @Test(expected = SSLException.class) + public void testTrustedButNotAllowedClient() throws Throwable { + File trustCertFile = + IntegrationTestUtils.getFile(getClass(), "mutual_auth_client_cert_chain.pem"); + File serverKeyFile = IntegrationTestUtils.getFile(getClass(), "mutual_auth_server.key"); + File serverCrtFile = IntegrationTestUtils.getFile(getClass(), "mutual_auth_server.crt"); + File clientKeyFile = IntegrationTestUtils.getFile(getClass(), "mutual_auth_client.key"); + File clientCrtFile = IntegrationTestUtils.getFile(getClass(), "mutual_auth_client.crt"); + + SslContext serverSslCtx = + Cli.buildSslContext( + trustCertFile, serverCrtFile, serverKeyFile, Lists.newArrayList("NettyTestServer")); + + testHTTPS(serverSslCtx, trustCertFile, clientKeyFile, clientCrtFile, null); + } + + @Test(expected = SSLException.class) + public void testAllowedClientWithUntrustedCert() throws Throwable { + File trustCertFile = + IntegrationTestUtils.getFile(getClass(), "mutual_auth_client_cert_chain.pem"); + File serverKeyFile = IntegrationTestUtils.getFile(getClass(), "mutual_auth_server.key"); + File serverCrtFile = IntegrationTestUtils.getFile(getClass(), "mutual_auth_server.crt"); + File clientTrustFile = + IntegrationTestUtils.getFile(getClass(), "mutual_auth_invalid_client_cert_chain.pem"); + File clientKeyFile = IntegrationTestUtils.getFile(getClass(), "mutual_auth_invalid_client.key"); + File clientCrtFile = IntegrationTestUtils.getFile(getClass(), "mutual_auth_invalid_client.crt"); + + SslContext serverSslCtx = + Cli.buildSslContext( + trustCertFile, + serverCrtFile, + serverKeyFile, + Lists.newArrayList("NettyTestInvalidClient")); + + testHTTPS(serverSslCtx, clientTrustFile, clientKeyFile, clientCrtFile, null); + } + + @Test(expected = SSLException.class) + public void testCliAppliesAllowedClients() throws Throwable { + File trustCertFile = + IntegrationTestUtils.getFile(getClass(), "mutual_auth_client_cert_chain.pem"); + File serverKeyFile = IntegrationTestUtils.getFile(getClass(), "mutual_auth_server.key"); + File serverCrtFile = IntegrationTestUtils.getFile(getClass(), "mutual_auth_server.crt"); + File clientKeyFile = IntegrationTestUtils.getFile(getClass(), "mutual_auth_client.key"); + File clientCrtFile = IntegrationTestUtils.getFile(getClass(), "mutual_auth_client.crt"); + + Cli cli = + new Cli( + Lists.newArrayList(BASE_URI), + null, + null, + false, + Lists.newArrayList(), + trustCertFile.getAbsolutePath(), + serverCrtFile.getAbsolutePath(), + serverKeyFile.getAbsolutePath(), + Lists.newArrayList("NettyTestServer")); + cli.checkTLSDeps(); + cli.createSSLContext(); + + testHTTPS(cli.getSslContext(), trustCertFile, clientKeyFile, clientCrtFile, null); + } + public void testHTTPS( File servertTrustCrtFile, File serverKeyFile, @@ -222,6 +304,16 @@ public void testHTTPS( .sessionTimeout(0) .build(); + testHTTPS(serverSslCtx, clientTrustCrtFile, clientKeyFile, clientCrtFile, clientKeyPassword); + } + + private void testHTTPS( + SslContext serverSslCtx, + File clientTrustCrtFile, + File clientKeyFile, + File clientCrtFile, + String clientKeyPassword) + throws Throwable { SslContext clientSslCtx = clientCrtFile == null ? null