| Age | Commit message (Collapse) | Author |
|---|
|
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>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Many ASRock boards will not render MokManager correctly if the Unicode
Box Drawing characters are used.
Signed-off-by: Tony Persson <tony@tonypersson.se>
|
|
Now that we've got "objcopy --target efi-app-aarch64" and similar, we
don't have to go through heroic effort to try to make aarch64 builds
work.
This patch updates to a gnu-efi branch that has newer aarch64 crt0 code,
and makes efi_aarch64_efi.lds be nearly identical to efi_x86_64_efi.lds.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This is the standard Contributor Covenant.
See-also: https://www.contributor-covenant.org/
Signed-off-by: Robbie Harwood <rharwood@redhat.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>
|
|
* test_strncpy(). The test intentionally copies less than the total length
of the string, so it looks like there is nothing we can do about it in
the test other than explicitly ignoring the warnings.
inlined from ‘test_strncpy’ at test-str.c:943:2:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’
output may be truncated copying 1 byte from a string of length 8191 [-Werror=stringop-truncation]
106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...
* test_strcat(). The fix for the warning below is to avoid using overlapping
buffers in src and dst. This is actually a requirement for both strcat() and
strncpy(), so fix both functions.
inlined from ‘test_strcat’ at test-str.c:1052:2:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:128:10: error: ‘__builtin___strcat_chk’
accessing 4097 or more bytes at offsets 4096 and 0 may overlap 1 byte at offset 4096 [-Werror=restrict]
128 | return __builtin___strcat_chk (__dest, __src, __bos (__dest));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...
Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com>
|
|
$ make test
...
mock-variables.c: In function 'mock_sv_attrs_match':
mock-variables.c:448:23: error: conversion from 'long unsigned int' to 'UINT32'
{aka 'unsigned int'} changes value from '18446744073709551551' to '4294967231' [-Werror=overflow]
448 | UINT32 mask = ~EFI_VARIABLE_APPEND_WRITE;
| ^
cc1: all warnings being treated as errors
Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.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>
|
|
With -Werror these were causing build failures with stricter gcc:
.../post-process-pe.c: In function 'load_pe':
.../post-process-pe.c:177:55: error: format '%ld' expects argument of type 'long int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
.../post-process-pe.c:192:56: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'UINT64' {aka 'long long unsigned int'} [-Werror=format=]
.../post-process-pe.c:236:31: error: format '%zu' expects argument of type 'size_t', but argument 2 has type 'UINT64' {aka 'long long unsigned int'} [-Werror=format=]
.../post-process-pe.c:39:32: note: in definition of macro 'debug'
.../post-process-pe.c:236:60: note: format string is defined here
.../post-process-pe.c:240:31: error: format '%zu' expects argument of type 'size_t', but argument 2 has type 'UINT64' {aka 'long long unsigned int'} [-Werror=format=]
.../post-process-pe.c:39:32: note: in definition of macro 'debug'
.../post-process-pe.c:240:60: note: format string is defined here
.../post-process-pe.c:274:30: error: format '%zu' expects argument of type 'size_t', but argument 2 has type 'UINTN' {aka 'long unsigned int'} [-Werror=format=]
.../post-process-pe.c:39:32: note: in definition of macro 'debug'
.../post-process-pe.c:274:34: note: format string is defined here
Signed-off-by: Steve McIntyre <steve@einval.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
|
|
Set fail-fast to false to get a more useful picture of results.
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Remove all our patches, all upstream now.
|
