Improve payload magic header handling

Currently, we use central directory's extra fields and filenames to
determine starting position of a zipentry's file data. However, central
directory's extra field might differ from extra field in local file
header. For example, the Extended-Timestamp field has different formats
depending on whether it's in local file header or central directory. We
should use local file header for computing offsets.

CRs-Fixed: 2804037

Test: Serve an OTA by update_device.py
Change-Id: I00d150d874b9c874bb713569ea14938e036f854e
(cherry picked from commit aba70abe81618542044dc20907f281a56b8b500e)

2 files changed, 44 insertions, 5 deletions

diff --git a/payload_consumer/payload_metadata.cc b/payload_consumer/payload_metadata.cc
index 09526460..19052b99 100644
--- a/payload_consumer/payload_metadata.cc
+++ b/payload_consumer/payload_metadata.cc
@@ -18,6 +18,7 @@
 
 #include <endian.h>
 
+#include <base/strings/stringprintf.h>
 #include <brillo/data_encoding.h>
 
 #include "update_engine/common/constants.h"
@@ -71,7 +72,18 @@ MetadataParseResult PayloadMetadata::ParsePayloadHeader(
 
   // Validate the magic string.
   if (memcmp(payload.data(), kDeltaMagic, sizeof(kDeltaMagic)) != 0) {
-    LOG(ERROR) << "Bad payload format -- invalid delta magic.";
+    LOG(ERROR) << "Bad payload format -- invalid delta magic: "
+               << base::StringPrintf("%02x%02x%02x%02x",
+                                     payload[0],
+                                     payload[1],
+                                     payload[2],
+                                     payload[3])
+               << " Expected: "
+               << base::StringPrintf("%02x%02x%02x%02x",
+                                     kDeltaMagic[0],
+                                     kDeltaMagic[1],
+                                     kDeltaMagic[2],
+                                     kDeltaMagic[3]);
     *error = ErrorCode::kDownloadInvalidMetadataMagicString;
     return MetadataParseResult::kError;
   }
diff --git a/scripts/update_device.py b/scripts/update_device.py
index 49f766da..371a89a8 100755
--- a/scripts/update_device.py
+++ b/scripts/update_device.py
@@ -19,12 +19,14 @@
 
 import argparse
 import BaseHTTPServer
+import binascii
 import hashlib
 import logging
 import os
 import socket
 import subprocess
 import sys
+import struct
 import threading
 import xml.etree.ElementTree
 import zipfile
@@ -85,6 +87,7 @@ class AndroidOTAPackage(object):
   OTA_PAYLOAD_PROPERTIES_TXT = 'payload_properties.txt'
   SECONDARY_OTA_PAYLOAD_BIN = 'secondary/payload.bin'
   SECONDARY_OTA_PAYLOAD_PROPERTIES_TXT = 'secondary/payload_properties.txt'
+  PAYLOAD_MAGIC_HEADER = b'CrAU'
 
   def __init__(self, otafilename, secondary_payload=False):
     self.otafilename = otafilename
@@ -93,10 +96,34 @@ class AndroidOTAPackage(object):
     payload_entry = (self.SECONDARY_OTA_PAYLOAD_BIN if secondary_payload else
                      self.OTA_PAYLOAD_BIN)
     payload_info = otazip.getinfo(payload_entry)
-    self.offset = payload_info.header_offset
-    self.offset += zipfile.sizeFileHeader
-    self.offset += len(payload_info.extra) + len(payload_info.filename)
-    self.size = payload_info.file_size
+
+    if payload_info.compress_type != 0:
+      logging.error(
+          "Expected layload to be uncompressed, got compression method %d",
+          payload_info.compress_type)
+    # Don't use len(payload_info.extra). Because that returns size of extra
+    # fields in central directory. We need to look at local file directory,
+    # as these two might have different sizes.
+    with open(otafilename, "rb") as fp:
+      fp.seek(payload_info.header_offset)
+      data = fp.read(zipfile.sizeFileHeader)
+      fheader = struct.unpack(zipfile.structFileHeader, data)
+      # Last two fields of local file header are filename length and
+      # extra length
+      filename_len = fheader[-2]
+      extra_len = fheader[-1]
+      self.offset = payload_info.header_offset
+      self.offset += zipfile.sizeFileHeader
+      self.offset += filename_len + extra_len
+      self.size = payload_info.file_size
+      fp.seek(self.offset)
+      payload_header = fp.read(4)
+      if payload_header != self.PAYLOAD_MAGIC_HEADER:
+        logging.warning(
+            "Invalid header, expeted %s, got %s."
+            "Either the offset is not correct, or payload is corrupted",
+            binascii.hexlify(self.PAYLOAD_MAGIC_HEADER),
+            payload_header)
 
     property_entry = (self.SECONDARY_OTA_PAYLOAD_PROPERTIES_TXT if
                       secondary_payload else self.OTA_PAYLOAD_PROPERTIES_TXT)