Re: issue 7911

Yves,

It seems to me that an implementation of any spec MAY make various 
checks against message correctness regardless of what the spec says. 
Again, because the treatment of malformed messages is outside the scope 
of what the spec defines, implementations are free to do what they want 
in this regard. What I still don't understand is:

* Why the WS-RA WG needs to define a standard fault subcode for this 
condition?

* Why "this condition" (the fact that the [Action] or [Body]/child does 
not match what the spec defines) in particular, out of all the other 
things that may not agree with what is defined in the spec, needs to be 
called out as checkable?

* Why this needs to be specified in WS-T, as opposed to any other spec?

With regards to the "security issue"; if I really thought that someone 
was trying to attack my system with cleverly malformed requests the very 
last thing I would want to do is send them a response that essentially 
says "I know what you are up to and I've taken steps to prevent it. Move 
on to more fertile ground." I'm much better off returning an HTTP 500 or 
something else that will give them the sense they are making progress so 
they continue to waste time on something that will never work.

- gp

You still haven't explained why the WS-RA WG needs to define a standard 
fault

On 10/27/2009 5:57 AM, Yves Lafon wrote:
> On Mon, 26 Oct 2009, Gilbert Pilz wrote:
>
>> Comments inline . . .
>>
>> - gp
>>
>> On 10/26/2009 1:50 PM, Yves Lafon wrote:
>>> On Mon, 26 Oct 2009, Gilbert Pilz wrote:
>>>
>>>> Yves,
>>>>
>>>> I think we need to break your proposal down into its constituent 
>>>> pieces. Firstly, there is the definition of the fault that is 
>>>> generated when/if an implementation detects that the first child of 
>>>> the [Body] does not match the [Action] property. I have two 
>>>> problems with this:
>>>>
>>>> F.1) What is the point of standardizing the definition of this 
>>>> fault? Since the mismatch in question is a result of a coding 
>>>> error, it doesn't seem reasonable to expect anyone to write client 
>>>> code that could detect this fault and address the underlying 
>>>> problem in any meaningful way. It seems to me that any SOAP fault 
>>>> with a code of soap11:Client/soap12:Sender and a 
>>>> soap11:faultstring/soap12:Reason that is equivalent to "the first 
>>>> child of the [Body] does not match the [Action] property" (properly 
>>>> translated of course) would serve the same purpose.
>>>
>>> Ok, so malicious code wanting to exploit a buffer overflow using 
>>> carafully crafted "wrong content" is a coding error?
>>> If you have an implementation that checks only the action property 
>>> to check access and another one that checks only the body element to 
>>> actually perform the request, then you have an issue.
>> If *you* think a particular type of invalid message is a potential 
>> vector for buffer overflow attacks, then *you* are certainly free to 
>> implement a check to prevent this. *We* (Oracle) might reach a 
>> different conclusion about the potential for such attacks. After all, 
>> we probably don't have the same architecture, we might not use the 
>> same programming languages, runtimes, etc. We should not be forced to 
>> implement such checks based on your conclusions (FWIW, Java code that 
>> does not make use of JNI is safe from buffer overflow attacks).
>
> the buffer overflow example was... an example that carefully crafted 
> "wrong" input is not due to a bad implementation. The case I described 
> was more tricking access controls, ie: betting on different behaviours 
> to execute something that (in theory) should not happen.
>
>> With regards to your "issue"; you are incorrect. To repeat myself, 
>> non-conforming implementations have no reasonable expectation of 
>> predictable, specification-defined behavior; as always, GIGO 
>> <http://en.wikipedia.org/wiki/Garbage_In,_Garbage_Out> applies.
> You can also refer to CICO (I let you find the proper wikipedia link)
>
>>> There is a difference between client requirements, currently in the 
>>> spec (body MUST start with ... and action MUST be ...), and receiver 
>>> requirements, especially in this case where we have two times the 
>>> same information. Failing to explicitely check that on the receiving 
>>> end leads to potential security issues.
>> I'm more than a little bit exasperated at the specter of "potential 
>> security issues" being used to defend various positions. If you have 
>> *specific* security threats in mind, let's discuss them. If not, 
>> please don't waste the WG's time with red herrings
> Well I clearly explained what it was. I can understand that you think 
> it's not a big enough issue that you want to implement such a check 
> (which is what you said above and below), and that's a completely 
> different discussion than saying that it can't happen. So let's focus 
> on that.
>
>>> F.2) Even if we stipulate that it is necessary to standardize the 
>>> definition of this fault, it's not clear to me that it's within the 
>>> scope of the WS-RA WG's charter to define what is, essentially, a 
>>> general-purpose SOAP/WS-Addressing fault.
>>>
>>> Ok, so why do we need a PutDenied fault? It's a generic ActionDenied 
>>> fault that WS-Addressing should deal with, no?
>> No it is not. PutDenied is very specific to WS-T. It says that, 
>> although the client supplied what is otherwise a completely valid 
>> WS-T Put request, the resource manager decided that, in this specific 
>> instance, it will not honor the request. The semantics are specific 
>> to the needs of WS-T and the Put request and are not generalizable, 
>> the way [Action]/[Body] mismatch is, to all SOAP requests.
>>>>
>>>> R.1) Why only WS-T? WS-Enum, WS-Eventing, and WS-Mex all have 
>>>> unique [Body] children. If it is important to check for this 
>>>> condition in WS-T, surely it is also important to check for this 
>>>> condition in the other specs as well? Even if we did modify your 
>>>> proposal to include WS-Enum, WS-Eventing, and WS-Mex, from the 
>>>> perspective of a general SOAP/WS-* stack, it seems really strange 
>>>> to perform this check just for these spec and not for any others 
>>>> (i.e. WS-RM, WS-Trust, etc.)
>>> Secondly, there is the requirement on all WS-T resource 
>>> implementations to check for this condition and generate the fault.
>>>
>>> You are right, every time action is duplicated by a body element, it 
>>> is needed.
>> OK. Just so everybody is clear on this: your proposal is basically 
>> calling for the retroactive definition of the 
>> ActionAndWrapperMismatch fault to apply to every spec that defines 
>> "duplicate" action properties and body elements (i.e. 
>> wsrm:CreateSequence, wst:RequestSecurityToken, 
>> wscoor:CreateCoordinationContext, etc.) *and* requires everyone with 
>> an implementation of these specifications to update their 
>> implementations to check for action/body mismatch (if they don't 
>> already do this) and return this newly defined fault.
>>>
>>>> R.2) For what it's worth, my developers don't want to implement 
>>>> this check. It is not required for the correct implementation of 
>>>> the WS-T protocol (when the client sent the invalid request we left 
>>>> the realm of what the protocol defines) and, in their view, it is 
>>>> not likely to happen often enough to be worth the runtime cost.
>>>>
>>>> *Counter Proposal*: Close With No Action
>
> So the your point is that you don't want to implement the check. Mine 
> is to say that there is an issue there that shouldn't be overlook. So, 
> would a SHOULD or a MAY in my proposal would be ok with you?
>
>