| Age | Commit message (Collapse) | Author |
|---|
|
In a few places we put dprintf() at places where the compiler will get
confused if it isn't a block or a statement.
Obviously, it should be a statement, so this makes it one.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
The function msleep uses gBS->Stall which waits for a specified number
of microseconds.
Reference: https://edk2-docs.gitbook.io/edk-ii-uefi-driver-writer-s-guide/5_uefi_services/51_services_that_uefi_drivers_commonly_use/517_stall
This reference even mentions an example sleeping for 10 microseconds: // Wait 10 uS. Notice the letter 'u'.
Therefore it's a good idea to call the function 'usleep' rather than
'msleep', so no one confuses it with milliseconds, and to change the
argument name to match as well.
Signed-off-by: Kamil Aronowski <kamil.aronowski@yahoo.com>
|
|
In preparation for renaming msleep() to usleep(), in some cases tests
were failing due to a mismatch between our declaration of the usleep()
function and what is being provided by unistd.h. This change simply
makes our function declared only when not in a unit test environment.
Signed-off-by: Kamil Aronowski <kamil.aronowski@yahoo.com>
|
|
(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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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 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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
Within previous versions of shim the MokListTrusted var did not
exist. The user had to opt in to using the feature.
Change the default behavior to an opt out model. Since old
shims will not have the BS MokListTrusted set, use inverse
logic that sets the MokListTrustedRT to 1 when the boot
service variable is missing.
Many Linux distros carry out of tree patches to trust the mok
keys by default. These out of tree patches can be dropped
when using a Linux kernel that supports MokListTrustedRT.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
|
|
On Debian(-derived) systems low-level system headers are under
/usr/include/<multi-arch path>, so look there too.
Otherwise we see stuff like:
gcc -O2 -fno-diagnostics-color -ggdb -std=gnu11 -isystem <foo>/shim.git/include/system -I<foo>/shim.git/gnu-efi/inc -I<foo>/shim.git/gnu-efi/inc/ia32 -I<foo>/shim.git/gnu-efi/inc/protocol -Iinclude -iquote . -isystem /usr/include -isystem /usr/lib/gcc/i686-linux-gnu/11/include -mno-mmx -mno-sse -mno-red-zone -nostdinc -maccumulate-outgoing-args -m32 -DMDE_CPU_IA32 -DPAGE_SIZE=4096 -fshort-wchar -fno-builtin -rdynamic -fno-inline -fno-eliminate-unused-debug-types -fno-eliminate-unused-debug-symbols -gpubnames -grecord-gcc-switches -Wall -Wextra -Wno-missing-field-initializers -Wsign-compare -Wno-deprecated-declarations -Wno-unused-but-set-variable -Wno-unused-variable -Wno-pointer-sign -Werror -Werror=nonnull -Werror=nonnull-compare -DEFI_FUNCTION_WRAPPER -DGNU_EFI_USE_MS_ABI -DPAGE_SIZE=4096 -DSHIM_UNIT_TEST -DDEFAULT_DEBUG_PRINT_STATE=0 -isystem include-fixed -o test-csv csv.c test-csv.c test.c libefi-test.a -lefivar
In file included from /usr/include/bits/errno.h:26,
from /usr/include/errno.h:28,
from /usr/include/efivar/efivar.h:24,
from include/test.h:51,
from shim.h:68,
from csv.c:6:
/usr/include/linux/errno.h:1:10: fatal error: asm/errno.h: No such file or directory
1 | #include <asm/errno.h>
| ^~~~~~~~~~~~~
compilation terminated.
In file included from /usr/include/bits/errno.h:26,
from /usr/include/errno.h:28,
from /usr/include/efivar/efivar.h:24,
from include/test.h:51,
from shim.h:68,
from test-csv.c:9:
/usr/include/linux/errno.h:1:10: fatal error: asm/errno.h: No such file or directory
1 | #include <asm/errno.h>
| ^~~~~~~~~~~~~
compilation terminated.
In file included from /usr/include/bits/errno.h:26,
from /usr/include/errno.h:28,
from /usr/include/efivar/efivar.h:24,
from include/test.h:51,
from shim.h:68,
from test.c:7:
/usr/include/linux/errno.h:1:10: fatal error: asm/errno.h: No such file or directory
1 | #include <asm/errno.h>
| ^~~~~~~~~~~~~
compilation terminated.
Signed-off-by: Steve McIntyre <steve@einval.com>
|
|
This implements SBAT verification via the shim_lock protocol
by moving verification inside the existing verify_buffer()
function that is shared by both shim_verify() and handle_image().
The .sbat section is optional for code verified via the shim_lock
protocol, unlike for code that is verified and executed directly
by shim. For executables that don't have a .sbat section,
verification is skipped when using the protocol.
A vendor can enforce SBAT verification for code verified via the
shim_lock protocol by revoking all pre-SBAT binaries via a dbx
update or by using vendor_dbx and then only signing binaries that
have a .sbat section from that point.
Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
|
|
- one missing free
- one minor deadcode issue
- two unchecked allocations
- one debug hexdump of a variable we just freed
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
cov-analysis-linux64-2020.09 is a lot more successful than the older
versions at building, but it still has some... issues. Among them, it
is of the belief that this:
void
foo(char *fmt, ...)
{
__builtin_va_list ap;
__builtin_ms_va_start(ap, fmt); /* <- here */
...
}
is an uninitialized use of "ap".
This patch adds defined(__COVERITY__) to the list of criteria for using
sysv va lists, which it has no such confusion about.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Several places in e.g. MokManager and our console library use
ST->ConOut->ClearScreen directly, without checking for the existence of
a console output device.
This patch adds function to our console library to do that correctly,
instead of using the bug-prone ad hoc implementation everywhere.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Copying the value of datasize_in to two further variables and then using
all three randomly in the code makes it hard to read.
datasize_in is never changed in generate_hash() so we can do with this
parameter alone. Rename it to datasize.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
|
|
On Arch Linux, the GCC version of "limits.h" is in the "include-fixed" instead
of the "include" directory. It needs to be included in the include directories
list, otherwise attempting to compile the test suite fails with the following
error:
In file included from /usr/include/efivar/efivar-dp.h:22,
from /usr/include/efivar/efivar.h:238,
from include/test.h:51,
from shim.h:68,
from csv.c:6:
/usr/include/limits.h:124:16: fatal error: limits.h: No such file or directory
124 | # include_next <limits.h>
| ^~~~~~~~~~
compilation terminated.
|
|
Currently valgrind shows a minor issue which is not introduced in this
patch series:
==2595397==
==2595397== HEAP SUMMARY:
==2595397== in use at exit: 16,368 bytes in 48 blocks
==2595397== total heap usage: 6,953 allocs, 6,905 frees, 9,146,749 bytes allocated
==2595397==
==2595397== 16,368 bytes in 48 blocks are definitely lost in loss record 1 of 1
==2595397== at 0x4845464: calloc (vg_replace_malloc.c:1117)
==2595397== by 0x4087F2: mock_efi_allocate_pool (test.c:72)
==2595397== by 0x4098DE: UnknownInlinedFun (misc.c:33)
==2595397== by 0x4098DE: AllocateZeroPool (misc.c:48)
==2595397== by 0x403D40: get_variable_attr (variables.c:301)
==2595397== by 0x4071C4: import_one_mok_state (mok.c:831)
==2595397== by 0x4072F4: import_mok_state (mok.c:908)
==2595397== by 0x407FA6: test_mok_mirror_0 (test-mok-mirror.c:205)
==2595397== by 0x4035B2: main (test-mok-mirror.c:378)
==2595397==
==2595397== LEAK SUMMARY:
==2595397== definitely lost: 16,368 bytes in 48 blocks
==2595397== indirectly lost: 0 bytes in 0 blocks
==2595397== possibly lost: 0 bytes in 0 blocks
==2595397== still reachable: 0 bytes in 0 blocks
==2595397== suppressed: 0 bytes in 0 blocks
==2595397==
This is because we're doing get_variable_attr() on the same variable
more than once and saving the value to our variables table. Each
additional time we do so leaks the previous one.
This patch solves the issue by not getting the variable again if it's
already set in the table, and adds a test case to check if we're doing
get_variable() of any variety on the same variable more than once.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Test that our mok mirroring doesn't ever try to delete any variable that
it has previously created, and that it properly mirrors at least
MokList, MokListX, and SbatLevel, at least when variables actually work.
These tests will fail (rather a lot) without 7f64fd6da9458b73c4.
Currently valgrind shows a memory leak in this code which is not
introduced in this patch series. Since all of our memory is freed on
Exit() or when kernel does ExitBootServices(), this doesn't have any
significant repercussions.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This adds a simple implementation of ST->ConfigurationTable,
ST->NumberOfTableEntries, and BS->InstallConfigurationTable to our test
harness.
Currently it is limited at 1024 entries, but that should be well more
than enough for any tests we've currently considered.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Some tests will need variables, and so we need a mock implementation of
the various calls relating to them.
This patch adds implementations for the EFI Runtime Services calls
GetVariable(), SetVariable(), GetNextVariableName(), and
QueryVariableInfo(). Additionally, it enforces tunable limits on
storage for variables, and (with only a little work) the limits can be
different for SetVariable() vs what is returned by QueryVariableInfo().
That is, it can lie to you like real systems do.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This adds more mock functions that just return various EFI error codes
in the EFIAPI ABI.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This adds a make target that builds the tests with gcov so we can
identify coverage gaps in the test suite.
It also makes a special test-lto invocation, so that a developer can run
these tests with the somewhat different optimization results LTO will
have.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
When writing new tests, if we get to the point where we have to use
libefivar for something, it's very common that I accidentally link it in
twice. When that happens, I typically spend an unfortunate amount of
time staring at FLTO's mangled names before I figure out what I've done
wrong.
This patch makes all the tests link against libefivar, thereby
avoiding the issue.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
None of this should ever actually get called when we're running any of
the unit tests we've got, but some older compilers (i.e. Centos 7's gcc)
fail to remove some of the intermediate functions, and that causes a
link error with the functions they call.
This patch makes the top level call go away as well, so that the
intermediates never have linkage to the underlying implementation
functions.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This lets us access the definitions for this structure, and the data
being used at runtime, from unit tests.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
5f08e671e4eb introduced a CompareGuid() call in the unit test harness,
but unfortunately it has a typo and thus only ever compares the first
pointer-sized word of the guid. With 4-GUIDs, this will usually produce
the correct results; with 1-GUIDs it often won't.
A second issue is that the memcmp() implementation of CompareGuid()
produces a different sort order than comparing field-by-field, and also
a different sort order than comparing the string representation. This
is often not a problem (edk2, for example, never compares anything
except equality of two GUIDs), but when writing test cases it is
extremely helpful to be able to look at a list that is sorted in an
intuitive order.
This patch introduces a guidcmp() function in the test suite, which
compares the binary data in the same order that comparing the two GUIDs'
string representations would.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This makes sure we clean up the builds that aren't for the EFI
environment after we build and run the unit tests.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
A couple of places snuck in where building with COMPILER=clang didn't
work right; this makes them work again.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
There is no need to check the parameters of strntoken() twice.
Fixes: c7bb10cf154a ("Tidy up our string primitives...")
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
|
|
This adds tests for all the cases we've documented in the
set_second_stage() comments. Each test checks that all of second_stage,
loader_str, and loader_str_size are set correctly.
Note that this adds a dependency on libefivar to build device paths to
test against.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This moves set_second_stage() and some of the helper functions it uses
out of shim.c, so that it's easier to write test cases for.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Some tests have some complex flows, and it's useful to be able to see
the call path when there's a failure.
This patch adds a very simple traceback printer, along with changing the
test build arguments to include more debug information.
The result you get from this traceback printer just gives you a function
name and the index into its .txt content, so to use it for more than
"which function calls which", you'll need to use eu-addr2line with the
output.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This test helper was conspicuously missing, so this patch just adds it
at the obvious place.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
In some test cases, it may be useful to call libefi.a functions, such as
the device path parsing functions, which allocate pages via
BS->AllocatePool() or BS->AllocatePages.
This patch ads a simple mock implementation of those functions, as well
as the EFI_SYSTEM_TABLE, EFI_BOOT_SERVICES, and EFI_RUNTIME_SERVICES
variables *ST, *BS, and *RT (respectively), and initializes them before
the test cases run.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
