diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java index 82b1bb3fd1e..1c1d6ca88de 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java @@ -74,7 +74,7 @@ public SslContext createNettySslContextForClient(ZKConfig config) SslContextBuilder sslContextBuilder = SslContextBuilder.forClient(); - KeyManager km = buildKeyManager(config); + KeyManager km = buildClientKeyManager(config); if (km != null) { sslContextBuilder.keyManager(km); } @@ -116,7 +116,7 @@ public SslContext createNettySslContextForServer(ZKConfig config) throw new X509Exception.SSLContextException( "Keystore is required for SSL server: " + getSslKeystoreLocationProperty()); } - return createNettySslContextForServer(config, km, buildTrustManager(config)); + return createNettySslContextForServer(config, km, buildServerTrustManager(config)); } public SslContext createNettySslContextForServer(ZKConfig config, KeyManager keyManager, TrustManager trustManager) throws SSLException { diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/SSLContextAndOptions.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/SSLContextAndOptions.java index c712f6acb6c..f45761fa396 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/SSLContextAndOptions.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/SSLContextAndOptions.java @@ -31,7 +31,7 @@ import org.slf4j.LoggerFactory; /** - * Wrapper class for an SSLContext + some config options that can't be set on the context when it is created but + * Wrapper class for SSLContexts and config options that can't be set on the context when it is created but * must be set on a secure socket created by the context after the socket creation. By wrapping the options in this * class we avoid reading from global system properties during socket configuration. This makes testing easier * since we can create different X509Util instances with different configurations in a single test process, and @@ -45,52 +45,68 @@ public class SSLContextAndOptions { private final String[] enabledProtocols; private final String[] cipherSuites; private final X509Util.ClientAuth clientAuth; - private final SSLContext sslContext; + private final SSLContext clientSSLContext; + private final SSLContext serverSSLContext; private final int handshakeDetectionTimeoutMillis; - /** + * Creates an SSLContextAndOptions with separate client and server contexts. * Note: constructor is intentionally package-private, only the X509Util class should be creating instances of this * class. * @param x509Util the X509Util that created this object. * @param config a ZKConfig that holds config properties. - * @param sslContext the SSLContext. + * @param clientSSLContext the SSLContext for connecting as a client. + * @param serverSSLContext the SSLContext for accepting connections as a server. */ - SSLContextAndOptions(final X509Util x509Util, final ZKConfig config, final SSLContext sslContext) { + SSLContextAndOptions(final X509Util x509Util, final ZKConfig config, + final SSLContext clientSSLContext, final SSLContext serverSSLContext) { this.x509Util = requireNonNull(x509Util); - this.sslContext = requireNonNull(sslContext); - this.enabledProtocols = getEnabledProtocols(requireNonNull(config), sslContext); + this.clientSSLContext = requireNonNull(clientSSLContext); + this.serverSSLContext = requireNonNull(serverSSLContext); + this.enabledProtocols = getEnabledProtocols(requireNonNull(config), clientSSLContext); this.cipherSuites = getCipherSuites(config); this.clientAuth = getClientAuth(config); this.handshakeDetectionTimeoutMillis = getHandshakeDetectionTimeoutMillis(config); } - public SSLContext getSSLContext() { - return sslContext; + /** + * Creates an SSLContextAndOptions with a single context used for both roles. + * Note: constructor is intentionally package-private, only the X509Util class should be creating instances of this + * class. + * @param x509Util the X509Util that created this object. + * @param config a ZKConfig that holds config properties. + * @param sslContext the SSLContext. + */ + SSLContextAndOptions(final X509Util x509Util, final ZKConfig config, final SSLContext sslContext) { + this(x509Util, config, sslContext, sslContext); + } + + public SSLContext getClientSSLContext() { + return clientSSLContext; } public SSLSocket createSSLSocket() throws IOException { - return configureSSLSocket((SSLSocket) sslContext.getSocketFactory().createSocket(), true); + return configureSSLSocket((SSLSocket) clientSSLContext.getSocketFactory().createSocket(), true); } public SSLSocket createSSLSocket(Socket socket, byte[] pushbackBytes) throws IOException { SSLSocket sslSocket; if (pushbackBytes != null && pushbackBytes.length > 0) { - sslSocket = (SSLSocket) sslContext.getSocketFactory() + sslSocket = (SSLSocket) serverSSLContext.getSocketFactory() .createSocket(socket, new ByteArrayInputStream(pushbackBytes), true); } else { - sslSocket = (SSLSocket) sslContext.getSocketFactory().createSocket(socket, null, socket.getPort(), true); + sslSocket = (SSLSocket) serverSSLContext.getSocketFactory().createSocket(socket, null, socket.getPort(), true); } return configureSSLSocket(sslSocket, false); } public SSLServerSocket createSSLServerSocket() throws IOException { - SSLServerSocket sslServerSocket = (SSLServerSocket) sslContext.getServerSocketFactory().createServerSocket(); + SSLServerSocket sslServerSocket = (SSLServerSocket) serverSSLContext.getServerSocketFactory().createServerSocket(); return configureSSLServerSocket(sslServerSocket); } public SSLServerSocket createSSLServerSocket(int port) throws IOException { - SSLServerSocket sslServerSocket = (SSLServerSocket) sslContext.getServerSocketFactory().createServerSocket(port); + SSLServerSocket sslServerSocket = (SSLServerSocket) serverSSLContext.getServerSocketFactory().createServerSocket(port); return configureSSLServerSocket(sslServerSocket); } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java index 2a91e87d217..eb066048b85 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java @@ -169,10 +169,18 @@ public io.netty.handler.ssl.ClientAuth toNettyClientAuth() { private final String sslKeystorePasswdProperty = getConfigPrefix() + "keyStore.password"; private final String sslKeystorePasswdPathProperty = getConfigPrefix() + "keyStore.passwordPath"; private final String sslKeystoreTypeProperty = getConfigPrefix() + "keyStore.type"; + private final String sslClientKeystoreLocationProperty = getConfigPrefix() + "client.keyStore.location"; + private final String sslClientKeystorePasswdProperty = getConfigPrefix() + "client.keyStore.password"; + private final String sslClientKeystorePasswdPathProperty = getConfigPrefix() + "client.keyStore.passwordPath"; + private final String sslClientKeystoreTypeProperty = getConfigPrefix() + "client.keyStore.type"; private final String sslTruststoreLocationProperty = getConfigPrefix() + "trustStore.location"; private final String sslTruststorePasswdProperty = getConfigPrefix() + "trustStore.password"; private final String sslTruststorePasswdPathProperty = getConfigPrefix() + "trustStore.passwordPath"; private final String sslTruststoreTypeProperty = getConfigPrefix() + "trustStore.type"; + private final String sslServerTruststoreLocationProperty = getConfigPrefix() + "server.trustStore.location"; + private final String sslServerTruststorePasswdProperty = getConfigPrefix() + "server.trustStore.password"; + private final String sslServerTruststorePasswdPathProperty = getConfigPrefix() + "server.trustStore.passwordPath"; + private final String sslServerTruststoreTypeProperty = getConfigPrefix() + "server.trustStore.type"; private final String sslContextSupplierClassProperty = getConfigPrefix() + "context.supplier.class"; private final String sslHostnameVerificationEnabledProperty = getConfigPrefix() + "hostnameVerification"; private final String sslClientHostnameVerificationEnabledProperty = getConfigPrefix() + "clientHostnameVerification"; @@ -185,10 +193,15 @@ public io.netty.handler.ssl.ClientAuth toNettyClientAuth() { private final AtomicReference defaultSSLContextAndOptions = new AtomicReference<>(null); private FileChangeWatcher keyStoreFileWatcher; + private FileChangeWatcher clientKeyStoreFileWatcher; private FileChangeWatcher trustStoreFileWatcher; + private FileChangeWatcher serverTrustStoreFileWatcher; public X509Util() { - keyStoreFileWatcher = trustStoreFileWatcher = null; + keyStoreFileWatcher = null; + clientKeyStoreFileWatcher = null; + trustStoreFileWatcher = null; + serverTrustStoreFileWatcher = null; } protected abstract String getConfigPrefix(); @@ -227,6 +240,22 @@ public String getSslKeystoreTypeProperty() { return sslKeystoreTypeProperty; } + public String getSslClientKeystoreLocationProperty() { + return sslClientKeystoreLocationProperty; + } + + public String getSslClientKeystorePasswdProperty() { + return sslClientKeystorePasswdProperty; + } + + public String getSslClientKeystorePasswdPathProperty() { + return sslClientKeystorePasswdPathProperty; + } + + public String getSslClientKeystoreTypeProperty() { + return sslClientKeystoreTypeProperty; + } + public String getSslTruststoreLocationProperty() { return sslTruststoreLocationProperty; } @@ -243,6 +272,22 @@ public String getSslTruststoreTypeProperty() { return sslTruststoreTypeProperty; } + public String getSslServerTruststoreLocationProperty() { + return sslServerTruststoreLocationProperty; + } + + public String getSslServerTruststorePasswdProperty() { + return sslServerTruststorePasswdProperty; + } + + public String getSslServerTruststorePasswdPathProperty() { + return sslServerTruststorePasswdPathProperty; + } + + public String getSslServerTruststoreTypeProperty() { + return sslServerTruststoreTypeProperty; + } + public String getSslContextSupplierClassProperty() { return sslContextSupplierClassProperty; } @@ -303,11 +348,11 @@ public boolean allowReverseDnsLookup(ZKConfig config) { } public SSLContext getDefaultSSLContext() throws X509Exception.SSLContextException { - return getDefaultSSLContextAndOptions().getSSLContext(); + return getDefaultSSLContextAndOptions().getClientSSLContext(); } public SSLContext createSSLContext(ZKConfig config) throws SSLContextException { - return createSSLContextAndOptions(config).getSSLContext(); + return createSSLContextAndOptions(config).getClientSSLContext(); } public SSLContextAndOptions getDefaultSSLContextAndOptions() throws X509Exception.SSLContextException { @@ -408,13 +453,17 @@ public SSLContextAndOptions createSSLContextAndOptionsFromConfig(ZKConfig config // There are legal states in some use cases for null KeyManager or TrustManager. // But if a user wanna specify one, location is required. Password defaults to empty string if it is not // specified by the user. - KeyManager[] keyManagers = null; - TrustManager[] trustManagers = null; + String defaultTlsProtocol = defaultTlsProtocol(config); + String protocol = config.getProperty(sslProtocolProperty, defaultTlsProtocol); + + // Client context — used for outgoing connections (client role) + KeyManager[] clientKeyManagers = null; + TrustManager[] clientTrustManagers = null; try { - KeyManager km = buildKeyManager(config); - if (km != null) { - keyManagers = new KeyManager[]{km}; + X509KeyManager clientKm = buildClientKeyManager(config); + if (clientKm != null) { + clientKeyManagers = new KeyManager[]{clientKm}; } } catch (KeyManagerException keyManagerException) { throw new SSLContextException("Failed to create KeyManager", keyManagerException); @@ -424,30 +473,61 @@ public SSLContextAndOptions createSSLContextAndOptionsFromConfig(ZKConfig config } try { - TrustManager tm = buildTrustManager(config); - if (tm != null) { - trustManagers = new TrustManager[]{tm}; + X509TrustManager clientTm = buildTrustManager(config); + if (clientTm != null) { + clientTrustManagers = new TrustManager[]{clientTm}; } } catch (TrustManagerException trustManagerException) { throw new SSLContextException("Failed to create TrustManager", trustManagerException); } catch (IllegalArgumentException e) { String trustStoreTypeProp = config.getProperty(sslTruststoreTypeProperty); throw new SSLContextException("Bad value for " - + sslTruststoreTypeProperty - + ": " - + trustStoreTypeProp, e); + + sslTruststoreTypeProperty + ": " + trustStoreTypeProp, e); } - String defaultTlsProtocol = defaultTlsProtocol(config); - String protocol = config.getProperty(sslProtocolProperty, defaultTlsProtocol); + + // Server context — used for incoming connections (server role) + KeyManager[] serverKeyManagers = null; + TrustManager[] serverTrustManagers = null; + + try { + X509KeyManager serverKm = buildKeyManager(config); + if (serverKm != null) { + serverKeyManagers = new KeyManager[]{serverKm}; + } + } catch (KeyManagerException keyManagerException) { + throw new SSLContextException("Failed to create KeyManager", keyManagerException); + } catch (IllegalArgumentException e) { + String keyStoreTypeProp = config.getProperty(sslKeystoreTypeProperty); + throw new SSLContextException("Bad value for " + sslKeystoreTypeProperty + ": " + keyStoreTypeProp, e); + } + try { - SSLContext sslContext = SSLContext.getInstance(protocol); - sslContext.init(keyManagers, trustManagers, null); - return new SSLContextAndOptions(this, config, sslContext); + X509TrustManager serverTm = buildServerTrustManager(config); + if (serverTm != null) { + serverTrustManagers = new TrustManager[]{serverTm}; + } + } catch (TrustManagerException trustManagerException) { + throw new SSLContextException("Failed to create TrustManager", trustManagerException); + } catch (IllegalArgumentException e) { + String trustStoreTypeProp = config.getProperty(sslServerTruststoreTypeProperty); + throw new SSLContextException("Bad value for " + + sslServerTruststoreTypeProperty + ": " + trustStoreTypeProp, e); + } + + try { + SSLContext clientCtx = SSLContext.getInstance(protocol); + clientCtx.init(clientKeyManagers, clientTrustManagers, null); + + SSLContext serverCtx = SSLContext.getInstance(protocol); + serverCtx.init(serverKeyManagers, serverTrustManagers, null); + + return new SSLContextAndOptions(this, config, clientCtx, serverCtx); } catch (NoSuchAlgorithmException | KeyManagementException sslContextInitException) { throw new SSLContextException(sslContextInitException); } } + public static KeyStore loadKeyStore( String keyStoreLocation, String keyStorePassword, @@ -506,6 +586,22 @@ public X509KeyManager buildKeyManager(ZKConfig config) throws KeyManagerExceptio return createKeyManager(keyStoreLocation, keyStorePassword, keyStoreType); } + /** + * Builds a key manager for the client role. If a dedicated client keystore + * is configured ({@code client.keyStore.location}), it is loaded and returned. + * Otherwise, falls back to the shared keystore via {@link #buildKeyManager(ZKConfig)}. + */ + public X509KeyManager buildClientKeyManager(ZKConfig config) throws KeyManagerException { + String clientKeyStoreLocation = config.getProperty(getSslClientKeystoreLocationProperty(), ""); + if (!clientKeyStoreLocation.isEmpty()) { + String clientKeyStorePassword = getPasswordFromConfigPropertyOrFile(config, + getSslClientKeystorePasswdProperty(), getSslClientKeystorePasswdPathProperty()); + String clientKeyStoreType = config.getProperty(getSslClientKeystoreTypeProperty()); + return createKeyManager(clientKeyStoreLocation, clientKeyStorePassword, clientKeyStoreType); + } + return buildKeyManager(config); + } + /** * Creates a key manager by loading the key store from the given file of * the given type, optionally decrypting it using the given password. @@ -548,6 +644,22 @@ public X509TrustManager buildTrustManager(ZKConfig config) throws TrustManagerEx return null; } + return buildX509TrustManager(config, trustStoreLocationProp, sslTruststorePasswdProperty, sslTruststorePasswdPathProperty, sslTruststoreTypeProperty); + } + + public X509TrustManager buildServerTrustManager(ZKConfig config) throws TrustManagerException { + String serverTrustStoreLocation = config.getProperty(sslServerTruststoreLocationProperty, ""); + if (serverTrustStoreLocation.isEmpty()) { + return buildTrustManager(config); + } + + return buildX509TrustManager(config, serverTrustStoreLocation, sslServerTruststorePasswdProperty, sslServerTruststorePasswdPathProperty, sslServerTruststoreTypeProperty); + } + + private X509TrustManager buildX509TrustManager(ZKConfig config, String trustStoreLocationProp, + String sslTruststorePasswdProperty, + String sslTruststorePasswdPathProperty, + String sslTruststoreTypeProperty) throws TrustManagerException { String trustStorePasswordProp = getPasswordFromConfigPropertyOrFile(config, sslTruststorePasswdProperty, sslTruststorePasswdPathProperty); String trustStoreTypeProp = config.getProperty(sslTruststoreTypeProperty); @@ -571,6 +683,7 @@ public X509TrustManager buildTrustManager(ZKConfig config) throws TrustManagerEx fipsMode); } + // @VisibleForTesting protected X509TrustManager createTrustManagerInternal( String trustStoreLocation, @@ -743,6 +856,15 @@ public void enableCertFileReloading() throws IOException { keyStoreFileWatcher = newKeyStoreFileWatcher; keyStoreFileWatcher.start(); } + FileChangeWatcher newClientKeyStoreFileWatcher = newFileChangeWatcher(config.getProperty(sslClientKeystoreLocationProperty)); + if (newClientKeyStoreFileWatcher != null) { + // stop old watcher if there is one + if (clientKeyStoreFileWatcher != null) { + clientKeyStoreFileWatcher.stop(); + } + clientKeyStoreFileWatcher = newClientKeyStoreFileWatcher; + clientKeyStoreFileWatcher.start(); + } FileChangeWatcher newTrustStoreFileWatcher = newFileChangeWatcher(config.getProperty(sslTruststoreLocationProperty)); if (newTrustStoreFileWatcher != null) { // stop old watcher if there is one @@ -752,6 +874,15 @@ public void enableCertFileReloading() throws IOException { trustStoreFileWatcher = newTrustStoreFileWatcher; trustStoreFileWatcher.start(); } + FileChangeWatcher newServerTrustStoreFileWatcher = newFileChangeWatcher(config.getProperty(sslServerTruststoreLocationProperty)); + if (newServerTrustStoreFileWatcher != null) { + // stop old watcher if there is one + if (serverTrustStoreFileWatcher != null) { + serverTrustStoreFileWatcher.stop(); + } + serverTrustStoreFileWatcher = newServerTrustStoreFileWatcher; + serverTrustStoreFileWatcher.start(); + } } /** @@ -765,10 +896,18 @@ public void close() { keyStoreFileWatcher.stop(); keyStoreFileWatcher = null; } + if (clientKeyStoreFileWatcher != null) { + clientKeyStoreFileWatcher.stop(); + clientKeyStoreFileWatcher = null; + } if (trustStoreFileWatcher != null) { trustStoreFileWatcher.stop(); trustStoreFileWatcher = null; } + if (serverTrustStoreFileWatcher != null) { + serverTrustStoreFileWatcher.stop(); + serverTrustStoreFileWatcher = null; + } } /** diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java index 442d8e7221d..a40bb3c533c 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java @@ -146,10 +146,18 @@ private void putSSLProperties(X509Util x509Util) { properties.put(x509Util.getSslKeystorePasswdProperty(), System.getProperty(x509Util.getSslKeystorePasswdProperty())); properties.put(x509Util.getSslKeystorePasswdPathProperty(), System.getProperty(x509Util.getSslKeystorePasswdPathProperty())); properties.put(x509Util.getSslKeystoreTypeProperty(), System.getProperty(x509Util.getSslKeystoreTypeProperty())); + properties.put(x509Util.getSslClientKeystoreLocationProperty(), System.getProperty(x509Util.getSslClientKeystoreLocationProperty())); + properties.put(x509Util.getSslClientKeystorePasswdProperty(), System.getProperty(x509Util.getSslClientKeystorePasswdProperty())); + properties.put(x509Util.getSslClientKeystorePasswdPathProperty(), System.getProperty(x509Util.getSslClientKeystorePasswdPathProperty())); + properties.put(x509Util.getSslClientKeystoreTypeProperty(), System.getProperty(x509Util.getSslClientKeystoreTypeProperty())); properties.put(x509Util.getSslTruststoreLocationProperty(), System.getProperty(x509Util.getSslTruststoreLocationProperty())); properties.put(x509Util.getSslTruststorePasswdProperty(), System.getProperty(x509Util.getSslTruststorePasswdProperty())); properties.put(x509Util.getSslTruststorePasswdPathProperty(), System.getProperty(x509Util.getSslTruststorePasswdPathProperty())); properties.put(x509Util.getSslTruststoreTypeProperty(), System.getProperty(x509Util.getSslTruststoreTypeProperty())); + properties.put(x509Util.getSslServerTruststoreLocationProperty(), System.getProperty(x509Util.getSslServerTruststoreLocationProperty())); + properties.put(x509Util.getSslServerTruststorePasswdProperty(), System.getProperty(x509Util.getSslServerTruststorePasswdProperty())); + properties.put(x509Util.getSslServerTruststorePasswdPathProperty(), System.getProperty(x509Util.getSslServerTruststorePasswdPathProperty())); + properties.put(x509Util.getSslServerTruststoreTypeProperty(), System.getProperty(x509Util.getSslServerTruststoreTypeProperty())); properties.put(x509Util.getSslContextSupplierClassProperty(), System.getProperty(x509Util.getSslContextSupplierClassProperty())); properties.put(x509Util.getSslClientHostnameVerificationEnabledProperty(), System.getProperty(x509Util.getSslClientHostnameVerificationEnabledProperty())); properties.put(x509Util.getSslHostnameVerificationEnabledProperty(), System.getProperty(x509Util.getSslHostnameVerificationEnabledProperty())); diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestContext.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestContext.java index 80632e45c01..45fad846d3d 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestContext.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestContext.java @@ -393,10 +393,18 @@ public void clearSystemProperties(X509Util x509Util) { System.clearProperty(x509Util.getSslKeystorePasswdProperty()); System.clearProperty(x509Util.getSslKeystorePasswdPathProperty()); System.clearProperty(x509Util.getSslKeystoreTypeProperty()); + System.clearProperty(x509Util.getSslClientKeystoreLocationProperty()); + System.clearProperty(x509Util.getSslClientKeystorePasswdProperty()); + System.clearProperty(x509Util.getSslClientKeystorePasswdPathProperty()); + System.clearProperty(x509Util.getSslClientKeystoreTypeProperty()); System.clearProperty(x509Util.getSslTruststoreLocationProperty()); System.clearProperty(x509Util.getSslTruststorePasswdProperty()); System.clearProperty(x509Util.getSslTruststorePasswdPathProperty()); System.clearProperty(x509Util.getSslTruststoreTypeProperty()); + System.clearProperty(x509Util.getSslServerTruststoreLocationProperty()); + System.clearProperty(x509Util.getSslServerTruststorePasswdProperty()); + System.clearProperty(x509Util.getSslServerTruststorePasswdPathProperty()); + System.clearProperty(x509Util.getSslServerTruststoreTypeProperty()); System.clearProperty(x509Util.getSslHostnameVerificationEnabledProperty()); } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestHelpers.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestHelpers.java index 761c718b865..192ad7ae5db 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestHelpers.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestHelpers.java @@ -165,6 +165,13 @@ public static X509Certificate newCert( public static X509Certificate newCert( X509Certificate caCert, KeyPair caKeyPair, X500Name certSubject, PublicKey certPublicKey, long expirationMillis, CertificateCustomization customization) throws Exception { + return newCert(caCert, caKeyPair, certSubject, certPublicKey, expirationMillis, + new KeyPurposeId[]{KeyPurposeId.id_kp_serverAuth, KeyPurposeId.id_kp_clientAuth}, customization); + } + + public static X509Certificate newCert( + X509Certificate caCert, KeyPair caKeyPair, X500Name certSubject, PublicKey certPublicKey, + long expirationMillis, KeyPurposeId[] keyPurposes, CertificateCustomization customization) throws Exception { if (!caKeyPair.getPublic().equals(caCert.getPublicKey())) { throw new IllegalArgumentException("CA private key does not match the public key in the CA cert"); } @@ -175,7 +182,7 @@ public static X509Certificate newCert( builder.addExtension(Extension.basicConstraints, true, new BasicConstraints(false)); // not a CA builder.addExtension(Extension.keyUsage, true, new KeyUsage(KeyUsage.digitalSignature | KeyUsage.keyEncipherment)); - builder.addExtension(Extension.extendedKeyUsage, true, new ExtendedKeyUsage(new KeyPurposeId[]{KeyPurposeId.id_kp_serverAuth, KeyPurposeId.id_kp_clientAuth})); + builder.addExtension(Extension.extendedKeyUsage, true, new ExtendedKeyUsage(keyPurposes)); builder.addExtension(Extension.subjectAlternativeName, false, getLocalhostSubjectAltNames()); if (customization != null) { @@ -184,6 +191,25 @@ public static X509Certificate newCert( return buildAndSignCertificate(caKeyPair.getPrivate(), builder); } + public static X509Certificate newServerOnlyCert(X509Certificate caCert, KeyPair caKeyPair, + String name, PublicKey certPublicKey) throws Exception { + return createSinglePurposeCert(name, caCert, caKeyPair, certPublicKey, KeyPurposeId.id_kp_serverAuth); + } + + public static X509Certificate newClientOnlyCert(X509Certificate caCert, KeyPair caKeyPair, + String name, PublicKey certPublicKey) throws Exception { + return createSinglePurposeCert(name, caCert, caKeyPair, certPublicKey, KeyPurposeId.id_kp_clientAuth); + } + + private static X509Certificate createSinglePurposeCert( + String name, X509Certificate caCert, KeyPair caKeyPair, PublicKey certPublicKey, KeyPurposeId keyPurposeId) + throws Exception { + X500NameBuilder nameBuilder = new X500NameBuilder(BCStyle.INSTANCE); + nameBuilder.addRDN(BCStyle.CN, name); + return newCert(caCert, caKeyPair, nameBuilder.build(), certPublicKey, Duration.ofDays(1).toMillis(), + new KeyPurposeId[]{keyPurposeId}, null); + } + /** * Returns subject alternative names for "localhost". * @return the subject alternative names for "localhost". diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java index f76b91ddf9f..d7f1354d2e8 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java @@ -23,18 +23,23 @@ import static org.apache.zookeeper.common.X509Util.TLS_1_3; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import io.netty.buffer.UnpooledByteBufAllocator; import io.netty.handler.ssl.JdkSslContext; import io.netty.handler.ssl.SslContext; +import java.io.File; +import java.io.FileOutputStream; import java.io.IOException; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.ServerSocket; import java.net.Socket; import java.nio.file.Path; +import java.security.KeyPair; import java.security.NoSuchAlgorithmException; +import java.security.cert.X509Certificate; import java.util.Arrays; import java.util.List; import java.util.concurrent.Callable; @@ -953,4 +958,276 @@ private void testCreateSSLContext_withWrongPasswordFromFile(final String keyPass x509Util.getDefaultSSLContext(); }); } + + @ParameterizedTest + @MethodSource("data") + @Timeout(value = 5) + public void testSeparateClientKeyStore_backwardCompat( + X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex) + throws Exception { + init(caKeyType, certKeyType, keyPassword, paramIndex); + // No clientKeyStore properties set — should behave identically to existing code + SSLContext ctx = x509Util.getDefaultSSLContext(); + assertNotNull(ctx); + } + + @ParameterizedTest + @MethodSource("data") + @Timeout(value = 15) + public void testSeparateClientKeyStore_singleEkuCertsHandshake( + X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex) + throws Exception { + init(caKeyType, certKeyType, keyPassword, paramIndex); + // Generate separate server-only and client-only certs + KeyPair serverKeyPair = X509TestHelpers.generateKeyPair(certKeyType); + X509Certificate serverCert = X509TestHelpers.newServerOnlyCert( + x509TestContext.getTrustStoreCertificates().get(0), + x509TestContext.getTrustStoreKeyPair(), + "server", serverKeyPair.getPublic()); + + KeyPair clientKeyPair = X509TestHelpers.generateKeyPair(certKeyType); + X509Certificate clientCert = X509TestHelpers.newClientOnlyCert( + x509TestContext.getTrustStoreCertificates().get(0), + x509TestContext.getTrustStoreKeyPair(), + "client", clientKeyPair.getPublic()); + + // Write server keystore + File serverKsFile = File.createTempFile("server_ks", ".jks", x509TestContext.getTempDir()); + serverKsFile.deleteOnExit(); + try (FileOutputStream fos = new FileOutputStream(serverKsFile)) { + fos.write(X509TestHelpers.certAndPrivateKeyToJavaKeyStoreBytes( + serverCert, serverKeyPair.getPrivate(), keyPassword)); + } + + // Write client keystore + File clientKsFile = File.createTempFile("client_ks", ".jks", x509TestContext.getTempDir()); + clientKsFile.deleteOnExit(); + try (FileOutputStream fos = new FileOutputStream(clientKsFile)) { + fos.write(X509TestHelpers.certAndPrivateKeyToJavaKeyStoreBytes( + clientCert, clientKeyPair.getPrivate(), keyPassword)); + } + + // Set properties: server keystore as keyStore, client keystore as clientKeyStore + System.setProperty(x509Util.getSslKeystoreLocationProperty(), serverKsFile.getAbsolutePath()); + System.setProperty(x509Util.getSslKeystorePasswdProperty(), keyPassword); + System.setProperty(x509Util.getSslKeystoreTypeProperty(), "JKS"); + System.setProperty(x509Util.getSslClientKeystoreLocationProperty(), clientKsFile.getAbsolutePath()); + System.setProperty(x509Util.getSslClientKeystorePasswdProperty(), keyPassword); + System.setProperty(x509Util.getSslClientKeystoreTypeProperty(), "JKS"); + + x509Util.close(); // reset cached context + x509Util = new ClientX509Util(); + + // Create server socket and client socket, perform handshake + int port = PortAssignment.unique(); + SSLServerSocket serverSocket = x509Util.createSSLServerSocket(port); + serverSocket.setSoTimeout(5000); + + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + Future serverFuture = executor.submit(() -> { + try (SSLSocket accepted = (SSLSocket) serverSocket.accept()) { + accepted.startHandshake(); + } + return null; + }); + + SSLSocket clientSocket = x509Util.createSSLSocket(); + clientSocket.connect(new InetSocketAddress(InetAddress.getLoopbackAddress(), port), 3000); + clientSocket.startHandshake(); + clientSocket.close(); + + serverFuture.get(5, TimeUnit.SECONDS); + } finally { + serverSocket.close(); + executor.shutdownNow(); + System.clearProperty(x509Util.getSslClientKeystoreLocationProperty()); + System.clearProperty(x509Util.getSslClientKeystorePasswdProperty()); + System.clearProperty(x509Util.getSslClientKeystoreTypeProperty()); + } + } + + @ParameterizedTest + @MethodSource("data") + @Timeout(value = 5) + public void testSeparateClientKeyStore_nettyClientUsesClientKeyStore( + X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex) + throws Exception { + init(caKeyType, certKeyType, keyPassword, paramIndex); + // Generate separate keystores + KeyPair clientKeyPair = X509TestHelpers.generateKeyPair(certKeyType); + X509Certificate clientCert = X509TestHelpers.newClientOnlyCert( + x509TestContext.getTrustStoreCertificates().get(0), + x509TestContext.getTrustStoreKeyPair(), + "client", clientKeyPair.getPublic()); + + File clientKsFile = File.createTempFile("client_ks", ".jks", x509TestContext.getTempDir()); + clientKsFile.deleteOnExit(); + try (FileOutputStream fos = new FileOutputStream(clientKsFile)) { + fos.write(X509TestHelpers.certAndPrivateKeyToJavaKeyStoreBytes( + clientCert, clientKeyPair.getPrivate(), keyPassword)); + } + + System.setProperty(x509Util.getSslClientKeystoreLocationProperty(), clientKsFile.getAbsolutePath()); + System.setProperty(x509Util.getSslClientKeystorePasswdProperty(), keyPassword); + System.setProperty(x509Util.getSslClientKeystoreTypeProperty(), "JKS"); + + try { + // Netty client context should load from clientKeyStore + SslContext clientCtx = ((ClientX509Util) x509Util).createNettySslContextForClient(new ZKConfig()); + assertNotNull(clientCtx); + + // Netty server context should still load from keyStore (the main one) + SslContext serverCtx = ((ClientX509Util) x509Util).createNettySslContextForServer(new ZKConfig()); + assertNotNull(serverCtx); + } finally { + System.clearProperty(x509Util.getSslClientKeystoreLocationProperty()); + System.clearProperty(x509Util.getSslClientKeystorePasswdProperty()); + System.clearProperty(x509Util.getSslClientKeystoreTypeProperty()); + } + } + + @ParameterizedTest + @MethodSource("data") + @Timeout(value = 5) + public void testSeparateServerTrustStore_backwardCompat( + X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex) + throws Exception { + init(caKeyType, certKeyType, keyPassword, paramIndex); + // Without server.trustStore configured, existing trustStore is used for both roles + x509Util.close(); + x509Util = new ClientX509Util(); + + int port = PortAssignment.unique(); + SSLServerSocket serverSocket = x509Util.createSSLServerSocket(port); + serverSocket.setSoTimeout(5000); + + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + Future serverFuture = executor.submit(() -> { + try (SSLSocket accepted = (SSLSocket) serverSocket.accept()) { + accepted.startHandshake(); + } + return null; + }); + + SSLSocket clientSocket = x509Util.createSSLSocket(); + clientSocket.connect(new InetSocketAddress(InetAddress.getLoopbackAddress(), port), 3000); + clientSocket.startHandshake(); + clientSocket.close(); + + serverFuture.get(5, TimeUnit.SECONDS); + } finally { + serverSocket.close(); + executor.shutdownNow(); + } + } + + @ParameterizedTest + @MethodSource("data") + @Timeout(value = 5) + public void testSeparateServerTrustStore_separateCAs( + X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex) + throws Exception { + init(caKeyType, certKeyType, keyPassword, paramIndex); + + // Create a separate CA for client certs + KeyPair clientCaKeyPair = X509TestHelpers.generateKeyPair(caKeyType); + X509Certificate clientCaCert = X509TestHelpers.newSelfSignedCACert( + new org.bouncycastle.asn1.x500.X500NameBuilder(org.bouncycastle.asn1.x500.style.BCStyle.INSTANCE) + .addRDN(org.bouncycastle.asn1.x500.style.BCStyle.CN, "Client CA") + .build(), + clientCaKeyPair, 86400000L); + + // The existing CA is used for server certs + KeyPair serverCaKeyPair = x509TestContext.getTrustStoreKeyPair(); + X509Certificate serverCaCert = x509TestContext.getTrustStoreCertificates().get(0); + + // Generate server cert signed by server CA + KeyPair serverKeyPair = X509TestHelpers.generateKeyPair(certKeyType); + X509Certificate serverCert = X509TestHelpers.newServerOnlyCert( + serverCaCert, serverCaKeyPair, "server", serverKeyPair.getPublic()); + + // Generate client cert signed by client CA + KeyPair clientKeyPair = X509TestHelpers.generateKeyPair(certKeyType); + X509Certificate clientCert = X509TestHelpers.newClientOnlyCert( + clientCaCert, clientCaKeyPair, "client", clientKeyPair.getPublic()); + + // Server keystore with server cert + File serverKsFile = File.createTempFile("server_ks", ".jks", x509TestContext.getTempDir()); + serverKsFile.deleteOnExit(); + try (FileOutputStream fos = new FileOutputStream(serverKsFile)) { + fos.write(X509TestHelpers.certAndPrivateKeyToJavaKeyStoreBytes( + serverCert, serverKeyPair.getPrivate(), keyPassword)); + } + + // Client keystore with client cert + File clientKsFile = File.createTempFile("client_ks", ".jks", x509TestContext.getTempDir()); + clientKsFile.deleteOnExit(); + try (FileOutputStream fos = new FileOutputStream(clientKsFile)) { + fos.write(X509TestHelpers.certAndPrivateKeyToJavaKeyStoreBytes( + clientCert, clientKeyPair.getPrivate(), keyPassword)); + } + + // Client truststore: trusts server CA (validates servers we connect to) + File clientTsFile = File.createTempFile("client_ts", ".jks", x509TestContext.getTempDir()); + clientTsFile.deleteOnExit(); + try (FileOutputStream fos = new FileOutputStream(clientTsFile)) { + fos.write(X509TestHelpers.certToJavaTrustStoreBytes(serverCaCert, keyPassword)); + } + + // Server truststore: trusts client CA (validates clients connecting to us) + File serverTsFile = File.createTempFile("server_ts", ".jks", x509TestContext.getTempDir()); + serverTsFile.deleteOnExit(); + try (FileOutputStream fos = new FileOutputStream(serverTsFile)) { + fos.write(X509TestHelpers.certToJavaTrustStoreBytes(clientCaCert, keyPassword)); + } + + System.setProperty(x509Util.getSslKeystoreLocationProperty(), serverKsFile.getAbsolutePath()); + System.setProperty(x509Util.getSslKeystorePasswdProperty(), keyPassword); + System.setProperty(x509Util.getSslKeystoreTypeProperty(), "JKS"); + System.setProperty(x509Util.getSslClientKeystoreLocationProperty(), clientKsFile.getAbsolutePath()); + System.setProperty(x509Util.getSslClientKeystorePasswdProperty(), keyPassword); + System.setProperty(x509Util.getSslClientKeystoreTypeProperty(), "JKS"); + System.setProperty(x509Util.getSslTruststoreLocationProperty(), clientTsFile.getAbsolutePath()); + System.setProperty(x509Util.getSslTruststorePasswdProperty(), keyPassword); + System.setProperty(x509Util.getSslTruststoreTypeProperty(), "JKS"); + System.setProperty(x509Util.getSslServerTruststoreLocationProperty(), serverTsFile.getAbsolutePath()); + System.setProperty(x509Util.getSslServerTruststorePasswdProperty(), keyPassword); + System.setProperty(x509Util.getSslServerTruststoreTypeProperty(), "JKS"); + + x509Util.close(); + x509Util = new ClientX509Util(); + + int port = PortAssignment.unique(); + SSLServerSocket serverSocket = x509Util.createSSLServerSocket(port); + serverSocket.setSoTimeout(5000); + + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + Future serverFuture = executor.submit(() -> { + try (SSLSocket accepted = (SSLSocket) serverSocket.accept()) { + accepted.startHandshake(); + } + return null; + }); + + SSLSocket clientSocket = x509Util.createSSLSocket(); + clientSocket.connect(new InetSocketAddress(InetAddress.getLoopbackAddress(), port), 3000); + clientSocket.startHandshake(); + clientSocket.close(); + + serverFuture.get(5, TimeUnit.SECONDS); + } finally { + serverSocket.close(); + executor.shutdownNow(); + System.clearProperty(x509Util.getSslClientKeystoreLocationProperty()); + System.clearProperty(x509Util.getSslClientKeystorePasswdProperty()); + System.clearProperty(x509Util.getSslClientKeystoreTypeProperty()); + System.clearProperty(x509Util.getSslServerTruststoreLocationProperty()); + System.clearProperty(x509Util.getSslServerTruststorePasswdProperty()); + System.clearProperty(x509Util.getSslServerTruststoreTypeProperty()); + } + } + }