| Age | Commit message (Collapse) | Author |
|---|
|
Network booting tends to expose things like a tfpt server
as a filesystem that doesn't implement directory listing
This will blindly try to ingest a revocations.efi file in
those cases, even if that may result in some console noise
when the file does not exist.
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
The netboot path up until now hardcodes DEFAULT_LOADER as
the only possible filename to load. This is pretty limiting
and needs to be fixed.
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
The ability to automatically apply SBATLevel revocations varies
from distro to distro. This allows distros that are able to
automatically apply SBATLevel revocations when shim is updated to
select a level by supplying SBAT_AUTOMATIC_DATE=<datestamp> on the
make command line. Currently the following options are available:
2021030218 no revocations - useful for distros that need to rely on
an externally delivered revocations.efi
2022052400 grub,2
2022111500 shim,2
grub,3
2023012900 shim,2
grub,3
grub.debian,4
If no datestamp is specified the build will default to the
most recent 2023012900.
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
When the term previous was introduced for revocations to be
automatically applied there was a hope that everytime a new
revocation was built into shim, the previous revocation could
be applied automatically. Further experience has shown the
real world to be more complex than that. The automatic payload
will realistically contain a set of revocations governed by
both the cadence at which a distro's customer base updates
as well as the severity of the issue being revoked.
This is not a functional change.
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
GCC 4 doesn't have __builtin_add_overflow() and friends, so this results
in a compiler error.
On platforms using that version, do the arithmetic without it.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Since shim is inherently updated by shipping a new shim, the
latest built in revocations can include the most recent shim
revocations. Since CVE-2023-40547 is high impact, this revocation
should be available to everyone as soon as possible.
GRUB2 CVE-2023-4692 and CVE-2023-4693 are in the ntfs module that
only some vendors ship. Since some vendors did not ship an updated
GRUB2 for these issues, the revocation for these CVEs is not
included in the payload at this time.
Signed-off-by: Jan Setje-Eilers <jan.setjeeilers@oracle.com>
|
|
Before applying an updated SbatLevel shim should re-run
introspection and never apply a revocation level that would
prevent the currently running shim from booting. The proper
way forward is to update shim first.
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
This adds support for applying SkuSiPolicy UEFI BS variables. These
varaibles are needed for non-dbx based Windows revocations and are
described here:
https://support.microsoft.com/en-us/topic/kb5027455-guidance-for-blocking-vulnerable-windows-boot-managers-522bb851-0a61-44ad-aa94-ad11119c5e91
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
Ingest SBAT Levels from revocations binary thereby allowing level
requirements to be updated independently from shipping a new shim.
Do not automatically apply any revocations from a stock shim at
this point.
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
We need to do arithmetic on untrusted values sometimes, so this patch
adds the following primitives as macros that wrap the compiler builtins.
bool checked_add(TYPE addend0, TYPE addend1, TYPE *sum)
bool checked_sub(TYPE minuend, TYPE subtrahend, TYPE *difference)
bool checked_mul(TYPE factor0, TYPE factor1, TYPE *product)
And also the following primitive which returns True if divisor is 0 and
False otherwise:
bool checked_div(TYPE dividend, TYPE divisor, TYPE *quotient)
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
With "gcc -fanalyzer" and "scan-build", it's convenient to be able to
continue even though the compiler has returned error on one or more
source files.
This makes it so compiler errors are ignored in some of those cases.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
shim takes several forms of input from several sources that are not
necessarily trustworthy. As such, we need to take measures to validate
that we don't have unacceptable results from bad inputs. One such
measure is "fuzzing" the inputs which parse untrusted data by running
them with randomized or partially randomized input.
This change adds such testing using clang's "libFuzzer" to our parser
for ".sbat" sections. I've run it for about half an hour and so far it
found one memory leak, but no other errors.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
shim takes several forms of input from several sources that are not
necessarily trustworthy. As such, we need to take measures to validate
that we don't have unacceptable results from bad inputs. One such
measure is "fuzzing" the inputs which parse untrusted data by running
them with randomized or partially randomized input.
This change adds such testing using clang's "libFuzzer" to our CSV
parser. I've run this on 24-cores at 4GHz for half an hour, and so far
each fuzzer has converged on 79% coverage. I expect the 21% that's not
getting covered are the EFI API mock interfaces we're building in from
test.c and similar. So far no errors have been found, which is what was
expected since this particular API is being manually fuzzed with ~8kB of
/dev/urandom on every build since 2021-02-23.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This adds a test case for our address sanitation checking function
ImageAddresS(). In doing so it addresses two issues:
- previously we allowed the address after the last byte of the image to
be computed (may need to revert this or fix some callers, we'll see...)
- bespoke overflow checking and using + directly instead of using
__builtin_add_overflow()
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This moves the parts of pe.c that *don't* depend on Cryptlib into
pe-relocate.c, so we can write test cases for them without having to
make a second openssl build without EFI support.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
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>
|
