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..e0f1169b7b1da669af7203082e2f3d559aa3ffd1 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,6 @@
 
 using base::ASCIIToUTF16;
 using content::BrowserThread;
-using crx_file::CrxFile;
 
 // The following macro makes histograms that record the length of paths
 // in this file much easier to read.
@@ -541,6 +544,10 @@ base::string16 SandboxedUnpacker::FailureReasonToString16(
       return ASCIIToUTF16("CRX_SIGNATURE_VERIFICATION_INITIALIZATION_FAILED");
     case CRX_SIGNATURE_VERIFICATION_FAILED:
       return ASCIIToUTF16("CRX_SIGNATURE_VERIFICATION_FAILED");
+    case CRX_FILE_IS_DELTA_UPDATE:
+      return ASCIIToUTF16("CRX_FILE_IS_DELTA_UPDATE");
+    case CRX_EXPECTED_HASH_INVALID:
+      return ASCIIToUTF16("CRX_EXPECTED_HASH_INVALID");
 
     case ERROR_SERIALIZING_MANIFEST_JSON:
       return ASCIIToUTF16("ERROR_SERIALIZING_MANIFEST_JSON");
@@ -600,51 +607,47 @@ 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);
+  std::vector<uint8_t> hash = {};

[Sorin Jianu, 2017/05/16 20:54:50]

is there any reason to use an initialization list?

[waffles, 2017/05/16 22:34:52]

Done.

+  if (!expected_hash.empty()) {
+    if (!base::HexStringToBytes(expected_hash, &hash)) {
+      FailWithPackageError(CRX_EXPECTED_HASH_INVALID);
+      return false;
+    }
+  }
+  const crx_file::VerifierResult result = crx_file::Verify(
+      crx_path, crx_file::VerifierFormat::CRX2_OR_CRX3,
+      std::vector<std::vector<uint8_t>>(), hash, &public_key_, &extension_id_);
 
-  switch (error) {
-    case CrxFile::ValidateError::NONE: {
+  switch (result) {
+    case crx_file::VerifierResult::OK_FULL: {
       if (!expected_hash.empty())
         UMA_HISTOGRAM_BOOLEAN("Extensions.SandboxUnpackHashCheck", true);
       return true;
     }
-
-    case CrxFile::ValidateError::CRX_FILE_NOT_READABLE:
+    case crx_file::VerifierResult::OK_DELTA:
+      FailWithPackageError(CRX_FILE_IS_DELTA_UPDATE);
+      break;
+    case crx_file::VerifierResult::ERROR_FILE_NOT_READABLE:
       FailWithPackageError(CRX_FILE_NOT_READABLE);
       break;
-    case CrxFile::ValidateError::CRX_HEADER_INVALID:
+    case crx_file::VerifierResult::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 crx_file::VerifierResult::ERROR_SIGNATURE_INITIALIZATION_FAILED:
       FailWithPackageError(CRX_SIGNATURE_VERIFICATION_INITIALIZATION_FAILED);
       break;
-    case CrxFile::ValidateError::CRX_SIGNATURE_VERIFICATION_FAILED:
+    case crx_file::VerifierResult::ERROR_SIGNATURE_VERIFICATION_FAILED:
       FailWithPackageError(CRX_SIGNATURE_VERIFICATION_FAILED);
       break;
-    case CrxFile::ValidateError::CRX_HASH_VERIFICATION_FAILED:
+    case crx_file::VerifierResult::ERROR_EXPECTED_HASH_INVALID:
+      FailWithPackageError(CRX_EXPECTED_HASH_INVALID);
+      break;
+    case crx_file::VerifierResult::ERROR_REQUIRED_PROOF_MISSING:
+      // We should never get this result, as we do not call
+      // verifier.RequireKeyProof.
+      NOTREACHED();
+      break;
+    case crx_file::VerifierResult::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());