Skip to content

Commit

Permalink
Stop parsing COEP in the browser process
Browse files Browse the repository at this point in the history
At https://crrev.com/c/2098190 I introduced a COEP parsing logic to
content/common but it was problematic - parsing an untrusted data in
the browser process is forbidden.

This CL moves the parsing logic from the browser process to the renderer
process.

Bug: 887967
Change-Id: I3bf4bd0f6d727573406e2f866df6c1ffdc7f59e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2132902
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757803}
  • Loading branch information
yutakahirano authored and Commit Bot committed Apr 9, 2020
1 parent baae534 commit 57010c9
Show file tree
Hide file tree
Showing 11 changed files with 90 additions and 80 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,7 @@ class CacheStorageCacheTest : public testing::Test {
nullptr /* side_data_blob */,
nullptr /* side_data_blob_for_cache_put */,
std::vector<network::mojom::ContentSecurityPolicyPtr>(),
network::CrossOriginEmbedderPolicy(),
false /* loaded_with_credentials */);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,7 @@ class CacheStorageManagerTest : public testing::Test {
nullptr /* side_data_blob */,
nullptr /* side_data_blob_for_cache_put */,
std::vector<network::mojom::ContentSecurityPolicyPtr>(),
network::CrossOriginEmbedderPolicy(),
false /* loaded_with_credentials */);

blink::mojom::BatchOperationPtr operation =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ blink::mojom::FetchAPIResponsePtr CreateResponse(
metadata.response().cors_exposed_header_names().end()),
nullptr /* side_data_blob */, nullptr /* side_data_blob_for_cache_put */,
std::vector<network::mojom::ContentSecurityPolicyPtr>(),
network::CrossOriginEmbedderPolicy(),
metadata.response().loaded_with_credentials());
}

Expand Down
1 change: 1 addition & 0 deletions content/common/background_fetch/background_fetch_types.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ blink::mojom::FetchAPIResponsePtr BackgroundFetchSettledFetch::CloneResponse(
CloneSerializedBlob(response->side_data_blob),
CloneSerializedBlob(response->side_data_blob_for_cache_put),
mojo::Clone(response->content_security_policy),
response->cross_origin_embedder_policy,
response->loaded_with_credentials);
}

Expand Down
56 changes: 2 additions & 54 deletions content/common/service_worker/service_worker_loader_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "content/public/common/content_features.h"
#include "mojo/public/cpp/bindings/self_owned_receiver.h"
#include "net/http/http_util.h"
#include "net/http/structured_headers.h"
#include "net/url_request/redirect_util.h"
#include "services/network/public/cpp/content_security_policy/content_security_policy.h"
#include "services/network/public/cpp/cross_origin_embedder_policy.h"
Expand Down Expand Up @@ -49,37 +48,6 @@ class BlobCompleteCaller : public blink::mojom::BlobReaderClient {
BlobCompleteCallback callback_;
};

std::pair<network::mojom::CrossOriginEmbedderPolicyValue,
base::Optional<std::string>>
ParseCrossOriginEmbedderPolicyValueInternal(
const net::HttpResponseHeaders* headers,
base::StringPiece header_name) {
static constexpr char kRequireCorp[] = "require-corp";
constexpr auto kNone = network::mojom::CrossOriginEmbedderPolicyValue::kNone;
using Item = net::structured_headers::Item;
std::string header_value;
if (!headers ||
!headers->GetNormalizedHeader(header_name.as_string(), &header_value)) {
return std::make_pair(kNone, base::nullopt);
}
const auto item = net::structured_headers::ParseItem(header_value);
if (!item || item->item.Type() != Item::kTokenType ||
item->item.GetString() != kRequireCorp) {
return std::make_pair(kNone, base::nullopt);
}
base::Optional<std::string> endpoint;
auto it = std::find_if(item->params.cbegin(), item->params.cend(),
[](const std::pair<std::string, Item>& param) {
return param.first == "report-to";
});
if (it != item->params.end() && it->second.Type() == Item::kStringType) {
endpoint = it->second.GetString();
}
return std::make_pair(
network::mojom::CrossOriginEmbedderPolicyValue::kRequireCorp,
std::move(endpoint));
}

} // namespace

// static
Expand Down Expand Up @@ -124,28 +92,6 @@ void ServiceWorkerLoaderHelpers::SaveResponseHeaders(
if (out_head->content_length == -1)
out_head->content_length = out_head->headers->GetContentLength();

// TODO(yhirano): Remove the code duplication with
// //services/network/url_loader.cc.
if (base::FeatureList::IsEnabled(
network::features::kCrossOriginEmbedderPolicy)) {
// Parse the Cross-Origin-Embedder-Policy and
// Cross-Origin-Embedder-Policy-Report-Only headers.

static constexpr char kCrossOriginEmbedderPolicyValueHeader[] =
"Cross-Origin-Embedder-Policy";
static constexpr char kCrossOriginEmbedderPolicyValueReportOnlyHeader[] =
"Cross-Origin-Embedder-Policy-Report-Only";
network::CrossOriginEmbedderPolicy coep;
std::tie(coep.value, coep.reporting_endpoint) =
ParseCrossOriginEmbedderPolicyValueInternal(
out_head->headers.get(), kCrossOriginEmbedderPolicyValueHeader);
std::tie(coep.report_only_value, coep.report_only_reporting_endpoint) =
ParseCrossOriginEmbedderPolicyValueInternal(
out_head->headers.get(),
kCrossOriginEmbedderPolicyValueReportOnlyHeader);
out_head->cross_origin_embedder_policy = coep;
}

// TODO(pmeuleman): Remove the code duplication with
// //services/network/url_loader.cc.
if (base::FeatureList::IsEnabled(
Expand Down Expand Up @@ -183,6 +129,8 @@ void ServiceWorkerLoaderHelpers::SaveResponseInfo(
out_head->did_service_worker_navigation_preload = false;
out_head->content_security_policy =
mojo::Clone(response.content_security_policy);
out_head->cross_origin_embedder_policy =
response.cross_origin_embedder_policy;
}

// static
Expand Down
34 changes: 34 additions & 0 deletions services/network/public/cpp/cross_origin_embedder_policy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,21 @@

#include "services/network/public/cpp/cross_origin_embedder_policy.h"

#include <algorithm>

#include "net/http/structured_headers.h"

namespace network {

namespace {
constexpr char kRequireCorp[] = "require-corp";
} // namespace

const char CrossOriginEmbedderPolicy::kHeaderName[] =
"cross-origin-embedder-policy";
const char CrossOriginEmbedderPolicy::kReportOnlyHeaderName[] =
"cross-origin-embedder-policy-report-only";

CrossOriginEmbedderPolicy::CrossOriginEmbedderPolicy() = default;
CrossOriginEmbedderPolicy::CrossOriginEmbedderPolicy(
const CrossOriginEmbedderPolicy& src) = default;
Expand All @@ -25,4 +38,25 @@ bool CrossOriginEmbedderPolicy::operator==(
report_only_reporting_endpoint == other.report_only_reporting_endpoint;
}

std::pair<mojom::CrossOriginEmbedderPolicyValue, base::Optional<std::string>>
CrossOriginEmbedderPolicy::Parse(base::StringPiece header_value) {
constexpr auto kNone = mojom::CrossOriginEmbedderPolicyValue::kNone;
using Item = net::structured_headers::Item;
const auto item = net::structured_headers::ParseItem(header_value);
if (!item || item->item.Type() != Item::kTokenType ||
item->item.GetString() != kRequireCorp) {
return std::make_pair(kNone, base::nullopt);
}
base::Optional<std::string> endpoint;
auto it = std::find_if(item->params.cbegin(), item->params.cend(),
[](const std::pair<std::string, Item>& param) {
return param.first == "report-to";
});
if (it != item->params.end() && it->second.Type() == Item::kStringType) {
endpoint = it->second.GetString();
}
return std::make_pair(mojom::CrossOriginEmbedderPolicyValue::kRequireCorp,
std::move(endpoint));
}

} // namespace network
17 changes: 17 additions & 0 deletions services/network/public/cpp/cross_origin_embedder_policy.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
#define SERVICES_NETWORK_PUBLIC_CPP_CROSS_ORIGIN_EMBEDDER_POLICY_H_

#include <string>
#include <utility>

#include "base/component_export.h"
#include "base/optional.h"
#include "base/strings/string_piece.h"
#include "services/network/public/mojom/cross_origin_embedder_policy.mojom-shared.h"

namespace network {
Expand All @@ -30,6 +32,21 @@ struct COMPONENT_EXPORT(NETWORK_CPP_BASE) CrossOriginEmbedderPolicy final {
mojom::CrossOriginEmbedderPolicyValue report_only_value =
mojom::CrossOriginEmbedderPolicyValue::kNone;
base::Optional<std::string> report_only_reporting_endpoint;

// Parses |header_value| and returns a pair of a COEP value and an optional
// reporting endpoint. This is usually used for two headers.
//
// CrossOriginEmbedderPolicyValue coep;
// std::tie(coep.value, coep.reporting_endpoint) =
// CrossOriginEmbedderPolicyValue::Parse(header_value);
// std::tie(coep.report_only_value, coep.report_only_reporting_endpoint) =
// CrossOriginEmbedderPolicyValue::Parse(report_only_header_value);
static std::pair<mojom::CrossOriginEmbedderPolicyValue,
base::Optional<std::string>>
Parse(base::StringPiece header_value);

static const char kHeaderName[];
static const char kReportOnlyHeaderName[];
};

} // namespace network
Expand Down
30 changes: 5 additions & 25 deletions services/network/url_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,7 @@ using ConcerningHeaderId = URLLoader::ConcerningHeaderId;
// mojo::core::Core::CreateDataPipe
constexpr size_t kBlockedBodyAllocationSize = 1;

constexpr char kCrossOriginEmbedderPolicyValueHeader[] =
"Cross-Origin-Embedder-Policy";
constexpr char kCrossOriginEmbedderPolicyValueReportOnlyHeader[] =
"Cross-Origin-Embedder-Policy-Report-Only";
constexpr char kCrossOriginOpenerPolicyHeader[] = "Cross-Origin-Opener-Policy";
constexpr char kRequireCorp[] = "require-corp";

// TODO: this duplicates some of PopulateResourceResponse in
// content/browser/loader/resource_loader.cc
Expand Down Expand Up @@ -446,28 +441,13 @@ std::pair<network::mojom::CrossOriginEmbedderPolicyValue,
ParseCrossOriginEmbedderPolicyValueInternal(
const net::HttpResponseHeaders* headers,
base::StringPiece header_name) {
constexpr auto kNone = mojom::CrossOriginEmbedderPolicyValue::kNone;
using Item = net::structured_headers::Item;
std::string header_value;
if (!headers ||
!headers->GetNormalizedHeader(header_name.as_string(), &header_value)) {
return std::make_pair(kNone, base::nullopt);
return std::make_pair(mojom::CrossOriginEmbedderPolicyValue::kNone,
base::nullopt);
}
const auto item = net::structured_headers::ParseItem(header_value);
if (!item || item->item.Type() != Item::kTokenType ||
item->item.GetString() != kRequireCorp) {
return std::make_pair(kNone, base::nullopt);
}
base::Optional<std::string> endpoint;
auto it = std::find_if(item->params.cbegin(), item->params.cend(),
[](const std::pair<std::string, Item>& param) {
return param.first == "report-to";
});
if (it != item->params.end() && it->second.Type() == Item::kStringType) {
endpoint = it->second.GetString();
}
return std::make_pair(mojom::CrossOriginEmbedderPolicyValue::kRequireCorp,
std::move(endpoint));
return CrossOriginEmbedderPolicy::Parse(header_value);
}

} // namespace
Expand Down Expand Up @@ -1547,10 +1527,10 @@ CrossOriginEmbedderPolicy URLLoader::ParseCrossOriginEmbedderPolicyValue(
CrossOriginEmbedderPolicy coep;
std::tie(coep.value, coep.reporting_endpoint) =
ParseCrossOriginEmbedderPolicyValueInternal(
headers, kCrossOriginEmbedderPolicyValueHeader);
headers, CrossOriginEmbedderPolicy::kHeaderName);
std::tie(coep.report_only_value, coep.report_only_reporting_endpoint) =
ParseCrossOriginEmbedderPolicyValueInternal(
headers, kCrossOriginEmbedderPolicyValueReportOnlyHeader);
headers, CrossOriginEmbedderPolicy::kReportOnlyHeaderName);
return coep;
}

Expand Down
5 changes: 5 additions & 0 deletions third_party/blink/public/mojom/fetch/fetch_api_response.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module blink.mojom;
import "mojo/public/mojom/base/time.mojom";
import "services/network/public/mojom/fetch_api.mojom";
import "services/network/public/mojom/content_security_policy.mojom";
import "services/network/public/mojom/cross_origin_embedder_policy.mojom";
import "third_party/blink/public/mojom/blob/serialized_blob.mojom";
import "third_party/blink/public/mojom/service_worker/service_worker_error_type.mojom";
import "url/mojom/url.mojom";
Expand Down Expand Up @@ -73,6 +74,10 @@ struct FetchAPIResponse {
// parsed CSP.
array<network.mojom.ContentSecurityPolicy> content_security_policy;

// The parsed representation of Cross-Origin-Embedder-Policy and
// Cross-Origin-Embedder-Policy-Report-Only headers.
network.mojom.CrossOriginEmbedderPolicy cross_origin_embedder_policy;

// True if the response was loaded with a Request where the credentials mode
// would potentially send cookies to the server:
//
Expand Down
3 changes: 2 additions & 1 deletion third_party/blink/renderer/core/fetch/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ include_rules = [
"+mojo/public/cpp/system/data_pipe_utils.h",
"+mojo/public/cpp/system/simple_watcher.h",
"+net/base/request_priority.h",
"+services/network/public/cpp/content_security_policy/content_security_policy.h",
"+services/network/public/cpp",
"+services/network/public/mojom",
"+url/gurl.h",
]
21 changes: 21 additions & 0 deletions third_party/blink/renderer/core/fetch/fetch_response_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "third_party/blink/renderer/core/fetch/fetch_response_data.h"

#include "services/network/public/cpp/cross_origin_embedder_policy.h"
#include "services/network/public/cpp/features.h"
#include "third_party/blink/public/mojom/fetch/fetch_api_response.mojom-blink.h"
#include "third_party/blink/renderer/core/fetch/fetch_header_list.h"
#include "third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h"
Expand Down Expand Up @@ -267,6 +269,25 @@ mojom::blink::FetchAPIResponsePtr FetchResponseData::PopulateFetchAPIResponse(
response->headers.insert(header.first, header.second);
response->content_security_policy = ParseContentSecurityPolicy(
HeaderList()->GetAsRawString(status_, status_message_), request_url);

if (base::FeatureList::IsEnabled(
network::features::kCrossOriginEmbedderPolicy)) {
network::CrossOriginEmbedderPolicy coep;
auto it =
response->headers.find(network::CrossOriginEmbedderPolicy::kHeaderName);
if (it != response->headers.end()) {
std::tie(coep.value, coep.reporting_endpoint) =
network::CrossOriginEmbedderPolicy::Parse(it->value.Utf8());
}
it = response->headers.find(
network::CrossOriginEmbedderPolicy::kReportOnlyHeaderName);
if (it != response->headers.end()) {
std::tie(coep.report_only_value, coep.report_only_reporting_endpoint) =
network::CrossOriginEmbedderPolicy::Parse(it->value.Utf8());
}
response->cross_origin_embedder_policy = std::move(coep);
}

return response;
}

Expand Down

0 comments on commit 57010c9

Please sign in to comment.