[
https://issues.apache.org/jira/browse/ZOOKEEPER-3905?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17171000#comment-17171000
]
Karan Mehta commented on ZOOKEEPER-3905:
----------------------------------------
[~andor] Let me clarify a few more things.
{quote}Question: is it possible to provide a cerificate which is trusted by the
server, handshake completes and later {{CerificateVerifier}} fails to
authenticate?
{quote}
Yes. Actually, we have added some more code in CerificateVerifier class for
authorization of clients. This is where it is failing. We do this check _only
if_ certificate is validated to be good (SSL level checks are passed)
{code:java}
String clientId = getClientId(clientCert);
boolean authZResult = authorizeClient(clientId);
if (!authZResult) {
LOG.error("Client id {} doesn't match the allowedList", clientId);
return KeeperException.Code.AUTHFAILED;
}
{code}
{quote}This is very odd, I cannot repro it with a real cluster/client.
{quote}
Actually, I saw this issue on a real cluster only. I didn't have any easy way
to reproduce this, so hacked up the code in this manner.
{quote}Exception is thrown later when the client timeouts, because while the
server is processing requests, it's unable to response to a non-registered
client. Weird.
{quote}
What I expected is that returning AUTHFAILED, would immediately terminate that
connection (and even send that response code back to client). Yes, what you
said is exactly right.
What do you think should be next steps here?
> 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
> 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)
