diff options
Diffstat (limited to 'chromium/components/crx_file')
-rw-r--r-- | chromium/components/crx_file/BUILD.gn | 1 | ||||
-rw-r--r-- | chromium/components/crx_file/OWNERS | 1 | ||||
-rw-r--r-- | chromium/components/crx_file/crx_verifier.cc | 107 | ||||
-rw-r--r-- | chromium/components/crx_file/crx_verifier.h | 8 | ||||
-rw-r--r-- | chromium/components/crx_file/crx_verifier_unittest.cc | 87 |
5 files changed, 104 insertions, 100 deletions
diff --git a/chromium/components/crx_file/BUILD.gn b/chromium/components/crx_file/BUILD.gn index 57667dbba03..4e6eacb23ac 100644 --- a/chromium/components/crx_file/BUILD.gn +++ b/chromium/components/crx_file/BUILD.gn @@ -49,6 +49,7 @@ bundle_data("unit_tests_bundle_data") { "//components/test/data/crx_file/valid.crx2", "//components/test/data/crx_file/valid_no_publisher.crx3", "//components/test/data/crx_file/valid_publisher.crx3", + "//components/test/data/crx_file/valid_test_publisher.crx3", ] outputs = [ "{{bundle_resources_dir}}/" + diff --git a/chromium/components/crx_file/OWNERS b/chromium/components/crx_file/OWNERS index 6fbc7dea785..9562f1b3e2a 100644 --- a/chromium/components/crx_file/OWNERS +++ b/chromium/components/crx_file/OWNERS @@ -2,3 +2,4 @@ file://extensions/OWNERS waffles@chromium.org sorin@chromium.org +# COMPONENT: Platform>Extensions diff --git a/chromium/components/crx_file/crx_verifier.cc b/chromium/components/crx_file/crx_verifier.cc index dbc70b43653..1f523a7f6e6 100644 --- a/chromium/components/crx_file/crx_verifier.cc +++ b/chromium/components/crx_file/crx_verifier.cc @@ -29,21 +29,21 @@ namespace crx_file { namespace { -// The maximum size the Crx2 parser will tolerate for a public key. -constexpr uint32_t kMaxPublicKeySize = 1 << 16; - -// The maximum size the Crx2 parser will tolerate for a signature. -constexpr uint32_t kMaxSignatureSize = 1 << 16; - // The maximum size the Crx3 parser will tolerate for a header. constexpr uint32_t kMaxHeaderSize = 1 << 18; -// The SHA256 hash of the "ecdsa_2017_public" Crx3 key. +// The SHA256 hash of the DER SPKI "ecdsa_2017_public" Crx3 key. constexpr uint8_t kPublisherKeyHash[] = { 0x61, 0xf7, 0xf2, 0xa6, 0xbf, 0xcf, 0x74, 0xcd, 0x0b, 0xc1, 0xfe, 0x24, 0x97, 0xcc, 0x9b, 0x04, 0x25, 0x4c, 0x65, 0x8f, 0x79, 0xf2, 0x14, 0x53, 0x92, 0x86, 0x7e, 0xa8, 0x36, 0x63, 0x67, 0xcf}; +// The SHA256 hash of the DER SPKI "ecdsa_2017_public" Crx3 test key. +constexpr uint8_t kPublisherTestKeyHash[] = { + 0x6c, 0x46, 0x41, 0x3b, 0x00, 0xd0, 0xfa, 0x0e, 0x72, 0xc8, 0xd2, + 0x5f, 0x64, 0xf3, 0xa6, 0x17, 0x03, 0x0d, 0xde, 0x21, 0x61, 0xbe, + 0xb7, 0x95, 0x91, 0x95, 0x83, 0x68, 0x12, 0xe9, 0x78, 0x1e}; + using VerifierCollection = std::vector<std::unique_ptr<crypto::SignatureVerifier>>; using RepeatedProof = google::protobuf::RepeatedPtrField<AsymmetricKeyProof>; @@ -98,7 +98,8 @@ VerifierResult VerifyCrx3( const std::vector<std::vector<uint8_t>>& required_key_hashes, std::string* public_key, std::string* crx_id, - bool require_publisher_key) { + bool require_publisher_key, + bool accept_publisher_test_key) { // Parse [header-size] and [header]. const uint32_t header_size = ReadAndHashLittleEndianUInt32(file, hash); if (header_size > kMaxHeaderSize) @@ -130,10 +131,6 @@ VerifierResult VerifyCrx3( // Create a set of all required key hashes. std::set<std::vector<uint8_t>> required_key_set(required_key_hashes.begin(), required_key_hashes.end()); - if (require_publisher_key) { - required_key_set.emplace(std::begin(kPublisherKeyHash), - std::end(kPublisherKeyHash)); - } using ProofFetcher = const RepeatedProof& (CrxFileHeader::*)() const; ProofFetcher rsa = &CrxFileHeader::sha256_with_rsa; @@ -149,6 +146,15 @@ VerifierResult VerifyCrx3( std::make_pair(rsa, crypto::SignatureVerifier::RSA_PKCS1_SHA256), std::make_pair(ecdsa, crypto::SignatureVerifier::ECDSA_SHA256)}; + std::vector<uint8_t> publisher_key(std::begin(kPublisherKeyHash), + std::end(kPublisherKeyHash)); + base::Optional<std::vector<uint8_t>> publisher_test_key; + if (accept_publisher_test_key) { + publisher_test_key.emplace(std::begin(kPublisherTestKeyHash), + std::end(kPublisherTestKeyHash)); + } + bool found_publisher_key = false; + // Initialize all verifiers and update them with // [prefix][signed-header-size][signed-header]. // Clear any elements of required_key_set that are encountered, and watch for @@ -162,6 +168,10 @@ VerifierResult VerifyCrx3( std::vector<uint8_t> key_hash(crypto::kSHA256Length); crypto::SHA256HashString(key, key_hash.data(), key_hash.size()); required_key_set.erase(key_hash); + DCHECK_EQ(accept_publisher_test_key, publisher_test_key.has_value()); + found_publisher_key = + found_publisher_key || key_hash == publisher_key || + (accept_publisher_test_key && key_hash == *publisher_test_key); auto v = std::make_unique<crypto::SignatureVerifier>(); static_assert(sizeof(unsigned char) == sizeof(uint8_t), "Unsupported char size."); @@ -178,6 +188,9 @@ VerifierResult VerifyCrx3( if (public_key_bytes.empty() || !required_key_set.empty()) return VerifierResult::ERROR_REQUIRED_PROOF_MISSING; + if (require_publisher_key && !found_publisher_key) + return VerifierResult::ERROR_REQUIRED_PROOF_MISSING; + // Update and finalize the verifiers with [archive]. if (!ReadHashAndVerifyArchive(file, hash, verifiers)) return VerifierResult::ERROR_SIGNATURE_VERIFICATION_FAILED; @@ -187,53 +200,6 @@ VerifierResult VerifyCrx3( return VerifierResult::OK_FULL; } -VerifierResult VerifyCrx2( - base::File* file, - crypto::SecureHash* hash, - const std::vector<std::vector<uint8_t>>& required_key_hashes, - std::string* public_key, - std::string* crx_id) { - const uint32_t key_size = ReadAndHashLittleEndianUInt32(file, hash); - if (key_size > kMaxPublicKeySize) - return VerifierResult::ERROR_HEADER_INVALID; - const uint32_t sig_size = ReadAndHashLittleEndianUInt32(file, hash); - if (sig_size > kMaxSignatureSize) - return VerifierResult::ERROR_HEADER_INVALID; - std::vector<uint8_t> key(key_size); - if (ReadAndHashBuffer(key.data(), key_size, file, hash) != - static_cast<int>(key_size)) - return VerifierResult::ERROR_HEADER_INVALID; - for (const auto& expected_hash : required_key_hashes) { - // In practice we expect zero or one key_hashes_ for Crx2 files. - std::vector<uint8_t> hash(crypto::kSHA256Length); - std::unique_ptr<crypto::SecureHash> sha256 = - crypto::SecureHash::Create(crypto::SecureHash::SHA256); - sha256->Update(key.data(), key.size()); - sha256->Finish(hash.data(), hash.size()); - if (hash != expected_hash) - return VerifierResult::ERROR_REQUIRED_PROOF_MISSING; - } - - std::vector<uint8_t> sig(sig_size); - if (ReadAndHashBuffer(sig.data(), sig_size, file, hash) != - static_cast<int>(sig_size)) - return VerifierResult::ERROR_HEADER_INVALID; - std::vector<std::unique_ptr<crypto::SignatureVerifier>> verifiers; - verifiers.push_back(std::make_unique<crypto::SignatureVerifier>()); - if (!verifiers[0]->VerifyInit(crypto::SignatureVerifier::RSA_PKCS1_SHA1, sig, - key)) { - return VerifierResult::ERROR_SIGNATURE_INITIALIZATION_FAILED; - } - - if (!ReadHashAndVerifyArchive(file, hash, verifiers)) - return VerifierResult::ERROR_SIGNATURE_VERIFICATION_FAILED; - - const std::string public_key_bytes(key.begin(), key.end()); - base::Base64Encode(public_key_bytes, public_key); - *crx_id = id_util::GenerateId(public_key_bytes); - return VerifierResult::OK_FULL; -} - } // namespace VerifierResult Verify( @@ -268,20 +234,17 @@ VerifierResult Verify( const uint32_t version = ReadAndHashLittleEndianUInt32(&file, file_hash.get()); VerifierResult result; - if (version == 2) - LOG(WARNING) << "File '" << crx_path - << "' is in CRX2 format, which is deprecated and will not be " - "supported in M78+"; - if (format == VerifierFormat::CRX2_OR_CRX3 && - (version == 2 || (diff && version == 0))) - result = VerifyCrx2(&file, file_hash.get(), required_key_hashes, - &public_key_local, &crx_id_local); - else if (version == 3) - result = VerifyCrx3(&file, file_hash.get(), required_key_hashes, - &public_key_local, &crx_id_local, - format == VerifierFormat::CRX3_WITH_PUBLISHER_PROOF); - else + if (version == 3) { + bool require_publisher_key = + format == VerifierFormat::CRX3_WITH_PUBLISHER_PROOF || + format == VerifierFormat::CRX3_WITH_TEST_PUBLISHER_PROOF; + result = + VerifyCrx3(&file, file_hash.get(), required_key_hashes, + &public_key_local, &crx_id_local, require_publisher_key, + format == VerifierFormat::CRX3_WITH_TEST_PUBLISHER_PROOF); + } else { result = VerifierResult::ERROR_HEADER_INVALID; + } if (result != VerifierResult::OK_FULL) return result; diff --git a/chromium/components/crx_file/crx_verifier.h b/chromium/components/crx_file/crx_verifier.h index 35cf1956a24..a2064428026 100644 --- a/chromium/components/crx_file/crx_verifier.h +++ b/chromium/components/crx_file/crx_verifier.h @@ -16,9 +16,11 @@ class FilePath; namespace crx_file { enum class VerifierFormat { - CRX2_OR_CRX3, // Accept Crx2 or Crx3. - CRX3, // Accept only Crx3. - CRX3_WITH_PUBLISHER_PROOF, // Accept only Crx3 with a publisher proof. + CRX3, // Accept only Crx3. + CRX3_WITH_TEST_PUBLISHER_PROOF, // Accept only Crx3 with a test or production + // publisher proof. + CRX3_WITH_PUBLISHER_PROOF, // Accept only Crx3 with a production + // publisher proof. }; enum class VerifierResult { diff --git a/chromium/components/crx_file/crx_verifier_unittest.cc b/chromium/components/crx_file/crx_verifier_unittest.cc index 54dd44f0cea..70bcd310476 100644 --- a/chromium/components/crx_file/crx_verifier_unittest.cc +++ b/chromium/components/crx_file/crx_verifier_unittest.cc @@ -32,34 +32,32 @@ constexpr char kOjjKey[] = "mCsHBHFWbqtGhSN4YCAw3DYQzwdTcIVaIA8f2Uo4AZ4INKkrEPRL8o9mZDYtO2YHIQg8pMSRMa" "6AawBNYi9tZScnmgl5L1qE6z5oIwIDAQAB"; +constexpr char kJlnHash[] = "jlnmailbicnpnbggmhfebbomaddckncf"; +constexpr char kJlnKey[] = + "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAtYd4M8wBjlPsc/wxS1/uXKMD6GtI7/" + "0GLxRe6UXTYtDk91u/" + "FdJRK9jHws+" + "FO6Yi3XcJGtMvuojiB1j4bdiYBfvRgfTD4b7krWtWM2udKPBtHI9ikAT5aom5Bda8rCPNyaqXC" + "6Ax+KTgQpeeJglYu7TTd/" + "AePyvlRHtCKNkcvRQLY0b6hccALqoTzyTueDX12c8Htg76syEPbz7hSIPPfq6KEGvuVSxWAejy" + "/y6EhwAdXRLpegul9KmL94OY1G6dpycUKwyKeXOcB6Qj5iKNcOqJAaSLxoOZby4G3cI1BcQpp/" + "3vYccJ4qouDMfaanLe8CvFlLp4VOn833aJ8PYpLQIDAQAB"; + } // namespace namespace crx_file { using CrxVerifierTest = testing::Test; -TEST_F(CrxVerifierTest, ValidFullCrx2) { - const std::vector<std::vector<uint8_t>> keys; - const std::vector<uint8_t> hash; - std::string public_key; - std::string crx_id; - - EXPECT_EQ(VerifierResult::OK_FULL, - Verify(TestFile("valid.crx2"), VerifierFormat::CRX2_OR_CRX3, keys, - hash, &public_key, &crx_id)); - EXPECT_EQ(std::string(kOjjHash), crx_id); - EXPECT_EQ(std::string(kOjjKey), public_key); -} - TEST_F(CrxVerifierTest, ValidFullCrx3) { const std::vector<std::vector<uint8_t>> keys; const std::vector<uint8_t> hash; std::string public_key = "UNSET"; std::string crx_id = "UNSET"; - EXPECT_EQ(VerifierResult::OK_FULL, Verify(TestFile("valid_no_publisher.crx3"), - VerifierFormat::CRX2_OR_CRX3, keys, - hash, &public_key, &crx_id)); + EXPECT_EQ(VerifierResult::OK_FULL, + Verify(TestFile("valid_no_publisher.crx3"), VerifierFormat::CRX3, + keys, hash, &public_key, &crx_id)); EXPECT_EQ(std::string(kOjjHash), crx_id); EXPECT_EQ(std::string(kOjjKey), public_key); @@ -94,9 +92,9 @@ TEST_F(CrxVerifierTest, VerifiesFileHash) { std::string public_key = "UNSET"; std::string crx_id = "UNSET"; - EXPECT_EQ(VerifierResult::OK_FULL, Verify(TestFile("valid_no_publisher.crx3"), - VerifierFormat::CRX2_OR_CRX3, keys, - hash, &public_key, &crx_id)); + EXPECT_EQ(VerifierResult::OK_FULL, + Verify(TestFile("valid_no_publisher.crx3"), VerifierFormat::CRX3, + keys, hash, &public_key, &crx_id)); EXPECT_EQ(std::string(kOjjHash), crx_id); EXPECT_EQ(std::string(kOjjKey), public_key); @@ -131,10 +129,9 @@ TEST_F(CrxVerifierTest, ChecksRequiredKeyHashes) { const std::vector<std::vector<uint8_t>> good_keys = {good_key}; std::string public_key = "UNSET"; std::string crx_id = "UNSET"; - EXPECT_EQ( - VerifierResult::OK_FULL, - Verify(TestFile("valid_no_publisher.crx3"), VerifierFormat::CRX2_OR_CRX3, - good_keys, hash, &public_key, &crx_id)); + EXPECT_EQ(VerifierResult::OK_FULL, + Verify(TestFile("valid_no_publisher.crx3"), VerifierFormat::CRX3, + good_keys, hash, &public_key, &crx_id)); EXPECT_EQ(std::string(kOjjHash), crx_id); EXPECT_EQ(std::string(kOjjKey), public_key); @@ -165,6 +162,15 @@ TEST_F(CrxVerifierTest, ChecksPinnedKey) { public_key = "UNSET"; crx_id = "UNSET"; EXPECT_EQ(VerifierResult::ERROR_REQUIRED_PROOF_MISSING, + Verify(TestFile("valid_test_publisher.crx3"), + VerifierFormat::CRX3_WITH_PUBLISHER_PROOF, keys, hash, + &public_key, &crx_id)); + EXPECT_EQ("UNSET", crx_id); + EXPECT_EQ("UNSET", public_key); + + public_key = "UNSET"; + crx_id = "UNSET"; + EXPECT_EQ(VerifierResult::ERROR_REQUIRED_PROOF_MISSING, Verify(TestFile("valid_no_publisher.crx3"), VerifierFormat::CRX3_WITH_PUBLISHER_PROOF, keys, hash, &public_key, &crx_id)); @@ -172,6 +178,37 @@ TEST_F(CrxVerifierTest, ChecksPinnedKey) { EXPECT_EQ("UNSET", public_key); } +TEST_F(CrxVerifierTest, ChecksPinnedKeyAcceptsTest) { + const std::vector<uint8_t> hash; + const std::vector<std::vector<uint8_t>> keys; + std::string public_key = "UNSET"; + std::string crx_id = "UNSET"; + EXPECT_EQ(VerifierResult::OK_FULL, + Verify(TestFile("valid_publisher.crx3"), + VerifierFormat::CRX3_WITH_TEST_PUBLISHER_PROOF, keys, hash, + &public_key, &crx_id)); + EXPECT_EQ(std::string(kOjjHash), crx_id); + EXPECT_EQ(std::string(kOjjKey), public_key); + + public_key = "UNSET"; + crx_id = "UNSET"; + EXPECT_EQ(VerifierResult::OK_FULL, + Verify(TestFile("valid_test_publisher.crx3"), + VerifierFormat::CRX3_WITH_TEST_PUBLISHER_PROOF, keys, hash, + &public_key, &crx_id)); + EXPECT_EQ(std::string(kJlnHash), crx_id); + EXPECT_EQ(std::string(kJlnKey), public_key); + + public_key = "UNSET"; + crx_id = "UNSET"; + EXPECT_EQ(VerifierResult::ERROR_REQUIRED_PROOF_MISSING, + Verify(TestFile("valid_no_publisher.crx3"), + VerifierFormat::CRX3_WITH_TEST_PUBLISHER_PROOF, keys, hash, + &public_key, &crx_id)); + EXPECT_EQ("UNSET", crx_id); + EXPECT_EQ("UNSET", public_key); +} + TEST_F(CrxVerifierTest, NullptrSafe) { const std::vector<uint8_t> hash; const std::vector<std::vector<uint8_t>> keys; @@ -187,8 +224,8 @@ TEST_F(CrxVerifierTest, RequiresDeveloperKey) { std::string public_key = "UNSET"; std::string crx_id = "UNSET"; EXPECT_EQ(VerifierResult::ERROR_REQUIRED_PROOF_MISSING, - Verify(TestFile("unsigned.crx3"), VerifierFormat::CRX2_OR_CRX3, - keys, hash, &public_key, &crx_id)); + Verify(TestFile("unsigned.crx3"), VerifierFormat::CRX3, keys, hash, + &public_key, &crx_id)); EXPECT_EQ("UNSET", crx_id); EXPECT_EQ("UNSET", public_key); } |