[PATCH v5 11/23] mkeficapsule: Add support for generating empty capsules

Sughosh Ganu sughosh.ganu at linaro.org
Mon Jun 13 14:33:38 CEST 2022


On Thu, 9 Jun 2022 at 21:58, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 6/9/22 14:29, Sughosh Ganu wrote:
> > The Dependable Boot specification[1] describes the structure of the
> > firmware accept and revert capsules. These are empty capsules which
> > are used for signalling the acceptance or rejection of the updated
> > firmware by the OS. Add support for generating these empty capsules.
> >
> > [1] - https://git.codelinaro.org/linaro/dependable-boot/mbfw/uploads/6f7ddfe3be24e18d4319e108a758d02e/mbfw.pdf
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > ---
> >   doc/mkeficapsule.1   |  29 ++++++---
> >   tools/eficapsule.h   |   8 +++
> >   tools/mkeficapsule.c | 139 +++++++++++++++++++++++++++++++++++++------
> >   3 files changed, 151 insertions(+), 25 deletions(-)

<snip>

> > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> > index 5f74d23b9e..e8eb6b070d 100644
> > --- a/tools/mkeficapsule.c
> > +++ b/tools/mkeficapsule.c
> > @@ -29,7 +29,16 @@ static const char *tool_name = "mkeficapsule";
> >   efi_guid_t efi_guid_fm_capsule = EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> >   efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
> >
> > -static const char *opts_short = "g:i:I:v:p:c:m:dh";
> > +static const char *opts_short = "g:i:I:v:p:c:m:dhAR";
> > +
> > +static bool empty_capsule;
> > +static unsigned char capsule;
> > +
> > +enum {
> > +     CAPSULE_NORMAL_BLOB = 0,
> > +     CAPSULE_ACCEPT,
> > +     CAPSULE_REVERT,
> > +} capsule_type;
> >
> >   static struct option options[] = {
> >       {"guid", required_argument, NULL, 'g'},
> > @@ -39,24 +48,47 @@ static struct option options[] = {
> >       {"certificate", required_argument, NULL, 'c'},
> >       {"monotonic-count", required_argument, NULL, 'm'},
> >       {"dump-sig", no_argument, NULL, 'd'},
> > +     {"fw-accept", no_argument, NULL, 'A'},
> > +     {"fw-revert", no_argument, NULL, 'R'},
> >       {"help", no_argument, NULL, 'h'},
> >       {NULL, 0, NULL, 0},
> >   };
> >
> >   static void print_usage(void)
> >   {
> > -     fprintf(stderr, "Usage: %s [options] <image blob> <output file>\n"
> > -             "Options:\n"
> > -
> > -             "\t-g, --guid <guid string>    guid for image blob type\n"
> > -             "\t-i, --index <index>         update image index\n"
> > -             "\t-I, --instance <instance>   update hardware instance\n"
> > -             "\t-p, --private-key <privkey file>  private key file\n"
> > -             "\t-c, --certificate <cert file>     signer's certificate file\n"
> > -             "\t-m, --monotonic-count <count>     monotonic count\n"
> > -             "\t-d, --dump_sig              dump signature (*.p7)\n"
> > -             "\t-h, --help                  print a help message\n",
> > -             tool_name);
> > +     if (empty_capsule) {
> > +             if (capsule == CAPSULE_ACCEPT) {
> > +                     fprintf(stderr, "Usage: %s [options] <output file>\n",
>
> My expectation is that this function always provides the same output.
>
> If different scenarios allow only specific combinations of arguments you
> may describe it here.

Okay

>
>
> > +                             tool_name);
> > +                     fprintf(stderr, "Options:\n"
> > +                             "\t-A, --fw-accept             firmware accept capsule\n"
> > +                             "\t-g, --guid <guid string>    guid for image blob type\n"
> > +                             "\t-h, --help                  print a help message\n"
> > +                             );
> > +             } else {
> > +                     fprintf(stderr, "Usage: %s [options] <output file>\n",
> > +                             tool_name);
> > +                     fprintf(stderr, "Options:\n"
> > +                             "\t-R, --fw-revert             firmware revert capsule\n"
> > +                             "\t-h, --help                  print a help message\n"
> > +                             );
> > +             }
> > +     } else {
> > +             fprintf(stderr, "Usage: %s [options] <image blob> <output file>\n"
> > +                     "Options:\n"
> > +
> > +                     "\t-g, --guid <guid string>    guid for image blob type\n"
> > +                     "\t-i, --index <index>         update image index\n"
> > +                     "\t-I, --instance <instance>   update hardware instance\n"
> > +                     "\t-p, --private-key <privkey file>  private key file\n"
> > +                     "\t-c, --certificate <cert file>     signer's certificate file\n"
> > +                     "\t-m, --monotonic-count <count>     monotonic count\n"
> > +                     "\t-d, --dump_sig              dump signature (*.p7)\n"
> > +                     "\t-A, --fw-accept             firmware accept capsule\n"
>
> "\t-A, --fw-accept             firmware accept capsule, requires GUID\n"
>
> > +                     "\t-R, --fw-revert             firmware revert capsule\n"
>
> "\t-R, --fw-revert             firmware revert capsule, takes no GUID\n"
>
> > +                     "\t-h, --help                  print a help message\n",
> > +                     tool_name);
> > +     }
> >   }
> >
> >   /**
> > @@ -564,6 +596,50 @@ void convert_uuid_to_guid(unsigned char *buf)
> >       buf[7] = c;
> >   }
> >
> > +static int create_empty_capsule(char *path, efi_guid_t *guid, bool fw_accept)
> > +{
> > +     struct efi_capsule_header header;
> > +     FILE *f = NULL;
> > +     int ret = -1;
> > +     efi_guid_t fw_accept_guid = FW_ACCEPT_OS_GUID;
> > +     efi_guid_t fw_revert_guid = FW_REVERT_OS_GUID;
> > +     efi_guid_t payload, capsule_guid;
> > +
> > +     f = fopen(path, "w");
> > +     if (!f) {
> > +             fprintf(stderr, "cannot open %s\n", path);
> > +             goto err;
> > +     }
> > +
> > +     capsule_guid = fw_accept ? fw_accept_guid : fw_revert_guid;
> > +
> > +     memcpy(&header.capsule_guid, &capsule_guid, sizeof(efi_guid_t));
> > +     header.header_size = sizeof(header);
> > +     header.flags = 0;
> > +
> > +     header.capsule_image_size = fw_accept ?
> > +             sizeof(header) + sizeof(efi_guid_t) : sizeof(header);
> > +
> > +     if (write_capsule_file(f, &header, sizeof(header),
> > +                            "Capsule header"))
> > +             goto err;
> > +
> > +     if (fw_accept) {
> > +             memcpy(&payload, guid, sizeof(efi_guid_t));
> > +             if (write_capsule_file(f, &payload, sizeof(payload),
> > +                                    "FW Accept Capsule Payload"))
> > +                     goto err;
> > +     }
> > +
> > +     ret = 0;
> > +
> > +err:
> > +     if (f)
> > +             fclose(f);
> > +
> > +     return ret;
> > +}
> > +
> >   /**
> >    * main - main entry function of mkeficapsule
> >    * @argc:   Number of arguments
> > @@ -639,22 +715,49 @@ int main(int argc, char **argv)
>
> >               case 'd':
> >                       dump_sig = 1;
> >                       break;
> > +             case 'A':
> > +                     capsule |= CAPSULE_ACCEPT;
> > +                     break;
> > +             case 'R':
> > +                     capsule |= CAPSULE_REVERT;
> > +                     break;
> >               case 'h':
>
> You must handle '?' and ':' here too. Just use
>
> default:

Okay

>
>
> >                       print_usage();
> >                       exit(EXIT_SUCCESS);
> >               }
> >       }
> >
> > +     if (capsule == (CAPSULE_ACCEPT | CAPSULE_REVERT)) {
> > +             fprintf(stderr,
> > +                     "Select either of Accept or Revert capsule generation\n");
> > +             exit(EXIT_FAILURE);
> > +     }
> > +
> > +     empty_capsule = (capsule == CAPSULE_ACCEPT ||
> > +                      capsule == CAPSULE_REVERT);
> > +
> >       /* check necessary parameters */
> > -     if ((argc != optind + 2) || !guid ||
> > -         ((privkey_file && !cert_file) ||
> > -          (!privkey_file && cert_file))) {
> > +     if ((!empty_capsule &&
> > +         ((argc != optind + 2) || !guid ||
> > +          ((privkey_file && !cert_file) ||
> > +           (!privkey_file && cert_file)))) ||
> > +         (empty_capsule &&
> > +         ((argc != optind + 1) ||
> > +          ((capsule == CAPSULE_ACCEPT) && !guid) ||
> > +          ((capsule == CAPSULE_REVERT) && guid)))) {
>
> Please, write a message explaining to the user what was wrong, e.g.
>
> "Option -A does not take a GUID" or
> "Option -R requires a GUID"

We are calling the print_usage function where I am adding the
additional information that you suggested above. I think that would
suffice. Else we are basically repeating the same information twice.

>
>
> >               print_usage();
> >               exit(EXIT_FAILURE);
> >       }
> >
> > -     if (create_fwbin(argv[argc - 1], argv[argc - 2], guid, index, instance,
> > -                      mcount, privkey_file, cert_file) < 0) {
> > +     if (empty_capsule) {
> > +             if (create_empty_capsule(argv[argc - 1], guid,
> > +                                      capsule == CAPSULE_ACCEPT) < 0) {
> > +                     fprintf(stderr, "Creating empty capsule failed\n");
>
> The called function should provide a message that describes what went wrong.

The create_empty_capsule function calls the write_capsule_file
function for creation of the capsule file, and passes the string that
is to be printed in case of an error condition.

-sughosh

>
> Best regards
>
> Heinrich
>
>
> > +                     exit(EXIT_FAILURE);
> > +             }
> > +     } else  if (create_fwbin(argv[argc - 1], argv[argc - 2], guid,
> > +                              index, instance, mcount, privkey_file,
> > +                              cert_file) < 0) {
> >               fprintf(stderr, "Creating firmware capsule failed\n");
> >               exit(EXIT_FAILURE);
> >       }
>


More information about the U-Boot mailing list