Skip to content

Commit

Permalink
[Credentialless] Block credentialled opaque response (CacheStorage)
Browse files Browse the repository at this point in the history
Discussion here:
w3c/ServiceWorker#1592

+ Plumb response's |request_include_credentials|
+ Modify CORP check to prevent vulnerable credentialled opaque responses
  from entering COEP:credentialless document via CacheStorage.

Change-Id: Ieaffd379da43904ba9b5dfe364489ef02ec20b2e
Bug: 1175099
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2886899
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#889719}
  • Loading branch information
ArthurSonzogni authored and Chromium LUCI CQ committed Jun 7, 2021
1 parent dfaca25 commit 22a2236
Show file tree
Hide file tree
Showing 23 changed files with 115 additions and 63 deletions.
1 change: 1 addition & 0 deletions content/browser/cache_storage/cache_storage.proto
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ message CacheResponse {
optional string request_method = 14;
optional int64 padding = 15;
optional int64 side_data_padding = 16;
optional bool request_include_credentials = 17;
}

message CacheMetadata {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,8 @@ class CacheStorageCacheTest : public testing::Test {
net::HttpResponseInfo::CONNECTION_INFO_UNKNOWN,
/*alpn_negotiated_protocol=*/"unknown",
/*was_fetched_via_spdy=*/false, /*has_range_requested=*/false,
/*auth_challenge_info=*/absl::nullopt);
/*auth_challenge_info=*/absl::nullopt,
/*request_include_credentials=*/true);
}

void CopySideDataToResponse(const std::string& uuid,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ bool ResponseBlockedByCrossOriginResourcePolicy(
response->url_list.back(), response->url_list.front(),
document_origin, corp_header_value, RequestMode::kNoCors,
document_origin, network::mojom::RequestDestination::kEmpty,
document_coep, coep_reporter ? coep_reporter.get() : nullptr)
response->request_include_credentials, document_coep,
coep_reporter ? coep_reporter.get() : nullptr)
.has_value();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,8 @@ class CacheStorageManagerTest : public testing::Test {
net::HttpResponseInfo::CONNECTION_INFO_UNKNOWN,
/*alpn_negotiated_protocol=*/"unknown",
/*was_fetched_via_spdy=*/false, /*has_range_requested=*/false,
/*auth_challenge_info=*/absl::nullopt);
/*auth_challenge_info=*/absl::nullopt,
/*request_include_credentials=*/true);

blink::mojom::BatchOperationPtr operation =
blink::mojom::BatchOperation::New();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,11 @@ blink::mojom::FetchAPIResponsePtr CreateResponse(
padding = storage::ComputeRandomResponsePadding();
}

bool request_include_credentials =
metadata.response().has_request_include_credentials()
? metadata.response().request_include_credentials()
: true;

// Note that |has_range_requested| can be safely set to false since it only
// affects HTTP 206 (Partial) responses, which are blocked from cache storage.
// See https://fetch.spec.whatwg.org/#main-fetch for usage of
Expand All @@ -462,7 +467,8 @@ blink::mojom::FetchAPIResponsePtr CreateResponse(
static_cast<net::HttpResponseInfo::ConnectionInfo>(
metadata.response().connection_info()),
alpn_negotiated_protocol, metadata.response().was_fetched_via_spdy(),
/*has_range_requested=*/false, /*auth_challenge_info=*/absl::nullopt);
/*has_range_requested=*/false, /*auth_challenge_info=*/absl::nullopt,
request_include_credentials);
}

int64_t CalculateSideDataPadding(
Expand Down Expand Up @@ -1929,6 +1935,8 @@ void LegacyCacheStorageCache::PutDidCreateEntry(
storage_key_, response_metadata, put_context->side_data_blob_size);
}
response_metadata->set_side_data_padding(side_data_padding);
response_metadata->set_request_include_credentials(
put_context->response->request_include_credentials);

// Get a temporary copy of the entry pointer before passing it in base::Bind.
disk_cache::Entry* temp_entry_ptr = put_context->cache_entry.get();
Expand Down
3 changes: 2 additions & 1 deletion content/common/background_fetch/background_fetch_types.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ blink::mojom::FetchAPIResponsePtr BackgroundFetchSettledFetch::CloneResponse(
CloneSerializedBlob(response->side_data_blob_for_cache_put),
mojo::Clone(response->parsed_headers), response->connection_info,
response->alpn_negotiated_protocol, response->was_fetched_via_spdy,
response->has_range_requested, response->auth_challenge_info);
response->has_range_requested, response->auth_challenge_info,
response->request_include_credentials);
}

// static
Expand Down
40 changes: 28 additions & 12 deletions services/network/public/cpp/cross_origin_resource_policy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ absl::optional<mojom::BlockedByResponseReason> IsBlockedInternal(
const absl::optional<url::Origin>& request_initiator,
mojom::RequestMode request_mode,
absl::optional<url::Origin> request_initiator_origin_lock,
bool request_include_credentials,
mojom::CrossOriginEmbedderPolicyValue embedder_policy) {
// Browser-initiated requests are not subject to Cross-Origin-Resource-Policy.
if (!request_initiator.has_value()) {
Expand All @@ -153,11 +154,21 @@ absl::optional<mojom::BlockedByResponseReason> IsBlockedInternal(
return absl::nullopt;
}

bool require_corp =
embedder_policy == mojom::CrossOriginEmbedderPolicyValue::kRequireCorp ||
(embedder_policy ==
mojom::CrossOriginEmbedderPolicyValue::kCredentialless &&
request_mode == mojom::RequestMode::kNavigate);
bool require_corp;
switch (embedder_policy) {
case mojom::CrossOriginEmbedderPolicyValue::kNone:
require_corp = false;
break;

case mojom::CrossOriginEmbedderPolicyValue::kCredentialless:
require_corp = request_mode == mojom::RequestMode::kNavigate ||
request_include_credentials;
break;

case mojom::CrossOriginEmbedderPolicyValue::kRequireCorp:
require_corp = true;
break;
}

// COEP https://mikewest.github.io/corpp/#corp-check
bool upgrade_to_same_origin = false;
Expand Down Expand Up @@ -222,6 +233,7 @@ absl::optional<mojom::BlockedByResponseReason> IsBlockedInternalWithReporting(
mojom::RequestMode request_mode,
absl::optional<url::Origin> request_initiator_origin_lock,
mojom::RequestDestination request_destination,
bool request_include_credentials,
const CrossOriginEmbedderPolicy& embedder_policy,
mojom::CrossOriginEmbedderPolicyReporter* reporter) {
constexpr auto kBlockedDueToCoep = mojom::BlockedByResponseReason::
Expand All @@ -231,7 +243,8 @@ absl::optional<mojom::BlockedByResponseReason> IsBlockedInternalWithReporting(
reporter) {
const auto result = IsBlockedInternal(
policy, request_url, request_initiator, request_mode,
request_initiator_origin_lock, embedder_policy.report_only_value);
request_initiator_origin_lock, request_include_credentials,
embedder_policy.report_only_value);
UMA_HISTOGRAM_ENUMERATION(
"NetworkService.CrossOriginResourcePolicy.ReportOnlyResult",
ToCorpResult(result));
Expand All @@ -249,7 +262,8 @@ absl::optional<mojom::BlockedByResponseReason> IsBlockedInternalWithReporting(

const auto result =
IsBlockedInternal(policy, request_url, request_initiator, request_mode,
request_initiator_origin_lock, embedder_policy.value);
request_initiator_origin_lock,
request_include_credentials, embedder_policy.value);
UMA_HISTOGRAM_ENUMERATION("NetworkService.CrossOriginResourcePolicy.Result",
ToCorpResult(result));
if (reporter &&
Expand Down Expand Up @@ -296,8 +310,8 @@ CrossOriginResourcePolicy::IsBlocked(

return IsBlockedInternalWithReporting(
policy, request_url, original_url, request_initiator, request_mode,
request_initiator_origin_lock, request_destination, embedder_policy,
reporter);
request_initiator_origin_lock, request_destination,
response.request_include_credentials, embedder_policy, reporter);
}

// static
Expand All @@ -310,6 +324,7 @@ CrossOriginResourcePolicy::IsBlockedByHeaderValue(
mojom::RequestMode request_mode,
absl::optional<url::Origin> request_initiator_origin_lock,
mojom::RequestDestination request_destination,
bool request_include_credentials,
const CrossOriginEmbedderPolicy& embedder_policy,
mojom::CrossOriginEmbedderPolicyReporter* reporter) {
// From https://fetch.spec.whatwg.org/#cross-origin-resource-policy-header:
Expand All @@ -321,8 +336,8 @@ CrossOriginResourcePolicy::IsBlockedByHeaderValue(

return IsBlockedInternalWithReporting(
policy, request_url, original_url, request_initiator, request_mode,
request_initiator_origin_lock, request_destination, embedder_policy,
reporter);
request_initiator_origin_lock, request_destination,
request_include_credentials, embedder_policy, reporter);
}

// static
Expand All @@ -342,7 +357,8 @@ CrossOriginResourcePolicy::IsNavigationBlocked(
return IsBlockedInternalWithReporting(
policy, request_url, original_url, request_initiator,
mojom::RequestMode::kNavigate, request_initiator_origin_lock,
request_destination, embedder_policy, reporter);
request_destination, response.request_include_credentials,
embedder_policy, reporter);
}

// static
Expand Down
1 change: 1 addition & 0 deletions services/network/public/cpp/cross_origin_resource_policy.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class COMPONENT_EXPORT(NETWORK_CPP) CrossOriginResourcePolicy {
mojom::RequestMode request_mode,
absl::optional<url::Origin> request_initiator_origin_lock,
mojom::RequestDestination request_destination,
bool request_include_credentials,
const CrossOriginEmbedderPolicy& embedder_policy,
mojom::CrossOriginEmbedderPolicyReporter* reporter) WARN_UNUSED_RESULT;

Expand Down
5 changes: 5 additions & 0 deletions services/network/public/mojom/url_response_head.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -235,4 +235,9 @@ struct URLResponseHead {
// covered by the wildcard in the preflight response.
// TODO(crbug.com/1176753): Remove this once the investigation is done.
bool has_authorization_covered_by_wildcard_on_preflight = false;

// The request's |includeCredentials| value from the "HTTP-network fetch"
// algorithm.
// See: https://fetch.spec.whatwg.org/#concept-http-network-fetch
bool request_include_credentials = true;
};
4 changes: 3 additions & 1 deletion services/network/url_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ void PopulateResourceResponse(net::URLRequest* request,
response->has_range_requested = request->extra_request_headers().HasHeader(
net::HttpRequestHeaders::kRange);
response->dns_aliases = request->response_info().dns_aliases;
response->request_include_credentials = request->allow_credentials();
}

// A subclass of net::UploadBytesElementReader which owns
Expand Down Expand Up @@ -1141,7 +1142,6 @@ void URLLoader::OnReceivedRedirect(net::URLRequest* url_request,

DCHECK(!deferred_redirect_url_);
deferred_redirect_url_ = std::make_unique<GURL>(redirect_info.new_url);
SetRequestCredentials(redirect_info.new_url);

// Send the redirect response to the client, allowing them to inspect it and
// optionally follow the redirect.
Expand Down Expand Up @@ -1191,6 +1191,8 @@ void URLLoader::OnReceivedRedirect(net::URLRequest* url_request,
return;
}

SetRequestCredentials(redirect_info.new_url);

// We may need to clear out old Sec- prefixed request headers. We'll attempt
// to do this before we re-add any.
MaybeRemoveSecHeaders(url_request_.get(), redirect_info.new_url);
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 @@ -104,4 +104,9 @@ struct FetchAPIResponse {
// Information related to an authentication challenge from the response, if
// there was one.
network.mojom.AuthChallengeInfo? auth_challenge_info;

// The request's |includeCredentials| value from the "HTTP-network fetch"
// algorithm.
// See: https://fetch.spec.whatwg.org/#concept-http-network-fetch
bool request_include_credentials = true;
};
6 changes: 6 additions & 0 deletions third_party/blink/public/platform/web_url_response.h
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,12 @@ class WebURLResponse {
BLINK_PLATFORM_EXPORT const absl::optional<net::AuthChallengeInfo>&
AuthChallengeInfo() const;

// The request's |includeCredentials| value from the "HTTP-network fetch"
// algorithm.
// See: https://fetch.spec.whatwg.org/#concept-http-network-fetch
BLINK_PLATFORM_EXPORT void SetRequestIncludeCredentials(bool);
BLINK_PLATFORM_EXPORT bool RequestIncludeCredentials() const;

#if INSIDE_BLINK
protected:
// Permit subclasses to set arbitrary ResourceResponse pointer as
Expand Down
14 changes: 14 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 @@ -174,6 +174,11 @@ String FetchResponseData::InternalMIMEType() const {
return mime_type_;
}

bool FetchResponseData::RequestIncludeCredentials() const {
return internal_response_ ? internal_response_->RequestIncludeCredentials()
: request_include_credentials_;
}

void FetchResponseData::SetURLList(const Vector<KURL>& url_list) {
url_list_ = url_list;
}
Expand Down Expand Up @@ -208,6 +213,7 @@ FetchResponseData* FetchResponseData::Clone(ScriptState* script_state,
new_response->alpn_negotiated_protocol_ = alpn_negotiated_protocol_;
new_response->was_fetched_via_spdy_ = was_fetched_via_spdy_;
new_response->has_range_requested_ = has_range_requested_;
new_response->request_include_credentials_ = request_include_credentials_;
if (auth_challenge_info_) {
new_response->auth_challenge_info_ =
std::make_unique<net::AuthChallengeInfo>(*auth_challenge_info_);
Expand Down Expand Up @@ -286,6 +292,7 @@ mojom::blink::FetchAPIResponsePtr FetchResponseData::PopulateFetchAPIResponse(
response->alpn_negotiated_protocol = alpn_negotiated_protocol_;
response->was_fetched_via_spdy = was_fetched_via_spdy_;
response->has_range_requested = has_range_requested_;
response->request_include_credentials = request_include_credentials_;
for (const auto& header : HeaderList()->List())
response->headers.insert(header.first, header.second);
response->parsed_headers = ParseHeaders(
Expand Down Expand Up @@ -367,6 +374,7 @@ void FetchResponseData::InitFromResourceResponse(
}

SetAuthChallengeInfo(response.AuthChallengeInfo());
SetRequestIncludeCredentials(response.RequestIncludeCredentials());
}

FetchResponseData::FetchResponseData(Type type,
Expand All @@ -393,6 +401,12 @@ void FetchResponseData::SetAuthChallengeInfo(
}
}

void FetchResponseData::SetRequestIncludeCredentials(
bool request_include_credentials) {
DCHECK(!internal_response_);
request_include_credentials_ = request_include_credentials;
}

void FetchResponseData::ReplaceBodyStreamBuffer(BodyStreamBuffer* buffer) {
if (type_ == Type::kBasic || type_ == Type::kCors) {
DCHECK(internal_response_);
Expand Down
7 changes: 7 additions & 0 deletions third_party/blink/renderer/core/fetch/fetch_response_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class CORE_EXPORT FetchResponseData final
return cors_exposed_header_names_;
}
bool HasRangeRequested() const { return has_range_requested_; }
bool RequestIncludeCredentials() const;

int64_t GetPadding() const { return padding_; }
void SetPadding(int64_t padding) { padding_ = padding; }
Expand Down Expand Up @@ -126,6 +127,7 @@ class CORE_EXPORT FetchResponseData final
}
void SetAuthChallengeInfo(
const absl::optional<net::AuthChallengeInfo>& auth_challenge_info);
void SetRequestIncludeCredentials(bool request_include_credentials);

// If the type is Default, replaces |buffer_|.
// If the type is Basic or CORS, replaces |buffer_| and
Expand Down Expand Up @@ -172,6 +174,11 @@ class CORE_EXPORT FetchResponseData final
// |because this member is empty in most cases.
std::unique_ptr<net::AuthChallengeInfo> auth_challenge_info_;

// The request's |includeCredentials| value from the "HTTP-network fetch"
// algorithm.
// See: https://fetch.spec.whatwg.org/#concept-http-network-fetch
bool request_include_credentials_ = true;

DISALLOW_COPY_AND_ASSIGN(FetchResponseData);
};

Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/core/fetch/response.cc
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,8 @@ FetchResponseData* Response::CreateUnfilteredFetchResponseDataWithoutBody(
WTF::AtomicString(fetch_api_response.alpn_negotiated_protocol));
response->SetWasFetchedViaSpdy(fetch_api_response.was_fetched_via_spdy);
response->SetHasRangeRequested(fetch_api_response.has_range_requested);
response->SetRequestIncludeCredentials(
fetch_api_response.request_include_credentials);

for (const auto& header : fetch_api_response.headers)
response->HeaderList()->Append(header.key, header.value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ bool CrossOriginResourcePolicyChecker::IsBlocked(
response.InternalURLList().back(),
response.InternalURLList().front(), initiator_origin,
corp_header_value, request_mode, initiator_origin,
request_destination, policy_,
request_destination,
response.GetResponse()->RequestIncludeCredentials(), policy_,
reporter_ ? reporter_.get() : nullptr)
.has_value();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,15 @@ WebURLResponse::AuthChallengeInfo() const {
return resource_response_->AuthChallengeInfo();
}

void WebURLResponse::SetRequestIncludeCredentials(
bool request_include_credentials) {
resource_response_->SetRequestIncludeCredentials(request_include_credentials);
}

bool WebURLResponse::RequestIncludeCredentials() const {
return resource_response_->RequestIncludeCredentials();
}

WebURLResponse::WebURLResponse(ResourceResponse& r) : resource_response_(&r) {}

} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,13 @@ class PLATFORM_EXPORT ResourceResponse final {
auth_challenge_info_ = value;
}

bool RequestIncludeCredentials() const {
return request_include_credentials_;
}
void SetRequestIncludeCredentials(bool request_include_credentials) {
request_include_credentials_ = request_include_credentials;
}

private:
void UpdateHeaderParsedState(const AtomicString& name);

Expand Down Expand Up @@ -710,6 +717,11 @@ class PLATFORM_EXPORT ResourceResponse final {
KURL web_bundle_url_;

absl::optional<net::AuthChallengeInfo> auth_challenge_info_;

// The request's |includeCredentials| value from the "HTTP-network fetch"
// algorithm.
// See: https://fetch.spec.whatwg.org/#concept-http-network-fetch
bool request_include_credentials_ = true;
};

} // namespace blink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,7 @@ void WebURLLoader::PopulateURLResponse(
}

response->SetAuthChallengeInfo(head.auth_challenge_info);
response->SetRequestIncludeCredentials(head.request_include_credentials);

const net::HttpResponseHeaders* headers = head.headers.get();
if (!headers)
Expand Down
Loading

0 comments on commit 22a2236

Please sign in to comment.