| Age | Commit message (Collapse) | Author |
|---|
|
When we've split a mok variable into name0..nameN, we re-use the same
buffer over and over, but we never free it at the end.
This frees it.
Resolves: Coverity CID 457501
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
If measuring a mok variable to the TPM returns failure, this function
returns, but never frees the data intended to be measured.
This frees it.
Resolves: Coverity CID 457503
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Coverity identified a resource leak of namen8 in mirror_mok_db(), and
upon further examination nothing is even using it any more; we're
allocating it and populating it for nothing.
This removes all that.
Resolves: Coverity CID 457510
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
In d972515e608e ("Save the debug and error logs in mok-variables") had a
few deficiencies: 1) the size of the result table isn't correctly
computed when either errlog or dbglog is 0 sized (much more likely for
the former), 2) when we save the error log we leak the allocation for
the previous mok variables, and 3) original mok variables were allocated
with AllocatePages(), but the new ones were allocated with
AllocateZeroPool(). The former guarantees page alignment, which we want
here.
This fixes all three of these.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
hughsie asked me to also make it observable at runtime whether the shim
binary that was used to boot was set as NX_COMPAT or not.
This adds that into the HSIStatus data as "shim-has-nx-compat-set".
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This adds three more entries to our HSI data:
has-dxe-services-table: technically only tells us if UEFI's
LocateProtocol will give us a DXE services
table, but practically also tells us if the
machine is implementing DXE in any way.
has-get-memory-space-descriptor: tells us if DXE->GetMemorySpaceDescriptor
is populated
has-set-memory-space-descriptor: tells us if DXE->SetMemorySpaceDescriptor
is populated
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Some machines have EFI Boot Services variables but not Runtime
variables, and thus it can be quite difficult to figure out what's going
on once the system is booted.
This changes mok variable mirroring to also mirror the following
variables to the mok variable config table:
AuditMode
BootOrder
BootCurrent
BootNext
Boot0000
Boot0001
Boot0002
Boot0003
Boot0004
Boot0005
Boot0006
DeployedMode
SecureBoot
SetupMode
SignatureSupport
Timeout
PK
KEK
db
dbx
Kernel_SkuSiStatus
There's no attempt to do anything involving creating runtime or
boot-services only variables, it just mirrors them into the config
table so they'll be exposed there.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Previously the mok mirror state flags were only used in the mok
mirroring code. But there are other consumers of that data, namely our
variable test cases, and it's useful for them to be able to check the
flags.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
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 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>
|
|
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>
|
|
If MokManager has to be entered but system is booting on disk on
EFI/BOOT/BOOTx.EFI entry, MokManager cannot be found because it's not in
that directory.
This indicates an issue with the BootOrder or the UEFI firmware is just
not taking BootOrder into account (seen on Lenovo ThinkPad P1 Gen 6 and
VMWare), or that the boot media has incorrectly been created without
MokManager.
This patch prints a related message and reboots after 10 seconds.
Reproducer:
1. Import a certificate using mokutil
2. Tell UEFI to boot on BOOTX64.EFI entry on next boot
Result without the patch with verbosity:
-----------------------------------------------------------------------
mok.c:1045:import_mok_state() checking mok request
shim.c:866:load_image() attempting to load \EFI\BOOT\mmx64.efi
Failed to open \EFI\BOOT\mmx64.efi - Not Found
Failed to load image \EFI\BOOT\mmx64.efi: Not Found
shim.c:888 load_image() Failed to open \EFI\BOOT\mmx64.efi - Not Found
shim.c:1115 read_image() Failed to load image \EFI\BOOT\mmx64.efi: Not Found
Failed to start MokManager: Not Found
mok.c:1047:import_mok_state() mok returned Not Found
Something has gone seriously wrong: import_mok_state() failed: Not Found
-----------------------------------------------------------------------
Signed-off-by: Renaud Métrich <rmetrich@redhat.com>
|
|
While booting a MacBookPro15,2 (the last Intel model, 2019), shim 15.8
gets stuck in RT->QueryVariableInfo(). Previously, these devices
shipped with EFI firmware version 1.10, and we had a quirk in place for
this (#364). However, Apple updated the firmware to version 2.40, but
it still doesn't implement runtime services.
This patch adds a test for Apple as the vendor, and treats that as
equivalent to having an older major UEFI version.
Signed-off-by: Eduard Acatrinei <eduard.acatrinei@certussoftware.ro>
|
|
On some ARM platform, jlinton noticed that when we fail to set a
variable (because it isn't supported at all, presumably), our error
message has an extra argument that doesn't match the format string.
This patch removes the extra argument.
Resolves: CVE-2023-40546
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
The code that mirrors MOK database to EFI variables gets the remaining
variable storage size from the firmware and subtracts the size needed
for any overhead to see if there is enough space to create a new entry.
However these calculations are on unsigned integer types, they can
underflow and result in huge values when the firmware is about to run
out of usable variable space. Explicitly check against this.
Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
Currently, when you boot linux you get a bright red message in the log
and the console like:
Feb 03 13:18:45 localhost.localdomain kernel: mokvar: EFI MOKvar config table is not in EFI runtime memory
We don't like bright red messages on the console, so this patch changes
the memory allocation for the mokvar config table so that it's in
runtime memory.
Signed-off-by: Peter Jones <pjones@redhat.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>
|
|
Introduce a new MOK variable called MokListTrustedRT. It allows an end-user
to decide if they want to trust MOKList keys within the soon to be booted
Linux kernel. This variable does not change any functionality within shim
itself. When Linux boots, if MokListTrustedRT is set and
EFI_VARIABLE_NON_VOLATILE is not set, keys in MokListRT are loaded into the
.machine keyring instead of the .platform keyring.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
|
|
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>
|
|
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>
|
|
This just makes one less thing we have to make sure is the same between
the test harnesses and the runtime code.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
For the firmware without the variable writing issues, MOK variables are
mirrored when only_first=TRUE. However, LibDeleteVariable() was called
in maybe_mirror_one_mok_variable() when only_first=FALSE, and this
could delete MOK variables that were just mirrored in the first round.
This bug was hidden since LibDeleteVariable() deletes BS+RT+NV variables
while we mirror MOK variables as BS+RT, and the firmware refused to
delete the mirrored MOK variable due to mismatching attributes. However,
some firmwares, such as VMWare, didn't enforce the attribute check and
just deleted the variables with matched name and GUID. In such system,
MokListRT was always removed before it reached OS.
Fixes: https://github.com/rhboot/shim/issues/386
Signed-off-by: Gary Lin <glin@suse.com>
|
|
Some UEFI environment such as u-boot doesn't implement
QueryVariableInfo(), so we couldn't rely on the function to estimate the
available space for RT variables. All we can do is to call SetVariable()
directly and check the return value of SetVariable().
Signed-off-by: Gary Lin <glin@suse.com>
|
|
Fix the case where data_size is 0, so config_template is
not implicitly copied like the size calculation above.
upstream-status: https://github.com/rhboot/shim/issues/249
Signed-off-by: Jonathan Yong <jonathan.yong@intel.com>
|
|
The EFI 1.10 spec (and presumably earlier revisions as well) didn't have
RT->QueryVariableInfo(), and on Chris Murphy's MacBookPro8,2 , that
memory appears to be initialized randomly.
This patch changes it to not call RT->QueryVariableInfo() if the
EFI_RUNTIME_SERVICES table's major revision is less than two, and
assumes our maximum variable size is 1024 in that case.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Linux kernel is picky when reserving the memory for x86 and it only
expects BootServicesData:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/platform/efi/quirks.c?h=v5.11#n254
Otherwise, the following error would show during system boot:
Apr 07 12:31:56.743925 localhost kernel: efi: Failed to lookup EFI memory descriptor for 0x000000003dcf8000
Although BootServicesData would be reclaimed after ExitBootService(),
linux kernel reserves MOK config table when it detects the existence of
the table, so it's fine to allocate the table as BootServicesData.
Signed-off-by: Gary Lin <glin@suse.com>
|
|
In 65be350308783a8ef537246c8ad0545b4e6ad069, import_mok_state() is split
up into a function that manages the whole mok state, and one that
handles the state machine for an individual state variable.
Unfortunately, the code that initializes the global ignore_db and
user_insecure_mode was copied from import_mok_state() into the new
import_one_mok_state() function, and thus re-initializes that state each
time it processes a MoK state variable, before even assessing if that
variable is set. As a result, we never honor either flag, and the
machine owner cannot disable trusting the system firmware's db/dbx
databases or disable validation altogether.
This patch removes the extra re-initialization, allowing those variables
to be set properly.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
|
|
There are multiple issues in the MOK variable mirroring code due
to volatile variable size constraints, which all result in boot
failures:
- If a signature is encountered which doesn't fit in to a single
variable, the code enters an infinite loop because the cursor
isn't advanced in mirror_mok_db() after the call to
mirror_one_esl().
- If an ESL is encountered which doesn't fit in to a single
variable, it looks like the intention is for the ESL to be split
across multiple variables. However, mirror_one_esl() will write
the maximum variable size on each call, regardless of how much
data is remaining for the current ESL. If the size of a ESL isn't
a multiple of the maximum variable size, the final call to
mirror_one_esl() will append data from the start of the next
ESL and the cursor in mirror_mok_db() will be advanced to an
arbitrary location in the next ESL. This either results in garbage
being mirrored (if you're lucky), or in my case - another infinite
loop as it appears to encounter a signature that doesn't fit in to
a single variable.
- If no signatures can be mirrored when mirror_mok_db() is called
with only_first=TRUE, it tries to create a variable with a single
SHA256 signature in it. But mirror_mok_db() returns an error
(EFI_INVALID_PARAMETER) regardless of whether this succeeds.
|
|
If the bootservices MOK payload fits in to a single volatile
runtime variable, don't create additional mirrored variables in
the second pass of mirror_mok_db().
|
|
The MOK variable mirroring makes use of variable_create_esl, which
can only create a well-formed EFI_SIGNATURE_LIST containing a single
signature. Fix fill_esl and variable_create_esl to support creating
a EFI_SIGNATURE_LIST with one or more supplied EFI_SIGNATURE_DATA
structures.
Introduce variable_create_esl_with_one_signature and
fill_esl_with_one_signature for code that does want to create a
EFI_SIGNATURE_LIST containing a single signature constructed from
a supplied signature data buffer and owner GUID.
|
|
This makes it so that if you build with ENABLE_SHIM_DEVEL, the SBAT we
use is named SBAT_DEVEL instead of SBAT, and it's expected to have
EFI_VARIABLE_RUNTIME_ACCESS set.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This fixes ENABLE_SHIM_DEVEL to actually work, and also makes our "goto
die" failure behavior change (to wait considerably longer) based on it.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This re-structures our includes so we can be sure everything is always
including all the system headers in a uniform, predictable way.
Temporarily it also adds a bunch of junk at all the places we use
variadic functions to specifically pick either the MS (cdecl) or ELF
ABIs.
I'm not 100% sure that's all correct (see later patch) but it's enough
to allow this to build.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This fixes the ifndef guard on NONNULL and __CONCAT3 and adds
definitions for:
- __CONCAT() for a##b with the intermediate tokenization step
- ALLOCFUNC for __malloc__
- DEPRECATED for __deprecated__
- PURE for __pure__
- RETURNS_NONNULL for __nonnull__
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This adds SBAT to our table of variables to mirror with our MoK state.
Currently it mirrors "SBAT" to a variable named "SbatRT", both using the
SHIM GUID.
Currently we enforce the current policy WRT these variables:
- we always delete SbatRT if it's present, for a couple of reasons:
- If we got here either something created it before us during boot,
which isn't a thing we believe anything should be doing, or it's an
NV variable, which it shouldn't be.
- we want to raise the error if it's NV+Authenticated
- we always delete SBAT (and do not mirror it) if it either
- doesn't have BS|NV set or
- does have RT set
- we're requiring !RT because we can't actually tell if it's an
authenticated variable or not, and we want to get the error if RT
is set and it is authenticated, because that means we've lost the
race between us and an attacker to create it.
- we always measure SBAT into PCR7 and add a log extension with the
measured hash
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Add a pile of documentation comments to help remember what all the
fields in our mok mirroring structure do.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
The license statements in our source files were getting to be a giant
mess, and mostly they all just say the same thing. I've switched most
of it to SPDX labels, but left copyright statements in place (where they
were not obviously incorrect copy-paste jobs that I did...).
If there's some change here you don't think is valid, let me know and
we can fix it up together.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
In the version of clang-format I've got locally[0],
WhitespaceSensitiveMacros seems to only work sometimes. That means that
if we ever run it on some particular things, it could seriously mess up
a bunch of our debugging output. That's not great.
In this patch, I've gone ahead and run clang-format on all the macros
that use __LINE__, which are the obvious places this is dangerous, and
then audited the result and fixed anything that's broken (including a
couple of places where it was already broken.)
[0] random:~/devel/github.com/shim/clang-format$ clang-format --version
clang-format version 11.0.0 (Fedora 11.0.0-2.fc33)
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
The code currently computes the size of the MoK variable in ram and
rounds up to a full page, but then actually allocates the exact size,
rather than the rounded up version. This should be completely safe, but
the intent was to round up to at least the page size boundary, and to
always guarantee rounding up /some/, to ensure extra 0-bytes at the end
of the buffer.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Everything was going just fine until I made a vendor_db with 17kB of
sha256 sums in it. And then the same source tree that had worked fine
without that threw errors and failed all over the place. I wrote some
code to diagnose the problem, and of course it was a failure in
mirroring MokList to MokListRT.
As Patrick noted in 741c61abba7, some systems have obnoxiously low
amounts of variable storage available:
mok.c:550:import_mok_state() BS+RT variable info:
MaximumVariableStorageSize:0x000000000000DFE4
RemainingVariableStorageSize:0x000000000000D21C
MaximumVariableSize:0x0000000000001FC4
The most annoying part is that on at least this edk2 build,
SetVariable() /does actually appear to set the variable/, but it returns
EFI_INVALID_PARAMETER. I'm not planning on relying on that behavior.
So... yeah, the largest *volatile* (i.e. RAM only) variable this edk2
build will let you create is less than two pages. It's only got 7.9G
free, so I guess it's feeling like space is a little tight.
We're also not quite preserving that return code well enough for his
workaround to work.
New plan. We try to create variables the normal way, but we don't
consider not having enough space to be fatal. In that case, we create
an EFI_SECURITY_LIST with one sha256sum in it, with a value of all 0,
and try to add that so we're sure there's /something/ there that's
innocuous. On systems where the first SetVariable() /
QueryVariableInfo() lied to us, the correct variable should be there,
otherwise the one with the zero-hash will be.
We then also build a config table to hold this info and install that.
The config table is a packed array of this struct:
struct mok_variable_config_entry {
CHAR8 name[256];
UINT64 data_size;
UINT8 data[];
};
There will be N+1 entries, and the last entry is all 0 for name and
data_size. The total allocation size will always be a multiple of 4096.
In the typical RHEL 7.9 case that means it'll be around 5 pages.
It's installed with this guid:
c451ed2b-9694-45d3-baba-ed9f8988a389
Anything that can go wrong will.
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream: not yet, I don't want people to read this before Wednesday.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
