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, 11 Jun 2018 15:07:18 GMT
Github user anmolnar commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/184#discussion_r194438668
  
    --- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java ---
    @@ -18,64 +18,119 @@
     package org.apache.zookeeper.common;
     
     
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import javax.net.ssl.CertPathTrustManagerParameters;
     import javax.net.ssl.KeyManager;
     import javax.net.ssl.KeyManagerFactory;
     import javax.net.ssl.SSLContext;
    +import javax.net.ssl.SSLParameters;
    +import javax.net.ssl.SSLServerSocket;
    +import javax.net.ssl.SSLSocket;
     import javax.net.ssl.TrustManager;
     import javax.net.ssl.TrustManagerFactory;
    +import javax.net.ssl.X509ExtendedTrustManager;
     import javax.net.ssl.X509KeyManager;
     import javax.net.ssl.X509TrustManager;
     import java.io.File;
     import java.io.FileInputStream;
     import java.io.IOException;
    +import java.net.Socket;
    +import java.security.InvalidAlgorithmParameterException;
    +import java.security.KeyManagementException;
     import java.security.KeyStore;
    +import java.security.KeyStoreException;
    +import java.security.NoSuchAlgorithmException;
    +import java.security.Security;
    +import java.security.UnrecoverableKeyException;
    +import java.security.cert.CertificateException;
    +import java.security.cert.PKIXBuilderParameters;
    +import java.security.cert.X509CertSelector;
    +import java.util.Arrays;
     
    -import org.slf4j.Logger;
    -import org.slf4j.LoggerFactory;
    -
    -import static org.apache.zookeeper.common.X509Exception.KeyManagerException;
    -import static org.apache.zookeeper.common.X509Exception.SSLContextException;
    -import static org.apache.zookeeper.common.X509Exception.TrustManagerException;
    +import org.apache.zookeeper.common.X509Exception.KeyManagerException;
    +import org.apache.zookeeper.common.X509Exception.SSLContextException;
    +import org.apache.zookeeper.common.X509Exception.TrustManagerException;
     
     /**
      * Utility code for X509 handling
      */
    -public class X509Util {
    +public abstract class X509Util {
         private static final Logger LOG = LoggerFactory.getLogger(X509Util.class);
     
    -    /**
    -     * @deprecated Use {@link ZKConfig#SSL_KEYSTORE_LOCATION}
    -     *             instead.
    -     */
    -    @Deprecated
    -    public static final String SSL_KEYSTORE_LOCATION = "zookeeper.ssl.keyStore.location";
    -    /**
    -     * @deprecated Use {@link ZKConfig#SSL_KEYSTORE_PASSWD}
    -     *             instead.
    -     */
    -    @Deprecated
    -    public static final String SSL_KEYSTORE_PASSWD = "zookeeper.ssl.keyStore.password";
    -    /**
    -     * @deprecated Use {@link ZKConfig#SSL_TRUSTSTORE_LOCATION}
    -     *             instead.
    -     */
    -    @Deprecated
    -    public static final String SSL_TRUSTSTORE_LOCATION = "zookeeper.ssl.trustStore.location";
    -    /**
    -     * @deprecated Use {@link ZKConfig#SSL_TRUSTSTORE_PASSWD}
    -     *             instead.
    -     */
    -    @Deprecated
    -    public static final String SSL_TRUSTSTORE_PASSWD = "zookeeper.ssl.trustStore.password";
    -    /**
    -     * @deprecated Use {@link ZKConfig#SSL_AUTHPROVIDER}
    -     *             instead.
    -     */
    -    @Deprecated
    -    public static final String SSL_AUTHPROVIDER = "zookeeper.ssl.authProvider";
    -
    -    public static SSLContext createSSLContext() throws SSLContextException {
    -        /**
    +    static final String DEFAULT_PROTOCOL = "TLSv1.2";
    +
    +    private String sslProtocolProperty = getConfigPrefix() + "protocol";
    +    private String cipherSuitesProperty = getConfigPrefix() + "ciphersuites";
    +    private String sslKeystoreLocationProperty = getConfigPrefix() + "keyStore.location";
    +    private String sslKeystorePasswdProperty = getConfigPrefix() + "keyStore.password";
    +    private String sslTruststoreLocationProperty = getConfigPrefix() + "trustStore.location";
    +    private String sslTruststorePasswdProperty = getConfigPrefix() + "trustStore.password";
    +    private String sslHostnameVerificationEnabledProperty = getConfigPrefix() + "hostnameVerification";
    +    private String sslCrlEnabledProperty = getConfigPrefix() + "crl";
    +    private String sslOcspEnabledProperty = getConfigPrefix() + "ocsp";
    +
    +    private String[] cipherSuites;
    --- End diff --
    
    Do you think we should follow the same approach with enabled protocols?
    
    Currently there're 2 settings we're dealing with in this patch regarding protocols:
    1. a default protocol setting which is configurable and set when we create default SSL
context,
    2. restriction of enabled protocols which should be set similarly to enabled cipher suites
when creating the socket.
    
    The latter one is missing from the code currently, hence the test is failing.
    
    
     


---

Mime
View raw message