SignedExchange: Bound certificate's validity period to 90 days

This implements WICG/webpackage#383. After this patch, SignedExchangeHandler rejects certificates that have validity period longer than 90 days, if - the validity period starts after 2019-05-01, or - the verification date is after 2019-08-01. Bug: 953165 Change-Id: I607952af6a315ca3c4f5743bc8b0dd4ae71a5057 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1569371 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-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@{#651623} Former-commit-id: 4a0bd3fa56c8f6dcfd2696ecbce2bbf3ceaf922c
bloomberg · Apr 17, 2019 · 387b75d · 387b75d
1 parent 4d8d3b5
commit 387b75d
Show file tree

Hide file tree

Showing 38 changed files with 671 additions and 105 deletions.
diff --git a/content/browser/web_package/signed_exchange_error.h b/content/browser/web_package/signed_exchange_error.h
@@ -49,7 +49,9 @@ enum class SignedExchangeLoadResult {
   kInvalidIntegrityHeader,
   // SXG has Variants / Variant-Key headers that don't match the request.
   kVariantMismatch,
-  kMaxValue = kVariantMismatch
+  // Certificate's validity period is too long.
+  kCertValidityPeriodTooLong,
+  kMaxValue = kCertValidityPeriodTooLong
 };
 
 struct SignedExchangeError {

diff --git a/content/browser/web_package/signed_exchange_handler.cc b/content/browser/web_package/signed_exchange_handler.cc
@@ -542,17 +542,62 @@ void SignedExchangeHandler::OnCertReceived(
                                     weak_factory_.GetWeakPtr())));
 }
 
-bool SignedExchangeHandler::CheckCertExtension(
+// https://wicg.github.io/webpackage/draft-yasskin-http-origin-signed-responses.html#cross-origin-cert-req
+SignedExchangeLoadResult SignedExchangeHandler::CheckCertRequirements(
     const net::X509Certificate* verified_cert) {
-  if (base::FeatureList::IsEnabled(
-          features::kAllowSignedHTTPExchangeCertsWithoutExtension))
-    return true;
-
   // https://wicg.github.io/webpackage/draft-yasskin-http-origin-signed-responses.html#cross-origin-trust
   // Step 6.2. Validate that main-certificate has the CanSignHttpExchanges
   // extension (Section 4.2). [spec text]
-  return net::asn1::HasCanSignHttpExchangesDraftExtension(
-      net::x509_util::CryptoBufferAsStringPiece(verified_cert->cert_buffer()));
+  if (!net::asn1::HasCanSignHttpExchangesDraftExtension(
+          net::x509_util::CryptoBufferAsStringPiece(
+              verified_cert->cert_buffer())) &&
+      !base::FeatureList::IsEnabled(
+          features::kAllowSignedHTTPExchangeCertsWithoutExtension)) {
+    signed_exchange_utils::ReportErrorAndTraceEvent(
+        devtools_proxy_.get(),
+        "Certificate must have CanSignHttpExchangesDraft extension. To ignore "
+        "this error for testing, enable "
+        "chrome://flags/#allow-sxg-certs-without-extension.",
+        std::make_pair(0 /* signature_index */,
+                       SignedExchangeError::Field::kSignatureCertUrl));
+    return SignedExchangeLoadResult::kCertRequirementsNotMet;
+  }
+
+  // - Clients MUST reject certificates with this extension that were issued
+  // after 2019-05-01 and have a Validity Period longer than 90 days. [spec
+  // text]
+  // - After 2019-08-01, clients MUST reject all certificates with this
+  // extension that have a Validity Period longer than 90 days. [spec text]
+  // TODO(crbug.com/953165): Simplify this logic after 2019-08-01.
+  base::TimeDelta validity_period =
+      verified_cert->valid_expiry() - verified_cert->valid_start();
+  if (validity_period > base::TimeDelta::FromDays(90)) {
+    // 2019-05-01 00:00:00 UTC.
+    const base::Time kRequirementStartDateForIssuance =
+        base::Time::UnixEpoch() + base::TimeDelta::FromSeconds(1556668800);
+    if (verified_cert->valid_start() >= kRequirementStartDateForIssuance) {
+      signed_exchange_utils::ReportErrorAndTraceEvent(
+          devtools_proxy_.get(),
+          "Signed Exchange's certificate issued after 2019-05-01 must not have "
+          "a validity period longer than 90 days.",
+          std::make_pair(0 /* signature_index */,
+                         SignedExchangeError::Field::kSignatureCertUrl));
+      return SignedExchangeLoadResult::kCertValidityPeriodTooLong;
+    }
+    // 2019-08-01 00:00:00 UTC.
+    const base::Time kRequirementStartDateForVerification =
+        base::Time::UnixEpoch() + base::TimeDelta::FromSeconds(1564617600);
+    if (GetVerificationTime() >= kRequirementStartDateForVerification) {
+      signed_exchange_utils::ReportErrorAndTraceEvent(
+          devtools_proxy_.get(),
+          "After 2019-08-01, Signed Exchange's certificate must not have a "
+          "validity period longer than 90 days.",
+          std::make_pair(0 /* signature_index */,
+                         SignedExchangeError::Field::kSignatureCertUrl));
+      return SignedExchangeLoadResult::kCertValidityPeriodTooLong;
+    }
+  }
+  return SignedExchangeLoadResult::kSuccess;
 }
 
 bool SignedExchangeHandler::CheckOCSPStatus(
@@ -616,16 +661,10 @@ void SignedExchangeHandler::OnVerifyCert(
     return;
   }
 
-  if (!CheckCertExtension(cv_result.verified_cert.get())) {
-    signed_exchange_utils::ReportErrorAndTraceEvent(
-        devtools_proxy_.get(),
-        "Certificate must have CanSignHttpExchangesDraft extension. To ignore "
-        "this error for testing, enable "
-        "chrome://flags/#allow-sxg-certs-without-extension.",
-        std::make_pair(0 /* signature_index */,
-                       SignedExchangeError::Field::kSignatureCertUrl));
-    RunErrorCallback(SignedExchangeLoadResult::kCertRequirementsNotMet,
-                     net::ERR_INVALID_SIGNED_EXCHANGE);
+  SignedExchangeLoadResult result =
+      CheckCertRequirements(cv_result.verified_cert.get());
+  if (result != SignedExchangeLoadResult::kSuccess) {
+    RunErrorCallback(result, net::ERR_INVALID_SIGNED_EXCHANGE);
     return;
   }
 

diff --git a/content/browser/web_package/signed_exchange_handler.h b/content/browser/web_package/signed_exchange_handler.h
@@ -125,7 +125,8 @@ class CONTENT_EXPORT SignedExchangeHandler {
   void OnCertReceived(
       SignedExchangeLoadResult result,
       std::unique_ptr<SignedExchangeCertificateChain> cert_chain);
-  bool CheckCertExtension(const net::X509Certificate* verified_cert);
+  SignedExchangeLoadResult CheckCertRequirements(
+      const net::X509Certificate* verified_cert);
   bool CheckOCSPStatus(const net::OCSPVerifyResult& ocsp_result);
 
   void OnVerifyCert(int32_t error_code,

diff --git a/content/browser/web_package/signed_exchange_handler_unittest.cc b/content/browser/web_package/signed_exchange_handler_unittest.cc
@@ -46,7 +46,9 @@ namespace content {
 
 namespace {
 
-const uint64_t kSignatureHeaderDate = 1520834000;
+const uint64_t kSignatureHeaderDate = 1564272000;  // 2019-07-28T00:00:00Z
+const uint64_t kCertValidityPeriodEnforcementDate =
+    1564617600;  // 2019-08-01T00:00:00Z
 const int kOutputBufferSize = 4096;
 
 constexpr char kTestSxgInnerURL[] = "https://test.example.org/test/";
@@ -562,6 +564,127 @@ TEST_P(SignedExchangeHandlerTest, CertWithoutExtensionShouldBeRejected) {
   ReadStream(source_, nullptr);
 }
 
+TEST_P(SignedExchangeHandlerTest,
+       CertIssuedAfterMay2019AndValidMoreThan90DaysShouldBeRejected) {
+  mock_cert_fetcher_factory_->ExpectFetch(
+      GURL("https://cert.example.org/cert.msg"),
+      GetTestFileContents(
+          "test.example.org-validity-too-long.public.pem.cbor"));
+
+  // Make the MockCertVerifier treat the certificate
+  // "prime256v1-sha256-validity-too-long.public.pem" as valid for
+  // "test.example.org".
+  scoped_refptr<net::X509Certificate> original_cert =
+      LoadCertificate("prime256v1-sha256-validity-too-long.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_cert_validity_too_long.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::kCertValidityPeriodTooLong, 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,
+       CertIssuedAfterMay2019AndValidFor90DaysShouldBeAccepted) {
+  SignedExchangeHandler::SetVerificationTimeForTesting(
+      base::Time::UnixEpoch() +
+      base::TimeDelta::FromSeconds(kCertValidityPeriodEnforcementDate));
+  mock_cert_fetcher_factory_->ExpectFetch(
+      GURL("https://cert.example.org/cert.msg"),
+      GetTestFileContents(
+          "test.example.org-valid-for-90-days.public.pem.cbor"));
+
+  // Make the MockCertVerifier treat the certificate
+  // "prime256v1-sha256-validity-too-long.public.pem" as valid for
+  // "test.example.org".
+  scoped_refptr<net::X509Certificate> original_cert =
+      LoadCertificate("prime256v1-sha256-valid-for-90-days.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_cert_valid_for_90_days.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::kSuccess, result());
+  EXPECT_EQ(net::OK, error());
+  std::string payload;
+  int rv = ReadPayloadStream(&payload);
+  std::string expected_payload = GetTestFileContents("test.html");
+
+  EXPECT_EQ(expected_payload, payload);
+  EXPECT_EQ(static_cast<int>(expected_payload.size()), rv);
+}
+
+TEST_P(SignedExchangeHandlerTest,
+       CertValidMoreThan90DaysShouldBeRejectedAfterAugust2019) {
+  SignedExchangeHandler::SetVerificationTimeForTesting(
+      base::Time::UnixEpoch() +
+      base::TimeDelta::FromSeconds(kCertValidityPeriodEnforcementDate));
+  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-validity-too-long.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.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::kCertValidityPeriodTooLong, 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, CertWithoutExtensionAllowedByFeatureFlag) {
   base::test::ScopedFeatureList scoped_feature_list_;
   scoped_feature_list_.InitAndEnableFeature(

diff --git a/content/browser/web_package/signed_exchange_reporter.cc b/content/browser/web_package/signed_exchange_reporter.cc
@@ -67,6 +67,7 @@ const char* GetResultTypeString(SignedExchangeLoadResult result) {
     case SignedExchangeLoadResult::kOCSPError:
       return kSXGResultCertVerificationError;
     case SignedExchangeLoadResult::kCertRequirementsNotMet:
+    case SignedExchangeLoadResult::kCertValidityPeriodTooLong:
       return kSXGResultCertVerificationError;
     case SignedExchangeLoadResult::kMerkleIntegrityError:
       return kSXGResultMiError;

diff --git a/content/public/test/signed_exchange_browser_test_helper.cc b/content/public/test/signed_exchange_browser_test_helper.cc
@@ -28,9 +28,9 @@
 namespace content {
 
 constexpr uint64_t SignedExchangeBrowserTestHelper::kSignatureHeaderDate =
-    1520834000;  // 2018-03-12T05:53:20Z
+    1564272000;  // 2019-07-28T00:00:00Z
 constexpr uint64_t SignedExchangeBrowserTestHelper::kSignatureHeaderExpires =
-    1520837600;  // 2018-03-12T06:53:20Z
+    1564876800;  // 2019-08-04T00:00:00Z
 
 SignedExchangeBrowserTestHelper::SignedExchangeBrowserTestHelper() = default;
 

diff --git a/content/test/data/sxg/ca.cnf b/content/test/data/sxg/ca.cnf
@@ -0,0 +1,33 @@
+[ca]
+default_ca = CA_root
+preserve   = yes
+
+[CA_root]
+dir            = out
+new_certs_dir  = $dir
+database       = $dir/index.txt
+serial         = $dir/serial
+certificate    = ../../../../net/data/ssl/certificates/root_ca_cert.pem
+private_key    = ../../../../net/data/ssl/certificates/root_ca_cert.pem
+default_md     = sha256
+unique_subject = no
+policy         = policy_anything
+
+[sxg_cert]
+basicConstraints = CA:FALSE
+# OID required for sxg since d54c469
+1.3.6.1.4.1.11129.2.1.22 = ASN1:NULL
+keyUsage = nonRepudiation, digitalSignature, keyEncipherment
+subjectKeyIdentifier = hash
+authorityKeyIdentifier = keyid,issuer
+subjectAltName=DNS:test.example.org
+
+[policy_anything]
+# Default signing policy
+countryName            = optional
+stateOrProvinceName    = optional
+localityName           = optional
+organizationName       = optional
+organizationalUnitName = optional
+commonName             = optional
+emailAddress           = optional
diff --git a/content/test/data/sxg/generate-test-certs.sh b/content/test/data/sxg/generate-test-certs.sh
@@ -13,30 +13,69 @@ dumpSPKIHash() {
       base64
 }
 
+rm -rf out
+mkdir out
+/bin/sh -c "echo 01 > out/serial"
+touch out/index.txt
+
 # Generate a "secp256r1 (== prime256v1) ecdsa with sha256" key/cert pair
 openssl ecparam -out prime256v1.key -name prime256v1 -genkey
 
 openssl req -new -sha256 -key prime256v1.key -out prime256v1-sha256.csr \
   -subj '/CN=test.example.org/O=Test/C=US'
 
-openssl x509 -req -days 360 -in prime256v1-sha256.csr \
-  -CA ../../../../net/data/ssl/certificates/root_ca_cert.pem \
-  -out prime256v1-sha256.public.pem -set_serial 1 \
-  -extfile x509.ext
+# Generate a certificate. This will be rejected after 2019-08-01, because
+# the validity period is more than 90 days.
+openssl ca -batch \
+  -config ca.cnf \
+  -extensions sxg_cert \
+  -startdate 190101000000Z \
+  -enddate   200101000000Z \
+  -in  prime256v1-sha256.csr \
+  -out prime256v1-sha256.public.pem
+
+# Generate a certificate without CanSignHttpExchangesDraft extension.
+openssl ca -batch \
+  -config ca.cnf \
+  -startdate 190101000000Z \
+  -enddate   200101000000Z \
+  -in  prime256v1-sha256.csr \
+  -out prime256v1-sha256-noext.public.pem
+
+# Generate a certificate whose validity period starts at 2019-05-01 and
+# valid for 91 days.
+openssl ca -batch \
+  -config ca.cnf \
+  -extensions sxg_cert \
+  -startdate 190501000000Z \
+  -enddate   190731000000Z \
+  -in  prime256v1-sha256.csr \
+  -out prime256v1-sha256-validity-too-long.public.pem
 
-openssl x509 -req -days 360 -in prime256v1-sha256.csr \
-  -CA ../../../../net/data/ssl/certificates/root_ca_cert.pem \
-  -out prime256v1-sha256-noext.public.pem -set_serial 1
+# Generate a certificate whose validity period starts at 2019-06-01 and
+# valid for 90 days.
+openssl ca -batch \
+  -config ca.cnf \
+  -extensions sxg_cert \
+  -startdate 190601000000Z \
+  -enddate   190830000000Z \
+  -in  prime256v1-sha256.csr \
+  -out prime256v1-sha256-valid-for-90-days.public.pem
 
 # Generate a "secp384r1 ecdsa with sha256" key/cert pair for negative test
 openssl ecparam -out secp384r1.key -name secp384r1 -genkey
 
 openssl req -new -sha256 -key secp384r1.key -out secp384r1-sha256.csr \
   --subj '/CN=test.example.org/O=Test/C=US'
 
-openssl x509 -req -days 360 -in secp384r1-sha256.csr \
-  -CA ../../../../net/data/ssl/certificates/root_ca_cert.pem \
-  -out secp384r1-sha256.public.pem -set_serial 1
+# Generate a certificate with the secp384r1-sha256 key.
+openssl ca -batch \
+  -config ca.cnf \
+  -extensions sxg_cert \
+  -startdate 190101000000Z \
+  -enddate   200101000000Z \
+  -in  secp384r1-sha256.csr \
+  -out secp384r1-sha256.public.pem
 
 echo
 echo "Update the test certs in signed_exchange_signature_verifier_unittest.cc"