Subject: [hybi] Revised WebSocket Feedback

From: Vladimir Katardjiev <[email-masked]>
Date: Mon, 8 Mar 2010 17:30:36 +0100

Heya,
 
So, after some implementation work it's time to channel some responses.

First, a copy/paste error. Page 31 seems to be missing Sec-WebSocket-Key2.

Same page, you probably meant | instead of " in 
	
	(such as |Cookie")

Also, this might be just my reading comprehension, but the following paragraph caused a double-take

	if a server assumes that its clients are authorized
      on the basis that they can connect [...] then the server
      should also verify that the client's handshake includes the
      invariant "Upgrade" and "Connection" parts of the handshake, and
      should send the server's handshake before changing any user data.

Checking Upgrade and Connection is redundant if Sec-WebSocket-* are present from a browser PoV;that was the point of them, no? Furthermore, it is not evident that by /sending/ a reply you're preventing cross-protocol attacks. I guess you meant by reading the client's handshake (which just happens to be a part of the "sending reply" section).

Right, time to graduate from reading comprehension into actual questions. The big addition obviously Sec-WebSocket-Key1/2. Are there any fundamental limitations on where spaces can and should occur in the header? According to my interpretation as it stands, the following string is a perfectly valid key: "    4abcd". Is this intended? And, if so, can the leading (or trailing, if so inclined) spaces cause issues when parsing by proxies/existing stacks?

Also, I must've missed the big discussion, but what's the rationale of having two key headers. One I can buy, but since it should already be impossible to create a Sec-* header from a browser, AND it is impossible to add spaces to the header value, AND it is a GET with a body, it's already triple impossible (impossibler? impossiblest?) for XHR/CORS to forge. If someone has a handy link to where this suggestion was born it'd be appreciated, because this seems like quite a bit of computation involved.

The junk characters in the handshake also seem redundant. If you already need to verify the correct number of spaces this means you are checking for spaces already. As for the garbage characters, all they made me do was replace("[^0-9]", '') instead of replace(' ', '') so I'd say they didn't alter the functionality of my code other than take up more CPU cycles.

The closing handshake is also interesting. Maybe state in 5.3. that the server shouldn't disconnect on 0xFF (instead of may; it still may, but is discouraged from doing so since it may cause data loss. If the target audience really is the kind of programmer you need to verify their code by actually ensuring they read the spaces, then make the default level of the protocol ensure data integrity, because that's what they'll see in the lab network).

Finally, and probably my smallest serious gripe, but what's up with the mix of references to ASCII and UTF-8 as characters. It is all technically correct, to nobody's surprise, but it's distracting to read. I hope it's just a transitional thing and it'll eventually be UTF-8 throughout but it was just too boring to do it all at once.

Cheers,
Vladimir

PS, some notes that border on the psychopathic crazy programmer in a dark dungeon mindset. These can be safely ignored.

Page 20:  EXAMPLE: For example

Acknowledgement: OK!

Page 22: -> If the byte is 0x3A (ASCII :)

The internet has damaged me so badly this was a smiley face.

---

From: Takeshi Yoshino <[email-masked]>
Date: Mon, 15 Mar 2010 18:30:38 +0900

Hi,

2010/3/9 Vladimir Katardjiev <[email-masked]>

> Checking Upgrade and Connection is redundant if Sec-WebSocket-* are present
> from a browser PoV;that was the point of them, no? Furthermore, it is not
> evident that by /sending/ a reply you're preventing cross-protocol attacks.
> I guess you meant by reading the client's handshake (which just happens to
> be a part of the "sending reply" section).
>
> Right, time to graduate from reading comprehension into actual questions.
> The big addition obviously Sec-WebSocket-Key1/2. Are there any fundamental
> limitations on where spaces can and should occur in the header? According to
> my interpretation as it stands, the following string is a perfectly valid
> key: "    4abcd". Is this intended? And, if so, can the leading (or
> trailing, if so inclined) spaces cause issues when parsing by
> proxies/existing stacks?
>
>
E.g. when /number_1/ is set to 0, /key_1/ will be "0" regardless of
/spaces_1/. Then, in step 21, we have to insert /spaces_1/ spaces into this
string with a length of 1. This is problematic since the only possible
positions for the first insertion are the head and tail of the target
string.

I agree that we should avoid putting spaces on the tail or the head (at
least putting a space for this algorithm on the head must be prohibited).
How about swapping step 21 and 22?

> Also, I must've missed the big discussion, but what's the rationale of
> having two key headers. One I can buy, but since it should already be
> impossible to create a Sec-* header from a browser, AND it is impossible to
> add spaces to the header value, AND it is a GET with a body, it's already
> triple impossible (impossibler? impossiblest?) for XHR/CORS to forge. If
> someone has a handy link to where this suggestion was born it'd be
> appreciated, because this seems like quite a bit of computation involved.
>
> The junk characters in the handshake also seem redundant. If you already
> need to verify the correct number of spaces this means you are checking for
> spaces already. As for the garbage characters, all they made me do was
> replace("[^0-9]", '') instead of replace(' ', '') so I'd say they didn't
> alter the functionality of my code other than take up more CPU cycles.
>
> The closing handshake is also interesting. Maybe state in 5.3. that the
> server shouldn't disconnect on 0xFF (instead of may; it still may, but is
> discouraged from doing so since it may cause data loss. If the target
> audience really is the kind of programmer you need to verify their code by
> actually ensuring they read the spaces, then make the default level of the
> protocol ensure data integrity, because that's what they'll see in the lab
> network).
>
> Finally, and probably my smallest serious gripe, but what's up with the mix
> of references to ASCII and UTF-8 as characters. It is all technically
> correct, to nobody's surprise, but it's distracting to read. I hope it's
> just a transitional thing and it'll eventually be UTF-8 throughout but it
> was just too boring to do it all at once.
>
> Cheers,
> Vladimir
>
> PS, some notes that border on the psychopathic crazy programmer in a dark
> dungeon mindset. These can be safely ignored.
>
> _______________________________________________
> hybi mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/hybi
>

---

From: Fumitoshi Ukai (鵜飼文敏) <[email-masked]>
Date: Thu, 18 Mar 2010 19:16:48 +0900

On Mon, Mar 15, 2010 at 18:30, Takeshi Yoshino <[email-masked]> wrote:

>> Checking Upgrade and Connection is redundant if Sec-WebSocket-* are
>> present from a browser PoV;that was the point of them, no? Furthermore, it
>> is not evident that by /sending/ a reply you're preventing cross-protocol
>> attacks. I guess you meant by reading the client's handshake (which just
>> happens to be a part of the "sending reply" section).
>>

or how about requiring /key_1/ string at step 20 should have 10 (or at least
some number of ) digits with leading zero, so we can insert random position
even if /product_1/ is single digit number ?

-- 
ukai

>> Finally, and probably my smallest serious gripe, but what's up with the
>> mix of references to ASCII and UTF-8 as characters. It is all technically

---

From: Takeshi Yoshino <[email-masked]>
Date: Wed, 31 Mar 2010 16:31:17 +0900

Two comments on the closing handshake.

http://www.whatwg.org/specs/web-socket-protocol/

----

In 1.4, it says "Upon receiving a 0xFF frame, the other peer sends an
identical 0xFF frame ...". But in 5.3, step 3-12, it only says that on
receiving 0xFF frame, we must set client terminated and abort the steps.
Sending ACK 0xFF back is not specified. On the other hand, in 4.2, step
3-8-1, client is asked to start the WebSocket closing handshake on receiving
a 0xFF frame from the server.

Maybe, we should add step 13 like this?
12. ...
13. terminate the Web Socket connection

Or the timing to start the WebSocket closing handshake in response to
client-initiated closing handshake is left to implementor's choice?

----

In the last paragraph of 5.3, "Once ... data to the server." Maybe typo,
s/server/client/ ?

Regards,

Takeshi

2010年3月18日19:16 Fumitoshi Ukai (鵜飼文敏) <[email-masked]>:

> On Mon, Mar 15, 2010 at 18:30, Takeshi Yoshino <[email-masked]>wrote:
>
>> Hi,
>>
> or how about requiring /key_1/ string at step 20 should have 10 (or at
> least some number of ) digits with leading zero, so we can insert random
> position even if /product_1/ is single digit number ?
>
> --
> ukai
>
>
>>
>>