HBASE-24179 Backport fix for "Netty SASL implementation does not wait for challenge response" to branch-2.x (#5472) (#5496)

- 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]>

Author: NihalJain

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() {

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<Boolean> 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);
   }

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;

hbase-examples/src/main/java/org/apache/hadoop/hbase/security/provider/example/ShadeSaslServerAuthenticationProvider.java

@@ -139,7 +139,6 @@ public ShadeSaslServerCallbackHandler(AtomicReference<UserGroupInformation> 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;

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()));
     }
   }
 }