From aea396619543cdfad8f9d5cb19d763a52d8a4634 Mon Sep 17 00:00:00 2001 From: Charles Connell Date: Wed, 17 Jan 2024 11:22:52 -0500 Subject: [PATCH 1/3] Expose client TLS certificate on RpcCallContext --- .../hadoop/hbase/ipc/NettyRpcServer.java | 21 ++++++++++++++++--- .../hadoop/hbase/ipc/RpcCallContext.java | 8 +++++++ .../apache/hadoop/hbase/ipc/ServerCall.java | 9 ++++++++ .../hadoop/hbase/ipc/ServerRpcConnection.java | 3 +++ .../namequeues/TestNamedQueueRecorder.java | 6 ++++++ .../hbase/namequeues/TestRpcLogDetails.java | 6 ++++++ .../region/TestRegionProcedureStore.java | 6 ++++++ 7 files changed, 56 insertions(+), 3 deletions(-) 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 ceff84a90e11..c831c59e7d31 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 @@ -27,6 +27,8 @@ import java.io.InterruptedIOException; import java.net.InetSocketAddress; import java.net.SocketAddress; +import java.security.cert.Certificate; +import java.security.cert.X509Certificate; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicReference; @@ -166,10 +168,10 @@ protected void initChannel(Channel ch) throws Exception { ChannelPipeline pipeline = ch.pipeline(); FixedLengthFrameDecoder preambleDecoder = new FixedLengthFrameDecoder(6); preambleDecoder.setSingleDecode(true); + NettyServerRpcConnection conn = createNettyServerRpcConnection(ch); if (conf.getBoolean(HBASE_SERVER_NETTY_TLS_ENABLED, false)) { - initSSL(pipeline, conf.getBoolean(HBASE_SERVER_NETTY_TLS_SUPPORTPLAINTEXT, true)); + initSSL(pipeline, conn, conf.getBoolean(HBASE_SERVER_NETTY_TLS_SUPPORTPLAINTEXT, true)); } - NettyServerRpcConnection conn = createNettyServerRpcConnection(ch); pipeline.addLast(NettyRpcServerPreambleHandler.DECODER_NAME, preambleDecoder) .addLast(new NettyRpcServerPreambleHandler(NettyRpcServer.this, conn)) // We need NettyRpcServerResponseEncoder here because NettyRpcServerPreambleHandler may @@ -378,7 +380,7 @@ public int getNumOpenConnections() { return allChannels.size(); } - private void initSSL(ChannelPipeline p, boolean supportPlaintext) + private void initSSL(ChannelPipeline p, NettyServerRpcConnection conn, boolean supportPlaintext) throws X509Exception, IOException { SslContext nettySslContext = getSslContext(); @@ -413,6 +415,19 @@ private void initSSL(ChannelPipeline p, boolean supportPlaintext) sslHandler.setWrapDataSize( conf.getInt(HBASE_SERVER_NETTY_TLS_WRAP_SIZE, DEFAULT_HBASE_SERVER_NETTY_TLS_WRAP_SIZE)); + sslHandler.handshakeFuture().addListener(future -> { + try { + Certificate[] certificates = sslHandler.engine().getSession().getPeerCertificates(); + if (certificates.length > 0) { + conn.clientCertificate = (X509Certificate) certificates[0]; + } else { + LOG.debug("No client certificate found for peer {}", remoteAddress); + } + } catch (Exception e) { + LOG.debug("Failure getting peer certificate for {}", remoteAddress, e); + } + }); + p.addLast("ssl", sslHandler); LOG.debug("SSL handler added for channel: {}", p.channel()); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcCallContext.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcCallContext.java index 4f299b4a85d2..8d8374f7321e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcCallContext.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcCallContext.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.ipc; import java.net.InetAddress; +import java.security.cert.X509Certificate; import java.util.Optional; import org.apache.hadoop.hbase.security.User; import org.apache.yetus.audience.InterfaceAudience; @@ -60,6 +61,13 @@ default Optional getRequestUserName() { return getRequestUser().map(User::getShortName); } + /** + * Returns the TLS certificate that the client presented to this HBase server when making its + * connection. TLS is orthogonal to Kerberos, so this is unrelated to + * {@link this#getRequestUser()}. Both, one, or neither, may be present. + */ + Optional getClientCertificate(); + /** Returns Address of remote client in this call */ InetAddress getRemoteAddress(); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java index ed688977b963..6df67ac0748a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java @@ -23,6 +23,7 @@ import java.io.IOException; import java.net.InetAddress; import java.nio.ByteBuffer; +import java.security.cert.X509Certificate; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -95,6 +96,7 @@ public abstract class ServerCall implements RpcCa protected final User user; protected final InetAddress remoteAddress; + protected final X509Certificate clientCertificate; protected RpcCallback rpcCallback; private long responseCellSize = 0; @@ -134,9 +136,11 @@ public abstract class ServerCall implements RpcCa if (connection != null) { this.user = connection.user; this.retryImmediatelySupported = connection.retryImmediatelySupported; + this.clientCertificate = connection.clientCertificate; } else { this.user = null; this.retryImmediatelySupported = false; + this.clientCertificate = null; } this.remoteAddress = remoteAddress; this.timeout = timeout; @@ -498,6 +502,11 @@ public Optional getRequestUser() { return Optional.ofNullable(user); } + @Override + public Optional getClientCertificate() { + return Optional.ofNullable(clientCertificate); + } + @Override public InetAddress getRemoteAddress() { return remoteAddress; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerRpcConnection.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerRpcConnection.java index 695f1e7050c4..b9bf66a64f98 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerRpcConnection.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerRpcConnection.java @@ -31,6 +31,7 @@ import java.net.InetSocketAddress; import java.nio.ByteBuffer; import java.security.GeneralSecurityException; +import java.security.cert.X509Certificate; import java.util.Collections; import java.util.Map; import java.util.Objects; @@ -133,6 +134,8 @@ abstract class ServerRpcConnection implements Closeable { protected User user = null; protected UserGroupInformation ugi = null; protected SaslServerAuthenticationProviders saslProviders = null; + // volatile because this gets set after this object is constructed, when TLS handshake finishes + protected volatile X509Certificate clientCertificate = null; public ServerRpcConnection(RpcServer rpcServer) { this.rpcServer = rpcServer; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestNamedQueueRecorder.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestNamedQueueRecorder.java index 35a1757115c9..742a61325e1d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestNamedQueueRecorder.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestNamedQueueRecorder.java @@ -22,6 +22,7 @@ import java.net.InetAddress; import java.security.PrivilegedAction; import java.security.PrivilegedExceptionAction; +import java.security.cert.X509Certificate; import java.util.Collections; import java.util.List; import java.util.Map; @@ -814,6 +815,11 @@ public Optional getRequestUser() { return getUser(userName); } + @Override + public Optional getClientCertificate() { + return Optional.empty(); + } + @Override public InetAddress getRemoteAddress() { return null; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestRpcLogDetails.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestRpcLogDetails.java index 8a93f2d0ff54..86624d2aed15 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestRpcLogDetails.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestRpcLogDetails.java @@ -24,6 +24,7 @@ import java.io.IOException; import java.net.InetAddress; import java.nio.ByteBuffer; +import java.security.cert.X509Certificate; import java.util.Arrays; import java.util.Collections; import java.util.Map; @@ -213,6 +214,11 @@ public Optional getRequestUser() { return null; } + @Override + public Optional getClientCertificate() { + return Optional.empty(); + } + @Override public InetAddress getRemoteAddress() { return null; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure2/store/region/TestRegionProcedureStore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure2/store/region/TestRegionProcedureStore.java index 305f0e29e952..fb98940a61d5 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure2/store/region/TestRegionProcedureStore.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure2/store/region/TestRegionProcedureStore.java @@ -23,6 +23,7 @@ import java.io.IOException; import java.net.InetAddress; +import java.security.cert.X509Certificate; import java.util.HashSet; import java.util.Map; import java.util.Optional; @@ -275,6 +276,11 @@ public Optional getRequestUser() { return Optional.empty(); } + @Override + public Optional getClientCertificate() { + return Optional.empty(); + } + @Override public InetAddress getRemoteAddress() { return null; From e956467455a0dc7f633daeed71c6587a6ae25dca Mon Sep 17 00:00:00 2001 From: Charles Connell Date: Wed, 24 Jan 2024 13:58:25 -0500 Subject: [PATCH 2/3] changes from PR feedback --- .../hadoop/hbase/ipc/NettyRpcServer.java | 20 ++++++++++++++----- .../hadoop/hbase/ipc/RpcCallContext.java | 6 +++--- .../apache/hadoop/hbase/ipc/ServerCall.java | 10 +++++----- .../hadoop/hbase/ipc/ServerRpcConnection.java | 3 +-- .../namequeues/TestNamedQueueRecorder.java | 2 +- .../hbase/namequeues/TestRpcLogDetails.java | 2 +- .../region/TestRegionProcedureStore.java | 2 +- 7 files changed, 27 insertions(+), 18 deletions(-) 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 c831c59e7d31..c291338e40c2 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 @@ -32,6 +32,7 @@ import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicReference; +import javax.net.ssl.SSLPeerUnverifiedException; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseInterfaceAudience; import org.apache.hadoop.hbase.HBaseServerBase; @@ -418,13 +419,22 @@ private void initSSL(ChannelPipeline p, NettyServerRpcConnection conn, boolean s sslHandler.handshakeFuture().addListener(future -> { try { Certificate[] certificates = sslHandler.engine().getSession().getPeerCertificates(); - if (certificates.length > 0) { - conn.clientCertificate = (X509Certificate) certificates[0]; - } else { - LOG.debug("No client certificate found for peer {}", remoteAddress); + if (certificates != null && certificates.length > 0) { + conn.clientCertificateChain = (X509Certificate[]) certificates; + } else if (sslHandler.engine().getNeedClientAuth()) { + LOG.error( + "Could not get peer certificate on TLS connection from {}, although one is required", + remoteAddress); + } + } catch (SSLPeerUnverifiedException e) { + if (sslHandler.engine().getNeedClientAuth()) { + LOG.error( + "Could not get peer certificate on TLS connection from {}, although one is required", + remoteAddress, e); } } catch (Exception e) { - LOG.debug("Failure getting peer certificate for {}", remoteAddress, e); + LOG.error("Unexpected error getting peer certificate for TLS connection from {}", + remoteAddress, e); } }); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcCallContext.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcCallContext.java index 8d8374f7321e..e852c5fe3d73 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcCallContext.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcCallContext.java @@ -62,11 +62,11 @@ default Optional getRequestUserName() { } /** - * Returns the TLS certificate that the client presented to this HBase server when making its + * Returns the TLS certificate(s) that the client presented to this HBase server when making its * connection. TLS is orthogonal to Kerberos, so this is unrelated to - * {@link this#getRequestUser()}. Both, one, or neither, may be present. + * {@link this#getRequestUser()}. Both, one, or neither may be present. */ - Optional getClientCertificate(); + Optional getClientCertificateChain(); /** Returns Address of remote client in this call */ InetAddress getRemoteAddress(); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java index 6df67ac0748a..531296f2966a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java @@ -96,7 +96,7 @@ public abstract class ServerCall implements RpcCa protected final User user; protected final InetAddress remoteAddress; - protected final X509Certificate clientCertificate; + protected final X509Certificate[] clientCertificateChain; protected RpcCallback rpcCallback; private long responseCellSize = 0; @@ -136,11 +136,11 @@ public abstract class ServerCall implements RpcCa if (connection != null) { this.user = connection.user; this.retryImmediatelySupported = connection.retryImmediatelySupported; - this.clientCertificate = connection.clientCertificate; + this.clientCertificateChain = connection.clientCertificateChain; } else { this.user = null; this.retryImmediatelySupported = false; - this.clientCertificate = null; + this.clientCertificateChain = null; } this.remoteAddress = remoteAddress; this.timeout = timeout; @@ -503,8 +503,8 @@ public Optional getRequestUser() { } @Override - public Optional getClientCertificate() { - return Optional.ofNullable(clientCertificate); + public Optional getClientCertificateChain() { + return Optional.ofNullable(clientCertificateChain); } @Override diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerRpcConnection.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerRpcConnection.java index b9bf66a64f98..4c32b2b6a5fa 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerRpcConnection.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerRpcConnection.java @@ -134,8 +134,7 @@ abstract class ServerRpcConnection implements Closeable { protected User user = null; protected UserGroupInformation ugi = null; protected SaslServerAuthenticationProviders saslProviders = null; - // volatile because this gets set after this object is constructed, when TLS handshake finishes - protected volatile X509Certificate clientCertificate = null; + protected X509Certificate[] clientCertificateChain = null; public ServerRpcConnection(RpcServer rpcServer) { this.rpcServer = rpcServer; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestNamedQueueRecorder.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestNamedQueueRecorder.java index 742a61325e1d..faf1d9919e4a 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestNamedQueueRecorder.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestNamedQueueRecorder.java @@ -816,7 +816,7 @@ public Optional getRequestUser() { } @Override - public Optional getClientCertificate() { + public Optional getClientCertificateChain() { return Optional.empty(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestRpcLogDetails.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestRpcLogDetails.java index 86624d2aed15..9c8d63714825 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestRpcLogDetails.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestRpcLogDetails.java @@ -215,7 +215,7 @@ public Optional getRequestUser() { } @Override - public Optional getClientCertificate() { + public Optional getClientCertificateChain() { return Optional.empty(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure2/store/region/TestRegionProcedureStore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure2/store/region/TestRegionProcedureStore.java index fb98940a61d5..cd86d3424d3e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure2/store/region/TestRegionProcedureStore.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure2/store/region/TestRegionProcedureStore.java @@ -277,7 +277,7 @@ public Optional getRequestUser() { } @Override - public Optional getClientCertificate() { + public Optional getClientCertificateChain() { return Optional.empty(); } From 93c7b4e2b640b7123b45cf89e578619afb502306 Mon Sep 17 00:00:00 2001 From: Charles Connell Date: Fri, 26 Jan 2024 08:42:03 -0500 Subject: [PATCH 3/3] fix javadoc --- .../main/java/org/apache/hadoop/hbase/ipc/RpcCallContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcCallContext.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcCallContext.java index e852c5fe3d73..43432324579b 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcCallContext.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcCallContext.java @@ -64,7 +64,7 @@ default Optional getRequestUserName() { /** * Returns the TLS certificate(s) that the client presented to this HBase server when making its * connection. TLS is orthogonal to Kerberos, so this is unrelated to - * {@link this#getRequestUser()}. Both, one, or neither may be present. + * {@link RpcCallContext#getRequestUser()}. Both, one, or neither may be present. */ Optional getClientCertificateChain();