[jboss-jira] [JBoss JIRA] (JGRP-1852) SASL challenge-response cycle does not process challenges

[Richard Achmatowicz (JIRA)]

    [ https://issues.jboss.org/browse/JGRP-1852?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12976216#comment-12976216 ] 

Richard Achmatowicz edited comment on JGRP-1852 at 6/13/14 4:23 PM:
--------------------------------------------------------------------

I'm not 100% confident about the termination conditions of this implementation as it stands and I think it could be improved by restructuring the implementation a little.
For reference, here is the LDAP-based example provided in the SASL code:.
{noformat}
client
------
SaslClient sc = Sasl.createSaslClient(mechs,authId,protocol,serverName,props,cbh);

// get initial response, send first message *and* get challenge from the server
byte[] response = (sc.hasInitialResponse() ? sc.evaluateChallenge(new byte[0]) : null);
LdapResult res = ldap.sendBindRequest(dn, sc.getname(), response);

// check if we need to continue
while (!sc.isComplete() && (res.status == SASL_BIND_IN_PROGRESS || res.status == SUCCESS)) {
    response = sc.evaluateChallenge(res.getBytes());

    // this indicates that we do not need to send another bind request (this should have been set on the basis of complete)
    if (res.status == SUCCESS) {
        // we're done; don't expect to send another BIND
        if (response != null) {
           throw new SaslException("Protocol error: attempting to send response after completion");
        }
        break;
    }
    res = ldap.sendBindRequest(dn, sc.getName(), response);
}

// authentication has finished - check if we need to encrypt subsequent messages
 if (sc.isComplete() && res.status == SUCCESS) {
    String qop = (String) sc.getNegotiatedProperty(Sasl.QOP);
    if (qop != null && (qop.equalsIgnoreCase("auth-int") || qop.equalsIgnoreCase("auth-conf"))) {
      // Use SaslClient.wrap() and SaslClient.unwrap() for future communication with server
      ldap.in = new SecureInputStream(sc, ldap.in);
      ldap.out = new SecureOutputStream(sc, ldap.out);
    }
 }


server
------
// get the initial bind request and create the server mechanism
response = ldap.readBindRequest();
SaslServer ss = Sasl.createSaslServer(mechanism,"ldap", myFQDN, props, callbackHandler);

// start to process responses from the client
while (!ss.isComplete()) {
    try {
        byte[] challenge = ss.evaluateResponse(response);
        if (ss.isComplete()) {
	    // when the authetication is complete, we can send the bind response SUCCESS and terminate 
            status = ldap.sendBindResponse(mechanism, challenge, SUCCESS);
        } else { 
	    // when authentication is not complete, we send an in_progress message and continue reading
            status = ldap.sendBindResponse(mechanism, challenge, SASL_BIND_IN_PROGRESS);
            response = ldap.readBindRequest();
        }
    } catch (SaslException e) {
        status = ldap.sendErrorResponse(e);
        break;
    }
}
// authenticatoion has completed - now check if we have negotiated encryption - but only if successful bind
if (ss.isComplete() && status == SUCCESS) {
   String qop = (String) sc.getNegotiatedProperty(Sasl.QOP);
   if (qop != null && (qop.equalsIgnoreCase("auth-int") || qop.equalsIgnoreCase("auth-conf"))) {
     // Use SaslServer.wrap() and SaslServer.unwrap() for future communication with client
     ldap.in = new SecureInputStream(ss, ldap.in);
     ldap.out = new SecureOutputStream(ss, ldap.out);
   }
}
{noformat}

Tristan's implementation roughly works like this:
- on the client side, add a SaslHeader to the original JOIN_REQ with the initial response, if any
- when the server receives the JOIN_REQ with the SaslHeader,  block on a latch 
- start the challenge-response cycle exchanging messages which only include a SaslHeader
- check for termination on the server side and when termination or an exception occurs, release the latch and send a JOIN_RSP indicating success or send a JOIN_RSP indicating failure

This makes it a little tricky to mimic the control flow and termination checking as in the example above, which uses both the isComplete() status as well as the SUCCESS/SASL_BIND_IN_PROGRESS markers on the bind requests/replies to ensure correct termination.

What we could do instead is to exchange JOIN_REQ / JOIN_RSP messages between client and server at each exchange. The presence of a SaslHeader would indicate that the SASL exchange has not yet completed - if a client receives a JOIN_RSP with a SaslHeader, then it needs to process that header before making a decision about whether or not the process had completed and pass the JOIN_RSP up the stack. I'm going to try to have a go at reworking the implementation in that way and so the code mimics the example above. I'm hoping that this will simplify the exception handling as well and purging of contexts no longer needed.

Also, in order to support encryption and the use of the SASL "integrity" and "confidentiality" negotiated properties, we need to keep the client and server contexts around after the exchange has completed successfully. If a message comes in which does not need authentication and the context has needsWrapping() == true, we need to encode the payload of the message using wrap on the sending side (and the inverse on the receiving side). This could be located in a code block after the authentication check so it doesn't get triggered when it should not.

Thoughts?
    



 


 


was (Author: rachmato):
I'm not 100% confident about the termination conditions of this implementation as it stands and I think it could be improved by restructuring the implementation a little.
For reference, here is the LDAP-based example provided in the SASL code:.
{noformat}
client
------
SaslClient sc = Sasl.createSaslClient(mechs,authId,protocol,serverName,props,cbh);

// get initial response, send first message *and* get challenge from the server
byte[] response = (sc.hasInitialResponse() ? sc.evaluateChallenge(new byte[0]) : null);
LdapResult res = ldap.sendBindRequest(dn, sc.getname(), response);

// check if we need to continue
while (!sc.isComplete() && (res.status == SASL_BIND_IN_PROGRESS || res.status == SUCCESS)) {
    response = sc.evaluateChallenge(res.getBytes());

    // this indicates that we do not need to send another bind request (this should have been set on the basis of complete)
    if (res.status == SUCCESS) {
        // we're done; don't expect to send another BIND
        if (response != null) {
           throw new SaslException("Protocol error: attempting to send response after completion");
        }
        break;
    }
    res = ldap.sendBindRequest(dn, sc.getName(), response);
}

// authentication has finished - check if we need to encrypt subsequent messages
 if (sc.isComplete() && res.status == SUCCESS) {
    String qop = (String) sc.getNegotiatedProperty(Sasl.QOP);
    if (qop != null && (qop.equalsIgnoreCase("auth-int") || qop.equalsIgnoreCase("auth-conf"))) {
      // Use SaslClient.wrap() and SaslClient.unwrap() for future communication with server
      ldap.in = new SecureInputStream(sc, ldap.in);
      ldap.out = new SecureOutputStream(sc, ldap.out);
    }
 }


server
------
// get the initial bind request and create the server mechanism
response = ldap.readBindRequest();
SaslServer ss = Sasl.createSaslServer(mechanism,"ldap", myFQDN, props, callbackHandler);

// start to process responses from the client
while (!ss.isComplete()) {
    try {
        byte[] challenge = ss.evaluateResponse(response);
        if (ss.isComplete()) {
	    // when the authetication is complete, we can send the bind response SUCCESS and terminate 
            status = ldap.sendBindResponse(mechanism, challenge, SUCCESS);
        } else { 
	    // when authentication is not complete, we send an in_progress message and continue reading
            status = ldap.sendBindResponse(mechanism, challenge, SASL_BIND_IN_PROGRESS);
            response = ldap.readBindRequest();
        }
    } catch (SaslException e) {
        status = ldap.sendErrorResponse(e);
        break;
    }
}
// authenticatoion has completed - now check if we have negotiated encryption - but only if successful bind
if (ss.isComplete() && status == SUCCESS) {
   String qop = (String) sc.getNegotiatedProperty(Sasl.QOP);
   if (qop != null && (qop.equalsIgnoreCase("auth-int") || qop.equalsIgnoreCase("auth-conf"))) {
     // Use SaslServer.wrap() and SaslServer.unwrap() for future communication with client
     ldap.in = new SecureInputStream(ss, ldap.in);
     ldap.out = new SecureOutputStream(ss, ldap.out);
   }
}
{noformat}

Tristan's implementation roughly works like this:
- on the client side, add a SaslHeader to the original JOIN_REQ with the initial response, if any
- when the server receives the JOIN_REQ with the SaslHeader,  block on a latch 
- start the challenge-response cycle exchanging messages which only include a SaslHeader
- check for termination on the server side and when termination or an exception occurs, release the latch and send a JOIN_RSP indicating success or send a JOIN_RSP indicating failure

This makes it a little tricky to mimic the control flow and termination checking as in the example above, which uses both the isComplete() status as well as the SUCCESS/SASL_BIND_IN_PROGRESS markers on the bind requests/replies to ensure correct termination.

What we could do instead is to exchange JOIN_REQ / JOIN_RSP messages between client and server at each exchange. The presence of a SaslHeader would indicate that the SASL exchange has not yet completed - if a client receives a JOIN_RSP with a SaslHeader, then it needs to process that header before making a decision about whether or not the process had completed and pass the JOIN_RSP up the stack. I'm going to try to have a go at reworking the implementation in that way and so the code mimics the example above. I'm hoping that this will simplify the exception handling as well and purging of contexts no longer needed.

Also, in order to support encryption and the use of the SASL "integrity" and "confidentiality" negotiated properties, we need to keep the client and server contexts around after the exchange has completed. If a message comes in which does not need authentication and the context has needsWrapping() == true, we need to encode the payload of the message using wrap on the sending side (and the inverse on the receiving side). This could be located in a code block after the authentication check so it doesn't get triggered when it should not.

Thoughts?
    



 


 

> SASL challenge-response cycle does not process challenges
> ---------------------------------------------------------
>
>                 Key: JGRP-1852
>                 URL: https://issues.jboss.org/browse/JGRP-1852
>             Project: JGroups
>          Issue Type: Bug
>      Security Level: Public(Everyone can see) 
>    Affects Versions: 3.5
>            Reporter: Richard Achmatowicz
>            Assignee: Tristan Tarrant
>             Fix For: 3.5
>
>
> The SASL challenge-response cycle between a client peer and a server peer should look like this:
> * client sends (possibly empty) response
> * server evaluates response and sends challenge
> * client evaluates challenge and returns response
> and so on until the cycle ends.
> The client sends responses in SASL headers marked Type.RESPONSE.; the server sends challenges in SASL headers marked Type.CHALLENGE.
> Due to a typo, all headers are marked as Type.RESPONSE, so that CHALLENGE messages were not being processed. The test case passes none the less!



--
This message was sent by Atlassian JIRA
(v6.2.6#6264)