Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [es-dev] Jakarta Security - app-mem-basic-decorate test

That sentence takes on a very different meaning when you truncate it - the complete sentence is:

"During validateRequest processing, a ServerAuthModule must NOT unwrap a message in MessageInfo, and must NOT establish a wrapped message in MessageInfo unless the ServerAuthModule returns AuthStatus.SUCCESS."

The second half of the sentence doesn't suddenly give the ServerAuthModule permission to unwrap the message, the ServerAuthModule should never have set it in the first place as it did not know at that time if it would be returning an AuthStatus.SUCCESS.


On Wed, Aug 17, 2022 at 8:47 PM arjan tijms <arjan.tijms@xxxxxxxxx> wrote:
Hi,

On Wed, Aug 17, 2022 at 9:17 PM Darran Lofthouse <darran.lofthouse@xxxxxxxxx> wrote:
"During validateRequest processing, a ServerAuthModule must NOT unwrap a message in MessageInfo"

This is a fairly specific "must NOT" statement which is why we added verification that this is adhered to. 

It says that, but note that the sentence continues as follows:

"During validateRequest processing, a ServerAuthModule must NOT unwrap a message in MessageInfo, and must NOT establish a wrapped message in MessageInfo"

Specifically note the difference between "unwrap a message in MessageInfo" and "establish a wrapped message in MessageInfo". They key here is in "unwrap" vs "establish a wrapped message".

I agree it's not super easy to interpret, but I think it means:

1. During request processing, the message that you get from MessageInfo, should not be unwrapped and then used.
2. Request processing should not result in setting a wrapped message in MessageInfo. This is made evident by the reference to the return value, which is obviously only observed by the caller of validateRequest.

E.g. the following should not be done for "unwrap a message in MessageInfo"

HttpServletResponse response = ( HttpServletResponse) messageInfo.getResponseMessage();

// Don't do this
if (response instanceof HttpServletResponseWrapper) {
    response = ((HttpServletResponseWrapper) response).getResponse();
}


and the following should also not be done:

validateRequest() {
  // ...

 messageInfo.setResponseMessage(new HttpServletResponseWrapper(response));
 return AuthStatus.SEND_FAILURE;
}


 
This is where in WildFly we prohibit unwrapping during the call to validateResponse which leads to the failure when line 77 is hit on the decorator. 

The example further reinforces this:

"For example, if during validateRequest processing a ServerAuthModule calls MessageInfo.setResponseMessage(), the response argument must be an HttpServletResponseWrapper that wraps the HttpServletResponse within MessageInfo."

During validateRequest the only allowed argument to these methods is an instance that wraps the existing message.

It's probably not the only allowed one, otherwise it would not say "for example", but I agree especially this one is tricky and open to interpretation. But in combination with the sentence above, that explicitly uses different verbiage with the "establish" word, I would go for the previous reading.

Even more so, since it's not even close to doing this:

// Really don't do this in validateRequest
HttpServletResponse response = ( HttpServletResponse) messageInfo.getResponseMessage();
if (response instanceof HttpServletResponseWrapper) {
    response = ((HttpServletResponseWrapper) response).getResponse();
    
    // Allowed by internal messageInfo validation rules, not allowed by validateRequest rules 
    messageInfo.setResponseMessage(response); 
}

That part of the rules is intended for the validateRequest/secureResponse balance (validateRequest wraps, secureResponse unwraps).

Kind regards,
Arjan Tijms
_______________________________________________
es-dev mailing list
es-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/es-dev

Back to the top