Commit 67c36e7a authored by Tsuyoshi Horo's avatar Tsuyoshi Horo Committed by Commit Bot

Verify the timestamp of signed exchange signature

Before this CL, SignedExchangeSignatureVerifier didn't verify
date and expires values on the "Signature" HTTP header.
This CL introduces SignedExchangeSignatureVerifier::VerifyTimestamps
to verify those.

The original CL http://crrev.com/c/923468 was created by kouhei@.

Bug: 803774
Change-Id: I21a2db04c9bf80787226631696e1e334219bf893
Reviewed-on: https://chromium-review.googlesource.com/958368Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543012}
parent c25b94ff
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "content/browser/web_package/signed_exchange_handler.h" #include "content/browser/web_package/signed_exchange_handler.h"
#include "base/feature_list.h" #include "base/feature_list.h"
#include "base/time/time.h"
#include "content/browser/loader/merkle_integrity_source_stream.h" #include "content/browser/loader/merkle_integrity_source_stream.h"
#include "content/browser/web_package/signed_exchange_cert_fetcher.h" #include "content/browser/web_package/signed_exchange_cert_fetcher.h"
#include "content/browser/web_package/signed_exchange_consts.h" #include "content/browser/web_package/signed_exchange_consts.h"
...@@ -39,6 +40,14 @@ constexpr char kMiHeader[] = "MI"; ...@@ -39,6 +40,14 @@ constexpr char kMiHeader[] = "MI";
net::CertVerifier* g_cert_verifier_for_testing = nullptr; net::CertVerifier* g_cert_verifier_for_testing = nullptr;
base::Optional<base::Time> g_verification_time_for_testing;
base::Time GetVerificationTime() {
if (g_verification_time_for_testing)
return *g_verification_time_for_testing;
return base::Time::Now();
}
} // namespace } // namespace
// static // static
...@@ -47,6 +56,12 @@ void SignedExchangeHandler::SetCertVerifierForTesting( ...@@ -47,6 +56,12 @@ void SignedExchangeHandler::SetCertVerifierForTesting(
g_cert_verifier_for_testing = cert_verifier; g_cert_verifier_for_testing = cert_verifier;
} }
// static
void SignedExchangeHandler::SetVerificationTimeForTesting(
base::Optional<base::Time> verification_time_for_testing) {
g_verification_time_for_testing = verification_time_for_testing;
}
SignedExchangeHandler::SignedExchangeHandler( SignedExchangeHandler::SignedExchangeHandler(
std::unique_ptr<net::SourceStream> body, std::unique_ptr<net::SourceStream> body,
ExchangeHeadersCallback headers_callback, ExchangeHeadersCallback headers_callback,
...@@ -201,7 +216,9 @@ void SignedExchangeHandler::OnCertReceived( ...@@ -201,7 +216,9 @@ void SignedExchangeHandler::OnCertReceived(
RunErrorCallback(net::ERR_FAILED); RunErrorCallback(net::ERR_FAILED);
return; return;
} }
if (SignedExchangeSignatureVerifier::Verify(*header_, cert) !=
if (SignedExchangeSignatureVerifier::Verify(*header_, cert,
GetVerificationTime()) !=
SignedExchangeSignatureVerifier::Result::kSuccess) { SignedExchangeSignatureVerifier::Result::kSuccess) {
RunErrorCallback(net::ERR_FAILED); RunErrorCallback(net::ERR_FAILED);
return; return;
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/time/time.h"
#include "content/browser/web_package/signed_exchange_header.h" #include "content/browser/web_package/signed_exchange_header.h"
#include "content/common/content_export.h" #include "content/common/content_export.h"
#include "mojo/public/cpp/system/data_pipe.h" #include "mojo/public/cpp/system/data_pipe.h"
...@@ -61,6 +62,9 @@ class CONTENT_EXPORT SignedExchangeHandler { ...@@ -61,6 +62,9 @@ class CONTENT_EXPORT SignedExchangeHandler {
// MockCertVerifier in browser tests instead of using the static method. // MockCertVerifier in browser tests instead of using the static method.
static void SetCertVerifierForTesting(net::CertVerifier* cert_verifier); static void SetCertVerifierForTesting(net::CertVerifier* cert_verifier);
static void SetVerificationTimeForTesting(
base::Optional<base::Time> verification_time_for_testing);
// Once constructed |this| starts reading the |body| and parses the response // Once constructed |this| starts reading the |body| and parses the response
// as a signed HTTP exchange. The response body of the exchange can be read // as a signed HTTP exchange. The response body of the exchange can be read
// from |payload_stream| passed to |headers_callback|. |url_loader_factory| // from |payload_stream| passed to |headers_callback|. |url_loader_factory|
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/strings/string_piece.h" #include "base/strings/string_piece.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/time/time.h"
#include "components/cbor/cbor_values.h" #include "components/cbor/cbor_values.h"
#include "components/cbor/cbor_writer.h" #include "components/cbor/cbor_writer.h"
#include "content/browser/web_package/signed_exchange_consts.h" #include "content/browser/web_package/signed_exchange_consts.h"
...@@ -193,11 +194,43 @@ base::Optional<std::vector<uint8_t>> GenerateSignedMessage( ...@@ -193,11 +194,43 @@ base::Optional<std::vector<uint8_t>> GenerateSignedMessage(
return message; return message;
} }
base::Time TimeFromSignedExchangeUnixTime(uint64_t t) {
return base::Time::UnixEpoch() + base::TimeDelta::FromSeconds(t);
}
// Implements steps 9-10 of
// https://wicg.github.io/webpackage/draft-yasskin-http-origin-signed-responses.html#rfc.section.3.6
bool VerifyTimestamps(const SignedExchangeHeader& header,
const base::Time& verification_time) {
base::Time expires_time =
TimeFromSignedExchangeUnixTime(header.signature().expires);
base::Time creation_time =
TimeFromSignedExchangeUnixTime(header.signature().date);
// 9. "If expires is more than 7 days (604800 seconds) after date, return
// "invalid"." [spec text]
if ((expires_time - creation_time).InSeconds() > 604800)
return false;
// 10. "If the current time is before date or after expires, return
// "invalid"."
if (verification_time < creation_time || expires_time < verification_time)
return false;
return true;
}
} // namespace } // namespace
SignedExchangeSignatureVerifier::Result SignedExchangeSignatureVerifier::Verify( SignedExchangeSignatureVerifier::Result SignedExchangeSignatureVerifier::Verify(
const SignedExchangeHeader& header, const SignedExchangeHeader& header,
scoped_refptr<net::X509Certificate> certificate) { scoped_refptr<net::X509Certificate> certificate,
const base::Time& verification_time) {
if (!VerifyTimestamps(header, verification_time)) {
DVLOG(1) << "Invalid timestamp";
return Result::kErrInvalidTimestamp;
}
if (!certificate) { if (!certificate) {
DVLOG(1) << "No certificate set."; DVLOG(1) << "No certificate set.";
return Result::kErrNoCertificate; return Result::kErrNoCertificate;
...@@ -237,8 +270,6 @@ SignedExchangeSignatureVerifier::Result SignedExchangeSignatureVerifier::Verify( ...@@ -237,8 +270,6 @@ SignedExchangeSignatureVerifier::Result SignedExchangeSignatureVerifier::Verify(
return Result::kErrInvalidSignatureIntegrity; return Result::kErrInvalidSignatureIntegrity;
} }
// TODO(crbug.com/803774): Verify header.signature().{date,expires}.
return Result::kSuccess; return Result::kSuccess;
} }
......
...@@ -14,6 +14,10 @@ ...@@ -14,6 +14,10 @@
#include "content/common/content_export.h" #include "content/common/content_export.h"
#include "net/cert/x509_certificate.h" #include "net/cert/x509_certificate.h"
namespace base {
class Time;
} // namespace base
namespace content { namespace content {
class SignedExchangeHeader; class SignedExchangeHeader;
...@@ -37,11 +41,13 @@ class CONTENT_EXPORT SignedExchangeSignatureVerifier final { ...@@ -37,11 +41,13 @@ class CONTENT_EXPORT SignedExchangeSignatureVerifier final {
kErrCertificateSHA256Mismatch, kErrCertificateSHA256Mismatch,
kErrInvalidSignatureFormat, kErrInvalidSignatureFormat,
kErrSignatureVerificationFailed, kErrSignatureVerificationFailed,
kErrInvalidSignatureIntegrity kErrInvalidSignatureIntegrity,
kErrInvalidTimestamp
}; };
static Result Verify(const SignedExchangeHeader& header, static Result Verify(const SignedExchangeHeader& header,
scoped_refptr<net::X509Certificate> certificate); scoped_refptr<net::X509Certificate> certificate,
const base::Time& verification_time);
static base::Optional<std::vector<uint8_t>> EncodeCanonicalExchangeHeaders( static base::Optional<std::vector<uint8_t>> EncodeCanonicalExchangeHeaders(
const SignedExchangeHeader& header); const SignedExchangeHeader& header);
......
...@@ -58,6 +58,9 @@ TEST(SignedExchangeSignatureVerifier, EncodeCanonicalExchangeHeaders) { ...@@ -58,6 +58,9 @@ TEST(SignedExchangeSignatureVerifier, EncodeCanonicalExchangeHeaders) {
testing::ElementsAreArray(kExpected, arraysize(kExpected))); testing::ElementsAreArray(kExpected, arraysize(kExpected)));
} }
const uint64_t kSignatureHeaderDate = 1517892341;
const uint64_t kSignatureHeaderExpires = 1517895941;
// clang-format off // clang-format off
constexpr char kSignatureHeader[] = constexpr char kSignatureHeader[] =
"sig; " "sig; "
...@@ -73,6 +76,23 @@ constexpr char kSignatureHeader[] = ...@@ -73,6 +76,23 @@ constexpr char kSignatureHeader[] =
"date=1517892341; expires=1517895941"; "date=1517892341; expires=1517895941";
// clang-format on // clang-format on
// |expires| (1518497142) is more than 7 days (604800 seconds) after |date|
// (1517892341).
// clang-format off
constexpr char kSignatureHeaderInvalidExpires[] =
"sig; "
"sig=*RhjjWuXi87riQUu90taBHFJgTo8XBhiCe9qTJMP7/XVPu2diRGipo06HoGsyXkidHiiW"
"743JgoNmO7CjfeVXLXQgKDxtGidATtPsVadAT4JpBDZJWSUg5qAbWcASXjyO38Uhq9gJkeu4w"
"1MRMGkvpgVXNjYhi5/9NUer1xEUuJh5UbIDhGrfMihwj+c30nW+qz0n5lCrYonk+Sc0jGcLgc"
"aDLptqRhOG5S+avwKmbQoqtD0JSc/53L5xXjppyvSA2fRmoDlqVQpX4uzRKq9cny7fZ3qgpZ/"
"YOCuT7wMj7oVEur175QLe2F8ktKH9arSEiquhFJxBIIIXza8PJnmL5w;"
"validityUrl=\"https://example.com/resource.validity.msg\"; "
"integrity=\"mi\"; "
"certUrl=\"https://example.com/cert.msg\"; "
"certSha256=*3wfzkF4oKGUwoQ0rE7U11FIdcA/8biGzlaACeRQQH6k; "
"date=1517892341; expires=1518497142";
// clang-format on
constexpr char kCertPEM[] = R"( constexpr char kCertPEM[] = R"(
-----BEGIN CERTIFICATE----- -----BEGIN CERTIFICATE-----
MIID9zCCAt+gAwIBAgIUde2ndSB4271TAGDk0Ft+WuCCGnMwDQYJKoZIhvcNAQEL MIID9zCCAt+gAwIBAgIUde2ndSB4271TAGDk0Ft+WuCCGnMwDQYJKoZIhvcNAQEL
...@@ -100,6 +120,9 @@ wYxI2+BLS6X5NpI= ...@@ -100,6 +120,9 @@ wYxI2+BLS6X5NpI=
-----END CERTIFICATE-----)"; -----END CERTIFICATE-----)";
TEST(SignedExchangeSignatureVerifier, Verify) { TEST(SignedExchangeSignatureVerifier, Verify) {
base::Time verification_time =
base::Time::UnixEpoch() +
base::TimeDelta::FromSeconds(kSignatureHeaderDate);
auto signature = SignedExchangeHeaderParser::ParseSignature(kSignatureHeader); auto signature = SignedExchangeHeaderParser::ParseSignature(kSignatureHeader);
ASSERT_TRUE(signature.has_value()); ASSERT_TRUE(signature.has_value());
ASSERT_EQ(1u, signature->size()); ASSERT_EQ(1u, signature->size());
...@@ -122,13 +145,44 @@ TEST(SignedExchangeSignatureVerifier, Verify) { ...@@ -122,13 +145,44 @@ TEST(SignedExchangeSignatureVerifier, Verify) {
auto certificate = certlist[0]; auto certificate = certlist[0];
EXPECT_EQ(SignedExchangeSignatureVerifier::Result::kSuccess, EXPECT_EQ(SignedExchangeSignatureVerifier::Result::kSuccess,
SignedExchangeSignatureVerifier::Verify(header, certificate)); SignedExchangeSignatureVerifier::Verify(header, certificate,
verification_time));
EXPECT_EQ(SignedExchangeSignatureVerifier::Result::kErrInvalidTimestamp,
SignedExchangeSignatureVerifier::Verify(
header, certificate,
base::Time::UnixEpoch() +
base::TimeDelta::FromSeconds(kSignatureHeaderDate - 1)));
EXPECT_EQ(SignedExchangeSignatureVerifier::Result::kSuccess,
SignedExchangeSignatureVerifier::Verify(
header, certificate,
base::Time::UnixEpoch() +
base::TimeDelta::FromSeconds(kSignatureHeaderExpires)));
EXPECT_EQ(SignedExchangeSignatureVerifier::Result::kErrInvalidTimestamp,
SignedExchangeSignatureVerifier::Verify(
header, certificate,
base::Time::UnixEpoch() +
base::TimeDelta::FromSeconds(kSignatureHeaderExpires + 1)));
SignedExchangeHeader invalid_expires_header(header);
auto invalid_expires_signature = SignedExchangeHeaderParser::ParseSignature(
kSignatureHeaderInvalidExpires);
ASSERT_TRUE(invalid_expires_signature.has_value());
ASSERT_EQ(1u, invalid_expires_signature->size());
invalid_expires_header.SetSignatureForTesting(
(*invalid_expires_signature)[0]);
EXPECT_EQ(SignedExchangeSignatureVerifier::Result::kErrInvalidTimestamp,
SignedExchangeSignatureVerifier::Verify(
invalid_expires_header, certificate, verification_time));
SignedExchangeHeader corrupted_header(header); SignedExchangeHeader corrupted_header(header);
corrupted_header.set_request_url(GURL("https://example.com/bad.html")); corrupted_header.set_request_url(GURL("https://example.com/bad.html"));
EXPECT_EQ( EXPECT_EQ(
SignedExchangeSignatureVerifier::Result::kErrSignatureVerificationFailed, SignedExchangeSignatureVerifier::Result::kErrSignatureVerificationFailed,
SignedExchangeSignatureVerifier::Verify(corrupted_header, certificate)); SignedExchangeSignatureVerifier::Verify(corrupted_header, certificate,
verification_time));
SignedExchangeHeader badsig_header(header); SignedExchangeHeader badsig_header(header);
SignedExchangeHeaderParser::Signature badsig = header.signature(); SignedExchangeHeaderParser::Signature badsig = header.signature();
...@@ -136,7 +190,8 @@ TEST(SignedExchangeSignatureVerifier, Verify) { ...@@ -136,7 +190,8 @@ TEST(SignedExchangeSignatureVerifier, Verify) {
badsig_header.SetSignatureForTesting(badsig); badsig_header.SetSignatureForTesting(badsig);
EXPECT_EQ( EXPECT_EQ(
SignedExchangeSignatureVerifier::Result::kErrSignatureVerificationFailed, SignedExchangeSignatureVerifier::Result::kErrSignatureVerificationFailed,
SignedExchangeSignatureVerifier::Verify(badsig_header, certificate)); SignedExchangeSignatureVerifier::Verify(badsig_header, certificate,
verification_time));
SignedExchangeHeader badsigsha256_header(header); SignedExchangeHeader badsigsha256_header(header);
SignedExchangeHeaderParser::Signature badsigsha256 = header.signature(); SignedExchangeHeaderParser::Signature badsigsha256 = header.signature();
...@@ -144,8 +199,8 @@ TEST(SignedExchangeSignatureVerifier, Verify) { ...@@ -144,8 +199,8 @@ TEST(SignedExchangeSignatureVerifier, Verify) {
badsigsha256_header.SetSignatureForTesting(badsigsha256); badsigsha256_header.SetSignatureForTesting(badsigsha256);
EXPECT_EQ( EXPECT_EQ(
SignedExchangeSignatureVerifier::Result::kErrCertificateSHA256Mismatch, SignedExchangeSignatureVerifier::Result::kErrCertificateSHA256Mismatch,
SignedExchangeSignatureVerifier::Verify(badsigsha256_header, SignedExchangeSignatureVerifier::Verify(badsigsha256_header, certificate,
certificate)); verification_time));
} }
} // namespace } // namespace
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/threading/thread_restrictions.h" #include "base/threading/thread_restrictions.h"
#include "base/time/time.h"
#include "content/browser/web_package/signed_exchange_handler.h" #include "content/browser/web_package/signed_exchange_handler.h"
#include "content/public/browser/navigation_controller.h" #include "content/public/browser/navigation_controller.h"
#include "content/public/browser/navigation_entry.h" #include "content/public/browser/navigation_entry.h"
...@@ -35,6 +36,8 @@ namespace content { ...@@ -35,6 +36,8 @@ namespace content {
namespace { namespace {
const uint64_t kSignatureHeaderDate = 1520834000;
const char* kMockHeaderFileSuffix = ".mock-http-headers"; const char* kMockHeaderFileSuffix = ".mock-http-headers";
class NavigationFailureObserver : public WebContentsObserver { class NavigationFailureObserver : public WebContentsObserver {
...@@ -75,6 +78,10 @@ class WebPackageRequestHandlerBrowserTest ...@@ -75,6 +78,10 @@ class WebPackageRequestHandlerBrowserTest
void SetUp() override { void SetUp() override {
SignedExchangeHandler::SetCertVerifierForTesting(mock_cert_verifier_.get()); SignedExchangeHandler::SetCertVerifierForTesting(mock_cert_verifier_.get());
SignedExchangeHandler::SetVerificationTimeForTesting(
base::Time::UnixEpoch() +
base::TimeDelta::FromSeconds(kSignatureHeaderDate));
if (is_network_service_enabled()) { if (is_network_service_enabled()) {
feature_list_.InitWithFeatures( feature_list_.InitWithFeatures(
{features::kSignedHTTPExchange, network::features::kNetworkService}, {features::kSignedHTTPExchange, network::features::kNetworkService},
...@@ -88,6 +95,8 @@ class WebPackageRequestHandlerBrowserTest ...@@ -88,6 +95,8 @@ class WebPackageRequestHandlerBrowserTest
void TearDownOnMainThread() override { void TearDownOnMainThread() override {
interceptor_.reset(); interceptor_.reset();
SignedExchangeHandler::SetCertVerifierForTesting(nullptr); SignedExchangeHandler::SetCertVerifierForTesting(nullptr);
SignedExchangeHandler::SetVerificationTimeForTesting(
base::Optional<base::Time>());
} }
protected: protected:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment