Generate and validate per-operation hashes
As part of securing the HTTP-based updates, we want to add a SHA256 hash
of the data blob for each operation so that they can't be tampered with
by a man in the middle. This CL adds support for generating and
including such hashes for each operation in the payload as well as
validating them in update_engine, if present.
BUG=chromium-os:34298
TEST=Tested on ZGB to make sure existing functionality works fine.
Existing unit tests cover all the new code paths.
Change-Id: Ie42ed1930a66ceaf183f36ce3af0dea719e44237
Reviewed-on: https://gerrit.chromium.org/gerrit/33389
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Commit-Ready: 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 06952c6..26f3133 100644
--- a/delta_diff_generator.cc
+++ b/delta_diff_generator.cc
@@ -1178,6 +1178,9 @@
ssize_t rc = pread(in_fd, &buf[0], buf.size(), op->data_offset());
TEST_AND_RETURN_FALSE(rc == static_cast<ssize_t>(buf.size()));
+ // Add the hash of the data blobs for this operation
+ TEST_AND_RETURN_FALSE(AddOperationHash(op, buf));
+
op->set_data_offset(out_file_size);
TEST_AND_RETURN_FALSE(writer.Write(&buf[0], buf.size()));
out_file_size += buf.size();
@@ -1185,6 +1188,19 @@
return true;
}
+bool DeltaDiffGenerator::AddOperationHash(
+ DeltaArchiveManifest_InstallOperation* op,
+ const vector<char>& buf) {
+ OmahaHashCalculator hasher;
+
+ TEST_AND_RETURN_FALSE(hasher.Update(&buf[0], buf.size()));
+ TEST_AND_RETURN_FALSE(hasher.Finalize());
+
+ const vector<char>& hash = hasher.raw_hash();
+ op->set_data_sha256_hash(hash.data(), hash.size());
+ return true;
+}
+
bool DeltaDiffGenerator::ConvertCutToFullOp(Graph* graph,
const CutEdgeVertexes& cut,
const string& new_root,
diff --git a/delta_diff_generator.h b/delta_diff_generator.h
index b646545..f4717e7 100644
--- a/delta_diff_generator.h
+++ b/delta_diff_generator.h
@@ -170,6 +170,15 @@
const std::string& data_blobs_path,
const std::string& new_data_blobs_path);
+ // Computes a SHA256 hash of the given buf and sets the hash value in the
+ // operation so that update_engine could verify. This hash should be set
+ // for all operations that have a non-zero data blob. One exception is the
+ // dummy operation for signature blob because the contents of the signature
+ // blob will not be available at payload creation time. So, update_engine will
+ // gracefully ignore the dummy signature operation.
+ static bool AddOperationHash(DeltaArchiveManifest_InstallOperation* op,
+ const std::vector<char>& buf);
+
// Handles allocation of temp blocks to a cut edge by converting the
// dest node to a full op. This removes the need for temp blocks, but
// comes at the cost of a worse compression ratio.
diff --git a/delta_performer.cc b/delta_performer.cc
index 46739b1..d0a38e0 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -340,19 +340,37 @@
return true;
}
- *error = ValidateOperationHash(op);
- if (*error != kActionCodeSuccess) {
- // Cannot proceed further as operation hash is invalid.
- // Higher level code will take care of retrying appropriately.
- return false;
+ bool should_log = (next_operation_num_ % 1000 == 0 ||
+ next_operation_num_ == total_operations - 1);
+
+ // Validate the operation only if the manifest 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.
+ if (!install_plan_->manifest_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.
+ //
+ // TODO(jaysri): Add a UMA stat to indicate that an operation hash
+ // was failed to be validated as expected.
+ //
+ // 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.
+ // return false;
+ LOG(INFO) << "Ignoring operation validation errors for now";
+ }
}
// Makes sure we unblock exit when this operation completes.
ScopedTerminatorExitUnblocker exit_unblocker =
ScopedTerminatorExitUnblocker(); // Avoids a compiler unused var bug.
// Log every thousandth operation, and also the first and last ones
- if ((next_operation_num_ % 1000 == 0) ||
- (next_operation_num_ + 1 == total_operations)) {
+ if (should_log) {
LOG(INFO) << "Performing operation " << (next_operation_num_ + 1) << "/"
<< total_operations;
}
@@ -719,10 +737,70 @@
}
ActionExitCode DeltaPerformer::ValidateOperationHash(
- const DeltaArchiveManifest_InstallOperation&
- operation) {
+ const DeltaArchiveManifest_InstallOperation& operation,
+ bool should_log) {
- // TODO(jaysri): To be implemented.
+ if (!operation.data_sha256_hash().size()) {
+ if (!operation.data_length()) {
+ // Operations that do not have any data blob won't have any operation hash
+ // either. So, these operations are always considered validated since the
+ // manifest that contains all the non-data-blob portions of the operation
+ // has already been validated.
+ return kActionCodeSuccess;
+ }
+
+ // TODO(jaysri): Add a UMA stat here so we're aware of any
+ // man-in-the-middle attempts to bypass these checks.
+ //
+ // TODO(jaysri): VALIDATION: no hash is present for the operation. 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.
+ // 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
+ // recognized by the fact that it's offset is mentioned in the manifest.
+ if (manifest_.signatures_offset() &&
+ manifest_.signatures_offset() == operation.data_offset()) {
+ 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";
+ }
+ return kActionCodeSuccess;
+ }
+
+ vector<char> expected_op_hash;
+ expected_op_hash.assign(operation.data_sha256_hash().data(),
+ (operation.data_sha256_hash().data() +
+ operation.data_sha256_hash().size()));
+
+ OmahaHashCalculator operation_hasher;
+ operation_hasher.Update(&buffer_[0], operation.data_length());
+ if (!operation_hasher.Finalize()) {
+ LOG(ERROR) << "Unable to compute actual hash of operation "
+ << next_operation_num_;
+ return kActionCodeDownloadOperationHashVerificationError;
+ }
+
+ vector<char> calculated_op_hash = operation_hasher.raw_hash();
+ if (calculated_op_hash != expected_op_hash) {
+ LOG(ERROR) << "Hash verification failed for operation "
+ << next_operation_num_ << ". Expected hash = ";
+ utils::HexDumpVector(expected_op_hash);
+ LOG(ERROR) << "Calculated hash over " << operation.data_length()
+ << " bytes at offset: " << operation.data_offset() << " = ";
+ utils::HexDumpVector(calculated_op_hash);
+ return kActionCodeDownloadOperationHashMismatch;
+ }
+
+ if (should_log)
+ LOG(INFO) << "Validated operation " << next_operation_num_ + 1;
+
return kActionCodeSuccess;
}
diff --git a/delta_performer.h b/delta_performer.h
index 8ea30c3..a9d70e7 100644
--- a/delta_performer.h
+++ b/delta_performer.h
@@ -155,7 +155,7 @@
// matches what's specified in the manifest in the payload.
// Returns kActionCodeSuccess on match or a suitable error code otherwise.
ActionExitCode ValidateOperationHash(
- const DeltaArchiveManifest_InstallOperation& operation);
+ const DeltaArchiveManifest_InstallOperation& operation, bool should_log);
// Interprets the given |protobuf| as a DeltaArchiveManifest protocol buffer
// of the given protobuf_length and verifies that the signed hash of the