diff --git a/.github/workflows/pullrequest.yml b/.github/workflows/pullrequest.yml index 18fe564..7a80738 100644 --- a/.github/workflows/pullrequest.yml +++ b/.github/workflows/pullrequest.yml @@ -135,10 +135,6 @@ jobs: efiarch: x64 makearch: x86_64 distro: centos8 - - arch: amd64 - efiarch: x64 - makearch: x86_64 - distro: centos7 - arch: amd64 efiarch: ia32 makearch: ia32 @@ -155,10 +151,6 @@ jobs: efiarch: ia32 makearch: ia32 distro: centos8 - - arch: amd64 - efiarch: ia32 - makearch: ia32 - distro: centos7 steps: - name: Checkout diff --git a/.gitignore b/.gitignore index 7fc55bc..9085e5a 100644 --- a/.gitignore +++ b/.gitignore @@ -33,7 +33,12 @@ Make.local /.cache/ /certdb/ /compile_commands.json +/compile_commands.events.json /cov-int/ +/crash-* +/fuzz-* +!/fuzz-*.c +/leak-* /post-process-pe /random.bin /sbat.*.csv diff --git a/.gitmodules b/.gitmodules index 78424fb..d0f1973 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,4 +1,4 @@ [submodule "gnu-efi"] path = gnu-efi url = https://github.com/rhboot/gnu-efi.git - branch = shim-15.6 + branch = shim-15.8 diff --git a/BUILDING b/BUILDING index 3b2e85d..17cd98d 100644 --- a/BUILDING +++ b/BUILDING @@ -78,6 +78,9 @@ Variables you could set to customize the build: - OSLABEL This is the label that will be put in BOOT$(EFI_ARCH).CSV for your OS. By default this is the same value as EFIDIR . +- POST_PROCESS_PE_FLAGS + This allows you to add flags to the invocation of "post-process-pe", for + example to disable the NX compatibility flag. Vendor SBAT data: It will sometimes be requested by reviewers that a build includes extra diff --git a/Cryptlib/Makefile b/Cryptlib/Makefile index 89fd5cd..626788c 100644 --- a/Cryptlib/Makefile +++ b/Cryptlib/Makefile @@ -11,7 +11,8 @@ INCLUDES = -I$(CRYPTDIR) -I$(CRYPTDIR)/Include \ -isystem $(TOPDIR)/include/system \ -isystem $(shell $(CC) -print-file-name=include) -WARNFLAGS += -Wno-unused-parameter +WARNFLAGS += -Wno-unused-parameter \ + -Wno-unused-but-set-variable CFLAGS = $(FEATUREFLAGS) \ $(OPTIMIZATIONS) \ diff --git a/Cryptlib/OpenSSL/Makefile b/Cryptlib/OpenSSL/Makefile index 795f471..d59c5d7 100644 --- a/Cryptlib/OpenSSL/Makefile +++ b/Cryptlib/OpenSSL/Makefile @@ -23,7 +23,7 @@ FEATUREFLAGS += -nostdinc WARNFLAGS += -Wno-empty-body \ -Wno-implicit-fallthrough \ $(if $(findstring gcc,$(CC)),-Wno-old-style-declaration) \ - $(if $(findstring gcc,$(CC)),-Wno-unused-but-set-variable) \ + -Wno-unused-but-set-variable \ -Wno-unused-parameter CFLAGS = $(FEATUREFLAGS) \ diff --git a/Cryptlib/SysCall/BaseMemAllocation.c b/Cryptlib/SysCall/BaseMemAllocation.c index 792b29e..7a565ff 100644 --- a/Cryptlib/SysCall/BaseMemAllocation.c +++ b/Cryptlib/SysCall/BaseMemAllocation.c @@ -15,6 +15,20 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include +// +// Extra header to record the memory buffer size from malloc routine. +// +#define CRYPTMEM_HEAD_SIGNATURE EFI_SIGNATURE_32('c','m','h','d') +typedef struct { + UINT32 Signature; + UINT32 Reserved; + UINTN Size; +} CRYPTMEM_HEAD; + +#define CRYPTMEM_OVERHEAD sizeof(CRYPTMEM_HEAD) + +#define MIN(a, b) ({(a) < (b) ? (a) : (b);}) + // // -- Memory-Allocation Routines -- // @@ -22,27 +36,84 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. /* Allocates memory blocks */ void *malloc (size_t size) { - return AllocatePool ((UINTN) size); + CRYPTMEM_HEAD *PoolHdr; + UINTN NewSize; + VOID *Data; + + // + // Adjust the size by the buffer header overhead + // + NewSize = (UINTN)(size) + CRYPTMEM_OVERHEAD; + + Data = AllocatePool (NewSize); + if (Data != NULL) { + PoolHdr = (CRYPTMEM_HEAD *)Data; + // + // Record the memory brief information + // + PoolHdr->Signature = CRYPTMEM_HEAD_SIGNATURE; + PoolHdr->Size = size; + + return (VOID *)(PoolHdr + 1); + } else { + // + // The buffer allocation failed. + // + return NULL; + } } /* Reallocate memory blocks */ void *realloc (void *ptr, size_t size) { - // - // BUG: hardcode OldSize == size! We have no any knowledge about - // memory size of original pointer ptr. - // - return ReallocatePool (ptr, (UINTN) size, (UINTN) size); + CRYPTMEM_HEAD *OldPoolHdr; + CRYPTMEM_HEAD *NewPoolHdr; + UINTN OldSize; + UINTN NewSize; + VOID *Data; + + NewSize = (UINTN)size + CRYPTMEM_OVERHEAD; + Data = AllocatePool (NewSize); + if (Data != NULL) { + NewPoolHdr = (CRYPTMEM_HEAD *)Data; + NewPoolHdr->Signature = CRYPTMEM_HEAD_SIGNATURE; + NewPoolHdr->Size = size; + if (ptr != NULL) { + // + // Retrieve the original size from the buffer header. + // + OldPoolHdr = (CRYPTMEM_HEAD *)ptr - 1; + ASSERT (OldPoolHdr->Signature == CRYPTMEM_HEAD_SIGNATURE); + OldSize = OldPoolHdr->Size; + + // + // Duplicate the buffer content. + // + CopyMem ((VOID *)(NewPoolHdr + 1), ptr, MIN (OldSize, size)); + FreePool ((VOID *)OldPoolHdr); + } + + return (VOID *)(NewPoolHdr + 1); + } else { + // + // The buffer allocation failed. + // + return NULL; + } } /* De-allocates or frees a memory block */ void free (void *ptr) { + CRYPTMEM_HEAD *PoolHdr; + // // In Standard C, free() handles a null pointer argument transparently. This // is not true of FreePool() below, so protect it. // if (ptr != NULL) { - FreePool (ptr); + PoolHdr = (CRYPTMEM_HEAD *)ptr - 1; + ASSERT (PoolHdr->Signature == CRYPTMEM_HEAD_SIGNATURE); + FreePool (PoolHdr); } } diff --git a/Make.defaults b/Make.defaults index c46164a..e75cd3c 100644 --- a/Make.defaults +++ b/Make.defaults @@ -139,6 +139,8 @@ CFLAGS = $(FEATUREFLAGS) \ $(INCLUDES) \ $(DEFINES) +POST_PROCESS_PE_FLAGS = + ifneq ($(origin OVERRIDE_SECURITY_POLICY), undefined) DEFINES += -DOVERRIDE_SECURITY_POLICY endif @@ -186,6 +188,9 @@ endif ifneq ($(origin VENDOR_DBX_FILE), undefined) DEFINES += -DVENDOR_DBX_FILE=\"$(VENDOR_DBX_FILE)\" endif +ifneq ($(origin SBAT_AUTOMATIC_DATE), undefined) +DEFINES += -DSBAT_AUTOMATIC_DATE=$(SBAT_AUTOMATIC_DATE) +endif LDFLAGS = --hash-style=sysv -nostdlib -znocombreloc -T $(EFI_LDS) -shared -Bsymbolic -L$(LOCAL_EFI_PATH) -L$(LIBDIR) -LCryptlib -LCryptlib/OpenSSL $(EFI_CRT_OBJS) --build-id=sha1 $(ARCH_LDFLAGS) --no-undefined diff --git a/Make.rules b/Make.rules index 7c6ec6c..96a8649 100644 --- a/Make.rules +++ b/Make.rules @@ -36,6 +36,6 @@ $(strip $(foreach x,$(DEFAULT_$(1)), endef %.o : %.S - $(CC) $(CFLAGS) -c -o $@ $< + $(CC) $(CFLAGS) -c -o $@ $< $(IGNORE_COMPILER_ERRORS) # vim:filetype=make diff --git a/Makefile b/Makefile index a9202f4..8283d56 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ default : all NAME = shim -VERSION = 15.7 +VERSION = 15.8 ifneq ($(origin RELEASE),undefined) DASHRELEASE ?= -$(RELEASE) else @@ -38,9 +38,9 @@ CFLAGS += -DENABLE_SHIM_CERT else TARGETS += $(MMNAME) $(FBNAME) endif -OBJS = shim.o globals.o mok.o netboot.o cert.o replacements.o tpm.o version.o errlog.o sbat.o sbat_data.o sbat_var.o pe.o httpboot.o csv.o load-options.o +OBJS = shim.o globals.o mok.o netboot.o cert.o replacements.o tpm.o version.o errlog.o sbat.o sbat_data.o sbat_var.o pe.o pe-relocate.o httpboot.o csv.o load-options.o KEYS = shim_cert.h ocsp.* ca.* shim.crt shim.csr shim.p12 shim.pem shim.key shim.cer -ORIG_SOURCES = shim.c globals.c mok.c netboot.c replacements.c tpm.c errlog.c sbat.c pe.c httpboot.c shim.h version.h $(wildcard include/*.h) cert.S sbat_var.S +ORIG_SOURCES = shim.c globals.c mok.c netboot.c replacements.c tpm.c errlog.c sbat.c pe.c pe-relocate.c httpboot.c shim.h version.h $(wildcard include/*.h) cert.S sbat_var.S MOK_OBJS = MokManager.o PasswordCrypt.o crypt_blowfish.o errlog.o sbat_data.o globals.o ORIG_MOK_SOURCES = MokManager.c PasswordCrypt.c crypt_blowfish.c shim.h $(wildcard include/*.h) FALLBACK_OBJS = fallback.o tpm.o errlog.o sbat_data.o globals.o @@ -76,6 +76,13 @@ ifneq ($(origin EFI_PATH),undefined) $(error EFI_PATH is no longer supported, you must build using the supplied copy of gnu-efi) endif +compile_commands.json : Makefile Make.rules Make.defaults + make clean + bear -- make COMPILER=clang test all + sed -i \ + -e 's/"-maccumulate-outgoing-args",//g' \ + $@ + update : git submodule update --init --recursive @@ -156,19 +163,19 @@ gnu-efi/$(ARCH_GNUEFI)/gnuefi/libgnuefi.a gnu-efi/$(ARCH_GNUEFI)/lib/libefi.a: ARCH=$(ARCH_GNUEFI) \ TOPDIR=$(TOPDIR)/gnu-efi \ -f $(TOPDIR)/gnu-efi/Makefile \ - lib gnuefi inc + lib gnuefi inc $(IGNORE_COMPILER_ERRORS) Cryptlib/libcryptlib.a: for i in Hash Hmac Cipher Rand Pk Pem SysCall; do mkdir -p Cryptlib/$$i; done - $(MAKE) TOPDIR=$(TOPDIR) VPATH=$(TOPDIR)/Cryptlib -C Cryptlib -f $(TOPDIR)/Cryptlib/Makefile + $(MAKE) TOPDIR=$(TOPDIR) VPATH=$(TOPDIR)/Cryptlib -C Cryptlib -f $(TOPDIR)/Cryptlib/Makefile $(IGNORE_COMPILER_ERRORS) Cryptlib/OpenSSL/libopenssl.a: for i in x509v3 x509 txt_db stack sha rsa rc4 rand pkcs7 pkcs12 pem ocsp objects modes md5 lhash kdf hmac evp err dso dh conf comp cmac buffer bn bio async/arch asn1 aes; do mkdir -p Cryptlib/OpenSSL/crypto/$$i; done - $(MAKE) TOPDIR=$(TOPDIR) VPATH=$(TOPDIR)/Cryptlib/OpenSSL -C Cryptlib/OpenSSL -f $(TOPDIR)/Cryptlib/OpenSSL/Makefile + $(MAKE) TOPDIR=$(TOPDIR) VPATH=$(TOPDIR)/Cryptlib/OpenSSL -C Cryptlib/OpenSSL -f $(TOPDIR)/Cryptlib/OpenSSL/Makefile $(IGNORE_COMPILER_ERRORS) lib/lib.a: | $(TOPDIR)/lib/Makefile $(wildcard $(TOPDIR)/include/*.[ch]) mkdir -p lib - $(MAKE) VPATH=$(TOPDIR)/lib TOPDIR=$(TOPDIR) -C lib -f $(TOPDIR)/lib/Makefile + $(MAKE) VPATH=$(TOPDIR)/lib TOPDIR=$(TOPDIR) -C lib -f $(TOPDIR)/lib/Makefile $(IGNORE_COMPILER_ERRORS) post-process-pe : $(TOPDIR)/post-process-pe.c $(HOSTCC) -std=gnu11 -Og -g3 -Wall -Wextra -Wno-missing-field-initializers -Werror -o $@ $< @@ -255,7 +262,7 @@ endif -j .rela* -j .dyn -j .reloc -j .eh_frame \ -j .vendor_cert -j .sbat -j .sbatlevel \ $(FORMAT) $< $@ - ./post-process-pe -vv $@ + ./post-process-pe -vv $(POST_PROCESS_PE_FLAGS) $@ ifneq ($(origin ENABLE_SHIM_HASH),undefined) %.hash : %.efi @@ -286,6 +293,15 @@ else $(PESIGN) -n certdb -i $< -c "shim" -s -o $@ -f endif +fuzz fuzz-clean fuzz-coverage fuzz-lto : + @make -f $(TOPDIR)/include/fuzz.mk \ + COMPILER="$(COMPILER)" \ + CROSS_COMPILE="$(CROSS_COMPILE)" \ + CLANG_WARNINGS="$(CLANG_WARNINGS)" \ + ARCH_DEFINES="$(ARCH_DEFINES)" \ + EFI_INCLUDES="$(EFI_INCLUDES)" \ + fuzz-clean $@ + test test-clean test-coverage test-lto : @make -f $(TOPDIR)/include/test.mk \ COMPILER="$(COMPILER)" \ @@ -295,14 +311,21 @@ test test-clean test-coverage test-lto : EFI_INCLUDES="$(EFI_INCLUDES)" \ test-clean $@ +$(patsubst %.c,%,$(wildcard fuzz-*.c)) : + @make -f $(TOPDIR)/include/fuzz.mk EFI_INCLUDES="$(EFI_INCLUDES)" ARCH_DEFINES="$(ARCH_DEFINES)" $@ + $(patsubst %.c,%,$(wildcard test-*.c)) : @make -f $(TOPDIR)/include/test.mk EFI_INCLUDES="$(EFI_INCLUDES)" ARCH_DEFINES="$(ARCH_DEFINES)" $@ -.PHONY : $(patsubst %.c,%,$(wildcard test-*.c)) test +clean-fuzz-objs: + @make -f $(TOPDIR)/include/fuzz.mk EFI_INCLUDES="$(EFI_INCLUDES)" ARCH_DEFINES="$(ARCH_DEFINES)" clean clean-test-objs: @make -f $(TOPDIR)/include/test.mk EFI_INCLUDES="$(EFI_INCLUDES)" ARCH_DEFINES="$(ARCH_DEFINES)" clean +.PHONY : $(patsubst %.c,%,$(wildcard fuzz-*.c)) fuzz +.PHONY : $(patsubst %.c,%,$(wildcard test-*.c)) test + clean-gnu-efi: @if [ -d gnu-efi ] ; then \ $(MAKE) -C gnu-efi \ @@ -322,7 +345,7 @@ clean-lib-objs: clean-shim-objs: @rm -rvf $(TARGET) *.o $(SHIM_OBJS) $(MOK_OBJS) $(FALLBACK_OBJS) $(KEYS) certdb $(BOOTCSVNAME) - @rm -vf *.debug *.so *.efi *.efi.* *.tar.* version.c buildid post-process-pe + @rm -vf *.debug *.so *.efi *.efi.* *.tar.* version.c buildid post-process-pe compile_commands.json @rm -vf Cryptlib/*.[oa] Cryptlib/*/*.[oa] @if [ -d .git ] ; then git clean -f -d -e 'Cryptlib/OpenSSL/*'; fi @@ -336,7 +359,7 @@ clean-cryptlib-objs: $(MAKE) -C Cryptlib -f $(TOPDIR)/Cryptlib/Makefile clean ; \ fi -clean: clean-shim-objs clean-test-objs clean-gnu-efi clean-openssl-objs clean-cryptlib-objs clean-lib-objs +clean: clean-shim-objs clean-fuzz-objs clean-test-objs clean-gnu-efi clean-openssl-objs clean-cryptlib-objs clean-lib-objs GITTAG = $(VERSION) diff --git a/MokVars.txt b/MokVars.txt index cdfec2c..baf8db9 100644 --- a/MokVars.txt +++ b/MokVars.txt @@ -53,6 +53,11 @@ The hash will be regenerated by MokManager after the user is requested to enter their password to confirm enrolment of the keys. If the hash matches MokAuth, the user will be prompted to enrol the keys. BS,RT,NV +ShimRetainProtocol: UINT8, read by Shim before uninstalling protocol. +If set to non-zero, Shim will keep the protocol in place. It can be +used by second stages to ensure the protocol is still available for +later stages, and can thus be used to verify additional PE files. BS,RT. + State variables: MokList: A list of authorized keys and hashes. An EFI_SIGNATURE_LIST diff --git a/README.md b/README.md index 60d51b6..5f40251 100644 --- a/README.md +++ b/README.md @@ -25,3 +25,8 @@ There are a couple of build options, and a couple of ways to customize the build, described in [BUILDING](BUILDING). See the [test plan](testplan.txt), and file a ticket if anything fails! + +In the event that the developers need to be contacted related to a security +incident or vulnerability, please mail [secalert@redhat.com]. + +[secalert@redhat.com]: mailto:secalert@redhat.com diff --git a/SBAT.example.md b/SBAT.example.md index 930f71a..fa86e57 100644 --- a/SBAT.example.md +++ b/SBAT.example.md @@ -5,14 +5,14 @@ SBAT: Current proposal ------------- the `.sbat` section has the following fields: -| field | meaning | -|---|---| -| component_name | the name we're comparing -| component_generation | the generation number for the comparison -| vendor_name | human readable vendor name -| vendor_package_name | human readable package name -| vendor_version | human readable package version (maybe machine parseable too, not specified here) -| vendor_url | url to look stuff up, contact, whatever. +| field | meaning | +|----------------------|----------------------------------------------------------------------------------| +| component_name | the name we're comparing | +| component_generation | the generation number for the comparison | +| vendor_name | human readable vendor name | +| vendor_package_name | human readable package name | +| vendor_version | human readable package version (maybe machine parseable too, not specified here) | +| vendor_url | url to look stuff up, contact, whatever. | `SBAT` EFI variable ----------------- diff --git a/SBAT.md b/SBAT.md index d447892..998f753 100644 --- a/SBAT.md +++ b/SBAT.md @@ -255,11 +255,11 @@ customer impact with as few re-releases as possible, while not creating an unnecessarily large UEFI revocation variable payload. | | prior to
disclosure\* | after
disclosure | after Vendor C's
first update | after Vendor C's
second update | after next global
disclosure | -|--------------------------------------------------------------------------------------|------------------------|---------------------|----------------------------------|----------------------------------|---------------------------------| -| GRUB global
generation number in
artifacts .sbat section | 3 | 4 | 4 | 4 | 5 | -| Vendor C's product-specific
generation number in artifact's
.sbat section | 1 | 1 | 2 | 3 | 1 | -| GRUB global
generation number in
UEFI SBAT revocation variable | 3 | 4 | 4 | 4 | 5 | -| Vendor C's product-specific
generation number in
UEFI SBAT revocation variable | not set | not set | 2 | 3 | not set | +|--------------------------------------------------------------------------------------|--------------------------|---------------------|----------------------------------|-----------------------------------|---------------------------------| +| GRUB global
generation number in
artifacts .sbat section | 3 | 4 | 4 | 4 | 5 | +| Vendor C's product-specific
generation number in artifact's
.sbat section | 1 | 1 | 2 | 3 | 1 | +| GRUB global
generation number in
UEFI SBAT revocation variable | 3 | 4 | 4 | 4 | 5 | +| Vendor C's product-specific
generation number in
UEFI SBAT revocation variable | not set | not set | 2 | 3 | not set | \* A disclosure is the event/date where a CVE and fixes for it are made public. @@ -307,7 +307,7 @@ most up to date UEFI metadata. Even prior to or without moving to one-shim, it is desirable to get every vendor onto as few shims as possible. Ideally a vendor would have a single shim signed with their certificate embedded and then use that certificate to sign -additional _key.EFI key files that then contain all the keys that the +additional `_key.EFI` key files that then contain all the keys that the individual components for their products are signed with. This file name needs to be registered at the time of shim review and should not be changed without going back to a shim review. A vendor should be able to store as many @@ -354,14 +354,14 @@ them. Adding a .sbat section containing the SBAT metadata structure to PE images. -| field | meaning | -|---|---| -| component_name | the name we're comparing -| component_generation | the generation number for the comparison -| vendor_name | human readable vendor name -| vendor_package_name | human readable package name -| vendor_version | human readable package version (maybe machine parseable too, not specified here) -| vendor_url | url to look stuff up, contact, whatever. +| field | meaning | +|----------------------|----------------------------------------------------------------------------------| +| component_name | the name we're comparing | +| component_generation | the generation number for the comparison | +| vendor_name | human readable vendor name | +| vendor_package_name | human readable package name | +| vendor_version | human readable package version (maybe machine parseable too, not specified here) | +| vendor_url | url to look stuff up, contact, whatever. | The format of this .sbat section is comma separated values, or more specifically ASCII encoded strings. @@ -448,7 +448,7 @@ fixed. The following show the evolution over a sample set of events: ## Starting point -Before CVEs are encountered, an undesirable moudule was built into the a fedora +Before CVEs are encountered, an undesirable module was built into Fedora's grub, so it's product-specific generation number has been bumped: ``` diff --git a/SbatLevel_Variable.txt b/SbatLevel_Variable.txt new file mode 100644 index 0000000..42a388e --- /dev/null +++ b/SbatLevel_Variable.txt @@ -0,0 +1,108 @@ +In order to apply SBAT based revocations on systems that will never +run shim, code running in boot services context needs to set the +following variable: + +Name: SbatLevel +Attributes: (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS) +Namespace Guid: 605dab50-e046-4300-abb6-3dd810dd8b23 + +Variable content: + +Initialized, no revocations: + +sbat,1,2021030218 + +To Revoke GRUB2 binaries impacted by + +* CVE-2021-3695 +* CVE-2021-3696 +* CVE-2021-3697 +* CVE-2022-28733 +* CVE-2022-28734 +* CVE-2022-28735 +* CVE-2022-28736 + +sbat,1,2022052400 +grub,2 + +and shim binaries impacted by + +* CVE-2022-28737 + +sbat,1,2022052400 +shim,2 +grub,2 + +Shim delivered both versions of these revocations with +the same 2022052400 date stamp, once as an opt-in latest +revocation with shim,2 and then as an automatic revocation without +shim,2 + + +To revoke GRUB2 grub binaries impacted by + +* CVE-2022-2601 +* CVE-2022-3775 + +sbat,1,2022111500 +shim,2 +grub,3 + +To revoke Debian's grub.3 which missed +the patches: + +sbat,1,2023012900 +shim,2 +grub,3 +grub.debian,4 + + +An additonal bug was fixed in shim that was not considered exploitable, +can be revoked by setting: + +sbat,1,2023012950 +shim,3 +grub,3 +grub.debian,4 + +shim did not deliver this payload at the time + + +To Revoke GRUB2 binaries impacted by: + +* CVE-2023-4692 +* CVE-2023-4693 + +These CVEs are in the ntfs module and vendors that do and do not +ship this module as part of their signed binary are split. + +sbat,1,2023091900 +shim,2 +grub,4 + +Since not everyone has shipped updated GRUB packages, shim did not +deliver this revocation at the time. + +To Revoke shim binaries impacted by: + +* CVE-2023-40547 +* CVE-2023-40546 +* CVE-2023-40548 +* CVE-2023-40549 +* CVE-2023-40550 +* CVE-2023-40551 + +sbat,1,2024010900 +shim,4 +grub,3 +grub.debian,4 + +Since http boot shim CVE is considerably more serious than then GRUB +ntfs CVEs shim is delivering the shim revocation without the updated +GRUB revocation as a latest payload. + +To revoke both the impacted shim and impacted GRUB binaries: + +sbat,1,2024 +shim,4 +grub,4 diff --git a/cert.S b/cert.S index 3697033..9642efc 100644 --- a/cert.S +++ b/cert.S @@ -52,3 +52,4 @@ vendor_deauthorized: #endif .Lvendor_deauthorized_end: .Lcert_table_end: + .section .note.GNU-stack,"a" diff --git a/commit b/commit deleted file mode 100644 index ba705c6..0000000 --- a/commit +++ /dev/null @@ -1 +0,0 @@ -11491619f4336fef41c3519877ba242161763580 \ No newline at end of file diff --git a/data/sbat.csv b/data/sbat.csv index 2beec75..1829372 100755 --- a/data/sbat.csv +++ b/data/sbat.csv @@ -1,2 +1,2 @@ sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md -shim,3,UEFI shim,shim,1,https://github.com/rhboot/shim +shim,4,UEFI shim,shim,1,https://github.com/rhboot/shim diff --git a/errlog.c b/errlog.c index cc6a89f..3c5e0af 100644 --- a/errlog.c +++ b/errlog.c @@ -32,6 +32,9 @@ VLogError(const char *file, int line, const char *func, const CHAR16 *fmt, ms_va_list args2; CHAR16 **newerrs; + if (file == NULL || func == NULL || fmt == NULL) + return EFI_INVALID_PARAMETER; + newerrs = ReallocatePool(errs, (nerrs + 1) * sizeof(*errs), (nerrs + 3) * sizeof(*errs)); if (!newerrs) diff --git a/fallback.c b/fallback.c index da4b25c..600cc7a 100644 --- a/fallback.c +++ b/fallback.c @@ -1006,14 +1006,14 @@ try_start_first_option(EFI_HANDLE parent_image_handle) EFI_HANDLE image_handle; if (get_fallback_verbose()) { - int fallback_verbose_wait = 500000; /* default to 0.5s */ + unsigned long fallback_verbose_wait = 500000; /* default to 0.5s */ #ifdef FALLBACK_VERBOSE_WAIT fallback_verbose_wait = FALLBACK_VERBOSE_WAIT; #endif console_print(L"Verbose enabled, sleeping for %d mseconds... " L"Press the Pause key now to hold for longer.\n", fallback_verbose_wait); - msleep(fallback_verbose_wait); + usleep(fallback_verbose_wait); } if (!first_new_option) { @@ -1036,7 +1036,7 @@ try_start_first_option(EFI_HANDLE parent_image_handle) } console_print(L"\n"); - msleep(500000000); + usleep(500000000); return efi_status; } @@ -1051,7 +1051,7 @@ try_start_first_option(EFI_HANDLE parent_image_handle) efi_status = BS->StartImage(image_handle, NULL, NULL); if (EFI_ERROR(efi_status)) { console_print(L"StartImage failed: %r\n", efi_status); - msleep(500000000); + usleep(500000000); } return efi_status; } @@ -1211,14 +1211,14 @@ reset: console_print(L"Reset System\n"); if (get_fallback_verbose()) { - int fallback_verbose_wait = 500000; /* default to 0.5s */ + unsigned long fallback_verbose_wait = 500000; /* default to 0.5s */ #ifdef FALLBACK_VERBOSE_WAIT fallback_verbose_wait = FALLBACK_VERBOSE_WAIT; #endif console_print(L"Verbose enabled, sleeping for %d mseconds... " L"Press the Pause key now to hold for longer.\n", fallback_verbose_wait); - msleep(fallback_verbose_wait); + usleep(fallback_verbose_wait); } RT->ResetSystem(EfiResetCold, EFI_SUCCESS, 0, NULL); diff --git a/fuzz-csv.c b/fuzz-csv.c new file mode 100644 index 0000000..bc701df --- /dev/null +++ b/fuzz-csv.c @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * test-csv.c - test our csv parser + */ + +#ifndef SHIM_UNIT_TEST +#define SHIM_UNIT_TEST +#endif +#include "shim.h" + +#include + +int +test_csv_simple_fuzz(char *random_bin, size_t random_bin_len) +{ + list_t entry_list; + size_t i; + char *current, *end; + list_t *pos = NULL; + EFI_STATUS efi_status; + + INIT_LIST_HEAD(&entry_list); + + current = &random_bin[0]; + current = current + 1 - 1; + end = current + random_bin_len - 1; + *end = '\0'; + + efi_status = parse_csv_data(current, end, 7, &entry_list); + if (efi_status != EFI_SUCCESS) + return 0; + if (list_size(&entry_list) <= 1) + goto fail; + + i = 0; + list_for_each(pos, &entry_list) { + struct csv_row *csv_row; + + csv_row = list_entry(pos, struct csv_row, list); + i++; + } + + free_csv_list(&entry_list); + + return 0; +fail: + free_csv_list(&entry_list); + return -1; +} + +int +LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) +{ + int rc; + uint8_t *data_copy; + + if (size < 1) + return 0; + + data_copy = malloc(size); + if (!data_copy) + return -1; + + memcpy(data_copy, data, size); + rc = test_csv_simple_fuzz((char *)data_copy, size); + free(data_copy); + + return rc; // Values other than 0 and -1 are reserved for future use. +} + +// vim:fenc=utf-8:tw=75:noet diff --git a/fuzz-pe-relocate.c b/fuzz-pe-relocate.c new file mode 100644 index 0000000..1f62234 --- /dev/null +++ b/fuzz-pe-relocate.c @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * fuzz-pe-relocate.c - fuzz our PE relocation code. + * Copyright Peter Jones + */ + +#ifndef SHIM_UNIT_TEST +#define SHIM_UNIT_TEST +#endif +#include "shim.h" + +UINT8 mok_policy = 0; + +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) +{ + uint8_t *data_copy; + EFI_STATUS status = 0; + size_t n = 0; + PE_COFF_LOADER_IMAGE_CONTEXT context = { 0, }; + + if (size < 1) + return 0; + + data_copy = malloc(size+1); + if (!data_copy) + return -1; + + memcpy(data_copy, data, size); + data_copy[size] = 0; + + status = read_header(data_copy, size, &context); + + free(data_copy); + + return 0; +} + +// vim:fenc=utf-8:tw=75:noet diff --git a/fuzz-sbat.c b/fuzz-sbat.c new file mode 100644 index 0000000..74d313b --- /dev/null +++ b/fuzz-sbat.c @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * fuzz-sbat-section.c - fuzz our .sbat parsing code + * Copyright Peter Jones + */ + +#ifndef SHIM_UNIT_TEST +#define SHIM_UNIT_TEST +#endif +#include "shim.h" + +#include + +list_t sbat_var; + +BOOLEAN +secure_mode() { + return 1; +} + +int +LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) +{ + uint8_t *data_copy; + EFI_STATUS status = 0; + size_t n = 0; + struct sbat_section_entry **entries = NULL; + + if (size < 1) + return 0; + + data_copy = malloc(size+1); + if (!data_copy) + return -1; + + memcpy(data_copy, data, size); + data_copy[size] = 0; + status = parse_sbat_section(data_copy, size, &n, &entries); + cleanup_sbat_section_entries(n, entries); + + free(data_copy); + + return 0; +} + +// vim:fenc=utf-8:tw=75:noet diff --git a/gnu-efi/Make.defaults b/gnu-efi/Make.defaults index 3b56150..59b6508 100755 --- a/gnu-efi/Make.defaults +++ b/gnu-efi/Make.defaults @@ -205,7 +205,7 @@ endif ASFLAGS += $(ARCH3264) LDFLAGS += -nostdlib --warn-common --no-undefined --fatal-warnings \ - --build-id=sha1 + --build-id=sha1 --no-warn-rwx-segments ifneq ($(ARCH),arm) export LIBGCC=$(shell $(CC) $(CFLAGS) $(ARCH3264) -print-libgcc-file-name) diff --git a/gnu-efi/apps/trivial.S b/gnu-efi/apps/trivial.S index 40bc68f..59db434 100644 --- a/gnu-efi/apps/trivial.S +++ b/gnu-efi/apps/trivial.S @@ -41,3 +41,4 @@ _start: hello: .byte 'h',0,'e',0,'l',0,'l',0,'o',0,'\n',0,'\r',0,0,0 #endif + .section .note.GNU-stack,"a" diff --git a/gnu-efi/gnuefi/crt0-efi-aarch64.S b/gnu-efi/gnuefi/crt0-efi-aarch64.S index 0fefec0..6a45aab 100644 --- a/gnu-efi/gnuefi/crt0-efi-aarch64.S +++ b/gnu-efi/gnuefi/crt0-efi-aarch64.S @@ -51,3 +51,4 @@ _start: .4byte .dummy1-.dummy0 // Page RVA .4byte 10 // Block Size (2*4+2) .2byte (IMAGE_REL_ABSOLUTE<<12) + 0 // reloc for dummy + .section .note.GNU-stack,"a" diff --git a/gnu-efi/gnuefi/crt0-efi-arm.S b/gnu-efi/gnuefi/crt0-efi-arm.S index 1efc21c..2a67438 100644 --- a/gnu-efi/gnuefi/crt0-efi-arm.S +++ b/gnu-efi/gnuefi/crt0-efi-arm.S @@ -191,3 +191,4 @@ _start: .L_DYNAMIC: .word _DYNAMIC - . + .section .note.GNU-stack,"a" diff --git a/gnu-efi/gnuefi/crt0-efi-ia32.S b/gnu-efi/gnuefi/crt0-efi-ia32.S index c9b3ea6..35c6541 100644 --- a/gnu-efi/gnuefi/crt0-efi-ia32.S +++ b/gnu-efi/gnuefi/crt0-efi-ia32.S @@ -75,3 +75,4 @@ _start: .4byte .dummy1-.dummy0 // Page RVA .4byte 10 // Block Size (2*4+2) .2byte (IMAGE_REL_ABSOLUTE<<12) + 0 // reloc for dummy + .section .note.GNU-stack,"a" diff --git a/gnu-efi/gnuefi/crt0-efi-ia64.S b/gnu-efi/gnuefi/crt0-efi-ia64.S index 40c3c83..cfe603b 100644 --- a/gnu-efi/gnuefi/crt0-efi-ia64.S +++ b/gnu-efi/gnuefi/crt0-efi-ia64.S @@ -85,3 +85,4 @@ _start_plabel: data4 12 // Block Size (2*4+2*2) data2 (IMAGE_REL_BASED_DIR64<<12) + 0 // reloc for plabel's entry point data2 (IMAGE_REL_BASED_DIR64<<12) + 8 // reloc for plabel's global pointer + .section .note.GNU-stack,"a" diff --git a/gnu-efi/gnuefi/crt0-efi-mips64el.S b/gnu-efi/gnuefi/crt0-efi-mips64el.S index 6a62aca..9169684 100644 --- a/gnu-efi/gnuefi/crt0-efi-mips64el.S +++ b/gnu-efi/gnuefi/crt0-efi-mips64el.S @@ -186,3 +186,4 @@ _pc: .end _start .set pop + .section .note.GNU-stack,"a" diff --git a/gnu-efi/gnuefi/crt0-efi-x86_64.S b/gnu-efi/gnuefi/crt0-efi-x86_64.S index 0d99c15..8a4aabf 100644 --- a/gnu-efi/gnuefi/crt0-efi-x86_64.S +++ b/gnu-efi/gnuefi/crt0-efi-x86_64.S @@ -72,3 +72,4 @@ _start: .4byte .dummy1-.dummy0 // Page RVA .4byte 10 // Block Size (2*4+2) .2byte (IMAGE_REL_ABSOLUTE<<12) + 0 // reloc for dummy + .section .note.GNU-stack,"a" diff --git a/gnu-efi/gnuefi/reloc_ia64.S b/gnu-efi/gnuefi/reloc_ia64.S index 40203bf..6e55a15 100644 --- a/gnu-efi/gnuefi/reloc_ia64.S +++ b/gnu-efi/gnuefi/reloc_ia64.S @@ -225,3 +225,4 @@ apply_FPTR64: fptr_mem_base: .space MAX_FUNCTION_DESCRIPTORS*16 fptr_mem_limit: + .section .note.GNU-stack,"a" diff --git a/gnu-efi/ia32/gnuefi/libgnuefi.a b/gnu-efi/ia32/gnuefi/libgnuefi.a new file mode 100644 index 0000000..e1a6097 Binary files /dev/null and b/gnu-efi/ia32/gnuefi/libgnuefi.a differ diff --git a/gnu-efi/ia32/lib/libefi.a b/gnu-efi/ia32/lib/libefi.a new file mode 100644 index 0000000..78a165b Binary files /dev/null and b/gnu-efi/ia32/lib/libefi.a differ diff --git a/gnu-efi/inc/aarch64/efisetjmp_arch.h b/gnu-efi/inc/aarch64/efisetjmp_arch.h index 8dbce07..ecedc40 100644 --- a/gnu-efi/inc/aarch64/efisetjmp_arch.h +++ b/gnu-efi/inc/aarch64/efisetjmp_arch.h @@ -28,6 +28,6 @@ typedef struct { UINT64 D13; UINT64 D14; UINT64 D15; -} ALIGN(JMPBUF_ALIGN) jmp_buf[1]; +} __attribute__((__aligned__(JMPBUF_ALIGN))) jmp_buf[1]; #endif /* GNU_EFI_AARCH64_SETJMP_H */ diff --git a/gnu-efi/inc/arm/efisetjmp_arch.h b/gnu-efi/inc/arm/efisetjmp_arch.h index 17f5dc0..0f1ca1e 100644 --- a/gnu-efi/inc/arm/efisetjmp_arch.h +++ b/gnu-efi/inc/arm/efisetjmp_arch.h @@ -16,6 +16,6 @@ typedef struct { UINT32 R12; UINT32 R13; UINT32 R14; -} ALIGN(JMPBUF_ALIGN) jmp_buf[1]; +} __attribute__((__aligned__(JMPBUF_ALIGN))) jmp_buf[1]; #endif /* GNU_EFI_ARM_SETJMP_H */ diff --git a/gnu-efi/inc/ia32/efisetjmp_arch.h b/gnu-efi/inc/ia32/efisetjmp_arch.h index a5c1a81..35d13b3 100644 --- a/gnu-efi/inc/ia32/efisetjmp_arch.h +++ b/gnu-efi/inc/ia32/efisetjmp_arch.h @@ -10,6 +10,6 @@ typedef struct { UINT32 Ebp; UINT32 Esp; UINT32 Eip; -} ALIGN(JMPBUF_ALIGN) jmp_buf[1]; +} __attribute__((__aligned__(JMPBUF_ALIGN))) jmp_buf[1]; #endif /* GNU_EFI_IA32_SETJMP_H */ diff --git a/gnu-efi/inc/ia64/efisetjmp_arch.h b/gnu-efi/inc/ia64/efisetjmp_arch.h index ceda448..fe290c9 100644 --- a/gnu-efi/inc/ia64/efisetjmp_arch.h +++ b/gnu-efi/inc/ia64/efisetjmp_arch.h @@ -42,6 +42,6 @@ typedef struct { UINT64 Predicates; UINT64 LoopCount; UINT64 FPSR; -} ALIGN(JMPBUF_ALIGN) jmp_buf[1]; +} __attribute__((__aligned__(JMPBUF_ALIGN))) jmp_buf[1]; #endif /* GNU_EFI_IA64_SETJMP_H */ diff --git a/gnu-efi/inc/mips64el/efisetjmp_arch.h b/gnu-efi/inc/mips64el/efisetjmp_arch.h index 2b8f756..1991a5c 100644 --- a/gnu-efi/inc/mips64el/efisetjmp_arch.h +++ b/gnu-efi/inc/mips64el/efisetjmp_arch.h @@ -29,6 +29,6 @@ typedef struct { UINT64 F30; UINT64 F31; #endif -} ALIGN(JMPBUF_ALIGN) jmp_buf[1]; +} __attribute__((__aligned__(JMPBUF_ALIGN))) jmp_buf[1]; #endif /* GNU_EFI_MIPS64EL_SETJMP_H */ diff --git a/gnu-efi/inc/x86_64/efisetjmp_arch.h b/gnu-efi/inc/x86_64/efisetjmp_arch.h index b1ad1fe..a5229f6 100644 --- a/gnu-efi/inc/x86_64/efisetjmp_arch.h +++ b/gnu-efi/inc/x86_64/efisetjmp_arch.h @@ -17,6 +17,6 @@ typedef struct { UINT64 Rip; UINT64 MxCsr; UINT8 XmmBuffer[160]; // XMM6 - XMM15 -} ALIGN(JMPBUF_ALIGN) jmp_buf[1]; +} __attribute__((__aligned__(JMPBUF_ALIGN))) jmp_buf[1]; #endif /* GNU_EFI_X86_64_SETJMP_H */ diff --git a/gnu-efi/lib/aarch64/efi_stub.S b/gnu-efi/lib/aarch64/efi_stub.S index 464eae5..fa951c9 100644 --- a/gnu-efi/lib/aarch64/efi_stub.S +++ b/gnu-efi/lib/aarch64/efi_stub.S @@ -1 +1,2 @@ /* This stub is a stub to make the build happy */ + .section .note.GNU-stack,"a" diff --git a/gnu-efi/lib/aarch64/setjmp.S b/gnu-efi/lib/aarch64/setjmp.S index 46c29b1..ce18bd8 100644 --- a/gnu-efi/lib/aarch64/setjmp.S +++ b/gnu-efi/lib/aarch64/setjmp.S @@ -58,3 +58,4 @@ longjmp: mov w0, #1 csel w0, w1, w0, ne br x30 + .section .note.GNU-stack,"a" diff --git a/gnu-efi/lib/arm/div.S b/gnu-efi/lib/arm/div.S index 71158b6..86e8069 100644 --- a/gnu-efi/lib/arm/div.S +++ b/gnu-efi/lib/arm/div.S @@ -153,3 +153,4 @@ label1: @ What to do about division by zero? For now, just return. ASM_PFX(__aeabi_idiv0): bx r14 + .section .note.GNU-stack,"a" diff --git a/gnu-efi/lib/arm/efi_stub.S b/gnu-efi/lib/arm/efi_stub.S index 464eae5..fa951c9 100644 --- a/gnu-efi/lib/arm/efi_stub.S +++ b/gnu-efi/lib/arm/efi_stub.S @@ -1 +1,2 @@ /* This stub is a stub to make the build happy */ + .section .note.GNU-stack,"a" diff --git a/gnu-efi/lib/arm/ldivmod.S b/gnu-efi/lib/arm/ldivmod.S index edbf89e..33afa60 100644 --- a/gnu-efi/lib/arm/ldivmod.S +++ b/gnu-efi/lib/arm/ldivmod.S @@ -59,3 +59,4 @@ L_Exit: + .section .note.GNU-stack,"a" diff --git a/gnu-efi/lib/arm/llsl.S b/gnu-efi/lib/arm/llsl.S index 0f5c407..c556cd1 100644 --- a/gnu-efi/lib/arm/llsl.S +++ b/gnu-efi/lib/arm/llsl.S @@ -39,3 +39,4 @@ ASM_PFX(__aeabi_llsl): lsl r1,r0,r3 mov r0,#0 bx lr + .section .note.GNU-stack,"a" diff --git a/gnu-efi/lib/arm/llsr.S b/gnu-efi/lib/arm/llsr.S index 432b27d..096b728 100644 --- a/gnu-efi/lib/arm/llsr.S +++ b/gnu-efi/lib/arm/llsr.S @@ -39,3 +39,4 @@ ASM_PFX(__aeabi_llsr): lsr r0,r1,r3 mov r1,#0 bx lr + .section .note.GNU-stack,"a" diff --git a/gnu-efi/lib/arm/mullu.S b/gnu-efi/lib/arm/mullu.S index 39b9a80..de71551 100644 --- a/gnu-efi/lib/arm/mullu.S +++ b/gnu-efi/lib/arm/mullu.S @@ -31,3 +31,4 @@ ASM_PFX(__aeabi_lmul): mla r1, r2, r1, ip mla r1, r3, lr, r1 ldmia sp!, {pc} + .section .note.GNU-stack,"a" diff --git a/gnu-efi/lib/arm/setjmp.S b/gnu-efi/lib/arm/setjmp.S index bd61a8d..851d1d5 100644 --- a/gnu-efi/lib/arm/setjmp.S +++ b/gnu-efi/lib/arm/setjmp.S @@ -23,3 +23,4 @@ setjmp: .type longjmp, %function longjmp: ldmia r0, {r3-r12,r14} + .section .note.GNU-stack,"a" diff --git a/gnu-efi/lib/arm/uldiv.S b/gnu-efi/lib/arm/uldiv.S index f478898..bd2de59 100644 --- a/gnu-efi/lib/arm/uldiv.S +++ b/gnu-efi/lib/arm/uldiv.S @@ -265,3 +265,4 @@ ASM_PFX(__aeabi_ldiv0): bx r14 + .section .note.GNU-stack,"a" diff --git a/gnu-efi/lib/ia32/efi_stub.S b/gnu-efi/lib/ia32/efi_stub.S index 464eae5..fa951c9 100644 --- a/gnu-efi/lib/ia32/efi_stub.S +++ b/gnu-efi/lib/ia32/efi_stub.S @@ -1 +1,2 @@ /* This stub is a stub to make the build happy */ + .section .note.GNU-stack,"a" diff --git a/gnu-efi/lib/ia32/setjmp.S b/gnu-efi/lib/ia32/setjmp.S index aa9c084..68a00a8 100644 --- a/gnu-efi/lib/ia32/setjmp.S +++ b/gnu-efi/lib/ia32/setjmp.S @@ -43,3 +43,4 @@ longjmp: movl (%edx), %ebx movl 4(%edx), %esi movl 8(%edx), %edi + .section .note.GNU-stack,"a" diff --git a/gnu-efi/lib/ia64/palproc.S b/gnu-efi/lib/ia64/palproc.S index c304a78..8ee6f9c 100644 --- a/gnu-efi/lib/ia64/palproc.S +++ b/gnu-efi/lib/ia64/palproc.S @@ -159,3 +159,4 @@ StackedComeBackFromPALCall: PROCEDURE_EXIT(MakeStackedPALCall) + .section .note.GNU-stack,"a" diff --git a/gnu-efi/lib/ia64/setjmp.S b/gnu-efi/lib/ia64/setjmp.S index bbb29d8..ba0fbd6 100644 --- a/gnu-efi/lib/ia64/setjmp.S +++ b/gnu-efi/lib/ia64/setjmp.S @@ -197,3 +197,4 @@ _skip_flushrs: invala mov ar.rsc = r16 br.ret.sptk b0 + .section .note.GNU-stack,"a" diff --git a/gnu-efi/lib/mips64el/efi_stub.S b/gnu-efi/lib/mips64el/efi_stub.S index 464eae5..fa951c9 100644 --- a/gnu-efi/lib/mips64el/efi_stub.S +++ b/gnu-efi/lib/mips64el/efi_stub.S @@ -1 +1,2 @@ /* This stub is a stub to make the build happy */ + .section .note.GNU-stack,"a" diff --git a/gnu-efi/lib/mips64el/setjmp.S b/gnu-efi/lib/mips64el/setjmp.S index 930aca4..a620a6e 100644 --- a/gnu-efi/lib/mips64el/setjmp.S +++ b/gnu-efi/lib/mips64el/setjmp.S @@ -90,3 +90,4 @@ longjmp: li $v0, 1 movn $v0, $a1, $a1 jr $ra + .section .note.GNU-stack,"a" diff --git a/gnu-efi/lib/x86_64/efi_stub.S b/gnu-efi/lib/x86_64/efi_stub.S index b431255..16542e2 100644 --- a/gnu-efi/lib/x86_64/efi_stub.S +++ b/gnu-efi/lib/x86_64/efi_stub.S @@ -187,3 +187,4 @@ ENTRY(efi_call10) ret #endif + .section .note.GNU-stack,"a" diff --git a/gnu-efi/lib/x86_64/setjmp.S b/gnu-efi/lib/x86_64/setjmp.S index e3e5195..56653d7 100644 --- a/gnu-efi/lib/x86_64/setjmp.S +++ b/gnu-efi/lib/x86_64/setjmp.S @@ -46,3 +46,4 @@ longjmp: cmp %rax,%rdx cmove %rcx,%rax jmp *0x38(%rdi) + .section .note.GNU-stack,"a" diff --git a/httpboot.c b/httpboot.c index dfa493b..ac9ea25 100644 --- a/httpboot.c +++ b/httpboot.c @@ -578,7 +578,13 @@ receive_http_response(EFI_HTTP_PROTOCOL *http, VOID **buffer, UINT64 *buf_size) } if (*buf_size == 0) { - perror(L"Failed to get Content-Lenght\n"); + perror(L"Failed to get Content-Length\n"); + goto error; + } + + if (*buf_size < rx_message.BodyLength) { + efi_status = EFI_BAD_BUFFER_SIZE; + perror(L"Invalid Content-Length\n"); goto error; } @@ -713,18 +719,20 @@ error: } EFI_STATUS -httpboot_fetch_buffer (EFI_HANDLE image, VOID **buffer, UINT64 *buf_size) +httpboot_fetch_buffer (EFI_HANDLE image, VOID **buffer, UINT64 *buf_size, + CHAR8 *name) { EFI_STATUS efi_status; EFI_HANDLE nic; - CHAR8 next_loader[sizeof DEFAULT_LOADER_CHAR]; + CHAR8 *next_loader; CHAR8 *next_uri = NULL; CHAR8 *hostname = NULL; if (!uri) return EFI_NOT_READY; - translate_slashes(next_loader, DEFAULT_LOADER_CHAR); + next_loader = (CHAR8 *)AllocatePool((strlen(name) + 1) * sizeof (CHAR8)); + translate_slashes(next_loader, name); /* Create the URI for the next loader based on the original URI */ efi_status = generate_next_uri(uri, next_loader, &next_uri); diff --git a/include/asm.h b/include/asm.h index 03b0655..f5118b2 100644 --- a/include/asm.h +++ b/include/asm.h @@ -40,11 +40,11 @@ static inline void wait_for_debug(void) { uint64_t a, b; int x; - extern void msleep(unsigned long msecs); + extern void usleep(unsigned long usecs); a = read_counter(); for (x = 0; x < 1000; x++) { - msleep(1000); + usleep(1000); b = read_counter(); if (a != b) break; diff --git a/include/compiler.h b/include/compiler.h index b0d595f..8e8a658 100644 --- a/include/compiler.h +++ b/include/compiler.h @@ -198,5 +198,55 @@ #error shim has no cache_invalidate() implementation for this compiler #endif /* __GNUC__ */ +#if defined(__GNUC__) && defined(__GNUC_MINOR__) +#define GNUC_PREREQ(maj, min) \ + ((__GNUC__ << 16) + __GNUC_MINOR__ >= ((maj) << 16) + (min)) +#else +#define GNUC_PREREQ(maj, min) 0 +#endif + +#if defined(__clang__) && defined(__clang_major__) && defined(__clang_minor__) +#define CLANG_PREREQ(maj, min) \ + ((__clang_major__ > (maj)) || \ + (__clang_major__ == (maj) && __clang_minor__ >= (min))) +#else +#define CLANG_PREREQ(maj, min) 0 +#endif + +#if GNUC_PREREQ(5, 1) || CLANG_PREREQ(3, 8) +#define checked_add(addend0, addend1, sum) \ + __builtin_add_overflow(addend0, addend1, sum) +#define checked_sub(minuend, subtrahend, difference) \ + __builtin_sub_overflow(minuend, subtrahend, difference) +#define checked_mul(factor0, factor1, product) \ + __builtin_mul_overflow(factor0, factor1, product) +#else +#define checked_add(a0, a1, s) \ + ({ \ + (*s) = ((a0) + (a1)); \ + 0; \ + }) +#define checked_sub(s0, s1, d) \ + ({ \ + (*d) = ((s0) - (s1)); \ + 0; \ + }) +#define checked_mul(f0, f1, p) \ + ({ \ + (*p) = ((f0) * (f1)); \ + 0; \ + }) +#endif + +#define checked_div(dividend, divisor, quotient) \ + ({ \ + bool _ret = True; \ + if ((divisor) != 0) { \ + _ret = False; \ + (quotient) = (dividend) / (divisor); \ + } \ + _ret; \ + }) + #endif /* !COMPILER_H_ */ // vim:fenc=utf-8:tw=75:et diff --git a/include/console.h b/include/console.h index 0c4a513..7ac4e11 100644 --- a/include/console.h +++ b/include/console.h @@ -106,8 +106,8 @@ extern UINT32 verbose; dprint_(L"%a:%d:%a() " fmt, __FILE__, __LINE__ - 1, __func__, \ ##__VA_ARGS__) #else -#define dprint_(...) -#define dprint(fmt, ...) +#define dprint_(...) ({ ; }) +#define dprint(fmt, ...) ({ ; }) #endif extern EFI_STATUS EFIAPI vdprint_(const CHAR16 *fmt, const char *file, int line, @@ -122,7 +122,9 @@ extern EFI_STATUS EFIAPI vdprint_(const CHAR16 *fmt, const char *file, int line, extern EFI_STATUS print_crypto_errors(EFI_STATUS rc, char *file, const char *func, int line); #define crypterr(rc) print_crypto_errors((rc), __FILE__, __func__, __LINE__) -extern VOID msleep(unsigned long msecs); +#ifndef SHIM_UNIT_TEST +extern VOID usleep(unsigned long usecs); +#endif /* This is used in various things to determine if we should print to the * console */ diff --git a/include/fanalyzer.mk b/include/fanalyzer.mk index e0bf4d7..a0679e3 100644 --- a/include/fanalyzer.mk +++ b/include/fanalyzer.mk @@ -21,6 +21,7 @@ fanalyzer-build-all : COMPILER=gcc fanalyzer-build-all : CCACHE_DISABLE=1 fanalyzer-build-all : FEATUREFLAGS+=-fanalyzer fanalyzer-build-all : WERRFLAGS=-Werror=analyzer-null-dereference +fanalyzer-build-all : IGNORE_COMPILER_ERRORS=" || :" fanalyzer-build-all : all fanalyzer-no-openssl : | fanalyzer-test diff --git a/include/fuzz.mk b/include/fuzz.mk new file mode 100644 index 0000000..f35df41 --- /dev/null +++ b/include/fuzz.mk @@ -0,0 +1,97 @@ +# SPDX-License-Identifier: BSD-2-Clause-Patent +# +# fuzz.mk - makefile to fuzz local test programs +# + +.SUFFIXES: + +include Make.defaults + +CC = clang +VALGRIND ?= +DEBUG_PRINTS ?= 0 +OPTIMIZATIONS ?= -Og -ggdb +FUZZ_ARGS ?= +CFLAGS = $(OPTIMIZATIONS) -std=gnu11 \ + -isystem $(TOPDIR)/include/system \ + $(EFI_INCLUDES) \ + -Iinclude -iquote . \ + -isystem /usr/include \ + -isystem $(shell $(CC) $(ARCH_CFLAGS) -print-file-name=include) \ + $(ARCH_CFLAGS) \ + -fsanitize=fuzzer,address \ + -fshort-wchar \ + -fno-builtin \ + -rdynamic \ + -fno-inline \ + -fno-eliminate-unused-debug-types \ + -fno-eliminate-unused-debug-symbols \ + -gpubnames \ + -grecord-gcc-switches \ + $(if $(findstring clang,$(CC)),-Wno-unknown-warning-option) \ + $(DEFAULT_WARNFLAGS) \ + -Wsign-compare \ + -Wno-deprecated-declarations \ + $(if $(findstring gcc,$(CC)),-Wno-unused-but-set-variable) \ + -Wno-unused-but-set-variable \ + -Wno-unused-variable \ + -Wno-pointer-sign \ + $(DEFAULT_WERRFLAGS) \ + -Werror=nonnull \ + $(shell $(CC) -Werror=nonnull-compare -E -x c /dev/null >/dev/null 2>&1 && echo -Werror=nonnull-compare) \ + $(ARCH_DEFINES) \ + -DEFI_FUNCTION_WRAPPER \ + -DGNU_EFI_USE_MS_ABI -DPAGE_SIZE=4096 \ + -DSHIM_UNIT_TEST \ + -DSHIM_ENABLE_LIBFUZZER \ + "-DDEFAULT_DEBUG_PRINT_STATE=$(DEBUG_PRINTS)" + +# On some systems (e.g. Arch Linux), limits.h is in the "include-fixed" instead +# of the "include" directory +CFLAGS += -isystem $(shell $(CC) $(ARCH_CFLAGS) -print-file-name=include-fixed) + +# And on Debian also check the multi-arch include path +CFLAGS += -isystem /usr/include/$(shell $(CC) $(ARCH_CFLAGS) -print-multiarch) + +libefi-test.a : + $(MAKE) -C gnu-efi \ + COMPILER="$(COMPILER)" \ + CC="$(CC)" \ + ARCH=$(ARCH_GNUEFI) \ + TOPDIR=$(TOPDIR)/gnu-efi \ + -f $(TOPDIR)/gnu-efi/Makefile \ + clean lib + mv gnu-efi/$(ARCH)/lib/libefi.a $@ + $(MAKE) -C gnu-efi \ + COMPILER="$(COMPILER)" \ + ARCH=$(ARCH_GNUEFI) \ + TOPDIR=$(TOPDIR)/gnu-efi \ + -f $(TOPDIR)/gnu-efi/Makefile \ + clean + +fuzz-sbat_FILES = csv.c lib/variables.c lib/guid.c sbat_var.S mock-variables.c +fuzz-sbat :: CFLAGS+=-DHAVE_GET_VARIABLE -DHAVE_GET_VARIABLE_ATTR -DHAVE_SHIM_LOCK_GUID + +fuzzers := $(patsubst %.c,%,$(wildcard fuzz-*.c)) + +$(fuzzers) :: fuzz-% : | libefi-test.a + +$(fuzzers) :: fuzz-% : test.c fuzz-%.c $(fuzz-%_FILES) + $(CC) $(CFLAGS) -o $@ $(sort $^ $(wildcard $*.c) $(fuzz-$*_FILES)) libefi-test.a -lefivar + $(VALGRIND) ./$@ -max_len=4096 -jobs=24 $(FUZZ_ARGS) + +fuzz : $(fuzzers) + $(MAKE) -f include/fuzz.mk fuzz-clean + +fuzz-clean : + @rm -vf random.bin libefi-test.a + @rm -vf vgcore.* fuzz*.log + +clean : fuzz-clean + +all : fuzz-clean fuzz + +.PHONY: $(fuzzers) all fuzz clean +.SECONDARY: random.bin + +# vim:ft=make diff --git a/include/guid.h b/include/guid.h index dad63f0..898c4fa 100644 --- a/include/guid.h +++ b/include/guid.h @@ -37,5 +37,6 @@ extern EFI_GUID SECURITY2_PROTOCOL_GUID; extern EFI_GUID EFI_MEMORY_ATTRIBUTE_PROTOCOL_GUID; extern EFI_GUID SHIM_LOCK_GUID; extern EFI_GUID MOK_VARIABLE_STORE; +extern EFI_GUID SECUREBOOT_EFI_NAMESPACE_GUID; #endif /* SHIM_GUID_H */ diff --git a/include/httpboot.h b/include/httpboot.h index ea9c57f..119c546 100644 --- a/include/httpboot.h +++ b/include/httpboot.h @@ -12,6 +12,6 @@ extern BOOLEAN find_httpboot(EFI_HANDLE device); extern EFI_STATUS httpboot_fetch_buffer(EFI_HANDLE image, VOID **buffer, - UINT64 *buf_size); + UINT64 *buf_size, CHAR8 *name); #endif /* SHIM_HTTPBOOT_H */ diff --git a/include/netboot.h b/include/netboot.h index 98b174a..a7bf6cd 100644 --- a/include/netboot.h +++ b/include/netboot.h @@ -5,7 +5,7 @@ extern BOOLEAN findNetboot(EFI_HANDLE image_handle); -extern EFI_STATUS parseNetbootinfo(EFI_HANDLE image_handle); +extern EFI_STATUS parseNetbootinfo(EFI_HANDLE image_handle, CHAR8 *name); extern EFI_STATUS FetchNetbootimage(EFI_HANDLE image_handle, VOID **buffer, UINT64 *bufsiz); diff --git a/include/pe.h b/include/pe.h index ccc8798..9ea9eb4 100644 --- a/include/pe.h +++ b/include/pe.h @@ -21,6 +21,20 @@ EFI_STATUS verify_image(void *data, unsigned int datasize, EFI_STATUS verify_sbat_section(char *SBATBase, size_t SBATSize); +EFI_STATUS +get_section_vma (UINTN section_num, + char *buffer, size_t bufsz UNUSED, + PE_COFF_LOADER_IMAGE_CONTEXT *context, + char **basep, size_t *sizep, + EFI_IMAGE_SECTION_HEADER **sectionp); + +EFI_STATUS +get_section_vma_by_name (char *name, size_t namesz, + char *buffer, size_t bufsz, + PE_COFF_LOADER_IMAGE_CONTEXT *context, + char **basep, size_t *sizep, + EFI_IMAGE_SECTION_HEADER **sectionp); + EFI_STATUS handle_image (void *data, unsigned int datasize, EFI_LOADED_IMAGE *li, diff --git a/include/peimage.h b/include/peimage.h index e97b29c..6eef105 100644 --- a/include/peimage.h +++ b/include/peimage.h @@ -29,6 +29,9 @@ #define ALIGN_VALUE(Value, Alignment) ((Value) + (((Alignment) - (Value)) & ((Alignment) - 1))) #define ALIGN_POINTER(Pointer, Alignment) ((VOID *) (ALIGN_VALUE ((UINTN)(Pointer), (Alignment)))) +// Check if `val` is evenly aligned to the page size. +#define IS_PAGE_ALIGNED(val) (!((val) & EFI_PAGE_MASK)) + // // PE32+ Subsystem type for EFI images // diff --git a/include/sbat.h b/include/sbat.h index c94c4fb..bb523e7 100644 --- a/include/sbat.h +++ b/include/sbat.h @@ -30,10 +30,15 @@ #define SBAT_POLICY L"SbatPolicy" #define SBAT_POLICY8 "SbatPolicy" +#define SSP_POLICY L"SSPPolicy" +#define SSP_POLICY8 "SSPPolicy" -#define SBAT_POLICY_LATEST 1 -#define SBAT_POLICY_PREVIOUS 2 -#define SBAT_POLICY_RESET 3 +#define POLICY_LATEST 1 +#define POLICY_AUTOMATIC 2 +#define POLICY_RESET 3 +#define POLICY_NOTREAD 255 + +#define REVOCATIONFILE L"revocations.efi" extern UINTN _sbat, _esbat; @@ -50,9 +55,10 @@ extern list_t sbat_var; #define SBAT_VAR_COLUMNS ((sizeof (struct sbat_var_entry) - sizeof(list_t)) / sizeof(CHAR8 *)) #define SBAT_VAR_REQUIRED_COLUMNS (SBAT_VAR_COLUMNS - 1) -EFI_STATUS parse_sbat_var(list_t *entries); +EFI_STATUS parse_sbat_var(list_t *entries, char *sbat_var_candidate); void cleanup_sbat_var(list_t *entries); -EFI_STATUS set_sbat_uefi_variable(void); +EFI_STATUS set_sbat_uefi_variable_internal(void); +EFI_STATUS set_sbat_uefi_variable(char *, char *); bool preserve_sbat_uefi_variable(UINT8 *sbat, UINTN sbatsize, UINT32 attributes, char *sbar_var); diff --git a/include/sbat_var_defs.h b/include/sbat_var_defs.h index 6b01573..f8cba02 100644 --- a/include/sbat_var_defs.h +++ b/include/sbat_var_defs.h @@ -3,6 +3,9 @@ #ifndef SBAT_VAR_DEFS_H_ #define SBAT_VAR_DEFS_H_ +#define QUOTEVAL(s) QUOTE(s) +#define QUOTE(s) #s + /* * This is the entry for the sbat data format */ @@ -13,11 +16,9 @@ SBAT_VAR_SIG SBAT_VAR_VERSION SBAT_VAR_ORIGINAL_DATE "\n" #if defined(ENABLE_SHIM_DEVEL) -#define SBAT_VAR_PREVIOUS_DATE "2022020101" -#define SBAT_VAR_PREVIOUS_REVOCATIONS "component,2\n" -#define SBAT_VAR_PREVIOUS \ - SBAT_VAR_SIG SBAT_VAR_VERSION SBAT_VAR_PREVIOUS_DATE "\n" \ - SBAT_VAR_PREVIOUS_REVOCATIONS +#define SBAT_VAR_AUTOMATIC_DATE "2021030218" +#define SBAT_VAR_AUTOMATIC \ + SBAT_VAR_SIG SBAT_VAR_VERSION SBAT_VAR_AUTOMATIC_DATE "\n" #define SBAT_VAR_LATEST_DATE "2022050100" #define SBAT_VAR_LATEST_REVOCATIONS "component,2\nothercomponent,2\n" @@ -25,21 +26,42 @@ SBAT_VAR_SIG SBAT_VAR_VERSION SBAT_VAR_LATEST_DATE "\n" \ SBAT_VAR_LATEST_REVOCATIONS #else /* !ENABLE_SHIM_DEVEL */ -/* - * As of 2022-11-16, most folks (including Ubuntu, SUSE, openSUSE) don't have - * a "shim,2" yet, so adding that here would end up unbootable. - */ -#define SBAT_VAR_PREVIOUS_DATE "2022052400" -#define SBAT_VAR_PREVIOUS_REVOCATIONS "grub,2\n" -#define SBAT_VAR_PREVIOUS \ - SBAT_VAR_SIG SBAT_VAR_VERSION SBAT_VAR_PREVIOUS_DATE "\n" \ - SBAT_VAR_PREVIOUS_REVOCATIONS -#define SBAT_VAR_LATEST_DATE "2022111500" -#define SBAT_VAR_LATEST_REVOCATIONS "shim,2\ngrub,3\n" +/* + * Some distros may want to apply revocations from 2022052400 + * or 2022111500 automatically. They can be selected by setting + * SBAT_AUTOMATIC_DATE= at build time. Otherwise the + * default is to apply the second to most recent revocations + * automatically. Distros that need to manage automatic updates + * externally from shim can choose the epoch 2021030218 emtpy + * revocations. + */ +#ifndef SBAT_AUTOMATIC_DATE +#define SBAT_AUTOMATIC_DATE 2023012900 +#endif /* SBAT_AUTOMATIC_DATE */ +#if SBAT_AUTOMATIC_DATE == 2021030218 +#define SBAT_VAR_AUTOMATIC_REVOCATIONS +#elif SBAT_AUTOMATIC_DATE == 2022052400 +#define SBAT_VAR_AUTOMATIC_REVOCATIONS "grub,2\n" +#elif SBAT_AUTOMATIC_DATE == 2022111500 +#define SBAT_VAR_AUTOMATIC_REVOCATIONS "shim,2\ngrub,3\n" +#elif SBAT_AUTOMATIC_DATE == 2023012900 +#define SBAT_VAR_AUTOMATIC_REVOCATIONS "shim,2\ngrub,3\ngrub.debian,4\n" +#else +#error "Unknown SBAT_AUTOMATIC_DATE" +#endif /* SBAT_AUTOMATIC_DATE == */ +#define SBAT_VAR_AUTOMATIC_DATE QUOTEVAL(SBAT_AUTOMATIC_DATE) +#define SBAT_VAR_AUTOMATIC \ + SBAT_VAR_SIG SBAT_VAR_VERSION SBAT_VAR_AUTOMATIC_DATE "\n" \ + SBAT_VAR_AUTOMATIC_REVOCATIONS + +/* + * Revocations for January 2024 shim CVEs + */ +#define SBAT_VAR_LATEST_DATE "2024010900" +#define SBAT_VAR_LATEST_REVOCATIONS "shim,4\ngrub,3\ngrub.debian,4\n" #define SBAT_VAR_LATEST \ SBAT_VAR_SIG SBAT_VAR_VERSION SBAT_VAR_LATEST_DATE "\n" \ SBAT_VAR_LATEST_REVOCATIONS #endif /* ENABLE_SHIM_DEVEL */ - #endif /* !SBAT_VAR_DEFS_H_ */ diff --git a/include/scan-build.mk b/include/scan-build.mk index 3ed7660..170ba83 100644 --- a/include/scan-build.mk +++ b/include/scan-build.mk @@ -22,6 +22,7 @@ scan-build-unchecked-openssl : Cryptlib/OpenSSL/libopenssl.a scan-build-all : CCACHE_DISABLE=1 scan-build-all : COMPILER=clang +scan-build-all : IGNORE_COMPILER_ERRORS=" || :" scan-build-all : | scan-test scan-build-all : +scan-build -o scan-results make $(MAKEARGS) $(DASHJ) CCACHE_DISABLE=1 all diff --git a/include/ssp.h b/include/ssp.h new file mode 100644 index 0000000..f25590c --- /dev/null +++ b/include/ssp.h @@ -0,0 +1,14 @@ +#ifndef SSP_H_ +#define SSP_H_ + +#define SSPVER_VAR_NAME L"SkuSiPolicyVersion" +#define SSPSIG_VAR_NAME L"SkuSiPolicyUpdateSigners" +#define SSP_VAR_ATTRS UEFI_VAR_NV_BS + +#define SSPVER_SIZE 8 +#define SSPSIG_SIZE 131 + +EFI_STATUS set_ssp_uefi_variable_internal(void); +EFI_STATUS set_ssp_uefi_variable(uint8_t*, uint8_t*, uint8_t*, uint8_t*); + +#endif /* !SSP_H_ */ diff --git a/include/ssp_var_defs.h b/include/ssp_var_defs.h new file mode 100644 index 0000000..4bfad87 --- /dev/null +++ b/include/ssp_var_defs.h @@ -0,0 +1,19 @@ +/* + * variable definitions to enable bootmgr self revocation + */ +#ifndef SSP_VAR_DEFS_H_ +#define SSP_VAR_DEFS_H_ + +uint8_t SkuSiPolicyVersion[] = { 0x2,0x0,0x0,0x0,0x0,0x0,0x2,0x0 }; +uint8_t SkuSiPolicyUpdateSigners[] = { +0x01,0x00,0x00,0x00,0x06,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, +0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x01,0x00,0x00,0x00, +0x0b,0x00,0x00,0x00,0xd0,0x91,0x73,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x02,0x00, +0x00,0x00,0x00,0x00,0x54,0xa6,0x78,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x02,0x00, +0x00,0x00,0x00,0x00,0x5c,0xa6,0x78,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x02,0x00, +0x00,0x00,0x00,0x00,0x64,0xa6,0x78,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, +0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, +0x00,0x00,0x00,0x00,0x0a,0x2b,0x06,0x01,0x04,0x01,0x82,0x37,0x0a,0x03,0x06,0x00, +0x00,0x00,0x00 }; + +#endif /* !SSP_VAR_DEFS_H_ */ diff --git a/include/test.mk b/include/test.mk index c0e2409..e6d4659 100644 --- a/include/test.mk +++ b/include/test.mk @@ -92,9 +92,12 @@ test-mock-variables: CFLAGS+=-DHAVE_SHIM_LOCK_GUID test-mok-mirror_FILES = mok.c globals.c tpm.c lib/guid.c lib/variables.c mock-variables.c test-mok-mirror: CFLAGS+=-DHAVE_START_IMAGE -DHAVE_SHIM_LOCK_GUID -test-sbat_FILES = csv.c lib/variables.c lib/guid.c sbat_var.S +test-sbat_FILES = csv.c lib/variables.c lib/guid.c sbat_var.S mock-variables.c test-sbat :: CFLAGS+=-DHAVE_GET_VARIABLE -DHAVE_GET_VARIABLE_ATTR -DHAVE_SHIM_LOCK_GUID +test-pe-relocate_FILES = globals.c +test-pe-relocate :: CFLAGS+=-DHAVE_SHIM_LOCK_GUID + test-str_FILES = lib/string.c tests := $(patsubst %.c,%,$(wildcard test-*.c)) diff --git a/lib/console.c b/lib/console.c index 6bbb8a7..a751f79 100644 --- a/lib/console.c +++ b/lib/console.c @@ -743,11 +743,13 @@ setup_verbosity(VOID) setup_console(-1); } +#ifndef SHIM_UNIT_TEST VOID -msleep(unsigned long msecs) +usleep(unsigned long usecs) { - BS->Stall(msecs); + BS->Stall(usecs); } +#endif /* This is used in various things to determine if we should print to the * console */ diff --git a/lib/guid.c b/lib/guid.c index 904629e..6e92cea 100644 --- a/lib/guid.c +++ b/lib/guid.c @@ -36,3 +36,4 @@ EFI_GUID SECURITY2_PROTOCOL_GUID = { 0x94ab2f58, 0x1438, 0x4ef1, {0x91, 0x52, 0x EFI_GUID EFI_MEMORY_ATTRIBUTE_PROTOCOL_GUID = { 0xf4560cf6, 0x40ec, 0x4b4a, {0xa1, 0x92, 0xbf, 0x1d, 0x57, 0xd0, 0xb1, 0x89} }; EFI_GUID SHIM_LOCK_GUID = {0x605dab50, 0xe046, 0x4300, {0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23 } }; EFI_GUID MOK_VARIABLE_STORE = {0xc451ed2b, 0x9694, 0x45d3, {0xba, 0xba, 0xed, 0x9f, 0x89, 0x88, 0xa3, 0x89} }; +EFI_GUID SECUREBOOT_EFI_NAMESPACE_GUID = {0x77fa9abd, 0x0359, 0x4d32, {0xbd, 0x60, 0x28, 0xf4, 0xe7, 0x8f, 0x78, 0x4b} }; diff --git a/model.c b/model.c index e122ba9..6052ff5 100644 --- a/model.c +++ b/model.c @@ -54,7 +54,7 @@ gcm_gmult_4bit(u64 Xi[2], u128 Htable[16]) } void -msleep(int n) +usleep(int n) { __coverity_sleep__(); } diff --git a/mok.c b/mok.c index 9811b35..0ac3415 100644 --- a/mok.c +++ b/mok.c @@ -291,7 +291,7 @@ mirror_one_esl(CHAR16 *name, EFI_GUID *guid, UINT32 attrs, &var, &varsz); if (EFI_ERROR(efi_status) || !var || !varsz) { LogError(L"Couldn't allocate %lu bytes for mok variable \"%s\": %r\n", - varsz, var, efi_status); + varsz, name, efi_status); return efi_status; } @@ -302,7 +302,7 @@ mirror_one_esl(CHAR16 *name, EFI_GUID *guid, UINT32 attrs, FreePool(var); if (EFI_ERROR(efi_status)) { LogError(L"Couldn't create mok variable \"%s\": %r\n", - varsz, var, efi_status); + name, efi_status); return efi_status; } @@ -423,12 +423,20 @@ mirror_mok_db(CHAR16 *name, CHAR8 *name8, EFI_GUID *guid, UINT32 attrs, } /* The name counts towards the size of the variable */ - max_var_sz -= (StrLen(namen) + 1) * 2; + SIZE_T namen_sz = (StrLen(namen) + 1) * 2; + if (max_var_sz > namen_sz) + max_var_sz -= namen_sz; + else + max_var_sz = 0; dprint(L"max_var_sz - name: %lx\n", max_var_sz); SIZE_T howmany; - howmany = MIN((max_var_sz - sizeof(*esl)) / esl->SignatureSize, - (esl_end_pos - pos) / esl->SignatureSize); + if (max_var_sz > sizeof(*esl)) + howmany = MIN((max_var_sz - sizeof(*esl)) / esl->SignatureSize, + (esl_end_pos - pos) / esl->SignatureSize); + else + howmany = 0; + if (howmany == 0) { /* No signatures from this ESL can be mirrored in to a * single variable, so skip it. diff --git a/netboot.c b/netboot.c index cf5882c..d8b1093 100644 --- a/netboot.c +++ b/netboot.c @@ -160,25 +160,30 @@ static CHAR8 *str2ip6(CHAR8 *str) return (CHAR8 *)ip; } -static BOOLEAN extract_tftp_info(CHAR8 *url) +static BOOLEAN extract_tftp_info(CHAR8 *url, CHAR8 *name) { CHAR8 *start, *end; CHAR8 ip6str[40]; CHAR8 ip6inv[16]; - CHAR8 template[sizeof DEFAULT_LOADER_CHAR]; + int template_len = 0; + CHAR8 *template; - translate_slashes(template, DEFAULT_LOADER_CHAR); + while (name[template_len++] != '\0'); + template = (CHAR8 *)AllocatePool((template_len + 1) * sizeof (CHAR8)); + translate_slashes(template, name); // to check against str2ip6() errors memset(ip6inv, 0, sizeof(ip6inv)); if (strncmp((const char *)url, (const char *)"tftp://", 7)) { console_print(L"URLS MUST START WITH tftp://\n"); + FreePool(template); return FALSE; } start = url + 7; if (*start != '[') { console_print(L"TFTP SERVER MUST BE ENCLOSED IN [..]\n"); + FreePool(template); return FALSE; } @@ -188,22 +193,28 @@ static BOOLEAN extract_tftp_info(CHAR8 *url) end++; if (end - start >= (int)sizeof(ip6str)) { console_print(L"TFTP URL includes malformed IPv6 address\n"); + FreePool(template); return FALSE; } } if (*end == '\0') { console_print(L"TFTP SERVER MUST BE ENCLOSED IN [..]\n"); + FreePool(template); return FALSE; } memset(ip6str, 0, sizeof(ip6str)); memcpy(ip6str, start, end - start); end++; memcpy(&tftp_addr.v6, str2ip6(ip6str), 16); - if (memcmp(&tftp_addr.v6, ip6inv, sizeof(ip6inv)) == 0) + if (memcmp(&tftp_addr.v6, ip6inv, sizeof(ip6inv)) == 0) { + FreePool(template); return FALSE; + } full_path = AllocateZeroPool(strlen(end)+strlen(template)+1); - if (!full_path) + if (!full_path) { + FreePool(template); return FALSE; + } memcpy(full_path, end, strlen(end)); end = (CHAR8 *)strrchr((char *)full_path, '/'); if (!end) @@ -211,10 +222,11 @@ static BOOLEAN extract_tftp_info(CHAR8 *url) memcpy(end, template, strlen(template)); end[strlen(template)] = '\0'; + FreePool(template); return TRUE; } -static EFI_STATUS parseDhcp6() +static EFI_STATUS parseDhcp6(CHAR8 *name) { EFI_PXE_BASE_CODE_DHCPV6_PACKET *packet = (EFI_PXE_BASE_CODE_DHCPV6_PACKET *)&pxe->Mode->DhcpAck.Raw; CHAR8 *bootfile_url; @@ -222,7 +234,7 @@ static EFI_STATUS parseDhcp6() bootfile_url = get_v6_bootfile_url(packet); if (!bootfile_url) return EFI_NOT_FOUND; - if (extract_tftp_info(bootfile_url) == FALSE) { + if (extract_tftp_info(bootfile_url, name) == FALSE) { FreePool(bootfile_url); return EFI_NOT_FOUND; } @@ -230,14 +242,16 @@ static EFI_STATUS parseDhcp6() return EFI_SUCCESS; } -static EFI_STATUS parseDhcp4() +static EFI_STATUS parseDhcp4(CHAR8 *name) { - CHAR8 template[sizeof DEFAULT_LOADER_CHAR]; - INTN template_len; + CHAR8 *template; + INTN template_len = 0; UINTN template_ofs = 0; EFI_PXE_BASE_CODE_DHCPV4_PACKET* pkt_v4 = (EFI_PXE_BASE_CODE_DHCPV4_PACKET *)&pxe->Mode->DhcpAck.Dhcpv4; - translate_slashes(template, DEFAULT_LOADER_CHAR); + while (name[template_len++] != '\0'); + template = (CHAR8 *)AllocatePool((template_len + 1) * sizeof (CHAR8)); + translate_slashes(template, name); template_len = strlen(template) + 1; if(pxe->Mode->ProxyOfferReceived) { @@ -263,30 +277,42 @@ static EFI_STATUS parseDhcp4() UINT8 *dir = pkt_v4->BootpBootFile; for (i = dir_len; i >= 0; i--) { - if (dir[i] == '/') + if ((dir[i] == '/') || (dir[i] == '\\')) break; } dir_len = (i >= 0) ? i + 1 : 0; full_path = AllocateZeroPool(dir_len + template_len); - if (!full_path) + if (!full_path) { + FreePool(template); return EFI_OUT_OF_RESOURCES; + } if (dir_len > 0) { strncpy(full_path, (CHAR8 *)dir, dir_len); if (full_path[dir_len-1] == '/' && template[0] == '/') full_path[dir_len-1] = '\0'; + /* + * If the path from DHCP is using backslash instead of slash, + * accept that and use it in the template in the same position + * as well. + */ + if (full_path[dir_len-1] == '\\' && template[0] == '/') { + full_path[dir_len-1] = '\0'; + template[0] = '\\'; + } } if (dir_len == 0 && dir[0] != '/' && template[0] == '/') template_ofs++; strcat(full_path, template + template_ofs); memcpy(&tftp_addr.v4, pkt_v4->BootpSiAddr, 4); + FreePool(template); return EFI_SUCCESS; } -EFI_STATUS parseNetbootinfo(EFI_HANDLE image_handle UNUSED) +EFI_STATUS parseNetbootinfo(EFI_HANDLE image_handle UNUSED, CHAR8 *netbootname) { EFI_STATUS efi_status; @@ -301,9 +327,9 @@ EFI_STATUS parseNetbootinfo(EFI_HANDLE image_handle UNUSED) * if its ipv4 or ipv6 */ if (pxe->Mode->UsingIpv6){ - efi_status = parseDhcp6(); + efi_status = parseDhcp6(netbootname); } else - efi_status = parseDhcp4(); + efi_status = parseDhcp4(netbootname); return efi_status; } @@ -315,7 +341,7 @@ EFI_STATUS FetchNetbootimage(EFI_HANDLE image_handle UNUSED, VOID **buffer, UINT BOOLEAN nobuffer = FALSE; UINTN blksz = 512; - console_print(L"Fetching Netboot Image\n"); + console_print(L"Fetching Netboot Image %a\n", full_path); if (*buffer == NULL) { *buffer = AllocatePool(4096 * 1024); if (!*buffer) diff --git a/pe-relocate.c b/pe-relocate.c new file mode 100644 index 0000000..bde7172 --- /dev/null +++ b/pe-relocate.c @@ -0,0 +1,554 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * pe-relocate.c - our PE relocation/loading (but not verification) code + * Copyright Peter Jones + */ + +#include "shim.h" + +/* + * Perform basic bounds checking of the intra-image pointers + */ +void * +ImageAddress (void *image, uint64_t size, uint64_t address) +{ + uintptr_t img_addr; + + /* ensure our local pointer isn't bigger than our size */ + if (address >= size) + return NULL; + + /* Insure our math won't overflow */ + img_addr = (uintptr_t)image; + if (checked_add(img_addr, address, &img_addr)) + return NULL; + + /* return the absolute pointer */ + return (void *)img_addr; +} + +/* + * Perform the actual relocation + */ +EFI_STATUS +relocate_coff (PE_COFF_LOADER_IMAGE_CONTEXT *context, + EFI_IMAGE_SECTION_HEADER *Section, + void *orig, void *data) +{ + EFI_IMAGE_BASE_RELOCATION *RelocBase, *RelocBaseEnd; + UINT64 Adjust; + UINT16 *Reloc, *RelocEnd; + char *Fixup, *FixupBase; + UINT16 *Fixup16; + UINT32 *Fixup32; + UINT64 *Fixup64; + int size = context->ImageSize; + void *ImageEnd = (char *)orig + size; + int n = 0; + + /* Alright, so here's how this works: + * + * context->RelocDir gives us two things: + * - the VA the table of base relocation blocks are (maybe) to be + * mapped at (RelocDir->VirtualAddress) + * - the virtual size (RelocDir->Size) + * + * The .reloc section (Section here) gives us some other things: + * - the name! kind of. (Section->Name) + * - the virtual size (Section->VirtualSize), which should be the same + * as RelocDir->Size + * - the virtual address (Section->VirtualAddress) + * - the file section size (Section->SizeOfRawData), which is + * a multiple of OptHdr->FileAlignment. Only useful for image + * validation, not really useful for iteration bounds. + * - the file address (Section->PointerToRawData) + * - a bunch of stuff we don't use that's 0 in our binaries usually + * - Flags (Section->Characteristics) + * + * and then the thing that's actually at the file address is an array + * of EFI_IMAGE_BASE_RELOCATION structs with some values packed behind + * them. The SizeOfBlock field of this structure includes the + * structure itself, and adding it to that structure's address will + * yield the next entry in the array. + */ + RelocBase = ImageAddress(orig, size, Section->PointerToRawData); + /* RelocBaseEnd here is the address of the first entry /past/ the + * table. */ + RelocBaseEnd = ImageAddress(orig, size, Section->PointerToRawData + + context->RelocDir->Size - 1); + + if (!RelocBase && !RelocBaseEnd) + return EFI_SUCCESS; + + if (!RelocBase || !RelocBaseEnd) { + perror(L"Reloc table overflows binary\n"); + return EFI_UNSUPPORTED; + } + + Adjust = (UINTN)data - context->ImageAddress; + + if (Adjust == 0) + return EFI_SUCCESS; + + while (RelocBase < RelocBaseEnd) { + Reloc = (UINT16 *) ((char *) RelocBase + sizeof (EFI_IMAGE_BASE_RELOCATION)); + + if (RelocBase->SizeOfBlock == 0) { + perror(L"Reloc %d block size 0 is invalid\n", n); + return EFI_UNSUPPORTED; + } else if (RelocBase->SizeOfBlock > context->RelocDir->Size) { + perror(L"Reloc %d block size %d greater than reloc dir" + "size %d, which is invalid\n", n, + RelocBase->SizeOfBlock, + context->RelocDir->Size); + return EFI_UNSUPPORTED; + } + + RelocEnd = (UINT16 *) ((char *) RelocBase + RelocBase->SizeOfBlock); + if ((void *)RelocEnd < orig || (void *)RelocEnd > ImageEnd) { + perror(L"Reloc %d entry overflows binary\n", n); + return EFI_UNSUPPORTED; + } + + FixupBase = ImageAddress(data, size, RelocBase->VirtualAddress); + if (!FixupBase) { + perror(L"Reloc %d Invalid fixupbase\n", n); + return EFI_UNSUPPORTED; + } + + while (Reloc < RelocEnd) { + Fixup = FixupBase + (*Reloc & 0xFFF); + switch ((*Reloc) >> 12) { + case EFI_IMAGE_REL_BASED_ABSOLUTE: + break; + + case EFI_IMAGE_REL_BASED_HIGH: + Fixup16 = (UINT16 *) Fixup; + *Fixup16 = (UINT16) (*Fixup16 + ((UINT16) ((UINT32) Adjust >> 16))); + break; + + case EFI_IMAGE_REL_BASED_LOW: + Fixup16 = (UINT16 *) Fixup; + *Fixup16 = (UINT16) (*Fixup16 + (UINT16) Adjust); + break; + + case EFI_IMAGE_REL_BASED_HIGHLOW: + Fixup32 = (UINT32 *) Fixup; + *Fixup32 = *Fixup32 + (UINT32) Adjust; + break; + + case EFI_IMAGE_REL_BASED_DIR64: + Fixup64 = (UINT64 *) Fixup; + *Fixup64 = *Fixup64 + (UINT64) Adjust; + break; + + default: + perror(L"Reloc %d Unknown relocation\n", n); + return EFI_UNSUPPORTED; + } + Reloc += 1; + } + RelocBase = (EFI_IMAGE_BASE_RELOCATION *) RelocEnd; + n++; + } + + return EFI_SUCCESS; +} + +EFI_STATUS +get_section_vma (UINTN section_num, + char *buffer, size_t bufsz UNUSED, + PE_COFF_LOADER_IMAGE_CONTEXT *context, + char **basep, size_t *sizep, + EFI_IMAGE_SECTION_HEADER **sectionp) +{ + EFI_IMAGE_SECTION_HEADER *sections = context->FirstSection; + EFI_IMAGE_SECTION_HEADER *section; + char *base = NULL, *end = NULL; + + if (section_num >= context->NumberOfSections) + return EFI_NOT_FOUND; + + if (context->FirstSection == NULL) { + perror(L"Invalid section %d requested\n", section_num); + return EFI_UNSUPPORTED; + } + + section = §ions[section_num]; + + base = ImageAddress (buffer, context->ImageSize, section->VirtualAddress); + end = ImageAddress (buffer, context->ImageSize, + section->VirtualAddress + section->Misc.VirtualSize - 1); + + if (!(section->Characteristics & EFI_IMAGE_SCN_MEM_DISCARDABLE)) { + if (!base) { + perror(L"Section %d has invalid base address\n", section_num); + return EFI_UNSUPPORTED; + } + if (!end) { + perror(L"Section %d has zero size\n", section_num); + return EFI_UNSUPPORTED; + } + } + + if (!(section->Characteristics & EFI_IMAGE_SCN_CNT_UNINITIALIZED_DATA) && + (section->VirtualAddress < context->SizeOfHeaders || + section->PointerToRawData < context->SizeOfHeaders)) { + perror(L"Section %d is inside image headers\n", section_num); + return EFI_UNSUPPORTED; + } + + if (end < base) { + perror(L"Section %d has negative size\n", section_num); + return EFI_UNSUPPORTED; + } + + *basep = base; + *sizep = end - base; + *sectionp = section; + return EFI_SUCCESS; +} + +EFI_STATUS +get_section_vma_by_name (char *name, size_t namesz, + char *buffer, size_t bufsz, + PE_COFF_LOADER_IMAGE_CONTEXT *context, + char **basep, size_t *sizep, + EFI_IMAGE_SECTION_HEADER **sectionp) +{ + UINTN i; + char namebuf[9]; + + if (!name || namesz == 0 || !buffer || bufsz < namesz || !context + || !basep || !sizep || !sectionp) + return EFI_INVALID_PARAMETER; + + /* + * This code currently is only used for ".reloc\0\0" and + * ".sbat\0\0\0", and it doesn't know how to look up longer section + * names. + */ + if (namesz > 8) + return EFI_UNSUPPORTED; + + SetMem(namebuf, sizeof(namebuf), 0); + CopyMem(namebuf, name, MIN(namesz, 8)); + + /* + * Copy the executable's sections to their desired offsets + */ + for (i = 0; i < context->NumberOfSections; i++) { + EFI_STATUS status; + EFI_IMAGE_SECTION_HEADER *section = NULL; + char *base = NULL; + size_t size = 0; + + status = get_section_vma(i, buffer, bufsz, context, &base, &size, §ion); + if (!EFI_ERROR(status)) { + if (CompareMem(section->Name, namebuf, 8) == 0) { + *basep = base; + *sizep = size; + *sectionp = section; + return EFI_SUCCESS; + } + continue; + } + + switch(status) { + case EFI_NOT_FOUND: + break; + } + } + + return EFI_NOT_FOUND; +} + +/* here's a chart: + * i686 x86_64 aarch64 + * 64-on-64: nyet yes yes + * 64-on-32: nyet yes nyet + * 32-on-32: yes yes no + */ +static int +allow_64_bit(void) +{ +#if defined(__x86_64__) || defined(__aarch64__) + return 1; +#elif defined(__i386__) || defined(__i686__) + /* Right now blindly assuming the kernel will correctly detect this + * and /halt the system/ if you're not really on a 64-bit cpu */ + if (in_protocol) + return 1; + return 0; +#else /* assuming everything else is 32-bit... */ + return 0; +#endif +} + +static int +allow_32_bit(void) +{ +#if defined(__x86_64__) +#if defined(ALLOW_32BIT_KERNEL_ON_X64) + if (in_protocol) + return 1; + return 0; +#else + return 0; +#endif +#elif defined(__i386__) || defined(__i686__) + return 1; +#elif defined(__aarch64__) + return 0; +#else /* assuming everything else is 32-bit... */ + return 1; +#endif +} + +static int +image_is_64_bit(EFI_IMAGE_OPTIONAL_HEADER_UNION *PEHdr) +{ + /* .Magic is the same offset in all cases */ + if (PEHdr->Pe32.OptionalHeader.Magic + == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) + return 1; + return 0; +} + +static const UINT16 machine_type = +#if defined(__x86_64__) + IMAGE_FILE_MACHINE_X64; +#elif defined(__aarch64__) + IMAGE_FILE_MACHINE_ARM64; +#elif defined(__arm__) + IMAGE_FILE_MACHINE_ARMTHUMB_MIXED; +#elif defined(__i386__) || defined(__i486__) || defined(__i686__) + IMAGE_FILE_MACHINE_I386; +#elif defined(__ia64__) + IMAGE_FILE_MACHINE_IA64; +#else +#error this architecture is not supported by shim +#endif + +static int +image_is_loadable(EFI_IMAGE_OPTIONAL_HEADER_UNION *PEHdr) +{ + /* If the machine type doesn't match the binary, bail, unless + * we're in an allowed 64-on-32 scenario */ + if (PEHdr->Pe32.FileHeader.Machine != machine_type) { + if (!(machine_type == IMAGE_FILE_MACHINE_I386 && + PEHdr->Pe32.FileHeader.Machine == IMAGE_FILE_MACHINE_X64 && + allow_64_bit())) { + return 0; + } + } + + /* If it's not a header type we recognize at all, bail */ + switch (PEHdr->Pe32Plus.OptionalHeader.Magic) { + case EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC: + case EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC: + break; + default: + return 0; + } + + /* and now just check for general 64-vs-32 compatibility */ + if (image_is_64_bit(PEHdr)) { + if (allow_64_bit()) + return 1; + } else { + if (allow_32_bit()) + return 1; + } + return 0; +} + +/* + * Read the binary header and grab appropriate information from it + */ +EFI_STATUS +read_header(void *data, unsigned int datasize, + PE_COFF_LOADER_IMAGE_CONTEXT *context) +{ + EFI_IMAGE_DOS_HEADER *DosHdr = data; + EFI_IMAGE_OPTIONAL_HEADER_UNION *PEHdr = data; + unsigned long HeaderWithoutDataDir, SectionHeaderOffset, OptHeaderSize; + unsigned long FileAlignment = 0; + UINT16 DllFlags; + size_t dos_sz = 0; + size_t tmpsz0, tmpsz1; + + if (datasize < sizeof (*DosHdr)) { + perror(L"Invalid image\n"); + return EFI_UNSUPPORTED; + } + + if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) { + if (DosHdr->e_lfanew < sizeof (*DosHdr) || + DosHdr->e_lfanew > datasize - 4) { + perror(L"Invalid image\n"); + return EFI_UNSUPPORTED; + } + + dos_sz = DosHdr->e_lfanew; + PEHdr = (EFI_IMAGE_OPTIONAL_HEADER_UNION *)((char *)data + DosHdr->e_lfanew); + } + + if (datasize - dos_sz < sizeof (PEHdr->Pe32)) { + perror(L"Invalid image\n"); + return EFI_UNSUPPORTED; + } + + if (image_is_64_bit(PEHdr) && + (datasize - dos_sz < sizeof (PEHdr->Pe32Plus))) { + perror(L"Invalid image\n"); + return EFI_UNSUPPORTED; + } + + if (!image_is_loadable(PEHdr)) { + perror(L"Platform does not support this image\n"); + return EFI_UNSUPPORTED; + } + + if (image_is_64_bit(PEHdr)) { + context->NumberOfRvaAndSizes = PEHdr->Pe32Plus.OptionalHeader.NumberOfRvaAndSizes; + context->SizeOfHeaders = PEHdr->Pe32Plus.OptionalHeader.SizeOfHeaders; + context->ImageSize = PEHdr->Pe32Plus.OptionalHeader.SizeOfImage; + context->SectionAlignment = PEHdr->Pe32Plus.OptionalHeader.SectionAlignment; + FileAlignment = PEHdr->Pe32Plus.OptionalHeader.FileAlignment; + OptHeaderSize = sizeof(EFI_IMAGE_OPTIONAL_HEADER64); + } else { + context->NumberOfRvaAndSizes = PEHdr->Pe32.OptionalHeader.NumberOfRvaAndSizes; + context->SizeOfHeaders = PEHdr->Pe32.OptionalHeader.SizeOfHeaders; + context->ImageSize = (UINT64)PEHdr->Pe32.OptionalHeader.SizeOfImage; + context->SectionAlignment = PEHdr->Pe32.OptionalHeader.SectionAlignment; + FileAlignment = PEHdr->Pe32.OptionalHeader.FileAlignment; + OptHeaderSize = sizeof(EFI_IMAGE_OPTIONAL_HEADER32); + } + + if (FileAlignment % 2 != 0) { + perror(L"File Alignment is invalid (%d)\n", FileAlignment); + return EFI_UNSUPPORTED; + } + if (FileAlignment == 0) + FileAlignment = 0x200; + if (context->SectionAlignment == 0) + context->SectionAlignment = PAGE_SIZE; + if (context->SectionAlignment < FileAlignment) + context->SectionAlignment = FileAlignment; + + context->NumberOfSections = PEHdr->Pe32.FileHeader.NumberOfSections; + + if (EFI_IMAGE_NUMBER_OF_DIRECTORY_ENTRIES < context->NumberOfRvaAndSizes) { + perror(L"Image header too large\n"); + return EFI_UNSUPPORTED; + } + + if (checked_mul(sizeof(EFI_IMAGE_DATA_DIRECTORY), EFI_IMAGE_NUMBER_OF_DIRECTORY_ENTRIES, &tmpsz0) || + checked_sub(OptHeaderSize, tmpsz0, &HeaderWithoutDataDir) || + checked_sub((size_t)PEHdr->Pe32.FileHeader.SizeOfOptionalHeader, HeaderWithoutDataDir, &tmpsz0) || + checked_mul((size_t)context->NumberOfRvaAndSizes, sizeof (EFI_IMAGE_DATA_DIRECTORY), &tmpsz1) || + (tmpsz0 != tmpsz1)) { + perror(L"Image header overflows data directory\n"); + return EFI_UNSUPPORTED; + } + + if (checked_add((size_t)DosHdr->e_lfanew, sizeof(UINT32), &tmpsz0) || + checked_add(tmpsz0, sizeof(EFI_IMAGE_FILE_HEADER), &tmpsz0) || + checked_add(tmpsz0, PEHdr->Pe32.FileHeader.SizeOfOptionalHeader, &SectionHeaderOffset)) { + perror(L"Image sections overflow image size\n"); + return EFI_UNSUPPORTED; + } + + if (checked_sub((size_t)context->ImageSize, SectionHeaderOffset, &tmpsz0) || + (tmpsz0 / EFI_IMAGE_SIZEOF_SECTION_HEADER <= context->NumberOfSections)) { + perror(L"Image sections overflow image size\n"); + return EFI_UNSUPPORTED; + } + + if (checked_sub((size_t)context->SizeOfHeaders, SectionHeaderOffset, &tmpsz0) || + (tmpsz0 / EFI_IMAGE_SIZEOF_SECTION_HEADER < (UINT32)context->NumberOfSections)) { + perror(L"Image sections overflow section headers\n"); + return EFI_UNSUPPORTED; + } + + if (checked_mul((size_t)context->NumberOfSections, sizeof(EFI_IMAGE_SECTION_HEADER), &tmpsz0) || + checked_add(tmpsz0, SectionHeaderOffset, &tmpsz0) || + (tmpsz0 > datasize)) { + perror(L"Image sections overflow section headers\n"); + return EFI_UNSUPPORTED; + } + + if (checked_sub((size_t)(uintptr_t)PEHdr, (size_t)(uintptr_t)data, &tmpsz0) || + checked_add(tmpsz0, sizeof(EFI_IMAGE_OPTIONAL_HEADER_UNION), &tmpsz0) || + (tmpsz0 > datasize)) { + perror(L"Invalid image\n"); + return EFI_UNSUPPORTED; + } + + if (PEHdr->Te.Signature != EFI_IMAGE_NT_SIGNATURE) { + perror(L"Unsupported image type\n"); + return EFI_UNSUPPORTED; + } + + if (PEHdr->Pe32.FileHeader.Characteristics & EFI_IMAGE_FILE_RELOCS_STRIPPED) { + perror(L"Unsupported image - Relocations have been stripped\n"); + return EFI_UNSUPPORTED; + } + + context->PEHdr = PEHdr; + + if (image_is_64_bit(PEHdr)) { + context->ImageAddress = PEHdr->Pe32Plus.OptionalHeader.ImageBase; + context->EntryPoint = PEHdr->Pe32Plus.OptionalHeader.AddressOfEntryPoint; + context->RelocDir = &PEHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC]; + context->SecDir = &PEHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY]; + DllFlags = PEHdr->Pe32Plus.OptionalHeader.DllCharacteristics; + } else { + context->ImageAddress = PEHdr->Pe32.OptionalHeader.ImageBase; + context->EntryPoint = PEHdr->Pe32.OptionalHeader.AddressOfEntryPoint; + context->RelocDir = &PEHdr->Pe32.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC]; + context->SecDir = &PEHdr->Pe32.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY]; + DllFlags = PEHdr->Pe32.OptionalHeader.DllCharacteristics; + } + + if ((mok_policy & MOK_POLICY_REQUIRE_NX) && + !(DllFlags & EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT)) { + perror(L"Policy requires NX, but image does not support NX\n"); + return EFI_UNSUPPORTED; + } + + if (checked_add((size_t)(uintptr_t)PEHdr, PEHdr->Pe32.FileHeader.SizeOfOptionalHeader, &tmpsz0) || + checked_add(tmpsz0, sizeof(UINT32), &tmpsz0) || + checked_add(tmpsz0, sizeof(EFI_IMAGE_FILE_HEADER), &tmpsz0)) { + perror(L"Invalid image\n"); + return EFI_UNSUPPORTED; + } + context->FirstSection = (EFI_IMAGE_SECTION_HEADER *)(uintptr_t)tmpsz0; + if ((uint64_t)(uintptr_t)(context->FirstSection) + > (uint64_t)(uintptr_t)data + datasize) { + perror(L"Invalid image\n"); + return EFI_UNSUPPORTED; + } + + if (context->ImageSize < context->SizeOfHeaders) { + perror(L"Invalid image\n"); + return EFI_UNSUPPORTED; + } + + if (checked_sub((size_t)(uintptr_t)context->SecDir, (size_t)(uintptr_t)data, &tmpsz0) || + (tmpsz0 > datasize - sizeof(EFI_IMAGE_DATA_DIRECTORY))) { + perror(L"Invalid image\n"); + return EFI_UNSUPPORTED; + } + + if (context->SecDir->VirtualAddress > datasize || + (context->SecDir->VirtualAddress == datasize && + context->SecDir->Size > 0)) { + perror(L"Malformed security header\n"); + return EFI_INVALID_PARAMETER; + } + return EFI_SUCCESS; +} + +// vim:fenc=utf-8:tw=75:noet diff --git a/pe.c b/pe.c index 9a3679e..3305601 100644 --- a/pe.c +++ b/pe.c @@ -21,152 +21,6 @@ #include -/* - * Perform basic bounds checking of the intra-image pointers - */ -void * -ImageAddress (void *image, uint64_t size, uint64_t address) -{ - /* ensure our local pointer isn't bigger than our size */ - if (address > size) - return NULL; - - /* Insure our math won't overflow */ - if (UINT64_MAX - address < (uint64_t)(intptr_t)image) - return NULL; - - /* return the absolute pointer */ - return image + address; -} - -/* - * Perform the actual relocation - */ -EFI_STATUS -relocate_coff (PE_COFF_LOADER_IMAGE_CONTEXT *context, - EFI_IMAGE_SECTION_HEADER *Section, - void *orig, void *data) -{ - EFI_IMAGE_BASE_RELOCATION *RelocBase, *RelocBaseEnd; - UINT64 Adjust; - UINT16 *Reloc, *RelocEnd; - char *Fixup, *FixupBase; - UINT16 *Fixup16; - UINT32 *Fixup32; - UINT64 *Fixup64; - int size = context->ImageSize; - void *ImageEnd = (char *)orig + size; - int n = 0; - - /* Alright, so here's how this works: - * - * context->RelocDir gives us two things: - * - the VA the table of base relocation blocks are (maybe) to be - * mapped at (RelocDir->VirtualAddress) - * - the virtual size (RelocDir->Size) - * - * The .reloc section (Section here) gives us some other things: - * - the name! kind of. (Section->Name) - * - the virtual size (Section->VirtualSize), which should be the same - * as RelocDir->Size - * - the virtual address (Section->VirtualAddress) - * - the file section size (Section->SizeOfRawData), which is - * a multiple of OptHdr->FileAlignment. Only useful for image - * validation, not really useful for iteration bounds. - * - the file address (Section->PointerToRawData) - * - a bunch of stuff we don't use that's 0 in our binaries usually - * - Flags (Section->Characteristics) - * - * and then the thing that's actually at the file address is an array - * of EFI_IMAGE_BASE_RELOCATION structs with some values packed behind - * them. The SizeOfBlock field of this structure includes the - * structure itself, and adding it to that structure's address will - * yield the next entry in the array. - */ - RelocBase = ImageAddress(orig, size, Section->PointerToRawData); - /* RelocBaseEnd here is the address of the first entry /past/ the - * table. */ - RelocBaseEnd = ImageAddress(orig, size, Section->PointerToRawData + - Section->Misc.VirtualSize); - - if (!RelocBase && !RelocBaseEnd) - return EFI_SUCCESS; - - if (!RelocBase || !RelocBaseEnd) { - perror(L"Reloc table overflows binary\n"); - return EFI_UNSUPPORTED; - } - - Adjust = (UINTN)data - context->ImageAddress; - - if (Adjust == 0) - return EFI_SUCCESS; - - while (RelocBase < RelocBaseEnd) { - Reloc = (UINT16 *) ((char *) RelocBase + sizeof (EFI_IMAGE_BASE_RELOCATION)); - - if (RelocBase->SizeOfBlock == 0) { - perror(L"Reloc %d block size 0 is invalid\n", n); - return EFI_UNSUPPORTED; - } else if (RelocBase->SizeOfBlock > context->RelocDir->Size) { - perror(L"Reloc %d block size %d greater than reloc dir" - "size %d, which is invalid\n", n, - RelocBase->SizeOfBlock, - context->RelocDir->Size); - return EFI_UNSUPPORTED; - } - - RelocEnd = (UINT16 *) ((char *) RelocBase + RelocBase->SizeOfBlock); - if ((void *)RelocEnd < orig || (void *)RelocEnd > ImageEnd) { - perror(L"Reloc %d entry overflows binary\n", n); - return EFI_UNSUPPORTED; - } - - FixupBase = ImageAddress(data, size, RelocBase->VirtualAddress); - if (!FixupBase) { - perror(L"Reloc %d Invalid fixupbase\n", n); - return EFI_UNSUPPORTED; - } - - while (Reloc < RelocEnd) { - Fixup = FixupBase + (*Reloc & 0xFFF); - switch ((*Reloc) >> 12) { - case EFI_IMAGE_REL_BASED_ABSOLUTE: - break; - - case EFI_IMAGE_REL_BASED_HIGH: - Fixup16 = (UINT16 *) Fixup; - *Fixup16 = (UINT16) (*Fixup16 + ((UINT16) ((UINT32) Adjust >> 16))); - break; - - case EFI_IMAGE_REL_BASED_LOW: - Fixup16 = (UINT16 *) Fixup; - *Fixup16 = (UINT16) (*Fixup16 + (UINT16) Adjust); - break; - - case EFI_IMAGE_REL_BASED_HIGHLOW: - Fixup32 = (UINT32 *) Fixup; - *Fixup32 = *Fixup32 + (UINT32) Adjust; - break; - - case EFI_IMAGE_REL_BASED_DIR64: - Fixup64 = (UINT64 *) Fixup; - *Fixup64 = *Fixup64 + (UINT64) Adjust; - break; - - default: - perror(L"Reloc %d Unknown relocation\n", n); - return EFI_UNSUPPORTED; - } - Reloc += 1; - } - RelocBase = (EFI_IMAGE_BASE_RELOCATION *) RelocEnd; - n++; - } - - return EFI_SUCCESS; -} - #define check_size_line(data, datasize_in, hashbase, hashsize, l) ({ \ if ((unsigned long)hashbase > \ (unsigned long)data + datasize_in) { \ @@ -185,113 +39,6 @@ relocate_coff (PE_COFF_LOADER_IMAGE_CONTEXT *context, }) #define check_size(d, ds, h, hs) check_size_line(d, ds, h, hs, __LINE__) -EFI_STATUS -get_section_vma (UINTN section_num, - char *buffer, size_t bufsz UNUSED, - PE_COFF_LOADER_IMAGE_CONTEXT *context, - char **basep, size_t *sizep, - EFI_IMAGE_SECTION_HEADER **sectionp) -{ - EFI_IMAGE_SECTION_HEADER *sections = context->FirstSection; - EFI_IMAGE_SECTION_HEADER *section; - char *base = NULL, *end = NULL; - - if (section_num >= context->NumberOfSections) - return EFI_NOT_FOUND; - - if (context->FirstSection == NULL) { - perror(L"Invalid section %d requested\n", section_num); - return EFI_UNSUPPORTED; - } - - section = §ions[section_num]; - - base = ImageAddress (buffer, context->ImageSize, section->VirtualAddress); - end = ImageAddress (buffer, context->ImageSize, - section->VirtualAddress + section->Misc.VirtualSize - 1); - - if (!(section->Characteristics & EFI_IMAGE_SCN_MEM_DISCARDABLE)) { - if (!base) { - perror(L"Section %d has invalid base address\n", section_num); - return EFI_UNSUPPORTED; - } - if (!end) { - perror(L"Section %d has zero size\n", section_num); - return EFI_UNSUPPORTED; - } - } - - if (!(section->Characteristics & EFI_IMAGE_SCN_CNT_UNINITIALIZED_DATA) && - (section->VirtualAddress < context->SizeOfHeaders || - section->PointerToRawData < context->SizeOfHeaders)) { - perror(L"Section %d is inside image headers\n", section_num); - return EFI_UNSUPPORTED; - } - - if (end < base) { - perror(L"Section %d has negative size\n", section_num); - return EFI_UNSUPPORTED; - } - - *basep = base; - *sizep = end - base; - *sectionp = section; - return EFI_SUCCESS; -} - -EFI_STATUS -get_section_vma_by_name (char *name, size_t namesz, - char *buffer, size_t bufsz, - PE_COFF_LOADER_IMAGE_CONTEXT *context, - char **basep, size_t *sizep, - EFI_IMAGE_SECTION_HEADER **sectionp) -{ - UINTN i; - char namebuf[9]; - - if (!name || namesz == 0 || !buffer || bufsz < namesz || !context - || !basep || !sizep || !sectionp) - return EFI_INVALID_PARAMETER; - - /* - * This code currently is only used for ".reloc\0\0" and - * ".sbat\0\0\0", and it doesn't know how to look up longer section - * names. - */ - if (namesz > 8) - return EFI_UNSUPPORTED; - - SetMem(namebuf, sizeof(namebuf), 0); - CopyMem(namebuf, name, MIN(namesz, 8)); - - /* - * Copy the executable's sections to their desired offsets - */ - for (i = 0; i < context->NumberOfSections; i++) { - EFI_STATUS status; - EFI_IMAGE_SECTION_HEADER *section = NULL; - char *base = NULL; - size_t size = 0; - - status = get_section_vma(i, buffer, bufsz, context, &base, &size, §ion); - if (!EFI_ERROR(status)) { - if (CompareMem(section->Name, namebuf, 8) == 0) { - *basep = base; - *sizep = size; - *sectionp = section; - return EFI_SUCCESS; - } - continue; - } - - switch(status) { - case EFI_NOT_FOUND: - break; - } - } - - return EFI_NOT_FOUND; -} /* * Calculate the SHA1 and SHA256 hashes of a binary @@ -585,249 +332,6 @@ done: return efi_status; } -/* here's a chart: - * i686 x86_64 aarch64 - * 64-on-64: nyet yes yes - * 64-on-32: nyet yes nyet - * 32-on-32: yes yes no - */ -static int -allow_64_bit(void) -{ -#if defined(__x86_64__) || defined(__aarch64__) - return 1; -#elif defined(__i386__) || defined(__i686__) - /* Right now blindly assuming the kernel will correctly detect this - * and /halt the system/ if you're not really on a 64-bit cpu */ - if (in_protocol) - return 1; - return 0; -#else /* assuming everything else is 32-bit... */ - return 0; -#endif -} - -static int -allow_32_bit(void) -{ -#if defined(__x86_64__) -#if defined(ALLOW_32BIT_KERNEL_ON_X64) - if (in_protocol) - return 1; - return 0; -#else - return 0; -#endif -#elif defined(__i386__) || defined(__i686__) - return 1; -#elif defined(__aarch64__) - return 0; -#else /* assuming everything else is 32-bit... */ - return 1; -#endif -} - -static int -image_is_64_bit(EFI_IMAGE_OPTIONAL_HEADER_UNION *PEHdr) -{ - /* .Magic is the same offset in all cases */ - if (PEHdr->Pe32Plus.OptionalHeader.Magic - == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) - return 1; - return 0; -} - -static const UINT16 machine_type = -#if defined(__x86_64__) - IMAGE_FILE_MACHINE_X64; -#elif defined(__aarch64__) - IMAGE_FILE_MACHINE_ARM64; -#elif defined(__arm__) - IMAGE_FILE_MACHINE_ARMTHUMB_MIXED; -#elif defined(__i386__) || defined(__i486__) || defined(__i686__) - IMAGE_FILE_MACHINE_I386; -#elif defined(__ia64__) - IMAGE_FILE_MACHINE_IA64; -#else -#error this architecture is not supported by shim -#endif - -static int -image_is_loadable(EFI_IMAGE_OPTIONAL_HEADER_UNION *PEHdr) -{ - /* If the machine type doesn't match the binary, bail, unless - * we're in an allowed 64-on-32 scenario */ - if (PEHdr->Pe32.FileHeader.Machine != machine_type) { - if (!(machine_type == IMAGE_FILE_MACHINE_I386 && - PEHdr->Pe32.FileHeader.Machine == IMAGE_FILE_MACHINE_X64 && - allow_64_bit())) { - return 0; - } - } - - /* If it's not a header type we recognize at all, bail */ - switch (PEHdr->Pe32Plus.OptionalHeader.Magic) { - case EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC: - case EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC: - break; - default: - return 0; - } - - /* and now just check for general 64-vs-32 compatibility */ - if (image_is_64_bit(PEHdr)) { - if (allow_64_bit()) - return 1; - } else { - if (allow_32_bit()) - return 1; - } - return 0; -} - -/* - * Read the binary header and grab appropriate information from it - */ -EFI_STATUS -read_header(void *data, unsigned int datasize, - PE_COFF_LOADER_IMAGE_CONTEXT *context) -{ - EFI_IMAGE_DOS_HEADER *DosHdr = data; - EFI_IMAGE_OPTIONAL_HEADER_UNION *PEHdr = data; - unsigned long HeaderWithoutDataDir, SectionHeaderOffset, OptHeaderSize; - unsigned long FileAlignment = 0; - UINT16 DllFlags; - - if (datasize < sizeof (PEHdr->Pe32)) { - perror(L"Invalid image\n"); - return EFI_UNSUPPORTED; - } - - if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) - PEHdr = (EFI_IMAGE_OPTIONAL_HEADER_UNION *)((char *)data + DosHdr->e_lfanew); - - if (!image_is_loadable(PEHdr)) { - perror(L"Platform does not support this image\n"); - return EFI_UNSUPPORTED; - } - - if (image_is_64_bit(PEHdr)) { - context->NumberOfRvaAndSizes = PEHdr->Pe32Plus.OptionalHeader.NumberOfRvaAndSizes; - context->SizeOfHeaders = PEHdr->Pe32Plus.OptionalHeader.SizeOfHeaders; - context->ImageSize = PEHdr->Pe32Plus.OptionalHeader.SizeOfImage; - context->SectionAlignment = PEHdr->Pe32Plus.OptionalHeader.SectionAlignment; - FileAlignment = PEHdr->Pe32Plus.OptionalHeader.FileAlignment; - OptHeaderSize = sizeof(EFI_IMAGE_OPTIONAL_HEADER64); - } else { - context->NumberOfRvaAndSizes = PEHdr->Pe32.OptionalHeader.NumberOfRvaAndSizes; - context->SizeOfHeaders = PEHdr->Pe32.OptionalHeader.SizeOfHeaders; - context->ImageSize = (UINT64)PEHdr->Pe32.OptionalHeader.SizeOfImage; - context->SectionAlignment = PEHdr->Pe32.OptionalHeader.SectionAlignment; - FileAlignment = PEHdr->Pe32.OptionalHeader.FileAlignment; - OptHeaderSize = sizeof(EFI_IMAGE_OPTIONAL_HEADER32); - } - - if (FileAlignment % 2 != 0) { - perror(L"File Alignment is invalid (%d)\n", FileAlignment); - return EFI_UNSUPPORTED; - } - if (FileAlignment == 0) - FileAlignment = 0x200; - if (context->SectionAlignment == 0) - context->SectionAlignment = PAGE_SIZE; - if (context->SectionAlignment < FileAlignment) - context->SectionAlignment = FileAlignment; - - context->NumberOfSections = PEHdr->Pe32.FileHeader.NumberOfSections; - - if (EFI_IMAGE_NUMBER_OF_DIRECTORY_ENTRIES < context->NumberOfRvaAndSizes) { - perror(L"Image header too small\n"); - return EFI_UNSUPPORTED; - } - - HeaderWithoutDataDir = OptHeaderSize - - sizeof (EFI_IMAGE_DATA_DIRECTORY) * EFI_IMAGE_NUMBER_OF_DIRECTORY_ENTRIES; - if (((UINT32)PEHdr->Pe32.FileHeader.SizeOfOptionalHeader - HeaderWithoutDataDir) != - context->NumberOfRvaAndSizes * sizeof (EFI_IMAGE_DATA_DIRECTORY)) { - perror(L"Image header overflows data directory\n"); - return EFI_UNSUPPORTED; - } - - SectionHeaderOffset = DosHdr->e_lfanew - + sizeof (UINT32) - + sizeof (EFI_IMAGE_FILE_HEADER) - + PEHdr->Pe32.FileHeader.SizeOfOptionalHeader; - if (((UINT32)context->ImageSize - SectionHeaderOffset) / EFI_IMAGE_SIZEOF_SECTION_HEADER - <= context->NumberOfSections) { - perror(L"Image sections overflow image size\n"); - return EFI_UNSUPPORTED; - } - - if ((context->SizeOfHeaders - SectionHeaderOffset) / EFI_IMAGE_SIZEOF_SECTION_HEADER - < (UINT32)context->NumberOfSections) { - perror(L"Image sections overflow section headers\n"); - return EFI_UNSUPPORTED; - } - - if ((((UINT8 *)PEHdr - (UINT8 *)data) + sizeof(EFI_IMAGE_OPTIONAL_HEADER_UNION)) > datasize) { - perror(L"Invalid image\n"); - return EFI_UNSUPPORTED; - } - - if (PEHdr->Te.Signature != EFI_IMAGE_NT_SIGNATURE) { - perror(L"Unsupported image type\n"); - return EFI_UNSUPPORTED; - } - - if (PEHdr->Pe32.FileHeader.Characteristics & EFI_IMAGE_FILE_RELOCS_STRIPPED) { - perror(L"Unsupported image - Relocations have been stripped\n"); - return EFI_UNSUPPORTED; - } - - context->PEHdr = PEHdr; - - if (image_is_64_bit(PEHdr)) { - context->ImageAddress = PEHdr->Pe32Plus.OptionalHeader.ImageBase; - context->EntryPoint = PEHdr->Pe32Plus.OptionalHeader.AddressOfEntryPoint; - context->RelocDir = &PEHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC]; - context->SecDir = &PEHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY]; - DllFlags = PEHdr->Pe32Plus.OptionalHeader.DllCharacteristics; - } else { - context->ImageAddress = PEHdr->Pe32.OptionalHeader.ImageBase; - context->EntryPoint = PEHdr->Pe32.OptionalHeader.AddressOfEntryPoint; - context->RelocDir = &PEHdr->Pe32.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC]; - context->SecDir = &PEHdr->Pe32.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY]; - DllFlags = PEHdr->Pe32.OptionalHeader.DllCharacteristics; - } - - if ((mok_policy & MOK_POLICY_REQUIRE_NX) && - !(DllFlags & EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT)) { - perror(L"Policy requires NX, but image does not support NX\n"); - return EFI_UNSUPPORTED; - } - - context->FirstSection = (EFI_IMAGE_SECTION_HEADER *)((char *)PEHdr + PEHdr->Pe32.FileHeader.SizeOfOptionalHeader + sizeof(UINT32) + sizeof(EFI_IMAGE_FILE_HEADER)); - - if (context->ImageSize < context->SizeOfHeaders) { - perror(L"Invalid image\n"); - return EFI_UNSUPPORTED; - } - - if ((unsigned long)((UINT8 *)context->SecDir - (UINT8 *)data) > - (datasize - sizeof(EFI_IMAGE_DATA_DIRECTORY))) { - perror(L"Invalid image\n"); - return EFI_UNSUPPORTED; - } - - if (context->SecDir->VirtualAddress > datasize || - (context->SecDir->VirtualAddress == datasize && - context->SecDir->Size > 0)) { - perror(L"Malformed security header\n"); - return EFI_INVALID_PARAMETER; - } - return EFI_SUCCESS; -} - EFI_STATUS verify_sbat_section(char *SBATBase, size_t SBATSize) { @@ -851,7 +355,11 @@ verify_sbat_section(char *SBATBase, size_t SBATSize) return in_protocol ? EFI_SUCCESS : EFI_SECURITY_VIOLATION; } - sbat_size = SBATSize + 1; + if (checked_add(SBATSize, 1, &sbat_size)) { + dprint(L"SBATSize + 1 would overflow\n"); + return EFI_SECURITY_VIOLATION; + } + sbat_data = AllocatePool(sbat_size); if (!sbat_data) { console_print(L"Failed to allocate .sbat section buffer\n"); @@ -937,7 +445,7 @@ get_mem_attrs (uintptr_t addr, size_t size, uint64_t *attrs) if (EFI_ERROR(efi_status) || !proto) return efi_status; - if (physaddr & 0xfff || size & 0xfff || size == 0 || attrs == NULL) { + if (!IS_PAGE_ALIGNED(physaddr) || !IS_PAGE_ALIGNED(size) || size == 0 || attrs == NULL) { dprint(L"%a called on 0x%llx-0x%llx and attrs 0x%llx\n", __func__, (unsigned long long)physaddr, (unsigned long long)(physaddr+size-1), @@ -971,7 +479,7 @@ update_mem_attrs(uintptr_t addr, uint64_t size, (unsigned long long)addr, (unsigned long long)size, &before, efi_status); - if (physaddr & 0xfff || size & 0xfff || size == 0) { + if (!IS_PAGE_ALIGNED(physaddr) || !IS_PAGE_ALIGNED(size) || size == 0) { dprint(L"%a called on 0x%llx-0x%llx (size 0x%llx) +%a%a%a -%a%a%a\n", __func__, (unsigned long long)physaddr, (unsigned long long)(physaddr + size - 1), @@ -990,10 +498,20 @@ update_mem_attrs(uintptr_t addr, uint64_t size, uefi_clear_attrs = shim_mem_attrs_to_uefi_mem_attrs (clear_attrs); dprint("translating clear_attrs from 0x%lx to 0x%lx\n", clear_attrs, uefi_clear_attrs); efi_status = EFI_SUCCESS; - if (uefi_set_attrs) + if (uefi_set_attrs) { efi_status = proto->SetMemoryAttributes(proto, physaddr, size, uefi_set_attrs); - if (!EFI_ERROR(efi_status) && uefi_clear_attrs) + if (EFI_ERROR(efi_status)) { + dprint(L"Failed to set memory attrs:0x%0x physaddr:0x%llx size:0x%0lx status:%r\n", + uefi_set_attrs, physaddr, size, efi_status); + } + } + if (!EFI_ERROR(efi_status) && uefi_clear_attrs) { efi_status = proto->ClearMemoryAttributes(proto, physaddr, size, uefi_clear_attrs); + if (EFI_ERROR(efi_status)) { + dprint(L"Failed to clear memory attrs:0x%0x physaddr:0x%llx size:0x%0lx status:%r\n", + uefi_clear_attrs, physaddr, size, efi_status); + } + } ret = efi_status; efi_status = get_mem_attrs (addr, size, &after); @@ -1243,6 +761,7 @@ handle_image (void *data, unsigned int datasize, (Section->Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE) && (mok_policy & MOK_POLICY_REQUIRE_NX)) { perror(L"Section %d is writable and executable\n", i); + BS->FreePages(*alloc_address, *alloc_pages); return EFI_UNSUPPORTED; } @@ -1268,6 +787,7 @@ handle_image (void *data, unsigned int datasize, if (CompareMem(Section->Name, ".reloc\0\0", 8) == 0) { if (RelocSection) { perror(L"Image has multiple relocation sections\n"); + BS->FreePages(*alloc_address, *alloc_pages); return EFI_UNSUPPORTED; } /* If it has nonzero sizes, and our bounds check @@ -1277,8 +797,12 @@ handle_image (void *data, unsigned int datasize, Section->Misc.VirtualSize && base && end && RelocBase == base && - RelocBaseEnd == end) { + RelocBaseEnd <= end) { RelocSection = Section; + } else { + perror(L"Relocation section is invalid \n"); + BS->FreePages(*alloc_address, *alloc_pages); + return EFI_UNSUPPORTED; } } @@ -1288,10 +812,12 @@ handle_image (void *data, unsigned int datasize, if (!base) { perror(L"Section %d has invalid base address\n", i); + BS->FreePages(*alloc_address, *alloc_pages); return EFI_UNSUPPORTED; } if (!end) { perror(L"Section %d has zero size\n", i); + BS->FreePages(*alloc_address, *alloc_pages); return EFI_UNSUPPORTED; } @@ -1299,6 +825,7 @@ handle_image (void *data, unsigned int datasize, (Section->VirtualAddress < context.SizeOfHeaders || Section->PointerToRawData < context.SizeOfHeaders)) { perror(L"Section %d is inside image headers\n", i); + BS->FreePages(*alloc_address, *alloc_pages); return EFI_UNSUPPORTED; } @@ -1307,6 +834,7 @@ handle_image (void *data, unsigned int datasize, } else { if (Section->PointerToRawData < context.SizeOfHeaders) { perror(L"Section %d is inside image headers\n", i); + BS->FreePages(*alloc_address, *alloc_pages); return EFI_UNSUPPORTED; } @@ -1324,7 +852,7 @@ handle_image (void *data, unsigned int datasize, if (context.NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC) { perror(L"Image has no relocation entry\n"); - FreePool(buffer); + BS->FreePages(*alloc_address, *alloc_pages); return EFI_UNSUPPORTED; } @@ -1337,7 +865,7 @@ handle_image (void *data, unsigned int datasize, if (EFI_ERROR(efi_status)) { perror(L"Relocation failed: %r\n", efi_status); - FreePool(buffer); + BS->FreePages(*alloc_address, *alloc_pages); return efi_status; } } @@ -1372,7 +900,11 @@ handle_image (void *data, unsigned int datasize, + Section->Misc.VirtualSize - 1); addr = (uintptr_t)base; - length = (uintptr_t)end - (uintptr_t)base + 1; + // Align the length up to PAGE_SIZE. This is required because + // platforms generally set memory attributes at page + // granularity, but the section length (unlike the section + // address) is not required to be aligned. + length = ALIGN_VALUE((uintptr_t)end - (uintptr_t)base + 1, PAGE_SIZE); if (Section->Characteristics & EFI_IMAGE_SCN_MEM_WRITE) { set_attrs |= MEM_ATTR_W; @@ -1399,10 +931,12 @@ handle_image (void *data, unsigned int datasize, if (!found_entry_point) { perror(L"Entry point is not within sections\n"); + BS->FreePages(*alloc_address, *alloc_pages); return EFI_UNSUPPORTED; } if (found_entry_point > 1) { perror(L"%d sections contain entry point\n", found_entry_point); + BS->FreePages(*alloc_address, *alloc_pages); return EFI_UNSUPPORTED; } diff --git a/replacements.c b/replacements.c index bf781a8..469e73a 100644 --- a/replacements.c +++ b/replacements.c @@ -78,8 +78,27 @@ replacement_start_image(EFI_HANDLE image_handle, UINTN *exit_data_size, CHAR16 * unhook_system_services(); if (image_handle == last_loaded_image) { + UINT8 retain_protocol = 0; + UINTN retain_protocol_size = sizeof(retain_protocol); + UINT32 retain_protocol_attrs = 0; + loader_is_participating = 1; - uninstall_shim_protocols(); + + /* If a boot component asks us, keep our protocol around - it will be used to + * validate further PE payloads (e.g.: by the UKI stub, before the kernel is booted). + * But also check that the variable was set by a boot component, to ensure that + * nobody at runtime can attempt to change shim's behaviour. */ + efi_status = RT->GetVariable(SHIM_RETAIN_PROTOCOL_VAR_NAME, + &SHIM_LOCK_GUID, + &retain_protocol_attrs, + &retain_protocol_size, + &retain_protocol); + if (EFI_ERROR(efi_status) || + (retain_protocol_attrs & EFI_VARIABLE_NON_VOLATILE) || + !(retain_protocol_attrs & EFI_VARIABLE_BOOTSERVICE_ACCESS) || + retain_protocol_size != sizeof(retain_protocol) || + retain_protocol == 0) + uninstall_shim_protocols(); } efi_status = BS->StartImage(image_handle, exit_data_size, exit_data); if (EFI_ERROR(efi_status)) { @@ -90,7 +109,7 @@ replacement_start_image(EFI_HANDLE image_handle, UINTN *exit_data_size, CHAR16 * console_print(L"Something has gone seriously wrong: %r\n", efi_status2); console_print(L"shim cannot continue, sorry.\n"); - msleep(5000000); + usleep(5000000); RT->ResetSystem(EfiResetShutdown, EFI_SECURITY_VIOLATION, 0, NULL); @@ -118,7 +137,7 @@ exit_boot_services(EFI_HANDLE image_key, UINTN map_key) console_print(L"Bootloader has not verified loaded image.\n"); console_print(L"System is compromised. halting.\n"); - msleep(5000000); + usleep(5000000); RT->ResetSystem(EfiResetShutdown, EFI_SECURITY_VIOLATION, 0, NULL); return EFI_SECURITY_VIOLATION; } @@ -143,7 +162,7 @@ do_exit(EFI_HANDLE ImageHandle, EFI_STATUS ExitStatus, console_print(L"Something has gone seriously wrong: %r\n", efi_status2); console_print(L"shim cannot continue, sorry.\n"); - msleep(5000000); + usleep(5000000); RT->ResetSystem(EfiResetShutdown, EFI_SECURITY_VIOLATION, 0, NULL); } diff --git a/sbat.c b/sbat.c index a08c5b2..0695612 100644 --- a/sbat.c +++ b/sbat.c @@ -4,18 +4,23 @@ */ #include "shim.h" +#include "ssp.h" +#include "ssp_var_defs.h" extern struct { - UINT32 previous_offset; + UINT32 automatic_offset; UINT32 latest_offset; } sbat_var_payload_header; +static UINT8 sbat_policy = POLICY_NOTREAD; +static UINT8 ssp_policy = POLICY_NOTREAD; + EFI_STATUS parse_sbat_section(char *section_base, size_t section_size, size_t *n_entries, struct sbat_section_entry ***entriesp) { - struct sbat_section_entry *entry = NULL, **entries; + struct sbat_section_entry *entry = NULL, **entries = NULL; EFI_STATUS efi_status = EFI_SUCCESS; list_t csv, *pos = NULL; char * end = section_base + section_size - 1; @@ -67,6 +72,13 @@ parse_sbat_section(char *section_base, size_t section_size, n++; } + /* + * Not necessarily actually an *error* since we eat newlines and + * the like; it could actually just be /empty/. + */ + if (n == 0) + goto out; + strtab = AllocateZeroPool(allocsz); if (!strtab) { efi_status = EFI_OUT_OF_RESOURCES; @@ -101,6 +113,7 @@ parse_sbat_section(char *section_base, size_t section_size, entry++; n++; } +out: *entriesp = entries; *n_entries = n; err: @@ -289,7 +302,7 @@ err: } EFI_STATUS -parse_sbat_var(list_t *entries) +parse_sbat_var(list_t *entries, char *sbat_var_candidate) { UINT8 *data = 0; UINTN datasize; @@ -301,10 +314,17 @@ parse_sbat_var(list_t *entries) return EFI_INVALID_PARAMETER; } - efi_status = get_variable(SBAT_VAR_NAME, &data, &datasize, SHIM_LOCK_GUID); - if (EFI_ERROR(efi_status)) { - LogError(L"Failed to read SBAT variable\n", efi_status); - return efi_status; + if (sbat_var_candidate == NULL) { + dprint(L"sbat_var_candidate is NULL, reading variable\n"); + efi_status = get_variable(SBAT_VAR_NAME, &data, &datasize, SHIM_LOCK_GUID); + if (EFI_ERROR(efi_status)) { + LogError(L"Failed to read SBAT variable\n", efi_status); + return efi_status; + } + } else { + datasize = strlen(sbat_var_candidate); + data = AllocatePool(datasize + 1); + memcpy(data, sbat_var_candidate, datasize); } /* @@ -312,8 +332,10 @@ parse_sbat_var(list_t *entries) * allocations, so use that here. */ efi_status = parse_sbat_var_data(entries, data, datasize+1); - if (EFI_ERROR(efi_status)) - return efi_status; + if (EFI_ERROR(efi_status)) { + dprint(L"parse_sbat_var_data() failed datasize: %d\n", datasize); + goto out; + } dprint(L"SBAT variable entries:\n"); list_for_each(pos, entries) { @@ -324,6 +346,8 @@ parse_sbat_var(list_t *entries) entry->component_generation, entry->sbat_datestamp); } +out: + FreePool(data); return efi_status; } @@ -388,6 +412,49 @@ preserve_sbat_uefi_variable(UINT8 *sbat, UINTN sbatsize, UINT32 attributes, return false; } +/* + * This looks kind of weird, but it comes directly from the MS + * documentation: + * https://support.microsoft.com/en-us/topic/kb5027455-guidance-for-blocking-vulnerable-windows-boot-managers-522bb851-0a61-44ad-aa94-ad11119c5e91 + */ +static UINT64 +ssp_ver_to_ull(UINT16 *ver) +{ + dprint("major: %u\n", ver[0]); + dprint("minor: %u\n", ver[1]); + dprint("rev: %u\n", ver[2]); + dprint("build: %u\n", ver[3]); + + return ((UINT64)ver[0] << 48) + + ((UINT64)ver[1] << 32) + + ((UINT64)ver[2] << 16) + + ver[3]; +} + +static bool +preserve_ssp_uefi_variable(UINT8 *ssp_applied, UINTN sspversize, UINT32 attributes, + uint8_t *ssp_candidate) +{ + UINT64 old, new; + + if (ssp_applied == NULL || ssp_candidate == NULL) + return false; + + if (sspversize != SSPVER_SIZE) + return false; + + if (!check_sbat_var_attributes(attributes)) + return false; + + old = ssp_ver_to_ull((UINT16 *)ssp_applied); + new = ssp_ver_to_ull((UINT16 *)ssp_candidate); + + if (new > old) + return false; + else + return true; +} + static void clear_sbat_policy() { @@ -399,74 +466,80 @@ clear_sbat_policy() } EFI_STATUS -set_sbat_uefi_variable(void) +set_sbat_uefi_variable(char *sbat_var_automatic, char *sbat_var_latest) { EFI_STATUS efi_status = EFI_SUCCESS; UINT32 attributes = 0; - char *sbat_var_previous; - char *sbat_var_latest; - UINT8 *sbat = NULL; - UINT8 *sbat_policy = NULL; + UINT8 *sbat_policyp = NULL; UINTN sbatsize = 0; UINTN sbat_policysize = 0; - char *sbat_var = NULL; + char *sbat_var_candidate = NULL; bool reset_sbat = false; - sbat_var_previous = (char *)&sbat_var_payload_header + sbat_var_payload_header.previous_offset; - sbat_var_latest = (char *)&sbat_var_payload_header + sbat_var_payload_header.latest_offset; + if (sbat_policy == POLICY_NOTREAD) { + efi_status = get_variable_attr(SBAT_POLICY, &sbat_policyp, + &sbat_policysize, SHIM_LOCK_GUID, + &attributes); + if (!EFI_ERROR(efi_status)) { + sbat_policy = *sbat_policyp; + clear_sbat_policy(); + } + } - efi_status = get_variable_attr(SBAT_POLICY, &sbat_policy, - &sbat_policysize, SHIM_LOCK_GUID, - &attributes); if (EFI_ERROR(efi_status)) { - dprint("Default sbat policy: previous\n"); - sbat_var = sbat_var_previous; + dprint("Default sbat policy: automatic\n"); + if (secure_mode()) { + sbat_var_candidate = sbat_var_automatic; + } else { + reset_sbat = true; + sbat_var_candidate = SBAT_VAR_ORIGINAL; + } } else { - switch (*sbat_policy) { - case SBAT_POLICY_LATEST: - dprint("Custom sbat policy: latest\n"); - sbat_var = sbat_var_latest; - clear_sbat_policy(); - break; - case SBAT_POLICY_PREVIOUS: - dprint("Custom sbat policy: previous\n"); - sbat_var = sbat_var_previous; - break; - case SBAT_POLICY_RESET: - if (secure_mode()) { - console_print(L"Cannot reset SBAT policy: Secure Boot is enabled.\n"); - sbat_var = sbat_var_previous; - } else { - dprint(L"Custom SBAT policy: reset OK\n"); - reset_sbat = true; - sbat_var = SBAT_VAR_ORIGINAL; - } - clear_sbat_policy(); - break; - default: - console_error(L"SBAT policy state %llu is invalid", - EFI_INVALID_PARAMETER); - sbat_var = sbat_var_previous; - clear_sbat_policy(); - break; + switch (sbat_policy) { + case POLICY_LATEST: + dprint("Custom sbat policy: latest\n"); + sbat_var_candidate = sbat_var_latest; + break; + case POLICY_AUTOMATIC: + dprint("Custom sbat policy: automatic\n"); + sbat_var_candidate = sbat_var_automatic; + break; + case POLICY_RESET: + if (secure_mode()) { + console_print(L"Cannot reset SBAT policy: Secure Boot is enabled.\n"); + sbat_var_candidate = sbat_var_automatic; + } else { + dprint(L"Custom SBAT policy: reset OK\n"); + reset_sbat = true; + sbat_var_candidate = SBAT_VAR_ORIGINAL; + } + break; + default: + console_error(L"SBAT policy state %llu is invalid", + EFI_INVALID_PARAMETER); + if (secure_mode()) { + sbat_var_candidate = sbat_var_automatic; + } else { + reset_sbat = true; + sbat_var_candidate = SBAT_VAR_ORIGINAL; + } + break; } } efi_status = get_variable_attr(SBAT_VAR_NAME, &sbat, &sbatsize, - SHIM_LOCK_GUID, &attributes); + SHIM_LOCK_GUID, &attributes); /* * Always set the SbatLevel UEFI variable if it fails to read. - * - * Don't try to set the SbatLevel UEFI variable if attributes match - * and the signature matches. */ if (EFI_ERROR(efi_status)) { dprint(L"SBAT read failed %r\n", efi_status); - } else if (preserve_sbat_uefi_variable(sbat, sbatsize, attributes, sbat_var) - && !reset_sbat) { + } else if (preserve_sbat_uefi_variable(sbat, sbatsize, attributes, + sbat_var_candidate) && + !reset_sbat) { dprint(L"preserving %s variable it is %d bytes, attributes are 0x%08x\n", SBAT_VAR_NAME, sbatsize, attributes); FreePool(sbat); @@ -474,6 +547,29 @@ set_sbat_uefi_variable(void) } else { FreePool(sbat); + /* + * parse the candidate SbatLevel and check that shim will not + * self revoke before writing SbatLevel variable + */ + dprint(L"shim SBAT reparse before application\n"); + efi_status = parse_sbat_var(&sbat_var, sbat_var_candidate); + if (EFI_ERROR(efi_status)) { + dprint(L"proposed SbatLevel failed to parse\n"); + return efi_status; + } + +#ifndef SHIM_UNIT_TEST + char *sbat_start = (char *)&_sbat; + char *sbat_end = (char *)&_esbat; + efi_status = verify_sbat_section(sbat_start, sbat_end - sbat_start - 1); + if (EFI_ERROR(efi_status)) { + CHAR16 *title = L"New SbatLevel would self-revoke current shim. Not applied"; + CHAR16 *message = L"Press any key to continue"; + console_countdown(title, message, 10); + return efi_status; + } +#endif /* SHIM_UNIT_TEST */ + /* delete previous variable */ dprint("%s variable is %d bytes, attributes are 0x%08x\n", SBAT_VAR_NAME, sbatsize, attributes); @@ -489,7 +585,7 @@ set_sbat_uefi_variable(void) /* set variable */ efi_status = set_variable(SBAT_VAR_NAME, SHIM_LOCK_GUID, SBAT_VAR_ATTRS, - strlen(sbat_var), sbat_var); + strlen(sbat_var_candidate), sbat_var_candidate); if (EFI_ERROR(efi_status)) { dprint(L"%s variable writing failed %r\n", SBAT_VAR_NAME, efi_status); @@ -504,18 +600,189 @@ set_sbat_uefi_variable(void) return efi_status; } - if (sbatsize != strlen(sbat_var) || - strncmp((const char *)sbat, sbat_var, strlen(sbat_var)) != 0) { + if (sbatsize != strlen(sbat_var_candidate) || + strncmp((const char *)sbat, sbat_var_candidate, + strlen(sbat_var_candidate)) != 0) { dprint("new sbatsize is %d, expected %d\n", sbatsize, - strlen(sbat_var)); + strlen(sbat_var_candidate)); efi_status = EFI_INVALID_PARAMETER; } else { dprint(L"%s variable initialization succeeded\n", SBAT_VAR_NAME); } FreePool(sbat); + return efi_status; +} + +EFI_STATUS +set_sbat_uefi_variable_internal(void) +{ + char *sbat_var_automatic; + char *sbat_var_latest; + + sbat_var_automatic = (char *)&sbat_var_payload_header + + sbat_var_payload_header.automatic_offset; + sbat_var_latest = (char *)&sbat_var_payload_header + + sbat_var_payload_header.latest_offset; + + return set_sbat_uefi_variable(sbat_var_automatic, sbat_var_latest); +} + +static void +clear_ssp_policy(void) +{ + EFI_STATUS efi_status = EFI_SUCCESS; + + efi_status = del_variable(SSP_POLICY, SHIM_LOCK_GUID); + if (EFI_ERROR(efi_status)) + console_error(L"Could not reset SSP Policy", efi_status); +} + +static EFI_STATUS +clear_ssp_uefi_variables(void) +{ + EFI_STATUS efi_status, rc = EFI_SUCCESS; + + /* delete previous variable */ + dprint("Deleting %s variable.\n", SSPVER_VAR_NAME); + efi_status = del_variable(SSPVER_VAR_NAME, SECUREBOOT_EFI_NAMESPACE_GUID); + if (EFI_ERROR(efi_status)) { + dprint(L"%s variable delete failed %r\n", SSPVER_VAR_NAME, + efi_status); + rc = efi_status; + } + dprint("Deleting %s variable.\n", SSPSIG_VAR_NAME); + efi_status = del_variable(SSPSIG_VAR_NAME, SECUREBOOT_EFI_NAMESPACE_GUID); + if (EFI_ERROR(efi_status)) { + dprint(L"%s variable delete failed %r\n", SSPSIG_VAR_NAME, + efi_status); + rc = efi_status; + } + + return rc; +} + +EFI_STATUS +set_ssp_uefi_variable(uint8_t *ssp_ver_automatic, uint8_t *ssp_sig_automatic, + uint8_t *ssp_ver_latest, uint8_t *ssp_sig_latest) +{ + EFI_STATUS efi_status = EFI_SUCCESS; + UINT32 attributes = 0; + + UINT8 *sspver = NULL; + UINT8 *policyp = NULL; + UINTN sspversize = 0; + UINTN policysize = 0; + + uint8_t *ssp_ver = NULL; + uint8_t *ssp_sig = NULL; + bool reset_ssp = false; + + _Static_assert(sizeof(SkuSiPolicyVersion) == SSPVER_SIZE, + "SkuSiPolicyVersion has unexpected size"); + _Static_assert(sizeof(SkuSiPolicyUpdateSigners) == SSPSIG_SIZE, + "SkuSiPolicyUpdateSigners has unexpected size"); + + if (ssp_policy == POLICY_NOTREAD) { + efi_status = get_variable_attr(SSP_POLICY, &policyp, + &policysize, SHIM_LOCK_GUID, + &attributes); + if (!EFI_ERROR(efi_status)) { + ssp_policy = *policyp; + clear_ssp_policy(); + } + } + + if (EFI_ERROR(efi_status)) { + dprint("Default SSP policy: automatic\n"); + ssp_ver = ssp_ver_automatic; + ssp_sig = ssp_sig_automatic; + } else { + switch (ssp_policy) { + case POLICY_LATEST: + dprint("Custom SSP policy: latest\n");\ + ssp_ver = ssp_ver_latest; + ssp_sig = ssp_sig_latest; + break; + case POLICY_AUTOMATIC: + dprint("Custom SSP policy: automatic\n"); + ssp_ver = ssp_ver_automatic; + ssp_sig = ssp_sig_automatic; + break; + case POLICY_RESET: + if (secure_mode()) { + console_print(L"Cannot reset SSP policy: Secure Boot is enabled.\n"); + ssp_ver = ssp_ver_automatic; + ssp_sig = ssp_sig_automatic; + } else { + dprint(L"Custom SSP policy: reset OK\n"); + reset_ssp = true; + } + break; + default: + console_error(L"SSP policy state %llu is invalid", + EFI_INVALID_PARAMETER); + ssp_ver = ssp_ver_automatic; + ssp_sig = ssp_sig_automatic; + break; + } + } + + if (!ssp_ver && !ssp_sig && !reset_ssp) { + dprint(L"No supplied SSP data, not setting variables\n"); + return EFI_SUCCESS; + } + + efi_status = get_variable_attr(SSPVER_VAR_NAME, &sspver, &sspversize, + SECUREBOOT_EFI_NAMESPACE_GUID, &attributes); + /* + * Since generally we want bootmgr to manage its own revocations, + * we are much less agressive trying to set those variables + */ + if (EFI_ERROR(efi_status)) { + dprint(L"SkuSiPolicyVersion read failed %r\n", efi_status); + } else if (preserve_ssp_uefi_variable(sspver, sspversize, attributes, ssp_ver) + && !reset_ssp) { + FreePool(sspver); + + dprint(L"preserving %s variable it is %d bytes, attributes are 0x%08x\n", + SSPVER_VAR_NAME, sspversize, attributes); + return EFI_SUCCESS; + } else { + FreePool(sspver); + + efi_status = clear_ssp_uefi_variables(); + } + + if (reset_ssp) + return efi_status; + + /* set variable */ + efi_status = set_variable(SSPVER_VAR_NAME, SECUREBOOT_EFI_NAMESPACE_GUID, + SSP_VAR_ATTRS, SSPVER_SIZE, ssp_ver); + if (EFI_ERROR(efi_status)) { + dprint(L"%s variable writing failed %r\n", SSPVER_VAR_NAME, + efi_status); + return efi_status; + } + dprint("done setting %s variable.\n", SSPSIG_VAR_NAME); + + efi_status = set_variable(SSPSIG_VAR_NAME, SECUREBOOT_EFI_NAMESPACE_GUID, + SSP_VAR_ATTRS, SSPSIG_SIZE, ssp_sig); + if (EFI_ERROR(efi_status)) { + dprint(L"%s variable writing failed %r\n", SSPSIG_VAR_NAME, + efi_status); + return efi_status; + } + dprint("done setting %s variable.\n", SSPSIG_VAR_NAME); return efi_status; } +EFI_STATUS +set_ssp_uefi_variable_internal(void) +{ + return set_ssp_uefi_variable(NULL, NULL, SkuSiPolicyVersion, + SkuSiPolicyUpdateSigners); +} // vim:fenc=utf-8:tw=75:noet diff --git a/sbat_var.S b/sbat_var.S index a115077..ed82a46 100644 --- a/sbat_var.S +++ b/sbat_var.S @@ -9,12 +9,15 @@ .type sbat_var_payload_header, %object .size sbat_var_payload_header, .Lsbat_var_payload_header_end - sbat_var_payload_header sbat_var_payload_header: - .4byte .Lsbat_var_previous - sbat_var_payload_header + .4byte .Lsbat_var_automatic - sbat_var_payload_header .4byte .Lsbat_var_latest - sbat_var_payload_header .Lsbat_var_payload_header_end: .balign 1, 0 -.Lsbat_var_previous: - .asciz SBAT_VAR_PREVIOUS +.Lsbat_var_automatic: + .ascii SBAT_VAR_AUTOMATIC + .byte 0 .balign 1, 0 .Lsbat_var_latest: - .asciz SBAT_VAR_LATEST + .ascii SBAT_VAR_LATEST + .byte 0 + .section .note.GNU-stack,"a" diff --git a/shim.c b/shim.c index 4437898..633163a 100644 --- a/shim.c +++ b/shim.c @@ -627,11 +627,13 @@ verify_buffer_authenticode (char *data, int datasize, return EFI_SECURITY_VIOLATION; } - if (context->SecDir->Size >= size) { + if (checked_add(context->SecDir->Size, context->SecDir->VirtualAddress, &offset) || + offset > size) { perror(L"Certificate Database size is too large\n"); return EFI_INVALID_PARAMETER; } + offset = 0; ret_efi_status = EFI_NOT_FOUND; do { WIN_CERTIFICATE_EFI_PKCS *sig = NULL; @@ -642,6 +644,12 @@ verify_buffer_authenticode (char *data, int datasize, if (!sig) break; + if ((uint64_t)(uintptr_t)&sig[1] + > (uint64_t)(uintptr_t)data + datasize) { + perror(L"Certificate size is too large for secruity database"); + return EFI_INVALID_PARAMETER; + } + sz = offset + offsetof(WIN_CERTIFICATE_EFI_PKCS, Hdr.dwLength) + sizeof(sig->Hdr.dwLength); if (sz > context->SecDir->Size) { @@ -709,6 +717,12 @@ verify_buffer_sbat (char *data, int datasize, Section = context->FirstSection; for (i = 0; i < context->NumberOfSections; i++, Section++) { + if ((uint64_t)(uintptr_t)&Section[1] + > (uintptr_t)(uintptr_t)data + datasize) { + perror(L"Section exceeds bounds of image\n"); + return EFI_UNSUPPORTED; + } + if (CompareMem(Section->Name, ".sbat\0\0\0", 8) != 0) continue; @@ -731,11 +745,17 @@ verify_buffer_sbat (char *data, int datasize, * and ignore the section if it isn't. */ if (Section->SizeOfRawData && Section->SizeOfRawData >= Section->Misc.VirtualSize) { + uint64_t boundary; SBATBase = ImageAddress(data, datasize, Section->PointerToRawData); SBATSize = Section->SizeOfRawData; dprint(L"sbat section base:0x%lx size:0x%lx\n", SBATBase, SBATSize); + if (checked_add((uint64_t)(uintptr_t)SBATBase, SBATSize, &boundary) || + (boundary > (uint64_t)(uintptr_t)data + datasize)) { + perror(L"Section exceeds bounds of image\n"); + return EFI_UNSUPPORTED; + } } } @@ -753,11 +773,11 @@ verify_buffer (char *data, int datasize, { EFI_STATUS efi_status; - efi_status = verify_buffer_sbat(data, datasize, context); + efi_status = verify_buffer_authenticode(data, datasize, context, sha256hash, sha1hash); if (EFI_ERROR(efi_status)) return efi_status; - return verify_buffer_authenticode(data, datasize, context, sha256hash, sha1hash); + return verify_buffer_sbat(data, datasize, context); } static int @@ -1050,6 +1070,23 @@ restore_loaded_image(VOID) CopyMem(shim_li, &shim_li_bak, sizeof(shim_li_bak)); } +/* If gets used on static data it probably needs boundary checking */ +void +str16_to_str8(CHAR16 *str16, CHAR8 **str8) +{ + int i = 0; + + while (str16[i++] != '\0'); + *str8 = (CHAR8 *)AllocatePool((i + 1) * sizeof (CHAR8)); + + i = 0; + while (str16[i] != '\0') { + (*str8)[i] = (CHAR8)str16[i]; + i++; + } + (*str8)[i] = '\0'; +} + /* * Load and run an EFI executable */ @@ -1059,6 +1096,7 @@ EFI_STATUS read_image(EFI_HANDLE image_handle, CHAR16 *ImagePath, EFI_STATUS efi_status; void *sourcebuffer = NULL; UINT64 sourcesize = 0; + CHAR8 *netbootname; /* * We need to refer to the loaded image protocol on the running @@ -1082,11 +1120,13 @@ EFI_STATUS read_image(EFI_HANDLE image_handle, CHAR16 *ImagePath, } if (findNetboot(shim_li->DeviceHandle)) { - efi_status = parseNetbootinfo(image_handle); + str16_to_str8(ImagePath, &netbootname); + efi_status = parseNetbootinfo(image_handle, netbootname); if (EFI_ERROR(efi_status)) { perror(L"Netboot parsing failed: %r\n", efi_status); return EFI_PROTOCOL_ERROR; } + FreePool(netbootname); efi_status = FetchNetbootimage(image_handle, &sourcebuffer, &sourcesize); if (EFI_ERROR(efi_status)) { @@ -1097,12 +1137,14 @@ EFI_STATUS read_image(EFI_HANDLE image_handle, CHAR16 *ImagePath, *data = sourcebuffer; *datasize = sourcesize; } else if (find_httpboot(shim_li->DeviceHandle)) { + str16_to_str8(ImagePath, &netbootname); efi_status = httpboot_fetch_buffer (image_handle, &sourcebuffer, - &sourcesize); + &sourcesize, + netbootname); if (EFI_ERROR(efi_status)) { - perror(L"Unable to fetch HTTP image: %r\n", - efi_status); + perror(L"Unable to fetch HTTP image %a: %r\n", + netbootname, efi_status); return efi_status; } *data = sourcebuffer; @@ -1207,7 +1249,7 @@ EFI_STATUS init_grub(EFI_HANDLE image_handle) efi_status = start_image(image_handle, MOK_MANAGER); if (EFI_ERROR(efi_status)) { console_print(L"start_image() returned %r\n", efi_status); - msleep(2000000); + usleep(2000000); return efi_status; } @@ -1222,7 +1264,7 @@ EFI_STATUS init_grub(EFI_HANDLE image_handle) console_print( L"start_image() returned %r, falling back to default loader\n", efi_status); - msleep(2000000); + usleep(2000000); load_options = NULL; load_options_size = 0; efi_status = start_image(image_handle, DEFAULT_LOADER); @@ -1230,7 +1272,7 @@ EFI_STATUS init_grub(EFI_HANDLE image_handle) if (EFI_ERROR(efi_status)) { console_print(L"start_image() returned %r\n", efi_status); - msleep(2000000); + usleep(2000000); } return efi_status; @@ -1275,24 +1317,10 @@ EFI_STATUS set_second_stage (EFI_HANDLE image_handle) return EFI_SUCCESS; } -static void * -ossl_malloc(size_t num) -{ - return AllocatePool(num); -} - -static void -ossl_free(void *addr) -{ - FreePool(addr); -} - static void init_openssl(void) { - CRYPTO_set_mem_functions(ossl_malloc, NULL, ossl_free); OPENSSL_init(); - CRYPTO_set_mem_functions(ossl_malloc, NULL, ossl_free); ERR_load_ERR_strings(); ERR_load_BN_strings(); ERR_load_RSA_strings(); @@ -1391,6 +1419,96 @@ uninstall_shim_protocols(void) #endif } +static void +check_section_helper(char *section_name, int len, void **pointer, + EFI_IMAGE_SECTION_HEADER *Section, void *data, + int datasize, size_t minsize) +{ + if (CompareMem(Section->Name, section_name, len) == 0) { + *pointer = ImageAddress(data, datasize, Section->PointerToRawData); + if (Section->SizeOfRawData < minsize) { + dprint(L"found and rejected %.*a bad size\n", len, section_name); + dprint(L"minsize: %d\n", minsize); + dprint(L"rawsize: %d\n", Section->SizeOfRawData); + return ; + } + if (!*pointer) { + return ; + } + dprint(L"found %.*a\n", len, section_name); + } +} + +#define check_section(section_name, pointer, section, data, datasize, minsize) \ + check_section_helper(section_name, sizeof(section_name) - 1, pointer, \ + section, data, datasize, minsize) + +EFI_STATUS +load_revocations_file(EFI_HANDLE image_handle, CHAR16 *PathName) +{ + EFI_STATUS efi_status = EFI_SUCCESS; + PE_COFF_LOADER_IMAGE_CONTEXT context; + EFI_IMAGE_SECTION_HEADER *Section; + int datasize = 0; + void *data = NULL; + unsigned int i; + char *sbat_var_automatic = NULL; + char *sbat_var_latest = NULL; + uint8_t *ssps_automatic = NULL; + uint8_t *sspv_automatic = NULL; + uint8_t *ssps_latest = NULL; + uint8_t *sspv_latest = NULL; + + efi_status = read_image(image_handle, L"revocations.efi", &PathName, + &data, &datasize); + if (EFI_ERROR(efi_status)) + return efi_status; + + efi_status = verify_image(data, datasize, shim_li, &context); + if (EFI_ERROR(efi_status)) { + dprint(L"revocations failed to verify\n"); + return efi_status; + } + dprint(L"verified revocations\n"); + + Section = context.FirstSection; + for (i = 0; i < context.NumberOfSections; i++, Section++) { + dprint(L"checking section \"%c%c%c%c%c%c%c%c\"\n", (char *)Section->Name); + check_section(".sbata\0\0", (void **)&sbat_var_automatic, Section, + data, datasize, sizeof(SBAT_VAR_ORIGINAL)); + check_section(".sbatl\0\0", (void **)&sbat_var_latest, Section, + data, datasize, sizeof(SBAT_VAR_ORIGINAL)); + check_section(".sspva\0\0", (void **)&sspv_automatic, Section, + data, datasize, SSPVER_SIZE); + check_section(".sspsa\0\0", (void **)&ssps_automatic, Section, + data, datasize, SSPSIG_SIZE); + check_section(".sspvl\0\0", (void **)&sspv_latest, Section, + data, datasize, SSPVER_SIZE); + check_section(".sspsl\0\0", (void **)&ssps_latest, Section, + data, datasize, SSPSIG_SIZE); + } + + if (sbat_var_latest && sbat_var_automatic) { + dprint(L"attempting to update SBAT_LEVEL\n"); + efi_status = set_sbat_uefi_variable(sbat_var_automatic, + sbat_var_latest); + } else { + dprint(L"no data for SBAT_LEVEL\n"); + } + + if ((sspv_automatic && ssps_automatic) || (sspv_latest && ssps_latest)) { + dprint(L"attempting to update SkuSiPolicy\n"); + efi_status = set_ssp_uefi_variable(sspv_automatic, ssps_automatic, + sspv_latest, ssps_latest); + + } else { + dprint(L"no data for SkuSiPolicy\n"); + } + + FreePool(data); + return efi_status; +} + EFI_STATUS load_cert_file(EFI_HANDLE image_handle, CHAR16 *filename, CHAR16 *PathName) { @@ -1437,9 +1555,12 @@ load_cert_file(EFI_HANDLE image_handle, CHAR16 *filename, CHAR16 *PathName) return EFI_SUCCESS; } -/* Read additional certificates from files (after verifying signatures) */ +/* + * Read additional certificates and SBAT Level requirements from files + * (after verifying signatures) + */ EFI_STATUS -load_certs(EFI_HANDLE image_handle) +load_unbundled_trust(EFI_HANDLE image_handle) { EFI_STATUS efi_status; EFI_LOADED_IMAGE *li = NULL; @@ -1450,6 +1571,7 @@ load_certs(EFI_HANDLE image_handle) EFI_FILE_IO_INTERFACE *drive; UINTN buffersize = 0; void *buffer = NULL; + BOOLEAN search_revocations = TRUE; efi_status = gBS->HandleProtocol(image_handle, &EFI_LOADED_IMAGE_GUID, (void **)&li); @@ -1466,7 +1588,15 @@ load_certs(EFI_HANDLE image_handle) efi_status = gBS->HandleProtocol(device, &EFI_SIMPLE_FILE_SYSTEM_GUID, (void **)&drive); if (EFI_ERROR(efi_status)) { - perror(L"Failed to find fs: %r\n", efi_status); + dprint(L"Failed to find fs on local drive (netboot?): %r \n", + efi_status); + /* + * Network boot cases do not support reading a directory. Try + * to read revocations.efi to pull in any unbundled SBATLevel + * updates unconditionally in those cases. This may produce + * console noise when the file is not present. + */ + load_cert_file(image_handle, REVOCATIONFILE, PathName); goto done; } @@ -1482,23 +1612,83 @@ load_certs(EFI_HANDLE image_handle) goto done; } - while (1) { - int old = buffersize; + if (!secure_mode()) + goto done; + + + while (true) { + UINTN old = buffersize; + efi_status = dir->Read(dir, &buffersize, buffer); if (efi_status == EFI_BUFFER_TOO_SMALL) { + if (buffersize == old) { + /* + * Some UEFI drivers or firmwares are not compliant with + * the EFI_FILE_PROTOCOL.Read() specs and do not return the + * required buffer size along with EFI_BUFFER_TOO_SMALL. + * Work around this by progressively increasing the buffer + * size, up to a certain point, until the call succeeds. + */ + perror(L"Error reading directory %s - non-compliant UEFI driver or firmware!\n", + PathName); + buffersize = (buffersize < 4) ? 4 : buffersize * 2; + if (buffersize > 1024) + goto done; + } buffer = ReallocatePool(buffer, old, buffersize); + if (buffer == NULL) { + perror(L"Failed to read directory %s - %r\n", + PathName, EFI_OUT_OF_RESOURCES); + goto done; + } continue; } else if (EFI_ERROR(efi_status)) { perror(L"Failed to read directory %s - %r\n", PathName, - efi_status); + efi_status); goto done; } info = (EFI_FILE_INFO *)buffer; - if (buffersize == 0 || !info) - goto done; + if (buffersize == 0 || !info) { + if (search_revocations) { + search_revocations = FALSE; + efi_status = root->Open(root, &dir, PathName, + EFI_FILE_MODE_READ, 0); + if (EFI_ERROR(efi_status)) { + perror(L"Failed to open %s - %r\n", + PathName, efi_status); + goto done; + } + continue; + } else { + goto done; + } + } - if (StrnCaseCmp(info->FileName, L"shim_certificate", 16) == 0) { + /* + * In the event that there are unprocessed revocation + * additions, they could be intended to ban any *new* trust + * anchors we find here. With that in mind, we always want to + * do a pass of loading revocations before we try to add + * anything new to our allowlist. This is done by making two + * passes over the directory, first to search for the + * revocations.efi file then to search for shim_certificate.efi + */ + if (search_revocations && + StrCaseCmp(info->FileName, REVOCATIONFILE) == 0) { + load_revocations_file(image_handle, PathName); + search_revocations = FALSE; + efi_status = root->Open(root, &dir, PathName, + EFI_FILE_MODE_READ, 0); + if (EFI_ERROR(efi_status)) { + perror(L"Failed to open %s - %r\n", + PathName, efi_status); + goto done; + } + } + + if (!search_revocations && + StrCaseCmp(info->FileName, L"shim_certificate.efi") == 0) { load_cert_file(image_handle, info->FileName, PathName); } } @@ -1637,7 +1827,7 @@ devel_egress(devel_egress_action action UNUSED) console_print(L"Waiting to %a...", reasons[action]); for (size_t sleepcount = 0; sleepcount < 10; sleepcount++) { console_print(L"%d...", 10 - sleepcount); - msleep(1000000); + usleep(1000000); } console_print(L"\ndoing %a\n", action); @@ -1708,7 +1898,7 @@ efi_main (EFI_HANDLE passed_image_handle, EFI_SYSTEM_TABLE *passed_systab) */ debug_hook(); - efi_status = set_sbat_uefi_variable(); + efi_status = set_sbat_uefi_variable_internal(); if (EFI_ERROR(efi_status) && secure_mode()) { perror(L"%s variable initialization failed\n", SBAT_VAR_NAME); msg = SET_SBAT; @@ -1717,13 +1907,19 @@ efi_main (EFI_HANDLE passed_image_handle, EFI_SYSTEM_TABLE *passed_systab) dprint(L"%s variable initialization failed: %r\n", SBAT_VAR_NAME, efi_status); } + efi_status = set_ssp_uefi_variable_internal(); + if (EFI_ERROR(efi_status)) { + dprint(L"%s variable initialization failed: %r\n", + SSPVER_VAR_NAME, efi_status); + } + dprint(L"%s variable initialization done\n", SSPVER_VAR_NAME); if (secure_mode()) { char *sbat_start = (char *)&_sbat; char *sbat_end = (char *)&_esbat; INIT_LIST_HEAD(&sbat_var); - efi_status = parse_sbat_var(&sbat_var); + efi_status = parse_sbat_var(&sbat_var, NULL); if (EFI_ERROR(efi_status)) { perror(L"Parsing %s variable failed: %r\n", SBAT_VAR_NAME, efi_status); @@ -1743,11 +1939,9 @@ efi_main (EFI_HANDLE passed_image_handle, EFI_SYSTEM_TABLE *passed_systab) init_openssl(); - if (secure_mode()) { - efi_status = load_certs(global_image_handle); - if (EFI_ERROR(efi_status)) { - LogError(L"Failed to load addon certificates\n"); - } + efi_status = load_unbundled_trust(global_image_handle); + if (EFI_ERROR(efi_status)) { + LogError(L"Failed to load addon certificates / sbat level\n"); } /* @@ -1775,12 +1969,18 @@ die: #if defined(ENABLE_SHIM_DEVEL) devel_egress(COLD_RESET); #else - msleep(5000000); + usleep(5000000); RT->ResetSystem(EfiResetShutdown, EFI_SECURITY_VIOLATION, 0, NULL); #endif } + /* + * This variable is supposed to be set by second stages, so ensure it is + * not set when we are starting up. + */ + (void) del_variable(SHIM_RETAIN_PROTOCOL_VAR_NAME, SHIM_LOCK_GUID); + efi_status = shim_init(); if (EFI_ERROR(efi_status)) { msg = SHIM_INIT; @@ -1792,7 +1992,7 @@ die: */ if (user_insecure_mode) { console_print(L"Booting in insecure mode\n"); - msleep(2000000); + usleep(2000000); } /* diff --git a/shim.h b/shim.h index 14824c6..5791a03 100644 --- a/shim.h +++ b/shim.h @@ -180,6 +180,7 @@ #include "include/replacements.h" #include "include/sbat.h" #include "include/sbat_var_defs.h" +#include "include/ssp.h" #if defined(OVERRIDE_SECURITY_POLICY) #include "include/security_policy.h" #endif @@ -281,18 +282,32 @@ verify_buffer (char *data, int datasize, #ifndef SHIM_UNIT_TEST #define perror_(file, line, func, fmt, ...) ({ \ UINTN __perror_ret = 0; \ + _Static_assert((fmt) != NULL, \ + "format specifier cannot be NULL"); \ if (!in_protocol) \ __perror_ret = console_print((fmt), ##__VA_ARGS__); \ LogError_(file, line, func, fmt, ##__VA_ARGS__); \ __perror_ret; \ }) -#define perror(fmt, ...) \ - perror_(__FILE__, __LINE__ - 1, __func__, fmt, ##__VA_ARGS__) -#define LogError(fmt, ...) \ - LogError_(__FILE__, __LINE__ - 1, __func__, fmt, ##__VA_ARGS__) +#define perror(fmt, ...) ({ \ + _Static_assert((fmt) != NULL, \ + "format specifier cannot be NULL"); \ + perror_(__FILE__, __LINE__ - 1, __func__, fmt, ##__VA_ARGS__); \ + }) +#define LogError(fmt, ...) ({ \ + _Static_assert((fmt) != NULL, \ + "format specifier cannot be NULL"); \ + LogError_(__FILE__, __LINE__ - 1, __func__, fmt, ##__VA_ARGS__);\ + }) #else -#define perror(fmt, ...) -#define LogError(fmt, ...) +#define perror(fmt, ...) ({ \ + _Static_assert((fmt) != NULL, \ + "format specifier cannot be NULL"); \ + }) +#define LogError(fmt, ...) ({ \ + _Static_assert((fmt) != NULL, \ + "format specifier cannot be NULL"); \ + }) #endif #ifdef ENABLE_SHIM_DEVEL @@ -305,6 +320,8 @@ verify_buffer (char *data, int datasize, #define DEBUG_VAR_NAME L"SHIM_DEBUG" #endif +#define SHIM_RETAIN_PROTOCOL_VAR_NAME L"ShimRetainProtocol" + char *translate_slashes(char *out, const char *str); #endif /* SHIM_H_ */ diff --git a/test-data/.gitignore b/test-data/.gitignore new file mode 100644 index 0000000..bbde87b --- /dev/null +++ b/test-data/.gitignore @@ -0,0 +1 @@ +!/*.efi diff --git a/test-data/grubx64.0.76.el7.1.efi b/test-data/grubx64.0.76.el7.1.efi new file mode 100755 index 0000000..29f6812 Binary files /dev/null and b/test-data/grubx64.0.76.el7.1.efi differ diff --git a/test-data/grubx64.0.76.el7.efi b/test-data/grubx64.0.76.el7.efi new file mode 100755 index 0000000..5a198f6 Binary files /dev/null and b/test-data/grubx64.0.76.el7.efi differ diff --git a/test-data/grubx64.0.80.el7.efi b/test-data/grubx64.0.80.el7.efi new file mode 100755 index 0000000..c5f28d9 Binary files /dev/null and b/test-data/grubx64.0.80.el7.efi differ diff --git a/test-pe-relocate.c b/test-pe-relocate.c new file mode 100644 index 0000000..5557779 --- /dev/null +++ b/test-pe-relocate.c @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * test-pe-reloc.c - attempt to test relocate_coff() + * Copyright Peter Jones + */ + +#ifndef SHIM_UNIT_TEST +#define SHIM_UNIT_TEST +#endif +#include "shim.h" + +static int +test_image_address(void) +{ + char image[4]; + void *ret; + + assert_equal_return(ImageAddress(image, sizeof(image), 0), &image[0], -1, "got %p expected %p\n"); + assert_equal_return(ImageAddress(image, sizeof(image), 4), NULL, -1, "got %p expected %p\n"); + assert_equal_return(ImageAddress((void *)1, 2, 3), NULL, -1, "got %p expected %p\n"); + assert_equal_return(ImageAddress((void *)-1ull, UINT64_MAX, UINT64_MAX), NULL, -1, "got %p expected %p\n"); + assert_equal_return(ImageAddress((void *)0, UINT64_MAX, UINT64_MAX), NULL, -1, "got %p expected %p\n"); + assert_equal_return(ImageAddress((void *)1, UINT64_MAX, UINT64_MAX), NULL, -1, "got %p expected %p\n"); + assert_equal_return(ImageAddress((void *)2, UINT64_MAX, UINT64_MAX), NULL, -1, "got %p expected %p\n"); + assert_equal_return(ImageAddress((void *)3, UINT64_MAX, UINT64_MAX), NULL, -1, "got %p expected %p\n"); + + return 0; +} + +int +main(void) +{ + int status = 0; + test(test_image_address); + + return status; +} + +// vim:fenc=utf-8:tw=75:noet diff --git a/test-pe-util.c b/test-pe-util.c new file mode 100644 index 0000000..d576548 --- /dev/null +++ b/test-pe-util.c @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * test-pe-util.c - test PE utilities + */ + +#ifndef SHIM_UNIT_TEST +#define SHIM_UNIT_TEST +#endif +#include "shim.h" + +static int +test_is_page_aligned(void) +{ + assert_true_return(IS_PAGE_ALIGNED(0), -1, "\n"); + assert_false_return(IS_PAGE_ALIGNED(1), -1, "\n"); + assert_false_return(IS_PAGE_ALIGNED(4095), -1, "\n"); + assert_true_return(IS_PAGE_ALIGNED(4096), -1, "\n"); + assert_false_return(IS_PAGE_ALIGNED(4097), -1, "\n"); + + return 0; +} + +int +main(void) +{ + int status = 0; + test(test_is_page_aligned); + + return status; +} diff --git a/test-sbat.c b/test-sbat.c index 72bebe7..b37efcd 100644 --- a/test-sbat.c +++ b/test-sbat.c @@ -5,6 +5,7 @@ #ifndef SHIM_UNIT_TEST #define SHIM_UNIT_TEST +#include "sbat_var_defs.h" #endif #include "shim.h" @@ -195,6 +196,22 @@ free_mock_sbat_entries(list_t *entries) /* * parse_sbat_section() tests */ +int +test_parse_sbat_tiny(void) +{ + char section_base[] = "\0a\00"; + size_t section_size = 2; + struct sbat_section_entry **entries; + size_t n = 0; + EFI_STATUS status; + + status = parse_sbat_section(section_base, section_size, &n, &entries); + assert_equal_return(status, EFI_SUCCESS, -1, "got %#hhx expected %#hhx\n"); + assert_equal_return(n, 0, -1, "got %#hhx expected %#hhx\n"); + + return 0; +} + int test_parse_sbat_section_null_sbat_base(void) { @@ -362,7 +379,7 @@ test_parse_sbat_var_null_list(void) EFI_STATUS status; INIT_LIST_HEAD(&sbat_var); - status = parse_sbat_var(NULL); + status = parse_sbat_var(NULL, NULL); cleanup_sbat_var(&sbat_var); assert_equal_return(status, EFI_INVALID_PARAMETER, -1, "got %#hhx expected %#hhx\n"); @@ -1107,11 +1124,43 @@ test_preserve_sbat_uefi_variable_bad_short(void) return 0; } +static int +test_sbat_var_asciz(void) +{ + EFI_STATUS status; + char buf[1024] = ""; + UINT32 attrs = 0; + UINTN size = sizeof(buf); + char expected[] = SBAT_VAR_AUTOMATIC; + + status = set_sbat_uefi_variable(SBAT_VAR_AUTOMATIC, SBAT_VAR_AUTOMATIC); + if (status != EFI_SUCCESS) + return -1; + + status = RT->GetVariable(SBAT_VAR_NAME, &SHIM_LOCK_GUID, &attrs, &size, buf); + if (status != EFI_SUCCESS) + return -1; + + /* + * this should be enough to get past "sbat,", which handles the + * first error. + */ + if (size < (strlen(SBAT_VAR_SIG) + 2) || size != strlen(expected)) + return -1; + + if (strncmp(expected, buf, size) != 0) + return -1; + + return 0; +} + int main(void) { int status = 0; + // parse_sbat section tests + test(test_parse_sbat_tiny); test(test_parse_sbat_section_null_sbat_base); test(test_parse_sbat_section_zero_sbat_size); test(test_parse_sbat_section_null_entries); @@ -1155,7 +1204,9 @@ main(void) test(test_preserve_sbat_uefi_variable_version_older); test(test_preserve_sbat_uefi_variable_version_olderlonger); - return 0; + test(test_sbat_var_asciz); + + return status; } // vim:fenc=utf-8:tw=75:noet