Make hash checks mandatory for HTTP downloads.
Currently we've made all the checks for metadata size, metadata signature
and operation hashes as optional. While they are still optional if we use
HTTPS for downloading the payload, we want to make them mandatory in case
of HTTP, so as to support HTTP downloads.
In this CL, we make these checks mandatory if the Omaha response has a
HTTP URL. This will not affect any scenarios of our test team because they
always use HTTPS URLs for payload URLs. But this would break the dev tools
and our hardware test lab scenarios because they use HTTP URLs and do not
generate the required manifest signature yet. So we waive this requirement
for dev/test images even though they use HTTP.
This CL will not have any effect until we decide to add a HTTP rule in
Omaha, which serves as a safety knob till we are confident with our
testing.
BUG=chromium-os:36808
TEST=Existing unit tests pass. Added new unit tests for most new code.
TEST=Ran manual tests on ZGB for every type of hash failure for HTTP.
TEST=Tested image_to_live to make sure hash checks are waived as expected.
Change-Id: I8c4408e3052635ccf4bee0c848781733c1f8e984
Reviewed-on: https://gerrit.chromium.org/gerrit/39293
Reviewed-by: Gaurav Shah <gauravsh@chromium.org>
Commit-Ready: Jay Srinivasan <jaysri@chromium.org>
Reviewed-by: Jay Srinivasan <jaysri@chromium.org>
Tested-by: Jay Srinivasan <jaysri@chromium.org>
diff --git a/delta_diff_generator.cc b/delta_diff_generator.cc
index 26f3133..94487c1 100644
--- a/delta_diff_generator.cc
+++ b/delta_diff_generator.cc
@@ -1321,7 +1321,8 @@
const string& old_kernel_part,
const string& new_kernel_part,
const string& output_path,
- const string& private_key_path) {
+ const string& private_key_path,
+ uint64_t* metadata_size) {
int old_image_block_count = 0, old_image_block_size = 0;
int new_image_block_count = 0, new_image_block_size = 0;
TEST_AND_RETURN_FALSE(utils::GetFilesystemSize(new_image,
@@ -1560,11 +1561,12 @@
signature_blob.size()));
}
- int64_t manifest_metadata_size =
+ *metadata_size =
strlen(kDeltaMagic) + 2 * sizeof(uint64_t) + serialized_manifest.size();
- ReportPayloadUsage(manifest, manifest_metadata_size, op_name_map);
+ ReportPayloadUsage(manifest, *metadata_size, op_name_map);
- LOG(INFO) << "All done. Successfully created delta file.";
+ LOG(INFO) << "All done. Successfully created delta file with "
+ << "metadata size = " << *metadata_size;
return true;
}
diff --git a/delta_diff_generator.h b/delta_diff_generator.h
index f4717e7..8ecac63 100644
--- a/delta_diff_generator.h
+++ b/delta_diff_generator.h
@@ -61,7 +61,8 @@
// private_key_path points to a private key used to sign the update.
// Pass empty string to not sign the update.
// output_path is the filename where the delta update should be written.
- // Returns true on success.
+ // Returns true on success. Also writes the size of the metadata into
+ // |metadata_size|.
static bool GenerateDeltaUpdateFile(const std::string& old_root,
const std::string& old_image,
const std::string& new_root,
@@ -69,7 +70,8 @@
const std::string& old_kernel_part,
const std::string& new_kernel_part,
const std::string& output_path,
- const std::string& private_key_path);
+ const std::string& private_key_path,
+ uint64_t* metadata_size);
// These functions are public so that the unit tests can access them:
@@ -249,7 +251,7 @@
DeltaArchiveManifest* manifest);
private:
- // This should never be constructed
+ // This should never be constructed
DISALLOW_IMPLICIT_CONSTRUCTORS(DeltaDiffGenerator);
};
diff --git a/delta_performer.cc b/delta_performer.cc
index e771da0..5412a0f 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -247,20 +247,14 @@
// even before waiting for that many number of bytes to be downloaded
// in the payload. This will prevent any attack which relies on us downloading
// data beyond the expected metadata size.
- if (install_plan_->metadata_size > 0 &&
- install_plan_->metadata_size != *metadata_size) {
- LOG(ERROR) << "Invalid metadata size. Expected = "
- << install_plan_->metadata_size
- << "Actual = " << *metadata_size;
- // Send a UMA Stat here to help with the decision to enforce
- // this check in a future release, as mentioned below.
- SendUmaStat(kActionCodeDownloadInvalidMetadataSize);
-
- // TODO(jaysri): VALIDATION: Initially we don't want to make this a fatal
- // error. But in the next release, we should uncomment the lines below
- // and remove the SendUmaStat call above.
- // *error = kActionCodeDownloadInvalidManifest;
- // return kMetadataParseError;
+ if (install_plan_->hash_checks_mandatory) {
+ if (install_plan_->metadata_size != *metadata_size) {
+ LOG(ERROR) << "Mandatory metadata size in Omaha response ("
+ << install_plan_->metadata_size << ") is missing/incorrect."
+ << ", Actual = " << *metadata_size;
+ *error = kActionCodeDownloadInvalidMetadataSize;
+ return kMetadataParseError;
+ }
}
// Now that we have validated the metadata size, we should wait for the full
@@ -273,23 +267,30 @@
// here. This is logged here (after we received the full metadata data) so
// that we just log once (instead of logging n times) if it takes n
// DeltaPerformer::Write calls to download the full manifest.
- if (install_plan_->metadata_size == 0)
- LOG(WARNING) << "No metadata size specified in Omaha. "
- << "Trusting metadata size in payload = " << *metadata_size;
- else
+ if (install_plan_->metadata_size == *metadata_size) {
LOG(INFO) << "Manifest size in payload matches expected value from Omaha";
+ } else {
+ // For mandatory-cases, we'd have already returned a kMetadataParseError
+ // above. We'll be here only for non-mandatory cases. Just send a UMA stat.
+ LOG(WARNING) << "Ignoring missing/incorrect metadata size ("
+ << install_plan_->metadata_size
+ << ") in Omaha response as validation is not mandatory. "
+ << "Trusting metadata size in payload = " << *metadata_size;
+ SendUmaStat(kActionCodeDownloadInvalidMetadataSize);
+ }
// We have the full metadata in |payload|. Verify its integrity
// and authenticity based on the information we have in Omaha response.
*error = ValidateMetadataSignature(&payload[0], *metadata_size);
if (*error != kActionCodeSuccess) {
- // Send a UMA Stat here to help with the decision to enforce
- // this check in a future release, as mentioned below.
- SendUmaStat(*error);
+ if (install_plan_->hash_checks_mandatory) {
+ LOG(ERROR) << "Mandatory metadata signature validation failed";
+ return kMetadataParseError;
+ }
- // TODO(jaysri): VALIDATION: Initially we don't want to make this a fatal
- // error. But in the next release, we should remove the line below and
- // return an error. We should also remove the SendUmaStat call above.
+ // For non-mandatory cases, just send a UMA stat.
+ LOG(WARNING) << "Ignoring metadata signature validation failures";
+ SendUmaStat(*error);
*error = kActionCodeSuccess;
}
@@ -359,25 +360,24 @@
// Validate the operation only if the metadata signature is present.
// Otherwise, keep the old behavior. This serves as a knob to disable
// the validation logic in case we find some regression after rollout.
+ // NOTE: If hash checks are mandatory and if metadata_signature is empty,
+ // we would have already failed in ParsePayloadMetadata method and thus not
+ // even be here. So no need to handle that case again here.
if (!install_plan_->metadata_signature.empty()) {
// Note: Validate must be called only if CanPerformInstallOperation is
// called. Otherwise, we might be failing operations before even if there
// isn't sufficient data to compute the proper hash.
*error = ValidateOperationHash(op, should_log);
if (*error != kActionCodeSuccess) {
- // Cannot proceed further as operation hash is invalid.
- // Higher level code will take care of retrying appropriately.
+ if (install_plan_->hash_checks_mandatory) {
+ LOG(ERROR) << "Mandatory operation hash check failed";
+ return false;
+ }
- // Send a UMA stat to indicate that an operation hash failed to be
- // validated as expected.
+ // For non-mandatory cases, just send a UMA stat.
+ LOG(WARNING) << "Ignoring operation validation errors";
SendUmaStat(*error);
-
- // TODO(jaysri): VALIDATION: For now, we don't treat this as fatal.
- // But once we're confident that the new code works fine in the field,
- // we should uncomment the line below. We should also remove the
- // SendUmaStat call above.
- // return false;
- LOG(INFO) << "Ignoring operation validation errors for now";
+ *error = kActionCodeSuccess;
}
}
@@ -697,20 +697,14 @@
const char* metadata, uint64_t metadata_size) {
if (install_plan_->metadata_signature.empty()) {
- // TODO(jaysri): If this is not present, we cannot validate the manifest.
- // This should never happen in normal circumstances, but this can be used
- // as a release-knob to turn off the new code path that verify
- // per-operation hashes. So, for now, we should not treat this as a
- // failure. Once we are confident this path is bug-free, we should treat
- // this as a failure so that we remain robust even if the connection to
- // Omaha is subjected to any SSL attack. Once we make this an error, we
- // should remove the SendUmaStat call below.
+ if (install_plan_->hash_checks_mandatory) {
+ LOG(ERROR) << "Missing mandatory metadata signature in Omaha response";
+ return kActionCodeDownloadMetadataSignatureMissingError;
+ }
+
+ // For non-mandatory cases, just send a UMA stat.
LOG(WARNING) << "Cannot validate metadata as the signature is empty";
-
- // Send a UMA stat here so we're aware of any man-in-the-middle attempts to
- // bypass these checks.
SendUmaStat(kActionCodeDownloadMetadataSignatureMissingError);
-
return kActionCodeSuccess;
}
@@ -766,20 +760,15 @@
// Operations that do not have any data blob won't have any operation hash
// either. So, these operations are always considered validated since the
// metadata that contains all the non-data-blob portions of the operation
- // has already been validated.
+ // has already been validated. This is true for both HTTP and HTTPS cases.
return kActionCodeSuccess;
}
- // Send a UMA stat here so we're aware of any man-in-the-middle attempts to
- // bypass these checks.
- SendUmaStat(kActionCodeDownloadOperationHashMissingError);
-
- // TODO(jaysri): VALIDATION: no hash is present for the operation. This
- // shouldn't happen normally for any client that has this code, because the
+ // No hash is present for an operation that has data blobs. This shouldn't
+ // happen normally for any client that has this code, because the
// corresponding update should have been produced with the operation
- // hashes. But if it happens it's likely that we've turned this feature off
- // in Omaha rule for some reason. Once we make these hashes mandatory, we
- // should return an error here. We should also remove the SendUmaStat call.
+ // hashes. So if it happens it means either we've turned operation hash
+ // generation off in DeltaDiffGenerator or it's a regression of some sort.
// One caveat though: The last operation is a dummy signature operation
// that doesn't have a hash at the time the manifest is created. So we
// should not complaint about that operation. This operation can be
@@ -789,9 +778,16 @@
LOG(INFO) << "Skipping hash verification for signature operation "
<< next_operation_num_ + 1;
} else {
- // TODO(jaysri): Uncomment this logging after fixing dev server
- // LOG(WARNING) << "Cannot validate operation " << next_operation_num_ + 1
- // << " as no expected hash present";
+ if (install_plan_->hash_checks_mandatory) {
+ LOG(ERROR) << "Missing mandatory operation hash for operation "
+ << next_operation_num_ + 1;
+ return kActionCodeDownloadOperationHashMissingError;
+ }
+
+ // For non-mandatory cases, just send a UMA stat.
+ LOG(WARNING) << "Cannot validate operation " << next_operation_num_ + 1
+ << " as there's no operation hash in manifest";
+ SendUmaStat(kActionCodeDownloadOperationHashMissingError);
}
return kActionCodeSuccess;
}
diff --git a/delta_performer_unittest.cc b/delta_performer_unittest.cc
index 7174780..f5493c2 100644
--- a/delta_performer_unittest.cc
+++ b/delta_performer_unittest.cc
@@ -41,36 +41,58 @@
extern const char* kUnittestPrivateKey2Path;
extern const char* kUnittestPublicKey2Path;
+static const size_t kBlockSize = 4096;
+static const char* kBogusMetadataSignature1 = "awSFIUdUZz2VWFiR+ku0Pj00V7bPQPQFYQSXjEXr3vaw3TE4xHV5CraY3/YrZpBvJ5z4dSBskoeuaO1TNC/S6E05t+yt36tE4Fh79tMnJ/z9fogBDXWgXLEUyG78IEQrYH6/eBsQGT2RJtBgXIXbZ9W+5G9KmGDoPOoiaeNsDuqHiBc/58OFsrxskH8E6vMSBmMGGk82mvgzic7ApcoURbCGey1b3Mwne/hPZ/bb9CIyky8Og9IfFMdL2uAweOIRfjoTeLYZpt+WN65Vu7jJ0cQN8e1y+2yka5112wpRf/LLtPgiAjEZnsoYpLUd7CoVpLRtClp97kN2+tXGNBQqkA==";
+
+static const int kDefaultKernelSize = 4096; // Something small for a test
+static const char* kNewDataString = "This is new data.";
+
namespace {
-const size_t kBlockSize = 4096;
-} // namespace {}
+struct DeltaState {
+ string a_img;
+ string b_img;
+ int image_size;
+ string delta_path;
+ uint64_t metadata_size;
-class DeltaPerformerTest : public ::testing::Test { };
+ string old_kernel;
+ vector<char> old_kernel_data;
-TEST(DeltaPerformerTest, ExtentsToByteStringTest) {
- uint64_t test[] = {1, 1, 4, 2, kSparseHole, 1, 0, 1};
- COMPILE_ASSERT(arraysize(test) % 2 == 0, array_size_uneven);
- const uint64_t block_size = 4096;
- const uint64_t file_length = 5 * block_size - 13;
+ string new_kernel;
+ vector<char> new_kernel_data;
- google::protobuf::RepeatedPtrField<Extent> extents;
- for (size_t i = 0; i < arraysize(test); i += 2) {
- Extent* extent = extents.Add();
- extent->set_start_block(test[i]);
- extent->set_num_blocks(test[i + 1]);
- }
+ // The in-memory copy of delta file.
+ vector<char> delta;
+};
- string expected_output = "4096:4096,16384:8192,-1:4096,0:4083";
- string actual_output;
- EXPECT_TRUE(DeltaPerformer::ExtentsToBsdiffPositionsString(extents,
- block_size,
- file_length,
- &actual_output));
- EXPECT_EQ(expected_output, actual_output);
-}
+enum SignatureTest {
+ kSignatureNone, // No payload signing.
+ kSignatureGenerator, // Sign the payload at generation time.
+ kSignatureGenerated, // Sign the payload after it's generated.
+ kSignatureGeneratedShell, // Sign the generated payload through shell cmds.
+ kSignatureGeneratedShellBadKey, // Sign with a bad key through shell cmds.
+ kSignatureGeneratedShellRotateCl1, // Rotate key, test client v1
+ kSignatureGeneratedShellRotateCl2, // Rotate key, test client v2
+};
-void CompareFilesByBlock(const string& a_file, const string& b_file) {
+// Different options that determine what we should fill into the
+// install_plan.metadata_signature to simulate the contents received in the
+// Omaha response.
+enum MetadataSignatureTest {
+ kEmptyMetadataSignature,
+ kInvalidMetadataSignature,
+ kValidMetadataSignature,
+};
+
+enum OperationHashTest {
+ kInvalidOperationData,
+ kValidOperationData,
+};
+
+} // namespace {}
+
+static void CompareFilesByBlock(const string& a_file, const string& b_file) {
vector<char> a_data, b_data;
EXPECT_TRUE(utils::ReadFile(a_file, &a_data)) << "file failed: " << a_file;
EXPECT_TRUE(utils::ReadFile(b_file, &b_data)) << "file failed: " << b_file;
@@ -85,8 +107,7 @@
}
}
-namespace {
-bool WriteSparseFile(const string& path, off_t size) {
+static bool WriteSparseFile(const string& path, off_t size) {
int fd = open(path.c_str(), O_CREAT | O_TRUNC | O_WRONLY, 0644);
TEST_AND_RETURN_FALSE_ERRNO(fd >= 0);
ScopedFdCloser fd_closer(&fd);
@@ -96,20 +117,8 @@
TEST_AND_RETURN_FALSE_ERRNO(return_code == 0);
return true;
}
-} // namespace {}
-namespace {
-enum SignatureTest {
- kSignatureNone, // No payload signing.
- kSignatureGenerator, // Sign the payload at generation time.
- kSignatureGenerated, // Sign the payload after it's generated.
- kSignatureGeneratedShell, // Sign the generated payload through shell cmds.
- kSignatureGeneratedShellBadKey, // Sign with a bad key through shell cmds.
- kSignatureGeneratedShellRotateCl1, // Rotate key, test client v1
- kSignatureGeneratedShellRotateCl2, // Rotate key, test client v2
-};
-
-size_t GetSignatureSize(const string& private_key_path) {
+static size_t GetSignatureSize(const string& private_key_path) {
const vector<char> data(1, 'x');
vector<char> hash;
EXPECT_TRUE(OmahaHashCalculator::RawHashOfData(data, &hash));
@@ -120,7 +129,8 @@
return signature.size();
}
-void SignGeneratedPayload(const string& payload_path) {
+static void SignGeneratedPayload(const string& payload_path,
+ uint64_t* out_metadata_size) {
int signature_size = GetSignatureSize(kUnittestPrivateKeyPath);
vector<char> hash;
ASSERT_TRUE(PayloadSigner::HashPayloadForSigning(
@@ -134,15 +144,16 @@
ASSERT_TRUE(PayloadSigner::AddSignatureToPayload(
payload_path,
vector<vector<char> >(1, signature),
- payload_path));
+ payload_path,
+ out_metadata_size));
EXPECT_TRUE(PayloadSigner::VerifySignedPayload(
payload_path,
kUnittestPublicKeyPath,
kSignatureMessageOriginalVersion));
}
-void SignGeneratedShellPayload(SignatureTest signature_test,
- const string& payload_path) {
+static void SignGeneratedShellPayload(SignatureTest signature_test,
+ const string& payload_path) {
string private_key_path = kUnittestPrivateKeyPath;
if (signature_test == kSignatureGeneratedShellBadKey) {
ASSERT_TRUE(utils::MakeTempFile("/tmp/key.XXXXXX",
@@ -234,29 +245,28 @@
}
}
-void DoSmallImageTest(bool full_kernel, bool full_rootfs, bool noop,
- SignatureTest signature_test) {
- string a_img, b_img;
- EXPECT_TRUE(utils::MakeTempFile("/tmp/a_img.XXXXXX", &a_img, NULL));
- ScopedPathUnlinker a_img_unlinker(a_img);
- EXPECT_TRUE(utils::MakeTempFile("/tmp/b_img.XXXXXX", &b_img, NULL));
- ScopedPathUnlinker b_img_unlinker(b_img);
+static void GenerateDeltaFile(bool full_kernel,
+ bool full_rootfs,
+ bool noop,
+ SignatureTest signature_test,
+ DeltaState *state) {
+ EXPECT_TRUE(utils::MakeTempFile("/tmp/a_img.XXXXXX", &state->a_img, NULL));
+ EXPECT_TRUE(utils::MakeTempFile("/tmp/b_img.XXXXXX", &state->b_img, NULL));
+ CreateExtImageAtPath(state->a_img, NULL);
- CreateExtImageAtPath(a_img, NULL);
-
- int image_size = static_cast<int>(utils::FileSize(a_img));
+ state->image_size = static_cast<int>(utils::FileSize(state->a_img));
// Extend the "partitions" holding the file system a bit.
EXPECT_EQ(0, System(base::StringPrintf(
"dd if=/dev/zero of=%s seek=%d bs=1 count=1",
- a_img.c_str(),
- image_size + 1024 * 1024 - 1)));
- EXPECT_EQ(image_size + 1024 * 1024, utils::FileSize(a_img));
+ state->a_img.c_str(),
+ state->image_size + 1024 * 1024 - 1)));
+ EXPECT_EQ(state->image_size + 1024 * 1024, utils::FileSize(state->a_img));
// Make some changes to the A image.
{
string a_mnt;
- ScopedLoopMounter b_mounter(a_img, &a_mnt, 0);
+ ScopedLoopMounter b_mounter(state->a_img, &a_mnt, 0);
EXPECT_TRUE(utils::WriteFile(StringPrintf("%s/hardtocompress",
a_mnt.c_str()).c_str(),
@@ -272,18 +282,19 @@
}
if (noop) {
- EXPECT_TRUE(file_util::CopyFile(FilePath(a_img), FilePath(b_img)));
+ EXPECT_TRUE(file_util::CopyFile(FilePath(state->a_img),
+ FilePath(state->b_img)));
} else {
- CreateExtImageAtPath(b_img, NULL);
+ CreateExtImageAtPath(state->b_img, NULL);
EXPECT_EQ(0, System(base::StringPrintf(
"dd if=/dev/zero of=%s seek=%d bs=1 count=1",
- b_img.c_str(),
- image_size + 1024 * 1024 - 1)));
- EXPECT_EQ(image_size + 1024 * 1024, utils::FileSize(b_img));
+ state->b_img.c_str(),
+ state->image_size + 1024 * 1024 - 1)));
+ EXPECT_EQ(state->image_size + 1024 * 1024, utils::FileSize(state->b_img));
// Make some changes to the B image.
string b_mnt;
- ScopedLoopMounter b_mounter(b_img, &b_mnt, 0);
+ ScopedLoopMounter b_mounter(state->b_img, &b_mnt, 0);
EXPECT_EQ(0, system(StringPrintf("cp %s/hello %s/hello2", b_mnt.c_str(),
b_mnt.c_str()).c_str()));
@@ -313,73 +324,84 @@
}
string old_kernel;
- EXPECT_TRUE(utils::MakeTempFile("/tmp/old_kernel.XXXXXX", &old_kernel, NULL));
- ScopedPathUnlinker old_kernel_unlinker(old_kernel);
+ EXPECT_TRUE(utils::MakeTempFile("/tmp/old_kernel.XXXXXX",
+ &state->old_kernel,
+ NULL));
string new_kernel;
- EXPECT_TRUE(utils::MakeTempFile("/tmp/new_kernel.XXXXXX", &new_kernel, NULL));
- ScopedPathUnlinker new_kernel_unlinker(new_kernel);
+ EXPECT_TRUE(utils::MakeTempFile("/tmp/new_kernel.XXXXXX",
+ &state->new_kernel,
+ NULL));
- vector<char> old_kernel_data(4096); // Something small for a test
- vector<char> new_kernel_data(old_kernel_data.size());
- FillWithData(&old_kernel_data);
- FillWithData(&new_kernel_data);
+ state->old_kernel_data.resize(kDefaultKernelSize);
+ state->new_kernel_data.resize(state->old_kernel_data.size());
+ FillWithData(&state->old_kernel_data);
+ FillWithData(&state->new_kernel_data);
// change the new kernel data
- const char* new_data_string = "This is new data.";
- strcpy(&new_kernel_data[0], new_data_string);
+ strcpy(&state->new_kernel_data[0], kNewDataString);
if (noop) {
- old_kernel_data = new_kernel_data;
+ state->old_kernel_data = state->new_kernel_data;
}
// Write kernels to disk
- EXPECT_TRUE(utils::WriteFile(
- old_kernel.c_str(), &old_kernel_data[0], old_kernel_data.size()));
- EXPECT_TRUE(utils::WriteFile(
- new_kernel.c_str(), &new_kernel_data[0], new_kernel_data.size()));
+ EXPECT_TRUE(utils::WriteFile(state->old_kernel.c_str(),
+ &state->old_kernel_data[0],
+ state->old_kernel_data.size()));
+ EXPECT_TRUE(utils::WriteFile(state->new_kernel.c_str(),
+ &state->new_kernel_data[0],
+ state->new_kernel_data.size()));
- string delta_path;
- EXPECT_TRUE(utils::MakeTempFile("/tmp/delta.XXXXXX", &delta_path, NULL));
- LOG(INFO) << "delta path: " << delta_path;
- ScopedPathUnlinker delta_path_unlinker(delta_path);
+ EXPECT_TRUE(utils::MakeTempFile("/tmp/delta.XXXXXX",
+ &state->delta_path,
+ NULL));
+ LOG(INFO) << "delta path: " << state->delta_path;
{
string a_mnt, b_mnt;
- ScopedLoopMounter a_mounter(a_img, &a_mnt, MS_RDONLY);
- ScopedLoopMounter b_mounter(b_img, &b_mnt, MS_RDONLY);
+ ScopedLoopMounter a_mounter(state->a_img, &a_mnt, MS_RDONLY);
+ ScopedLoopMounter b_mounter(state->b_img, &b_mnt, MS_RDONLY);
const string private_key =
signature_test == kSignatureGenerator ? kUnittestPrivateKeyPath : "";
EXPECT_TRUE(
DeltaDiffGenerator::GenerateDeltaUpdateFile(
full_rootfs ? "" : a_mnt,
- full_rootfs ? "" : a_img,
+ full_rootfs ? "" : state->a_img,
b_mnt,
- b_img,
- full_kernel ? "" : old_kernel,
- new_kernel,
- delta_path,
- private_key));
+ state->b_img,
+ full_kernel ? "" : state->old_kernel,
+ state->new_kernel,
+ state->delta_path,
+ private_key,
+ &state->metadata_size));
}
if (signature_test == kSignatureGenerated) {
- SignGeneratedPayload(delta_path);
+ // Generate the signed payload and update the metadata size in state to
+ // reflect the new size after adding the signature operation to the
+ // manifest.
+ SignGeneratedPayload(state->delta_path, &state->metadata_size);
} else if (signature_test == kSignatureGeneratedShell ||
signature_test == kSignatureGeneratedShellBadKey ||
signature_test == kSignatureGeneratedShellRotateCl1 ||
signature_test == kSignatureGeneratedShellRotateCl2) {
- SignGeneratedShellPayload(signature_test, delta_path);
+ SignGeneratedShellPayload(signature_test, state->delta_path);
}
+}
- // Read delta into memory.
- vector<char> delta;
- uint64_t metadata_size;
-
+static void ApplyDeltaFile(bool full_kernel, bool full_rootfs, bool noop,
+ SignatureTest signature_test, DeltaState* state,
+ bool hash_checks_mandatory,
+ OperationHashTest op_hash_test,
+ DeltaPerformer** performer) {
// Check the metadata.
{
DeltaArchiveManifest manifest;
- EXPECT_TRUE(PayloadSigner::LoadPayload(delta_path, &delta, &manifest,
- &metadata_size));
- LOG(INFO) << "Metadata size: " << metadata_size;
+ EXPECT_TRUE(PayloadSigner::LoadPayload(state->delta_path,
+ &state->delta,
+ &manifest,
+ &state->metadata_size));
+ LOG(INFO) << "Metadata size: " << state->metadata_size;
if (signature_test == kSignatureNone) {
EXPECT_FALSE(manifest.has_signatures_offset());
@@ -389,7 +411,7 @@
EXPECT_TRUE(manifest.has_signatures_size());
Signatures sigs_message;
EXPECT_TRUE(sigs_message.ParseFromArray(
- &delta[metadata_size + manifest.signatures_offset()],
+ &state->delta[state->metadata_size + manifest.signatures_offset()],
manifest.signatures_size()));
if (signature_test == kSignatureGeneratedShellRotateCl1 ||
signature_test == kSignatureGeneratedShellRotateCl2)
@@ -420,19 +442,20 @@
if (full_kernel) {
EXPECT_FALSE(manifest.has_old_kernel_info());
} else {
- EXPECT_EQ(old_kernel_data.size(), manifest.old_kernel_info().size());
+ EXPECT_EQ(state->old_kernel_data.size(),
+ manifest.old_kernel_info().size());
EXPECT_FALSE(manifest.old_kernel_info().hash().empty());
}
if (full_rootfs) {
EXPECT_FALSE(manifest.has_old_rootfs_info());
} else {
- EXPECT_EQ(image_size, manifest.old_rootfs_info().size());
+ EXPECT_EQ(state->image_size, manifest.old_rootfs_info().size());
EXPECT_FALSE(manifest.old_rootfs_info().hash().empty());
}
- EXPECT_EQ(new_kernel_data.size(), manifest.new_kernel_info().size());
- EXPECT_EQ(image_size, manifest.new_rootfs_info().size());
+ EXPECT_EQ(state->new_kernel_data.size(), manifest.new_kernel_info().size());
+ EXPECT_EQ(state->image_size, manifest.new_rootfs_info().size());
EXPECT_FALSE(manifest.new_kernel_info().hash().empty());
EXPECT_FALSE(manifest.new_rootfs_info().hash().empty());
@@ -440,7 +463,7 @@
PrefsMock prefs;
EXPECT_CALL(prefs, SetInt64(kPrefsManifestMetadataSize,
- metadata_size)).WillOnce(Return(true));
+ state->metadata_size)).WillOnce(Return(true));
EXPECT_CALL(prefs, SetInt64(kPrefsUpdateStateNextOperation, _))
.WillRepeatedly(Return(true));
EXPECT_CALL(prefs, GetInt64(kPrefsUpdateStateNextOperation, _))
@@ -449,7 +472,7 @@
.WillRepeatedly(Return(true));
EXPECT_CALL(prefs, SetString(kPrefsUpdateStateSHA256Context, _))
.WillRepeatedly(Return(true));
- if (signature_test != kSignatureNone) {
+ if (op_hash_test == kValidOperationData && signature_test != kSignatureNone) {
EXPECT_CALL(prefs, SetString(kPrefsUpdateStateSignedSHA256Context, _))
.WillOnce(Return(true));
EXPECT_CALL(prefs, SetString(kPrefsUpdateStateSignatureBlob, _))
@@ -458,385 +481,401 @@
// Update the A image in place.
InstallPlan install_plan;
- MockSystemState mock_system_state;
- DeltaPerformer performer(&prefs, &mock_system_state, &install_plan);
- EXPECT_TRUE(utils::FileExists(kUnittestPublicKeyPath));
- performer.set_public_key_path(kUnittestPublicKeyPath);
-
- EXPECT_EQ(image_size,
- OmahaHashCalculator::RawHashOfFile(a_img,
- image_size,
- &install_plan.rootfs_hash));
- EXPECT_TRUE(OmahaHashCalculator::RawHashOfData(old_kernel_data,
- &install_plan.kernel_hash));
-
- EXPECT_EQ(0, performer.Open(a_img.c_str(), 0, 0));
- EXPECT_TRUE(performer.OpenKernel(old_kernel.c_str()));
-
- // Write at some number of bytes per operation. Arbitrarily chose 5.
- const size_t kBytesPerWrite = 5;
- for (size_t i = 0; i < delta.size(); i += kBytesPerWrite) {
- size_t count = min(delta.size() - i, kBytesPerWrite);
- EXPECT_TRUE(performer.Write(&delta[i], count));
- }
-
- // Wrapper around close. Returns 0 on success or -errno on error.
- EXPECT_EQ(0, performer.Close());
-
- CompareFilesByBlock(old_kernel, new_kernel);
- CompareFilesByBlock(a_img, b_img);
-
- vector<char> updated_kernel_partition;
- EXPECT_TRUE(utils::ReadFile(old_kernel, &updated_kernel_partition));
- EXPECT_EQ(0, strncmp(&updated_kernel_partition[0], new_data_string,
- strlen(new_data_string)));
-
- ActionExitCode expect_verify_result = kActionCodeSuccess;
- switch (signature_test) {
- case kSignatureNone:
- expect_verify_result = kActionCodeSignedDeltaPayloadExpectedError;
- break;
- case kSignatureGeneratedShellBadKey:
- expect_verify_result = kActionCodeDownloadPayloadPubKeyVerificationError;
- break;
- default: break; // appease gcc
- }
- EXPECT_EQ(expect_verify_result, performer.VerifyPayload(
- OmahaHashCalculator::OmahaHashOfData(delta),
- delta.size()));
-
- performer.set_public_key_path("/public/key/does/not/exists");
- EXPECT_EQ(kActionCodeSuccess, performer.VerifyPayload(
- OmahaHashCalculator::OmahaHashOfData(delta),
- delta.size()));
-
- uint64_t new_kernel_size;
- vector<char> new_kernel_hash;
- uint64_t new_rootfs_size;
- vector<char> new_rootfs_hash;
- EXPECT_TRUE(performer.GetNewPartitionInfo(&new_kernel_size,
- &new_kernel_hash,
- &new_rootfs_size,
- &new_rootfs_hash));
- EXPECT_EQ(4096, new_kernel_size);
- vector<char> expected_new_kernel_hash;
- EXPECT_TRUE(OmahaHashCalculator::RawHashOfData(new_kernel_data,
- &expected_new_kernel_hash));
- EXPECT_TRUE(expected_new_kernel_hash == new_kernel_hash);
- EXPECT_EQ(image_size, new_rootfs_size);
- vector<char> expected_new_rootfs_hash;
- EXPECT_EQ(image_size,
- OmahaHashCalculator::RawHashOfFile(b_img,
- image_size,
- &expected_new_rootfs_hash));
- EXPECT_TRUE(expected_new_rootfs_hash == new_rootfs_hash);
-}
-
-// TODO(jaysri): Refactor the previous unit test so we can reuse a lot of
-// code between these two methods.
-void DoManifestTest() {
- bool full_kernel = false;
- bool full_rootfs = false;
- string a_img, b_img;
- EXPECT_TRUE(utils::MakeTempFile("/tmp/a_img.XXXXXX", &a_img, NULL));
- ScopedPathUnlinker a_img_unlinker(a_img);
- EXPECT_TRUE(utils::MakeTempFile("/tmp/b_img.XXXXXX", &b_img, NULL));
- ScopedPathUnlinker b_img_unlinker(b_img);
-
- CreateExtImageAtPath(a_img, NULL);
-
- int image_size = static_cast<int>(utils::FileSize(a_img));
-
- // Extend the "partitions" holding the file system a bit.
- EXPECT_EQ(0, System(base::StringPrintf(
- "dd if=/dev/zero of=%s seek=%d bs=1 count=1",
- a_img.c_str(),
- image_size + 1024 * 1024 - 1)));
- EXPECT_EQ(image_size + 1024 * 1024, utils::FileSize(a_img));
-
- // Make some changes to the A image.
- {
- string a_mnt;
- ScopedLoopMounter b_mounter(a_img, &a_mnt, 0);
-
- EXPECT_TRUE(utils::WriteFile(StringPrintf("%s/hardtocompress",
- a_mnt.c_str()).c_str(),
- reinterpret_cast<const char*>(kRandomString),
- sizeof(kRandomString) - 1));
- // Write 1 MiB of 0xff to try to catch the case where writing a bsdiff
- // patch fails to zero out the final block.
- vector<char> ones(1024 * 1024, 0xff);
- EXPECT_TRUE(utils::WriteFile(StringPrintf("%s/ones",
- a_mnt.c_str()).c_str(),
- &ones[0],
- ones.size()));
- }
-
- {
- CreateExtImageAtPath(b_img, NULL);
- EXPECT_EQ(0, System(base::StringPrintf(
- "dd if=/dev/zero of=%s seek=%d bs=1 count=1",
- b_img.c_str(),
- image_size + 1024 * 1024 - 1)));
- EXPECT_EQ(image_size + 1024 * 1024, utils::FileSize(b_img));
-
- // Make some changes to the B image.
- string b_mnt;
- ScopedLoopMounter b_mounter(b_img, &b_mnt, 0);
-
- EXPECT_EQ(0, system(StringPrintf("cp %s/hello %s/hello2", b_mnt.c_str(),
- b_mnt.c_str()).c_str()));
- EXPECT_EQ(0, system(StringPrintf("rm %s/hello", b_mnt.c_str()).c_str()));
- EXPECT_EQ(0, system(StringPrintf("mv %s/hello2 %s/hello", b_mnt.c_str(),
- b_mnt.c_str()).c_str()));
- EXPECT_EQ(0, system(StringPrintf("echo foo > %s/foo",
- b_mnt.c_str()).c_str()));
- EXPECT_EQ(0, system(StringPrintf("touch %s/emptyfile",
- b_mnt.c_str()).c_str()));
- EXPECT_TRUE(WriteSparseFile(StringPrintf("%s/fullsparse", b_mnt.c_str()),
- 1024 * 1024));
- EXPECT_EQ(0, system(StringPrintf("dd if=/dev/zero of=%s/partsparese bs=1 "
- "seek=4096 count=1",
- b_mnt.c_str()).c_str()));
- EXPECT_EQ(0, system(StringPrintf("cp %s/srchardlink0 %s/tmp && "
- "mv %s/tmp %s/srchardlink1",
- b_mnt.c_str(), b_mnt.c_str(),
- b_mnt.c_str(), b_mnt.c_str()).c_str()));
- EXPECT_EQ(0, system(StringPrintf("rm %s/boguslink && "
- "echo foobar > %s/boguslink",
- b_mnt.c_str(), b_mnt.c_str()).c_str()));
- EXPECT_TRUE(utils::WriteFile(StringPrintf("%s/hardtocompress",
- b_mnt.c_str()).c_str(),
- reinterpret_cast<const char*>(kRandomString),
- sizeof(kRandomString)));
- }
-
- string old_kernel;
- EXPECT_TRUE(utils::MakeTempFile("/tmp/old_kernel.XXXXXX", &old_kernel, NULL));
- ScopedPathUnlinker old_kernel_unlinker(old_kernel);
-
- string new_kernel;
- EXPECT_TRUE(utils::MakeTempFile("/tmp/new_kernel.XXXXXX", &new_kernel, NULL));
- ScopedPathUnlinker new_kernel_unlinker(new_kernel);
-
- vector<char> old_kernel_data(4096); // Something small for a test
- vector<char> new_kernel_data(old_kernel_data.size());
- FillWithData(&old_kernel_data);
- FillWithData(&new_kernel_data);
-
- // change the new kernel data
- const char* new_data_string = "This is new data.";
- strcpy(&new_kernel_data[0], new_data_string);
-
- // Write kernels to disk
- EXPECT_TRUE(utils::WriteFile(
- old_kernel.c_str(), &old_kernel_data[0], old_kernel_data.size()));
- EXPECT_TRUE(utils::WriteFile(
- new_kernel.c_str(), &new_kernel_data[0], new_kernel_data.size()));
-
- string delta_path;
- EXPECT_TRUE(utils::MakeTempFile("/tmp/delta.XXXXXX", &delta_path, NULL));
- LOG(INFO) << "delta path: " << delta_path;
- ScopedPathUnlinker delta_path_unlinker(delta_path);
- {
- string a_mnt, b_mnt;
- ScopedLoopMounter a_mounter(a_img, &a_mnt, MS_RDONLY);
- ScopedLoopMounter b_mounter(b_img, &b_mnt, MS_RDONLY);
- const string private_key = kUnittestPrivateKeyPath;
- EXPECT_TRUE(
- DeltaDiffGenerator::GenerateDeltaUpdateFile(
- full_rootfs ? "" : a_mnt,
- full_rootfs ? "" : a_img,
- b_mnt,
- b_img,
- full_kernel ? "" : old_kernel,
- new_kernel,
- delta_path,
- private_key));
- }
-
- // Read delta into memory.
- vector<char> delta;
- uint64_t metadata_size;
- // Check the metadata.
- {
- DeltaArchiveManifest manifest;
- EXPECT_TRUE(PayloadSigner::LoadPayload(delta_path, &delta, &manifest,
- &metadata_size));
-
- LOG(INFO) << "Metadata size: " << metadata_size;
-
- {
- EXPECT_TRUE(manifest.has_signatures_offset());
- EXPECT_TRUE(manifest.has_signatures_size());
- Signatures sigs_message;
- EXPECT_TRUE(sigs_message.ParseFromArray(
- &delta[metadata_size + manifest.signatures_offset()],
- manifest.signatures_size()));
- EXPECT_EQ(1, sigs_message.signatures_size());
- const Signatures_Signature& signature = sigs_message.signatures(0);
- EXPECT_EQ(1, signature.version());
-
- uint64_t expected_sig_data_length = 0;
- vector<string> key_paths (1, kUnittestPrivateKeyPath);
- EXPECT_TRUE(PayloadSigner::SignatureBlobLength(
- key_paths,
- &expected_sig_data_length));
- EXPECT_EQ(expected_sig_data_length, manifest.signatures_size());
- EXPECT_FALSE(signature.data().empty());
- }
-
- if (full_kernel) {
- EXPECT_FALSE(manifest.has_old_kernel_info());
- } else {
- EXPECT_EQ(old_kernel_data.size(), manifest.old_kernel_info().size());
- EXPECT_FALSE(manifest.old_kernel_info().hash().empty());
- }
-
- if (full_rootfs) {
- EXPECT_FALSE(manifest.has_old_rootfs_info());
- } else {
- EXPECT_EQ(image_size, manifest.old_rootfs_info().size());
- EXPECT_FALSE(manifest.old_rootfs_info().hash().empty());
- }
-
- EXPECT_EQ(new_kernel_data.size(), manifest.new_kernel_info().size());
- EXPECT_EQ(image_size, manifest.new_rootfs_info().size());
-
- EXPECT_FALSE(manifest.new_kernel_info().hash().empty());
- EXPECT_FALSE(manifest.new_rootfs_info().hash().empty());
-
- }
-
- PrefsMock prefs;
- EXPECT_CALL(prefs, SetInt64(kPrefsManifestMetadataSize,
- metadata_size)).WillOnce(Return(true));
- EXPECT_CALL(prefs, SetInt64(kPrefsUpdateStateNextOperation, _))
- .WillRepeatedly(Return(true));
- EXPECT_CALL(prefs, GetInt64(kPrefsUpdateStateNextOperation, _))
- .WillOnce(Return(false));
- EXPECT_CALL(prefs, SetInt64(kPrefsUpdateStateNextDataOffset, _))
- .WillRepeatedly(Return(true));
- EXPECT_CALL(prefs, SetString(kPrefsUpdateStateSHA256Context, _))
- .WillRepeatedly(Return(true));
- EXPECT_CALL(prefs, SetString(kPrefsUpdateStateSignedSHA256Context, _))
- .WillRepeatedly(Return(true));
- EXPECT_CALL(prefs, SetString(kPrefsUpdateStateSignatureBlob, _))
- .WillOnce(Return(true));
-
- // Update the A image in place.
- InstallPlan install_plan;
- install_plan.metadata_size = metadata_size;
- LOG(INFO) << "Setting payload metadata size in Omaha = " << metadata_size;
+ install_plan.hash_checks_mandatory = hash_checks_mandatory;
+ install_plan.metadata_size = state->metadata_size;
+ LOG(INFO) << "Setting payload metadata size in Omaha = "
+ << state->metadata_size;
ASSERT_TRUE(PayloadSigner::GetMetadataSignature(
- &delta[0],
- metadata_size,
+ &state->delta[0],
+ state->metadata_size,
kUnittestPrivateKeyPath,
&install_plan.metadata_signature));
EXPECT_FALSE(install_plan.metadata_signature.empty());
MockSystemState mock_system_state;
- DeltaPerformer performer(&prefs, &mock_system_state, &install_plan);
+ *performer = new DeltaPerformer(&prefs, &mock_system_state, &install_plan);
EXPECT_TRUE(utils::FileExists(kUnittestPublicKeyPath));
- performer.set_public_key_path(kUnittestPublicKeyPath);
+ (*performer)->set_public_key_path(kUnittestPublicKeyPath);
- EXPECT_EQ(image_size,
- OmahaHashCalculator::RawHashOfFile(a_img,
- image_size,
+ EXPECT_EQ(state->image_size,
+ OmahaHashCalculator::RawHashOfFile(state->a_img,
+ state->image_size,
&install_plan.rootfs_hash));
- EXPECT_TRUE(OmahaHashCalculator::RawHashOfData(old_kernel_data,
+ EXPECT_TRUE(OmahaHashCalculator::RawHashOfData(state->old_kernel_data,
&install_plan.kernel_hash));
- EXPECT_EQ(0, performer.Open(a_img.c_str(), 0, 0));
- EXPECT_TRUE(performer.OpenKernel(old_kernel.c_str()));
+ EXPECT_EQ(0, (*performer)->Open(state->a_img.c_str(), 0, 0));
+ EXPECT_TRUE((*performer)->OpenKernel(state->old_kernel.c_str()));
+
+ ActionExitCode expected_error, actual_error;
+ bool continue_writing;
+ switch(op_hash_test) {
+ case kInvalidOperationData: {
+ // Muck with some random offset post the metadata size so that
+ // some operation hash will result in a mismatch.
+ int some_offset = state->metadata_size + 300;
+ LOG(INFO) << "Tampered value at offset: " << some_offset;
+ state->delta[some_offset]++;
+ expected_error = kActionCodeDownloadOperationHashMismatch;
+ continue_writing = false;
+ break;
+ }
+
+ case kValidOperationData:
+ default:
+ // no change.
+ expected_error = kActionCodeSuccess;
+ continue_writing = true;
+ break;
+ }
// Write at some number of bytes per operation. Arbitrarily chose 5.
const size_t kBytesPerWrite = 5;
- for (size_t i = 0; i < delta.size(); i += kBytesPerWrite) {
- size_t count = min(delta.size() - i, kBytesPerWrite);
- EXPECT_TRUE(performer.Write(&delta[i], count));
+ for (size_t i = 0; i < state->delta.size(); i += kBytesPerWrite) {
+ size_t count = min(state->delta.size() - i, kBytesPerWrite);
+ bool write_succeeded = ((*performer)->Write(&state->delta[i],
+ count,
+ &actual_error));
+ // Normally write_succeeded should be true every time and
+ // actual_error should be kActionCodeSuccess. If so, continue the loop.
+ // But if we seeded an operation hash error above, then write_succeeded
+ // will be false. The failure may happen at any operation n. So, all
+ // Writes until n-1 should succeed and the nth operation will fail with
+ // actual_error. In this case, we should bail out of the loop because
+ // we cannot proceed applying the delta.
+ if (!write_succeeded) {
+ LOG(INFO) << "Write failed. Checking if it failed with expected error";
+ EXPECT_EQ(expected_error, actual_error);
+ if (!continue_writing) {
+ LOG(INFO) << "Cannot continue writing. Bailing out.";
+ break;
+ }
+ }
+
+ EXPECT_EQ(kActionCodeSuccess, actual_error);
}
- // Wrapper around close. Returns 0 on success or -errno on error.
- EXPECT_EQ(0, performer.Close());
+ // If we had continued all the way through, Close should succeed.
+ // Otherwise, it should fail. Check appropriately.
+ bool close_result = (*performer)->Close();
+ if (continue_writing)
+ EXPECT_EQ(0, close_result);
+ else
+ EXPECT_LE(0, close_result);
+}
- CompareFilesByBlock(old_kernel, new_kernel);
- CompareFilesByBlock(a_img, b_img);
+void VerifyPayloadResult(DeltaPerformer* performer,
+ DeltaState* state,
+ ActionExitCode expected_result) {
+ if (!performer) {
+ EXPECT_TRUE(!"Skipping payload verification since performer is NULL.");
+ return;
+ }
+
+ LOG(INFO) << "Verifying payload for expected result "
+ << expected_result;
+ EXPECT_EQ(expected_result, performer->VerifyPayload(
+ OmahaHashCalculator::OmahaHashOfData(state->delta),
+ state->delta.size()));
+ LOG(INFO) << "Verified payload.";
+
+ if (expected_result != kActionCodeSuccess) {
+ // no need to verify new partition if VerifyPayload failed.
+ return;
+ }
+
+ CompareFilesByBlock(state->old_kernel, state->new_kernel);
+ CompareFilesByBlock(state->a_img, state->b_img);
vector<char> updated_kernel_partition;
- EXPECT_TRUE(utils::ReadFile(old_kernel, &updated_kernel_partition));
- EXPECT_EQ(0, strncmp(&updated_kernel_partition[0], new_data_string,
- strlen(new_data_string)));
-
- ActionExitCode expect_verify_result = kActionCodeSuccess;
- LOG(INFO) << "Verifying Payload ...";
- EXPECT_EQ(expect_verify_result, performer.VerifyPayload(
- OmahaHashCalculator::OmahaHashOfData(delta),
- delta.size()));
-
- LOG(INFO) << "Verified Payload";
+ EXPECT_TRUE(utils::ReadFile(state->old_kernel, &updated_kernel_partition));
+ EXPECT_EQ(0, strncmp(&updated_kernel_partition[0], kNewDataString,
+ strlen(kNewDataString)));
uint64_t new_kernel_size;
vector<char> new_kernel_hash;
uint64_t new_rootfs_size;
vector<char> new_rootfs_hash;
- EXPECT_TRUE(performer.GetNewPartitionInfo(&new_kernel_size,
+ EXPECT_TRUE(performer->GetNewPartitionInfo(&new_kernel_size,
&new_kernel_hash,
&new_rootfs_size,
&new_rootfs_hash));
- EXPECT_EQ(4096, new_kernel_size);
+ EXPECT_EQ(kDefaultKernelSize, new_kernel_size);
vector<char> expected_new_kernel_hash;
- EXPECT_TRUE(OmahaHashCalculator::RawHashOfData(new_kernel_data,
+ EXPECT_TRUE(OmahaHashCalculator::RawHashOfData(state->new_kernel_data,
&expected_new_kernel_hash));
EXPECT_TRUE(expected_new_kernel_hash == new_kernel_hash);
- EXPECT_EQ(image_size, new_rootfs_size);
+ EXPECT_EQ(state->image_size, new_rootfs_size);
vector<char> expected_new_rootfs_hash;
- EXPECT_EQ(image_size,
- OmahaHashCalculator::RawHashOfFile(b_img,
- image_size,
+ EXPECT_EQ(state->image_size,
+ OmahaHashCalculator::RawHashOfFile(state->b_img,
+ state->image_size,
&expected_new_rootfs_hash));
EXPECT_TRUE(expected_new_rootfs_hash == new_rootfs_hash);
}
-} // namespace {}
+
+void VerifyPayload(DeltaPerformer* performer,
+ DeltaState* state,
+ SignatureTest signature_test) {
+ ActionExitCode expected_result = kActionCodeSuccess;
+ switch (signature_test) {
+ case kSignatureNone:
+ expected_result = kActionCodeSignedDeltaPayloadExpectedError;
+ break;
+ case kSignatureGeneratedShellBadKey:
+ expected_result = kActionCodeDownloadPayloadPubKeyVerificationError;
+ break;
+ default: break; // appease gcc
+ }
+
+ VerifyPayloadResult(performer, state, expected_result);
+}
+
+void DoSmallImageTest(bool full_kernel, bool full_rootfs, bool noop,
+ SignatureTest signature_test,
+ bool hash_checks_mandatory) {
+ DeltaState state;
+ DeltaPerformer *performer;
+ GenerateDeltaFile(full_kernel, full_rootfs, noop, signature_test, &state);
+ ScopedPathUnlinker a_img_unlinker(state.a_img);
+ ScopedPathUnlinker b_img_unlinker(state.b_img);
+ ScopedPathUnlinker delta_unlinker(state.delta_path);
+ ScopedPathUnlinker old_kernel_unlinker(state.old_kernel);
+ ScopedPathUnlinker new_kernel_unlinker(state.new_kernel);
+ ApplyDeltaFile(full_kernel, full_rootfs, noop, signature_test,
+ &state, hash_checks_mandatory, kValidOperationData,
+ &performer);
+ VerifyPayload(performer, &state, signature_test);
+}
+
+// Calls delta performer's Write method by pretending to pass in bytes from a
+// delta file whose metadata size is actual_metadata_size and tests if all
+// checks are correctly performed if the install plan contains
+// expected_metadata_size and that the result of the parsing are as per
+// hash_checks_mandatory flag.
+void DoMetadataSizeTest(uint64_t expected_metadata_size,
+ uint64_t actual_metadata_size,
+ bool hash_checks_mandatory) {
+ PrefsMock prefs;
+ InstallPlan install_plan;
+ install_plan.hash_checks_mandatory = hash_checks_mandatory;
+ MockSystemState mock_system_state;
+ DeltaPerformer performer(&prefs, &mock_system_state, &install_plan);
+ EXPECT_EQ(0, performer.Open("/dev/null", 0, 0));
+ EXPECT_TRUE(performer.OpenKernel("/dev/null"));
+
+ // Set a valid magic string and version number 1.
+ EXPECT_TRUE(performer.Write("CrAU", 4));
+ uint64_t version = htobe64(1);
+ EXPECT_TRUE(performer.Write(&version, 8));
+
+ install_plan.metadata_size = expected_metadata_size;
+ ActionExitCode error_code;
+ // When filling in size in manifest, exclude the size of the 20-byte header.
+ uint64_t size_in_manifest = htobe64(actual_metadata_size - 20);
+ bool result = performer.Write(&size_in_manifest, 8, &error_code);
+ if (expected_metadata_size == actual_metadata_size ||
+ !hash_checks_mandatory) {
+ EXPECT_TRUE(result);
+ } else {
+ EXPECT_FALSE(result);
+ EXPECT_EQ(kActionCodeDownloadInvalidMetadataSize, error_code);
+ }
+
+ EXPECT_LT(performer.Close(), 0);
+}
+
+// Generates a valid delta file but tests the delta performer by suppling
+// different metadata signatures as per omaha_metadata_signature flag and
+// sees if the result of the parsing are as per hash_checks_mandatory flag.
+void DoMetadataSignatureTest(MetadataSignatureTest metadata_signature_test,
+ SignatureTest signature_test,
+ bool hash_checks_mandatory) {
+ DeltaState state;
+
+ // Using kSignatureNone since it doesn't affect the results of our test.
+ // If we've to use other signature options, then we'd have to get the
+ // metadata size again after adding the signing operation to the manifest.
+ GenerateDeltaFile(true, true, false, signature_test, &state);
+
+ ScopedPathUnlinker a_img_unlinker(state.a_img);
+ ScopedPathUnlinker b_img_unlinker(state.b_img);
+ ScopedPathUnlinker delta_unlinker(state.delta_path);
+ ScopedPathUnlinker old_kernel_unlinker(state.old_kernel);
+ ScopedPathUnlinker new_kernel_unlinker(state.new_kernel);
+
+ // Loads the payload and parses the manifest.
+ vector<char> payload;
+ EXPECT_TRUE(utils::ReadFile(state.delta_path, &payload));
+ LOG(INFO) << "Payload size: " << payload.size();
+
+ InstallPlan install_plan;
+ install_plan.hash_checks_mandatory = hash_checks_mandatory;
+ install_plan.metadata_size = state.metadata_size;
+
+ DeltaPerformer::MetadataParseResult expected_result, actual_result;
+ ActionExitCode expected_error, actual_error;
+
+ // Fill up the metadata signature in install plan according to the test.
+ switch (metadata_signature_test) {
+ case kEmptyMetadataSignature:
+ install_plan.metadata_signature.clear();
+ expected_result = DeltaPerformer::kMetadataParseError;
+ expected_error = kActionCodeDownloadMetadataSignatureMissingError;
+ break;
+
+ case kInvalidMetadataSignature:
+ install_plan.metadata_signature = kBogusMetadataSignature1;
+ expected_result = DeltaPerformer::kMetadataParseError;
+ expected_error = kActionCodeDownloadMetadataSignatureMismatch;
+ break;
+
+ case kValidMetadataSignature:
+ default:
+ // Set the install plan's metadata size to be the same as the one
+ // in the manifest so that we pass the metadata size checks. Only
+ // then we can get to manifest signature checks.
+ ASSERT_TRUE(PayloadSigner::GetMetadataSignature(
+ &payload[0],
+ state.metadata_size,
+ kUnittestPrivateKeyPath,
+ &install_plan.metadata_signature));
+ EXPECT_FALSE(install_plan.metadata_signature.empty());
+ expected_result = DeltaPerformer::kMetadataParseSuccess;
+ expected_error = kActionCodeSuccess;
+ break;
+ }
+
+ // Ignore the expected result/error if hash checks are not mandatory.
+ if (!hash_checks_mandatory) {
+ expected_result = DeltaPerformer::kMetadataParseSuccess;
+ expected_error = kActionCodeSuccess;
+ }
+
+ // Create the delta performer object.
+ PrefsMock prefs;
+ MockSystemState mock_system_state;
+ DeltaPerformer delta_performer(&prefs, &mock_system_state, &install_plan);
+
+ // Use the public key corresponding to the private key used above to
+ // sign the metadata.
+ EXPECT_TRUE(utils::FileExists(kUnittestPublicKeyPath));
+ delta_performer.set_public_key_path(kUnittestPublicKeyPath);
+
+ // Parse the delta payload we created.
+ DeltaArchiveManifest manifest;
+ uint64_t parsed_metadata_size;
+
+ // Init actual_error with an invalid value so that we make sure
+ // ParsePayloadMetadata properly populates it in all cases.
+ actual_error = kActionCodeUmaReportedMax;
+ actual_result = delta_performer.ParsePayloadMetadata(payload, &manifest,
+ &parsed_metadata_size, &actual_error);
+
+ EXPECT_EQ(expected_result, actual_result);
+ EXPECT_EQ(expected_error, actual_error);
+
+ // Check that the parsed metadata size is what's expected. This test
+ // implicitly confirms that the metadata signature is valid, if required.
+ EXPECT_EQ(state.metadata_size, parsed_metadata_size);
+}
+
+void DoOperationHashMismatchTest(OperationHashTest op_hash_test,
+ bool hash_checks_mandatory) {
+ DeltaState state;
+ GenerateDeltaFile(true, true, false, kSignatureGenerated, &state);
+ ScopedPathUnlinker a_img_unlinker(state.a_img);
+ ScopedPathUnlinker b_img_unlinker(state.b_img);
+ ScopedPathUnlinker delta_unlinker(state.delta_path);
+ ScopedPathUnlinker old_kernel_unlinker(state.old_kernel);
+ ScopedPathUnlinker new_kernel_unlinker(state.new_kernel);
+ DeltaPerformer *performer;
+ ApplyDeltaFile(true, true, false, kSignatureGenerated,
+ &state, hash_checks_mandatory, op_hash_test, &performer);
+}
+
+class DeltaPerformerTest : public ::testing::Test { };
+
+TEST(DeltaPerformerTest, ExtentsToByteStringTest) {
+ uint64_t test[] = {1, 1, 4, 2, kSparseHole, 1, 0, 1};
+ COMPILE_ASSERT(arraysize(test) % 2 == 0, array_size_uneven);
+ const uint64_t block_size = 4096;
+ const uint64_t file_length = 5 * block_size - 13;
+
+ google::protobuf::RepeatedPtrField<Extent> extents;
+ for (size_t i = 0; i < arraysize(test); i += 2) {
+ Extent* extent = extents.Add();
+ extent->set_start_block(test[i]);
+ extent->set_num_blocks(test[i + 1]);
+ }
+
+ string expected_output = "4096:4096,16384:8192,-1:4096,0:4083";
+ string actual_output;
+ EXPECT_TRUE(DeltaPerformer::ExtentsToBsdiffPositionsString(extents,
+ block_size,
+ file_length,
+ &actual_output));
+ EXPECT_EQ(expected_output, actual_output);
+}
TEST(DeltaPerformerTest, RunAsRootSmallImageTest) {
- DoSmallImageTest(false, false, false, kSignatureGenerator);
+ bool hash_checks_mandatory = false;
+ DoSmallImageTest(false, false, false, kSignatureGenerator,
+ hash_checks_mandatory);
}
TEST(DeltaPerformerTest, RunAsRootFullKernelSmallImageTest) {
- DoSmallImageTest(true, false, false, kSignatureGenerator);
+ bool hash_checks_mandatory = false;
+ DoSmallImageTest(true, false, false, kSignatureGenerator,
+ hash_checks_mandatory);
}
TEST(DeltaPerformerTest, RunAsRootFullSmallImageTest) {
- DoSmallImageTest(true, true, false, kSignatureGenerator);
+ bool hash_checks_mandatory = true;
+ DoSmallImageTest(true, true, false, kSignatureGenerator,
+ hash_checks_mandatory);
}
TEST(DeltaPerformerTest, RunAsRootNoopSmallImageTest) {
- DoSmallImageTest(false, false, true, kSignatureGenerator);
+ bool hash_checks_mandatory = false;
+ DoSmallImageTest(false, false, true, kSignatureGenerator,
+ hash_checks_mandatory);
}
TEST(DeltaPerformerTest, RunAsRootSmallImageSignNoneTest) {
- DoSmallImageTest(false, false, false, kSignatureNone);
+ bool hash_checks_mandatory = false;
+ DoSmallImageTest(false, false, false, kSignatureNone,
+ hash_checks_mandatory);
}
TEST(DeltaPerformerTest, RunAsRootSmallImageSignGeneratedTest) {
- DoSmallImageTest(false, false, false, kSignatureGenerated);
+ bool hash_checks_mandatory = true;
+ DoSmallImageTest(false, false, false, kSignatureGenerated,
+ hash_checks_mandatory);
}
TEST(DeltaPerformerTest, RunAsRootSmallImageSignGeneratedShellTest) {
- DoSmallImageTest(false, false, false, kSignatureGeneratedShell);
+ bool hash_checks_mandatory = false;
+ DoSmallImageTest(false, false, false, kSignatureGeneratedShell,
+ hash_checks_mandatory);
}
TEST(DeltaPerformerTest, RunAsRootSmallImageSignGeneratedShellBadKeyTest) {
- DoSmallImageTest(false, false, false, kSignatureGeneratedShellBadKey);
+ bool hash_checks_mandatory = false;
+ DoSmallImageTest(false, false, false, kSignatureGeneratedShellBadKey,
+ hash_checks_mandatory);
}
TEST(DeltaPerformerTest, RunAsRootSmallImageSignGeneratedShellRotateCl1Test) {
- DoSmallImageTest(false, false, false, kSignatureGeneratedShellRotateCl1);
+ bool hash_checks_mandatory = false;
+ DoSmallImageTest(false, false, false, kSignatureGeneratedShellRotateCl1,
+ hash_checks_mandatory);
}
TEST(DeltaPerformerTest, RunAsRootSmallImageSignGeneratedShellRotateCl2Test) {
- DoSmallImageTest(false, false, false, kSignatureGeneratedShellRotateCl2);
+ bool hash_checks_mandatory = false;
+ DoSmallImageTest(false, false, false, kSignatureGeneratedShellRotateCl2,
+ hash_checks_mandatory);
}
TEST(DeltaPerformerTest, BadDeltaMagicTest) {
@@ -868,9 +907,57 @@
EXPECT_FALSE(DeltaPerformer::IsIdempotentOperation(op));
}
-TEST(DeltaPerformerTest, RunAsRootRunValidManifestTest) {
- DoManifestTest();
+TEST(DeltaPerformerTest, MissingMandatoryMetadataSizeTest) {
+ DoMetadataSizeTest(0, 75456, true);
}
+TEST(DeltaPerformerTest, MissingNonMandatoryMetadataSizeTest) {
+ DoMetadataSizeTest(0, 123456, false);
+}
+
+TEST(DeltaPerformerTest, InvalidMandatoryMetadataSizeTest) {
+ DoMetadataSizeTest(13000, 140000, true);
+}
+
+TEST(DeltaPerformerTest, InvalidNonMandatoryMetadataSizeTest) {
+ DoMetadataSizeTest(40000, 50000, false);
+}
+
+TEST(DeltaPerformerTest, ValidMandatoryMetadataSizeTest) {
+ DoMetadataSizeTest(85376, 85376, true);
+}
+
+TEST(DeltaPerformerTest, RunAsRootMandatoryEmptyMetadataSignatureTest) {
+ DoMetadataSignatureTest(kEmptyMetadataSignature, kSignatureGenerated, true);
+}
+
+TEST(DeltaPerformerTest, RunAsRootNonMandatoryEmptyMetadataSignatureTest) {
+ DoMetadataSignatureTest(kEmptyMetadataSignature, kSignatureGenerated, false);
+}
+
+TEST(DeltaPerformerTest, RunAsRootMandatoryInvalidMetadataSignatureTest) {
+ DoMetadataSignatureTest(kInvalidMetadataSignature, kSignatureGenerated, true);
+}
+
+TEST(DeltaPerformerTest, RunAsRootNonMandatoryInvalidMetadataSignatureTest) {
+ DoMetadataSignatureTest(kInvalidMetadataSignature, kSignatureGenerated,
+ false);
+}
+
+TEST(DeltaPerformerTest, RunAsRootMandatoryValidMetadataSignature1Test) {
+ DoMetadataSignatureTest(kValidMetadataSignature, kSignatureNone, true);
+}
+
+TEST(DeltaPerformerTest, RunAsRootMandatoryValidMetadataSignature2Test) {
+ DoMetadataSignatureTest(kValidMetadataSignature, kSignatureGenerated, true);
+}
+
+TEST(DeltaPerformerTest, RunAsRootNonMandatoryValidMetadataSignatureTest) {
+ DoMetadataSignatureTest(kValidMetadataSignature, kSignatureGenerated, false);
+}
+
+TEST(DeltaPerformerTest, RunAsRootMandatoryOperationHashMismatchTest) {
+ DoOperationHashMismatchTest(kInvalidOperationData, true);
+}
} // namespace chromeos_update_engine
diff --git a/generate_delta_main.cc b/generate_delta_main.cc
index b651d00..aa2b9cb 100644
--- a/generate_delta_main.cc
+++ b/generate_delta_main.cc
@@ -153,9 +153,11 @@
CHECK(utils::ReadFile(*it, &signature));
signatures.push_back(signature);
}
+ uint64_t final_metadata_size;
CHECK(PayloadSigner::AddSignatureToPayload(
- FLAGS_in_file, signatures, FLAGS_out_file));
- LOG(INFO) << "Done signing payload.";
+ FLAGS_in_file, signatures, FLAGS_out_file, &final_metadata_size));
+ LOG(INFO) << "Done signing payload. Final metadata size = "
+ << final_metadata_size;
}
void VerifySignedPayload() {
@@ -261,6 +263,7 @@
LOG(FATAL) << "old_dir or new_dir not directory";
}
}
+ uint64_t metadata_size;
if (!DeltaDiffGenerator::GenerateDeltaUpdateFile(FLAGS_old_dir,
FLAGS_old_image,
FLAGS_new_dir,
@@ -268,7 +271,8 @@
FLAGS_old_kernel,
FLAGS_new_kernel,
FLAGS_out_file,
- FLAGS_private_key)) {
+ FLAGS_private_key,
+ &metadata_size)) {
return 1;
}
return 0;
diff --git a/install_plan.h b/install_plan.h
index d277f66..c0fa4cd 100644
--- a/install_plan.h
+++ b/install_plan.h
@@ -33,8 +33,17 @@
install_path(install_path),
kernel_install_path(kernel_install_path),
kernel_size(0),
- rootfs_size(0) {}
- InstallPlan() : is_resume(false), payload_size(0), metadata_size(0) {}
+ rootfs_size(0),
+ hash_checks_mandatory(false) {}
+
+ // Default constructor: Initialize all members which don't have a class
+ // initializer.
+ InstallPlan() : is_resume(false),
+ payload_size(0),
+ metadata_size(0),
+ kernel_size(0),
+ rootfs_size(0),
+ hash_checks_mandatory(false) {}
bool is_resume;
std::string download_url; // url to download from
@@ -62,6 +71,10 @@
std::vector<char> kernel_hash;
std::vector<char> rootfs_hash;
+ // True if payload hash checks are mandatory based on the system state and
+ // the Omaha response.
+ bool hash_checks_mandatory;
+
bool operator==(const InstallPlan& that) const {
return ((is_resume == that.is_resume) &&
(download_url == that.download_url) &&
@@ -84,7 +97,8 @@
<< ", metadata size: " << metadata_size
<< ", metadata signature: " << metadata_signature
<< ", install_path: " << install_path
- << ", kernel_install_path: " << kernel_install_path;
+ << ", kernel_install_path: " << kernel_install_path
+ << ", hash_checks_mandatory: " << hash_checks_mandatory;
}
};
diff --git a/libcurl_http_fetcher.cc b/libcurl_http_fetcher.cc
index 4f66a54..59f065e 100644
--- a/libcurl_http_fetcher.cc
+++ b/libcurl_http_fetcher.cc
@@ -8,6 +8,7 @@
#include <string>
#include <base/logging.h>
+#include <base/string_util.h>
#include <base/stringprintf.h>
#include "update_engine/certificate_checker.h"
@@ -168,16 +169,16 @@
CHECK_EQ(curl_easy_setopt(curl_handle_, CURLOPT_MAXREDIRS, kMaxRedirects),
CURLE_OK);
- // If we are running in test mode or dev mode (the call to IsOfficialBuild is
- // a misnomer that needs to be fixed), then lock down the appropriate curl
- // options for HTTP or HTTPS depending on the url.
+ // If we are running in test mode or using a dev/test build, then lock down
+ // the appropriate curl options for HTTP or HTTPS depending on the url.
if (!is_test_mode_ && IsOfficialBuild()) {
- if (url_to_use.find("http://") == 0)
+ if (StartsWithASCII(url_to_use, "http://", false))
SetCurlOptionsForHttp();
else
SetCurlOptionsForHttps();
} else {
- LOG(INFO) << "Not setting http(s) curl options for test/dev mode";
+ LOG(INFO) << "Not setting http(s) curl options because we are in "
+ << "test mode or running a dev/test image";
}
CHECK_EQ(curl_multi_add_handle(curl_multi_handle_, curl_handle_), CURLM_OK);
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index e952a29..a181542 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -7,6 +7,7 @@
#include <string>
#include <base/logging.h>
+#include "base/string_util.h"
#include "update_engine/delta_performer.h"
#include "update_engine/prefs_interface.h"
@@ -38,7 +39,7 @@
install_plan_.payload_hash = response.hash;
install_plan_.metadata_size = response.metadata_size;
install_plan_.metadata_signature = response.metadata_signature;
-
+ install_plan_.hash_checks_mandatory = AreHashChecksMandatory(response);
install_plan_.is_resume =
DeltaPerformer::CanResumeUpdate(prefs_, response.hash);
if (!install_plan_.is_resume) {
@@ -88,4 +89,35 @@
return true;
}
+bool OmahaResponseHandlerAction::AreHashChecksMandatory(
+ const OmahaResponse& response) {
+ // All our internal testing uses dev server which doesn't generate metadata
+ // signatures yet. So, in order not to break image_to_live or other AU tools,
+ // we should waive the hash checks for those cases. Since all internal
+ // testing is done using a dev_image or test_image, we can use that as a
+ // criteria for waiving. This criteria reduces the attack surface as
+ // opposed to waiving the checks when we're in dev mode, because we do want
+ // to enforce the hash checks when our end customers run in dev mode if they
+ // are using an official build, so that they are protected more.
+ if (!utils::IsOfficialBuild()) {
+ LOG(INFO) << "Waiving payload hash checks for unofficial builds";
+ return false;
+ }
+
+ // TODO(jaysri): VALIDATION: For official builds, we currently waive hash
+ // checks for HTTPS until we have rolled out at least once and are confident
+ // nothing breaks. chromium-os:37082 tracks turning this on for HTTPS
+ // eventually.
+ if (StartsWithASCII(response.codebase, "https://", false)) {
+ LOG(INFO) << "Waiving payload hash checks since Omaha response "
+ << "only has HTTPS URL(s)";
+ return false;
+ }
+
+ // No exceptions apply. So hash checks are mandatory, by default.
+ LOG(INFO) << "Mandating payload hash checks since Omaha response "
+ << "contains HTTP URL(s)";
+ return true;
+}
+
} // namespace chromeos_update_engine
diff --git a/omaha_response_handler_action.h b/omaha_response_handler_action.h
index d9bf01f..32c0f0f 100644
--- a/omaha_response_handler_action.h
+++ b/omaha_response_handler_action.h
@@ -66,6 +66,10 @@
static bool GetInstallDev(const std::string& boot_dev,
std::string* install_dev);
+ // Returns true if payload hash checks are mandatory based on the state
+ // of the system and the contents of the Omaha response. False otherwise.
+ bool AreHashChecksMandatory(const OmahaResponse& response);
+
// Update Engine preference store.
PrefsInterface* prefs_;
diff --git a/omaha_response_handler_action_unittest.cc b/omaha_response_handler_action_unittest.cc
index bed0f04..fd3f23e 100644
--- a/omaha_response_handler_action_unittest.cc
+++ b/omaha_response_handler_action_unittest.cc
@@ -171,4 +171,34 @@
EXPECT_EQ("", install_plan.install_path);
}
+TEST_F(OmahaResponseHandlerActionTest, HashChecksForHttpTest) {
+ OmahaResponse in;
+ in.update_exists = true;
+ in.display_version = "a.b.c.d";
+ in.codebase = "http://test.should/need/hash.checks.signed";
+ in.more_info_url = "http://more/info";
+ in.hash = "HASHj+";
+ in.size = 12;
+ InstallPlan install_plan;
+ EXPECT_TRUE(DoTest(in, "/dev/sda5", &install_plan));
+ EXPECT_EQ(in.codebase, install_plan.download_url);
+ EXPECT_EQ(in.hash, install_plan.payload_hash);
+ EXPECT_TRUE(install_plan.hash_checks_mandatory);
+}
+
+TEST_F(OmahaResponseHandlerActionTest, HashChecksForHttpsTest) {
+ OmahaResponse in;
+ in.update_exists = true;
+ in.display_version = "a.b.c.d";
+ in.codebase = "https://test.should.not/need/hash.checks.signed";
+ in.more_info_url = "http://more/info";
+ in.hash = "HASHj+";
+ in.size = 12;
+ InstallPlan install_plan;
+ EXPECT_TRUE(DoTest(in, "/dev/sda5", &install_plan));
+ EXPECT_EQ(in.codebase, install_plan.download_url);
+ EXPECT_EQ(in.hash, install_plan.payload_hash);
+ EXPECT_FALSE(install_plan.hash_checks_mandatory);
+}
+
} // namespace chromeos_update_engine
diff --git a/payload_signer.cc b/payload_signer.cc
index b75f393..ff60949 100644
--- a/payload_signer.cc
+++ b/payload_signer.cc
@@ -80,10 +80,13 @@
// Given an unsigned payload under |payload_path| and the |signature_blob_size|
// generates an updated payload that includes a dummy signature op in its
-// manifest. Returns true on success, false otherwise.
+// manifest. It populates |out_metadata_size| with the size of the final
+// manifest after adding the dummy signature operation. Returns true on
+// success, false otherwise.
bool AddSignatureOpToPayload(const string& payload_path,
int signature_blob_size,
- vector<char>* out_payload) {
+ vector<char>* out_payload,
+ uint64_t* out_metadata_size) {
const int kProtobufOffset = 20;
const int kProtobufSizeOffset = 12;
@@ -116,6 +119,7 @@
memcpy(&payload[kProtobufSizeOffset], &size_be, sizeof(size_be));
LOG(INFO) << "Updated payload size: " << payload.size();
out_payload->swap(payload);
+ *out_metadata_size = serialized_manifest.size() + kProtobufOffset;
return true;
}
} // namespace {}
@@ -344,9 +348,11 @@
TEST_AND_RETURN_FALSE(ConvertSignatureToProtobufBlob(signatures,
&signature_blob));
vector<char> payload;
+ uint64_t final_metadata_size;
TEST_AND_RETURN_FALSE(AddSignatureOpToPayload(payload_path,
signature_blob.size(),
- &payload));
+ &payload,
+ &final_metadata_size));
// Calculates the hash on the updated payload. Note that the payload includes
// the signature op but doesn't include the signature blob at the end.
TEST_AND_RETURN_FALSE(OmahaHashCalculator::RawHashOfData(payload,
@@ -373,7 +379,8 @@
bool PayloadSigner::AddSignatureToPayload(
const string& payload_path,
const vector<vector<char> >& signatures,
- const string& signed_payload_path) {
+ 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.
@@ -383,7 +390,8 @@
vector<char> payload;
TEST_AND_RETURN_FALSE(AddSignatureOpToPayload(payload_path,
signature_blob.size(),
- &payload));
+ &payload,
+ out_metadata_size));
// Appends the signature blob to the end of the payload and writes the new
// payload.
payload.insert(payload.end(), signature_blob.begin(), signature_blob.end());
diff --git a/payload_signer.h b/payload_signer.h
index 08135d2..6e7965c 100644
--- a/payload_signer.h
+++ b/payload_signer.h
@@ -75,11 +75,14 @@
// 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. Returns true on success, false otherwise.
+ // 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<std::vector<char> >& signatures,
- const std::string& signed_payload_path);
+ const std::string& signed_payload_path,
+ uint64_t* out_metadata_size);
// Returns false if the payload signature can't be verified. Returns true
// otherwise and sets |out_hash| to the signed payload hash.