Merge branch 'rhboot:main' into post-process-pe

rhboot · Dec 30, 2023 · cfddd7a · cfddd7a
2 parents f88a010 + 66e6579
commit cfddd7a
Show file tree

Hide file tree

Showing 41 changed files with 1,049 additions and 603 deletions.
diff --git 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
@@ -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/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
@@ -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/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
@@ -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 $@ $<
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<br>disclosure\* | after<br>disclosure | after Vendor C's<br>first update | after Vendor C's<br>second update | after next global<br>disclosure |
-|--------------------------------------------------------------------------------------|------------------------|---------------------|----------------------------------|----------------------------------|---------------------------------|
-| GRUB global<br>generation number in<br>artifacts .sbat section                       | 3                      | 4                   | 4                                | 4                                | 5                               |
-| Vendor C's product-specific<br>generation number in artifact's<br>.sbat section      | 1                      | 1                   | 2                                | 3                                | 1                               |
-| GRUB global<br>generation number in<br>UEFI SBAT revocation variable                 | 3                      | 4                   | 4                                | 4                                | 5                               |
-| Vendor C's product-specific<br>generation number in<br>UEFI SBAT revocation variable | not set                | not set             | 2                                | 3                                | not set                         |
+|--------------------------------------------------------------------------------------|--------------------------|---------------------|----------------------------------|-----------------------------------|---------------------------------|
+| GRUB global<br>generation number in<br>artifacts .sbat section                       | 3                        | 4                   | 4                                | 4                                 | 5                               |
+| Vendor C's product-specific<br>generation number in artifact's<br>.sbat section      | 1                        | 1                   | 2                                | 3                                 | 1                               |
+| GRUB global<br>generation number in<br>UEFI SBAT revocation variable                 | 3                        | 4                   | 4                                | 4                                 | 5                               |
+| Vendor C's product-specific<br>generation number in<br>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 <Vendor>_key.EFI key files that then contain all the keys that the
+additional `<Vendor>_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
@@ -0,0 +1,42 @@
+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 GRUB binaries impacted by
+
+* CVE-2021-3695
+* CVE-2021-3696
+* CVE-2021-3697
+* CVE-2022-28733
+* CVE-2022-28734
+* CVE-2022-28735
+* CVE-2022-28736
+* CVE-2022-28737
+
+sbat,1,2022052400
+grub,2
+
+To revoke the above and also grub binaries impacted by
+
+* CVE-2022-2601
+* CVE-2022-3775
+
+sbat,1,2022111500
+grub,3
+
+An additonal bug was fixed in shim that was not considered exploitable
+and can be revoked by setting:
+
+sbat,1,2022111500
+shim,2
+grub,3
diff --git 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/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 @@ efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *systab)
 	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);