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

[Krzysztof Kozlowski]

On 10/05/2021 10:48, Dimitri John Ledkov wrote:
> On Mon, May 10, 2021 at 3:33 PM Krzysztof Kozlowski
> <krzysztof.kozlowski at canonical.com> wrote:
>>
>> 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.
>>
> 
> In my mind, it means "the function will not arbitrarily decide to
> override the caller's decision as to which certs to load". And I see
> in other places in the kernel that static functions have const bool
> arguments. E.g. see the charming functions in
> arch/x86/net/bpf_jit_comp32.c
> 
> But yeah, I can drop that, if it really is confusing.

I understand now, you gave a meaning to the "const". To me it looks
unusual and I would prefer the proper documentation, but such patter
indeed appears in few places, so I don't mind keeping it.

Best regards,
Krzysztof