[
https://issues.apache.org/jira/browse/ZOOKEEPER-938?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13062658#comment-13062658
]
jirapos...@reviews.apache.org commented on ZOOKEEPER-938:
---------------------------------------------------------
-----------------------------------------------------------
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.xxxx 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:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/1059/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-07-09 19:51:29)
bq.
bq.
bq. Review request for zookeeper.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Support Kerberos authentication of clients.
bq.
bq.
bq. This addresses bug ZOOKEEPER-938.
bq. https://issues.apache.org/jira/browse/ZOOKEEPER-938
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. src/java/main/org/apache/zookeeper/ClientCnxn.java 87477df
bq. src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java d0e74fa
bq. src/java/main/org/apache/zookeeper/LoginThread.java PRE-CREATION
bq. src/java/main/org/apache/zookeeper/Watcher.java e72105c
bq. src/java/main/org/apache/zookeeper/ZooDefs.java f77ac20
bq. src/java/main/org/apache/zookeeper/ZooKeeper.java f2ab4a6
bq. src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java
PRE-CREATION
bq. src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java
b690817
bq. src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java bab8998
bq. src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java
e7e1846
bq. src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java c6ab5dd
bq. src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
deb1e7a
bq. src/java/main/org/apache/zookeeper/server/Request.java 80d2b99
bq. src/java/main/org/apache/zookeeper/server/SaslServerCallbackHandler.java
PRE-CREATION
bq. src/java/main/org/apache/zookeeper/server/ServerCnxn.java 6d69073
bq. src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 72af158
bq. src/java/main/org/apache/zookeeper/server/ServerConfig.java ec710cd
bq. src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java
PRE-CREATION
bq. src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java
52d3820
bq. src/java/main/org/apache/zookeeper/server/auth/DigestLoginModule.java
PRE-CREATION
bq.
src/java/main/org/apache/zookeeper/server/auth/SASLAuthenticationProvider.java
PRE-CREATION
bq. src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
c6d9c09
bq. src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java
969a482
bq. src/java/test/org/apache/zookeeper/test/SaslAuthFailTest.java
PRE-CREATION
bq. src/java/test/org/apache/zookeeper/test/SaslAuthTest.java PRE-CREATION
bq. src/zookeeper.jute 34eac78
bq.
bq. Diff: https://reviews.apache.org/r/1059/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Passes all unit tests on my local machine.
bq.
bq. 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)
bq.
bq.
bq. Thanks,
bq.
bq. Eugene
bq.
bq.
> Support Kerberos authentication of clients.
> -------------------------------------------
>
> Key: ZOOKEEPER-938
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-938
> Project: ZooKeeper
> Issue Type: New Feature
> Components: java client, server
> Reporter: Eugene Koontz
> Assignee: Eugene Koontz
> Fix For: 3.4.0, 3.5.0
>
> Attachments: NIOServerCnxn.patch, ZOOKEEPER-938.patch,
> ZOOKEEPER-938.patch, ZOOKEEPER-938.patch, ZOOKEEPER-938.patch,
> ZOOKEEPER-938.patch, ZOOKEEPER-938.patch, ZOOKEEPER-938.patch,
> ZOOKEEPER-938.patch, ZOOKEEPER-938.patch, ZOOKEEPER-938.patch,
> ZOOKEEPER-938.patch, jaas.conf, sasl.patch
>
>
> Support Kerberos authentication of clients.
> The following usage would let an admin use Kerberos authentication to assign
> ACLs to authenticated clients.
> 1. Admin logs into zookeeper (not necessarily through Kerberos however).
> 2. Admin decides that a new node called '/mynode' should be owned by the user
> 'zkclient' and have full permissions on this.
> 3. Admin does: zk> create /mynode content sasl:zkcli...@foofers.org:cdrwa
> 4. User 'zkclient' logins to kerberos using the command line utility 'kinit'.
> 5. User connects to zookeeper server using a Kerberos-enabled version of
> zkClient (ZookeeperMain).
> 6. Behind the scenes, the client and server exchange authentication
> information. User is now authenticated as 'zkclient'.
> 7. User accesses /mynode with permissions 'cdrwa'.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
