Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(42)

Unified Diff: extensions/browser/sandboxed_unpacker.cc

Issue 2874503002: Refactor CRX verification in preparation to support CRX₃ files. (Closed)
Patch Set: through #44 Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: extensions/browser/sandboxed_unpacker.cc
diff --git a/extensions/browser/sandboxed_unpacker.cc b/extensions/browser/sandboxed_unpacker.cc
index 730ac6b1d018f55ad7c3c14ddb7da59d72499427..000ac706075db4fbe48e59b1099fd1808b0911ed 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>
Devlin 2017/05/16 22:55:02 memory's in the header, so no need for it here.
waffles 2017/05/17 17:18:22 Done.
#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;
+ 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());

Powered by Google App Engine
This is Rietveld 408576698