From 79eee8be5f86d71878c7ad208dea42027611babf Mon Sep 17 00:00:00 2001 From: Mike Beaton Date: Sun, 24 Nov 2024 10:04:54 +0000 Subject: [PATCH] CreateVault: Fix `sign.command` and update signing docs Fix operation of `sign.command` when printable characters occur immediately before `=BEGIN OC VAULT=`. `strings` finds the location of the first printable character in such a sequence. `hexdump` automatically works on 16 byte boundaries, so still finds the correct offset. Use `BASE_ALIGNAS` to enforce the required alignment, which will not be correct on all builds unless enforced (note alignment is required purely for locating the structure correctly from external script as above, not for reading in C). Remove struct packing, since structs had better be naturally packed anyway (if not, reading from them without arbitrary-alignment-safe code, as we do, would be undefined behaviour). Add static asserts to confirm correct naturally packed layout. Update the docs to refer to `sign.command` rather than to include the signing commands explicitly - otherwise we have two places that need to be kept in sync for signing commands, and note that the commands in the two places were already out of sync. Signed-off-by: Mike Beaton --- Changelog.md | 1 + Docs/Configuration.tex | 11 +++-------- Library/OcMainLib/OpenCoreVault.c | 24 +++++++++++++++++------- Utilities/CreateVault/sign.command | 2 +- 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/Changelog.md b/Changelog.md index 741ab07d2a1..480a389c686 100644 --- a/Changelog.md +++ b/Changelog.md @@ -6,6 +6,7 @@ OpenCore Changelog - Added Arrow Lake CPU detection - Fixed Raptor Lake CPU detection - Supported booting with TuneD in Fedora 41 in OpenLinuxBoot +- Fixed failure of vault `sign.command` to insert signature in correct location in some circumstances #### v1.0.2 - Fixed error in macrecovery when running headless, thx @mkorje diff --git a/Docs/Configuration.tex b/Docs/Configuration.tex index 707439e05b3..b74e45a6a11 100755 --- a/Docs/Configuration.tex +++ b/Docs/Configuration.tex @@ -4724,7 +4724,7 @@ \subsection{Security Properties}\label{miscsecurityprops} \href{https://github.com/acidanthera/OpenCorePkg/tree/master/Utilities/CreateVault}{RsaTool}. - The complete set of commands to: + The steps to binary patch \texttt{OpenCore.efi} are: \begin{itemize} \tightlist @@ -4734,14 +4734,9 @@ \subsection{Security Properties}\label{miscsecurityprops} \item Create \texttt{vault.sig}. \end{itemize} - Can look as follows: + A script to do this is privided in OpenCore releases: \begin{lstlisting}[label=createvault, style=ocbash] -cd /Volumes/EFI/EFI/OC -/path/to/create_vault.sh . -/path/to/RsaTool -sign vault.plist vault.sig vault.pub -off=$(($(strings -a -t d OpenCore.efi | grep "=BEGIN OC VAULT=" | cut -f1 -d' ')+16)) -dd of=OpenCore.efi if=vault.pub bs=1 seek=$off count=528 conv=notrunc -rm vault.pub +/Utilities/CreateVault/sign.command /Volumes/EFI/EFI/OC \end{lstlisting} \emph{Note 1}: While it may appear obvious, an external diff --git a/Library/OcMainLib/OpenCoreVault.c b/Library/OcMainLib/OpenCoreVault.c index af5a54f3492..99c287b1571 100644 --- a/Library/OcMainLib/OpenCoreVault.c +++ b/Library/OcMainLib/OpenCoreVault.c @@ -14,24 +14,21 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include -#pragma pack(push, 1) - -typedef PACKED struct { +typedef struct { OC_RSA_PUBLIC_KEY_HDR Hdr; UINT64 Data[(2 * (2048 / OC_CHAR_BIT)) / sizeof (UINT64)]; } OC_RSA_PUBLIC_KEY_2048; -typedef PACKED struct { +typedef struct { CHAR8 StartMagic[16]; OC_RSA_PUBLIC_KEY_2048 VaultKey; CHAR8 EndMagic[16]; } OC_BUILTIN_VAULT_KEY; -#pragma pack(pop) - +BASE_ALIGNAS (16) STATIC OC_BUILTIN_VAULT_KEY - mOpenCoreVaultKey = { +mOpenCoreVaultKey = { .StartMagic = { '=', 'B', 'E', 'G', 'I', 'N', ' ', 'O', 'C', ' ', 'V', 'A', 'U', 'L', 'T', '=' }, .EndMagic = { '=', '=', 'E', 'N', 'D', ' ', 'O', 'C', ' ', 'V', 'A', 'U', 'L', 'T', '=', '=' } }; @@ -44,6 +41,19 @@ OcGetVaultKey ( UINT32 Index; BOOLEAN AllZero; + STATIC_ASSERT ( + sizeof (OC_RSA_PUBLIC_KEY_2048) == sizeof (((OC_RSA_PUBLIC_KEY_2048 *)NULL)->Hdr) + sizeof (((OC_RSA_PUBLIC_KEY_2048 *)NULL)->Data), + "OC_RSA_PUBLIC_KEY_2048 is not naturally packed" + ); + STATIC_ASSERT ( + OFFSET_OF (OC_BUILTIN_VAULT_KEY, VaultKey) == 16, + "offsetof(OC_BUILTIN_VAULT_KEY.VaultKey)" + ); + STATIC_ASSERT ( + OFFSET_OF (OC_BUILTIN_VAULT_KEY, EndMagic) == 16 + sizeof (OC_RSA_PUBLIC_KEY_2048), + "offsetof(OC_BUILTIN_VAULT_KEY.EndMagic)" + ); + // // TODO: Perhaps try to get the key from firmware too? // diff --git a/Utilities/CreateVault/sign.command b/Utilities/CreateVault/sign.command index ffbdec17f07..cbb2366b934 100755 --- a/Utilities/CreateVault/sign.command +++ b/Utilities/CreateVault/sign.command @@ -61,7 +61,7 @@ echo "Signing ${OCBin}..." ./RsaTool -sign "${OCPath}/vault.plist" "${OCPath}/vault.sig" "${PubKey}" || abort "Failed to patch ${PubKey}" echo "Bin-patching ${OCBin}..." -off=$(($(/usr/bin/strings -a -t d "${OCBin}" | /usr/bin/grep "=BEGIN OC VAULT=" | /usr/bin/awk '{print $1}') + 16)) +off=$((0x$(/usr/bin/hexdump -C "${OCBin}" | /usr/bin/grep "=BEGIN OC VAULT=" | /usr/bin/awk '{print $1}') + 16)) if [ "${off}" -le 16 ]; then abort "${OCBin} is borked" fi