Support needed for generating metadata signature in paygen

The metadata is the first portion of a payload that contains the following:
1. magic string ("CrOS")
2. version number
3. length of the manifest protobuf
4. manifest protobuf itself
<payload blobs begin here>
<payload signature as the last blob>

Currently we have a manifest signature which protects only #4 above. In
this CL we're extending the scope of manifest signature to include the rest
of the metadata (1-4). The reason we need to do this is to protect the
version value in HTTP as we're going to use it in future to have the
flexibility to change the protobuf format of the manifest.

Besides this change, this CL also contains:

1. Renaming of manifest_size and manifest_signature to metadata_size and
metadata_signature respectively to reflect the above change and keep
consistent terminology throughout. Also it renames protobuf_offset and
protobuf_length to manifest_offset and manifest_size to increase the
contextual semantics of the protobuf.

2. Addition of a new command-line option --out_metadata_hash_file in
delta_generator so that au_generate can use it in a subsequent CL to get
the SHA256 hash of the payload metadata in order to get it signed with
the signer.

3. Reusing LoadPayload in unit tests to get rid of some hardcoding. Also
updated delta_performer to localize such hardcoded constants within that
class and not have callers worry about those values.

BUG=chromium-os:33603
TEST=Tested on ZGB. Reran existing unit tests.
Change-Id: Iace5aebe8f7d054a0fa3a224a588ef52d85f510b
Reviewed-on: https://gerrit.chromium.org/gerrit/33726
Commit-Ready: Jay Srinivasan <jaysri@chromium.org>
Reviewed-by: Jay Srinivasan <jaysri@chromium.org>
Tested-by: Jay Srinivasan <jaysri@chromium.org>
diff --git a/action_processor.h b/action_processor.h
index 3c9c262..4de4f84 100644
--- a/action_processor.h
+++ b/action_processor.h
@@ -45,12 +45,12 @@
   kActionCodeDownloadPayloadPubKeyVerificationError = 18,
   kActionCodePostinstallBootedFromFirmwareB = 19,
   kActionCodeDownloadStateInitializationError = 20,
-  kActionCodeDownloadInvalidManifest = 21,
+  kActionCodeDownloadInvalidMetadataMagicString = 21,
   kActionCodeDownloadSignatureMissingInManifest = 22,
   kActionCodeDownloadManifestParseError = 23,
-  kActionCodeDownloadManifestSignatureError = 24,
-  kActionCodeDownloadManifestSignatureVerificationError = 25,
-  kActionCodeDownloadManifestSignatureMismatch = 26,
+  kActionCodeDownloadMetadataSignatureError = 24,
+  kActionCodeDownloadMetadataSignatureVerificationError = 25,
+  kActionCodeDownloadMetadataSignatureMismatch = 26,
   kActionCodeDownloadOperationHashVerificationError = 27,
   kActionCodeDownloadOperationExecutionError = 28,
   kActionCodeDownloadOperationHashMismatch = 29,
diff --git a/delta_performer.cc b/delta_performer.cc
index d0a38e0..bcba546 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -34,13 +34,12 @@
 
 namespace chromeos_update_engine {
 
+const uint64_t DeltaPerformer::kDeltaVersionSize = 8;
+const uint64_t DeltaPerformer::kDeltaManifestSizeSize = 8;
 const char DeltaPerformer::kUpdatePayloadPublicKeyPath[] =
     "/usr/share/update_engine/update-payload-key.pub.pem";
 
 namespace {
-
-const int kDeltaVersionLength = 8;
-const int kDeltaProtobufLengthLength = 8;
 const int kUpdateStateOperationInvalid = -1;
 const int kMaxResumedUpdateFailures = 10;
 
@@ -196,6 +195,17 @@
 
 }  // namespace {}
 
+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;
+}
+
+
 DeltaPerformer::MetadataParseResult DeltaPerformer::ParsePayloadMetadata(
     const std::vector<char>& payload,
     DeltaArchiveManifest* manifest,
@@ -203,44 +213,44 @@
     ActionExitCode* error) {
   *error = kActionCodeSuccess;
 
-  const uint64_t protobuf_offset = strlen(kDeltaMagic) + kDeltaVersionLength +
-                                   kDeltaProtobufLengthLength;
-  if (payload.size() < protobuf_offset) {
-    // Don't have enough bytes to know the protobuf length.
+  // manifest_offset is the byte offset where the manifest protobuf begins.
+  const uint64_t manifest_offset = GetManifestOffset();
+  if (payload.size() < manifest_offset) {
+    // Don't have enough bytes to even know the manifest size.
     return kMetadataParseInsufficientData;
   }
 
-  // Validate the magic number.
+  // Validate the magic string.
   if (memcmp(payload.data(), kDeltaMagic, strlen(kDeltaMagic)) != 0) {
     LOG(ERROR) << "Bad payload format -- invalid delta magic.";
-    *error = kActionCodeDownloadInvalidManifest;
+    *error = kActionCodeDownloadInvalidMetadataMagicString;
     return kMetadataParseError;
   }
 
   // TODO(jaysri): Compare the version number and skip unknown manifest
   // versions. We don't check the version at all today.
 
-  // Next, parse the proto buf length.
-  uint64_t protobuf_length;
-  COMPILE_ASSERT(sizeof(protobuf_length) == kDeltaProtobufLengthLength,
-                 protobuf_length_size_mismatch);
-  memcpy(&protobuf_length,
-         &payload[strlen(kDeltaMagic) + kDeltaVersionLength],
-         kDeltaProtobufLengthLength);
-  protobuf_length = be64toh(protobuf_length);  // switch big endian to host
+  // Next, parse the manifest size.
+  uint64_t manifest_size;
+  COMPILE_ASSERT(sizeof(manifest_size) == kDeltaManifestSizeSize,
+                 manifest_size_size_mismatch);
+  memcpy(&manifest_size,
+         &payload[GetManifestSizeOffset()],
+         kDeltaManifestSizeSize);
+  manifest_size = be64toh(manifest_size);  // switch big endian to host
 
   // Now, check if the metasize we computed matches what was passed in
   // through Omaha Response.
-  *metadata_size = protobuf_offset + protobuf_length;
+  *metadata_size = manifest_offset + manifest_size;
 
-  // If the manifest size is present in install plan, check for it immediately
+  // 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 manifest size.
-  if (install_plan_->manifest_size > 0 &&
-      install_plan_->manifest_size != *metadata_size) {
-      LOG(ERROR) << "Invalid manifest size. Expected = "
-                 << install_plan_->manifest_size
+  // 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;
       // TODO(jaysri): Add a UMA Stat here to help with the decision to enforce
       // this check in a future release, as mentioned below.
@@ -258,19 +268,18 @@
   }
 
   // Log whether we validated the size or simply trusting what's in the payload
-  // here. This is logged here (after we received the full manifest data) so
+  // 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_->manifest_size == 0)
-    LOG(WARNING) << "No manifest size specified in Omaha. "
-                 << "Trusting manifest size in payload = " << *metadata_size;
+  if (install_plan_->metadata_size == 0)
+    LOG(WARNING) << "No metadata size specified in Omaha. "
+                 << "Trusting metadata size in payload = " << *metadata_size;
   else
     LOG(INFO) << "Manifest size in payload matches expected value from Omaha";
 
-  // We have the full proto buffer in |payload|. Verify its integrity
-  // and authenticity based on what Omaha says.
-  *error = ValidateManifestSignature(&payload[protobuf_offset],
-                                     protobuf_length);
+  // 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) {
     // TODO(jaysri): Add a UMA Stat here to help with the decision to enforce
     // this check in a future release, as mentioned below.
@@ -281,8 +290,9 @@
     *error = kActionCodeSuccess;
   }
 
-  // The proto buffer in |payload| is deemed valid. Parse it.
-  if (!manifest->ParseFromArray(&payload[protobuf_offset], protobuf_length)) {
+  // The metadata in |payload| is deemed valid. So, it's now safe to
+  // parse the protobuf.
+  if (!manifest->ParseFromArray(&payload[manifest_offset], manifest_size)) {
     LOG(ERROR) << "Unable to parse manifest in update file.";
     *error = kActionCodeDownloadManifestParseError;
     return kMetadataParseError;
@@ -343,10 +353,10 @@
     bool should_log = (next_operation_num_ % 1000 == 0 ||
                        next_operation_num_ == total_operations - 1);
 
-    // Validate the operation only if the manifest signature is present.
+    // 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.
-    if (!install_plan_->manifest_signature.empty()) {
+    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.
@@ -678,10 +688,10 @@
   return true;
 }
 
-ActionExitCode DeltaPerformer::ValidateManifestSignature(
-    const char* protobuf, uint64_t protobuf_length) {
+ActionExitCode DeltaPerformer::ValidateMetadataSignature(
+    const char* metadata, uint64_t metadata_size) {
 
-  if (install_plan_->manifest_signature.empty()) {
+  if (install_plan_->metadata_signature.empty()) {
     // 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
@@ -689,47 +699,47 @@
     // 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.
-    LOG(WARNING) << "Cannot validate manifest as the signature is empty";
+    LOG(WARNING) << "Cannot validate metadata as the signature is empty";
     return kActionCodeSuccess;
   }
 
   // Convert base64-encoded signature to raw bytes.
-  vector<char> manifest_signature;
-  if (!OmahaHashCalculator::Base64Decode(install_plan_->manifest_signature,
-                                         &manifest_signature)) {
-    LOG(ERROR) << "Unable to decode base64 manifest signature: "
-               << install_plan_->manifest_signature;
-    return kActionCodeDownloadManifestSignatureError;
+  vector<char> metadata_signature;
+  if (!OmahaHashCalculator::Base64Decode(install_plan_->metadata_signature,
+                                         &metadata_signature)) {
+    LOG(ERROR) << "Unable to decode base64 metadata signature: "
+               << install_plan_->metadata_signature;
+    return kActionCodeDownloadMetadataSignatureError;
   }
 
-  vector<char> expected_manifest_hash;
-  if (!PayloadSigner::GetRawHashFromSignature(manifest_signature,
+  vector<char> expected_metadata_hash;
+  if (!PayloadSigner::GetRawHashFromSignature(metadata_signature,
                                               public_key_path_,
-                                              &expected_manifest_hash)) {
-    LOG(ERROR) << "Unable to compute expected hash from manifest signature";
-    return kActionCodeDownloadManifestSignatureError;
+                                              &expected_metadata_hash)) {
+    LOG(ERROR) << "Unable to compute expected hash from metadata signature";
+    return kActionCodeDownloadMetadataSignatureError;
   }
 
-  OmahaHashCalculator manifest_hasher;
-  manifest_hasher.Update(protobuf, protobuf_length);
-  if (!manifest_hasher.Finalize()) {
+  OmahaHashCalculator metadata_hasher;
+  metadata_hasher.Update(metadata, metadata_size);
+  if (!metadata_hasher.Finalize()) {
     LOG(ERROR) << "Unable to compute actual hash of manifest";
-    return kActionCodeDownloadManifestSignatureVerificationError;
+    return kActionCodeDownloadMetadataSignatureVerificationError;
   }
 
-  vector<char> calculated_manifest_hash = manifest_hasher.raw_hash();
-  PayloadSigner::PadRSA2048SHA256Hash(&calculated_manifest_hash);
-  if (calculated_manifest_hash.empty()) {
-    LOG(ERROR) << "Computed actual hash of manifest is empty.";
-    return kActionCodeDownloadManifestSignatureVerificationError;
+  vector<char> calculated_metadata_hash = metadata_hasher.raw_hash();
+  PayloadSigner::PadRSA2048SHA256Hash(&calculated_metadata_hash);
+  if (calculated_metadata_hash.empty()) {
+    LOG(ERROR) << "Computed actual hash of metadata is empty.";
+    return kActionCodeDownloadMetadataSignatureVerificationError;
   }
 
-  if (calculated_manifest_hash != expected_manifest_hash) {
+  if (calculated_metadata_hash != expected_metadata_hash) {
     LOG(ERROR) << "Manifest hash verification failed. Expected hash = ";
-    utils::HexDumpVector(expected_manifest_hash);
+    utils::HexDumpVector(expected_metadata_hash);
     LOG(ERROR) << "Calculated hash = ";
-    utils::HexDumpVector(calculated_manifest_hash);
-    return kActionCodeDownloadManifestSignatureMismatch;
+    utils::HexDumpVector(calculated_metadata_hash);
+    return kActionCodeDownloadMetadataSignatureMismatch;
   }
 
   LOG(INFO) << "Manifest signature matches expected value in Omaha response";
@@ -744,7 +754,7 @@
     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
+      // metadata that contains all the non-data-blob portions of the operation
       // has already been validated.
       return kActionCodeSuccess;
     }
diff --git a/delta_performer.h b/delta_performer.h
index a9d70e7..a484096 100644
--- a/delta_performer.h
+++ b/delta_performer.h
@@ -32,6 +32,8 @@
     kMetadataParseInsufficientData,
   };
 
+  static const uint64_t kDeltaVersionSize;
+  static const uint64_t kDeltaManifestSizeSize;
   static const char kUpdatePayloadPublicKeyPath[];
 
   DeltaPerformer(PrefsInterface* prefs, InstallPlan* install_plan)
@@ -133,6 +135,15 @@
     public_key_path_ = public_key_path;
   }
 
+  // Returns the byte offset at which the manifest protobuf begins in a
+  // payload.
+  static uint64_t GetManifestOffset();
+
+  // 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();
+
  private:
   friend class DeltaPerformerTest;
   FRIEND_TEST(DeltaPerformerTest, IsIdempotentOperationTest);
@@ -159,13 +170,13 @@
 
   // Interprets the given |protobuf| as a DeltaArchiveManifest protocol buffer
   // of the given protobuf_length and verifies that the signed hash of the
-  // manifest matches what's specified in the install plan from Omaha.
+  // metadata matches what's specified in the install plan from Omaha.
   // Returns kActionCodeSuccess on match or a suitable error code otherwise.
   // This method must be called before any part of the |protobuf| is parsed
   // so that a man-in-the-middle attack on the SSL connection to the payload
   // server doesn't exploit any vulnerability in the code that parses the
   // protocol buffer.
-  ActionExitCode ValidateManifestSignature(const char* protobuf,
+  ActionExitCode ValidateMetadataSignature(const char* protobuf,
                                            uint64_t protobuf_length);
 
   // Returns true on success.
diff --git a/delta_performer_unittest.cc b/delta_performer_unittest.cc
index 96b780c..bfb57eb 100644
--- a/delta_performer_unittest.cc
+++ b/delta_performer_unittest.cc
@@ -371,23 +371,14 @@
 
   // Read delta into memory.
   vector<char> delta;
-  EXPECT_TRUE(utils::ReadFile(delta_path, &delta));
-
-  uint64_t manifest_metadata_size;
+  uint64_t metadata_size;
 
   // Check the metadata.
   {
-    LOG(INFO) << "delta size: " << delta.size();
     DeltaArchiveManifest manifest;
-    const int kManifestSizeOffset = 12;
-    const int kManifestOffset = 20;
-    uint64_t manifest_size = 0;
-    memcpy(&manifest_size, &delta[kManifestSizeOffset], sizeof(manifest_size));
-    manifest_size = be64toh(manifest_size);
-    LOG(INFO) << "manifest size: " << manifest_size;
-    EXPECT_TRUE(manifest.ParseFromArray(&delta[kManifestOffset],
-                                        manifest_size));
-    manifest_metadata_size = kManifestOffset + manifest_size;
+    EXPECT_TRUE(PayloadSigner::LoadPayload(delta_path, &delta, &manifest,
+                                           &metadata_size));
+    LOG(INFO) << "Metadata size: " << metadata_size;
 
     if (signature_test == kSignatureNone) {
       EXPECT_FALSE(manifest.has_signatures_offset());
@@ -397,7 +388,7 @@
       EXPECT_TRUE(manifest.has_signatures_size());
       Signatures sigs_message;
       EXPECT_TRUE(sigs_message.ParseFromArray(
-          &delta[manifest_metadata_size + manifest.signatures_offset()],
+          &delta[metadata_size + manifest.signatures_offset()],
           manifest.signatures_size()));
       if (signature_test == kSignatureGeneratedShellRotateCl1 ||
           signature_test == kSignatureGeneratedShellRotateCl2)
@@ -448,7 +439,7 @@
 
   PrefsMock prefs;
   EXPECT_CALL(prefs, SetInt64(kPrefsManifestMetadataSize,
-                              manifest_metadata_size)).WillOnce(Return(true));
+                              metadata_size)).WillOnce(Return(true));
   EXPECT_CALL(prefs, SetInt64(kPrefsUpdateStateNextOperation, _))
       .WillRepeatedly(Return(true));
   EXPECT_CALL(prefs, GetInt64(kPrefsUpdateStateNextOperation, _))
@@ -664,29 +655,21 @@
 
   // Read delta into memory.
   vector<char> delta;
-  EXPECT_TRUE(utils::ReadFile(delta_path, &delta));
-
-  uint64_t manifest_metadata_size;
-  const int kManifestSizeOffset = 12;
-  const int kManifestOffset = 20;
+  uint64_t metadata_size;
   // Check the metadata.
   {
-    LOG(INFO) << "delta size: " << delta.size();
     DeltaArchiveManifest manifest;
-    uint64_t manifest_size = 0;
-    memcpy(&manifest_size, &delta[kManifestSizeOffset], sizeof(manifest_size));
-    manifest_size = be64toh(manifest_size);
-    LOG(INFO) << "manifest size: " << manifest_size;
-    EXPECT_TRUE(manifest.ParseFromArray(&delta[kManifestOffset],
-                                        manifest_size));
-    manifest_metadata_size = kManifestOffset + manifest_size;
+    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[manifest_metadata_size + manifest.signatures_offset()],
+          &delta[metadata_size + manifest.signatures_offset()],
           manifest.signatures_size()));
       EXPECT_EQ(1, sigs_message.signatures_size());
       const Signatures_Signature& signature = sigs_message.signatures(0);
@@ -725,7 +708,7 @@
 
   PrefsMock prefs;
   EXPECT_CALL(prefs, SetInt64(kPrefsManifestMetadataSize,
-                              manifest_metadata_size)).WillOnce(Return(true));
+                              metadata_size)).WillOnce(Return(true));
   EXPECT_CALL(prefs, SetInt64(kPrefsUpdateStateNextOperation, _))
       .WillRepeatedly(Return(true));
   EXPECT_CALL(prefs, GetInt64(kPrefsUpdateStateNextOperation, _))
@@ -741,14 +724,14 @@
 
   // Update the A image in place.
   InstallPlan install_plan;
-  install_plan.manifest_size = manifest_metadata_size;
-  LOG(INFO) << "Setting Omaha manifest size = " << manifest_metadata_size;
-  ASSERT_TRUE(PayloadSigner::GetManifestSignature(
-      &delta[kManifestOffset],
-      manifest_metadata_size-kManifestOffset,
+  install_plan.metadata_size = metadata_size;
+  LOG(INFO) << "Setting payload metadata size in Omaha  = " << metadata_size;
+  ASSERT_TRUE(PayloadSigner::GetMetadataSignature(
+      &delta[0],
+      metadata_size,
       kUnittestPrivateKeyPath,
-      &install_plan.manifest_signature));
-  EXPECT_FALSE(install_plan.manifest_signature.empty());
+      &install_plan.metadata_signature));
+  EXPECT_FALSE(install_plan.metadata_signature.empty());
 
   DeltaPerformer performer(&prefs, &install_plan);
   EXPECT_TRUE(utils::FileExists(kUnittestPublicKeyPath));
diff --git a/generate_delta_main.cc b/generate_delta_main.cc
index f5fe5e7..6991813 100644
--- a/generate_delta_main.cc
+++ b/generate_delta_main.cc
@@ -41,6 +41,7 @@
               "and apply delta over old_image (for debugging)");
 DEFINE_string(out_file, "", "Path to output delta payload file");
 DEFINE_string(out_hash_file, "", "Path to output hash file");
+DEFINE_string(out_metadata_hash_file, "", "Path to output metadata hash file");
 DEFINE_string(private_key, "", "Path to private key in .pem format");
 DEFINE_string(public_key, "", "Path to public key in .pem format");
 DEFINE_int32(public_key_version,
@@ -78,32 +79,63 @@
   return S_ISDIR(stbuf.st_mode);
 }
 
+void ParseSignatureSizes(vector<int>* sizes) {
+  LOG_IF(FATAL, FLAGS_signature_size.empty())
+      << "Must pass --signature_size to calculate hash for signing.";
+  vector<string> strsizes;
+  base::SplitString(FLAGS_signature_size, ':', &strsizes);
+  for (vector<string>::iterator it = strsizes.begin(), e = strsizes.end();
+       it != e; ++it) {
+    int size = 0;
+    bool parsing_successful = base::StringToInt(*it, &size);
+    LOG_IF(FATAL, !parsing_successful)
+        << "Invalid signature size: " << *it;
+    sizes->push_back(size);
+  }
+}
+
+
 void CalculatePayloadHashForSigning() {
   LOG(INFO) << "Calculating payload hash for signing.";
   LOG_IF(FATAL, FLAGS_in_file.empty())
       << "Must pass --in_file to calculate hash for signing.";
   LOG_IF(FATAL, FLAGS_out_hash_file.empty())
       << "Must pass --out_hash_file to calculate hash for signing.";
-  LOG_IF(FATAL, FLAGS_signature_size.empty())
-      << "Must pass --signature_size to calculate hash for signing.";
   vector<int> sizes;
-  vector<string> strsizes;
-  base::SplitString(FLAGS_signature_size, ':', &strsizes);
-  for (vector<string>::iterator it = strsizes.begin(), e = strsizes.end();
-       it != e; ++it) {
-    int size = 0;
-    LOG_IF(FATAL, !base::StringToInt(*it, &size))
-        << "Not an integer: " << *it;
-    sizes.push_back(size);
-  }
+  ParseSignatureSizes(&sizes);
+
   vector<char> hash;
-  CHECK(PayloadSigner::HashPayloadForSigning(
-      FLAGS_in_file, sizes, &hash));
-  CHECK(utils::WriteFile(
-      FLAGS_out_hash_file.c_str(), hash.data(), hash.size()));
+  bool result = PayloadSigner::HashPayloadForSigning(FLAGS_in_file, sizes,
+                                                     &hash);
+  CHECK(result);
+
+  result = utils::WriteFile(FLAGS_out_hash_file.c_str(), hash.data(),
+                            hash.size());
+  CHECK(result);
   LOG(INFO) << "Done calculating payload hash for signing.";
 }
 
+
+void CalculateMetadataHashForSigning() {
+  LOG(INFO) << "Calculating metadata hash for signing.";
+  LOG_IF(FATAL, FLAGS_in_file.empty())
+      << "Must pass --in_file to calculate metadata hash for signing.";
+  LOG_IF(FATAL, FLAGS_out_metadata_hash_file.empty())
+      << "Must pass --out_metadata_hash_file to calculate metadata hash.";
+  vector<int> sizes;
+  ParseSignatureSizes(&sizes);
+
+  vector<char> hash;
+  bool result = PayloadSigner::HashMetadataForSigning(FLAGS_in_file, &hash);
+  CHECK(result);
+
+  result = utils::WriteFile(FLAGS_out_metadata_hash_file.c_str(), hash.data(),
+                            hash.size());
+  CHECK(result);
+
+  LOG(INFO) << "Done calculating metadata hash for signing.";
+}
+
 void SignPayload() {
   LOG(INFO) << "Signing payload.";
   LOG_IF(FATAL, FLAGS_in_file.empty())
@@ -189,8 +221,19 @@
                        logging::DONT_LOCK_LOG_FILE,
                        logging::APPEND_TO_OLD_LOG_FILE,
                        logging::DISABLE_DCHECK_FOR_NON_OFFICIAL_RELEASE_BUILDS);
-  if (!FLAGS_signature_size.empty() || !FLAGS_out_hash_file.empty()) {
-    CalculatePayloadHashForSigning();
+  if (!FLAGS_signature_size.empty()) {
+    bool work_done = false;
+    if (!FLAGS_out_hash_file.empty()) {
+      CalculatePayloadHashForSigning();
+      work_done = true;
+    }
+    if (!FLAGS_out_metadata_hash_file.empty()) {
+      CalculateMetadataHashForSigning();
+      work_done = true;
+    }
+    if (!work_done) {
+      LOG(FATAL) << "Neither payload hash file nor metadata hash file supplied";
+    }
     return 0;
   }
   if (!FLAGS_signature_file.empty()) {
diff --git a/install_plan.h b/install_plan.h
index 67662b4..d277f66 100644
--- a/install_plan.h
+++ b/install_plan.h
@@ -20,29 +20,29 @@
               const std::string& url,
               uint64_t payload_size,
               const std::string& payload_hash,
-              uint64_t manifest_size,
-              const std::string& manifest_signature,
+              uint64_t metadata_size,
+              const std::string& metadata_signature,
               const std::string& install_path,
               const std::string& kernel_install_path)
       : is_resume(is_resume),
         download_url(url),
         payload_size(payload_size),
         payload_hash(payload_hash),
-        manifest_size(manifest_size),
-        manifest_signature(manifest_signature),
+        metadata_size(metadata_size),
+        metadata_signature(metadata_signature),
         install_path(install_path),
         kernel_install_path(kernel_install_path),
         kernel_size(0),
         rootfs_size(0) {}
-  InstallPlan() : is_resume(false), payload_size(0), manifest_size(0) {}
+  InstallPlan() : is_resume(false), payload_size(0), metadata_size(0) {}
 
   bool is_resume;
   std::string download_url;  // url to download from
 
   uint64_t payload_size;                 // size of the payload
   std::string payload_hash ;             // SHA256 hash of the payload
-  uint64_t manifest_size;                // size of the manifest
-  std::string manifest_signature;        // signature of the manifest
+  uint64_t metadata_size;                // size of the metadata
+  std::string metadata_signature;        // signature of the  metadata
   std::string install_path;              // path to install device
   std::string kernel_install_path;       // path to kernel install device
 
@@ -67,8 +67,8 @@
             (download_url == that.download_url) &&
             (payload_size == that.payload_size) &&
             (payload_hash == that.payload_hash) &&
-            (manifest_size == that.manifest_size) &&
-            (manifest_signature == that.manifest_signature) &&
+            (metadata_size == that.metadata_size) &&
+            (metadata_signature == that.metadata_signature) &&
             (install_path == that.install_path) &&
             (kernel_install_path == that.kernel_install_path));
   }
@@ -81,8 +81,8 @@
               << ", url: " << download_url
               << ", payload size: " << payload_size
               << ", payload hash: " << payload_hash
-              << ", manifest size: " << manifest_size
-              << ", manifest signature: " << manifest_signature
+              << ", metadata size: " << metadata_size
+              << ", metadata signature: " << metadata_signature
               << ", install_path: " << install_path
               << ", kernel_install_path: " << kernel_install_path;
   }
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index 4b68c0d..a43c501 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -494,10 +494,10 @@
   output_object.more_info_url = XmlGetProperty(updatecheck_node, "MoreInfo");
   output_object.hash = XmlGetProperty(updatecheck_node, "sha256");
   output_object.size = ParseInt(XmlGetProperty(updatecheck_node, "size"));
-  output_object.manifest_size =
-      ParseInt(XmlGetProperty(updatecheck_node, "ManifestSize"));
-  output_object.manifest_signature =
-      XmlGetProperty(updatecheck_node, "ManifestSignatureRsa");
+  output_object.metadata_size =
+      ParseInt(XmlGetProperty(updatecheck_node, "MetadataSize"));
+  output_object.metadata_signature =
+      XmlGetProperty(updatecheck_node, "MetadataSignatureRsa");
   output_object.needs_admin =
       XmlGetProperty(updatecheck_node, "needsadmin") == "true";
   output_object.prompt = XmlGetProperty(updatecheck_node, "Prompt") == "true";
diff --git a/omaha_request_action.h b/omaha_request_action.h
index e7dfb08..73641b4 100644
--- a/omaha_request_action.h
+++ b/omaha_request_action.h
@@ -34,7 +34,7 @@
       : update_exists(false),
         poll_interval(0),
         size(0),
-        manifest_size(0),
+        metadata_size(0),
         needs_admin(false),
         prompt(false) {}
   // True iff there is an update to be downloaded.
@@ -48,10 +48,10 @@
   std::string codebase;
   std::string more_info_url;
   std::string hash;
-  std::string manifest_signature;
+  std::string metadata_signature;
   std::string deadline;
   off_t size;
-  off_t manifest_size;
+  off_t metadata_size;
   bool needs_admin;
   bool prompt;
 };
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index a2c3405..e952a29 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -36,8 +36,8 @@
   install_plan_.download_url = response.codebase;
   install_plan_.payload_size = response.size;
   install_plan_.payload_hash = response.hash;
-  install_plan_.manifest_size = response.manifest_size;
-  install_plan_.manifest_signature = response.manifest_signature;
+  install_plan_.metadata_size = response.metadata_size;
+  install_plan_.metadata_signature = response.metadata_signature;
 
   install_plan_.is_resume =
       DeltaPerformer::CanResumeUpdate(prefs_, response.hash);
diff --git a/payload_signer.cc b/payload_signer.cc
index 19a9230..1df710f 100644
--- a/payload_signer.cc
+++ b/payload_signer.cc
@@ -78,25 +78,6 @@
   return true;
 }
 
-bool LoadPayload(const string& payload_path,
-                 vector<char>* out_payload,
-                 DeltaArchiveManifest* out_manifest,
-                 uint64_t* out_metadata_size) {
-  vector<char> payload;
-  // Loads the payload and parses the manifest.
-  TEST_AND_RETURN_FALSE(utils::ReadFile(payload_path, &payload));
-  LOG(INFO) << "Payload size: " << payload.size();
-  ActionExitCode error = kActionCodeSuccess;
-  InstallPlan install_plan;
-  DeltaPerformer delta_performer(NULL, &install_plan);
-  TEST_AND_RETURN_FALSE(delta_performer.ParsePayloadMetadata(
-      payload, out_manifest, out_metadata_size, &error) ==
-                        DeltaPerformer::kMetadataParseSuccess);
-  LOG(INFO) << "Metadata size: " << *out_metadata_size;
-  out_payload->swap(payload);
-  return true;
-}
-
 // 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.
@@ -110,7 +91,7 @@
   vector<char> payload;
   DeltaArchiveManifest manifest;
   uint64_t metadata_size;
-  TEST_AND_RETURN_FALSE(LoadPayload(
+  TEST_AND_RETURN_FALSE(PayloadSigner::LoadPayload(
       payload_path, &payload, &manifest, &metadata_size));
   TEST_AND_RETURN_FALSE(!manifest.has_signatures_offset() &&
                         !manifest.has_signatures_size());
@@ -139,6 +120,25 @@
 }
 }  // namespace {}
 
+bool PayloadSigner::LoadPayload(const string& payload_path,
+                 vector<char>* out_payload,
+                 DeltaArchiveManifest* out_manifest,
+                 uint64_t* out_metadata_size) {
+  vector<char> payload;
+  // Loads the payload and parses the manifest.
+  TEST_AND_RETURN_FALSE(utils::ReadFile(payload_path, &payload));
+  LOG(INFO) << "Payload size: " << payload.size();
+  ActionExitCode error = kActionCodeSuccess;
+  InstallPlan install_plan;
+  DeltaPerformer delta_performer(NULL, &install_plan);
+  TEST_AND_RETURN_FALSE(delta_performer.ParsePayloadMetadata(
+      payload, out_manifest, out_metadata_size, &error) ==
+                        DeltaPerformer::kMetadataParseSuccess);
+  LOG(INFO) << "Metadata size: " << *out_metadata_size;
+  out_payload->swap(payload);
+  return true;
+}
+
 bool PayloadSigner::SignHash(const vector<char>& hash,
                              const string& private_key_path,
                              vector<char>* out_signature) {
@@ -328,7 +328,7 @@
   return true;
 }
 
-bool PayloadSigner::HashPayloadForSigning(const std::string& payload_path,
+bool PayloadSigner::HashPayloadForSigning(const string& payload_path,
                                           const vector<int>& signature_sizes,
                                           vector<char>* out_hash_data) {
   // TODO(petkov): Reduce memory usage -- the payload is manipulated in memory.
@@ -354,6 +354,22 @@
   return true;
 }
 
+bool PayloadSigner::HashMetadataForSigning(const string& payload_path,
+                                           vector<char>* out_metadata_hash) {
+  // Extract the manifest first.
+  vector<char> payload;
+  DeltaArchiveManifest manifest_proto;
+  uint64_t metadata_size;
+  TEST_AND_RETURN_FALSE(LoadPayload(
+      payload_path, &payload, &manifest_proto, &metadata_size));
+
+  // Calculates the hash on the manifest.
+  TEST_AND_RETURN_FALSE(OmahaHashCalculator::RawHashOfBytes(&payload[0],
+                                                            metadata_size,
+                                                            out_metadata_hash));
+  return true;
+}
+
 bool PayloadSigner::AddSignatureToPayload(
     const string& payload_path,
     const vector<vector<char> >& signatures,
@@ -388,19 +404,19 @@
   return true;
 }
 
-bool PayloadSigner::GetManifestSignature(const char* manifest,
-                                         size_t manifest_size,
+bool PayloadSigner::GetMetadataSignature(const char* const metadata,
+                                         size_t metadata_size,
                                          const string& private_key_path,
                                          string* out_signature) {
   // 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.
-  vector<char> manifest_hash;
-  TEST_AND_RETURN_FALSE(OmahaHashCalculator::RawHashOfBytes(manifest,
-                                                            manifest_size,
-                                                            &manifest_hash));
+  vector<char> metadata_hash;
+  TEST_AND_RETURN_FALSE(OmahaHashCalculator::RawHashOfBytes(metadata,
+                                                            metadata_size,
+                                                            &metadata_hash));
 
   vector<char> signature;
-  TEST_AND_RETURN_FALSE(SignHash(manifest_hash,
+  TEST_AND_RETURN_FALSE(SignHash(metadata_hash,
                                  private_key_path,
                                  &signature));
 
diff --git a/payload_signer.h b/payload_signer.h
index 1c4a3eb..08135d2 100644
--- a/payload_signer.h
+++ b/payload_signer.h
@@ -9,6 +9,7 @@
 #include <vector>
 
 #include <base/basictypes.h>
+#include "update_engine/update_metadata.pb.h"
 
 // This class encapsulates methods used for payload signing and signature
 // verification. See update_metadata.proto for more info.
@@ -41,13 +42,35 @@
       const std::vector<std::string>& private_key_paths,
       uint64_t* out_length);
 
-  // Given an unsigned payload in |payload_path| (with no dummy signature op)
-  // and the raw |signature_sizes| calculates the raw hash that needs to be
-  // signed in |out_hash_data|. Returns true on success, false otherwise.
+  // Given an unsigned payload in |payload_path| (with no dummy signature op),
+  // this method does two things:
+  // 1. It calculates the raw SHA256 hash of the entire payload in
+  // |payload_path| and returns the result in |out_hash_data|.
+  // 2. But before calculating the hash, it also inserts a dummy signature
+  // operation at the end of the manifest. This signature operation points to a
+  // blob offset and length that'll be later filled in by paygen with the
+  // actual signature from signer. But since the hash includes the signature
+  // operation, the signature operation has to be inserted before we compute
+  // the hash. Note, the hash doesn't cover the signature itself - it covers
+  // only the signature operation in manifest. Since paygen may add multiple
+  // signatures of different sizes (1024, 2048, etc.) it specifies a list
+  // of those |signature_sizes| so that the signature blob length is
+  // calculated to be of the correct size before the hash is computed.
+  // Returns true on success, false otherwise.
   static bool HashPayloadForSigning(const std::string& payload_path,
                                     const std::vector<int>& signature_sizes,
                                     std::vector<char>* out_hash_data);
 
+  // Given an unsigned payload in |payload_path|, calculates the raw hash of
+  // just the metadata (not the entire payload) that needs to be signed in
+  // |out_hash_data|. Note: This method should be called only after calling the
+  // HashPayloadForSigning method so that the metadata hash is computed on the
+  // final metadata that will be in the payload (because HashPayloadForSigning
+  // adds a dummy signature operation to the manifest).
+  // Returns true on success, false otherwise.
+  static bool HashMetadataForSigning(const std::string& payload_path,
+                                     std::vector<char>* out_metadata_hash);
+
   // Given an unsigned payload in |payload_path| (with no dummy signature op)
   // and the raw |signatures| updates the payload to include the signature thus
   // turning it into a signed payload. The new payload is stored in
@@ -95,11 +118,24 @@
   // 2048 bits. Returns true on success, false otherwise.
   static bool PadRSA2048SHA256Hash(std::vector<char>* hash);
 
-  static bool GetManifestSignature(const char* manifest,
-                                   size_t manifest_size,
+  // Computes the SHA256 hash of the first metadata_size bytes of |metadata|
+  // and signs the hash with the given private_key_path and writes the signed
+  // hash in |out_signature|. Returns true if successful or false if there was
+  // any error in the computations.
+  static bool GetMetadataSignature(const char* const metadata,
+                                   size_t metadata_size,
                                    const std::string& private_key_path,
                                    std::string* out_signature);
 
+  // Reads the payload from the given |payload_path| into the |out_payload|
+  // vector. It also parses the manifest protobuf in the payload and returns it
+  // in |out_manifest| along with the size of the entire metadata in
+  // |out_metadata_size|.
+  static bool LoadPayload(const std::string& payload_path,
+                          std::vector<char>* out_payload,
+                          DeltaArchiveManifest* out_manifest,
+                          uint64_t* out_metadata_size);
+
  private:
   // This should never be constructed
   DISALLOW_IMPLICIT_CONSTRUCTORS(PayloadSigner);