zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From anmolnar <...@git.apache.org>
Subject [GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...
Date Mon, 01 Oct 2018 12:52:00 GMT
Github user anmolnar commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/184#discussion_r221596357
  
    --- 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 --
    
    Very good question. I have no idea. :(
    Sounds like it only makes sense this way: why would you verify client certs and not the
servers?
    @ivmaykov Does it ring a bell?


---

Mime
View raw message