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

Grzegorz Grzybek updated ZOOKEEPER-3138:
----------------------------------------
    Description: 
I'm in the process of reconfiguring the ensemble to use mutual quorum peer 
authentication using SASL (ZOOKEEPER-1045).

In order to understand the impact on my code, I've checked _how it works_. Now 
I'm running & debugging 
{{org.apache.zookeeper.server.quorum.auth.QuorumDigestAuthTest#testValidCredentials()}}
 test case.

I have now six threads (3 peers contacting each other):
* "QuorumConnectionThread-[myid=0]-2@1483" prio=5 tid=0x2b nid=NA runnable
* "QuorumConnectionThread-[myid=0]-3@1491" prio=5 tid=0x36 nid=NA runnable
* "QuorumConnectionThread-[myid=1]-1@1481" prio=5 tid=0x2d nid=NA runnable
* "QuorumConnectionThread-[myid=1]-4@1505" prio=5 tid=0x3c nid=NA runnable
* "QuorumConnectionThread-[myid=2]-2@1495" prio=5 tid=0x37 nid=NA runnable
* "QuorumConnectionThread-[myid=2]-4@1506" prio=5 tid=0x3d nid=NA runnable

at this point of invocation:
{noformat}
  java.lang.Thread.State: RUNNABLE
          at 
org.apache.zookeeper.server.quorum.auth.SaslQuorumServerCallbackHandler.handleNameCallback(SaslQuorumServerCallbackHandler.java:101)
          at 
org.apache.zookeeper.server.quorum.auth.SaslQuorumServerCallbackHandler.handle(SaslQuorumServerCallbackHandler.java:82)
          at 
com.sun.security.sasl.digest.DigestMD5Server.validateClientResponse(DigestMD5Server.java:589)
          at 
com.sun.security.sasl.digest.DigestMD5Server.evaluateResponse(DigestMD5Server.java:244)
          at 
org.apache.zookeeper.server.quorum.auth.SaslQuorumAuthServer.authenticate(SaslQuorumAuthServer.java:100)
          at 
org.apache.zookeeper.server.quorum.QuorumCnxManager.handleConnection(QuorumCnxManager.java:467)
          at 
org.apache.zookeeper.server.quorum.QuorumCnxManager.receiveConnection(QuorumCnxManager.java:386)
          at 
org.apache.zookeeper.server.quorum.QuorumCnxManager$QuorumConnectionReceiverThread.run(QuorumCnxManager.java:422)
          at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
          at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
          at java.lang.Thread.run(Thread.java:748)
{noformat}

which is this line:
{code:java}
private void handleNameCallback(NameCallback nc) {
    // check to see if this user is in the user password database.
    if (credentials.get(nc.getDefaultName()) == null) {
        LOG.warn("User '{}' not found in list of DIGEST-MD5 authenticateable 
users.",
                nc.getDefaultName());
        return;
    }
    nc.setName(nc.getDefaultName());
    /* >>> */ userName = nc.getDefaultName(); /* <<< */
}
{code}

each pair of threads is operating on single instance of 
{{org.apache.zookeeper.server.quorum.auth.SaslQuorumServerCallbackHandler#userName}}.
 In the stack trace we have both shared and local variables/fields:
* {{o.a.z.server.quorum.QuorumCnxManager.QuorumConnectionReceiverThread#sock}} 
is thread-specific (ok)
* {{o.a.z.server.quorum.QuorumCnxManager#authServer}} is peer-specific 
(instance of {{o.a.z.server.quorum.auth.SaslQuorumAuthServer}}) but without a 
state that changes
* {{javax.security.sasl.SaslServer}} is thread-specific (ok) - this instance is 
created to handle sasl authentication, but is created using peer-specific JAAS 
subject (which is ok) and peer-specific 
{{o.a.z.server.quorum.auth.SaslQuorumAuthServer#serverLogin.callbackHadler}} 
{color:red}which is potentially a problem{color}

Each (out of six) thread handles different connection, but each pair (for given 
QuorumPeer) calls 
{{o.a.z.server.quorum.auth.SaslQuorumServerCallbackHandler#handleNameCallback()}}
 which modifies shared (peer-specific) field - {{userName}}.

I understand that [according to the example from 
Wiki|https://cwiki.apache.org/confluence/display/ZOOKEEPER/Server-Server+mutual+authentication]
 all peers may use the same credentials (in simplest case).

But the "userName" comes from data sent by each peer, like this:
{noformat}
charset=utf-8,\
username="test",\
realm="zk-quorum-sasl-md5",\
nonce="iBqYWtaCrEE013S6Dv6xiOsR9uX2l/qKZcEZ1pm2",\
nc=00000001,\
cnonce="LVaL9XYFjNxVBPCjPewXjEBsj9GuwIfBN/RXsKt5",\
digest-uri="zookeeper-quorum/zk-quorum-sasl-md5",\
maxbuf=65536,\
response=dd4e9e2115ec2e304484d5191f3fc771,\
qop=auth,\
authzid="test"
{noformat}

*And I can imagine such JAAS configuration for DIGEST-MD5 SASL algorithm, that 
each peer uses own credentials and is able to validate other peers' specific 
credentials.*:
{noformat}
QuorumServer {
       org.apache.zookeeper.server.auth.DigestLoginModule required
       user_peer1="peer1";
       user_peer2="peer2";
       user_peer3="peer3";
};
QuorumLearner1 {
       org.apache.zookeeper.server.auth.DigestLoginModule required
       username="peer1"
       password="peer1";
};
...
QuorumLearner2 {
       org.apache.zookeeper.server.auth.DigestLoginModule required
       username="peer2"
       password="peer2";
};
{noformat}

Isn't it a race condition? Like this (having 3 peers):
||thread handling peer 2 → peer 1 connection||thread handling peer 3 → peer 1 
connection||
|sets o.a.z.s.q.auth.SaslQuorumServerCallbackHandler#userName to "peer2"| |
| |sets o.a.z.s.q.auth.SaslQuorumServerCallbackHandler#userName to "peer3"|
|sets PasswordCallback.password to 
o.a.z.s.q.auth.SaslQuorumServerCallbackHandler#credentials.get("peer3")| |
| | continues ...|
|com.sun.security.sasl.digest.DigestMD5Base#generateResponseValue() generates 
expected response using:
* username: "peer2"
* password of user "peer3"| |

Please verify.

  was:
I'm in the process of reconfiguring the ensemble to use mutual quorum peer 
authentication using SASL (ZOOKEEPER-1045).

In order to understand the impact on my code, I've checked _how it works_. Now 
I'm running & debugging 
{{org.apache.zookeeper.server.quorum.auth.QuorumDigestAuthTest#testValidCredentials()}}
 test case.

I have now six threads (3 peers contacting each other):
* "QuorumConnectionThread-[myid=0]-2@1483" prio=5 tid=0x2b nid=NA runnable
* "QuorumConnectionThread-[myid=0]-3@1491" prio=5 tid=0x36 nid=NA runnable
* "QuorumConnectionThread-[myid=1]-1@1481" prio=5 tid=0x2d nid=NA runnable
* "QuorumConnectionThread-[myid=1]-4@1505" prio=5 tid=0x3c nid=NA runnable
* "QuorumConnectionThread-[myid=2]-2@1495" prio=5 tid=0x37 nid=NA runnable
* "QuorumConnectionThread-[myid=2]-4@1506" prio=5 tid=0x3d nid=NA runnable

at this point of invocation:
{noformat}
  java.lang.Thread.State: RUNNABLE
          at 
org.apache.zookeeper.server.quorum.auth.SaslQuorumServerCallbackHandler.handleNameCallback(SaslQuorumServerCallbackHandler.java:101)
          at 
org.apache.zookeeper.server.quorum.auth.SaslQuorumServerCallbackHandler.handle(SaslQuorumServerCallbackHandler.java:82)
          at 
com.sun.security.sasl.digest.DigestMD5Server.validateClientResponse(DigestMD5Server.java:589)
          at 
com.sun.security.sasl.digest.DigestMD5Server.evaluateResponse(DigestMD5Server.java:244)
          at 
org.apache.zookeeper.server.quorum.auth.SaslQuorumAuthServer.authenticate(SaslQuorumAuthServer.java:100)
          at 
org.apache.zookeeper.server.quorum.QuorumCnxManager.handleConnection(QuorumCnxManager.java:467)
          at 
org.apache.zookeeper.server.quorum.QuorumCnxManager.receiveConnection(QuorumCnxManager.java:386)
          at 
org.apache.zookeeper.server.quorum.QuorumCnxManager$QuorumConnectionReceiverThread.run(QuorumCnxManager.java:422)
          at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
          at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
          at java.lang.Thread.run(Thread.java:748)
{noformat}

which is this line:
{code:java}
private void handleNameCallback(NameCallback nc) {
    // check to see if this user is in the user password database.
    if (credentials.get(nc.getDefaultName()) == null) {
        LOG.warn("User '{}' not found in list of DIGEST-MD5 authenticateable 
users.",
                nc.getDefaultName());
        return;
    }
    nc.setName(nc.getDefaultName());
    /* >>> */ userName = nc.getDefaultName(); /* <<< */
}
{code}

each pair of threads is operating on single instance of 
{{org.apache.zookeeper.server.quorum.auth.SaslQuorumServerCallbackHandler#userName}}.
 In the stack trace we have both shared and local variables/fields:
* {{o.a.z.server.quorum.QuorumCnxManager.QuorumConnectionReceiverThread#sock}} 
is thread-specific (ok)
* {{o.a.z.server.quorum.QuorumCnxManager#authServer}} is peer-specific 
(instance of {{o.a.z.server.quorum.auth.SaslQuorumAuthServer}}) but without a 
state that changes
* {{javax.security.sasl.SaslServer}} is thread-specific (ok) - this instance is 
created to handle sasl authentication, but is created using peer-specific JAAS 
subject (which is ok) and peer-specific 
{{o.a.z.server.quorum.auth.SaslQuorumAuthServer#serverLogin.callbackHadler}} 
{color:red}which is potentially a problem{color}

Each (out of six) thread handles different connection, but each pair (for given 
QuorumPeer) calls 
{{o.a.z.server.quorum.auth.SaslQuorumServerCallbackHandler#handleNameCallback()}}
 which modifies shared (peer-specific) field - {{userName}}.

I understand that [according to the example from 
Wiki|https://cwiki.apache.org/confluence/display/ZOOKEEPER/Server-Server+mutual+authentication]:
{noformat}
QuorumServer {
       org.apache.zookeeper.server.auth.DigestLoginModule required
       user_peer1="peer1";
       user_peer2="peer2";
       user_peer3="peer3";
};
QuorumLearner1 {
       org.apache.zookeeper.server.auth.DigestLoginModule required
       username="peer1"
       password="peer1";
};
...
{noformat}

All peers use the same credentials, but the "userName" comes from data sent by 
each peer, like this:
{noformat}
charset=utf-8,\
username="test",\
realm="zk-quorum-sasl-md5",\
nonce="iBqYWtaCrEE013S6Dv6xiOsR9uX2l/qKZcEZ1pm2",\
nc=00000001,\
cnonce="LVaL9XYFjNxVBPCjPewXjEBsj9GuwIfBN/RXsKt5",\
digest-uri="zookeeper-quorum/zk-quorum-sasl-md5",\
maxbuf=65536,\
response=dd4e9e2115ec2e304484d5191f3fc771,\
qop=auth,\
authzid="test"
{noformat}

*And I can imagine such JAAS configuration for DIGEST-MD5 SASL algorithm, that 
each peer uses own credentials and is able to validate other peers' specific 
credentials.*

Isn't it a race condition? Like this (having 3 peers):
||thread handling peer 2 → peer 1 connection||thread handling peer 3 → peer 1 
connection||
|sets o.a.z.s.q.auth.SaslQuorumServerCallbackHandler#userName to "peer2"| |
| |sets o.a.z.s.q.auth.SaslQuorumServerCallbackHandler#userName to "peer3"|
|sets PasswordCallback.password to 
o.a.z.s.q.auth.SaslQuorumServerCallbackHandler#credentials.get("peer3")| |
| | continues ...|
|com.sun.security.sasl.digest.DigestMD5Base#generateResponseValue() generates 
expected response using:
* username: "peer2"
* password of user "peer3"| |

Please verify.


> Potential race condition with Quorum Peer mutual authentication via SASL
> ------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-3138
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3138
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: quorum
>    Affects Versions: 3.4.13
>            Reporter: Grzegorz Grzybek
>            Priority: Major
>
> I'm in the process of reconfiguring the ensemble to use mutual quorum peer 
> authentication using SASL (ZOOKEEPER-1045).
> In order to understand the impact on my code, I've checked _how it works_. 
> Now I'm running & debugging 
> {{org.apache.zookeeper.server.quorum.auth.QuorumDigestAuthTest#testValidCredentials()}}
>  test case.
> I have now six threads (3 peers contacting each other):
> * "QuorumConnectionThread-[myid=0]-2@1483" prio=5 tid=0x2b nid=NA runnable
> * "QuorumConnectionThread-[myid=0]-3@1491" prio=5 tid=0x36 nid=NA runnable
> * "QuorumConnectionThread-[myid=1]-1@1481" prio=5 tid=0x2d nid=NA runnable
> * "QuorumConnectionThread-[myid=1]-4@1505" prio=5 tid=0x3c nid=NA runnable
> * "QuorumConnectionThread-[myid=2]-2@1495" prio=5 tid=0x37 nid=NA runnable
> * "QuorumConnectionThread-[myid=2]-4@1506" prio=5 tid=0x3d nid=NA runnable
> at this point of invocation:
> {noformat}
>   java.lang.Thread.State: RUNNABLE
>         at 
> org.apache.zookeeper.server.quorum.auth.SaslQuorumServerCallbackHandler.handleNameCallback(SaslQuorumServerCallbackHandler.java:101)
>         at 
> org.apache.zookeeper.server.quorum.auth.SaslQuorumServerCallbackHandler.handle(SaslQuorumServerCallbackHandler.java:82)
>         at 
> com.sun.security.sasl.digest.DigestMD5Server.validateClientResponse(DigestMD5Server.java:589)
>         at 
> com.sun.security.sasl.digest.DigestMD5Server.evaluateResponse(DigestMD5Server.java:244)
>         at 
> org.apache.zookeeper.server.quorum.auth.SaslQuorumAuthServer.authenticate(SaslQuorumAuthServer.java:100)
>         at 
> org.apache.zookeeper.server.quorum.QuorumCnxManager.handleConnection(QuorumCnxManager.java:467)
>         at 
> org.apache.zookeeper.server.quorum.QuorumCnxManager.receiveConnection(QuorumCnxManager.java:386)
>         at 
> org.apache.zookeeper.server.quorum.QuorumCnxManager$QuorumConnectionReceiverThread.run(QuorumCnxManager.java:422)
>         at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
>         at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
>         at java.lang.Thread.run(Thread.java:748)
> {noformat}
> which is this line:
> {code:java}
> private void handleNameCallback(NameCallback nc) {
>     // check to see if this user is in the user password database.
>     if (credentials.get(nc.getDefaultName()) == null) {
>         LOG.warn("User '{}' not found in list of DIGEST-MD5 authenticateable 
> users.",
>                 nc.getDefaultName());
>         return;
>     }
>     nc.setName(nc.getDefaultName());
>     /* >>> */ userName = nc.getDefaultName(); /* <<< */
> }
> {code}
> each pair of threads is operating on single instance of 
> {{org.apache.zookeeper.server.quorum.auth.SaslQuorumServerCallbackHandler#userName}}.
>  In the stack trace we have both shared and local variables/fields:
> * 
> {{o.a.z.server.quorum.QuorumCnxManager.QuorumConnectionReceiverThread#sock}} 
> is thread-specific (ok)
> * {{o.a.z.server.quorum.QuorumCnxManager#authServer}} is peer-specific 
> (instance of {{o.a.z.server.quorum.auth.SaslQuorumAuthServer}}) but without a 
> state that changes
> * {{javax.security.sasl.SaslServer}} is thread-specific (ok) - this instance 
> is created to handle sasl authentication, but is created using peer-specific 
> JAAS subject (which is ok) and peer-specific 
> {{o.a.z.server.quorum.auth.SaslQuorumAuthServer#serverLogin.callbackHadler}} 
> {color:red}which is potentially a problem{color}
> Each (out of six) thread handles different connection, but each pair (for 
> given QuorumPeer) calls 
> {{o.a.z.server.quorum.auth.SaslQuorumServerCallbackHandler#handleNameCallback()}}
>  which modifies shared (peer-specific) field - {{userName}}.
> I understand that [according to the example from 
> Wiki|https://cwiki.apache.org/confluence/display/ZOOKEEPER/Server-Server+mutual+authentication]
>  all peers may use the same credentials (in simplest case).
> But the "userName" comes from data sent by each peer, like this:
> {noformat}
> charset=utf-8,\
> username="test",\
> realm="zk-quorum-sasl-md5",\
> nonce="iBqYWtaCrEE013S6Dv6xiOsR9uX2l/qKZcEZ1pm2",\
> nc=00000001,\
> cnonce="LVaL9XYFjNxVBPCjPewXjEBsj9GuwIfBN/RXsKt5",\
> digest-uri="zookeeper-quorum/zk-quorum-sasl-md5",\
> maxbuf=65536,\
> response=dd4e9e2115ec2e304484d5191f3fc771,\
> qop=auth,\
> authzid="test"
> {noformat}
> *And I can imagine such JAAS configuration for DIGEST-MD5 SASL algorithm, 
> that each peer uses own credentials and is able to validate other peers' 
> specific credentials.*:
> {noformat}
> QuorumServer {
>        org.apache.zookeeper.server.auth.DigestLoginModule required
>        user_peer1="peer1";
>        user_peer2="peer2";
>        user_peer3="peer3";
> };
> QuorumLearner1 {
>        org.apache.zookeeper.server.auth.DigestLoginModule required
>        username="peer1"
>        password="peer1";
> };
> ...
> QuorumLearner2 {
>        org.apache.zookeeper.server.auth.DigestLoginModule required
>        username="peer2"
>        password="peer2";
> };
> {noformat}
> Isn't it a race condition? Like this (having 3 peers):
> ||thread handling peer 2 → peer 1 connection||thread handling peer 3 → peer 1 
> connection||
> |sets o.a.z.s.q.auth.SaslQuorumServerCallbackHandler#userName to "peer2"| |
> | |sets o.a.z.s.q.auth.SaslQuorumServerCallbackHandler#userName to "peer3"|
> |sets PasswordCallback.password to 
> o.a.z.s.q.auth.SaslQuorumServerCallbackHandler#credentials.get("peer3")| |
> | | continues ...|
> |com.sun.security.sasl.digest.DigestMD5Base#generateResponseValue() generates 
> expected response using:
> * username: "peer2"
> * password of user "peer3"| |
> Please verify.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to