This is an automated email from the ASF dual-hosted git repository.
nihaljain pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2 by this push:
new 3481aefce99 HBASE-24179 Backport fix for "Netty SASL implementation
does not wait for challenge response" to branch-2.x (#5472)
3481aefce99 is described below
commit 3481aefce99637a34d97860100beabf600b65845
Author: Nihal Jain <[email protected]>
AuthorDate: Wed Oct 25 13:47:27 2023 +0530
HBASE-24179 Backport fix for "Netty SASL implementation does not wait for
challenge response" to branch-2.x (#5472)
- Backport HBASE-23881 "Netty SASL implementation does not wait for
challenge response"
- Backport HBASE-24263 "TestDelegationToken is broken"
- Fix assertion to check for InvalidToken.class.getName() to ensure bug is
fixed
Signed-off-by: Duo Zhang <[email protected]>
---
.../hbase/security/AbstractHBaseSaslRpcClient.java | 10 +++++++--
.../security/NettyHBaseSaslRpcClientHandler.java | 26 ++++++++++++++++++----
.../ShadeSaslClientAuthenticationProvider.java | 6 +++++
.../ShadeSaslServerAuthenticationProvider.java | 1 -
.../TestShadeSaslAuthenticationProvider.java | 2 +-
.../hbase/ipc/NettyHBaseSaslRpcServerHandler.java | 2 +-
.../hbase/ipc/SimpleServerRpcConnection.java | 2 +-
7 files changed, 39 insertions(+), 10 deletions(-)
diff --git
a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/AbstractHBaseSaslRpcClient.java
b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/AbstractHBaseSaslRpcClient.java
index b2bf6a4f536..87b2287a601 100644
---
a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/AbstractHBaseSaslRpcClient.java
+++
b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/AbstractHBaseSaslRpcClient.java
@@ -83,12 +83,18 @@ public abstract class AbstractHBaseSaslRpcClient {
}
}
+ /**
+ * Computes the initial response a client sends to a server to begin the
SASL challenge/response
+ * handshake. If the client's SASL mechanism does not have an initial
response, an empty token
+ * will be returned without querying the evaluateChallenge method, as an
authentication processing
+ * must be started by client.
+ * @return The client's initial response to send the server (which may be
empty).
+ */
public byte[] getInitialResponse() throws SaslException {
if (saslClient.hasInitialResponse()) {
return saslClient.evaluateChallenge(EMPTY_TOKEN);
- } else {
- return EMPTY_TOKEN;
}
+ return EMPTY_TOKEN;
}
public boolean isComplete() {
diff --git
a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/NettyHBaseSaslRpcClientHandler.java
b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/NettyHBaseSaslRpcClientHandler.java
index 324b8ee70ed..48e631c7629 100644
---
a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/NettyHBaseSaslRpcClientHandler.java
+++
b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/NettyHBaseSaslRpcClientHandler.java
@@ -57,6 +57,8 @@ public class NettyHBaseSaslRpcClientHandler extends
SimpleChannelInboundHandler<
private final Configuration conf;
+ private final SaslClientAuthenticationProvider provider;
+
// flag to mark if Crypto AES encryption is enable
private boolean needProcessConnectionHeader = false;
@@ -71,6 +73,7 @@ public class NettyHBaseSaslRpcClientHandler extends
SimpleChannelInboundHandler<
this.saslPromise = saslPromise;
this.ugi = ugi;
this.conf = conf;
+ this.provider = provider;
this.saslRpcClient = new NettyHBaseSaslRpcClient(conf, provider, token,
serverAddr,
securityInfo, fallbackAllowed, conf.get("hbase.rpc.protection",
SaslUtil.QualityOfProtection.AUTHENTICATION.name().toLowerCase()));
@@ -87,6 +90,10 @@ public class NettyHBaseSaslRpcClientHandler extends
SimpleChannelInboundHandler<
return;
}
+ // HBASE-23881 Clearly log when the client thinks that the SASL
negotiation is complete.
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("SASL negotiation for {} is complete",
provider.getSaslAuthMethod().getName());
+ }
saslRpcClient.setupSaslHandler(ctx.pipeline(), HANDLER_NAME);
removeHandlers(ctx);
@@ -125,10 +132,19 @@ public class NettyHBaseSaslRpcClientHandler extends
SimpleChannelInboundHandler<
return saslRpcClient.getInitialResponse();
}
});
- if (initialResponse != null) {
- writeResponse(ctx, initialResponse);
- }
- tryComplete(ctx);
+ assert initialResponse != null;
+ writeResponse(ctx, initialResponse);
+ // HBASE-23881 We do not want to check if the SaslClient thinks the
handshake is
+ // complete as, at this point, we've not heard a back from the server
with it's reply
+ // to our first challenge response. We should wait for at least one reply
+ // from the server before calling negotiation complete.
+ //
+ // Each SASL mechanism has its own handshake. Some mechanisms calculate
a single client buffer
+ // to be sent to the server while others have multiple exchanges to
negotiate authentication.
+ // GSSAPI(Kerberos) and DIGEST-MD5 both are examples of mechanisms which
have multiple steps.
+ // Mechanisms which have multiple steps will not return true on
`SaslClient#isComplete()`
+ // until the handshake has fully completed. Mechanisms which only send a
single buffer may
+ // return true on `isComplete()` after that initial response is
calculated.
} catch (Exception e) {
// the exception thrown by handlerAdded will not be passed to the
exceptionCaught below
// because netty will remove a handler if handlerAdded throws an
exception.
@@ -163,6 +179,8 @@ public class NettyHBaseSaslRpcClientHandler extends
SimpleChannelInboundHandler<
});
if (response != null) {
writeResponse(ctx, response);
+ } else {
+ LOG.trace("SASL challenge response was empty, not sending response to
server.");
}
tryComplete(ctx);
}
diff --git
a/hbase-examples/src/main/java/org/apache/hadoop/hbase/security/provider/example/ShadeSaslClientAuthenticationProvider.java
b/hbase-examples/src/main/java/org/apache/hadoop/hbase/security/provider/example/ShadeSaslClientAuthenticationProvider.java
index 45bf5f015bf..d0930a0f314 100644
---
a/hbase-examples/src/main/java/org/apache/hadoop/hbase/security/provider/example/ShadeSaslClientAuthenticationProvider.java
+++
b/hbase-examples/src/main/java/org/apache/hadoop/hbase/security/provider/example/ShadeSaslClientAuthenticationProvider.java
@@ -60,6 +60,12 @@ public class ShadeSaslClientAuthenticationProvider extends
ShadeSaslAuthenticati
return userInfoPB.build();
}
+ @Override
+ public boolean canRetry() {
+ // A static username/password either works or it doesn't. No kind of
relogin/retry necessary.
+ return false;
+ }
+
static class ShadeSaslClientCallbackHandler implements CallbackHandler {
private final String username;
private final char[] password;
diff --git
a/hbase-examples/src/main/java/org/apache/hadoop/hbase/security/provider/example/ShadeSaslServerAuthenticationProvider.java
b/hbase-examples/src/main/java/org/apache/hadoop/hbase/security/provider/example/ShadeSaslServerAuthenticationProvider.java
index 3454bc30a1d..9f0e70133db 100644
---
a/hbase-examples/src/main/java/org/apache/hadoop/hbase/security/provider/example/ShadeSaslServerAuthenticationProvider.java
+++
b/hbase-examples/src/main/java/org/apache/hadoop/hbase/security/provider/example/ShadeSaslServerAuthenticationProvider.java
@@ -139,7 +139,6 @@ public class ShadeSaslServerAuthenticationProvider extends
ShadeSaslAuthenticati
@Override
public void handle(Callback[] callbacks) throws InvalidToken,
UnsupportedCallbackException {
- LOG.info("SaslServerCallbackHandler called", new Exception());
NameCallback nc = null;
PasswordCallback pc = null;
AuthorizeCallback ac = null;
diff --git
a/hbase-examples/src/test/java/org/apache/hadoop/hbase/security/provider/example/TestShadeSaslAuthenticationProvider.java
b/hbase-examples/src/test/java/org/apache/hadoop/hbase/security/provider/example/TestShadeSaslAuthenticationProvider.java
index 75894d69b0a..4ee753b1d26 100644
---
a/hbase-examples/src/test/java/org/apache/hadoop/hbase/security/provider/example/TestShadeSaslAuthenticationProvider.java
+++
b/hbase-examples/src/test/java/org/apache/hadoop/hbase/security/provider/example/TestShadeSaslAuthenticationProvider.java
@@ -305,7 +305,7 @@ public class TestShadeSaslAuthenticationProvider {
rootCause.printStackTrace(new PrintWriter(writer));
String text = writer.toString();
assertTrue("Message did not contain expected text",
- text.contains("Connection reset by peer"));
+ text.contains(InvalidToken.class.getName()));
}
}
}
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/NettyHBaseSaslRpcServerHandler.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/NettyHBaseSaslRpcServerHandler.java
index dd6f84daae3..47c748d6d70 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/NettyHBaseSaslRpcServerHandler.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/NettyHBaseSaslRpcServerHandler.java
@@ -89,7 +89,7 @@ class NettyHBaseSaslRpcServerHandler extends
SimpleChannelInboundHandler<ByteBuf
rpcServer.metrics.authenticationFailure();
String clientIP = this.toString();
// attempting user could be null
- RpcServer.AUDITLOG.warn("{}{}: {}", RpcServer.AUTH_FAILED_FOR, clientIP,
+ RpcServer.AUDITLOG.warn("{} {}: {}", RpcServer.AUTH_FAILED_FOR, clientIP,
conn.saslServer != null ? conn.saslServer.getAttemptingUser() :
"Unknown");
NettyFutureUtils.safeClose(ctx);
}
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/SimpleServerRpcConnection.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/SimpleServerRpcConnection.java
index ac705d7a26f..0ab011d13c3 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/SimpleServerRpcConnection.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/SimpleServerRpcConnection.java
@@ -229,7 +229,7 @@ class SimpleServerRpcConnection extends ServerRpcConnection
{
this.rpcServer.metrics.authenticationFailure();
String clientIP = this.toString();
// attempting user could be null
- RpcServer.AUDITLOG.warn("{}{}: {}", RpcServer.AUTH_FAILED_FOR,
clientIP,
+ RpcServer.AUDITLOG.warn("{} {}: {}", RpcServer.AUTH_FAILED_FOR,
clientIP,
saslServer.getAttemptingUser());
throw e;
} finally {
