You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@zookeeper.apache.org by "Karan Mehta (Jira)" <ji...@apache.org> on 2020/08/03 20:03:01 UTC
[jira] [Updated] (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:all-tabpanel ]
Karan Mehta updated ZOOKEEPER-3905:
-----------------------------------
Description:
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.
was:
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.
> 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)