PayloadVerifier should not depend on DeltaPerformer to load payload.
The implementation in DeltaPerformer is designed for situation that payload
might only partially available, but in PayloadVerifier we already have the
whole payload. So I implemented LoadPayload in PayloadSigner which logic is
simpler and supports both version 1 and 2.
VerifySignedPayload is also moved to PayloadSigner since it's not used in
update engine daemon.
This patch also fixed wrong metadata size out in version 2 and misspelling
of metadata in WritePayload in PayloadFile.
Bug: 23981164
TEST=unit test added.
Change-Id: Id1917fc891dbf2075978a273d1a4ee3c4ecf0571
diff --git a/payload_generator/generate_delta_main.cc b/payload_generator/generate_delta_main.cc
index 5924f06..9c3655a 100644
--- a/payload_generator/generate_delta_main.cc
+++ b/payload_generator/generate_delta_main.cc
@@ -35,7 +35,6 @@
#include "update_engine/payload_generator/delta_diff_utils.h"
#include "update_engine/payload_generator/payload_generation_config.h"
#include "update_engine/payload_generator/payload_signer.h"
-#include "update_engine/payload_verifier.h"
#include "update_engine/prefs.h"
#include "update_engine/terminator.h"
#include "update_engine/update_metadata.pb.h"
@@ -184,7 +183,7 @@
<< "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));
+ CHECK(PayloadSigner::VerifySignedPayload(in_file, public_key));
LOG(INFO) << "Done verifying signed payload.";
}
diff --git a/payload_generator/payload_file.cc b/payload_generator/payload_file.cc
index 670027b..15dd35e 100644
--- a/payload_generator/payload_file.cc
+++ b/payload_generator/payload_file.cc
@@ -25,7 +25,6 @@
#include "update_engine/omaha_hash_calculator.h"
#include "update_engine/payload_constants.h"
#include "update_engine/payload_generator/annotated_operation.h"
-#include "update_engine/payload_generator/delta_diff_generator.h"
#include "update_engine/payload_generator/delta_diff_utils.h"
#include "update_engine/payload_generator/payload_signer.h"
@@ -101,7 +100,7 @@
bool PayloadFile::WritePayload(const string& payload_file,
const string& data_blobs_path,
const string& private_key_path,
- uint64_t* medatata_size_out) {
+ uint64_t* metadata_size_out) {
// Reorder the data blobs with the manifest_.
string ordered_blobs_path;
TEST_AND_RETURN_FALSE(utils::MakeTempFile(
@@ -167,13 +166,17 @@
TEST_AND_RETURN_FALSE(
PayloadSigner::SignatureBlobLength(vector<string>(1, private_key_path),
&signature_blob_length));
- AddSignatureOp(next_blob_offset, signature_blob_length, &manifest_);
+ PayloadSigner::AddSignatureOp(next_blob_offset, signature_blob_length,
+ &manifest_);
}
// Serialize protobuf
string serialized_manifest;
TEST_AND_RETURN_FALSE(manifest_.AppendToString(&serialized_manifest));
+ uint64_t metadata_size =
+ sizeof(kDeltaMagic) + 2 * sizeof(uint64_t) + serialized_manifest.size();
+
LOG(INFO) << "Writing final delta file header...";
DirectFileWriter writer;
TEST_AND_RETURN_FALSE_ERRNO(writer.Open(payload_file.c_str(),
@@ -195,6 +198,7 @@
// Write metadata signature size.
uint32_t zero = htobe32(0);
TEST_AND_RETURN_FALSE(writer.Write(&zero, sizeof(zero)));
+ metadata_size += sizeof(zero);
}
// Write protobuf
@@ -231,9 +235,8 @@
signature_blob.size()));
}
- *medatata_size_out =
- sizeof(kDeltaMagic) + 2 * sizeof(uint64_t) + serialized_manifest.size();
- ReportPayloadUsage(*medatata_size_out);
+ ReportPayloadUsage(metadata_size);
+ *metadata_size_out = metadata_size;
return true;
}
@@ -317,24 +320,4 @@
100.0, static_cast<intmax_t>(total_size), "", "<total>");
}
-void AddSignatureOp(uint64_t signature_blob_offset,
- uint64_t signature_blob_length,
- DeltaArchiveManifest* manifest) {
- LOG(INFO) << "Making room for signature in file";
- manifest->set_signatures_offset(signature_blob_offset);
- LOG(INFO) << "set? " << manifest->has_signatures_offset();
- // Add a dummy op at the end to appease older clients
- InstallOperation* dummy_op = manifest->add_kernel_install_operations();
- dummy_op->set_type(InstallOperation::REPLACE);
- dummy_op->set_data_offset(signature_blob_offset);
- manifest->set_signatures_offset(signature_blob_offset);
- dummy_op->set_data_length(signature_blob_length);
- manifest->set_signatures_size(signature_blob_length);
- Extent* dummy_extent = dummy_op->add_dst_extents();
- // Tell the dummy op to write this data to a big sparse hole
- dummy_extent->set_start_block(kSparseHole);
- dummy_extent->set_num_blocks((signature_blob_length + kBlockSize - 1) /
- kBlockSize);
-}
-
} // namespace chromeos_update_engine
diff --git a/payload_generator/payload_file.h b/payload_generator/payload_file.h
index b586d73..51584d6 100644
--- a/payload_generator/payload_file.h
+++ b/payload_generator/payload_file.h
@@ -52,7 +52,7 @@
bool WritePayload(const std::string& payload_file,
const std::string& data_blobs_path,
const std::string& private_key_path,
- uint64_t* medatata_size_out);
+ uint64_t* metadata_size_out);
private:
FRIEND_TEST(PayloadFileTest, ReorderBlobsTest);
@@ -98,12 +98,6 @@
std::vector<Partition> part_vec_;
};
-// Adds a dummy operation that points to a signature blob located at the
-// specified offset/length.
-void AddSignatureOp(uint64_t signature_blob_offset,
- uint64_t signature_blob_length,
- DeltaArchiveManifest* manifest);
-
} // namespace chromeos_update_engine
#endif // UPDATE_ENGINE_PAYLOAD_GENERATOR_PAYLOAD_FILE_H_
diff --git a/payload_generator/payload_signer.cc b/payload_generator/payload_signer.cc
index 25ead8a..f5cd02e 100644
--- a/payload_generator/payload_signer.cc
+++ b/payload_generator/payload_signer.cc
@@ -24,7 +24,10 @@
#include <brillo/data_encoding.h>
#include <openssl/pem.h>
+#include "update_engine/delta_performer.h"
#include "update_engine/omaha_hash_calculator.h"
+#include "update_engine/payload_constants.h"
+#include "update_engine/payload_generator/delta_diff_generator.h"
#include "update_engine/payload_generator/payload_file.h"
#include "update_engine/payload_verifier.h"
#include "update_engine/subprocess.h"
@@ -83,9 +86,10 @@
// Loads the payload.
brillo::Blob payload;
DeltaArchiveManifest manifest;
- uint64_t metadata_size;
- TEST_AND_RETURN_FALSE(PayloadVerifier::LoadPayload(
- payload_path, &payload, &manifest, &metadata_size));
+ uint64_t metadata_size, major_version;
+ uint32_t metadata_signature_size;
+ TEST_AND_RETURN_FALSE(PayloadSigner::LoadPayload(payload_path, &payload,
+ &manifest, &major_version, &metadata_size, &metadata_signature_size));
// Is there already a signature op in place?
if (manifest.has_signatures_size()) {
@@ -103,9 +107,9 @@
LOG(INFO) << "Matching signature sizes already present.";
} else {
// Updates the manifest to include the signature operation.
- AddSignatureOp(payload.size() - metadata_size,
- signature_blob_size,
- &manifest);
+ PayloadSigner::AddSignatureOp(payload.size() - metadata_size,
+ signature_blob_size,
+ &manifest);
// Updates the payload to include the new manifest.
string serialized_manifest;
@@ -134,6 +138,99 @@
}
} // namespace
+void PayloadSigner::AddSignatureOp(uint64_t signature_blob_offset,
+ uint64_t signature_blob_length,
+ DeltaArchiveManifest* manifest) {
+ LOG(INFO) << "Making room for signature in file";
+ manifest->set_signatures_offset(signature_blob_offset);
+ LOG(INFO) << "set? " << manifest->has_signatures_offset();
+ // Add a dummy op at the end to appease older clients
+ InstallOperation* dummy_op = manifest->add_kernel_install_operations();
+ dummy_op->set_type(InstallOperation::REPLACE);
+ dummy_op->set_data_offset(signature_blob_offset);
+ manifest->set_signatures_offset(signature_blob_offset);
+ dummy_op->set_data_length(signature_blob_length);
+ manifest->set_signatures_size(signature_blob_length);
+ Extent* dummy_extent = dummy_op->add_dst_extents();
+ // Tell the dummy op to write this data to a big sparse hole
+ dummy_extent->set_start_block(kSparseHole);
+ dummy_extent->set_num_blocks((signature_blob_length + kBlockSize - 1) /
+ kBlockSize);
+}
+
+bool PayloadSigner::LoadPayload(const std::string& payload_path,
+ brillo::Blob* out_payload,
+ DeltaArchiveManifest* out_manifest,
+ uint64_t* out_major_version,
+ uint64_t* out_metadata_size,
+ uint32_t* out_metadata_signature_size) {
+ brillo::Blob payload;
+ TEST_AND_RETURN_FALSE(utils::ReadFile(payload_path, &payload));
+ TEST_AND_RETURN_FALSE(payload.size() >=
+ DeltaPerformer::kMaxPayloadHeaderSize);
+ const uint8_t* read_pointer = payload.data();
+ TEST_AND_RETURN_FALSE(
+ memcmp(read_pointer, kDeltaMagic, sizeof(kDeltaMagic)) == 0);
+ read_pointer += sizeof(kDeltaMagic);
+
+ uint64_t major_version;
+ memcpy(&major_version, read_pointer, sizeof(major_version));
+ read_pointer += sizeof(major_version);
+ major_version = be64toh(major_version);
+ TEST_AND_RETURN_FALSE(major_version == kChromeOSMajorPayloadVersion ||
+ major_version == kBrilloMajorPayloadVersion);
+ if (out_major_version)
+ *out_major_version = major_version;
+
+ uint64_t manifest_size = 0;
+ memcpy(&manifest_size, read_pointer, sizeof(manifest_size));
+ read_pointer += sizeof(manifest_size);
+ manifest_size = be64toh(manifest_size);
+
+ uint32_t metadata_signature_size = 0;
+ if (major_version == kBrilloMajorPayloadVersion) {
+ memcpy(&metadata_signature_size, read_pointer,
+ sizeof(metadata_signature_size));
+ read_pointer += sizeof(metadata_signature_size);
+ metadata_signature_size = be32toh(metadata_signature_size);
+ }
+ if (out_metadata_signature_size)
+ *out_metadata_signature_size = metadata_signature_size;
+
+ *out_metadata_size = read_pointer - payload.data() + manifest_size;
+ TEST_AND_RETURN_FALSE(payload.size() >= *out_metadata_size);
+ if (out_manifest)
+ TEST_AND_RETURN_FALSE(
+ out_manifest->ParseFromArray(read_pointer, manifest_size));
+ *out_payload = std::move(payload);
+ return true;
+}
+
+bool PayloadSigner::VerifySignedPayload(const string& payload_path,
+ const string& public_key_path) {
+ brillo::Blob payload;
+ DeltaArchiveManifest manifest;
+ uint64_t metadata_size, major_version;
+ uint32_t metadata_signature_size;
+ TEST_AND_RETURN_FALSE(LoadPayload(payload_path, &payload, &manifest,
+ &major_version, &metadata_size, &metadata_signature_size));
+ TEST_AND_RETURN_FALSE(manifest.has_signatures_offset() &&
+ manifest.has_signatures_size());
+ CHECK_EQ(payload.size(),
+ metadata_size + manifest.signatures_offset() +
+ manifest.signatures_size());
+ brillo::Blob signature_blob(
+ payload.begin() + metadata_size + manifest.signatures_offset(),
+ payload.end());
+ brillo::Blob hash;
+ TEST_AND_RETURN_FALSE(OmahaHashCalculator::RawHashOfBytes(
+ payload.data(), metadata_size + manifest.signatures_offset(), &hash));
+ TEST_AND_RETURN_FALSE(PayloadVerifier::PadRSA2048SHA256Hash(&hash));
+ TEST_AND_RETURN_FALSE(PayloadVerifier::VerifySignature(
+ signature_blob, public_key_path, hash));
+ return true;
+}
+
bool PayloadSigner::SignHash(const brillo::Blob& hash,
const string& private_key_path,
brillo::Blob* out_signature) {
diff --git a/payload_generator/payload_signer.h b/payload_generator/payload_signer.h
index 11116a3..d702ccc 100644
--- a/payload_generator/payload_signer.h
+++ b/payload_generator/payload_signer.h
@@ -32,6 +32,31 @@
class PayloadSigner {
public:
+ // Reads the payload from the given |payload_path| into the |out_payload|
+ // vector. It also parses the manifest protobuf in the payload and returns it
+ // in |out_manifest| if not null, along with the major version of the payload
+ // in |out_major_version| if not null, the size of the entire metadata in
+ // |out_metadata_size| and the size of metadata signature in
+ // |out_metadata_signature_size| if not null.
+ static bool LoadPayload(const std::string& payload_path,
+ brillo::Blob* out_payload,
+ DeltaArchiveManifest* out_manifest,
+ uint64_t* out_major_version,
+ uint64_t* out_metadata_size,
+ uint32_t* out_metadata_signature_size);
+
+ // Returns true if the payload in |payload_path| is signed and its hash can be
+ // verified using the public key in |public_key_path| with the signature
+ // of a given version in the signature blob. Returns false otherwise.
+ static bool VerifySignedPayload(const std::string& payload_path,
+ const std::string& public_key_path);
+
+ // Adds a dummy operation that points to a signature blob located at the
+ // specified offset/length.
+ static void AddSignatureOp(uint64_t signature_blob_offset,
+ uint64_t signature_blob_length,
+ DeltaArchiveManifest* manifest);
+
// Given a raw |hash| and a private key in |private_key_path| calculates the
// raw signature in |out_signature|. Returns true on success, false otherwise.
static bool SignHash(const brillo::Blob& hash,
diff --git a/payload_generator/payload_signer_unittest.cc b/payload_generator/payload_signer_unittest.cc
index 9ef6614..7763411 100644
--- a/payload_generator/payload_signer_unittest.cc
+++ b/payload_generator/payload_signer_unittest.cc
@@ -22,6 +22,7 @@
#include <base/logging.h>
#include <gtest/gtest.h>
+#include "update_engine/payload_generator/payload_file.h"
#include "update_engine/payload_verifier.h"
#include "update_engine/update_metadata.pb.h"
#include "update_engine/utils.h"
@@ -119,9 +120,40 @@
PayloadVerifier::PadRSA2048SHA256Hash(&padded_hash_data_);
}
+ void DoWriteAndLoadPayloadTest(const PayloadGenerationConfig& config) {
+ PayloadFile payload;
+ payload.Init(config);
+ string payload_path;
+ EXPECT_TRUE(utils::MakeTempFile("payload.XXXXXX", &payload_path, nullptr));
+ ScopedPathUnlinker payload_path_unlinker(payload_path);
+ uint64_t metadata_size;
+ EXPECT_TRUE(
+ payload.WritePayload(payload_path, "/dev/null", "", &metadata_size));
+ brillo::Blob payload_blob;
+ DeltaArchiveManifest manifest;
+ uint64_t load_metadata_size, load_major_version;
+ EXPECT_TRUE(PayloadSigner::LoadPayload(payload_path, &payload_blob,
+ &manifest, &load_major_version, &load_metadata_size, nullptr));
+ EXPECT_EQ(utils::FileSize(payload_path), payload_blob.size());
+ EXPECT_EQ(config.major_version, load_major_version);
+ EXPECT_EQ(metadata_size, load_metadata_size);
+ }
+
brillo::Blob padded_hash_data_{std::begin(kDataHash), std::end(kDataHash)};
};
+TEST_F(PayloadSignerTest, LoadPayloadV1Test) {
+ PayloadGenerationConfig config;
+ config.major_version = kChromeOSMajorPayloadVersion;
+ DoWriteAndLoadPayloadTest(config);
+}
+
+TEST_F(PayloadSignerTest, LoadPayloadV2Test) {
+ PayloadGenerationConfig config;
+ config.major_version = kBrilloMajorPayloadVersion;
+ DoWriteAndLoadPayloadTest(config);
+}
+
TEST_F(PayloadSignerTest, SignSimpleTextTest) {
brillo::Blob signature_blob;
SignSampleData(&signature_blob, {kUnittestPrivateKeyPath});