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




src/java/main/org/apache/zookeeper/server/quorum/Learner.java (line 246)
<https://reviews.apache.org/r/47354/#comment217150>

    This check seems redundant to me - when we are here we either already have 
a leader elected or we are asking a new leader election. Now during leader 
election the leader and the learners that participated should authenticate each 
other mutually (in QuorumCnxManager authServer.authenticate(sock, din), and 
authLearner.authenticate(sock)). 
    
    Note that it is possible that the leader and this specific learner that 
asking to connect to leader has not been mutually verified during LE - because 
we don't have 1 to 1 connections between every pair of servers. This should be 
fine as the authentication / authz will be checked when the actual connection 
is being made, again in QuorumCnxManager.



src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java (line 167)
<https://reviews.apache.org/r/47354/#comment217180>

    Similarly here, should we remove the auth check and delegate the auth check 
in QuorumCnxManager instead?


- Michael Han


On Sept. 15, 2016, 4:48 p.m., Rakesh R wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47354/
> -----------------------------------------------------------
> 
> (Updated Sept. 15, 2016, 4:48 p.m.)
> 
> 
> Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul 
> Gutierrez Segales.
> 
> 
> Bugs: ZOOKEEPER-1045
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1045
> 
> 
> Repository: zookeeper-git
> 
> 
> Description
> -------
> 
> Quorum mutual authentication using SASL mechanism - Digest/Kerberos
> 
> 
> Diffs
> -----
> 
>   build.xml ee6834c 
>   ivy.xml 95b0e5a 
>   src/java/main/org/apache/zookeeper/Login.java aaa220c 
>   src/java/main/org/apache/zookeeper/SaslClientCallbackHandler.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java 21ef0fa 
>   src/java/main/org/apache/zookeeper/server/ZooKeeperSaslServer.java 71870ce 
>   
> src/java/main/org/apache/zookeeper/server/auth/SaslServerCallbackHandler.java 
> 2fbd6ed 
>   src/java/main/org/apache/zookeeper/server/quorum/Leader.java c83d352 
>   src/java/main/org/apache/zookeeper/server/quorum/Learner.java 647b8a2 
>   src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java 
> 8a748c7 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java 
> 20e5f16 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 2f0f21b 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java 
> 0924ef6 
>   src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java 
> e9c8007 
>   
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthLearner.java
>  PRE-CREATION 
>   
> src/java/main/org/apache/zookeeper/server/quorum/auth/NullQuorumAuthServer.java
>  PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java 
> PRE-CREATION 
>   
> src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthServer.java 
> PRE-CREATION 
>   src/java/main/org/apache/zookeeper/server/quorum/auth/README.md 
> PRE-CREATION 
>   
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java
>  PRE-CREATION 
>   
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java
>  PRE-CREATION 
>   
> src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumServerCallbackHandler.java
>  PRE-CREATION 
>   src/java/main/org/apache/zookeeper/util/SecurityUtils.java PRE-CREATION 
>   src/java/test/data/kerberos/minikdc-krb5.conf PRE-CREATION 
>   src/java/test/data/kerberos/minikdc.ldiff PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/CnxManagerTest.java 
> 8db7fa8 
>   
> src/java/test/org/apache/zookeeper/server/quorum/FLEBackwardElectionRoundTest.java
>  c1259d1 
>   src/java/test/org/apache/zookeeper/server/quorum/FLECompatibilityTest.java 
> 72e4fc9 
>   src/java/test/org/apache/zookeeper/server/quorum/FLEDontCareTest.java 
> a4c0cb0 
>   src/java/test/org/apache/zookeeper/server/quorum/FLELostMessageTest.java 
> 39a53ca 
>   src/java/test/org/apache/zookeeper/server/quorum/LearnerTest.java 2ae57ce 
>   src/java/test/org/apache/zookeeper/server/quorum/QuorumCnxManagerTest.java 
> PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java 
> 85817b2 
>   src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java ab8ce42 
>   
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosSecurityTestcase.java
>  PRE-CREATION 
>   
> src/java/test/org/apache/zookeeper/server/quorum/auth/KerberosTestUtils.java 
> PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdc.java 
> PRE-CREATION 
>   src/java/test/org/apache/zookeeper/server/quorum/auth/MiniKdcTest.java 
> PRE-CREATION 
>   
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java 
> PRE-CREATION 
>   
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumAuthUpgradeTest.java
>  PRE-CREATION 
>   
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java
>  PRE-CREATION 
>   
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java
>  PRE-CREATION 
>   
> src/java/test/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java
>  PRE-CREATION 
>   src/java/test/org/apache/zookeeper/test/FLEPredicateTest.java 8088505 
>   src/zookeeper.jute 6521e54 
> 
> Diff: https://reviews.apache.org/r/47354/diff/
> 
> 
> Testing
> -------
> 
> Added unit test cases to verify the changes.
> 
> 
> Thanks,
> 
> Rakesh R
> 
>

Reply via email to