[GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...

[anmolnar <gi...@git.apache.org>]

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

    https://github.com/apache/zookeeper/pull/184#discussion_r221566036
  
    --- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java ---
    @@ -160,43 +239,130 @@ public static X509KeyManager createKeyManager(String keyStoreLocation, String ke
                 }
                 throw new KeyManagerException("Couldn't find X509KeyManager");
     
    -        } catch (Exception e) {
    -            throw new KeyManagerException(e);
    +        } catch (IOException|CertificateException|UnrecoverableKeyException|NoSuchAlgorithmException|KeyStoreException
    +                keyManagerCreationException) {
    +            throw new KeyManagerException(keyManagerCreationException);
             } finally {
                 if (inputStream != null) {
                     try {
                         inputStream.close();
    -                } catch (IOException e) {}
    +                } catch (IOException ioException) {
    +                    LOG.info("Failed to close key store input stream", ioException);
    +                }
                 }
             }
         }
     
    -    public static X509TrustManager createTrustManager(String trustStoreLocation, String trustStorePassword)
    +    public static X509TrustManager createTrustManager(String trustStoreLocation, String trustStorePassword,
    +                                                      boolean crlEnabled, boolean ocspEnabled,
    +                                                      final boolean serverHostnameVerificationEnabled,
    +                                                      final boolean clientHostnameVerificationEnabled)
                 throws TrustManagerException {
             FileInputStream inputStream = null;
             try {
    -            char[] trustStorePasswordChars = trustStorePassword.toCharArray();
                 File trustStoreFile = new File(trustStoreLocation);
                 KeyStore ts = KeyStore.getInstance("JKS");
                 inputStream = new FileInputStream(trustStoreFile);
    -            ts.load(inputStream, trustStorePasswordChars);
    -            TrustManagerFactory tmf = TrustManagerFactory.getInstance("SunX509");
    -            tmf.init(ts);
    +            if (trustStorePassword != null) {
    +                char[] trustStorePasswordChars = trustStorePassword.toCharArray();
    +                ts.load(inputStream, trustStorePasswordChars);
    +            } else {
    +                ts.load(inputStream, null);
    +            }
    +
    +            PKIXBuilderParameters pbParams = new PKIXBuilderParameters(ts, new X509CertSelector());
    +            if (crlEnabled || ocspEnabled) {
    +                pbParams.setRevocationEnabled(true);
    +                System.setProperty("com.sun.net.ssl.checkRevocation", "true");
    +                System.setProperty("com.sun.security.enableCRLDP", "true");
    +                if (ocspEnabled) {
    +                    Security.setProperty("ocsp.enable", "true");
    +                }
    +
    +            } else {
    +                pbParams.setRevocationEnabled(false);
    +            }
     
    -            for (TrustManager tm : tmf.getTrustManagers()) {
    -                if (tm instanceof X509TrustManager) {
    -                    return (X509TrustManager) tm;
    +            // Revocation checking is only supported with the PKIX algorithm
    +            TrustManagerFactory tmf = TrustManagerFactory.getInstance("PKIX");
    +            tmf.init(new CertPathTrustManagerParameters(pbParams));
    +
    +            for (final TrustManager tm : tmf.getTrustManagers()) {
    +                if (tm instanceof X509ExtendedTrustManager) {
    +                    return new ZKTrustManager((X509ExtendedTrustManager) tm,
    +                            serverHostnameVerificationEnabled, clientHostnameVerificationEnabled);
                     }
                 }
                 throw new TrustManagerException("Couldn't find X509TrustManager");
    -        } catch (Exception e) {
    -            throw new TrustManagerException(e);
    +        } catch (IOException|CertificateException|NoSuchAlgorithmException|InvalidAlgorithmParameterException|KeyStoreException
    +                 trustManagerCreationException) {
    +            throw new TrustManagerException(trustManagerCreationException);
             } finally {
                 if (inputStream != null) {
                     try {
                         inputStream.close();
    -                } catch (IOException e) {}
    +                } catch (IOException ioException) {
    +                    LOG.info("failed to close TrustStore input stream", ioException);
    +                }
                 }
             }
         }
    -}
    \ No newline at end of file
    +
    +    public SSLSocket createSSLSocket() throws X509Exception, IOException {
    +        SSLSocket sslSocket = (SSLSocket) getDefaultSSLContext().getSocketFactory().createSocket();
    +        configureSSLSocket(sslSocket);
    +
    +        return sslSocket;
    +    }
    +
    +    public SSLSocket createSSLSocket(Socket socket) throws X509Exception, IOException {
    +        SSLSocket sslSocket = (SSLSocket) getDefaultSSLContext().getSocketFactory().createSocket(socket, null, socket.getPort(), true);
    +        configureSSLSocket(sslSocket);
    +
    +        return sslSocket;
    +    }
    +
    +    private void configureSSLSocket(SSLSocket sslSocket) {
    +        if (cipherSuites != null) {
    +            SSLParameters sslParameters = sslSocket.getSSLParameters();
    +            LOG.debug("Setup cipher suites for client socket: {}", Arrays.toString(cipherSuites));
    +            sslParameters.setCipherSuites(cipherSuites);
    +            sslSocket.setSSLParameters(sslParameters);
    +        }
    +    }
    +
    +    public SSLServerSocket createSSLServerSocket() throws X509Exception, IOException {
    +        SSLServerSocket sslServerSocket = (SSLServerSocket) getDefaultSSLContext().getServerSocketFactory().createServerSocket();
    +        configureSSLServerSocket(sslServerSocket);
    +
    +        return sslServerSocket;
    +    }
    +
    +    public SSLServerSocket createSSLServerSocket(int port) throws X509Exception, IOException {
    +        SSLServerSocket sslServerSocket = (SSLServerSocket) getDefaultSSLContext().getServerSocketFactory().createServerSocket(port);
    +        configureSSLServerSocket(sslServerSocket);
    +
    +        return sslServerSocket;
    +    }
    +
    +    private void configureSSLServerSocket(SSLServerSocket sslServerSocket) {
    +        SSLParameters sslParameters = sslServerSocket.getSSLParameters();
    +        sslParameters.setNeedClientAuth(true);
    +        if (cipherSuites != null) {
    --- End diff --
    
    Makes sense. I removed them from both `configure` methods (server & client).


---