summaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2021-03-06Restore loaded image of shim at Exit()Gary Lin
When grub2 invoked Exit() in AArch64 AAVMF, the VM crashed with the following messsages: Unloading driver at 0x000B7D7B000 Synchronous Exception at 0x00000000BF5D5E68 AllocatePool: failed to allocate 800 bytes Synchronous Exception at 0x00000000BF5D5E68 The similar error also showed when I modified MokManager to call gBS->Exit() at the end of efi_main(). However, if MokManager just returned, the error never showed. One significant difference is whether the loaded image was restored or not, and the firmware seems to need the original ImageBase pointer to do clean-up. To avoid the potential crash, this commit adds restore_loaded_image() so that we can restore the loaded image both in start_image() and do_exit(). Signed-off-by: Gary Lin <glin@suse.com>
2021-02-25Fix column size check in SBAT variable parsing.Thomas Frauendorfer | Miray Software
Fix a typo that inverted the check that validates that a CSV column is not empty. Signed-off-by: Thomas Frauendorfer | Miray Software <tf@miray.de>
2021-02-25sbat: Don't assume VirtualSize == SizeOfRawDataChris Coulson
The current code rejects the .sbat section if the VirtualSize and SizeOfRawData fields of the section header aren't the same, but this is too strict. The VirtualSize corresponds to the size of the SBAT metadata and isn't necessarily a multiple of the file alignment. The on-disk size (SizeOfRawData) is a multiple of the file alignment and is zero padded. Make sure that the on-disk size is at least as large as virtual size, and ignore the .sbat section if it isn't. This should fix https://github.com/rhboot/shim/issues/281. Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
2021-02-25SBAT: update the raw Markdown to look less terriblePaul Moore
The raw text of the SBAT.md file is a bit of a mess, this patch attempts to introduce some consistentcy to the text without affecting the rendered Markdown. The only content change was the addition of a missing period ('.') at the end of sentence/paragraph; all of the other changes were purely formatting changes. Signed-off-by: Paul Moore <pmoore2@cisco.com>
2021-02-25SBAT: fix some typos in the SBAT docsPaul Moore
Signed-off-by: Paul Moore <pmoore2@cisco.com>
2021-02-25sbat: fix the residual "resource section" for SBATGary Lin
Signed-off-by: Gary Lin <glin@suse.com>
2021-02-25Fix two errant 'shim,0' outdated sbat cases.Peter Jones
Two places we missed still have 0 for an sbat version - one doc and one in our data csv. This fixes those. Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-25sbat: fix the gcc warningsGary Lin
This commit fixes the following gcc warnings for casting. sbat.c: In function 'verify_single_entry': sbat.c:157:13: error: pointer targets in passing argument 1 of 'strcmp' differ in signedness [-Werror=pointer-sign] if (strcmp(entry->component_name, sbat_var_entry->component_name) == 0) { ^~~~~ In file included from Cryptlib/Include/string.h:15:0, from sbat.c:7: Cryptlib/Include/OpenSslSupport.h:299:16: note: expected 'const char *' but argument is of type 'const CHAR8 * {aka const unsigned char *}' int strcmp (const char *, const char *); ^~~~~~ sbat.c:157:36: error: pointer targets in passing argument 2 of 'strcmp' differ in signedness [-Werror=pointer-sign] if (strcmp(entry->component_name, sbat_var_entry->component_name) == 0) { ^~~~~~~~~~~~~~ In file included from Cryptlib/Include/string.h:15:0, from sbat.c:7: Cryptlib/Include/OpenSslSupport.h:299:16: note: expected 'const char *' but argument is of type 'const CHAR8 * {aka const unsigned char *}' int strcmp (const char *, const char *); ^~~~~~ sbat.c:165:19: error: pointer targets in passing argument 1 of 'AsciiStrDecimalToUintn' differ in signedness [-Werror=pointer-sign] sbat_gen = atoi(entry->component_generation); ^ Cryptlib/Include/OpenSslSupport.h:380:66: note: in definition of macro 'atoi' #define atoi(nptr) AsciiStrDecimalToUintn(nptr) ^~~~ In file included from Cryptlib/Include/OpenSslSupport.h:21:0, from Cryptlib/Include/string.h:15, from sbat.c:7: Cryptlib/Library/BaseLib.h:9:7: note: expected 'const char *' but argument is of type 'const CHAR8 * {aka const unsigned char *}' UINTN AsciiStrDecimalToUintn(const char *String); ^~~~~~~~~~~~~~~~~~~~~~ In file included from Cryptlib/Include/string.h:15:0, from sbat.c:7: sbat.c:166:23: error: pointer targets in passing argument 1 of 'AsciiStrDecimalToUintn' differ in signedness [-Werror=pointer-sign] sbat_var_gen = atoi(sbat_var_entry->component_generation); ^ Cryptlib/Include/OpenSslSupport.h:380:66: note: in definition of macro 'atoi' #define atoi(nptr) AsciiStrDecimalToUintn(nptr) ^~~~ In file included from Cryptlib/Include/OpenSslSupport.h:21:0, from Cryptlib/Include/string.h:15, from sbat.c:7: Cryptlib/Library/BaseLib.h:9:7: note: expected 'const char *' but argument is of type 'const CHAR8 * {aka const unsigned char *}' UINTN AsciiStrDecimalToUintn(const char *String); ^~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors make: *** [<builtin>: sbat.o] Error 1 Signed-off-by: Gary Lin <glin@suse.com>
2021-02-25Make verify_sbat() more testablePeter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-25Fix-up and enable a bunch of .sbat section parsing tests.Peter Jones
This brings all the tests Chris Co wrote about parsing the .sbat section back. Some of the actual test functions became redundant, and some new ones were needed, but all of the actual test cases should be represented here. Note that building and running this test does not quite work yet /on this branch/. In order to do that, we need some cleanups and reorganizing that I don't want to push just yet, which can be found on https://github.com/rhboot/shim/tree/test-reorg Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-25Add initial sbat unit testing codeChris Co
[currently #if 0'd to oblivion by pjones] Signed-off-by: Chris Co <chrco@microsoft.com>
2021-02-25Add test cases for our CSV parser.Peter Jones
This does a couple of straightforward tests on our CSV parser, and then for good measure it does two with random data - one that's just random data, one that's had all the zeros changed to nonzero values. Note that building and running this test does not quite work yet /on this branch/. In order to do that, we need some cleanups and reorganizing that I don't want to push just yet, which can be found on https://github.com/rhboot/shim/tree/test-reorg Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-25Add a tester for our string functions.Peter Jones
Note that building and running this test does not quite work yet /on this branch/. In order to do that, we need some cleanups and reorganizing that I don't want to push just yet, which can be found on https://github.com/rhboot/shim/tree/test-reorg Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-25make 'make test' able to run unit test harnessesPeter Jones
This adds a couple of make targets to do unit tests that are linked to libc: test-FOO : builds and runs test-FOO for any test-FOO.c test : builds and runs all test-FOO tests Note that building and running this test does not quite work yet /on this branch/. In order to do that, we need some cleanups and reorganizing that I don't want to push just yet, which can be found on https://github.com/rhboot/shim/tree/test-reorg Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-25SBAT: make our SBAT variable parser use the CSV parserPeter Jones
This makes our SBAT variable parser use the generic CSV parser, and also changes its API slightly to produce a more testable intermediate interface. Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-25SBAT: make our sbat section parser use the csv parserPeter Jones
This makes the .sbat section parser use parse_csv_data(). It also re-names a couple of the structs, because they were still too easy to get lost in. Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-25Add a stand-alone CSV parser.Peter Jones
This adds a simple to use, one-function-call CSV parser that takes a blob of data and gives you a linked list with an array of values. Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-25Fix our debuginfo paths hopefullyPeter Jones
2021-02-25includes: include all gnu-efi includes at one place.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-25Move is_utf8_bom() to str.hPeter Jones
This moves is_utf8_bom() to str.h, and also adds two #defines, UTF8_BOM and UTF8_BOM_SIZE. Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-25Add a list_size() primitivePeter Jones
This adds list_size(), which tells us how many elements are in a list. Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-25Tidy up our string primitives...Peter Jones
This adds constraints some of our compilers can check to all of our string primitives, as well as adding implementations of: - strdup - strdup - stpcpy - stpcpy - strnchrnul - strchrnul with a limit - strntoken - a tokenizer Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-25Fix all the places we need UNUSED on arguments.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-25compiler.h: fix a typo and add some more function attribute macrosPeter Jones
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>
2021-02-25Make 'make fanalyzer' targetsPeter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-25Move the coverity and scan-build makefiles out of the top directoryPeter Jones
I'm adding even more of this stuff, and it's feeling pretty cluttered, so this moves the scan-build and coverity makefiles into include/, where we'll see them less. Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-22SBAT: mirror SBAT to SbatRT and extend to PCR7 + logPeter Jones
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>
2021-02-22Document struct mok_state_variable better.Peter Jones
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>
2021-02-22tpm: minor cleanup: use EV_IPL not 0xdPeter Jones
This does two things: - consolidates all our TPM event type #defines to one place - uses EV_IPL instead of hard-coding 0xd Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-22Drop --set-section-alignment from Makefile since linker ALIGN(4096)Jan Setje-Eilers
already enforces the alignment, clarify that objcopy only needs to do the alignment in the SBAT spec.
2021-02-22Fix bashisms in Make.coveritySteve McIntyre
Not everybody uses bash as /bin/sh, and the scripting here is safe just using single brackets. Signed-off-by: Steve McIntyre <93sam@debian.org>
2021-02-20Add --set-section-alignment '.sbat=512' to objcopy command lineupstream/15+1613861442.888f5b5Jan Setje-Eilers
2021-02-20Include missing .text sections in PE/COFF binaryChris Coulson
At the default -Os optimization level, gcc emits ".text.startup" and ".text.unlikely" sections for static initializers and noreturn functions which end up in the intermediate ELF binary: $ objdump -h build-x64/shimx64.efi.so build-x64/shimx64.efi.so: file format elf64-x86-64 Sections: Idx Name Size VMA LMA File off Algn 0 .text 00046e7b 0000000000001000 0000000000001000 00001000 2**10 CONTENTS, ALLOC, LOAD, READONLY, CODE 1 .text.startup 00000118 0000000000047e7b 0000000000047e7b 00047e7b 2**0 CONTENTS, ALLOC, LOAD, READONLY, CODE 2 .text.unlikely 00000046 0000000000047f93 0000000000047f93 00047f93 2**0 CONTENTS, ALLOC, LOAD, READONLY, CODE 3 .data 000315e8 0000000000048000 0000000000048000 00048000 2**9 These additional .text.* sections are omitted from the final PE/COFF binary, resulting in a crash when processing the ctors. Taking a look at _init_array in gdb: (gdb) p/x &_init_array $1 = 0x78510 (gdb) p/x &_init_array_end $2 = 0x7851c (gdb) x/x (void*)&_init_array 0x78510 <_init_array>: 0x00047e7b (gdb) x/x (void*)(&_init_array)+8 0x78518 <_init_array+8>: 0x00000000 See that 0x00047e7b falls inside the padding between the .text and .data sections: $ objdump -h build-x64/shimx64.efi build-x64/shimx64.efi: file format pei-x86-64 Sections: Idx Name Size VMA LMA File off Algn 0 .text 00046e7b 0000000000001000 0000000000001000 00000400 2**10 CONTENTS, ALLOC, LOAD, READONLY, CODE 1 .data 000315e8 0000000000048000 0000000000048000 00047400 2**9 Adjust the linker script to merge the .text.startup and .text.unlikely sections in to the .text section. [edited by pjones to use .text.* instead of naming the sections individually, and to sync up with what other arches have in .text]
2021-02-19build: Pass the correct paths to sbsignChris Coulson
Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
2021-02-19sbat: Fix two NULL derefs found with "gcc -fanalyzer"Peter Jones
"gcc -fanalyzer" found two NULL pointer checks we're missing in sbat.c: include/str.h: In function ‘get_sbat_field.part.0’: sbat.c:20:14: error: dereference of NULL ‘offset’ [CWE-476] [-Werror=analyzer-null-dereference] 20 | if (!*offset) and include/str.h: In function ‘parse_sbat’: sbat.c:140:27: error: dereference of NULL ‘current’ [CWE-476] [-Werror=analyzer-null-dereference] 140 | } while (entry && *current != '\0'); Both are simple, and this patch fixes them. Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-19sbat: make shim to parse it's own .sbat section on initJavier Martinez Canillas
This is needed for shim to verify itself when booting, to make sure that shim binaries can't be executed anymore after been revoked by SBAT. Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
2021-02-19shim: initialize OpenSSL after parsing SBAT dataJavier Martinez Canillas
A following patch will make shim to verify its .sbat section and it should be done before doing the OpenSSL initialization. But having the debugger attached may be useful at this point. Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
2021-02-19sbat: include NULL terminator when calculating buffer end in parse_sbat()Javier Martinez Canillas
The parse_sbat() function is currently removing the last character of the passed buffer, which will usually be a null-terminated string to parse. There's no reason to do this and just take the whole size as specified by the caller. Reported-by: Chris Coulson <chris.coulson@canonical.com> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
2021-02-19pe.c: move sbat verification to its own function.Peter Jones
handle_image() is quite huge and complex. This patch moves the SBAT validation code from handle_image() to a new function, handle_sbat(). Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-19Don't re-parse the SBAT EFI variable for each binary we load.Javier Martinez Canillas
On a typical boot we validate at least two binaries; parsing the SBAT EFI variable each time, when it should not be changing, is not worth the effort. This patch moves the parsing out to some setup code, instead of doing it during the verification stage. Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-19parse_sbat: handle the realloc failure leak and batch allocations.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-19pe.c: parse SBAT variable and perform basic verificationAlex Burmashev
Per Peter Jones suggestion, we will be flexible in what data we expect while parsing the variable. Three fields are mandatory: component_generation, component_name_size, component_name However we also support adding comments and additional information to be added after component name, with ',' as a separator. Those information will be ignored and not used for verification purposes. So: grub,1 and grub,1,wow,this,is,my,comment will provide exactly same set of data for verification. [0]: https://github.com/rhboot/shim/blob/main/SBAT.md Signed-off-by: Alex Burmashev <alexander.burmashev@oracle.com> Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-19sbat: drop the struct sbat and just use two variables insteadPeter Jones
The struct sbat isn't doing anything and only has two fields so let's pass pass those two to the functions directly instead of storing it in a struct. Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-19shim: use an enum for efi_main's error messages.Peter Jones
Numbering the error messages in efi_main directly was a mistake, and the following patches just make it more apparent. This makes it an enum so we don't have to re-number at more than one place when we add or remove them. Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-19Fix EV_EFI_VARIABLE_AUTHORITY event in eventlogHai Huang
Currently, for an EV_EFI_VARIABLE_AUTHORITY event, the shim puts only EFI_SIGNATURE_DATA.SignatureData in the VariableData field, but omits EFI_SIGNATURE_DATA.SignatureOwner. According to reference implementation in EDK2, the entire EFI_SIGNATURE_DATA is put into the VariableData field, shown here: https://github.com/tianocore/edk2/blob/master/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c#L1032
2021-02-19build: add some basic $EFI_PATH checkingPaul Moore
Not all distributions put the crt0-efi-$(ARCH).o file under $LIB_DIR/gnuefi, some stash it directly in $LIB_DIR. In an effort to make the build a bit more user friendly, check if $LIB_DIR/gnuefi exits before setting $EFI_PATH to that value; if $LIB_DIR/gnuefi does not exist, fallback to $LIB_DIR for $EFI_PATH. Signed-off-by: Paul Moore <pmoore2@cisco.com>
2021-02-17add list_empty to linked list primitivesAlex Burmashev
Signed-off-by: Alex Burmashev <alexander.burmashev@oracle.com>
2021-02-17strndupa: allocate len + 1, so that \0 is not lostAlex Burmashev
Signed-off-by: Alex Burmashev <alexander.burmashev@oracle.com>
2021-02-17Fix an off by one in strnlena()Peter Jones
I wrote a test case for strnlena() and strndupa() and of course both were off by one in the opposite directions... ... but the next patch obviates the need for them, hopefully, so this will wind up getting dropped.
2021-02-17Fixups after Peter's reviewSteve McIntyre
generated by cgit v1.2.3 (git 2.39.1) at 2025-09-03 16:56:57 +0000