Keep SSLInfo for SXG perfetching

Currently SSLInfo is removed by SignedExchangeLoader::OnHTTPExchangeFound() when
kURLLoadOptionSendSSLInfoWithResponse flag is not set. So this CL changes
SignedExchangePrefetchHandler to set the flag to store the SSLInfo in
PrefetchedSignedExchangeCache. And changes InnerResponseURLLoader to clear the
SSLInfo if the request is not for main frame main resource and
report_raw_headers flag is not set.

The case of issue 998359 should be covered by
SignedExchangeRequestHandlerBrowserTest.Simple/3. But this test was not executed
as expected. The cached SXG in PrefetchedSignedExchangeCache was not correctly
used for the next navigation. So this CL changes the test to use JS to trigger
the navigation from renderer, and to use the overridden verification_time in
PrefetchedSignedExchangeCache.

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

content/browser/web_package/prefetched_signed_exchange_cache.cc

@@ -141,6 +141,14 @@ class InnerResponseURLLoader : public network::mojom::URLLoader {
     DCHECK(response_.headers);
     DCHECK(request.request_initiator);
 
+    // Keep the SSLInfo only when the request is for main frame main resource,
+    // or report_raw_headers is set. Users can inspect the certificate for the
+    // main frame using the info bubble in Omnibox, and for the subresources in
+    // DevTools' Security panel.
+    if ((request.resource_type != static_cast<int>(ResourceType::kMainFrame)) &&
+        !request.report_raw_headers) {
+      response_.ssl_info = base::nullopt;
+    }
     UpdateRequestResponseStartTime(&response_);
     response_.encoded_data_length = 0;
     if (is_navigation_request) {
@@ -481,8 +489,8 @@ bool CanStoreEntry(const PrefetchedSignedExchangeCache::Entry& entry) {
 }
 
 bool CanUseEntry(const PrefetchedSignedExchangeCache::Entry& entry,
-                 const base::Time& now) {
-  if (entry.signature_expire_time() < now)
+                 const base::Time& verification_time) {
+  if (entry.signature_expire_time() < verification_time)
     return false;
 
   const std::unique_ptr<const network::ResourceResponseHead>& outer_response =
@@ -490,15 +498,16 @@ bool CanUseEntry(const PrefetchedSignedExchangeCache::Entry& entry,
 
   // Use the prefetched entry within kPrefetchReuseMins minutes without
   // validation.
-  if (outer_response->headers->GetCurrentAge(
-          outer_response->request_time, outer_response->response_time, now) <
+  if (outer_response->headers->GetCurrentAge(outer_response->request_time,
+                                             outer_response->response_time,
+                                             verification_time) <
       base::TimeDelta::FromMinutes(net::HttpCache::kPrefetchReuseMins)) {
     return true;
   }
   // We use the prefetched entry when we don't need the validation.
   if (outer_response->headers->RequiresValidation(
-          outer_response->request_time, outer_response->response_time, now) !=
-      net::VALIDATION_NONE) {
+          outer_response->request_time, outer_response->response_time,
+          verification_time) != net::VALIDATION_NONE) {
     return false;
   }
   return true;
@@ -643,6 +652,8 @@ void PrefetchedSignedExchangeCache::Store(
     return;
   const GURL outer_url = cached_exchange->outer_url();
   exchanges_[outer_url] = std::move(cached_exchange);
+  for (TestObserver& observer : test_observers_)
+    observer.OnStored(this, outer_url);
 }
 
 void PrefetchedSignedExchangeCache::Clear() {
@@ -655,14 +666,16 @@ PrefetchedSignedExchangeCache::MaybeCreateInterceptor(const GURL& outer_url) {
   const auto it = exchanges_.find(outer_url);
   if (it == exchanges_.end())
     return nullptr;
-  const base::Time now = base::Time::Now();
+  const base::Time verification_time =
+      signed_exchange_utils::GetVerificationTime();
   const std::unique_ptr<const Entry>& exchange = it->second;
-  if (!CanUseEntry(*exchange.get(), now)) {
+  if (!CanUseEntry(*exchange.get(), verification_time)) {
     exchanges_.erase(it);
     return nullptr;
   }
   return std::make_unique<PrefetchedNavigationLoaderInterceptor>(
-      exchange->Clone(), GetInfoListForNavigation(*exchange, now));
+      exchange->Clone(),
+      GetInfoListForNavigation(*exchange, verification_time));
 }
 
 const PrefetchedSignedExchangeCache::EntryMap&
@@ -707,7 +720,7 @@ void PrefetchedSignedExchangeCache::RecordHistograms() {
 std::vector<mojom::PrefetchedSignedExchangeInfoPtr>
 PrefetchedSignedExchangeCache::GetInfoListForNavigation(
     const Entry& main_exchange,
-    const base::Time& now) {
+    const base::Time& verification_time) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   const url::Origin outer_url_origin =
@@ -720,7 +733,7 @@ PrefetchedSignedExchangeCache::GetInfoListForNavigation(
   EntryMap::iterator exchanges_it = exchanges_.begin();
   while (exchanges_it != exchanges_.end()) {
     const std::unique_ptr<const Entry>& exchange = exchanges_it->second;
-    if (!CanUseEntry(*exchange.get(), now)) {
+    if (!CanUseEntry(*exchange.get(), verification_time)) {
       exchanges_.erase(exchanges_it++);
       continue;
     }
@@ -749,4 +762,14 @@ PrefetchedSignedExchangeCache::GetInfoListForNavigation(
   return info_list;
 }
 
+void PrefetchedSignedExchangeCache::AddObserverForTesting(
+    TestObserver* observer) {
+  test_observers_.AddObserver(observer);
+}
+
+void PrefetchedSignedExchangeCache::RemoveObserverForTesting(
+    const TestObserver* observer) {
+  test_observers_.RemoveObserver(observer);
+}
+
 }  // namespace content

content/browser/web_package/prefetched_signed_exchange_cache.h

@@ -8,6 +8,7 @@
 #include <map>
 
 #include "base/memory/ref_counted.h"
+#include "base/observer_list.h"
 #include "base/time/time.h"
 #include "content/common/content_export.h"
 #include "content/common/prefetched_signed_exchange_info.mojom.h"
@@ -99,6 +100,13 @@ class CONTENT_EXPORT PrefetchedSignedExchangeCache
     DISALLOW_COPY_AND_ASSIGN(Entry);
   };
 
+  // A test observer to monitor the cache entry.
+  class TestObserver : public base::CheckedObserver {
+   public:
+    virtual void OnStored(PrefetchedSignedExchangeCache* cache,
+                          const GURL& outer_url) = 0;
+  };
+
   using EntryMap = std::map<GURL /* outer_url */, std::unique_ptr<const Entry>>;
 
   PrefetchedSignedExchangeCache();
@@ -114,6 +122,10 @@ class CONTENT_EXPORT PrefetchedSignedExchangeCache
 
   void RecordHistograms();
 
+  // Adds/removes test observers.
+  void AddObserverForTesting(TestObserver* observer);
+  void RemoveObserverForTesting(const TestObserver* observer);
+
  private:
   friend class base::RefCountedThreadSafe<PrefetchedSignedExchangeCache>;
 
@@ -130,6 +142,8 @@ class CONTENT_EXPORT PrefetchedSignedExchangeCache
 
   EntryMap exchanges_;
 
+  base::ObserverList<TestObserver> test_observers_;
+
   DISALLOW_COPY_AND_ASSIGN(PrefetchedSignedExchangeCache);
 };
 

content/browser/web_package/signed_exchange_handler.cc

@@ -74,14 +74,6 @@ constexpr char kSXGWithoutNoSniffErrorMessage[] =
 
 network::mojom::NetworkContext* g_network_context_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();
-}
-
 bool IsSupportedSignedExchangeVersion(
     const base::Optional<SignedExchangeVersion>& version) {
   return version == SignedExchangeVersion::kB3;
@@ -166,12 +158,6 @@ void SignedExchangeHandler::SetNetworkContextForTesting(
   g_network_context_for_testing = network_context;
 }
 
-// static
-void SignedExchangeHandler::SetVerificationTimeForTesting(
-    base::Optional<base::Time> verification_time_for_testing) {
-  g_verification_time_for_testing = verification_time_for_testing;
-}
-
 SignedExchangeHandler::SignedExchangeHandler(
     bool is_secure_transport,
     bool has_nosniff,
@@ -494,7 +480,7 @@ void SignedExchangeHandler::OnCertReceived(
   const SignedExchangeSignatureVerifier::Result verify_result =
       SignedExchangeSignatureVerifier::Verify(
           *version_, *envelope_, unverified_cert_chain_.get(),
-          GetVerificationTime(), devtools_proxy_.get());
+          signed_exchange_utils::GetVerificationTime(), devtools_proxy_.get());
   UMA_HISTOGRAM_ENUMERATION(kHistogramSignatureVerificationResult,
                             verify_result);
   if (verify_result != SignedExchangeSignatureVerifier::Result::kSuccess) {

content/browser/web_package/signed_exchange_handler.h

@@ -81,9 +81,6 @@ class CONTENT_EXPORT SignedExchangeHandler {
   static void SetNetworkContextForTesting(
       network::mojom::NetworkContext* network_context);
 
-  static void SetVerificationTimeForTesting(
-      base::Optional<base::Time> verification_time_for_testing);
-
   // 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
   // from |payload_stream| passed to |headers_callback|. |cert_fetcher_factory|

content/browser/web_package/signed_exchange_handler_unittest.cc

@@ -17,6 +17,7 @@
 #include "content/browser/web_package/signed_exchange_devtools_proxy.h"
 #include "content/browser/web_package/signed_exchange_signature_verifier.h"
 #include "content/browser/web_package/signed_exchange_test_utils.h"
+#include "content/browser/web_package/signed_exchange_utils.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/content_paths.h"
@@ -183,7 +184,7 @@ class SignedExchangeHandlerTest
 
   void SetUp() override {
     original_client_ = SetBrowserClientForTesting(&browser_client_);
-    SignedExchangeHandler::SetVerificationTimeForTesting(
+    signed_exchange_utils::SetVerificationTimeForTesting(
         base::Time::UnixEpoch() +
         base::TimeDelta::FromSeconds(kSignatureHeaderDate));
     feature_list_.InitAndEnableFeature(features::kSignedHTTPExchange);
@@ -210,7 +211,7 @@ class SignedExchangeHandlerTest
     }
     SignedExchangeHandler::SetNetworkContextForTesting(nullptr);
     network::NetworkContext::SetCertVerifierForTesting(nullptr);
-    SignedExchangeHandler::SetVerificationTimeForTesting(
+    signed_exchange_utils::SetVerificationTimeForTesting(
         base::Optional<base::Time>());
     SetBrowserClientForTesting(original_client_);
   }
@@ -588,7 +589,7 @@ TEST_P(SignedExchangeHandlerTest,
        CertValidMoreThan90DaysShouldBeAllowedByIgnoreErrorsSPKIListFlag) {
   SetIgnoreCertificateErrorsSPKIList(kPEMECDSAP256SPKIHash);
 
-  SignedExchangeHandler::SetVerificationTimeForTesting(
+  signed_exchange_utils::SetVerificationTimeForTesting(
       base::Time::UnixEpoch() +
       base::TimeDelta::FromSeconds(kCertValidityPeriodEnforcementDate));
   mock_cert_fetcher_factory_->ExpectFetch(

content/browser/web_package/signed_exchange_prefetch_handler.cc

@@ -39,10 +39,16 @@ SignedExchangePrefetchHandler::SignedExchangePrefetchHandler(
   loader_client_binding_.Bind(mojo::MakeRequest(&client));
   scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory =
       std::move(network_loader_factory);
+
+  // We need the SSLInfo as the prefetched signed exchange may be used as a main
+  // frame main resource. Users can inspect the certificate for the main frame
+  // using the info bubble in Omnibox.
+  const uint32_t url_loader_options =
+      network::mojom::kURLLoadOptionSendSSLInfoWithResponse;
+
   signed_exchange_loader_ = std::make_unique<SignedExchangeLoader>(
       resource_request, response_head, std::move(response_body),
-      std::move(client), std::move(endpoints),
-      network::mojom::kURLLoadOptionNone,
+      std::move(client), std::move(endpoints), url_loader_options,
       false /* should_redirect_to_fallback */,
       std::make_unique<SignedExchangeDevToolsProxy>(
           resource_request.url, response_head, frame_tree_node_id_getter,

content/browser/web_package/signed_exchange_utils.cc

@@ -26,6 +26,10 @@
 namespace content {
 namespace signed_exchange_utils {
 
+namespace {
+base::Optional<base::Time> g_verification_time_for_testing;
+}  // namespace
+
 void ReportErrorAndTraceEvent(
     SignedExchangeDevToolsProxy* devtools_proxy,
     const std::string& error_message,
@@ -259,5 +263,16 @@ int MakeRequestID() {
   return --*request_id;
 }
 
+base::Time GetVerificationTime() {
+  if (g_verification_time_for_testing)
+    return *g_verification_time_for_testing;
+  return base::Time::Now();
+}
+
+void SetVerificationTimeForTesting(
+    base::Optional<base::Time> verification_time_for_testing) {
+  g_verification_time_for_testing = verification_time_for_testing;
+}
+
 }  // namespace signed_exchange_utils
 }  // namespace content

content/browser/web_package/signed_exchange_utils.h

@@ -93,6 +93,14 @@ network::ResourceResponseHead CreateRedirectResponseHead(
 // any thread.
 int MakeRequestID();
 
+// Returns the time to be used for verifying signed exchange. Can be overridden
+// using SetVerificationTimeForTesting().
+base::Time GetVerificationTime();
+
+// Override the time which is used for verifying signed exchange.
+CONTENT_EXPORT void SetVerificationTimeForTesting(
+    base::Optional<base::Time> verification_time_for_testing);
+
 }  // namespace signed_exchange_utils
 }  // namespace content
 

content/public/test/signed_exchange_browser_test_helper.cc

@@ -11,7 +11,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/post_task.h"
 #include "base/threading/thread_restrictions.h"
-#include "content/browser/web_package/signed_exchange_handler.h"
+#include "content/browser/web_package/signed_exchange_utils.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/content_features.h"
@@ -34,14 +34,14 @@ SignedExchangeBrowserTestHelper::SignedExchangeBrowserTestHelper() = default;
 SignedExchangeBrowserTestHelper::~SignedExchangeBrowserTestHelper() = default;
 
 void SignedExchangeBrowserTestHelper::SetUp() {
-  SignedExchangeHandler::SetVerificationTimeForTesting(
+  signed_exchange_utils::SetVerificationTimeForTesting(
       base::Time::UnixEpoch() +
       base::TimeDelta::FromSeconds(kSignatureHeaderDate));
 }
 
 void SignedExchangeBrowserTestHelper::TearDownOnMainThread() {
   interceptor_.reset();
-  SignedExchangeHandler::SetVerificationTimeForTesting(
+  signed_exchange_utils::SetVerificationTimeForTesting(
       base::Optional<base::Time>());
 }