[jira] [Comment Edited] (ZOOKEEPER-3905) Race condition causes sessions to be created for clients even though their certificate authentication has failed

["Andor Molnar (Jira)" <ji...@apache.org>]

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

Andor Molnar edited comment on ZOOKEEPER-3905 at 8/4/20, 2:19 PM:
------------------------------------------------------------------

_"it's unable to response to a non-registered client"_

ConnectRequest/ConnectResponse is different, because ZookeeperServer doesn't care about _channelClosing_ flag and sends the response to the buffer no matter what. :)


was (Author: andorm):
_"it's unable to response to a non-registered client"_

__ConnectRequest/ConnectResponse is different, because ZookeeperServer doesn't care about _channelClosing_ flag and sends the response to the buffer no matter what. :)

> 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)