[UPSTREAM][RFC PATCH] integrity: Load mokx certs from the EFI MOK config table

[Krzysztof Kozlowski]

On 10/05/2021 10:14, Dimitri John Ledkov wrote:
> Refactor load_moklist_certs() to load either MokListRT into db, or
> MokListXRT into dbx. Call load_moklist_certs() twice - first to load
> mokx certs into dbx, then mok certs into db.
> 
> This thus now attempts to load mokx certs via the EFI MOKvar config
> table first, and if that fails, via the EFI variable. Previously mokx
> certs were only loaded via the EFI variable. Which fails when
> MokListXRT is large and instead of MokListXRT is only available as
> MokListXRT{1,2,3}. This is the case with Ubuntu's 15.4 based
> shim. This patch is required to address CVE-2020-26541 when
> certificates are revoked via MokListXRT.
> 
> Fixes: ebd9c2ae369a45bdd9f8615484db09be58fc242b
> 
> Signed-off-by: Dimitri John Ledkov <dimitri.ledkov at canonical.com>
> ---
>  @Ubuntu kernel-team / security-team
> 
>  Presubmitting this patch internally, to see if it's ok to submit
>  upstream. Intention is to send this to keyrings@
>  linux-security-module@ linux-kernel@ Eric Snowberg etc that have
>  introduced the referenced feature into v5.13-rc1 which on ubuntu is
>  buggy and doesn't address the CVE in question.
> 
>  security/integrity/platform_certs/load_uefi.c | 73 ++++++++++---------
>  1 file changed, 39 insertions(+), 34 deletions(-)
> 
> diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
> index f290f78c3f301..e1f4ebb71abe0 100644
> --- a/security/integrity/platform_certs/load_uefi.c
> +++ b/security/integrity/platform_certs/load_uefi.c
> @@ -66,17 +66,18 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
>  }
>  
>  /*
> - * load_moklist_certs() - Load MokList certs
> + * load_moklist_certs() - Load Mok(X)List certs
> + * @load_db: Load MokListRT into db when true; MokListXRT into dbx when false
>   *
> - * Load the certs contained in the UEFI MokListRT database into the
> - * platform trusted keyring.
> + * Load the certs contained in the UEFI MokList(X)RT database into the
> + * platform trusted/denied keyring.
>   *
>   * This routine checks the EFI MOK config table first. If and only if
> - * that fails, this routine uses the MokListRT ordinary UEFI variable.
> + * that fails, this routine uses the MokList(X)RT ordinary UEFI variable.
>   *
>   * Return:	Status
>   */
> -static int __init load_moklist_certs(void)
> +static int __init load_moklist_certs(const bool load_db)

What's the point of having "const" value argument? It's a noop, I think,
and confusing one.

>  {
>  	struct efi_mokvar_table_entry *mokvar_entry;
>  	efi_guid_t mok_var = EFI_SHIM_LOCK_GUID;
> @@ -84,41 +85,54 @@ static int __init load_moklist_certs(void)
>  	unsigned long moksize;
>  	efi_status_t status;
>  	int rc;
> +	char *mokvar_name = "MokListRT";
> +	efi_char16_t *efivar_name = L"MokListRT";
> +	char *parse_mokvar_name = "UEFI:MokListRT (MOKvar table)";
> +	char *parse_efivar_name = "UEFI:MokListRT";

OTOH, this should be pointer to const. Here it is important because code
should not modify the contents.

Best regards,
Krzysztof