OpenDuet/OcBootManagementLib: Disable W^X auto-fix in Duet, and move …

…fix for all Apple images to FixupAppleEfiImages quirk

Duet then matches behaviour of OVMF, and quirk can be used in both Duet and OVMF to load legacy Apple images.

Changelog.md

@@ -2,6 +2,8 @@ OpenCore Changelog
 ==================
 #### v0.9.7
 - Updated recovery_urls.txt
+- Changed OpenDuet to enforce `W^X` settings rather than fixing them in loaded images
+- Updated `FixupAppleEfiImages` quirk to fix `W^X` errors in Apple signed binaries from 10.7-10.12
 
 #### v0.9.6
 - Updated builtin firmware versions for SMBIOS and the rest

Docs/Configuration.md5

@@ -1 +1 @@
-d1bc9e116ec6651795559dd0661d9290
+9c1d9e52a55d0dfa1923f22aa158db81

Docs/Configuration.tex

@@ -1617,26 +1617,32 @@ \subsection{Quirks Properties}\label{booterpropsquirks}
   \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.
+  macOS 10.4 to 10.12 due to these files containing \texttt{W\^{}X} errors
+  (in all versions) and illegal overlapping sections (in 10.4 and 10.5 32-bit
+  versions only).
 
   This quirk detects these issues and pre-processes such images in memory,
-  so that a modern loader can accept them.
+  so that a modern loader will 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.
+  start these 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 1}: When enabled, this quirk is applied to all Apple-specific Fat
+  binaries (32-bit and 64-bit versions in one image), and to any other
+  Apple-signed boot images that are not being processed for Apple secure boot.
 
-  \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.
+  \emph{Note 2}: The quirk is never applied during the Apple secure boot path for
+  newer macOS. The Apple secure boot path includes its own separate mitigations
+  for \texttt{boot.efi} \texttt{W\^{}X} issues.
+
+  \emph{Note 3}: This quirk is needed for macOS 10.4 to 10.12 (and
+  higher, if Apple secure boot is not enabled), but only when the firmware
+  itself includes a modern, more secure PE COFF image loader. This applies to
+  current builds of OpenDuet, and to OVMF if built from audk source code.
 
 \item
   \texttt{ForceBooterSignature}\\

Docs/Differences/Differences.tex

@@ -1,7 +1,7 @@
 \documentclass[]{article}
 %DIF LATEXDIFF DIFFERENCE FILE
-%DIF DEL PreviousConfiguration.tex   Mon Nov  6 21:48:25 2023
-%DIF ADD ../Configuration.tex        Mon Nov  6 21:48:25 2023
+%DIF DEL PreviousConfiguration.tex   Mon Nov  6 21:35:43 2023
+%DIF ADD ../Configuration.tex        Tue Nov  7 22:04:03 2023
 
 \usepackage{lmodern}
 \usepackage{amssymb,amsmath}
@@ -1677,26 +1677,37 @@ \subsection{Quirks Properties}\label{booterpropsquirks}
   \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.
+  \DIFdelbegin \DIFdel{Mac OS X }\DIFdelend \DIFaddbegin \DIFadd{macOS }\DIFaddend 10.4 \DIFdelbegin \DIFdel{and 10.5 }\DIFdelend \DIFaddbegin \DIFadd{to 10.12 }\DIFaddend due to these files containing \texttt{W\^{}X} errors
+  \DIFaddbegin \DIFadd{(in all versions) }\DIFaddend and illegal overlapping sections \DIFaddbegin \DIFadd{(in 10.4 and 10.5 32-bit
+  versions only)}\DIFaddend .
 
   This quirk detects these issues and pre-processes such images in memory,
-  so that a modern loader can accept them.
+  so that a modern loader \DIFdelbegin \DIFdel{can }\DIFdelend \DIFaddbegin \DIFadd{will }\DIFaddend 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.
+  start \DIFdelbegin \DIFdel{such }\DIFdelend \DIFaddbegin \DIFadd{these }\DIFaddend 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 1}: \DIFdelbegin \DIFdel{The quirk is only applied to }\DIFdelend \DIFaddbegin \DIFadd{When enabled, this quirk is applied to all }\DIFaddend Apple-specific \DIFdelbegin \DIFdel{`fat' (both }\DIFdelend \DIFaddbegin \DIFadd{Fat
+  binaries (}\DIFaddend 32-bit and 64-bit versions in one image)\DIFdelbegin \texttt{\DIFdel{.efi}} %DIFAUXCMD
+\DIFdel{files, and }\DIFdelend \DIFaddbegin \DIFadd{, and to any other
+  Apple-signed boot images that are not being processed for Apple secure boot.
+}
 
-  \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.
+  \emph{\DIFadd{Note 2}}\DIFadd{: The quirk }\DIFaddend is never applied during the Apple secure boot path for
+  newer macOS. \DIFaddbegin \DIFadd{The Apple secure boot path includes its own separate mitigations
+  for }\texttt{\DIFadd{boot.efi}} \texttt{\DIFadd{W\^{}X}} \DIFadd{issues.
+}\DIFaddend 
+
+  \emph{Note \DIFdelbegin \DIFdel{2}\DIFdelend \DIFaddbegin \DIFadd{3}\DIFaddend }: \DIFdelbegin \DIFdel{The quirk is only needed for loading Mac OS X }\DIFdelend \DIFaddbegin \DIFadd{This quirk is needed for macOS }\DIFaddend 10.4 \DIFdelbegin \DIFdel{and 10.5, and even then
+  only if }\DIFdelend \DIFaddbegin \DIFadd{to 10.12 (and
+  higher, if Apple secure boot is not enabled), but only when }\DIFaddend the firmware
+  itself includes a modern, more secure PE COFF image loader. This \DIFdelbegin \DIFdel{includes
+  }\DIFdelend \DIFaddbegin \DIFadd{applies to
+  }\DIFaddend current builds of OpenDuet\DIFaddbegin \DIFadd{, and to OVMF if built from audk source code}\DIFaddend .
 
 \item
   \texttt{ForceBooterSignature}\\

Library/OcBootManagementLib/ImageLoader.c

@@ -785,9 +785,12 @@ InternalEfiLoadImage (
   )
 {
   EFI_STATUS  SecureBootStatus;
+  EFI_STATUS  FilterStatus;
   EFI_STATUS  Status;
   VOID        *AllocatedBuffer;
   UINT32      RealSize;
+  UINT32      SignedFileSize;
+  BOOLEAN     IsFat;
 
   mImageLoaderCapsHandle = NULL;
 
@@ -857,37 +860,61 @@ InternalEfiLoadImage (
   if (SourceBuffer != NULL) {
     RealSize = (UINT32)SourceSize;
  #ifdef MDE_CPU_IA32
-    Status = FatFilterArchitecture32 ((UINT8 **)&SourceBuffer, &RealSize);
+    FilterStatus = FatFilterArchitecture32 ((UINT8 **)&SourceBuffer, &RealSize);
  #else
-    Status = FatFilterArchitecture64 ((UINT8 **)&SourceBuffer, &RealSize);
+    FilterStatus = FatFilterArchitecture64 ((UINT8 **)&SourceBuffer, &RealSize);
  #endif
 
+    IsFat = !EFI_ERROR (FilterStatus) && (RealSize != SourceSize) && (RealSize >= EFI_PAGE_SIZE);
+
+    if (IsFat) {
+      mImageLoaderCaps = DetectCapabilities (SourceBuffer, RealSize);
+    }
+
     //
-    // This is FAT image.
-    // Determine its capabilities.
+    // Use mImageLoaderConfigure != NULL as a proxy for loaded kernel support,
+    // and only apply FixupAppleEfiImages while this is set.
     //
-    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"));
+    if (mFixupAppleEfiImages && (mImageLoaderConfigure != NULL)) {
+      if (SecureBootStatus == EFI_SUCCESS) {
+        DEBUG ((DEBUG_INFO, "OCB: Secure boot, fixup efi ignored\n"));
+        Status = EFI_SUCCESS;
+      } else if (IsFat) {
+        DEBUG ((DEBUG_INFO, "OCB: Fat binary, fixup efi...\n"));
+        Status = OcPatchLegacyEfi (SourceBuffer, RealSize);
+      } else {
+        //
+        // Overlapping sections not expected outside of fat binaries (and even then
+        // only in 32-bit slices), so verify signature allowing for W^X errors only.
+        //
+        SignedFileSize = RealSize;
+        Status         = PeCoffVerifyAppleSignature (SourceBuffer, &SignedFileSize);
+        if (!EFI_ERROR (Status)) {
+          DEBUG ((
+            DEBUG_INFO,
+            "OCB: Apple signed binary %u->%u, fixup efi...\n",
+            RealSize,
+            SignedFileSize
+            ));
+          RealSize = SignedFileSize;
+          Status   = OcPatchLegacyEfi (SourceBuffer, RealSize);
         } 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;
-          }
+          DEBUG ((DEBUG_INFO, "OCB: Not Apple signed binary, fixup efi ignored\n"));
+          Status = EFI_SUCCESS;
         }
       }
 
-      mImageLoaderCaps = DetectCapabilities (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)) {
+        if (AllocatedBuffer != NULL) {
+          FreePool (AllocatedBuffer);
+        }
+
+        return Status;
+      }
     }
 
     DEBUG ((
@@ -898,10 +925,10 @@ InternalEfiLoadImage (
       SourceBuffer,
       RealSize,
       mImageLoaderCaps,
-      Status
+      FilterStatus
       ));
 
-    if (!EFI_ERROR (Status)) {
+    if (!EFI_ERROR (FilterStatus)) {
       SourceSize = RealSize;
     } else if (AllocatedBuffer != NULL) {
       SourceBuffer = NULL;

OpenDuetPkg.dsc

@@ -259,7 +259,6 @@
   gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel|0x0
   gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x0
   gOpenCorePkgTokenSpaceGuid.PcdCanaryAllowRdtscFallback|TRUE
-  gEfiMdePkgTokenSpaceGuid.PcdImageLoaderRemoveXForWX|TRUE
 
 [BuildOptions]
   MSFT:NOOPT_*_*_CC_FLAGS    = -D OC_TARGET_RELEASE=1 /FAcs -Dinline=__inline  /GS /kernel