From f91f493825921d1aa5b73899824f9da6396a736f Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Tue, 14 Feb 2023 16:29:22 -0500 Subject: sign_uefi: Add support for crdyboot The crdyboot bootloader (see https://chromium.googlesource.com/chromiumos/platform/crdyboot) for the reven board requires some additional handling for signing: * A public key for verifying the kernel must be injected into the `.vbpubk` section. * Then the file must be signed with `sbsign` in the usual way. Testing commands: ``` scripts/keygeneration/make_arv_root.sh arv scripts/keygeneration/create_new_keys.sh \ --arv-root-path arv --uefi --output reven scripts/image_signing/sign_official_build.sh recovery \ ~/chromiumos/src/build/images/reven/latest/chromiumos_image.bin \ reven \ ~/chromiumos/src/build/images/reven/latest/chromiumos_image.signed Then boot the image in a UEFI VM. ``` BRANCH=none BUG=b:256176281 TEST=make runtests TEST=See testing notes above Change-Id: Id454ff0677c397b2c399f39981862ac18c2c9985 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/4250562 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/4381000 Reviewed-by: Cheng Yueh Commit-Queue: Cheng Yueh Tested-by: Phoebe Wang Auto-Submit: Phoebe Wang --- scripts/image_signing/sign_uefi.py | 42 ++++++++++++++++++++++++-- scripts/image_signing/sign_uefi_unittest.py | 47 ++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 3 deletions(-) diff --git a/scripts/image_signing/sign_uefi.py b/scripts/image_signing/sign_uefi.py index 53ac87c6..839f413d 100755 --- a/scripts/image_signing/sign_uefi.py +++ b/scripts/image_signing/sign_uefi.py @@ -10,6 +10,7 @@ The target directory can be either the root of ESP or /boot of root filesystem. import argparse import logging +import os from pathlib import Path import shutil import subprocess @@ -96,6 +97,32 @@ class Signer: sys.exit("Verification failed") +def inject_vbpubk(efi_file: os.PathLike, key_dir: os.PathLike): + """Update a UEFI executable's vbpubk section. + + The crdyboot bootloader contains an embedded public key in the + ".vbpubk" section. This function replaces the data in the existing + section (normally containing a dev key) with the real key. + + Args: + efi_file: Path of a UEFI file. + key_dir: Path of the UEFI key directory. + """ + section_name = ".vbpubk" + logging.info("updating section %s in %s", section_name, efi_file.name) + section_data_path = key_dir / "../kernel_subkey.vbpubk" + subprocess.run( + [ + "sudo", + "objcopy", + "--update-section", + f"{section_name}={section_data_path}", + efi_file, + ], + check=True, + ) + + def sign_target_dir(target_dir, key_dir, efi_glob): """Sign various EFI files under |target_dir|. @@ -119,12 +146,18 @@ def sign_target_dir(target_dir, key_dir, efi_glob): ensure_file_exists(sign_key, "No signing key") with tempfile.TemporaryDirectory() as working_dir: - signer = Signer(Path(working_dir), sign_key, sign_cert, verify_cert) + working_dir = Path(working_dir) + signer = Signer(working_dir, sign_key, sign_cert, verify_cert) for efi_file in sorted(bootloader_dir.glob(efi_glob)): if efi_file.is_file(): signer.sign_efi_file(efi_file) + for efi_file in sorted(bootloader_dir.glob("crdyboot*.efi")): + if efi_file.is_file(): + inject_vbpubk(efi_file, key_dir) + signer.sign_efi_file(efi_file) + for syslinux_kernel_file in sorted(syslinux_dir.glob("vmlinuz.?")): if syslinux_kernel_file.is_file(): signer.sign_efi_file(syslinux_kernel_file) @@ -165,7 +198,12 @@ def main(argv: Optional[List[str]] = None) -> Optional[int]: parser = get_parser() opts = parser.parse_args(argv) - for tool in ("sbattach", "sbsign", "sbverify"): + for tool in ( + "objcopy", + "sbattach", + "sbsign", + "sbverify", + ): ensure_executable_available(tool) sign_target_dir(opts.target_dir, opts.key_dir, opts.efi_glob) diff --git a/scripts/image_signing/sign_uefi_unittest.py b/scripts/image_signing/sign_uefi_unittest.py index 2cd4342c..de5229c5 100755 --- a/scripts/image_signing/sign_uefi_unittest.py +++ b/scripts/image_signing/sign_uefi_unittest.py @@ -20,8 +20,9 @@ import sign_uefi class Test(unittest.TestCase): """Test sign_uefi.py.""" + @mock.patch("sign_uefi.inject_vbpubk") @mock.patch.object(sign_uefi.Signer, "sign_efi_file") - def test_successful_sign(self, mock_sign): + def test_successful_sign(self, mock_sign, mock_inject_vbpubk): with tempfile.TemporaryDirectory() as tmp_dir: tmp_dir = Path(tmp_dir) @@ -50,6 +51,8 @@ class Test(unittest.TestCase): (efi_boot_dir / "bootx64.efi").touch() (efi_boot_dir / "testia32.efi").touch() (efi_boot_dir / "testx64.efi").touch() + (efi_boot_dir / "crdybootia32.efi").touch() + (efi_boot_dir / "crdybootx64.efi").touch() (syslinux_dir / "vmlinuz.A").touch() (syslinux_dir / "vmlinuz.B").touch() (target_dir / "vmlinuz-5.10.156").touch() @@ -69,6 +72,9 @@ class Test(unittest.TestCase): # the boot*.efi files don't. mock.call(efi_boot_dir / "testia32.efi"), mock.call(efi_boot_dir / "testx64.efi"), + # Two crdyboot files. + mock.call(efi_boot_dir / "crdybootia32.efi"), + mock.call(efi_boot_dir / "crdybootx64.efi"), # Two syslinux kernels. mock.call(syslinux_dir / "vmlinuz.A"), mock.call(syslinux_dir / "vmlinuz.B"), @@ -77,6 +83,45 @@ class Test(unittest.TestCase): ], ) + # Check that `inject_vbpubk` was called on both the crdyboot + # executables. + self.assertEqual( + mock_inject_vbpubk.call_args_list, + [ + mock.call(efi_boot_dir / "crdybootia32.efi", key_dir), + mock.call(efi_boot_dir / "crdybootx64.efi", key_dir), + ], + ) + + @mock.patch("sign_uefi.subprocess.run") + def test_inject_vbpubk(self, mock_run): + with tempfile.TemporaryDirectory() as tmp_dir: + tmp_dir = Path(tmp_dir) + + key_dir = tmp_dir / "key_dir" + uefi_key_dir = key_dir / "uefi" + uefi_key_dir.mkdir(parents=True) + + efi_file = tmp_dir / "test_efi_file" + sign_uefi.inject_vbpubk(efi_file, uefi_key_dir) + + # Check that the expected command runs. + self.assertEqual( + mock_run.call_args_list, + [ + mock.call( + [ + "sudo", + "objcopy", + "--update-section", + f'.vbpubk={uefi_key_dir / "../kernel_subkey.vbpubk"}', + efi_file, + ], + check=True, + ) + ], + ) + if __name__ == "__main__": unittest.main() -- cgit v1.2.1