ZOOKEEPER-3893: Enhance documentation for property ssl.clientAuth

[sankalpbhatia]

Currently, the documentation for this property just mentions TBD (check https://zookeeper.apache.org/doc/r3.5.7/zookeeperAdmin.html). I believe this was ignored until versions 3.5.7 (https://issues.apache.org/jira/browse/ZOOKEEPER-3674) and a fix was made. It will be good to add some documentation around it.

[sankalpbhatia]

I changed this to 3.5.7 because I believe this property was ignored until that version.
Please correct me if wrong. https://issues.apache.org/jira/browse/ZOOKEEPER-3674

[symat]

actually the original feature was present since 3.5.5, it was added in: https://issues.apache.org/jira/browse/ZOOKEEPER-3176

In ZOOKEEPER-3674 it is said to not work. But I don't know if it was actually true. I found ZOOKEEPER-3388 what was actually released in 5.7 and touched this part. Maybe this is why it is said to be fixed in 3.5.7? I really don't know.

With regards to the documentation, I'm happy to keep 3.5.5, unless someone can try with 3.5.5 and 3.5.6 to see if it is really broken.

[sankalpbhatia]

Thanks for your comment @symat . Let me try this out for 3.5.5 and 3.5.6 and will get back.

[symat]

that would be really valuable, thank you for taking the time on these tests!

[sankalpbhatia]

Sorry for the delay @symat. I was able to take some time and verify that this doesn't work with versions 3.5.5 and 3.5.6

Here are the steps we can use to reproduce it with the following zk cfg

dataDir=/usr/local/var/lib/zookeeper
secureClientPort=2182
clientPort=2181
maxClientCnxns=0
admin.enableServer=false
serverCnxnFactory=org.apache.zookeeper.server.NettyServerCnxnFactory

ssl.clientAuth=none
ssl.keyStore.location=/path/to/keystore
ssl.keyStore.password=password
ssl.trustStore.location=/path/to/truststore
ssl.trustStore.password=password
serverCnxnFactory=org.apache.zookeeper.server.NettyServerCnxnFactory

1. build the 3.5.5 source using command 
   git checkout release-3.5.5 && mvn clean install -DskipTests

2. Start zk server

bin/zkServer.sh start-foreground /usr/local/Cellar/kafka/2.4.0/.bottle/etc/kafka/zookeeper.properties 

3. Try connecting using client 

/Volumes/workplace/zookeeper/bin(f0fdd529) » export CLIENT_JVMFLAGS="
-Dzookeeper.clientCnxnSocket=org.apache.zookeeper.ClientCnxnSocketNetty
-Dzookeeper.client.secure=true
-Dzookeeper.ssl.hostnameVerification=false
-Dzookeeper.ssl.trustStore.location=/path/to/truststore
-Dzookeeper.ssl.trustStore.password=changeit"

./zkCli.sh -server localhost:2182

fails with exception 

Exception caught
io.netty.handler.codec.DecoderException: javax.net.ssl.SSLHandshakeException: null cert chain

We get the same error for building with 3.5.6. Building and running the 3.5.7 source however, works with the same steps. 

I did a bit of deep dive on the code and verified it was ZOOKEEPER-3388 which fixed this issue indeed. 

The initSSL() method prior to ZK-3388 was always setting clientAuth=required through the code sslEngine.setNeedClientAuth(true);(Check PR for ZK-3388). The pull request changed it to use the property derived from the config file. 

To summarise, I think it is safe to say that this works only from 3.5.7 onwards only.

[ctubbsii]

To make the historical record perfectly clear, you could have something like:

Suggested change

	**New in 3.5.7:**
	**Added in 3.5.5, but broken until 3.5.7:**

[sankalpbhatia]

Thanks for the suggestion. Changed

[eolivelli]

LGTM
thank you !

@symat can you please take a look an merge ?
this change is to be picked up to 3.5 branch IMO

[symat]

great, thanks @sankalpbhatia, I'll merge this today / soon