[jira] [Comment Edited] (ZOOKEEPER-3905) Race condition causes sessions to be created for clients even though their certificate authentication has failed
[
https://issues.apache.org/jira/browse/ZOOKEEPER-3905?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17172387#comment-17172387
]
Mate Szalay-Beko edited comment on ZOOKEEPER-3905 at 8/6/20, 1:57 PM:
--
does this error happen only when custom authentication provider is defined
together with SSL?
Is SASL a custom provider in this case? Does the error happen with failed SASL
authentication when SSL is also used?
was (Author: symat):
does this error happens only when custom authentication provider is defined
with SSL?
Is SASL a custom provider in this case? Does the error happen with failed SASL
authentication when SSL is also used?
> 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, 3.6.2
>Reporter: Karan Mehta
>Assignee: Andor Molnar
>Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
> 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)
[jira] [Comment Edited] (ZOOKEEPER-3905) Race condition causes sessions to be created for clients even though their certificate authentication has failed
[
https://issues.apache.org/jira/browse/ZOOKEEPER-3905?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)
