Re: [hybi] Review of draft-ietf-hybi-thewebsocketprotocol-13

["Richard L. Barnes" <[e-mail]>]

>> Section 2, "Implementations MAY impose implementation-specific limits..."
>> This paragraph has been removed in -13.
>> 
> No, it got moved to its own section "Implementation-Specific Limits" under the "Security Considerations"

Yeah, sorry, wrote that before I got to the Security Considerations :)


>> While the "MAY" doesn't specify a requirement, it seems like it would be helpful to implementers in light of the exhaustion/DoS possibilities presented by huge frames and fragmentation.  I would even argue that it should be a "SHOULD".
>> 
> I am Ok with changing MAY to SHOULD.

I'm assuming from your response below that you're OK with going to MUST as well?


>> (I'm guessing this has to do with 'deflate'?)  At the very least, it seems like there should be a requirement that when an intermediary sees an extension that it doesn't understand, it MUST NOT fragment or coalesce frames.
>> 
> This requirement is already in the spec. In Section 5.4:
> 
>  o  An intermediary MUST NOT change the fragmentation of a message if
>     any reserved bit values are used and the meaning of these values
>     is not known to the intermediary.
> 
>  o  An intermediary MUST NOT change the fragmentation of any message
>     in the context of a connection where extensions have been
>     negotiated and the intermediary is not aware of the semantics of
>     the negotiated extensions.

Thanks, I had missed those requirements.  


>> Section 8
>> Should be moved to Section 5.6.  It really only applies to text frames, and even then only if text frames are required to be UTF-8.
>> 
> I think it also applies to the handshake itself (in presence of possible extensions).

Even still, it would probably be more helpful to have individual recommendations in the handshake and framing sections.


>> Section 10
>> In this section, and overall, it would be helpful if this document could briefly describe the browser/JS model and assign some terms that can be used consistently throughout. 
>> Section 10.2, "... should only respond with the corresponding "Sec-WebSocket-Accept" if it is an accepted origin. "
>> If I understand this correctly, the server causes the handshake to fail by omitting the "Accept".
>> 
> This section doesn't define normative behaviour
> 
>> Shouldn't it either return an HTTP error or Fail the Websocket Connection_ ?
>> 
> Yes. Any suggestions how to make this clearer?

OLD: 
"
Servers that are not intended to process input from any Web page but only for certain sites SHOULD verify the "Origin" field is an origin they expect, and should only respond with the corresponding "Sec-WebSocket-Accept" if it is an accepted origin.
"
NEW:
"
Servers that are not intended to process input from any Web page but only for certain sites SHOULD verify the "Origin" field is an origin they expect.  If the origin indicated is unacceptable to the server, then it SHOULD respond with an HTTP 403 Unauthorized error response and _Fail the WebSocket Connection_.
"


>> Section 10.3, "It is also necessary that once the transmission of a frame from a client has begun, the payload (application supplied data) of that frame must not be capable of being modified by the application."
>> This seems to further argue against the idea that giant frames are useful.
>> 
> Yes.
> 
>> They're clearly not useful for streaming (the opposite is suggested in Section 5.4, see above), since the application would have to provide all the data at once.  
>> Section 10.3
>> This section should note that even given the masking constraints in this document, proxies are still vulnerable to poisoning by non-browser clients that do not perform masking.

Suggested text:
"
Despite the protection provided by masking, non-compliant HTTP proxies will still be vulnerable to poisoning attacks of this type by clients and servers that do not apply masking.
"


>> Section 10.4
>> Suggest making this a "SHOULD" or even "MUST".  If your implementation does not constrain these things, then it will be vulnerable to exhaustion attacks.
>> 
> Ok.

So you're changing this to MUST?