Content-Security-Policy upgrade-insecure-requests should not be applied to 127.0.0.0/8
Reported by
rkno...@box.com,
Mar 21 2018
|
||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.181 Safari/537.36 Steps to reproduce the problem: 1. Set up a web page to include the Content-Security-Policy header with the upgrade-insecure-requests directive 2. In the supplied web page, make a request to http://127.0.0.1:<someport> via XMLHttpRequest object. 3. Note that the XMHttpRequest is made via https in the console logs. What is the expected behavior? Given that per the W3C spec 127.0.0.0/8 and ::1 are considered a trustworthy origin when using HTTP: https://www.w3.org/TR/secure-contexts/#is-origin-trustworthy The expectation is that HTTP requests to 127.0.0.0/8 and ::1 are NOT modified to HTTPS when the upgrade-insecure-requests directive is applied. What went wrong? CORS requests to 127.0.0.0/8 and ::1 made over HTTP are being modified to be made over HTTPS. The local application listening therefor misses the requests, since they are listening for HTTP requests. Did this work before? No Does this work in other browsers? N/A Chrome version: 65.0.3325.181 Channel: stable OS Version: 6.1 (Windows 7, Windows Server 2008 R2) Flash Version: Note that the fix to treat 127.0.0.0/8 and ::1 as trusted origins went into Chromium with the fix for https://bugs.chromium.org/p/chromium/issues/detail?id=607878
,
Mar 23 2018
rknotts@ - Thanks for filing the issue...!! Could you please provide a sample test file/url to test the issue from TE-end. This will help us in triaging the issue further. Thanks...!!
,
Mar 23 2018
Hi, attaching a file which replicates the issue. You can see in the Developer Tools that the request to http://127.0.0.1/ is sent by https if the Content-Security-Policy is set, and is sent by http if it is not set (can comment out that line from the file to test that case). Let me know if you have any problems with this.
,
Mar 23 2018
Thank you for providing more feedback. Adding the requester to the cc list. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 27 2018
Yup. Seems like a bug in our implementation.
,
Apr 24 2018
Appreciate the consideration given to this bug report. It's been a month since the last update. Any chance this will be picked up in an upcoming development sprint?
,
May 3 2018
Hello... Is there any update on this issue?
,
Sep 10
Can we please get an update on this?
,
Sep 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/babf69903174083558f79ac4f220644a39730e76 commit babf69903174083558f79ac4f220644a39730e76 Author: Mike West <mkwst@chromium.org> Date: Fri Sep 14 11:53:42 2018 Don't upgrade trusted URLs. `http://127.0.0.1` does not need to be upgraded, as it's "potentially trustworthy" in and of itself. Bug: 824462 Change-Id: I6e67abf258557e6be97ba1540f363c8586a5a24e Reviewed-on: https://chromium-review.googlesource.com/1224390 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Carlos IL <carlosil@chromium.org> Reviewed-by: Andy Paicu <andypaicu@chromium.org> Reviewed-by: Adam Rice <ricea@chromium.org> Commit-Queue: Mike West <mkwst@chromium.org> Cr-Commit-Position: refs/heads/master@{#591326} [modify] https://crrev.com/babf69903174083558f79ac4f220644a39730e76/content/browser/frame_host/navigation_handle_impl_browsertest.cc [modify] https://crrev.com/babf69903174083558f79ac4f220644a39730e76/content/common/content_security_policy/csp_context.cc [modify] https://crrev.com/babf69903174083558f79ac4f220644a39730e76/content/common/content_security_policy/csp_context_unittest.cc [delete] https://crrev.com/cb7d3c4ab1933773657c87a8c9b32bd9db41619c/third_party/WebKit/LayoutTests/http/tests/security/upgrade-insecure-requests/basic-upgrade.https.html [modify] https://crrev.com/babf69903174083558f79ac4f220644a39730e76/third_party/WebKit/LayoutTests/http/tests/security/upgrade-insecure-requests/form-upgrade.html [modify] https://crrev.com/babf69903174083558f79ac4f220644a39730e76/third_party/WebKit/LayoutTests/http/tests/security/upgrade-insecure-requests/iframe-upgrade.https.html [modify] https://crrev.com/babf69903174083558f79ac4f220644a39730e76/third_party/WebKit/LayoutTests/http/tests/security/upgrade-insecure-requests/sandbox-upgrade.https.php [modify] https://crrev.com/babf69903174083558f79ac4f220644a39730e76/third_party/blink/renderer/core/loader/form_submission.cc [modify] https://crrev.com/babf69903174083558f79ac4f220644a39730e76/third_party/blink/renderer/core/loader/frame_loader.cc [modify] https://crrev.com/babf69903174083558f79ac4f220644a39730e76/third_party/blink/renderer/modules/websockets/dom_websocket.cc [modify] https://crrev.com/babf69903174083558f79ac4f220644a39730e76/third_party/blink/renderer/modules/websockets/dom_websocket_test.cc
,
Sep 17
,
Sep 17
Glad to know this is fixed. Which version of Chrome will have this fix ?
,
Sep 17
It should be in Chrome M71
,
Sep 17
Dearest release managers, WDYT about merging this back to M70? We have reports from some enterprisy users that this is breaking `box.com`'s integration with Concur.
,
Sep 17
This bug requires manual review: M70 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 18
How safe is this merge overall? Seems like it's been broken since March, so let's target M71?
,
Sep 18
The change is pretty straightforward, something like changing `!url.ProtocolIs("http")` to `!url.ProtocolIs("http") || SecurityOrigin::Create(url)->IsPotentiallyTrustworthy()` in ~4 places in the codebase. Most of the CL is tests.
The feature has indeed been broken since March. What I hear from `box.com` is that they've had some users staying on an old version of their product in order to avoid the bug. That old version goes out of support in October, around the time 70 is aiming to hit stable. I think this is worth merging back to 70 to reduce the impact on users who will lose the existing workaround in the nearish future.
,
Sep 18
We have several Box customers for whom this is a significant blocker. I would love to see it pulled into 70 if at all possible.
,
Sep 18
Thanks for more feedback. Approving merge to M70, branch:3538
,
Sep 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/daa0505d0f357c205f645d06cc90454e56332fdf commit daa0505d0f357c205f645d06cc90454e56332fdf Author: Mike West <mkwst@chromium.org> Date: Tue Sep 18 17:09:33 2018 Don't upgrade trusted URLs. `http://127.0.0.1` does not need to be upgraded, as it's "potentially trustworthy" in and of itself. Bug: 824462 Change-Id: I6e67abf258557e6be97ba1540f363c8586a5a24e Reviewed-on: https://chromium-review.googlesource.com/1224390 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Carlos IL <carlosil@chromium.org> Reviewed-by: Andy Paicu <andypaicu@chromium.org> Reviewed-by: Adam Rice <ricea@chromium.org> Commit-Queue: Mike West <mkwst@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#591326}(cherry picked from commit babf69903174083558f79ac4f220644a39730e76) Reviewed-on: https://chromium-review.googlesource.com/1231297 Reviewed-by: Mike West <mkwst@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#499} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/daa0505d0f357c205f645d06cc90454e56332fdf/content/browser/frame_host/navigation_handle_impl_browsertest.cc [modify] https://crrev.com/daa0505d0f357c205f645d06cc90454e56332fdf/content/common/content_security_policy/csp_context.cc [modify] https://crrev.com/daa0505d0f357c205f645d06cc90454e56332fdf/content/common/content_security_policy/csp_context_unittest.cc [delete] https://crrev.com/c41969b7255003d5a46bc3be155638e851f85be8/third_party/WebKit/LayoutTests/http/tests/security/upgrade-insecure-requests/basic-upgrade.https.html [modify] https://crrev.com/daa0505d0f357c205f645d06cc90454e56332fdf/third_party/WebKit/LayoutTests/http/tests/security/upgrade-insecure-requests/form-upgrade.html [modify] https://crrev.com/daa0505d0f357c205f645d06cc90454e56332fdf/third_party/WebKit/LayoutTests/http/tests/security/upgrade-insecure-requests/iframe-upgrade.https.html [modify] https://crrev.com/daa0505d0f357c205f645d06cc90454e56332fdf/third_party/WebKit/LayoutTests/http/tests/security/upgrade-insecure-requests/sandbox-upgrade.https.php [modify] https://crrev.com/daa0505d0f357c205f645d06cc90454e56332fdf/third_party/blink/renderer/core/loader/form_submission.cc [modify] https://crrev.com/daa0505d0f357c205f645d06cc90454e56332fdf/third_party/blink/renderer/core/loader/frame_loader.cc [modify] https://crrev.com/daa0505d0f357c205f645d06cc90454e56332fdf/third_party/blink/renderer/modules/websockets/dom_websocket.cc [modify] https://crrev.com/daa0505d0f357c205f645d06cc90454e56332fdf/third_party/blink/renderer/modules/websockets/dom_websocket_test.cc
,
Sep 18
Thanks, merged.
,
Sep 18
Box integrations across the board, including hundreds of thousands of users with the Box.com and SalesForce integration, for example, are completely out of luck. Box kept saying this was a "2-3 week fix". The financial cost to companies affected has been in the millions of dollars and thousands of lost hours of time. This has caused other financial and non-financial impacts which are unquantifiable. Please fix this as soon as possible. This has had an enormous impact on an unquantifiable amount of users and companies. |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by krajshree@chromium.org
, Mar 22 2018