diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/HBaseTrustManager.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/HBaseTrustManager.java index ca4756a6131c..15beb5f60bc9 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/HBaseTrustManager.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/HBaseTrustManager.java @@ -92,8 +92,13 @@ public void checkServerTrusted(X509Certificate[] chain, String authType, Socket public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) throws CertificateException { x509ExtendedTrustManager.checkClientTrusted(chain, authType, engine); - if (hostnameVerificationEnabled) { + if (hostnameVerificationEnabled && engine != null) { try { + if (engine.getPeerHost() == null) { + LOG.warn( + "Cannot perform client hostname verification, because peer information is not available"); + return; + } performHostVerification(InetAddress.getByName(engine.getPeerHost()), chain[0]); } catch (UnknownHostException e) { throw new CertificateException("Failed to verify host", e); diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/tls/TestHBaseTrustManager.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/tls/TestHBaseTrustManager.java index 07fc87e01354..cfe197ae8d37 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/tls/TestHBaseTrustManager.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/tls/TestHBaseTrustManager.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.io.crypto.tls; import static org.junit.Assert.assertThrows; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -36,6 +37,7 @@ import java.util.Date; import java.util.List; import java.util.Random; +import javax.net.ssl.SSLEngine; import javax.net.ssl.X509ExtendedTrustManager; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.testclassification.MiscTests; @@ -86,6 +88,8 @@ public class TestHBaseTrustManager { private InetAddress mockInetAddressWithHostname; private Socket mockSocketWithoutHostname; private Socket mockSocketWithHostname; + private SSLEngine mockSSLEngineWithoutPeerhost; + private SSLEngine mockSSLEngineWithPeerhost; @BeforeClass public static void createKeyPair() throws Exception { @@ -126,6 +130,12 @@ public void setup() throws Exception { mockSocketWithHostname = mock(Socket.class); when(mockSocketWithHostname.getInetAddress()) .thenAnswer((Answer) invocationOnMock -> mockInetAddressWithHostname); + + mockSSLEngineWithoutPeerhost = mock(SSLEngine.class); + doReturn(null).when(mockSSLEngineWithoutPeerhost).getPeerHost(); + + mockSSLEngineWithPeerhost = mock(SSLEngine.class); + doReturn(IP_ADDRESS).when(mockSSLEngineWithPeerhost).getPeerHost(); } @SuppressWarnings("JavaUtilDate") @@ -352,4 +362,35 @@ public void testClientTrustedWithHostnameVerificationEnabledWithHostnameNoRevers mockSocketWithHostname); } + @Test + public void testClientTrustedSslEngineWithPeerHostReverseLookup() throws Exception { + HBaseTrustManager trustManager = + new HBaseTrustManager(mockX509ExtendedTrustManager, true, true); + X509Certificate[] certificateChain = createSelfSignedCertificateChain(null, HOSTNAME); + trustManager.checkClientTrusted(certificateChain, null, mockSSLEngineWithPeerhost); + } + + @Test(expected = CertificateException.class) + public void testClientTrustedSslEngineWithPeerHostNoReverseLookup() throws Exception { + HBaseTrustManager trustManager = + new HBaseTrustManager(mockX509ExtendedTrustManager, true, false); + X509Certificate[] certificateChain = createSelfSignedCertificateChain(null, HOSTNAME); + trustManager.checkClientTrusted(certificateChain, null, mockSSLEngineWithPeerhost); + } + + @Test + public void testClientTrustedSslEngineWithoutPeerHost() throws Exception { + HBaseTrustManager trustManager = + new HBaseTrustManager(mockX509ExtendedTrustManager, true, false); + X509Certificate[] certificateChain = createSelfSignedCertificateChain(null, HOSTNAME); + trustManager.checkClientTrusted(certificateChain, null, mockSSLEngineWithoutPeerhost); + } + + @Test + public void testClientTrustedSslEngineNotAvailable() throws Exception { + HBaseTrustManager trustManager = + new HBaseTrustManager(mockX509ExtendedTrustManager, true, false); + X509Certificate[] certificateChain = createSelfSignedCertificateChain(null, HOSTNAME); + trustManager.checkClientTrusted(certificateChain, null, (SSLEngine) null); + } } diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/tls/X509TestContext.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/tls/X509TestContext.java index ad4ffe0ab5a4..86f9818f523d 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/tls/X509TestContext.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/tls/X509TestContext.java @@ -461,12 +461,13 @@ public X509TestContext cloneWithNewKeystoreCert(X509Certificate cert) { } public void regenerateStores(X509KeyType keyStoreKeyType, X509KeyType trustStoreKeyType, - KeyStoreFileType keyStoreFileType, KeyStoreFileType trustStoreFileType) + KeyStoreFileType keyStoreFileType, KeyStoreFileType trustStoreFileType, + String... subjectAltNames) throws GeneralSecurityException, IOException, OperatorCreationException { trustStoreKeyPair = X509TestHelpers.generateKeyPair(trustStoreKeyType); keyStoreKeyPair = X509TestHelpers.generateKeyPair(keyStoreKeyType); - createCertificates(); + createCertificates(subjectAltNames); switch (keyStoreFileType) { case JKS: @@ -499,7 +500,7 @@ public void regenerateStores(X509KeyType keyStoreKeyType, X509KeyType trustStore } } - private void createCertificates() + private void createCertificates(String... subjectAltNames) throws GeneralSecurityException, IOException, OperatorCreationException { X500NameBuilder caNameBuilder = new X500NameBuilder(BCStyle.INSTANCE); caNameBuilder.addRDN(BCStyle.CN, @@ -510,7 +511,7 @@ private void createCertificates() X500NameBuilder nameBuilder = new X500NameBuilder(BCStyle.INSTANCE); nameBuilder.addRDN(BCStyle.CN, MethodHandles.lookup().lookupClass().getCanonicalName() + " Zookeeper Test"); - keyStoreCertificate = newCert(nameBuilder.build()); + keyStoreCertificate = newCert(nameBuilder.build(), subjectAltNames); } /** diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcServer.java index 1d93fbd0f668..b21b6e19c78e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcServer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcServer.java @@ -21,7 +21,6 @@ import static org.apache.hadoop.hbase.io.crypto.tls.X509Util.HBASE_SERVER_NETTY_TLS_ENABLED; import static org.apache.hadoop.hbase.io.crypto.tls.X509Util.HBASE_SERVER_NETTY_TLS_SUPPORTPLAINTEXT; import static org.apache.hadoop.hbase.io.crypto.tls.X509Util.HBASE_SERVER_NETTY_TLS_WRAP_SIZE; -import static org.apache.hadoop.hbase.io.crypto.tls.X509Util.TLS_CONFIG_REVERSE_DNS_LOOKUP_ENABLED; import java.io.IOException; import java.io.InterruptedIOException; @@ -386,8 +385,24 @@ private void initSSL(ChannelPipeline p, NettyServerRpcConnection conn, boolean s throws X509Exception, IOException { SslContext nettySslContext = getSslContext(); + /* + * our HostnameVerifier gets the host name from SSLEngine, so we have to construct the engine + * properly by passing the remote address + */ + if (supportPlaintext) { - p.addLast("ssl", new OptionalSslHandler(nettySslContext)); + SocketAddress remoteAddress = p.channel().remoteAddress(); + OptionalSslHandler optionalSslHandler; + + if (remoteAddress instanceof InetSocketAddress) { + InetSocketAddress remoteInetAddress = (InetSocketAddress) remoteAddress; + optionalSslHandler = new OptionalSslHandlerWithHostPort(nettySslContext, + remoteInetAddress.getHostString(), remoteInetAddress.getPort()); + } else { + optionalSslHandler = new OptionalSslHandler(nettySslContext); + } + + p.addLast("ssl", optionalSslHandler); LOG.debug("Dual mode SSL handler added for channel: {}", p.channel()); } else { SocketAddress remoteAddress = p.channel().remoteAddress(); @@ -395,21 +410,8 @@ private void initSSL(ChannelPipeline p, NettyServerRpcConnection conn, boolean s if (remoteAddress instanceof InetSocketAddress) { InetSocketAddress remoteInetAddress = (InetSocketAddress) remoteAddress; - String host; - - if (conf.getBoolean(TLS_CONFIG_REVERSE_DNS_LOOKUP_ENABLED, true)) { - host = remoteInetAddress.getHostName(); - } else { - host = remoteInetAddress.getHostString(); - } - - int port = remoteInetAddress.getPort(); - - /* - * our HostnameVerifier gets the host name from SSLEngine, so we have to construct the - * engine properly by passing the remote address - */ - sslHandler = nettySslContext.newHandler(p.channel().alloc(), host, port); + sslHandler = nettySslContext.newHandler(p.channel().alloc(), + remoteInetAddress.getHostString(), remoteInetAddress.getPort()); } else { sslHandler = nettySslContext.newHandler(p.channel().alloc()); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/OptionalSslHandlerWithHostPort.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/OptionalSslHandlerWithHostPort.java new file mode 100644 index 000000000000..d349f1c6c783 --- /dev/null +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/OptionalSslHandlerWithHostPort.java @@ -0,0 +1,40 @@ +/* + * 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.ipc; + +import org.apache.hbase.thirdparty.io.netty.channel.ChannelHandlerContext; +import org.apache.hbase.thirdparty.io.netty.handler.ssl.OptionalSslHandler; +import org.apache.hbase.thirdparty.io.netty.handler.ssl.SslContext; +import org.apache.hbase.thirdparty.io.netty.handler.ssl.SslHandler; + +class OptionalSslHandlerWithHostPort extends OptionalSslHandler { + + private final String host; + private final int port; + + public OptionalSslHandlerWithHostPort(SslContext sslContext, String host, int port) { + super(sslContext); + this.host = host; + this.port = port; + } + + @Override + protected SslHandler newSslHandler(ChannelHandlerContext context, SslContext sslContext) { + return sslContext.newHandler(context.alloc(), host, port); + } +} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/TestMutualTlsClientSideNonLocalhost.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/TestMutualTlsClientSideNonLocalhost.java new file mode 100644 index 000000000000..f22d3f086470 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/TestMutualTlsClientSideNonLocalhost.java @@ -0,0 +1,178 @@ +/* + * 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.security; + +import static org.apache.hadoop.hbase.ipc.TestProtobufRpcServiceImpl.SERVICE; + +import java.io.File; +import java.io.IOException; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.security.GeneralSecurityException; +import java.security.Security; +import java.util.List; +import org.apache.commons.io.FileUtils; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HBaseCommonTestingUtil; +import org.apache.hadoop.hbase.io.crypto.tls.KeyStoreFileType; +import org.apache.hadoop.hbase.io.crypto.tls.X509KeyType; +import org.apache.hadoop.hbase.io.crypto.tls.X509TestContext; +import org.apache.hadoop.hbase.io.crypto.tls.X509TestContextProvider; +import org.apache.hadoop.hbase.io.crypto.tls.X509Util; +import org.apache.hadoop.hbase.ipc.FifoRpcScheduler; +import org.apache.hadoop.hbase.ipc.NettyRpcClient; +import org.apache.hadoop.hbase.ipc.NettyRpcServer; +import org.apache.hadoop.hbase.ipc.RpcClient; +import org.apache.hadoop.hbase.ipc.RpcClientFactory; +import org.apache.hadoop.hbase.ipc.RpcServer; +import org.apache.hadoop.hbase.ipc.RpcServerFactory; +import org.apache.hadoop.hbase.ipc.TestProtobufRpcServiceImpl; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.apache.hadoop.hbase.testclassification.RPCTests; +import org.bouncycastle.jce.provider.BouncyCastleProvider; +import org.bouncycastle.operator.OperatorCreationException; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import org.apache.hbase.thirdparty.com.google.common.collect.Lists; +import org.apache.hbase.thirdparty.com.google.common.io.Closeables; + +import org.apache.hadoop.hbase.shaded.ipc.protobuf.generated.TestProtos; +import org.apache.hadoop.hbase.shaded.ipc.protobuf.generated.TestRpcServiceProtos; + +/** + * Tests for client-side mTLS focusing on client hostname verification in the case when client and + * server are on different hosts. We try to simulate this behaviour by querying the hostname with + *

+ * InetAddress.getLocalHost() + *

+ * Certificates are generated with the hostname in Subject Alternative Names, server binds + * non-localhost interface and client connects via remote IP address. Parameter is set to verify + * both TLS/plaintext and TLS-only cases. + */ +@RunWith(Parameterized.class) +@Category({ RPCTests.class, MediumTests.class }) +public class TestMutualTlsClientSideNonLocalhost { + + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestMutualTlsClientSideNonLocalhost.class); + + protected static HBaseCommonTestingUtil UTIL; + + protected static File DIR; + + protected static X509TestContextProvider PROVIDER; + + private X509TestContext x509TestContext; + + protected RpcServer rpcServer; + + protected RpcClient rpcClient; + private TestRpcServiceProtos.TestProtobufRpcProto.BlockingInterface stub; + + @Parameterized.Parameter(0) + public boolean supportPlaintext; + + @Parameterized.Parameters(name = "{index}: supportPlaintext={0}") + public static List data() { + return List.of(true, false); + } + + @BeforeClass + public static void setUpBeforeClass() throws IOException { + UTIL = new HBaseCommonTestingUtil(); + Security.addProvider(new BouncyCastleProvider()); + DIR = + new File(UTIL.getDataTestDir(AbstractTestTlsRejectPlainText.class.getSimpleName()).toString()) + .getCanonicalFile(); + FileUtils.forceMkdir(DIR); + Configuration conf = UTIL.getConfiguration(); + conf.setClass(RpcClientFactory.CUSTOM_RPC_CLIENT_IMPL_CONF_KEY, NettyRpcClient.class, + RpcClient.class); + conf.setClass(RpcServerFactory.CUSTOM_RPC_SERVER_IMPL_CONF_KEY, NettyRpcServer.class, + RpcServer.class); + conf.setBoolean(X509Util.HBASE_SERVER_NETTY_TLS_ENABLED, true); + conf.setBoolean(X509Util.HBASE_CLIENT_NETTY_TLS_ENABLED, true); + PROVIDER = new X509TestContextProvider(conf, DIR); + } + + @AfterClass + public static void cleanUp() { + Security.removeProvider(BouncyCastleProvider.PROVIDER_NAME); + UTIL.cleanupTestDir(); + } + + @Before + public void setUp() throws Exception { + x509TestContext = PROVIDER.get(X509KeyType.RSA, X509KeyType.RSA, "keyPassword".toCharArray()); + x509TestContext.setConfigurations(KeyStoreFileType.JKS, KeyStoreFileType.JKS); + + Configuration serverConf = new Configuration(UTIL.getConfiguration()); + Configuration clientConf = new Configuration(UTIL.getConfiguration()); + + initialize(serverConf, clientConf); + + InetSocketAddress isa = new InetSocketAddress(InetAddress.getLocalHost(), 0); + + rpcServer = new NettyRpcServer(null, "testRpcServer", + Lists.newArrayList(new RpcServer.BlockingServiceAndInterface(SERVICE, null)), isa, serverConf, + new FifoRpcScheduler(serverConf, 1), true); + rpcServer.start(); + + rpcClient = new NettyRpcClient(clientConf); + stub = TestProtobufRpcServiceImpl.newBlockingStub(rpcClient, rpcServer.getListenerAddress()); + } + + private void initialize(Configuration serverConf, Configuration clientConf) + throws GeneralSecurityException, IOException, OperatorCreationException { + serverConf.setBoolean(X509Util.HBASE_SERVER_NETTY_TLS_SUPPORTPLAINTEXT, supportPlaintext); + clientConf.setBoolean(X509Util.HBASE_CLIENT_NETTY_TLS_VERIFY_SERVER_HOSTNAME, true); + x509TestContext.regenerateStores(X509KeyType.RSA, X509KeyType.RSA, KeyStoreFileType.JKS, + KeyStoreFileType.JKS, InetAddress.getLocalHost().getHostName()); + } + + @After + public void tearDown() throws IOException { + if (rpcServer != null) { + rpcServer.stop(); + } + Closeables.close(rpcClient, true); + x509TestContext.clearConfigurations(); + x509TestContext.getConf().unset(X509Util.TLS_CONFIG_OCSP); + x509TestContext.getConf().unset(X509Util.TLS_CONFIG_CLR); + x509TestContext.getConf().unset(X509Util.TLS_CONFIG_PROTOCOL); + System.clearProperty("com.sun.net.ssl.checkRevocation"); + System.clearProperty("com.sun.security.enableCRLDP"); + Security.setProperty("ocsp.enable", Boolean.FALSE.toString()); + Security.setProperty("com.sun.security.enableCRLDP", Boolean.FALSE.toString()); + } + + @Test + public void testClientAuth() throws Exception { + stub.echo(null, TestProtos.EchoRequestProto.newBuilder().setMessage("hello world").build()); + } +}