Re: issue 7911

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

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.
> 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 
<http://en.wikipedia.org/wiki/Red_herring_%28idiom%29>.
> 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
>>
>> - gp
>>
>> On 10/26/2009 6:15 AM, Yves Lafon wrote:
>>> On Thu, 22 Oct 2009, Gilbert Pilz wrote:
>>>
>>>> Just went to look at issue 7911 in the issues list and found a 
>>>> whole discussion thread in the comments section. I might have 
>>>> missed the entire thing if I didn't happen to stumble upon it. 
>>>> Could people please constrain themselves to discussing issues on 
>>>> the mailing list and use the comments section for major 
>>>> developments (i.e. new proposals, directional resolutions, etc.)?
>>>
>>> You are entirely right, I should have moved discussion here...
>>> Here is my proposal for resolving that issue:
>>>
>>> In 3, before the definition of the different resource operations:
>>>
>>> "If the [Body] first child is not matching the
>>> [Action] parameter for any defined Resource Operation, the recipient 
>>> MUST
>>> generate a ActionAndWrapperMismatch fault"
>>>
>>> and add the fault:
>>> <<
>>> 5.4 ActionAndWrapperMismatch
>>>
>>> This fault is generated if the [Body] first child is not matching the
>>> [Action]
>>>
>>> [Code]    s:Sender
>>> [Subcode] wst:ActionAndWrapperMismatch
>>> [Reason]  Action and Wrapper mismatch
>>> [Detail]  none
>>>>>
>>>
>>>
>>>
>>
>