This is an automated email from the ASF dual-hosted git repository.
nihaljain pushed a commit to branch branch-2.4
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2.4 by this push:
new c8b602cfeb8 HBASE-24179 Backport fix for "Netty SASL implementation
does not wait for challenge response" to branch-2.x (#5472) (#5497)
c8b602cfeb8 is described below
commit c8b602cfeb887207374ea0fa099ec0fa58a31595
Author: Nihal Jain <[email protected]>
AuthorDate: Sat Nov 4 01:50:59 2023 +0530
HBASE-24179 Backport fix for "Netty SASL implementation does not wait for
challenge response" to branch-2.x (#5472) (#5497)
- 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]>
Signed-off-by: Wellington Chevreuil <[email protected]>
---
.../hbase/security/AbstractHBaseSaslRpcClient.java | 10 +++++++--
.../security/NettyHBaseSaslRpcClientHandler.java | 26 ++++++++++++++++++----
.../ShadeSaslClientAuthenticationProvider.java | 6 +++++
.../ShadeSaslServerAuthenticationProvider.java | 1 -
.../TestShadeSaslAuthenticationProvider.java | 2 +-
5 files changed, 37 insertions(+), 8 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 525a78d0ae8..e57ca56390c 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
@@ -53,6 +53,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;
@@ -67,6 +69,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()));
@@ -83,6 +86,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());
setCryptoAESOption();
@@ -110,10 +117,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.
@@ -145,6 +161,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 399906b056b..b5d8ffec7d5 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
@@ -138,7 +138,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()));
}
}
}
