Map WebSocket URL schemes to HTTP URL schemes for auth purposes.

Map WebSocket URL schemes to HTTP URL schemes for auth purposes.

This permits WebSocket connections to inherit credentials from HTTP pages, and
matches the behaviour of other browsers.

Design doc: https://docs.google.com/a/chromium.org/document/d/129rLtf5x3hvhP5rayLiSxnEjOXS8Z7EnLJgBL4CdwjI/edit

Also consider any 401 or 407 results that reach the WebSocketStream
URLRequest::Delegate to be unrecoverable errors.

Also ensure that the response headers are reported back to the renderer
when the developer tools are open and a 401 error happens.

BUG=123862
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=286108

[tyoshino (SeeGerritForStatus), 2014-07-07 19:26:44]

lgtm

https://codereview.chromium.org/336263005/diff/40001/net/websockets/websocket...
File net/websockets/websocket_stream.cc (right):

https://codereview.chromium.org/336263005/diff/40001/net/websockets/websocket...
net/websockets/websocket_stream.cc:146: void OnFinishOpeningHandshake() {
this looks similar to
WebSocketBasicHandshakeStream::OnFinishOpeningHandshake

[Adam Rice, 2014-07-08 10:23:45]

+jgraettinger for net OWNERS.

https://codereview.chromium.org/336263005/diff/40001/net/websockets/websocket...
File net/websockets/websocket_stream.cc (right):

https://codereview.chromium.org/336263005/diff/40001/net/websockets/websocket...
net/websockets/websocket_stream.cc:146: void OnFinishOpeningHandshake() {
On 2014/07/07 19:26:44, tyoshino wrote:
> this looks similar to
> WebSocketBasicHandshakeStream::OnFinishOpeningHandshake

Yes. Factored out in a separate function used by both.

[Johnny, 2014-07-08 20:27:40]

lgtm % comments

https://codereview.chromium.org/336263005/diff/50001/net/tools/testserver/tes...
File net/tools/testserver/testserver.py (right):

https://codereview.chromium.org/336263005/diff/50001/net/tools/testserver/tes...
net/tools/testserver/testserver.py:2210: # TODO(ricea): Generalise this to
support basic auth for HTTP too.
nit: s/Generalise/Generalize/

https://codereview.chromium.org/336263005/diff/50001/net/websockets/websocket...
File net/websockets/websocket_stream_test.cc (right):

https://codereview.chromium.org/336263005/diff/50001/net/websockets/websocket...
net/websockets/websocket_stream_test.cc:293: std::string request1;
Trailing underscores?

https://codereview.chromium.org/336263005/diff/50001/net/websockets/websocket...
net/websockets/websocket_stream_test.cc:1155: socket_data.Pass());
Why is ownership of |socket_data| passed to WebSocketTestURLRequestContextHost
here? Shouldn't it be referencing an un-owned pointer, while the test body
retains ownership of |socket_data|?

I realize this arrangement wasn't added in this CL.

[Adam Rice, 2014-07-09 06:35:02]

https://codereview.chromium.org/336263005/diff/50001/net/tools/testserver/tes...
File net/tools/testserver/testserver.py (right):

https://codereview.chromium.org/336263005/diff/50001/net/tools/testserver/tes...
net/tools/testserver/testserver.py:2210: # TODO(ricea): Generalise this to
support basic auth for HTTP too.
On 2014/07/08 20:27:40, Johnny wrote:
> nit: s/Generalise/Generalize/

Done.

https://codereview.chromium.org/336263005/diff/50001/net/websockets/websocket...
File net/websockets/websocket_stream_test.cc (right):

https://codereview.chromium.org/336263005/diff/50001/net/websockets/websocket...
net/websockets/websocket_stream_test.cc:293: std::string request1;
On 2014/07/08 20:27:40, Johnny wrote:
> Trailing underscores?

Okay, that's weird. In my brain this was a struct, but looking at the code I
clearly wrote a class.

Underscores added. Also added DISALLOW_COPY_AND_ASSIGN().

https://codereview.chromium.org/336263005/diff/50001/net/websockets/websocket...
net/websockets/websocket_stream_test.cc:1155: socket_data.Pass());
On 2014/07/08 20:27:40, Johnny wrote:
> Why is ownership of |socket_data| passed to WebSocketTestURLRequestContextHost
> here? Shouldn't it be referencing an un-owned pointer, while the test body
> retains ownership of |socket_data|?
> 
> I realize this arrangement wasn't added in this CL.

Short explanation:
WebSocketDeterministicMockClientSocketFactoryMaker::SetExpectations() came
first, and then
WebSocketDeterministicMockClientSocketFactoryMaker::AddRawExpectation() came
second.

I agree that the resulting ownership semantics are bad, since the
DeterministicSocketData object is only "shallowly" owned by
WebSocketDeterministicMockClientSocketFactoryMaker.

I added a bug to redesign this: crbug.com/392383

[Adam Rice, 2014-07-09 09:26:31]

+mmenke for chrome/browser/net

[mmenke, 2014-07-09 14:19:00]

LGTM rubberstamp, deferring to Johnny on this one.

https://codereview.chromium.org/336263005/diff/70001/chrome/browser/net/webso...
File chrome/browser/net/websocket_browsertest.cc (right):

https://codereview.chromium.org/336263005/diff/70001/chrome/browser/net/webso...
chrome/browser/net/websocket_browsertest.cc:129: content::NotificationRegistrar
registrar_;
nit:  DISALLOW_COPY_AND_ASSIGN.

[Johnny, 2014-07-09 16:16:17]

PS5 lgtm % mmenke's nit.

https://codereview.chromium.org/336263005/diff/50001/net/websockets/websocket...
File net/websockets/websocket_stream_test.cc (right):

https://codereview.chromium.org/336263005/diff/50001/net/websockets/websocket...
net/websockets/websocket_stream_test.cc:1155: socket_data.Pass());
On 2014/07/09 06:35:01, Adam Rice wrote:
> On 2014/07/08 20:27:40, Johnny wrote:
> > Why is ownership of |socket_data| passed to
WebSocketTestURLRequestContextHost
> > here? Shouldn't it be referencing an un-owned pointer, while the test body
> > retains ownership of |socket_data|?
> > 
> > I realize this arrangement wasn't added in this CL.
> 
> Short explanation:
> WebSocketDeterministicMockClientSocketFactoryMaker::SetExpectations() came
> first, and then
> WebSocketDeterministicMockClientSocketFactoryMaker::AddRawExpectation() came
> second.
> 
> I agree that the resulting ownership semantics are bad, since the
> DeterministicSocketData object is only "shallowly" owned by
> WebSocketDeterministicMockClientSocketFactoryMaker.
> 
> I added a bug to redesign this: crbug.com/392383

SGTM.

[Adam Rice, 2014-07-10 05:30:28]

https://codereview.chromium.org/336263005/diff/70001/chrome/browser/net/webso...
File chrome/browser/net/websocket_browsertest.cc (right):

https://codereview.chromium.org/336263005/diff/70001/chrome/browser/net/webso...
chrome/browser/net/websocket_browsertest.cc:129: content::NotificationRegistrar
registrar_;
On 2014/07/09 14:19:00, mmenke wrote:
> nit:  DISALLOW_COPY_AND_ASSIGN.

Done.

[Adam Rice, 2014-07-10 05:31:10]

The CQ bit was checked by ricea@chromium.org

[commit-bot: I haz the power, 2014-07-10 05:32:15]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ricea@chromium.org/336263005/90001

[commit-bot: I haz the power, 2014-07-10 11:14:07]

Change committed as 282307

[Adam Rice, 2014-07-28 04:57:17]

Re-opening to land again now that the tests have been de-flaked.

[Adam Rice, 2014-07-28 04:58:08]

The CQ bit was checked by ricea@chromium.org

[commit-bot: I haz the power, 2014-07-28 04:59:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ricea@chromium.org/336263005/90001

[commit-bot: I haz the power, 2014-07-28 09:06:42]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-07-28 09:20:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-28 09:20:53]

Try jobs failed on following builders:
  linux_chromium_chromeos_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Adam Rice, 2014-07-29 03:35:46]

The CQ bit was checked by ricea@chromium.org

[commit-bot: I haz the power, 2014-07-29 03:36:44]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ricea@chromium.org/336263005/130001

[commit-bot: I haz the power, 2014-07-29 07:42:33]

Change committed as 286108

[Ryan Sleevi, 2014-07-29 19:54:32]

https://codereview.chromium.org/336263005/diff/130001/net/http/http_network_t...
File net/http/http_network_transaction.cc (right):

https://codereview.chromium.org/336263005/diff/130001/net/http/http_network_t...
net/http/http_network_transaction.cc:1559: }
This seems like a violation of SOP.

ws://foo.bar != SOP as http://foo.bar

Is sharing credentials really correct here? I can't help but feel like, no, it's
not and shouldn't be.

[tyoshino (SeeGerritForStatus), 2014-07-30 03:39:54]

https://codereview.chromium.org/336263005/diff/130001/net/http/http_network_t...
File net/http/http_network_transaction.cc (right):

https://codereview.chromium.org/336263005/diff/130001/net/http/http_network_t...
net/http/http_network_transaction.cc:1559: }
On 2014/07/29 19:54:32, Ryan Sleevi wrote:
> This seems like a violation of SOP.
> 
> ws://foo.bar != SOP as http://foo.bar
> 
> Is sharing credentials really correct here? I can't help but feel like, no,
it's
> not and shouldn't be.

The default port is the same as HTTP for both secure and insecure. So the
request goes to the same TCP endpoint as one initiated for a URL with http
scheme. The server may differentiate incoming connection based on upgrade and
dispatches to different backends, but WebSocket was originally specified to be
able to share credentials with HTTP e.g. cookie, or other authentication info.
See the step 12 in http://tools.ietf.org/html/rfc6455#section-4.1. So, it's
server implementor or service developer's duty to take into account that
credentials stored for HTTP requests may be sent together with ws:// handshake.
It's bad that the step 12 doesn't introduce any algorithm like this to make the
authentication system work as the spec intended, but I believe this is ok.

[Adam Rice, 2014-07-30 03:43:34]

https://codereview.chromium.org/336263005/diff/130001/net/http/http_network_t...
File net/http/http_network_transaction.cc (right):

https://codereview.chromium.org/336263005/diff/130001/net/http/http_network_t...
net/http/http_network_transaction.cc:1559: }
On 2014/07/29 19:54:32, Ryan Sleevi wrote:
> This seems like a violation of SOP.
> 
> ws://foo.bar != SOP as http://foo.bar
> 
> Is sharing credentials really correct here? I can't help but feel like, no,
it's
> not and shouldn't be.

The issue is ws: both is and isn't the same protocol as http:. The same thing
applies for cookies; quoting the WhatWG WebSocket API 'The headers to send
appropriate cookies must be a Cookie header whose value is the cookie-string
computed from the user's cookie store and the URL url; for these purposes this
is not a "non-HTTP" API.'

The cookie behaviour has been long standardised and works without any
WebSocket-specific hacks because Chrome's cookie store doesn't really know about
URL schemes.

Authentication behaviour is not really standardised anywhere; RFC6455 mentions
it in passing, and the WhatWG standard effectively disallows it by banning
handling of status codes other than 101.

However, not implementing authentication leaves us incompatible with Firefox and
IE and means that a number of Intranet applications don't work with Chrome,
blocking Chrome adoption. Since we never popup UI for a WebSocket connection,
inheriting credentials from HTTP is the only reasonable implementation and is
also what Firefox and IE do.

My way of looking at it is that ws://foo.bar is "mostly" SOP as http://foo.bar.
They share cookies, credentials, client certificates, SSL exceptions, etc.

Another view some people take is that ws: is not a "real" URL scheme at all.

[tyoshino (SeeGerritForStatus), 2014-07-30 03:44:30]

On 2014/07/30 03:39:54, tyoshino wrote:
>
https://codereview.chromium.org/336263005/diff/130001/net/http/http_network_t...
> File net/http/http_network_transaction.cc (right):
> 
>
https://codereview.chromium.org/336263005/diff/130001/net/http/http_network_t...
> net/http/http_network_transaction.cc:1559: }
> On 2014/07/29 19:54:32, Ryan Sleevi wrote:
> > This seems like a violation of SOP.
> > 
> > ws://foo.bar != SOP as http://foo.bar
> > 
> > Is sharing credentials really correct here? I can't help but feel like, no,
> it's
> > not and shouldn't be.
> 
> The default port is the same as HTTP for both secure and insecure. So the
> request goes to the same TCP endpoint as one initiated for a URL with http
> scheme. The server may differentiate incoming connection based on upgrade and
> dispatches to different backends, but WebSocket was originally specified to be
> able to share credentials with HTTP e.g. cookie, or other authentication info.
> See the step 12 in http://tools.ietf.org/html/rfc6455#section-4.1. So, it's
> server implementor or service developer's duty to take into account that
> credentials stored for HTTP requests may be sent together with ws://
handshake.
> It's bad that the step 12 doesn't introduce any algorithm like this to make
the
> authentication system work as the spec intended, but I believe this is ok.

Ah, sorry. The spec doesn't say so much. The section 4.1 and
http://tools.ietf.org/html/rfc6455#section-10.5 say that auth available for HTTP
should be able to be employed also for WebSockets, but doesn't say that they
should share the same realm.

My understanding is that they should, but the spec doesn't say so.

[tyoshino (SeeGerritForStatus), 2014-07-30 03:46:00]

On 2014/07/30 03:43:34, Adam Rice wrote:
>
https://codereview.chromium.org/336263005/diff/130001/net/http/http_network_t...
> File net/http/http_network_transaction.cc (right):
> 
>
https://codereview.chromium.org/336263005/diff/130001/net/http/http_network_t...
> net/http/http_network_transaction.cc:1559: }
> On 2014/07/29 19:54:32, Ryan Sleevi wrote:
> > This seems like a violation of SOP.
> > 
> > ws://foo.bar != SOP as http://foo.bar
> > 
> > Is sharing credentials really correct here? I can't help but feel like, no,
> it's
> > not and shouldn't be.
> 
> The issue is ws: both is and isn't the same protocol as http:. The same thing
> applies for cookies; quoting the WhatWG WebSocket API 'The headers to send
> appropriate cookies must be a Cookie header whose value is the cookie-string
> computed from the user's cookie store and the URL url; for these purposes this
> is not a "non-HTTP" API.'
> 
> The cookie behaviour has been long standardised and works without any
> WebSocket-specific hacks because Chrome's cookie store doesn't really know
about
> URL schemes.
> 

Fair enough.

> Authentication behaviour is not really standardised anywhere; RFC6455 mentions
> it in passing, and the WhatWG standard effectively disallows it by banning
> handling of status codes other than 101.
> 
> However, not implementing authentication leaves us incompatible with Firefox
and
> IE and means that a number of Intranet applications don't work with Chrome,
> blocking Chrome adoption. Since we never popup UI for a WebSocket connection,
> inheriting credentials from HTTP is the only reasonable implementation and is
> also what Firefox and IE do.
> 
> My way of looking at it is that ws://foo.bar is "mostly" SOP as
http://foo.bar.
> They share cookies, credentials, client certificates, SSL exceptions, etc.
> 
> Another view some people take is that ws: is not a "real" URL scheme at all.