From 76c0447e204c7e4ce918c4887ce8aae0e0816271 Mon Sep 17 00:00:00 2001

From: Peter Jones <pjones@redhat.com>

Date: Thu, 23 Jul 2020 16:32:05 -0400

Subject: [PATCH 58/62] Handle binaries with multiple signatures.

This adds support for multiple signatures.  It first tries validating

the binary by hash, first against our dbx lists, then against our db

lists.  If it isn't allowed or rejected at that step, it continues to

the normal routine of checking all the signatures.

At this point it does *not* reject a binary just because a signature is

by a cert on a dbx list, though that will override any db list that

certificate is listed on.  If at any point any assertion about the

binary or signature list being well-formed fails, the binary is

immediately rejected, though we do allow skipping over signatures

which have an unsupported sig->Hdr.wCertificateType.

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

Upstream: pr#210

---

 shim.c | 287 +++++++++++++++++++++++++++++++++++++++------------------

 1 file changed, 198 insertions(+), 89 deletions(-)

diff --git a/shim.c b/shim.c

index ee62248ca4e..d10a1ba1cac 100644

--- a/shim.c

+++ b/shim.c

@@ -690,7 +690,7 @@ static EFI_STATUS check_whitelist (WIN_CERTIFICATE_EFI_PKCS *cert,

 	}

 	update_verification_method(VERIFIED_BY_NOTHING);

-	return EFI_SECURITY_VIOLATION;

+	return EFI_NOT_FOUND;

 }

 /*

@@ -1004,6 +1004,103 @@ done:

 	return efi_status;

 }

+static EFI_STATUS

+verify_one_signature(WIN_CERTIFICATE_EFI_PKCS *sig,

+		     UINT8 *sha256hash, UINT8 *sha1hash)

+{

+	EFI_STATUS efi_status;

+

+	/*

+	 * Ensure that the binary isn't blacklisted

+	 */

+	drain_openssl_errors();

+	efi_status = check_blacklist(sig, sha256hash, sha1hash);

+	if (EFI_ERROR(efi_status)) {

+		perror(L"Binary is blacklisted: %r\n", efi_status);

+		PrintErrors();

+		ClearErrors();

+		crypterr(efi_status);

+		return efi_status;

+	}

+

+	/*

+	 * Check whether the binary is whitelisted in any of the firmware

+	 * databases

+	 */

+	drain_openssl_errors();

+	efi_status = check_whitelist(sig, sha256hash, sha1hash);

+	if (EFI_ERROR(efi_status)) {

+		if (efi_status != EFI_NOT_FOUND) {

+			dprint(L"check_whitelist(): %r\n", efi_status);

+			PrintErrors();

+			ClearErrors();

+			crypterr(efi_status);

+		}

+	} else {

+		drain_openssl_errors();

+		return efi_status;

+	}

+

+	efi_status = EFI_NOT_FOUND;

+#if defined(ENABLE_SHIM_CERT)

+	/*

+	 * Check against the shim build key

+	 */

+	drain_openssl_errors();

+	if (build_cert && build_cert_size) {

+		dprint("verifying against shim cert\n");

+	}

+	if (build_cert && build_cert_size &&

+	    AuthenticodeVerify(sig->CertData,

+		       sig->Hdr.dwLength - sizeof(sig->Hdr),

+		       build_cert, build_cert_size, sha256hash,

+		       SHA256_DIGEST_SIZE)) {

+		dprint(L"AuthenticodeVerify(shim_cert) succeeded\n");

+		update_verification_method(VERIFIED_BY_CERT);

+		tpm_measure_variable(L"Shim", SHIM_LOCK_GUID,

+				     build_cert_size, build_cert);

+		efi_status = EFI_SUCCESS;

+		drain_openssl_errors();

+		return efi_status;

+	} else {

+		dprint(L"AuthenticodeVerify(shim_cert) failed\n");

+		PrintErrors();

+		ClearErrors();

+		crypterr(EFI_NOT_FOUND);

+	}

+#endif /* defined(ENABLE_SHIM_CERT) */

+

+#if defined(VENDOR_CERT_FILE)

+	/*

+	 * And finally, check against shim's built-in key

+	 */

+	drain_openssl_errors();

+	if (vendor_cert_size) {

+		dprint("verifying against vendor_cert\n");

+	}

+	if (vendor_cert_size &&

+	    AuthenticodeVerify(sig->CertData,

+			       sig->Hdr.dwLength - sizeof(sig->Hdr),

+			       vendor_cert, vendor_cert_size,

+			       sha256hash, SHA256_DIGEST_SIZE)) {

+		dprint(L"AuthenticodeVerify(vendor_cert) succeeded\n");

+		update_verification_method(VERIFIED_BY_CERT);

+		tpm_measure_variable(L"Shim", SHIM_LOCK_GUID,

+				     vendor_cert_size, vendor_cert);

+		efi_status = EFI_SUCCESS;

+		drain_openssl_errors();

+		return efi_status;

+	} else {

+		dprint(L"AuthenticodeVerify(vendor_cert) failed\n");

+		PrintErrors();

+		ClearErrors();

+		crypterr(EFI_NOT_FOUND);

+	}

+#endif /* defined(VENDOR_CERT_FILE) */

+

+	return efi_status;

+}

+

 /*

  * Check that the signature is valid and matches the binary

  */

@@ -1011,40 +1108,14 @@ static EFI_STATUS verify_buffer (char *data, int datasize,

 				 PE_COFF_LOADER_IMAGE_CONTEXT *context,

 				 UINT8 *sha256hash, UINT8 *sha1hash)

 {

-	EFI_STATUS efi_status = EFI_SECURITY_VIOLATION;

-	WIN_CERTIFICATE_EFI_PKCS *cert = NULL;

-	unsigned int size = datasize;

+	EFI_STATUS ret_efi_status;

+	size_t size = datasize;

+	size_t offset = 0;

+	unsigned int i = 0;

 	if (datasize < 0)

 		return EFI_INVALID_PARAMETER;

-	if (context->SecDir->Size != 0) {

-		if (context->SecDir->Size >= size) {

-			perror(L"Certificate Database size is too large\n");

-			return EFI_INVALID_PARAMETER;

-		}

-

-		cert = ImageAddress (data, size,

-				     context->SecDir->VirtualAddress);

-

-		if (!cert) {

-			perror(L"Certificate located outside the image\n");

-			return EFI_INVALID_PARAMETER;

-		}

-

-		if (cert->Hdr.dwLength > context->SecDir->Size) {

-			perror(L"Certificate list size is inconsistent with PE headers");

-			return EFI_INVALID_PARAMETER;

-		}

-

-		if (cert->Hdr.wCertificateType !=

-		    WIN_CERT_TYPE_PKCS_SIGNED_DATA) {

-			perror(L"Unsupported certificate type %x\n",

-				cert->Hdr.wCertificateType);

-			return EFI_UNSUPPORTED;

-		}

-	}

-

 	/*

 	 * Clear OpenSSL's error log, because we get some DSO unimplemented

 	 * errors during its intialization, and we don't want those to look

@@ -1052,81 +1123,119 @@ static EFI_STATUS verify_buffer (char *data, int datasize,

 	 */

 	drain_openssl_errors();

-	efi_status = generate_hash(data, datasize, context, sha256hash, sha1hash);

-	if (EFI_ERROR(efi_status)) {

-		LogError(L"generate_hash: %r\n", efi_status);

-		return efi_status;

+	ret_efi_status = generate_hash(data, datasize, context, sha256hash, sha1hash);

+	if (EFI_ERROR(ret_efi_status)) {

+		dprint(L"generate_hash: %r\n", ret_efi_status);

+		PrintErrors();

+		ClearErrors();

+		crypterr(ret_efi_status);

+		return ret_efi_status;

 	}

 	/*

-	 * Ensure that the binary isn't blacklisted

+	 * Ensure that the binary isn't blacklisted by hash

 	 */

-	efi_status = check_blacklist(cert, sha256hash, sha1hash);

-	if (EFI_ERROR(efi_status)) {

+	drain_openssl_errors();

+	ret_efi_status = check_blacklist(NULL, sha256hash, sha1hash);

+	if (EFI_ERROR(ret_efi_status)) {

 		perror(L"Binary is blacklisted\n");

-		LogError(L"Binary is blacklisted: %r\n", efi_status);

-		return efi_status;

+		dprint(L"Binary is blacklisted: %r\n", ret_efi_status);

+		PrintErrors();

+		ClearErrors();

+		crypterr(ret_efi_status);

+		return ret_efi_status;

 	}

 	/*

-	 * Check whether the binary is whitelisted in any of the firmware

-	 * databases

+	 * Check whether the binary is whitelisted by hash in any of the

+	 * firmware databases

 	 */

-	efi_status = check_whitelist(cert, sha256hash, sha1hash);

-	if (EFI_ERROR(efi_status)) {

-		LogError(L"check_whitelist(): %r\n", efi_status);

+	drain_openssl_errors();

+	ret_efi_status = check_whitelist(NULL, sha256hash, sha1hash);

+	if (EFI_ERROR(ret_efi_status)) {

+		dprint(L"check_whitelist: %r\n", ret_efi_status);

+		if (ret_efi_status != EFI_NOT_FOUND) {

+			PrintErrors();

+			ClearErrors();

+			crypterr(ret_efi_status);

+			return ret_efi_status;

+		}

 	} else {

 		drain_openssl_errors();

-		return efi_status;

+		return ret_efi_status;

 	}

-	if (cert) {

-#if defined(ENABLE_SHIM_CERT)

-		/*

-		 * Check against the shim build key

-		 */

-		if (sizeof(shim_cert) &&

-		    AuthenticodeVerify(cert->CertData,

-			       cert->Hdr.dwLength - sizeof(cert->Hdr),

-			       shim_cert, sizeof(shim_cert), sha256hash,

-			       SHA256_DIGEST_SIZE)) {

-			update_verification_method(VERIFIED_BY_CERT);

-			tpm_measure_variable(L"Shim", SHIM_LOCK_GUID,

-					     sizeof(shim_cert), shim_cert);

-			efi_status = EFI_SUCCESS;

-			drain_openssl_errors();

-			return efi_status;

-		} else {

-			LogError(L"AuthenticodeVerify(shim_cert) failed\n");

+	if (context->SecDir->Size == 0) {

+		dprint(L"No signatures found\n");

+		return EFI_SECURITY_VIOLATION;

+	}

+

+	if (context->SecDir->Size >= size) {

+		perror(L"Certificate Database size is too large\n");

+		return EFI_INVALID_PARAMETER;

+	}

+

+	ret_efi_status = EFI_NOT_FOUND;

+	do {

+		WIN_CERTIFICATE_EFI_PKCS *sig = NULL;

+		size_t sz;

+

+		sig = ImageAddress(data, size,

+				   context->SecDir->VirtualAddress + offset);

+		if (!sig)

+			break;

+

+		sz = offset + offsetof(WIN_CERTIFICATE_EFI_PKCS, Hdr.dwLength)

+		     + sizeof(sig->Hdr.dwLength);

+		if (sz > context->SecDir->Size) {

+			perror(L"Certificate size is too large for secruity database");

+			return EFI_INVALID_PARAMETER;

+		}

+

+		sz = sig->Hdr.dwLength;

+		if (sz > context->SecDir->Size - offset) {

+			perror(L"Certificate size is too large for secruity database");

+			return EFI_INVALID_PARAMETER;

 		}

-#endif /* defined(ENABLE_SHIM_CERT) */

-

-#if defined(VENDOR_CERT_FILE)

-		/*

-		 * And finally, check against shim's built-in key

-		 */

-		if (vendor_authorized_size &&

-		    AuthenticodeVerify(cert->CertData,

-				       cert->Hdr.dwLength - sizeof(cert->Hdr),

-				       vendor_authorized, vendor_authorized_size,

-				       sha256hash, SHA256_DIGEST_SIZE)) {

-			update_verification_method(VERIFIED_BY_CERT);

-			tpm_measure_variable(L"Shim", SHIM_LOCK_GUID,

-					     vendor_authorized_size, vendor_authorized);

-			efi_status = EFI_SUCCESS;

-			drain_openssl_errors();

-			return efi_status;

+

+		if (sz < sizeof(sig->Hdr)) {

+			perror(L"Certificate size is too small for certificate data");

+			return EFI_INVALID_PARAMETER;

+		}

+

+		if (sig->Hdr.wCertificateType == WIN_CERT_TYPE_PKCS_SIGNED_DATA) {

+			EFI_STATUS efi_status;

+

+			dprint(L"Attempting to verify signature %d:\n", i++);

+

+			efi_status = verify_one_signature(sig, sha256hash, sha1hash);

+

+			/*

+			 * If we didn't get EFI_SECURITY_VIOLATION from

+			 * checking the hashes above, then any dbx entries are

+			 * for a certificate, not this individual binary.

+			 *

+			 * So don't clobber successes with security violation

+			 * here; that just means it isn't a success.

+			 */

+			if (ret_efi_status != EFI_SUCCESS)

+				ret_efi_status = efi_status;

 		} else {

-			LogError(L"AuthenticodeVerify(vendor_authorized) failed\n");

+			perror(L"Unsupported certificate type %x\n",

+				sig->Hdr.wCertificateType);

 		}

-#endif /* defined(VENDOR_CERT_FILE) */

-	}

+		offset = ALIGN_VALUE(offset + sz, 8);

+	} while (offset < context->SecDir->Size);

-	LogError(L"Binary is not whitelisted\n");

-	crypterr(EFI_SECURITY_VIOLATION);

-	PrintErrors();

-	efi_status = EFI_SECURITY_VIOLATION;

-	return efi_status;

+	if (ret_efi_status != EFI_SUCCESS) {

+		dprint(L"Binary is not whitelisted\n");

+		PrintErrors();

+		ClearErrors();

+		crypterr(EFI_SECURITY_VIOLATION);

+		ret_efi_status = EFI_SECURITY_VIOLATION;

+	}

+	drain_openssl_errors();

+	return ret_efi_status;

 }

 /*

-- 

2.26.2