[
https://issues.apache.org/jira/browse/ZOOKEEPER-3905?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17171158#comment-17171158
]
Karan Mehta commented on ZOOKEEPER-3905:
----------------------------------------
[~andor] yes, thats exactly what we do. We use a custom X509AuthProvider. It is
a light fork from branch-3.5. It extends X509AuthenticationProvider class, with
a small business logic for authorization (which I posted in code above). It was
needed to address our internal security requirements.
{quote}That means you have modified ZooKeeper's source code which essentially
created this problem. This is not supported and we don't have anything to fix
here. We might double check the certificate in this process and
CertificateVerifier is redundant.
{quote}
Makes sense, this piece of code was added a long time ago and things might have
changed since. If this isn't supported, how should we address it?
> 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)
