Peter Jones [Tue, 16 Feb 2021 20:15:47 +0000 (15:15 -0500)]
SBAT: make the variable be CSV in our spec.
We noticed that we'd originally specified the SBAT variable as binary
records, but talked as if they're CSV. Woops. Anyway, this makes them
CSV, which also means they don't need the size field.
Chris Co [Tue, 16 Feb 2021 06:21:12 +0000 (06:21 +0000)]
sbat: add minor fixes to parse_sbat
Add parameter checking to parse_sbat().
Set end pointer to be sbat_base + sbat_size - 1. We directly
dereference the end pointer but this is technically outside of
our sbat_base buffer range.
Remove current and end while loops that account for extra CRLF
or LF characters before and after the .sbat section. We will
rely on automated tooling to verify the .sbat section is sane.
Remove the overwriting of *(end - 1) with '\0'. This behavior
causes a segfault in the unit test. parse_sbat_entry() expects
a very specific pattern "_,_,_,_,_,_\n" for every entry and uses
strchrnul() to process each individual field. When *(end - 1)='\0'
is present, it short-circuits the final \n and causes the final
get_sbat_field() to return NULL, thereby setting current = NULL.
Eventually parse_sbat attempts to access current in the do-while
condition and the segfault happens.
Peter Jones [Wed, 27 Jan 2021 17:14:01 +0000 (12:14 -0500)]
Add ENABLE_SHIM_DEVEL config to change what our debug variable name is
Currently, if you have two boot entries, say one for
\EFI\fedora\shimx64.efi and one for \EFI\devel\shimx64.efi, and you set
the efi variable SHIM_DEBUG=1, both of these will trigger, and you need
to write your debugging scripts to allow each of the builds to continue.
This is a pain.
This patch makes it so on your development build, it will instead check
SHIM_DEVEL_DEBUG, thus meaning you can have it pause for a debugger only
on the development branch and not the OS you need to boot to scp in a
new development build.
Peter Jones [Mon, 26 Aug 2019 20:12:05 +0000 (16:12 -0400)]
console: Fix a typo in the EFI warning list in gnu-efi
Some versions of gnu-efi have a typo, in which "EFI_WARN_UNKNOWN_GLYPH"
is accidentally "EFI_WARN_UNKOWN_GLYPH". Work around that, so that we
can use the not-silly one in console.c's list of error and warning
messages.
Gary Lin [Wed, 23 May 2018 10:13:05 +0000 (18:13 +0800)]
fallback: show a countdown menu before reset
Some machines with the faulty firmware may keep booting the default boot
path instead of the boot option we create. To avoid the infinite reset
loop, this commit introduce a countdown screen before fallback resets the
system, so the user can interrupt the system reset and choose to boot
the restored boot option. The "Always continue boot" option creates a
BS+RT+NV variable, FB_NO_REBOOT, to make fallback boot the first boot
option afterward without asking. The user can revert the behavior by
removing the variable.
https://github.com/rhboot/shim/issues/128
Signed-off-by: Gary Lin <glin@suse.com>
This is a backport from devel of:
Some machines with the faulty firmware may keep booting the default boot
path instead of the boot option we create. To avoid the infinite reset
loop, this commit introduce a countdown screen before fallback resets the
system, so the user can interrupt the system reset and choose to boot
the restored boot option. The "Always continue boot" option creates a
BS+RT+NV variable, FB_NO_REBOOT, to make fallback boot the first boot
option afterward without asking. The user can revert the behavior by
removing the variable.
https://github.com/rhboot/shim/issues/128
Signed-off-by: Gary Lin <glin@suse.com> Signed-off-by: Peter Jones <pjones@redhat.com>
Gary Lin [Fri, 22 Nov 2019 09:22:15 +0000 (17:22 +0800)]
lib: move print_crypto_errors() out of console.c
print_crypto_errors() will pull in the whole openssl library which
bloats the size of fallback.efi. Move the function to an independent
file (lib/print_crypto.c) to reduce the file size of fallback.efi from
1.3MB to 93KB.
Luca Boccassi [Fri, 15 Feb 2019 21:42:10 +0000 (21:42 +0000)]
Makefile: use fixed build host if SOURCE_DATE_EPOCH is defined
If SOURCE_DATE_EPOCH is defined then we can be reasonably sure the
user wants the build to be fully reproducible, so use a fixed string.
In case of a cross build, using uname -s -m -p -i o will still report
the host's kernel architecture, which will trip some CIs like
Debian's.
Makefile: use fixed build host if SOURCE_DATE_EPOCH is defined
If SOURCE_DATE_EPOCH is defined then we can be reasonably sure the
user wants the build to be fully reproducible, so use a fixed string.
In case of a cross build, using uname -s -m -p -i o will still report
the host's kernel architecture, which will trip some CIs like
Debian's.
Signed-off-by: Luca Boccassi <bluca@debian.org> Signed-off-by: Luca Boccassi <bluca@debian.org> Signed-off-by: Peter Jones <pjones@redhat.com>
Peter Jones [Fri, 11 Dec 2020 20:54:55 +0000 (15:54 -0500)]
Fix up a bunch of our license statements and add SPDX most places
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.
Colin Walters [Tue, 23 Jun 2020 01:57:05 +0000 (01:57 +0000)]
Convert README -> README.md
One of the really great things about Github IMO is how
"front and center" the README file in a repository is (just
compare with Sourceforge).
Github renders it more nicely if the file is declared to be Markdown,
so let's do that. Add a bit of formatting: using code fences
for code, hyperlinks for other files etc.
I also added a title block from the Fedora package `Summary`
since while I know in theory shim is independent of bootloaders,
let's say what the 95% case is here.
Peter Jones [Sat, 13 Feb 2021 18:02:14 +0000 (13:02 -0500)]
get_variable: always allocate a NUL character at the end.
Sometimes we're loading structures that are parsed in string-like ways,
but can't necessarily be trusted to be zero-terminated. Solve that by
making sure we always have enough aligned, trailing zero bytes to always
have at least one NUL character, no matter which character type is being
parsed.
Peter Jones [Sat, 13 Feb 2021 00:35:28 +0000 (01:35 +0100)]
Add an example SBAT workflow document
Add a file that contains example workflows for SBAT and better illustrate
what type of content is expected to be present in both the .sbat section
and the SBAT authenticated EFI variable.
SBAT is a new Generation Number Based Revocation meant to replace the DBX
Revocation List Files mechanism. It is more flexible and allow to revoke
sets of binaries, instead of having to list all of them as with the DBX.
Metadata that includes the vendor, product family, product, component,
version and generation are added to artifacts in a .sbat section. This
is protected by the digital signature and so it cannot be tampered.
Signed-off-by: Jan Setje-Eilers <jan.setjeeilers@oracle.com> Signed-off-by: Peter Jones <pjones@redhat.com> Signed-off-by: Gary Lin <glin@suse.com>
Peter Jones [Wed, 10 Feb 2021 01:02:26 +0000 (20:02 -0500)]
SBAT: parse a copy of the table that's got a NUL at the end
Right now we allocate the PE file's contents in RW memory, but hopefully
that won't always be the case. Our SBAT parsing, however, very much
expects to be able to edit it. We also don't actually know that shim's
.sbat section is loaded r/w, so we can't necessarily write there.
This patch copies the SBAT data to its own buffer, plus one NUL byte at
the end, so we can always be sure that will work.
Add a function to parse the SBAT metadata from the .sbat section
Parse the SBAT [0] Version-Based Revocation Metadata that's contained in a
.sbat data section of the loaded PE binary. This information is used along
with data in a SBAT variable to determine if a EFI binary has been revoked.
Peter Jones [Tue, 9 Feb 2021 18:39:48 +0000 (13:39 -0500)]
includes: add strchra() and strchrnula() impls
Unfortunately GNU-EFI doesn't currently implement ascii versions of
strchr() or strchrnul(), and we kind of need them, so add an
implementation here for now.
Peter Jones [Fri, 12 Feb 2021 17:41:41 +0000 (12:41 -0500)]
efi bins: add an easy way for vendors to add .sbat data
In cases where we accept vendor shim binaries with additional patches,
it may become necessary to identify those builds with additional SBAT
data. When we consider such patches, we should be proactive in asking
vendors to include that data in the .sbat sections of their trusted EFI
binaries.
This patch adds any data in data/sbat.*.csv (after a quick sanitizing
pass) after data/sbat.csv in the .sbat section, so that no changes to
the upstream data/sbat.csv are ever required.
The Secure Boot Advanced Targeting (SBAT) [0] is a Generation Number Based
Revocation mechanism that is meant to replace the DBX revocation file list.
Binaries must contain a .sbat data section that has a set entries, each of
them consisting of UTF-8 strings as comma separated values. Allow to embed
this information into the fwupd EFI binary at build time.
The SBAT metadata must contain at least two entries. One that defines the
SBAT version used and another one that defines the component generation.
This patch adds a sbat.csv that contains these two entries and downstream
users can override if additional entries are needed due changes that make
them diverge from upstream code and potentially add other vulnerabilities.
The same SBAT metadata is added to the fallback and MOK manager binaries
because these are built from the same shim source. These need to have SBAT
metadata as well to be booted if a .sbat section is mandatory.
Peter Jones [Fri, 29 Jan 2021 20:27:26 +0000 (15:27 -0500)]
Work around some clang-format oddnesses
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)
Peter Jones [Fri, 29 Jan 2021 20:11:18 +0000 (15:11 -0500)]
Always use lower case for our local include file names.
clang-format doesn't allow you to specify an include sort order, and
just assumes asciibetical is a pretty good order, which doesn't work as
well as you would hope.
This makes them all lower case so they don't need to be re-sorted.
I also went through and checked that we're using quoted local includes
at all the appropriate places.
Peter Jones [Fri, 29 Jan 2021 19:01:28 +0000 (14:01 -0500)]
Add a .clang-format file.
There's clearly not enough conformance to a single coding style here.
To some extent that can't really change - I don't intend to adopt edk2's
style for the main codebase, but re-formatting the code we borrow from
edk2 would be insane.
This commit adds a .clang-format file, to be used on new files as such:
clang-format --style=file -i foo.c
It can also be used when new free-standing code is added to existing
files, using the clang-format --lines= option.
The starting style in this is pretty close to the Linux kernel style,
with a couple of minor modifications.
Paul Moore [Thu, 29 Oct 2020 13:49:36 +0000 (09:49 -0400)]
shim: compile time option to bypass the ExitBootServices() check
On systems where a second stage bootloader is not used, and the Linux
Kernel is booted directly from shim, shim's ExitBootServices() hook
can cause problems as the kernel never calls the shim's verification
protocol. In this case calling the shim verification protocol is
unnecessary and redundant as shim has already verified the kernel
when shim loaded the kernel as the second stage loader.
This functionality is disabled by default and must be enabled via the
DISABLE_EBS_PROTECTION macro/define at build time.
Peter Jones [Thu, 30 Jul 2020 18:34:22 +0000 (14:34 -0400)]
hexdump.h: fix arithmetic error.
When I modified the hexdumper to help debug MokListRT mirroring not
working because of PcdMaxVolatileVariableSize being tiny, I
inadvertently added something that is effectively:
hexdump(..., char *buf, ..., int position)
{
unsigned long begin = (position % 16);
unsigned long i;
...
for (i = 0; i < begin; i++) {
...
}
...
}
Unfortunately, in c if 0x8 is set in position, that means begin is
0xfffffffffffff8, because signed integer math is horrifying:
Peter Jones [Mon, 17 Aug 2020 19:47:19 +0000 (15:47 -0400)]
Fix some mokmanager deletion paths
This fixes several codepaths where MokList and MokListX are supposed to
be deleted, but are not. It also adds debug logging to much of the
deletion codepath.
Fix buffer overrun due DEFAULT_LOADER length miscalculation
The DEFAULT_LOADER is a UCS-2 string and the StrLen() function returns the
number of UCS-2 encoded characters in the string. But the allocated memory
is in bytes, so only half of the needed memory to store it is allocated.
This leads to a buffer overrun when the StrCpy() function attempts to copy
the DEFAULT_LOADER to the allocated buffer.
Fixes: 354bd9b1931 ("Actually check for errors from set_second_stage()") Reported-by: Stuart Hayes <stuart_hayes@dell.com> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Peter Jones [Tue, 4 Aug 2020 16:42:43 +0000 (12:42 -0400)]
mirror_one_mok_variable(): round allocation up to a full page
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.
Peter Jones [Fri, 24 Jul 2020 02:09:03 +0000 (22:09 -0400)]
Also use a config table to mirror mok variables.
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:
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:
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.
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>
Peter Jones [Thu, 23 Jul 2020 20:32:05 +0000 (16:32 -0400)]
Handle binaries with multiple signatures.
This adds support for multiple signatures. It first tries validating
the binary by hash, first against our dbx lists, then against our db
lists. If it isn't allowed or rejected at that step, it continues to
the normal routine of checking all the signatures.
At this point it does *not* reject a binary just because a signature is
by a cert on a dbx list, though that will override any db list that
certificate is listed on. If at any point any assertion about the
binary or signature list being well-formed fails, the binary is
immediately rejected, though we do allow skipping over signatures
which have an unsupported sig->Hdr.wCertificateType.
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream: pr#210
Peter Jones [Thu, 23 Jul 2020 16:36:56 +0000 (12:36 -0400)]
Add support for vendor_db built-in shim authorized list.
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
Some mainboards do not update the ProxyOffset dhcp information when using
proxy dhcp and boot menus.
This adds a fallback to check the PxeReply field if no boot information is
found in the v4 dhcp or proxy dhcp information
tpm: Include information about PE/COFF images in the TPM Event Log
The "TCG PC Client Specific Platform Firmware Profile Specification" says
that when measuring a PE/COFF image, the TCG_PCR_EVENT2 structure Event
field MUST contain a UEFI_IMAGE_LOAD_EVENT structure.
Currently an empty UEFI_IMAGE_LOAD_EVENT structure is passed so users only
have the hash of the PE/COFF image, but not information such the file path
of the binary.
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Upstream-commit-id: c252b9ee94c
shim: Update EFI_LOADED_IMAGE with the second stage loader file path
When shim loads the second stage loader (e.g: GRUB) the FilePath field of
the EFI_LOADED_IMAGE structure isn't updated with the path of the loaded
binary. So it still contains the file path of the shim binary.
This isn't a problem since the file path is currently not used. But should
be used to set the DevicePath field of the EFI_IMAGE_LOAD_EVENT structure
that is logged when measuring the PE/COFF binaries. In that case the TPM
Event Log will have an incorrect file path for the measured binary, i.e:
Laszlo Ersek [Tue, 28 Jan 2020 22:33:46 +0000 (23:33 +0100)]
translate_slashes(): don't write to string literals
Currently, all three invocations of the translate_slashes() function may
lead to writes to the string literal that is #defined with the
DEFAULT_LOADER_CHAR macro. According to ISO C99 6.4.5p6, this is undefined
behavior ("If the program attempts to modify such an array, the behavior
is undefined").
This bug crashes shim on e.g. the 64-bit ArmVirtQemu platform ("Data
abort: Permission fault"), where the platform firmware maps the .text
section (which contains the string literal) read-only.
Modify translate_slashes() so that it copies and translates characters
from an input array of "char" to an output array of "CHAR8".
While at it, fix another bug. Before this patch, if translate_slashes()
ever encountered a double backslash (translating it to a single forward
slash), then the output would end up shorter than the input. However, the
output was not NUL-terminated in-place, therefore the original string
length (and according trailing garbage) would be preserved. After this
patch, the NUL-termination on contraction is automatic, as the output
array's contents are indeterminate when entering the function, and so we
must NUL-terminate it anyway.
Peter Jones [Mon, 18 Nov 2019 18:58:46 +0000 (13:58 -0500)]
Actually check for errors from set_second_stage()
This changes shim_init() to check for errors from set_second_stage().
In order to make that work, it also does the following:
- correctly /always/ allocate second_stage, not sometimes allocate and
sometimes point at .data
- test for LoadOptionSize == 0 and return success
- print an error message for the failure so we can see it.
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream-commit-id: 354bd9b1931
Chris Coulson [Thu, 26 Sep 2019 19:01:01 +0000 (20:01 +0100)]
tpm: Don't log duplicate identical events
According to the comment in tpm_measure_variable ("Don't measure something that we've already measured"), shim
shouldn't measure duplicate events if they are identical, which also aligns with section 2.3.4.8 of the TCG PC
Client Platform Firmware Profile Specification ("If it has been measured previously, it MUST NOT be measured
again"). This is currently broken because tpm_data_measured() uses the return value of CompareGuid() incorrectly.
projects / efi-boot-shim.git / log