From patchwork Wed Aug 5 17:20:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Trammell Hudson X-Patchwork-Id: 11701999 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A32DF138A for ; Wed, 5 Aug 2020 17:23:52 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 771F82177B for ; Wed, 5 Aug 2020 17:23:52 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=trmm.net header.i=@trmm.net header.b="AGrU1KVL" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 771F82177B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=trmm.net Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1k3N6C-000498-Qw; Wed, 05 Aug 2020 17:21:08 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1k3N6B-000493-Ii for xen-devel@lists.xenproject.org; Wed, 05 Aug 2020 17:21:07 +0000 X-Inumbo-ID: 886d5c0b-818e-472d-8033-dc25ab1f62df Received: from mail-40133.protonmail.ch (unknown [185.70.40.133]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 886d5c0b-818e-472d-8033-dc25ab1f62df; Wed, 05 Aug 2020 17:21:01 +0000 (UTC) Date: Wed, 05 Aug 2020 17:20:56 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=trmm.net; s=protonmail; t=1596648059; bh=l1doubPoPn+XYU/lLBZRazYcvNj3vmi0JDcutwQoyQ0=; h=Date:To:From:Reply-To:Subject:From; b=AGrU1KVLW16bS57DpbUTZZG+rG2iKEZMXg4QnH1b0D64iZwGnAEdbbIks8RsTVj6z JIIcK8OSVTUAf4V55cq3v8YyQjEJ+UUbYTTbBLHj5zpYKheiWsFOHSw3bRKA9xKdx6 9evw2IJpXS2OSKwtwAbDeByh8jhFeaBtI/mGxNFQ= To: "xen-devel@lists.xenproject.org" From: Trammell Hudson Subject: [RFC] efi/boot: Unified Xen executable for UEFI Secure Boot support Message-ID: MIME-Version: 1.0 X-Spam-Status: No, score=-1.2 required=7.0 tests=ALL_TRUSTED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF shortcircuit=no autolearn=disabled version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on mail.protonmail.ch X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Reply-To: Trammell Hudson Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" This preliminary patch adds support for bundling the Xen hypervisor, xen.cfg, the Linux kernel, initrd and XSM into a single "unified" EFI executable that can be signed by sbsigntool for verification by UEFI Secure Boot. It is inspired by systemd-boot's unified kernel technique and borrows the function to locate PE sections from systemd's LGPL'ed code. The configuration, kernel, etc are added after building using objcopy to add named sections for each input file. This allows an administrator to update the components independently without requiring rebuilding xen. During EFI boot, Xen looks at its own loaded image to locate the PE sections and, if secure boot is enabled, only allows use of the unified components. The resulting EFI executable can be invoked directly from the UEFI Boot Manager, removing the need to use a separate loader like grub. Unlike the shim based verification, the signature covers the entire Xen+config+kernel+initrd unified file. This also ensures that properly configured platforms will measure the entire runtime into the TPM for unsealing secrets or remote attestation. It has been tested on qemu OVMF with Secure Boot enabled, as well as on real Thinkpad hardware. The EFI console is very slow, although it works and is able to boot into dom0. diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c index 5a520bf..b7b08b6 100644 --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -102,6 +102,7 @@ union string { struct file { UINTN size; + bool need_to_free; union { EFI_PHYSICAL_ADDRESS addr; void *ptr; @@ -330,13 +331,13 @@ static void __init noreturn blexit(const CHAR16 *str) if ( !efi_bs ) efi_arch_halt(); - if ( cfg.addr ) + if ( cfg.addr && cfg.need_to_free) efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size)); - if ( kernel.addr ) + if ( kernel.addr && kernel.need_to_free) efi_bs->FreePages(kernel.addr, PFN_UP(kernel.size)); - if ( ramdisk.addr ) + if ( ramdisk.addr && ramdisk.need_to_free) efi_bs->FreePages(ramdisk.addr, PFN_UP(ramdisk.size)); - if ( xsm.addr ) + if ( xsm.addr && xsm.need_to_free) efi_bs->FreePages(xsm.addr, PFN_UP(xsm.size)); efi_arch_blexit(); @@ -619,6 +620,7 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name, what = what ?: L"Seek"; else { + file->need_to_free = true; file->addr = min(1UL << (32 + PAGE_SHIFT), HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START); ret = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData, @@ -665,6 +667,136 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name, return true; } + +struct DosFileHeader { + UINT8 Magic[2]; + UINT16 LastSize; + UINT16 nBlocks; + UINT16 nReloc; + UINT16 HdrSize; + UINT16 MinAlloc; + UINT16 MaxAlloc; + UINT16 ss; + UINT16 sp; + UINT16 Checksum; + UINT16 ip; + UINT16 cs; + UINT16 RelocPos; + UINT16 nOverlay; + UINT16 reserved[4]; + UINT16 OEMId; + UINT16 OEMInfo; + UINT16 reserved2[10]; + UINT32 ExeHeader; +} __attribute__((packed)); + +#define PE_HEADER_MACHINE_I386 0x014c +#define PE_HEADER_MACHINE_X64 0x8664 +#define PE_HEADER_MACHINE_ARM64 0xaa64 + +struct PeFileHeader { + UINT16 Machine; + UINT16 NumberOfSections; + UINT32 TimeDateStamp; + UINT32 PointerToSymbolTable; + UINT32 NumberOfSymbols; + UINT16 SizeOfOptionalHeader; + UINT16 Characteristics; +} __attribute__((packed)); + +struct PeHeader { + UINT8 Magic[4]; + struct PeFileHeader FileHeader; +} __attribute__((packed)); + +struct PeSectionHeader { + UINT8 Name[8]; + UINT32 VirtualSize; + UINT32 VirtualAddress; + UINT32 SizeOfRawData; + UINT32 PointerToRawData; + UINT32 PointerToRelocations; + UINT32 PointerToLinenumbers; + UINT16 NumberOfRelocations; + UINT16 NumberOfLinenumbers; + UINT32 Characteristics; +} __attribute__((packed)); + +static void * __init pe_find_section(const void * const image_base, + const char * section_name, UINTN * size_out) +{ + const CHAR8 * const base = image_base; + const struct DosFileHeader * dos = (const void*) base; + const struct PeHeader * pe; + const UINTN name_len = strlen(section_name); + UINTN offset; + + if ( base == NULL ) + return NULL; + + if ( memcmp(dos->Magic, "MZ", 2) != 0 ) + return NULL; + + pe = (const void *) &base[dos->ExeHeader]; + if ( memcmp(pe->Magic, "PE\0\0", 4) != 0 ) + return NULL; + + /* PE32+ Subsystem type */ + if (pe->FileHeader.Machine != PE_HEADER_MACHINE_X64 + && pe->FileHeader.Machine != PE_HEADER_MACHINE_ARM64 + && pe->FileHeader.Machine != PE_HEADER_MACHINE_I386) + return NULL; + + if ( pe->FileHeader.NumberOfSections > 96 ) + return NULL; + + offset = dos->ExeHeader + sizeof(*pe) + pe->FileHeader.SizeOfOptionalHeader; + + for (UINTN i = 0; i < pe->FileHeader.NumberOfSections; i++) + { + const struct PeSectionHeader *const sect = (const struct PeSectionHeader *)&base[offset]; + if ( memcmp(sect->Name, section_name, name_len) == 0 ) + { + if ( size_out ) + *size_out = sect->VirtualSize; + return (void*)(sect->VirtualAddress + (uintptr_t) image_base); + } + + offset += sizeof(*sect); + } + + return NULL; +} + +static bool __init read_section(const void * const image_base, + char * const name, struct file *file, char *options) +{ + union string name_string = { .s = name + 1 }; + if ( !image_base ) + return false; + + file->ptr = pe_find_section(image_base, name, &file->size); + if ( !file->ptr ) + return false; + + file->need_to_free = false; + + if ( file == &cfg ) + return true; + + s2w(&name_string); + PrintStr(name_string.w); + PrintStr(L": "); + DisplayUint(file->addr, 2 * sizeof(file->addr)); + PrintStr(L"-"); + DisplayUint(file->addr + file->size, 2 * sizeof(file->addr)); + PrintStr(newline); + efi_arch_handle_module(file, name_string.w, options); + efi_bs->FreePool(name_string.w); + + return true; +} + static void __init pre_parse(const struct file *cfg) { char *ptr = cfg->ptr, *end = ptr + cfg->size; @@ -968,6 +1100,21 @@ static void __init setup_efi_pci(void) efi_bs->FreePool(handles); } +static bool __init efi_secure_boot(void) +{ + static const EFI_GUID global_guid = EFI_GLOBAL_VARIABLE; + uint8_t buf[8]; + UINTN size = sizeof(buf); + + if ( efi_rs->GetVariable(L"SecureBoot", (EFI_GUID *)&global_guid, NULL, &size, buf) != EFI_SUCCESS ) + return false; + + if ( size != 1 ) + return false; + + return buf[0] != 0; +} + static void __init efi_variables(void) { EFI_STATUS status; @@ -1143,6 +1290,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) static EFI_GUID __initdata loaded_image_guid = LOADED_IMAGE_PROTOCOL; static EFI_GUID __initdata shim_lock_guid = SHIM_LOCK_PROTOCOL_GUID; EFI_LOADED_IMAGE *loaded_image; + void * image_base = NULL; EFI_STATUS status; unsigned int i, argc; CHAR16 **argv, *file_name, *cfg_file_name = NULL, *options = NULL; @@ -1153,6 +1301,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) bool base_video = false; char *option_str; bool use_cfg_file; + bool secure = false; __set_bit(EFI_BOOT, &efi_flags); __set_bit(EFI_LOADER, &efi_flags); @@ -1171,6 +1320,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) PrintErrMesg(L"No Loaded Image Protocol", status); efi_arch_load_addr_check(loaded_image); + if ( loaded_image ) + image_base = loaded_image->ImageBase; + + secure = efi_secure_boot(); if ( use_cfg_file ) { @@ -1249,9 +1402,19 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) /* Get the file system interface. */ dir_handle = get_parent_handle(loaded_image, &file_name); - /* Read and parse the config file. */ - if ( !cfg_file_name ) + if ( read_section(image_base, ".config", &cfg, NULL) ) + { + if ( secure ) + PrintStr(L"Secure Boot enabled: "); + PrintStr(L"Using unified config file\r\n"); + } + else if ( secure ) + { + blexit(L"Secure Boot enabled, but configuration not included."); + } + else if ( !cfg_file_name ) { + /* Read and parse the config file. */ CHAR16 *tail; while ( (tail = point_tail(file_name)) != NULL ) @@ -1303,27 +1466,47 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable) efi_arch_cfg_file_early(dir_handle, section.s); option_str = split_string(name.s); - read_file(dir_handle, s2w(&name), &kernel, option_str); - efi_bs->FreePool(name.w); - if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL, - (void **)&shim_lock)) && - (status = shim_lock->Verify(kernel.ptr, kernel.size)) != EFI_SUCCESS ) - PrintErrMesg(L"Dom0 kernel image could not be verified", status); - - name.s = get_value(&cfg, section.s, "ramdisk"); - if ( name.s ) + if ( !read_section(image_base, ".kernel", &kernel, option_str) ) { - read_file(dir_handle, s2w(&name), &ramdisk, NULL); + if ( secure ) + blexit(L"Secure Boot enabled, but no kernel included"); + read_file(dir_handle, s2w(&name), &kernel, option_str); efi_bs->FreePool(name.w); + + if ( !EFI_ERROR(efi_bs->LocateProtocol(&shim_lock_guid, NULL, + (void **)&shim_lock)) && + (status = shim_lock->Verify(kernel.ptr, kernel.size)) != EFI_SUCCESS ) + PrintErrMesg(L"Dom0 kernel image could not be verified", status); } - name.s = get_value(&cfg, section.s, "xsm"); - if ( name.s ) + if ( !read_section(image_base, ".ramdisk", &ramdisk, NULL) ) { - read_file(dir_handle, s2w(&name), &xsm, NULL); - efi_bs->FreePool(name.w); + if ( secure ) + blexit(L"Secure Boot enabled, but no initrd included"); + name.s = get_value(&cfg, section.s, "ramdisk"); + if ( name.s ) + { + read_file(dir_handle, s2w(&name), &ramdisk, NULL); + efi_bs->FreePool(name.w); + } + } + +#ifdef CONFIG_XSM + if ( !read_section(image_base, ".xsm", &xsm, NULL) ) + { +#ifndef CONFIG_XSM_FLASK_POLICY + if ( secure ) + blexit(L"Secure Boot enabled, but no FLASK policy included"); +#endif + name.s = get_value(&cfg, section.s, "xsm"); + if ( name.s ) + { + read_file(dir_handle, s2w(&name), &xsm, NULL); + efi_bs->FreePool(name.w); + } } +#endif /* * EFI_LOAD_OPTION does not supply an image name as first component: diff --git a/xen/scripts/unify-xen b/xen/scripts/unify-xen new file mode 100755 index 0000000..b6072b1 --- /dev/null +++ b/xen/scripts/unify-xen @@ -0,0 +1,68 @@ +#!/bin/bash +# Merge a Linux kernel, initrd and commandline into xen.efi to produce a single signed +# EFI executable. +# +# turn off "expressions don't expand in single quotes" +# and "can't follow non-constant sources" +# shellcheck disable=SC2016 disable=SC1090 +set -e -o pipefail +export LC_ALL=C + +die() { echo "$@" >&2 ; exit 1 ; } +warn() { echo "$@" >&2 ; } +debug() { [ "$VERBOSE" == 1 ] && echo "$@" >&2 ; } + +cleanup() { + rm -rf "$TMP" +} + +TMP=$(mktemp -d) +TMP_MOUNT=n +trap cleanup EXIT + +######################################## + +# Usage +# unify xen.efi xen.cfg bzimage initrd +# Xen goes up to a pad at 00400000 + +XEN="$1" +CONFIG="$2" +KERNEL="$3" +RAMDISK="$4" +# --change-section-vma .config=0x0500000 \ +# --change-section-vma .kernel=0x0510000 \ +# --change-section-vma .ramdisk=0x3000000 \ + +objcopy \ + --add-section .kernel="$KERNEL" \ + --add-section .ramdisk="$RAMDISK" \ + --add-section .config="$CONFIG" \ + --change-section-vma .config=0xffff82d041000000 \ + --change-section-vma .kernel=0xffff82d041010000 \ + --change-section-vma .ramdisk=0xffff82d042000000 \ + "$XEN" \ + "$TMP/xen.efi" \ +|| die "$TMP/xen.efi: unable to create" + +KEY_ENGINE="" +KEY="/etc/safeboot/signing.key" +CERT="/etc/safeboot/cert.pem" + +for try in 1 2 3 ; do + warn "$TMP/xen.efi: Signing (ignore warnings about gaps)" + sbsign.safeboot \ + $KEY_ENGINE \ + --key "$KEY" \ + --cert "$CERT" \ + --output "xen.signed.efi" \ + "$TMP/xen.efi" \ + && break + + if [ "$try" == 3 ]; then + die "xen.signed.efi: failed after $try tries" + fi + + warn "$OUTDIR/linux.efi: signature failed! Try $try." +done +