From 9cec74c5033a3ed4a1030ba37cd91ca42adc03a1 Mon Sep 17 00:00:00 2001 From: Andrew Purtell Date: Sun, 24 Jul 2022 23:06:31 -0700 Subject: [PATCH] HBASE-27204 BlockingRpcClient will hang for 20 seconds when SASL is enabled after finishing negotiation (#4642) Revert "HBASE-24579: Failed SASL authentication does not result in an exception on client side (#1921)" This reverts commit bd79c4065ccb13a5e217d844376b3e7b9489d2fe. When Kerberos authentication succeeds, on the server side, after receiving the final SASL token from the client, we simply wait for the client to continue by sending the connection header. After HBASE-24579, on the client side, an additional readStatus() was added, which mistakenly assumes that after negotiation has completed a status code will be sent. However when authentication has succeeded the server will not send one. As a result the client will hang and only throw an exception when the configured read timeout is reached, which is 20 seconds by default. We cannot unilaterally send the expected additional status code from the server side because older clients will not expect it. The first call will fail because the client finds unexpected bytes in the stream ahead of the call response. Fabricating a call response also does not seem a viable strategy for backwards compatibility. The HBASE-24579 change needs to be reconsidered given the difficult backwards compatibility challenges here. Signed-off-by: Duo Zhang Signed-off-by: Viraj Jasani --- .../hbase/security/HBaseSaslRpcClient.java | 8 ----- .../security/TestHBaseSaslRpcClient.java | 30 ------------------- 2 files changed, 38 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcClient.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcClient.java index f9350edcf01..93ad9245f65 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcClient.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/security/HBaseSaslRpcClient.java @@ -145,14 +145,6 @@ public class HBaseSaslRpcClient extends AbstractHBaseSaslRpcClient { } } - try { - readStatus(inStream); - } catch (IOException e) { - if (e instanceof RemoteException) { - LOG.debug("Sasl connection failed: ", e); - throw e; - } - } if (LOG.isDebugEnabled()) { LOG.debug("SASL client context established. Negotiated QoP: " + saslClient.getNegotiatedProperty(Sasl.QOP)); diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/security/TestHBaseSaslRpcClient.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/security/TestHBaseSaslRpcClient.java index 8d82ba538bd..60bd80fb58f 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/security/TestHBaseSaslRpcClient.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/security/TestHBaseSaslRpcClient.java @@ -50,10 +50,8 @@ import org.apache.hadoop.hbase.testclassification.SmallTests; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.io.DataInputBuffer; import org.apache.hadoop.io.DataOutputBuffer; -import org.apache.hadoop.io.WritableUtils; import org.apache.hadoop.security.token.Token; import org.apache.hadoop.security.token.TokenIdentifier; -import org.junit.Assert; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; @@ -307,32 +305,4 @@ public class TestHBaseSaslRpcClient { private Token createTokenMock() { return mock(Token.class); } - - @Test(expected = IOException.class) - public void testFailedEvaluateResponse() throws IOException { - // prep mockin the SaslClient - SimpleSaslClientAuthenticationProvider mockProvider = - Mockito.mock(SimpleSaslClientAuthenticationProvider.class); - SaslClient mockClient = Mockito.mock(SaslClient.class); - Assert.assertNotNull(mockProvider); - Assert.assertNotNull(mockClient); - Mockito.when(mockProvider.createClient(Mockito.any(), Mockito.any(), Mockito.any(), - Mockito.any(), Mockito.anyBoolean(), Mockito.any())).thenReturn(mockClient); - HBaseSaslRpcClient rpcClient = new HBaseSaslRpcClient(HBaseConfiguration.create(), mockProvider, - createTokenMock(), Mockito.mock(InetAddress.class), Mockito.mock(SecurityInfo.class), false); - - // simulate getting an error from a failed saslServer.evaluateResponse - DataOutputBuffer errorBuffer = new DataOutputBuffer(); - errorBuffer.writeInt(SaslStatus.ERROR.state); - WritableUtils.writeString(errorBuffer, IOException.class.getName()); - WritableUtils.writeString(errorBuffer, "Invalid Token"); - - DataInputBuffer in = new DataInputBuffer(); - in.reset(errorBuffer.getData(), 0, errorBuffer.getLength()); - DataOutputBuffer out = new DataOutputBuffer(); - - // simulate that authentication exchange has completed quickly after sending the token - Mockito.when(mockClient.isComplete()).thenReturn(true); - rpcClient.saslConnect(in, out); - } }