diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md index 916f7531b8e..c0ea9c3a792 100644 --- a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md +++ b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md @@ -1714,6 +1714,16 @@ and [SASL authentication for ZooKeeper](https://cwiki.apache.org/confluence/disp Disabling it only recommended for testing purposes. Default: true +* *ssl.allowReverseDnsLookup* and *ssl.quorum.allowReverseDnsLookup* : + (Java system properties: **zookeeper.ssl.allowReverseDnsLookup** and **zookeeper.ssl.quorum.allowReverseDnsLookup**) + **New in 3.8.6:** + 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 34be0beceb3..1737ddb6e52 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 @@ -32,6 +32,11 @@ protected boolean shouldVerifyClientHostname() { return false; } + @Override + protected boolean shouldAllowReverseDnsLookup() { + return false; + } + public String getSslAuthProviderProperty() { return sslAuthProviderProperty; } 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 d4cf19f5306..9fc96077239 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 @@ -30,4 +30,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 b53800cda63..2f8ec88a72a 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 @@ -160,6 +160,7 @@ public io.netty.handler.ssl.ClientAuth toNettyClientAuth() { private final String sslTruststoreTypeProperty = getConfigPrefix() + "trustStore.type"; private final String sslContextSupplierClassProperty = getConfigPrefix() + "context.supplier.class"; private final String sslHostnameVerificationEnabledProperty = getConfigPrefix() + "hostnameVerification"; + private final String sslAllowReverseDnsLookupProperty = getConfigPrefix() + "allowReverseDnsLookup"; private final String sslCrlEnabledProperty = getConfigPrefix() + "crl"; private final String sslOcspEnabledProperty = getConfigPrefix() + "ocsp"; private final String sslClientAuthProperty = getConfigPrefix() + "clientAuth"; @@ -178,6 +179,8 @@ public X509Util() { protected abstract boolean shouldVerifyClientHostname(); + protected abstract boolean shouldAllowReverseDnsLookup(); + public String getSslProtocolProperty() { return sslProtocolProperty; } @@ -234,6 +237,10 @@ public String getSslHostnameVerificationEnabledProperty() { return sslHostnameVerificationEnabledProperty; } + public String getSslAllowReverseDnsLookupProperty() { + return sslAllowReverseDnsLookupProperty; + } + public String getSslCrlEnabledProperty() { return sslCrlEnabledProperty; } @@ -272,6 +279,10 @@ public boolean isClientHostnameVerificationEnabled(ZKConfig config) { return isServerHostnameVerificationEnabled(config) && shouldVerifyClientHostname(); } + public boolean allowReverseDnsLookup(ZKConfig config) { + return config.getBoolean(this.getSslAllowReverseDnsLookupProperty(), shouldAllowReverseDnsLookup()); + } + public SSLContext getDefaultSSLContext() throws X509Exception.SSLContextException { return getDefaultSSLContextAndOptions().getSSLContext(); } @@ -389,6 +400,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()) { @@ -398,7 +410,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) { @@ -534,6 +546,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 = ""; @@ -568,7 +581,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 0d98d35ba43..7aa68cc8530 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 @@ -127,6 +127,7 @@ private void putSSLProperties(X509Util x509Util) { properties.put(x509Util.getSslTruststoreTypeProperty(), System.getProperty(x509Util.getSslTruststoreTypeProperty())); properties.put(x509Util.getSslContextSupplierClassProperty(), System.getProperty(x509Util.getSslContextSupplierClassProperty())); 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 3c29b5f0848..246c13f715b 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 @@ -80,6 +80,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; @@ -112,6 +113,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 ddffd426dd8..a4ac4754373 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 @@ -361,6 +361,7 @@ public void testLoadPEMTrustStore( false, true, true, + false, false); } @@ -382,6 +383,7 @@ public void testLoadPEMTrustStoreNullPassword( false, true, true, + false, false); } @@ -401,6 +403,7 @@ public void testLoadPEMTrustStoreAutodetectStoreFileType( false, true, true, + false, false); } @@ -476,6 +479,7 @@ public void testLoadJKSTrustStore( true, true, true, + false, false); } @@ -497,6 +501,7 @@ public void testLoadJKSTrustStoreNullPassword( false, true, true, + false, false); } @@ -515,6 +520,7 @@ public void testLoadJKSTrustStoreAutodetectStoreFileType( true, true, true, + false, false); } @@ -534,6 +540,7 @@ public void testLoadJKSTrustStoreWithWrongPassword( true, true, true, + false, false); }); } @@ -609,6 +616,7 @@ public void testLoadPKCS12TrustStore( true, true, true, + false, false); } @@ -630,6 +638,7 @@ public void testLoadPKCS12TrustStoreNullPassword( false, true, true, + false, false); } @@ -648,6 +657,7 @@ public void testLoadPKCS12TrustStoreAutodetectStoreFileType( true, true, true, + false, false); } @@ -667,6 +677,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 44d4a3af7b6..1096d55b61e 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