Check hashes for extensions without verified contents

Extensions outside Chrome Web Store (self-hosted ones) don't have
hashes signed by Chrome Web Store (stored in verified_contents.json
file), so historically are not checked.

This commit adds support for extensions to be "should check, but
shouldn't verify": if such an extension has computed_hashes.json file,
we'll use it to check extension's content and report appropriately to
delegate.

Bug: 958794
Change-Id: Id8bce11fa42e5a8099f65d3013cb25f2271f51c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1735454
Commit-Queue: Oleg Davydov <burunduk@chromium.org>
Reviewed-by: Nikita Podguzov <nikitapodguzov@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712724}

extensions/browser/content_verifier.cc

@@ -82,6 +82,11 @@ bool HasPageFileExt(const base::FilePath& requested_path) {
 std::unique_ptr<ContentVerifierIOData::ExtensionData> CreateIOData(
     const Extension* extension,
     ContentVerifierDelegate* delegate) {
+  ContentVerifierDelegate::VerifierSourceType source_type =
+      delegate->GetVerifierSourceType(*extension);
+  if (source_type == ContentVerifierDelegate::VerifierSourceType::NONE)
+    return nullptr;
+
   // The browser image paths from the extension may not be relative (eg
   // they might have leading '/' or './'), so we strip those to make
   // comparing to actual relative paths work later on.
@@ -115,7 +120,7 @@ std::unique_ptr<ContentVerifierIOData::ExtensionData> CreateIOData(
 
   return std::make_unique<ContentVerifierIOData::ExtensionData>(
       std::move(image_paths), std::move(background_or_content_paths),
-      extension->version());
+      extension->version(), source_type);
 }
 
 }  // namespace
@@ -180,6 +185,7 @@ class ContentVerifier::HashHelper {
   // Must be called on IO thread. The method responds through |callback| on IO
   // thread.
   void GetContentHash(ContentHash::FetchKey fetch_key,
+                      ContentVerifierDelegate::VerifierSourceType source_type,
                       bool force_missing_computed_hashes_creation,
                       ContentHashCallback callback) {
     DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
@@ -204,6 +210,7 @@ class ContentVerifier::HashHelper {
         FROM_HERE,
         base::BindOnce(
             &HashHelper::ReadHashOnFileTaskRunner, std::move(fetch_key),
+            source_type,
             base::BindRepeating(&IsCancelledChecker::IsCancelled, checker),
             base::BindOnce(&HashHelper::DidReadHash, weak_factory_.GetWeakPtr(),
                            callback_key, checker)));
@@ -286,12 +293,11 @@ class ContentVerifier::HashHelper {
 
   static void ReadHashOnFileTaskRunner(
       ContentHash::FetchKey fetch_key,
+      ContentVerifierDelegate::VerifierSourceType source_type,
       const IsCancelledCallback& is_cancelled,
       ContentHash::CreatedCallback created_callback) {
     ContentHash::Create(
-        std::move(fetch_key),
-        ContentVerifierDelegate::VerifierSourceType::SIGNED_HASHES,
-        is_cancelled,
+        std::move(fetch_key), source_type, is_cancelled,
         base::BindOnce(&HashHelper::ForwardToIO, std::move(created_callback)));
   }
 
@@ -483,12 +489,16 @@ void ContentVerifier::GetContentHash(
     return;
   }
 
+  const ContentVerifierIOData::ExtensionData* data =
+      io_data_.GetData(extension_id);
+  DCHECK(data);
   ContentHash::FetchKey fetch_key =
       GetFetchKey(extension_id, extension_root, extension_version);
   // Since |shutdown_on_io_| = false, GetOrCreateHashHelper() must return
   // non-nullptr instance of HashHelper.
   GetOrCreateHashHelper()->GetContentHash(
-      std::move(fetch_key), force_missing_computed_hashes_creation,
+      std::move(fetch_key), data->source_type,
+      force_missing_computed_hashes_creation,
       base::BindOnce(&ContentVerifier::DidGetContentHash, this, cache_key,
                      std::move(callback)));
 }
@@ -516,13 +526,13 @@ void ContentVerifier::OnExtensionLoaded(
   if (shutdown_on_ui_)
     return;
 
-  if (delegate_->GetVerifierSourceType(*extension) ==
-      ContentVerifierDelegate::VerifierSourceType::SIGNED_HASHES) {
-    base::PostTask(
-        FROM_HERE, {content::BrowserThread::IO},
-        base::BindOnce(&ContentVerifier::OnExtensionLoadedOnIO, this,
-                       extension->id(), extension->path(), extension->version(),
-                       CreateIOData(extension, delegate_.get())));
+  std::unique_ptr<ContentVerifierIOData::ExtensionData> io_data =
+      CreateIOData(extension, delegate_.get());
+  if (io_data) {
+    base::PostTask(FROM_HERE, {content::BrowserThread::IO},
+                   base::BindOnce(&ContentVerifier::OnExtensionLoadedOnIO, this,
+                                  extension->id(), extension->path(),
+                                  extension->version(), std::move(io_data)));
   }
 }
 
@@ -612,6 +622,16 @@ ContentHash::FetchKey ContentVerifier::GetFetchKey(
     const base::Version& extension_version) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
+  const ContentVerifierIOData::ExtensionData* data =
+      io_data_.GetData(extension_id);
+  DCHECK(data);
+  if (data->source_type ==
+      ContentVerifierDelegate::VerifierSourceType::UNSIGNED_HASHES) {
+    return ContentHash::FetchKey(extension_id, extension_root,
+                                 extension_version, mojo::NullRemote(),
+                                 GURL::EmptyGURL(), ContentVerifierKey());
+  }
+
   // Create a new mojo pipe. It's safe to pass this around and use immediately,
   // even though it needs to finish initialization on the UI thread.
   mojo::PendingRemote<network::mojom::URLLoaderFactory>

extensions/browser/content_verifier/content_hash.h

@@ -150,24 +150,50 @@ class ContentHash : public base::RefCountedThreadSafe<ContentHash> {
  private:
   friend class base::RefCountedThreadSafe<ContentHash>;
 
+  using GetVerifiedContentsCallback = base::OnceCallback<void(
+      FetchKey key,
+      std::unique_ptr<VerifiedContents> verified_contents,
+      bool did_attempt_fetch)>;
+
   ContentHash(const ExtensionId& id,
               const base::FilePath& root,
+              ContentVerifierDelegate::VerifierSourceType source_type,
               std::unique_ptr<VerifiedContents> verified_contents,
               std::unique_ptr<ComputedHashes::Reader> computed_hashes);
   ~ContentHash();
 
+  // Step 1/2: verified_contents.json.
+  static void GetVerifiedContents(
+      FetchKey key,
+      ContentVerifierDelegate::VerifierSourceType source_type,
+      const IsCancelledCallback& is_cancelled,
+      GetVerifiedContentsCallback);
   static void FetchVerifiedContents(FetchKey key,
                                     const IsCancelledCallback& is_cancelled,
-                                    CreatedCallback created_callback);
+                                    GetVerifiedContentsCallback callback);
+  static std::unique_ptr<VerifiedContents> StoreAndRetrieveVerifiedContents(
+      std::unique_ptr<std::string> fetched_contents,
+      const FetchKey& key);
   static void DidFetchVerifiedContents(
-      CreatedCallback created_callback,
-      const IsCancelledCallback& is_cancelled,
+      GetVerifiedContentsCallback callback,
       FetchKey key,
       std::unique_ptr<std::string> fetched_contents);
 
-  static void DispatchFetchFailure(FetchKey key,
-                                   CreatedCallback created_callback,
-                                   const IsCancelledCallback& is_cancelled);
+  // Step 2/2: computed_hashes.json.
+  static void GetComputedHashes(
+      ContentVerifierDelegate::VerifierSourceType source_type,
+      const IsCancelledCallback& is_cancelled,
+      CreatedCallback created_callback,
+      FetchKey key,
+      std::unique_ptr<VerifiedContents> verified_contents,
+      bool did_attempt_fetch);
+
+  static void DispatchFetchFailure(
+      const ExtensionId& extension_id,
+      const base::FilePath& extension_root,
+      ContentVerifierDelegate::VerifierSourceType source_type,
+      CreatedCallback created_callback,
+      const IsCancelledCallback& is_cancelled);
 
   static void RecordFetchResult(bool success);
 
@@ -183,14 +209,24 @@ class ContentHash : public base::RefCountedThreadSafe<ContentHash> {
   bool CreateHashes(const base::FilePath& hashes_file,
                     const IsCancelledCallback& is_cancelled);
 
+  // Builds hashes for one resource and checks them against
+  // verified_contents.json if needed. Returns nullopt if nothing should be
+  // added to computed_hashes.json for this resource.
+  base::Optional<std::vector<std::string>> ComputeAndCheckResourceHash(
+      const base::FilePath& full_path,
+      const base::FilePath& relative_unix_path);
+
   // Builds computed_hashes. Possibly after creating computed_hashes.json file
   // if necessary.
-  void BuildComputedHashes(bool attempted_fetching_verified_contents,
+  void BuildComputedHashes(bool did_fetch_verified_contents,
                            bool force_build,
                            const IsCancelledCallback& is_cancelled);
 
+  bool has_verified_contents() const { return verified_contents_ != nullptr; }
+
   const ExtensionId extension_id_;
   const base::FilePath extension_root_;
+  ContentVerifierDelegate::VerifierSourceType source_type_;
 
   bool succeeded_ = false;
 

extensions/browser/content_verifier/content_hash_unittest.cc

@@ -231,4 +231,41 @@ TEST_F(ContentHashUnittest, ExtensionWithSignedHashes) {
   EXPECT_TRUE(result->success);
 }
 
+TEST_F(ContentHashUnittest, ExtensionWithUnsignedHashes) {
+  TestExtensionBuilder builder;
+  builder.WriteManifest();
+  builder.WriteResource(FILE_PATH_LITERAL("background.js"),
+                        "console.log('Nothing special');");
+  builder.WriteComputedHashes();
+
+  scoped_refptr<Extension> extension = LoadExtension(builder);
+  ASSERT_NE(nullptr, extension);
+
+  std::unique_ptr<ContentHashResult> result = CreateContentHash(
+      extension.get(),
+      ContentVerifierDelegate::VerifierSourceType::UNSIGNED_HASHES,
+      builder.GetTestContentVerifierPublicKey());
+  DCHECK(result);
+
+  EXPECT_TRUE(result->success);
+}
+
+TEST_F(ContentHashUnittest, ExtensionWithoutHashes) {
+  TestExtensionBuilder builder;
+  builder.WriteManifest();
+  builder.WriteResource(FILE_PATH_LITERAL("background.js"),
+                        "console.log('Nothing special');");
+
+  scoped_refptr<Extension> extension = LoadExtension(builder);
+  ASSERT_NE(nullptr, extension);
+
+  std::unique_ptr<ContentHashResult> result = CreateContentHash(
+      extension.get(),
+      ContentVerifierDelegate::VerifierSourceType::UNSIGNED_HASHES,
+      builder.GetTestContentVerifierPublicKey());
+  DCHECK(result);
+
+  EXPECT_FALSE(result->success);
+}
+
 }  // namespace extensions

extensions/browser/content_verifier/test_utils.cc

@@ -136,10 +136,11 @@ MockContentVerifierDelegate::~MockContentVerifierDelegate() = default;
 
 ContentVerifierDelegate::VerifierSourceType
 MockContentVerifierDelegate::GetVerifierSourceType(const Extension& extension) {
-  return VerifierSourceType::SIGNED_HASHES;
+  return verifier_source_type_;
 }
 
 ContentVerifierKey MockContentVerifierDelegate::GetPublicKey() {
+  DCHECK_EQ(VerifierSourceType::SIGNED_HASHES, verifier_source_type_);
   return ContentVerifierKey(kWebstoreSignaturesPublicKey,
                             kWebstoreSignaturesPublicKeySize);
 }
@@ -147,6 +148,7 @@ ContentVerifierKey MockContentVerifierDelegate::GetPublicKey() {
 GURL MockContentVerifierDelegate::GetSignatureFetchUrl(
     const ExtensionId& extension_id,
     const base::Version& version) {
+  DCHECK_EQ(VerifierSourceType::SIGNED_HASHES, verifier_source_type_);
   std::string url =
       base::StringPrintf("http://localhost/getsignature?id=%s&version=%s",
                          extension_id.c_str(), version.GetString().c_str());
@@ -155,7 +157,6 @@ GURL MockContentVerifierDelegate::GetSignatureFetchUrl(
 
 std::set<base::FilePath> MockContentVerifierDelegate::GetBrowserImagePaths(
     const extensions::Extension* extension) {
-  ADD_FAILURE() << "Unexpected call for this test";
   return std::set<base::FilePath>();
 }
 
@@ -167,6 +168,11 @@ void MockContentVerifierDelegate::VerifyFailed(
 
 void MockContentVerifierDelegate::Shutdown() {}
 
+void MockContentVerifierDelegate::SetVerifierSourceType(
+    VerifierSourceType type) {
+  verifier_source_type_ = type;
+}
+
 // VerifierObserver -----------------------------------------------------------
 VerifierObserver::VerifierObserver() {
   EXPECT_TRUE(

extensions/browser/content_verifier/test_utils.h

@@ -123,7 +123,12 @@ class MockContentVerifierDelegate : public ContentVerifierDelegate {
                     ContentVerifyJob::FailureReason reason) override;
   void Shutdown() override;
 
+  // Modifier.
+  void SetVerifierSourceType(VerifierSourceType type);
+
  private:
+  VerifierSourceType verifier_source_type_ = VerifierSourceType::SIGNED_HASHES;
+
   DISALLOW_COPY_AND_ASSIGN(MockContentVerifierDelegate);
 };
 

extensions/browser/content_verifier_io_data.cc

@@ -13,10 +13,12 @@ namespace extensions {
 ContentVerifierIOData::ExtensionData::ExtensionData(
     std::unique_ptr<std::set<base::FilePath>> browser_image_paths,
     std::unique_ptr<std::set<base::FilePath>> background_or_content_paths,
-    const base::Version& version)
+    const base::Version& version,
+    ContentVerifierDelegate::VerifierSourceType source_type)
     : browser_image_paths(std::move(browser_image_paths)),
       background_or_content_paths(std::move(background_or_content_paths)),
-      version(version) {}
+      version(version),
+      source_type(source_type) {}
 
 ContentVerifierIOData::ContentVerifierIOData() {
 }

extensions/browser/content_verifier_io_data.h

@@ -13,6 +13,7 @@
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/version.h"
+#include "extensions/browser/content_verifier_delegate.h"
 
 namespace extensions {
 
@@ -26,11 +27,13 @@ class ContentVerifierIOData {
     // Set of file paths used as background scripts, pages or content scripts.
     std::unique_ptr<std::set<base::FilePath>> background_or_content_paths;
     base::Version version;
+    ContentVerifierDelegate::VerifierSourceType source_type;
 
     ExtensionData(
         std::unique_ptr<std::set<base::FilePath>> browser_image_paths,
         std::unique_ptr<std::set<base::FilePath>> background_or_content_paths,
-        const base::Version& version);
+        const base::Version& version,
+        ContentVerifierDelegate::VerifierSourceType source_type);
     ~ExtensionData();
   };
 

extensions/test/data/content_hash_fetcher/with_verified_contents_corrupted/README.txt

+This archive contains:
+  _metadata/verified_contents.json with a valid signature (see notes about
+with_verified_contents archive, it's the same) and hash of one file,
+background.js.
+  background.js, which contents differs from the original one
+  _metadata/computed_hashes.json, which contains hash of modified background.js
+(so this hash effectively differs from one stored in verified_contents.json).
+
+The purpose of this archive is to check that content verifier will catch such
+type of corruption too.