Review Board 1.7.22


ZOOKEEPER-1185 : Send AuthFailed event to client if SASL authentication fails

Review Request #1959 - Created Sept. 19, 2011 and updated

Eugene Koontz
ZOOKEEPER-1185
Reviewers
zookeeper
zookeeper-git
There are 3 places where ClientCnxn should queue a AuthFailed event if client fails to authenticate. Without sending this event, clients may be stuck watching for a SaslAuthenticated event that will never come (since the client failed to authenticate).

This patch fixes SaslAuthFailTest.testBadSaslAuthNotifiesWatch() to test for the AuthFailed event : previously, the test was incorrectly not testing for this event.

It also removes the testBadSaslAuthNotifiesWatch() method from the SaslAuthTest class : this method belongs in SaslAuthFailTest, not SaslAuthTest. The former tests unsuccessful SASL authentication; the latter, successful SASL authentication. 
All unit tests pass. Also tested with an HBase cluster with an hbase shell running as an unauthenticated Zookeeper client. As expected, hbase shell could not access cluster, but, as expected, did not hang.
src/java/main/org/apache/zookeeper/ClientCnxn.java
Revision db15348 New Change
[20] 555 lines
[+20] [+] private void processEvent(Object event) {
556
                  } else if (p.cb instanceof ZooKeeperSaslClient.ServerSaslResponseCallback) {
556
                  } else if (p.cb instanceof ZooKeeperSaslClient.ServerSaslResponseCallback) {
557
                      ZooKeeperSaslClient.ServerSaslResponseCallback cb = (ZooKeeperSaslClient.ServerSaslResponseCallback) p.cb;
557
                      ZooKeeperSaslClient.ServerSaslResponseCallback cb = (ZooKeeperSaslClient.ServerSaslResponseCallback) p.cb;
558
                      SetSASLResponse rsp = (SetSASLResponse) p.response;
558
                      SetSASLResponse rsp = (SetSASLResponse) p.response;
559
                      // TODO : check rc (== 0, etc) as with other packet types.
559
                      // TODO : check rc (== 0, etc) as with other packet types.
560
                      cb.processResult(rc,null,p.ctx,rsp.getToken(),null);
560
                      cb.processResult(rc,null,p.ctx,rsp.getToken(),null);

    
   
561
                      ClientCnxn clientCnxn = (ClientCnxn)p.ctx;

    
   
562
                      if ((p.ctx == null) || (clientCnxn.zooKeeperSaslClient == null) ||

    
   
563
                              (clientCnxn.zooKeeperSaslClient.hasFailed())) {

    
   
564
                          queueEvent(new WatchedEvent(EventType.None,

    
   
565
                                  KeeperState.AuthFailed, null));

    
   
566
                      }
561
                  } else if (p.response instanceof GetDataResponse) {
567
                  } else if (p.response instanceof GetDataResponse) {
562
                      DataCallback cb = (DataCallback) p.cb;
568
                      DataCallback cb = (DataCallback) p.cb;
563
                      GetDataResponse rsp = (GetDataResponse) p.response;
569
                      GetDataResponse rsp = (GetDataResponse) p.response;
564
                      if (rc == 0) {
570
                      if (rc == 0) {
565
                          cb.processResult(rc, clientPath, p.ctx, rsp
571
                          cb.processResult(rc, clientPath, p.ctx, rsp
[+20] [20] 377 lines
[+20] [+] private void startConnect() throws IOException {
943
                catch (LoginException e) {
949
                catch (LoginException e) {
944
                    LOG.warn("Zookeeper client cannot authenticate using the Client section of the supplied "
950
                    LOG.warn("Zookeeper client cannot authenticate using the Client section of the supplied "
945
                      + "configuration file: '" + System.getProperty("java.security.auth.login.config")
951
                      + "configuration file: '" + System.getProperty("java.security.auth.login.config")
946
                      + "'. Will continue connection to Zookeeper server without SASL authentication, if Zookeeper "
952
                      + "'. Will continue connection to Zookeeper server without SASL authentication, if Zookeeper "
947
                      + "server allows it.");
953
                      + "server allows it.");

    
   
954
                    eventThread.queueEvent(new WatchedEvent(

    
   
955
                            Watcher.Event.EventType.None,

    
   
956
                            KeeperState.AuthFailed, null));
948
                }
957
                }
949
            }
958
            }
950
            clientCnxnSocket.connect(addr);
959
            clientCnxnSocket.connect(addr);
951
        }
960
        }
952

    
   
961

   
[+20] [20] 24 lines
[+20] [+] public void run() {
977
                                zooKeeperSaslClient.initialize();
986
                                zooKeeperSaslClient.initialize();
978
                            }
987
                            }
979
                            catch (SaslException e) {
988
                            catch (SaslException e) {
980
                                LOG.error("SASL authentication with Zookeeper Quorum member failed: " + e);
989
                                LOG.error("SASL authentication with Zookeeper Quorum member failed: " + e);
981
                                state = States.AUTH_FAILED;
990
                                state = States.AUTH_FAILED;

    
   
991
                                eventThread.queueEvent(new WatchedEvent(

    
   
992
                                        Watcher.Event.EventType.None,

    
   
993
                                        KeeperState.AuthFailed,null));
982
                            }
994
                            }
983
                            if (zooKeeperSaslClient.readyToSendSaslAuthEvent()) {
995
                            if (zooKeeperSaslClient.readyToSendSaslAuthEvent()) {
984
                                eventThread.queueEvent(new WatchedEvent(
996
                                eventThread.queueEvent(new WatchedEvent(
985
                                  Watcher.Event.EventType.None,
997
                                  Watcher.Event.EventType.None,
986
                                  Watcher.Event.KeeperState.SaslAuthenticated, null));
998
                                  Watcher.Event.KeeperState.SaslAuthenticated, null));
[+20] [20] 309 lines
src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java
Revision 43382c8 New Change
 
src/java/test/org/apache/zookeeper/test/SaslAuthFailTest.java
Revision 8de7c2a New Change
 
src/java/test/org/apache/zookeeper/test/SaslAuthTest.java
Revision fd20346 New Change
 
  1. src/java/main/org/apache/zookeeper/ClientCnxn.java: Loading...
  2. src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java: Loading...
  3. src/java/test/org/apache/zookeeper/test/SaslAuthFailTest.java: Loading...
  4. src/java/test/org/apache/zookeeper/test/SaslAuthTest.java: Loading...