SignedEchange: Fallback if additional content-encodings are applied

Before this patch, Content-Encoding header of inner response was not
checked (content was assumed to be "mi-sha256-03" encoded).

Currently we only support "mi-sha256-03" inner encoding, so this patch
makes SignedExchangeHandler reject (causing fallback redirect) the
signed exchange if its inner Content-Encoding is not just "mi-sha256-03".

This also refactors the logic to check integrity scheme, by moving them
into single place, SignedExchangeHandler::CreateResponseBodyStream().

Note: This shouldn't affect amppackager, because it strips additional
content-encodings in b3.
https://github.com/ampproject/amppackager/issues/243

Bug: 931055, 934629
Change-Id: Icc0bde36194d4a387ab30968cccf1bef4c81fe60
Reviewed-on: https://chromium-review.googlesource.com/c/1482351Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635413}

content/browser/web_package/signed_exchange_envelope.cc

@@ -262,13 +262,6 @@ bool ParseResponseMap(const cbor::Value& value,
     return false;
   }
 
-  found = out->response_headers().find("digest");
-  if (found == out->response_headers().end()) {
-    signed_exchange_utils::ReportErrorAndTraceEvent(
-        devtools_proxy, "Signed exchange has no Digest: header");
-    return false;
-  }
-
   return true;
 }
 

content/browser/web_package/signed_exchange_error.cc

@@ -18,8 +18,6 @@ SignedExchangeError::GetFieldFromSignatureVerifierResult(
     case SignedExchangeSignatureVerifier::Result::
         kErrSignatureVerificationFailed:
       return Field::kSignatureSig;
-    case SignedExchangeSignatureVerifier::Result::kErrInvalidSignatureIntegrity:
-      return Field::kSignatureIintegrity;
     case SignedExchangeSignatureVerifier::Result::kErrUnsupportedCertType:
       return Field::kSignatureCertUrl;
     case SignedExchangeSignatureVerifier::Result::kErrValidityPeriodTooLong:
@@ -33,6 +31,8 @@ SignedExchangeError::GetFieldFromSignatureVerifierResult(
         kErrNoCertificateSHA256_deprecated:
     case SignedExchangeSignatureVerifier::Result::
         kErrInvalidSignatureFormat_deprecated:
+    case SignedExchangeSignatureVerifier::Result::
+        kErrInvalidSignatureIntegrity_deprecated:
     case SignedExchangeSignatureVerifier::Result::
         kErrInvalidTimestamp_deprecated:
       NOTREACHED();

content/browser/web_package/signed_exchange_handler.cc

@@ -4,6 +4,7 @@
 
 #include "content/browser/web_package/signed_exchange_handler.h"
 
+#include <memory>
 #include <utility>
 
 #include "base/bind.h"
@@ -53,7 +54,7 @@ namespace content {
 
 namespace {
 
-constexpr char kDigestHeader[] = "Digest";
+constexpr char kDigestHeader[] = "digest";
 constexpr char kHistogramSignatureVerificationResult[] =
     "SignedExchange.SignatureVerificationResult";
 constexpr char kHistogramCertVerificationResult[] =
@@ -653,11 +654,12 @@ void SignedExchangeHandler::OnVerifyCert(
   response_head.load_timing.send_end = now;
   response_head.load_timing.receive_headers_end = now;
 
-  std::string digest_header_value;
-  response_head.headers->EnumerateHeader(nullptr, kDigestHeader,
-                                         &digest_header_value);
-  auto mi_stream = std::make_unique<MerkleIntegritySourceStream>(
-      digest_header_value, std::move(source_));
+  auto body_stream = CreateResponseBodyStream();
+  if (!body_stream) {
+    RunErrorCallback(SignedExchangeLoadResult::kInvalidIntegrityHeader,
+                     net::ERR_INVALID_SIGNED_EXCHANGE);
+    return;
+  }
 
   net::SSLInfo ssl_info;
   ssl_info.cert = cv_result.verified_cert;
@@ -679,8 +681,50 @@ void SignedExchangeHandler::OnVerifyCert(
   response_head.ssl_info = std::move(ssl_info);
   std::move(headers_callback_)
       .Run(SignedExchangeLoadResult::kSuccess, net::OK,
-           envelope_->request_url().url, response_head, std::move(mi_stream));
+           envelope_->request_url().url, response_head, std::move(body_stream));
   state_ = State::kHeadersCallbackCalled;
 }
 
+// https://wicg.github.io/webpackage/loading.html#read-a-body
+std::unique_ptr<net::SourceStream>
+SignedExchangeHandler::CreateResponseBodyStream() {
+  if (!base::EqualsCaseInsensitiveASCII(envelope_->signature().integrity,
+                                        "digest/mi-sha256-03")) {
+    signed_exchange_utils::ReportErrorAndTraceEvent(
+        devtools_proxy_.get(),
+        "The current implemention only supports \"digest/mi-sha256-03\" "
+        "integrity scheme.",
+        std::make_pair(0 /* signature_index */,
+                       SignedExchangeError::Field::kSignatureIintegrity));
+    return nullptr;
+  }
+  const auto& headers = envelope_->response_headers();
+  auto digest_iter = headers.find(kDigestHeader);
+  if (digest_iter == headers.end()) {
+    signed_exchange_utils::ReportErrorAndTraceEvent(
+        devtools_proxy_.get(), "Signed exchange has no Digest: header");
+    return nullptr;
+  }
+
+  // For now, we allow only mi-sha256-03 content encoding.
+  // TODO(crbug.com/934629): Handle other content codings, such as gzip and br.
+  auto content_encoding_iter = headers.find("content-encoding");
+  if (content_encoding_iter == headers.end()) {
+    signed_exchange_utils::ReportErrorAndTraceEvent(
+        devtools_proxy_.get(),
+        "Signed exchange has no Content-Encoding: header");
+    return nullptr;
+  }
+  if (!base::LowerCaseEqualsASCII(content_encoding_iter->second,
+                                  "mi-sha256-03")) {
+    signed_exchange_utils::ReportErrorAndTraceEvent(
+        devtools_proxy_.get(),
+        "Exchange's Content-Encoding must be \"mi-sha256-03\".");
+    return nullptr;
+  }
+
+  return std::make_unique<MerkleIntegritySourceStream>(digest_iter->second,
+                                                       std::move(source_));
+}
+
 }  // namespace content

content/browser/web_package/signed_exchange_handler.h

@@ -128,6 +128,7 @@ class CONTENT_EXPORT SignedExchangeHandler {
   void OnVerifyCert(int32_t error_code,
                     const net::CertVerifyResult& cv_result,
                     const net::ct::CTVerifyResult& ct_result);
+  std::unique_ptr<net::SourceStream> CreateResponseBodyStream();
 
   const bool is_secure_transport_;
   const bool has_nosniff_;

content/browser/web_package/signed_exchange_handler_unittest.cc

@@ -460,6 +460,41 @@ TEST_P(SignedExchangeHandlerTest, MimeType) {
   EXPECT_EQ(rv, static_cast<int>(expected_payload.size()));
 }
 
+TEST_P(SignedExchangeHandlerTest, AdditionalContentEncodingShouldBeRejected) {
+  mock_cert_fetcher_factory_->ExpectFetch(
+      GURL("https://cert.example.org/cert.msg"),
+      GetTestFileContents("test.example.org.public.pem.cbor"));
+
+  // Make the MockCertVerifier treat the certificate
+  // "prime256v1-sha256.public.pem" as valid for "test.example.org".
+  scoped_refptr<net::X509Certificate> original_cert =
+      LoadCertificate("prime256v1-sha256.public.pem");
+  net::CertVerifyResult dummy_result;
+  dummy_result.verified_cert = original_cert;
+  dummy_result.cert_status = net::OK;
+  dummy_result.ocsp_result.response_status = net::OCSPVerifyResult::PROVIDED;
+  dummy_result.ocsp_result.revocation_status = net::OCSPRevocationStatus::GOOD;
+  auto mock_cert_verifier = std::make_unique<net::MockCertVerifier>();
+  mock_cert_verifier->AddResultForCertAndHost(original_cert, "test.example.org",
+                                              dummy_result, net::OK);
+  SetCertVerifier(std::move(mock_cert_verifier));
+
+  std::string contents =
+      GetTestFileContents("test.example.org_test.html.gz.sxg");
+  source_->AddReadResult(contents.data(), contents.size(), net::OK, GetParam());
+  source_->AddReadResult(nullptr, 0, net::OK, GetParam());
+
+  CreateSignedExchangeHandler(CreateTestURLRequestContext());
+  WaitForHeader();
+
+  ASSERT_TRUE(read_header());
+  EXPECT_EQ(SignedExchangeLoadResult::kInvalidIntegrityHeader, result());
+  EXPECT_EQ(net::ERR_INVALID_SIGNED_EXCHANGE, error());
+  EXPECT_EQ(kTestSxgInnerURL, inner_url());
+  // Drain the MockSourceStream, otherwise its destructer causes DCHECK failure.
+  ReadStream(source_, nullptr);
+}
+
 TEST_P(SignedExchangeHandlerTest, HeaderParseError) {
   const uint8_t data[] = {'s',  'x',  'g',  '1',  '-',  'b',  '2',  '\0',
                           0x00, 0x00, 0x01, 0x00, 0x00, 0x01, 0x00, 0x00};

content/browser/web_package/signed_exchange_signature_verifier.cc

@@ -318,15 +318,6 @@ SignedExchangeSignatureVerifier::Result SignedExchangeSignatureVerifier::Verify(
         devtools_proxy, "Failed to verify signature \"sig\".");
     return Result::kErrSignatureVerificationFailed;
   }
-
-  if (!base::EqualsCaseInsensitiveASCII(envelope.signature().integrity,
-                                        "digest/mi-sha256-03")) {
-    signed_exchange_utils::ReportErrorAndTraceEvent(
-        devtools_proxy,
-        "The current implemention only supports \"digest/mi-sha256-03\" "
-        "integrity scheme.");
-    return Result::kErrInvalidSignatureIntegrity;
-  }
   return Result::kSuccess;
 }
 

content/browser/web_package/signed_exchange_signature_verifier.h

@@ -42,7 +42,7 @@ class CONTENT_EXPORT SignedExchangeSignatureVerifier final {
     kErrCertificateSHA256Mismatch,
     kErrInvalidSignatureFormat_deprecated,
     kErrSignatureVerificationFailed,
-    kErrInvalidSignatureIntegrity,
+    kErrInvalidSignatureIntegrity_deprecated,
     kErrInvalidTimestamp_deprecated,
     kErrUnsupportedCertType,
     kErrValidityPeriodTooLong,

content/browser/web_package/signed_exchange_utils.cc

@@ -118,14 +118,6 @@ SignedExchangeLoadResult GetLoadResultFromSignatureVerifierResult(
       //     requestUrlBytes, and signed exchange version version, return
       //     "signature_verification_error"." [spec text]
       return SignedExchangeLoadResult::kSignatureVerificationError;
-    case SignedExchangeSignatureVerifier::Result::kErrInvalidSignatureIntegrity:
-      // "Creating the response stream.
-      //   - If signature’s integrity header is:
-      //     - "digest", "mi-sha256-03"
-      //       ...
-      //     - Anything else
-      //       Return an error string "invalid_integrity_header". " [spec text]
-      return SignedExchangeLoadResult::kInvalidIntegrityHeader;
     case SignedExchangeSignatureVerifier::Result::kErrUnsupportedCertType:
       // "Validating a signature
       //   ...
@@ -172,6 +164,8 @@ SignedExchangeLoadResult GetLoadResultFromSignatureVerifierResult(
         kErrNoCertificateSHA256_deprecated:
     case SignedExchangeSignatureVerifier::Result::
         kErrInvalidSignatureFormat_deprecated:
+    case SignedExchangeSignatureVerifier::Result::
+        kErrInvalidSignatureIntegrity_deprecated:
     case SignedExchangeSignatureVerifier::Result::
         kErrInvalidTimestamp_deprecated:
       NOTREACHED();

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

@@ -111,6 +111,21 @@ gen-signedexchange \
   -date 2018-03-12T05:53:20Z \
   -o test.example.org_hello.txt.sxg
 
+# Generate the signed exchange whose content is gzip-encoded.
+gzip -c test.html >$tmpdir/test.html.gz
+gen-signedexchange \
+  -version 1b3 \
+  -uri https://test.example.org/test/ \
+  -status 200 \
+  -content $tmpdir/test.html.gz \
+  -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-Encoding: gzip' \
+  -date 2018-03-12T05:53:20Z \
+  -o test.example.org_test.html.gz.sxg
+
 echo "Update the test signatures in "
 echo "signed_exchange_signature_verifier_unittest.cc with the followings:"
 echo "===="