try to show errors more usefully.

Signed-off-by: Peter Jones <pjones@redhat.com>

Makefile

@@ -142,9 +142,9 @@ CFLAGS += -DENABLE_SHIM_CERT
 else
 TARGETS += $(MMNAME) $(FBNAME)
 endif
-OBJS	= shim.o netboot.o cert.o replacements.o tpm.o version.o
+OBJS	= shim.o netboot.o cert.o replacements.o tpm.o version.o errlog.o
 KEYS	= shim_cert.h ocsp.* ca.* shim.crt shim.csr shim.p12 shim.pem shim.key shim.cer
-ORIG_SOURCES	= shim.c shim.h netboot.c include/PeImage.h include/wincert.h include/console.h replacements.c replacements.h tpm.c tpm.h version.h
+ORIG_SOURCES	= shim.c shim.h netboot.c include/PeImage.h include/wincert.h include/console.h replacements.c replacements.h tpm.c tpm.h version.h errlog.c
 MOK_OBJS = MokManager.o PasswordCrypt.o crypt_blowfish.o
 ORIG_MOK_SOURCES = MokManager.c shim.h include/console.h PasswordCrypt.c PasswordCrypt.h crypt_blowfish.c crypt_blowfish.h
 FALLBACK_OBJS = fallback.o tpm.o

errlog.c

@@ -0,0 +1,90 @@
+/*
+ * errlog.c
+ * Copyright 2017 Peter Jones <pjones@redhat.com>
+ *
+ * Distributed under terms of the GPLv3 license.
+ */
+
+#include "shim.h"
+
+#ifdef LogError
+#undef LogError
+#endif
+
+static CHAR16 **errs = NULL;
+static UINTN nerrs = 0;
+
+EFI_STATUS
+VLogError(const char *file, int line, const char *func, CHAR16 *fmt, va_list args)
+{
+	va_list args2;
+	UINTN size = 0, size2;
+	CHAR16 **newerrs;
+
+	size = SPrint(NULL, 0, L"%a:%d %a() ", file, line, func);
+	va_copy(args2, args);
+	size2 = VSPrint(NULL, 0, fmt, args2);
+	va_end(args2);
+
+	newerrs = ReallocatePool(errs, (nerrs + 1) * sizeof(*errs),
+				       (nerrs + 3) * sizeof(*errs));
+	if (!newerrs)
+		return EFI_OUT_OF_RESOURCES;
+
+	newerrs[nerrs] = AllocatePool(size*2+2);
+	if (!newerrs[nerrs])
+		return EFI_OUT_OF_RESOURCES;
+	newerrs[nerrs+1] = AllocatePool(size2*2+2);
+	if (!newerrs[nerrs+1])
+		return EFI_OUT_OF_RESOURCES;
+
+	SPrint(newerrs[nerrs], size*2+2, L"%a:%d %a() ", file, line, func);
+	va_copy(args2, args);
+	VSPrint(newerrs[nerrs+1], size2*2+2, fmt, args2);
+	va_end(args2);
+
+	nerrs += 2;
+	newerrs[nerrs] = NULL;
+	errs = newerrs;
+
+	return EFI_SUCCESS;
+}
+
+EFI_STATUS
+LogError(const char *file, int line, const char *func, CHAR16 *fmt, ...)
+{
+	va_list args;
+	EFI_STATUS status;
+
+	va_start(args, fmt);
+	status = VLogError(file, line, func, fmt, args);
+	va_end(args);
+
+	return status;
+}
+
+VOID
+PrintErrors(VOID)
+{
+	UINTN i;
+
+	if (!verbose)
+		return;
+
+	for (i = 0; i < nerrs; i++)
+		Print(L"%s", errs[i]);
+}
+
+VOID
+ClearErrors(VOID)
+{
+	UINTN i;
+
+	for (i = 0; i < nerrs; i++)
+		FreePool(errs[i]);
+	FreePool(errs);
+	nerrs = 0;
+	errs = NULL;
+}
+
+// vim:fenc=utf-8:tw=75

netboot.c

@@ -33,10 +33,8 @@
  * Corporation.
  */
 
-#include <efi.h>
-#include <efilib.h>
-#include <string.h>
 #include "shim.h"
+#include <string.h>
 #include "netboot.h"
 #include "str.h"
 

shim.c

@@ -33,31 +33,8 @@
  * Corporation.
  */
 
-#include <efi.h>
-#include <efilib.h>
-#include <Library/BaseCryptLib.h>
-#include "PeImage.h"
 #include "shim.h"
-#include "netboot.h"
-#include "httpboot.h"
-#include "replacements.h"
-#include "tpm.h"
-#include "ucs2.h"
 
-#include "guid.h"
-#include "variables.h"
-#include "efiauthenticated.h"
-#include "security_policy.h"
-#include "console.h"
-#include "version.h"
-
-#ifdef ENABLE_SHIM_CERT
-#include "shim_cert.h"
-#endif
-
-#include <stdarg.h>
-
-#include <Library/BaseCryptLib.h>
 #include <openssl/err.h>
 #include <openssl/bn.h>
 #include <openssl/dh.h>
@@ -71,6 +48,8 @@
 #include <openssl/rsa.h>
 #include <openssl/dso.h>
 
+#include <Library/BaseCryptLib.h>
+
 #define FALLBACK L"\\fb" EFI_ARCH L".efi"
 #define MOK_MANAGER L"\\mm" EFI_ARCH L".efi"
 
@@ -89,6 +68,7 @@ static UINT8 in_protocol;
 		UINTN __perror_ret = 0;					\
 		if (!in_protocol)					\
 			__perror_ret = Print((fmt), ##__VA_ARGS__);	\
+		LogError(fmt, ##__VA_ARGS__);				\
 		__perror_ret;						\
 	})
 
@@ -466,9 +446,7 @@ static CHECK_STATUS check_db_cert_in_ram(EFI_SIGNATURE_LIST *CertList,
 			Cert = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertList + sizeof (EFI_SIGNATURE_LIST) + CertList->SignatureHeaderSize);
 			CertSize = CertList->SignatureSize - sizeof(EFI_GUID);
 			if (verify_x509(Cert->SignatureData, CertSize)) {
-				drain_openssl_errors();
 				if (verify_eku(Cert->SignatureData, CertSize)) {
-					drain_openssl_errors();
 					IsFound = AuthenticodeVerify (data->CertData,
 								      data->Hdr.dwLength - sizeof(data->Hdr),
 								      Cert->SignatureData,
@@ -477,12 +455,14 @@ static CHECK_STATUS check_db_cert_in_ram(EFI_SIGNATURE_LIST *CertList,
 					if (IsFound) {
 						tpm_measure_variable(dbname, guid, CertSize, Cert->SignatureData);
 						return DATA_FOUND;
+						drain_openssl_errors();
+					} else {
+						LogError(L"AuthenticodeVerify(): %d\n", IsFound);
 					}
 				}
 			} else if (verbose) {
 				console_notify(L"Not a DER encoding x.509 Certificate");
 			}
-			drain_openssl_errors();
 		}
 
 		dbsize -= CertList->SignatureListSize;
@@ -598,36 +578,50 @@ static EFI_STATUS check_blacklist (WIN_CERTIFICATE_EFI_PKCS *cert,
 	if (check_db_hash_in_ram(dbx, vendor_dbx_size, sha256hash,
 				 SHA256_DIGEST_SIZE, EFI_CERT_SHA256_GUID,
 				 L"dbx", secure_var) ==
-				DATA_FOUND)
+				DATA_FOUND) {
+		LogError(L"binary sha256hash found in vendor dbx\n");
 		return EFI_SECURITY_VIOLATION;
+	}
 	if (check_db_hash_in_ram(dbx, vendor_dbx_size, sha1hash,
 				 SHA1_DIGEST_SIZE, EFI_CERT_SHA1_GUID,
 				 L"dbx", secure_var) ==
-				DATA_FOUND)
+				DATA_FOUND) {
+		LogError(L"binary sha1hash found in vendor dbx\n");
 		return EFI_SECURITY_VIOLATION;
+	}
 	if (cert && check_db_cert_in_ram(dbx, vendor_dbx_size, cert,
 					 sha256hash, L"dbx",
-					 secure_var) == DATA_FOUND)
+					 secure_var) == DATA_FOUND) {
+		LogError(L"cert sha256hash found in vendor dbx\n");
 		return EFI_SECURITY_VIOLATION;
-
+	}
 	if (check_db_hash(L"dbx", secure_var, sha256hash, SHA256_DIGEST_SIZE,
-			  EFI_CERT_SHA256_GUID) == DATA_FOUND)
+			  EFI_CERT_SHA256_GUID) == DATA_FOUND) {
+		LogError(L"binary sha256hash found in system dbx\n");
 		return EFI_SECURITY_VIOLATION;
+	}
 	if (check_db_hash(L"dbx", secure_var, sha1hash, SHA1_DIGEST_SIZE,
-			  EFI_CERT_SHA1_GUID) == DATA_FOUND)
+			  EFI_CERT_SHA1_GUID) == DATA_FOUND) {
+		LogError(L"binary sha1hash found in system dbx\n");
 		return EFI_SECURITY_VIOLATION;
+	}
 	if (cert && check_db_cert(L"dbx", secure_var, cert, sha256hash) ==
-				DATA_FOUND)
+				DATA_FOUND) {
+		LogError(L"cert sha256hash found in system dbx\n");
 		return EFI_SECURITY_VIOLATION;
+	}
 	if (check_db_hash(L"MokListX", shim_var, sha256hash, SHA256_DIGEST_SIZE,
 			  EFI_CERT_SHA256_GUID) == DATA_FOUND) {
+		LogError(L"binary sha256hash found in Mok dbx\n");
 		return EFI_SECURITY_VIOLATION;
 	}
 	if (cert && check_db_cert(L"MokListX", shim_var, cert, sha256hash) ==
 				DATA_FOUND) {
+		LogError(L"cert sha256hash found in Mok dbx\n");
 		return EFI_SECURITY_VIOLATION;
 	}
 
+	drain_openssl_errors();
 	return EFI_SUCCESS;
 }
 
@@ -651,18 +645,24 @@ static EFI_STATUS check_whitelist (WIN_CERTIFICATE_EFI_PKCS *cert,
 					EFI_CERT_SHA256_GUID) == DATA_FOUND) {
 			update_verification_method(VERIFIED_BY_HASH);
 			return EFI_SUCCESS;
+		} else {
+			LogError(L"check_db_hash(db, sha256hash) != DATA_FOUND\n");
 		}
 		if (check_db_hash(L"db", secure_var, sha1hash, SHA1_DIGEST_SIZE,
 					EFI_CERT_SHA1_GUID) == DATA_FOUND) {
 			verification_method = VERIFIED_BY_HASH;
 			update_verification_method(VERIFIED_BY_HASH);
 			return EFI_SUCCESS;
+		} else {
+			LogError(L"check_db_hash(db, sha1hash) != DATA_FOUND\n");
 		}
 		if (cert && check_db_cert(L"db", secure_var, cert, sha256hash)
 					== DATA_FOUND) {
 			verification_method = VERIFIED_BY_CERT;
 			update_verification_method(VERIFIED_BY_CERT);
 			return EFI_SUCCESS;
+		} else {
+			LogError(L"check_db_cert(db, sha256hash) != DATA_FOUND\n");
 		}
 	}
 
@@ -671,16 +671,19 @@ static EFI_STATUS check_whitelist (WIN_CERTIFICATE_EFI_PKCS *cert,
 		verification_method = VERIFIED_BY_HASH;
 		update_verification_method(VERIFIED_BY_HASH);
 		return EFI_SUCCESS;
+	} else {
+		LogError(L"check_db_hash(MokList, sha256hash) != DATA_FOUND\n");
 	}
 	if (cert && check_db_cert(L"MokList", shim_var, cert, sha256hash) ==
 				DATA_FOUND) {
 		verification_method = VERIFIED_BY_CERT;
 		update_verification_method(VERIFIED_BY_CERT);
 		return EFI_SUCCESS;
+	} else {
+		LogError(L"check_db_cert(MokList, sha256hash) != DATA_FOUND\n");
 	}
 
 	update_verification_method(VERIFIED_BY_NOTHING);
-	crypterr(EFI_SECURITY_VIOLATION);
 	return EFI_SECURITY_VIOLATION;
 }
 
@@ -1058,15 +1061,19 @@ static EFI_STATUS verify_buffer (char *data, int datasize,
 	drain_openssl_errors();
 
 	status = generate_hash(data, datasize, context, sha256hash, sha1hash);
-	if (status != EFI_SUCCESS)
+	if (status != EFI_SUCCESS) {
+		LogError(L"generate_hash: %r\n", status);
 		return status;
+	}
 
 	/*
 	 * Check that the MOK database hasn't been modified
 	 */
 	status = verify_mok();
-	if (status != EFI_SUCCESS)
+	if (status != EFI_SUCCESS) {
+		LogError(L"verify_mok: %r\n", status);
 		return status;
+	}
 
 	/*
 	 * Ensure that the binary isn't blacklisted
@@ -1074,6 +1081,7 @@ static EFI_STATUS verify_buffer (char *data, int datasize,
 	status = check_blacklist(cert, sha256hash, sha1hash);
 	if (status != EFI_SUCCESS) {
 		perror(L"Binary is blacklisted\n");
+		LogError(L"Binary is blacklisted: %r\n", status);
 		return status;
 	}
 
@@ -1082,8 +1090,12 @@ static EFI_STATUS verify_buffer (char *data, int datasize,
 	 * databases
 	 */
 	status = check_whitelist(cert, sha256hash, sha1hash);
-	if (status == EFI_SUCCESS)
+	if (status == EFI_SUCCESS) {
+		drain_openssl_errors();
 		return status;
+	} else {
+		LogError(L"check_whitelist(): %r\n", status);
+	}
 
 	if (cert) {
 #if defined(ENABLE_SHIM_CERT)
@@ -1098,7 +1110,10 @@ static EFI_STATUS verify_buffer (char *data, int datasize,
 			update_verification_method(VERIFIED_BY_CERT);
 			tpm_measure_variable(L"Shim", shim_var, sizeof(shim_cert), shim_cert);
 			status = EFI_SUCCESS;
+			drain_openssl_errors();
 			return status;
+		} else {
+			LogError(L"AuthenticodeVerify(shim_cert) failed\n");
 		}
 #endif /* defined(ENABLE_SHIM_CERT) */
 
@@ -1113,10 +1128,16 @@ static EFI_STATUS verify_buffer (char *data, int datasize,
 			update_verification_method(VERIFIED_BY_CERT);
 			tpm_measure_variable(L"Shim", shim_var, vendor_cert_size, vendor_cert);
 			status = EFI_SUCCESS;
+			drain_openssl_errors();
 			return status;
+		} else {
+			LogError(L"AuthenticodeVerify(vendor_cert) failed\n");
 		}
 	}
 
+	LogError(L"Binary is not whitelisted\n");
+	crypterr(EFI_SECURITY_VIOLATION);
+	PrintErrors();
 	status = EFI_SECURITY_VIOLATION;
 	return status;
 }
@@ -1900,6 +1921,8 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath)
 
 		if (efi_status != EFI_SUCCESS) {
 			perror(L"Failed to load image %s: %r\n", PathName, efi_status);
+			PrintErrors();
+			ClearErrors();
 			goto done;
 		}
 	}
@@ -1917,6 +1940,8 @@ EFI_STATUS start_image(EFI_HANDLE image_handle, CHAR16 *ImagePath)
 
 	if (efi_status != EFI_SUCCESS) {
 		perror(L"Failed to load image: %r\n", efi_status);
+		PrintErrors();
+		ClearErrors();
 		CopyMem(li, &li_bak, sizeof(li_bak));
 		goto done;
 	}

shim.h

@@ -1,3 +1,9 @@
+#ifndef SHIM_H_
+#define SHIM_H_
+
+#include <efi.h>
+#include <efilib.h>
+
 #include "PeImage.h"
 
 extern EFI_GUID SHIM_LOCK_GUID;
@@ -37,13 +43,17 @@ typedef struct _SHIM_LOCK {
 
 extern EFI_STATUS shim_init(void);
 extern void shim_fini(void);
+extern EFI_STATUS LogError(const char *file, int line, const char *func, CHAR16 *fmt, ...);
+extern EFI_STATUS VLogError(const char *file, int line, const char *func, CHAR16 *fmt, va_list args);
+extern VOID PrintErrors(VOID);
+extern VOID ClearErrors(VOID);
 
 #ifdef __x86_64__
 #ifndef DEFAULT_LOADER
 #define DEFAULT_LOADER L"\\grubx64.efi"
 #endif
 #ifndef DEFAULT_LOADER_CHAR
-#define DEFAULT_LOADER_CHAR L"\\grubx64.efi"
+#define DEFAULT_LOADER_CHAR "\\grubx64.efi"
 #endif
 #ifndef EFI_ARCH
 #define EFI_ARCH L"x64"
@@ -58,7 +68,7 @@ extern void shim_fini(void);
 #define DEFAULT_LOADER L"\\grubia32.efi"
 #endif
 #ifndef DEFAULT_LOADER_CHAR
-#define DEFAULT_LOADER_CHAR L"\\grubia32.efi"
+#define DEFAULT_LOADER_CHAR "\\grubia32.efi"
 #endif
 #ifndef EFI_ARCH
 #define EFI_ARCH L"ia32"
@@ -73,7 +83,7 @@ extern void shim_fini(void);
 #define DEFAULT_LOADER L"\\grubaa64.efi"
 #endif
 #ifndef DEFAULT_LOADER_CHAR
-#define DEFAULT_LOADER_CHAR L"\\grubaa64.efi"
+#define DEFAULT_LOADER_CHAR "\\grubaa64.efi"
 #endif
 #ifndef EFI_ARCH
 #define EFI_ARCH L"aa64"
@@ -88,7 +98,7 @@ extern void shim_fini(void);
 #define DEFAULT_LOADER L"\\grubarm.efi"
 #endif
 #ifndef DEFAULT_LOADER_CHAR
-#define DEFAULT_LOADER_CHAR L"\\grubarm.efi"
+#define DEFAULT_LOADER_CHAR "\\grubarm.efi"
 #endif
 #ifndef EFI_ARCH
 #define EFI_ARCH L"arm"
@@ -97,3 +107,24 @@ extern void shim_fini(void);
 #define DEBUGDIR L"/usr/lub/debug/usr/share/shim/arm/"
 #endif
 #endif
+
+#include "netboot.h"
+#include "httpboot.h"
+#include "replacements.h"
+#include "tpm.h"
+#include "ucs2.h"
+
+#include "guid.h"
+#include "variables.h"
+#include "efiauthenticated.h"
+#include "security_policy.h"
+#include "console.h"
+#include "version.h"
+
+#ifdef ENABLE_SHIM_CERT
+#include "shim_cert.h"
+#endif
+
+#define LogError(fmt, ...) LogError(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
+
+#endif /* SHIM_H_ */