[PATCH v7 04/17] efi_loader: variable: support variable authentication

[Heinrich Schuchardt]

On 4/20/20 9:22 PM, Sughosh Ganu wrote:
>
> On Tue, 14 Apr 2020 at 08:23, AKASHI Takahiro
> <takahiro.akashi at linaro.org <mailto:takahiro.akashi at linaro.org>> wrote:
>
>     With this commit, EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS
>     is supported for authenticated variables and the system secure state
>     will transfer between setup mode and user mode as UEFI specification
>     section 32.3 describes.
>
>     Internally, authentication data is stored as part of authenticated
>     variable's value. It is nothing but a pkcs7 message (but we need some
>     wrapper, see efi_variable_parse_signature()) and will be validated by
>     efi_variable_authenticate(), hence efi_signature_verify_with_db().
>
>     Associated time value will be encoded in "{...,time=...}" along with
>     other UEFI variable's attributes.
>
>     Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org
>     <mailto:takahiro.akashi at linaro.org>>
>     ---
>      include/efi_loader.h          |   3 +
>      lib/efi_loader/efi_variable.c | 666 ++++++++++++++++++++++++++++------
>      2 files changed, 565 insertions(+), 104 deletions(-) 
>
>
> <snip>
>  
>
>     diff --git a/lib/efi_loader/efi_variable.c
>     b/lib/efi_loader/efi_variable.c
>     index fe2f26459136..adb78470f2d6 100644
>     --- a/lib/efi_loader/efi_variable.c
>     +++ b/lib/efi_loader/efi_variable.c
>     @@ -10,8 +10,14 @@
>      #include <env_internal.h>
>      #include <hexdump.h>
>      #include <malloc.h>
>     +#include <rtc.h>
>      #include <search.h>
>     +#include <linux/compat.h>
>      #include <u-boot/crc.h>
>     +#include "../lib/crypto/pkcs7_parser.h"
>     +
>     +const efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
>     +static bool efi_secure_boot;
>
>      #define READ_ONLY BIT(31)
>
>     @@ -106,9 +112,10 @@ static const char *prefix(const char *str,
>     const char *prefix)
>       *
>       * @str:       value of U-Boot variable
>       * @attrp:     pointer to UEFI attributes
>     + * @timep:     pointer to time attribute
>       * Return:     pointer to remainder of U-Boot variable value
>       */
>     -static const char *parse_attr(const char *str, u32 *attrp)
>     +static const char *parse_attr(const char *str, u32 *attrp, u64 *timep)
>      {
>             u32 attr = 0;
>             char sep = '{';
>     @@ -131,6 +138,12 @@ static const char *parse_attr(const char *str,
>     u32 *attrp)
>                             attr |= EFI_VARIABLE_BOOTSERVICE_ACCESS;
>                     } else if ((s = prefix(str, "run"))) {
>                             attr |= EFI_VARIABLE_RUNTIME_ACCESS;
>     +               } else if ((s = prefix(str, "time="))) {
>     +                       attr |=
>     EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
>     +                       hex2bin((u8 *)timep, s, sizeof(*timep));
>     +                       s += sizeof(*timep) * 2;
>     +               } else if (*str == '}') {
>     +                       break;
>                     } else {
>                             printf("invalid attribute: %s\n", str);
>                             break;
>     @@ -148,48 +161,291 @@ static const char *parse_attr(const char
>     *str, u32 *attrp)
>      }
>
>      /**
>     - * efi_get_variable() - retrieve value of a UEFI variable
>     + * efi_secure_boot_enabled - return if secure boot is enabled or not
>       *
>     - * This function implements the GetVariable runtime service.
>     + * Return:     true if enabled, false if disabled
>     + */
>     +bool efi_secure_boot_enabled(void)
>     +{
>     +       return efi_secure_boot;
>     +}
>     +
>     +#ifdef CONFIG_EFI_SECURE_BOOT
>     +static u8 pkcs7_hdr[] = {
>     +       /* SEQUENCE */
>     +       0x30, 0x82, 0x05, 0xc7,
>     +       /* OID: pkcs7-signedData */
>     +       0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07,
>     0x02,
>     +       /* Context Structured? */
>     +       0xa0, 0x82, 0x05, 0xb8,
>     +};
>     +
>     +/**
>     + * efi_variable_parse_signature - parse a signature in variable
>     + * @buf:       Pointer to variable's value
>     + * @buflen:    Length of @buf
>       *
>     - * See the Unified Extensible Firmware Interface (UEFI)
>     specification for
>     - * details.
>     + * Parse a signature embedded in variable's value and instantiate
>     + * a pkcs7_message structure. Since pkcs7_parse_message() accepts only
>     + * pkcs7's signedData, some header needed be prepended for correctly
>     + * parsing authentication data, particularly for variable's.
>       *
>     - * @variable_name:     name of the variable
>     - * @vendor:            vendor GUID
>     - * @attributes:                attributes of the variable
>     - * @data_size:         size of the buffer to which the variable
>     value is copied
>     - * @data:              buffer to which the variable value is copied
>     - * Return:             status code
>     + * Return:     Pointer to pkcs7_message structure on success, NULL
>     on error
>       */
>     -efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
>     -                                    const efi_guid_t *vendor, u32
>     *attributes,
>     -                                    efi_uintn_t *data_size, void *data)
>     +static struct pkcs7_message *efi_variable_parse_signature(const
>     void *buf,
>     +                                                         size_t buflen)
>
>
> This is a generic function used for parsing the pkcs7 header. This will
> also be used for capsule authentication. Can you move this under
> efi_signature.c as an api, and change the name to something like
> efi_parse_pkcs7_header.
>
> -sughosh

Hello Sughosh,

the patch is already merged. Could you, please, provide a follow-up
patch with the suggested change.

Best regards

Heinrich