summaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
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
2021-02-17Second attempt at SBAT doc tweaksSteve McIntyre
2021-02-17fix nitJan Setje-Eilers
Co-authored-by: Christopher Co <christopher.co@microsoft.com>
2021-02-17fix nitJan Setje-Eilers
Co-authored-by: Christopher Co <christopher.co@microsoft.com>
2021-02-17fix nitJan Setje-Eilers
Co-authored-by: Christopher Co <christopher.co@microsoft.com>
2021-02-17fix notJan Setje-Eilers
Co-authored-by: Christopher Co <christopher.co@microsoft.com>
2021-02-17SBAT: Fix all the docs examples to start with version 1Peter Jones
Since we have atol() but not strtoul(), we need to make all versions positive integers. Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-17SBAT: make the variable be CSV in our spec.Peter Jones
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. Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-16sbat: add minor fixes to parse_sbatChris Co
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. Signed-off-by: Chris Co <chrco@microsoft.com>
2021-02-16sbat: use correct type for parse_sbat_var() return valueJavier Martinez Canillas
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
2021-02-16sbat: remove unused buffer parameter in parse_sbat() functionJavier Martinez Canillas
It's a left over from an early implementation that was never cleaned. Reported-by: Christopher Co <christopher.co@microsoft.com> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
2021-02-16sbat: clang-format the whole thing.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-16Use gcc -Os instead of -O0.Peter Jones
Signed-off-by: Peter Jones <pjones@redhat.com>
2021-02-16Rename check_{white,black}list to check_{allow,deny}listChris Coulson
v2 - updated for conflicts and to include documentation (pjones)