Factor out the RSA verification in payload verifier am: 6cf830b891 am: 3fdffe38ce am: baffc44756
am: d0b9ea54ad
Change-Id: Ie9964279f9932ef6ef5943ac8c1a6a372031948c
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 3ff98ca..8049af7 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -1794,8 +1794,12 @@
TEST_AND_RETURN_VAL(ErrorCode::kDownloadPayloadPubKeyVerificationError,
hash_data.size() == kSHA256Size);
- if (!PayloadVerifier::VerifySignature(
- signatures_message_data_, public_key, hash_data)) {
+ auto payload_verifier = PayloadVerifier::CreateInstance(public_key);
+ if (!payload_verifier) {
+ LOG(ERROR) << "Failed to create the payload verifier from " << public_key;
+ return ErrorCode::kDownloadPayloadPubKeyVerificationError;
+ }
+ if (!payload_verifier->VerifySignature(signatures_message_data_, hash_data)) {
// The autoupdate_CatchBadSignatures test checks for this string
// in log-files. Keep in sync.
LOG(ERROR) << "Public key verification failed, thus update failed.";
diff --git a/payload_consumer/payload_metadata.cc b/payload_consumer/payload_metadata.cc
index 3739767..c81d3a9 100644
--- a/payload_consumer/payload_metadata.cc
+++ b/payload_consumer/payload_metadata.cc
@@ -201,32 +201,26 @@
return ErrorCode::kDownloadMetadataSignatureVerificationError;
}
+ auto payload_verifier = PayloadVerifier::CreateInstance(pem_public_key);
+ if (!payload_verifier) {
+ LOG(ERROR) << "Failed to create the payload verifier from "
+ << pem_public_key;
+ return ErrorCode::kDownloadMetadataSignatureVerificationError;
+ }
+
if (!metadata_signature_blob.empty()) {
- brillo::Blob expected_metadata_hash;
- if (!PayloadVerifier::GetRawHashFromSignature(
- metadata_signature_blob, pem_public_key, &expected_metadata_hash)) {
- LOG(ERROR) << "Unable to compute expected hash from metadata signature";
- return ErrorCode::kDownloadMetadataSignatureError;
- }
-
- brillo::Blob padded_metadata_hash = metadata_hash;
- if (!PayloadVerifier::PadRSASHA256Hash(&padded_metadata_hash,
- expected_metadata_hash.size())) {
- LOG(ERROR) << "Failed to pad the SHA256 hash to "
- << expected_metadata_hash.size() << " bytes.";
- return ErrorCode::kDownloadMetadataSignatureVerificationError;
- }
-
- if (padded_metadata_hash != expected_metadata_hash) {
- LOG(ERROR) << "Manifest hash verification failed. Expected hash = ";
- utils::HexDumpVector(expected_metadata_hash);
- LOG(ERROR) << "Calculated hash = ";
- utils::HexDumpVector(padded_metadata_hash);
+ brillo::Blob decrypted_signature;
+ if (!payload_verifier->VerifyRawSignature(
+ metadata_signature_blob, metadata_hash, &decrypted_signature)) {
+ LOG(ERROR) << "Manifest hash verification failed. Decrypted hash = ";
+ utils::HexDumpVector(decrypted_signature);
+ LOG(ERROR) << "Calculated hash before padding = ";
+ utils::HexDumpVector(metadata_hash);
return ErrorCode::kDownloadMetadataSignatureMismatch;
}
} else {
- if (!PayloadVerifier::VerifySignature(
- metadata_signature_protobuf, pem_public_key, metadata_hash)) {
+ if (!payload_verifier->VerifySignature(metadata_signature_protobuf,
+ metadata_hash)) {
LOG(ERROR) << "Manifest hash verification failed.";
return ErrorCode::kDownloadMetadataSignatureMismatch;
}
diff --git a/payload_consumer/payload_verifier.cc b/payload_consumer/payload_verifier.cc
index 3a3ccbf..b2c5be4 100644
--- a/payload_consumer/payload_verifier.cc
+++ b/payload_consumer/payload_verifier.cc
@@ -51,9 +51,30 @@
} // namespace
-bool PayloadVerifier::VerifySignature(const string& signature_proto,
- const string& pem_public_key,
- const brillo::Blob& sha256_hash_data) {
+std::unique_ptr<PayloadVerifier> PayloadVerifier::CreateInstance(
+ const std::string& pem_public_key) {
+ std::unique_ptr<BIO, decltype(&BIO_free)> bp(
+ BIO_new_mem_buf(pem_public_key.data(), pem_public_key.size()), BIO_free);
+ if (!bp) {
+ LOG(ERROR) << "Failed to read " << pem_public_key << " into buffer.";
+ return nullptr;
+ }
+
+ auto pub_key = std::unique_ptr<EVP_PKEY, decltype(&EVP_PKEY_free)>(
+ PEM_read_bio_PUBKEY(bp.get(), nullptr, nullptr, nullptr), EVP_PKEY_free);
+ if (!pub_key) {
+ LOG(ERROR) << "Failed to parse the public key in " << pem_public_key;
+ return nullptr;
+ }
+
+ return std::unique_ptr<PayloadVerifier>(
+ new PayloadVerifier(std::move(pub_key)));
+}
+
+bool PayloadVerifier::VerifySignature(
+ const string& signature_proto, const brillo::Blob& sha256_hash_data) const {
+ TEST_AND_RETURN_FALSE(public_key_ != nullptr);
+
Signatures signatures;
LOG(INFO) << "signature blob size = " << signature_proto.size();
TEST_AND_RETURN_FALSE(signatures.ParseFromString(signature_proto));
@@ -69,17 +90,14 @@
const Signatures::Signature& signature = signatures.signatures(i);
brillo::Blob sig_data(signature.data().begin(), signature.data().end());
brillo::Blob sig_hash_data;
- if (!GetRawHashFromSignature(sig_data, pem_public_key, &sig_hash_data))
- continue;
-
- brillo::Blob padded_hash_data = sha256_hash_data;
- if (PadRSASHA256Hash(&padded_hash_data, sig_hash_data.size()) &&
- padded_hash_data == sig_hash_data) {
+ if (VerifyRawSignature(sig_data, sha256_hash_data, &sig_hash_data)) {
LOG(INFO) << "Verified correct signature " << i + 1 << " out of "
<< signatures.signatures_size() << " signatures.";
return true;
}
- tested_hashes.push_back(sig_hash_data);
+ if (!sig_hash_data.empty()) {
+ tested_hashes.push_back(sig_hash_data);
+ }
}
LOG(ERROR) << "None of the " << signatures.signatures_size()
<< " signatures is correct. Expected hash before padding:";
@@ -91,24 +109,43 @@
return false;
}
-bool PayloadVerifier::GetRawHashFromSignature(const brillo::Blob& sig_data,
- const string& pem_public_key,
- brillo::Blob* out_hash_data) {
+bool PayloadVerifier::VerifyRawSignature(
+ const brillo::Blob& sig_data,
+ const brillo::Blob& sha256_hash_data,
+ brillo::Blob* decrypted_sig_data) const {
+ TEST_AND_RETURN_FALSE(public_key_ != nullptr);
+
+ int key_type = EVP_PKEY_id(public_key_.get());
+ TEST_AND_RETURN_FALSE(key_type == EVP_PKEY_RSA);
+ brillo::Blob sig_hash_data;
+ TEST_AND_RETURN_FALSE(
+ GetRawHashFromSignature(sig_data, public_key_.get(), &sig_hash_data));
+
+ if (decrypted_sig_data != nullptr) {
+ *decrypted_sig_data = sig_hash_data;
+ }
+
+ brillo::Blob padded_hash_data = sha256_hash_data;
+ TEST_AND_RETURN_FALSE(
+ PadRSASHA256Hash(&padded_hash_data, sig_hash_data.size()));
+
+ return padded_hash_data == sig_hash_data;
+}
+
+bool PayloadVerifier::GetRawHashFromSignature(
+ const brillo::Blob& sig_data,
+ const EVP_PKEY* public_key,
+ brillo::Blob* out_hash_data) const {
// The code below executes the equivalent of:
//
- // openssl rsautl -verify -pubin -inkey <(echo |pem_public_key|)
+ // openssl rsautl -verify -pubin -inkey <(echo pem_public_key)
// -in |sig_data| -out |out_hash_data|
-
- BIO* bp = BIO_new_mem_buf(pem_public_key.data(), pem_public_key.size());
- char dummy_password[] = {' ', 0}; // Ensure no password is read from stdin.
- RSA* rsa = PEM_read_bio_RSA_PUBKEY(bp, nullptr, nullptr, dummy_password);
- BIO_free(bp);
+ RSA* rsa = EVP_PKEY_get0_RSA(public_key);
TEST_AND_RETURN_FALSE(rsa != nullptr);
unsigned int keysize = RSA_size(rsa);
if (sig_data.size() > 2 * keysize) {
LOG(ERROR) << "Signature size is too big for public key size.";
- RSA_free(rsa);
return false;
}
@@ -116,7 +153,6 @@
brillo::Blob hash_data(keysize);
int decrypt_size = RSA_public_decrypt(
sig_data.size(), sig_data.data(), hash_data.data(), rsa, RSA_NO_PADDING);
- RSA_free(rsa);
TEST_AND_RETURN_FALSE(decrypt_size > 0 &&
decrypt_size <= static_cast<int>(hash_data.size()));
hash_data.resize(decrypt_size);
diff --git a/payload_consumer/payload_verifier.h b/payload_consumer/payload_verifier.h
index af8e05f..b5d5457 100644
--- a/payload_consumer/payload_verifier.h
+++ b/payload_consumer/payload_verifier.h
@@ -17,38 +17,23 @@
#ifndef UPDATE_ENGINE_PAYLOAD_CONSUMER_PAYLOAD_VERIFIER_H_
#define UPDATE_ENGINE_PAYLOAD_CONSUMER_PAYLOAD_VERIFIER_H_
+#include <memory>
#include <string>
+#include <utility>
-#include <base/macros.h>
#include <brillo/secure_blob.h>
+#include <openssl/evp.h>
#include "update_engine/update_metadata.pb.h"
-// This class encapsulates methods used for payload signature verification.
-// See payload_generator/payload_signer.h for payload signing.
+// This class holds the public key and implements methods used for payload
+// signature verification. See payload_generator/payload_signer.h for payload
+// signing.
namespace chromeos_update_engine {
class PayloadVerifier {
public:
- // Interprets |signature_proto| as a protocol buffer containing the Signatures
- // message and decrypts each signature data using the |pem_public_key|.
- // |pem_public_key| should be a PEM format RSA public key data.
- // Pads the 32 bytes |sha256_hash_data| to 256 or 512 bytes according to the
- // PKCS#1 v1.5 standard; and returns whether *any* of the decrypted hashes
- // matches the padded hash data. In case of any error parsing the signatures
- // or the public key, returns false.
- static bool VerifySignature(const std::string& signature_proto,
- const std::string& pem_public_key,
- const brillo::Blob& sha256_hash_data);
-
- // Decrypts |sig_data| with the given |pem_public_key| and populates
- // |out_hash_data| with the decoded raw hash. |pem_public_key| should be a PEM
- // format RSA public key data. Returns true if successful, false otherwise.
- static bool GetRawHashFromSignature(const brillo::Blob& sig_data,
- const std::string& pem_public_key,
- brillo::Blob* out_hash_data);
-
// Pads a SHA256 hash so that it may be encrypted/signed with RSA2048 or
// RSA4096 using the PKCS#1 v1.5 scheme.
// hash should be a pointer to vector of exactly 256 bits. |rsa_size| must be
@@ -57,9 +42,41 @@
// Returns true on success, false otherwise.
static bool PadRSASHA256Hash(brillo::Blob* hash, size_t rsa_size);
+ // Parses the input as a PEM encoded public string. And creates a
+ // PayloadVerifier with that public key for signature verification.
+ static std::unique_ptr<PayloadVerifier> CreateInstance(
+ const std::string& pem_public_key);
+
+ // Interprets |signature_proto| as a protocol buffer containing the
+ // |Signatures| message and decrypts each signature data using the stored
+ // public key. Pads the 32 bytes |sha256_hash_data| to 256 or 512 bytes
+ // according to the PKCS#1 v1.5 standard; and returns whether *any* of the
+ // decrypted hashes matches the padded hash data. In case of any error parsing
+ // the signatures, returns false.
+ bool VerifySignature(const std::string& signature_proto,
+ const brillo::Blob& sha256_hash_data) const;
+
+ // Verifies if |sig_data| is a raw signature of the hash |sha256_hash_data|.
+ // If PayloadVerifier is using RSA as the public key, further puts the
+ // decrypted data of |sig_data| into |decrypted_sig_data|.
+ bool VerifyRawSignature(const brillo::Blob& sig_data,
+ const brillo::Blob& sha256_hash_data,
+ brillo::Blob* decrypted_sig_data) const;
+
private:
- // This should never be constructed
- DISALLOW_IMPLICIT_CONSTRUCTORS(PayloadVerifier);
+ explicit PayloadVerifier(
+ std::unique_ptr<EVP_PKEY, decltype(&EVP_PKEY_free)>&& public_key)
+ : public_key_(std::move(public_key)) {}
+
+ // Decrypts |sig_data| with the given |public_key| and populates
+ // |out_hash_data| with the decoded raw hash. Returns true if successful,
+ // false otherwise.
+ bool GetRawHashFromSignature(const brillo::Blob& sig_data,
+ const EVP_PKEY* public_key,
+ brillo::Blob* out_hash_data) const;
+
+ std::unique_ptr<EVP_PKEY, decltype(&EVP_PKEY_free)> public_key_{nullptr,
+ nullptr};
};
} // namespace chromeos_update_engine
diff --git a/payload_generator/payload_signer.cc b/payload_generator/payload_signer.cc
index 9739052..3c9ce95 100644
--- a/payload_generator/payload_signer.cc
+++ b/payload_generator/payload_signer.cc
@@ -18,6 +18,7 @@
#include <endian.h>
+#include <memory>
#include <utility>
#include <base/logging.h>
@@ -255,14 +256,18 @@
string public_key;
TEST_AND_RETURN_FALSE(utils::ReadFile(public_key_path, &public_key));
TEST_AND_RETURN_FALSE(payload_hash.size() == kSHA256Size);
+
+ auto payload_verifier = PayloadVerifier::CreateInstance(public_key);
+ TEST_AND_RETURN_FALSE(payload_verifier != nullptr);
+
TEST_AND_RETURN_FALSE(
- PayloadVerifier::VerifySignature(signature, public_key, payload_hash));
+ payload_verifier->VerifySignature(signature, payload_hash));
if (metadata_signature_size) {
signature.assign(payload.begin() + metadata_size,
payload.begin() + metadata_size + metadata_signature_size);
TEST_AND_RETURN_FALSE(metadata_hash.size() == kSHA256Size);
TEST_AND_RETURN_FALSE(
- PayloadVerifier::VerifySignature(signature, public_key, metadata_hash));
+ payload_verifier->VerifySignature(signature, metadata_hash));
}
return true;
}
@@ -280,27 +285,39 @@
FILE* fprikey = fopen(private_key_path.c_str(), "rb");
TEST_AND_RETURN_FALSE(fprikey != nullptr);
- RSA* rsa = PEM_read_RSAPrivateKey(fprikey, nullptr, nullptr, nullptr);
+
+ std::unique_ptr<EVP_PKEY, decltype(&EVP_PKEY_free)> private_key(
+ PEM_read_PrivateKey(fprikey, nullptr, nullptr, nullptr), EVP_PKEY_free);
fclose(fprikey);
- TEST_AND_RETURN_FALSE(rsa != nullptr);
+ TEST_AND_RETURN_FALSE(private_key != nullptr);
- brillo::Blob padded_hash = hash;
- PayloadVerifier::PadRSASHA256Hash(&padded_hash, RSA_size(rsa));
+ int key_type = EVP_PKEY_id(private_key.get());
+ brillo::Blob signature;
+ if (key_type == EVP_PKEY_RSA) {
+ RSA* rsa = EVP_PKEY_get0_RSA(private_key.get());
+ TEST_AND_RETURN_FALSE(rsa != nullptr);
- brillo::Blob signature(RSA_size(rsa));
- ssize_t signature_size = RSA_private_encrypt(padded_hash.size(),
- padded_hash.data(),
- signature.data(),
- rsa,
- RSA_NO_PADDING);
- RSA_free(rsa);
- if (signature_size < 0) {
- LOG(ERROR) << "Signing hash failed: "
- << ERR_error_string(ERR_get_error(), nullptr);
+ brillo::Blob padded_hash = hash;
+ PayloadVerifier::PadRSASHA256Hash(&padded_hash, RSA_size(rsa));
+
+ signature.resize(RSA_size(rsa));
+ ssize_t signature_size = RSA_private_encrypt(padded_hash.size(),
+ padded_hash.data(),
+ signature.data(),
+ rsa,
+ RSA_NO_PADDING);
+
+ if (signature_size < 0) {
+ LOG(ERROR) << "Signing hash failed: "
+ << ERR_error_string(ERR_get_error(), nullptr);
+ return false;
+ }
+ TEST_AND_RETURN_FALSE(static_cast<size_t>(signature_size) ==
+ signature.size());
+ } else {
+ LOG(ERROR) << "key_type " << key_type << " isn't supported for signing";
return false;
}
- TEST_AND_RETURN_FALSE(static_cast<size_t>(signature_size) ==
- signature.size());
out_signature->swap(signature);
return true;
}
diff --git a/payload_generator/payload_signer_unittest.cc b/payload_generator/payload_signer_unittest.cc
index eaf8776..457b34c 100644
--- a/payload_generator/payload_signer_unittest.cc
+++ b/payload_generator/payload_signer_unittest.cc
@@ -131,19 +131,15 @@
GetBuildArtifactsPath(kUnittestPrivateKeyRSA4096Path)});
// Either public key should pass the verification.
- string public_key;
- EXPECT_TRUE(utils::ReadFile(GetBuildArtifactsPath(kUnittestPublicKeyPath),
- &public_key));
- EXPECT_TRUE(
- PayloadVerifier::VerifySignature(signature, public_key, hash_data_));
- EXPECT_TRUE(utils::ReadFile(GetBuildArtifactsPath(kUnittestPublicKey2Path),
- &public_key));
- EXPECT_TRUE(
- PayloadVerifier::VerifySignature(signature, public_key, hash_data_));
- EXPECT_TRUE(utils::ReadFile(
- GetBuildArtifactsPath(kUnittestPublicKeyRSA4096Path), &public_key));
- EXPECT_TRUE(
- PayloadVerifier::VerifySignature(signature, public_key, hash_data_));
+ for (const auto& path : {kUnittestPublicKeyPath,
+ kUnittestPublicKey2Path,
+ kUnittestPublicKeyRSA4096Path}) {
+ string public_key;
+ EXPECT_TRUE(utils::ReadFile(GetBuildArtifactsPath(path), &public_key));
+ auto payload_verifier = PayloadVerifier::CreateInstance(public_key);
+ EXPECT_TRUE(payload_verifier != nullptr);
+ EXPECT_TRUE(payload_verifier->VerifySignature(signature, hash_data_));
+ }
}
TEST_F(PayloadSignerTest, VerifySignatureTest) {
@@ -153,13 +149,17 @@
string public_key;
EXPECT_TRUE(utils::ReadFile(GetBuildArtifactsPath(kUnittestPublicKeyPath),
&public_key));
- EXPECT_TRUE(
- PayloadVerifier::VerifySignature(signature, public_key, hash_data_));
+ auto payload_verifier = PayloadVerifier::CreateInstance(public_key);
+ EXPECT_TRUE(payload_verifier != nullptr);
+ EXPECT_TRUE(payload_verifier->VerifySignature(signature, hash_data_));
+
// Passing the invalid key should fail the verification.
+ public_key.clear();
EXPECT_TRUE(utils::ReadFile(GetBuildArtifactsPath(kUnittestPublicKey2Path),
&public_key));
- EXPECT_TRUE(
- PayloadVerifier::VerifySignature(signature, public_key, hash_data_));
+ payload_verifier = PayloadVerifier::CreateInstance(public_key);
+ EXPECT_TRUE(payload_verifier != nullptr);
+ EXPECT_FALSE(payload_verifier->VerifySignature(signature, hash_data_));
}
TEST_F(PayloadSignerTest, SkipMetadataSignatureTest) {