update_payload: Add checks for new operations and minor version.
Paycheck now checks to make sure delta payloads with minor version 1 do
not have SOURCE_COPY or SOURCE_BSDIFF operations and that payloads with
minor version 2 do not have MOVE or BSDIFF operations. It also checks that
the minor version given matches the payload type (delta or full).
BUG=chromium:459799
TEST=`./checker_unittest.py` and running paycheck on payloads.
Change-Id: I2a61e44760ae2b672015acdf8683501327b5d197
Reviewed-on: https://chromium-review.googlesource.com/253050
Reviewed-by: Alex Deymo <deymo@chromium.org>
Commit-Queue: Allie Wood <alliewood@chromium.org>
Trybot-Ready: Allie Wood <alliewood@chromium.org>
Tested-by: Allie Wood <alliewood@chromium.org>
diff --git a/scripts/update_payload/checker.py b/scripts/update_payload/checker.py
index d7d86c0..0bc62ad 100644
--- a/scripts/update_payload/checker.py
+++ b/scripts/update_payload/checker.py
@@ -593,6 +593,10 @@
'New rootfs content (%d) exceed partition size (%d).' %
(self.new_rootfs_fs_size, rootfs_part_size))
+ # Check: minor_version makes sense for the payload type. This check should
+ # run after the payload type has been set.
+ self._CheckMinorVersion(report, manifest.minor_version, self.payload_type)
+
def _CheckLength(self, length, total_blocks, op_name, length_name):
"""Checks whether a length matches the space designated in extents.
@@ -771,7 +775,7 @@
raise error.PayloadError('%s: excess dst blocks.' % op_name)
def _CheckBsdiffOperation(self, data_length, total_dst_blocks, op_name):
- """Specific checks for BSDIFF operations.
+ """Specific checks for BSDIFF and SOURCE_BSDIFF operations.
Args:
data_length: The length of the data blob associated with the operation.
@@ -793,6 +797,30 @@
(op_name, data_length, total_dst_blocks, self.block_size,
total_dst_blocks * self.block_size))
+ def _CheckSourceCopyOperation(self, data_offset, total_src_blocks,
+ total_dst_blocks, op_name):
+ """Specific checks for SOURCE_COPY.
+
+ Args:
+ op: The operation object from the manifest.
+ data_offset: The offset of a data blob for the operation.
+ total_src_blocks: Total number of blocks in src_extents.
+ total_dst_blocks: Total number of blocks in dst_extents.
+ op_name: Operation name for error reporting.
+
+ Raises:
+ error.PayloadError if any check fails.
+ """
+ # Check: No data_{offset,length}.
+ if data_offset is not None:
+ raise error.PayloadError('%s: contains data_{offset,length}.' % op_name)
+
+ # Check: total_src_blocks == total_dst_blocks.
+ if total_src_blocks != total_dst_blocks:
+ raise error.PayloadError(
+ '%s: total src blocks (%d) != total dst blocks (%d).' %
+ (op_name, total_src_blocks, total_dst_blocks))
+
def _CheckOperation(self, op, op_name, is_last, old_block_counters,
new_block_counters, old_usable_size, new_usable_size,
prev_data_offset, allow_signature, blob_hash_counts):
@@ -888,8 +916,11 @@
elif op.type == common.OpType.MOVE:
self._CheckMoveOperation(op, data_offset, total_src_blocks,
total_dst_blocks, op_name)
- elif op.type == common.OpType.BSDIFF:
+ elif op.type in (common.OpType.BSDIFF, common.OpType.SOURCE_BSDIFF):
self._CheckBsdiffOperation(data_length, total_dst_blocks, op_name)
+ elif op.type == common.OpType.SOURCE_COPY:
+ self._CheckSourceCopyOperation(data_offset, total_src_blocks,
+ total_dst_blocks, op_name)
else:
assert False, 'cannot get here'
@@ -944,6 +975,8 @@
common.OpType.REPLACE_BZ: 0,
common.OpType.MOVE: 0,
common.OpType.BSDIFF: 0,
+ common.OpType.SOURCE_COPY: 0,
+ common.OpType.SOURCE_BSDIFF: 0,
}
# Total blob sizes for each operation type.
op_blob_totals = {
@@ -951,6 +984,8 @@
common.OpType.REPLACE_BZ: 0,
# MOVE operations don't have blobs.
common.OpType.BSDIFF: 0,
+ # SOURCE_COPY operations don't have blobs.
+ common.OpType.SOURCE_BSDIFF: 0,
}
# Counts of hashed vs unhashed operations.
blob_hash_counts = {
@@ -1021,6 +1056,19 @@
'%s: not all blocks written exactly once during full update.' %
base_name)
+ # Check: SOURCE_COPY and SOURCE_BSDIFF ops shouldn't be in minor version 1.
+ if (self.payload.manifest.minor_version == 1 and
+ (op_counts[common.OpType.SOURCE_COPY] or
+ op_counts[common.OpType.SOURCE_BSDIFF])):
+ raise error.PayloadError(
+ 'SOURCE_COPY/SOURCE_BSDIFF not allowed with minor version 1.')
+
+ # Check: MOVE and BSDIFF ops shouldn't be in minor version 2.
+ if (self.payload.manifest.minor_version == 2 and
+ (op_counts[common.OpType.MOVE] or op_counts[common.OpType.BSDIFF])):
+ raise error.PayloadError(
+ 'MOVE/BSDIFF not allowed with minor version 2.')
+
return total_data_used
def _CheckSignatures(self, report, pubkey_file_name):
@@ -1074,6 +1122,36 @@
raise error.PayloadError('Unknown signature version (%d).' %
sig.version)
+ def _CheckMinorVersion(self, report, minor_version, payload_type):
+ """Checks that the minor version matches the payload type.
+
+ Args:
+ report: The report object to add to.
+ minor_version: The minor version of the payload.
+ payload_type: The type of payload (full or delta).
+
+ Raises:
+ error.PayloadError if any of the checks fails.
+ """
+ report.AddField('minor version', minor_version)
+
+ # Minor version 0 implies a full payload.
+ if minor_version == 0:
+ if payload_type != _TYPE_FULL:
+ raise error.PayloadError(
+ 'Minor version 0 not compatible with payload type: %s.'
+ % payload_type)
+
+ # Minor version 1 or 2 implies a delta payload.
+ elif minor_version == 1 or minor_version == 2:
+ if payload_type != _TYPE_DELTA:
+ raise error.PayloadError(
+ 'Minor version %d not compatible with payload type: %s.'
+ % (minor_version, payload_type))
+
+ else:
+ raise error.PayloadError('Unsupported minor version: %d' % minor_version)
+
def Run(self, pubkey_file_name=None, metadata_sig_file=None,
rootfs_part_size=0, kernel_part_size=0, report_out_file=None):
"""Checker entry point, invoking all checks.
diff --git a/scripts/update_payload/checker_unittest.py b/scripts/update_payload/checker_unittest.py
index 60ae1a2..442fcaa 100755
--- a/scripts/update_payload/checker_unittest.py
+++ b/scripts/update_payload/checker_unittest.py
@@ -33,6 +33,8 @@
'REPLACE_BZ': common.OpType.REPLACE_BZ,
'MOVE': common.OpType.MOVE,
'BSDIFF': common.OpType.BSDIFF,
+ 'SOURCE_COPY': common.OpType.SOURCE_COPY,
+ 'SOURCE_BSDIFF': common.OpType.SOURCE_BSDIFF,
}
return op_name_to_type[op_name]
@@ -83,7 +85,7 @@
"""
def MockPayload(self):
- """Create a mock payload object, complete with a mock menifest."""
+ """Create a mock payload object, complete with a mock manifest."""
payload = self.mox.CreateMock(update_payload.Payload)
payload.is_init = True
payload.manifest = self.mox.CreateMock(
@@ -755,6 +757,26 @@
payload_checker._CheckBsdiffOperation,
10000, 2, 'foo')
+ def testCheckSourceCopyOperation_Pass(self):
+ """Tests _CheckSourceCopyOperation(); pass case."""
+ payload_checker = checker.PayloadChecker(self.MockPayload())
+ self.assertIsNone(
+ payload_checker._CheckSourceCopyOperation(None, 134, 134, 'foo'))
+
+ def testCheckSourceCopyOperation_FailContainsData(self):
+ """Tests _CheckSourceCopyOperation(); message contains data."""
+ payload_checker = checker.PayloadChecker(self.MockPayload())
+ self.assertRaises(update_payload.PayloadError,
+ payload_checker._CheckSourceCopyOperation,
+ 134, 0, 0, 'foo')
+
+ def testCheckSourceCopyOperation_FailBlockCountsMismatch(self):
+ """Tests _CheckSourceCopyOperation(); src and dst block totals not equal."""
+ payload_checker = checker.PayloadChecker(self.MockPayload())
+ self.assertRaises(update_payload.PayloadError,
+ payload_checker._CheckSourceCopyOperation,
+ None, 0, 1, 'foo')
+
def DoCheckOperationTest(self, op_type_name, is_last, allow_signature,
allow_unhashed, fail_src_extents, fail_dst_extents,
fail_mismatched_data_offset_length,
@@ -764,7 +786,8 @@
"""Parametric testing of _CheckOperation().
Args:
- op_type_name: 'REPLACE', 'REPLACE_BZ', 'MOVE' or 'BSDIFF'.
+ op_type_name: 'REPLACE', 'REPLACE_BZ', 'MOVE', 'BSDIFF', 'SOURCE_COPY',
+ or 'SOURCE_BSDIFF'.
is_last: Whether we're testing the last operation in a sequence.
allow_signature: Whether we're testing a signature-capable operation.
allow_unhashed: Whether we're allowing to not hash the data.
@@ -801,7 +824,8 @@
op.type = op_type
total_src_blocks = 0
- if op_type in (common.OpType.MOVE, common.OpType.BSDIFF):
+ if op_type in (common.OpType.MOVE, common.OpType.BSDIFF,
+ common.OpType.SOURCE_COPY, common.OpType.SOURCE_BSDIFF):
if fail_src_extents:
self.AddToMessage(op.src_extents,
self.NewExtentList((0, 0)))
@@ -810,7 +834,7 @@
self.NewExtentList((0, 16)))
total_src_blocks = 16
- if op_type != common.OpType.MOVE:
+ if op_type not in (common.OpType.MOVE, common.OpType.SOURCE_COPY):
if not fail_mismatched_data_offset_length:
op.data_length = 16 * block_size - 8
if fail_prev_data_offset:
@@ -935,6 +959,54 @@
self.assertEqual(rootfs_data_length,
payload_checker._CheckOperations(*args))
+ def testCheckOperationsMinorVersion(self):
+ """Tests _CheckOperations; checks op compatibility with minor version.
+
+ SOURCE_COPY and SOURCE_BSDIFF operations are not supported in payloads
+ with delta minor version 1. MOVE and BSDIFF operations are not supported
+ in payloads with delta minor version 2.
+ """
+ # Create test objects and parameters.
+ payload_checker = checker.PayloadChecker(self.MockPayload())
+ report = checker._PayloadReport()
+ rootfs_part_size = test_utils.MiB(8)
+
+ FakeOp = collections.namedtuple('FakeOp', ['type'])
+ args = ([FakeOp(common.OpType.SOURCE_COPY),
+ FakeOp(common.OpType.SOURCE_BSDIFF)],
+ report, 'foo', 0, rootfs_part_size, rootfs_part_size, 0, False)
+
+ try:
+ # Mock _CheckOperation() so it will pass.
+ self.mox.StubOutWithMock(payload_checker, '_CheckOperation')
+ payload_checker._CheckOperation(mox.IgnoreArg(), mox.IgnoreArg(),
+ mox.IgnoreArg(), mox.IgnoreArg(),
+ mox.IgnoreArg(), mox.IgnoreArg(),
+ mox.IgnoreArg(), mox.IgnoreArg(),
+ mox.IgnoreArg(), mox.IgnoreArg()
+ ).MultipleTimes().AndReturn(0)
+ self.mox.ReplayAll()
+
+ # Fail, minor version 1 should not allow SOURCE_COPY or SOURCE_BSDIFF.
+ payload_checker.payload.manifest.minor_version = 1
+ self.assertRaises(update_payload.PayloadError,
+ payload_checker._CheckOperations,
+ *args)
+
+ # Pass, minor version 2 should allow these operations.
+ payload_checker.payload.manifest.minor_version = 2
+ self.assertEquals(payload_checker._CheckOperations(*args), 0)
+
+ # Fail, minor version 2 should not allow MOVE or BSDIFF.
+ args = ([FakeOp(common.OpType.MOVE), FakeOp(common.OpType.BSDIFF)],
+ report, 'foo', 0, rootfs_part_size, rootfs_part_size, 0, False)
+ self.assertRaises(update_payload.PayloadError,
+ payload_checker._CheckOperations,
+ *args)
+
+ finally:
+ self.mox.UnsetStubs()
+
def DoCheckSignaturesTest(self, fail_empty_sigs_blob, fail_missing_pseudo_op,
fail_mismatched_pseudo_op, fail_sig_missing_fields,
fail_unknown_sig_version, fail_incorrect_sig):
@@ -1006,6 +1078,29 @@
else:
self.assertIsNone(payload_checker._CheckSignatures(*args))
+ def DoCheckMinorVersionTest(self, minor_version, payload_type):
+ """Parametric testing for CheckMinorVersion().
+
+ Args:
+ minor_version: The payload minor version to test with.
+ payload_type: The type of the payload we're testing, delta or full.
+ """
+ # Create the test object.
+ payload_checker = checker.PayloadChecker(self.MockPayload())
+ report = checker._PayloadReport()
+
+ should_succeed = (
+ (minor_version == 0 and payload_type == checker._TYPE_FULL) or
+ (minor_version == 1 and payload_type == checker._TYPE_DELTA) or
+ (minor_version == 2 and payload_type == checker._TYPE_DELTA))
+ args = (report, minor_version, payload_type)
+
+ if should_succeed:
+ self.assertIsNone(payload_checker._CheckMinorVersion(*args))
+ else:
+ self.assertRaises(update_payload.PayloadError,
+ payload_checker._CheckMinorVersion, *args)
+
def DoRunTest(self, fail_wrong_payload_type, fail_invalid_block_size,
fail_mismatched_block_size, fail_excess_data):
# Generate a test payload. For this test, we generate a full update that
@@ -1064,7 +1159,6 @@
else:
self.assertIsNone(payload_checker.Run(**kwargs))
-
# This implements a generic API, hence the occasional unused args.
# pylint: disable=W0613
def ValidateCheckOperationTest(op_type_name, is_last, allow_signature,
@@ -1082,8 +1176,8 @@
fail_src_extents or fail_src_length)):
return False
- # MOVE operations don't carry data.
- if (op_type == common.OpType.MOVE and (
+ # MOVE and SOURCE_COPY operations don't carry data.
+ if (op_type in (common.OpType.MOVE, common.OpType.SOURCE_COPY) and (
fail_mismatched_data_offset_length or fail_data_hash or
fail_prev_data_offset)):
return False
@@ -1166,7 +1260,8 @@
# Add all _CheckOperation() test cases.
AddParametricTests('CheckOperation',
{'op_type_name': ('REPLACE', 'REPLACE_BZ', 'MOVE',
- 'BSDIFF'),
+ 'BSDIFF', 'SOURCE_COPY',
+ 'SOURCE_BSDIFF'),
'is_last': (True, False),
'allow_signature': (True, False),
'allow_unhashed': (True, False),
@@ -1194,6 +1289,12 @@
'fail_unknown_sig_version': (True, False),
'fail_incorrect_sig': (True, False)})
+ # Add all _CheckMinorVersion() test cases.
+ AddParametricTests('CheckMinorVersion',
+ {'minor_version': (0, 1, 2, 555),
+ 'payload_type': (checker._TYPE_FULL,
+ checker._TYPE_DELTA)})
+
# Add all Run() test cases.
AddParametricTests('Run',
{'fail_wrong_payload_type': (True, False),