Re: [PATCH 4/4] integrity: Load mokx variables into the blacklist keyring

On 12/03/2021 21:49, Eric Snowberg wrote:

quoted

On Mar 12, 2021, at 11:39 AM, Dimitri John Ledkov [off-list ref] wrote:

On 25/02/2021 20:59, David Howells wrote:

quoted

From: Eric Snowberg <eric.snowberg@oracle.com>

During boot the Secure Boot Forbidden Signature Database, dbx,
is loaded into the blacklist keyring.  Systems booted with shim
have an equivalent Forbidden Signature Database called mokx.
Currently mokx is only used by shim and grub, the contents are
ignored by the kernel.

Add the ability to load mokx into the blacklist keyring during boot.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jarkko Sakkinen <jarkko@kernel.org>
Link: https://lore.kernel.org/r/20210122181054.32635-5-eric.snowberg@oracle.com/ (local) # v5
Link: https://lore.kernel.org/r/c33c8e3839a41e9654f41cc92c7231104931b1d7.camel@HansenPartnership.com/ (local)
---

security/integrity/platform_certs/load_uefi.c |   20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index ee4b4c666854..f290f78c3f30 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c

@@ -132,8 +132,9 @@ static int __init load_moklist_certs(void)

static int __init load_uefi_certs(void)
{
	efi_guid_t secure_var = EFI_IMAGE_SECURITY_DATABASE_GUID;
-	void *db = NULL, *dbx = NULL;
-	unsigned long dbsize = 0, dbxsize = 0;
+	efi_guid_t mok_var = EFI_SHIM_LOCK_GUID;
+	void *db = NULL, *dbx = NULL, *mokx = NULL;
+	unsigned long dbsize = 0, dbxsize = 0, mokxsize = 0;
	efi_status_t status;
	int rc = 0;

@@ -175,6 +176,21 @@ static int __init load_uefi_certs(void)

		kfree(dbx);
	}

+	mokx = get_cert_list(L"MokListXRT", &mok_var, &mokxsize, &status);
+	if (!mokx) {
+		if (status == EFI_NOT_FOUND)
+			pr_debug("mokx variable wasn't found\n");
+		else
+			pr_info("Couldn't get mokx list\n");
+	} else {
+		rc = parse_efi_signature_list("UEFI:MokListXRT",
+					      mokx, mokxsize,
+					      get_handler_for_dbx);
+		if (rc)
+			pr_err("Couldn't parse mokx signatures %d\n", rc);
+		kfree(mokx);
+	}
+

My preference would be if the above hunk was moved into the
load_moklist_certs() function which is called just below. Such that
loading of MokListRT & MOkListXRT are done next to each other.

And also implement loading the same way it is done for MokListRT -
specifically via the EFI MOKvar config table & then via a variable.

See 726bd8965a5f112d9601f7ce68effa1e46e02bf2 otherwise large MokListXRT
will fail to parse.

Is this support available from shim now?  Previously I thought only
MOK could be loaded from the config table, not MOKx.

It is about to become available across all distributions with the next
shim as everyone is about to ship SBAT capable shims.

From my system with the next shim & 5.10 kernel I have:

$ ls /sys/firmware/efi/mok-variables/
MokIgnoreDB  MokListRT  MokListXRT  MokSBStateRT  SbatRT

It's not just a single Mok variable, but _all_ mok variables are
available from the mok-table that are used to determine mok state.
Including whether or not db should be ignored, whether or not signature
verification is turned off, and what are the SBAT generation revocations
are, in addition to MokListRT & MokListXRT.

For example, kernel could gain further functionality to honor the user
choices and disable loading db controlled by MokIgnoreDB especially
since shim chooses to not consider db certificates & hashes as trust-worthy.

Regards,

Dimitri.