Parse metadata signature size in payload version 2.

This patch only parse the field and skip the signature without verifying it.

Bug: 23946683
TEST=unit test added.

Change-Id: I53e049c35f8c21d325aeb415ac9a2daf980fcda1
diff --git a/delta_performer.cc b/delta_performer.cc
index c8a00d3..4025b1f 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -56,8 +56,13 @@
 
 namespace chromeos_update_engine {
 
+const uint64_t DeltaPerformer::kDeltaVersionOffset = sizeof(kDeltaMagic);
 const uint64_t DeltaPerformer::kDeltaVersionSize = 8;
+const uint64_t DeltaPerformer::kDeltaManifestSizeOffset =
+    kDeltaVersionOffset + kDeltaVersionSize;
 const uint64_t DeltaPerformer::kDeltaManifestSizeSize = 8;
+const uint64_t DeltaPerformer::kDeltaMetadataSignatureSizeSize = 4;
+const uint64_t DeltaPerformer::kMaxPayloadHeaderSize = 24;
 const uint64_t DeltaPerformer::kSupportedMajorPayloadVersion = 1;
 const uint64_t DeltaPerformer::kSupportedMinorPayloadVersion = 2;
 
@@ -327,25 +332,39 @@
 
 }  // namespace
 
-uint64_t DeltaPerformer::GetVersionOffset() {
-  // Manifest size is stored right after the magic string and the version.
-  return strlen(kDeltaMagic);
+bool DeltaPerformer::GetMetadataSignatureSizeOffset(
+    uint64_t* out_offset) const {
+  if (GetMajorVersion() == kBrilloMajorPayloadVersion) {
+    *out_offset = kDeltaManifestSizeOffset + kDeltaManifestSizeSize;
+    return true;
+  }
+  return false;
 }
 
-uint64_t DeltaPerformer::GetManifestSizeOffset() {
-  // Manifest size is stored right after the magic string and the version.
-  return strlen(kDeltaMagic) + kDeltaVersionSize;
-}
-
-uint64_t DeltaPerformer::GetManifestOffset() {
-  // Actual manifest begins right after the manifest size field.
-  return GetManifestSizeOffset() + kDeltaManifestSizeSize;
+bool DeltaPerformer::GetManifestOffset(uint64_t* out_offset) const {
+  // Actual manifest begins right after the manifest size field or
+  // metadata signature size field if major version >= 2.
+  if (major_payload_version_ == kChromeOSMajorPayloadVersion) {
+    *out_offset = kDeltaManifestSizeOffset + kDeltaManifestSizeSize;
+    return true;
+  }
+  if (major_payload_version_ == kBrilloMajorPayloadVersion) {
+    *out_offset = kDeltaManifestSizeOffset + kDeltaManifestSizeSize +
+                  kDeltaMetadataSignatureSizeSize;
+    return true;
+  }
+  LOG(ERROR) << "Unknown major payload version: " << major_payload_version_;
+  return false;
 }
 
 uint64_t DeltaPerformer::GetMetadataSize() const {
   return metadata_size_;
 }
 
+uint64_t DeltaPerformer::GetMajorVersion() const {
+  return major_payload_version_;
+}
+
 uint32_t DeltaPerformer::GetMinorVersion() const {
   if (manifest_.has_minor_version()) {
     return manifest_.minor_version();
@@ -363,56 +382,82 @@
   return true;
 }
 
+bool DeltaPerformer::IsHeaderParsed() const {
+  return metadata_size_ != 0;
+}
 
 DeltaPerformer::MetadataParseResult DeltaPerformer::ParsePayloadMetadata(
     const chromeos::Blob& payload, ErrorCode* error) {
   *error = ErrorCode::kSuccess;
-  const uint64_t manifest_offset = GetManifestOffset();
-  uint64_t manifest_size = (metadata_size_ ?
-                            metadata_size_ - manifest_offset : 0);
+  uint64_t manifest_offset;
 
-  if (!manifest_size) {
-    // Ensure we have data to cover the payload header.
-    if (payload.size() < manifest_offset)
+  if (!IsHeaderParsed()) {
+    // Ensure we have data to cover the major payload version.
+    if (payload.size() < kDeltaManifestSizeOffset)
       return kMetadataParseInsufficientData;
 
     // Validate the magic string.
-    if (memcmp(payload.data(), kDeltaMagic, strlen(kDeltaMagic)) != 0) {
+    if (memcmp(payload.data(), kDeltaMagic, sizeof(kDeltaMagic)) != 0) {
       LOG(ERROR) << "Bad payload format -- invalid delta magic.";
       *error = ErrorCode::kDownloadInvalidMetadataMagicString;
       return kMetadataParseError;
     }
 
     // Extract the payload version from the metadata.
-    uint64_t major_payload_version;
-    COMPILE_ASSERT(sizeof(major_payload_version) == kDeltaVersionSize,
+    COMPILE_ASSERT(sizeof(major_payload_version_) == kDeltaVersionSize,
                    major_payload_version_size_mismatch);
-    memcpy(&major_payload_version,
-           &payload[GetVersionOffset()],
+    memcpy(&major_payload_version_,
+           &payload[kDeltaVersionOffset],
            kDeltaVersionSize);
     // switch big endian to host
-    major_payload_version = be64toh(major_payload_version);
+    major_payload_version_ = be64toh(major_payload_version_);
 
-    if (major_payload_version != kSupportedMajorPayloadVersion) {
+    if (major_payload_version_ != supported_major_version_) {
       LOG(ERROR) << "Bad payload format -- unsupported payload version: "
-          << major_payload_version;
+          << major_payload_version_;
       *error = ErrorCode::kUnsupportedMajorPayloadVersion;
       return kMetadataParseError;
     }
 
+    // Get the manifest offset now that we have payload version.
+    if (!GetManifestOffset(&manifest_offset)) {
+      *error = ErrorCode::kUnsupportedMajorPayloadVersion;
+      return kMetadataParseError;
+    }
+    // Check again with the manifest offset.
+    if (payload.size() < manifest_offset)
+      return kMetadataParseInsufficientData;
+
     // Next, parse the manifest size.
-    COMPILE_ASSERT(sizeof(manifest_size) == kDeltaManifestSizeSize,
+    COMPILE_ASSERT(sizeof(manifest_size_) == kDeltaManifestSizeSize,
                    manifest_size_size_mismatch);
-    memcpy(&manifest_size,
-           &payload[GetManifestSizeOffset()],
+    memcpy(&manifest_size_,
+           &payload[kDeltaManifestSizeOffset],
            kDeltaManifestSizeSize);
-    manifest_size = be64toh(manifest_size);  // switch big endian to host
+    manifest_size_ = be64toh(manifest_size_);  // switch big endian to host
+
+    uint32_t metadata_signature_size = 0;
+    if (GetMajorVersion() == kBrilloMajorPayloadVersion) {
+      // Parse the metadata signature size.
+      COMPILE_ASSERT(sizeof(metadata_signature_size) ==
+                     kDeltaMetadataSignatureSizeSize,
+                     metadata_signature_size_size_mismatch);
+      uint64_t metadata_signature_size_offset;
+      if (!GetMetadataSignatureSizeOffset(&metadata_signature_size_offset)) {
+        *error = ErrorCode::kError;
+        return kMetadataParseError;
+      }
+      memcpy(&metadata_signature_size,
+             &payload[metadata_signature_size_offset],
+             kDeltaMetadataSignatureSizeSize);
+      metadata_signature_size = be32toh(metadata_signature_size);
+    }
 
     // If the metadata size is present in install plan, check for it immediately
     // 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.
-    metadata_size_ = manifest_offset + manifest_size;
+    metadata_size_ = manifest_offset + manifest_size_ + metadata_signature_size;
     if (install_plan_->hash_checks_mandatory) {
       if (install_plan_->metadata_size != metadata_size_) {
         LOG(ERROR) << "Mandatory metadata size in Omaha response ("
@@ -460,8 +505,12 @@
     *error = ErrorCode::kSuccess;
   }
 
+  if (!GetManifestOffset(&manifest_offset)) {
+    *error = ErrorCode::kUnsupportedMajorPayloadVersion;
+    return kMetadataParseError;
+  }
   // The payload metadata is deemed valid, it's safe to parse the protobuf.
-  if (!manifest_.ParseFromArray(&payload[manifest_offset], manifest_size)) {
+  if (!manifest_.ParseFromArray(&payload[manifest_offset], manifest_size_)) {
     LOG(ERROR) << "Unable to parse manifest in update file.";
     *error = ErrorCode::kDownloadManifestParseError;
     return kMetadataParseError;
@@ -485,11 +534,11 @@
   UpdateOverallProgress(false, "Completed ");
 
   while (!manifest_valid_) {
-    // Read data up to the needed limit; this is either the payload header size,
-    // or the full metadata size (once it becomes known).
-    const bool do_read_header = !metadata_size_;
+    // Read data up to the needed limit; this is either maximium payload header
+    // size, or the full metadata size (once it becomes known).
+    const bool do_read_header = !IsHeaderParsed();
     CopyDataToBuffer(&c_bytes, &count,
-                     (do_read_header ? GetManifestOffset() :
+                     (do_read_header ? kMaxPayloadHeaderSize :
                       metadata_size_));
 
     MetadataParseResult result = ParsePayloadMetadata(buffer_, error);
@@ -497,7 +546,7 @@
       return false;
     if (result == kMetadataParseInsufficientData) {
       // If we just processed the header, make an attempt on the manifest.
-      if (do_read_header && metadata_size_)
+      if (do_read_header && IsHeaderParsed())
         continue;
 
       return true;
diff --git a/delta_performer.h b/delta_performer.h
index 087c2ad..8345174 100644
--- a/delta_performer.h
+++ b/delta_performer.h
@@ -50,8 +50,12 @@
     kMetadataParseInsufficientData,
   };
 
+  static const uint64_t kDeltaVersionOffset;
   static const uint64_t kDeltaVersionSize;
+  static const uint64_t kDeltaManifestSizeOffset;
   static const uint64_t kDeltaManifestSizeSize;
+  static const uint64_t kDeltaMetadataSignatureSizeSize;
+  static const uint64_t kMaxPayloadHeaderSize;
   static const uint64_t kSupportedMajorPayloadVersion;
   static const uint64_t kSupportedMinorPayloadVersion;
 
@@ -81,6 +85,8 @@
         manifest_parsed_(false),
         manifest_valid_(false),
         metadata_size_(0),
+        manifest_size_(0),
+        major_payload_version_(0),
         next_operation_num_(0),
         buffer_offset_(0),
         last_updated_buffer_offset_(kuint64max),
@@ -93,6 +99,7 @@
         last_progress_chunk_(0),
         forced_progress_log_wait_(
             base::TimeDelta::FromSeconds(kProgressLogTimeoutSeconds)),
+        supported_major_version_(kSupportedMajorPayloadVersion),
         supported_minor_version_(kSupportedMinorPayloadVersion) {}
 
   // Opens the kernel. Should be called before or after Open(), but before
@@ -186,26 +193,30 @@
     public_key_path_ = public_key_path;
   }
 
-  // Returns the byte offset at which the payload version can be found.
-  static uint64_t GetVersionOffset();
+  // Set |*out_offset| to the byte offset where the size of the metadata signature
+  // is stored in a payload. Return true on success, if this field is not
+  // present in the payload, return false.
+  bool GetMetadataSignatureSizeOffset(uint64_t* out_offset) const;
 
-  // Returns the byte offset where the size of the manifest is stored in
-  // a payload. This offset precedes the actual start of the manifest
-  // that's returned by the GetManifestOffset method.
-  static uint64_t GetManifestSizeOffset();
-
-  // Returns the byte offset at which the manifest protobuf begins in a
-  // payload.
-  static uint64_t GetManifestOffset();
+  // Set |*out_offset| to the byte offset at which the manifest protobuf begins
+  // in a payload. Return true on success, false if the offset is unknown.
+  bool GetManifestOffset(uint64_t* out_offset) const;
 
   // Returns the size of the payload metadata, which includes the payload header
-  // and the manifest. Is the header was not yet parsed, returns zero.
+  // and the manifest. If the header was not yet parsed, returns zero.
   uint64_t GetMetadataSize() const;
 
   // If the manifest was successfully parsed, copies it to |*out_manifest_p|.
   // Returns true on success.
   bool GetManifest(DeltaArchiveManifest* out_manifest_p) const;
 
+  // Return true if header parsing is finished and no errors occurred.
+  bool IsHeaderParsed() const;
+
+  // Returns the major payload version. If the version was not yet parsed,
+  // returns zero.
+  uint64_t GetMajorVersion() const;
+
   // Returns the delta minor version. If this value is defined in the manifest,
   // it returns that value, otherwise it returns the default value.
   uint32_t GetMinorVersion() const;
@@ -331,6 +342,8 @@
   bool manifest_parsed_;
   bool manifest_valid_;
   uint64_t metadata_size_;
+  uint64_t manifest_size_;
+  uint64_t major_payload_version_;
 
   // Index of the next operation to perform in the manifest.
   size_t next_operation_num_;
@@ -380,6 +393,9 @@
   const base::TimeDelta forced_progress_log_wait_;
   base::Time forced_progress_log_time_;
 
+  // The payload major payload version supported by DeltaPerformer.
+  uint64_t supported_major_version_;
+
   // The delta minor payload version supported by DeltaPerformer.
   uint32_t supported_minor_version_;
 
diff --git a/delta_performer_unittest.cc b/delta_performer_unittest.cc
index 839f253..d2ec5b0 100644
--- a/delta_performer_unittest.cc
+++ b/delta_performer_unittest.cc
@@ -260,6 +260,9 @@
     EXPECT_EQ(install_plan_.metadata_size, performer_.GetMetadataSize());
   }
 
+  void SetSupportedMajorVersion(uint64_t major_version) {
+    performer_.supported_major_version_ = major_version;
+  }
   FakePrefs prefs_;
   InstallPlan install_plan_;
   FakeSystemState fake_system_state_;
@@ -460,11 +463,38 @@
                         ErrorCode::kUnsupportedMinorPayloadVersion);
 }
 
+TEST_F(DeltaPerformerTest, BrilloMetadataSignatureSizeTest) {
+  SetSupportedMajorVersion(kBrilloMajorPayloadVersion);
+  EXPECT_EQ(0, performer_.Open("/dev/null", 0, 0));
+  EXPECT_TRUE(performer_.OpenKernel("/dev/null"));
+  EXPECT_TRUE(performer_.Write(kDeltaMagic, sizeof(kDeltaMagic)));
+
+  uint64_t major_version = htobe64(kBrilloMajorPayloadVersion);
+  EXPECT_TRUE(performer_.Write(&major_version, 8));
+
+  uint64_t manifest_size = rand() % 256;
+  uint64_t manifest_size_be = htobe64(manifest_size);
+  EXPECT_TRUE(performer_.Write(&manifest_size_be, 8));
+
+  uint32_t metadata_signature_size = rand() % 256;
+  uint32_t metadata_signature_size_be = htobe32(metadata_signature_size);
+  EXPECT_TRUE(performer_.Write(&metadata_signature_size_be, 4));
+
+  EXPECT_LT(performer_.Close(), 0);
+
+  EXPECT_TRUE(performer_.IsHeaderParsed());
+  EXPECT_EQ(kBrilloMajorPayloadVersion, performer_.GetMajorVersion());
+  uint64_t manifest_offset;
+  EXPECT_TRUE(performer_.GetManifestOffset(&manifest_offset));
+  EXPECT_EQ(24, manifest_offset);  // 4 + 8 + 8 + 4
+  EXPECT_EQ(24 + manifest_size + metadata_signature_size,
+            performer_.GetMetadataSize());
+}
+
 TEST_F(DeltaPerformerTest, BadDeltaMagicTest) {
   EXPECT_EQ(0, performer_.Open("/dev/null", 0, 0));
   EXPECT_TRUE(performer_.OpenKernel("/dev/null"));
   EXPECT_TRUE(performer_.Write("junk", 4));
-  EXPECT_TRUE(performer_.Write("morejunk", 8));
   EXPECT_FALSE(performer_.Write("morejunk", 8));
   EXPECT_LT(performer_.Close(), 0);
 }
@@ -476,10 +506,9 @@
   EXPECT_CALL(*(fake_system_state_.mock_payload_state()),
               DownloadProgress(4)).Times(1);
   EXPECT_CALL(*(fake_system_state_.mock_payload_state()),
-              DownloadProgress(8)).Times(2);
+              DownloadProgress(8)).Times(1);
 
   EXPECT_TRUE(performer_.Write("junk", 4));
-  EXPECT_TRUE(performer_.Write("morejunk", 8));
   EXPECT_FALSE(performer_.Write("morejunk", 8));
   EXPECT_LT(performer_.Close(), 0);
 }
diff --git a/payload_constants.cc b/payload_constants.cc
index b28461b..cc18430 100644
--- a/payload_constants.cc
+++ b/payload_constants.cc
@@ -28,7 +28,7 @@
 const char kLegacyPartitionNameKernel[] = "boot";
 const char kLegacyPartitionNameRoot[] = "system";
 
-const char kDeltaMagic[] = "CrAU";
+const char kDeltaMagic[4] = {'C', 'r', 'A', 'U'};
 const char kBspatchPath[] = "bspatch";
 
 const char* InstallOperationTypeName(InstallOperation_Type op_type) {
diff --git a/payload_constants.h b/payload_constants.h
index 81bc36a..38cc075 100644
--- a/payload_constants.h
+++ b/payload_constants.h
@@ -48,7 +48,7 @@
 extern const char kLegacyPartitionNameRoot[];
 
 extern const char kBspatchPath[];
-extern const char kDeltaMagic[];
+extern const char kDeltaMagic[4];
 
 // A block number denoting a hole on a sparse file. Used on Extents to refer to
 // section of blocks not present on disk on a sparse file.
diff --git a/payload_generator/payload_file.cc b/payload_generator/payload_file.cc
index f0a66cc..cdcc967 100644
--- a/payload_generator/payload_file.cc
+++ b/payload_generator/payload_file.cc
@@ -191,7 +191,7 @@
   ScopedFileWriterCloser writer_closer(&writer);
 
   // Write header
-  TEST_AND_RETURN_FALSE(writer.Write(kDeltaMagic, strlen(kDeltaMagic)));
+  TEST_AND_RETURN_FALSE(writer.Write(kDeltaMagic, sizeof(kDeltaMagic)));
 
   // Write major version number
   TEST_AND_RETURN_FALSE(WriteUint64AsBigEndian(&writer, major_version_));
@@ -241,7 +241,7 @@
   }
 
   *medatata_size_out =
-      strlen(kDeltaMagic) + 2 * sizeof(uint64_t) + serialized_manifest.size();
+      sizeof(kDeltaMagic) + 2 * sizeof(uint64_t) + serialized_manifest.size();
   ReportPayloadUsage(*medatata_size_out);
   return true;
 }