Support overwriting existing signatures when signing OTA
Bug: 258771794
Test: Run brillo_update_payload hash && sign on an already signed
payload
Change-Id: I6a108fd245672053a5e1e42a6e09ccde868c2944
diff --git a/payload_consumer/delta_performer_integration_test.cc b/payload_consumer/delta_performer_integration_test.cc
index d56c1ab..bffee8d 100644
--- a/payload_consumer/delta_performer_integration_test.cc
+++ b/payload_consumer/delta_performer_integration_test.cc
@@ -602,7 +602,7 @@
if (signature_test == kSignatureGeneratedPlaceholderMismatch) {
signature_size -= 1;
LOG(INFO) << "Inserting mismatched placeholder signature.";
- ASSERT_FALSE(InsertSignaturePlaceholder(
+ ASSERT_TRUE(InsertSignaturePlaceholder(
signature_size, state->delta_file->path(), &state->metadata_size));
return;
}
@@ -640,6 +640,7 @@
ASSERT_TRUE(payload_metadata.ParsePayloadHeader(state->delta));
state->metadata_size = payload_metadata.GetMetadataSize();
LOG(INFO) << "Metadata size: " << state->metadata_size;
+ LOG(INFO) << "Payload size: " << state->delta.size();
state->metadata_signature_size =
payload_metadata.GetMetadataSignatureSize();
LOG(INFO) << "Metadata signature size: " << state->metadata_signature_size;
diff --git a/payload_consumer/payload_metadata.h b/payload_consumer/payload_metadata.h
index f23b668..a38405d 100644
--- a/payload_consumer/payload_metadata.h
+++ b/payload_consumer/payload_metadata.h
@@ -26,7 +26,6 @@
#include <brillo/secure_blob.h>
#include "update_engine/common/error_code.h"
-#include "update_engine/common/platform_constants.h"
#include "update_engine/payload_consumer/payload_verifier.h"
#include "update_engine/update_metadata.pb.h"
diff --git a/payload_generator/payload_signer.cc b/payload_generator/payload_signer.cc
index d9f0dd7..33b8033 100644
--- a/payload_generator/payload_signer.cc
+++ b/payload_generator/payload_signer.cc
@@ -28,17 +28,14 @@
#include <brillo/data_encoding.h>
#include <openssl/err.h>
#include <openssl/pem.h>
+#include <unistd.h>
#include "update_engine/common/constants.h"
#include "update_engine/common/hash_calculator.h"
#include "update_engine/common/subprocess.h"
#include "update_engine/common/utils.h"
-#include "update_engine/payload_consumer/delta_performer.h"
-#include "update_engine/payload_consumer/payload_constants.h"
#include "update_engine/payload_consumer/payload_metadata.h"
#include "update_engine/payload_consumer/payload_verifier.h"
-#include "update_engine/payload_generator/delta_diff_generator.h"
-#include "update_engine/payload_generator/payload_file.h"
#include "update_engine/update_metadata.pb.h"
using std::string;
@@ -122,45 +119,35 @@
DeltaArchiveManifest manifest;
TEST_AND_RETURN_FALSE(payload_metadata.GetManifest(payload, &manifest));
- // Is there already a signature op in place?
- if (manifest.has_signatures_size()) {
- // The signature op is tied to the size of the signature blob, but not it's
- // contents. We don't allow the manifest to change if there is already an op
- // present, because that might invalidate previously generated
- // hashes/signatures.
- if (manifest.signatures_size() != payload_signature.size()) {
- LOG(ERROR) << "Attempt to insert different signature sized blob. "
- << "(current:" << manifest.signatures_size()
- << "new:" << payload_signature.size() << ")";
- return false;
- }
-
- LOG(INFO) << "Matching signature sizes already present.";
- } else {
- // Updates the manifest to include the signature operation.
- PayloadSigner::AddSignatureToManifest(
- payload.size() - metadata_size - metadata_signature_size,
- payload_signature.size(),
- &manifest);
-
- // Updates the payload to include the new manifest.
- string serialized_manifest;
- TEST_AND_RETURN_FALSE(manifest.AppendToString(&serialized_manifest));
- LOG(INFO) << "Updated protobuf size: " << serialized_manifest.size();
- payload.erase(payload.begin() + manifest_offset,
- payload.begin() + metadata_size);
- payload.insert(payload.begin() + manifest_offset,
- serialized_manifest.begin(),
- serialized_manifest.end());
-
- // Updates the protobuf size.
- uint64_t size_be = htobe64(serialized_manifest.size());
- memcpy(&payload[kProtobufSizeOffset], &size_be, sizeof(size_be));
- metadata_size = serialized_manifest.size() + manifest_offset;
-
- LOG(INFO) << "Updated payload size: " << payload.size();
- LOG(INFO) << "Updated metadata size: " << metadata_size;
+ // Erase existing signatures.
+ if (manifest.has_signatures_offset()) {
+ payload.resize(manifest.signatures_offset() + metadata_size +
+ metadata_signature_size);
}
+
+ // Updates the manifest to include the signature operation.
+ PayloadSigner::AddSignatureToManifest(
+ payload.size() - metadata_size - metadata_signature_size,
+ payload_signature.size(),
+ &manifest);
+
+ // Updates the payload to include the new manifest.
+ string serialized_manifest;
+ TEST_AND_RETURN_FALSE(manifest.AppendToString(&serialized_manifest));
+ LOG(INFO) << "Updated protobuf size: " << serialized_manifest.size();
+ payload.erase(payload.begin() + manifest_offset,
+ payload.begin() + metadata_size);
+ payload.insert(payload.begin() + manifest_offset,
+ serialized_manifest.begin(),
+ serialized_manifest.end());
+
+ // Updates the protobuf size.
+ uint64_t size_be = htobe64(serialized_manifest.size());
+ memcpy(&payload[kProtobufSizeOffset], &size_be, sizeof(size_be));
+ metadata_size = serialized_manifest.size() + manifest_offset;
+
+ LOG(INFO) << "Updated payload size: " << payload.size();
+ LOG(INFO) << "Updated metadata size: " << metadata_size;
uint64_t signatures_offset =
metadata_size + metadata_signature_size + manifest.signatures_offset();
LOG(INFO) << "Signature Blob Offset: " << signatures_offset;
@@ -471,6 +458,13 @@
&signatures_offset));
LOG(INFO) << "Signed payload size: " << payload.size();
+ const auto ret =
+ HANDLE_EINTR(truncate(signed_payload_path.c_str(), payload.size()));
+ if (ret < 0) {
+ PLOG(ERROR) << "Failed to truncate file " << signed_payload_path
+ << " to size " << payload.size();
+ return false;
+ }
TEST_AND_RETURN_FALSE(utils::WriteFile(
signed_payload_path.c_str(), payload.data(), payload.size()));
return true;
diff --git a/payload_generator/payload_signer_unittest.cc b/payload_generator/payload_signer_unittest.cc
index 2bfc820..96e4431 100644
--- a/payload_generator/payload_signer_unittest.cc
+++ b/payload_generator/payload_signer_unittest.cc
@@ -158,33 +158,6 @@
EXPECT_FALSE(payload_verifier->VerifySignature(signature, hash_data_));
}
-TEST_F(PayloadSignerTest, SkipMetadataSignatureTest) {
- ScopedTempFile payload_file("payload.XXXXXX");
- PayloadGenerationConfig config;
- config.version.major = kBrilloMajorPayloadVersion;
- PayloadFile payload;
- EXPECT_TRUE(payload.Init(config));
- uint64_t metadata_size;
- EXPECT_TRUE(payload.WritePayload(
- payload_file.path(), "/dev/null", "", &metadata_size));
- const vector<size_t> sizes = {256};
- brillo::Blob unsigned_payload_hash, unsigned_metadata_hash;
- EXPECT_TRUE(PayloadSigner::HashPayloadForSigning(payload_file.path(),
- sizes,
- &unsigned_payload_hash,
- &unsigned_metadata_hash));
- EXPECT_TRUE(
- payload.WritePayload(payload_file.path(),
- "/dev/null",
- GetBuildArtifactsPath(kUnittestPrivateKeyPath),
- &metadata_size));
- brillo::Blob signed_payload_hash, signed_metadata_hash;
- EXPECT_TRUE(PayloadSigner::HashPayloadForSigning(
- payload_file.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) {
ScopedTempFile payload_file("payload.XXXXXX");
PayloadGenerationConfig config;