Bug 81361

Summary: [WebSocket]A client MUST close a connection if it detects a masked frame
Product: WebKit Reporter: Li Yin <li.yin>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, bashi, tkent, webkit.review.bot, yutak
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Li Yin
Reported 2012-03-16 09:30:16 PDT
From RFC6455, A server MUST NOT mask any frames that it sends to the client. A client MUST close a connection if it detects a masked frame.
Attachments
Patch (4.12 KB, patch)
2012-03-16 09:47 PDT, Li Yin
no flags
Patch (6.35 KB, patch)
2012-03-16 16:49 PDT, Li Yin
no flags
Patch (17.13 KB, patch)
2012-03-21 00:19 PDT, Li Yin
no flags
Li Yin
Comment 1 2012-03-16 09:47:08 PDT
Li Yin
Comment 2 2012-03-16 09:52:33 PDT
We can get the notes from the followed address. http://tools.ietf.org/html/rfc6455#section-5.1
Yuta Kitamura
Comment 3 2012-03-16 10:19:40 PDT
Comment on attachment 132303 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132303&action=review This is a good thing to do. Thanks for doing this. > Source/WebCore/Modules/websockets/WebSocketChannel.cpp:647 > + if (frame.masked) { > + fail("A server MUST NOT mask any frames that it sends to the client."); > + return false; > + } > + This check should be done inside parseFrame(), and you can safely remove unmasking code there (line 595-600). You can also remove the stale comment in WebSocketChannel.h ("May modify part of ...").
Li Yin
Comment 4 2012-03-16 16:49:28 PDT
Li Yin
Comment 5 2012-03-19 17:28:32 PDT
Hi tkent,yuta, Can you review this patch? Thanks.
Yuta Kitamura
Comment 6 2012-03-20 10:31:43 PDT
Comment on attachment 132417 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132417&action=review Except for some style issues, the patch looks fine to me. (Please wait for comments from a reviewer, as I'm not a WebKit reviewer yet.) > Source/WebCore/ChangeLog:7 > + A client MUST close a connection if it detects a masked frame > + https://bugs.webkit.org/show_bug.cgi?id=81361 > + A server MUST NOT mask any frames that it sends to the client. > + > + Reviewed by NOBODY (OOPS!). Usual ChangeLog format is: <bug title> <bug URL> Reviewed by ... <more description> > Source/WebCore/Modules/websockets/WebSocketChannel.cpp:562 > + fail("A server MUST NOT mask any frames that it sends to the client."); Nitpick: Capital "MUST NOT" sounds too exaggerated; just small letters would work fine.
Li Yin
Comment 7 2012-03-21 00:19:07 PDT
Li Yin
Comment 8 2012-03-21 00:28:05 PDT
Add a new test case to track whether the browser failed the connection or not, when it received the masked frame from the server. In addition, rename the test case masked-frame* into unmasked-frame*, and using unmasked frames to replace masked frames.
Yuta Kitamura
Comment 9 2012-03-21 11:25:51 PDT
Comment on attachment 132983 [details] Patch Looks fine to me. Please wait for comments from a reviewer.
Kent Tamura
Comment 10 2012-03-21 17:03:39 PDT
Comment on attachment 132983 [details] Patch ok
WebKit Review Bot
Comment 11 2012-03-22 23:08:57 PDT
Comment on attachment 132983 [details] Patch Clearing flags on attachment: 132983 Committed r111829: <http://trac.webkit.org/changeset/111829>
WebKit Review Bot
Comment 12 2012-03-22 23:09:02 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.