Skip to content

Commit

Permalink
ZOOKEEPER-4393 Problem to connect to zookeeper in FIPS mode (apache#2008
Browse files Browse the repository at this point in the history 
) (#54)

Co-authored-by: Andor Molnár <andor@apache.org>
  • Loading branch information
@anurag-harness @anmolnar
anurag-harness and anmolnar committed Aug 31, 2023
1 parent 2c05a66 commit 1d99df6
Show file tree
Hide file tree
Showing 10 changed files with 269 additions and 120 deletions.
9 changes: 9 additions & 0 deletions zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import java.util.concurrent.locks.ReentrantLock;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLParameters;
import org.apache.zookeeper.ClientCnxn.EndOfStreamException;
import org.apache.zookeeper.ClientCnxn.Packet;
import org.apache.zookeeper.client.ZKClientConfig;
Expand Down Expand Up @@ -452,6 +453,14 @@ private synchronized void initSSL(ChannelPipeline pipeline) throws SSLContextExc
sslContext = x509Util.createSSLContext(clientConfig);
sslEngine = sslContext.createSSLEngine(host, port);
sslEngine.setUseClientMode(true);
if (x509Util.getFipsMode(clientConfig) && x509Util.isServerHostnameVerificationEnabled(clientConfig)) {
SSLParameters sslParameters = sslEngine.getSSLParameters();
sslParameters.setEndpointIdentificationAlgorithm("HTTPS");
sslEngine.setSSLParameters(sslParameters);
if (LOG.isDebugEnabled()) {
LOG.debug("Server hostname verification: enabled HTTPS style endpoint identification algorithm");
}
}
}
}
pipeline.addLast("ssl", new SslHandler(sslEngine));
Expand Down
43 changes: 23 additions & 20 deletions zookeeper-server/src/main/java/org/apache/zookeeper/client/FourLetterWordMain.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,27 +92,28 @@ public static String send4LetterWord(
boolean secure,
int timeout) throws IOException, SSLContextException {
LOG.info("connecting to {} {}", host, port);
Socket sock;
InetSocketAddress hostaddress = host != null
? new InetSocketAddress(host, port)
: new InetSocketAddress(InetAddress.getByName(null), port);
if (secure) {
LOG.info("using secure socket");
try (X509Util x509Util = new ClientX509Util()) {
SSLContext sslContext = x509Util.getDefaultSSLContext();
SSLSocketFactory socketFactory = sslContext.getSocketFactory();
SSLSocket sslSock = (SSLSocket) socketFactory.createSocket();
sslSock.connect(hostaddress, timeout);
sslSock.startHandshake();
sock = sslSock;
}
} else {
sock = new Socket();
sock.connect(hostaddress, timeout);
}
sock.setSoTimeout(timeout);

Socket sock = null;
BufferedReader reader = null;
try {
InetSocketAddress hostaddress = host != null
? new InetSocketAddress(host, port)
: new InetSocketAddress(InetAddress.getByName(null), port);
if (secure) {
LOG.info("using secure socket");
try (X509Util x509Util = new ClientX509Util()) {
SSLContext sslContext = x509Util.getDefaultSSLContext();
SSLSocketFactory socketFactory = sslContext.getSocketFactory();
SSLSocket sslSock = (SSLSocket) socketFactory.createSocket();
sslSock.connect(hostaddress, timeout);
sslSock.startHandshake();
sock = sslSock;
}
} else {
sock = new Socket();
sock.connect(hostaddress, timeout);
}
sock.setSoTimeout(timeout);
OutputStream outstream = sock.getOutputStream();
outstream.write(cmd.getBytes(UTF_8));
outstream.flush();
Expand All @@ -133,7 +134,9 @@ public static String send4LetterWord(
} catch (SocketTimeoutException e) {
throw new IOException("Exception while executing four letter word: " + cmd, e);
} finally {
sock.close();
if (sock != null) {
sock.close();
}
if (reader != null) {
reader.close();
}
Expand Down
31 changes: 25 additions & 6 deletions zookeeper-server/src/main/java/org/apache/zookeeper/common/SSLContextAndOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.apache.zookeeper.common;

import static java.util.Objects.requireNonNull;
import io.netty.handler.ssl.DelegatingSslContext;
import io.netty.handler.ssl.IdentityCipherSuiteFilter;
import io.netty.handler.ssl.JdkSslContext;
import io.netty.handler.ssl.SslContext;
Expand All @@ -29,6 +30,7 @@
import java.util.Collections;
import java.util.List;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLParameters;
import javax.net.ssl.SSLServerSocket;
import javax.net.ssl.SSLSocket;
Expand All @@ -53,6 +55,8 @@ public class SSLContextAndOptions {
private final X509Util.ClientAuth clientAuth;
private final SSLContext sslContext;
private final int handshakeDetectionTimeoutMillis;
private final ZKConfig config;


/**
* Note: constructor is intentionally package-private, only the X509Util class should be creating instances of this
Expand All @@ -70,6 +74,7 @@ public class SSLContextAndOptions {
this.cipherSuitesAsList = Collections.unmodifiableList(Arrays.asList(ciphers));
this.clientAuth = getClientAuth(config);
this.handshakeDetectionTimeoutMillis = getHandshakeDetectionTimeoutMillis(config);
this.config = config;
}

public SSLContext getSSLContext() {
Expand Down Expand Up @@ -101,18 +106,32 @@ public SSLServerSocket createSSLServerSocket(int port) throws IOException {
return configureSSLServerSocket(sslServerSocket);
}

public SslContext createNettyJdkSslContext(SSLContext sslContext, boolean isClientSocket) {
return new JdkSslContext(
public SslContext createNettyJdkSslContext(SSLContext sslContext) {
SslContext sslContext1 = new JdkSslContext(
sslContext,
isClientSocket,
false,
cipherSuitesAsList,
IdentityCipherSuiteFilter.INSTANCE,
null,
isClientSocket
? X509Util.ClientAuth.NONE.toNettyClientAuth()
: clientAuth.toNettyClientAuth(),
clientAuth.toNettyClientAuth(),
enabledProtocols,
false);

if (x509Util.getFipsMode(config) && x509Util.isClientHostnameVerificationEnabled(config)) {
return new DelegatingSslContext(sslContext1) {
@Override
protected void initEngine(SSLEngine sslEngine) {
SSLParameters sslParameters = sslEngine.getSSLParameters();
sslParameters.setEndpointIdentificationAlgorithm("HTTPS");
sslEngine.setSSLParameters(sslParameters);
if (LOG.isDebugEnabled()) {
LOG.debug("Client hostname verification: enabled HTTPS style endpoint identification algorithm");
}
}
};
} else {
return sslContext1;
}
}

public int getHandshakeDetectionTimeoutMillis() {
Expand Down
112 changes: 70 additions & 42 deletions zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public abstract class X509Util implements Closeable, AutoCloseable {
private static final Logger LOG = LoggerFactory.getLogger(X509Util.class);

private static final String REJECT_CLIENT_RENEGOTIATION_PROPERTY = "jdk.tls.rejectClientInitiatedRenegotiation";
private static final String FIPS_MODE_PROPERTY = "zookeeper.fips-mode";

static {
// Client-initiated renegotiation in TLS is unsafe and
Expand Down Expand Up @@ -145,36 +146,30 @@ public io.netty.handler.ssl.ClientAuth toNettyClientAuth() {
}
}

private String sslProtocolProperty = getConfigPrefix() + "protocol";
private String sslEnabledProtocolsProperty = getConfigPrefix() + "enabledProtocols";
private String cipherSuitesProperty = getConfigPrefix() + "ciphersuites";
private String sslKeystoreLocationProperty = getConfigPrefix() + "keyStore.location";
private String sslKeystorePasswdProperty = getConfigPrefix() + "keyStore.password";
private String sslKeystorePasswdPathProperty = getConfigPrefix() + "keyStore.passwordPath";
private String sslKeystoreTypeProperty = getConfigPrefix() + "keyStore.type";
private String sslTruststoreLocationProperty = getConfigPrefix() + "trustStore.location";
private String sslTruststorePasswdProperty = getConfigPrefix() + "trustStore.password";
private String sslTruststorePasswdPathProperty = getConfigPrefix() + "trustStore.passwordPath";
private String sslTruststoreTypeProperty = getConfigPrefix() + "trustStore.type";
private String sslContextSupplierClassProperty = getConfigPrefix() + "context.supplier.class";
private String sslHostnameVerificationEnabledProperty = getConfigPrefix() + "hostnameVerification";
private String sslCrlEnabledProperty = getConfigPrefix() + "crl";
private String sslOcspEnabledProperty = getConfigPrefix() + "ocsp";
private String sslClientAuthProperty = getConfigPrefix() + "clientAuth";
private String sslHandshakeDetectionTimeoutMillisProperty = getConfigPrefix() + "handshakeDetectionTimeoutMillis";

private ZKConfig zkConfig;
private AtomicReference<SSLContextAndOptions> defaultSSLContextAndOptions = new AtomicReference<>(null);
private final String sslProtocolProperty = getConfigPrefix() + "protocol";
private final String sslEnabledProtocolsProperty = getConfigPrefix() + "enabledProtocols";
private final String cipherSuitesProperty = getConfigPrefix() + "ciphersuites";
private final String sslKeystoreLocationProperty = getConfigPrefix() + "keyStore.location";
private final String sslKeystorePasswdProperty = getConfigPrefix() + "keyStore.password";
private final String sslKeystorePasswdPathProperty = getConfigPrefix() + "keyStore.passwordPath";
private final String sslKeystoreTypeProperty = getConfigPrefix() + "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 sslContextSupplierClassProperty = getConfigPrefix() + "context.supplier.class";
private final String sslHostnameVerificationEnabledProperty = getConfigPrefix() + "hostnameVerification";
private final String sslCrlEnabledProperty = getConfigPrefix() + "crl";
private final String sslOcspEnabledProperty = getConfigPrefix() + "ocsp";
private final String sslClientAuthProperty = getConfigPrefix() + "clientAuth";
private final String sslHandshakeDetectionTimeoutMillisProperty = getConfigPrefix() + "handshakeDetectionTimeoutMillis";

private final AtomicReference<SSLContextAndOptions> defaultSSLContextAndOptions = new AtomicReference<>(null);

private FileChangeWatcher keyStoreFileWatcher;
private FileChangeWatcher trustStoreFileWatcher;

public X509Util() {
this(null);
}

public X509Util(ZKConfig zkConfig) {
this.zkConfig = zkConfig;
keyStoreFileWatcher = trustStoreFileWatcher = null;
}

Expand Down Expand Up @@ -260,6 +255,22 @@ public String getSslHandshakeDetectionTimeoutMillisProperty() {
return sslHandshakeDetectionTimeoutMillisProperty;
}

public String getFipsModeProperty() {
return FIPS_MODE_PROPERTY;
}

public boolean getFipsMode(ZKConfig config) {
return config.getBoolean(FIPS_MODE_PROPERTY, true);
}

public boolean isServerHostnameVerificationEnabled(ZKConfig config) {
return config.getBoolean(this.getSslHostnameVerificationEnabledProperty(), true);
}

public boolean isClientHostnameVerificationEnabled(ZKConfig config) {
return isServerHostnameVerificationEnabled(config) && shouldVerifyClientHostname();
}

public SSLContext getDefaultSSLContext() throws X509Exception.SSLContextException {
return getDefaultSSLContextAndOptions().getSSLContext();
}
Expand Down Expand Up @@ -295,7 +306,7 @@ private SSLContextAndOptions createSSLContextAndOptions() throws SSLContextExcep
* configuration from system property. Reading property from
* configuration will be same reading from system property
*/
return createSSLContextAndOptions(zkConfig == null ? new ZKConfig() : zkConfig);
return createSSLContextAndOptions(new ZKConfig());
}

/**
Expand Down Expand Up @@ -375,14 +386,18 @@ public SSLContextAndOptions createSSLContextAndOptionsFromConfig(ZKConfig config

boolean sslCrlEnabled = config.getBoolean(this.sslCrlEnabledProperty);
boolean sslOcspEnabled = config.getBoolean(this.sslOcspEnabledProperty);
boolean sslServerHostnameVerificationEnabled = config.getBoolean(this.getSslHostnameVerificationEnabledProperty(), true);
boolean sslClientHostnameVerificationEnabled = sslServerHostnameVerificationEnabled && shouldVerifyClientHostname();
boolean sslServerHostnameVerificationEnabled = isServerHostnameVerificationEnabled(config);
boolean sslClientHostnameVerificationEnabled = isClientHostnameVerificationEnabled(config);
boolean fipsMode = getFipsMode(config);

if (trustStoreLocationProp.isEmpty()) {
LOG.warn("{} not specified", getSslTruststoreLocationProperty());
} else {
try {
trustManagers = new TrustManager[]{createTrustManager(trustStoreLocationProp, trustStorePasswordProp, trustStoreTypeProp, sslCrlEnabled, sslOcspEnabled, sslServerHostnameVerificationEnabled, sslClientHostnameVerificationEnabled)};
trustManagers = new TrustManager[]{
createTrustManager(trustStoreLocationProp, trustStorePasswordProp, trustStoreTypeProp, sslCrlEnabled,
sslOcspEnabled, sslServerHostnameVerificationEnabled, sslClientHostnameVerificationEnabled,
fipsMode)};
} catch (TrustManagerException trustManagerException) {
throw new SSLContextException("Failed to create TrustManager", trustManagerException);
} catch (IllegalArgumentException e) {
Expand Down Expand Up @@ -487,16 +502,17 @@ public static X509KeyManager createKeyManager(
/**
* Creates a trust manager by loading the trust store from the given file
* of the given type, optionally decrypting it using the given password.
* @param trustStoreLocation the location of the trust store file.
* @param trustStorePassword optional password to decrypt the trust store
* (only applies to JKS trust stores). If empty,
* assumes the trust store is not encrypted.
* @param trustStoreTypeProp must be JKS, PEM, PKCS12, BCFKS or null. If
* null, attempts to autodetect the trust store
* type from the file extension (e.g. .jks / .pem).
* @param crlEnabled enable CRL (certificate revocation list) checks.
* @param ocspEnabled enable OCSP (online certificate status protocol)
* checks.
*
* @param trustStoreLocation the location of the trust store file.
* @param trustStorePassword optional password to decrypt the trust store
* (only applies to JKS trust stores). If empty,
* assumes the trust store is not encrypted.
* @param trustStoreTypeProp must be JKS, PEM, PKCS12, BCFKS or null. If
* null, attempts to autodetect the trust store
* type from the file extension (e.g. .jks / .pem).
* @param crlEnabled enable CRL (certificate revocation list) checks.
* @param ocspEnabled enable OCSP (online certificate status protocol)
* checks.
* @param serverHostnameVerificationEnabled if true, verify hostnames of
* remote servers that client
* sockets created by this
Expand All @@ -516,7 +532,8 @@ public static X509TrustManager createTrustManager(
boolean crlEnabled,
boolean ocspEnabled,
final boolean serverHostnameVerificationEnabled,
final boolean clientHostnameVerificationEnabled) throws TrustManagerException {
final boolean clientHostnameVerificationEnabled,
final boolean fipsMode) throws TrustManagerException {
if (trustStorePassword == null) {
trustStorePassword = "";
}
Expand All @@ -540,7 +557,17 @@ public static X509TrustManager createTrustManager(

for (final TrustManager tm : tmf.getTrustManagers()) {
if (tm instanceof X509ExtendedTrustManager) {
return new ZKTrustManager((X509ExtendedTrustManager) tm, serverHostnameVerificationEnabled, clientHostnameVerificationEnabled);
if (fipsMode) {
if (LOG.isDebugEnabled()) {
LOG.debug("FIPS mode is ON: selecting standard x509 trust manager {}", tm);
}
return (X509TrustManager) tm;
}
if (LOG.isDebugEnabled()) {
LOG.debug("FIPS mode is OFF: creating ZKTrustManager");
}
return new ZKTrustManager((X509ExtendedTrustManager) tm, serverHostnameVerificationEnabled,
clientHostnameVerificationEnabled);
}
}
throw new TrustManagerException("Couldn't find X509TrustManager");
Expand Down Expand Up @@ -606,7 +633,7 @@ private FileChangeWatcher newFileChangeWatcher(String fileLocation) throws IOExc
*/
public void enableCertFileReloading() throws IOException {
LOG.info("enabling cert file reloading");
ZKConfig config = zkConfig == null ? new ZKConfig() : zkConfig;
ZKConfig config = new ZKConfig();
FileChangeWatcher newKeyStoreFileWatcher = newFileChangeWatcher(config.getProperty(sslKeystoreLocationProperty));
if (newKeyStoreFileWatcher != null) {
// stop old watcher if there is one
Expand All @@ -633,6 +660,7 @@ public void enableCertFileReloading() throws IOException {
*/
@Override
public void close() {
defaultSSLContextAndOptions.set(null);
if (keyStoreFileWatcher != null) {
keyStoreFileWatcher.stop();
keyStoreFileWatcher = null;
Expand Down
2 changes: 2 additions & 0 deletions zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public ZKConfig(String configPath) throws ConfigException {
public ZKConfig(File configFile) throws ConfigException {
this();
addConfiguration(configFile);
LOG.info("ZK Config {}", this.properties);
}

private void init() {
Expand Down Expand Up @@ -130,6 +131,7 @@ private void putSSLProperties(X509Util x509Util) {
properties.put(x509Util.getSslOcspEnabledProperty(), System.getProperty(x509Util.getSslOcspEnabledProperty()));
properties.put(x509Util.getSslClientAuthProperty(), System.getProperty(x509Util.getSslClientAuthProperty()));
properties.put(x509Util.getSslHandshakeDetectionTimeoutMillisProperty(), System.getProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty()));
properties.put(x509Util.getFipsModeProperty(), System.getProperty(x509Util.getFipsModeProperty()));
}

/**
Expand Down
Loading

0 comments on commit 1d99df6

Please sign in to comment.