| Age | Commit message (Collapse) | Author |
|---|
|
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>
|
|
Potential new signing strategies ( for example signing grub, fwupdate
and vmlinuz with separate certificates ) require shim to support a
vendor provided bundle of trusted certificates and hashes, which allows
shim to trust EFI binaries matching either certificate by signature or
hash in the vendor_db. Functionality is similar to vendor_dbx.
This also improves the mirroring quite a bit.
Upstream: pr#206
|
|
A certain someone's default editor template leaked in to a couple of
source files, and claims they're GPL licensed. They're not.
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream-commit-id: 476cbff1110
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream-commit-id: 617b9007668
|
|
If the build cert is enabled, we should also mirror it to MokListRT.
Signed-off-by: Gary Lin <glin@suse.com>
Upstream-commit-id: aecbe1f99b6
|
|
There's no reason to complicate the logic with a goto here, instead just
pull the logic we're jumping to out to a helper function.
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream-commit-id: 29c11483101
|
|
When there is no key in MokList, import_mok_state() just skipped MokList
even though it should always mirror the vendor cert. Besides, the faulty
check of 'present' and 'addend' invalidates the mirroring of MokListXRT,
MokSBStateRT, and MokIgnoreDB.
https://github.com/rhboot/shim/issues/154
Signed-off-by: Gary Lin <glin@suse.com>
Upstream-commit-id: 4b27ae034ba
|
|
Without this, if a Mok variable doesn't exist in Boot Services, it will also
not be copied to Runtime, even if we have data to be added to it (vendor cert).
This patch makes sure that if we have extra data to append, we still mirror
the variable.
Signed-off-by: Patrick Uiterwijk <patrick@puiterwijk.org>
Upstream-commit-id: 9ab0d796bdc
|
|
This makes it so shim's idea of Mok variables all resides in one table
of data, and we don't need a bunch of nearly identical ad-hoc functions
to handle each of them.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
