| Age | Commit message (Collapse) | Author |
|---|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Due to the issues addressed in the 2022-11-15 batch of grub CVEs[0], we
need to bump the sbat version from grub. This patch changes it from 2
to 3.
[0] https://lists.gnu.org/archive/html/grub-devel/2022-11/msg00059.html
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Though we don't need to bump SBAT_LEVEL for this, we've decided to
change the level to 3 here in case
53509eaf2253e23bfb552e9386fd0877abe592b4 turns out to be worse than we
think it is, so we can fix that easily later.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This adds a few more UEFI functions to our coverity model, so we see a
few less false positives during scanning. It also fixes an error in our
model for OpenSSL's OBJ_dup().
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
AuthenticodeVerify() calls OpenSSLs d2i_PKCS7() API to parse asn encoded
signed authenticode pkcs#7 data. when this successfully returns, a type
check is done by calling PKCS7_type_is_signed() and then
Pkcs7->d.sign->contents->type is used. It is possible to construct an asn1
blob that successfully decodes and have d2i_PKCS7() return a valid pointer
and have PKCS7_type_is_signed() also return success but have Pkcs7->d.sign
be a NULL pointer.
Looking at how PKCS7_verify() [inside of OpenSSL] implements checking for
pkcs7 structs it does the following:
- call PKCS7_type_is_signed()
- call PKCS7_get_detached()
Looking into how PKCS7_get_detatched() is implemented, it checks to see if
p7->d.sign is NULL or if p7->d.sign->contents->d.ptr is NULL.
As such, the fix is to do the same as OpenSSL after calling d2i_PKCS7().
- Add call to PKS7_get_detached() to existing error handling
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Cherry-picked-from: https://github.com/tianocore/edk2/commit/26442d11e620a9e81c019a24a4ff38441c64ba10
|
|
MokListTrusted was added by mistake to PCR 7 in 4e513405. The value of
MokListTrusted does not alter the behavior of secure boot so, as per
https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_PFP_r1p05_v23_pub.pdf#page=36
(section 3.3.4 PCR usage) so it should not be factored in the value of
PCR 7.
See:
https://github.com/rhboot/shim/pull/423
https://github.com/rhboot/shim/commit/4e513405b4f1641710115780d19dcec130c5208f
Fixes https://github.com/rhboot/shim/issues/484
Fixes https://github.com/rhboot/shim/issues/492
Signed-off-by: Arthur Gautier <arthur.gautier@arista.com>
|
|
Remove timestamps, user names, etc. from the tarball so that
it can be built reproducibly by multiple people, on different
machines.
The outer bzip2 layer might still be different, no reproducible
bzip2 known.
Signed-off-by: Julian Andres Klode <julian.klode@canonical.com>
|
|
Seen mokmanager image load failure '2 sections contain entry point'
for shim built on Oracle Linux 9 aarch64. found_entry_point counter in
handle_image() uses SizeOfRawData to calculate section boundary.
PE spec defines VirtualSize for the total size of the section when loaded
into memory. SizeOfRawData is the size of the section (for object files)
or the size of the initialized data on disk.
Fix this issue by updating section in-memory size limit to VirtualSize.
Resolves: #517
Signed-off-by: Ilya Okomin <ilya.okomin@oracle.com>
|
|
This changes the alignment of UINT64 data to 8 bytes on IA32, which
matches EDK2's understanding of alignment. In particular this change
affects the offset where shim writes `EFI_LOADED_IMAGE.ImageSize`.
Fixes https://github.com/rhboot/shim/issues/515
Signed-off-by: Nicholas Bishop <nicholasbishop@google.com>
|
|
Use the EFI RT memory function CopyMem instead of memcpy in load_cert_file.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
|
|
0214cd9cef5a fixes a NULL pointer dereference problem, it introduces two
new problems. First it incorrectly assumes li.FilePath is a string.
Second, it puts EFI_LOADED_IMAGE li on the stack. It has been found
that not all archectures can handle this being on the stack.
The shim_li variable will be setup properly from the read_image
call. Use the global shim_li variable instead when calling
verify_image.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
|
|
We've seen crashes in early GRUB code on an ARM Cortex-A72-based
platform that point at seemingly harmless instructions. Flushing
the i-cache of those instructions prior to executing has been
shown to avoid the problem, which has parallels with this story:
https://www.mail-archive.com/osv-dev@googlegroups.com/msg06203.html
Add a cache flushing utility function and provide an implementation
using a GCC intrinsic. This will need to be extended to support other
compilers. Note that this intrinsic is a no-op for x86 platforms.
This fixes issue #498.
Signed-off-by: dann frazier <dann.frazier@canonical.com>
|
|
In 6c8d08c0af4768c715b79c8ec25141d56e34f8b4 ("shim: Ignore UEFI
LoadOptions that are just NUL characters."), a check was added to
discard load options that are entirely NUL. We now see some firmwares
that start LoadOptions with a NUL, and then follow it with garbage (path
to directory containing loaders). Widen the check to just discard
anything that starts with a NUL.
Resolves: #490
Related: #95
See-also: https://bugzilla.redhat.com/show_bug.cgi?id=2113005
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
|
|
Intel Trust Domain Extensions (Intel TDX) extends Virtual Machine
Extensions (VMX) and Multi-Key Total Memory Encryption (MK-TME) with a
new kind of virtual machine guest called a Trust Domain(TD)[1]. A TD
runs in a CPU mode that is designed to protect the confidentiality of
its memory contents and its CPU state from any other software, including
the hosting Virtual Machine Monitor (VMM).
Trust Domain Virtual Firmware (TDVF) is required to provide Intel TDX
implementation and service for EFI_CC_MEASUREMENT_PROTOCOL[2]. The bugzilla
for TDVF is at https://bugzilla.tianocore.org/show_bug.cgi?id=3625.
To support CC measurement/attestation with Intel TDX technology, these 4
RTMR registers will be extended by TDX service like TPM/TPM2 PCR:
- RTMR[0] for TDVF configuration
- RTMR[1] for the TD OS loader and kernel
- RTMR[2] for the OS application
- RTMR[3] reserved for special usage only
Add a TDX Implementation for CC Measurement protocol along with
TPM/TPM2 protocol.
References:
[1] https://software.intel.com/content/dam/develop/external/us/en/documents/tdx-whitepaper-v4.pdf
[2] https://software.intel.com/content/dam/develop/external/us/en/documents/tdx-virtual-firmware-design-guide-rev-1.pdf
[3] https://software.intel.com/content/dam/develop/external/us/en/documents/intel-tdx-guest-hypervisor-communication-interface-1.0-344426-002.pdf
Signed-off-by: Lu Ken <ken.lu@intel.com>
[rharwood: style pass on code and commit message]
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
|
|
It's been suggested that we should link to the test plan in the readme.
This seems pretty reasonable to me, so here it is.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
When calling back into shim from grub, the MokListRT may contain additional
entries not available in the original MokList, an example being the certs
included via user_cert. Use the MokListRT instead when calling check_db_cert
and check_db_hash.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
|
|
Given a set of EFI variables and boot assets, it should be possible
to compute what the value of PCR 7 will be on the next boot.
As shim manages the contents of the SbatLevel variable and this is
measured to PCR 7, export the payloads that shim contains in a new
COFF section (.sbatlevel) so that it can be introspected by code
outside of shim.
The new section works a bit like .vendor_cert - it contains a header
and then the payload. In this case, the header contains no size fields
because the strings are NULL terminated. Shim uses this new section
internally in set_sbat_uefi_variable.
The .sbatlevel section starts with a 4 byte version field which is
not used by shim but may be useful for external auditors if the
format of the section contents change in the future.
Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
PR 446 ("Add verify_image") duplicates some of the code affected by
Chris Coulson's defense in depth patch against CVE-2022-28737 ("pe:
Perform image verification earlier when loading grub").
This patch makes the same change to the new function.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
bump shim SBAT generation requirement to 2 for CVE-2022-28737
bump GRUB2 SBAT generation requirement to 2 for CVE-2021-3695
Signed-off-by: Jan Setje-Eilers <jan.setjeeilers@oracle.com>
|
|
Signed-off-by: Jan Setje-Eilers <jan.setjeeilers@oracle.com>
|
|
The second stage loader was being verified after loading it into
memory. As an additional hardening measure to avoid performing risky
memcpys using header fields from a potentially specially crafted image,
perform the verification before this so that it can be rejected earlier.
Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
|
|
During image loading, the size of the destination buffer for the image
is determined by the SizeOfImage field in the optional header. The start
and end virtual addresses of each section, as determined by each section's
VirtualAddress and VirtualSize fields, are bounds checked against the
allocated buffer. However, the amount of data copied to the destination
buffer is determined by the section's SizeOfRawData filed. If this is
larger than the VirtualSize, then the copy can overflow the destination
buffer.
Fix this by limiting the amount of data to copy to the section's
VirtualSize. In the case where a section has SizeOfRawData > VirtualSize,
the excess data is discarded.
This fixes CVE-2022-28737
Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
|
|
Since booting from removable media can be hard to detect,
setting a persistent latest SBAT policy is risky in a typical
client system. This changes latest to be a one-shot operation
that could be set at the time of an OS update if desired.
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
|
|
We're told what the size limit is for one of the two things this needs
to handle, we should honor it.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Coverity pointed out that AllocateZeroPool() can error, and then we
immediately dereference the NULL.
This handles the error case.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Coverity noticed that the refactoring of handle_image() wildly
misunderstood how we deal with file paths.
This reworks it to not have a bunch of NULL dereferences.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Coverity complains:
CID 373676 (#3 of 3): Unrecoverable parse warning (PARSE_ERROR)
1. arguments_provided_for_attribute: attribute "__malloc__" does not take arguments
This is, of course, just plain wrong. Even so, I'm tired of looking at
it, so this patch wraps the #define we use for that attribute in a check
to see if it's being built by Coverity.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
preserve_sbat_uefi_variable() shoud really deal with the sbat
metadata version as a numerical value that could gain more digits.
It also needs to only compare the datestamp since the actual
metadata can grow and shrink
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
scan-build noticed that when we split this out, this assignment was no
longer in a loop, and so doesn't do anything.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
scan-build says info->FileName returned from a successful call to
dir->Read() can be NULL. I don't think that would be a compliant
implementation, but anything's possible.
This patch checks it for NULL-ness before the StrCaseCmp().
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
There are a couple of places where the code we've got right now just
uses integers to decode one of our MoK variables. That's bad.
This patch replaces those with symbolic names.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
scan-build noticed that there's a path where we'll pass some data from
the read_image() to e.g. the string functions, but it might be an
unassigned pointer on one of the code paths. I don't think you can
actually hit it without returning from an error first, but best to
initialize these anyway.
This patch initializes data to NULL and datasize to 0.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
scan-build invoked clang in a way that complains about our
SIGNATURE_XX() macro's sizes being used to assign to things that are
that size in post-process-pe.c.
This patch makes them cast the results to the appropriately sized type.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
When using clang-analyzer, scan-build sets CC to
/usr/bin/../libexec/ccc-analyzer, which for whatever reason when we're
in the sub-make to build gnu-efi, then gets confused and invokes gcc
rather than clang. This causes gnu-efi's attempt to check which
compiler it's using to fail, because "/usr/bin/../libexec/ccc-analyzer
-v" invokes GCC. At that point ccc-analyzer complains that it can't
find the clang invocation in its own output, which it chose not to use
clang for, as such:
/usr/bin/../libexec/ccc-analyzer -I/home/pjones/devel/github.com/shim/worktree/gnu-efi//lib -I/home/pjones/devel/github.com/shim/worktree/gnu-efi/inc -I/home/pjones/devel/github.com/shim/worktree/gnu-efi/inc/x86_64 -I/home/pjones/devel/github.com/shim/worktree/gnu-efi/inc/protocol -Wno-error=pragmas -mno-red-zone -mno-avx -fpic -Os -Wall -Wextra -Wno-missing-field-initializers -Werror -fshort-wchar -fno-strict-aliasing -ffreestanding -fno-stack-protector -fno-stack-check -nostdinc -isystem /home/pjones/devel/github.com/shim/worktree/gnu-efi/../include/system -isystem /usr/lib/gcc/x86_64-redhat-linux/11/include -DCONFIG_x86_64 -DGNU_EFI_USE_MS_ABI -DGNU_EFI_USE_EXTERNAL_STDARG -maccumulate-outgoing-args --std=c11 -c /home/pjones/devel/github.com/shim/worktree/gnu-efi//lib/smbios.c -o smbios.o
could not find clang line
make[3]: *** [/home/pjones/devel/github.com/shim/worktree/gnu-efi//lib/../Make.rules:52: smbios.o] Error 1
This patch passes CCC_CC=$(COMPILER) to the gnu-efi sub-make, which
forces ccc-analyzer to use $(COMPILER), which is still clang.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Support for updating SBAT revocations to latest or previous revocations.
Allow SBAT revocations to be reset to empty metadata only when UEFI
Secure Boot is disabled.
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
Currently, system firmware has no means to discover that an EFI
Application is compatible with the security feature variously known as
NX or w^x.
Since at least Revision 8.1, the PE spec supports setting a flag the
Optional Header's DllCharacteristics field to inform loaders that an
application supports being loaded with NX enabled.
In the case of UEFI, there are several things that should be enabled if
this flag is set:
- EFI_BOOT_SERVICES.AllocatePages() with MemoryType = EfiLoaderCode,
EfiBootServicesCode, EfiRuntimeServicesCode, etc, currently must set
memory as rwx. This flag set implies that rw- is appropriate, and
that the application knows how to use the EFI_MEMORY_ATTRIBUTE
protocol to change that to r-x.
- EFI_BOOT_SERVICES.AllocatePool() - same as AllocatePages()
- EFI_BOOT_SERVICES.LoadImage()
- currently must set the stack as rwx. This flag states that it is
allowed to be rw-
- currently a binary can probably have writable PLTs? This flag
allows the loader to not set them writable
- I have heard that some firmwares have the 0 page mapped rwx.
Obviously this should not be done.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This adds a new MoK variable, MokPolicy (&MokPolicyRT) that's intended
as a bitmask of machine owner policy choices, and the bit
MOK_POLICY_REQUIRE_NX. This bit specifies whether it is permissible to
load binaries which do not support NX mitigations, and it currently
defaults to allowing such binaries to be loaded.
The broader intention here is to migrate all of the MoK policy variables
that are really just on/off flags to this variable.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This adds support in our PE loader for NX support utilizing the
EFI_MEMORY_ATTRIBUTE protocol. Specifically, it changes the loader such
that:
- binaries without the EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT flag set
in the Optional Header are rejected as EFI_UNSUPPORTED
- binaries with non-discardable sections that have both the
EFI_SCN_MEM_WRITE and EFI_SCN_MEM_EXECUTE flags set are rejected as
EFI_UNSUPPORTED
- if the EFI_MEMORY_ATTRIBUTE protocol is installed, then:
- sections without the EFI_SCN_MEM_READ flag set will be marked with
EFI_MEMORY_RP
- sections without the EFI_SCN_MEM_WRITE flag set will be marked with
EFI_MEMORY_RO
- sections without the EFI_SCN_MEM_EXECUTE flag set will be marked
with EFI_MEMORY_XP
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This patch adds some missing definitions for PE header flags. We don't
use all of them, but it's less confusing with the list matching the
spec, except where the spec is obviously wrong.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
There is no 's' argument to post-process-pe, so we shouldn't tell getopt
that there is.
This patch takes the 's' out of the getopt short option list.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Heavily inspired by Matthew Garrett's patch "Allow additional certificates
to be loaded from a signed binary".
Add support for loading a binary, verifying its signature, and then
scanning it for embedded certificates. This is intended to make it
possible to decouple shim builds from vendor signatures. In order to
add new signatures to shim, an EFI Signature List should be generated
and then added to the .db section of a well-formed EFI binary. This
binary should then be signed with a key that shim already trusts (either
a built-in key, one present in the platform firmware or
one present in MOK) and placed in the same directory as shim with a
filename starting "shim_certificate" (eg, "shim_certificate_oracle").
Shim will read multiple files and incorporate the signatures from all of
them. Note that each section *must* be an EFI Signature List, not a raw
certificate.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
|
|
Separate out image reading from image launch in order to be able to load
an image without executing it.
Signed-off-by: Matthew Garrett <mgarrett@aurora.tech>
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
|
|
In the future we will want to examine binaries without wanting to
execute them. Create verify_image based off existing handle_image
code.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This makes SHIM_VERBOSE / SHIM_DEVEL_VERBOSE work the same way as
SHIM_DEBUG / SHIM_DEVEL_DEBUG when shim is built with ENABLE_SHIM_DEVEL
set.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
