From 78ee748f908a259a0126cdf10802f8a8fd37fd67 Mon Sep 17 00:00:00 2001 From: Andor Molnar Date: Tue, 30 Jan 2024 22:14:45 +0100 Subject: [PATCH 1/3] HBASE-28337. Fix Shade authenticator positive UT, add back tryComplete() to client handler --- .../NettyHBaseSaslRpcClientHandler.java | 6 ++++++ .../TestShadeSaslAuthenticationProvider.java | 20 ++++++++++--------- 2 files changed, 17 insertions(+), 9 deletions(-) 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 48e631c76299..cc71355d4297 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 @@ -145,6 +145,12 @@ public byte[] run() throws Exception { // 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. + + // HBASE-28337 We still want to check if the SaslClient completed the handshake, because + // there are certain mechs like PLAIN which doesn't have a server response after the + // initial authentication request. We cannot remove this tryComplete(), otherwise mechs + // like PLAIN will fail with call timeout. + tryComplete(ctx); } 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. 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 a479310691b1..5da287e9fb9e 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 @@ -212,21 +212,23 @@ public String run() throws Exception { @Test public void testPositiveAuthentication() throws Exception { final Configuration clientConf = new Configuration(CONF); - try (Connection conn = ConnectionFactory.createConnection(clientConf)) { + try (Connection conn1 = ConnectionFactory.createConnection(clientConf)) { UserGroupInformation user1 = UserGroupInformation.createUserForTesting("user1", new String[0]); - user1.addToken(ShadeClientTokenUtil.obtainToken(conn, "user1", USER1_PASSWORD)); + user1.addToken(ShadeClientTokenUtil.obtainToken(conn1, "user1", USER1_PASSWORD)); user1.doAs(new PrivilegedExceptionAction() { @Override public Void run() throws Exception { - try (Table t = conn.getTable(tableName)) { - Result r = t.get(new Get(Bytes.toBytes("r1"))); - assertNotNull(r); - assertFalse("Should have read a non-empty Result", r.isEmpty()); - final Cell cell = r.getColumnLatestCell(Bytes.toBytes("f1"), Bytes.toBytes("q1")); - assertTrue("Unexpected value", CellUtil.matchingValue(cell, Bytes.toBytes("1"))); + try (Connection conn = ConnectionFactory.createConnection(clientConf)) { + try (Table t = conn.getTable(tableName)) { + Result r = t.get(new Get(Bytes.toBytes("r1"))); + assertNotNull(r); + assertFalse("Should have read a non-empty Result", r.isEmpty()); + final Cell cell = r.getColumnLatestCell(Bytes.toBytes("f1"), Bytes.toBytes("q1")); + assertTrue("Unexpected value", CellUtil.matchingValue(cell, Bytes.toBytes("1"))); - return null; + return null; + } } } }); From d4fab9e22cca2d7b8b141096568365afb48c110b Mon Sep 17 00:00:00 2001 From: Andor Molnar Date: Tue, 30 Jan 2024 23:15:37 +0100 Subject: [PATCH 2/3] HBASE-28337. Cannot determine root cause on client side, because the server shuts down the connection --- .../TestShadeSaslAuthenticationProvider.java | 17 ----------------- 1 file changed, 17 deletions(-) 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 5da287e9fb9e..4f4461c03655 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 @@ -270,7 +270,6 @@ public Void run() throws Exception { } catch (Exception e) { LOG.info("Caught exception in negative Master connectivity test", e); assertEquals("Found unexpected exception", pair.getSecond(), e.getClass()); - validateRootCause(Throwables.getRootCause(e)); } return null; } @@ -289,7 +288,6 @@ public Void run() throws Exception { } catch (Exception e) { LOG.info("Caught exception in negative RegionServer connectivity test", e); assertEquals("Found unexpected exception", pair.getSecond(), e.getClass()); - validateRootCause(Throwables.getRootCause(e)); } return null; } @@ -303,19 +301,4 @@ public Void run() throws Exception { } }); } - - void validateRootCause(Throwable rootCause) { - LOG.info("Root cause was", rootCause); - if (rootCause instanceof RemoteException) { - RemoteException re = (RemoteException) rootCause; - IOException actualException = re.unwrapRemoteException(); - assertEquals(InvalidToken.class, actualException.getClass()); - } else { - StringWriter writer = new StringWriter(); - rootCause.printStackTrace(new PrintWriter(writer)); - String text = writer.toString(); - assertTrue("Message did not contain expected text", - text.contains(InvalidToken.class.getName())); - } - } } From 09b6583cc80225d0cc4a9be6e6e48168fc0256d7 Mon Sep 17 00:00:00 2001 From: Andor Molnar Date: Wed, 31 Jan 2024 11:56:34 +0100 Subject: [PATCH 3/3] HBASE-28337. Spotless apply --- .../example/TestShadeSaslAuthenticationProvider.java | 6 ------ 1 file changed, 6 deletions(-) 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 4f4461c03655..26a8943096a9 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 @@ -27,8 +27,6 @@ import java.io.File; import java.io.IOException; import java.io.OutputStreamWriter; -import java.io.PrintWriter; -import java.io.StringWriter; import java.nio.charset.StandardCharsets; import java.security.PrivilegedExceptionAction; import java.util.ArrayList; @@ -69,10 +67,8 @@ import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.CommonFSUtils; import org.apache.hadoop.hbase.util.Pair; -import org.apache.hadoop.ipc.RemoteException; import org.apache.hadoop.minikdc.MiniKdc; import org.apache.hadoop.security.UserGroupInformation; -import org.apache.hadoop.security.token.SecretManager.InvalidToken; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; @@ -84,8 +80,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.apache.hbase.thirdparty.com.google.common.base.Throwables; - @Category({ MediumTests.class, SecurityTests.class }) public class TestShadeSaslAuthenticationProvider { private static final Logger LOG =