> On Sept. 19, 2016, 7:21 p.m., Michael Han wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/Learner.java, line 248
> > <https://reviews.apache.org/r/47354/diff/9/?file=1498958#file1498958line248>
> >
> >     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.

Chat with Rakesh more about this - the check here is required because not all 
connections between peer are done through QuorumCnxManager - in particular in 
reconfig case a new peer joining quorum will first query who the leader is and 
once he gots the leader adddress he will attemp direct connect to leader (which 
by pass QuorumCnxManager). So this is not an issue, and existing code in patch 
is doing fine.


- Michael


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


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