Review Request: Support Kerberos authentication of clients.

2011-07-09 Thread Eugene Koontz

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1059/
---

Review request for zookeeper.


Summary
---

Support Kerberos authentication of clients.


This addresses bug ZOOKEEPER-938.
https://issues.apache.org/jira/browse/ZOOKEEPER-938


Diffs
-

  src/java/main/org/apache/zookeeper/ClientCnxn.java 87477df 
  src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java d0e74fa 
  src/java/main/org/apache/zookeeper/LoginThread.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/Watcher.java e72105c 
  src/java/main/org/apache/zookeeper/ZooDefs.java f77ac20 
  src/java/main/org/apache/zookeeper/ZooKeeper.java f2ab4a6 
  src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 
PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java b690817 
  src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java bab8998 
  src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java e7e1846 
  src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java c6ab5dd 
  src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java deb1e7a 
  src/java/main/org/apache/zookeeper/server/Request.java 80d2b99 
  src/java/main/org/apache/zookeeper/server/SaslServerCallbackHandler.java 
PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6d69073 
  src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 72af158 
  src/java/main/org/apache/zookeeper/server/ServerConfig.java ec710cd 
  src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 
PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 52d3820 
  src/java/main/org/apache/zookeeper/server/auth/DigestLoginModule.java 
PRE-CREATION 
  
src/java/main/org/apache/zookeeper/server/auth/SASLAuthenticationProvider.java 
PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 
c6d9c09 
  src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 969a482 
  src/java/test/org/apache/zookeeper/test/SaslAuthFailTest.java PRE-CREATION 
  src/java/test/org/apache/zookeeper/test/SaslAuthTest.java PRE-CREATION 
  src/zookeeper.jute 34eac78 

Diff: https://reviews.apache.org/r/1059/diff


Testing
---

Passes all unit tests on my local machine. Also tested as part of a 3-node 
quorum on an HBase cluster which was then loaded with data using the Faulkner 
load test 
(https://github.com/ekoontz/hbase-ec2/blob/4a3a1a11810ed6acd96593e2818cbd7d2ffc8950/faulkner/faulkner.rb)


Thanks,

Eugene



Re: Review Request: Support Kerberos authentication of clients.

2011-07-09 Thread Eugene Koontz

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1059/
---

(Updated 2011-07-09 19:51:29.949643)


Review request for zookeeper.


Summary
---

Support Kerberos authentication of clients.


This addresses bug ZOOKEEPER-938.
https://issues.apache.org/jira/browse/ZOOKEEPER-938


Diffs
-

  src/java/main/org/apache/zookeeper/ClientCnxn.java 87477df 
  src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java d0e74fa 
  src/java/main/org/apache/zookeeper/LoginThread.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/Watcher.java e72105c 
  src/java/main/org/apache/zookeeper/ZooDefs.java f77ac20 
  src/java/main/org/apache/zookeeper/ZooKeeper.java f2ab4a6 
  src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 
PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java b690817 
  src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java bab8998 
  src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java e7e1846 
  src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java c6ab5dd 
  src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java deb1e7a 
  src/java/main/org/apache/zookeeper/server/Request.java 80d2b99 
  src/java/main/org/apache/zookeeper/server/SaslServerCallbackHandler.java 
PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6d69073 
  src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 72af158 
  src/java/main/org/apache/zookeeper/server/ServerConfig.java ec710cd 
  src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 
PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 52d3820 
  src/java/main/org/apache/zookeeper/server/auth/DigestLoginModule.java 
PRE-CREATION 
  
src/java/main/org/apache/zookeeper/server/auth/SASLAuthenticationProvider.java 
PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 
c6d9c09 
  src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 969a482 
  src/java/test/org/apache/zookeeper/test/SaslAuthFailTest.java PRE-CREATION 
  src/java/test/org/apache/zookeeper/test/SaslAuthTest.java PRE-CREATION 
  src/zookeeper.jute 34eac78 

Diff: https://reviews.apache.org/r/1059/diff


Testing (updated)
---

Passes all unit tests on my local machine. 

Also tested in a kerberos-enabled 3-node quorum in an HBase cluster which was 
then loaded with data using the Faulkner load test 
(https://github.com/ekoontz/hbase-ec2/blob/4a3a1a11810ed6acd96593e2818cbd7d2ffc8950/faulkner/faulkner.rb)


Thanks,

Eugene



Re: Review Request: Support Kerberos authentication of clients.

2011-07-09 Thread Benjamin Reed

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1059/#review1019
---



src/java/main/org/apache/zookeeper/ClientCnxn.java
https://reviews.apache.org/r/1059/#comment2094

we don't want to make the state visible. we don't want applications to have 
to deal with new states, especially ones that depend on the authentication type.



src/java/main/org/apache/zookeeper/ClientCnxn.java
https://reviews.apache.org/r/1059/#comment2095

these don't seem like equivalent states. CONNECTEDREADONLY also needs SASL



src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java
https://reviews.apache.org/r/1059/#comment2096

this should be done in the session verification of at the ServerCnxn not 
here.



src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java
https://reviews.apache.org/r/1059/#comment2097

this should also be done much closer to the server connection. probably in 
ZooKeeperServer.processPacket(). that is where the auth processing is being 
done anyway.



src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java
https://reviews.apache.org/r/1059/#comment2098

the requireClientAuthScheme is general functionality that belongs here, but 
otherwise there is too much SASL code here. this logic should be moved into 
another class, perhaps SASLServerCallbackHandler, so that we are just hooking 
into ServerCnxnFactory.



src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java
https://reviews.apache.org/r/1059/#comment2099

i don't think we should be passing the SASL stuff through ServerCnxnFactory 
so non-opaquely. the configuration will set zookeeper. system properties 
from the config file and the SASL handler can grab them there.



src/java/main/org/apache/zookeeper/server/ServerConfig.java
https://reviews.apache.org/r/1059/#comment2100

these are properties used by a specific auth provider, we shouldn't have 
them exposed in a general class.



src/java/main/org/apache/zookeeper/server/ServerConfig.java
https://reviews.apache.org/r/1059/#comment2101

sasl should be able to figure out the renewtime based on the token since 
the token comes with an expiration. right?



src/java/main/org/apache/zookeeper/server/auth/SASLAuthenticationProvider.java
https://reviews.apache.org/r/1059/#comment2093

is there really no checking that we can do here?



src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
https://reviews.apache.org/r/1059/#comment2102

this property is too specific to be exposed here.


- Benjamin


On 2011-07-09 19:51:29, Eugene Koontz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/1059/
 ---
 
 (Updated 2011-07-09 19:51:29)
 
 
 Review request for zookeeper.
 
 
 Summary
 ---
 
 Support Kerberos authentication of clients.
 
 
 This addresses bug ZOOKEEPER-938.
 https://issues.apache.org/jira/browse/ZOOKEEPER-938
 
 
 Diffs
 -
 
   src/java/main/org/apache/zookeeper/ClientCnxn.java 87477df 
   src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java d0e74fa 
   src/java/main/org/apache/zookeeper/LoginThread.java PRE-CREATION 
   src/java/main/org/apache/zookeeper/Watcher.java e72105c 
   src/java/main/org/apache/zookeeper/ZooDefs.java f77ac20 
   src/java/main/org/apache/zookeeper/ZooKeeper.java f2ab4a6 
   src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 
 PRE-CREATION 
   src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java 
 b690817 
   src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java bab8998 
   src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java e7e1846 
   src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java c6ab5dd 
   src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 
 deb1e7a 
   src/java/main/org/apache/zookeeper/server/Request.java 80d2b99 
   src/java/main/org/apache/zookeeper/server/SaslServerCallbackHandler.java 
 PRE-CREATION 
   src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6d69073 
   src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 72af158 
   src/java/main/org/apache/zookeeper/server/ServerConfig.java ec710cd 
   src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 
 PRE-CREATION 
   src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 52d3820 
   src/java/main/org/apache/zookeeper/server/auth/DigestLoginModule.java 
 PRE-CREATION 
   
 src/java/main/org/apache/zookeeper/server/auth/SASLAuthenticationProvider.java
  PRE-CREATION 
   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 
 c6d9c09 
   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 
 969a482 
   

Review Request: Support Kerberos authentication of clients.

2011-07-08 Thread Patrick Hunt

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1049/
---

Review request for zookeeper, Benjamin Reed and Mahadev Konar.


Summary
---

Support Kerberos authentication of clients.


This addresses bug ZOOKEEPER-938.
https://issues.apache.org/jira/browse/ZOOKEEPER-938


Diffs
-

  src/java/main/org/apache/zookeeper/ClientCnxn.java 87477df 
  src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java d0e74fa 
  src/java/main/org/apache/zookeeper/LoginThread.java PRE-CREATION 
  src/java/main/org/apache/zookeeper/Watcher.java e72105c 
  src/java/main/org/apache/zookeeper/ZooDefs.java f77ac20 
  src/java/main/org/apache/zookeeper/ZooKeeper.java f2ab4a6 
  src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 
PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java b690817 
  src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java bab8998 
  src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java e7e1846 
  src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java c6ab5dd 
  src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java deb1e7a 
  src/java/main/org/apache/zookeeper/server/Request.java 80d2b99 
  src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6d69073 
  src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 72af158 
  src/java/main/org/apache/zookeeper/server/ServerConfig.java ec710cd 
  src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 
PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java 52d3820 
  src/java/main/org/apache/zookeeper/server/auth/DigestLoginModule.java 
PRE-CREATION 
  
src/java/main/org/apache/zookeeper/server/auth/SASLAuthenticationProvider.java 
PRE-CREATION 
  src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 
c6d9c09 
  src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 969a482 
  src/java/test/org/apache/zookeeper/test/SaslAuthFailTest.java PRE-CREATION 
  src/java/test/org/apache/zookeeper/test/SaslAuthTest.java PRE-CREATION 
  src/zookeeper.jute 34eac78 

Diff: https://reviews.apache.org/r/1049/diff


Testing
---


Thanks,

Patrick