Skip to content

Commit 54e25d1

Browse files
anmolnaranmolnar
committed
ZOOKEEPER-4986: Disable reverse DNS lookup in TLS client and server
Reviewers: kezhuw Author: anmolnar Closes apache#2325 from anmolnar/ZOOKEEPER-4986
1 parent 2d4d4eb commit 54e25d1

File tree

13 files changed

+188
-41
lines changed

13 files changed

+188
-41
lines changed

‎zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1759,6 +1759,16 @@ and [SASL authentication for ZooKeeper](https://cwiki.apache.org/confluence/disp
17591759
This option requires the corresponding *hostnameVerification* option to be `true`, or it will be ignored.
17601760
Default: true for quorum, false for clients
17611761

1762+
* *ssl.allowReverseDnsLookup* and *ssl.quorum.allowReverseDnsLookup* :
1763+
(Java system properties: **zookeeper.ssl.allowReverseDnsLookup** and **zookeeper.ssl.quorum.allowReverseDnsLookup**)
1764+
**New in 3.9.5:**
1765+
Allow reverse DNS lookup in both server- and client hostname verifications if the hostname verification is enabled in
1766+
`ZKTrustManager`. Supported in both quorum and client TLS protocols. Not supported in FIPS mode. Reverse DNS lookups are
1767+
expensive and unnecessary in most cases. Make sure that certificates are created with all required Subject Alternative
1768+
Names (SAN) for successful identity verification. It's recommended to add SAN:IP entries for identity verification
1769+
of client certificates.
1770+
Default: false
1771+
17621772
* *ssl.crl* and *ssl.quorum.crl* :
17631773
(Java system properties: **zookeeper.ssl.crl** and **zookeeper.ssl.quorum.crl**)
17641774
**New in 3.5.5:**

‎zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ protected boolean shouldVerifyClientHostname() {
5252
return false;
5353
}
5454

55+
@Override
56+
protected boolean shouldAllowReverseDnsLookup() {
57+
return false;
58+
}
59+
5560
public String getSslAuthProviderProperty() {
5661
return sslAuthProviderProperty;
5762
}
@@ -202,14 +207,16 @@ private TrustManager getTrustManager(ZKConfig config) throws X509Exception.Trust
202207
boolean sslOcspEnabled = config.getBoolean(getSslOcspEnabledProperty());
203208
boolean sslServerHostnameVerificationEnabled = isServerHostnameVerificationEnabled(config);
204209
boolean sslClientHostnameVerificationEnabled = isClientHostnameVerificationEnabled(config);
210+
boolean allowReverseDnsLookup = allowReverseDnsLookup(config);
205211

206212
if (trustStoreLocation.isEmpty()) {
207213
LOG.warn("{} not specified", getSslTruststoreLocationProperty());
208214
return null;
209215
} else {
210216
return createTrustManager(trustStoreLocation, trustStorePassword, trustStoreType,
211217
sslCrlEnabled, sslOcspEnabled, sslServerHostnameVerificationEnabled,
212-
sslClientHostnameVerificationEnabled, getFipsMode(config));
218+
sslClientHostnameVerificationEnabled, allowReverseDnsLookup,
219+
getFipsMode(config));
213220
}
214221
}
215222
}

‎zookeeper-server/src/main/java/org/apache/zookeeper/common/QuorumX509Util.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,8 @@ protected boolean shouldVerifyClientHostname() {
3333
return true;
3434
}
3535

36+
@Override
37+
protected boolean shouldAllowReverseDnsLookup() {
38+
return true;
39+
}
3640
}

‎zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ public io.netty.handler.ssl.ClientAuth toNettyClientAuth() {
198198
private final String sslContextSupplierClassProperty = getConfigPrefix() + "context.supplier.class";
199199
private final String sslHostnameVerificationEnabledProperty = getConfigPrefix() + "hostnameVerification";
200200
private final String sslClientHostnameVerificationEnabledProperty = getConfigPrefix() + "clientHostnameVerification";
201+
private final String sslAllowReverseDnsLookupProperty = getConfigPrefix() + "allowReverseDnsLookup";
201202
private final String sslCrlEnabledProperty = getConfigPrefix() + "crl";
202203
private final String sslOcspEnabledProperty = getConfigPrefix() + "ocsp";
203204
private final String sslClientAuthProperty = getConfigPrefix() + "clientAuth";
@@ -216,6 +217,8 @@ public X509Util() {
216217

217218
protected abstract boolean shouldVerifyClientHostname();
218219

220+
protected abstract boolean shouldAllowReverseDnsLookup();
221+
219222
public String getSslProtocolProperty() {
220223
return sslProtocolProperty;
221224
}
@@ -276,6 +279,10 @@ public String getSslClientHostnameVerificationEnabledProperty() {
276279
return sslClientHostnameVerificationEnabledProperty;
277280
}
278281

282+
public String getSslAllowReverseDnsLookupProperty() {
283+
return sslAllowReverseDnsLookupProperty;
284+
}
285+
279286
public String getSslCrlEnabledProperty() {
280287
return sslCrlEnabledProperty;
281288
}
@@ -315,6 +322,10 @@ public boolean isClientHostnameVerificationEnabled(ZKConfig config) {
315322
&& config.getBoolean(this.getSslClientHostnameVerificationEnabledProperty(), shouldVerifyClientHostname());
316323
}
317324

325+
public boolean allowReverseDnsLookup(ZKConfig config) {
326+
return config.getBoolean(this.getSslAllowReverseDnsLookupProperty(), shouldAllowReverseDnsLookup());
327+
}
328+
318329
public SSLContext getDefaultSSLContext() throws X509Exception.SSLContextException {
319330
return getDefaultSSLContextAndOptions().getSSLContext();
320331
}
@@ -432,6 +443,7 @@ public SSLContextAndOptions createSSLContextAndOptionsFromConfig(ZKConfig config
432443
boolean sslOcspEnabled = config.getBoolean(this.sslOcspEnabledProperty);
433444
boolean sslServerHostnameVerificationEnabled = isServerHostnameVerificationEnabled(config);
434445
boolean sslClientHostnameVerificationEnabled = isClientHostnameVerificationEnabled(config);
446+
boolean allowReverseDnsLookup = allowReverseDnsLookup(config);
435447
boolean fipsMode = getFipsMode(config);
436448

437449
if (trustStoreLocationProp.isEmpty()) {
@@ -441,7 +453,7 @@ public SSLContextAndOptions createSSLContextAndOptionsFromConfig(ZKConfig config
441453
trustManagers = new TrustManager[]{
442454
createTrustManager(trustStoreLocationProp, trustStorePasswordProp, trustStoreTypeProp, sslCrlEnabled,
443455
sslOcspEnabled, sslServerHostnameVerificationEnabled, sslClientHostnameVerificationEnabled,
444-
fipsMode)};
456+
allowReverseDnsLookup, fipsMode)};
445457
} catch (TrustManagerException trustManagerException) {
446458
throw new SSLContextException("Failed to create TrustManager", trustManagerException);
447459
} catch (IllegalArgumentException e) {
@@ -577,6 +589,7 @@ public static X509TrustManager createTrustManager(
577589
boolean ocspEnabled,
578590
final boolean serverHostnameVerificationEnabled,
579591
final boolean clientHostnameVerificationEnabled,
592+
final boolean allowReverseDnsLookup,
580593
final boolean fipsMode) throws TrustManagerException {
581594
if (trustStorePassword == null) {
582595
trustStorePassword = "";
@@ -611,7 +624,7 @@ public static X509TrustManager createTrustManager(
611624
LOG.debug("FIPS mode is OFF: creating ZKTrustManager");
612625
}
613626
return new ZKTrustManager((X509ExtendedTrustManager) tm, serverHostnameVerificationEnabled,
614-
clientHostnameVerificationEnabled);
627+
clientHostnameVerificationEnabled, allowReverseDnsLookup);
615628
}
616629
}
617630
throw new TrustManagerException("Couldn't find X509TrustManager");

‎zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ private void putSSLProperties(X509Util x509Util) {
151151
properties.put(x509Util.getSslContextSupplierClassProperty(), System.getProperty(x509Util.getSslContextSupplierClassProperty()));
152152
properties.put(x509Util.getSslClientHostnameVerificationEnabledProperty(), System.getProperty(x509Util.getSslClientHostnameVerificationEnabledProperty()));
153153
properties.put(x509Util.getSslHostnameVerificationEnabledProperty(), System.getProperty(x509Util.getSslHostnameVerificationEnabledProperty()));
154+
properties.put(x509Util.getSslAllowReverseDnsLookupProperty(), System.getProperty(x509Util.getSslAllowReverseDnsLookupProperty()));
154155
properties.put(x509Util.getSslCrlEnabledProperty(), System.getProperty(x509Util.getSslCrlEnabledProperty()));
155156
properties.put(x509Util.getSslOcspEnabledProperty(), System.getProperty(x509Util.getSslOcspEnabledProperty()));
156157
properties.put(x509Util.getSslClientAuthProperty(), System.getProperty(x509Util.getSslClientAuthProperty()));

‎zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKTrustManager.java‎

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ public class ZKTrustManager extends X509ExtendedTrustManager {
4242
private final X509ExtendedTrustManager x509ExtendedTrustManager;
4343
private final boolean serverHostnameVerificationEnabled;
4444
private final boolean clientHostnameVerificationEnabled;
45+
private final boolean allowReverseDnsLookup;
4546

4647
private final ZKHostnameVerifier hostnameVerifier;
4748

@@ -57,22 +58,26 @@ public class ZKTrustManager extends X509ExtendedTrustManager {
5758
ZKTrustManager(
5859
X509ExtendedTrustManager x509ExtendedTrustManager,
5960
boolean serverHostnameVerificationEnabled,
60-
boolean clientHostnameVerificationEnabled) {
61+
boolean clientHostnameVerificationEnabled,
62+
boolean allowReverseDnsLookup) {
6163
this(x509ExtendedTrustManager,
6264
serverHostnameVerificationEnabled,
6365
clientHostnameVerificationEnabled,
64-
new ZKHostnameVerifier());
66+
new ZKHostnameVerifier(),
67+
allowReverseDnsLookup);
6568
}
6669

6770
ZKTrustManager(
6871
X509ExtendedTrustManager x509ExtendedTrustManager,
6972
boolean serverHostnameVerificationEnabled,
7073
boolean clientHostnameVerificationEnabled,
71-
ZKHostnameVerifier hostnameVerifier) {
74+
ZKHostnameVerifier hostnameVerifier,
75+
boolean allowReverseDnsLookup) {
7276
this.x509ExtendedTrustManager = x509ExtendedTrustManager;
7377
this.serverHostnameVerificationEnabled = serverHostnameVerificationEnabled;
7478
this.clientHostnameVerificationEnabled = clientHostnameVerificationEnabled;
7579
this.hostnameVerifier = hostnameVerifier;
80+
this.allowReverseDnsLookup = allowReverseDnsLookup;
7681
}
7782

7883
@Override
@@ -166,31 +171,46 @@ public void checkServerTrusted(X509Certificate[] chain, String authType) throws
166171
* @param certificate Peer's certificate
167172
* @throws CertificateException Thrown if the provided certificate doesn't match the peer hostname.
168173
*/
169-
private void performHostVerification(
170-
InetAddress inetAddress,
171-
X509Certificate certificate
172-
) throws CertificateException {
174+
private void performHostVerification(InetAddress inetAddress, X509Certificate certificate)
175+
throws CertificateException {
173176
String hostAddress = "";
174177
String hostName = "";
175178
try {
176179
hostAddress = inetAddress.getHostAddress();
177-
if (LOG.isDebugEnabled()) {
178-
LOG.debug("Trying to verify host address first: {}", hostAddress);
179-
}
180180
hostnameVerifier.verify(hostAddress, certificate);
181181
} catch (SSLException addressVerificationException) {
182+
// If we fail with hostAddress, we should try the hostname.
183+
// The inetAddress may have been created with a hostname, in which case getHostName() will
184+
// return quickly below. If not, a reverse lookup will happen, which can be expensive.
185+
// We provide the option to skip the reverse lookup if preferring to fail fast.
186+
187+
// Handle logging here to aid debugging. The easiest way to check for an existing
188+
// hostname is through toString, see javadoc.
189+
String inetAddressString = inetAddress.toString();
190+
if (!inetAddressString.startsWith("/")) {
191+
LOG.debug(
192+
"Failed to verify host address: {}, but inetAddress {} has a hostname, trying that",
193+
hostAddress, inetAddressString, addressVerificationException);
194+
} else if (allowReverseDnsLookup) {
195+
LOG.debug(
196+
"Failed to verify host address: {}, attempting to verify host name with reverse dns",
197+
hostAddress, addressVerificationException);
198+
} else {
199+
LOG.debug("Failed to verify host address: {}, but reverse dns lookup is disabled",
200+
hostAddress, addressVerificationException);
201+
throw new CertificateException(
202+
"Failed to verify host address, and reverse lookup is disabled",
203+
addressVerificationException);
204+
}
205+
182206
try {
183207
hostName = inetAddress.getHostName();
184-
if (LOG.isDebugEnabled()) {
185-
LOG.debug(
186-
"Failed to verify host address: {}, trying to verify host name: {}",
187-
hostAddress, hostName);
188-
}
189208
hostnameVerifier.verify(hostName, certificate);
190209
} catch (SSLException hostnameVerificationException) {
191210
LOG.error("Failed to verify host address: {}", hostAddress, addressVerificationException);
192211
LOG.error("Failed to verify hostname: {}", hostName, hostnameVerificationException);
193-
throw new CertificateException("Failed to verify both host address and host name", hostnameVerificationException);
212+
throw new CertificateException("Failed to verify both host address and host name",
213+
hostnameVerificationException);
194214
}
195215
}
196216
}

‎zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ public X509AuthenticationProvider() throws X509Exception {
8888
boolean crlEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslCrlEnabledProperty()));
8989
boolean ocspEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslOcspEnabledProperty()));
9090
boolean hostnameVerificationEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslHostnameVerificationEnabledProperty()));
91+
boolean allowReverseDnsLookup = Boolean.parseBoolean(config.getProperty(x509Util.getSslAllowReverseDnsLookupProperty()));
9192

9293
X509KeyManager km = null;
9394
X509TrustManager tm = null;
@@ -120,6 +121,7 @@ public X509AuthenticationProvider() throws X509Exception {
120121
ocspEnabled,
121122
hostnameVerificationEnabled,
122123
false,
124+
allowReverseDnsLookup,
123125
fipsMode);
124126
} catch (TrustManagerException e) {
125127
LOG.error("Failed to create trust manager", e);

‎zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,7 @@ public void testLoadPEMTrustStore(
381381
false,
382382
true,
383383
true,
384+
false,
384385
false);
385386
}
386387

@@ -402,6 +403,7 @@ public void testLoadPEMTrustStoreNullPassword(
402403
false,
403404
true,
404405
true,
406+
false,
405407
false);
406408

407409
}
@@ -421,6 +423,7 @@ public void testLoadPEMTrustStoreAutodetectStoreFileType(
421423
false,
422424
true,
423425
true,
426+
false,
424427
false);
425428
}
426429

@@ -496,6 +499,7 @@ public void testLoadJKSTrustStore(
496499
true,
497500
true,
498501
true,
502+
false,
499503
false);
500504
}
501505

@@ -517,6 +521,7 @@ public void testLoadJKSTrustStoreNullPassword(
517521
false,
518522
true,
519523
true,
524+
false,
520525
false);
521526
}
522527

@@ -535,6 +540,7 @@ public void testLoadJKSTrustStoreAutodetectStoreFileType(
535540
true,
536541
true,
537542
true,
543+
false,
538544
false);
539545
}
540546

@@ -554,6 +560,7 @@ public void testLoadJKSTrustStoreWithWrongPassword(
554560
true,
555561
true,
556562
true,
563+
false,
557564
false);
558565
});
559566
}
@@ -629,6 +636,7 @@ public void testLoadPKCS12TrustStore(
629636
true,
630637
true,
631638
true,
639+
false,
632640
false);
633641
}
634642

@@ -650,6 +658,7 @@ public void testLoadPKCS12TrustStoreNullPassword(
650658
false,
651659
true,
652660
true,
661+
false,
653662
false);
654663
}
655664

@@ -668,6 +677,7 @@ public void testLoadPKCS12TrustStoreAutodetectStoreFileType(
668677
true,
669678
true,
670679
true,
680+
false,
671681
false);
672682
}
673683

@@ -687,6 +697,7 @@ public void testLoadPKCS12TrustStoreWithWrongPassword(
687697
true,
688698
true,
689699
true,
700+
false,
690701
false);
691702
});
692703
}

0 commit comments

Comments
 (0)