Bug 89068 - Do not allow mixed-content WebSockets (ws:// WebSockets on an https:// page)
Summary: Do not allow mixed-content WebSockets (ws:// WebSockets on an https:// page)
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Yuta Kitamura
URL:
Keywords: WebExposed
: 62253 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-06-14 00:23 PDT by Yuta Kitamura
Modified: 2023-05-12 07:54 PDT (History)
14 users (show)

See Also:


Attachments
Patch (5.76 KB, patch)
2012-06-14 01:05 PDT, Yuta Kitamura
no flags Details | Formatted Diff | Diff
Patch v2 (10.27 KB, patch)
2012-06-14 02:40 PDT, Yuta Kitamura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yuta Kitamura 2012-06-14 00:23:08 PDT
Reported in a Chromium bug: http://code.google.com/p/chromium/issues/detail?id=85271

> Reported by bsmith@mozilla.com, Jun 7, 2011
> See https://bugzilla.mozilla.org/show_bug.cgi?id=662692
> 
> When a script attempts to open a WebSocket to a non-TLS-protected server ("ws://"), the attempt should fail if the document was delivered over HTTPS.
> 
> Since there are basically no existing WebSockets servers, there is no compatibility reason to allow ws:// (as opposed to wss://) WebSockets from an https:// webpage. We would > like to move to a stronger policy prohibiting all mixed content, and prohibiting https://+ws:// from the start will help prevent WebSockets from adding to the problem.
> 
> In addition to addressing this in code, we should make sure the W3C spec notes this.

Firefox already does this and WebSocket API draft now has the following clause:
http://dev.w3.org/html5/websockets/#the-websocket-interface
> 2. If secure is false but the origin of the entry script has a scheme component that is itself a secure protocol, e.g. HTTPS, then throw a SecurityError exception.
Comment 1 Adam Barth 2012-06-14 00:30:01 PDT
"Since there are basically no existing WebSockets servers" <-- This isn't really true, but it's probably early enough to make this sort of change.
Comment 2 Yuta Kitamura 2012-06-14 01:05:26 PDT
Created attachment 147513 [details]
Patch
Comment 3 Adam Barth 2012-06-14 01:07:31 PDT
Comment on attachment 147513 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147513&action=review

> Source/WebCore/Modules/websockets/WebSocket.cpp:217
> +    if (SecurityOrigin::isSecure(KURL(KURL(), scriptExecutionContext()->securityOrigin()->toString())) && m_url.protocolIs("ws")) {

This is not correct.  You want to do something more like what http://trac.webkit.org/browser/trunk/Source/WebCore/loader/FrameLoader.cpp#L869 does.
Comment 4 Adam Barth 2012-06-14 01:22:13 PDT
By the way, you should consider moving isMixedContent into SecurityOrigin so you can call it directly from this code.
Comment 5 Yuta Kitamura 2012-06-14 02:40:54 PDT
Created attachment 147534 [details]
Patch v2
Comment 6 Yuta Kitamura 2012-06-14 02:43:25 PDT
Comment on attachment 147513 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147513&action=review

>> Source/WebCore/Modules/websockets/WebSocket.cpp:217
>> +    if (SecurityOrigin::isSecure(KURL(KURL(), scriptExecutionContext()->securityOrigin()->toString())) && m_url.protocolIs("ws")) {
> 
> This is not correct.  You want to do something more like what http://trac.webkit.org/browser/trunk/Source/WebCore/loader/FrameLoader.cpp#L869 does.

Thanks. I uploaded patch v2 which moves the implementation of isMixedContent() to SecurityOrigin so both FrameLoader and WebSocket can use it.
Comment 7 Alexey Proskuryakov 2012-06-14 08:39:01 PDT
> We would like to move to a stronger policy prohibiting all mixed content

Do we agree that it's feasible and desirable? Because otherwise, limiting WebSocket alone just makes no sense.
Comment 8 Yuta Kitamura 2012-06-14 18:40:35 PDT
(In reply to comment #7)
> > We would like to move to a stronger policy prohibiting all mixed content
> 
> Do we agree that it's feasible and desirable? Because otherwise, limiting WebSocket alone just makes no sense.

I'm not particularly good at this security area, but Adam might be able to
comment about this.

As for WebSockets, I think there's few reason not to do this, because:
(1) WebSocket API spec explicitly mandates this mixed-contentness check, and
(2) Firefox is already doing this and has proven that this won't make a heavy
    compatibility impact.
Comment 9 Adam Barth 2012-06-14 20:18:12 PDT
Another viewpoint is that WebSockets are more like XMLHttpRequest with CORS.  In the case of XMLHttpRequest, we let an HTTPS page make a CORS-enabled request to HTTP URLs.

I'm inclined to think we should follow the spec and Firefox here which will encourage folks to build more secure web sites.  I don't feel that strongly about it.  If other folks have strong opinions, we might want to raise the issue in the working group.
Comment 10 Alexey Proskuryakov 2012-06-15 10:02:36 PDT
If I were to guess about Mozilla's plan, that's likely to block mixed content XMLHttpRequests next, and stylesheets/scripts/images after that, which is quite ambitious. I cannot see any reason to do this for WebSockets alone.

I agree that this change likely has no effect on compatibility. It would be good to know that it has a positive effect on anything.
Comment 11 Adam Barth 2012-06-15 10:41:28 PDT
> It would be good to know that it has a positive effect on anything.

It seems like it will have a positive effect on security if it guides folks to use TLS for their WebSocket connections.
Comment 12 Noah Richards 2012-07-02 17:10:18 PDT
If this is added, would it be possible to add an exception for connections to localhost? Google talk is experimenting with using websockets to connect to a locally running daemon process, but we don't do it on Firefox because of this feature.
Comment 13 Adam Barth 2012-07-02 21:26:12 PDT
> If this is added, would it be possible to add an exception for connections to localhost?

Nope.  HTTP to localhost is treated as being insecure all over the platform.  We shouldn't have a one-off exception here.

> Google talk is experimenting with using websockets to connect to a locally running daemon process, but we don't do it on Firefox because of this feature.

You might want to use wss instead.
Comment 14 Noah Richards 2012-07-09 10:21:12 PDT
(In reply to comment #13)
> > If this is added, would it be possible to add an exception for connections to localhost?
> 
> Nope.  HTTP to localhost is treated as being insecure all over the platform.  We shouldn't have a one-off exception here.

Fair enough. Something like CORS would work for us here as well (as you already pointed out in comment #9).

> 
> > Google talk is experimenting with using websockets to connect to a locally running daemon process, but we don't do it on Firefox because of this feature.
> 
> You might want to use wss instead.

We'd like to, but haven't yet decided on how to handle certs for localhost.
Comment 15 Adam Barth 2012-07-09 10:35:47 PDT
The more I think about this, the more I think we should treat WebSockets the same way we treat XMLHttpRequest with CORS given that WebSockets essentially has CORS built in.  Let's talk with bsmith to see if we can get on the same page as Firefox.

Yuta: Would you like to start an email thread with bsmith or would you prefer that I start the conversation?
Comment 16 Yuta Kitamura 2012-07-10 01:01:56 PDT
(In reply to comment #15)
> Yuta: Would you like to start an email thread with bsmith or would you prefer that I start the conversation?

Feel free to start the conversation from you (and cc me). I'm sure you are far better on these topics than I ;)
Comment 17 Nicholas Wilson 2013-07-19 09:09:27 PDT
I don't know where the best place to discuss this is, but for me this change really isn't desirable.

The idea behind WebSockets is to bring sockets to the browser, so it can do all the normal sockety things applications would do. Code in an HTTPS page is the like the application you installed on your machine: it's a secure block of code that you trust, and it's allowed to connect out to untrusted machines and communicate with them.

I posted a hefty protest at Firefox's behaviour on their bug (https://bugzilla.mozilla.org/show_bug.cgi?id=662692#c11), and I really hope WebKit doesn't follow.

To put it another way: A websocket connection can't be mixed content because it isn't "content". It's passed through Javascript first, which can make a decision whether or not to accept the data or abort. This is very different from a mixed content image, which just gets loaded.
Comment 18 Alexey Proskuryakov 2013-07-30 08:33:50 PDT
*** Bug 62253 has been marked as a duplicate of this bug. ***
Comment 19 Anders Carlsson 2014-02-05 10:57:12 PST
Comment on attachment 147534 [details]
Patch v2

Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.
Comment 20 Roberto Machorro 2014-08-20 08:26:31 PDT
Are there any practical suggestions for SSL certs under localhost? All CAs want a valid TLD, and a self-signed localhost is too clunky for deploying to clients - it still requires user approval.
Comment 21 Brent Fulgham 2016-05-18 20:45:40 PDT
John: Can you review this issue and let me know if there are steps we should take here.
Comment 22 Anne van Kesteren 2023-05-12 07:54:46 PDT
WebSocket.cpp uses MixedContentChecker these days. There's a FIXME next to it to share more logic, but this is in place.