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

[Sughosh Ganu]

hello Heinrich,

On Tue, 21 Apr 2020 at 01:00, Heinrich Schuchardt <xypron.glpk at gmx.de>
wrote:

> 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.
>

Will do, as part of my capsule authentication changes. Thanks.

-sughosh