[hybi] Fwd: Gen-ART last call review of draft-ietf-hybi-thewebsocketprotocol-10

Peter Saint-Andre <stpeter@stpeter.im> Thu, 21 July 2011 02:46 UTC

Return-Path: <stpeter@stpeter.im>
X-Original-To: hybi@ietfa.amsl.com
Delivered-To: hybi@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id D4E3E21F86E0 for <hybi@ietfa.amsl.com>; Wed, 20 Jul 2011 19:46:05 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -102.799
X-Spam-Level:
X-Spam-Status: No, score=-102.799 tagged_above=-999 required=5 tests=[AWL=-0.200, BAYES_00=-2.599, USER_IN_WHITELIST=-100]
Received: from mail.ietf.org ([64.170.98.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id MN9EztVGmblO for <hybi@ietfa.amsl.com>; Wed, 20 Jul 2011 19:46:05 -0700 (PDT)
Received: from stpeter.im (mailhost.stpeter.im [207.210.219.225]) by ietfa.amsl.com (Postfix) with ESMTP id 2815421F854E for <hybi@ietf.org>; Wed, 20 Jul 2011 19:46:05 -0700 (PDT)
Received: from squire.local (unknown [216.17.179.111]) (Authenticated sender: stpeter) by stpeter.im (Postfix) with ESMTPSA id 63EF3410EE for <hybi@ietf.org>; Wed, 20 Jul 2011 20:46:48 -0600 (MDT)
Message-ID: <4E2792EB.2070408@stpeter.im>
Date: Wed, 20 Jul 2011 20:46:03 -0600
From: Peter Saint-Andre <stpeter@stpeter.im>
User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2.15) Gecko/20110303 Thunderbird/3.1.9
MIME-Version: 1.0
To: "hybi@ietf.org" <hybi@ietf.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: [hybi] Fwd: Gen-ART last call review of draft-ietf-hybi-thewebsocketprotocol-10
X-BeenThere: hybi@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: Server-Initiated HTTP <hybi.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/hybi>, <mailto:hybi-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/hybi>
List-Post: <mailto:hybi@ietf.org>
List-Help: <mailto:hybi-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/hybi>, <mailto:hybi-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 21 Jul 2011 02:46:05 -0000

FYI, this was sent to ietf@ietf.org but not the WG list.

-------- Original Message --------
Subject: Gen-ART  last call review of 
draft-ietf-hybi-thewebsocketprotocol-10
Date: Tue, 19 Jul 2011 23:01:53 -0400
From: Richard L. Barnes <rbarnes@bbn.com>
To: General Area Review Team <gen-art@ietf.org>, 
draft-ietf-websec-thewebsocketprotocol@tools.ietf.org,	IETF Discussion 
<ietf@ietf.org>

I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at
<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-hybi-thewebsocketprotocol-10.txt
Reviewer: Richard Barnes
Review Date: 19 July 2011
IETF LC End Date: 25 July 2011
IESG Telechat date: (if known) -

Summary:
Not ready.

Major issues:

[Is GET/Upgrade appropriate?]
It seems like the use of GET here goes beyond the normal "safe" 
semantics for that method.  To quote RFC 2616:
"
    In particular, the convention has been established that the GET and
    HEAD methods SHOULD NOT have the significance of taking an action
    other than retrieval.
"
In addition, alternative protocols specified in Upgrade headers are 
generally optional, whereas in this case, the transaction fails overall 
if the server does not support the websocket protocol.  It doesn't look 
to me like any of the current methods provide much better semantics 
(except possiblyCONNECT or the use of OPTIONS as in RFC 2817), and it 
seems like defining a new method could help get around the Upgrade issue 
as well.

[Huge buffers]
The frame length can be 7, 16, or 64 bits long.  Since the client is 
expected to buffer data until the end of a frame, this is asking clients 
to buffer 128 B, 64 KB, or 16 EB.  If it were 32 bits, the max would be 
4 MB.  Why not just make this a 32-bit fixed length field?

[Why is masking necessary?]
I seriously question the necessity of the masking of data frames.  As I 
understand it, the goal is to prevent proxies that don't understand 
Upgrade from confusing WebSocket data with HTTP data.  This risk seems a 
little dubious to me; has such a poisoning attack been demonstrated?  It 
seems like there are much simpler ways of doing this, like using a 
method other than GET (either CONNECT or something new).

[Why only client-to-server masking?]
Why isn't masking required on server-to-client frames?

[Unlimited buffering with fragmentation]
Much like with the frame length issue above, the fragmentation mechanism 
here seems like it imposes a heavy burden on the receive side.  Since 
the receiving client is supposed to buffer data until the end of a 
frame, it seems like fragmentation could be used to cause a receiving 
client to buffer a frame of indefinite size.


Minor issues:

[Setting protocol]
In the NOTE in Section 1.3, the document observes that Sec- headers 
can't be set in XHRs.  But clearly the Sec-WebSocket-Protocol header can 
be set through some API that applications use to get to this protocol. 
So it seems a little misleading to imply that scripts can't set these 
headers (besides -Version and -Origin).

[Use existing Origin header]
Why is this document creating a separate Sec-WebSocket-Origin header, 
rather than using the Origin header from draft-ietf-websec-origin?

[Need a reference to CORS]
It would be helpful to have a reference to Cross-Origin Resource Sharing 
in the NOTE in this section, since the pre-request check that is 
described is exactly a CORS priming request.

[Key is over-engineered]
What is the need for the hash and GUID in the handshake processing of 
the key?  It seems like it would be sufficient for the server to do 
anything deterministic with the key, even something as simple as 
incrementing it or flipping the bits.



Nits / Editorial comments:

[Why not plain sockets?]
The introduction makes clear why this protocol is needed instead of 
HTTP, but not why this protocol improves over providing a plain socket 
interface.  Presumably this is because the HTTP header provides a space 
where the browser can inject trusted information?

[Put the handshake section first]
It would really increase readability to put all the 
negotiation/handshake parts up front, before you get to the data format 
(Section 4).  As it is, when I got to the extension data part, I didn't 
realize that there was an extension mechanism, since it hadn't been 
mentioned yet.

[Stray sections]
It seems like Section 8 (Error Handling) should be moved up to Section 
4.6, and Section 9 (Extensions) should be part of section 4.

_______________________________________________
Ietf mailing list
Ietf@ietf.org
https://www.ietf.org/mailman/listinfo/ietf