commit b5bd075b5e2cb67ecd445d6df0dc8563d59fdad1
author Kelvin Zhang <zhangkelvin@google.com> Thu Mar 10 15:45:25 2022 -0800
committer Kelvin Zhang <zhangkelvin@google.com> Mon Mar 14 13:47:07 2022 -0700
tree a57e23193787f962b8da84ca019af4082084dc89
parent 596a3206dac75841719f76e67f64576cfaca2331

Verify hashes of source copy blocks at install time

In VABC, all copy ops are performed upfront without actually verifying
the hash. This CL adds the missing hash checks.

Test: th
Bug: 223471136
Change-Id: I1ec7ddee3122c9614484b2d8b7e9f38128428080

payload_consumer/partition_writer.cc

diff --git a/payload_consumer/partition_writer.cc b/payload_consumer/partition_writer.cc
index b94f8c7..1fb929e 100644
--- a/payload_consumer/partition_writer.cc
+++ b/payload_consumer/partition_writer.cc
@@ -295,6 +295,16 @@
   return std::make_unique<DirectExtentWriter>(target_fd_);
 }
 
+bool PartitionWriter::ValidateSourceHash(const InstallOperation& operation,
+                                         const FileDescriptorPtr source_fd,
+                                         size_t block_size,
+                                         ErrorCode* error) {
+  brillo::Blob source_hash;
+  TEST_AND_RETURN_FALSE_ERRNO(fd_utils::ReadAndHashExtents(
+      source_fd, operation.src_extents(), block_size, &source_hash));
+  return ValidateSourceHash(source_hash, operation, source_fd, error);
+}
+
 bool PartitionWriter::ValidateSourceHash(const brillo::Blob& calculated_hash,
                                          const InstallOperation& operation,
                                          const FileDescriptorPtr source_fd,

payload_consumer/partition_writer.h

diff --git a/payload_consumer/partition_writer.h b/payload_consumer/partition_writer.h
index 89e5884..e620c47 100644
--- a/payload_consumer/partition_writer.h
+++ b/payload_consumer/partition_writer.h
@@ -46,6 +46,10 @@
                                  const InstallOperation& operation,
                                  const FileDescriptorPtr source_fd,
                                  ErrorCode* error);
+  static bool ValidateSourceHash(const InstallOperation& operation,
+                                 const FileDescriptorPtr source_fd,
+                                 size_t block_size,
+                                 ErrorCode* error);
 
   // Perform necessary initialization work before InstallOperation can be
   // applied to this partition

payload_consumer/vabc_partition_writer.cc

diff --git a/payload_consumer/vabc_partition_writer.cc b/payload_consumer/vabc_partition_writer.cc
index 9db88a9..583ba4c 100644
--- a/payload_consumer/vabc_partition_writer.cc
+++ b/payload_consumer/vabc_partition_writer.cc
@@ -33,6 +33,7 @@
 #include "update_engine/payload_consumer/extent_map.h"
 #include "update_engine/payload_consumer/extent_reader.h"
 #include "update_engine/payload_consumer/file_descriptor.h"
+#include "update_engine/payload_consumer/file_descriptor_utils.h"
 #include "update_engine/payload_consumer/install_plan.h"
 #include "update_engine/payload_consumer/partition_writer.h"
 #include "update_engine/payload_consumer/snapshot_extent_writer.h"
@@ -267,9 +268,13 @@
 
 [[nodiscard]] bool VABCPartitionWriter::PerformSourceCopyOperation(
     const InstallOperation& operation, ErrorCode* error) {
-  // TODO(zhangkelvin) Probably just ignore SOURCE_COPY? They should be taken
-  // care of during Init();
-  return true;
+  // COPY ops are already handled during Init(), no need to do actual work, but
+  // we still want to verify that all blocks contain expected data.
+  auto source_fd = std::make_shared<EintrSafeFileDescriptor>();
+  TEST_AND_RETURN_FALSE_ERRNO(
+      source_fd->Open(install_part_.source_path.c_str(), O_RDONLY));
+  return PartitionWriter::ValidateSourceHash(
+      operation, source_fd, block_size_, error);
 }
 
 bool VABCPartitionWriter::PerformReplaceOperation(const InstallOperation& op,