Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/184#discussion_r221087073
  
    --- 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 --
    
    is there a particular reason that `sslClientHostnameVerificationEnabled` 
depends on `sslServerHostnameVerificationEnabled`, instead of just depends on 
`shouldVerifyClientHostname()` ?


---

Reply via email to