OcPeCoffExtLib: Add support for FixupAppleEfiImages quirk (#498)

Changelog.md

@@ -5,6 +5,7 @@ OpenCore Changelog
 - Fixed hang while generating boot entries on some systems
 - Added `efidebug.tool` support for 32-bit on 32-bit using GDB or LLDB
 - Fixed potential incorrect values in kernel image capabilities calculation
+- Added `FixupAppleEfiImages` quirk to allow booting Mac OS X 10.4 and 10.5 boot.efi images on modern secure image loaders
 
 #### v0.9.5
 - Fixed GUID formatting for legacy NVRAM saving

Docs/Configuration.tex

@@ -1610,6 +1610,34 @@ \subsection{Quirks Properties}\label{booterpropsquirks}
   \texttt{RebuildAppleMemoryMap} if the firmware supports memory attributes table (MAT).
   Refer to the \texttt{OCABC: MAT support is 1/0} log entry to determine whether MAT is supported.
 
+\item
+  \texttt{FixupAppleEfiImages}\\
+  \textbf{Type}: \texttt{plist\ boolean}\\
+  \textbf{Failsafe}: \texttt{false}\\
+  \textbf{Description}: Fix errors in early Mac OS X boot.efi images.
+
+  Modern secure PE loaders will refuse to load \texttt{boot.efi} images from
+  Mac OS X 10.4 and 10.5 due to these files containing \texttt{W\^{}X} errors
+  and illegal overlapping sections.
+
+  This quirk detects these issues and pre-processes such images in memory,
+  so that a modern loader can accept them.
+
+  Pre-processing in memory is incompatible with secure boot, as the image loaded
+  is not the image on disk, so you cannot sign files which are loaded in this way
+  based on their original disk image contents.
+  Certain firmware will offer to register the hash of new, unknown images - this would
+  still work. On the other hand, it is not particularly realistic to want to
+  start such early, insecure images with secure boot anyway.
+
+  \emph{Note 1}: The quirk is only applied to Apple-specific `fat' (both 32-bit and 64-bit
+  versions in one image) \texttt{.efi} files, and is never applied during the Apple secure
+  boot path for newer macOS.
+
+  \emph{Note 2}: The quirk is only needed for loading Mac OS X 10.4 and 10.5, and even then
+  only if the firmware itself includes a modern, more secure PE COFF image loader. This includes
+  current builds of OpenDuet.
+
 \item
   \texttt{ForceBooterSignature}\\
   \textbf{Type}: \texttt{plist\ boolean}\\

Docs/Sample.plist

@@ -319,6 +319,8 @@
 			<true/>
 			<key>EnableWriteUnprotector</key>
 			<true/>
+			<key>FixupAppleEfiImages</key>
+			<false/>
 			<key>ForceBooterSignature</key>
 			<false/>
 			<key>ForceExitBootServices</key>

Docs/SampleCustom.plist

@@ -319,6 +319,8 @@
 			<true/>
 			<key>EnableWriteUnprotector</key>
 			<true/>
+			<key>FixupAppleEfiImages</key>
+			<false/>
 			<key>ForceBooterSignature</key>
 			<false/>
 			<key>ForceExitBootServices</key>

Include/Acidanthera/Library/OcBootManagementLib.h

@@ -1814,7 +1814,8 @@ OcRegisterBootstrapBootOption (
 **/
 VOID
 OcImageLoaderInit (
-  IN     CONST BOOLEAN  ProtectUefiServices
+  IN     CONST BOOLEAN  ProtectUefiServices,
+  IN     CONST BOOLEAN  FixupAppleEfiImages
   );
 
 /**

Include/Acidanthera/Library/OcConfigurationLib.h

@@ -142,6 +142,7 @@ OC_DECLARE (OC_BOOTER_PATCH_ARRAY)
   _(BOOLEAN                     , DiscardHibernateMap       ,     , FALSE  , ()) \
   _(BOOLEAN                     , EnableSafeModeSlide       ,     , FALSE  , ()) \
   _(BOOLEAN                     , EnableWriteUnprotector    ,     , FALSE  , ()) \
+  _(BOOLEAN                     , FixupAppleEfiImages       ,     , FALSE  , ()) \
   _(BOOLEAN                     , ForceBooterSignature      ,     , FALSE  , ()) \
   _(BOOLEAN                     , ForceExitBootServices     ,     , FALSE  , ()) \
   _(BOOLEAN                     , ProtectMemoryRegions      ,     , FALSE  , ()) \

Include/Acidanthera/Library/OcPeCoffExtLib.h

@@ -43,4 +43,19 @@ PeCoffGetApfsDriverVersion (
   OUT APFS_DRIVER_VERSION  **DriverVersionPtr
   );
 
+/**
+  Detect and patch W^X and section overlap errors in legacy boot.efi.
+  Expected to make changes in 10.4 and 10.5 only.
+
+  @param[in]  DriverBuffer      Image buffer.
+  @param[in]  DriverSize        Size of the image.
+
+  @retval EFI_SUCCESS on success.
+**/
+EFI_STATUS
+OcPatchLegacyEfi (
+  IN  VOID    *DriverBuffer,
+  IN  UINT32  DriverSize
+  );
+
 #endif // OC_PE_COFF_EXT_LIB_H

Library/OcBootManagementLib/ImageLoader.c

@@ -36,6 +36,7 @@
 #include <Library/OcFileLib.h>
 #include <Library/OcMachoLib.h>
 #include <Library/OcMiscLib.h>
+#include <Library/OcPeCoffExtLib.h>
 #include <Library/OcStringLib.h>
 #include <Library/UefiImageLib.h>
 #include <Library/UefiBootServicesTableLib.h>
@@ -82,6 +83,7 @@ STATIC EFI_HANDLE                 mImageLoaderCapsHandle;
 STATIC BOOLEAN                    mImageLoaderEnabled;
 
 STATIC BOOLEAN  mProtectUefiServices;
+STATIC BOOLEAN  mFixupAppleEfiImages;
 
 STATIC EFI_IMAGE_LOAD          mPreservedLoadImage;
 STATIC EFI_IMAGE_START         mPreservedStartImage;
@@ -865,6 +867,26 @@ InternalEfiLoadImage (
     // Determine its capabilities.
     //
     if (!EFI_ERROR (Status) && (RealSize != SourceSize) && (RealSize >= EFI_PAGE_SIZE)) {
+      if (mFixupAppleEfiImages) {
+        if (SecureBootStatus == EFI_SUCCESS) {
+          DEBUG ((DEBUG_INFO, "OCB: Secure boot, fixup legacy efi ignored\n"));
+        } else {
+          Status = OcPatchLegacyEfi (SourceBuffer, RealSize);
+          //
+          // Error can mean incompletely patched image, so we should fail.
+          // Any error not the result of incomplete patching would in general not load anyway.
+          //
+          if (EFI_ERROR (Status)) {
+            DEBUG ((DEBUG_WARN, "OCB: PatchLegacyEfi - %r\n", Status));
+            if (AllocatedBuffer != NULL) {
+              FreePool (AllocatedBuffer);
+            }
+
+            return Status;
+          }
+        }
+      }
+
       mImageLoaderCaps = DetectCapabilities (SourceBuffer, RealSize);
     }
 
@@ -1087,10 +1109,12 @@ InternalEfiExit (
 
 VOID
 OcImageLoaderInit (
-  IN     CONST BOOLEAN  ProtectUefiServices
+  IN     CONST BOOLEAN  ProtectUefiServices,
+  IN     CONST BOOLEAN  FixupAppleEfiImages
   )
 {
   mProtectUefiServices = ProtectUefiServices;
+  mFixupAppleEfiImages = FixupAppleEfiImages;
 
   mOriginalEfiLoadImage   = gBS->LoadImage;
   mOriginalEfiStartImage  = gBS->StartImage;

Library/OcBootManagementLib/OcBootManagementLib.inf

@@ -110,6 +110,7 @@
   OcFlexArrayLib
   OcMachoLib
   OcMiscLib
+  OcPeCoffExtLib
   OcRtcLib
   OcTypingLib
   OcVariableLib

Library/OcConfigurationLib/OcConfigurationLib.c

@@ -189,6 +189,7 @@ OC_SCHEMA
   OC_SCHEMA_BOOLEAN_IN ("DiscardHibernateMap",    OC_GLOBAL_CONFIG, Booter.Quirks.DiscardHibernateMap),
   OC_SCHEMA_BOOLEAN_IN ("EnableSafeModeSlide",    OC_GLOBAL_CONFIG, Booter.Quirks.EnableSafeModeSlide),
   OC_SCHEMA_BOOLEAN_IN ("EnableWriteUnprotector", OC_GLOBAL_CONFIG, Booter.Quirks.EnableWriteUnprotector),
+  OC_SCHEMA_BOOLEAN_IN ("FixupAppleEfiImages",    OC_GLOBAL_CONFIG, Booter.Quirks.FixupAppleEfiImages),
   OC_SCHEMA_BOOLEAN_IN ("ForceBooterSignature",   OC_GLOBAL_CONFIG, Booter.Quirks.ForceBooterSignature),
   OC_SCHEMA_BOOLEAN_IN ("ForceExitBootServices",  OC_GLOBAL_CONFIG, Booter.Quirks.ForceExitBootServices),
   OC_SCHEMA_BOOLEAN_IN ("ProtectMemoryRegions",   OC_GLOBAL_CONFIG, Booter.Quirks.ProtectMemoryRegions),

Library/OcMainLib/OpenCoreUefi.c

@@ -899,7 +899,7 @@ OcLoadUefiSupport (
 
   OcReinstallProtocols (Config);
 
-  OcImageLoaderInit (Config->Booter.Quirks.ProtectUefiServices);
+  OcImageLoaderInit (Config->Booter.Quirks.ProtectUefiServices, Config->Booter.Quirks.FixupAppleEfiImages);
 
   OcLoadAppleSecureBoot (Config, CpuInfo);
 

Library/OcPeCoffExtLib/BasePeCoffLib2Internals.h

@@ -0,0 +1,55 @@
+/** @file
+  Provides shared private definitions across this library.
+
+  Copyright (c) 2020 - 2021, Marvin Häuser. All rights reserved.<BR>
+  Copyright (c) 2020, Vitaly Cheptsov. All rights reserved.<BR>
+  Copyright (c) 2020, ISP RAS. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-3-Clause
+**/
+
+#ifndef BASE_PE_COFF_LIB2_INTERNALS_H_
+#define BASE_PE_COFF_LIB2_INTERNALS_H_
+
+//
+// PcdImageLoaderRelocTypePolicy bits.
+//
+
+///
+/// If set, ARM Thumb Image relocations are supported.
+///
+#define PCD_RELOC_TYPE_POLICY_ARM  BIT0
+
+///
+/// Denotes the alignment requirement for Image certificate sizes.
+///
+#define IMAGE_CERTIFICATE_ALIGN  8U
+
+//
+// The PE/COFF specification guarantees an 8 Byte alignment for certificate
+// sizes. This is larger than the alignment requirement for WIN_CERTIFICATE
+// implied by the UEFI ABI. ASSERT this holds.
+//
+STATIC_ASSERT (
+  ALIGNOF (WIN_CERTIFICATE) <= IMAGE_CERTIFICATE_ALIGN,
+  "The PE/COFF specification guarantee does not suffice."
+  );
+
+//
+// The 4 Byte alignment guaranteed by the PE/COFF specification has been
+// replaced with ALIGNOF (EFI_IMAGE_BASE_RELOCATION_BLOCK) for proof simplicity.
+// This obviously was the original intention of the specification. ASSERT in
+// case the equality is not given.
+//
+STATIC_ASSERT (
+  sizeof (UINT32) == ALIGNOF (EFI_IMAGE_BASE_RELOCATION_BLOCK),
+  "The current model violates the PE/COFF specification"
+  );
+
+// FIXME:
+RETURN_STATUS
+PeCoffLoadImageInplaceNoBase (
+  IN OUT PE_COFF_LOADER_IMAGE_CONTEXT  *Context
+  );
+
+#endif // BASE_PE_COFF_LIB_INTERNALS_H_

Library/OcPeCoffExtLib/OcPeCoffExtInternal.h

@@ -29,4 +29,31 @@ typedef struct APPLE_SIGNATURE_CONTEXT_ {
   UINT8                Signature[256];
 } APPLE_SIGNATURE_CONTEXT;
 
+/**
+  Fix W^X and section overlap issues in loaded TE, PE32, or PE32+ Image in
+  memory while initialising Context.
+
+  Closely based on PeCoffInitializeContext from PeCoffLib2.
+
+  The approach of modifying the image in memory is basically incompatible
+  with secure boot, athough:
+    a) Certain firmware may allow optionally registering the hash of any
+       image which does not load, which would still work.
+    b) It is fairly crazy anyway to want to apply secure boot to the old,
+       insecure .efi files which need these fixups.
+
+  @param[out] Context     The context describing the Image.
+  @param[in]  FileBuffer  The file data to parse as PE Image.
+  @param[in]  FileSize    The size, in Bytes, of FileBuffer.
+
+  @retval RETURN_SUCCESS  The Image context has been initialised successfully.
+  @retval other           The file data is malformed.
+**/
+RETURN_STATUS
+InternalPeCoffFixup (
+  OUT PE_COFF_LOADER_IMAGE_CONTEXT  *Context,
+  IN  CONST VOID                    *FileBuffer,
+  IN  UINT32                        FileSize
+  );
+
 #endif // OC_PE_COFF_EXT_INTERNAL_H

Library/OcPeCoffExtLib/OcPeCoffExtLib.c

@@ -33,6 +33,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Library/UefiLib.h>
 #include <Library/OcCryptoLib.h>
 #include <Library/OcAppleKeysLib.h>
+#include <Library/OcStringLib.h>
 #include <Guid/AppleCertificate.h>
 
 #include "OcPeCoffExtInternal.h"
@@ -493,7 +494,7 @@ PeCoffGetApfsDriverVersion (
      || (ImageContext.ImageType != PeCoffLoaderTypePe32Plus)
      || (ImageContext.Subsystem != EFI_IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER))
   {
-    DEBUG ((DEBUG_INFO, "OCPE: PeCoff unsupported image\n"));
+    DEBUG ((DEBUG_INFO, "OCPE: PeCoff apfs unsupported image\n"));
     return EFI_UNSUPPORTED;
   }
 
@@ -541,3 +542,25 @@ PeCoffGetApfsDriverVersion (
   *DriverVersionPtr = DriverVersion;
   return EFI_SUCCESS;
 }
+
+EFI_STATUS
+OcPatchLegacyEfi (
+  IN  VOID    *DriverBuffer,
+  IN  UINT32  DriverSize
+  )
+{
+  EFI_STATUS                    ImageStatus;
+  PE_COFF_LOADER_IMAGE_CONTEXT  ImageContext;
+
+  ImageStatus = InternalPeCoffFixup (
+                  &ImageContext,
+                  DriverBuffer,
+                  DriverSize
+                  );
+  if (EFI_ERROR (ImageStatus)) {
+    DEBUG ((DEBUG_WARN, "OCPE: PeCoff legacy patch failure - %r\n", ImageStatus));
+    return EFI_UNSUPPORTED;
+  }
+
+  return EFI_SUCCESS;
+}

Library/OcPeCoffExtLib/OcPeCoffExtLib.inf

@@ -25,12 +25,13 @@
 
 
 #
-#  VALID_ARCHITECTURES           = X64
+#  VALID_ARCHITECTURES           = IA32 X64
 #
 
 [Sources]
   OcPeCoffExtInternal.h
   OcPeCoffExtLib.c
+  OcPeCoffFixup.c
 
 [Packages]
   MdePkg/MdePkg.dec
@@ -48,7 +49,13 @@
   DebugLib
   OcAppleKeysLib
   OcCryptoLib
+  OcStringLib
 
 [Guids]
   gAppleEfiCertificateGuid
   gEfiCertTypeRsa2048Sha256Guid
+
+[FixedPcd]
+  gEfiMdePkgTokenSpaceGuid.PcdImageLoaderAlignmentPolicy
+  gEfiMdePkgTokenSpaceGuid.PcdImageLoaderAllowMisalignedOffset
+  gEfiMdePkgTokenSpaceGuid.PcdDebugRaisePropertyMask