New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 881234 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Incorrect request headers when request is upgraded from http: to https:

Project Member Reported by vogelheim@google.com, Sep 6

Issue description

AddAdditionalRequestHeader (in navigation_request.cc) is called before CSP handling, which might upgrade a request from http: to https:. Thus, headers that depend on the difference between http: and https: will be incorrectly set, since they will not be modified accordingly.

This probably affects Origin: Sec-Metadata: and Sec-Origin-Policy:

Also: "As specified, we ought to be processing upgrades quite early (2.3 in https://fetch.spec.whatwg.org/#main-fetch), before tacking on these additional headers."


---------

For background and additional thoughts, see discussion on cl 1177607.
https://chromium-review.googlesource.com/c/chromium/src/+/1177607

Specifically, this sequence of comments on patch set 3:
https://chromium-review.googlesource.com/c/chromium/src/+/1177607/3/content/browser/frame_host/origin_policy_throttle.cc#72


 
Components: UI>Browser>Navigation
Cc: carlosil@chromium.org andypaicu@chromium.org vogelheim@chromium.org mkwst@chromium.org alex...@chromium.org arthurso...@chromium.org est...@chromium.org clamy@chromium.org
Labels: -Pri-3 OS-Chrome OS-Mac OS-Windows Pri-2
Status: Available (was: Unconfirmed)
Adding a few folks who've worked on upgrade-insecure-requests or CSP for PlzNavigate.  Looks like the browser-side upgrade in NavigationRequest was introduced in estark's r478478, at which point the Origin header probably started to be affected by this issue.  Sec-Metadata and Sec-Origin-Policy were added to AddAdditionalRequestHeaders() later and also took dependencies on the original URL.

Anyone have any thoughts on fixing this? Can we move AddAdditionalRequestHeaders(), along with initialization of |begin_params_->headers|, to be called after CheckContentSecurityPolicy(), but before creating the NavigationHandle in NavigationRequest::BeginNavigation()?  Looks like there are also some other headers being set in NavigationRequest's constructor, which should probably be moved to AddAdditionalRequestHeaders().

Sign in to add a comment