Skip to content

Commit 66c4efe

Browse files
anmolnaranmolnar
authored
ZOOKEEPER-4986: Disable reverse DNS lookup in TLS client and server
Reviewers: kezhuw Author: anmolnar Closes #2325 from anmolnar/ZOOKEEPER-4986
1 parent 51c4f2b commit 66c4efe

File tree

12 files changed

+177
-41
lines changed

12 files changed

+177
-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
@@ -1764,6 +1764,16 @@ and [SASL authentication for ZooKeeper](https://cwiki.apache.org/confluence/disp
17641764
This option requires the corresponding *hostnameVerification* option to be `true`, or it will be ignored.
17651765
Default: true for quorum, false for clients
17661766

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

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,14 +206,16 @@ private TrustManager getTrustManager(ZKConfig config) throws X509Exception.Trust
206206
boolean sslOcspEnabled = config.getBoolean(getSslOcspEnabledProperty(), Boolean.parseBoolean(Security.getProperty("ocsp.enable")));
207207
boolean sslServerHostnameVerificationEnabled = isServerHostnameVerificationEnabled(config);
208208
boolean sslClientHostnameVerificationEnabled = isClientHostnameVerificationEnabled(config);
209+
boolean allowReverseDnsLookup = allowReverseDnsLookup(config);
209210

210211
if (trustStoreLocation.isEmpty()) {
211212
LOG.warn("{} not specified", getSslTruststoreLocationProperty());
212213
return null;
213214
} else {
214215
return createTrustManager(trustStoreLocation, trustStorePassword, trustStoreType,
215216
sslCrlEnabled, sslOcspEnabled, sslServerHostnameVerificationEnabled,
216-
sslClientHostnameVerificationEnabled, getFipsMode(config));
217+
sslClientHostnameVerificationEnabled, allowReverseDnsLookup,
218+
getFipsMode(config));
217219
}
218220
}
219221
}

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ public io.netty.handler.ssl.ClientAuth toNettyClientAuth() {
166166
private final String sslContextSupplierClassProperty = getConfigPrefix() + "context.supplier.class";
167167
private final String sslHostnameVerificationEnabledProperty = getConfigPrefix() + "hostnameVerification";
168168
private final String sslClientHostnameVerificationEnabledProperty = getConfigPrefix() + "clientHostnameVerification";
169+
private final String sslAllowReverseDnsLookupProperty = getConfigPrefix() + "allowReverseDnsLookup";
169170
private final String sslCrlEnabledProperty = getConfigPrefix() + "crl";
170171
private final String sslOcspEnabledProperty = getConfigPrefix() + "ocsp";
171172
private final String sslClientAuthProperty = getConfigPrefix() + "clientAuth";
@@ -244,6 +245,10 @@ public String getSslClientHostnameVerificationEnabledProperty() {
244245
return sslClientHostnameVerificationEnabledProperty;
245246
}
246247

248+
public String getSslAllowReverseDnsLookupProperty() {
249+
return sslAllowReverseDnsLookupProperty;
250+
}
251+
247252
public String getSslCrlEnabledProperty() {
248253
return sslCrlEnabledProperty;
249254
}
@@ -283,6 +288,10 @@ public boolean isClientHostnameVerificationEnabled(ZKConfig config) {
283288
&& config.getBoolean(this.getSslClientHostnameVerificationEnabledProperty(), shouldVerifyClientHostname());
284289
}
285290

291+
public boolean allowReverseDnsLookup(ZKConfig config) {
292+
return config.getBoolean(this.getSslAllowReverseDnsLookupProperty(), false);
293+
}
294+
286295
public SSLContext getDefaultSSLContext() throws X509Exception.SSLContextException {
287296
return getDefaultSSLContextAndOptions().getSSLContext();
288297
}
@@ -401,6 +410,7 @@ public SSLContextAndOptions createSSLContextAndOptionsFromConfig(ZKConfig config
401410

402411
boolean sslServerHostnameVerificationEnabled = isServerHostnameVerificationEnabled(config);
403412
boolean sslClientHostnameVerificationEnabled = isClientHostnameVerificationEnabled(config);
413+
boolean allowReverseDnsLookup = allowReverseDnsLookup(config);
404414
boolean fipsMode = getFipsMode(config);
405415

406416
if (trustStoreLocationProp.isEmpty()) {
@@ -410,7 +420,7 @@ public SSLContextAndOptions createSSLContextAndOptionsFromConfig(ZKConfig config
410420
trustManagers = new TrustManager[]{
411421
createTrustManager(trustStoreLocationProp, trustStorePasswordProp, trustStoreTypeProp, sslCrlEnabled,
412422
sslOcspEnabled, sslServerHostnameVerificationEnabled, sslClientHostnameVerificationEnabled,
413-
fipsMode)};
423+
allowReverseDnsLookup, fipsMode)};
414424
} catch (TrustManagerException trustManagerException) {
415425
throw new SSLContextException("Failed to create TrustManager", trustManagerException);
416426
} catch (IllegalArgumentException e) {
@@ -546,6 +556,7 @@ public static X509TrustManager createTrustManager(
546556
boolean ocspEnabled,
547557
final boolean serverHostnameVerificationEnabled,
548558
final boolean clientHostnameVerificationEnabled,
559+
final boolean allowReverseDnsLookup,
549560
final boolean fipsMode) throws TrustManagerException {
550561
if (trustStorePassword == null) {
551562
trustStorePassword = "";
@@ -604,7 +615,7 @@ public static X509TrustManager createTrustManager(
604615
LOG.debug("FIPS mode is OFF: creating ZKTrustManager");
605616
}
606617
return new ZKTrustManager((X509ExtendedTrustManager) tm, serverHostnameVerificationEnabled,
607-
clientHostnameVerificationEnabled);
618+
clientHostnameVerificationEnabled, allowReverseDnsLookup);
608619
}
609620
}
610621
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
@@ -89,6 +89,7 @@ public X509AuthenticationProvider() throws X509Exception {
8989
boolean crlEnabled = config.getBoolean(x509Util.getSslCrlEnabledProperty(), Boolean.getBoolean("com.sun.net.ssl.checkRevocation"));
9090
boolean ocspEnabled = config.getBoolean(x509Util.getSslOcspEnabledProperty(), Boolean.parseBoolean(Security.getProperty("ocsp.enable")));
9191
boolean hostnameVerificationEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslHostnameVerificationEnabledProperty()));
92+
boolean allowReverseDnsLookup = Boolean.parseBoolean(config.getProperty(x509Util.getSslAllowReverseDnsLookupProperty()));
9293

9394
X509KeyManager km = null;
9495
X509TrustManager tm = null;
@@ -121,6 +122,7 @@ public X509AuthenticationProvider() throws X509Exception {
121122
ocspEnabled,
122123
hostnameVerificationEnabled,
123124
false,
125+
allowReverseDnsLookup,
124126
fipsMode);
125127
} catch (TrustManagerException e) {
126128
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
@@ -332,6 +332,7 @@ public void testLoadPEMTrustStore(
332332
false,
333333
true,
334334
true,
335+
false,
335336
false);
336337
}
337338

@@ -353,6 +354,7 @@ public void testLoadPEMTrustStoreNullPassword(
353354
false,
354355
true,
355356
true,
357+
false,
356358
false);
357359

358360
}
@@ -372,6 +374,7 @@ public void testLoadPEMTrustStoreAutodetectStoreFileType(
372374
false,
373375
true,
374376
true,
377+
false,
375378
false);
376379
}
377380

@@ -447,6 +450,7 @@ public void testLoadJKSTrustStore(
447450
true,
448451
true,
449452
true,
453+
false,
450454
false);
451455
}
452456

@@ -468,6 +472,7 @@ public void testLoadJKSTrustStoreNullPassword(
468472
false,
469473
true,
470474
true,
475+
false,
471476
false);
472477
}
473478

@@ -486,6 +491,7 @@ public void testLoadJKSTrustStoreAutodetectStoreFileType(
486491
true,
487492
true,
488493
true,
494+
false,
489495
false);
490496
}
491497

@@ -505,6 +511,7 @@ public void testLoadJKSTrustStoreWithWrongPassword(
505511
true,
506512
true,
507513
true,
514+
false,
508515
false);
509516
});
510517
}
@@ -580,6 +587,7 @@ public void testLoadPKCS12TrustStore(
580587
true,
581588
true,
582589
true,
590+
false,
583591
false);
584592
}
585593

@@ -601,6 +609,7 @@ public void testLoadPKCS12TrustStoreNullPassword(
601609
false,
602610
true,
603611
true,
612+
false,
604613
false);
605614
}
606615

@@ -619,6 +628,7 @@ public void testLoadPKCS12TrustStoreAutodetectStoreFileType(
619628
true,
620629
true,
621630
true,
631+
false,
622632
false);
623633
}
624634

@@ -638,6 +648,7 @@ public void testLoadPKCS12TrustStoreWithWrongPassword(
638648
true,
639649
true,
640650
true,
651+
false,
641652
false);
642653
});
643654
}

0 commit comments

Comments
 (0)