boot: Rework shim image verification

This moves the shim security arch override to the new
ReinstallProtocolInterface based interface. This also has the benefit to
reduce the time window in which we have this override active and also
actually removes it, which was not previously done.

The shim hooks themselves are also modernized too. The upcalls should
really not be neccessary if shim is happy with the provided binary.

3 files changed, 44 insertions, 84 deletions

diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c
index e1a5e861c5..4150b16ecf 100644
--- a/src/boot/efi/boot.c
+++ b/src/boot/efi/boot.c
@@ -2379,7 +2379,7 @@ static EFI_STATUS image_start(
         if (err != EFI_SUCCESS)
                 return log_error_status_stall(err, L"Error preparing initrd: %r", err);
 
-        err = BS->LoadImage(false, parent_image, path, NULL, 0, &image);
+        err = shim_load_image(parent_image, path, &image);
         if (err != EFI_SUCCESS)
                 return log_error_status_stall(err, L"Error loading %s: %r", entry->loader, err);
 
@@ -2686,12 +2686,6 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) {
         if (err != EFI_SUCCESS)
                 return log_error_status_stall(err, L"Unable to open root directory: %r", err);
 
-        if (secure_boot_enabled() && shim_loaded()) {
-                err = security_policy_install();
-                if (err != EFI_SUCCESS)
-                        return log_error_status_stall(err, L"Error installing security policy: %r", err);
-        }
-
         (void) load_drivers(image, loaded_image, root_dir);
 
         config_load_all_entries(&config, loaded_image, loaded_image_path, root_dir);
diff --git a/src/boot/efi/shim.c b/src/boot/efi/shim.c
index 79c89c067e..3ae058cb84 100644
--- a/src/boot/efi/shim.c
+++ b/src/boot/efi/shim.c
@@ -13,6 +13,7 @@
 
 #include "missing_efi.h"
 #include "util.h"
+#include "secure-boot.h"
 #include "shim.h"
 
 #if defined(__x86_64__) || defined(__i386__)
@@ -55,121 +56,86 @@ static bool shim_validate(void *data, uint32_t size) {
         return shim_lock->shim_verify(data, size) == EFI_SUCCESS;
 }
 
-/* Handle to the original authenticator for security1 protocol */
-static EFI_SECURITY_FILE_AUTHENTICATION_STATE esfas = NULL;
-
-/* Handle to the original authenticator for security2 protocol */
-static EFI_SECURITY2_FILE_AUTHENTICATION es2fa = NULL;
-
-/*
- * Perform shim/MOK and Secure Boot authentication on a binary that's already been
- * loaded into memory. This function does the platform SB authentication first
- * but preserves its return value in case of its failure, so that it can be
- * returned in case of a shim/MOK authentication failure. This is done because
- * the SB failure code seems to vary from one implementation to another, and I
- * don't want to interfere with that at this time.
- */
-static EFIAPI EFI_STATUS security2_policy_authentication(
-                const EFI_SECURITY2_ARCH_PROTOCOL *this,
+static EFIAPI EFI_STATUS security2_hook(
+                const SecurityOverride *this,
                 const EFI_DEVICE_PATH *device_path,
                 void *file_buffer,
                 UINTN file_size,
                 BOOLEAN boot_policy) {
-        EFI_STATUS err;
 
         assert(this);
-        /* device_path and file_buffer may be NULL */
-
-        /* Chain original security policy */
-        err = es2fa(this, device_path, file_buffer, file_size, boot_policy);
-
-        /* if OK, don't bother with MOK check */
-        if (err == EFI_SUCCESS)
-                return err;
+        assert(this->hook == security2_hook);
 
         if (shim_validate(file_buffer, file_size))
                 return EFI_SUCCESS;
 
-        return err;
+        return this->original_security2->FileAuthentication(
+                        this->original_security2, device_path, file_buffer, file_size, boot_policy);
 }
 
-/*
- * Perform both shim/MOK and platform Secure Boot authentication. This function loads
- * the file and performs shim/MOK authentication first simply to avoid double loads
- * of Linux kernels, which are much more likely to be shim/MOK-signed than platform-signed,
- * since kernels are big and can take several seconds to load on some computers and
- * filesystems. This also has the effect of returning whatever the platform code is for
- * authentication failure, be it EFI_ACCESS_DENIED, EFI_SECURITY_VIOLATION, or something
- * else. (This seems to vary between implementations.)
- */
-static EFIAPI EFI_STATUS security_policy_authentication(
-                const EFI_SECURITY_ARCH_PROTOCOL *this,
+static EFIAPI EFI_STATUS security_hook(
+                const SecurityOverride *this,
                 uint32_t authentication_status,
-                const EFI_DEVICE_PATH *device_path_const) {
+                const EFI_DEVICE_PATH *device_path) {
+
         EFI_STATUS err;
-        _cleanup_free_ char16_t *dev_path_str = NULL;
-        EFI_HANDLE h;
-        _cleanup_free_ char *file_buffer = NULL;
-        UINTN file_size;
 
         assert(this);
+        assert(this->hook == security_hook);
 
-        if (!device_path_const)
-                return EFI_INVALID_PARAMETER;
+        if (!device_path)
+                return this->original_security->FileAuthenticationState(
+                                this->original_security, authentication_status, device_path);
 
-        EFI_DEVICE_PATH *dp = (EFI_DEVICE_PATH *) device_path_const;
-        err = BS->LocateDevicePath(&FileSystemProtocol, &dp, &h);
+        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(h, &root);
+        err = open_volume(device_handle, &root);
         if (err != EFI_SUCCESS)
                 return err;
 
-        err = device_path_to_str(dp, &dev_path_str);
+        _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, dev_path_str, 0, 0, &file_buffer, &file_size);
+        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;
 
         if (shim_validate(file_buffer, file_size))
                 return EFI_SUCCESS;
 
-        /* Try using the platform's native policy.... */
-        return esfas(this, authentication_status, device_path_const);
+        return this->original_security->FileAuthenticationState(
+                        this->original_security, authentication_status, device_path);
 }
 
-EFI_STATUS security_policy_install(void) {
-        EFI_SECURITY_ARCH_PROTOCOL *security_protocol;
-        EFI_SECURITY2_ARCH_PROTOCOL *security2_protocol = NULL;
-        EFI_STATUS err;
+EFI_STATUS shim_load_image(EFI_HANDLE parent, const EFI_DEVICE_PATH *device_path, EFI_HANDLE *ret_image) {
+        assert(device_path);
+        assert(ret_image);
 
-        /* Already Installed */
-        if (esfas)
-                return EFI_ALREADY_STARTED;
+        bool have_shim = shim_loaded();
 
-        /*
-         * Don't bother with status here. The call is allowed
-         * to fail, since SECURITY2 was introduced in PI 1.2.1.
-         * Use security2_protocol == NULL as indicator.
-         */
-        BS->LocateProtocol(&(EFI_GUID) EFI_SECURITY2_ARCH_PROTOCOL_GUID, NULL, (void **) &security2_protocol);
+        SecurityOverride security_override = {
+                .hook = security_hook,
+        }, security2_override = {
+                .hook = security2_hook,
+        };
 
-        err = BS->LocateProtocol(&(EFI_GUID) EFI_SECURITY_ARCH_PROTOCOL_GUID, NULL, (void**) &security_protocol);
-         /* This one is mandatory, so there's a serious problem */
-        if (err != EFI_SUCCESS)
-                return err;
+        if (have_shim)
+                install_security_override(&security_override, &security2_override);
 
-        esfas = security_protocol->FileAuthenticationState;
-        security_protocol->FileAuthenticationState = security_policy_authentication;
+        EFI_STATUS ret = BS->LoadImage(
+                        /*BootPolicy=*/false, parent, (EFI_DEVICE_PATH *) device_path, NULL, 0, ret_image);
 
-        if (security2_protocol) {
-                es2fa = security2_protocol->FileAuthentication;
-                security2_protocol->FileAuthentication = security2_policy_authentication;
-        }
+        if (have_shim)
+                uninstall_security_override(&security_override, &security2_override);
 
-        return EFI_SUCCESS;
+        return ret;
 }
diff --git a/src/boot/efi/shim.h b/src/boot/efi/shim.h
index ddbc88c52c..6d213f5efa 100644
--- a/src/boot/efi/shim.h
+++ b/src/boot/efi/shim.h
@@ -10,7 +10,7 @@
 #pragma once
 
 #include <efi.h>
+#include <stdbool.h>
 
 bool shim_loaded(void);
-
-EFI_STATUS security_policy_install(void);
+EFI_STATUS shim_load_image(EFI_HANDLE parent, const EFI_DEVICE_PATH *device_path, EFI_HANDLE *ret_image);