| Age | Commit message (Collapse) | Author |
|---|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
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.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
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.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
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.
[0]: https://github.com/rhboot/shim/blob/sbat/SBAT.md
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
|
|
This adds the "sbat" branch to the list of branches where a build is
done if a PR is submitted against that branch.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This makes each of the f32/f33/f34 distro builds use the same steps to
do the build, as well as making each of them build both x64 and ia32
targets.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
I renamed PeImage.h to pe.h when I de-capitalized them, but this turns
out to be a bad idea because we already have a pe.h on the SBAT branch.
Woops.
This moves it to peimage.h
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>
|
|
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.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
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.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
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.
Signed-off-by: Paul Moore <pmoore2@cisco.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This could still use a bit of work, but it's better than Travis
failing...
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
sizeof(EFI_IMAGE_LOAD_EVENT) needs to represent the size of the header
so we can add the actual device path size to it to compute the event.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
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:
include/hexdump.h:99:vhexdumpf() &data[offset]:0x9E77E6BC size-offset:0x14
include/hexdump.h:15:prepare_hex() position:0x9E77E6BC
include/hexdump.h:17:prepare_hex() before:0xFFFFFFFFFFFFFFFC size:0x14
include/hexdump.h:19:prepare_hex() before:0xFFFFFFFFFFFFFFFC after:0x0
include/hexdump.h:21:prepare_hex() buf:0x000000009E77E2BC offset:0 &buf[offset]:0x000000009E77E2BC
Woops.
This could further have been prevented in /some/ cases by simply not
preparing the hexdump buffer when "verbose" is disabled.
This patch makes "pos" be unsigned in all cases, and also checks for
verbose in vhexdumpf() and simply returns if it is 0.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
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.
|
|
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>
|
|
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>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream: pr#213
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream: pr#211
|
|
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
|
|
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
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream: pr#206
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream: pr#212
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream: pr#212
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream: pr#212
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream: pr#212
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream: pr#212
|
|
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
|
|
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
Upstream-commit-id: cc7ebe0f9f4
|
|
typo
Upstream-commit-id: bc24c9eb1d4
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream-commit-id: 71439f848f6
|
|
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
|
|
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:
$ hexdump -Cv /sys/kernel/security/tpm0/binary_bios_measurements
...
00000a50 00 00 00 00 00 00 04 04 34 00 5c 00 45 00 46 00 |........4.\.E.F.|
00000a60 49 00 5c 00 72 00 65 00 64 00 68 00 61 00 74 00 |I.\.r.e.d.h.a.t.|
00000a70 5c 00 73 00 68 00 69 00 6d 00 78 00 36 00 34 00 |\.s.h.i.m.x.6.4.|
00000a80 2e 00 65 00 66 00 69 00 00 00 7f ff 04 00 00 00 |..e.f.i.........|
00000a90 00 00 00 00 00 00 af 08 00 00 00 0d 00 00 00 b5 |................|
00000aa0 cd d0 8f bb 16 31 e2 80 8b e8 58 75 c9 89 18 95 |.....1....Xu....|
00000ab0 d2 de 15 15 00 00 00 67 72 75 62 5f 63 6d 64 20 |.......grub_cmd |
00000ac0 73 65 74 20 70 61 67 65 72 3d 31 00 08 00 00 00 |set pager=1.....|
...
So update the EFI_LOADED_IMAGE structure with the second stage loader file
path to have the correct value in the log, i.e:
$ hexdump -Cv /sys/kernel/security/tpm0/binary_bios_measurements
...
00000a50 00 00 00 00 00 00 04 04 34 00 5c 00 45 00 46 00 |........4.\.E.F.|
00000a60 49 00 5c 00 72 00 65 00 64 00 68 00 61 00 74 00 |I.\.r.e.d.h.a.t.|
00000a70 5c 00 67 00 72 00 75 00 62 00 78 00 36 00 34 00 |\.g.r.u.b.x.6.4.|
00000a80 2e 00 65 00 66 00 69 00 00 00 7f ff 04 00 00 00 |..e.f.i.........|
00000a90 00 00 00 00 00 00 af 08 00 00 00 0d 00 00 00 b5 |................|
00000aa0 cd d0 8f bb 16 31 e2 80 8b e8 58 75 c9 89 18 95 |.....1....Xu....|
00000ab0 d2 de 15 15 00 00 00 67 72 75 62 5f 63 6d 64 20 |.......grub_cmd |
00000ac0 73 65 74 20 70 61 67 65 72 3d 31 00 08 00 00 00 |set pager=1.....|
...
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Upstream-commit-id: cd7d42d493d
|
|
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.
Fixes: 8e9124227d18475d3bc634c33518963fc8db7c98
Fixes: e62b69a5b0b87c6df7a4fc23906134945309e927
Fixes: 3d79bcb2651b9eae809b975b3e03e2f96c067072
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1795654
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Upstream-commit-id: 9813e8bc8b3
|
|
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
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream-commit-id: 173d35fe8f5
|
|
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.
Upstream-commit-id: 103adc89ce5
|
|
When compiling with GCC 9, there are a couple of errors of the form
MokManager.c: In function ‘write_back_mok_list’:
MokManager.c:1056:19: error: taking address of packed member of ‘struct <anonymous>’ may result in an unaligned pointer value [-Werror=address-of-packed-member]
1056 | if (CompareGuid(&(list[i].Type), &X509_GUID) == 0)
| ^~~~~~~~~~~~~~~
Copying the member of the packed struct to a temporary variable and
pointing to that variable solves the problem.
Upstream-commit-id: 58532e12e9a
|
|
There are lots of hi-dpi laptops nowadays, as doing mok enrollment, the font
is too small to see.
https://bugs.launchpad.net/ubuntu/+source/shim/+bug/1822043
This patch checks if the resolution is larger than Full HD (1920x1080) and
current console output columns and rows is in a good mode. Then swith the
console output to a better mode.
Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
Upstream-commit-id: cf05af6d899
|
|
tpm_measure_variable() calculates VarLogSize by adding the size of VarName
and VarData to the size of EFI_VARIABLE_DATA_TREE, and then subtracting
the size of the UnicodeName and VariableData members. This results in a
calculation that is 5 bytes larger than necessary because it doesn't take
in to account the padding of these members. The effect of this is that
shim measures an additional 5 zero bytes when measuring UEFI variables
(at least on 64-bit architectures).
Byte packing EFI_VARIABLE_DATA_TREE fixes this.
Upstream-commit-id: 7e4d3f1c8c7
|
|
tpm_log_event_raw() allocates a buffer for the EFI_TCG2_EVENT structure
that is one byte larger than necessary, and sets event->Size accordingly.
The result of this is that the event data recorded in the log differs
from the data that is measured to the TPM (it has an extra zero byte
at the end).
Upstream-commit-id: 8a27a4809a6
|
|
Upstream-commit-id: 6a73ca814af
|
|
Some versions of OpenSSL seem to go back and forth as to whether NULL
for these names are okay. Don't risk it.
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream-commit-id: 46b76a01717
|
