From 8ea1957b8bd6b58ffd7d04d2f0d2d6ff6823c368 Mon Sep 17 00:00:00 2001 From: Amin Hassani Date: Wed, 5 Dec 2018 12:06:21 -0800 Subject: update_payload: Add the remaining major version 2 signature supports This patch fixes the issues with signatures (sizes) in major version 2 and a few minor issues with the payload and metadata sizes. BUG=chromium:862679 TEST=manually signing the payload and running update_payload_check TEST=unittests Change-Id: I9b431379b0574a150474a913f1ec4a11e86288ae Reviewed-on: https://chromium-review.googlesource.com/1363339 Commit-Ready: Amin Hassani Tested-by: Amin Hassani Reviewed-by: Sen Jiang --- scripts/update_payload/checker.py | 44 ++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 17 deletions(-) (limited to 'scripts/update_payload/checker.py') diff --git a/scripts/update_payload/checker.py b/scripts/update_payload/checker.py index 746d4be7..7a319f76 100644 --- a/scripts/update_payload/checker.py +++ b/scripts/update_payload/checker.py @@ -627,7 +627,7 @@ class PayloadChecker(object): self._CheckPresentIff(self.sigs_offset, self.sigs_size, 'signatures_offset', 'signatures_size', 'manifest') - if self.major_version == 1: + if self.major_version == common.CHROMEOS_MAJOR_PAYLOAD_VERSION: for real_name, proto_name in common.CROS_PARTITIONS: self.old_part_info[real_name] = self._CheckOptionalSubMsg( manifest, 'old_%s_info' % proto_name, report) @@ -1069,8 +1069,9 @@ class PayloadChecker(object): # Type-specific checks. if op.type in (common.OpType.REPLACE, common.OpType.REPLACE_BZ): self._CheckReplaceOperation(op, data_length, total_dst_blocks, op_name) - elif op.type == common.OpType.REPLACE_XZ and (self.minor_version >= 3 or - self.major_version >= 2): + elif (op.type == common.OpType.REPLACE_XZ and + (self.minor_version >= 3 or + self.major_version >= common.BRILLO_MAJOR_PAYLOAD_VERSION)): self._CheckReplaceOperation(op, data_length, total_dst_blocks, op_name) elif op.type == common.OpType.MOVE and self.minor_version == 1: self._CheckMoveOperation(op, data_offset, total_src_blocks, @@ -1251,16 +1252,19 @@ class PayloadChecker(object): last_ops_section = (self.payload.manifest.kernel_install_operations or self.payload.manifest.install_operations) - fake_sig_op = last_ops_section[-1] - # Check: signatures_{offset,size} must match the last (fake) operation. - if not (fake_sig_op.type == common.OpType.REPLACE and - self.sigs_offset == fake_sig_op.data_offset and - self.sigs_size == fake_sig_op.data_length): - raise error.PayloadError( - 'Signatures_{offset,size} (%d+%d) does not match last operation ' - '(%d+%d).' % - (self.sigs_offset, self.sigs_size, fake_sig_op.data_offset, - fake_sig_op.data_length)) + + # Only major version 1 has the fake signature OP at the end. + if self.major_version == common.CHROMEOS_MAJOR_PAYLOAD_VERSION: + fake_sig_op = last_ops_section[-1] + # Check: signatures_{offset,size} must match the last (fake) operation. + if not (fake_sig_op.type == common.OpType.REPLACE and + self.sigs_offset == fake_sig_op.data_offset and + self.sigs_size == fake_sig_op.data_length): + raise error.PayloadError('Signatures_{offset,size} (%d+%d) does not' + ' match last operation (%d+%d).' % + (self.sigs_offset, self.sigs_size, + fake_sig_op.data_offset, + fake_sig_op.data_length)) # Compute the checksum of all data up to signature blob. # TODO(garnold) we're re-reading the whole data section into a string @@ -1314,9 +1318,9 @@ class PayloadChecker(object): try: # Check metadata_size (if provided). - if metadata_size and self.payload.data_offset != metadata_size: + if metadata_size and self.payload.metadata_size != metadata_size: raise error.PayloadError('Invalid payload metadata size in payload(%d) ' - 'vs given(%d)' % (self.payload.data_offset, + 'vs given(%d)' % (self.payload.metadata_size, metadata_size)) # Check metadata signature (if provided). @@ -1343,7 +1347,7 @@ class PayloadChecker(object): # Part 3: Examine partition operations. install_operations = [] - if self.major_version == 1: + if self.major_version == common.CHROMEOS_MAJOR_PAYLOAD_VERSION: # partitions field should not ever exist in major version 1 payloads self._CheckRepeatedElemNotPresent(manifest, 'partitions', 'manifest') @@ -1386,10 +1390,16 @@ class PayloadChecker(object): operations, report, '%s_install_operations' % part, self.old_fs_sizes[part], self.new_fs_sizes[part], old_fs_usable_size, new_fs_usable_size, total_blob_size, - self.major_version == 1 and part == common.KERNEL) + (self.major_version == common.CHROMEOS_MAJOR_PAYLOAD_VERSION + and part == common.KERNEL)) # Check: Operations data reach the end of the payload file. used_payload_size = self.payload.data_offset + total_blob_size + # Major versions 2 and higher have a signature at the end, so it should be + # considered in the total size of the image. + if self.major_version >= common.BRILLO_MAJOR_PAYLOAD_VERSION: + used_payload_size += self.sigs_size + if used_payload_size != payload_file_size: raise error.PayloadError( 'Used payload size (%d) different from actual file size (%d).' % -- cgit v1.2.3 From 72b80edb1ef2db08564929549eb8d7a1e0b24542 Mon Sep 17 00:00:00 2001 From: Amin Hassani Date: Wed, 12 Dec 2018 23:15:30 -0800 Subject: update_payload: Fix problem with signature size on unsigned payloads If the payload is unsigned (e.g. for test image), then the checker.sigs_file would not exist. So check for that variable before using it. BUG=chromium:794404 BUG=chromium:914705 TEST=manually signing the payload ran update_payload_check with signed and unsigned image TEST=unittests Change-Id: I871137eadef00d012ee926d12fd4eee36a454487 Reviewed-on: https://chromium-review.googlesource.com/1375023 Commit-Ready: Amin Hassani Tested-by: Amin Hassani Reviewed-by: Sen Jiang --- scripts/update_payload/checker.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'scripts/update_payload/checker.py') diff --git a/scripts/update_payload/checker.py b/scripts/update_payload/checker.py index 7a319f76..15f11ae6 100644 --- a/scripts/update_payload/checker.py +++ b/scripts/update_payload/checker.py @@ -1397,7 +1397,8 @@ class PayloadChecker(object): used_payload_size = self.payload.data_offset + total_blob_size # Major versions 2 and higher have a signature at the end, so it should be # considered in the total size of the image. - if self.major_version >= common.BRILLO_MAJOR_PAYLOAD_VERSION: + if (self.major_version >= common.BRILLO_MAJOR_PAYLOAD_VERSION and + self.sigs_size): used_payload_size += self.sigs_size if used_payload_size != payload_file_size: -- cgit v1.2.3