From 519625977d19b7842d9b2ded8be12ed0aecbaefc Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Tue, 15 Nov 2022 18:22:38 +0100 Subject: [PATCH] boot: Rework security arch override This simplifies the caller interface for security arch overrides by only having to pass a validator and an optional context. (cherry picked from commit 5489c13bae119dc5f6e65be8d7f241aa7d54c023) Related: #2138081 --- src/boot/efi/linux.c | 61 ++++++++------------- src/boot/efi/secure-boot.c | 105 +++++++++++++++++++++++++++++-------- src/boot/efi/secure-boot.h | 28 +++------- src/boot/efi/shim.c | 104 +++++++++++------------------------- 4 files changed, 146 insertions(+), 152 deletions(-) diff --git a/src/boot/efi/linux.c b/src/boot/efi/linux.c index 75b9507709..dd7eb48c8c 100644 --- a/src/boot/efi/linux.c +++ b/src/boot/efi/linux.c @@ -20,35 +20,26 @@ #define STUB_PAYLOAD_GUID \ { 0x55c5d1f8, 0x04cd, 0x46b5, { 0x8a, 0x20, 0xe5, 0x6c, 0xbb, 0x30, 0x52, 0xd0 } } -static EFIAPI EFI_STATUS security_hook( - const SecurityOverride *this, uint32_t authentication_status, const EFI_DEVICE_PATH *file) { +typedef struct { + const void *addr; + size_t len; + const EFI_DEVICE_PATH *device_path; +} ValidationContext; - assert(this); - assert(this->hook == security_hook); +static bool validate_payload( + const void *ctx, const EFI_DEVICE_PATH *device_path, const void *file_buffer, size_t file_size) { - if (file == this->payload_device_path) - return EFI_SUCCESS; + const ValidationContext *payload = ASSERT_PTR(ctx); - return this->original_security->FileAuthenticationState( - this->original_security, authentication_status, file); -} - -static EFIAPI EFI_STATUS security2_hook( - const SecurityOverride *this, - const EFI_DEVICE_PATH *device_path, - void *file_buffer, - size_t file_size, - BOOLEAN boot_policy) { - - assert(this); - assert(this->hook == security2_hook); + if (device_path != payload->device_path) + return false; - if (file_buffer == this->payload && file_size == this->payload_len && - device_path == this->payload_device_path) - return EFI_SUCCESS; + /* Security arch (1) protocol does not provide a file buffer. Instead we are supposed to fetch the payload + * ourselves, which is not needed as we already have everything in memory and the device paths match. */ + if (file_buffer && (file_buffer != payload->addr || file_size != payload->len)) + return false; - return this->original_security2->FileAuthentication( - this->original_security2, device_path, file_buffer, file_size, boot_policy); + return true; } static EFI_STATUS load_image(EFI_HANDLE parent, const void *source, size_t len, EFI_HANDLE *ret_image) { @@ -79,19 +70,13 @@ static EFI_STATUS load_image(EFI_HANDLE parent, const void *source, size_t len, /* We want to support unsigned kernel images as payload, which is safe to do under secure boot * because it is embedded in this stub loader (and since it is already running it must be trusted). */ - SecurityOverride security_override = { - .hook = security_hook, - .payload = source, - .payload_len = len, - .payload_device_path = &payload_device_path.payload.Header, - }, security2_override = { - .hook = security2_hook, - .payload = source, - .payload_len = len, - .payload_device_path = &payload_device_path.payload.Header, - }; - - install_security_override(&security_override, &security2_override); + install_security_override( + validate_payload, + &(ValidationContext) { + .addr = source, + .len = len, + .device_path = &payload_device_path.payload.Header, + }); EFI_STATUS ret = BS->LoadImage( /*BootPolicy=*/false, @@ -101,7 +86,7 @@ static EFI_STATUS load_image(EFI_HANDLE parent, const void *source, size_t len, len, ret_image); - uninstall_security_override(&security_override, &security2_override); + uninstall_security_override(); return ret; } diff --git a/src/boot/efi/secure-boot.c b/src/boot/efi/secure-boot.c index 171b2c96b3..0e615c55e0 100644 --- a/src/boot/efi/secure-boot.c +++ b/src/boot/efi/secure-boot.c @@ -127,10 +127,60 @@ out_deallocate: return err; } -static EFI_STATUS install_security_override_one(EFI_GUID guid, SecurityOverride *override) { +static struct SecurityOverride { + /* Our own security arch instances that we register onto original_handle, thereby replacing the + * firmware provided instances. */ + EFI_SECURITY_ARCH_PROTOCOL override; + EFI_SECURITY2_ARCH_PROTOCOL override2; + + /* These are saved so we can uninstall our own instance later. */ + EFI_HANDLE original_handle, original_handle2; + EFI_SECURITY_ARCH_PROTOCOL *original_security; + EFI_SECURITY2_ARCH_PROTOCOL *original_security2; + + security_validator_t validator; + const void *validator_ctx; +} security_override; + +static EFIAPI EFI_STATUS security_hook( + const EFI_SECURITY_ARCH_PROTOCOL *this, + uint32_t authentication_status, + const EFI_DEVICE_PATH *file) { + + assert(security_override.validator); + assert(security_override.original_security); + + if (security_override.validator(security_override.validator_ctx, file, NULL, 0)) + return EFI_SUCCESS; + + return security_override.original_security->FileAuthenticationState( + security_override.original_security, authentication_status, file); +} + +static EFIAPI EFI_STATUS security2_hook( + const EFI_SECURITY2_ARCH_PROTOCOL *this, + const EFI_DEVICE_PATH *device_path, + void *file_buffer, + size_t file_size, + BOOLEAN boot_policy) { + + assert(security_override.validator); + assert(security_override.original_security2); + + if (security_override.validator(security_override.validator_ctx, device_path, file_buffer, file_size)) + return EFI_SUCCESS; + + return security_override.original_security2->FileAuthentication( + security_override.original_security2, device_path, file_buffer, file_size, boot_policy); +} + +static EFI_STATUS install_security_override_one( + EFI_GUID guid, void *override, EFI_HANDLE *ret_original_handle, void **ret_original_security) { EFI_STATUS err; assert(override); + assert(ret_original_handle); + assert(ret_original_security); _cleanup_free_ EFI_HANDLE *handles = NULL; size_t n_handles = 0; @@ -152,8 +202,8 @@ static EFI_STATUS install_security_override_one(EFI_GUID guid, SecurityOverride if (err != EFI_SUCCESS) return log_error_status_stall(err, u"Error overriding security arch protocol: %r", err); - override->original = security; - override->original_handle = handles[0]; + *ret_original_security = security; + *ret_original_handle = handles[0]; return EFI_SUCCESS; } @@ -161,35 +211,46 @@ static EFI_STATUS install_security_override_one(EFI_GUID guid, SecurityOverride * Specification) with the provided override instances. If not running in secure boot or the protocols are * not available nothing happens. The override instances are provided with the necessary info to undo this * in uninstall_security_override(). */ -void install_security_override(SecurityOverride *override, SecurityOverride *override2) { - assert(override); - assert(override2); +void install_security_override(security_validator_t validator, const void *validator_ctx) { + assert(validator); if (!secure_boot_enabled()) return; - (void) install_security_override_one((EFI_GUID) EFI_SECURITY_ARCH_PROTOCOL_GUID, override); - (void) install_security_override_one((EFI_GUID) EFI_SECURITY2_ARCH_PROTOCOL_GUID, override2); -} + security_override = (struct SecurityOverride) { + { .FileAuthenticationState = security_hook, }, + { .FileAuthentication = security2_hook, }, + .validator = validator, + .validator_ctx = validator_ctx, + }; -void uninstall_security_override(SecurityOverride *override, SecurityOverride *override2) { - assert(override); - assert(override2); + (void) install_security_override_one( + (EFI_GUID) EFI_SECURITY_ARCH_PROTOCOL_GUID, + &security_override.override, + &security_override.original_handle, + (void **) &security_override.original_security); + (void) install_security_override_one( + (EFI_GUID) EFI_SECURITY2_ARCH_PROTOCOL_GUID, + &security_override.override2, + &security_override.original_handle2, + (void **) &security_override.original_security2); +} +void uninstall_security_override(void) { /* We use assert_se here to guarantee the system is not in a weird state in the unlikely case of an * error restoring the original protocols. */ - if (override->original_handle) + if (security_override.original_handle) assert_se(BS->ReinstallProtocolInterface( - override->original_handle, - &(EFI_GUID) EFI_SECURITY_ARCH_PROTOCOL_GUID, - override, - override->original) == EFI_SUCCESS); + security_override.original_handle, + &(EFI_GUID) EFI_SECURITY_ARCH_PROTOCOL_GUID, + &security_override.override, + security_override.original_security) == EFI_SUCCESS); - if (override2->original_handle) + if (security_override.original_handle2) assert_se(BS->ReinstallProtocolInterface( - override2->original_handle, - &(EFI_GUID) EFI_SECURITY2_ARCH_PROTOCOL_GUID, - override2, - override2->original) == EFI_SUCCESS); + security_override.original_handle2, + &(EFI_GUID) EFI_SECURITY2_ARCH_PROTOCOL_GUID, + &security_override.override2, + security_override.original_security2) == EFI_SUCCESS); } diff --git a/src/boot/efi/secure-boot.h b/src/boot/efi/secure-boot.h index 91b6770edb..e98de81c2a 100644 --- a/src/boot/efi/secure-boot.h +++ b/src/boot/efi/secure-boot.h @@ -17,23 +17,11 @@ SecureBootMode secure_boot_mode(void); EFI_STATUS secure_boot_enroll_at(EFI_FILE *root_dir, const char16_t *path); -typedef struct { - void *hook; - - /* End of EFI_SECURITY_ARCH(2)_PROTOCOL. The rest is our own protocol instance data. */ - - EFI_HANDLE original_handle; - union { - void *original; - EFI_SECURITY_ARCH_PROTOCOL *original_security; - EFI_SECURITY2_ARCH_PROTOCOL *original_security2; - }; - - /* Used by the stub to identify the embedded image. */ - const void *payload; - size_t payload_len; - const EFI_DEVICE_PATH *payload_device_path; -} SecurityOverride; - -void install_security_override(SecurityOverride *override, SecurityOverride *override2); -void uninstall_security_override(SecurityOverride *override, SecurityOverride *override2); +typedef bool (*security_validator_t)( + const void *ctx, + const EFI_DEVICE_PATH *device_path, + const void *file_buffer, + size_t file_size); + +void install_security_override(security_validator_t validator, const void *validator_ctx); +void uninstall_security_override(void); diff --git a/src/boot/efi/shim.c b/src/boot/efi/shim.c index 3ae058cb84..ac224336bc 100644 --- a/src/boot/efi/shim.c +++ b/src/boot/efi/shim.c @@ -23,7 +23,7 @@ #endif struct ShimLock { - EFI_STATUS __sysv_abi__ (*shim_verify) (void *buffer, uint32_t size); + EFI_STATUS __sysv_abi__ (*shim_verify) (const void *buffer, uint32_t size); /* context is actually a struct for the PE header, but it isn't needed so void is sufficient just do define the interface * see shim.c/shim.h and PeHeader.h in the github shim repo */ @@ -41,79 +41,45 @@ bool shim_loaded(void) { return BS->LocateProtocol((EFI_GUID*) SHIM_LOCK_GUID, NULL, (void**) &shim_lock) == EFI_SUCCESS; } -static bool shim_validate(void *data, uint32_t size) { - struct ShimLock *shim_lock; - - if (!data) - return false; - - if (BS->LocateProtocol((EFI_GUID*) SHIM_LOCK_GUID, NULL, (void**) &shim_lock) != EFI_SUCCESS) - return false; - - if (!shim_lock) - return false; - - return shim_lock->shim_verify(data, size) == EFI_SUCCESS; -} - -static EFIAPI EFI_STATUS security2_hook( - const SecurityOverride *this, - const EFI_DEVICE_PATH *device_path, - void *file_buffer, - UINTN file_size, - BOOLEAN boot_policy) { - - assert(this); - assert(this->hook == security2_hook); - - if (shim_validate(file_buffer, file_size)) - return EFI_SUCCESS; - - return this->original_security2->FileAuthentication( - this->original_security2, device_path, file_buffer, file_size, boot_policy); -} - -static EFIAPI EFI_STATUS security_hook( - const SecurityOverride *this, - uint32_t authentication_status, - const EFI_DEVICE_PATH *device_path) { +static bool shim_validate( + const void *ctx, const EFI_DEVICE_PATH *device_path, const void *file_buffer, size_t file_size) { EFI_STATUS err; + _cleanup_free_ char *file_buffer_owned = NULL; - assert(this); - assert(this->hook == security_hook); + if (!file_buffer) { + if (!device_path) + return false; - if (!device_path) - return this->original_security->FileAuthenticationState( - this->original_security, authentication_status, device_path); + EFI_HANDLE device_handle; + EFI_DEVICE_PATH *file_dp = (EFI_DEVICE_PATH *) device_path; + err = BS->LocateDevicePath(&FileSystemProtocol, &file_dp, &device_handle); + if (err != EFI_SUCCESS) + return false; - EFI_HANDLE device_handle; - EFI_DEVICE_PATH *dp = (EFI_DEVICE_PATH *) device_path; - err = BS->LocateDevicePath(&FileSystemProtocol, &dp, &device_handle); - if (err != EFI_SUCCESS) - return err; + _cleanup_(file_closep) EFI_FILE *root = NULL; + err = open_volume(device_handle, &root); + if (err != EFI_SUCCESS) + return false; - _cleanup_(file_closep) EFI_FILE *root = NULL; - err = open_volume(device_handle, &root); - if (err != EFI_SUCCESS) - return err; + _cleanup_free_ char16_t *dp_str = NULL; + err = device_path_to_str(file_dp, &dp_str); + if (err != EFI_SUCCESS) + return false; - _cleanup_free_ char16_t *dp_str = NULL; - err = device_path_to_str(dp, &dp_str); - if (err != EFI_SUCCESS) - return err; + err = file_read(root, dp_str, 0, 0, &file_buffer_owned, &file_size); + if (err != EFI_SUCCESS) + return false; - char *file_buffer; - size_t file_size; - err = file_read(root, dp_str, 0, 0, &file_buffer, &file_size); - if (err != EFI_SUCCESS) - return err; + file_buffer = file_buffer_owned; + } - if (shim_validate(file_buffer, file_size)) - return EFI_SUCCESS; + struct ShimLock *shim_lock; + err = BS->LocateProtocol((EFI_GUID *) SHIM_LOCK_GUID, NULL, (void **) &shim_lock); + if (err != EFI_SUCCESS) + return false; - return this->original_security->FileAuthenticationState( - this->original_security, authentication_status, device_path); + return shim_lock->shim_verify(file_buffer, file_size) == EFI_SUCCESS; } EFI_STATUS shim_load_image(EFI_HANDLE parent, const EFI_DEVICE_PATH *device_path, EFI_HANDLE *ret_image) { @@ -122,20 +88,14 @@ EFI_STATUS shim_load_image(EFI_HANDLE parent, const EFI_DEVICE_PATH *device_path bool have_shim = shim_loaded(); - SecurityOverride security_override = { - .hook = security_hook, - }, security2_override = { - .hook = security2_hook, - }; - if (have_shim) - install_security_override(&security_override, &security2_override); + install_security_override(shim_validate, NULL); EFI_STATUS ret = BS->LoadImage( /*BootPolicy=*/false, parent, (EFI_DEVICE_PATH *) device_path, NULL, 0, ret_image); if (have_shim) - uninstall_security_override(&security_override, &security2_override); + uninstall_security_override(); return ret; }