You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2022/07/31 13:23:35 UTC

[GitHub] [hbase] Apache9 commented on a diff in pull request #4666: HBASE-26666 Add native TLS encryption support to RPC server/client

Apache9 commented on code in PR #4666:
URL: https://github.com/apache/hbase/pull/4666#discussion_r933983025


##########
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcConnection.java:
##########
@@ -157,11 +167,11 @@ public void cleanupConnection() {
   private void established(Channel ch) throws IOException {
     assert eventLoop.inEventLoop();
     ChannelPipeline p = ch.pipeline();
-    String addBeforeHandler = p.context(BufferCallBeforeInitHandler.class).name();
-    p.addBefore(addBeforeHandler, null,
+    p.addBefore(BufferCallBeforeInitHandler.NAME, null,
       new IdleStateHandler(0, rpcClient.minIdleTimeBeforeClose, 0, TimeUnit.MILLISECONDS));
-    p.addBefore(addBeforeHandler, null, new LengthFieldBasedFrameDecoder(Integer.MAX_VALUE, 0, 4));
-    p.addBefore(addBeforeHandler, null,
+    p.addBefore(BufferCallBeforeInitHandler.NAME, null,

Review Comment:
   The addBefore method will return a ChannelPipeline so let's chain the calls as addBefore(xxx).addBefore(xxx)



##########
hbase-common/src/main/java/org/apache/hadoop/hbase/exceptions/X509Exception.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.exceptions;
+
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * This file has been copied from the Apache ZooKeeper project.
+ * @see <a href=
+ *      "https://github.com/apache/zookeeper/blob/c74658d398cdc1d207aa296cb6e20de00faec03e/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Exception.java">Base
+ *      revision</a>
+ */
+@SuppressWarnings("serial")
+@InterfaceAudience.Private
+public class X509Exception extends Exception {
+
+  public X509Exception(String message) {
+    super(message);
+  }
+
+  public X509Exception(Throwable cause) {
+    super(cause);
+  }
+
+  public X509Exception(String message, Throwable cause) {
+    super(message, cause);
+  }
+
+  public static class KeyManagerException extends X509Exception {
+
+    public KeyManagerException(String message) {
+      super(message);
+    }
+
+    public KeyManagerException(Throwable cause) {
+      super(cause);
+    }
+
+  }
+
+  public static class TrustManagerException extends X509Exception {
+
+    public TrustManagerException(String message) {
+      super(message);
+    }
+
+    public TrustManagerException(Throwable cause) {
+      super(cause);
+    }
+
+  }
+
+  public static class SSLContextException extends X509Exception {

Review Comment:
   Ditto.



##########
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcConnection.java:
##########
@@ -90,6 +94,9 @@ class NettyRpcConnection extends RpcConnection {
   // connection in this event loop, to avoid locking everywhere.
   private final EventLoop eventLoop;
 
+  private final X509Util x509Util;

Review Comment:
   The class is named *Util but we need to instantiate it  when using...
   
   Better make it a true util, i.e, only provide static methods, or give it a better name.



##########
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcClient.java:
##########
@@ -81,4 +96,31 @@ protected void closeInternal() {
       group.shutdownGracefully();
     }
   }
+
+  private ByteBufAllocator getByteBufAllocator(Configuration conf) throws IOException {

Review Comment:
   Is this change related? If you want to port the logic in our server implementation here, please open a separated issue for it. One issue for one change.



##########
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcConnection.java:
##########
@@ -296,6 +317,32 @@ public void operationComplete(ChannelFuture future) throws Exception {
             established(ch);
           }
         }
+
+        private void fail(Channel ch, Throwable error) {
+          failInit(ch, toIOE(error));
+          rpcClient.failedServers.addToFailedServers(remoteId.getAddress(), error);
+        }
+
+        @Override
+        public void operationComplete(ChannelFuture future) throws Exception {
+          Channel ch = future.channel();
+          if (!future.isSuccess()) {
+            fail(ch, future.cause());
+            return;
+          }
+          SslHandler sslHandler = ch.pipeline().get(SslHandler.class);
+          if (sslHandler != null) {
+            sslHandler.handshakeFuture().addListener(f -> {

Review Comment:
   Please also use NettyFutureUtils.addListener



##########
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcConnection.java:
##########
@@ -217,9 +227,9 @@ private void saslNegotiate(final Channel ch) {
       failInit(ch, e);
       return;
     }
-    ch.pipeline().addFirst("SaslDecoder", new SaslChallengeDecoder()).addAfter("SaslDecoder",
-      "SaslHandler", saslHandler);
-    NettyFutureUtils.addListener(saslPromise, new FutureListener<Boolean>() {
+    ch.pipeline().addBefore(BufferCallBeforeInitHandler.NAME, null, new SaslChallengeDecoder());
+    ch.pipeline().addBefore(BufferCallBeforeInitHandler.NAME, null, saslHandler);
+    saslPromise.addListener(new FutureListener<Boolean>() {

Review Comment:
   Maybe you just copied my code so not your fault, but let's change this line back, to use NettyFutureUtils.addListener. It is used to eliminate the FutureReturnValueIgnored warning from error prone.



##########
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcConnection.java:
##########
@@ -217,9 +227,9 @@ private void saslNegotiate(final Channel ch) {
       failInit(ch, e);
       return;
     }
-    ch.pipeline().addFirst("SaslDecoder", new SaslChallengeDecoder()).addAfter("SaslDecoder",
-      "SaslHandler", saslHandler);
-    NettyFutureUtils.addListener(saslPromise, new FutureListener<Boolean>() {
+    ch.pipeline().addBefore(BufferCallBeforeInitHandler.NAME, null, new SaslChallengeDecoder());
+    ch.pipeline().addBefore(BufferCallBeforeInitHandler.NAME, null, saslHandler);

Review Comment:
   Ditto



##########
hbase-common/src/main/java/org/apache/hadoop/hbase/exceptions/X509Exception.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.exceptions;
+
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * This file has been copied from the Apache ZooKeeper project.
+ * @see <a href=
+ *      "https://github.com/apache/zookeeper/blob/c74658d398cdc1d207aa296cb6e20de00faec03e/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Exception.java">Base
+ *      revision</a>
+ */
+@SuppressWarnings("serial")
+@InterfaceAudience.Private
+public class X509Exception extends Exception {
+
+  public X509Exception(String message) {
+    super(message);
+  }
+
+  public X509Exception(Throwable cause) {
+    super(cause);
+  }
+
+  public X509Exception(String message, Throwable cause) {
+    super(message, cause);
+  }
+
+  public static class KeyManagerException extends X509Exception {
+
+    public KeyManagerException(String message) {
+      super(message);
+    }
+
+    public KeyManagerException(Throwable cause) {
+      super(cause);
+    }
+
+  }
+
+  public static class TrustManagerException extends X509Exception {

Review Comment:
   Just move it to a new file? What is the advantage to make it an inner class?



##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/X509Util.java:
##########
@@ -0,0 +1,394 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.io.crypto.tls;
+
+import java.io.BufferedInputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.security.GeneralSecurityException;
+import java.security.KeyStore;
+import java.security.Security;
+import java.security.cert.PKIXBuilderParameters;
+import java.security.cert.X509CertSelector;
+import java.util.Arrays;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.CertPathTrustManagerParameters;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLException;
+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 org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.exceptions.X509Exception;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hbase.thirdparty.io.netty.handler.ssl.SslContext;
+import org.apache.hbase.thirdparty.io.netty.handler.ssl.SslContextBuilder;
+
+/**
+ * Utility code for X509 handling Default cipher suites: Performance testing done by Facebook
+ * engineers shows that on Intel x86_64 machines, Java9 performs better with GCM and Java8 performs
+ * better with CBC, so these seem like reasonable defaults.
+ * <p/>
+ * This file has been copied from the Apache ZooKeeper project.
+ * @see <a href=
+ *      "https://github.com/apache/zookeeper/blob/c74658d398cdc1d207aa296cb6e20de00faec03e/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java">Base
+ *      revision</a>
+ */
+@InterfaceAudience.Private
+public class X509Util {
+
+  private static final Logger LOG = LoggerFactory.getLogger(X509Util.class);
+
+  // Config
+  static final String CONFIG_PREFIX = "hbase.rpc.tls.";
+  public static final String TLS_CONFIG_PROTOCOL = CONFIG_PREFIX + "protocol";
+  static final String TLS_CONFIG_KEYSTORE_LOCATION = CONFIG_PREFIX + "keystore.location";
+  static final String TLS_CONFIG_KEYSTORE_TYPE = CONFIG_PREFIX + "keystore.type";
+  static final String TLS_CONFIG_KEYSTORE_PASSWORD = CONFIG_PREFIX + "keystore.password";
+  static final String TLS_CONFIG_TRUSTSTORE_LOCATION = CONFIG_PREFIX + "truststore.location";
+  static final String TLS_CONFIG_TRUSTSTORE_TYPE = CONFIG_PREFIX + "truststore.type";
+  static final String TLS_CONFIG_TRUSTSTORE_PASSWORD = CONFIG_PREFIX + "truststore.password";
+  public static final String TLS_CONFIG_CLR = CONFIG_PREFIX + "clr";
+  public static final String TLS_CONFIG_OCSP = CONFIG_PREFIX + "ocsp";
+  private static final String TLS_ENABLED_PROTOCOLS = CONFIG_PREFIX + "enabledProtocols";
+  private static final String TLS_CIPHER_SUITES = CONFIG_PREFIX + "ciphersuites";
+
+  public static final String HBASE_CLIENT_NETTY_TLS_ENABLED = "hbase.client.netty.tls.enabled";
+  public static final String HBASE_SERVER_NETTY_TLS_ENABLED = "hbase.server.netty.tls.enabled";
+
+  public static final String HBASE_SERVER_NETTY_TLS_SUPPORTPLAINTEXT =
+    "hbase.server.netty.tls.supportplaintext";
+
+  public static final String HBASE_CLIENT_NETTY_TLS_HANDSHAKETIMEOUT =
+    "hbase.client.netty.tls.handshaketimeout";
+  public static final int DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS = 5000;
+
+  public static final String DEFAULT_PROTOCOL = "TLSv1.2";
+
+  private static String[] getGCMCiphers() {

Review Comment:
   No existing thirdparty libraries can provide these things? We need to record these in our code base?



##########
hbase-common/src/main/java/org/apache/hadoop/hbase/exceptions/X509Exception.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.exceptions;
+
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * This file has been copied from the Apache ZooKeeper project.
+ * @see <a href=
+ *      "https://github.com/apache/zookeeper/blob/c74658d398cdc1d207aa296cb6e20de00faec03e/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Exception.java">Base
+ *      revision</a>
+ */
+@SuppressWarnings("serial")
+@InterfaceAudience.Private
+public class X509Exception extends Exception {

Review Comment:
   Make it extends HBaseException? And also give it a serial id.



##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/KeyStoreFileType.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.io.crypto.tls;
+
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * This enum represents the file type of a KeyStore or TrustStore. Currently, JKS (Java keystore),
+ * PEM, PKCS12, and BCFKS types are supported.
+ * <p/>
+ * This file has been copied from the Apache ZooKeeper project.
+ * @see <a href=
+ *      "https://github.com/apache/zookeeper/blob/c74658d398cdc1d207aa296cb6e20de00faec03e/zookeeper-server/src/main/java/org/apache/zookeeper/common/KeyStoreFileType.java">Base
+ *      revision</a>
+ */
+@InterfaceAudience.Private
+public enum KeyStoreFileType {
+  JKS(".jks"),
+  PEM(".pem"),
+  PKCS12(".p12"),
+  BCFKS(".bcfks");
+
+  private final String defaultFileExtension;
+
+  KeyStoreFileType(String defaultFileExtension) {

Review Comment:
   Make it private?



##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/X509Util.java:
##########
@@ -0,0 +1,394 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.io.crypto.tls;
+
+import java.io.BufferedInputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.security.GeneralSecurityException;
+import java.security.KeyStore;
+import java.security.Security;
+import java.security.cert.PKIXBuilderParameters;
+import java.security.cert.X509CertSelector;
+import java.util.Arrays;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.CertPathTrustManagerParameters;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLException;
+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 org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.exceptions.X509Exception;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hbase.thirdparty.io.netty.handler.ssl.SslContext;
+import org.apache.hbase.thirdparty.io.netty.handler.ssl.SslContextBuilder;
+
+/**
+ * Utility code for X509 handling Default cipher suites: Performance testing done by Facebook
+ * engineers shows that on Intel x86_64 machines, Java9 performs better with GCM and Java8 performs
+ * better with CBC, so these seem like reasonable defaults.
+ * <p/>
+ * This file has been copied from the Apache ZooKeeper project.
+ * @see <a href=
+ *      "https://github.com/apache/zookeeper/blob/c74658d398cdc1d207aa296cb6e20de00faec03e/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java">Base
+ *      revision</a>
+ */
+@InterfaceAudience.Private
+public class X509Util {
+
+  private static final Logger LOG = LoggerFactory.getLogger(X509Util.class);
+
+  // Config
+  static final String CONFIG_PREFIX = "hbase.rpc.tls.";
+  public static final String TLS_CONFIG_PROTOCOL = CONFIG_PREFIX + "protocol";
+  static final String TLS_CONFIG_KEYSTORE_LOCATION = CONFIG_PREFIX + "keystore.location";
+  static final String TLS_CONFIG_KEYSTORE_TYPE = CONFIG_PREFIX + "keystore.type";
+  static final String TLS_CONFIG_KEYSTORE_PASSWORD = CONFIG_PREFIX + "keystore.password";
+  static final String TLS_CONFIG_TRUSTSTORE_LOCATION = CONFIG_PREFIX + "truststore.location";
+  static final String TLS_CONFIG_TRUSTSTORE_TYPE = CONFIG_PREFIX + "truststore.type";
+  static final String TLS_CONFIG_TRUSTSTORE_PASSWORD = CONFIG_PREFIX + "truststore.password";
+  public static final String TLS_CONFIG_CLR = CONFIG_PREFIX + "clr";
+  public static final String TLS_CONFIG_OCSP = CONFIG_PREFIX + "ocsp";
+  private static final String TLS_ENABLED_PROTOCOLS = CONFIG_PREFIX + "enabledProtocols";
+  private static final String TLS_CIPHER_SUITES = CONFIG_PREFIX + "ciphersuites";
+
+  public static final String HBASE_CLIENT_NETTY_TLS_ENABLED = "hbase.client.netty.tls.enabled";
+  public static final String HBASE_SERVER_NETTY_TLS_ENABLED = "hbase.server.netty.tls.enabled";
+
+  public static final String HBASE_SERVER_NETTY_TLS_SUPPORTPLAINTEXT =
+    "hbase.server.netty.tls.supportplaintext";
+
+  public static final String HBASE_CLIENT_NETTY_TLS_HANDSHAKETIMEOUT =
+    "hbase.client.netty.tls.handshaketimeout";
+  public static final int DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS = 5000;
+
+  public static final String DEFAULT_PROTOCOL = "TLSv1.2";
+
+  private static String[] getGCMCiphers() {
+    return new String[] { "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+      "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
+      "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384" };
+  }
+
+  private static String[] getCBCCiphers() {
+    return new String[] { "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256",
+      "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256", "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
+      "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384",
+      "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384", "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
+      "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA" };
+  }
+
+  private static String[] concatArrays(String[] left, String[] right) {
+    String[] result = new String[left.length + right.length];
+    System.arraycopy(left, 0, result, 0, left.length);
+    System.arraycopy(right, 0, result, left.length, right.length);
+    return result;
+  }
+
+  // On Java 8, prefer CBC ciphers since AES-NI support is lacking and GCM is slower than CBC.
+  private static final String[] DEFAULT_CIPHERS_JAVA8 =
+    concatArrays(getCBCCiphers(), getGCMCiphers());
+  // On Java 9 and later, prefer GCM ciphers due to improved AES-NI support.
+  // Note that this performance assumption might not hold true for architectures other than x86_64.
+  private static final String[] DEFAULT_CIPHERS_JAVA9 =
+    concatArrays(getGCMCiphers(), getCBCCiphers());
+
+  private final Configuration config;
+  private final AtomicReference<SslContext> sslContextForServer = new AtomicReference<>();
+  private final AtomicReference<SslContext> sslContextForClient = new AtomicReference<>();
+
+  public X509Util(Configuration hbaseConfig) {
+    this.config = hbaseConfig;
+  }
+
+  static String[] getDefaultCipherSuites() {
+    return getDefaultCipherSuitesForJavaVersion(System.getProperty("java.specification.version"));
+  }
+
+  static String[] getDefaultCipherSuitesForJavaVersion(String javaVersion) {
+    Objects.requireNonNull(javaVersion);
+    if (javaVersion.matches("\\d+")) {
+      // Must be Java 9 or later
+      LOG.debug("Using Java9+ optimized cipher suites for Java version {}", javaVersion);
+      return DEFAULT_CIPHERS_JAVA9;
+    } else if (javaVersion.startsWith("1.")) {
+      // Must be Java 1.8 or earlier
+      LOG.debug("Using Java8 optimized cipher suites for Java version {}", javaVersion);
+      return DEFAULT_CIPHERS_JAVA8;
+    } else {
+      LOG.debug("Could not parse java version {}, using Java8 optimized cipher suites",
+        javaVersion);
+      return DEFAULT_CIPHERS_JAVA8;
+    }
+  }
+
+  public SslContext getSslContextForClient()
+    throws X509Exception.SSLContextException, SSLException {
+    SslContext result = sslContextForClient.get();
+    if (result == null) {
+      result = createSslContextForClient();

Review Comment:
   If we use lazy creation here, then I do not think we need to throw X509Exception when constructing NettyRpcClient?



##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/X509Util.java:
##########
@@ -0,0 +1,394 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.io.crypto.tls;
+
+import java.io.BufferedInputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.security.GeneralSecurityException;
+import java.security.KeyStore;
+import java.security.Security;
+import java.security.cert.PKIXBuilderParameters;
+import java.security.cert.X509CertSelector;
+import java.util.Arrays;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.CertPathTrustManagerParameters;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLException;
+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 org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.exceptions.X509Exception;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hbase.thirdparty.io.netty.handler.ssl.SslContext;
+import org.apache.hbase.thirdparty.io.netty.handler.ssl.SslContextBuilder;
+
+/**
+ * Utility code for X509 handling Default cipher suites: Performance testing done by Facebook
+ * engineers shows that on Intel x86_64 machines, Java9 performs better with GCM and Java8 performs
+ * better with CBC, so these seem like reasonable defaults.
+ * <p/>
+ * This file has been copied from the Apache ZooKeeper project.
+ * @see <a href=
+ *      "https://github.com/apache/zookeeper/blob/c74658d398cdc1d207aa296cb6e20de00faec03e/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java">Base
+ *      revision</a>
+ */
+@InterfaceAudience.Private
+public class X509Util {
+
+  private static final Logger LOG = LoggerFactory.getLogger(X509Util.class);
+
+  // Config
+  static final String CONFIG_PREFIX = "hbase.rpc.tls.";
+  public static final String TLS_CONFIG_PROTOCOL = CONFIG_PREFIX + "protocol";
+  static final String TLS_CONFIG_KEYSTORE_LOCATION = CONFIG_PREFIX + "keystore.location";
+  static final String TLS_CONFIG_KEYSTORE_TYPE = CONFIG_PREFIX + "keystore.type";
+  static final String TLS_CONFIG_KEYSTORE_PASSWORD = CONFIG_PREFIX + "keystore.password";
+  static final String TLS_CONFIG_TRUSTSTORE_LOCATION = CONFIG_PREFIX + "truststore.location";
+  static final String TLS_CONFIG_TRUSTSTORE_TYPE = CONFIG_PREFIX + "truststore.type";
+  static final String TLS_CONFIG_TRUSTSTORE_PASSWORD = CONFIG_PREFIX + "truststore.password";
+  public static final String TLS_CONFIG_CLR = CONFIG_PREFIX + "clr";
+  public static final String TLS_CONFIG_OCSP = CONFIG_PREFIX + "ocsp";
+  private static final String TLS_ENABLED_PROTOCOLS = CONFIG_PREFIX + "enabledProtocols";
+  private static final String TLS_CIPHER_SUITES = CONFIG_PREFIX + "ciphersuites";
+
+  public static final String HBASE_CLIENT_NETTY_TLS_ENABLED = "hbase.client.netty.tls.enabled";
+  public static final String HBASE_SERVER_NETTY_TLS_ENABLED = "hbase.server.netty.tls.enabled";
+
+  public static final String HBASE_SERVER_NETTY_TLS_SUPPORTPLAINTEXT =
+    "hbase.server.netty.tls.supportplaintext";
+
+  public static final String HBASE_CLIENT_NETTY_TLS_HANDSHAKETIMEOUT =
+    "hbase.client.netty.tls.handshaketimeout";
+  public static final int DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS = 5000;
+
+  public static final String DEFAULT_PROTOCOL = "TLSv1.2";
+
+  private static String[] getGCMCiphers() {
+    return new String[] { "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+      "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
+      "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384" };
+  }
+
+  private static String[] getCBCCiphers() {
+    return new String[] { "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256",
+      "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256", "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
+      "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384",
+      "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384", "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
+      "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA" };
+  }
+
+  private static String[] concatArrays(String[] left, String[] right) {
+    String[] result = new String[left.length + right.length];
+    System.arraycopy(left, 0, result, 0, left.length);
+    System.arraycopy(right, 0, result, left.length, right.length);
+    return result;
+  }
+
+  // On Java 8, prefer CBC ciphers since AES-NI support is lacking and GCM is slower than CBC.
+  private static final String[] DEFAULT_CIPHERS_JAVA8 =
+    concatArrays(getCBCCiphers(), getGCMCiphers());
+  // On Java 9 and later, prefer GCM ciphers due to improved AES-NI support.
+  // Note that this performance assumption might not hold true for architectures other than x86_64.
+  private static final String[] DEFAULT_CIPHERS_JAVA9 =
+    concatArrays(getGCMCiphers(), getCBCCiphers());
+
+  private final Configuration config;
+  private final AtomicReference<SslContext> sslContextForServer = new AtomicReference<>();
+  private final AtomicReference<SslContext> sslContextForClient = new AtomicReference<>();
+
+  public X509Util(Configuration hbaseConfig) {
+    this.config = hbaseConfig;
+  }
+
+  static String[] getDefaultCipherSuites() {
+    return getDefaultCipherSuitesForJavaVersion(System.getProperty("java.specification.version"));
+  }
+
+  static String[] getDefaultCipherSuitesForJavaVersion(String javaVersion) {
+    Objects.requireNonNull(javaVersion);
+    if (javaVersion.matches("\\d+")) {
+      // Must be Java 9 or later
+      LOG.debug("Using Java9+ optimized cipher suites for Java version {}", javaVersion);
+      return DEFAULT_CIPHERS_JAVA9;
+    } else if (javaVersion.startsWith("1.")) {
+      // Must be Java 1.8 or earlier
+      LOG.debug("Using Java8 optimized cipher suites for Java version {}", javaVersion);
+      return DEFAULT_CIPHERS_JAVA8;
+    } else {
+      LOG.debug("Could not parse java version {}, using Java8 optimized cipher suites",
+        javaVersion);
+      return DEFAULT_CIPHERS_JAVA8;
+    }
+  }
+
+  public SslContext getSslContextForClient()
+    throws X509Exception.SSLContextException, SSLException {
+    SslContext result = sslContextForClient.get();
+    if (result == null) {
+      result = createSslContextForClient();
+      if (!sslContextForClient.compareAndSet(null, result)) {
+        // lost the race, another thread already set the value
+        result = sslContextForClient.get();
+      }
+    }
+    return result;
+  }
+
+  private SslContext createSslContextForClient()
+    throws X509Exception.SSLContextException, SSLException {
+
+    SslContextBuilder sslContextBuilder = SslContextBuilder.forClient();
+
+    String keyStoreLocation = config.get(TLS_CONFIG_KEYSTORE_LOCATION, "");
+    String keyStorePassword = config.get(TLS_CONFIG_KEYSTORE_PASSWORD, "");
+    String keyStoreType = config.get(TLS_CONFIG_KEYSTORE_TYPE, "");
+
+    if (keyStoreLocation.isEmpty()) {
+      LOG.warn("{} not specified", TLS_CONFIG_KEYSTORE_LOCATION);
+    } else {
+      try {
+        sslContextBuilder
+          .keyManager(createKeyManager(keyStoreLocation, keyStorePassword, keyStoreType));
+      } catch (X509Exception.KeyManagerException keyManagerException) {
+        throw new X509Exception.SSLContextException("Failed to create KeyManager",

Review Comment:
   What is the advantage to wrap the existing X509Exception to just change it to another X509Exception, when both of these Exceptions are defined by us?



##########
hbase-server/src/test/resources/log4j2.properties:
##########
@@ -0,0 +1,137 @@
+#/**

Review Comment:
   We have a log4j2.properties in the hbase-logging module, please modify that file if you want to do some trick in the log output for tests(but I do not think it is necessary?)



##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/X509Util.java:
##########
@@ -0,0 +1,394 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.io.crypto.tls;
+
+import java.io.BufferedInputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.security.GeneralSecurityException;
+import java.security.KeyStore;
+import java.security.Security;
+import java.security.cert.PKIXBuilderParameters;
+import java.security.cert.X509CertSelector;
+import java.util.Arrays;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.CertPathTrustManagerParameters;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLException;
+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 org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.exceptions.X509Exception;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hbase.thirdparty.io.netty.handler.ssl.SslContext;
+import org.apache.hbase.thirdparty.io.netty.handler.ssl.SslContextBuilder;
+
+/**
+ * Utility code for X509 handling Default cipher suites: Performance testing done by Facebook
+ * engineers shows that on Intel x86_64 machines, Java9 performs better with GCM and Java8 performs
+ * better with CBC, so these seem like reasonable defaults.
+ * <p/>
+ * This file has been copied from the Apache ZooKeeper project.
+ * @see <a href=
+ *      "https://github.com/apache/zookeeper/blob/c74658d398cdc1d207aa296cb6e20de00faec03e/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java">Base
+ *      revision</a>
+ */
+@InterfaceAudience.Private
+public class X509Util {
+
+  private static final Logger LOG = LoggerFactory.getLogger(X509Util.class);
+
+  // Config
+  static final String CONFIG_PREFIX = "hbase.rpc.tls.";
+  public static final String TLS_CONFIG_PROTOCOL = CONFIG_PREFIX + "protocol";
+  static final String TLS_CONFIG_KEYSTORE_LOCATION = CONFIG_PREFIX + "keystore.location";
+  static final String TLS_CONFIG_KEYSTORE_TYPE = CONFIG_PREFIX + "keystore.type";
+  static final String TLS_CONFIG_KEYSTORE_PASSWORD = CONFIG_PREFIX + "keystore.password";
+  static final String TLS_CONFIG_TRUSTSTORE_LOCATION = CONFIG_PREFIX + "truststore.location";
+  static final String TLS_CONFIG_TRUSTSTORE_TYPE = CONFIG_PREFIX + "truststore.type";
+  static final String TLS_CONFIG_TRUSTSTORE_PASSWORD = CONFIG_PREFIX + "truststore.password";
+  public static final String TLS_CONFIG_CLR = CONFIG_PREFIX + "clr";
+  public static final String TLS_CONFIG_OCSP = CONFIG_PREFIX + "ocsp";
+  private static final String TLS_ENABLED_PROTOCOLS = CONFIG_PREFIX + "enabledProtocols";
+  private static final String TLS_CIPHER_SUITES = CONFIG_PREFIX + "ciphersuites";
+
+  public static final String HBASE_CLIENT_NETTY_TLS_ENABLED = "hbase.client.netty.tls.enabled";
+  public static final String HBASE_SERVER_NETTY_TLS_ENABLED = "hbase.server.netty.tls.enabled";
+
+  public static final String HBASE_SERVER_NETTY_TLS_SUPPORTPLAINTEXT =
+    "hbase.server.netty.tls.supportplaintext";
+
+  public static final String HBASE_CLIENT_NETTY_TLS_HANDSHAKETIMEOUT =
+    "hbase.client.netty.tls.handshaketimeout";
+  public static final int DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS = 5000;
+
+  public static final String DEFAULT_PROTOCOL = "TLSv1.2";
+
+  private static String[] getGCMCiphers() {
+    return new String[] { "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+      "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
+      "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384" };
+  }
+
+  private static String[] getCBCCiphers() {
+    return new String[] { "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256",
+      "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256", "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
+      "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384",
+      "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384", "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
+      "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA" };
+  }
+
+  private static String[] concatArrays(String[] left, String[] right) {
+    String[] result = new String[left.length + right.length];
+    System.arraycopy(left, 0, result, 0, left.length);
+    System.arraycopy(right, 0, result, left.length, right.length);
+    return result;
+  }
+
+  // On Java 8, prefer CBC ciphers since AES-NI support is lacking and GCM is slower than CBC.
+  private static final String[] DEFAULT_CIPHERS_JAVA8 =
+    concatArrays(getCBCCiphers(), getGCMCiphers());
+  // On Java 9 and later, prefer GCM ciphers due to improved AES-NI support.
+  // Note that this performance assumption might not hold true for architectures other than x86_64.
+  private static final String[] DEFAULT_CIPHERS_JAVA9 =
+    concatArrays(getGCMCiphers(), getCBCCiphers());
+
+  private final Configuration config;
+  private final AtomicReference<SslContext> sslContextForServer = new AtomicReference<>();
+  private final AtomicReference<SslContext> sslContextForClient = new AtomicReference<>();
+
+  public X509Util(Configuration hbaseConfig) {
+    this.config = hbaseConfig;
+  }
+
+  static String[] getDefaultCipherSuites() {
+    return getDefaultCipherSuitesForJavaVersion(System.getProperty("java.specification.version"));
+  }
+
+  static String[] getDefaultCipherSuitesForJavaVersion(String javaVersion) {
+    Objects.requireNonNull(javaVersion);
+    if (javaVersion.matches("\\d+")) {
+      // Must be Java 9 or later
+      LOG.debug("Using Java9+ optimized cipher suites for Java version {}", javaVersion);
+      return DEFAULT_CIPHERS_JAVA9;
+    } else if (javaVersion.startsWith("1.")) {
+      // Must be Java 1.8 or earlier
+      LOG.debug("Using Java8 optimized cipher suites for Java version {}", javaVersion);
+      return DEFAULT_CIPHERS_JAVA8;
+    } else {
+      LOG.debug("Could not parse java version {}, using Java8 optimized cipher suites",
+        javaVersion);
+      return DEFAULT_CIPHERS_JAVA8;
+    }
+  }
+
+  public SslContext getSslContextForClient()
+    throws X509Exception.SSLContextException, SSLException {
+    SslContext result = sslContextForClient.get();
+    if (result == null) {
+      result = createSslContextForClient();
+      if (!sslContextForClient.compareAndSet(null, result)) {
+        // lost the race, another thread already set the value
+        result = sslContextForClient.get();
+      }
+    }
+    return result;
+  }
+
+  private SslContext createSslContextForClient()
+    throws X509Exception.SSLContextException, SSLException {
+
+    SslContextBuilder sslContextBuilder = SslContextBuilder.forClient();
+
+    String keyStoreLocation = config.get(TLS_CONFIG_KEYSTORE_LOCATION, "");
+    String keyStorePassword = config.get(TLS_CONFIG_KEYSTORE_PASSWORD, "");
+    String keyStoreType = config.get(TLS_CONFIG_KEYSTORE_TYPE, "");
+
+    if (keyStoreLocation.isEmpty()) {
+      LOG.warn("{} not specified", TLS_CONFIG_KEYSTORE_LOCATION);
+    } else {
+      try {
+        sslContextBuilder
+          .keyManager(createKeyManager(keyStoreLocation, keyStorePassword, keyStoreType));
+      } catch (X509Exception.KeyManagerException keyManagerException) {
+        throw new X509Exception.SSLContextException("Failed to create KeyManager",
+          keyManagerException);
+      }
+    }
+
+    String trustStoreLocation = config.get(TLS_CONFIG_TRUSTSTORE_LOCATION, "");
+    String trustStorePassword = config.get(TLS_CONFIG_TRUSTSTORE_PASSWORD, "");
+    String trustStoreType = config.get(TLS_CONFIG_TRUSTSTORE_TYPE, "");
+
+    boolean sslCrlEnabled = config.getBoolean(TLS_CONFIG_CLR, false);
+    boolean sslOcspEnabled = config.getBoolean(TLS_CONFIG_OCSP, false);
+
+    if (trustStoreLocation.isEmpty()) {
+      LOG.warn("{} not specified", TLS_CONFIG_TRUSTSTORE_LOCATION);

Review Comment:
   Will this cause the later sslContextBuilder.build() to fail?



##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/X509Util.java:
##########
@@ -0,0 +1,394 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.io.crypto.tls;
+
+import java.io.BufferedInputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.security.GeneralSecurityException;
+import java.security.KeyStore;
+import java.security.Security;
+import java.security.cert.PKIXBuilderParameters;
+import java.security.cert.X509CertSelector;
+import java.util.Arrays;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.CertPathTrustManagerParameters;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLException;
+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 org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.exceptions.X509Exception;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hbase.thirdparty.io.netty.handler.ssl.SslContext;
+import org.apache.hbase.thirdparty.io.netty.handler.ssl.SslContextBuilder;
+
+/**
+ * Utility code for X509 handling Default cipher suites: Performance testing done by Facebook
+ * engineers shows that on Intel x86_64 machines, Java9 performs better with GCM and Java8 performs
+ * better with CBC, so these seem like reasonable defaults.
+ * <p/>
+ * This file has been copied from the Apache ZooKeeper project.
+ * @see <a href=
+ *      "https://github.com/apache/zookeeper/blob/c74658d398cdc1d207aa296cb6e20de00faec03e/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java">Base
+ *      revision</a>
+ */
+@InterfaceAudience.Private
+public class X509Util {
+
+  private static final Logger LOG = LoggerFactory.getLogger(X509Util.class);
+
+  // Config
+  static final String CONFIG_PREFIX = "hbase.rpc.tls.";
+  public static final String TLS_CONFIG_PROTOCOL = CONFIG_PREFIX + "protocol";
+  static final String TLS_CONFIG_KEYSTORE_LOCATION = CONFIG_PREFIX + "keystore.location";
+  static final String TLS_CONFIG_KEYSTORE_TYPE = CONFIG_PREFIX + "keystore.type";
+  static final String TLS_CONFIG_KEYSTORE_PASSWORD = CONFIG_PREFIX + "keystore.password";
+  static final String TLS_CONFIG_TRUSTSTORE_LOCATION = CONFIG_PREFIX + "truststore.location";
+  static final String TLS_CONFIG_TRUSTSTORE_TYPE = CONFIG_PREFIX + "truststore.type";
+  static final String TLS_CONFIG_TRUSTSTORE_PASSWORD = CONFIG_PREFIX + "truststore.password";
+  public static final String TLS_CONFIG_CLR = CONFIG_PREFIX + "clr";
+  public static final String TLS_CONFIG_OCSP = CONFIG_PREFIX + "ocsp";
+  private static final String TLS_ENABLED_PROTOCOLS = CONFIG_PREFIX + "enabledProtocols";
+  private static final String TLS_CIPHER_SUITES = CONFIG_PREFIX + "ciphersuites";
+
+  public static final String HBASE_CLIENT_NETTY_TLS_ENABLED = "hbase.client.netty.tls.enabled";
+  public static final String HBASE_SERVER_NETTY_TLS_ENABLED = "hbase.server.netty.tls.enabled";
+
+  public static final String HBASE_SERVER_NETTY_TLS_SUPPORTPLAINTEXT =
+    "hbase.server.netty.tls.supportplaintext";
+
+  public static final String HBASE_CLIENT_NETTY_TLS_HANDSHAKETIMEOUT =
+    "hbase.client.netty.tls.handshaketimeout";
+  public static final int DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS = 5000;
+
+  public static final String DEFAULT_PROTOCOL = "TLSv1.2";
+
+  private static String[] getGCMCiphers() {
+    return new String[] { "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+      "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
+      "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384" };
+  }
+
+  private static String[] getCBCCiphers() {
+    return new String[] { "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256",
+      "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256", "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
+      "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384",
+      "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384", "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
+      "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA" };
+  }
+
+  private static String[] concatArrays(String[] left, String[] right) {
+    String[] result = new String[left.length + right.length];
+    System.arraycopy(left, 0, result, 0, left.length);
+    System.arraycopy(right, 0, result, left.length, right.length);
+    return result;
+  }
+
+  // On Java 8, prefer CBC ciphers since AES-NI support is lacking and GCM is slower than CBC.
+  private static final String[] DEFAULT_CIPHERS_JAVA8 =
+    concatArrays(getCBCCiphers(), getGCMCiphers());
+  // On Java 9 and later, prefer GCM ciphers due to improved AES-NI support.
+  // Note that this performance assumption might not hold true for architectures other than x86_64.
+  private static final String[] DEFAULT_CIPHERS_JAVA9 =
+    concatArrays(getGCMCiphers(), getCBCCiphers());
+
+  private final Configuration config;
+  private final AtomicReference<SslContext> sslContextForServer = new AtomicReference<>();
+  private final AtomicReference<SslContext> sslContextForClient = new AtomicReference<>();
+
+  public X509Util(Configuration hbaseConfig) {
+    this.config = hbaseConfig;
+  }
+
+  static String[] getDefaultCipherSuites() {
+    return getDefaultCipherSuitesForJavaVersion(System.getProperty("java.specification.version"));
+  }
+
+  static String[] getDefaultCipherSuitesForJavaVersion(String javaVersion) {
+    Objects.requireNonNull(javaVersion);
+    if (javaVersion.matches("\\d+")) {
+      // Must be Java 9 or later
+      LOG.debug("Using Java9+ optimized cipher suites for Java version {}", javaVersion);
+      return DEFAULT_CIPHERS_JAVA9;
+    } else if (javaVersion.startsWith("1.")) {
+      // Must be Java 1.8 or earlier
+      LOG.debug("Using Java8 optimized cipher suites for Java version {}", javaVersion);
+      return DEFAULT_CIPHERS_JAVA8;
+    } else {
+      LOG.debug("Could not parse java version {}, using Java8 optimized cipher suites",
+        javaVersion);
+      return DEFAULT_CIPHERS_JAVA8;
+    }
+  }
+
+  public SslContext getSslContextForClient()
+    throws X509Exception.SSLContextException, SSLException {
+    SslContext result = sslContextForClient.get();
+    if (result == null) {
+      result = createSslContextForClient();
+      if (!sslContextForClient.compareAndSet(null, result)) {
+        // lost the race, another thread already set the value
+        result = sslContextForClient.get();
+      }
+    }
+    return result;
+  }
+
+  private SslContext createSslContextForClient()
+    throws X509Exception.SSLContextException, SSLException {
+
+    SslContextBuilder sslContextBuilder = SslContextBuilder.forClient();
+
+    String keyStoreLocation = config.get(TLS_CONFIG_KEYSTORE_LOCATION, "");
+    String keyStorePassword = config.get(TLS_CONFIG_KEYSTORE_PASSWORD, "");
+    String keyStoreType = config.get(TLS_CONFIG_KEYSTORE_TYPE, "");
+
+    if (keyStoreLocation.isEmpty()) {
+      LOG.warn("{} not specified", TLS_CONFIG_KEYSTORE_LOCATION);

Review Comment:
   TLS_CONFIG_KEYSTORE_LOCATION is a constant? Then just concat it? It will be convert to a String literal at compile time so no performance issue.



##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/X509Util.java:
##########
@@ -0,0 +1,394 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.io.crypto.tls;
+
+import java.io.BufferedInputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.security.GeneralSecurityException;
+import java.security.KeyStore;
+import java.security.Security;
+import java.security.cert.PKIXBuilderParameters;
+import java.security.cert.X509CertSelector;
+import java.util.Arrays;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.net.ssl.CertPathTrustManagerParameters;
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLException;
+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 org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.exceptions.X509Exception;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hbase.thirdparty.io.netty.handler.ssl.SslContext;
+import org.apache.hbase.thirdparty.io.netty.handler.ssl.SslContextBuilder;
+
+/**
+ * Utility code for X509 handling Default cipher suites: Performance testing done by Facebook
+ * engineers shows that on Intel x86_64 machines, Java9 performs better with GCM and Java8 performs
+ * better with CBC, so these seem like reasonable defaults.
+ * <p/>
+ * This file has been copied from the Apache ZooKeeper project.
+ * @see <a href=
+ *      "https://github.com/apache/zookeeper/blob/c74658d398cdc1d207aa296cb6e20de00faec03e/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java">Base
+ *      revision</a>
+ */
+@InterfaceAudience.Private
+public class X509Util {
+
+  private static final Logger LOG = LoggerFactory.getLogger(X509Util.class);
+
+  // Config
+  static final String CONFIG_PREFIX = "hbase.rpc.tls.";
+  public static final String TLS_CONFIG_PROTOCOL = CONFIG_PREFIX + "protocol";
+  static final String TLS_CONFIG_KEYSTORE_LOCATION = CONFIG_PREFIX + "keystore.location";
+  static final String TLS_CONFIG_KEYSTORE_TYPE = CONFIG_PREFIX + "keystore.type";
+  static final String TLS_CONFIG_KEYSTORE_PASSWORD = CONFIG_PREFIX + "keystore.password";
+  static final String TLS_CONFIG_TRUSTSTORE_LOCATION = CONFIG_PREFIX + "truststore.location";
+  static final String TLS_CONFIG_TRUSTSTORE_TYPE = CONFIG_PREFIX + "truststore.type";
+  static final String TLS_CONFIG_TRUSTSTORE_PASSWORD = CONFIG_PREFIX + "truststore.password";
+  public static final String TLS_CONFIG_CLR = CONFIG_PREFIX + "clr";
+  public static final String TLS_CONFIG_OCSP = CONFIG_PREFIX + "ocsp";
+  private static final String TLS_ENABLED_PROTOCOLS = CONFIG_PREFIX + "enabledProtocols";
+  private static final String TLS_CIPHER_SUITES = CONFIG_PREFIX + "ciphersuites";
+
+  public static final String HBASE_CLIENT_NETTY_TLS_ENABLED = "hbase.client.netty.tls.enabled";
+  public static final String HBASE_SERVER_NETTY_TLS_ENABLED = "hbase.server.netty.tls.enabled";
+
+  public static final String HBASE_SERVER_NETTY_TLS_SUPPORTPLAINTEXT =
+    "hbase.server.netty.tls.supportplaintext";
+
+  public static final String HBASE_CLIENT_NETTY_TLS_HANDSHAKETIMEOUT =
+    "hbase.client.netty.tls.handshaketimeout";
+  public static final int DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS = 5000;
+
+  public static final String DEFAULT_PROTOCOL = "TLSv1.2";
+
+  private static String[] getGCMCiphers() {
+    return new String[] { "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
+      "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
+      "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384" };
+  }
+
+  private static String[] getCBCCiphers() {
+    return new String[] { "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256",
+      "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256", "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
+      "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384",
+      "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384", "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
+      "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA" };
+  }
+
+  private static String[] concatArrays(String[] left, String[] right) {
+    String[] result = new String[left.length + right.length];
+    System.arraycopy(left, 0, result, 0, left.length);
+    System.arraycopy(right, 0, result, left.length, right.length);
+    return result;
+  }
+
+  // On Java 8, prefer CBC ciphers since AES-NI support is lacking and GCM is slower than CBC.
+  private static final String[] DEFAULT_CIPHERS_JAVA8 =
+    concatArrays(getCBCCiphers(), getGCMCiphers());
+  // On Java 9 and later, prefer GCM ciphers due to improved AES-NI support.
+  // Note that this performance assumption might not hold true for architectures other than x86_64.
+  private static final String[] DEFAULT_CIPHERS_JAVA9 =
+    concatArrays(getGCMCiphers(), getCBCCiphers());
+
+  private final Configuration config;

Review Comment:
   So the only shared thing here is the Configuration? Then I think we could just make all the methods in this class static, just let the caller pass in a Configuration, and move the AtomicReference to caller classes, so we can make this class a real 'util'.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org