367 lines
11 KiB
Diff
367 lines
11 KiB
Diff
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
|
|
|