diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md index 4e7d81888cc..c90de4f305d 100644 --- a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md +++ b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md @@ -1759,6 +1759,16 @@ and [SASL authentication for ZooKeeper](https://cwiki.apache.org/confluence/disp This option requires the corresponding *hostnameVerification* option to be `true`, or it will be ignored. Default: true for quorum, false for clients +* *ssl.allowReverseDnsLookup* and *ssl.quorum.allowReverseDnsLookup* : + (Java system properties: **zookeeper.ssl.allowReverseDnsLookup** and **zookeeper.ssl.quorum.allowReverseDnsLookup**) + **New in 3.9.5:** + Allow reverse DNS lookup in both server- and client hostname verifications if the hostname verification is enabled in + `ZKTrustManager`. Supported in both quorum and client TLS protocols. Not supported in FIPS mode. Reverse DNS lookups are + expensive and unnecessary in most cases. Make sure that certificates are created with all required Subject Alternative + Names (SAN) for successful identity verification. It's recommended to add SAN:IP entries for identity verification + of client certificates. + Default: false (for Client connections), true (for Quorum connections) + * *ssl.crl* and *ssl.quorum.crl* : (Java system properties: **zookeeper.ssl.crl** and **zookeeper.ssl.quorum.crl**) **New in 3.5.5:** 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 178994545e7..f1d1b164bcf 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 @@ -52,6 +52,11 @@ protected boolean shouldVerifyClientHostname() { return false; } + @Override + protected boolean shouldAllowReverseDnsLookup() { + return false; + } + public String getSslAuthProviderProperty() { return sslAuthProviderProperty; } @@ -202,6 +207,7 @@ private TrustManager getTrustManager(ZKConfig config) throws X509Exception.Trust boolean sslOcspEnabled = config.getBoolean(getSslOcspEnabledProperty()); boolean sslServerHostnameVerificationEnabled = isServerHostnameVerificationEnabled(config); boolean sslClientHostnameVerificationEnabled = isClientHostnameVerificationEnabled(config); + boolean allowReverseDnsLookup = allowReverseDnsLookup(config); if (trustStoreLocation.isEmpty()) { LOG.warn("{} not specified", getSslTruststoreLocationProperty()); @@ -209,7 +215,8 @@ private TrustManager getTrustManager(ZKConfig config) throws X509Exception.Trust } else { return createTrustManager(trustStoreLocation, trustStorePassword, trustStoreType, sslCrlEnabled, sslOcspEnabled, sslServerHostnameVerificationEnabled, - sslClientHostnameVerificationEnabled, getFipsMode(config)); + sslClientHostnameVerificationEnabled, allowReverseDnsLookup, + getFipsMode(config)); } } } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/QuorumX509Util.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/QuorumX509Util.java index af3ee1bdeab..b0d17309fed 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/QuorumX509Util.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/QuorumX509Util.java @@ -33,4 +33,8 @@ protected boolean shouldVerifyClientHostname() { return true; } + @Override + protected boolean shouldAllowReverseDnsLookup() { + return true; + } } 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 17818207e9e..4cc54d605a1 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 @@ -198,6 +198,7 @@ public io.netty.handler.ssl.ClientAuth toNettyClientAuth() { private final String sslContextSupplierClassProperty = getConfigPrefix() + "context.supplier.class"; private final String sslHostnameVerificationEnabledProperty = getConfigPrefix() + "hostnameVerification"; private final String sslClientHostnameVerificationEnabledProperty = getConfigPrefix() + "clientHostnameVerification"; + private final String sslAllowReverseDnsLookupProperty = getConfigPrefix() + "allowReverseDnsLookup"; private final String sslCrlEnabledProperty = getConfigPrefix() + "crl"; private final String sslOcspEnabledProperty = getConfigPrefix() + "ocsp"; private final String sslClientAuthProperty = getConfigPrefix() + "clientAuth"; @@ -216,6 +217,8 @@ public X509Util() { protected abstract boolean shouldVerifyClientHostname(); + protected abstract boolean shouldAllowReverseDnsLookup(); + public String getSslProtocolProperty() { return sslProtocolProperty; } @@ -276,6 +279,10 @@ public String getSslClientHostnameVerificationEnabledProperty() { return sslClientHostnameVerificationEnabledProperty; } + public String getSslAllowReverseDnsLookupProperty() { + return sslAllowReverseDnsLookupProperty; + } + public String getSslCrlEnabledProperty() { return sslCrlEnabledProperty; } @@ -315,6 +322,10 @@ public boolean isClientHostnameVerificationEnabled(ZKConfig config) { && config.getBoolean(this.getSslClientHostnameVerificationEnabledProperty(), shouldVerifyClientHostname()); } + public boolean allowReverseDnsLookup(ZKConfig config) { + return config.getBoolean(this.getSslAllowReverseDnsLookupProperty(), shouldAllowReverseDnsLookup()); + } + public SSLContext getDefaultSSLContext() throws X509Exception.SSLContextException { return getDefaultSSLContextAndOptions().getSSLContext(); } @@ -432,6 +443,7 @@ public SSLContextAndOptions createSSLContextAndOptionsFromConfig(ZKConfig config boolean sslOcspEnabled = config.getBoolean(this.sslOcspEnabledProperty); boolean sslServerHostnameVerificationEnabled = isServerHostnameVerificationEnabled(config); boolean sslClientHostnameVerificationEnabled = isClientHostnameVerificationEnabled(config); + boolean allowReverseDnsLookup = allowReverseDnsLookup(config); boolean fipsMode = getFipsMode(config); if (trustStoreLocationProp.isEmpty()) { @@ -441,7 +453,7 @@ public SSLContextAndOptions createSSLContextAndOptionsFromConfig(ZKConfig config trustManagers = new TrustManager[]{ createTrustManager(trustStoreLocationProp, trustStorePasswordProp, trustStoreTypeProp, sslCrlEnabled, sslOcspEnabled, sslServerHostnameVerificationEnabled, sslClientHostnameVerificationEnabled, - fipsMode)}; + allowReverseDnsLookup, fipsMode)}; } catch (TrustManagerException trustManagerException) { throw new SSLContextException("Failed to create TrustManager", trustManagerException); } catch (IllegalArgumentException e) { @@ -577,6 +589,7 @@ public static X509TrustManager createTrustManager( boolean ocspEnabled, final boolean serverHostnameVerificationEnabled, final boolean clientHostnameVerificationEnabled, + final boolean allowReverseDnsLookup, final boolean fipsMode) throws TrustManagerException { if (trustStorePassword == null) { trustStorePassword = ""; @@ -611,7 +624,7 @@ public static X509TrustManager createTrustManager( LOG.debug("FIPS mode is OFF: creating ZKTrustManager"); } return new ZKTrustManager((X509ExtendedTrustManager) tm, serverHostnameVerificationEnabled, - clientHostnameVerificationEnabled); + clientHostnameVerificationEnabled, allowReverseDnsLookup); } } throw new TrustManagerException("Couldn't find X509TrustManager"); 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 47fd943860c..f6221eeb7f4 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 @@ -151,6 +151,7 @@ private void putSSLProperties(X509Util x509Util) { properties.put(x509Util.getSslContextSupplierClassProperty(), System.getProperty(x509Util.getSslContextSupplierClassProperty())); properties.put(x509Util.getSslClientHostnameVerificationEnabledProperty(), System.getProperty(x509Util.getSslClientHostnameVerificationEnabledProperty())); properties.put(x509Util.getSslHostnameVerificationEnabledProperty(), System.getProperty(x509Util.getSslHostnameVerificationEnabledProperty())); + properties.put(x509Util.getSslAllowReverseDnsLookupProperty(), System.getProperty(x509Util.getSslAllowReverseDnsLookupProperty())); properties.put(x509Util.getSslCrlEnabledProperty(), System.getProperty(x509Util.getSslCrlEnabledProperty())); properties.put(x509Util.getSslOcspEnabledProperty(), System.getProperty(x509Util.getSslOcspEnabledProperty())); properties.put(x509Util.getSslClientAuthProperty(), System.getProperty(x509Util.getSslClientAuthProperty())); diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKTrustManager.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKTrustManager.java index cbadd1e0af9..e2af9f6f077 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKTrustManager.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKTrustManager.java @@ -42,6 +42,7 @@ public class ZKTrustManager extends X509ExtendedTrustManager { private final X509ExtendedTrustManager x509ExtendedTrustManager; private final boolean serverHostnameVerificationEnabled; private final boolean clientHostnameVerificationEnabled; + private final boolean allowReverseDnsLookup; private final ZKHostnameVerifier hostnameVerifier; @@ -57,22 +58,26 @@ public class ZKTrustManager extends X509ExtendedTrustManager { ZKTrustManager( X509ExtendedTrustManager x509ExtendedTrustManager, boolean serverHostnameVerificationEnabled, - boolean clientHostnameVerificationEnabled) { + boolean clientHostnameVerificationEnabled, + boolean allowReverseDnsLookup) { this(x509ExtendedTrustManager, serverHostnameVerificationEnabled, clientHostnameVerificationEnabled, - new ZKHostnameVerifier()); + new ZKHostnameVerifier(), + allowReverseDnsLookup); } ZKTrustManager( X509ExtendedTrustManager x509ExtendedTrustManager, boolean serverHostnameVerificationEnabled, boolean clientHostnameVerificationEnabled, - ZKHostnameVerifier hostnameVerifier) { + ZKHostnameVerifier hostnameVerifier, + boolean allowReverseDnsLookup) { this.x509ExtendedTrustManager = x509ExtendedTrustManager; this.serverHostnameVerificationEnabled = serverHostnameVerificationEnabled; this.clientHostnameVerificationEnabled = clientHostnameVerificationEnabled; this.hostnameVerifier = hostnameVerifier; + this.allowReverseDnsLookup = allowReverseDnsLookup; } @Override @@ -166,31 +171,46 @@ public void checkServerTrusted(X509Certificate[] chain, String authType) throws * @param certificate Peer's certificate * @throws CertificateException Thrown if the provided certificate doesn't match the peer hostname. */ - private void performHostVerification( - InetAddress inetAddress, - X509Certificate certificate - ) throws CertificateException { + private void performHostVerification(InetAddress inetAddress, X509Certificate certificate) + throws CertificateException { String hostAddress = ""; String hostName = ""; try { hostAddress = inetAddress.getHostAddress(); - if (LOG.isDebugEnabled()) { - LOG.debug("Trying to verify host address first: {}", hostAddress); - } hostnameVerifier.verify(hostAddress, certificate); } catch (SSLException addressVerificationException) { + // If we fail with hostAddress, we should try the hostname. + // The inetAddress may have been created with a hostname, in which case getHostName() will + // return quickly below. If not, a reverse lookup will happen, which can be expensive. + // We provide the option to skip the reverse lookup if preferring to fail fast. + + // Handle logging here to aid debugging. The easiest way to check for an existing + // hostname is through toString, see javadoc. + String inetAddressString = inetAddress.toString(); + if (!inetAddressString.startsWith("/")) { + LOG.debug( + "Failed to verify host address: {}, but inetAddress {} has a hostname, trying that", + hostAddress, inetAddressString, addressVerificationException); + } else if (allowReverseDnsLookup) { + LOG.debug( + "Failed to verify host address: {}, attempting to verify host name with reverse dns", + hostAddress, addressVerificationException); + } else { + LOG.debug("Failed to verify host address: {}, but reverse dns lookup is disabled", + hostAddress, addressVerificationException); + throw new CertificateException( + "Failed to verify host address, and reverse lookup is disabled", + addressVerificationException); + } + try { hostName = inetAddress.getHostName(); - if (LOG.isDebugEnabled()) { - LOG.debug( - "Failed to verify host address: {}, trying to verify host name: {}", - hostAddress, hostName); - } hostnameVerifier.verify(hostName, certificate); } catch (SSLException hostnameVerificationException) { LOG.error("Failed to verify host address: {}", hostAddress, addressVerificationException); LOG.error("Failed to verify hostname: {}", hostName, hostnameVerificationException); - throw new CertificateException("Failed to verify both host address and host name", hostnameVerificationException); + throw new CertificateException("Failed to verify both host address and host name", + hostnameVerificationException); } } } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java index 4ea925320f6..1c8edcb8658 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java @@ -88,6 +88,7 @@ public X509AuthenticationProvider() throws X509Exception { boolean crlEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslCrlEnabledProperty())); boolean ocspEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslOcspEnabledProperty())); boolean hostnameVerificationEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslHostnameVerificationEnabledProperty())); + boolean allowReverseDnsLookup = Boolean.parseBoolean(config.getProperty(x509Util.getSslAllowReverseDnsLookupProperty())); X509KeyManager km = null; X509TrustManager tm = null; @@ -120,6 +121,7 @@ public X509AuthenticationProvider() throws X509Exception { ocspEnabled, hostnameVerificationEnabled, false, + allowReverseDnsLookup, fipsMode); } catch (TrustManagerException e) { LOG.error("Failed to create trust manager", e); 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 827d80a9aec..fe9eea34810 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 @@ -381,6 +381,7 @@ public void testLoadPEMTrustStore( false, true, true, + false, false); } @@ -402,6 +403,7 @@ public void testLoadPEMTrustStoreNullPassword( false, true, true, + false, false); } @@ -421,6 +423,7 @@ public void testLoadPEMTrustStoreAutodetectStoreFileType( false, true, true, + false, false); } @@ -496,6 +499,7 @@ public void testLoadJKSTrustStore( true, true, true, + false, false); } @@ -517,6 +521,7 @@ public void testLoadJKSTrustStoreNullPassword( false, true, true, + false, false); } @@ -535,6 +540,7 @@ public void testLoadJKSTrustStoreAutodetectStoreFileType( true, true, true, + false, false); } @@ -554,6 +560,7 @@ public void testLoadJKSTrustStoreWithWrongPassword( true, true, true, + false, false); }); } @@ -629,6 +636,7 @@ public void testLoadPKCS12TrustStore( true, true, true, + false, false); } @@ -650,6 +658,7 @@ public void testLoadPKCS12TrustStoreNullPassword( false, true, true, + false, false); } @@ -668,6 +677,7 @@ public void testLoadPKCS12TrustStoreAutodetectStoreFileType( true, true, true, + false, false); } @@ -687,6 +697,7 @@ public void testLoadPKCS12TrustStoreWithWrongPassword( true, true, true, + false, false); }); } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/ZKTrustManagerTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/ZKTrustManagerTest.java index 7b4e8783ee3..a2753b29bc6 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/common/ZKTrustManagerTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/ZKTrustManagerTest.java @@ -19,6 +19,7 @@ package org.apache.zookeeper.common; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -27,13 +28,16 @@ import java.math.BigInteger; import java.net.InetAddress; import java.net.Socket; +import java.net.UnknownHostException; import java.security.KeyPair; import java.security.KeyPairGenerator; import java.security.Security; +import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; +import java.util.Collections; import java.util.Date; import java.util.LinkedHashMap; import java.util.List; @@ -61,10 +65,10 @@ import org.burningwave.tools.net.HostResolutionRequestInterceptor; import org.burningwave.tools.net.MappedHostResolver; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -113,16 +117,14 @@ public static void removeBouncyCastleProvider() throws Exception { @BeforeEach public void setup() throws Exception { mockX509ExtendedTrustManager = mock(X509ExtendedTrustManager.class); + mockSocket = createSocketWithHostname(); + } - InetAddress mockInetAddress = InetAddress.getByName(HOSTNAME); - - mockSocket = mock(Socket.class); - when(mockSocket.getInetAddress()).thenAnswer(new Answer() { - @Override - public Object answer(InvocationOnMock invocationOnMock) throws Throwable { - return mockInetAddress; - } - }); + @AfterEach + public void tearDown() throws Exception { + if (mockSocket != null) { + mockSocket.close(); + } } private X509Certificate[] createSelfSignedCertifcateChain(String ipAddress, String hostname) throws Exception { @@ -161,7 +163,7 @@ private X509Certificate[] createSelfSignedCertifcateChain(String ipAddress, Stri public void testServerHostnameVerificationWithHostnameVerificationDisabled() throws Exception { VerifiableHostnameVerifier hostnameVerifier = new VerifiableHostnameVerifier(); ZKTrustManager zkTrustManager = new ZKTrustManager(mockX509ExtendedTrustManager, false, false, - hostnameVerifier); + hostnameVerifier, false); X509Certificate[] certificateChain = createSelfSignedCertifcateChain(IP_ADDRESS, HOSTNAME); zkTrustManager.checkServerTrusted(certificateChain, null, mockSocket); @@ -175,7 +177,7 @@ public void testServerHostnameVerificationWithHostnameVerificationDisabled() thr public void testServerHostnameVerificationWithHostnameVerificationDisabledAndClientHostnameVerificationEnabled() throws Exception { VerifiableHostnameVerifier hostnameVerifier = new VerifiableHostnameVerifier(); ZKTrustManager zkTrustManager = new ZKTrustManager(mockX509ExtendedTrustManager, false, true, - hostnameVerifier); + hostnameVerifier, false); X509Certificate[] certificateChain = createSelfSignedCertifcateChain(IP_ADDRESS, HOSTNAME); zkTrustManager.checkServerTrusted(certificateChain, null, mockSocket); @@ -190,7 +192,7 @@ public void testServerHostnameVerificationWithHostnameVerificationDisabledAndCli public void testServerHostnameVerificationWithIPAddress() throws Exception { VerifiableHostnameVerifier hostnameVerifier = new VerifiableHostnameVerifier(); ZKTrustManager zkTrustManager = new ZKTrustManager(mockX509ExtendedTrustManager, true, false, - hostnameVerifier); + hostnameVerifier, false); X509Certificate[] certificateChain = createSelfSignedCertifcateChain(IP_ADDRESS, null); zkTrustManager.checkServerTrusted(certificateChain, null, mockSocket); @@ -205,7 +207,7 @@ public void testServerHostnameVerificationWithIPAddress() throws Exception { public void testServerHostnameVerificationWithHostname() throws Exception { VerifiableHostnameVerifier hostnameVerifier = new VerifiableHostnameVerifier(); ZKTrustManager zkTrustManager = new ZKTrustManager(mockX509ExtendedTrustManager, true, false, - hostnameVerifier); + hostnameVerifier, false); X509Certificate[] certificateChain = createSelfSignedCertifcateChain(null, HOSTNAME); zkTrustManager.checkServerTrusted(certificateChain, null, mockSocket); @@ -220,7 +222,7 @@ public void testServerHostnameVerificationWithHostname() throws Exception { public void testClientHostnameVerificationWithHostnameVerificationDisabled() throws Exception { VerifiableHostnameVerifier hostnameVerifier = new VerifiableHostnameVerifier(); ZKTrustManager zkTrustManager = new ZKTrustManager(mockX509ExtendedTrustManager, false, true, - hostnameVerifier); + hostnameVerifier, false); X509Certificate[] certificateChain = createSelfSignedCertifcateChain(null, HOSTNAME); zkTrustManager.checkClientTrusted(certificateChain, null, mockSocket); @@ -235,7 +237,7 @@ public void testClientHostnameVerificationWithHostnameVerificationDisabled() thr public void testClientHostnameVerificationWithClientHostnameVerificationDisabled() throws Exception { VerifiableHostnameVerifier hostnameVerifier = new VerifiableHostnameVerifier(); ZKTrustManager zkTrustManager = new ZKTrustManager(mockX509ExtendedTrustManager, true, - false, hostnameVerifier); + false, hostnameVerifier, false); X509Certificate[] certificateChain = createSelfSignedCertifcateChain(null, HOSTNAME); zkTrustManager.checkClientTrusted(certificateChain, null, mockSocket); @@ -250,7 +252,7 @@ public void testClientHostnameVerificationWithClientHostnameVerificationDisabled public void testClientHostnameVerificationWithIPAddress() throws Exception { VerifiableHostnameVerifier hostnameVerifier = new VerifiableHostnameVerifier(); ZKTrustManager zkTrustManager = new ZKTrustManager(mockX509ExtendedTrustManager, true, true, - hostnameVerifier); + hostnameVerifier, false); X509Certificate[] certificateChain = createSelfSignedCertifcateChain(IP_ADDRESS, null); zkTrustManager.checkClientTrusted(certificateChain, null, mockSocket); @@ -265,7 +267,7 @@ public void testClientHostnameVerificationWithIPAddress() throws Exception { public void testClientHostnameVerificationWithHostname() throws Exception { VerifiableHostnameVerifier hostnameVerifier = new VerifiableHostnameVerifier(); ZKTrustManager zkTrustManager = new ZKTrustManager(mockX509ExtendedTrustManager, true, true, - hostnameVerifier); + hostnameVerifier, false); X509Certificate[] certificateChain = createSelfSignedCertifcateChain(null, HOSTNAME); zkTrustManager.checkClientTrusted(certificateChain, null, mockSocket); @@ -276,6 +278,64 @@ public void testClientHostnameVerificationWithHostname() throws Exception { verify(mockX509ExtendedTrustManager, times(1)).checkClientTrusted(certificateChain, null, mockSocket); } + @Test + public void testClientHostnameVerificationWithIpAddress_CertHostnameSan_NoReverseLookup_Fail() throws Exception { + VerifiableHostnameVerifier hostnameVerifier = new VerifiableHostnameVerifier(); + ZKTrustManager zkTrustManager = new ZKTrustManager(mockX509ExtendedTrustManager, true, true, + hostnameVerifier, false); + + X509Certificate[] certificateChain = createSelfSignedCertifcateChain(null, HOSTNAME); + try (Socket s = createSocketWithIpAddress()) { + assertThrows(CertificateException.class, () -> zkTrustManager.checkClientTrusted(certificateChain, null, s)); + verify(s, times(1)).getInetAddress(); + assertEquals(Collections.singletonList(IP_ADDRESS), hostnameVerifier.hosts); + verify(mockX509ExtendedTrustManager, times(1)).checkClientTrusted(certificateChain, null, s); + } + } + + @Test + public void testClientHostnameVerificationWithIpAddress_CertHostnameSan_WithReverseLookup() throws Exception { + VerifiableHostnameVerifier hostnameVerifier = new VerifiableHostnameVerifier(); + ZKTrustManager zkTrustManager = new ZKTrustManager(mockX509ExtendedTrustManager, true, true, + hostnameVerifier, true); + + X509Certificate[] certificateChain = createSelfSignedCertifcateChain(null, HOSTNAME); + try (Socket s = createSocketWithIpAddress()) { + zkTrustManager.checkClientTrusted(certificateChain, null, s); + verify(s, times(1)).getInetAddress(); + assertEquals(Arrays.asList(IP_ADDRESS, HOSTNAME), hostnameVerifier.hosts); + verify(mockX509ExtendedTrustManager, times(1)).checkClientTrusted(certificateChain, null, s); + } + } + + @Test + public void testClientHostnameVerificationWithIpAddress_CertIpSan() throws Exception { + VerifiableHostnameVerifier hostnameVerifier = new VerifiableHostnameVerifier(); + ZKTrustManager zkTrustManager = new ZKTrustManager(mockX509ExtendedTrustManager, true, true, + hostnameVerifier, false); + + X509Certificate[] certificateChain = createSelfSignedCertifcateChain(IP_ADDRESS, null); + try (Socket s = createSocketWithIpAddress()) { + zkTrustManager.checkClientTrusted(certificateChain, null, s); + verify(s, times(1)).getInetAddress(); + assertEquals(Collections.singletonList(IP_ADDRESS), hostnameVerifier.hosts); + verify(mockX509ExtendedTrustManager, times(1)).checkClientTrusted(certificateChain, null, s); + } + } + + private Socket createSocketWithHostname() throws UnknownHostException { + InetAddress mockInetAddress = InetAddress.getByName(HOSTNAME); + Socket s = mock(Socket.class); + when(s.getInetAddress()).thenAnswer((Answer) invocationOnMock -> mockInetAddress); + return s; + } + + private Socket createSocketWithIpAddress() throws UnknownHostException { + InetAddress mockInetAddress = InetAddress.getByName(IP_ADDRESS); + Socket s = mock(Socket.class); + when(s.getInetAddress()).thenAnswer((Answer) invocationOnMock -> mockInetAddress); + return s; + } static class VerifiableHostnameVerifier extends ZKHostnameVerifier { diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java index 5a3c5541302..3177024f525 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java @@ -142,6 +142,7 @@ public class QuorumSSLTest extends QuorumPeerTestBase { private static final char[] PASSWORD = "testpass".toCharArray(); private static final String HOSTNAME = "localhost"; + private static final String IPADDRESS = "127.0.0.1"; private QuorumX509Util quorumX509Util; @@ -487,6 +488,7 @@ private void clearSSLSystemProperties() { System.clearProperty(quorumX509Util.getSslTruststorePasswdProperty()); System.clearProperty(quorumX509Util.getSslTruststorePasswdPathProperty()); System.clearProperty(quorumX509Util.getSslHostnameVerificationEnabledProperty()); + System.clearProperty(quorumX509Util.getSslAllowReverseDnsLookupProperty()); System.clearProperty(quorumX509Util.getSslOcspEnabledProperty()); System.clearProperty(quorumX509Util.getSslCrlEnabledProperty()); System.clearProperty(quorumX509Util.getCipherSuitesProperty()); @@ -700,6 +702,8 @@ public void testHostnameVerificationForInvalidMultiAddressServerConfig(boolean f @Timeout(value = 5, unit = TimeUnit.MINUTES) public void testHostnameVerificationWithInvalidIpAddressAndValidHostname(boolean fipsEnabled) throws Exception { System.setProperty(quorumX509Util.getFipsModeProperty(), Boolean.toString(fipsEnabled)); + // We need reverse DNS lookup to get this working, because quorum is connecting via ip addresses + System.setProperty(quorumX509Util.getSslAllowReverseDnsLookupProperty(), Boolean.toString(true)); String badhostnameKeystorePath = tmpDir + "/badhost.jks"; X509Certificate badHostCert = buildEndEntityCert( @@ -805,7 +809,7 @@ public void testCertificateRevocationList(boolean fipsEnabled) throws Exception rootCertificate, rootKeyPair.getPrivate(), HOSTNAME, - null, + IPADDRESS, crlPath, null); writeKeystore(revokedInCRLCert, defaultKeyPair, revokedInCRLKeystorePath); @@ -835,7 +839,7 @@ public void testCertificateRevocationList(boolean fipsEnabled) throws Exception rootCertificate, rootKeyPair.getPrivate(), HOSTNAME, - null, + IPADDRESS, crlPath, null); writeKeystore(validCertificate, defaultKeyPair, validKeystorePath); @@ -874,7 +878,7 @@ public void testOCSP(boolean fipsEnabled) throws Exception { rootCertificate, rootKeyPair.getPrivate(), HOSTNAME, - null, + IPADDRESS, null, ocspPort); writeKeystore(revokedInOCSPCert, defaultKeyPair, revokedInOCSPKeystorePath); @@ -908,7 +912,7 @@ public void testOCSP(boolean fipsEnabled) throws Exception { rootCertificate, rootKeyPair.getPrivate(), HOSTNAME, - null, + IPADDRESS, null, ocspPort); writeKeystore(validCertificate, defaultKeyPair, validKeystorePath); diff --git a/zookeeper-server/src/test/resources/data/ssl/README.md b/zookeeper-server/src/test/resources/data/ssl/README.md index b8823d8a3de..26d4d0b6fff 100644 --- a/zookeeper-server/src/test/resources/data/ssl/README.md +++ b/zookeeper-server/src/test/resources/data/ssl/README.md @@ -1,6 +1,21 @@ SSL test data =================== +Create keystore with certificate +``` +keytool -genkeypair -alias test -keyalg RSA -keysize 2048 -dname "CN=localhost,OU=ZooKeeper,O=Apache,L=Unknown,ST=Unknown,C=Unknown" -keypass testpass -keystore keystore.jks -storepass testpass -ext SAN=DNS:localhost,IP:127.0.0.1 +``` + +Export certificate to file +``` +keytool -exportcert -alias test -keystore keystore.jks -file test.cer -rfc +``` + +Create truststore +``` +keytool -importcert -alias test -file test.cer -keystore truststore.jks -storepass testpass +``` + testKeyStore.jks --- Testing keystore, password is "testpass". diff --git a/zookeeper-server/src/test/resources/data/ssl/testKeyStore.jks b/zookeeper-server/src/test/resources/data/ssl/testKeyStore.jks index 40a7d0b7eae..60e5d52b656 100644 Binary files a/zookeeper-server/src/test/resources/data/ssl/testKeyStore.jks and b/zookeeper-server/src/test/resources/data/ssl/testKeyStore.jks differ diff --git a/zookeeper-server/src/test/resources/data/ssl/testTrustStore.jks b/zookeeper-server/src/test/resources/data/ssl/testTrustStore.jks index 33f09c11dfa..bced986e430 100644 Binary files a/zookeeper-server/src/test/resources/data/ssl/testTrustStore.jks and b/zookeeper-server/src/test/resources/data/ssl/testTrustStore.jks differ