Check all signatures regardless of the version.
The update_engine daemon had a fixed version number for the public key
used to verify both the metadata and whole payload signatures. The
public key itself is installed by the signer, implying that the source
code and the signer need to be in sync if we ever need to roll the
payload key.
This situation becomes more of a problem if we don't control when the
version number included in the source code is updated in the built
image sent for payload generation and signing.
This patch makes update_engine ignore the version number associated
with a signature and instead tries to verify all the signatures
included in the payload against the public key found in the code. This
effectively deprecates the key version number. To be compatible with
old versions, the version number 1 is included in all signatures.
Bug: 23601118
Test: Added unittests.
Change-Id: I4f96cc207ad6b9c011def5ce586d0e0e85af28ab
diff --git a/payload_generator/generate_delta_main.cc b/payload_generator/generate_delta_main.cc
index d38c5c5..a600c9c 100644
--- a/payload_generator/generate_delta_main.cc
+++ b/payload_generator/generate_delta_main.cc
@@ -171,15 +171,13 @@
}
void VerifySignedPayload(const string& in_file,
- const string& public_key,
- int public_key_version) {
+ const string& public_key) {
LOG(INFO) << "Verifying signed payload.";
LOG_IF(FATAL, in_file.empty())
<< "Must pass --in_file to verify signed payload.";
LOG_IF(FATAL, public_key.empty())
<< "Must pass --public_key to verify signed payload.";
- CHECK(PayloadVerifier::VerifySignedPayload(in_file, public_key,
- public_key_version));
+ CHECK(PayloadVerifier::VerifySignedPayload(in_file, public_key));
LOG(INFO) << "Done verifying signed payload.";
}
@@ -241,9 +239,8 @@
"Path to output metadata hash file");
DEFINE_string(private_key, "", "Path to private key in .pem format");
DEFINE_string(public_key, "", "Path to public key in .pem format");
- DEFINE_int32(public_key_version,
- chromeos_update_engine::kSignatureMessageCurrentVersion,
- "Key-check version # of client");
+ DEFINE_int32(public_key_version, -1,
+ "DEPRECATED. Key-check version # of client");
DEFINE_string(prefs_dir, "/tmp/update_engine_prefs",
"Preferences directory, used with apply_delta");
DEFINE_string(signature_size, "",
@@ -344,8 +341,9 @@
return 0;
}
if (!FLAGS_public_key.empty()) {
- VerifySignedPayload(FLAGS_in_file, FLAGS_public_key,
- FLAGS_public_key_version);
+ LOG_IF(WARNING, FLAGS_public_key_version != -1)
+ << "--public_key_version is deprecated and ignored.";
+ VerifySignedPayload(FLAGS_in_file, FLAGS_public_key);
return 0;
}
if (!FLAGS_in_file.empty()) {
diff --git a/payload_generator/payload_signer.cc b/payload_generator/payload_signer.cc
index 218b432..4a2bb95 100644
--- a/payload_generator/payload_signer.cc
+++ b/payload_generator/payload_signer.cc
@@ -38,22 +38,21 @@
namespace {
+// The payload verifier will check all the signatures included in the payload
+// regardless of the version field. Old version of the verifier require the
+// version field to be included and be 1.
+const uint32_t kSignatureMessageLegacyVersion = 1;
+
// Given raw |signatures|, packs them into a protobuf and serializes it into a
// binary blob. Returns true on success, false otherwise.
bool ConvertSignatureToProtobufBlob(const vector<chromeos::Blob>& signatures,
chromeos::Blob* out_signature_blob) {
// Pack it into a protobuf
Signatures out_message;
- uint32_t version = kSignatureMessageOriginalVersion;
- LOG_IF(WARNING, kSignatureMessageCurrentVersion -
- kSignatureMessageOriginalVersion + 1 < signatures.size())
- << "You may want to support clients in the range ["
- << kSignatureMessageOriginalVersion << ", "
- << kSignatureMessageCurrentVersion << "] inclusive, but you only "
- << "provided " << signatures.size() << " signatures.";
for (const chromeos::Blob& signature : signatures) {
Signatures_Signature* sig_message = out_message.add_signatures();
- sig_message->set_version(version++);
+ // Set all the signatures with the same version number.
+ sig_message->set_version(kSignatureMessageLegacyVersion);
sig_message->set_data(signature.data(), signature.size());
}
@@ -156,16 +155,10 @@
padded_hash.data(),
padded_hash.size()));
- // This runs on the server, so it's okay to cop out and call openssl
- // executable rather than properly use the library
- vector<string> cmd;
- base::SplitString("openssl rsautl -raw -sign -inkey x -in x -out x",
- ' ',
- &cmd);
- cmd[cmd.size() - 5] = private_key_path;
- cmd[cmd.size() - 3] = hash_path;
- cmd[cmd.size() - 1] = sig_path;
-
+ // This runs on the server, so it's okay to copy out and call openssl
+ // executable rather than properly use the library.
+ vector<string> cmd = {"openssl", "rsautl", "-raw", "-sign", "-inkey",
+ private_key_path, "-in", hash_path, "-out", sig_path};
int return_code = 0;
TEST_AND_RETURN_FALSE(Subprocess::SynchronousExec(cmd, &return_code,
nullptr));
diff --git a/payload_generator/payload_signer_unittest.cc b/payload_generator/payload_signer_unittest.cc
index 1cd45ce..cedb432 100644
--- a/payload_generator/payload_signer_unittest.cc
+++ b/payload_generator/payload_signer_unittest.cc
@@ -94,30 +94,37 @@
};
namespace {
-void SignSampleData(chromeos::Blob* out_signature_blob) {
+void SignSampleData(chromeos::Blob* out_signature_blob,
+ const vector<string>& private_keys) {
string data_path;
- ASSERT_TRUE(
- utils::MakeTempFile("data.XXXXXX", &data_path, nullptr));
+ ASSERT_TRUE(utils::MakeTempFile("data.XXXXXX", &data_path, nullptr));
ScopedPathUnlinker data_path_unlinker(data_path);
ASSERT_TRUE(utils::WriteFile(data_path.c_str(),
kDataToSign,
strlen(kDataToSign)));
uint64_t length = 0;
- EXPECT_TRUE(PayloadSigner::SignatureBlobLength(
- vector<string> (1, kUnittestPrivateKeyPath),
- &length));
+ EXPECT_TRUE(PayloadSigner::SignatureBlobLength(private_keys, &length));
EXPECT_GT(length, 0);
EXPECT_TRUE(PayloadSigner::SignPayload(
data_path,
- vector<string>(1, kUnittestPrivateKeyPath),
+ private_keys,
out_signature_blob));
EXPECT_EQ(length, out_signature_blob->size());
}
} // namespace
-TEST(PayloadSignerTest, SimpleTest) {
+class PayloadSignerTest : public ::testing::Test {
+ protected:
+ void SetUp() override {
+ PayloadVerifier::PadRSA2048SHA256Hash(&padded_hash_data_);
+ }
+
+ chromeos::Blob padded_hash_data_{std::begin(kDataHash), std::end(kDataHash)};
+};
+
+TEST_F(PayloadSignerTest, SignSimpleTextTest) {
chromeos::Blob signature_blob;
- SignSampleData(&signature_blob);
+ SignSampleData(&signature_blob, {kUnittestPrivateKeyPath});
// Check the signature itself
Signatures signatures;
@@ -125,7 +132,7 @@
signature_blob.size()));
EXPECT_EQ(1, signatures.signatures_size());
const Signatures_Signature& signature = signatures.signatures(0);
- EXPECT_EQ(kSignatureMessageOriginalVersion, signature.version());
+ EXPECT_EQ(1, signature.version());
const string sig_data = signature.data();
ASSERT_EQ(arraysize(kDataSignature), sig_data.size());
for (size_t i = 0; i < arraysize(kDataSignature); i++) {
@@ -133,20 +140,31 @@
}
}
-TEST(PayloadSignerTest, VerifySignatureTest) {
+TEST_F(PayloadSignerTest, VerifyAllSignatureTest) {
chromeos::Blob signature_blob;
- SignSampleData(&signature_blob);
+ SignSampleData(&signature_blob,
+ {kUnittestPrivateKeyPath, kUnittestPrivateKey2Path});
- chromeos::Blob hash_data;
+ // Either public key should pass the verification.
EXPECT_TRUE(PayloadVerifier::VerifySignature(signature_blob,
- kUnittestPublicKeyPath,
- &hash_data));
- chromeos::Blob padded_hash_data(std::begin(kDataHash), std::end(kDataHash));
- PayloadVerifier::PadRSA2048SHA256Hash(&padded_hash_data);
- ASSERT_EQ(padded_hash_data.size(), hash_data.size());
- for (size_t i = 0; i < padded_hash_data.size(); i++) {
- EXPECT_EQ(padded_hash_data[i], hash_data[i]);
- }
+ kUnittestPublicKeyPath,
+ padded_hash_data_));
+ EXPECT_TRUE(PayloadVerifier::VerifySignature(signature_blob,
+ kUnittestPublicKey2Path,
+ padded_hash_data_));
+}
+
+TEST_F(PayloadSignerTest, VerifySignatureTest) {
+ chromeos::Blob signature_blob;
+ SignSampleData(&signature_blob, {kUnittestPrivateKeyPath});
+
+ EXPECT_TRUE(PayloadVerifier::VerifySignature(signature_blob,
+ kUnittestPublicKeyPath,
+ padded_hash_data_));
+ // Passing the invalid key should fail the verification.
+ EXPECT_FALSE(PayloadVerifier::VerifySignature(signature_blob,
+ kUnittestPublicKey2Path,
+ padded_hash_data_));
}
} // namespace chromeos_update_engine