Adding UMA Histogram reporting when nosniff header is present.
This UMA statistic should help us understand why we are seeing so many
responses blocked without being sniffed in the CacheHeuristic +
ProtectedMimeType category. For completeness, we also report
which responses have a nosniff header in the CORSHeuristic +
ProtectedMimeType category.
Bug: 815290
Change-Id: Idf5c9f3a65b19e681c8eab51f376e02ec2eb10a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1709669
Commit-Queue: Kristina Nelson <krstnmnlsn@google.com>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680686}
diff --git a/content/browser/loader/cross_site_document_resource_handler_unittest.cc b/content/browser/loader/cross_site_document_resource_handler_unittest.cc
index c312f83..b82ca19 100644
--- a/content/browser/loader/cross_site_document_resource_handler_unittest.cc
+++ b/content/browser/loader/cross_site_document_resource_handler_unittest.cc
@@ -2568,6 +2568,9 @@
const bool seems_sensitive_from_cache_heuristic =
network::CrossOriginReadBlocking::ResponseAnalyzer::
SeemsSensitiveFromCacheHeuristic(response->head);
+ const bool expect_nosniff =
+ network::CrossOriginReadBlocking::ResponseAnalyzer::HasNoSniff(
+ response->head);
// Call OnResponseStarted.
ASSERT_EQ(MockResourceLoader::Status::IDLE,
@@ -2675,6 +2678,15 @@
testing::ElementsAre(base::Bucket(supports_range_requests, 1)));
expected_counts[cors_protected + blocked_with_range_support] = 1;
}
+ if (scenario.mime_type_bucket == MimeTypeBucket::kProtected &&
+ scenario.protection_decision ==
+ CrossOriginProtectionDecision::kBlock) {
+ EXPECT_THAT(histograms.GetAllSamples(
+ cors_protected + ".BlockedWithoutSniffing.HasNoSniff"),
+ testing::ElementsAre(base::Bucket(expect_nosniff, 1)));
+ expected_counts[cors_protected + ".BlockedWithoutSniffing.HasNoSniff"] =
+ 1;
+ }
}
if (seems_sensitive_from_cache_heuristic) {
expected_counts[cache_base + mime_type_bucket] = 1;
@@ -2689,6 +2701,15 @@
testing::ElementsAre(base::Bucket(supports_range_requests, 1)));
expected_counts[cache_protected + blocked_with_range_support] = 1;
}
+ if (scenario.mime_type_bucket == MimeTypeBucket::kProtected &&
+ scenario.protection_decision ==
+ CrossOriginProtectionDecision::kBlock) {
+ EXPECT_THAT(histograms.GetAllSamples(
+ cache_protected + ".BlockedWithoutSniffing.HasNoSniff"),
+ testing::ElementsAre(base::Bucket(expect_nosniff, 1)));
+ expected_counts[cache_protected +
+ ".BlockedWithoutSniffing.HasNoSniff"] = 1;
+ }
}
EXPECT_THAT(
diff --git a/services/network/cross_origin_read_blocking.cc b/services/network/cross_origin_read_blocking.cc
index 4c244aa4..18667282 100644
--- a/services/network/cross_origin_read_blocking.cc
+++ b/services/network/cross_origin_read_blocking.cc
@@ -601,6 +601,7 @@
seems_sensitive_from_cache_heuristic_(
SeemsSensitiveFromCacheHeuristic(response)),
supports_range_requests_(SupportsRangeRequests(response)),
+ has_nosniff_header_(HasNoSniff(response)),
content_length_(response.content_length),
http_response_code_(response.headers ? response.headers->response_code()
: 0) {
@@ -747,18 +748,6 @@
}
}
- // We intend to block the response at this point. However, we will usually
- // sniff the contents to confirm the MIME type, to avoid blocking incorrectly
- // labeled JavaScript, JSONP, etc files.
- //
- // Note: if there is a nosniff header, it means we should honor the response
- // mime type without trying to confirm it.
- std::string nosniff_header;
- response.headers->GetNormalizedHeader("x-content-type-options",
- &nosniff_header);
- bool has_nosniff_header =
- base::LowerCaseEqualsASCII(nosniff_header, "nosniff");
-
// Some types (e.g. ZIP) are protected without any confirmation sniffing.
if (canonical_mime_type == MimeType::kNeverSniffed)
return kBlock;
@@ -820,13 +809,20 @@
}
}
+ // We intend to block the response at this point. However, we will usually
+ // sniff the contents to confirm the MIME type, to avoid blocking incorrectly
+ // labeled JavaScript, JSONP, etc files.
+ //
+ // Note: if there is a nosniff header, it means we should honor the response
+ // mime type without trying to confirm it.
+ //
// Decide whether to block based on the MIME type.
switch (canonical_mime_type) {
case MimeType::kHtml:
case MimeType::kXml:
case MimeType::kJson:
case MimeType::kPlain:
- if (has_nosniff_header)
+ if (HasNoSniff(response))
return kBlock;
else
return kNeedToSniffMore;
@@ -851,6 +847,17 @@
}
// static
+bool CrossOriginReadBlocking::ResponseAnalyzer::HasNoSniff(
+ const ResourceResponseInfo& response) {
+ if (!response.headers)
+ return false;
+ std::string nosniff_header;
+ response.headers->GetNormalizedHeader("x-content-type-options",
+ &nosniff_header);
+ return base::LowerCaseEqualsASCII(nosniff_header, "nosniff");
+}
+
+// static
bool CrossOriginReadBlocking::ResponseAnalyzer::SeemsSensitiveFromCORSHeuristic(
const ResourceResponseInfo& response) {
// Check if the response has an Access-Control-Allow-Origin with a value other
@@ -1165,6 +1172,10 @@
"SiteIsolation.CORBProtection.CORSHeuristic.ProtectedMimeType."
"BlockedWithRangeSupport",
supports_range_requests_);
+ UMA_HISTOGRAM_BOOLEAN(
+ "SiteIsolation.CORBProtection.CORSHeuristic.ProtectedMimeType."
+ "BlockedWithoutSniffing.HasNoSniff",
+ has_nosniff_header_);
} else if (protection_decision ==
CrossOriginProtectionDecision::kBlockedAfterSniffing) {
UMA_HISTOGRAM_BOOLEAN(
@@ -1195,6 +1206,10 @@
"SiteIsolation.CORBProtection.CacheHeuristic.ProtectedMimeType."
"BlockedWithRangeSupport",
supports_range_requests_);
+ UMA_HISTOGRAM_BOOLEAN(
+ "SiteIsolation.CORBProtection.CacheHeuristic.ProtectedMimeType."
+ "BlockedWithoutSniffing.HasNoSniff",
+ has_nosniff_header_);
} else if (protection_decision ==
CrossOriginProtectionDecision::kBlockedAfterSniffing) {
UMA_HISTOGRAM_BOOLEAN(
diff --git a/services/network/cross_origin_read_blocking.h b/services/network/cross_origin_read_blocking.h
index f4b499e..e7a8aaa 100644
--- a/services/network/cross_origin_read_blocking.h
+++ b/services/network/cross_origin_read_blocking.h
@@ -185,6 +185,9 @@
const base::Optional<url::Origin>& request_initiator_site_lock,
MimeType canonical_mime_type);
+ // Returns true if the response has a nosniff header.
+ static bool HasNoSniff(const ResourceResponseInfo& response);
+
// Checks if the response seems sensitive for CORB protection logging.
// Returns true if the Access-Control-Allow-Origin header has a value other
// than *.
@@ -239,6 +242,7 @@
const bool seems_sensitive_from_cors_heuristic_;
const bool seems_sensitive_from_cache_heuristic_;
const bool supports_range_requests_;
+ const bool has_nosniff_header_;
// |hypothetical_sniffing_mode_| is true if we need to sniff only because of
// the CORB protection logging (and otherwise, CORB would not sniff).
bool hypothetical_sniffing_mode_ = false;
diff --git a/services/network/cross_origin_read_blocking_unittest.cc b/services/network/cross_origin_read_blocking_unittest.cc
index 658d687..e1c78e7 100644
--- a/services/network/cross_origin_read_blocking_unittest.cc
+++ b/services/network/cross_origin_read_blocking_unittest.cc
@@ -1862,6 +1862,113 @@
Verdict::kAllowBecauseOutOfData, // verdict
0, // verdict_packet
},
+
+ // These responses confirm we are correctly reporting when a nosniff header
+ // is present. This should *only* be reported when a blocked, sensitive,
+ // protected mime-type response has a nosniff header.
+ //
+ // This first response satisfies all criteria except it does not have a
+ // nosniff header.
+ {
+ "Cache heuristic with no nosniff header",
+ __LINE__,
+ "http://a.com/resource.html", // target_url
+ "http://a.com/", // initiator_origin
+ "http://a.com/", // initiator_site_lock
+ "HTTP/1.1 200 OK\n"
+ "Cache-Control: private\n"
+ "Vary: origin", // response_headers
+ "text/html", // response_content_type
+ MimeType::kHtml, // canonical_mime_type
+ MimeTypeBucket::kProtected, // mime_type_bucket
+ {"<html><head>this should sniff as HTML"}, // packets
+ true, // resource_is_sensitive
+ CrossOriginProtectionDecision::
+ kBlockedAfterSniffing, // protection_decision
+ Verdict::kAllow, // verdict
+ 0, // verdict_packet
+ },
+ // These next responses have nosniff headers but are missing one of the
+ // other criteria.
+ {
+ "Cache heuristic with nosniff header but not a protected type",
+ __LINE__,
+ "http://a.com/resource.js", // target_url
+ "http://a.com/", // initiator_origin
+ "http://a.com/", // initiator_site_lock
+ "HTTP/1.1 200 OK\n"
+ "X-Content-Type-Options: nosniff\n"
+ "Cache-Control: private\n"
+ "Vary: origin", // response_headers
+ "application/javascript", // response_content_type
+ MimeType::kOthers, // canonical_mime_type
+ MimeTypeBucket::kPublic, // mime_type_bucket
+ {"var x=3;"}, // packets
+ true, // resource_is_sensitive
+ CrossOriginProtectionDecision::
+ kAllowedAfterSniffing, // protection_decision
+ Verdict::kAllow, // verdict
+ 0, // verdict_packet
+ },
+ {
+ "Cache heuristic with nosniff header but protection decision != kBlock",
+ __LINE__,
+ "http://a.com/resource.html", // target_url
+ "http://a.com/", // initiator_origin
+ "http://a.com/", // initiator_site_lock
+ "HTTP/1.1 200 OK\n"
+ "X-Content-Type-Options: nosniff\n"
+ "Access-Control-Allow-Origin: *\n"
+ "Cache-Control: private\n"
+ "Vary: origin", // response_headers
+ "text/html", // response_content_type
+ MimeType::kHtml, // canonical_mime_type
+ MimeTypeBucket::kProtected, // mime_type_bucket
+ {"<html><head>this should sniff as HTML"}, // packets
+ true, // resource_is_sensitive
+ CrossOriginProtectionDecision::kAllow, // protection_decision
+ Verdict::kAllow, // verdict
+ kVerdictPacketForHeadersBasedVerdict, // verdict_packet
+ },
+ // These responses satisfies all criteria and should report its nosniff
+ // header.
+ {
+ "Nosniff header and satisfies the CORS heuristic",
+ __LINE__,
+ "http://a.com/resource.html", // target_url
+ "http://a.com/", // initiator_origin
+ "http://a.com/", // initiator_site_lock
+ "HTTP/1.1 200 OK\n"
+ "X-Content-Type-Options: nosniff\n"
+ "Access-Control-Allow-Origin: http://www.a.com/", // response_headers
+ "text/html", // response_content_type
+ MimeType::kHtml, // canonical_mime_type
+ MimeTypeBucket::kProtected, // mime_type_bucket
+ {"<html><head>this should sniff as HTML"}, // packets
+ true, // resource_is_sensitive
+ CrossOriginProtectionDecision::kBlock, // protection_decision
+ Verdict::kAllow, // verdict
+ kVerdictPacketForHeadersBasedVerdict, // verdict_packet
+ },
+ {
+ "Nosniff header and satisfies the Cache heuristic",
+ __LINE__,
+ "http://a.com/resource.html", // target_url
+ "http://a.com/", // initiator_origin
+ "http://a.com/", // initiator_site_lock
+ "HTTP/1.1 200 OK\n"
+ "X-Content-Type-Options: nosniff\n"
+ "Cache-Control: private\n"
+ "Vary: origin", // response_headers
+ "text/html", // response_content_type
+ MimeType::kHtml, // canonical_mime_type
+ MimeTypeBucket::kProtected, // mime_type_bucket
+ {/* empty body doesn't sniff as html */}, // packets
+ true, // resource_is_sensitive
+ CrossOriginProtectionDecision::kBlock, // protection_decision
+ Verdict::kAllow, // verdict
+ kVerdictPacketForHeadersBasedVerdict, // verdict_packet
+ },
};
} // namespace
@@ -2142,6 +2249,8 @@
const bool supports_range_requests =
network::CrossOriginReadBlocking::ResponseAnalyzer::SupportsRangeRequests(
response);
+ const bool expect_nosniff =
+ network::CrossOriginReadBlocking::ResponseAnalyzer::HasNoSniff(response);
// Run the analyzer and confirm it allows/blocks correctly.
RunAnalyzerOnScenario(response);
@@ -2193,6 +2302,7 @@
std::string cache_base = "SiteIsolation.CORBProtection.CacheHeuristic";
std::string cors_protected = cors_base + ".ProtectedMimeType";
std::string cache_protected = cache_base + ".ProtectedMimeType";
+ std::string blocked_nosniff = ".BlockedWithoutSniffing.HasNoSniff";
if (seems_sensitive_from_cors_heuristic) {
expected_counts[cors_base + mime_type_bucket] = 1;
EXPECT_THAT(histograms.GetAllSamples(cors_base + mime_type_bucket),
@@ -2206,6 +2316,13 @@
testing::ElementsAre(base::Bucket(supports_range_requests, 1)));
expected_counts[cors_protected + blocked_with_range_support] = 1;
}
+ if (scenario.mime_type_bucket == MimeTypeBucket::kProtected &&
+ scenario.protection_decision ==
+ CrossOriginProtectionDecision::kBlock) {
+ EXPECT_THAT(histograms.GetAllSamples(cors_protected + blocked_nosniff),
+ testing::ElementsAre(base::Bucket(expect_nosniff, 1)));
+ expected_counts[cors_protected + blocked_nosniff] = 1;
+ }
}
if (seems_sensitive_from_cache_heuristic) {
expected_counts[cache_base + mime_type_bucket] = 1;
@@ -2220,6 +2337,13 @@
testing::ElementsAre(base::Bucket(supports_range_requests, 1)));
expected_counts[cache_protected + blocked_with_range_support] = 1;
}
+ if (scenario.mime_type_bucket == MimeTypeBucket::kProtected &&
+ scenario.protection_decision ==
+ CrossOriginProtectionDecision::kBlock) {
+ EXPECT_THAT(histograms.GetAllSamples(cache_protected + blocked_nosniff),
+ testing::ElementsAre(base::Bucket(expect_nosniff, 1)));
+ expected_counts[cache_protected + blocked_nosniff] = 1;
+ }
}
EXPECT_THAT(
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index 25482361..6786f87 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -5990,6 +5990,11 @@
<int value="1" label="Has distilled data"/>
</enum>
+<enum name="BooleanHasNoSniff">
+ <int value="0" label="Does not have nosniff"/>
+ <int value="1" label="Has nosniff"/>
+</enum>
+
<enum name="BooleanHasPath">
<int value="0" label="No path"/>
<int value="1" label="Has path"/>
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index 1df6349..605f782 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -127750,6 +127750,20 @@
</summary>
</histogram>
+<histogram base="true"
+ name="SiteIsolation.CORBProtection.ProtectedMimeType.BlockedWithoutSniffing.HasNoSniff"
+ enum="BooleanHasNoSniff" expires_after="M79">
+ <owner>krstnmnlsn@chromium.org</owner>
+ <owner>creis@chromium.org</owner>
+ <summary>
+ True if the response has a nosniff header. If the nosniff header is not
+ present, then CORB must have decided to block without sniffing due to a
+ partial response, or because the MIME type was a never sniff type. Only
+ reported on resources CORB would have blocked/protected without sniffing
+ that have a protected MIME type.
+ </summary>
+</histogram>
+
<histogram name="SiteIsolation.CORBProtection.SensitiveResource"
enum="BooleanSensitive" expires_after="M79">
<owner>krstnmnlsn@chromium.org</owner>
@@ -159158,6 +159172,15 @@
name="Net.QuicSession.ConnectionCloseErrorCodeServerGoogle"/>
</histogram_suffixes>
+<histogram_suffixes name="HeuristicType" separator="." ordering="prefix,2">
+ <suffix name="CacheHeuristic"
+ label="In this case the response satisfies the Cache heuristic."/>
+ <suffix name="CORSHeuristic"
+ label="In this case the response satisfies the CORS heuristic."/>
+ <affected-histogram
+ name="SiteIsolation.CORBProtection.ProtectedMimeType.BlockedWithoutSniffing.HasNoSniff"/>
+</histogram_suffixes>
+
<histogram_suffixes name="HighDownloadBandwidth" separator=".">
<suffix name="HighDownloadBandwidth" label="download with high bandwidth."/>
<affected-histogram name="Download.Parallelizable.DownloadTime"/>
