Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZOOKEEPER-4393 Problem to connect to zookeeper in FIPS mode (#2008) #54

Merged
merged 1 commit into from
Aug 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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
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
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