paycheck: enforce physical partition size correctly
During payload checking, payload has wrongly interpreted the size
reported in the update payload to be the physical partition size,
whereas this is in fact the size of the filesystem portion only (a
misnomer). This sometimes caused it to emit errors on out-of-bounds
operations, which are otherwise harmless in real-world scenarios.
This CL makes a clear distinction between the two, with the following
semantics:
- The payload's embedded filesystem size must by <= the physical
partition sizes.
- Reading/writing from/to the new partition must be within the physical
partition size boundaries, and not the filesystem ones.
- Reading from the old partition is only allowed from filesystem
boundaries; this is unchanged from current behavior and appears to be
consistent with how we perform delta updates.
- Old/new SHA256 verification during payload application is now limited
to the allotted filesystem portion only (and not the full partition
size). This is consistent with the update engine's semantics.
- Other than that, this change currently has no further effect on
payload application, which remains more permissive wrt to partition
sizes. This also means that the sizes of partitions resulting from
a payload application will not necessarily abide by the predetermined
physical partition sizes. This is in line with the prevailing
division of responsibilities between payload checking (strict) and
application (relaxed).
BUG=chromium:221847
TEST=Payload checking respects partition size override
TEST=Unit tests pass
TEST=Integration tests pass
Change-Id: I0dbc88d538c0cc53b7551f4dfa8f543bcf480cd5
Reviewed-on: https://gerrit.chromium.org/gerrit/50103
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>
Commit-Queue: David James <davidjames@chromium.org>
diff --git a/scripts/update_payload/applier.py b/scripts/update_payload/applier.py
index 6780e9a..3b7b1a6 100644
--- a/scripts/update_payload/applier.py
+++ b/scripts/update_payload/applier.py
@@ -28,25 +28,25 @@
#
# Helper functions.
#
-def _VerifySha256(file_obj, expected_hash, name, max_length=-1):
+def _VerifySha256(file_obj, expected_hash, name, length=-1):
"""Verifies the SHA256 hash of a file.
Args:
file_obj: file object to read
expected_hash: the hash digest we expect to be getting
name: name string of this hash, for error reporting
- max_length: maximum length of data to read/hash (optional)
+ length: precise length of data to verify (optional)
Raises:
- PayloadError if file hash fails to verify.
+ PayloadError if computed hash doesn't match expected one, or if fails to
+ read the specified length of data.
"""
# pylint: disable=E1101
hasher = hashlib.sha256()
block_length = 1024 * 1024
- if max_length < 0:
- max_length = sys.maxint
+ max_length = length if length >= 0 else sys.maxint
- while max_length != 0:
+ while max_length > 0:
read_length = min(max_length, block_length)
data = file_obj.read(read_length)
if not data:
@@ -54,6 +54,11 @@
max_length -= len(data)
hasher.update(data)
+ if length >= 0 and max_length > 0:
+ raise PayloadError(
+ 'insufficient data (%d instead of %d) when verifying %s' %
+ (length - max_length, length, name))
+
actual_hash = hasher.digest()
if actual_hash != expected_hash:
raise PayloadError('%s hash (%s) not as expected (%s)' %
@@ -319,7 +324,8 @@
if src_part_file_name:
# Verify the source partition.
with open(src_part_file_name, 'rb') as src_part_file:
- _VerifySha256(src_part_file, src_part_info.hash, part_name)
+ _VerifySha256(src_part_file, src_part_info.hash, part_name,
+ length=src_part_info.size)
# Copy the src partition to the dst one.
shutil.copyfile(src_part_file_name, dst_part_file_name)
@@ -335,7 +341,8 @@
# Verify the resulting partition.
with open(dst_part_file_name, 'rb') as dst_part_file:
- _VerifySha256(dst_part_file, dst_part_info.hash, part_name)
+ _VerifySha256(dst_part_file, dst_part_info.hash, part_name,
+ length=dst_part_info.size)
def Run(self, dst_kernel_part, dst_rootfs_part, src_kernel_part=None,
src_rootfs_part=None):