浏览代码

ZOOKEEPER-1152. Exceptions thrown from handleAuthentication can cause buffer corruption issues in NIOServer.

git-svn-id: https://svn.apache.org/repos/asf/zookeeper/trunk@1159929 13f79535-47bb-0310-9956-ffa450edef68
Benjamin Reed 13 年之前
父节点
当前提交
c17d3501c9

+ 2 - 0
CHANGES.txt

@@ -428,6 +428,8 @@ NEW FEATURES:
   ZOOKEEPER-938. Support Kerberos authentication of clients. (Eugene Koontz
   ZOOKEEPER-938. Support Kerberos authentication of clients. (Eugene Koontz
   via mahadev)
   via mahadev)
 
 
+  ZOOKEEPER-1152. Exceptions thrown from handleAuthentication can cause buffer corruption issues in NIOServer. (camille via breed)
+
 Release 3.3.0 - 2010-03-24
 Release 3.3.0 - 2010-03-24
 
 
 Non-backward compatible changes:
 Non-backward compatible changes:

+ 11 - 3
src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java

@@ -48,6 +48,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.slf4j.LoggerFactory;
 import org.apache.zookeeper.Environment;
 import org.apache.zookeeper.Environment;
 import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.KeeperException.SessionExpiredException;
 import org.apache.zookeeper.KeeperException.SessionExpiredException;
 import org.apache.zookeeper.ZooDefs.OpCode;
 import org.apache.zookeeper.ZooDefs.OpCode;
 import org.apache.zookeeper.data.ACL;
 import org.apache.zookeeper.data.ACL;
@@ -862,9 +863,16 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider {
             ZooKeeperServer.byteBuffer2Record(incomingBuffer, authPacket);
             ZooKeeperServer.byteBuffer2Record(incomingBuffer, authPacket);
             String scheme = authPacket.getScheme();
             String scheme = authPacket.getScheme();
             AuthenticationProvider ap = ProviderRegistry.getProvider(scheme);
             AuthenticationProvider ap = ProviderRegistry.getProvider(scheme);
-            if (ap == null
-                    || (ap.handleAuthentication(cnxn, authPacket.getAuth())
-                            != KeeperException.Code.OK)) {
+            Code authReturn = KeeperException.Code.AUTHFAILED;
+            if(ap != null) {
+                try {
+                    authReturn = ap.handleAuthentication(cnxn, authPacket.getAuth());
+                } catch(RuntimeException e) {
+                    LOG.warn("Caught runtime exception from AuthenticationProvider: " + scheme + " due to " + e);
+                    authReturn = KeeperException.Code.AUTHFAILED;                   
+                }
+            }
+            if (authReturn!= KeeperException.Code.OK) {
                 if (ap == null) {
                 if (ap == null) {
                     LOG.warn("No authentication provider for scheme: "
                     LOG.warn("No authentication provider for scheme: "
                             + scheme + " has "
                             + scheme + " has "

+ 19 - 1
src/java/test/org/apache/zookeeper/test/AuthTest.java

@@ -35,7 +35,8 @@ public class AuthTest extends ClientBase {
     static {
     static {
         // password is test
         // password is test
         System.setProperty("zookeeper.DigestAuthenticationProvider.superDigest",
         System.setProperty("zookeeper.DigestAuthenticationProvider.superDigest",
-                "super:D/InIHSb7yEEbrWz8b9l71RjZJU=");        
+                "super:D/InIHSb7yEEbrWz8b9l71RjZJU=");    
+        System.setProperty("zookeeper.authProvider.1", "org.apache.zookeeper.test.InvalidAuthProvider");
     }
     }
 
 
     private AtomicInteger authFailed = new AtomicInteger(0);
     private AtomicInteger authFailed = new AtomicInteger(0);
@@ -75,6 +76,23 @@ public class AuthTest extends ClientBase {
             zk.close();
             zk.close();
         }
         }
     }
     }
+    
+    @Test
+    public void testBadAuthThenSendOtherCommands() throws Exception {
+        ZooKeeper zk = createClient();     
+        try {        
+            zk.addAuthInfo("INVALID", "BAR".getBytes());
+            zk.exists("/foobar", false);             
+            zk.getData("/path1", false, null);
+            Assert.fail("Should get auth state error");
+        } catch(KeeperException.AuthFailedException e) {
+            Assert.assertEquals("Should have called my watcher", 
+                    1, authFailed.get());
+        }
+        finally {
+            zk.close();          
+        }
+    }
 
 
     
     
     @Test
     @Test