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

2020-08-12 Thread Mate Szalay-Beko (Jira) 


 [ 
https://issues.apache.org/jira/browse/ZOOKEEPER-3905?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Mate Szalay-Beko updated ZOOKEEPER-3905:

Fix Version/s: 3.5.9
   3.6.2

> 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
> Fix For: 3.7.0, 3.6.2, 3.5.9
>
>  Time Spent: 5.5h
>  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] [Updated] (ZOOKEEPER-3905) Race condition causes sessions to be created for clients even though their certificate authentication has failed

2020-08-08 Thread Enrico Olivelli (Jira) 


 [ 
https://issues.apache.org/jira/browse/ZOOKEEPER-3905?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Enrico Olivelli updated ZOOKEEPER-3905:
---
Fix Version/s: 3.7.0

> 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
> Fix For: 3.7.0
>
>  Time Spent: 3h 40m
>  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] [Updated] (ZOOKEEPER-3905) Race condition causes sessions to be created for clients even though their certificate authentication has failed

2020-08-04 Thread Karan Mehta (Jira) 


 [ 
https://issues.apache.org/jira/browse/ZOOKEEPER-3905?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Karan Mehta updated ZOOKEEPER-3905:
---
Affects Version/s: 3.6.2

> 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
>
> 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] [Updated] (ZOOKEEPER-3905) Race condition causes sessions to be created for clients even though their certificate authentication has failed

2020-08-03 Thread Karan Mehta (Jira) 


 [ 
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