Delta generator can create payload and metadata hashes in one run.

The delta generator can now correctly generate both payload and metadata
hashes in a single pass for both signed and unsigned payloads.

This will work for the current paygen flow (which was required):
  1) generate payload.
  2) generate payload hash.
  3) sign payload hash.
  4) insert payload signature.
  5) generate metadata hash.

And the improved flow:
  1) generate payload.
  2) generate payload and metadata hash.
  3) sign both hashes.
  4) insert payload signature.

BUG=chromium:224453
TEST=Unittests + Manual paygen_payload tests.

Change-Id: Icafccc38ca782ab15ac7dd2c076c043850aa630d
Reviewed-on: https://chromium-review.googlesource.com/173939
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Commit-Queue: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
diff --git a/generate_delta_main.cc b/generate_delta_main.cc
index ede8ab9..b239450 100644
--- a/generate_delta_main.cc
+++ b/generate_delta_main.cc
@@ -119,18 +119,24 @@
   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) {
+void ParseSignatureSizes(const string& signature_sizes_flag,
+                         vector<int>* signature_sizes) {
+  signature_sizes->clear();
+  vector<string> split_strings;
+
+  base::SplitString(signature_sizes_flag, ':', &split_strings);
+  for (vector<string>::iterator i = split_strings.begin();
+       i < split_strings.end();
+       i++) {
     int size = 0;
-    bool parsing_successful = base::StringToInt(*it, &size);
+    bool parsing_successful = base::StringToInt(*i, &size);
     LOG_IF(FATAL, !parsing_successful)
-        << "Invalid signature size: " << *it;
-    sizes->push_back(size);
+        << "Invalid signature size: " << *i;
+
+    LOG_IF(FATAL, size != (2048 / 8)) <<
+        "Only signature sizes of 256 bytes are supported.";
+
+    signature_sizes->push_back(size);
   }
 }
 
@@ -168,42 +174,39 @@
   return true;
 }
 
-
-void CalculatePayloadHashForSigning() {
+void CalculatePayloadHashForSigning(const vector<int> &sizes,
+                                    const string& out_hash_file) {
   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())
+  LOG_IF(FATAL, out_hash_file.empty())
       << "Must pass --out_hash_file to calculate hash for signing.";
-  vector<int> sizes;
-  ParseSignatureSizes(&sizes);
 
   vector<char> hash;
   bool result = PayloadSigner::HashPayloadForSigning(FLAGS_in_file, sizes,
                                                      &hash);
   CHECK(result);
 
-  result = utils::WriteFile(FLAGS_out_hash_file.c_str(), hash.data(),
-                            hash.size());
+  result = utils::WriteFile(out_hash_file.c_str(), hash.data(), hash.size());
   CHECK(result);
   LOG(INFO) << "Done calculating payload hash for signing.";
 }
 
 
-void CalculateMetadataHashForSigning() {
+void CalculateMetadataHashForSigning(const vector<int> &sizes,
+                                     const string& out_metadata_hash_file) {
   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())
+  LOG_IF(FATAL, 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);
+  bool result = PayloadSigner::HashMetadataForSigning(FLAGS_in_file, sizes,
+                                                      &hash);
   CHECK(result);
 
-  result = utils::WriteFile(FLAGS_out_metadata_hash_file.c_str(), hash.data(),
+  result = utils::WriteFile(out_metadata_hash_file.c_str(), hash.data(),
                             hash.size());
   CHECK(result);
 
@@ -296,18 +299,17 @@
                        logging::DONT_LOCK_LOG_FILE,
                        logging::APPEND_TO_OLD_LOG_FILE,
                        logging::DISABLE_DCHECK_FOR_NON_OFFICIAL_RELEASE_BUILDS);
-  if (!FLAGS_signature_size.empty()) {
-    bool work_done = false;
+
+  vector<int> signature_sizes;
+  ParseSignatureSizes(FLAGS_signature_size, &signature_sizes);
+
+  if (!FLAGS_out_hash_file.empty() || !FLAGS_out_metadata_hash_file.empty()) {
     if (!FLAGS_out_hash_file.empty()) {
-      CalculatePayloadHashForSigning();
-      work_done = true;
+      CalculatePayloadHashForSigning(signature_sizes, FLAGS_out_hash_file);
     }
     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";
+      CalculateMetadataHashForSigning(signature_sizes,
+                                      FLAGS_out_metadata_hash_file);
     }
     return 0;
   }
diff --git a/payload_signer.cc b/payload_signer.cc
index 345a241..3ed2a49 100644
--- a/payload_signer.cc
+++ b/payload_signer.cc
@@ -387,9 +387,12 @@
   return true;
 }
 
-bool PayloadSigner::HashPayloadForSigning(const string& payload_path,
-                                          const vector<int>& signature_sizes,
-                                          vector<char>* out_hash_data) {
+bool PayloadSigner::PrepPayloadForHashing(
+        const string& payload_path,
+        const vector<int>& signature_sizes,
+        vector<char>* payload_out,
+        uint64_t* metadata_size_out,
+        uint64_t* signatures_offset_out) {
   // TODO(petkov): Reduce memory usage -- the payload is manipulated in memory.
 
   // Loads the payload and adds the signature op to it.
@@ -402,15 +405,27 @@
   vector<char> signature_blob;
   TEST_AND_RETURN_FALSE(ConvertSignatureToProtobufBlob(signatures,
                                                        &signature_blob));
-  vector<char> payload;
-  uint64_t final_metadata_size;
-  uint64_t signatures_offset;
   TEST_AND_RETURN_FALSE(AddSignatureOpToPayload(payload_path,
                                                 signature_blob.size(),
-                                                &payload,
-                                                &final_metadata_size,
-                                                &signatures_offset));
+                                                payload_out,
+                                                metadata_size_out,
+                                                signatures_offset_out));
 
+  return true;
+}
+
+bool PayloadSigner::HashPayloadForSigning(const string& payload_path,
+                                          const vector<int>& signature_sizes,
+                                          vector<char>* out_hash_data) {
+  vector<char> payload;
+  uint64_t metadata_size;
+  uint64_t signatures_offset;
+
+  TEST_AND_RETURN_FALSE(PrepPayloadForHashing(payload_path,
+                                              signature_sizes,
+                                              &payload,
+                                              &metadata_size,
+                                              &signatures_offset));
 
   // Calculates the hash on the updated payload. Note that we stop calculating
   // before we reach the signature information.
@@ -421,13 +436,17 @@
 }
 
 bool PayloadSigner::HashMetadataForSigning(const string& payload_path,
+                                           const vector<int>& signature_sizes,
                                            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));
+  uint64_t signatures_offset;
+
+  TEST_AND_RETURN_FALSE(PrepPayloadForHashing(payload_path,
+                                              signature_sizes,
+                                              &payload,
+                                              &metadata_size,
+                                              &signatures_offset));
 
   // Calculates the hash on the manifest.
   TEST_AND_RETURN_FALSE(OmahaHashCalculator::RawHashOfBytes(&payload[0],
diff --git a/payload_signer.h b/payload_signer.h
index 6e7965c..95e6cc5 100644
--- a/payload_signer.h
+++ b/payload_signer.h
@@ -42,33 +42,41 @@
       const std::vector<std::string>& private_key_paths,
       uint64_t* out_length);
 
-  // Given an unsigned payload in |payload_path| (with no dummy signature op),
+  // This is a helper method for HashPayloadforSigning and
+  // HashMetadataForSigning. It loads the payload into memory, and inserts
+  // signature placeholders if Signatures aren't already present.
+  static bool PrepPayloadForHashing(
+        const std::string& payload_path,
+        const std::vector<int>& signature_sizes,
+        std::vector<char>* payload_out,
+        uint64_t* metadata_size_out,
+        uint64_t* signatures_offset_out);
+
+  // Given an unsigned payload in |payload_path|,
   // 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.
+  // 1. Uses PrepPayloadForHashing to inserts placeholder signature operations
+  //    to make the manifest match what the final signed payload will look
+  //    like based on |signatures_sizes|, if needed.
+  // 2. It calculates the raw SHA256 hash of the payload in |payload_path|
+  //    (except signatures) and returns the result in |out_hash_data|.
+  //
+  // The dummy signatures are not preserved or written to disk.
   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.
+  // Given an unsigned payload in |payload_path|,
+  // this method does two things:
+  // 1. Uses PrepPayloadForHashing to inserts placeholder signature operations
+  //    to make the manifest match what the final signed payload will look
+  //    like based on |signatures_sizes|, if needed.
+  // 2. It calculates the raw SHA256 hash of the metadata from the payload in
+  //    |payload_path| (except signatures) and returns the result in
+  //    |out_metadata_hash|.
+  //
+  // The dummy signatures are not preserved or written to disk.
   static bool HashMetadataForSigning(const std::string& payload_path,
+                                     const std::vector<int>& signature_sizes,
                                      std::vector<char>* out_metadata_hash);
 
   // Given an unsigned payload in |payload_path| (with no dummy signature op)