Skip to content

Commit

Permalink
[Security] Do COOP & COEP parsing on redirects
Browse files Browse the repository at this point in the history
Parsing of the COOP and COEP header is currently not done for redirects.
This is a requirement for COOP to have that info even for redirects.
So moving it to the general repeated function.

Bug: 922191
Change-Id: Ifba6b4e9f0a649dd4492989bd17af5cd4d84a348
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2066931
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745127}
  • Loading branch information
hemeryar authored and Commit Bot committed Feb 27, 2020
1 parent 5084b4e commit 85eaa77
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 16 deletions.
55 changes: 55 additions & 0 deletions content/browser/cross_origin_opener_policy_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// found in the LICENSE file.

#include "base/test/scoped_feature_list.h"
#include "content/browser/frame_host/navigation_request.h"
#include "content/browser/frame_host/render_frame_host_impl.h"
#include "content/browser/web_contents/web_contents_impl.h"
#include "content/public/test/content_browser_test.h"
Expand Down Expand Up @@ -200,4 +201,58 @@ IN_PROC_BROWSER_TEST_F(CrossOriginOpenerPolicyBrowserTest,
web_contents()->GetController().GetLastCommittedEntry()->GetPageType(),
PAGE_TYPE_NORMAL);
}

class CrossOriginPolicyHeadersObserver : public WebContentsObserver {
public:
explicit CrossOriginPolicyHeadersObserver(
WebContents* web_contents,
network::mojom::CrossOriginEmbedderPolicyValue expected_coep,
network::mojom::CrossOriginOpenerPolicy expected_coop)
: WebContentsObserver(web_contents),
expected_coep_(expected_coep),
expected_coop_(expected_coop) {}

~CrossOriginPolicyHeadersObserver() override = default;

void DidRedirectNavigation(NavigationHandle* navigation_handle) override {
// Verify that the COOP/COEP headers were parsed.
NavigationRequest* navigation_request =
static_cast<NavigationRequest*>(navigation_handle);
CHECK(navigation_request->response()->cross_origin_embedder_policy.value ==
expected_coep_);
CHECK(navigation_request->response()->cross_origin_opener_policy ==
expected_coop_);
}

void DidFinishNavigation(NavigationHandle* navigation_handle) override {
// Verify that the COOP/COEP headers were parsed.
NavigationRequest* navigation_request =
static_cast<NavigationRequest*>(navigation_handle);
CHECK(navigation_request->response()->cross_origin_embedder_policy.value ==
expected_coep_);
CHECK(navigation_request->response()->cross_origin_opener_policy ==
expected_coop_);
}

private:
network::mojom::CrossOriginEmbedderPolicyValue expected_coep_;
network::mojom::CrossOriginOpenerPolicy expected_coop_;
};

IN_PROC_BROWSER_TEST_F(CrossOriginOpenerPolicyBrowserTest,
RedirectsParseCoopAndCoepHeaders) {
GURL redirect_initial_page(embedded_test_server()->GetURL(
"a.com", "/cross-origin-opener-policy_redirect_initial.html"));
GURL redirect_final_page(embedded_test_server()->GetURL(
"a.com", "/cross-origin-opener-policy_redirect_final.html"));

CrossOriginPolicyHeadersObserver obs(
web_contents(),
network::mojom::CrossOriginEmbedderPolicyValue::kRequireCorp,
network::mojom::CrossOriginOpenerPolicy::kSameOrigin);

EXPECT_TRUE(
NavigateToURL(shell(), redirect_initial_page, redirect_final_page));
}

} // namespace content
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Document with "Cross-Origin-Opener-Policy: same-origin"
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
HTTP/1.1 200 OK
Content-Type: text/html
Cross-Origin-Opener-Policy: same-origin
Cross-Origin-Embedder-Policy: require-corp
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Document with "Cross-Origin-Opener-Policy: same-origin"
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
HTTP/1.1 301 Moved Permanently
Location: cross-origin-opener-policy_redirect_final.html
Cross-Origin-Opener-Policy: same-origin
Cross-Origin-Embedder-Policy: require-corp
36 changes: 20 additions & 16 deletions services/network/url_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,26 @@ void PopulateResourceResponse(net::URLRequest* request,
response->ssl_info = request->ssl_info();
}

// TODO(ahemery): Cross origin isolation headers should only be parsed for
// secure contexts. Check here using IsUrlPotentiallyTrustworthy() once the
// tests are updated to use HTTPS.
if (base::FeatureList::IsEnabled(features::kCrossOriginIsolation)) {
// Parse the Cross-Origin-Embedder-Policy and
// Cross-Origin-Embedder-Policy-Report-Only headers.
response->cross_origin_embedder_policy =
URLLoader::ParseCrossOriginEmbedderPolicyValue(
request->response_headers());

// Parse the Cross-Origin-Opener-Policy header.
std::string raw_coop_string;
if (request->response_headers() &&
request->response_headers()->GetNormalizedHeader(
kCrossOriginOpenerPolicyHeader, &raw_coop_string)) {
response->cross_origin_opener_policy =
ParseCrossOriginOpenerPolicyHeader(raw_coop_string);
}
}

response->request_start = request->creation_time();
response->response_start = base::TimeTicks::Now();
response->encoded_data_length = request->GetTotalReceivedBytes();
Expand Down Expand Up @@ -1153,22 +1173,6 @@ void URLLoader::OnResponseStarted(net::URLRequest* url_request, int net_error) {
&(response_->content_security_policy));
}

if (base::FeatureList::IsEnabled(features::kCrossOriginIsolation)) {
// Parse the Cross-Origin-Embedder-Policy and
// Cross-Origin-Embedder-Policy-Report-Only headers.
response_->cross_origin_embedder_policy =
ParseCrossOriginEmbedderPolicyValue(url_request_->response_headers());

// Parse the Cross-Origin-Opener-Policy header.
std::string raw_coop_string;
if (url_request_->response_headers() &&
url_request_->response_headers()->GetNormalizedHeader(
kCrossOriginOpenerPolicyHeader, &raw_coop_string)) {
response_->cross_origin_opener_policy =
ParseCrossOriginOpenerPolicyHeader(raw_coop_string);
}
}

// If necessary, retrieve the associated origin policy, before sending the
// response to the client.
if (origin_policy_manager_ && url_request_->response_headers()) {
Expand Down

0 comments on commit 85eaa77

Please sign in to comment.