SignedExchange: Prevent content sniffing for inner responses

This patch implements the following restrictions on Signed Exchange
inner responses, added in https://github.com/WICG/webpackage/pull/354:

- Inner response must have a Content-Type header (step 7. of [1]).
- Clients must not sniff a media type from the payload bytes.
  In the loading spec, parser injects "X-Content-Type-Options: nosniff"
  header while parsing response headers (step 8. of [1]) to achieve
  this.

[1] https://wicg.github.io/webpackage/loading.html#parse-b2-cbor-headers

Bug: 1051390
Change-Id: Ia7274f32e43dac28affd718a2965d575b955353c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2059751Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742451}

content/browser/web_package/signed_exchange_envelope.cc

@@ -245,20 +245,37 @@ bool ParseResponseMap(const cbor::Value& value,
             response_code));
     return false;
   }
+
+  // https://wicg.github.io/webpackage/loading.html#parsing-b2-cbor-headers
+  // 7. If responseHeaders does not contain `Content-Type`, return a failure.
+  // [spec text]
+  // Note: "Parsing b3 CBOR headers" algorithm should have the same step.
+  // See https://github.com/WICG/webpackage/issues/555
+  auto content_type_iter = out->response_headers().find("content-type");
+  if (content_type_iter == out->response_headers().end()) {
+    signed_exchange_utils::ReportErrorAndTraceEvent(
+        devtools_proxy,
+        "Exchange's inner response must have Content-Type header.");
+    return false;
+  }
+  // https://wicg.github.io/webpackage/loading.html#parsing-b2-cbor-headers
+  // 8. Set `X-Content-Type-Options`/`nosniff` in responseHeaders. [spec text]
+  // Note: "Parsing b3 CBOR headers" algorithm should have the same step.
+  // See https://github.com/WICG/webpackage/issues/555
+  out->SetResponseHeader("x-content-type-options", "nosniff");
+
   // Note: This does not reject content-type like "application/signed-exchange"
   // (no "v=" parameter). In that case, SignedExchangeRequestHandler does not
   // handle the inner response and UA just downloads it.
   // See https://github.com/WICG/webpackage/issues/299 for details.
-  auto found = out->response_headers().find("content-type");
-  if (found != out->response_headers().end() &&
-      signed_exchange_utils::GetSignedExchangeVersion(found->second)
+  if (signed_exchange_utils::GetSignedExchangeVersion(content_type_iter->second)
           .has_value()) {
     signed_exchange_utils::ReportErrorAndTraceEvent(
         devtools_proxy,
         base::StringPrintf(
             "Exchange's inner response must not be a signed-exchange. "
             "conetent-type: %s",
-            found->second.c_str()));
+            content_type_iter->second.c_str()));
     return false;
   }
 
@@ -346,6 +363,14 @@ bool SignedExchangeEnvelope::AddResponseHeader(base::StringPiece name,
   return true;
 }
 
+void SignedExchangeEnvelope::SetResponseHeader(base::StringPiece name,
+                                               base::StringPiece value) {
+  std::string name_str = name.as_string();
+  DCHECK_EQ(name_str, base::ToLowerASCII(name))
+      << "Response header names should be always lower-cased.";
+  response_headers_[name_str] = value.as_string();
+}
+
 scoped_refptr<net::HttpResponseHeaders>
 SignedExchangeEnvelope::BuildHttpResponseHeaders() const {
   std::string header_str("HTTP/1.1 ");

content/browser/web_package/signed_exchange_envelope.h

@@ -51,6 +51,9 @@ class CONTENT_EXPORT SignedExchangeEnvelope {
   // AddResponseHeader returns false on duplicated keys. |name| must be
   // lower-cased.
   bool AddResponseHeader(base::StringPiece name, base::StringPiece value);
+  // SetResponseHeader replaces existing value, if any. |name| must be
+  // lower-cased.
+  void SetResponseHeader(base::StringPiece name, base::StringPiece value);
   scoped_refptr<net::HttpResponseHeaders> BuildHttpResponseHeaders() const;
 
   const base::span<const uint8_t> cbor_header() const {

content/browser/web_package/signed_exchange_envelope_unittest.cc

@@ -103,7 +103,7 @@ TEST_P(SignedExchangeEnvelopeTest, ParseGoldenFile) {
   EXPECT_EQ(envelope->request_url().url,
             GURL("https://test.example.org/test/"));
   EXPECT_EQ(envelope->response_code(), static_cast<net::HttpStatusCode>(200u));
-  EXPECT_EQ(envelope->response_headers().size(), 3u);
+  EXPECT_EQ(envelope->response_headers().size(), 4u);
   EXPECT_EQ(envelope->response_headers().find("content-encoding")->second,
             "mi-sha256-03");
 }
@@ -115,7 +115,13 @@ TEST_P(SignedExchangeEnvelopeTest, ValidHeader) {
   ASSERT_TRUE(header.has_value());
   EXPECT_EQ(header->request_url().url, GURL("https://test.example.org/test/"));
   EXPECT_EQ(header->response_code(), static_cast<net::HttpStatusCode>(200u));
-  EXPECT_EQ(header->response_headers().size(), 2u);
+  EXPECT_EQ(header->response_headers().size(), 3u);
+
+  EXPECT_EQ(header->response_headers().find("content-type")->second,
+            "text/html");
+  EXPECT_EQ(header->response_headers().find("digest")->second, "foo");
+  EXPECT_EQ(header->response_headers().find("x-content-type-options")->second,
+            "nosniff");  // Injected by SignedExchangeEnvelope.
 }
 
 TEST_P(SignedExchangeEnvelopeTest, InformationalResponseCode) {
@@ -123,6 +129,7 @@ TEST_P(SignedExchangeEnvelopeTest, InformationalResponseCode) {
       GetParam(), "https://test.example.org/test/", kSignatureString,
       {
           {kStatusKey, "100"},
+          {"content-type", "text/html"},
       });
   ASSERT_FALSE(header.has_value());
 }
@@ -131,6 +138,7 @@ TEST_P(SignedExchangeEnvelopeTest, RelativeURL) {
   auto header = GenerateHeaderAndParse(GetParam(), "test/", kSignatureString,
                                        {
                                            {kStatusKey, "200"},
+                                           {"content-type", "text/html"},
                                        });
   ASSERT_FALSE(header.has_value());
 }
@@ -140,6 +148,7 @@ TEST_P(SignedExchangeEnvelopeTest, HttpURLShouldFail) {
       GetParam(), "http://test.example.org/test/", kSignatureString,
       {
           {kStatusKey, "200"},
+          {"content-type", "text/html"},
       });
   ASSERT_FALSE(header.has_value());
 }
@@ -154,7 +163,8 @@ TEST_P(SignedExchangeEnvelopeTest, RedirectStatusShouldFail) {
 TEST_P(SignedExchangeEnvelopeTest, Status300ShouldFail) {
   auto header = GenerateHeaderAndParse(
       GetParam(), "https://test.example.org/test/", kSignatureString,
-      {{kStatusKey, "300"}});  // 300 is not a redirect status.
+      {{kStatusKey, "300"},  // 300 is not a redirect status.
+       {"content-type", "text/html"}});
   ASSERT_FALSE(header.has_value());
 }
 
@@ -163,6 +173,7 @@ TEST_P(SignedExchangeEnvelopeTest, StatefulResponseHeader) {
       GetParam(), "https://test.example.org/test/", kSignatureString,
       {
           {kStatusKey, "200"},
+          {"content-type", "text/html"},
           {"set-cookie", "foo=bar"},
       });
   ASSERT_FALSE(header.has_value());
@@ -171,7 +182,9 @@ TEST_P(SignedExchangeEnvelopeTest, StatefulResponseHeader) {
 TEST_P(SignedExchangeEnvelopeTest, UppercaseResponseMap) {
   auto header = GenerateHeaderAndParse(
       GetParam(), "https://test.example.org/test/", kSignatureString,
-      {{kStatusKey, "200"}, {"Content-Length", "123"}});
+      {{kStatusKey, "200"},
+       {"content-type", "text/html"},
+       {"Content-Length", "123"}});
   ASSERT_FALSE(header.has_value());
 }
 
@@ -182,6 +195,24 @@ TEST_P(SignedExchangeEnvelopeTest, InvalidValidityURLHeader) {
   ASSERT_FALSE(header.has_value());
 }
 
+TEST_P(SignedExchangeEnvelopeTest, NoContentType) {
+  auto header =
+      GenerateHeaderAndParse(GetParam(), "https://test.example.org/test/",
+                             kSignatureString, {{kStatusKey, "200"}});
+  ASSERT_FALSE(header.has_value());
+}
+
+TEST_P(SignedExchangeEnvelopeTest, XContentTypeOptionsShouldBeOverwritten) {
+  auto header = GenerateHeaderAndParse(
+      GetParam(), "https://test.example.org/test/", kSignatureString,
+      {{kStatusKey, "200"},
+       {"content-type", "text/html"},
+       {"x-content-type-options", "foo"}});
+  ASSERT_TRUE(header.has_value());
+  EXPECT_EQ(header->response_headers().find("x-content-type-options")->second,
+            "nosniff");
+}
+
 TEST_P(SignedExchangeEnvelopeTest, InnerResponseIsSXG) {
   auto header = GenerateHeaderAndParse(
       GetParam(), "https://test.example.org/test/", kSignatureString,
@@ -195,6 +226,7 @@ TEST_P(SignedExchangeEnvelopeTest, CacheControlNoStore) {
       GetParam(), "https://test.example.org/test/", kSignatureString,
       {
           {kStatusKey, "200"},
+          {"content-type", "text/html"},
           {"cache-control", "no-store"},
       });
   ASSERT_FALSE(header.has_value());
@@ -205,6 +237,7 @@ TEST_P(SignedExchangeEnvelopeTest, CacheControlSecondValueIsNoStore) {
       GetParam(), "https://test.example.org/test/", kSignatureString,
       {
           {kStatusKey, "200"},
+          {"content-type", "text/html"},
           {"cache-control", "max-age=300, no-store"},
       });
   ASSERT_FALSE(header.has_value());
@@ -215,6 +248,7 @@ TEST_P(SignedExchangeEnvelopeTest, CacheControlPrivateWithValue) {
       GetParam(), "https://test.example.org/test/", kSignatureString,
       {
           {kStatusKey, "200"},
+          {"content-type", "text/html"},
           {"cache-control", "private=foo"},
       });
   ASSERT_FALSE(header.has_value());
@@ -225,6 +259,7 @@ TEST_P(SignedExchangeEnvelopeTest, CacheControlNoStoreInQuotedString) {
       GetParam(), "https://test.example.org/test/", kSignatureString,
       {
           {kStatusKey, "200"},
+          {"content-type", "text/html"},
           {"cache-control", "foo=\"300, no-store\""},
           {"digest", "foo"},
       });
@@ -236,6 +271,7 @@ TEST_P(SignedExchangeEnvelopeTest, CacheControlParseError) {
       GetParam(), "https://test.example.org/test/", kSignatureString,
       {
           {kStatusKey, "200"},
+          {"content-type", "text/html"},
           {"cache-control", "max-age=\"abc"},
       });
   ASSERT_FALSE(header.has_value());

content/browser/web_package/signed_exchange_request_handler_browsertest.cc

@@ -800,6 +800,31 @@ IN_PROC_BROWSER_TEST_F(SignedExchangeRequestHandlerDownloadBrowserTest,
             observer->observed_content_disposition());
 }
 
+IN_PROC_BROWSER_TEST_F(SignedExchangeRequestHandlerDownloadBrowserTest,
+                       DownloadInnerResponse) {
+  InstallMockCert();
+  InstallMockCertChainInterceptor();
+  std::unique_ptr<DownloadObserver> observer =
+      std::make_unique<DownloadObserver>(BrowserContext::GetDownloadManager(
+          shell()->web_contents()->GetBrowserContext()));
+
+  embedded_test_server()->ServeFilesFromSourceDirectory("content/test/data");
+  ASSERT_TRUE(embedded_test_server()->Start());
+  EXPECT_TRUE(
+      NavigateToURL(shell(), embedded_test_server()->GetURL("/empty.html")));
+
+  const std::string load_sxg =
+      "const iframe = document.createElement('iframe');"
+      "iframe.src = './sxg/test.example.org_bad_content_type.sxg';"
+      "document.body.appendChild(iframe);";
+  // Since the inner response has an invalid Content-Type and MIME sniffing
+  // is disabled for Signed Exchange inner response, this should download the
+  // inner response of the exchange.
+  EXPECT_TRUE(ExecuteScript(shell()->web_contents(), load_sxg));
+  observer->WaitUntilDownloadCreated();
+  EXPECT_EQ(GURL("https://test.example.org/test/"), observer->observed_url());
+}
+
 IN_PROC_BROWSER_TEST_F(SignedExchangeRequestHandlerDownloadBrowserTest,
                        DataURLDownload) {
   const GURL sxg_url = GURL("data:application/signed-exchange,");

content/test/data/sxg/generate-test-sxgs.sh

@@ -189,6 +189,23 @@ gen-signedexchange \
   -expire 168h \
   -o test.example.org_hello.txt.sxg
 
+# Generate the signed exchange whose content is a HTML but content-type is
+# an invalid value.
+gen-signedexchange \
+  -version 1b3 \
+  -uri https://test.example.org/test/ \
+  -status 200 \
+  -content test.html \
+  -certificate prime256v1-sha256.public.pem \
+  -certUrl https://cert.example.org/cert.msg \
+  -validityUrl https://test.example.org/resource.validity.msg \
+  -privateKey prime256v1.key \
+  -responseHeader 'Content-Type: 0' \
+  -date $signature_date \
+  -expire 168h \
+  -o test.example.org_bad_content_type.sxg \
+  -miRecordSize 100
+
 # Generate the signed exchange whose content is gzip-encoded.
 gzip -c test.html >$tmpdir/test.html.gz
 gen-signedexchange \

content/test/data/sxg/test.example.org_bad_content_type.sxg.mock-http-headers

+HTTP/1.1 200 OK
+Content-Type: application/signed-exchange;v=b3
+X-Content-Type-Options: nosniff

third_party/blink/web_tests/external/wpt/signed-exchange/resources/generate-test-sxgs.sh

@@ -545,6 +545,7 @@ gen-signedexchange \
   -version $sxg_version \
   -uri $inner_url_origin/signed-exchange/resources/sxg-subresource-script.js \
   -status 200 \
+  -responseHeader "Content-Type: application/javascript" \
   -content sxg-subresource-script-inner.js \
   -certificate $certfile \
   -certUrl $cert_url_origin/signed-exchange/resources/$certfile.cbor \