Skip to content

Commit

Permalink
Add Cross Origin Read Blocking check in network service memory cache
Browse files Browse the repository at this point in the history
Don't use a response from the network service in-memory cache when
CORB checks don't pass. If the checks don't pass CorsURLLoader falls
back to a network::URLLoader. network::URLLoader will protect a
response accordingly.

Design doc:
https://docs.google.com/document/d/15e1mzyCiBm8ERTO-rM2iXoSAXL98J9fcpejwphlYIEI/edit?usp=sharing

Bug: 1339708
Change-Id: I4f78e0d6f125f73a28743198a455c3b8de4632a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3750647
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1023990}
  • Loading branch information
bashi authored and Chromium LUCI CQ committed Jul 13, 2022
1 parent 73a44d4 commit 6f2c2b8
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 0 deletions.
41 changes: 41 additions & 0 deletions services/network/network_service_memory_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,18 @@

#include "services/network/network_service_memory_cache.h"

#include <algorithm>

#include "base/bit_cast.h"
#include "base/memory/ref_counted_memory.h"
#include "base/memory/scoped_refptr.h"
#include "base/metrics/field_trial_params.h"
#include "base/metrics/histogram_functions.h"
#include "base/numerics/safe_conversions.h"
#include "base/strings/strcat.h"
#include "base/strings/string_piece.h"
#include "net/base/load_flags.h"
#include "net/base/mime_sniffer.h"
#include "net/base/network_isolation_key.h"
#include "net/http/http_cache.h"
#include "net/http/http_request_info.h"
Expand All @@ -23,6 +28,7 @@
#include "services/network/network_context.h"
#include "services/network/network_service_memory_cache_url_loader.h"
#include "services/network/network_service_memory_cache_writer.h"
#include "services/network/public/cpp/corb/corb_api.h"
#include "services/network/public/cpp/cross_origin_resource_policy.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/request_destination.h"
Expand Down Expand Up @@ -68,6 +74,36 @@ std::string GenerateCacheKeyForResourceRequest(
/*use_single_keyed_cache=*/false, /*single_key_checksum=*/"");
}

bool CheckCrossOriginReadBlocking(const ResourceRequest& resource_request,
const mojom::URLResponseHead& response,
const base::RefCountedBytes& content) {
// Using an empty per-URLLoaderFactory state may result in blocking the stored
// response unnecessarily. Such a false-positive result is fine because when
// the stored response is blocked, CorsURLLoader falls back to a URLLoader and
// the URLLoader performs appropriate Opaque Resource Blocking checks.
//
// TODO(https://crbug.com/1339708): Consider moving CORB/ORB handling from
// URLLoader to CorsURLLoader. It will eliminate the need for CORB/ORB checks
// here.
corb::PerFactoryState state;
auto analyzer = corb::ResponseAnalyzer::Create(state);
corb::ResponseAnalyzer::Decision decision =
analyzer->Init(resource_request.url, resource_request.request_initiator,
resource_request.mode, response);

if (decision == corb::ResponseAnalyzer::Decision::kSniffMore) {
const size_t size =
std::min(static_cast<size_t>(net::kMaxBytesToSniff), content.size());
decision = analyzer->Sniff(
base::StringPiece(base::bit_cast<const char*>(content.front()), size));
if (decision == corb::ResponseAnalyzer::Decision::kSniffMore)
decision = analyzer->HandleEndOfSniffableResponseBody();
DCHECK_NE(decision, corb::ResponseAnalyzer::Decision::kSniffMore);
}

return decision == corb::ResponseAnalyzer::Decision::kAllow;
}

bool MatchVaryHeader(const ResourceRequest& resource_request,
const net::HttpVaryData& vary_data,
const net::HttpResponseHeaders& cached_response_headers,
Expand Down Expand Up @@ -289,6 +325,11 @@ absl::optional<std::string> NetworkServiceMemoryCache::CanServe(
if (blocked_reason.has_value())
return absl::nullopt;

if (!CheckCrossOriginReadBlocking(resource_request, *response,
*it->second->content)) {
return absl::nullopt;
}

if (!MatchVaryHeader(
resource_request, it->second->vary_data, *response->headers,
network_context_->url_request_context()->enable_brotli())) {
Expand Down
60 changes: 60 additions & 0 deletions services/network/network_service_memory_cache_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "mojo/public/cpp/bindings/remote.h"
#include "mojo/public/cpp/system/data_pipe_utils.h"
#include "net/base/features.h"
#include "net/base/mime_sniffer.h"
#include "net/base/schemeful_site.h"
#include "net/base/url_util.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
Expand Down Expand Up @@ -96,6 +97,32 @@ std::unique_ptr<net::test_server::HttpResponse> CacheableResponseHandler(
return response;
}

// Used for cross origin read blocking check.
std::unique_ptr<net::test_server::HttpResponse> CorbCheckHandler(
const net::test_server::HttpRequest& request) {
if (request.GetURL().path_piece() == "/corb_nosniff") {
auto response = std::make_unique<net::test_server::BasicHttpResponse>();
response->AddCustomHeader("cache-control", "max-age=60");
response->AddCustomHeader("x-content-type-options", "nosniff");
response->AddCustomHeader("content-type", "text/javascript");
response->set_content("{\"key\": true}");
return response;
}

if (request.GetURL().path_piece() == "/corb_sniff") {
auto response = std::make_unique<net::test_server::BasicHttpResponse>();
response->AddCustomHeader("cache-control", "max-age=60");
response->AddCustomHeader("content-type", "text/html");
// Set a html content of which size is larger than net::kMaxBytestosniff.
std::string content("<html>sniffed content");
content.append(std::string(net::kMaxBytesToSniff, ' '));
response->set_content(content);
return response;
}

return nullptr;
}

} // namespace

class NetworkServiceMemoryCacheTest : public testing::Test {
Expand All @@ -116,6 +143,7 @@ class NetworkServiceMemoryCacheTest : public testing::Test {
test_server_.AddDefaultHandlers();
test_server_.RegisterRequestHandler(
base::BindRepeating(&CacheableResponseHandler));
test_server_.RegisterRequestHandler(base::BindRepeating(&CorbCheckHandler));
ASSERT_TRUE(test_server_.Start());

network_service_ = NetworkService::CreateForTesting();
Expand Down Expand Up @@ -445,6 +473,38 @@ TEST_F(NetworkServiceMemoryCacheTest, CanServe_CorpBlocked) {
cross_origin_embedder_policy));
}

TEST_F(NetworkServiceMemoryCacheTest, CanServe_CorbBlockedNoSniff) {
ResourceRequest request = CreateRequest("/corb_nosniff");
StoreResponseToMemoryCache(request);
ASSERT_TRUE(CanServeFromMemoryCache(request));

const auto other_origin =
url::Origin::Create(GURL("https://other-origin.test"));
net::SchemefulSite other_site(other_origin);
net::NetworkIsolationKey network_isolation_key(
/*top_frame_site=*/other_site, /*frame_site=*/other_site);

request.request_initiator = other_origin;

ASSERT_FALSE(CanServeFromMemoryCache(request, network_isolation_key));
}

TEST_F(NetworkServiceMemoryCacheTest, CanServe_CorbBlockedSniff) {
ResourceRequest request = CreateRequest("/corb_sniff");
StoreResponseToMemoryCache(request);
ASSERT_TRUE(CanServeFromMemoryCache(request));

const auto other_origin =
url::Origin::Create(GURL("https://other-origin.test"));
net::SchemefulSite other_site(other_origin);
net::NetworkIsolationKey network_isolation_key(
/*top_frame_site=*/other_site, /*frame_site=*/other_site);

request.request_initiator = other_origin;

ASSERT_FALSE(CanServeFromMemoryCache(request, network_isolation_key));
}

TEST_F(NetworkServiceMemoryCacheTest, CanServe_VaryHeaderAcceptEncoding) {
// The `/echoheadercache` handler sends `Vary: foo` header.
ResourceRequest request = CreateRequest("/echoheadercache?Accept-Encoding");
Expand Down

0 comments on commit 6f2c2b8

Please sign in to comment.