| Age | Commit message (Collapse) | Author |
|---|
|
Previously processing full padding-aligned Section->Misc.VirtualSize
relied on padding reloc entries being inserted by GenFw, which is
not required by spec.
This changes it to only process the amount referenced by Size, rather
than VirtualSize which may be bigger than the data present.
Signed-off-by: Mike Beaton <mjsbeaton@gmail.com>
|
|
shim uses path delimiters to determine the file path for the second
stage. Currently only / (slash) is detected, but some DHCP
implementations supply \ (backslash) as the path specifier.
This patch changes it to accept either delimiter.
Fixes issue #524.
Signed-off-by: Alberto Perez <aperezguevar@microsoft.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
A rendering error which caused the `<Vendor>_key.EFI` text to be
rendered as `_key.EFI` has been fixed. The text was being rendered incorrectly by GitHub since the <Vendor> part was being treated as an HTML tag and therefore ignored.
Two misspellings have been fixed
Tables have been reformatted to be more readable as plaintext. Rendering remains the same.
Signed-off-by: Kamil Aronowski <kamil.aronowski@yahoo.com>
|
|
Following the discovery of more problematic firmwares and drivers
affected by the issue f23883ccf78f1f605a272f9e5700f47e5494a71d is
designed to address (e.g. https://github.com/rhboot/shim/issues/558),
this patch further improves the code so that, instead of simply bailing
out, we progressively increase the buffer sizes, until either success
or a maximum size limit is reached.
In most cases, this workaround should be enough to ensure completion
of the directory read and thus provide full shim functionality (while
still warning the user about the non-compliance of their environment).
Signed-off-by: Pete Batard <pete@akeo.ie>
|
|
(See https://bugs.debian.org/1024617)
One of the Debian builds of grub bumped the SBAT to 3, but didn't
include the patches needed. Add "grub.debian,4" to block those
binaries.
Signed-off-by: Steve McIntyre <steve@einval.com>
|
|
Fix the `main` function in `test-sbat.c` to return the `status` variable
like the other `test-*.c` files.
Signed-off-by: Nicholas Bishop <nicholasbishop@google.com>
|
|
These calls did not check the return value. If they had, it would have
shown that the calls were failing due to passing `NULL` for the
`realloc` function pointer. That causes an early return, so the calls
weren't actually doing anything.
The `malloc`/`realloc`/`free` functions defined in
Cryptlib/SysCall/BaseMemAllocation.c are what actually get used, so just
drop the explicit call to `CRYPTO_set_mem_functions`.
Signed-off-by: Nicholas Bishop <nicholasbishop@google.com>
|
|
If the ShimRetainProtocol variable is set, avoid uninstalling our
protocol.
For example, this allows sd-stub in a UKI to use the shim protocol to
validate PE binaries, even if it is executed by a second stage, before
the kernel is loaded.
Ensure that the variable is volatile and for BootServices access.
Also delete it on startup, so that we can be sure it was really set by
a second stage.
Example use case in sd-boot/sd-stub:
https://github.com/systemd/systemd/pull/27358
Signed-off-by: Luca Boccassi <bluca@debian.org>
|
|
On DELL R350 booting DVD through RFS with BIOS 1.4.2 in Secure Boot,
firmware returns EFI_BUFFER_TOO_SMALL but with new buffersize set to 0,
which causes the load_certs() code to loop forever:
while (1) {
efi_status = dir->Read(dir, &buffersize, buffer);
if (efi_status == EFI_BUFFER_TOO_SMALL) {
...
continue;
}
...
}
This commit prevents such infinite loop.
Signed-off-by: Renaud Métrich <rmetrich@redhat.com>
|
|
|
|
|
|
|
|
This makes some checks in `get_mem_attrs` and `update_mem_attrs`
clearer.
Also add `test-pe-util.c` with a test for the new macro. The file is
named that way instead of `test-pe.c` to avoid having to get `pe.c`
building in the unit test environment.
Signed-off-by: Nicholas Bishop <nicholasbishop@google.com>
|
|
Setting memory attributes is generally done at page granularity, and
this is enforced by checks in `get_mem_attrs` and
`update_mem_attrs`. But unlike the section address, the section size
isn't necessarily aligned to 4KiB. Round up the section size to fix
this.
Signed-off-by: Nicholas Bishop <nicholasbishop@google.com>
|
|
There is one long-standing problem in CRT realloc wrapper, which will
cause the obvious buffer overflow issue when re-allocating one bigger
memory block:
void *realloc (void *ptr, size_t size)
{
//
// BUG: hardcode OldSize == size! We have no any knowledge about
// memory size of original pointer ptr.
//
return ReallocatePool ((UINTN) size, (UINTN) size, ptr);
}
This patch introduces one extra header to record the memory buffer size
information when allocating memory block from malloc routine, and re-wrap
the realloc() and free() routines to remove this BUG.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ting Ye <ting.ye@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long <qin.long@intel.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Validated-by: Jian J Wang <jian.j.wang@intel.com>
Cherry picked from https://github.com/tianocore/edk2.git, commit
cf8197a39d07179027455421a182598bd6989999. Changes:
* `SIGNATURE_32` -> `EFI_SIGNATURE_32`
* Added definition of `MIN`
Fixes https://github.com/rhboot/shim/issues/538
Signed-off-by: Nicholas Bishop <nicholasbishop@google.com>
|
|
Currently by default, when we build shim we do not set the PE
NX-compatibility DLL Characteristic flag. This signifies to the
firmware that shim (including the components it loads) is not prepared
for several related firmware changes:
- non-executable stack
- non-executable pages from AllocatePages()/AllocatePool()/etc.
- non-writable 0 page (not strictly related but some firmware will be
transitioning at the same time)
- the need to use the UEFI 2.10 Memory Attribute Protocol to set page
permissions.
This patch changes that default to be enabled by default. Distributors
of shim will need to ensure that either their builds disable this bit
(using "post-process-pe -N"), or that the bootloaders and kernels you
support loading are all compliant with this change. A new make
variable, POST_PROCESS_PE_FLAGS, has been added to simplify doing so.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
As required by policy for signing new shim binaries.
|
|
We're using 657b2483ca6e9fcf2ad8ac7ee577ff546d24c3aa, which is the
15.7 release plus the one patch we're applying.
|
|
|
|
Closes: #1022180
|
|
Also import patch to deal with buggy binutils
|
|
Update to upstream version '15.7'
with Debian dir f802105ae061241b13ab962854f56388092fc703
|
|
|
|
In https://github.com/rhboot/shim/issues/533 , iokomin noticed that
gas in binutils before 2.36 appears to be incorrectly concatenating
string literals in '.asciz' directives, including an extra NUL character
in between the strings, and this will cause us to incorrectly parse the
.sbatlevel section in shim binaries.
This patch adds test cases that will cause the build to fail if this has
happened, as well as changing sbat_var.S to to use '.ascii' and '.byte'
to construct the data, rather than using '.asciz'.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
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>
|
|
|
|
Remove all our patches, all upstream now
|
|
|
|
Update to upstream version '15.6'
with Debian dir 952ad3d5a92a2003f3496a79d1875a951c255396
|
|
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>
|
