You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by Ian Dimayuga <ia...@microsoft.com> on 2015/03/12 01:01:38 UTC
Review Request 31963: Patch in X509AuthenticationProvider on top of
ZOOKEEPER-2125
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/
-----------------------------------------------------------
Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
Repository: zookeeper-git
Description
-------
Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
Diffs
-----
src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
src/java/test/org/apache/zookeeper/server/TestServerCnxn.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/31963/diff/
Testing
-------
Thanks,
Ian Dimayuga
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Ian Dimayuga <ia...@microsoft.com>.
> On March 13, 2015, 6:33 a.m., Rakesh R wrote:
> > src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java, line 233
> > <https://reviews.apache.org/r/31963/diff/1/?file=891814#file891814line233>
> >
> > I couldn't see anyone is calling this. Do we need this method?
This should be used in the Netty factory. I missed an integration point--thanks for catching.
> On March 13, 2015, 6:33 a.m., Rakesh R wrote:
> > src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java, line 80
> > <https://reviews.apache.org/r/31963/diff/1/?file=891814#file891814line80>
> >
> > One general doubt about the exception handling in the constructor:
> >
> > Should we continue with the server startup if there is an exception while instantiating the X509AuthenticationProvider ?
> >
> > Down the code I could see #handleAuthentication() is checking trustManager == null and failing all the clients KeeperException.Code.AUTHFAILED. Since no clients are allowed to establish a connection, it would good we fail the server startup rather than rejecting the client connections. What do you say?
Yes, because X509AuthenticationProvider is created by ProviderRegistry whether we want SSL or not.
Instead, I can make it so that NettyServerCnxnFactory.initSSL() fails when there is an issue getting the trust and key managers.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/#review76341
-----------------------------------------------------------
On March 12, 2015, 12:01 a.m., Ian Dimayuga wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31963/
> -----------------------------------------------------------
>
> (Updated March 12, 2015, 12:01 a.m.)
>
>
> Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
> src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
> src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
> src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
> src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
> src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/TestServerCnxn.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31963/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Dimayuga
>
>
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Rakesh R <ra...@huawei.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/#review76341
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
<https://reviews.apache.org/r/31963/#comment123882>
Since the connection establishment failed, can we close the connection before return the call.
cnxn#close();
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
<https://reviews.apache.org/r/31963/#comment123889>
I could see handleAuthentication(cnxn, null) can return KeeperException.Code.AUTHFAILED. In that case, we should have a check and if it is successful then continue with the logic otherwise cnxn#close(). isn't it?
if(KeeperException.Code.OK == authProvider.handleAuthentication(cnxn, null)){
// success
}else{
// failure
}
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
<https://reviews.apache.org/r/31963/#comment123883>
Since the connection establishment failed, can we close the connection before return the call.
cnxn#close();
src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
<https://reviews.apache.org/r/31963/#comment123894>
Please make trustManager and keyManager var as 'final'
src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
<https://reviews.apache.org/r/31963/#comment123900>
One general doubt about the exception handling in the constructor:
Should we continue with the server startup if there is an exception while instantiating the X509AuthenticationProvider ?
Down the code I could see #handleAuthentication() is checking trustManager == null and failing all the clients KeeperException.Code.AUTHFAILED. Since no clients are allowed to establish a connection, it would good we fail the server startup rather than rejecting the client connections. What do you say?
src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
<https://reviews.apache.org/r/31963/#comment123887>
Can we include cnxn#sessionId() also in the log message, this would be helpful when debugging issues.
src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
<https://reviews.apache.org/r/31963/#comment123896>
Should we log an error message?
src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
<https://reviews.apache.org/r/31963/#comment123901>
I couldn't see anyone is calling this. Do we need this method?
src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
<https://reviews.apache.org/r/31963/#comment123902>
I couldn't see anyone is calling this. Do we need this method?
src/java/test/org/apache/zookeeper/server/TestServerCnxn.java
<https://reviews.apache.org/r/31963/#comment123903>
Please add Apache License header.
src/java/test/org/apache/zookeeper/server/TestServerCnxn.java
<https://reviews.apache.org/r/31963/#comment123906>
As this is for mocking the actual impl, ca we rename this to MockServerCnxn.java ?
src/java/test/org/apache/zookeeper/test/X509AuthTest.java
<https://reviews.apache.org/r/31963/#comment123904>
Please add Apache License header.
src/java/test/org/apache/zookeeper/test/X509AuthTest.java
<https://reviews.apache.org/r/31963/#comment123905>
Great! Good unit testcases. Can we add one end to end test case together with the ZK-2125 patch. So that will be able to see the behaviors of authProvider like,
1) when a good client(trusted) connects to the server.
2) when a bad client(untrusted) connects to the server.
3) when start a server with invalid ssl configs
- Rakesh R
On March 12, 2015, 12:01 a.m., Ian Dimayuga wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31963/
> -----------------------------------------------------------
>
> (Updated March 12, 2015, 12:01 a.m.)
>
>
> Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
> src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
> src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
> src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
> src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
> src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
> src/java/test/org/apache/zookeeper/server/TestServerCnxn.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31963/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Dimayuga
>
>
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Hongchao Deng <hd...@cloudera.com>.
> On March 27, 2015, 12:27 a.m., Hongchao Deng wrote:
> > src/java/test/org/apache/zookeeper/test/SSLAuthTest.java, line 91
> > <https://reviews.apache.org/r/31963/diff/7/?file=907220#file907220line91>
> >
> > It's the same thing as testRejection(). They both have set the wrong configuration.
>
> Ian Dimayuga wrote:
> This however is not the same thing. testRejection() actually configures a certificate, but the certificate is from the untrustedKeyStore, so it is rejected. A handshake takes place, which does not happen for testMisconfiguration(). After the handshake, X509AuthenticationProvider rejects the unknown certificate.
Got this. Can you add this comments to testRejection()? The rest of the work looks good to me :)
- Hongchao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/#review77988
-----------------------------------------------------------
On March 27, 2015, 12:20 a.m., Ian Dimayuga wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31963/
> -----------------------------------------------------------
>
> (Updated March 27, 2015, 12:20 a.m.)
>
>
> Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
> src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
> src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
> src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
> src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
> src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
> src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
> src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
> src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
> src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
> src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
> src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
> src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
> src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31963/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Dimayuga
>
>
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Ian Dimayuga <ia...@microsoft.com>.
> On March 27, 2015, 12:27 a.m., Hongchao Deng wrote:
> > src/java/test/org/apache/zookeeper/test/SSLAuthTest.java, line 73
> > <https://reviews.apache.org/r/31963/diff/7/?file=907220#file907220line73>
> >
> > > perform an end-to-end test that includes X509AuthenticationProvider as a gate to authentication...
> >
> > But the patch set "x509" as default authentication so the SSLTest will do the same. Testing ClientBase and TestableZooKeeper isn't the goal here just because you enable secure channel on test helpers. I think it's only needed to test "new" stuff.
Okay, I can remove this one.
> On March 27, 2015, 12:27 a.m., Hongchao Deng wrote:
> > src/java/test/org/apache/zookeeper/test/SSLAuthTest.java, line 91
> > <https://reviews.apache.org/r/31963/diff/7/?file=907220#file907220line91>
> >
> > It's the same thing as testRejection(). They both have set the wrong configuration.
This however is not the same thing. testRejection() actually configures a certificate, but the certificate is from the untrustedKeyStore, so it is rejected. A handshake takes place, which does not happen for testMisconfiguration(). After the handshake, X509AuthenticationProvider rejects the unknown certificate.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/#review77988
-----------------------------------------------------------
On March 27, 2015, 12:20 a.m., Ian Dimayuga wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31963/
> -----------------------------------------------------------
>
> (Updated March 27, 2015, 12:20 a.m.)
>
>
> Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
> src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
> src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
> src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
> src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
> src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
> src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
> src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
> src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
> src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
> src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
> src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
> src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
> src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31963/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Dimayuga
>
>
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Hongchao Deng <hd...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/#review77988
-----------------------------------------------------------
src/java/test/org/apache/zookeeper/test/SSLAuthTest.java
<https://reviews.apache.org/r/31963/#comment126373>
> perform an end-to-end test that includes X509AuthenticationProvider as a gate to authentication...
But the patch set "x509" as default authentication so the SSLTest will do the same. Testing ClientBase and TestableZooKeeper isn't the goal here just because you enable secure channel on test helpers. I think it's only needed to test "new" stuff.
src/java/test/org/apache/zookeeper/test/SSLAuthTest.java
<https://reviews.apache.org/r/31963/#comment126374>
It's the same thing as testRejection(). They both have set the wrong configuration.
- Hongchao Deng
On March 27, 2015, 12:20 a.m., Ian Dimayuga wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31963/
> -----------------------------------------------------------
>
> (Updated March 27, 2015, 12:20 a.m.)
>
>
> Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
> src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
> src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
> src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
> src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
> src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
> src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
> src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
> src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
> src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
> src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
> src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
> src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
> src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31963/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Dimayuga
>
>
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Ian Dimayuga <ia...@microsoft.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/
-----------------------------------------------------------
(Updated March 27, 2015, 5:51 p.m.)
Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
Changes
-------
Rebase against trunk for final diff
Repository: zookeeper-git
Description
-------
Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
Diffs (updated)
-----
src/java/main/org/apache/zookeeper/ZooKeeperMain.java 496e88748cf6aa291c8b71583a28fdb55fdf7761
src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
src/java/main/org/apache/zookeeper/common/X509Util.java 29484486b30a6c925cb320f878778071c24b189d
src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 77cb3c5702714ce9b2d11782ca9e8f34ba9b0cc9
src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java b08fd0dc8ad238aadca07bcc9b151e04a4c470a2
src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/31963/diff/
Testing
-------
Thanks,
Ian Dimayuga
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Ian Dimayuga <ia...@microsoft.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/
-----------------------------------------------------------
(Updated March 27, 2015, 12:48 a.m.)
Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
Changes
-------
Remove redundant test case and clarify testRejection case
Repository: zookeeper-git
Description
-------
Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
Diffs (updated)
-----
src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/31963/diff/
Testing
-------
Thanks,
Ian Dimayuga
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Ian Dimayuga <ia...@microsoft.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/
-----------------------------------------------------------
(Updated March 27, 2015, 12:20 a.m.)
Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
Changes
-------
Fix formatting in X509AuthTest
Repository: zookeeper-git
Description
-------
Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
Diffs (updated)
-----
src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/31963/diff/
Testing
-------
Thanks,
Ian Dimayuga
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Ian Dimayuga <ia...@microsoft.com>.
> On March 25, 2015, 9:31 p.m., Hongchao Deng wrote:
> > src/java/test/org/apache/zookeeper/test/SSLAuthTest.java, line 73
> > <https://reviews.apache.org/r/31963/diff/6/?file=905786#file905786line73>
> >
> > I think this test is duplicate to SSLTest.
This leverages the ClientBase and TestableZooKeeper to perform an end-to-end test that includes X509AuthenticationProvider as a gate to authentication, as Rakesh requested above.
> On March 25, 2015, 9:31 p.m., Hongchao Deng wrote:
> > src/java/test/org/apache/zookeeper/test/SSLAuthTest.java, line 91
> > <https://reviews.apache.org/r/31963/diff/6/?file=905786#file905786line91>
> >
> > I don't have strong opinion.. But it seems a little unnecessary that a test didn't setup any SSL and fail.. I think it'd better add test to clariy the new features/bug fixes you made.
The SSL misconfiguration test helps show that a regression won't silently accept connections.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/#review77798
-----------------------------------------------------------
On March 25, 2015, 8:02 p.m., Ian Dimayuga wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31963/
> -----------------------------------------------------------
>
> (Updated March 25, 2015, 8:02 p.m.)
>
>
> Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
> src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
> src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
> src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
> src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
> src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
> src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
> src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
> src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
> src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
> src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
> src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
> src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
> src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31963/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Dimayuga
>
>
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Hongchao Deng <hd...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/#review77798
-----------------------------------------------------------
src/java/test/org/apache/zookeeper/test/SSLAuthTest.java
<https://reviews.apache.org/r/31963/#comment126075>
I think this test is duplicate to SSLTest.
src/java/test/org/apache/zookeeper/test/SSLAuthTest.java
<https://reviews.apache.org/r/31963/#comment126080>
I don't have strong opinion.. But it seems a little unnecessary that a test didn't setup any SSL and fail.. I think it'd better add test to clariy the new features/bug fixes you made.
src/java/test/org/apache/zookeeper/test/X509AuthTest.java
<https://reviews.apache.org/r/31963/#comment126082>
Can you move all tests up before the helper classes/functions?
- Hongchao Deng
On March 25, 2015, 8:02 p.m., Ian Dimayuga wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31963/
> -----------------------------------------------------------
>
> (Updated March 25, 2015, 8:02 p.m.)
>
>
> Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
> src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
> src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
> src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
> src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
> src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
> src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
> src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
> src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
> src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
> src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
> src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
> src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
> src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31963/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Dimayuga
>
>
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Ian Dimayuga <ia...@microsoft.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/
-----------------------------------------------------------
(Updated March 25, 2015, 8:02 p.m.)
Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
Changes
-------
Address Hongchao's comments
Repository: zookeeper-git
Description
-------
Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
Diffs (updated)
-----
src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/31963/diff/
Testing
-------
Thanks,
Ian Dimayuga
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Ian Dimayuga <ia...@microsoft.com>.
> On March 25, 2015, 6:28 p.m., Hongchao Deng wrote:
> > src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java, line 309
> > <https://reviews.apache.org/r/31963/diff/4/?file=905602#file905602line309>
> >
> > Can we get rid of certificate chain in each cnxn and pass it as authData in handleAuthentication()?
> >
> > Actually I think changing authData from byte[] to object makes more sense since we are not taking the data over network or serialized.
authData is not semantically the same as a certificate chain, since it can be passed with a ZOO_ADD_AUTH request. It does indeed come over the network which is why it's a byte[] (see org.apache.zookeeper.server.auth.AuthenticationProvider)
byte[] authData is defined in AuthenticationProvider which is meant to be extended--changing it to object would be a huge breaking change which I'm not sure is in scope here.
In this case, I'm inclined to leave the chain tied to the cnxn, since it was determined at cnxn establishment. Meanwhile the "x509" scheme authInfo is also added right after the handshake, so clients won't be calling ZOO_ADD_AUTH for x509.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/#review77766
-----------------------------------------------------------
On March 25, 2015, 8:02 p.m., Ian Dimayuga wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31963/
> -----------------------------------------------------------
>
> (Updated March 25, 2015, 8:02 p.m.)
>
>
> Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
> src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
> src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
> src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
> src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
> src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
> src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
> src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
> src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
> src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
> src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
> src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
> src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
> src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31963/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Dimayuga
>
>
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Hongchao Deng <hd...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/#review77766
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java
<https://reviews.apache.org/r/31963/#comment125963>
If we add secure ability to 4lw api, it'd better to add options in the CLI.
Such as:
FourLetterWordMain <host> <port> <cmd> <secure(optional)>
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
<https://reviews.apache.org/r/31963/#comment125969>
Please keep the parenthesis after "if":
if (...) {
}
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java
<https://reviews.apache.org/r/31963/#comment125967>
Can we get rid of certificate chain in each cnxn and pass it as authData in handleAuthentication()?
Actually I think changing authData from byte[] to object makes more sense since we are not taking the data over network or serialized.
src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
<https://reviews.apache.org/r/31963/#comment125973>
Is this duplicate to the X509Util? Can you refactor it a bit more?
- Hongchao Deng
On March 25, 2015, 6:28 p.m., Ian Dimayuga wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31963/
> -----------------------------------------------------------
>
> (Updated March 25, 2015, 6:28 p.m.)
>
>
> Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
> src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
> src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
> src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
> src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
> src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
> src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
> src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
> src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
> src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
> src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
> src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
> src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
> src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31963/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Dimayuga
>
>
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Ian Dimayuga <ia...@microsoft.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/
-----------------------------------------------------------
(Updated March 25, 2015, 6:28 p.m.)
Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
Changes
-------
Fix exception declaration and assertions
Repository: zookeeper-git
Description
-------
Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
Diffs (updated)
-----
src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/31963/diff/
Testing
-------
Thanks,
Ian Dimayuga
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Ian Dimayuga <ia...@microsoft.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/
-----------------------------------------------------------
(Updated March 25, 2015, 5:28 p.m.)
Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
Changes
-------
Fix log message format strings
Repository: zookeeper-git
Description
-------
Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
Diffs (updated)
-----
src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/31963/diff/
Testing
-------
Thanks,
Ian Dimayuga
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Rakesh R <ra...@huawei.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/#review77710
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java
<https://reviews.apache.org/r/31963/#comment125902>
NumberFormatException - can we remove this considering this as RuntimeException.
src/java/test/org/apache/zookeeper/test/SSLAuthTest.java
<https://reviews.apache.org/r/31963/#comment125903>
Sorry for not identifying this earlier. Can we make the assertion better like,
Assert.assertFalse("Missing SSL configuration should not result in successful connection", watcher.clientConnected.await(1000, TimeUnit.MILLISECONDS));
- Rakesh R
On March 23, 2015, 10:29 p.m., Ian Dimayuga wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31963/
> -----------------------------------------------------------
>
> (Updated March 23, 2015, 10:29 p.m.)
>
>
> Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
> src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
> src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
> src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
> src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
> src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
> src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
> src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
> src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
> src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
> src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
> src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
> src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
> src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31963/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Dimayuga
>
>
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Ian Dimayuga <ia...@microsoft.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/
-----------------------------------------------------------
(Updated March 23, 2015, 10:29 p.m.)
Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
Changes
-------
Fix findbugs warnings and whitespace issues
Repository: zookeeper-git
Description
-------
Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
Diffs (updated)
-----
src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/31963/diff/
Testing
-------
Thanks,
Ian Dimayuga
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Rakesh R <ra...@huawei.com>.
> On March 22, 2015, 6:14 p.m., Rakesh R wrote:
> > src/java/test/org/apache/zookeeper/test/ClientBase.java, line 251
> > <https://reviews.apache.org/r/31963/diff/2/?file=898722#file898722line251>
> >
> > Please modify the log by using {} instead of +
> >
> > LOG.error("server {} not up", hp, e);
>
> Ian Dimayuga wrote:
> I don't think there is an overload that supports logging an exception and interpolating format parameters.
>
> http://www.slf4j.org/api/org/slf4j/Logger.html#error%28java.lang.String,%20java.lang.Throwable%29
I have tested it and working fine. I can show you one reference where this kinda usage exists in our project.
class: StatisHostProvider#resolveAndShuffle(Collection<InetSocketAddress> serverAddresses)
} catch (UnknownHostException ex) {
LOG.warn("No IP address found for server: {}", address, ex);
}
- Rakesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/#review77353
-----------------------------------------------------------
On March 23, 2015, 10:29 p.m., Ian Dimayuga wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31963/
> -----------------------------------------------------------
>
> (Updated March 23, 2015, 10:29 p.m.)
>
>
> Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
> src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
> src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
> src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
> src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
> src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
> src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
> src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
> src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
> src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
> src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
> src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
> src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
> src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31963/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Dimayuga
>
>
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Ian Dimayuga <ia...@microsoft.com>.
> On March 22, 2015, 6:14 p.m., Rakesh R wrote:
> > src/java/test/org/apache/zookeeper/test/ClientBase.java, line 251
> > <https://reviews.apache.org/r/31963/diff/2/?file=898722#file898722line251>
> >
> > Please modify the log by using {} instead of +
> >
> > LOG.error("server {} not up", hp, e);
I don't think there is an overload that supports logging an exception and interpolating format parameters.
http://www.slf4j.org/api/org/slf4j/Logger.html#error%28java.lang.String,%20java.lang.Throwable%29
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/#review77353
-----------------------------------------------------------
On March 23, 2015, 10:29 p.m., Ian Dimayuga wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31963/
> -----------------------------------------------------------
>
> (Updated March 23, 2015, 10:29 p.m.)
>
>
> Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
> src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
> src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
> src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
> src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
> src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
> src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
> src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
> src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
> src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
> src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
> src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
> src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
> src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31963/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Dimayuga
>
>
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Rakesh R <ra...@huawei.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/#review77353
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
<https://reviews.apache.org/r/31963/#comment125263>
Please remove this import. This is not used, right?
src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java
<https://reviews.apache.org/r/31963/#comment125258>
Can we throw SSLContextException directly back to the user rather than cathing and doing e.printStacktrace(). This way it would get -1 in the commandline echo $?
isn't it?
src/java/test/org/apache/zookeeper/server/MockServerCnxn.java
<https://reviews.apache.org/r/31963/#comment125259>
Sorry for the late comment. Previously I didn't notice this. There is one more formatting issue and I could see similar cases few more places in te patch. Could you please correct the same.
For example,
@Override
void close() {
}
@Override
public void sendResponse(ReplyHeader h, Record r, String tag)
throws IOException {
}
@Override
void sendCloseSession() {
}
Here it requires a newline between the methods like,
@Override
void close() {
}
@Override
public void sendResponse(ReplyHeader h, Record r, String tag)
throws IOException {
}
@Override
void sendCloseSession() {
}
src/java/test/org/apache/zookeeper/test/ClientBase.java
<https://reviews.apache.org/r/31963/#comment125262>
please add one empty line before this method.
src/java/test/org/apache/zookeeper/test/ClientBase.java
<https://reviews.apache.org/r/31963/#comment125261>
Please modify the log by using {} instead of +
LOG.error("server {} not up", hp, e);
src/java/test/org/apache/zookeeper/test/ClientBase.java
<https://reviews.apache.org/r/31963/#comment125260>
please add one empty line before this method.
- Rakesh R
On March 18, 2015, 4:34 a.m., Ian Dimayuga wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31963/
> -----------------------------------------------------------
>
> (Updated March 18, 2015, 4:34 a.m.)
>
>
> Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
> src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
> src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
> src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
> src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
> src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
> src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
> src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
> src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
> src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
> src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
> src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
> src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
> src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
> src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
> src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/31963/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ian Dimayuga
>
>
Re: Review Request 31963: Patch in X509AuthenticationProvider on top
of ZOOKEEPER-2125
Posted by Ian Dimayuga <ia...@microsoft.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31963/
-----------------------------------------------------------
(Updated March 18, 2015, 4:34 a.m.)
Review request for zookeeper, fpj, Hongchao Deng, and Rakesh R.
Changes
-------
Address Rakesh's comments
Improve error handling
Add untrusted key for testing
Add end-to-end SSL X509 authentication tests
Allow for extensibility in authentication providers on the server
Repository: zookeeper-git
Description
-------
Patch in X509AuthenticationProvider on top of ZOOKEEPER-2125
Diffs (updated)
-----
src/java/main/org/apache/zookeeper/ZooKeeperMain.java 83273d3407f08a95dbd78a3e2b6a4f031bd956d7
src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java e41465ab93a3a59dbced8294e83b1651ad0dfe69
src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION
src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java e02753f4fb926a8cc6c7a7c10af42f949c1e210c
src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java b4bdc82f8b52f736c6ee3d67bb793a3616c1b436
src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java 09a794844978456fc3580adc22b6064e2a12cf77
src/java/main/org/apache/zookeeper/server/ServerCnxn.java a47d85662970cc0c219a46b226737a8689f8fe96
src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 14037722c569d560acef56de0b5a7ae13464128c
src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java 406015f84a51e6afcfe704b881f8494bdd687a25
src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java PRE-CREATION
src/java/test/data/ssl/testUntrustedKeyStore.jks PRE-CREATION
src/java/test/org/apache/zookeeper/server/MockServerCnxn.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/ClientBase.java 8915dfc81d286d5114d47f5781c62b29231f7774
src/java/test/org/apache/zookeeper/test/FourLetterWordsQuorumTest.java 49d90f76e07e088009d30ceff3ad1597d19c31d4
src/java/test/org/apache/zookeeper/test/FourLetterWordsTest.java 281b1786954993618ba7085897b742349acdd7c0
src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION
src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/31963/diff/
Testing
-------
Thanks,
Ian Dimayuga