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 b2bf6a4f5363..87b2287a6014 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 @@ protected AbstractHBaseSaslRpcClient(Configuration conf, } } + /** + * 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 525a78d0ae8d..e57ca56390c4 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 NettyHBaseSaslRpcClientHandler(Promise saslPromise, UserGroupInf 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 @@ private void tryComplete(ChannelHandlerContext ctx) { 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 byte[] run() throws Exception { 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 byte[] run() throws Exception { }); 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 45bf5f015bf2..d0930a0f3148 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 UserInformation getUserInfo(User user) { 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 3454bc30a1d0..9f0e70133db5 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 ShadeSaslServerCallbackHandler(AtomicReference atte @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 75894d69b0a0..4ee753b1d26e 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 @@ void validateRootCause(Throwable rootCause) { 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())); } } }