diff mbox series

[v10,6/9] tools: mkeficapsule: allow for specifying GUID explicitly

Message ID 20220201012740.63070-7-takahiro.akashi@linaro.org
State Superseded
Headers show
Series efi_loader: capsule: improve capsule authentication support | expand

Commit Message

AKASHI Takahiro Feb. 1, 2022, 1:27 a.m. UTC
The existing options, "--fit" and "--raw," are only used to put a proper
GUID in a capsule header, where GUID identifies a particular FMP (Firmware
Management Protocol) driver which then would handle the firmware binary in
a capsule. In fact, mkeficapsule does the exact same job in creating
a capsule file whatever the firmware binary type is.

To prepare for the future extension, the command syntax will be a bit
modified to allow users to specify arbitrary GUID for their own FMP driver.
OLD:
   [--fit <image> | --raw <image>] <capsule file>
NEW:
   [--fit | --raw | --guid <guid-string>] <image> <capsule file>

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 doc/develop/uefi/uefi.rst |  4 +-
 doc/mkeficapsule.1        | 26 ++++++++----
 tools/Makefile            |  2 +-
 tools/mkeficapsule.c      | 85 ++++++++++++++++++++++++++++-----------
 4 files changed, 84 insertions(+), 33 deletions(-)

Comments

Heinrich Schuchardt Feb. 5, 2022, 7:32 p.m. UTC | #1
On 2/1/22 02:27, AKASHI Takahiro wrote:
> The existing options, "--fit" and "--raw," are only used to put a proper
> GUID in a capsule header, where GUID identifies a particular FMP (Firmware
> Management Protocol) driver which then would handle the firmware binary in
> a capsule. In fact, mkeficapsule does the exact same job in creating
> a capsule file whatever the firmware binary type is.
>
> To prepare for the future extension, the command syntax will be a bit
> modified to allow users to specify arbitrary GUID for their own FMP driver.
> OLD:
>     [--fit <image> | --raw <image>] <capsule file>
> NEW:
>     [--fit | --raw | --guid <guid-string>] <image> <capsule file>u
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>   doc/develop/uefi/uefi.rst |  4 +-
>   doc/mkeficapsule.1        | 26 ++++++++----
>   tools/Makefile            |  2 +-
>   tools/mkeficapsule.c      | 85 ++++++++++++++++++++++++++++-----------
>   4 files changed, 84 insertions(+), 33 deletions(-)
>
> diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> index 7e1eb8256259..a1a2afd60bbc 100644
> --- a/doc/develop/uefi/uefi.rst
> +++ b/doc/develop/uefi/uefi.rst
> @@ -375,8 +375,8 @@ and used by the steps highlighted below.
>         --private-key CRT.key \
>         --certificate CRT.crt \
>         --index 1 --instance 0 \
> -      [--fit <FIT image> | --raw <raw image>] \
> -      <capsule_file_name>
> +      [--fit | --raw | --guid <guid-string] \
> +      <image_blob> <capsule_file_name>
>
>   4. Insert the signature list into a device tree in the following format::
>
> diff --git a/doc/mkeficapsule.1 b/doc/mkeficapsule.1
> index 680362f5c4e9..8babb27ee8b2 100644
> --- a/doc/mkeficapsule.1
> +++ b/doc/mkeficapsule.1
> @@ -8,7 +8,7 @@ mkeficapsule \- Generate EFI capsule file for U-Boot
>
>   .SH SYNOPSIS
>   .B mkeficapsule
> -.RI [ options "] " capsule-file
> +.RI [ options "] " image-blob " " capsule-file
>
>   .SH "DESCRIPTION"
>   .B mkeficapsule
> @@ -24,7 +24,7 @@ In this case, the update will be authenticated by verifying the signature
>   before applying.
>
>   .B mkeficapsule
> -supports two different format of image files:
> +takes any type of image files, including:
>   .TP
>   .I raw image
>   format is a single binary blob of any type of firmware.
> @@ -36,18 +36,30 @@ multiple binary blobs in a single capsule file.
>   This type of image file can be generated by
>   .BR mkimage .
>
> +.PP
> +If you want to use other types than above two, you should explicitly
> +specify a guid for the FMP driver.
> +
>   .SH "OPTIONS"
>   One of
> -.BR --fit " or " --raw
> +.BR --fit ", " --raw " or " --guid
>   option must be specified.
>
>   .TP
> -.BI "-f\fR,\fB --fit " fit-image-file
> -Specify a FIT image file
> +.BR -f ", " --fit
> +Indicate that the blob is a FIT image file
>
>   .TP
> -.BI "-r\fR,\fB --raw " raw-image-file
> -Specify a raw image file
> +.BR -r ", " --raw
> +Indicate that the blob is a raw image file
> +
> +.TP
> +.BI "-g\fR,\fB --guid " guid-string
> +Specify guid for image blob type. The format is:
> +    xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
> +
> +The first three elements are in little endian, while the rest
> +is in big endian.
>
>   .TP
>   .BI "-i\fR,\fB --index " index
> diff --git a/tools/Makefile b/tools/Makefile
> index 8da07d60a755..5409ff2879c6 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -238,7 +238,7 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs
>   hostprogs-$(CONFIG_ASN1_COMPILER)	+= asn1_compiler
>   HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include
>
> -HOSTLDLIBS_mkeficapsule += -lgnutls
> +HOSTLDLIBS_mkeficapsule += -lgnutls -luuid
>   hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
>
>   # We build some files with extra pedantic flags to try to minimize things
> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> index b996c66ad26a..7ff1f999db85 100644
> --- a/tools/mkeficapsule.c
> +++ b/tools/mkeficapsule.c
> @@ -15,7 +15,7 @@
>
>   #include <sys/stat.h>
>   #include <sys/types.h>
> -
> +#include <uuid/uuid.h>

https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/3589/logs/128:

2022-02-05T16:53:55.9574749Z   HOSTCC  tools/mkeficapsule
2022-02-05T16:53:56.1341775Z tools/mkeficapsule.c:18:10: fatal error:
uuid/uuid.h: No such file or directory
2022-02-05T16:53:56.1343964Z    18 | #include <uuid/uuid.h>
2022-02-05T16:53:56.1344702Z       |          ^~~~~~~~~~~~~
2022-02-05T16:53:56.1347501Z compilation terminated.
2022-02-05T16:53:56.1392680Z make[2]: *** [scripts/Makefile.host:95:
tools/mkeficapsule] Error 1

Instead of adding a dependency on uuid-dev (linux-utils) it might be
better to write your own uuid_parse() function.

https://u-boot.readthedocs.io/en/latest/develop/ci_testing.html
describes how to trigger the Azure pipelines using a pull request on Github.

Please, trigger the test before resubmitting.

Best regards

Heinrich

>   #include <linux/kconfig.h>
>
>   #include <gnutls/gnutls.h>
> @@ -33,11 +33,12 @@ efi_guid_t efi_guid_image_type_uboot_raw =
>   		EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;
>   efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
>
> -static const char *opts_short = "f:r:i:I:v:p:c:m:dh";
> +static const char *opts_short = "frg:i:I:v:p:c:m:dh";
>
>   static struct option options[] = {
> -	{"fit", required_argument, NULL, 'f'},
> -	{"raw", required_argument, NULL, 'r'},
> +	{"fit", no_argument, NULL, 'f'},
> +	{"raw", no_argument, NULL, 'r'},
> +	{"guid", required_argument, NULL, 'g'},
>   	{"index", required_argument, NULL, 'i'},
>   	{"instance", required_argument, NULL, 'I'},
>   	{"private-key", required_argument, NULL, 'p'},
> @@ -50,11 +51,12 @@ static struct option options[] = {
>
>   static void print_usage(void)
>   {
> -	printf("Usage: %s [options] <output file>\n"
> +	fprintf(stderr, "Usage: %s [options] <image blob> <output file>\n"
>   		"Options:\n"
>
> -		"\t-f, --fit <fit image>       new FIT image file\n"
> -		"\t-r, --raw <raw image>       new raw image file\n"
> +		"\t-f, --fit                   FIT image type\n"
> +		"\t-r, --raw                   raw image type\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"
> @@ -541,6 +543,37 @@ err:
>   	return ret;
>   }
>
> +/**
> + * convert_uuid_to_guid() - convert UUID to GUID
> + * @buf:	UUID binary
> + *
> + * UUID and GUID have the same data structure, but their binary
> + * formats are different due to the endianness. See lib/uuid.c.
> + * Since uuid_parse() can handle only UUID, this function must
> + * be called to get correct data for GUID when parsing a string.
> + *
> + * The correct data will be returned in @buf.
> + */
> +void convert_uuid_to_guid(unsigned char *buf)
> +{
> +	unsigned char c;
> +
> +	c = buf[0];
> +	buf[0] = buf[3];
> +	buf[3] = c;
> +	c = buf[1];
> +	buf[1] = buf[2];
> +	buf[2] = c;
> +
> +	c = buf[4];
> +	buf[4] = buf[5];
> +	buf[5] = c;
> +
> +	c = buf[6];
> +	buf[6] = buf[7];
> +	buf[7] = c;
> +}
> +
>   /**
>    * main - main entry function of mkeficapsule
>    * @argc:	Number of arguments
> @@ -555,14 +588,13 @@ err:
>    */
>   int main(int argc, char **argv)
>   {
> -	char *file;
>   	efi_guid_t *guid;
> +	unsigned char uuid_buf[16];
>   	unsigned long index, instance;
>   	uint64_t mcount;
>   	char *privkey_file, *cert_file;
>   	int c, idx;
>
> -	file = NULL;
>   	guid = NULL;
>   	index = 0;
>   	instance = 0;
> @@ -577,21 +609,34 @@ int main(int argc, char **argv)
>
>   		switch (c) {
>   		case 'f':
> -			if (file) {
> -				fprintf(stderr, "Image already specified\n");
> +			if (guid) {
> +				fprintf(stderr,
> +					"Image type already specified\n");
>   				exit(EXIT_FAILURE);
>   			}
> -			file = optarg;
>   			guid = &efi_guid_image_type_uboot_fit;
>   			break;
>   		case 'r':
> -			if (file) {
> -				fprintf(stderr, "Image already specified\n");
> +			if (guid) {
> +				fprintf(stderr,
> +					"Image type already specified\n");
>   				exit(EXIT_FAILURE);
>   			}
> -			file = optarg;
>   			guid = &efi_guid_image_type_uboot_raw;
>   			break;
> +		case 'g':
> +			if (guid) {
> +				fprintf(stderr,
> +					"Image type already specified\n");
> +				exit(EXIT_FAILURE);
> +			}
> +			if (uuid_parse(optarg, uuid_buf)) {
> +				fprintf(stderr, "Wrong guid format\n");
> +				exit(EXIT_FAILURE);
> +			}
> +			convert_uuid_to_guid(uuid_buf);
> +			guid = (efi_guid_t *)uuid_buf;
> +			break;
>   		case 'i':
>   			index = strtoul(optarg, NULL, 0);
>   			break;
> @@ -627,20 +672,14 @@ int main(int argc, char **argv)
>   	}
>
>   	/* check necessary parameters */
> -	if ((argc != optind + 1) || !file ||
> +	if ((argc != optind + 2) || !guid ||
>   	    ((privkey_file && !cert_file) ||
>   	     (!privkey_file && cert_file))) {
>   		print_usage();
>   		exit(EXIT_FAILURE);
>   	}
>
> -	/* need a fit image file or raw image file */
> -	if (!file) {
> -		print_usage();
> -		exit(EXIT_SUCCESS);
> -	}
> -
> -	if (create_fwbin(argv[optind], file, guid, index, instance,
> +	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);
AKASHI Takahiro Feb. 7, 2022, 3 a.m. UTC | #2
On Sat, Feb 05, 2022 at 08:32:37PM +0100, Heinrich Schuchardt wrote:
> On 2/1/22 02:27, AKASHI Takahiro wrote:
> > The existing options, "--fit" and "--raw," are only used to put a proper
> > GUID in a capsule header, where GUID identifies a particular FMP (Firmware
> > Management Protocol) driver which then would handle the firmware binary in
> > a capsule. In fact, mkeficapsule does the exact same job in creating
> > a capsule file whatever the firmware binary type is.
> > 
> > To prepare for the future extension, the command syntax will be a bit
> > modified to allow users to specify arbitrary GUID for their own FMP driver.
> > OLD:
> >     [--fit <image> | --raw <image>] <capsule file>
> > NEW:
> >     [--fit | --raw | --guid <guid-string>] <image> <capsule file>u
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > ---
> >   doc/develop/uefi/uefi.rst |  4 +-
> >   doc/mkeficapsule.1        | 26 ++++++++----
> >   tools/Makefile            |  2 +-
> >   tools/mkeficapsule.c      | 85 ++++++++++++++++++++++++++++-----------
> >   4 files changed, 84 insertions(+), 33 deletions(-)
> > 
> > diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> > index 7e1eb8256259..a1a2afd60bbc 100644
> > --- a/doc/develop/uefi/uefi.rst
> > +++ b/doc/develop/uefi/uefi.rst
> > @@ -375,8 +375,8 @@ and used by the steps highlighted below.
> >         --private-key CRT.key \
> >         --certificate CRT.crt \
> >         --index 1 --instance 0 \
> > -      [--fit <FIT image> | --raw <raw image>] \
> > -      <capsule_file_name>
> > +      [--fit | --raw | --guid <guid-string] \
> > +      <image_blob> <capsule_file_name>
> > 
> >   4. Insert the signature list into a device tree in the following format::
> > 
> > diff --git a/doc/mkeficapsule.1 b/doc/mkeficapsule.1
> > index 680362f5c4e9..8babb27ee8b2 100644
> > --- a/doc/mkeficapsule.1
> > +++ b/doc/mkeficapsule.1
> > @@ -8,7 +8,7 @@ mkeficapsule \- Generate EFI capsule file for U-Boot
> > 
> >   .SH SYNOPSIS
> >   .B mkeficapsule
> > -.RI [ options "] " capsule-file
> > +.RI [ options "] " image-blob " " capsule-file
> > 
> >   .SH "DESCRIPTION"
> >   .B mkeficapsule
> > @@ -24,7 +24,7 @@ In this case, the update will be authenticated by verifying the signature
> >   before applying.
> > 
> >   .B mkeficapsule
> > -supports two different format of image files:
> > +takes any type of image files, including:
> >   .TP
> >   .I raw image
> >   format is a single binary blob of any type of firmware.
> > @@ -36,18 +36,30 @@ multiple binary blobs in a single capsule file.
> >   This type of image file can be generated by
> >   .BR mkimage .
> > 
> > +.PP
> > +If you want to use other types than above two, you should explicitly
> > +specify a guid for the FMP driver.
> > +
> >   .SH "OPTIONS"
> >   One of
> > -.BR --fit " or " --raw
> > +.BR --fit ", " --raw " or " --guid
> >   option must be specified.
> > 
> >   .TP
> > -.BI "-f\fR,\fB --fit " fit-image-file
> > -Specify a FIT image file
> > +.BR -f ", " --fit
> > +Indicate that the blob is a FIT image file
> > 
> >   .TP
> > -.BI "-r\fR,\fB --raw " raw-image-file
> > -Specify a raw image file
> > +.BR -r ", " --raw
> > +Indicate that the blob is a raw image file
> > +
> > +.TP
> > +.BI "-g\fR,\fB --guid " guid-string
> > +Specify guid for image blob type. The format is:
> > +    xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
> > +
> > +The first three elements are in little endian, while the rest
> > +is in big endian.
> > 
> >   .TP
> >   .BI "-i\fR,\fB --index " index
> > diff --git a/tools/Makefile b/tools/Makefile
> > index 8da07d60a755..5409ff2879c6 100644
> > --- a/tools/Makefile
> > +++ b/tools/Makefile
> > @@ -238,7 +238,7 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs
> >   hostprogs-$(CONFIG_ASN1_COMPILER)	+= asn1_compiler
> >   HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include
> > 
> > -HOSTLDLIBS_mkeficapsule += -lgnutls
> > +HOSTLDLIBS_mkeficapsule += -lgnutls -luuid
> >   hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
> > 
> >   # We build some files with extra pedantic flags to try to minimize things
> > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> > index b996c66ad26a..7ff1f999db85 100644
> > --- a/tools/mkeficapsule.c
> > +++ b/tools/mkeficapsule.c
> > @@ -15,7 +15,7 @@
> > 
> >   #include <sys/stat.h>
> >   #include <sys/types.h>
> > -
> > +#include <uuid/uuid.h>
> 
> https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/3589/logs/128:
> 
> 2022-02-05T16:53:55.9574749Z   HOSTCC  tools/mkeficapsule
> 2022-02-05T16:53:56.1341775Z tools/mkeficapsule.c:18:10: fatal error:
> uuid/uuid.h: No such file or directory
> 2022-02-05T16:53:56.1343964Z    18 | #include <uuid/uuid.h>
> 2022-02-05T16:53:56.1344702Z       |          ^~~~~~~~~~~~~
> 2022-02-05T16:53:56.1347501Z compilation terminated.
> 2022-02-05T16:53:56.1392680Z make[2]: *** [scripts/Makefile.host:95:
> tools/mkeficapsule] Error 1
> 
> Instead of adding a dependency on uuid-dev (linux-utils) it might be
> better to write your own uuid_parse() function.

I don't want to re-invent a library function even if it looks quite easy.
Looking at azure-pipelines.yml, the build process makes use of msys2 on
Windows and brew on MacOS. Adding dependencies seems not difficult
(although I don't know the exact package names for the library and headers).

Apparently, this is not only about libuuid but also libgnutls.

Please note, in addition, that there is no current user for "--guid"
option. We may drop this patch for now if appropriate.

-Takahiro Akashi

> https://u-boot.readthedocs.io/en/latest/develop/ci_testing.html
> describes how to trigger the Azure pipelines using a pull request on Github.
> 
> Please, trigger the test before resubmitting.
> 
> Best regards
> 
> Heinrich
> 
> >   #include <linux/kconfig.h>
> > 
> >   #include <gnutls/gnutls.h>
> > @@ -33,11 +33,12 @@ efi_guid_t efi_guid_image_type_uboot_raw =
> >   		EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;
> >   efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
> > 
> > -static const char *opts_short = "f:r:i:I:v:p:c:m:dh";
> > +static const char *opts_short = "frg:i:I:v:p:c:m:dh";
> > 
> >   static struct option options[] = {
> > -	{"fit", required_argument, NULL, 'f'},
> > -	{"raw", required_argument, NULL, 'r'},
> > +	{"fit", no_argument, NULL, 'f'},
> > +	{"raw", no_argument, NULL, 'r'},
> > +	{"guid", required_argument, NULL, 'g'},
> >   	{"index", required_argument, NULL, 'i'},
> >   	{"instance", required_argument, NULL, 'I'},
> >   	{"private-key", required_argument, NULL, 'p'},
> > @@ -50,11 +51,12 @@ static struct option options[] = {
> > 
> >   static void print_usage(void)
> >   {
> > -	printf("Usage: %s [options] <output file>\n"
> > +	fprintf(stderr, "Usage: %s [options] <image blob> <output file>\n"
> >   		"Options:\n"
> > 
> > -		"\t-f, --fit <fit image>       new FIT image file\n"
> > -		"\t-r, --raw <raw image>       new raw image file\n"
> > +		"\t-f, --fit                   FIT image type\n"
> > +		"\t-r, --raw                   raw image type\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"
> > @@ -541,6 +543,37 @@ err:
> >   	return ret;
> >   }
> > 
> > +/**
> > + * convert_uuid_to_guid() - convert UUID to GUID
> > + * @buf:	UUID binary
> > + *
> > + * UUID and GUID have the same data structure, but their binary
> > + * formats are different due to the endianness. See lib/uuid.c.
> > + * Since uuid_parse() can handle only UUID, this function must
> > + * be called to get correct data for GUID when parsing a string.
> > + *
> > + * The correct data will be returned in @buf.
> > + */
> > +void convert_uuid_to_guid(unsigned char *buf)
> > +{
> > +	unsigned char c;
> > +
> > +	c = buf[0];
> > +	buf[0] = buf[3];
> > +	buf[3] = c;
> > +	c = buf[1];
> > +	buf[1] = buf[2];
> > +	buf[2] = c;
> > +
> > +	c = buf[4];
> > +	buf[4] = buf[5];
> > +	buf[5] = c;
> > +
> > +	c = buf[6];
> > +	buf[6] = buf[7];
> > +	buf[7] = c;
> > +}
> > +
> >   /**
> >    * main - main entry function of mkeficapsule
> >    * @argc:	Number of arguments
> > @@ -555,14 +588,13 @@ err:
> >    */
> >   int main(int argc, char **argv)
> >   {
> > -	char *file;
> >   	efi_guid_t *guid;
> > +	unsigned char uuid_buf[16];
> >   	unsigned long index, instance;
> >   	uint64_t mcount;
> >   	char *privkey_file, *cert_file;
> >   	int c, idx;
> > 
> > -	file = NULL;
> >   	guid = NULL;
> >   	index = 0;
> >   	instance = 0;
> > @@ -577,21 +609,34 @@ int main(int argc, char **argv)
> > 
> >   		switch (c) {
> >   		case 'f':
> > -			if (file) {
> > -				fprintf(stderr, "Image already specified\n");
> > +			if (guid) {
> > +				fprintf(stderr,
> > +					"Image type already specified\n");
> >   				exit(EXIT_FAILURE);
> >   			}
> > -			file = optarg;
> >   			guid = &efi_guid_image_type_uboot_fit;
> >   			break;
> >   		case 'r':
> > -			if (file) {
> > -				fprintf(stderr, "Image already specified\n");
> > +			if (guid) {
> > +				fprintf(stderr,
> > +					"Image type already specified\n");
> >   				exit(EXIT_FAILURE);
> >   			}
> > -			file = optarg;
> >   			guid = &efi_guid_image_type_uboot_raw;
> >   			break;
> > +		case 'g':
> > +			if (guid) {
> > +				fprintf(stderr,
> > +					"Image type already specified\n");
> > +				exit(EXIT_FAILURE);
> > +			}
> > +			if (uuid_parse(optarg, uuid_buf)) {
> > +				fprintf(stderr, "Wrong guid format\n");
> > +				exit(EXIT_FAILURE);
> > +			}
> > +			convert_uuid_to_guid(uuid_buf);
> > +			guid = (efi_guid_t *)uuid_buf;
> > +			break;
> >   		case 'i':
> >   			index = strtoul(optarg, NULL, 0);
> >   			break;
> > @@ -627,20 +672,14 @@ int main(int argc, char **argv)
> >   	}
> > 
> >   	/* check necessary parameters */
> > -	if ((argc != optind + 1) || !file ||
> > +	if ((argc != optind + 2) || !guid ||
> >   	    ((privkey_file && !cert_file) ||
> >   	     (!privkey_file && cert_file))) {
> >   		print_usage();
> >   		exit(EXIT_FAILURE);
> >   	}
> > 
> > -	/* need a fit image file or raw image file */
> > -	if (!file) {
> > -		print_usage();
> > -		exit(EXIT_SUCCESS);
> > -	}
> > -
> > -	if (create_fwbin(argv[optind], file, guid, index, instance,
> > +	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);
>
Sughosh Ganu Feb. 7, 2022, 11:58 a.m. UTC | #3
On Mon, 7 Feb 2022 at 08:30, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>
> On Sat, Feb 05, 2022 at 08:32:37PM +0100, Heinrich Schuchardt wrote:
> > On 2/1/22 02:27, AKASHI Takahiro wrote:
> > > The existing options, "--fit" and "--raw," are only used to put a proper
> > > GUID in a capsule header, where GUID identifies a particular FMP (Firmware
> > > Management Protocol) driver which then would handle the firmware binary in
> > > a capsule. In fact, mkeficapsule does the exact same job in creating
> > > a capsule file whatever the firmware binary type is.
> > >
> > > To prepare for the future extension, the command syntax will be a bit
> > > modified to allow users to specify arbitrary GUID for their own FMP driver.
> > > OLD:
> > >     [--fit <image> | --raw <image>] <capsule file>
> > > NEW:
> > >     [--fit | --raw | --guid <guid-string>] <image> <capsule file>u
> > >
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >   doc/develop/uefi/uefi.rst |  4 +-
> > >   doc/mkeficapsule.1        | 26 ++++++++----
> > >   tools/Makefile            |  2 +-
> > >   tools/mkeficapsule.c      | 85 ++++++++++++++++++++++++++++-----------
> > >   4 files changed, 84 insertions(+), 33 deletions(-)
> > >
> > > diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> > > index 7e1eb8256259..a1a2afd60bbc 100644
> > > --- a/doc/develop/uefi/uefi.rst
> > > +++ b/doc/develop/uefi/uefi.rst
> > > @@ -375,8 +375,8 @@ and used by the steps highlighted below.
> > >         --private-key CRT.key \
> > >         --certificate CRT.crt \
> > >         --index 1 --instance 0 \
> > > -      [--fit <FIT image> | --raw <raw image>] \
> > > -      <capsule_file_name>
> > > +      [--fit | --raw | --guid <guid-string] \
> > > +      <image_blob> <capsule_file_name>
> > >
> > >   4. Insert the signature list into a device tree in the following format::
> > >
> > > diff --git a/doc/mkeficapsule.1 b/doc/mkeficapsule.1
> > > index 680362f5c4e9..8babb27ee8b2 100644
> > > --- a/doc/mkeficapsule.1
> > > +++ b/doc/mkeficapsule.1
> > > @@ -8,7 +8,7 @@ mkeficapsule \- Generate EFI capsule file for U-Boot
> > >
> > >   .SH SYNOPSIS
> > >   .B mkeficapsule
> > > -.RI [ options "] " capsule-file
> > > +.RI [ options "] " image-blob " " capsule-file
> > >
> > >   .SH "DESCRIPTION"
> > >   .B mkeficapsule
> > > @@ -24,7 +24,7 @@ In this case, the update will be authenticated by verifying the signature
> > >   before applying.
> > >
> > >   .B mkeficapsule
> > > -supports two different format of image files:
> > > +takes any type of image files, including:
> > >   .TP
> > >   .I raw image
> > >   format is a single binary blob of any type of firmware.
> > > @@ -36,18 +36,30 @@ multiple binary blobs in a single capsule file.
> > >   This type of image file can be generated by
> > >   .BR mkimage .
> > >
> > > +.PP
> > > +If you want to use other types than above two, you should explicitly
> > > +specify a guid for the FMP driver.
> > > +
> > >   .SH "OPTIONS"
> > >   One of
> > > -.BR --fit " or " --raw
> > > +.BR --fit ", " --raw " or " --guid
> > >   option must be specified.
> > >
> > >   .TP
> > > -.BI "-f\fR,\fB --fit " fit-image-file
> > > -Specify a FIT image file
> > > +.BR -f ", " --fit
> > > +Indicate that the blob is a FIT image file
> > >
> > >   .TP
> > > -.BI "-r\fR,\fB --raw " raw-image-file
> > > -Specify a raw image file
> > > +.BR -r ", " --raw
> > > +Indicate that the blob is a raw image file
> > > +
> > > +.TP
> > > +.BI "-g\fR,\fB --guid " guid-string
> > > +Specify guid for image blob type. The format is:
> > > +    xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
> > > +
> > > +The first three elements are in little endian, while the rest
> > > +is in big endian.
> > >
> > >   .TP
> > >   .BI "-i\fR,\fB --index " index
> > > diff --git a/tools/Makefile b/tools/Makefile
> > > index 8da07d60a755..5409ff2879c6 100644
> > > --- a/tools/Makefile
> > > +++ b/tools/Makefile
> > > @@ -238,7 +238,7 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs
> > >   hostprogs-$(CONFIG_ASN1_COMPILER) += asn1_compiler
> > >   HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include
> > >
> > > -HOSTLDLIBS_mkeficapsule += -lgnutls
> > > +HOSTLDLIBS_mkeficapsule += -lgnutls -luuid
> > >   hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
> > >
> > >   # We build some files with extra pedantic flags to try to minimize things
> > > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> > > index b996c66ad26a..7ff1f999db85 100644
> > > --- a/tools/mkeficapsule.c
> > > +++ b/tools/mkeficapsule.c
> > > @@ -15,7 +15,7 @@
> > >
> > >   #include <sys/stat.h>
> > >   #include <sys/types.h>
> > > -
> > > +#include <uuid/uuid.h>
> >
> > https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/3589/logs/128:
> >
> > 2022-02-05T16:53:55.9574749Z   HOSTCC  tools/mkeficapsule
> > 2022-02-05T16:53:56.1341775Z tools/mkeficapsule.c:18:10: fatal error:
> > uuid/uuid.h: No such file or directory
> > 2022-02-05T16:53:56.1343964Z    18 | #include <uuid/uuid.h>
> > 2022-02-05T16:53:56.1344702Z       |          ^~~~~~~~~~~~~
> > 2022-02-05T16:53:56.1347501Z compilation terminated.
> > 2022-02-05T16:53:56.1392680Z make[2]: *** [scripts/Makefile.host:95:
> > tools/mkeficapsule] Error 1
> >
> > Instead of adding a dependency on uuid-dev (linux-utils) it might be
> > better to write your own uuid_parse() function.
>
> I don't want to re-invent a library function even if it looks quite easy.
> Looking at azure-pipelines.yml, the build process makes use of msys2 on
> Windows and brew on MacOS. Adding dependencies seems not difficult
> (although I don't know the exact package names for the library and headers).
>
> Apparently, this is not only about libuuid but also libgnutls.
>
> Please note, in addition, that there is no current user for "--guid"
> option. We may drop this patch for now if appropriate.

With the FWU Multi Bank Update feature, there will be use of the
--guid option. So it will be needed. Else we will be required to use
the GenerateCapsule script in EDK2 for generating capsules with
specific GUID values.

-sughosh

>
> -Takahiro Akashi
>
> > https://u-boot.readthedocs.io/en/latest/develop/ci_testing.html
> > describes how to trigger the Azure pipelines using a pull request on Github.
> >
> > Please, trigger the test before resubmitting.
> >
> > Best regards
> >
> > Heinrich
> >
> > >   #include <linux/kconfig.h>
> > >
> > >   #include <gnutls/gnutls.h>
> > > @@ -33,11 +33,12 @@ efi_guid_t efi_guid_image_type_uboot_raw =
> > >             EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;
> > >   efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
> > >
> > > -static const char *opts_short = "f:r:i:I:v:p:c:m:dh";
> > > +static const char *opts_short = "frg:i:I:v:p:c:m:dh";
> > >
> > >   static struct option options[] = {
> > > -   {"fit", required_argument, NULL, 'f'},
> > > -   {"raw", required_argument, NULL, 'r'},
> > > +   {"fit", no_argument, NULL, 'f'},
> > > +   {"raw", no_argument, NULL, 'r'},
> > > +   {"guid", required_argument, NULL, 'g'},
> > >     {"index", required_argument, NULL, 'i'},
> > >     {"instance", required_argument, NULL, 'I'},
> > >     {"private-key", required_argument, NULL, 'p'},
> > > @@ -50,11 +51,12 @@ static struct option options[] = {
> > >
> > >   static void print_usage(void)
> > >   {
> > > -   printf("Usage: %s [options] <output file>\n"
> > > +   fprintf(stderr, "Usage: %s [options] <image blob> <output file>\n"
> > >             "Options:\n"
> > >
> > > -           "\t-f, --fit <fit image>       new FIT image file\n"
> > > -           "\t-r, --raw <raw image>       new raw image file\n"
> > > +           "\t-f, --fit                   FIT image type\n"
> > > +           "\t-r, --raw                   raw image type\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"
> > > @@ -541,6 +543,37 @@ err:
> > >     return ret;
> > >   }
> > >
> > > +/**
> > > + * convert_uuid_to_guid() - convert UUID to GUID
> > > + * @buf:   UUID binary
> > > + *
> > > + * UUID and GUID have the same data structure, but their binary
> > > + * formats are different due to the endianness. See lib/uuid.c.
> > > + * Since uuid_parse() can handle only UUID, this function must
> > > + * be called to get correct data for GUID when parsing a string.
> > > + *
> > > + * The correct data will be returned in @buf.
> > > + */
> > > +void convert_uuid_to_guid(unsigned char *buf)
> > > +{
> > > +   unsigned char c;
> > > +
> > > +   c = buf[0];
> > > +   buf[0] = buf[3];
> > > +   buf[3] = c;
> > > +   c = buf[1];
> > > +   buf[1] = buf[2];
> > > +   buf[2] = c;
> > > +
> > > +   c = buf[4];
> > > +   buf[4] = buf[5];
> > > +   buf[5] = c;
> > > +
> > > +   c = buf[6];
> > > +   buf[6] = buf[7];
> > > +   buf[7] = c;
> > > +}
> > > +
> > >   /**
> > >    * main - main entry function of mkeficapsule
> > >    * @argc: Number of arguments
> > > @@ -555,14 +588,13 @@ err:
> > >    */
> > >   int main(int argc, char **argv)
> > >   {
> > > -   char *file;
> > >     efi_guid_t *guid;
> > > +   unsigned char uuid_buf[16];
> > >     unsigned long index, instance;
> > >     uint64_t mcount;
> > >     char *privkey_file, *cert_file;
> > >     int c, idx;
> > >
> > > -   file = NULL;
> > >     guid = NULL;
> > >     index = 0;
> > >     instance = 0;
> > > @@ -577,21 +609,34 @@ int main(int argc, char **argv)
> > >
> > >             switch (c) {
> > >             case 'f':
> > > -                   if (file) {
> > > -                           fprintf(stderr, "Image already specified\n");
> > > +                   if (guid) {
> > > +                           fprintf(stderr,
> > > +                                   "Image type already specified\n");
> > >                             exit(EXIT_FAILURE);
> > >                     }
> > > -                   file = optarg;
> > >                     guid = &efi_guid_image_type_uboot_fit;
> > >                     break;
> > >             case 'r':
> > > -                   if (file) {
> > > -                           fprintf(stderr, "Image already specified\n");
> > > +                   if (guid) {
> > > +                           fprintf(stderr,
> > > +                                   "Image type already specified\n");
> > >                             exit(EXIT_FAILURE);
> > >                     }
> > > -                   file = optarg;
> > >                     guid = &efi_guid_image_type_uboot_raw;
> > >                     break;
> > > +           case 'g':
> > > +                   if (guid) {
> > > +                           fprintf(stderr,
> > > +                                   "Image type already specified\n");
> > > +                           exit(EXIT_FAILURE);
> > > +                   }
> > > +                   if (uuid_parse(optarg, uuid_buf)) {
> > > +                           fprintf(stderr, "Wrong guid format\n");
> > > +                           exit(EXIT_FAILURE);
> > > +                   }
> > > +                   convert_uuid_to_guid(uuid_buf);
> > > +                   guid = (efi_guid_t *)uuid_buf;
> > > +                   break;
> > >             case 'i':
> > >                     index = strtoul(optarg, NULL, 0);
> > >                     break;
> > > @@ -627,20 +672,14 @@ int main(int argc, char **argv)
> > >     }
> > >
> > >     /* check necessary parameters */
> > > -   if ((argc != optind + 1) || !file ||
> > > +   if ((argc != optind + 2) || !guid ||
> > >         ((privkey_file && !cert_file) ||
> > >          (!privkey_file && cert_file))) {
> > >             print_usage();
> > >             exit(EXIT_FAILURE);
> > >     }
> > >
> > > -   /* need a fit image file or raw image file */
> > > -   if (!file) {
> > > -           print_usage();
> > > -           exit(EXIT_SUCCESS);
> > > -   }
> > > -
> > > -   if (create_fwbin(argv[optind], file, guid, index, instance,
> > > +   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);
> >
diff mbox series

Patch

diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index 7e1eb8256259..a1a2afd60bbc 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -375,8 +375,8 @@  and used by the steps highlighted below.
       --private-key CRT.key \
       --certificate CRT.crt \
       --index 1 --instance 0 \
-      [--fit <FIT image> | --raw <raw image>] \
-      <capsule_file_name>
+      [--fit | --raw | --guid <guid-string] \
+      <image_blob> <capsule_file_name>
 
 4. Insert the signature list into a device tree in the following format::
 
diff --git a/doc/mkeficapsule.1 b/doc/mkeficapsule.1
index 680362f5c4e9..8babb27ee8b2 100644
--- a/doc/mkeficapsule.1
+++ b/doc/mkeficapsule.1
@@ -8,7 +8,7 @@  mkeficapsule \- Generate EFI capsule file for U-Boot
 
 .SH SYNOPSIS
 .B mkeficapsule
-.RI [ options "] " capsule-file
+.RI [ options "] " image-blob " " capsule-file
 
 .SH "DESCRIPTION"
 .B mkeficapsule
@@ -24,7 +24,7 @@  In this case, the update will be authenticated by verifying the signature
 before applying.
 
 .B mkeficapsule
-supports two different format of image files:
+takes any type of image files, including:
 .TP
 .I raw image
 format is a single binary blob of any type of firmware.
@@ -36,18 +36,30 @@  multiple binary blobs in a single capsule file.
 This type of image file can be generated by
 .BR mkimage .
 
+.PP
+If you want to use other types than above two, you should explicitly
+specify a guid for the FMP driver.
+
 .SH "OPTIONS"
 One of
-.BR --fit " or " --raw
+.BR --fit ", " --raw " or " --guid
 option must be specified.
 
 .TP
-.BI "-f\fR,\fB --fit " fit-image-file
-Specify a FIT image file
+.BR -f ", " --fit
+Indicate that the blob is a FIT image file
 
 .TP
-.BI "-r\fR,\fB --raw " raw-image-file
-Specify a raw image file
+.BR -r ", " --raw
+Indicate that the blob is a raw image file
+
+.TP
+.BI "-g\fR,\fB --guid " guid-string
+Specify guid for image blob type. The format is:
+    xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+
+The first three elements are in little endian, while the rest
+is in big endian.
 
 .TP
 .BI "-i\fR,\fB --index " index
diff --git a/tools/Makefile b/tools/Makefile
index 8da07d60a755..5409ff2879c6 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -238,7 +238,7 @@  hostprogs-$(CONFIG_MIPS) += mips-relocs
 hostprogs-$(CONFIG_ASN1_COMPILER)	+= asn1_compiler
 HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include
 
-HOSTLDLIBS_mkeficapsule += -lgnutls
+HOSTLDLIBS_mkeficapsule += -lgnutls -luuid
 hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
 
 # We build some files with extra pedantic flags to try to minimize things
diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
index b996c66ad26a..7ff1f999db85 100644
--- a/tools/mkeficapsule.c
+++ b/tools/mkeficapsule.c
@@ -15,7 +15,7 @@ 
 
 #include <sys/stat.h>
 #include <sys/types.h>
-
+#include <uuid/uuid.h>
 #include <linux/kconfig.h>
 
 #include <gnutls/gnutls.h>
@@ -33,11 +33,12 @@  efi_guid_t efi_guid_image_type_uboot_raw =
 		EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID;
 efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
 
-static const char *opts_short = "f:r:i:I:v:p:c:m:dh";
+static const char *opts_short = "frg:i:I:v:p:c:m:dh";
 
 static struct option options[] = {
-	{"fit", required_argument, NULL, 'f'},
-	{"raw", required_argument, NULL, 'r'},
+	{"fit", no_argument, NULL, 'f'},
+	{"raw", no_argument, NULL, 'r'},
+	{"guid", required_argument, NULL, 'g'},
 	{"index", required_argument, NULL, 'i'},
 	{"instance", required_argument, NULL, 'I'},
 	{"private-key", required_argument, NULL, 'p'},
@@ -50,11 +51,12 @@  static struct option options[] = {
 
 static void print_usage(void)
 {
-	printf("Usage: %s [options] <output file>\n"
+	fprintf(stderr, "Usage: %s [options] <image blob> <output file>\n"
 		"Options:\n"
 
-		"\t-f, --fit <fit image>       new FIT image file\n"
-		"\t-r, --raw <raw image>       new raw image file\n"
+		"\t-f, --fit                   FIT image type\n"
+		"\t-r, --raw                   raw image type\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"
@@ -541,6 +543,37 @@  err:
 	return ret;
 }
 
+/**
+ * convert_uuid_to_guid() - convert UUID to GUID
+ * @buf:	UUID binary
+ *
+ * UUID and GUID have the same data structure, but their binary
+ * formats are different due to the endianness. See lib/uuid.c.
+ * Since uuid_parse() can handle only UUID, this function must
+ * be called to get correct data for GUID when parsing a string.
+ *
+ * The correct data will be returned in @buf.
+ */
+void convert_uuid_to_guid(unsigned char *buf)
+{
+	unsigned char c;
+
+	c = buf[0];
+	buf[0] = buf[3];
+	buf[3] = c;
+	c = buf[1];
+	buf[1] = buf[2];
+	buf[2] = c;
+
+	c = buf[4];
+	buf[4] = buf[5];
+	buf[5] = c;
+
+	c = buf[6];
+	buf[6] = buf[7];
+	buf[7] = c;
+}
+
 /**
  * main - main entry function of mkeficapsule
  * @argc:	Number of arguments
@@ -555,14 +588,13 @@  err:
  */
 int main(int argc, char **argv)
 {
-	char *file;
 	efi_guid_t *guid;
+	unsigned char uuid_buf[16];
 	unsigned long index, instance;
 	uint64_t mcount;
 	char *privkey_file, *cert_file;
 	int c, idx;
 
-	file = NULL;
 	guid = NULL;
 	index = 0;
 	instance = 0;
@@ -577,21 +609,34 @@  int main(int argc, char **argv)
 
 		switch (c) {
 		case 'f':
-			if (file) {
-				fprintf(stderr, "Image already specified\n");
+			if (guid) {
+				fprintf(stderr,
+					"Image type already specified\n");
 				exit(EXIT_FAILURE);
 			}
-			file = optarg;
 			guid = &efi_guid_image_type_uboot_fit;
 			break;
 		case 'r':
-			if (file) {
-				fprintf(stderr, "Image already specified\n");
+			if (guid) {
+				fprintf(stderr,
+					"Image type already specified\n");
 				exit(EXIT_FAILURE);
 			}
-			file = optarg;
 			guid = &efi_guid_image_type_uboot_raw;
 			break;
+		case 'g':
+			if (guid) {
+				fprintf(stderr,
+					"Image type already specified\n");
+				exit(EXIT_FAILURE);
+			}
+			if (uuid_parse(optarg, uuid_buf)) {
+				fprintf(stderr, "Wrong guid format\n");
+				exit(EXIT_FAILURE);
+			}
+			convert_uuid_to_guid(uuid_buf);
+			guid = (efi_guid_t *)uuid_buf;
+			break;
 		case 'i':
 			index = strtoul(optarg, NULL, 0);
 			break;
@@ -627,20 +672,14 @@  int main(int argc, char **argv)
 	}
 
 	/* check necessary parameters */
-	if ((argc != optind + 1) || !file ||
+	if ((argc != optind + 2) || !guid ||
 	    ((privkey_file && !cert_file) ||
 	     (!privkey_file && cert_file))) {
 		print_usage();
 		exit(EXIT_FAILURE);
 	}
 
-	/* need a fit image file or raw image file */
-	if (!file) {
-		print_usage();
-		exit(EXIT_SUCCESS);
-	}
-
-	if (create_fwbin(argv[optind], file, guid, index, instance,
+	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);