Expand CRX verifier to verify CRX₃ files.

components/crx_file/crx_verifier.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/crx_file/crx_verifier.h"
 
 #include <cstring>
 #include <memory>
+#include <set>
+#include <utility>
 
 #include "base/base64.h"
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/memory/ptr_util.h"
+#include "base/strings/string_number_conversions.h"
 #include "components/crx_file/crx2_file.h"
+#include "components/crx_file/crx3.pb.h"
 #include "components/crx_file/id_util.h"
 #include "crypto/secure_hash.h"
 #include "crypto/secure_util.h"
 #include "crypto/sha2.h"
 #include "crypto/signature_verifier.h"
 
 namespace crx_file {
 
 namespace {
 
-// The maximum size the crx2 parser will tolerate for a public key.
+// 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.
+// 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 salt for Crx3 signing, encoded in UTF8.
+constexpr unsigned char kSalt[] = u8"CRX3 SignedData\0";
+
+// The SHA256 hash of the "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};
+
 int ReadAndHashBuffer(uint8_t* buffer,
                       int length,
                       base::File* file,
                       crypto::SecureHash* hash) {
   static_assert(sizeof(char) == sizeof(uint8_t), "Unsupported char size.");
   int read = file->ReadAtCurrentPos(reinterpret_cast<char*>(buffer), length);
   hash->Update(buffer, read);
   return read;
 }
 
 // Returns UINT32_MAX in the case of an unexpected EOF or read error, else
 // returns the read uint32.
 uint32_t ReadAndHashLittleEndianUInt32(base::File* file,
                                        crypto::SecureHash* hash) {
   uint8_t buffer[4] = {};
   if (ReadAndHashBuffer(buffer, 4, file, hash) != 4)
     return UINT32_MAX;
   return buffer[3] << 24 | buffer[2] << 16 | buffer[1] << 8 | buffer[0];
 }
 
+bool ReadHashAndVerifyArchive(
+    base::File* file,
+    crypto::SecureHash* hash,
+    const std::vector<std::unique_ptr<crypto::SignatureVerifier>>& verifiers) {
+  uint8_t buffer[1 << 12] = {};
+  size_t len;

[Sorin Jianu, 2017/05/17 23:54:53]

not initialized

[waffles, 2017/05/18 00:43:53]

Done, thanks; messed it up in the merge.

+  while ((len = ReadAndHashBuffer(buffer, arraysize(buffer), file, hash)) > 0) {
+    for (auto& verifier : verifiers)
+      verifier->VerifyUpdate(buffer, len);
+  }
+  for (auto& verifier : verifiers) {
+    if (!verifier->VerifyFinal())
+      return false;
+  }
+  return true;
+}
+
 VerifierResult VerifyCrx3(
     base::File* file,
     crypto::SecureHash* hash,
     const std::vector<std::vector<uint8_t>>& required_key_hashes,
     std::string* public_key,
-    std::string* crx_id) {
-  // Crx3 files are not yet supported - treat this as a malformed header.
-  return VerifierResult::ERROR_HEADER_INVALID;
+    std::string* crx_id,
+    bool require_publisher_key) {
+  // Parse header data.
+  const uint32_t header_size = ReadAndHashLittleEndianUInt32(file, hash);
+  if (header_size > kMaxHeaderSize)
+    return VerifierResult::ERROR_HEADER_INVALID;
+  std::vector<uint8_t> header_bytes(header_size);
+  if (ReadAndHashBuffer(header_bytes.data(), header_size, file, hash) !=
+      static_cast<int>(header_size))
+    return VerifierResult::ERROR_HEADER_INVALID;
+  CrxFileHeader header;
+  if (!header.ParseFromArray(header_bytes.data(), header_size))
+    return VerifierResult::ERROR_HEADER_INVALID;
+
+  // Parse signed header data.
+  const std::string& signed_header_data_str = header.signed_header_data();
+  SignedData signed_header_data;
+  if (!signed_header_data.ParseFromString(signed_header_data_str))
+    return VerifierResult::ERROR_HEADER_INVALID;
+  const std::string& crx_id_encoded = signed_header_data.crx_id();
+  const std::string declared_crx_id =
+      base::HexEncode(crx_id_encoded.data(), crx_id_encoded.size());
+
+  // Compute the signing prefix for the signed header data size.
+  const int signed_header_size = signed_header_data_str.size();
+  const uint8_t header_size_octets[] = {
+      signed_header_size, signed_header_size >> 8, signed_header_size >> 16,
+      signed_header_size >> 24};
+
+  // Set up the required keys.
+  std::set<std::vector<uint8_t>> required_key_set(required_key_hashes.begin(),
+                                                  required_key_hashes.end());
+  if (require_publisher_key) {
+    const std::vector<uint8_t> publisher_key(
+        kPublisherKeyHash, kPublisherKeyHash + arraysize(kPublisherKeyHash));

[Sorin Jianu, 2017/05/17 23:54:53]

I've started using begin and end for initializers like these.

[waffles, 2017/05/18 00:43:53]

Done, thanks!

+    required_key_set.insert(publisher_key);
+  }
+  std::string public_key_bytes;
+  std::vector<std::unique_ptr<crypto::SignatureVerifier>> verifiers = {};

[Sorin Jianu, 2017/05/17 23:54:53]

do we need {} ?

[waffles, 2017/05/18 00:43:53]

Done.

+  const std::vector<
+      std::pair<std::function<const google::protobuf::RepeatedPtrField<
+                    AsymmetricKeyProof>&()>,
+                crypto::SignatureVerifier::SignatureAlgorithm>>
+      proof_types = {
+          std::make_pair([&] { return header.sha256_with_rsa(); },
+                         crypto::SignatureVerifier::RSA_PKCS1_SHA256),
+          std::make_pair([&] { return header.sha256_with_ecdsa(); },
+                         crypto::SignatureVerifier::ECDSA_SHA256)};
+  // Initialize all verifiers and update them with the salts / prefixes.
+  for (auto proof_type : proof_types) {

[Sorin Jianu, 2017/05/17 23:54:53]

maybe const auto& or auto& ?

[waffles, 2017/05/18 00:43:53]

Done.

+    for (auto proof : proof_type.first()) {

[Sorin Jianu, 2017/05/17 23:54:53]

maybe const auto & if possible?

[waffles, 2017/05/18 00:43:53]

Done.

+      const std::string& key = proof.public_key();
+      const std::string& sig = proof.signature();
+      if (id_util::GenerateId(key) == declared_crx_id)
+        public_key_bytes = key;
+      std::vector<uint8_t> key_hash(crypto::kSHA256Length);
+      crypto::SHA256HashString(key, key_hash.data(), key_hash.size());
+      required_key_set.erase(key_hash);
+      std::unique_ptr<crypto::SignatureVerifier> v =
+          base::MakeUnique<crypto::SignatureVerifier>();
+      static_assert(sizeof(unsigned char) == sizeof(uint8_t),
+                    "Unsupported char size.");
+      if (!v->VerifyInit(
+              proof_type.second, reinterpret_cast<const uint8_t*>(sig.data()),
+              sig.size(), reinterpret_cast<const uint8_t*>(key.data()),
+              key.size()))
+        return VerifierResult::ERROR_SIGNATURE_INITIALIZATION_FAILED;
+      v->VerifyUpdate(kSalt, arraysize(kSalt));

[Sorin Jianu, 2017/05/17 23:54:53]

do we want arraysize here or sizeof? It won't make a difference but which one is
more logically correct?

[waffles, 2017/05/18 00:43:53]

I think arraysize, since that is semantically consistent with other callers (all
other callers of this method in the code base use std::string::size or
std::vector<uint8_t>::size). Let me know if I'm confused.

+      v->VerifyUpdate(header_size_octets, arraysize(header_size_octets));
+      v->VerifyUpdate(
+          reinterpret_cast<const uint8_t*>(signed_header_data_str.data()),
+          signed_header_data_str.size());
+      verifiers.push_back(std::move(v));
+    }
+  }
+  if (!public_key_bytes.empty() || !required_key_set.empty())
+    return VerifierResult::ERROR_REQUIRED_PROOF_MISSING;
+
+  // Verify the entire archive.
+  if (!ReadHashAndVerifyArchive(file, hash, verifiers))
+    return VerifierResult::ERROR_SIGNATURE_VERIFICATION_FAILED;
+
+  if (public_key)
+    base::Base64Encode(public_key_bytes, public_key);
+  if (crx_id)
+    *crx_id = declared_crx_id;
+  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.
+    // 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;
-  crypto::SignatureVerifier verifier;
-  if (!verifier.VerifyInit(crypto::SignatureVerifier::RSA_PKCS1_SHA1,
-                           sig.data(), sig.size(), key.data(), key.size())) {
+  std::vector<std::unique_ptr<crypto::SignatureVerifier>> verifiers;
+  verifiers.push_back(base::MakeUnique<crypto::SignatureVerifier>());
+  if (!verifiers[0]->VerifyInit(crypto::SignatureVerifier::RSA_PKCS1_SHA1,
+                                sig.data(), sig.size(), key.data(),
+                                key.size())) {
     return VerifierResult::ERROR_SIGNATURE_INITIALIZATION_FAILED;
   }
 
-  // Read the rest of the file.
-  uint8_t buffer[1 << 12] = {};
-  size_t len = 0;
-  while ((len = ReadAndHashBuffer(buffer, arraysize(buffer), file, hash)) > 0)
-    verifier.VerifyUpdate(buffer, len);
-  if (!verifier.VerifyFinal())
+  if (!ReadHashAndVerifyArchive(file, hash, verifiers))
     return VerifierResult::ERROR_SIGNATURE_VERIFICATION_FAILED;
 
   const std::string public_key_bytes(key.begin(), key.end());
   if (public_key)
     base::Base64Encode(public_key_bytes, public_key);
   if (crx_id)
     *crx_id = id_util::GenerateId(public_key_bytes);
   return VerifierResult::OK_FULL;
 }
 
@@ skipping 27 matching lines @@
 
   // Version number.
   const uint32_t version =
       ReadAndHashLittleEndianUInt32(&file, file_hash.get());
   VerifierResult result;
   if (format == VerifierFormat::CRX2_OR_CRX3 &&
       (version == 2 || (diff && version == 0)))
     result = VerifyCrx2(&file, file_hash.get(), required_key_hashes, public_key,
                         crx_id);
   else if (version == 3)
-    result = VerifyCrx3(&file, file_hash.get(), required_key_hashes, public_key,
-                        crx_id);
+    result =
+        VerifyCrx3(&file, file_hash.get(), required_key_hashes, public_key,
+                   crx_id, format == VerifierFormat::CRX3_WITH_PUBLISHER_PROOF);
   else
     result = VerifierResult::ERROR_HEADER_INVALID;
   if (result != VerifierResult::OK_FULL)
     return result;
 
   // Finalize file hash.
   uint8_t final_hash[crypto::kSHA256Length] = {};
   file_hash->Finish(final_hash, sizeof(final_hash));
   if (!required_file_hash.empty()) {
     if (required_file_hash.size() != crypto::kSHA256Length)
       return VerifierResult::ERROR_EXPECTED_HASH_INVALID;
     if (!crypto::SecureMemEqual(final_hash, required_file_hash.data(),
                                 crypto::kSHA256Length))
       return VerifierResult::ERROR_FILE_HASH_FAILED;
   }
   return diff ? VerifierResult::OK_DELTA : VerifierResult::OK_FULL;
 }
 
 }  // namespace crx_file