I have been selected as the Applications Area Review Team reviewer for this draft (for background on apps-review, please see http://www.apps.ietf.org/content/applications-area-review-team).
Please resolve these comments along with any other Last Call comments you may receive. Please wait for direction from your document shepherd or AD before posting a new version of the draft.

Document: draft-ietf-hybi-thewebsocketprotocol (reviewed -10)
Title: The WebSocket protocol
Reviewer: Lisa Dusseault
Review Date: July 20, 2011

Readiness Summary: This draft is almost ready for publication on the Standards Track, but it has a few issues that should be fixed or thought about before publication.

Document Summary: The Websocket protocol defines a HTTP handshake to transition from a Web context to a bi-directional connection.  It also defines framing and masking for use in the bi-directional context.  It addresses a number of security concerns involving untrusted application code running in browsers, but assumes that browsers are trusted and servers hosting Websockets are trusted to a greater extent. 

Major Issues

1.  Masking

It would be good to state what the purpose of masking is.  The sentence is "Frames sent from the client to the server are masked to avoid confusing network intermediaries", but I didn't upon reading this, or looking at the specification text for masking, understand why network intermediaries would be confused.  A citation or further explanation would be good.  I see later there's an explanation in page 50, so a forward reference would also work.

2.  Cookie handling

Section 5.1, 
 The request MAY include headers associated with sending cookies,
    as defined by the appropriate specifications [RFC6265].  These
    headers are referred to as _Headers to Send Appropriate
    Cookies_.

    and

 "Additionally, if any headers in the
  server's handshake indicate that cookies should be set (as defined by
  [RFC6265]), these cookies are referred to as _Cookies Set During the
  Server's Opening Handshake_."

 and 

Section 5.2.1, point 8
 	8.  Optionally, other headers, such as those used to send cookies to
     a server.  Unknown headers MUST be ignored.

First, a nit:   these important underlined terms are referred to where?  I didn't see anywhere where these terms are used other than where they are defined here.

The more substantive issue: I'm left unclear as to whether cookies are really expected to be used, or how the client might know that it needs to use cookies or else the application will not work.  In many Web sites, the site will not work if cookies are not used by the client, and this is sufficiently rare that it's OK.  Is that OK for a Websockets app?  How will the user know how to fix the problem?  Since Websockets can't as easily reply with a Web page to explain how to enable cookies, it would be good to be more clear on this.  

3.  Failing a WebSocket Connection

Section 7.1.7 is supposed to describe how to _Fail the WebSocket Connection_.  It explains how the client does so.  However, there's at least one case in section 9.1 where the server receiving a malformed Sec-WebSocket-Extensions header must _Fail the WebSocket Connection_.  How does the server do this? 

4.  Dropping with extreme prejudice

From the Security Considerations: 

"If at any time a server is faced with data that it does not
understand, or that violates some criteria by which the server
determines safety of input, or when the server sees an opening
handshake that does not correspond to the values the server is
expecting (e.g. incorrect path or origin), the server SHOULD just
disconnect.  It is always safe to disconnect."

This seems pretty excessive.  When the server just disconnects, the client can't tell much about what went wrong, and whether an automated retry would be any use at all.  This is bad for deployed use, and even worse for development/debugging.  Yet we're recommending servers be that unhelpful?  Wouldn't it be better to recommend that the server reply with an error response (some HTTP status codes defined here, too) that can help the client diagnose an incorrect origin?  An error like that is often going to be a mis-configuration on one side or the other, rather than an attack.

I'd also like to know what the websockets server implementations currently do.


Minor Issues

The reservation of different ranges of error codes for use by extensions, libraries and frameworks, and application code, seems to be intended to avoid conflicts between those kinds of use.  I don't think it will work very well.  I don't understand where a library ends and application code begins, even in code I write where I write an error_handler to be called by the library I'm using (which might be a "library" only for convenience sake or might be a library that dozens of other software programs use).  

The border between code and "extensions" is not even very clear; if somebody writes an extension intending to publish an internet-draft but never gets around to it, they may have ended up using the wrong range with the right intentions.

The border between extensions and "this protocol" is not very clear; if I submit a internet-draft with a feature proposed to be a requirement for websockets, I might prefer to use a 1000-1999 range error code but if the feature is relegated to an optional draft it should change to 2000-2999 range. 

This kind of ambiguity leads to arguments at best, unnecessary changes to code (when specs are changed late after such arguments) at worst.  I'm not sure what to recommend; perhaps only one range reserved for  "this protocol and extensions published within the IETF process", and another range for "libraries, frameworks and application code".


Thanks,
Lisa Dusseault