Refactor CRX verification in preparation to support CRX₃ files.

extensions/browser/sandboxed_unpacker.cc

Index: extensions/browser/sandboxed_unpacker.cc
diff --git a/extensions/browser/sandboxed_unpacker.cc b/extensions/browser/sandboxed_unpacker.cc
index 730ac6b1d018f55ad7c3c14ddb7da59d72499427..7f6a8ad0ea723e382b186de6a2aea51756d19bad 100644
--- a/extensions/browser/sandboxed_unpacker.cc
+++ b/extensions/browser/sandboxed_unpacker.cc
@@ -7,8 +7,11 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include <memory>
 #include <set>
 #include <tuple>
+#include <utility>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ -17,10 +20,11 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/path_service.h"
 #include "base/sequenced_task_runner.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "build/build_config.h"
-#include "components/crx_file/crx_file.h"
+#include "components/crx_file/crx_verifier.h"
 #include "content/public/browser/browser_thread.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
@@ -39,7 +43,7 @@
 
 using base::ASCIIToUTF16;
 using content::BrowserThread;
-using crx_file::CrxFile;
+using crx_file::CrxVerifier;
 
 // The following macro makes histograms that record the length of paths
 // in this file much easier to read.
@@ -523,24 +527,14 @@ base::string16 SandboxedUnpacker::FailureReasonToString16(
       return ASCIIToUTF16("CRX_FILE_NOT_READABLE");
     case CRX_HEADER_INVALID:
       return ASCIIToUTF16("CRX_HEADER_INVALID");
-    case CRX_MAGIC_NUMBER_INVALID:
-      return ASCIIToUTF16("CRX_MAGIC_NUMBER_INVALID");
-    case CRX_VERSION_NUMBER_INVALID:
-      return ASCIIToUTF16("CRX_VERSION_NUMBER_INVALID");
-    case CRX_EXCESSIVELY_LARGE_KEY_OR_SIGNATURE:
-      return ASCIIToUTF16("CRX_EXCESSIVELY_LARGE_KEY_OR_SIGNATURE");
-    case CRX_ZERO_KEY_LENGTH:
-      return ASCIIToUTF16("CRX_ZERO_KEY_LENGTH");
-    case CRX_ZERO_SIGNATURE_LENGTH:
-      return ASCIIToUTF16("CRX_ZERO_SIGNATURE_LENGTH");
-    case CRX_PUBLIC_KEY_INVALID:
-      return ASCIIToUTF16("CRX_PUBLIC_KEY_INVALID");
-    case CRX_SIGNATURE_INVALID:
-      return ASCIIToUTF16("CRX_SIGNATURE_INVALID");
+    case CRX_FILE_IS_DELTA_UPDATE:
+      return ASCIIToUTF16("CRX_FILE_IS_DELTA_UPDATE");
     case CRX_SIGNATURE_VERIFICATION_INITIALIZATION_FAILED:
       return ASCIIToUTF16("CRX_SIGNATURE_VERIFICATION_INITIALIZATION_FAILED");
     case CRX_SIGNATURE_VERIFICATION_FAILED:
       return ASCIIToUTF16("CRX_SIGNATURE_VERIFICATION_FAILED");
+    case CRX_EXPECTED_HASH_INVALID:
+      return ASCIIToUTF16("CRX_EXPECTED_HASH_INVALID");
 
     case ERROR_SERIALIZING_MANIFEST_JSON:
       return ASCIIToUTF16("ERROR_SERIALIZING_MANIFEST_JSON");
@@ -600,51 +594,49 @@ void SandboxedUnpacker::FailWithPackageError(FailureReason reason) {
 
 bool SandboxedUnpacker::ValidateSignature(const base::FilePath& crx_path,
                                           const std::string& expected_hash) {
-  CrxFile::ValidateError error = CrxFile::ValidateSignature(
-      crx_path, expected_hash, &public_key_, &extension_id_, nullptr);
+  CrxVerifier verifier;
+  verifier.GetPublicKey(&public_key_);
+  verifier.GetCrxId(&extension_id_);
+  if (!expected_hash.empty()) {
+    std::vector<uint8_t> hash;
+    if (!base::HexStringToBytes(expected_hash, &hash)) {
+      FailWithPackageError(CRX_EXPECTED_HASH_INVALID);
+      return false;
+    }
+    verifier.RequireFileHash(hash);
+  }
+  CrxVerifier::Result result = verifier.Verify(crx_path);

[Sorin Jianu, 2017/05/15 19:49:53]

const?

[waffles, 2017/05/16 00:29:03]

Done.

 
-  switch (error) {
-    case CrxFile::ValidateError::NONE: {
+  switch (result) {
+    case CrxVerifier::Result::OK_FULL: {
       if (!expected_hash.empty())
         UMA_HISTOGRAM_BOOLEAN("Extensions.SandboxUnpackHashCheck", true);
       return true;
     }
-
-    case CrxFile::ValidateError::CRX_FILE_NOT_READABLE:
+    case CrxVerifier::Result::OK_DELTA:
+      FailWithPackageError(CRX_FILE_IS_DELTA_UPDATE);
+      break;
+    case CrxVerifier::Result::ERROR_FILE_NOT_READABLE:
       FailWithPackageError(CRX_FILE_NOT_READABLE);
       break;
-    case CrxFile::ValidateError::CRX_HEADER_INVALID:
+    case CrxVerifier::Result::ERROR_HEADER_INVALID:
       FailWithPackageError(CRX_HEADER_INVALID);
       break;
-    case CrxFile::ValidateError::CRX_MAGIC_NUMBER_INVALID:
-      FailWithPackageError(CRX_MAGIC_NUMBER_INVALID);
-      break;
-    case CrxFile::ValidateError::CRX_VERSION_NUMBER_INVALID:
-      FailWithPackageError(CRX_VERSION_NUMBER_INVALID);
-      break;
-    case CrxFile::ValidateError::CRX_EXCESSIVELY_LARGE_KEY_OR_SIGNATURE:
-      FailWithPackageError(CRX_EXCESSIVELY_LARGE_KEY_OR_SIGNATURE);
-      break;
-    case CrxFile::ValidateError::CRX_ZERO_KEY_LENGTH:
-      FailWithPackageError(CRX_ZERO_KEY_LENGTH);
-      break;
-    case CrxFile::ValidateError::CRX_ZERO_SIGNATURE_LENGTH:
-      FailWithPackageError(CRX_ZERO_SIGNATURE_LENGTH);
-      break;
-    case CrxFile::ValidateError::CRX_PUBLIC_KEY_INVALID:
-      FailWithPackageError(CRX_PUBLIC_KEY_INVALID);
-      break;
-    case CrxFile::ValidateError::CRX_SIGNATURE_INVALID:
-      FailWithPackageError(CRX_SIGNATURE_INVALID);
-      break;
-    case CrxFile::ValidateError::
-        CRX_SIGNATURE_VERIFICATION_INITIALIZATION_FAILED:
+    case CrxVerifier::Result::ERROR_SIGNATURE_INITIALIZATION_FAILED:
       FailWithPackageError(CRX_SIGNATURE_VERIFICATION_INITIALIZATION_FAILED);
       break;
-    case CrxFile::ValidateError::CRX_SIGNATURE_VERIFICATION_FAILED:
+    case CrxVerifier::Result::ERROR_SIGNATURE_VERIFICATION_FAILED:
       FailWithPackageError(CRX_SIGNATURE_VERIFICATION_FAILED);
       break;
-    case CrxFile::ValidateError::CRX_HASH_VERIFICATION_FAILED:
+    case CrxVerifier::Result::ERROR_EXPECTED_HASH_INVALID:
+      FailWithPackageError(CRX_EXPECTED_HASH_INVALID);
+      break;
+    case CrxVerifier::Result::ERROR_REQUIRED_PROOF_MISSING:
+      // We should never get this result, as we do not call
+      // verifier.RequireKeyProof.
+      NOTREACHED();
+      break;
+    case CrxVerifier::Result::ERROR_FILE_HASH_FAILED:
       // We should never get this result unless we had specifically asked for
       // verification of the crx file's hash.
       CHECK(!expected_hash.empty());