This is the test case file.

Deleted: xhrlogin.html 431 bytes

xhrlogin.html 431 bytes View Download

Reproduced on Windows 7 using the same canary version.

Also weird, the following message is emitted -
[Deprecation] Subresource requests whose URLs contain embedded credentials (e.g. `https://user:pass@host/`) are blocked. See https://www.chromestatus.com/feature/5669008342777856 for more details.

However, deprecation usually means it does not have an effect at the moment, but it does.

This seems like an important regression, so I marked it as a stable release blocker.

For the record, this also affects XMLHTTPRequest calls from extensions; it's broken several remote control addons including https://chrome.google.com/webstore/detail/play-to-kodi/fncjhcjfnnooidlkijollckpakkebden and https://chrome.google.com/webstore/detail/remote-torrent-adder/oabphaconndgibllomdcjbfdghcmenci

> Looks like a collateral damage, the XHR API broke as well, because it is using name:password@host syntax internally.

As you note, XHR uses the same mechanism per spec, and it was included in the usage numbers we evaluated when deprecating/removing. That was an intentional change; my goal is indeed to block developer-controlled access to basic/digest auth

Despite the low usage numbers, it sounds like that change is having a negative impact on SAP. Can you help me understand your usage?
 
> However, deprecation usually means it does not have an effect at the moment, but it does.

I believe we usually keep deprecation messages around for a release or two after deprecation takes effect so that developers understand why the thing that no longer works is not working.

> This seems like an important regression, so I marked it as a stable release blocker.

Makes sense, thanks! If we need to bump the deprecation down the line a bit to accommodate workflows that aren't showing up in our usage numbers, we'll need to do it before hitting stable.


> For the record, this also affects XMLHTTPRequest calls from extensions.

That's correct. Extension usage was also included in the metrics we evaluated, though I admit I didn't dig through the web store to verify impact on individual extensions.

Would it be conceivable to add a permission to allow extensions to bypass this restriction?

This could be an explicit permission, or an extension to the host name permission as described at https://developer.chrome.com/extensions/xhr#requesting-permission - i.e. "http://someuser@localhost/", "http://*@somesite/".

I'd argue that this is in keeping with the spirit of the original intent and respects the elevated permissions that extensions usually have (i.e. the ability to bypass SOP).

Apologies if this is off-topic - I'm aware that the original bug is regarding actual webpages, but this is an important use case for my day-to-day use; I'd hate to see Chrome lose a whole class of extensions (i.e. remote control) through this.

predatory.kangaroo@: Would you mind filing a separate bug? I agree that the risks of exposing basic/digest auth to extensions is lower than exposing it to the web, so it's worth chatting. I'd prefer to kill it entirely, and if we keep it in extensions than I can't actually delete code. But let's chat about use cases. :)

Certainly, reported the extension side at https://bugs.chromium.org/p/chromium/issues/detail?id=708131

Instead of passing name and password in the open-call, you can just set a custom header

xhr.setRequestHeader ("Authorization", "Basic " + btoa(username + ":" + password));

So I don't see any value in breaking the XHR API and forcing applications which still want or need basic auth to rewrite their application, you can't prevent basic auth anyway?

In SAPUI5 we have an API for OData access, where we allow to pass username and password. We do not recommend to do so for security reasons and avoid it in applications shipped by SAP, but we know customers often are still using basic authentication, also with SAPUI5.

Thanks!

> Instead of passing name and password in the open-call, you can just set a custom header
>
> xhr.setRequestHeader ("Authorization", "Basic " + btoa(username + ":" + password));
>
> So I don't see any value in breaking the XHR API and forcing applications which still want or need basic auth to rewrite their application, you can't prevent basic auth anyway?

Correct, that's a totally reasonable workaround.

The nice thing about it is that it should require a CORS preflight for cross-origin resources, as it's no longer considered a "simple" request. This means that it can't be used to brute-force the credentials of non-cooperative endpoints, nor can it be used to execute CSRF-style attacks on routers and other IoT devices with hard-coded/default username/password values.

> In SAPUI5 we have an API for OData access, where we allow to pass username and password. We do not recommend to do so for security reasons and avoid it in applications shipped by SAP, but we know customers often are still using basic authentication, also with SAPUI5.

Can you point me to any docs and/or help me understand how widespread the practice might be?

From my point of view, it is a bad idea to intentionally break web standards, just because usage counters indicate they are rarely used. It still has been specified this way and there is no good reason to disable this functionality and to break the standard. 

The purpose of a standard is that developers can rely on them. As long a program is based on standard APIs it is expected to work cross browser and stay working in future releases of the browser. Your change breaks this contract, unlike all other browsers, unlike all previous versions of Chrome, the standardized API of the XMLHttpRequest is not working properly anymore. Please don't!

https://xhr.spec.whatwg.org/#the-open()-method

> From my point of view, it is a bad idea to intentionally break web standards, just because usage counters indicate they are rarely used. 

Understood. We generally try not to break things without reason. Blink's process is documented at https://www.chromium.org/blink#TOC-Policy-for-shipping-and-removing-web-platform-API-features, and boils down to evaluating the interoperability risk of changes before making them. Based on the metrics we can gather, this change seems to be within a level of risk that we can accept.

Learning that some of SAP's customers may rely on the behavior is certainly new information, however. It's possible that there just aren't many folks who rely on this behavior. Alternatively, it's quite possible that those customers have opted-out of sharing usage metrics with us, which makes it harder to evaluate the impact of a change.

> It still has been specified this way and there is no good reason to disable this functionality and to break the standard. 

I've outlined the reasons I consider "good" above. I recognize that you don't agree with the justification I've presented, but I think there are real security implications to the current functionality, and it looks like we can mitigate them without too much impact to the ecosystem.

> https://xhr.spec.whatwg.org/#the-open()-method

FWIW, https://github.com/whatwg/fetch/pull/465 is a patch against the Fetch standard that maps to what we're doing in Chrome. If it sticks, I agree that we'd certainly want to go back and update the XHR standard as well.

Happily for everyone, however, I'm not the decider. :)

FYI rbyers@, jochen@, foolip@: as the relevant API owners who approved the change. I can imagine a few ways of approaching things:

1. We revert, and find new ways of measuring.
2. We add a flag that enterprises can flip.
3. We press on with the status quo and see whether more reports flow in.

WDYT?

+palmer@, FYI, since this will impact your ability to remove the code entirely. :)

This is not about reverting the change. The approach to disallow credentials embedded in URLs is valid, there should be no more HTML pages, which have credentials in URLs. It just shouldn't have impact on the XHR object, where no such URL is passed by the application, but it is created internally. 

would the SAP app still break if we'd make open() strip the username/password out of the URL and put it into the request header instead?

the point of the change is to make it more difficult to brute force passwords. If you still allow it via a simple XHR call, it's pointless to remove it.

"would the SAP app still break if we'd make open() strip the username/password out of the URL and put it into the request header instead?"

I don't understand this proposal. Isn't this exactly what Chrome is already doing today? Sending a request without user/password contained in the URL, if the server returns a 401, it repeats the request with included Authorization header?

By the way, when doing a CORS request with XHR and user/password provided and the server does not return CORS allow headers on the 401 response, the request including the Authorization header is never sent. So I don't get how this could possibly misused for brute force attacks either.

As discussed in the Intent to Remove thread for this, we have no precise formula for making these trade-offs, we lean on experience from past removals that have worked out or been painful. In this case, what mkwst@ found in an analysis of httparchive hits was that many cases were mistakes, and even one malicious case was found.

So, I think we made the right decision at the time, but decisions are always tentative and if there's more breakage than we'd expect we can revisit.

For this issue, would SAP's XHR login work if there was a CORS pre-flight? If so then one workaround would be to consider these requests non-simple in one way or another.

mkwst@, what would be a way for SAP to test this? Just adding any random header to the request I guess?

#18 - see the suggestion in comment 8 and the explained enthusiasm in comment 9. :)

The question is whether that *actually* works for SAP or not, given that it'll involve a CORS pre-flight request.

I will try to answer your questions:

How widely this is used in SAPUI5 applications? Honestly, we don't know and we don't have a possibility to find out, our customers usually don't share their custom applications with us, nor do we have access to their systems. The problem is that usually core business processes are affected when SAP applications stop working and we want to avoid the risk of this to happen as much as possible.

Would it work us if there was a preflight request for CORS requests? We don't care much about CORS, so this doesn't matter to us. We use custom headers anyway when doing OData requests. As long passing name and password in XHR.open will result in basic authentication, everything is fine for us.

Can you please shed some light on me regarding the security issues which are adressed by making this a complex request? Do you have any links, where such brute force attacks are described? I really don't see how creating the Authorization header in the XHR does improve the situation from a security point of view? The basic authentication mechanism currently does send a kind of preflight request (without credentials) anyway. 

As far I understand, by adding the Authorization header in the XHR it will already be included in the initial request, even if the server doesn't require basic authentication? Is it a good idea to send unencrypted credentials to a server that didn't even ask for it? 

I'll start with a motivating scenario described at [1] in the hopes of clarifying my goal:

Assume that you have a TP-Link WR1043ND router at home that you bought in 2013, and haven't touched its firmware since. It is vulnerable to a trivial CSRF attack that allows anyone on the web to change your DNS server by making a GET request to `/userRpm/LanDhcpServerRpm.htm` with a few GET parameters. That page is protected by basic auth, but the username and password are `admin`/`admin`. In the status quo, this is trivially exploitable from the web by making a request to `http://admin:admin@192.168.1.1/...`. The actual attack did this via `<style>@import url(...)</style>`, but any subresource request would do, including XHR.

The important thing to note here is that the only thing that matters to the attacker is that the request hits the server, causing the state change. The browser will block the attacker's access to the response, as the endpoint didn't opt-in via CORS headers, but that's irrelevant, as the damage has already been done.

Forcing a preflight for these kinds of requests via the workaround you've suggested would first send an OPTIONS request with an `Access-Control-Request-Headers: authorization` header, asking permission to contact the server with these credentials. Only if the endpoint explicitly opted in would the above attack be possible. My intuition is that routers (and other IoT devices, like printers, and drones, and lightbulbs, and etc. that don't expect the internet to talk to them) would not.

> By the way, when doing a CORS request with XHR and user/password provided
> and the server does not return CORS allow headers on the 401 response, the
> request including the Authorization header is never sent.

I don't think this is accurate. As an example:

```
var x = new XMLHttpRequest();
x.open("GET", "https://www.w3.org/Member/", true, "My W3C Username", "My W3C Password"); 
x.withCredentials = true;
x.send(); 
```

sends an authenticated GET request. The response is blocked, as the endpoint doesn't serve reasonable CORS headers, but the request goes through without a preflight.

> So I don't get how this could possibly misused for brute force attacks either.

At least in Chrome, I agree with you that XHR isn't useful for bruteforce attacks, as we'll pop up a basic authentication dialog upon failure that results in a 401. We'll sufficiently mitigate that risk by dropping usernames/passwords from URLs.

> How widely this is used in SAPUI5 applications?

Is the API endpoint controlled by SAP? If so, would y'all be able to respond to OPTIONS requests, allowing `Authorization` headers from your customers' origins?

[1]: http://www.jakoblell.com/blog/2013/10/30/real-world-csrf-attack-hijacks-dns-server-configuration-of-tp-link-routers-2/

It occurs to me that we could reduce the risk of this change by changing XHR's internal implementation to treat:

```
x.open(..., ..., ..., "user", "pass")
```

as

```
x.open(..., ..., ...);
xhr.setRequestHeader ("Authorization", "Basic " + btoa(username + ":" + password));
```

internally. This would still require the endpoint to respond reasonably to an OPTIONS preflight, but it wouldn't force SAP's users to make any changes to their XHR code.

that was what I meant to propose in comment #14

I see. It might be a good idea!

Sounds nice. So, the conversion would happen in the Main Fetch step or earlier.

#22

I understand this is the motivation of the fix in general, but what you describe does not match the current implementation/behaviour of basic authentication with XHR in Chrome.

Neither would the attack on the TP-Link router work using an XHR request, nor does the request sent to "www.w3.org/Member/" in your example includes the credentials passed in the open() call.

The reason is, whatever credentials are provided, Chrome first sends a request without Authorization header. Only if the server returns a 401 response with a "WWW-Authenticate: Basic" header, a second request is sent, including the Authorization header with the given credentials.

In case of a cross origin requests, the second request is never sent, unless the 401 response comes with a matching "Access-Control-Allow-Origin" header (no wildcard allowed) and "Access-Control-Allow-Credentials" header.

#23

HTTP Authentication, as specified in https://tools.ietf.org/html/rfc7235 uses a challenge/response scheme, so when requesting a resource, the server returns information that authorization is required and which kind of authorization is requested. Just sending some basic authentication information, without knowing whether it is required at all or whether basic authentication is used, sounds not like a good idea to me.

> In case of a cross origin requests, the second request is never sent, unless the 401 response comes with a matching "Access-Control-Allow-Origin" header (no wildcard allowed) and "Access-Control-Allow-Credentials" header.

Thanks for poking at this again. I'm a little embarrassed to admit that I think you're right. :)

I realize now that I executed that example in the console of `w3.org`, so of course the `Authorization` header was present. It also seems that Chrome's sending the `Authorization` header on cross-origin requests if there's already a cached authorization entry (which might or might not actually match the spec). Trying things out in a fresh incognito window gives me results in line with your suggestions.

I'll dig into this a little more to make sure that I'm wrong. If so, then even though I'd prefer to just kill this mechanism entirely, I'll certainly agree that me wanting to do something isn't enough to justify frustrating unknown numbers of SAP customers, and find a way to carve out XHR.

CCing Anne, as I'd probably end up asking for a more specific `initiator` for XHR requests in Fetch.

malte.wedel@ - do you know whether those servers of your customers are typically running HTTPS? If that's the case, we could limit this at least to HTTPS connections?

jochen@ - While I fully agree that nobody should do basic authentication without HTTPS, for intranet scenarios this is not always the case. As I tried to point out before, you can't prevent setting an Authorization header anyway, so if you break basic authentication on the XHR API, people will start setting an Authorization header themselves and send it with every request, so it will rather get less secure than before.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6

commit fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6
Author: mkwst <mkwst@chromium.org>
Date: Wed Apr 12 14:38:23 2017

Carve out an exception for embedded credentials in XHR.

As discussed in  https://crbug.com/707761 , the security justification for
restricting username/password in XHR is weaker than I thought it was.
I'd still _like_ to remove developer-controlled usernames and passwords
from the platform, but I was incorrect to point to them as an actual
vulnerability, given the way basic/digest auth actually works (requiring
CORS-same-originness, and handshaking through a 401 response).

So, this patch limits the previous restrictions against embedded
credentials to non-XHR use cases. That will make SAP happy, and should
resolve the other complaints this change has generated.

BUG= 707761 ,708131,504300

Review-Url: https://codereview.chromium.org/2808753003
Cr-Commit-Position: refs/heads/master@{#464019}

[modify] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/chrome/browser/ui/login/login_handler_browsertest.cc
[modify] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/third_party/WebKit/LayoutTests/http/tests/cachestorage/resources/credentials-iframe.html
[modify] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/third_party/WebKit/LayoutTests/http/tests/cachestorage/serviceworker/credentials.html
[modify] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/third_party/WebKit/LayoutTests/http/tests/inspector/network/network-xhr-replay-expected.txt
[modify] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/third_party/WebKit/LayoutTests/http/tests/inspector/network/network-xhr-replay.html
[modify] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/fetch-access-control-login.html
[modify] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/fetch-access-control.php
[modify] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects-async-expected.txt
[modify] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/failed-auth-expected.txt
[modify] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/failed-auth.html
[add] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/logout-expected.txt
[add] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/logout.html
[modify] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/null-auth-expected.txt
[modify] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/null-auth.php
[add] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/remember-bad-password-expected.txt
[add] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/third_party/WebKit/LayoutTests/http/tests/xmlhttprequest/remember-bad-password.html
[modify] https://crrev.com/fd04d4a0b5f4a35c4acd66a0b35773deb33e8bb6/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp

Any updates? Can we close this?

Do we want to consider a merge for the carve out?

> Do we want to consider a merge for the carve out?

This landed in the same release as the original change. No merge necessary.

[Auto-generated comment by a script] We noticed that this issue is targeted for M-59; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-59 label, otherwise remove Merge-TBD label. Thanks.