From f9beb20e99abbfcbea7cf222ba692aa3cbf10df7 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Thu, 4 May 2023 14:21:42 +0100 Subject: [image] Allow for images to be hidden from lists of all images When invoking a kernel via the UEFI shim, the kernel (and potentially also a helper binary such as GRUB) must be accessible via the virtual filesystem exposed via EFI_SIMPLE_FILE_SYSTEM_PROTOCOL but must not be present in the magic initrd constructed from all registered images. Allow for images to be flagged as hidden, which will cause them to be excluded from API-level lists of all images such as the virtual filesystem directory contents, the magic initrd, or the Multiboot module list. Hidden images remain visible to iPXE commands including "imgstat", which will show a "[HIDDEN]" flag for such images. Signed-off-by: Michael Brown --- src/usr/imgmgmt.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/usr') diff --git a/src/usr/imgmgmt.c b/src/usr/imgmgmt.c index b7fc8293d..94f3d2ce0 100644 --- a/src/usr/imgmgmt.c +++ b/src/usr/imgmgmt.c @@ -165,6 +165,8 @@ void imgstat ( struct image *image ) { printf ( " [SELECTED]" ); if ( image->flags & IMAGE_AUTO_UNREGISTER ) printf ( " [AUTOFREE]" ); + if ( image->flags & IMAGE_HIDDEN ) + printf ( " [HIDDEN]" ); if ( image->cmdline ) printf ( " \"%s\"", image->cmdline ); printf ( "\n" ); -- cgit v1.2.3-55-g7522 From c4a8d90387437425faec91bdee42a254944bab76 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Sat, 13 May 2023 20:27:58 +0100 Subject: [image] Generalise concept of selected image Most image flags are independent values: any combination of flags may be set for any image, and the flags for one image are independent of the flags for any other image. The "selected" flag does not follow this pattern: at most one image may be marked as selected at any time. When invoking a kernel via the UEFI shim, there will be multiple "special" images: the selected kernel itself, the shim image, and potentially a shim-signed GRUB binary to be used as a crutch to assist shim in loading the kernel (since current versions of the UEFI shim are not capable of directly loading a Linux kernel). Remove the "selected" image flag and replace it with a general concept of an image tag with the same semantics: a given tag may be assigned to at most one image, an image may be found by its tag only while the image is currently registered, and a tag will survive unregistration and reregistration of an image (if it has not already been assigned to a new image). For visual consistency, also replace the current image pointer with a current image tag. The image pointer stored within the image tag holds only a weak reference to the image, since the selection of an image should not prevent that image from being freed. (The strong reference to the currently executing image is held locally within the execution scope of image_exec(), and is logically separate from the current image pointer.) Signed-off-by: Michael Brown --- src/core/image.c | 69 +++++++++++++++++++++++--------------------- src/hci/commands/image_cmd.c | 2 +- src/image/script.c | 7 +++-- src/include/ipxe/image.h | 47 +++++++++++++++++++++++------- src/interface/efi/efi_file.c | 2 +- src/usr/imgmgmt.c | 8 +++-- 6 files changed, 85 insertions(+), 50 deletions(-) (limited to 'src/usr') diff --git a/src/core/image.c b/src/core/image.c index b280eb4d3..3e65b5edf 100644 --- a/src/core/image.c +++ b/src/core/image.c @@ -56,8 +56,15 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); /** List of registered images */ struct list_head images = LIST_HEAD_INIT ( images ); +/** Image selected for execution */ +struct image_tag selected_image __image_tag = { + .name = "SELECTED", +}; + /** Currently-executing image */ -struct image *current_image; +struct image_tag current_image __image_tag = { + .name = "CURRENT", +}; /** Current image trust requirement */ static int require_trusted_images = 0; @@ -72,8 +79,13 @@ static int require_trusted_images_permanent = 0; */ static void free_image ( struct refcnt *refcnt ) { struct image *image = container_of ( refcnt, struct image, refcnt ); + struct image_tag *tag; DBGC ( image, "IMAGE %s freed\n", image->name ); + for_each_table_entry ( tag, IMAGE_TAGS ) { + if ( tag->image == image ) + tag->image = NULL; + } free ( image->name ); free ( image->cmdline ); uri_put ( image->uri ); @@ -261,12 +273,6 @@ int register_image ( struct image *image ) { return rc; } - /* Avoid ending up with multiple "selected" images on - * re-registration - */ - if ( image_find_selected() ) - image->flags &= ~IMAGE_SELECTED; - /* Add to image list */ image_get ( image ); image->flags |= IMAGE_REGISTERED; @@ -320,6 +326,23 @@ struct image * find_image ( const char *name ) { return NULL; } +/** + * Find image by tag + * + * @v tag Image tag + * @ret image Executable image, or NULL + */ +struct image * find_image_tag ( struct image_tag *tag ) { + struct image *image; + + for_each_image ( image ) { + if ( tag->image == image ) + return image; + } + + return NULL; +} + /** * Execute image * @@ -346,13 +369,13 @@ int image_exec ( struct image *image ) { if ( image->uri ) churi ( image->uri ); - /* Preserve record of any currently-running image */ - saved_current_image = current_image; + /* Set as currently running image */ + saved_current_image = image_tag ( image, ¤t_image ); /* Take out a temporary reference to the image, so that it * does not get freed when temporarily unregistered. */ - current_image = image_get ( image ); + image_get ( image ); /* Check that this image can be executed */ if ( ! ( image->type && image->type->exec ) ) { @@ -419,7 +442,7 @@ int image_exec ( struct image *image ) { image_put ( image ); /* Restore previous currently-running image */ - current_image = saved_current_image; + image_tag ( saved_current_image, ¤t_image ); /* Reset current working directory */ churi ( old_cwuri ); @@ -442,7 +465,7 @@ int image_exec ( struct image *image ) { * registered until the currently-executing image returns. */ int image_replace ( struct image *replacement ) { - struct image *image = current_image; + struct image *image = current_image.image; int rc; /* Sanity check */ @@ -478,37 +501,17 @@ int image_replace ( struct image *replacement ) { * @ret rc Return status code */ int image_select ( struct image *image ) { - struct image *tmp; - - /* Unselect all other images */ - for_each_image ( tmp ) - tmp->flags &= ~IMAGE_SELECTED; /* Check that this image can be executed */ if ( ! ( image->type && image->type->exec ) ) return -ENOEXEC; /* Mark image as selected */ - image->flags |= IMAGE_SELECTED; + image_tag ( image, &selected_image ); return 0; } -/** - * Find selected image - * - * @ret image Executable image, or NULL - */ -struct image * image_find_selected ( void ) { - struct image *image; - - for_each_image ( image ) { - if ( image->flags & IMAGE_SELECTED ) - return image; - } - return NULL; -} - /** * Change image trust requirement * diff --git a/src/hci/commands/image_cmd.c b/src/hci/commands/image_cmd.c index 4a7c500a4..bf97b4deb 100644 --- a/src/hci/commands/image_cmd.c +++ b/src/hci/commands/image_cmd.c @@ -129,7 +129,7 @@ static int imgsingle_exec ( int argc, char **argv, &image ) ) != 0 ) goto err_acquire; } else { - image = image_find_selected(); + image = find_image_tag ( &selected_image ); if ( ! image ) { printf ( "No image selected\n" ); goto err_acquire; diff --git a/src/image/script.c b/src/image/script.c index b34df1e21..49b356403 100644 --- a/src/image/script.c +++ b/src/image/script.c @@ -311,6 +311,7 @@ static int terminate_on_label_found ( int rc ) { * @ret rc Return status code */ static int goto_exec ( int argc, char **argv ) { + struct image *image = current_image.image; struct goto_options opts; size_t saved_offset; int rc; @@ -320,7 +321,7 @@ static int goto_exec ( int argc, char **argv ) { return rc; /* Sanity check */ - if ( ! current_image ) { + if ( ! image ) { rc = -ENOTTY; printf ( "Not in a script: %s\n", strerror ( rc ) ); return rc; @@ -331,10 +332,10 @@ static int goto_exec ( int argc, char **argv ) { /* Find label */ saved_offset = script_offset; - if ( ( rc = process_script ( current_image, goto_find_label, + if ( ( rc = process_script ( image, goto_find_label, terminate_on_label_found ) ) != 0 ) { script_offset = saved_offset; - DBGC ( current_image, "[%04zx] No such label :%s\n", + DBGC ( image, "[%04zx] No such label :%s\n", script_offset, goto_label ); return rc; } diff --git a/src/include/ipxe/image.h b/src/include/ipxe/image.h index e00af82ef..cd51188d3 100644 --- a/src/include/ipxe/image.h +++ b/src/include/ipxe/image.h @@ -61,19 +61,16 @@ struct image { }; /** Image is registered */ -#define IMAGE_REGISTERED 0x00001 - -/** Image is selected for execution */ -#define IMAGE_SELECTED 0x0002 +#define IMAGE_REGISTERED 0x0001 /** Image is trusted */ -#define IMAGE_TRUSTED 0x0004 +#define IMAGE_TRUSTED 0x0002 /** Image will be automatically unregistered after execution */ -#define IMAGE_AUTO_UNREGISTER 0x0008 +#define IMAGE_AUTO_UNREGISTER 0x0004 /** Image will be hidden from enumeration */ -#define IMAGE_HIDDEN 0x0010 +#define IMAGE_HIDDEN 0x0008 /** An executable image type */ struct image_type { @@ -153,8 +150,23 @@ struct image_type { /** An executable image type */ #define __image_type( probe_order ) __table_entry ( IMAGE_TYPES, probe_order ) +/** An image tag */ +struct image_tag { + /** Name */ + const char *name; + /** Image (weak reference, nullified when image is freed) */ + struct image *image; +}; + +/** Image tag table */ +#define IMAGE_TAGS __table ( struct image_tag, "image_tags" ) + +/** An image tag */ +#define __image_tag __table_entry ( IMAGE_TAGS, 01 ) + extern struct list_head images; -extern struct image *current_image; +extern struct image_tag current_image; +extern struct image_tag selected_image; /** Iterate over all registered images */ #define for_each_image( image ) \ @@ -181,11 +193,11 @@ extern int image_set_len ( struct image *image, size_t len ); extern int image_set_data ( struct image *image, userptr_t data, size_t len ); extern int register_image ( struct image *image ); extern void unregister_image ( struct image *image ); -struct image * find_image ( const char *name ); +extern struct image * find_image ( const char *name ); +extern struct image * find_image_tag ( struct image_tag *tag ); extern int image_exec ( struct image *image ); extern int image_replace ( struct image *replacement ); extern int image_select ( struct image *image ); -extern struct image * image_find_selected ( void ); extern int image_set_trust ( int require_trusted, int permanent ); extern struct image * image_memory ( const char *name, userptr_t data, size_t len ); @@ -244,4 +256,19 @@ static inline void image_untrust ( struct image *image ) { image->flags &= ~IMAGE_TRUSTED; } +/** + * Tag image + * + * @v image Image + * @v tag Image tag + * @ret prev Previous tagged image (if any) + */ +static inline struct image * image_tag ( struct image *image, + struct image_tag *tag ) { + struct image *prev = tag->image; + + tag->image = image; + return prev; +} + #endif /* _IPXE_IMAGE_H */ diff --git a/src/interface/efi/efi_file.c b/src/interface/efi/efi_file.c index 266de4a6c..2ae3a0cb4 100644 --- a/src/interface/efi/efi_file.c +++ b/src/interface/efi/efi_file.c @@ -420,7 +420,7 @@ efi_file_open ( EFI_FILE_PROTOCOL *this, EFI_FILE_PROTOCOL **new, if ( ( strncasecmp ( name, "grub", 4 ) == 0 ) && ( ( sep = strrchr ( name, '.' ) ) != NULL ) && ( strcasecmp ( sep, ".efi" ) == 0 ) && - ( ( image = image_find_selected() ) != NULL ) ) { + ( ( image = find_image_tag ( &selected_image ) ) != NULL ) ) { return efi_file_open_image ( image, wname, new ); } diff --git a/src/usr/imgmgmt.c b/src/usr/imgmgmt.c index 94f3d2ce0..92bf236f9 100644 --- a/src/usr/imgmgmt.c +++ b/src/usr/imgmgmt.c @@ -156,13 +156,17 @@ int imgacquire ( const char *name_uri, unsigned long timeout, * @v image Executable/loadable image */ void imgstat ( struct image *image ) { + struct image_tag *tag; + printf ( "%s : %zd bytes", image->name, image->len ); if ( image->type ) printf ( " [%s]", image->type->name ); + for_each_table_entry ( tag, IMAGE_TAGS ) { + if ( tag->image == image ) + printf ( " [%s]", tag->name ); + } if ( image->flags & IMAGE_TRUSTED ) printf ( " [TRUSTED]" ); - if ( image->flags & IMAGE_SELECTED ) - printf ( " [SELECTED]" ); if ( image->flags & IMAGE_AUTO_UNREGISTER ) printf ( " [AUTOFREE]" ); if ( image->flags & IMAGE_HIDDEN ) -- cgit v1.2.3-55-g7522 From 28184b7c22ca2297bd5c0ad9d333bc8620d38915 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Mon, 22 May 2023 14:11:22 +0100 Subject: [efi] Add support for executing images via a shim Add support for using a shim as a helper to execute an EFI image. When a shim has been specified via shim(), the shim image will be passed to LoadImage() instead of the selected EFI image and the command line will be prepended with the name of the selected EFI image. The selected EFI image will be accessible to the shim via the virtual filesystem as a hidden file. Reduce the Secure Boot attack surface by removing, where possible, the spurious requirement for a third party second stage loader binary such as GRUB to be used solely in order to call the "shim lock protocol" entry point. Do not install the EFI PXE APIs when using a shim, since if shim finds EFI_PXE_BASE_CODE_PROTOCOL on the loaded image's device handle then it will attempt to download files afresh instead of using the files already downloaded by iPXE and exposed via the EFI_SIMPLE_FILE_SYSTEM protocol. (Experience shows that there is no point in trying to get a fix for this upstreamed into shim.) Signed-off-by: Michael Brown --- src/image/efi_image.c | 31 ++++- src/include/ipxe/efi/efi_image.h | 27 +++++ src/include/ipxe/efi/efi_shim.h | 23 ++++ src/include/ipxe/errfile.h | 1 + src/include/ipxe/image.h | 9 ++ src/include/usr/shimmgmt.h | 16 +++ src/interface/efi/efi_shim.c | 251 +++++++++++++++++++++++++++++++++++++++ src/usr/shimmgmt.c | 58 +++++++++ 8 files changed, 413 insertions(+), 3 deletions(-) create mode 100644 src/include/ipxe/efi/efi_image.h create mode 100644 src/include/ipxe/efi/efi_shim.h create mode 100644 src/include/usr/shimmgmt.h create mode 100644 src/interface/efi/efi_shim.c create mode 100644 src/usr/shimmgmt.c (limited to 'src/usr') diff --git a/src/image/efi_image.c b/src/image/efi_image.c index 437134035..104753a85 100644 --- a/src/image/efi_image.c +++ b/src/image/efi_image.c @@ -31,6 +31,8 @@ FILE_LICENCE ( GPL2_OR_LATER ); #include #include #include +#include +#include #include #include #include @@ -134,6 +136,8 @@ static int efi_image_exec ( struct image *image ) { EFI_LOADED_IMAGE_PROTOCOL *image; void *interface; } loaded; + struct image *shim; + struct image *exec; EFI_HANDLE handle; EFI_MEMORY_TYPE type; wchar_t *cmdline; @@ -150,6 +154,15 @@ static int efi_image_exec ( struct image *image ) { goto err_no_snpdev; } + /* Use shim instead of directly executing image if applicable */ + shim = ( efi_can_load ( image ) ? + NULL : find_image_tag ( &efi_shim ) ); + exec = ( shim ? shim : image ); + if ( shim ) { + DBGC ( image, "EFIIMAGE %s executing via %s\n", + image->name, shim->name ); + } + /* Re-register as a hidden image to allow for access via file I/O */ toggle = ( ~image->flags & IMAGE_HIDDEN ); image->flags |= IMAGE_HIDDEN; @@ -178,7 +191,7 @@ static int efi_image_exec ( struct image *image ) { } /* Create device path for image */ - path = efi_image_path ( image, snpdev->path ); + path = efi_image_path ( exec, snpdev->path ); if ( ! path ) { DBGC ( image, "EFIIMAGE %s could not create device path\n", image->name ); @@ -195,11 +208,20 @@ static int efi_image_exec ( struct image *image ) { goto err_cmdline; } + /* Install shim special handling if applicable */ + if ( shim && + ( ( rc = efi_shim_install ( shim, snpdev->handle, + &cmdline ) ) != 0 ) ){ + DBGC ( image, "EFIIMAGE %s could not install shim handling: " + "%s\n", image->name, strerror ( rc ) ); + goto err_shim_install; + } + /* Attempt loading image */ handle = NULL; if ( ( efirc = bs->LoadImage ( FALSE, efi_image_handle, path, - user_to_virt ( image->data, 0 ), - image->len, &handle ) ) != 0 ) { + user_to_virt ( exec->data, 0 ), + exec->len, &handle ) ) != 0 ) { /* Not an EFI image */ rc = -EEFI_LOAD ( efirc ); DBGC ( image, "EFIIMAGE %s could not load: %s\n", @@ -289,6 +311,9 @@ static int efi_image_exec ( struct image *image ) { if ( rc != 0 ) bs->UnloadImage ( handle ); err_load_image: + if ( shim ) + efi_shim_uninstall(); + err_shim_install: free ( cmdline ); err_cmdline: free ( path ); diff --git a/src/include/ipxe/efi/efi_image.h b/src/include/ipxe/efi/efi_image.h new file mode 100644 index 000000000..0fc0402b1 --- /dev/null +++ b/src/include/ipxe/efi/efi_image.h @@ -0,0 +1,27 @@ +#ifndef _IPXE_EFI_IMAGE_H +#define _IPXE_EFI_IMAGE_H + +/** @file + * + * EFI images + * + */ + +FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); + +#include + +extern struct image_type efi_image_type[] __image_type ( PROBE_NORMAL ); + +/** + * Check if EFI image can be loaded directly + * + * @v image EFI image + * @ret can_load EFI image can be loaded directly + */ +static inline int efi_can_load ( struct image *image ) { + + return ( image->type == efi_image_type ); +} + +#endif /* _IPXE_EFI_IMAGE_H */ diff --git a/src/include/ipxe/efi/efi_shim.h b/src/include/ipxe/efi/efi_shim.h new file mode 100644 index 000000000..ad8d24dce --- /dev/null +++ b/src/include/ipxe/efi/efi_shim.h @@ -0,0 +1,23 @@ +#ifndef _IPXE_EFI_SHIM_H +#define _IPXE_EFI_SHIM_H + +/** @file + * + * UEFI shim special handling + * + */ + +FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); + +#include +#include + +extern int efi_shim_require_loader; +extern int efi_shim_allow_pxe; +extern struct image_tag efi_shim __image_tag; + +extern int efi_shim_install ( struct image *shim, EFI_HANDLE handle, + wchar_t **cmdline ); +extern void efi_shim_uninstall ( void ); + +#endif /* _IPXE_EFI_SHIM_H */ diff --git a/src/include/ipxe/errfile.h b/src/include/ipxe/errfile.h index 235611a51..daa038c52 100644 --- a/src/include/ipxe/errfile.h +++ b/src/include/ipxe/errfile.h @@ -405,6 +405,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); #define ERRFILE_dhe ( ERRFILE_OTHER | 0x005a0000 ) #define ERRFILE_efi_cmdline ( ERRFILE_OTHER | 0x005b0000 ) #define ERRFILE_efi_rng ( ERRFILE_OTHER | 0x005c0000 ) +#define ERRFILE_efi_shim ( ERRFILE_OTHER | 0x005d0000 ) /** @} */ diff --git a/src/include/ipxe/image.h b/src/include/ipxe/image.h index cd51188d3..bfbf23687 100644 --- a/src/include/ipxe/image.h +++ b/src/include/ipxe/image.h @@ -256,6 +256,15 @@ static inline void image_untrust ( struct image *image ) { image->flags &= ~IMAGE_TRUSTED; } +/** + * Mark image as hidden + * + * @v image Image + */ +static inline void image_hide ( struct image *image ) { + image->flags |= IMAGE_HIDDEN; +} + /** * Tag image * diff --git a/src/include/usr/shimmgmt.h b/src/include/usr/shimmgmt.h new file mode 100644 index 000000000..5030607ae --- /dev/null +++ b/src/include/usr/shimmgmt.h @@ -0,0 +1,16 @@ +#ifndef _USR_SHIMMGMT_H +#define _USR_SHIMMGMT_H + +/** @file + * + * EFI shim management + * + */ + +FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); + +#include + +extern int shim ( struct image *image, int require_loader, int allow_pxe ); + +#endif /* _USR_SHIMMGMT_H */ diff --git a/src/interface/efi/efi_shim.c b/src/interface/efi/efi_shim.c new file mode 100644 index 000000000..9b1b69e80 --- /dev/null +++ b/src/interface/efi/efi_shim.c @@ -0,0 +1,251 @@ +/* + * Copyright (C) 2022 Michael Brown . + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + * + * You can also choose to distribute this program under the terms of + * the Unmodified Binary Distribution Licence (as given in the file + * COPYING.UBDL), provided that you have satisfied its requirements. + */ + +FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/** @file + * + * UEFI shim special handling + * + */ + +FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); + +/** + * Require use of a third party loader binary + * + * The UEFI shim is gradually becoming less capable of directly + * executing a Linux kernel image, due to an ever increasing list of + * assumptions that it will only ever be used in conjunction with a + * second stage loader binary such as GRUB. + * + * For example: shim will erroneously complain if the image that it + * loads and executes does not in turn call in to the "shim lock + * protocol" to verify a separate newly loaded binary before calling + * ExitBootServices(), even if no such separate binary is used or + * required. + * + * Experience shows that there is unfortunately no point in trying to + * get a fix for this upstreamed into shim. We therefore default to + * reducing the Secure Boot attack surface by removing, where + * possible, this spurious requirement for the use of an additional + * second stage loader. + * + * This option may be used to require the use of an additional second + * stage loader binary, in case this behaviour is ever desirable. + */ +int efi_shim_require_loader = 0; + +/** + * Allow use of PXE base code protocol + * + * We provide shim with access to all of the relevant downloaded files + * via our EFI_SIMPLE_FILE_SYSTEM_PROTOCOL interface. However, shim + * will instead try to redownload the files via TFTP since it prefers + * to use the EFI_PXE_BASE_CODE_PROTOCOL installed on the same handle. + * + * Experience shows that there is unfortunately no point in trying to + * get a fix for this upstreamed into shim. We therefore default to + * working around this undesirable behaviour by stopping the PXE base + * code protocol before invoking shim. + * + * This option may be used to allow shim to use the PXE base code + * protocol, in case this behaviour is ever desirable. + */ +int efi_shim_allow_pxe = 0; + +/** UEFI shim image */ +struct image_tag efi_shim __image_tag = { + .name = "SHIM", +}; + +/** Original GetMemoryMap() function */ +static EFI_GET_MEMORY_MAP efi_shim_orig_map; + +/** + * Unlock UEFI shim + * + * @v len Memory map size + * @v map Memory map + * @v key Memory map key + * @v desclen Descriptor size + * @v descver Descriptor version + * @ret efirc EFI status code + * + */ +static EFIAPI EFI_STATUS efi_shim_unlock ( UINTN *len, + EFI_MEMORY_DESCRIPTOR *map, + UINTN *key, UINTN *desclen, + UINT32 *descver ) { + EFI_BOOT_SERVICES *bs = efi_systab->BootServices; + uint8_t empty[0]; + union { + EFI_SHIM_LOCK_PROTOCOL *lock; + void *interface; + } u; + EFI_STATUS efirc; + + /* Locate shim lock protocol */ + if ( ( efirc = bs->LocateProtocol ( &efi_shim_lock_protocol_guid, + NULL, &u.interface ) ) == 0 ) { + u.lock->Verify ( empty, sizeof ( empty ) ); + DBGC ( &efi_shim, "SHIM unlocked via %p\n", u.lock ); + } + + /* Hand off to original GetMemoryMap() */ + return efi_shim_orig_map ( len, map, key, desclen, descver ); +} + +/** + * Inhibit use of PXE base code + * + * @v handle EFI handle + * @ret rc Return status code + */ +static int efi_shim_inhibit_pxe ( EFI_HANDLE handle ) { + EFI_BOOT_SERVICES *bs = efi_systab->BootServices; + union { + EFI_PXE_BASE_CODE_PROTOCOL *pxe; + void *interface; + } u; + EFI_STATUS efirc; + int rc; + + /* Locate PXE base code */ + if ( ( efirc = bs->OpenProtocol ( handle, + &efi_pxe_base_code_protocol_guid, + &u.interface, efi_image_handle, NULL, + EFI_OPEN_PROTOCOL_GET_PROTOCOL ))!=0){ + rc = -EEFI ( efirc ); + DBGC ( &efi_shim, "SHIM could not open PXE base code: %s\n", + strerror ( rc ) ); + goto err_no_base; + } + + /* Stop PXE base code */ + if ( ( efirc = u.pxe->Stop ( u.pxe ) ) != 0 ) { + rc = -EEFI ( efirc ); + DBGC ( &efi_shim, "SHIM could not stop PXE base code: %s\n", + strerror ( rc ) ); + goto err_stop; + } + + /* Success */ + rc = 0; + DBGC ( &efi_shim, "SHIM stopped PXE base code\n" ); + + err_stop: + bs->CloseProtocol ( handle, &efi_pxe_base_code_protocol_guid, + efi_image_handle, NULL ); + err_no_base: + return rc; +} + +/** + * Update command line + * + * @v shim Shim image + * @v cmdline Command line to update + * @ret rc Return status code + */ +static int efi_shim_cmdline ( struct image *shim, wchar_t **cmdline ) { + wchar_t *shimcmdline; + int len; + int rc; + + /* Construct new command line */ + len = ( shim->cmdline ? + efi_asprintf ( &shimcmdline, "%s %s", shim->name, + shim->cmdline ) : + efi_asprintf ( &shimcmdline, "%s %ls", shim->name, + *cmdline ) ); + if ( len < 0 ) { + rc = len; + DBGC ( &efi_shim, "SHIM could not construct command line: " + "%s\n", strerror ( rc ) ); + return rc; + } + + /* Replace command line */ + free ( *cmdline ); + *cmdline = shimcmdline; + + return 0; +} + +/** + * Install UEFI shim special handling + * + * @v shim Shim image + * @v handle EFI device handle + * @v cmdline Command line to update + * @ret rc Return status code + */ +int efi_shim_install ( struct image *shim, EFI_HANDLE handle, + wchar_t **cmdline ) { + EFI_BOOT_SERVICES *bs = efi_systab->BootServices; + int rc; + + /* Intercept GetMemoryMap() via boot services table */ + efi_shim_orig_map = bs->GetMemoryMap; + if ( ! efi_shim_require_loader ) + bs->GetMemoryMap = efi_shim_unlock; + + /* Stop PXE base code */ + if ( ( ! efi_shim_allow_pxe ) && + ( ( rc = efi_shim_inhibit_pxe ( handle ) ) != 0 ) ) { + goto err_inhibit_pxe; + } + + /* Update command line */ + if ( ( rc = efi_shim_cmdline ( shim, cmdline ) ) != 0 ) + goto err_cmdline; + + return 0; + + err_cmdline: + err_inhibit_pxe: + bs->GetMemoryMap = efi_shim_orig_map; + return rc; +} + +/** + * Uninstall UEFI shim special handling + * + */ +void efi_shim_uninstall ( void ) { + EFI_BOOT_SERVICES *bs = efi_systab->BootServices; + + /* Restore original GetMemoryMap() */ + bs->GetMemoryMap = efi_shim_orig_map; +} diff --git a/src/usr/shimmgmt.c b/src/usr/shimmgmt.c new file mode 100644 index 000000000..ba9c34803 --- /dev/null +++ b/src/usr/shimmgmt.c @@ -0,0 +1,58 @@ +/* + * Copyright (C) 2023 Michael Brown . + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of the + * License, or any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + * + * You can also choose to distribute this program under the terms of + * the Unmodified Binary Distribution Licence (as given in the file + * COPYING.UBDL), provided that you have satisfied its requirements. + */ + +FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); + +#include +#include +#include + +/** @file + * + * EFI shim management + * + */ + +/** + * Set shim image + * + * @v image Shim image, or NULL to clear shim + * @v require_loader Require use of a third party loader + * @v allow_pxe Allow use of PXE base code + * @ret rc Return status code + */ +int shim ( struct image *image, int require_loader, int allow_pxe ) { + + /* Record (or clear) shim image */ + image_tag ( image, &efi_shim ); + + /* Avoid including image in constructed initrd */ + if ( image ) + image_hide ( image ); + + /* Record configuration */ + efi_shim_require_loader = require_loader; + efi_shim_allow_pxe = allow_pxe; + + return 0; +} -- cgit v1.2.3-55-g7522 From 5b4318143648272b36736c1d1d5d1acbda9a5876 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Tue, 23 May 2023 14:55:08 +0100 Subject: [efi] Support versions of shim that perform SBAT verification The UEFI shim implements a fairly nicely designed revocation mechanism designed around the concept of security generations. Unfortunately nobody in the shim community has thus far added the relevant metadata to the Linux kernel, with the result that current versions of shim are incapable of booting current versions of the Linux kernel. Experience shows that there is unfortunately no point in trying to get a fix for this upstreamed into shim. We therefore default to working around this undesirable behaviour by patching data read from the "SbatLevel" variable used to hold SBAT configuration. Signed-off-by: Michael Brown --- src/hci/commands/shim_cmd.c | 7 +- src/include/ipxe/efi/efi_shim.h | 1 + src/include/usr/shimmgmt.h | 3 +- src/interface/efi/efi_shim.c | 150 +++++++++++++++++++++++++++++++++++++++- src/usr/shimmgmt.c | 5 +- 5 files changed, 160 insertions(+), 6 deletions(-) (limited to 'src/usr') diff --git a/src/hci/commands/shim_cmd.c b/src/hci/commands/shim_cmd.c index 00bd0acb1..9150af3fd 100644 --- a/src/hci/commands/shim_cmd.c +++ b/src/hci/commands/shim_cmd.c @@ -44,6 +44,8 @@ struct shim_options { int require_loader; /** Allow PXE base code protocol */ int allow_pxe; + /** Allow SBAT variable access */ + int allow_sbat; }; /** "shim" option list */ @@ -54,6 +56,8 @@ static struct option_descriptor shim_opts[] = { struct shim_options, require_loader, parse_flag ), OPTION_DESC ( "allow-pxe", 'p', no_argument, struct shim_options, allow_pxe, parse_flag ), + OPTION_DESC ( "allow-sbat", 's', no_argument, + struct shim_options, allow_sbat, parse_flag ), }; /** "shim" command descriptor */ @@ -94,7 +98,8 @@ static int shim_exec ( int argc, char **argv ) { } /* (Un)register as shim */ - if ( ( rc = shim ( image, opts.require_loader, opts.allow_pxe ) ) != 0 ) + if ( ( rc = shim ( image, opts.require_loader, opts.allow_pxe, + opts.allow_sbat ) ) != 0 ) goto err_shim; err_shim: diff --git a/src/include/ipxe/efi/efi_shim.h b/src/include/ipxe/efi/efi_shim.h index ad8d24dce..21f24315a 100644 --- a/src/include/ipxe/efi/efi_shim.h +++ b/src/include/ipxe/efi/efi_shim.h @@ -14,6 +14,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); extern int efi_shim_require_loader; extern int efi_shim_allow_pxe; +extern int efi_shim_allow_sbat; extern struct image_tag efi_shim __image_tag; extern int efi_shim_install ( struct image *shim, EFI_HANDLE handle, diff --git a/src/include/usr/shimmgmt.h b/src/include/usr/shimmgmt.h index 5030607ae..0c59f54a8 100644 --- a/src/include/usr/shimmgmt.h +++ b/src/include/usr/shimmgmt.h @@ -11,6 +11,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); #include -extern int shim ( struct image *image, int require_loader, int allow_pxe ); +extern int shim ( struct image *image, int require_loader, int allow_pxe, + int allow_sbat ); #endif /* _USR_SHIMMGMT_H */ diff --git a/src/interface/efi/efi_shim.c b/src/interface/efi/efi_shim.c index 14d4806f2..a46d79d0c 100644 --- a/src/interface/efi/efi_shim.c +++ b/src/interface/efi/efi_shim.c @@ -84,6 +84,26 @@ int efi_shim_require_loader = 0; */ int efi_shim_allow_pxe = 0; +/** + * Allow SBAT variable access + * + * The UEFI shim implements a fairly nicely designed revocation + * mechanism designed around the concept of security generations. + * Unfortunately nobody in the shim community has thus far added the + * relevant metadata to the Linux kernel, with the result that current + * versions of shim are incapable of booting current versions of the + * Linux kernel. + * + * Experience shows that there is unfortunately no point in trying to + * get a fix for this upstreamed into shim. We therefore default to + * working around this undesirable behaviour by patching data read + * from the "SbatLevel" variable used to hold SBAT configuration. + * + * This option may be used to allow shim unpatched access to the + * "SbatLevel" variable, in case this behaviour is ever desirable. + */ +int efi_shim_allow_sbat = 0; + /** UEFI shim image */ struct image_tag efi_shim __image_tag = { .name = "SHIM", @@ -92,6 +112,33 @@ struct image_tag efi_shim __image_tag = { /** Original GetMemoryMap() function */ static EFI_GET_MEMORY_MAP efi_shim_orig_get_memory_map; +/** Original ExitBootServices() function */ +static EFI_EXIT_BOOT_SERVICES efi_shim_orig_exit_boot_services; + +/** Original SetVariable() function */ +static EFI_SET_VARIABLE efi_shim_orig_set_variable; + +/** Original GetVariable() function */ +static EFI_GET_VARIABLE efi_shim_orig_get_variable; + +/** Verify read from SbatLevel variable */ +static int efi_shim_sbatlevel_verify; + +/** + * Check if variable is SbatLevel + * + * @v name Variable name + * @v guid Variable namespace GUID + * @ret is_sbatlevel Variable is SbatLevel + */ +static int efi_shim_is_sbatlevel ( const CHAR16 *name, const EFI_GUID *guid ) { + static CHAR16 sbatlevel[] = L"SbatLevel"; + EFI_GUID *shimlock = &efi_shim_lock_protocol_guid; + + return ( ( memcmp ( name, sbatlevel, sizeof ( sbatlevel ) ) == 0 ) && + ( memcmp ( guid, shimlock, sizeof ( *shimlock ) ) == 0 ) ); +} + /** * Unlock UEFI shim * @@ -137,6 +184,92 @@ static EFIAPI EFI_STATUS efi_shim_get_memory_map ( UINTN *len, descver ); } +/** + * Wrap ExitBootServices() + * + * @v handle Image handle + * @v key Memory map key + * @ret efirc EFI status code + */ +static EFIAPI EFI_STATUS efi_shim_exit_boot_services ( EFI_HANDLE handle, + UINTN key ) { + EFI_RUNTIME_SERVICES *rs = efi_systab->RuntimeServices; + + /* Restore original runtime services functions */ + rs->SetVariable = efi_shim_orig_set_variable; + rs->GetVariable = efi_shim_orig_get_variable; + + /* Hand off to original ExitBootServices() */ + return efi_shim_orig_exit_boot_services ( handle, key ); +} + +/** + * Wrap SetVariable() + * + * @v name Variable name + * @v guid Variable namespace GUID + * @v attrs Attributes + * @v len Buffer size + * @v data Data buffer + * @ret efirc EFI status code + */ +static EFI_STATUS EFIAPI +efi_shim_set_variable ( CHAR16 *name, EFI_GUID *guid, UINT32 attrs, + UINTN len, VOID *data ) { + EFI_STATUS efirc; + + /* Call original SetVariable() */ + efirc = efi_shim_orig_set_variable ( name, guid, attrs, len, data ); + + /* Allow verification of SbatLevel variable content */ + if ( efi_shim_is_sbatlevel ( name, guid ) && ( efirc == 0 ) ) { + DBGC ( &efi_shim, "SHIM detected write to %ls:\n", name ); + DBGC_HDA ( &efi_shim, 0, data, len ); + efi_shim_sbatlevel_verify = 1; + } + + return efirc; +} + +/** + * Wrap GetVariable() + * + * @v name Variable name + * @v guid Variable namespace GUID + * @v attrs Attributes to fill in + * @v len Buffer size + * @v data Data buffer + * @ret efirc EFI status code + */ +static EFI_STATUS EFIAPI +efi_shim_get_variable ( CHAR16 *name, EFI_GUID *guid, UINT32 *attrs, + UINTN *len, VOID *data ) { + char *value = data; + EFI_STATUS efirc; + + /* Call original GetVariable() */ + efirc = efi_shim_orig_get_variable ( name, guid, attrs, len, data ); + + /* Patch SbatLevel variable if applicable */ + if ( efi_shim_is_sbatlevel ( name, guid ) && data && ( efirc == 0 ) ) { + if ( efi_shim_allow_sbat ) { + DBGC ( &efi_shim, "SHIM allowing read from %ls:\n", + name ); + } else if ( efi_shim_sbatlevel_verify ) { + DBGC ( &efi_shim, "SHIM allowing one read from %ls:\n", + name ); + efi_shim_sbatlevel_verify = 0; + } else { + DBGC ( &efi_shim, "SHIM patching read from %ls:\n", + name ); + value[0] = '\0'; + } + DBGC_HDA ( &efi_shim, 0, data, *len ); + } + + return efirc; +} + /** * Inhibit use of PXE base code * @@ -225,6 +358,7 @@ static int efi_shim_cmdline ( struct image *shim, wchar_t **cmdline ) { int efi_shim_install ( struct image *shim, EFI_HANDLE handle, wchar_t **cmdline ) { EFI_BOOT_SERVICES *bs = efi_systab->BootServices; + EFI_RUNTIME_SERVICES *rs = efi_systab->RuntimeServices; int rc; /* Stop PXE base code */ @@ -237,11 +371,17 @@ int efi_shim_install ( struct image *shim, EFI_HANDLE handle, if ( ( rc = efi_shim_cmdline ( shim, cmdline ) ) != 0 ) return rc; - /* Record original boot services functions */ + /* Record original boot and runtime services functions */ efi_shim_orig_get_memory_map = bs->GetMemoryMap; + efi_shim_orig_exit_boot_services = bs->ExitBootServices; + efi_shim_orig_set_variable = rs->SetVariable; + efi_shim_orig_get_variable = rs->GetVariable; - /* Wrap relevant boot services functions */ + /* Wrap relevant boot and runtime services functions */ bs->GetMemoryMap = efi_shim_get_memory_map; + bs->ExitBootServices = efi_shim_exit_boot_services; + rs->SetVariable = efi_shim_set_variable; + rs->GetVariable = efi_shim_get_variable; return 0; } @@ -252,7 +392,11 @@ int efi_shim_install ( struct image *shim, EFI_HANDLE handle, */ void efi_shim_uninstall ( void ) { EFI_BOOT_SERVICES *bs = efi_systab->BootServices; + EFI_RUNTIME_SERVICES *rs = efi_systab->RuntimeServices; - /* Restore original boot services functions */ + /* Restore original boot and runtime services functions */ bs->GetMemoryMap = efi_shim_orig_get_memory_map; + bs->ExitBootServices = efi_shim_orig_exit_boot_services; + rs->SetVariable = efi_shim_orig_set_variable; + rs->GetVariable = efi_shim_orig_get_variable; } diff --git a/src/usr/shimmgmt.c b/src/usr/shimmgmt.c index ba9c34803..6ac1ac35e 100644 --- a/src/usr/shimmgmt.c +++ b/src/usr/shimmgmt.c @@ -39,9 +39,11 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); * @v image Shim image, or NULL to clear shim * @v require_loader Require use of a third party loader * @v allow_pxe Allow use of PXE base code + * @v allow_sbat Allow SBAT variable access * @ret rc Return status code */ -int shim ( struct image *image, int require_loader, int allow_pxe ) { +int shim ( struct image *image, int require_loader, int allow_pxe, + int allow_sbat ) { /* Record (or clear) shim image */ image_tag ( image, &efi_shim ); @@ -53,6 +55,7 @@ int shim ( struct image *image, int require_loader, int allow_pxe ) { /* Record configuration */ efi_shim_require_loader = require_loader; efi_shim_allow_pxe = allow_pxe; + efi_shim_allow_sbat = allow_sbat; return 0; } -- cgit v1.2.3-55-g7522