Fixed
Status Update
No update yet.
Comments
mm...@chromium.org
[Empty comment from Monorail migration]
[Monorail components: -Internals>Network Blink>SecurityFeature]
[Monorail components: -Internals>Network Blink>SecurityFeature]
mk...@chromium.org
Ideally, we'd move `Sec-Fetch-Mode` and `Sec-Fetch-User` to be added in the same place as `Sec-Fetch-Site`. I think that somehow folds into the work covered by https://bugs.chromium.org/p/chromium/issues/detail?id=963260 ?
lukasza@: Did we put together a plan here?
lukasza@: Did we put together a plan here?
mk...@chromium.org
[Empty comment from Monorail migration]
lu...@chromium.org
I've opened https://crbug.com/chromium/972263 to track moving Sec-Fetch-Mode into the NetworkService. This seems fairly easy (famous last words?) and seems desirable - I can try to tackle this in the next few days.
I am not sure what can be done about Sec-Fetch-User and Sec-Fetch-Dest - I think the NetworkService is not aware whether the request had an associated user gesture and doesn't know the destination (script/image/xhr/etc) of the request. I am not sure if this is related tohttps://crbug.com/chromium/963260 . It seems to me that whatever code does the HTTP->HTTPS upgrade is copying some, but not all headers? I think we just need to find this code and make sure it copies all headers (although maybe Sec-* headers are special and shouldn't be blindly copied?).
I am not sure what can be done about Sec-Fetch-User and Sec-Fetch-Dest - I think the NetworkService is not aware whether the request had an associated user gesture and doesn't know the destination (script/image/xhr/etc) of the request. I am not sure if this is related to
lu...@chromium.org
falken@, can you double-check if what I said in https://crbug.com/chromium/971938#c4 about https://crbug.com/chromium/963260 makes sense? I don't really know what exactly is meant by "Implement HTTP header layer division".
fa...@chromium.org
I think that makes sense. If you want to add the headers only when going to the network/cache layer, yes moving to Network Service sounds good.
https://crbug.com/chromium/963260 is about this: https://fetch.spec.whatwg.org/#http-header-layer-division , which we don't currently implement. For header injection via URLLoaderThrottle it happens before service workers, but some headers are intended to be added before service workers and some are intended to be after.
mk...@chromium.org
[Empty comment from Monorail migration]
mk...@chromium.org
aaj@ tells me that this is blocking their server-side rollout.
mk...@chromium.org
[Empty comment from Monorail migration]
mk...@chromium.org
[Empty comment from Monorail migration]
bu...@chops-service-accounts.iam.gserviceaccount.com
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src.git/+/5546163adf70ae04f63ca19752a3062615913de4
commit 5546163adf70ae04f63ca19752a3062615913de4
Author: Mike West <mkwst@chromium.org>
Date: Thu Aug 22 12:23:42 2019
Temporarily send `Sec-Fetch-*` headers via non-secure transport.
After [1], we're stripping `Sec-Fetch-*` headers from non-secure
transport. This is lovely! Except insofar as it also strips the headers
from secure redirect targets. Which is a problem, as it gives attackers
the option of laundering their requests through HTTP to strip the
headers.
This patch removes the secure transport restriction while we figure out
a cleaner way of removing the headers on a per-hop basis.
[1]:https://chromium-review.googlesource.com/c/chromium/src/+/1647354
Bug: 995745, 971938, 964053
Change-Id: Icec4e685902b7be2983bb81b7289ac9b45467782
Reviewed-on:https://chromium-review.googlesource.com/c/chromium/src/+/1762079
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Eric Lawrence [MSFT] <ericlaw@microsoft.com>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689428}
[modify]https://crrev.com/5546163adf70ae04f63ca19752a3062615913de4/content/browser/frame_host/navigation_request.cc
[modify]https://crrev.com/5546163adf70ae04f63ca19752a3062615913de4/content/browser/loader/browser_initiated_resource_request.cc
[modify]https://crrev.com/5546163adf70ae04f63ca19752a3062615913de4/services/network/url_loader.cc
[modify]https://crrev.com/5546163adf70ae04f63ca19752a3062615913de4/services/network/url_loader_unittest.cc
[modify]https://crrev.com/5546163adf70ae04f63ca19752a3062615913de4/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc
[modify]https://crrev.com/5546163adf70ae04f63ca19752a3062615913de4/third_party/blink/web_tests/TestExpectations
[modify]https://crrev.com/5546163adf70ae04f63ca19752a3062615913de4/third_party/blink/web_tests/external/wpt/fetch/sec-metadata/redirect/multiple-redirect-https-downgrade-upgrade.tentative.sub.html
[modify]https://crrev.com/5546163adf70ae04f63ca19752a3062615913de4/third_party/blink/web_tests/external/wpt/fetch/sec-metadata/redirect/redirect-http-upgrade.tentative.sub.html
[add]https://crrev.com/5546163adf70ae04f63ca19752a3062615913de4/third_party/blink/web_tests/external/wpt/fetch/sec-metadata/redirect/redirect-https-downgrade.tentative.sub-expected.txt
[modify]https://crrev.com/5546163adf70ae04f63ca19752a3062615913de4/third_party/blink/web_tests/external/wpt/fetch/sec-metadata/redirect/redirect-https-downgrade.tentative.sub.html
[modify]https://crrev.com/5546163adf70ae04f63ca19752a3062615913de4/third_party/blink/web_tests/external/wpt/fetch/sec-metadata/resources/redirectTestHelper.sub.js
[delete]https://crrev.com/cfd096e272123ca46a50fff3858633e23ca0d60e/third_party/blink/web_tests/virtual/blink-cors/external/wpt/fetch/sec-metadata/redirect/multiple-redirect-https-downgrade-upgrade.tentative.sub-expected.txt
[delete]https://crrev.com/cfd096e272123ca46a50fff3858633e23ca0d60e/third_party/blink/web_tests/virtual/blink-cors/external/wpt/fetch/sec-metadata/redirect/redirect-http-upgrade.tentative.sub-expected.txt
commit 5546163adf70ae04f63ca19752a3062615913de4
Author: Mike West <mkwst@chromium.org>
Date: Thu Aug 22 12:23:42 2019
Temporarily send `Sec-Fetch-*` headers via non-secure transport.
After [1], we're stripping `Sec-Fetch-*` headers from non-secure
transport. This is lovely! Except insofar as it also strips the headers
from secure redirect targets. Which is a problem, as it gives attackers
the option of laundering their requests through HTTP to strip the
headers.
This patch removes the secure transport restriction while we figure out
a cleaner way of removing the headers on a per-hop basis.
[1]:
Bug: 995745, 971938, 964053
Change-Id: Icec4e685902b7be2983bb81b7289ac9b45467782
Reviewed-on:
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Eric Lawrence [MSFT] <ericlaw@microsoft.com>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689428}
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[add]
[modify]
[modify]
[delete]
[delete]
mk...@chromium.org
[Empty comment from Monorail migration]
bu...@chops-service-accounts.iam.gserviceaccount.com
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src.git/+/17df479e95b1b2a463fcdb7e335142f30e30e0e9
commit 17df479e95b1b2a463fcdb7e335142f30e30e0e9
Author: Mike West <mkwst@chromium.org>
Date: Fri Sep 06 12:04:16 2019
Revert "Temporarily send `Sec-Fetch-*` headers via non-secure transport."
This commit reverts the change in [1] which sent `Sec-Fetch-*` headers
over non-secure transport. Now that [2] has landed, we can impose the
restriction safely, as we're consistently setting `Sec-Fetch-Mode`
whenever we set `Sec-Fetch-Site`.
The `*.cc` file changes are a strict revert of the original patch. The
test changes are rewrites of tests that originally landed with incorrect
expectations. Now that the code is sending the `Sec-Fetch-Mode` header
when it's supposed to, the tests need to change as well.
[1]:https://chromium-review.googlesource.com/c/chromium/src/+/1762079
[2]:https://chromium-review.googlesource.com/c/chromium/src/+/1780725
Bug: 995745, 971938, 964053
Change-Id: I240ed9e62d18583c2d48e290d1cae086a387fc8b
Reviewed-on:https://chromium-review.googlesource.com/c/chromium/src/+/1787318
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694207}
[modify]https://crrev.com/17df479e95b1b2a463fcdb7e335142f30e30e0e9/content/browser/frame_host/navigation_request.cc
[modify]https://crrev.com/17df479e95b1b2a463fcdb7e335142f30e30e0e9/content/browser/loader/browser_initiated_resource_request.cc
[modify]https://crrev.com/17df479e95b1b2a463fcdb7e335142f30e30e0e9/services/network/url_loader.cc
[modify]https://crrev.com/17df479e95b1b2a463fcdb7e335142f30e30e0e9/services/network/url_loader_unittest.cc
[modify]https://crrev.com/17df479e95b1b2a463fcdb7e335142f30e30e0e9/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc
[modify]https://crrev.com/17df479e95b1b2a463fcdb7e335142f30e30e0e9/third_party/blink/web_tests/TestExpectations
[modify]https://crrev.com/17df479e95b1b2a463fcdb7e335142f30e30e0e9/third_party/blink/web_tests/external/wpt/fetch/sec-metadata/appcache.tentative.https.sub.html
[modify]https://crrev.com/17df479e95b1b2a463fcdb7e335142f30e30e0e9/third_party/blink/web_tests/external/wpt/fetch/sec-metadata/preload.tentative.https.sub.html
[modify]https://crrev.com/17df479e95b1b2a463fcdb7e335142f30e30e0e9/third_party/blink/web_tests/external/wpt/fetch/sec-metadata/redirect/multiple-redirect-https-downgrade-upgrade.tentative.sub.html
[modify]https://crrev.com/17df479e95b1b2a463fcdb7e335142f30e30e0e9/third_party/blink/web_tests/external/wpt/fetch/sec-metadata/redirect/redirect-http-upgrade.tentative.sub.html
[modify]https://crrev.com/17df479e95b1b2a463fcdb7e335142f30e30e0e9/third_party/blink/web_tests/external/wpt/fetch/sec-metadata/redirect/redirect-https-downgrade.tentative.sub.html
[modify]https://crrev.com/17df479e95b1b2a463fcdb7e335142f30e30e0e9/third_party/blink/web_tests/external/wpt/fetch/sec-metadata/resources/redirectTestHelper.sub.js
[add]https://crrev.com/17df479e95b1b2a463fcdb7e335142f30e30e0e9/third_party/blink/web_tests/virtual/blink-cors/external/wpt/fetch/sec-metadata/redirect/multiple-redirect-https-downgrade-upgrade.tentative.sub-expected.txt
[add]https://crrev.com/17df479e95b1b2a463fcdb7e335142f30e30e0e9/third_party/blink/web_tests/virtual/blink-cors/external/wpt/fetch/sec-metadata/redirect/redirect-http-upgrade.tentative.sub-expected.txt
commit 17df479e95b1b2a463fcdb7e335142f30e30e0e9
Author: Mike West <mkwst@chromium.org>
Date: Fri Sep 06 12:04:16 2019
Revert "Temporarily send `Sec-Fetch-*` headers via non-secure transport."
This commit reverts the change in [1] which sent `Sec-Fetch-*` headers
over non-secure transport. Now that [2] has landed, we can impose the
restriction safely, as we're consistently setting `Sec-Fetch-Mode`
whenever we set `Sec-Fetch-Site`.
The `*.cc` file changes are a strict revert of the original patch. The
test changes are rewrites of tests that originally landed with incorrect
expectations. Now that the code is sending the `Sec-Fetch-Mode` header
when it's supposed to, the tests need to change as well.
[1]:
[2]:
Bug: 995745, 971938, 964053
Change-Id: I240ed9e62d18583c2d48e290d1cae086a387fc8b
Reviewed-on:
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694207}
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[add]
[add]
bu...@chops-service-accounts.iam.gserviceaccount.com
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src.git/+/c03d589f642ce786c36940fb2254ff6e8dc1dd21
commit c03d589f642ce786c36940fb2254ff6e8dc1dd21
Author: Mike West <mkwst@chromium.org>
Date: Fri Sep 13 18:30:12 2019
Revert "Temporarily send `Sec-Fetch-*` headers via non-secure transport."
This commit reverts the change in [1] which sent `Sec-Fetch-*` headers
over non-secure transport. Now that [2] has landed, we can impose the
restriction safely, as we're consistently setting `Sec-Fetch-Mode`
whenever we set `Sec-Fetch-Site`.
The `*.cc` file changes are a strict revert of the original patch. The
test changes are rewrites of tests that originally landed with incorrect
expectations. Now that the code is sending the `Sec-Fetch-Mode` header
when it's supposed to, the tests need to change as well.
[1]:https://chromium-review.googlesource.com/c/chromium/src/+/1762079
[2]:https://chromium-review.googlesource.com/c/chromium/src/+/1780725
(cherry picked from commit 17df479e95b1b2a463fcdb7e335142f30e30e0e9)
Bug: 995745, 971938, 964053
Change-Id: I240ed9e62d18583c2d48e290d1cae086a387fc8b
Reviewed-on:https://chromium-review.googlesource.com/c/chromium/src/+/1787318
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#694207}
Reviewed-on:https://chromium-review.googlesource.com/c/chromium/src/+/1804294
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/branch-heads/3904@{#157}
Cr-Branched-From: 675968a8c657a3bd9c1c2c20c5d2935577bbc5e6-refs/heads/master@{#693954}
[modify]https://crrev.com/c03d589f642ce786c36940fb2254ff6e8dc1dd21/content/browser/frame_host/navigation_request.cc
[modify]https://crrev.com/c03d589f642ce786c36940fb2254ff6e8dc1dd21/content/browser/loader/browser_initiated_resource_request.cc
[modify]https://crrev.com/c03d589f642ce786c36940fb2254ff6e8dc1dd21/services/network/url_loader.cc
[modify]https://crrev.com/c03d589f642ce786c36940fb2254ff6e8dc1dd21/services/network/url_loader_unittest.cc
[modify]https://crrev.com/c03d589f642ce786c36940fb2254ff6e8dc1dd21/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc
[modify]https://crrev.com/c03d589f642ce786c36940fb2254ff6e8dc1dd21/third_party/blink/web_tests/TestExpectations
[modify]https://crrev.com/c03d589f642ce786c36940fb2254ff6e8dc1dd21/third_party/blink/web_tests/external/wpt/fetch/sec-metadata/appcache.tentative.https.sub.html
[modify]https://crrev.com/c03d589f642ce786c36940fb2254ff6e8dc1dd21/third_party/blink/web_tests/external/wpt/fetch/sec-metadata/preload.tentative.https.sub.html
[modify]https://crrev.com/c03d589f642ce786c36940fb2254ff6e8dc1dd21/third_party/blink/web_tests/external/wpt/fetch/sec-metadata/redirect/multiple-redirect-https-downgrade-upgrade.tentative.sub.html
[modify]https://crrev.com/c03d589f642ce786c36940fb2254ff6e8dc1dd21/third_party/blink/web_tests/external/wpt/fetch/sec-metadata/redirect/redirect-http-upgrade.tentative.sub.html
[modify]https://crrev.com/c03d589f642ce786c36940fb2254ff6e8dc1dd21/third_party/blink/web_tests/external/wpt/fetch/sec-metadata/redirect/redirect-https-downgrade.tentative.sub.html
[modify]https://crrev.com/c03d589f642ce786c36940fb2254ff6e8dc1dd21/third_party/blink/web_tests/external/wpt/fetch/sec-metadata/resources/redirectTestHelper.sub.js
[add]https://crrev.com/c03d589f642ce786c36940fb2254ff6e8dc1dd21/third_party/blink/web_tests/virtual/blink-cors/external/wpt/fetch/sec-metadata/redirect/multiple-redirect-https-downgrade-upgrade.tentative.sub-expected.txt
[add]https://crrev.com/c03d589f642ce786c36940fb2254ff6e8dc1dd21/third_party/blink/web_tests/virtual/blink-cors/external/wpt/fetch/sec-metadata/redirect/redirect-http-upgrade.tentative.sub-expected.txt
commit c03d589f642ce786c36940fb2254ff6e8dc1dd21
Author: Mike West <mkwst@chromium.org>
Date: Fri Sep 13 18:30:12 2019
Revert "Temporarily send `Sec-Fetch-*` headers via non-secure transport."
This commit reverts the change in [1] which sent `Sec-Fetch-*` headers
over non-secure transport. Now that [2] has landed, we can impose the
restriction safely, as we're consistently setting `Sec-Fetch-Mode`
whenever we set `Sec-Fetch-Site`.
The `*.cc` file changes are a strict revert of the original patch. The
test changes are rewrites of tests that originally landed with incorrect
expectations. Now that the code is sending the `Sec-Fetch-Mode` header
when it's supposed to, the tests need to change as well.
[1]:
[2]:
(cherry picked from commit 17df479e95b1b2a463fcdb7e335142f30e30e0e9)
Bug: 995745, 971938, 964053
Change-Id: I240ed9e62d18583c2d48e290d1cae086a387fc8b
Reviewed-on:
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#694207}
Reviewed-on:
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/branch-heads/3904@{#157}
Cr-Branched-From: 675968a8c657a3bd9c1c2c20c5d2935577bbc5e6-refs/heads/master@{#693954}
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[add]
[add]
mk...@chromium.org
[Empty comment from Monorail migration]
[Monorail components: Blink>SecurityFeature>FetchMetadata]
[Monorail components: Blink>SecurityFeature>FetchMetadata]
mk...@chromium.org
[Empty comment from Monorail migration]
mk...@chromium.org
`Sec-Fetch-Mode` has moved to the network service, `Sec-Fetch-User` is https://chromium-review.googlesource.com/c/chromium/src/+/1930742 (https://crbug.com/chromium/1027528 ), and `Sec-Fetch-Dest` is https://chromium-review.googlesource.com/c/chromium/src/+/1948825 (https://crbug.com/chromium/1029742 ). Closing this out in favor of those more specific issues.
is...@google.com
This issue was migrated from crbug.com/chromium/971938?no_tracker_redirect=1
[Multiple monorail components: Blink>SecurityFeature, Blink>SecurityFeature>FetchMetadata]
[Monorail blocked-on:crbug.com/chromium/972263 , crbug.com/chromium/995745 ]
[Monorail blocking:crbug.com/chromium/843478 ]
[Monorail mergedwith:crbug.com/chromium/989502 , crbug.com/chromium/990523 ]
[Monorail components added to Component Tags custom field.]
[Multiple monorail components: Blink>SecurityFeature, Blink>SecurityFeature>FetchMetadata]
[Monorail blocked-on:
[Monorail blocking:
[Monorail mergedwith:
[Monorail components added to Component Tags custom field.]
Description
It appears that other request headers such as Sec-Fetch-Mode are added at a higher level when the requests are prepared but are not attempted to be added again during redirects whereas Sec-Fetch-Site is handled lower down at the UrlLoader layer and each redirect will attempt to add the header if it's missing.
This may potentially be by-design if Sec-Fetch-Site is meant to operate differently.