fix(shim): let the firmware decide on images the shim can't verify

The security hooks returned their own errors for calls the shim can't
verify, instead of passing them to the original firmware hooks. The
Security2 hook rejected a missing device path or file buffer, which are
optional in the PI specification, and a boot policy of TRUE. The
Security hook returned NOT_FOUND for any file it couldn't read, like
one in a firmware volume.

Pass these calls to the original hook, which is what the firmware does
without the hook installed. This matches systemd-boot, and the
firmware still enforces its own verification, so nothing is let
through that the firmware would reject.

crates/eficore/src/shim/hook.rs

@@ -82,33 +82,13 @@ impl SecurityHook {
         status.is_success()
     }
 
-    /// File authentication state verifier for the EFI_SECURITY_ARCH protocol.
-    /// Takes the `path` and determines the verification.
-    unsafe extern "efiapi" fn arch_file_authentication_state(
+    /// Call the original EFI_SECURITY_ARCH hook with `this`, `status`, and `path`.
+    /// This lets the firmware decide on images that the shim can't verify.
+    unsafe fn original_file_authentication_state(
         this: *const SecurityArchProtocol,
         status: u32,
         path: *const FfiDevicePath,
     ) -> Status {
-        // Verify the path is not null.
-        if path.is_null() {
-            return Status::INVALID_PARAMETER;
-        }
-
-        // Construct a shim input from the path.
-        let input = ShimInput::SecurityHookPath(path);
-
-        // Convert the input to an owned data buffer.
-        let input = match input.into_owned_data_buffer() {
-            Ok(input) => input,
-            // If an error occurs, log the error and return the not found status.
-            Err(error) => {
-                warn!("unable to read data to be authenticated: {}", error);
-                return Status::NOT_FOUND;
-            }
-        };
-
-        // Verify the input, if it fails, call the original hook.
-        if !Self::verify(input) {
         // Acquire the global hook state to grab the original hook.
         let function = match GLOBAL_HOOK_STATE.lock().as_ref() {
             // The hook state is available, so we can acquire the original hook.
@@ -125,38 +105,18 @@ impl SecurityHook {
         // SAFETY: This function is safe to call as it is stored by us and is required
         // in the UEFI specification.
         unsafe { function(this, status, path) }
-        } else {
-            Status::SUCCESS
-        }
     }
 
-    /// File authentication verifier for the EFI_SECURITY_ARCH2 protocol.
-    /// Takes the `path` and a file buffer to determine the verification.
-    unsafe extern "efiapi" fn arch2_file_authentication(
+    /// Call the original EFI_SECURITY_ARCH2 hook with `this`, `path`, `file_buffer`,
+    /// `file_size`, and `boot_policy`.
+    /// This lets the firmware decide on images that the shim can't verify.
+    unsafe fn original_file_authentication(
         this: *const SecurityArch2Protocol,
         path: *const FfiDevicePath,
         file_buffer: *const u8,
         file_size: usize,
         boot_policy: Boolean,
     ) -> Status {
-        // Verify the path and file buffer are not null.
-        if path.is_null() || file_buffer.is_null() {
-            return Status::INVALID_PARAMETER;
-        }
-
-        // If the boot policy is true, we can't continue as we don't support that.
-        if bool::from(boot_policy) {
-            return Status::INVALID_PARAMETER;
-        }
-
-        // Construct a slice out of the file buffer and size.
-        let buffer = unsafe { slice::from_raw_parts(file_buffer, file_size) };
-
-        // Construct a shim input from the path.
-        let input = ShimInput::SecurityHookBuffer(Some(path), buffer);
-
-        // Verify the input, if it fails, call the original hook.
-        if !Self::verify(input) {
         // Acquire the global hook state to grab the original hook.
         let function = match GLOBAL_HOOK_STATE.lock().as_ref() {
             // The hook state is available, so we can acquire the original hook.
@@ -173,6 +133,77 @@ impl SecurityHook {
         // SAFETY: This function is safe to call as it is stored by us and is required
         // in the UEFI specification.
         unsafe { function(this, path, file_buffer, file_size, boot_policy) }
+    }
+
+    /// File authentication state verifier for the EFI_SECURITY_ARCH protocol.
+    /// Takes the `path` and determines the verification.
+    /// Anything the shim can't verify is passed to the original hook.
+    unsafe extern "efiapi" fn arch_file_authentication_state(
+        this: *const SecurityArchProtocol,
+        status: u32,
+        path: *const FfiDevicePath,
+    ) -> Status {
+        // Without a path, there is nothing for the shim to read and verify.
+        if path.is_null() {
+            // SAFETY: The arguments are passed through unchanged from the firmware.
+            return unsafe { Self::original_file_authentication_state(this, status, path) };
+        }
+
+        // Construct a shim input from the path.
+        let input = ShimInput::SecurityHookPath(path);
+
+        // Convert the input to an owned data buffer.
+        let input = match input.into_owned_data_buffer() {
+            Ok(input) => input,
+            // If the data can't be read, such as for a path the shim can't access,
+            // let the original hook decide.
+            Err(error) => {
+                warn!("unable to read data to be authenticated: {}", error);
+                // SAFETY: The arguments are passed through unchanged from the firmware.
+                return unsafe { Self::original_file_authentication_state(this, status, path) };
+            }
+        };
+
+        // Verify the input, if it fails, call the original hook.
+        if !Self::verify(input) {
+            // SAFETY: The arguments are passed through unchanged from the firmware.
+            unsafe { Self::original_file_authentication_state(this, status, path) }
+        } else {
+            Status::SUCCESS
+        }
+    }
+
+    /// File authentication verifier for the EFI_SECURITY_ARCH2 protocol.
+    /// Takes the `path` and a file buffer to determine the verification.
+    /// Anything the shim can't verify is passed to the original hook.
+    unsafe extern "efiapi" fn arch2_file_authentication(
+        this: *const SecurityArch2Protocol,
+        path: *const FfiDevicePath,
+        file_buffer: *const u8,
+        file_size: usize,
+        boot_policy: Boolean,
+    ) -> Status {
+        // The path and file buffer are optional, and the shim can't verify without them.
+        // The shim also doesn't support the boot policy.
+        if path.is_null() || file_buffer.is_null() || bool::from(boot_policy) {
+            // SAFETY: The arguments are passed through unchanged from the firmware.
+            return unsafe {
+                Self::original_file_authentication(this, path, file_buffer, file_size, boot_policy)
+            };
+        }
+
+        // Construct a slice out of the file buffer and size.
+        let buffer = unsafe { slice::from_raw_parts(file_buffer, file_size) };
+
+        // Construct a shim input from the path.
+        let input = ShimInput::SecurityHookBuffer(Some(path), buffer);
+
+        // Verify the input, if it fails, call the original hook.
+        if !Self::verify(input) {
+            // SAFETY: The arguments are passed through unchanged from the firmware.
+            unsafe {
+                Self::original_file_authentication(this, path, file_buffer, file_size, boot_policy)
+            }
         } else {
             Status::SUCCESS
         }