This is an automated email from the ASF dual-hosted git repository.

apurtell 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 33b3bbe5cac HBASE-27204 BlockingRpcClient will hang for 20 seconds 
when SASL is enabled after finishing negotiation (#4642)
33b3bbe5cac is described below

commit 33b3bbe5cacd549ed40bb87de919d76202fd8a94
Author: Andrew Purtell <[email protected]>
AuthorDate: Sun Jul 24 23:06:31 2022 -0700

    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 <[email protected]>
    Signed-off-by: Viraj Jasani <[email protected]>
    
    Conflicts:
            
hbase-client/src/test/java/org/apache/hadoop/hbase/security/TestHBaseSaslRpcClient.java
---
 .../hadoop/hbase/security/HBaseSaslRpcClient.java  |  8 ------
 .../hbase/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 0919df8676b..013c7886766 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,12 +50,10 @@ 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.apache.log4j.Level;
 import org.apache.log4j.Logger;
-import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
 import org.junit.Rule;
@@ -313,32 +311,4 @@ public class TestHBaseSaslRpcClient {
   private Token<? extends TokenIdentifier> 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);
-  }
 }

Reply via email to