[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-3905?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17170820#comment-17170820
 ] 

Andor Molnar commented on ZOOKEEPER-3905:
-----------------------------------------

Okay, it was long time ago, but I see the following which might be an issue 
here. In the test you mentioned the problem is that although client certificate 
is not trusted (hence the patch to force it), the test is able to create ZK 
client successfully and establishing a session.

Exception is thrown later when the client timeouts, because while the server is 
processing requests, it's unable to response to a non-registered client. Weird.

I'm trying to repro it with a "real" server.

> Race condition causes sessions to be created for clients even though their 
> certificate authentication has failed
> ----------------------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-3905
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3905
>             Project: ZooKeeper
>          Issue Type: Bug
>    Affects Versions: 3.5.8
>            Reporter: Karan Mehta
>            Priority: Major
>
> To reproduce, apply the diff and run 
> ClientSSLTest#testSecureStandaloneServer() test. The logs would show that a 
> valid session was created before connection was rejected and client had to 
> retry
>  
> {code:java}
> diff --git 
> a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKTrustManager.java
>  
> b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKTrustManager.java
>  index aa02145b2..df1bdcc0f 100644
>  — 
> a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKTrustManager.java
>  +++ 
> b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKTrustManager.java
>  @@ -111,6 +111,7 @@ public void checkServerTrusted(X509Certificate[] chain, 
> String authType, SSLEngi
>  @Override
>  public void checkClientTrusted(X509Certificate[] chain, String authType) 
> throws CertificateException
> { x509ExtendedTrustManager.checkClientTrusted(chain, authType); + throw new 
> CertificateException(); }
> @Override
> {code}
>   
>  What should have happened:
>  Server should instantly close the client's connection and NOT process any 
> request.
>   
>  Potential threat:
>  Malicious clients may be able to put unnecessary load/traffic on the leader 
> by creating these sessions.
>   
>  Race Condition:
>  In CertificateVerifier#operationComplete(), `addCnxn(cnxn)` method is only 
> called after auth is completed. NettyServerCnxn#close() returns as a no-op 
> [here|https://github.com/apache/zookeeper/blob/branch-3.5/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java#L105].
>   
>  I see this as an issue. Please assess the risk and let me know if this is a 
> legit behavior or not.
>   



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to