| Age | Commit message (Collapse) | Author |
|---|
|
hughsie asked me if I can make shim tell userland what kinds of accesses
are allowed to the heap, stack, and allocations on the running platform,
so that these could be reported up through fwupd's Host Security ID
program (see https://fwupd.github.io/libfwupdplugin/hsi.html ).
This adds a new config-only (i.e. not a UEFI variable) variable
generated during boot, "/sys/firmware/efi/mok-variables/HSIStatus",
which tells us those properties as well as if the EFI Memory Attribute
Protocol is present.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This moves the EFI Memory Attribute Protocol helper functions to their
own file, since they're not related to PE things.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Currently when you've added a variable and not correctly changed the
test cases to match, you get a message like:
./test-mok-mirror
test-mok-mirror: setting variable sort policy to MOCK_SORT_DESCENDING
test-mok-mirror: setting delete policy to MOCK_VAR_DELETE_ATTR_ALLOW_ZERO
running test_mok_mirror_with_enough_space
test_mok_mirror_with_enough_space: passed
running test_mok_mirror_setvar_out_of_resources
check_config_table:232:mok.name[0] 72 != test.name[0] 0
check_config_table:232:Assertion `mok_entry->name[0] == mock_entry->name[0]' failed.
This adds another two lines:
test-mok-mirror: Failed on entry 4 mok.name:"HSIStatus" mock.name:""
test-mok-mirror: Entry is missing in expected variable list.
Or:
test-mok-mirror: Failed on entry 4 mok.name:"" mock.name:"HSIStatus"
test-mok-mirror: Entry is missing in found variable list.
Which will usually tell you which variable you forgot to add that's
present in test data, or what's missing in the test data and present
in the expected data.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This adds a member to the mok_state_variable struct to provide a
callback function for formatting external data. It basically has
snprintf()-like semantics for filling the buffer, but without the actual
printf-like formatting bits.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This adds a mok variable flag "MOK_VARIABLE_CONFIG_ONLY" to specify that
the data should be added to our UEFI config table, but shim should not
create a legacy UEFI variable.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This changes get_mem_attrs() to return EFI_UNSUPPORTED if
LibLocateProtocol() does not return an error but does give us a NULL
pointer.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Previously when there were no load options, this would go in the debug
log:
load-options.c:313:parse_load_options() full load options:
include/hexdump.h:92:vhexdumpf() hexdump of a NULL pointer!
This changes it to say:
load-options.c:315:parse_load_options() LoadOptions is empty
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This changes our debug and error logging to save the entire logs into
mok-variables as "shim-dbg.txt" and "shim-log.txt".
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This moves decls for errlog.c into errlog.h
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This fixes some minor errors with the testing of how ALIGN() and similar
are defined, and makes an explicit "ALIGN_UP()" macro to complement the
existing ALIGN_DOWN() macro.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
The CopyMem() introduced in "ad8692e avoid EFIv2 runtime services on
Apple x86 machines" copies 100 CHAR16s no matter what. NX enabled
firmware catches this and the boot breaks on those systems when the
value is smaller than that and it's up against a page boundary with a
page that's not mapped as readable.
https://uefi.org/specs/UEFI/2.10/04_EFI_System_Table.html says
that FirmwareVendor is a pointer to a NUL terminated string that
identifies the vendor that produces the system firmware for the platform.
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
This changes post-process-pe to give warnings, and optionally errors, if
a shim binary is built with Section Alignment or characteristics are not
compatible with NX, or if the EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT
flag is not set and require_nx_compat is true.
Co-authored-by: Peter Jones <pjones@redhat.com>
Co-authored-by: Kamil Aronowski <kamil.aronowski@yahoo.com>
Signed-off-by: Dennis Tseng <dennis.tseng@suse.com>
|
|
Revocation metadata has been consolidated into SbatLevel_Variable.txt and
can be delivered both built into shim as well as via revocations_sbat.efi
binaries. This adds a short text file describing how revocation levels
can be built into these components and delivered.
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
Since we can't read the directory, we can try to load
shim_certificate_[0..9].efi explicitly and give up after
the first one that fails to load.
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
While a revocations.efi binary can contain either SBAT revocations,
SkuSi revocations, or both, it is desirable to package them separately
so that higher level tools such as fwupd can decide which ones to put
in place at a given moment. This changes revocations.efi to
revocations_sbat.efi and revocations_sku.efi
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
Bugfix: In the netboot case revocations.efi files were read, but
processed as shim_certificate.efi files which is simply wrong.
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
Reading files during a netboot comes with the caveat that
fetching files from a network does not support anything
like listing a directory. In the past this has meant that
we do not try to open optional files during a netboot.
However at least the revocation.efi file is now tested
during a netboot, which will print an error when it is not
found. Since that error is spurious we should allow for
those errors to be suppressed.
This is also desirable since we will likely go looking for
additional files in the near future.
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
load_image() takes an optional parameter, DevicePath, in addition to the
SourceBuffer. Currently in shim_load_image() we don't check to see if
it's provided in the case where there's no SourceBuffer, even though it
can't work without it.
This adds that test and errors in that case, as well as avoiding
duplicating it when it's not present.
Signed-off-by: Mate Kukri <mate.kukri@canonical.com>
|
|
This isn't desirable when GRUB has control of the screen, and would mess
its content up.
Signed-off-by: Mate Kukri <mate.kukri@canonical.com>
|
|
When verifying an image, if we're "in" a shim protocol call, we require
the binary have an SBAT section. If it's not present we raise an
EFI_SECURITY_VIOLATION error code. Currently loader protocol's
load_image() is not marked as in protocol, so it instead will return
EFI_SUCCESS when verifying the SBAT section.
This patch changes that to be in protocol, so that SBAT will be required
on any images loaded with shim's loader protocol. This will bring SBAT
enforcement in-line with the shim_lock protocol.
Signed-off-by: Mate Kukri <mate.kukri@canonical.com>
|
|
Currently the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL and EFI_LOAD_FILE2_PROTOCOL
are supported.
Signed-off-by: Mate Kukri <mate.kukri@canonical.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This adds an implementation of Exit() and UnloadImage(), removes the
whole "loader_is_participating" mechanism and its supporting code, and
removes DISABLE_EBS_PROTECTION.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This moves some things around to help with loader protocol changes:
- Move replacements.c to loader-proto.c
- likewise with replacements.h
- move the SHIM_IMAGE_LOADER decl to loader-proto.h
- move the LoadImage / StartImage interface setup to an init function
- move shim_load_image() / shim_start_image() to loader-proto.c
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Define a new protocol for loading and starting images, encapsulating
shim's PE loading facilities and verification/authentication against the
same set of certificates that shim_lock::verify() authenticates against.
This removes the need for loaders like GRUB to implement their own PE
loader in order to be able to invoke loaded images as PE applications,
rather than implementing a bespoke OS dependent handover protocol (e.g.,
invoke Linux via its EFI stub)
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
|
|
Add the ability for shim to load an optional configuration file. This
new file is called "options.csv". The configuration file is completely
optional. If used, it is located in the same directory as the booted
shim. The "options.csv" file currently allows a single entry. Other
options could be added to it in the future. The first and only entry in
the file is the name of the secondary boot loader shim will load. The
"options.csv" file is in Unicode LE format.
This allows a signed shim to directly load a UKI without the need to
rename it to grub. Shim's transitive trust is maintained. If the
alternative 2nd stage can not be verified, it will not boot.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
|
|
Move some functions currently used within fallback that will later also
be used in shim.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
|
|
This adds documentation for the ENABLE_CODESIGN_EKU build option.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This commit also supersedes PR#232 which was closed on Jul 1, 2021.
So that original codesign EKU codes cannot be bothered.
To enable the codesign check, ENABLE_CODESIGN_EKU can be set to 1.
To disable the codesign check, ENABLE_CODESIGN_EKU can be set to 0
or just omit this flag.
For example:
make xxxx ENABLE_CODESIGN_EKU=1 xxxx shim.efi
Signed-off-by: Dennis Tseng <dennis.tseng@suse.com>
|
|
Also modify the ModSign EKU check to use VerifyEKUsInPkcs7Signature() to
check the signer certificate instead of the certificate directly from the
key database.
This commit supersedes the PR#232 and PR#661 (Apply the EKU checks) so
that author's original codes can be quite independent of other modification.
To answer the question in PR#232, author also changed the conditional
statement to EFI_Status != EFI_SUCCESS right after VerifyEKUsInPkcs7Signature()
in Cryptlib/Pk/CryptPkcs7Verify.c
Signed-off-by: Dennis Tseng <dennis.tseng@suse.com>
Signed-off-by: Gary Lin <glin@suse.com>
|
|
Comments to clarify that revocations should only be recorded
in SbatLevel_Variable.txt and not in any other header files.
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
Build changes to generate include/generated_sbat_var_defs.h from
SbatLevel_Variable.txt and use that header file. From here on
forward SbatLevel_Variable.txt should be the only place a new
revocation needs to be recorded.
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
This adds the utility program generate_sbat_var_defs, which can be used
to generate the sbar_var_defs.h header file from the human readable
SbatLevel_Variable.txt file.
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
Add revocations for
- January 2024 shim CVEs
- October 2023 grub CVEs
- Debian/Ubuntu (peimage) CVE-2024-2312
to SbatLevel_Variable.txt. This was missed when they were commited
to include/sbat_var_defs.h
Signed-off-by: Jan Setje-Eilers <Jan.SetjeEilers@oracle.com>
|
|
For multiple reasons, it may be useful for different keys to be used to
sign different parts of the boot chain (e.g. a different key for GRUB
and the Linux kernel). Allow this by loading concatenated
EFI_SIGNATURE_LISTs from shim_certificate.efi rather than only the
first.
At the same time, be a bit more robust by checking for allocation
failures and overflows due to invalid data in the binary.
Use the smaller of VirtualSize and SizeOfRawData since the latter is
rounded up to the section alignment and therefore may contain
non-certificate data.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
|
|
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
|
|
When memory attributes cannot be updated due to misalignment with 4K or
when the size is 0, the debug printout lacks sufficient clarity to
indicate the issue. To enhance troubleshooting, it is crucial to
generate an error log that explicitly states the failure to execute the
expected action. This error log will be visible even when the debug
level log is not enabled, thereby significantly reducing debugging time.
Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
|
|
Sometimes using a temp file to make test-random.h breaks builds because
clean and test get made in parallel.
Since debugging parallel make is anything but easy, it's better to just
not use the intermediate file. This patch makes it use a pipe instead.
Because we live in the worst possible world, we have to support RHEL 9,
in which xxd does not support "-n prefix", and so we have to build the
header variable definition lines in shell.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Only certain errors trigger fall back to the default loader name. This
change allows fall back when encountering `EFI_TFTP_ERROR` and
`EFI_HTTP_ERROR`.
This fixes the issue reported in
https://github.com/rhboot/shim/issues/649 that prevents boot on some
models of PC.
Signed-off-by: Nathan O'Meara <Nathan.OMeara@tanium.com>
|
|
This just turns off the clang-tidy warning about our SBAT_VAR_NAME
string compositing in the error message list in efi_main(), as it's the
only warning in the whole file and it's bugging me.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
If HandleProtocol or OpenVolume fails, the entries array will become
non-contiguous, i.e. will have NULL pointers between valid volume
names in the array. Because of that count_lines may return a lower
number of entries than expected. As a result one may not browse all
valid filesystems in the file explorer.
Add a second index variable that will increment only on successfully
created filesystem entries. As a result, count_lines should return
proper length and there won't be any lost partitions or accesses to
invalid entries.
Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>
|
|
In case GetInfo of volume root fails, it is still possible
to form a volume name from the DevicePath. Do not skip given
SimpleFS volume handle and try to form a name from DevicePath.
That way we do not lose some filesystems from file browser.
This change already fixes the problem of a hanging platform
when trying to enroll a key from disk. However, there is still
a chance of having a non-contiguous array of entries, which
will be fixed in next commit.
Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>
|
|
The loop retrieving the SimpleFS volume labels and names may
skip some volumes if either HandleProtocol or OpenVolume or
GetInfo fails. Those skipped volumes would have uninitialized
pointers to their names in the respective entries indices. This
would lead to accessing random memory in console_select, because
count_lines would not catch the holes with non-existing entries.
On affected platforms the result is a hang of the MokManager while
trying to enroll a key from disk. The issue has been triggered on
a TianoCore EDK2 UEFIPayload based firmware for x86 platforms with
additional filesystem drivers: ExFAT, NTFS, EXT2 and EXT4.
Use AllocateZeroPool to ensure entries array will be initialized
with NULL pointers. Handling the non-existing entries will be
added in subsequent commits.
Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>
|
|
In 70366a286552760863bacb521fb00c654586b494, I introduced a test case
for test-mok-mirror to test the behavior when SetVariable() gives
EFI_OUT_OF_RESOURCES. Unfortunately this includes a memory error in its
cleanup functions:
==1972634== Invalid read of size 8
==1972634== at 0x4032F3: mock_sv_adjust_usage_data (mock-variables.c:468)
==1972634== by 0x40387B: mock_delete_variable (mock-variables.c:541)
==1972634== by 0x4014E8: mock_reset_variables (mock-variables.c:1353)
==1972634== by 0x401348: mock_finalize_vars_and_configs (mock-variables.c:1405)
==1972634== by 0x401731: main (test-mok-mirror.c:688)
==1972634== Address 0x1ffeffc4f8 is on thread 1's stack
==1972634== 6784 bytes below stack pointer
This is caused because the test data uses a linked-list data structure
in an array in the test data, and we try to iterate the list to free the
data, but of course the list pointers aren't initialized because it's an
array.
Whoops.
This patch makes it so we don't try to clean up that list, because we
don't need to.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This adds another test case for mirroring mok variables - in this case
there is arbitrarily not enough space in the SetVariable() storage
backend for some or all of the variables, and so some variables are
mirrored to both variables and the config space storage, and some only
to the config space storage.
Currently the test only ensures that the out-of-resources condition on
the variable storage is handled correctly; we don't (yet) test the
config space. That's an avenue for future work.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This makes a generic test_mok_mirror() which can validate the result of
(hopefully) any run of any result of import_mok_state(), and changes
test_mok_mirror_0() to use it, and also documents what's being tested
and the results it expects to see.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This splits up the API for setting default usage limits, adding a test
API function to set alternate usage limits.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
When SetVariable() is called, currently the test for being out of space
is correct, but the accounting is backwards - success adds more space
available.
This fixes the accounting and (with SHIM_DEBUG) logs the variable size.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
CertList->SignatureSize is of type UINT32 which is always positive.
If CertList->SignatureListSize == 0, then
CertList->SignatureListSize <= CertList->SignatureSize
is also true.
Remove the redundant CertList->SignatureListSize == 0 checks.
A message "Corrupted signature list" is better suited then
"Invalid MOK detected! Ignoring MOK List." in this case.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
|
|
The extend operation still occurs even if `*_log_extend_event`
returns EFI_VOLUME_FULL.
Let's print a warning when we first see this error code, but otherwise
continue booting.
Bailing on this condition has caused machines with limited event log
space to become unbootable with TPM 2.0 enabled. (fixes #654)
Signed-off-by: Mate Kukri <mate.kukri@canonical.com>
|
