[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=12975837#comment-12975837 ] 

Richard Achmatowicz edited comment on JGRP-1852 at 6/12/14 4:43 PM:
--------------------------------------------------------------------

The culprit is here in the SaslServerContext: 
{noformat}
    @Override
    public Message nextMessage(Address address, SaslHeader header) throws SaslException {
        Message message = new Message(address).setFlag(Message.Flag.OOB);
        byte[] challenge = server.evaluateResponse(header.getPayload());
        if (server.isComplete()) {
            latch.countDown();
        }
        if (challenge != null) {
            return message.putHeader(SASL.SASL_ID, new SaslHeader(Type.RESPONSE, challenge));
        } else {
            return null;
        }
    }
{noformat}
The type should be Type.CHALLENGE. I noticed this by accident when looking at the logs with trace enabled; there were no messages indicating that challenges were being processed.

When this change is made, a further problem turns up in the processing of challenges in the up():call - the result of calling SaslClientContext.evaluateChallenge() may be null, and in this case a message should not be sent back to the server. I was getting an NPE in UNICAST3 when the challenge-response cycle ended and msg == null was being sent up the stack. So I changes the code to look like this:

{noformat}
switch (saslHeader.getType()) {
case CHALLENGE:
    try {
        if (log.isTraceEnabled())
            log.trace("%s: received CHALLENGE from %s", getAddress(), remoteAddress);
        // the response computed can be null if the challenge-response cycle has ended
        Message response = saslContext.nextMessage(remoteAddress, saslHeader);
        if (response != null) {
            if (log.isTraceEnabled())
                log.trace("%s: sending RESPONSE to %s", getAddress(), remoteAddress);
            down_prot.down(new Event(Event.MSG, response));
        } else {
           if (!saslContext.isSuccessful()) {
               throw new SaslException("computed response is null but challenge-response cycle  complete!");
            }
            if (log.isTraceEnabled())
                log.trace("%s: authentication complete from %s", getAddress(), remoteAddress);
        }
    } catch (SaslException e) {
        disposeContext(remoteAddress);
        if (log.isWarnEnabled()) {
            log.warn("failed to validate CHALLENGE from " + remoteAddress + ", token", e);
        }
    }
    break;
    ....
{noformat}





was (Author: rachmato):
The culprit is here in the SaslServerContext: 
{noformat}
    @Override
    public Message nextMessage(Address address, SaslHeader header) throws SaslException {
        Message message = new Message(address).setFlag(Message.Flag.OOB);
        byte[] challenge = server.evaluateResponse(header.getPayload());
        if (server.isComplete()) {
            latch.countDown();
        }
        if (challenge != null) {
            return message.putHeader(SASL.SASL_ID, new SaslHeader(Type.RESPONSE, challenge));
        } else {
            return null;
        }
    }
{noformat}
The type should be Type.CHALLENGE. I noticed this by accident when looking at the logs with trace enabled; there were no messages indicating that challenges were being processed.

When this change is made, a further problem turns up in the processing of challenges in the up():call - the result of calling SaslClientContext.evaluateChallenge() may be null, and in this case a message should not be sent back to the server. I was getting an NPE in UNICAST3 when the challenge-response cycle ended and msg == null was being sent up the stack. So I changes the code to look like this:

{nofoamt}
switch (saslHeader.getType()) {
case CHALLENGE:
    try {
        if (log.isTraceEnabled())
            log.trace("%s: received CHALLENGE from %s", getAddress(), remoteAddress);
        // the response computed can be null if the challenge-response cycle has ended
        Message response = saslContext.nextMessage(remoteAddress, saslHeader);
        if (response != null) {
            if (log.isTraceEnabled())
                log.trace("%s: sending RESPONSE to %s", getAddress(), remoteAddress);
            down_prot.down(new Event(Event.MSG, response));
        } else {
           if (!saslContext.isSuccessful()) {
               throw new SaslException("computed response is null but challenge-response cycle  complete!");
            }
            if (log.isTraceEnabled())
                log.trace("%s: authentication complete from %s", getAddress(), remoteAddress);
        }
    } catch (SaslException e) {
        disposeContext(remoteAddress);
        if (log.isWarnEnabled()) {
            log.warn("failed to validate CHALLENGE from " + remoteAddress + ", token", e);
        }
    }
    break;
{nooformat}




> 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: Bela Ban
>
> 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)