Github user ivmaykov commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/184#discussion_r222111859
--- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java ---
@@ -84,61 +163,61 @@ public static SSLContext createSSLContext() throws
SSLContextException {
return createSSLContext(config);
}
- public static SSLContext createSSLContext(ZKConfig config) throws
SSLContextException {
+ public SSLContext createSSLContext(ZKConfig config) throws
SSLContextException {
KeyManager[] keyManagers = null;
TrustManager[] trustManagers = null;
- String keyStoreLocationProp =
config.getProperty(ZKConfig.SSL_KEYSTORE_LOCATION);
- String keyStorePasswordProp =
config.getProperty(ZKConfig.SSL_KEYSTORE_PASSWD);
+ String keyStoreLocationProp =
config.getProperty(sslKeystoreLocationProperty);
+ String keyStorePasswordProp =
config.getProperty(sslKeystorePasswdProperty);
// There are legal states in some use cases for null KeyManager or
TrustManager.
// But if a user wanna specify one, location and password are
required.
if (keyStoreLocationProp == null && keyStorePasswordProp == null) {
- LOG.warn("keystore not specified for client connection");
+ LOG.warn(getSslKeystoreLocationProperty() + " not specified");
} else {
if (keyStoreLocationProp == null) {
- throw new SSLContextException("keystore location not
specified for client connection");
+ throw new
SSLContextException(getSslKeystoreLocationProperty() + " not specified");
}
if (keyStorePasswordProp == null) {
- throw new SSLContextException("keystore password not
specified for client connection");
+ throw new
SSLContextException(getSslKeystorePasswdProperty() + " not specified");
}
try {
keyManagers = new KeyManager[]{
createKeyManager(keyStoreLocationProp,
keyStorePasswordProp)};
- } catch (KeyManagerException e) {
- throw new SSLContextException("Failed to create
KeyManager", e);
+ } catch (KeyManagerException keyManagerException) {
+ throw new SSLContextException("Failed to create
KeyManager", keyManagerException);
}
}
- String trustStoreLocationProp =
config.getProperty(ZKConfig.SSL_TRUSTSTORE_LOCATION);
- String trustStorePasswordProp =
config.getProperty(ZKConfig.SSL_TRUSTSTORE_PASSWD);
+ String trustStoreLocationProp =
config.getProperty(sslTruststoreLocationProperty);
+ String trustStorePasswordProp =
config.getProperty(sslTruststorePasswdProperty);
- if (trustStoreLocationProp == null && trustStorePasswordProp ==
null) {
- LOG.warn("Truststore not specified for client connection");
+ boolean sslCrlEnabled =
config.getBoolean(this.sslCrlEnabledProperty);
+ boolean sslOcspEnabled =
config.getBoolean(this.sslOcspEnabledProperty);
+ boolean sslServerHostnameVerificationEnabled =
config.getBoolean(this.getSslHostnameVerificationEnabledProperty(), true);
+ boolean sslClientHostnameVerificationEnabled =
sslServerHostnameVerificationEnabled && shouldVerifyClientHostname();
--- End diff --
the local variable `sslServerHostnameVerificationEnabled` is read from the
config option `getSslHostnameVerificationEnabledProperty` - the config option
is not server-specific. So I think the confusion is from local variable naming.
Change it to something like:
boolean hostnameVerificationEnabled =
config.getBoolean(this.getSslHostnameVerificationEnabledProperty(), true);
// If hostname verification is enabled, we always verify servers
boolean sslServerHostnameVerificationEnabled =
hostnameVerificationEnabled;
// We verify clients only if hostname verification is enabled in the
config,
// and `shouldVerifyClientHostname()` returns true.
boolean sslClientHostnameVerificationEnabled =
shouldVerifyClientHostname() && hostnameVerificationEnabled;
---
