delta_generator: Include metadata signature in major version 2.
--metadata_signature_file flag does what it should now.
Note that you should only pass this flag if the payload you are signing is
version 2.
Bug: 23981164
TEST=unit test added.
Change-Id: I613cd6a5fef188eca37c46c3f8a0a41d1c22f2fd
diff --git a/payload_generator/generate_delta_main.cc b/payload_generator/generate_delta_main.cc
index 4dd0b66..fa56a51 100644
--- a/payload_generator/generate_delta_main.cc
+++ b/payload_generator/generate_delta_main.cc
@@ -125,28 +125,35 @@
LOG(INFO) << "Done calculating hash for signing.";
}
+void SignatureFileFlagToBlobs(const string& signature_file_flag,
+ vector<brillo::Blob>* signatures) {
+ vector<string> signature_files;
+ base::SplitString(signature_file_flag, ':', &signature_files);
+ for (const string& signature_file : signature_files) {
+ brillo::Blob signature;
+ CHECK(utils::ReadFile(signature_file, &signature));
+ signatures->push_back(signature);
+ }
+}
+
void SignPayload(const string& in_file,
const string& out_file,
- const string& signature_file,
+ const string& payload_signature_file,
+ const string& metadata_signature_file,
const string& out_metadata_size_file) {
LOG(INFO) << "Signing payload.";
LOG_IF(FATAL, in_file.empty())
<< "Must pass --in_file to sign payload.";
LOG_IF(FATAL, out_file.empty())
<< "Must pass --out_file to sign payload.";
- LOG_IF(FATAL, signature_file.empty())
+ LOG_IF(FATAL, payload_signature_file.empty())
<< "Must pass --signature_file to sign payload.";
- vector<brillo::Blob> signatures;
- vector<string> signature_files;
- base::SplitString(signature_file, ':', &signature_files);
- for (const string& signature_file : signature_files) {
- brillo::Blob signature;
- CHECK(utils::ReadFile(signature_file, &signature));
- signatures.push_back(signature);
- }
+ vector<brillo::Blob> signatures, metadata_signatures;
+ SignatureFileFlagToBlobs(payload_signature_file, &signatures);
+ SignatureFileFlagToBlobs(metadata_signature_file, &metadata_signatures);
uint64_t final_metadata_size;
- CHECK(PayloadSigner::AddSignatureToPayload(
- in_file, signatures, out_file, &final_metadata_size));
+ CHECK(PayloadSigner::AddSignatureToPayload(in_file, signatures,
+ metadata_signatures, out_file, &final_metadata_size));
LOG(INFO) << "Done signing payload. Final metadata size = "
<< final_metadata_size;
if (!out_metadata_size_file.empty()) {
@@ -347,7 +354,7 @@
}
if (!FLAGS_signature_file.empty()) {
SignPayload(FLAGS_in_file, FLAGS_out_file, FLAGS_signature_file,
- FLAGS_out_metadata_size_file);
+ FLAGS_metadata_signature_file, FLAGS_out_metadata_size_file);
return 0;
}
if (!FLAGS_public_key.empty()) {
diff --git a/payload_generator/payload_file.cc b/payload_generator/payload_file.cc
index 22efb94..17baf46 100644
--- a/payload_generator/payload_file.cc
+++ b/payload_generator/payload_file.cc
@@ -161,8 +161,8 @@
// Signatures appear at the end of the blobs. Note the offset in the
// manifest_.
+ uint64_t signature_blob_length = 0;
if (!private_key_path.empty()) {
- uint64_t signature_blob_length = 0;
TEST_AND_RETURN_FALSE(
PayloadSigner::SignatureBlobLength(vector<string>(1, private_key_path),
&signature_blob_length));
@@ -194,11 +194,17 @@
TEST_AND_RETURN_FALSE(WriteUint64AsBigEndian(&writer,
serialized_manifest.size()));
+ // Write metadata signature size.
+ uint32_t metadata_signature_size = 0;
if (major_version_ == kBrilloMajorPayloadVersion) {
- // Write metadata signature size.
- uint32_t zero = htobe32(0);
- TEST_AND_RETURN_FALSE(writer.Write(&zero, sizeof(zero)));
- metadata_size += sizeof(zero);
+ // Metadata signature has the same size as payload signature, because they
+ // are both the same kind of signature for the same kind of hash.
+ uint32_t metadata_signature_size = htobe32(signature_blob_length);
+ TEST_AND_RETURN_FALSE(writer.Write(&metadata_signature_size,
+ sizeof(metadata_signature_size)));
+ metadata_size += sizeof(metadata_signature_size);
+ // Set correct size instead of big endian size.
+ metadata_signature_size = signature_blob_length;
}
// Write protobuf
@@ -207,6 +213,21 @@
TEST_AND_RETURN_FALSE(writer.Write(serialized_manifest.data(),
serialized_manifest.size()));
+ // Write metadata signature blob.
+ if (major_version_ == kBrilloMajorPayloadVersion &&
+ !private_key_path.empty()) {
+ brillo::Blob metadata_hash, metadata_signature;
+ TEST_AND_RETURN_FALSE(OmahaHashCalculator::RawHashOfFile(payload_file,
+ metadata_size,
+ &metadata_hash));
+ TEST_AND_RETURN_FALSE(
+ PayloadSigner::SignHashWithKeys(metadata_hash,
+ vector<string>(1, private_key_path),
+ &metadata_signature));
+ TEST_AND_RETURN_FALSE(writer.Write(metadata_signature.data(),
+ metadata_signature.size()));
+ }
+
// Append the data blobs
LOG(INFO) << "Writing final delta file data blobs...";
int blobs_fd = open(ordered_blobs_path.c_str(), O_RDONLY, 0);
@@ -223,7 +244,7 @@
TEST_AND_RETURN_FALSE(writer.Write(buf.data(), rc));
}
- // Write signature blob.
+ // Write payload signature blob.
if (!private_key_path.empty()) {
LOG(INFO) << "Signing the update...";
brillo::Blob signature_blob;
@@ -231,8 +252,8 @@
payload_file,
vector<string>(1, private_key_path),
metadata_size,
- 0,
- metadata_size + manifest_.signatures_offset(),
+ metadata_signature_size,
+ metadata_size + metadata_signature_size + manifest_.signatures_offset(),
&signature_blob));
TEST_AND_RETURN_FALSE(writer.Write(signature_blob.data(),
signature_blob.size()));
diff --git a/payload_generator/payload_signer.cc b/payload_generator/payload_signer.cc
index 714b6a4..fa8c4ba 100644
--- a/payload_generator/payload_signer.cc
+++ b/payload_generator/payload_signer.cc
@@ -423,21 +423,27 @@
bool PayloadSigner::AddSignatureToPayload(
const string& payload_path,
- const vector<brillo::Blob>& signatures,
+ const vector<brillo::Blob>& payload_signatures,
+ const vector<brillo::Blob>& metadata_signatures,
const string& signed_payload_path,
uint64_t *out_metadata_size) {
// TODO(petkov): Reduce memory usage -- the payload is manipulated in memory.
// Loads the payload and adds the signature op to it.
- brillo::Blob signature_blob;
- TEST_AND_RETURN_FALSE(ConvertSignatureToProtobufBlob(signatures,
+ brillo::Blob signature_blob, metadata_signature_blob;
+ TEST_AND_RETURN_FALSE(ConvertSignatureToProtobufBlob(payload_signatures,
&signature_blob));
+ if (!metadata_signatures.empty()) {
+ TEST_AND_RETURN_FALSE(
+ ConvertSignatureToProtobufBlob(metadata_signatures,
+ &metadata_signature_blob));
+ }
brillo::Blob payload;
uint64_t signatures_offset;
uint32_t metadata_signature_size;
TEST_AND_RETURN_FALSE(AddSignatureBlobToPayload(payload_path,
signature_blob,
- brillo::Blob(),
+ metadata_signature_blob,
&payload,
out_metadata_size,
&metadata_signature_size,
diff --git a/payload_generator/payload_signer.h b/payload_generator/payload_signer.h
index c2ba340..8691499 100644
--- a/payload_generator/payload_signer.h
+++ b/payload_generator/payload_signer.h
@@ -107,15 +107,17 @@
brillo::Blob* out_metadata_hash);
// Given an unsigned payload in |payload_path| (with no dummy signature op)
- // and the raw |signatures| updates the payload to include the signature thus
- // turning it into a signed payload. The new payload is stored in
- // |signed_payload_path|. |payload_path| and |signed_payload_path| can point
- // to the same file. Populates |out_metadata_size| with the size of the
- // metadata after adding the signature operation in the manifest.Returns true
- // on success, false otherwise.
+ // and the raw |payload_signatures| and |metadata_signatures| updates the
+ // payload to include the signature thus turning it into a signed payload. The
+ // new payload is stored in |signed_payload_path|. |payload_path| and
+ // |signed_payload_path| can point to the same file. Populates
+ // |out_metadata_size| with the size of the metadata after adding the
+ // signature operation in the manifest. Returns true on success, false
+ // otherwise.
static bool AddSignatureToPayload(
const std::string& payload_path,
- const std::vector<brillo::Blob>& signatures,
+ const std::vector<brillo::Blob>& payload_signatures,
+ const std::vector<brillo::Blob>& metadata_signatures,
const std::string& signed_payload_path,
uint64_t* out_metadata_size);
diff --git a/payload_generator/payload_signer_unittest.cc b/payload_generator/payload_signer_unittest.cc
index 991c428..42fcc63 100644
--- a/payload_generator/payload_signer_unittest.cc
+++ b/payload_generator/payload_signer_unittest.cc
@@ -23,6 +23,7 @@
#include <gtest/gtest.h>
#include "update_engine/omaha_hash_calculator.h"
+#include "update_engine/payload_constants.h"
#include "update_engine/payload_generator/payload_file.h"
#include "update_engine/payload_verifier.h"
#include "update_engine/update_metadata.pb.h"
@@ -200,4 +201,45 @@
padded_hash_data_));
}
+TEST_F(PayloadSignerTest, SkipMetadataSignatureTest) {
+ string payload_path;
+ EXPECT_TRUE(utils::MakeTempFile("payload.XXXXXX", &payload_path, nullptr));
+ ScopedPathUnlinker payload_path_unlinker(payload_path);
+
+ PayloadGenerationConfig config;
+ config.major_version = kBrilloMajorPayloadVersion;
+ PayloadFile payload;
+ EXPECT_TRUE(payload.Init(config));
+ uint64_t metadata_size;
+ EXPECT_TRUE(
+ payload.WritePayload(payload_path, "/dev/null", "", &metadata_size));
+ const vector<int> sizes = {256};
+ brillo::Blob unsigned_payload_hash, unsigned_metadata_hash;
+ EXPECT_TRUE(PayloadSigner::HashPayloadForSigning(
+ payload_path, sizes, &unsigned_payload_hash, &unsigned_metadata_hash));
+ EXPECT_TRUE(payload.WritePayload(
+ payload_path, "/dev/null", kUnittestPrivateKeyPath, &metadata_size));
+ brillo::Blob signed_payload_hash, signed_metadata_hash;
+ EXPECT_TRUE(PayloadSigner::HashPayloadForSigning(
+ payload_path, sizes, &signed_payload_hash, &signed_metadata_hash));
+ EXPECT_EQ(unsigned_payload_hash, signed_payload_hash);
+ EXPECT_EQ(unsigned_metadata_hash, signed_metadata_hash);
+}
+
+TEST_F(PayloadSignerTest, VerifySignedPayloadTest) {
+ string payload_path;
+ EXPECT_TRUE(utils::MakeTempFile("payload.XXXXXX", &payload_path, nullptr));
+ ScopedPathUnlinker payload_path_unlinker(payload_path);
+
+ PayloadGenerationConfig config;
+ config.major_version = kBrilloMajorPayloadVersion;
+ PayloadFile payload;
+ EXPECT_TRUE(payload.Init(config));
+ uint64_t metadata_size;
+ EXPECT_TRUE(payload.WritePayload(
+ payload_path, "/dev/null", kUnittestPrivateKeyPath, &metadata_size));
+ EXPECT_TRUE(PayloadSigner::VerifySignedPayload(payload_path,
+ kUnittestPublicKeyPath));
+}
+
} // namespace chromeos_update_engine