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

Hi,

This is essentially resolved by updating the test to not use the controversial code anymore. 

This should not affect the purpose or coverage of the test, as this specific code was for the test an internal implementation detail. Strictly speaking, the test could have used a static variable as well, or even always set something random like the current time in the response. The one and only purpose of the test is to verify a decorator can be applied correctly (and somewhat that it is dynamically executed).

As for the Jakarta Authentication behaviour that was discussed; we should certainly clarify this case for the next (major) version of the specification.

Kind regards,
Arjan Tijms





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

On Wed, Aug 17, 2022 at 9:59 PM Darran Lofthouse <darran.lofthouse@xxxxxxxxx> wrote:
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.

For delegating I think it should be allowed to do so indeed, but after the end of the call (as observed by the caller), the restriction should be as stated by the spec.

For the complete sentence, the interpretation is difficult, I agree, but it also comes down to how the comma is used:

"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."

-->

(During validateRequest processing) ->

(ServerAuthModule must NOT unwrap a message in MessageInfo) && 
(must NOT establish a wrapped message in MessageInfo unless the ServerAuthModule returns AuthStatus.SUCCESS)

At least that is my interpretation based on the comma and my experience with the specific type of language that Ron Monzillo (author of said text) typically uses.

It also helps to zoom out, and look at what the actual intention (spirit) of the statements is. The sections quoted speak about various restrictions. First, the internal restrictions of MessageInfo, intended to not break the request/response wrapping chain. Secondly, the responsibility of validateRequest to pass a wrapped request down the chain (if needed and if status success) and for secureResponse to unwrap this again (and not mix up those two).

In other words, why would the spec try to prevent to restore the MessageInfo to the state that was passed-in? What would be the idea behind that?

Kind regards,
Arjan Tijms

 


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
_______________________________________________
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