Bug 89068 – Do not allow mixed-content WebSockets (ws:// WebSockets on an https:// page)

• Component: WebCore Misc.
• Status: RESOLVED CONFIGURATION CHANGED

[Yuta Kitamura]

Reported in a Chromium bug: http://code.google.com/p/chromium/issues/detail?id=85271

> Reported by [e-mail], 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.

[Adam Barth]

"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.

[Yuta Kitamura]

Created attachment 147513 [details]
Patch

[Adam Barth]

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.

[Adam Barth]

By the way, you should consider moving isMixedContent into SecurityOrigin so you can call it directly from this code.

[Yuta Kitamura]

Created attachment 147534 [details]
Patch v2

[Yuta Kitamura]

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.

[Alexey Proskuryakov]

> 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.

[Yuta Kitamura]

(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.

[Adam Barth]

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.

[Alexey Proskuryakov]

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.

[Adam Barth]

> 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.

[Noah Richards]

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.

[Adam Barth]

> 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.

[Noah Richards]

(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.

[Adam Barth]

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?

[Yuta Kitamura]

(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 ;)

[Nicholas Wilson]

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.

[Alexey Proskuryakov]

*** Bug 62253 has been marked as a duplicate of this bug. ***

[Anders Carlsson]

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.

[Roberto Machorro]

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.

[Brent Fulgham]

John: Can you review this issue and let me know if there are steps we should take here.

[Anne van Kesteren]

WebSocket.cpp uses MixedContentChecker these days. There's a FIXME next to it to share more logic, but this is in place.