[PATCH 5/8] efi_loader: Make the pkcs7 header parsing function an extern

[Akashi Takahiro <takahiro.akashi@linaro.org>]

On Thu, May 07, 2020 at 04:48:30PM +0530, Sughosh Ganu wrote:
> On Thu, 7 May 2020 at 13:04, Akashi Takahiro <takahiro.akashi@linaro.org>
> wrote:
> 
> > On Thu, Apr 30, 2020 at 11:06:27PM +0530, Sughosh Ganu wrote:
> > > The pkcs7 header parsing functionality is pretty generic, and can be
> > > used by other features like capsule authentication. Make the function
> > > as an extern, also changing it's name to efi_parse_pkcs7_header.
> >
> > The patch itself is fine to me, but is "pkcs7 header" a common term?
> >
> 
> I haven't come across it in any other code base. I used it since in the
> concept of a capsule, the signature is prepended to the capsule payload. If
> you can think of a better name, please suggest so. I will change it in the
> next version.

Simply, efi_parse_pkcs7_signature()?

In addition, please update the function description, which still
mentions "variable."

-Takahiro Akashi

> -sughosh
> 
> 
> >
> > -Takahiro Akashi
> >
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > ---
> > >  include/efi_loader.h           |  2 +
> > >  lib/efi_loader/efi_signature.c | 78 ++++++++++++++++++++++++++++++++
> > >  lib/efi_loader/efi_variable.c  | 82 +---------------------------------
> > >  3 files changed, 82 insertions(+), 80 deletions(-)
> > >
> > > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > > index b7638d5825..8d923451ce 100644
> > > --- a/include/efi_loader.h
> > > +++ b/include/efi_loader.h
> > > @@ -781,6 +781,8 @@ bool efi_secure_boot_enabled(void);
> > >
> > >  bool efi_image_parse(void *efi, size_t len, struct efi_image_regions
> > **regp,
> > >                    WIN_CERTIFICATE **auth, size_t *auth_len);
> > > +struct pkcs7_message *efi_parse_pkcs7_header(const void *buf, size_t
> > buflen);
> > > +
> > >  #endif /* CONFIG_EFI_SECURE_BOOT */
> > >
> > >  #ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > > diff --git a/lib/efi_loader/efi_signature.c
> > b/lib/efi_loader/efi_signature.c
> > > index bf6f39aab2..9897f5418e 100644
> > > --- a/lib/efi_loader/efi_signature.c
> > > +++ b/lib/efi_loader/efi_signature.c
> > > @@ -25,6 +25,84 @@ const efi_guid_t efi_guid_cert_x509_sha256 =
> > EFI_CERT_X509_SHA256_GUID;
> > >
> > >  #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_parse_pkcs7_header - parse a signature in variable
> > > + * @buf:     Pointer to the payload's value
> > > + * @buflen:  Length of @buf
> > > + *
> > > + * 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.
> > > + *
> > > + * Return:   Pointer to pkcs7_message structure on success, NULL on
> > error
> > > + */
> > > +struct pkcs7_message *efi_parse_pkcs7_header(const void *buf, size_t
> > buflen)
> > > +{
> > > +     u8 *ebuf;
> > > +     size_t ebuflen, len;
> > > +     struct pkcs7_message *msg;
> > > +
> > > +     /*
> > > +      * This is the best assumption to check if the binary is
> > > +      * already in a form of pkcs7's signedData.
> > > +      */
> > > +     if (buflen > sizeof(pkcs7_hdr) &&
> > > +         !memcmp(&((u8 *)buf)[4], &pkcs7_hdr[4], 11)) {
> > > +             msg = pkcs7_parse_message(buf, buflen);
> > > +             goto out;
> > > +     }
> > > +
> > > +     /*
> > > +      * Otherwise, we should add a dummy prefix sequence for pkcs7
> > > +      * message parser to be able to process.
> > > +      * NOTE: EDK2 also uses similar hack in WrapPkcs7Data()
> > > +      * in CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c
> > > +      * TODO:
> > > +      * The header should be composed in a more refined manner.
> > > +      */
> > > +     debug("Makeshift prefix added to authentication data\n");
> > > +     ebuflen = sizeof(pkcs7_hdr) + buflen;
> > > +     if (ebuflen <= 0x7f) {
> > > +             debug("Data is too short\n");
> > > +             return NULL;
> > > +     }
> > > +
> > > +     ebuf = malloc(ebuflen);
> > > +     if (!ebuf) {
> > > +             debug("Out of memory\n");
> > > +             return NULL;
> > > +     }
> > > +
> > > +     memcpy(ebuf, pkcs7_hdr, sizeof(pkcs7_hdr));
> > > +     memcpy(ebuf + sizeof(pkcs7_hdr), buf, buflen);
> > > +     len = ebuflen - 4;
> > > +     ebuf[2] = (len >> 8) & 0xff;
> > > +     ebuf[3] = len & 0xff;
> > > +     len = ebuflen - 0x13;
> > > +     ebuf[0x11] = (len >> 8) & 0xff;
> > > +     ebuf[0x12] = len & 0xff;
> > > +
> > > +     msg = pkcs7_parse_message(ebuf, ebuflen);
> > > +
> > > +     free(ebuf);
> > > +
> > > +out:
> > > +     if (IS_ERR(msg))
> > > +             return NULL;
> > > +
> > > +     return msg;
> > > +}
> > > +
> > >  /**
> > >   * efi_hash_regions - calculate a hash value
> > >   * @regs:    List of regions
> > > diff --git a/lib/efi_loader/efi_variable.c
> > b/lib/efi_loader/efi_variable.c
> > > index 6c2dd82306..be34a2cadd 100644
> > > --- a/lib/efi_loader/efi_variable.c
> > > +++ b/lib/efi_loader/efi_variable.c
> > > @@ -415,85 +415,7 @@ 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
> > > - *
> > > - * 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.
> > > - *
> > > - * Return:   Pointer to pkcs7_message structure on success, NULL on
> > error
> > > - */
> > > -static struct pkcs7_message *efi_variable_parse_signature(const void
> > *buf,
> > > -                                                       size_t buflen)
> > > -{
> > > -     u8 *ebuf;
> > > -     size_t ebuflen, len;
> > > -     struct pkcs7_message *msg;
> > > -
> > > -     /*
> > > -      * This is the best assumption to check if the binary is
> > > -      * already in a form of pkcs7's signedData.
> > > -      */
> > > -     if (buflen > sizeof(pkcs7_hdr) &&
> > > -         !memcmp(&((u8 *)buf)[4], &pkcs7_hdr[4], 11)) {
> > > -             msg = pkcs7_parse_message(buf, buflen);
> > > -             goto out;
> > > -     }
> > > -
> > > -     /*
> > > -      * Otherwise, we should add a dummy prefix sequence for pkcs7
> > > -      * message parser to be able to process.
> > > -      * NOTE: EDK2 also uses similar hack in WrapPkcs7Data()
> > > -      * in CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c
> > > -      * TODO:
> > > -      * The header should be composed in a more refined manner.
> > > -      */
> > > -     debug("Makeshift prefix added to authentication data\n");
> > > -     ebuflen = sizeof(pkcs7_hdr) + buflen;
> > > -     if (ebuflen <= 0x7f) {
> > > -             debug("Data is too short\n");
> > > -             return NULL;
> > > -     }
> > > -
> > > -     ebuf = malloc(ebuflen);
> > > -     if (!ebuf) {
> > > -             debug("Out of memory\n");
> > > -             return NULL;
> > > -     }
> > > -
> > > -     memcpy(ebuf, pkcs7_hdr, sizeof(pkcs7_hdr));
> > > -     memcpy(ebuf + sizeof(pkcs7_hdr), buf, buflen);
> > > -     len = ebuflen - 4;
> > > -     ebuf[2] = (len >> 8) & 0xff;
> > > -     ebuf[3] = len & 0xff;
> > > -     len = ebuflen - 0x13;
> > > -     ebuf[0x11] = (len >> 8) & 0xff;
> > > -     ebuf[0x12] = len & 0xff;
> > > -
> > > -     msg = pkcs7_parse_message(ebuf, ebuflen);
> > > -
> > > -     free(ebuf);
> > > -
> > > -out:
> > > -     if (IS_ERR(msg))
> > > -             return NULL;
> > > -
> > > -     return msg;
> > > -}
> > > +#ifdef CONFIG_SECURE_BOOT
> > >
> > >  /**
> > >   * efi_variable_authenticate - authenticate a variable
> > > @@ -591,7 +513,7 @@ static efi_status_t efi_variable_authenticate(u16
> > *variable,
> > >       /* variable's signature list */
> > >       if (auth->auth_info.hdr.dwLength < sizeof(auth->auth_info))
> > >               goto err;
> > > -     var_sig = efi_variable_parse_signature(auth->auth_info.cert_data,
> > > +     var_sig = efi_parse_pkcs7_header(auth->auth_info.cert_data,
> > >                                              auth->auth_info.hdr.dwLength
> > >                                                  -
> > sizeof(auth->auth_info));
> > >       if (IS_ERR(var_sig)) {
> > > --
> > > 2.17.1
> > >
> >