[PATCH v3 0/5] efi_loader: capsule: improve capsule authentication support

[PATCH v3 0/5] efi_loader: capsule: improve capsule authentication support

[AKASHI Takahiro]

# This patch set is basically the same as v2, but removed authentication-
# related pytest because the discussion about where to save a public
# key is still going on. The command, mkeficapule, is independent from
# the result.

As I proposed and discussed in [1] and [2], I have made a couple of
improvements on the current implementation of capsule update in this
patch set.

* add signing feature to mkeficapsule
* add "--guid" option to mkeficapsule
* add man page of mkeficapsule

[1] https://lists.denx.de/pipermail/u-boot/2021-April/447918.html
[2] https://lists.denx.de/pipermail/u-boot/2021-July/455292.html

Prerequisite patches
====================
None

Test
====
* locally passed the pytest which is included in this patch series
  on sandbox built.

Todo
====
* add capsule authentication pytest

Changes
=======
v3 (Aug 31, 2021)
* rebased on v2021.10-rc3
* remove pytest-related patches
* add function descriptions in mkeficapsule.c
* correct format specifiers in printf()
* let main() return 0 or -1 only
* update doc/develop/uefi/uefi.rst for syntax change of mkeficapsule

v2 (July 28, 2021)
* rebased on v2021.10-rc*
* removed dependency on target's configuration
* removed fdtsig.sh and others
* add man page
* update the UEFI document
* add dedicate defconfig for testing on sandbox
* add gitlab CI support
* add "--guid" option to mkeficapsule
  (yet rather RFC)

Initial release (May 12, 2021)
* based on v2021.07-rc2

AKASHI Takahiro (5):
  tools: mkeficapsule: add firmwware image signing
  tools: mkeficapsule: add man page
  doc: update UEFI document for usage of mkeficapsule
  tools: mkeficapsule: allow for specifying GUID explicitly
  test/py: efi_capsule: align with the syntax change of mkeficapsule

 MAINTAINERS                                |   1 +
 doc/develop/uefi/uefi.rst                  |  31 +-
 doc/mkeficapsule.1                         |  98 +++++
 test/py/tests/test_efi_capsule/conftest.py |   4 +-
 tools/Kconfig                              |   7 +
 tools/Makefile                             |   8 +-
 tools/mkeficapsule.c                       | 471 +++++++++++++++++++--
 7 files changed, 554 insertions(+), 66 deletions(-)
 create mode 100644 doc/mkeficapsule.1

-- 
2.33.0

[PATCH v3 1/5] tools: mkeficapsule: add firmwware image signing

[AKASHI Takahiro]

With this enhancement, mkeficapsule will be able to sign a capsule
file when it is created. A signature added will be used later
in the verification at FMP's SetImage() call.

To do that, We need specify additional command parameters:
  -monotonic-cout <count> : monotonic count
  -private-key <private key file> : private key file
  -certificate <certificate file> : certificate file
Only when all of those parameters are given, a signature will be added
to a capsule file.

Users are expected to maintain and increment the monotonic count at
every time of the update for each firmware image.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 tools/Kconfig        |   7 +
 tools/Makefile       |   8 +-
 tools/mkeficapsule.c | 435 +++++++++++++++++++++++++++++++++++++++----
 3 files changed, 416 insertions(+), 34 deletions(-)

diff --git a/tools/Kconfig b/tools/Kconfig
index d6f82cd949b5..9a37ed035311 100644
--- a/tools/Kconfig
+++ b/tools/Kconfig
@@ -20,4 +20,11 @@ config TOOLS_LIBCRYPTO
 	  This selection does not affect target features, such as runtime FIT
 	  signature verification.
 
+config TOOLS_MKEFICAPSULE
+	bool "Build efimkcapsule command"
+	default y if EFI_CAPSULE_ON_DISK
+	help
+	  This command allows users to create a UEFI capsule file and,
+	  optionally sign that file. If you want to enable UEFI capsule
+	  update feature on your target, you certainly need this.
 endmenu
diff --git a/tools/Makefile b/tools/Makefile
index 4a86321f6467..6ea3033dbfb8 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -237,8 +237,12 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs
 hostprogs-$(CONFIG_ASN1_COMPILER)	+= asn1_compiler
 HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include
 
-mkeficapsule-objs	:= mkeficapsule.o $(LIBFDT_OBJS)
-hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule
+HOSTLDLIBS_mkeficapsule += -luuid
+ifeq ($(CONFIG_TOOLS_LIBCRYPTO),y)
+HOSTLDLIBS_mkeficapsule += \
+	$(shell pkg-config --libs libssl libcrypto 2> /dev/null || echo "-lssl -lcrypto")
+endif
+hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
 
 # We build some files with extra pedantic flags to try to minimize things
 # that won't build on some weird host compiler -- though there are lots of
diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
index 4995ba4e0c2a..090fb02de68d 100644
--- a/tools/mkeficapsule.c
+++ b/tools/mkeficapsule.c
@@ -15,6 +15,16 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 
+#include <linux/kconfig.h>
+#ifdef CONFIG_TOOLS_LIBCRYPTO
+#include <openssl/asn1.h>
+#include <openssl/bio.h>
+#include <openssl/evp.h>
+#include <openssl/err.h>
+#include <openssl/pem.h>
+#include <openssl/pkcs7.h>
+#endif
+
 typedef __u8 u8;
 typedef __u16 u16;
 typedef __u32 u32;
@@ -38,12 +48,25 @@ efi_guid_t efi_guid_image_type_uboot_fit =
 		EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID;
 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;
+
+#ifdef CONFIG_TOOLS_LIBCRYPTO
+static const char *opts_short = "f:r:i:I:v:p:c:m:dh";
+#else
+static const char *opts_short = "f:r:i:I:v:h";
+#endif
 
 static struct option options[] = {
 	{"fit", required_argument, NULL, 'f'},
 	{"raw", required_argument, NULL, 'r'},
 	{"index", required_argument, NULL, 'i'},
 	{"instance", required_argument, NULL, 'I'},
+#ifdef CONFIG_TOOLS_LIBCRYPTO
+	{"private-key", required_argument, NULL, 'p'},
+	{"certificate", required_argument, NULL, 'c'},
+	{"monotonic-count", required_argument, NULL, 'm'},
+	{"dump-sig", no_argument, NULL, 'd'},
+#endif
 	{"help", no_argument, NULL, 'h'},
 	{NULL, 0, NULL, 0},
 };
@@ -57,16 +80,280 @@ static void print_usage(void)
 	       "\t-r, --raw <raw image>       new raw image file\n"
 	       "\t-i, --index <index>         update image index\n"
 	       "\t-I, --instance <instance>   update hardware instance\n"
+#ifdef CONFIG_TOOLS_LIBCRYPTO
+	       "\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"
+#endif
 	       "\t-h, --help                  print a help message\n",
 	       tool_name);
 }
 
+/**
+ * auth_context - authentication context
+ * @key_file:	Path to a private key file
+ * @cert_file:	Path to a certificate file
+ * @image_data:	Pointer to firmware data
+ * @image_size:	Size of firmware data
+ * @auth:	Authentication header
+ * @sig_data:	Signature data
+ * @sig_size:	Size of signature data
+ *
+ * Data structure used in create_auth_data(). @key_file through
+ * @image_size are input parameters. @auth, @sig_data and @sig_size
+ * are filled in by create_auth_data().
+ */
+struct auth_context {
+	char *key_file;
+	char *cert_file;
+	u8 *image_data;
+	size_t image_size;
+	struct efi_firmware_image_authentication auth;
+	u8 *sig_data;
+	size_t sig_size;
+};
+
+static int dump_sig;
+
+#ifdef CONFIG_TOOLS_LIBCRYPTO
+/**
+ * fileio-read_pkey - read out a private key
+ * @filename:	Path to a private key file
+ *
+ * Read out a private key file and parse it into "EVP_PKEY" structure.
+ *
+ * Return:
+ * * Pointer to private key structure  - on success
+ * * NULL - on failure
+ */
+static EVP_PKEY *fileio_read_pkey(const char *filename)
+{
+	EVP_PKEY *key = NULL;
+	BIO *bio;
+
+	bio = BIO_new_file(filename, "r");
+	if (!bio)
+		goto out;
+
+	key = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL);
+
+out:
+	BIO_free_all(bio);
+	if (!key) {
+		printf("Can't load key from file '%s'\n", filename);
+		ERR_print_errors_fp(stderr);
+	}
+
+	return key;
+}
+
+/**
+ * fileio-read_cert - read out a certificate
+ * @filename:	Path to a certificate file
+ *
+ * Read out a certificate file and parse it into "X509" structure.
+ *
+ * Return:
+ * * Pointer to certificate structure  - on success
+ * * NULL - on failure
+ */
+static X509 *fileio_read_cert(const char *filename)
+{
+	X509 *cert = NULL;
+	BIO *bio;
+
+	bio = BIO_new_file(filename, "r");
+	if (!bio)
+		goto out;
+
+	cert = PEM_read_bio_X509(bio, NULL, NULL, NULL);
+
+out:
+	BIO_free_all(bio);
+	if (!cert) {
+		printf("Can't load certificate from file '%s'\n", filename);
+		ERR_print_errors_fp(stderr);
+	}
+
+	return cert;
+}
+
+/**
+ * create_auth_data - compose authentication data in capsule
+ * @auth_context:	Pointer to authentication context
+ *
+ * Fill up an authentication header (.auth) and signature data (.sig_data)
+ * in @auth_context, using library functions from openssl.
+ * All the parameters in @auth_context must be filled in by a caller.
+ *
+ * Return:
+ * * 0  - on success
+ * * -1 - on failure
+ */
+static int create_auth_data(struct auth_context *ctx)
+{
+	EVP_PKEY *key = NULL;
+	X509 *cert = NULL;
+	BIO *data_bio = NULL;
+	const EVP_MD *md;
+	PKCS7 *p7;
+	int flags, ret = -1;
+
+	OpenSSL_add_all_digests();
+	OpenSSL_add_all_ciphers();
+	ERR_load_crypto_strings();
+
+	key = fileio_read_pkey(ctx->key_file);
+	if (!key)
+		goto err;
+	cert = fileio_read_cert(ctx->cert_file);
+	if (!cert)
+		goto err;
+
+	/*
+	 * create a BIO, containing:
+	 *  * firmware image
+	 *  * monotonic count
+	 * in this order!
+	 * See EDK2's FmpAuthenticatedHandlerRsa2048Sha256()
+	 */
+	data_bio = BIO_new(BIO_s_mem());
+	BIO_write(data_bio, ctx->image_data, ctx->image_size);
+	BIO_write(data_bio, &ctx->auth.monotonic_count,
+		  sizeof(ctx->auth.monotonic_count));
+
+	md = EVP_get_digestbyname("SHA256");
+	if (!md)
+		goto err;
+
+	/* create signature */
+	/* TODO: maybe add PKCS7_NOATTR and PKCS7_NOSMIMECAP */
+	flags = PKCS7_BINARY | PKCS7_DETACHED;
+	p7 = PKCS7_sign(NULL, NULL, NULL, data_bio, flags | PKCS7_PARTIAL);
+	if (!p7)
+		goto err;
+	if (!PKCS7_sign_add_signer(p7, cert, key, md, flags))
+		goto err;
+	if (!PKCS7_final(p7, data_bio, flags))
+		goto err;
+
+	/* convert pkcs7 into DER */
+	ctx->sig_data = NULL;
+	ctx->sig_size = ASN1_item_i2d((ASN1_VALUE *)p7, &ctx->sig_data,
+				      ASN1_ITEM_rptr(PKCS7));
+	if (!ctx->sig_size)
+		goto err;
+
+	/* fill auth_info */
+	ctx->auth.auth_info.hdr.dwLength = sizeof(ctx->auth.auth_info)
+						+ ctx->sig_size;
+	ctx->auth.auth_info.hdr.wRevision = WIN_CERT_REVISION_2_0;
+	ctx->auth.auth_info.hdr.wCertificateType = WIN_CERT_TYPE_EFI_GUID;
+	memcpy(&ctx->auth.auth_info.cert_type, &efi_guid_cert_type_pkcs7,
+	       sizeof(efi_guid_cert_type_pkcs7));
+
+	ret = 0;
+err:
+	BIO_free_all(data_bio);
+	EVP_PKEY_free(key);
+	X509_free(cert);
+
+	return ret;
+}
+
+/**
+ * dump_signature - dump out a signature
+ * @path:	Path to a capsule file
+ * @signature:	Signature data
+ * @sig_size:	Size of signature data
+ *
+ * Signature data pointed to by @signature will be saved into
+ * a file whose file name is @path with ".p7" suffix.
+ *
+ * Return:
+ * * 0  - on success
+ * * -1 - on failure
+ */
+static int dump_signature(const char *path, u8 *signature, size_t sig_size)
+{
+	char *sig_path;
+	FILE *f;
+	size_t size;
+	int ret = -1;
+
+	sig_path = malloc(strlen(path) + 3 + 1);
+	if (!sig_path)
+		return ret;
+
+	sprintf(sig_path, "%s.p7", path);
+	f = fopen(sig_path, "w");
+	if (!f)
+		goto err;
+
+	size = fwrite(signature, 1, sig_size, f);
+	if (size == sig_size)
+		ret = 0;
+
+	fclose(f);
+err:
+	free(sig_path);
+	return ret;
+}
+
+/**
+ * free_sig_data - free out signature data
+ * @ctx:	Pointer to authentication context
+ *
+ * Free signature data allocated in create_auth_data().
+ */
+static void free_sig_data(struct auth_context *ctx)
+{
+	if (ctx->sig_size)
+		OPENSSL_free(ctx->sig_data);
+}
+#else
+static int create_auth_data(struct auth_context *ctx)
+{
+	return 0;
+}
+
+static int dump_signature(const char *path, u8 *signature, size_t sig_size)
+{
+	return 0;
+}
+
+static void free_sig_data(struct auth_context *ctx) {}
+#endif
+
+/**
+ * create_fwbin - create an uefi capsule file
+ * @path:	Path to a created capsule file
+ * @bin:	Path to a firmware binary to encapsulate
+ * @guid:	GUID of related FMP driver
+ * @index:	Index number in capsule
+ * @instance:	Instance number in capsule
+ * @mcount:	Monolitic count in authentication information
+ * @private_file:	Path to a private key file
+ * @cert_file:	Path to a certificate file
+ *
+ * This function actually does the job of creating an uefi capsule file.
+ * All the arguments must be supplied.
+ * If either @private_file ror @cert_file is NULL, the capsule file
+ * won't be signed.
+ *
+ * Return:
+ * * 0  - on success
+ * * -1 - on failure
+ */
 static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
-			unsigned long index, unsigned long instance)
+			unsigned long index, unsigned long instance,
+			uint64_t mcount, char *privkey_file, char *cert_file)
 {
 	struct efi_capsule_header header;
 	struct efi_firmware_management_capsule_header capsule;
 	struct efi_firmware_management_capsule_image_header image;
+	struct auth_context auth_context;
 	FILE *f, *g;
 	struct stat bin_stat;
 	u8 *data;
@@ -76,8 +363,9 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
 #ifdef DEBUG
 	printf("For output: %s\n", path);
 	printf("\tbin: %s\n\ttype: %pUl\n", bin, guid);
-	printf("\tindex: %ld\n\tinstance: %ld\n", index, instance);
+	printf("\tindex: %lu\n\tinstance: %lu\n", index, instance);
 #endif
+	auth_context.sig_size = 0;
 
 	g = fopen(bin, "r");
 	if (!g) {
@@ -93,11 +381,34 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
 		printf("cannot allocate memory: %zx\n", (size_t)bin_stat.st_size);
 		goto err_1;
 	}
-	f = fopen(path, "w");
-	if (!f) {
-		printf("cannot open %s\n", path);
+
+	size = fread(data, 1, bin_stat.st_size, g);
+	if (size < bin_stat.st_size) {
+		printf("read failed (%zx)\n", size);
 		goto err_2;
 	}
+
+	/* first, calculate signature to determine its size */
+	if (privkey_file && cert_file) {
+		auth_context.key_file = privkey_file;
+		auth_context.cert_file = cert_file;
+		auth_context.auth.monotonic_count = mcount;
+		auth_context.image_data = data;
+		auth_context.image_size = bin_stat.st_size;
+
+		if (create_auth_data(&auth_context)) {
+			printf("Signing firmware image failed\n");
+			goto err_3;
+		}
+
+		if (dump_sig &&
+		    dump_signature(path, auth_context.sig_data,
+				   auth_context.sig_size)) {
+			printf("Creating signature file failed\n");
+			goto err_3;
+		}
+	}
+
 	header.capsule_guid = efi_guid_fm_capsule;
 	header.header_size = sizeof(header);
 	/* TODO: The current implementation ignores flags */
@@ -106,11 +417,20 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
 					+ sizeof(capsule) + sizeof(u64)
 					+ sizeof(image)
 					+ bin_stat.st_size;
+	if (auth_context.sig_size)
+		header.capsule_image_size += sizeof(auth_context.auth)
+				+ auth_context.sig_size;
+
+	f = fopen(path, "w");
+	if (!f) {
+		printf("cannot open %s\n", path);
+		goto err_3;
+	}
 
 	size = fwrite(&header, 1, sizeof(header), f);
 	if (size < sizeof(header)) {
 		printf("write failed (%zx)\n", size);
-		goto err_3;
+		goto err_4;
 	}
 
 	capsule.version = 0x00000001;
@@ -119,13 +439,13 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
 	size = fwrite(&capsule, 1, sizeof(capsule), f);
 	if (size < (sizeof(capsule))) {
 		printf("write failed (%zx)\n", size);
-		goto err_3;
+		goto err_4;
 	}
 	offset = sizeof(capsule) + sizeof(u64);
 	size = fwrite(&offset, 1, sizeof(offset), f);
 	if (size < sizeof(offset)) {
 		printf("write failed (%zx)\n", size);
-		goto err_3;
+		goto err_4;
 	}
 
 	image.version = 0x00000003;
@@ -135,34 +455,53 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
 	image.reserved[1] = 0;
 	image.reserved[2] = 0;
 	image.update_image_size = bin_stat.st_size;
+	if (auth_context.sig_size)
+		image.update_image_size += sizeof(auth_context.auth)
+				+ auth_context.sig_size;
 	image.update_vendor_code_size = 0; /* none */
 	image.update_hardware_instance = instance;
 	image.image_capsule_support = 0;
+	if (auth_context.sig_size)
+		image.image_capsule_support |= CAPSULE_SUPPORT_AUTHENTICATION;
 
 	size = fwrite(&image, 1, sizeof(image), f);
 	if (size < sizeof(image)) {
 		printf("write failed (%zx)\n", size);
-		goto err_3;
+		goto err_4;
 	}
-	size = fread(data, 1, bin_stat.st_size, g);
-	if (size < bin_stat.st_size) {
-		printf("read failed (%zx)\n", size);
-		goto err_3;
+
+	if (auth_context.sig_size) {
+		size = fwrite(&auth_context.auth, 1,
+			      sizeof(auth_context.auth), f);
+		if (size < sizeof(auth_context.auth)) {
+			printf("write failed (%zx)\n", size);
+			goto err_4;
+		}
+		size = fwrite(auth_context.sig_data, 1,
+			      auth_context.sig_size, f);
+		if (size < auth_context.sig_size) {
+			printf("write failed (%zx)\n", size);
+			goto err_4;
+		}
 	}
+
 	size = fwrite(data, 1, bin_stat.st_size, f);
 	if (size < bin_stat.st_size) {
 		printf("write failed (%zx)\n", size);
-		goto err_3;
+		goto err_4;
 	}
 
 	fclose(f);
 	fclose(g);
 	free(data);
+	free_sig_data(&auth_context);
 
 	return 0;
 
-err_3:
+err_4:
 	fclose(f);
+err_3:
+	free_sig_data(&auth_context);
 err_2:
 	free(data);
 err_1:
@@ -171,23 +510,37 @@ err_1:
 	return -1;
 }
 
-/*
- * Usage:
- *   $ mkeficapsule -f <firmware binary> <output file>
+/**
+ * main - main entry function of mkeficapsule
+ * @argc:	Number of arguments
+ * @argv:	Array of pointers to arguments
+ *
+ * Create an uefi capsule file, optionally signing it.
+ * Parse all the arguments and pass them on to create_fwbin().
+ *
+ * Return:
+ * * 0  - on success
+ * * -1 - on failure
  */
 int main(int argc, char **argv)
 {
 	char *file;
 	efi_guid_t *guid;
 	unsigned long index, instance;
+	uint64_t mcount;
+	char *privkey_file, *cert_file;
 	int c, idx;
 
 	file = NULL;
 	guid = NULL;
 	index = 0;
 	instance = 0;
+	mcount = 0;
+	privkey_file = NULL;
+	cert_file = NULL;
+	dump_sig = 0;
 	for (;;) {
-		c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx);
+		c = getopt_long(argc, argv, opts_short, options, &idx);
 		if (c == -1)
 			break;
 
@@ -214,29 +567,47 @@ int main(int argc, char **argv)
 		case 'I':
 			instance = strtoul(optarg, NULL, 0);
 			break;
+#ifdef CONFIG_TOOLS_LIBCRYPTO
+		case 'p':
+			if (privkey_file) {
+				printf("Private Key already specified\n");
+				return -1;
+			}
+			privkey_file = optarg;
+			break;
+		case 'c':
+			if (cert_file) {
+				printf("Certificate file already specified\n");
+				return -1;
+			}
+			cert_file = optarg;
+			break;
+		case 'm':
+			mcount = strtoul(optarg, NULL, 0);
+			break;
+		case 'd':
+			dump_sig = 1;
+			break;
+#endif /* CONFIG_TOOLS_LIBCRYPTO */
 		case 'h':
 			print_usage();
 			return 0;
 		}
 	}
 
-	/* need an output file */
-	if (argc != optind + 1) {
-		print_usage();
-		exit(EXIT_FAILURE);
-	}
-
-	/* need a fit image file or raw image file */
-	if (!file) {
+	/* check necessary parameters */
+	if ((argc != optind + 1) || !file ||
+	    ((privkey_file && !cert_file) ||
+	     (!privkey_file && cert_file))) {
 		print_usage();
-		exit(EXIT_SUCCESS);
+		return -1;
 	}
 
-	if (create_fwbin(argv[optind], file, guid, index, instance)
-			< 0) {
+	if (create_fwbin(argv[optind], file, guid, index, instance,
+			 mcount, privkey_file, cert_file) < 0) {
 		printf("Creating firmware capsule failed\n");
-		exit(EXIT_FAILURE);
+		return -1;
 	}
 
-	exit(EXIT_SUCCESS);
+	return 0;
 }
-- 
2.33.0

Re: [PATCH v3 1/5] tools: mkeficapsule: add firmwware image signing

[Heinrich Schuchardt]

On 8/31/21 4:46 AM, AKASHI Takahiro wrote:
> With this enhancement, mkeficapsule will be able to sign a capsule
> file when it is created. A signature added will be used later
> in the verification at FMP's SetImage() call.
>
> To do that, We need specify additional command parameters:

nits

%s/need/need to/

>    -monotonic-cout <count> : monotonic count
>    -private-key <private key file> : private key file
>    -certificate <certificate file> : certificate file
> Only when all of those parameters are given, a signature will be added
> to a capsule file.
>
> Users are expected to maintain and increment the monotonic count at

%s/at//

> every time of the update for each firmware image.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   tools/Kconfig        |   7 +
>   tools/Makefile       |   8 +-
>   tools/mkeficapsule.c | 435 +++++++++++++++++++++++++++++++++++++++----
>   3 files changed, 416 insertions(+), 34 deletions(-)
>
> diff --git a/tools/Kconfig b/tools/Kconfig
> index d6f82cd949b5..9a37ed035311 100644
> --- a/tools/Kconfig
> +++ b/tools/Kconfig
> @@ -20,4 +20,11 @@ config TOOLS_LIBCRYPTO
>   	  This selection does not affect target features, such as runtime FIT
>   	  signature verification.
>
> +config TOOLS_MKEFICAPSULE
> +	bool "Build efimkcapsule command"

Having different versions of efimkcapsule with different capabilities is
very confusing for users.

Please, add

	select TOOLS_LIBCRYPTO

> +	default y if EFI_CAPSULE_ON_DISK
> +	help
> +	  This command allows users to create a UEFI capsule file and,
> +	  optionally sign that file. If you want to enable UEFI capsule
> +	  update feature on your target, you certainly need this.
>   endmenu
> diff --git a/tools/Makefile b/tools/Makefile
> index 4a86321f6467..6ea3033dbfb8 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -237,8 +237,12 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs
>   hostprogs-$(CONFIG_ASN1_COMPILER)	+= asn1_compiler
>   HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include
>
> -mkeficapsule-objs	:= mkeficapsule.o $(LIBFDT_OBJS)
> -hostprogs-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += mkeficapsule
> +HOSTLDLIBS_mkeficapsule += -luuid
> +ifeq ($(CONFIG_TOOLS_LIBCRYPTO),y)
> +HOSTLDLIBS_mkeficapsule += \
> +	$(shell pkg-config --libs libssl libcrypto 2> /dev/null || echo "-lssl -lcrypto")
> +endif
> +hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
>
>   # We build some files with extra pedantic flags to try to minimize things
>   # that won't build on some weird host compiler -- though there are lots of
> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> index 4995ba4e0c2a..090fb02de68d 100644
> --- a/tools/mkeficapsule.c
> +++ b/tools/mkeficapsule.c
> @@ -15,6 +15,16 @@
>   #include <sys/stat.h>
>   #include <sys/types.h>
>
> +#include <linux/kconfig.h>
> +#ifdef CONFIG_TOOLS_LIBCRYPTO
> +#include <openssl/asn1.h>
> +#include <openssl/bio.h>
> +#include <openssl/evp.h>
> +#include <openssl/err.h>
> +#include <openssl/pem.h>
> +#include <openssl/pkcs7.h>
> +#endif
> +
>   typedef __u8 u8;
>   typedef __u16 u16;
>   typedef __u32 u32;
> @@ -38,12 +48,25 @@ efi_guid_t efi_guid_image_type_uboot_fit =
>   		EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID;
>   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;
> +
> +#ifdef CONFIG_TOOLS_LIBCRYPTO
> +static const char *opts_short = "f:r:i:I:v:p:c:m:dh";
> +#else
> +static const char *opts_short = "f:r:i:I:v:h";
> +#endif
>
>   static struct option options[] = {
>   	{"fit", required_argument, NULL, 'f'},
>   	{"raw", required_argument, NULL, 'r'},
>   	{"index", required_argument, NULL, 'i'},
>   	{"instance", required_argument, NULL, 'I'},
> +#ifdef CONFIG_TOOLS_LIBCRYPTO
> +	{"private-key", required_argument, NULL, 'p'},
> +	{"certificate", required_argument, NULL, 'c'},
> +	{"monotonic-count", required_argument, NULL, 'm'},
> +	{"dump-sig", no_argument, NULL, 'd'},
> +#endif

If I get you right, you ideentified two use cases:

* unsigned capsules
* signed capsules

Should it be allowable to have unsigned capsules with some board
configurations then mkeficapsule compiled with CONFIG_TOOLS_LIBCRYPTO=y
must be able to produce unsigned capsules.

>   	{"help", no_argument, NULL, 'h'},
>   	{NULL, 0, NULL, 0},
>   };
> @@ -57,16 +80,280 @@ static void print_usage(void)
>   	       "\t-r, --raw <raw image>       new raw image file\n"
>   	       "\t-i, --index <index>         update image index\n"
>   	       "\t-I, --instance <instance>   update hardware instance\n"
> +#ifdef CONFIG_TOOLS_LIBCRYPTO
> +	       "\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"
> +#endif
>   	       "\t-h, --help                  print a help message\n",
>   	       tool_name);
>   }
>
> +/**
> + * auth_context - authentication context
> + * @key_file:	Path to a private key file
> + * @cert_file:	Path to a certificate file
> + * @image_data:	Pointer to firmware data
> + * @image_size:	Size of firmware data
> + * @auth:	Authentication header
> + * @sig_data:	Signature data
> + * @sig_size:	Size of signature data
> + *
> + * Data structure used in create_auth_data(). @key_file through
> + * @image_size are input parameters. @auth, @sig_data and @sig_size
> + * are filled in by create_auth_data().
> + */
> +struct auth_context {
> +	char *key_file;
> +	char *cert_file;
> +	u8 *image_data;
> +	size_t image_size;
> +	struct efi_firmware_image_authentication auth;
> +	u8 *sig_data;
> +	size_t sig_size;
> +};
> +
> +static int dump_sig;
> +
> +#ifdef CONFIG_TOOLS_LIBCRYPTO
> +/**
> + * fileio-read_pkey - read out a private key
> + * @filename:	Path to a private key file
> + *
> + * Read out a private key file and parse it into "EVP_PKEY" structure.
> + *
> + * Return:
> + * * Pointer to private key structure  - on success
> + * * NULL - on failure
> + */
> +static EVP_PKEY *fileio_read_pkey(const char *filename)
> +{
> +	EVP_PKEY *key = NULL;
> +	BIO *bio;
> +
> +	bio = BIO_new_file(filename, "r");
> +	if (!bio)
> +		goto out;
> +
> +	key = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL);
> +
> +out:
> +	BIO_free_all(bio);
> +	if (!key) {
> +		printf("Can't load key from file '%s'\n", filename);
> +		ERR_print_errors_fp(stderr);
> +	}
> +
> +	return key;
> +}
> +
> +/**
> + * fileio-read_cert - read out a certificate
> + * @filename:	Path to a certificate file
> + *
> + * Read out a certificate file and parse it into "X509" structure.
> + *
> + * Return:
> + * * Pointer to certificate structure  - on success
> + * * NULL - on failure
> + */
> +static X509 *fileio_read_cert(const char *filename)
> +{
> +	X509 *cert = NULL;
> +	BIO *bio;
> +
> +	bio = BIO_new_file(filename, "r");
> +	if (!bio)
> +		goto out;
> +
> +	cert = PEM_read_bio_X509(bio, NULL, NULL, NULL);
> +
> +out:
> +	BIO_free_all(bio);
> +	if (!cert) {
> +		printf("Can't load certificate from file '%s'\n", filename);
> +		ERR_print_errors_fp(stderr);
> +	}
> +
> +	return cert;
> +}
> +
> +/**
> + * create_auth_data - compose authentication data in capsule
> + * @auth_context:	Pointer to authentication context
> + *
> + * Fill up an authentication header (.auth) and signature data (.sig_data)
> + * in @auth_context, using library functions from openssl.
> + * All the parameters in @auth_context must be filled in by a caller.
> + *
> + * Return:
> + * * 0  - on success
> + * * -1 - on failure
> + */
> +static int create_auth_data(struct auth_context *ctx)
> +{
> +	EVP_PKEY *key = NULL;
> +	X509 *cert = NULL;
> +	BIO *data_bio = NULL;
> +	const EVP_MD *md;
> +	PKCS7 *p7;
> +	int flags, ret = -1;
> +
> +	OpenSSL_add_all_digests();
> +	OpenSSL_add_all_ciphers();
> +	ERR_load_crypto_strings();
> +
> +	key = fileio_read_pkey(ctx->key_file);
> +	if (!key)
> +		goto err;
> +	cert = fileio_read_cert(ctx->cert_file);
> +	if (!cert)
> +		goto err;
> +
> +	/*
> +	 * create a BIO, containing:
> +	 *  * firmware image
> +	 *  * monotonic count
> +	 * in this order!
> +	 * See EDK2's FmpAuthenticatedHandlerRsa2048Sha256()
> +	 */
> +	data_bio = BIO_new(BIO_s_mem());
> +	BIO_write(data_bio, ctx->image_data, ctx->image_size);
> +	BIO_write(data_bio, &ctx->auth.monotonic_count,
> +		  sizeof(ctx->auth.monotonic_count));
> +
> +	md = EVP_get_digestbyname("SHA256");
> +	if (!md)
> +		goto err;
> +
> +	/* create signature */
> +	/* TODO: maybe add PKCS7_NOATTR and PKCS7_NOSMIMECAP */
> +	flags = PKCS7_BINARY | PKCS7_DETACHED;
> +	p7 = PKCS7_sign(NULL, NULL, NULL, data_bio, flags | PKCS7_PARTIAL);
> +	if (!p7)
> +		goto err;
> +	if (!PKCS7_sign_add_signer(p7, cert, key, md, flags))
> +		goto err;
> +	if (!PKCS7_final(p7, data_bio, flags))
> +		goto err;
> +
> +	/* convert pkcs7 into DER */
> +	ctx->sig_data = NULL;
> +	ctx->sig_size = ASN1_item_i2d((ASN1_VALUE *)p7, &ctx->sig_data,
> +				      ASN1_ITEM_rptr(PKCS7));
> +	if (!ctx->sig_size)
> +		goto err;
> +
> +	/* fill auth_info */
> +	ctx->auth.auth_info.hdr.dwLength = sizeof(ctx->auth.auth_info)
> +						+ ctx->sig_size;
> +	ctx->auth.auth_info.hdr.wRevision = WIN_CERT_REVISION_2_0;
> +	ctx->auth.auth_info.hdr.wCertificateType = WIN_CERT_TYPE_EFI_GUID;
> +	memcpy(&ctx->auth.auth_info.cert_type, &efi_guid_cert_type_pkcs7,
> +	       sizeof(efi_guid_cert_type_pkcs7));
> +
> +	ret = 0;
> +err:
> +	BIO_free_all(data_bio);
> +	EVP_PKEY_free(key);
> +	X509_free(cert);
> +
> +	return ret;
> +}
> +
> +/**
> + * dump_signature - dump out a signature
> + * @path:	Path to a capsule file
> + * @signature:	Signature data
> + * @sig_size:	Size of signature data
> + *
> + * Signature data pointed to by @signature will be saved into
> + * a file whose file name is @path with ".p7" suffix.
> + *
> + * Return:
> + * * 0  - on success
> + * * -1 - on failure
> + */
> +static int dump_signature(const char *path, u8 *signature, size_t sig_size)
> +{
> +	char *sig_path;
> +	FILE *f;
> +	size_t size;
> +	int ret = -1;
> +
> +	sig_path = malloc(strlen(path) + 3 + 1);
> +	if (!sig_path)
> +		return ret;
> +
> +	sprintf(sig_path, "%s.p7", path);
> +	f = fopen(sig_path, "w");
> +	if (!f)
> +		goto err;
> +
> +	size = fwrite(signature, 1, sig_size, f);
> +	if (size == sig_size)
> +		ret = 0;
> +
> +	fclose(f);
> +err:
> +	free(sig_path);
> +	return ret;
> +}
> +
> +/**
> + * free_sig_data - free out signature data
> + * @ctx:	Pointer to authentication context
> + *
> + * Free signature data allocated in create_auth_data().
> + */
> +static void free_sig_data(struct auth_context *ctx)
> +{
> +	if (ctx->sig_size)
> +		OPENSSL_free(ctx->sig_data);
> +}
> +#else
> +static int create_auth_data(struct auth_context *ctx)
> +{
> +	return 0;
> +}
> +
> +static int dump_signature(const char *path, u8 *signature, size_t sig_size)
> +{
> +	return 0;
> +}
> +
> +static void free_sig_data(struct auth_context *ctx) {}
> +#endif
> +
> +/**
> + * create_fwbin - create an uefi capsule file
> + * @path:	Path to a created capsule file
> + * @bin:	Path to a firmware binary to encapsulate
> + * @guid:	GUID of related FMP driver
> + * @index:	Index number in capsule
> + * @instance:	Instance number in capsule
> + * @mcount:	Monolitic count in authentication information
> + * @private_file:	Path to a private key file
> + * @cert_file:	Path to a certificate file
> + *
> + * This function actually does the job of creating an uefi capsule file.
> + * All the arguments must be supplied.
> + * If either @private_file ror @cert_file is NULL, the capsule file
> + * won't be signed.
> + *
> + * Return:
> + * * 0  - on success
> + * * -1 - on failure
> + */
>   static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
> -			unsigned long index, unsigned long instance)
> +			unsigned long index, unsigned long instance,
> +			uint64_t mcount, char *privkey_file, char *cert_file)
>   {
>   	struct efi_capsule_header header;
>   	struct efi_firmware_management_capsule_header capsule;
>   	struct efi_firmware_management_capsule_image_header image;
> +	struct auth_context auth_context;
>   	FILE *f, *g;
>   	struct stat bin_stat;
>   	u8 *data;
> @@ -76,8 +363,9 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
>   #ifdef DEBUG
>   	printf("For output: %s\n", path);
>   	printf("\tbin: %s\n\ttype: %pUl\n", bin, guid);
> -	printf("\tindex: %ld\n\tinstance: %ld\n", index, instance);
> +	printf("\tindex: %lu\n\tinstance: %lu\n", index, instance);
>   #endif
> +	auth_context.sig_size = 0;
>
>   	g = fopen(bin, "r");
>   	if (!g) {
> @@ -93,11 +381,34 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
>   		printf("cannot allocate memory: %zx\n", (size_t)bin_stat.st_size);
>   		goto err_1;
>   	}
> -	f = fopen(path, "w");
> -	if (!f) {
> -		printf("cannot open %s\n", path);
> +
> +	size = fread(data, 1, bin_stat.st_size, g);
> +	if (size < bin_stat.st_size) {
> +		printf("read failed (%zx)\n", size);
>   		goto err_2;
>   	}
> +
> +	/* first, calculate signature to determine its size */
> +	if (privkey_file && cert_file) {
> +		auth_context.key_file = privkey_file;
> +		auth_context.cert_file = cert_file;
> +		auth_context.auth.monotonic_count = mcount;
> +		auth_context.image_data = data;
> +		auth_context.image_size = bin_stat.st_size;
> +
> +		if (create_auth_data(&auth_context)) {
> +			printf("Signing firmware image failed\n");
> +			goto err_3;
> +		}
> +
> +		if (dump_sig &&
> +		    dump_signature(path, auth_context.sig_data,
> +				   auth_context.sig_size)) {
> +			printf("Creating signature file failed\n");
> +			goto err_3;
> +		}
> +	}
> +
>   	header.capsule_guid = efi_guid_fm_capsule;
>   	header.header_size = sizeof(header);
>   	/* TODO: The current implementation ignores flags */
> @@ -106,11 +417,20 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
>   					+ sizeof(capsule) + sizeof(u64)
>   					+ sizeof(image)
>   					+ bin_stat.st_size;
> +	if (auth_context.sig_size)
> +		header.capsule_image_size += sizeof(auth_context.auth)
> +				+ auth_context.sig_size;
> +
> +	f = fopen(path, "w");
> +	if (!f) {
> +		printf("cannot open %s\n", path);
> +		goto err_3;
> +	}
>
>   	size = fwrite(&header, 1, sizeof(header), f);
>   	if (size < sizeof(header)) {
>   		printf("write failed (%zx)\n", size);
> -		goto err_3;
> +		goto err_4;
>   	}
>
>   	capsule.version = 0x00000001;
> @@ -119,13 +439,13 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
>   	size = fwrite(&capsule, 1, sizeof(capsule), f);
>   	if (size < (sizeof(capsule))) {
>   		printf("write failed (%zx)\n", size);
> -		goto err_3;
> +		goto err_4;
>   	}
>   	offset = sizeof(capsule) + sizeof(u64);
>   	size = fwrite(&offset, 1, sizeof(offset), f);
>   	if (size < sizeof(offset)) {
>   		printf("write failed (%zx)\n", size);
> -		goto err_3;
> +		goto err_4;
>   	}
>
>   	image.version = 0x00000003;
> @@ -135,34 +455,53 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
>   	image.reserved[1] = 0;
>   	image.reserved[2] = 0;
>   	image.update_image_size = bin_stat.st_size;
> +	if (auth_context.sig_size)
> +		image.update_image_size += sizeof(auth_context.auth)
> +				+ auth_context.sig_size;
>   	image.update_vendor_code_size = 0; /* none */
>   	image.update_hardware_instance = instance;
>   	image.image_capsule_support = 0;
> +	if (auth_context.sig_size)
> +		image.image_capsule_support |= CAPSULE_SUPPORT_AUTHENTICATION;
>
>   	size = fwrite(&image, 1, sizeof(image), f);
>   	if (size < sizeof(image)) {
>   		printf("write failed (%zx)\n", size);
> -		goto err_3;
> +		goto err_4;
>   	}
> -	size = fread(data, 1, bin_stat.st_size, g);
> -	if (size < bin_stat.st_size) {
> -		printf("read failed (%zx)\n", size);
> -		goto err_3;
> +
> +	if (auth_context.sig_size) {
> +		size = fwrite(&auth_context.auth, 1,
> +			      sizeof(auth_context.auth), f);
> +		if (size < sizeof(auth_context.auth)) {
> +			printf("write failed (%zx)\n", size);
> +			goto err_4;
> +		}
> +		size = fwrite(auth_context.sig_data, 1,
> +			      auth_context.sig_size, f);
> +		if (size < auth_context.sig_size) {
> +			printf("write failed (%zx)\n", size);
> +			goto err_4;
> +		}
>   	}
> +
>   	size = fwrite(data, 1, bin_stat.st_size, f);
>   	if (size < bin_stat.st_size) {
>   		printf("write failed (%zx)\n", size);
> -		goto err_3;
> +		goto err_4;
>   	}
>
>   	fclose(f);
>   	fclose(g);
>   	free(data);
> +	free_sig_data(&auth_context);
>
>   	return 0;
>
> -err_3:
> +err_4:
>   	fclose(f);
> +err_3:
> +	free_sig_data(&auth_context);
>   err_2:
>   	free(data);
>   err_1:
> @@ -171,23 +510,37 @@ err_1:
>   	return -1;
>   }
>
> -/*
> - * Usage:
> - *   $ mkeficapsule -f <firmware binary> <output file>
> +/**
> + * main - main entry function of mkeficapsule
> + * @argc:	Number of arguments
> + * @argv:	Array of pointers to arguments
> + *
> + * Create an uefi capsule file, optionally signing it.
> + * Parse all the arguments and pass them on to create_fwbin().
> + *
> + * Return:
> + * * 0  - on success
> + * * -1 - on failure
>    */
>   int main(int argc, char **argv)
>   {
>   	char *file;
>   	efi_guid_t *guid;
>   	unsigned long index, instance;
> +	uint64_t mcount;
> +	char *privkey_file, *cert_file;
>   	int c, idx;
>
>   	file = NULL;
>   	guid = NULL;
>   	index = 0;
>   	instance = 0;
> +	mcount = 0;
> +	privkey_file = NULL;
> +	cert_file = NULL;
> +	dump_sig = 0;
>   	for (;;) {
> -		c = getopt_long(argc, argv, "f:r:i:I:v:h", options, &idx);
> +		c = getopt_long(argc, argv, opts_short, options, &idx);
>   		if (c == -1)
>   			break;
>
> @@ -214,29 +567,47 @@ int main(int argc, char **argv)
>   		case 'I':
>   			instance = strtoul(optarg, NULL, 0);
>   			break;
> +#ifdef CONFIG_TOOLS_LIBCRYPTO
> +		case 'p':
> +			if (privkey_file) {
> +				printf("Private Key already specified\n");
> +				return -1;
> +			}
> +			privkey_file = optarg;
> +			break;
> +		case 'c':
> +			if (cert_file) {
> +				printf("Certificate file already specified\n");
> +				return -1;
> +			}
> +			cert_file = optarg;
> +			break;
> +		case 'm':
> +			mcount = strtoul(optarg, NULL, 0);
> +			break;
> +		case 'd':
> +			dump_sig = 1;
> +			break;
> +#endif /* CONFIG_TOOLS_LIBCRYPTO */
>   		case 'h':
>   			print_usage();
>   			return 0;
>   		}
>   	}
>
> -	/* need an output file */
> -	if (argc != optind + 1) {
> -		print_usage();
> -		exit(EXIT_FAILURE);
> -	}
> -
> -	/* need a fit image file or raw image file */
> -	if (!file) {
> +	/* check necessary parameters */
> +	if ((argc != optind + 1) || !file ||

This needs to be changed to support signed and unsigned capsules.

> +	    ((privkey_file && !cert_file) ||
> +	     (!privkey_file && cert_file))) {
>   		print_usage();
> -		exit(EXIT_SUCCESS);
> +		return -1;

Please use EXIT_FALURE.

>   	}
>
> -	if (create_fwbin(argv[optind], file, guid, index, instance)
> -			< 0) {
> +	if (create_fwbin(argv[optind], file, guid, index, instance,
> +			 mcount, privkey_file, cert_file) < 0) {
>   		printf("Creating firmware capsule failed\n");
> -		exit(EXIT_FAILURE);
> +		return -1;

return EXIT_FAILURE;

>   	}
>
> -	exit(EXIT_SUCCESS);
> +	return 0;

In main, please, use

	return EXIT_SUCCESS;

Best regards

Heinrich

>   }
>

[PATCH v3 2/5] tools: mkeficapsule: add man page

[AKASHI Takahiro]

Add a man page for mkeficapsule command.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 MAINTAINERS        |  1 +
 doc/mkeficapsule.1 | 91 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+)
 create mode 100644 doc/mkeficapsule.1

diff --git a/MAINTAINERS b/MAINTAINERS
index 4cf0c33c5d58..0bb2de007982 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -701,6 +701,7 @@ S:	Maintained
 T:	git https://source.denx.de/u-boot/custodians/u-boot-efi.git
 F:	doc/api/efi.rst
 F:	doc/develop/uefi/*
+F:	doc/mkeficapsule.1
 F:	doc/usage/bootefi.rst
 F:	drivers/rtc/emul_rtc.c
 F:	include/capitalization.h
diff --git a/doc/mkeficapsule.1 b/doc/mkeficapsule.1
new file mode 100644
index 000000000000..7c2341160ea4
--- /dev/null
+++ b/doc/mkeficapsule.1
@@ -0,0 +1,91 @@
+.TH MAEFICAPSULE 1 "May 2021"
+
+.SH NAME
+mkeficapsule \- Generate EFI capsule file for U-Boot
+
+.SH SYNOPSIS
+.B mkeficapsule
+.RB [\fIoptions\fP] " \fIcapsule-file\fP"
+
+.SH "DESCRIPTION"
+The
+\fBmkeficapsule\fP
+command is used to create an EFI capsule file for use with the U-Boot
+EFI capsule update.
+A capsule file may contain various type of firmware blobs which
+are to be applied to the system and must be placed in the specific
+directory on the UEFI system partition. An update will be automatically
+executed at next reboot.
+
+Optionally, a capsule file can be signed with a given private key.
+In this case, the update will be authenticated by verifying the signature
+before applying.
+
+\fBmkeficapsule\fP supports two different format of image files:
+.TP
+.I raw image
+format is a single binary blob of any type of firmware.
+
+.TP
+.I FIT (Flattened Image Tree) image
+format
+is the same as used in the new \fIuImage\fP format and allows for
+multiple binary blobs in a single capsule file.
+This type of image file can be generated by \fBmkimage\fP.
+
+.SH "OPTIONS"
+
+.TP
+.BI "-f, --fit \fIfit-image-file\fP"
+Specify a FIT image file
+
+.TP
+.BI "-r, --raw \fIraw-image-file\fP"
+Specify a raw image file
+
+.TP
+.BI "-i, --index \fIindex\fP"
+Specify an image index
+
+.TP
+.BI "-I, --instance \fIinstance\fP"
+Specify a hardware instance
+
+.TP
+.BI "-h, --help"
+Print a help message
+
+.TP 0
+.B With signing:
+
+.TP
+.BI "-p, --private-key \fIprivate-key-file\fP"
+Specify signer's private key file in PEM
+
+.TP
+.BI "-c, --certificate \fIcertificate-file\fP"
+Specify signer's certificate file in EFI certificate list format
+
+.TP
+.BI "-m, --monotonic-count \fIcount\fP"
+Specify a monotonic count which is set to be monotonically incremented
+at every firmware update.
+
+.TP
+.BI "-d, --dump_sig"
+Dump signature data into *.p7 file
+
+.PP
+.SH FILES
+.TP
+.BI "\fI/EFI/UpdateCapsule\fP"
+The directory in which all capsule files be placed
+
+.SH SEE ALSO
+.B mkimage
+
+.SH AUTHORS
+Written by AKASHI Takahiro <takahiro.akashi@linaro.org>
+
+.SH HOMEPAGE
+http://www.denx.de/wiki/U-Boot/WebHome
-- 
2.33.0

Re: [PATCH v3 2/5] tools: mkeficapsule: add man page

[Heinrich Schuchardt]

On 8/31/21 4:46 AM, AKASHI Takahiro wrote:
> Add a man page for mkeficapsule command.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   MAINTAINERS        |  1 +
>   doc/mkeficapsule.1 | 91 ++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 92 insertions(+)
>   create mode 100644 doc/mkeficapsule.1
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4cf0c33c5d58..0bb2de007982 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -701,6 +701,7 @@ S:	Maintained
>   T:	git https://source.denx.de/u-boot/custodians/u-boot-efi.git
>   F:	doc/api/efi.rst
>   F:	doc/develop/uefi/*
> +F:	doc/mkeficapsule.1
>   F:	doc/usage/bootefi.rst
>   F:	drivers/rtc/emul_rtc.c
>   F:	include/capitalization.h
> diff --git a/doc/mkeficapsule.1 b/doc/mkeficapsule.1
> new file mode 100644
> index 000000000000..7c2341160ea4
> --- /dev/null
> +++ b/doc/mkeficapsule.1
> @@ -0,0 +1,91 @@

Please, add license information.

.\" SPDX-License-Identifier: GPL-2.0
.\"

or

.\" SPDX-License-Identifier: CC-BY-SA-4.0
.\"

> +.TH MAEFICAPSULE 1 "May 2021"
> +
> +.SH NAME
> +mkeficapsule \- Generate EFI capsule file for U-Boot
> +
> +.SH SYNOPSIS
> +.B mkeficapsule
> +.RB [\fIoptions\fP] " \fIcapsule-file\fP"
> +
> +.SH "DESCRIPTION"
> +The
> +\fBmkeficapsule\fP
> +command is used to create an EFI capsule file for use with the U-Boot
> +EFI capsule update.
> +A capsule file may contain various type of firmware blobs which
> +are to be applied to the system and must be placed in the specific
> +directory on the UEFI system partition. An update will be automatically
> +executed at next reboot.
> +
> +Optionally, a capsule file can be signed with a given private key.
> +In this case, the update will be authenticated by verifying the signature
> +before applying.
> +
> +\fBmkeficapsule\fP supports two different format of image files:
> +.TP
> +.I raw image
> +format is a single binary blob of any type of firmware.
> +
> +.TP
> +.I FIT (Flattened Image Tree) image
> +format
> +is the same as used in the new \fIuImage\fP format and allows for
> +multiple binary blobs in a single capsule file.
> +This type of image file can be generated by \fBmkimage\fP.
> +
> +.SH "OPTIONS"
> +
> +.TP
> +.BI "-f, --fit \fIfit-image-file\fP"
> +Specify a FIT image file
> +
> +.TP
> +.BI "-r, --raw \fIraw-image-file\fP"
> +Specify a raw image file
> +
> +.TP
> +.BI "-i, --index \fIindex\fP"
> +Specify an image index
> +
> +.TP
> +.BI "-I, --instance \fIinstance\fP"
> +Specify a hardware instance
> +
> +.TP
> +.BI "-h, --help"
> +Print a help message
> +
> +.TP 0
> +.B With signing:
> +
> +.TP
> +.BI "-p, --private-key \fIprivate-key-file\fP"
> +Specify signer's private key file in PEM
> +
> +.TP
> +.BI "-c, --certificate \fIcertificate-file\fP"
> +Specify signer's certificate file in EFI certificate list format
> +
> +.TP
> +.BI "-m, --monotonic-count \fIcount\fP"
> +Specify a monotonic count which is set to be monotonically incremented
> +at every firmware update.
> +
> +.TP
> +.BI "-d, --dump_sig"
> +Dump signature data into *.p7 file

Please, describe which parameters are obligatory for

* signed capsules
* unsigned capsules

Best regards

Heinrich

> +
> +.PP
> +.SH FILES
> +.TP
> +.BI "\fI/EFI/UpdateCapsule\fP"
> +The directory in which all capsule files be placed
> +
> +.SH SEE ALSO
> +.B mkimage
> +
> +.SH AUTHORS
> +Written by AKASHI Takahiro <takahiro.akashi@linaro.org>
> +
> +.SH HOMEPAGE
> +http://www.denx.de/wiki/U-Boot/WebHome
>

[PATCH v3 3/5] doc: update UEFI document for usage of mkeficapsule

[AKASHI Takahiro]

Now we can use mkeficapsule command instead of EDK-II's script
to create a signed capsule file. So update the instruction for
capsule authentication.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 doc/develop/uefi/uefi.rst | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index 64fe9346c7f2..5ccb455da984 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -347,23 +347,20 @@ and used by the steps highlighted below::
             -keyout CRT.key -out CRT.crt -nodes -days 365
         $ cert-to-efi-sig-list CRT.crt CRT.esl
 
-        $ openssl x509 -in CRT.crt -out CRT.cer -outform DER
-        $ openssl x509 -inform DER -in CRT.cer -outform PEM -out CRT.pub.pem
-
-        $ openssl pkcs12 -export -out CRT.pfx -inkey CRT.key -in CRT.crt
-        $ openssl pkcs12 -in CRT.pfx -nodes -out CRT.pem
-
-The capsule file can be generated by using the GenerateCapsule.py
-script in EDKII::
-
-    $ ./BaseTools/BinWrappers/PosixLike/GenerateCapsule -e -o \
-      <capsule_file_name> --monotonic-count <val> --fw-version \
-      <val> --lsv <val> --guid \
-      e2bb9c06-70e9-4b14-97a3-5a7913176e3f --verbose \
-      --update-image-index <val> --signer-private-cert \
-      /path/to/CRT.pem --trusted-public-cert \
-      /path/to/CRT.pub.pem --other-public-cert /path/to/CRT.pub.pem \
-      <u-boot.bin>
+The signed capsule file can be generated by using tools/mkeficapsule.
+To build this tool, enable::
+
+    CONFIG_TOOLS_MKEFICAPSULE=y
+    CONFIG_TOOLS_LIBCRYPTO=y
+
+To generate and sign the capsule file::
+
+    $ mkeficapsule --monotonic-count 1 \
+      --private-key CRT.key \
+      --certificate CRT.crt \
+      --index 1 --instance 0 \
+      [--fit <FIT image> | --raw <raw image>] \
+      <capsule_file_name>
 
 Place the capsule generated in the above step on the EFI System
 Partition under the EFI/UpdateCapsule directory
-- 
2.33.0

Re: [PATCH v3 3/5] doc: update UEFI document for usage of mkeficapsule

[Heinrich Schuchardt]

On 8/31/21 4:46 AM, AKASHI Takahiro wrote:
> Now we can use mkeficapsule command instead of EDK-II's script
> to create a signed capsule file. So update the instruction for
> capsule authentication.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   doc/develop/uefi/uefi.rst | 31 ++++++++++++++-----------------
>   1 file changed, 14 insertions(+), 17 deletions(-)
>
> diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> index 64fe9346c7f2..5ccb455da984 100644
> --- a/doc/develop/uefi/uefi.rst
> +++ b/doc/develop/uefi/uefi.rst
> @@ -347,23 +347,20 @@ and used by the steps highlighted below::
>               -keyout CRT.key -out CRT.crt -nodes -days 365
>           $ cert-to-efi-sig-list CRT.crt CRT.esl
>
> -        $ openssl x509 -in CRT.crt -out CRT.cer -outform DER
> -        $ openssl x509 -inform DER -in CRT.cer -outform PEM -out CRT.pub.pem
> -
> -        $ openssl pkcs12 -export -out CRT.pfx -inkey CRT.key -in CRT.crt
> -        $ openssl pkcs12 -in CRT.pfx -nodes -out CRT.pem
> -
> -The capsule file can be generated by using the GenerateCapsule.py
> -script in EDKII::
> -
> -    $ ./BaseTools/BinWrappers/PosixLike/GenerateCapsule -e -o \
> -      <capsule_file_name> --monotonic-count <val> --fw-version \
> -      <val> --lsv <val> --guid \
> -      e2bb9c06-70e9-4b14-97a3-5a7913176e3f --verbose \
> -      --update-image-index <val> --signer-private-cert \
> -      /path/to/CRT.pem --trusted-public-cert \
> -      /path/to/CRT.pub.pem --other-public-cert /path/to/CRT.pub.pem \
> -      <u-boot.bin>
> +The signed capsule file can be generated by using tools/mkeficapsule.
> +To build this tool, enable::
> +
> +    CONFIG_TOOLS_MKEFICAPSULE=y
> +    CONFIG_TOOLS_LIBCRYPTO=y
> +
> +To generate and sign the capsule file::
> +
> +    $ mkeficapsule --monotonic-count 1 \
> +      --private-key CRT.key \
> +      --certificate CRT.crt \
> +      --index 1 --instance 0 \
> +      [--fit <FIT image> | --raw <raw image>] \
> +      <capsule_file_name>

Patch 1 allows signed and unsigned capsules. So both should be described
here.

Best regards

Heinrich

>
>   Place the capsule generated in the above step on the EFI System
>   Partition under the EFI/UpdateCapsule directory
>

Re: [PATCH v3 3/5] doc: update UEFI document for usage of mkeficapsule

[AKASHI Takahiro]

On Tue, Aug 31, 2021 at 08:07:18AM +0200, Heinrich Schuchardt wrote:
> On 8/31/21 4:46 AM, AKASHI Takahiro wrote:
> > Now we can use mkeficapsule command instead of EDK-II's script
> > to create a signed capsule file. So update the instruction for
> > capsule authentication.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >   doc/develop/uefi/uefi.rst | 31 ++++++++++++++-----------------
> >   1 file changed, 14 insertions(+), 17 deletions(-)
> > 
> > diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> > index 64fe9346c7f2..5ccb455da984 100644
> > --- a/doc/develop/uefi/uefi.rst
> > +++ b/doc/develop/uefi/uefi.rst
> > @@ -347,23 +347,20 @@ and used by the steps highlighted below::
> >               -keyout CRT.key -out CRT.crt -nodes -days 365
> >           $ cert-to-efi-sig-list CRT.crt CRT.esl
> > 
> > -        $ openssl x509 -in CRT.crt -out CRT.cer -outform DER
> > -        $ openssl x509 -inform DER -in CRT.cer -outform PEM -out CRT.pub.pem
> > -
> > -        $ openssl pkcs12 -export -out CRT.pfx -inkey CRT.key -in CRT.crt
> > -        $ openssl pkcs12 -in CRT.pfx -nodes -out CRT.pem
> > -
> > -The capsule file can be generated by using the GenerateCapsule.py
> > -script in EDKII::
> > -
> > -    $ ./BaseTools/BinWrappers/PosixLike/GenerateCapsule -e -o \
> > -      <capsule_file_name> --monotonic-count <val> --fw-version \
> > -      <val> --lsv <val> --guid \
> > -      e2bb9c06-70e9-4b14-97a3-5a7913176e3f --verbose \
> > -      --update-image-index <val> --signer-private-cert \
> > -      /path/to/CRT.pem --trusted-public-cert \
> > -      /path/to/CRT.pub.pem --other-public-cert /path/to/CRT.pub.pem \
> > -      <u-boot.bin>
> > +The signed capsule file can be generated by using tools/mkeficapsule.
> > +To build this tool, enable::
> > +
> > +    CONFIG_TOOLS_MKEFICAPSULE=y
> > +    CONFIG_TOOLS_LIBCRYPTO=y
> > +
> > +To generate and sign the capsule file::
> > +
> > +    $ mkeficapsule --monotonic-count 1 \
> > +      --private-key CRT.key \
> > +      --certificate CRT.crt \
> > +      --index 1 --instance 0 \
> > +      [--fit <FIT image> | --raw <raw image>] \
> > +      <capsule_file_name>
> 
> Patch 1 allows signed and unsigned capsules. So both should be described
> here.

Sughosh's original doc doesn't have a description for *unsigned* case,
but adding some is not a big deal.

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> > 
> >   Place the capsule generated in the above step on the EFI System
> >   Partition under the EFI/UpdateCapsule directory
> > 
> 

[PATCH v3 4/5] tools: mkeficapsule: allow for specifying GUID explicitly

[AKASHI Takahiro]

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>
---
 doc/develop/uefi/uefi.rst |  4 ++--
 doc/mkeficapsule.1        | 19 +++++++++++-----
 tools/mkeficapsule.c      | 46 ++++++++++++++++++++++++---------------
 3 files changed, 43 insertions(+), 26 deletions(-)

diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
index 5ccb455da984..c23fc2125fb2 100644
--- a/doc/develop/uefi/uefi.rst
+++ b/doc/develop/uefi/uefi.rst
@@ -359,8 +359,8 @@ To generate and sign the capsule file::
       --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] \
+      <binary_blob> <capsule_file_name>
 
 Place the capsule generated in the above step on the EFI System
 Partition under the EFI/UpdateCapsule directory
diff --git a/doc/mkeficapsule.1 b/doc/mkeficapsule.1
index 7c2341160ea4..ab2aa3719744 100644
--- a/doc/mkeficapsule.1
+++ b/doc/mkeficapsule.1
@@ -5,7 +5,7 @@ mkeficapsule \- Generate EFI capsule file for U-Boot
 
 .SH SYNOPSIS
 .B mkeficapsule
-.RB [\fIoptions\fP] " \fIcapsule-file\fP"
+.RB [\fIoptions\fP] " \fIimage-blob\fP \fIcapsule-file\fP"
 
 .SH "DESCRIPTION"
 The
@@ -21,7 +21,7 @@ Optionally, a capsule file can be signed with a given private key.
 In this case, the update will be authenticated by verifying the signature
 before applying.
 
-\fBmkeficapsule\fP supports two different format of image files:
+\fBmkeficapsule\fP takes any type of image files, including:
 .TP
 .I raw image
 format is a single binary blob of any type of firmware.
@@ -33,15 +33,22 @@ is the same as used in the new \fIuImage\fP format and allows for
 multiple binary blobs in a single capsule file.
 This type of image file can be generated by \fBmkimage\fP.
 
+If you want to use other types than above two, you should explicitly
+specify a guid for the FMP driver.
+
 .SH "OPTIONS"
 
 .TP
-.BI "-f, --fit \fIfit-image-file\fP"
-Specify a FIT image file
+.BI "-f, --fit
+Indicate that the blob is a FIT image file
+
+.TP
+.BI "-r, --raw
+Indicate that the blob is a raw image file
 
 .TP
-.BI "-r, --raw \fIraw-image-file\fP"
-Specify a raw image file
+.BI "-g, --guid \fIguid-string\fP"
+Specify guid for image blob type
 
 .TP
 .BI "-i, --index \fIindex\fP"
diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
index 090fb02de68d..c730cf468d2b 100644
--- a/tools/mkeficapsule.c
+++ b/tools/mkeficapsule.c
@@ -14,7 +14,7 @@
 
 #include <sys/stat.h>
 #include <sys/types.h>
-
+#include <uuid/uuid.h>
 #include <linux/kconfig.h>
 #ifdef CONFIG_TOOLS_LIBCRYPTO
 #include <openssl/asn1.h>
@@ -51,14 +51,15 @@ efi_guid_t efi_guid_image_type_uboot_raw =
 efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
 
 #ifdef CONFIG_TOOLS_LIBCRYPTO
-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";
 #else
-static const char *opts_short = "f:r:i:I:v:h";
+static const char *opts_short = "frg:i:I:v:h";
 #endif
 
 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'},
 #ifdef CONFIG_TOOLS_LIBCRYPTO
@@ -73,11 +74,12 @@ static struct option options[] = {
 
 static void print_usage(void)
 {
-	printf("Usage: %s [options] <output file>\n"
+	printf("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"
 #ifdef CONFIG_TOOLS_LIBCRYPTO
@@ -524,14 +526,13 @@ err_1:
  */
 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;
@@ -546,21 +547,30 @@ int main(int argc, char **argv)
 
 		switch (c) {
 		case 'f':
-			if (file) {
-				printf("Image already specified\n");
+			if (guid) {
+				printf("Image type already specified\n");
 				return -1;
 			}
-			file = optarg;
 			guid = &efi_guid_image_type_uboot_fit;
 			break;
 		case 'r':
-			if (file) {
-				printf("Image already specified\n");
+			if (guid) {
+				printf("Image type already specified\n");
 				return -1;
 			}
-			file = optarg;
 			guid = &efi_guid_image_type_uboot_raw;
 			break;
+		case 'g':
+			if (guid) {
+				printf("Image type already specified\n");
+				return -1;
+			}
+			if (uuid_parse(optarg, uuid_buf)) {
+				printf("Wrong guid format\n");
+				return -1;
+			}
+			guid = (efi_guid_t *)uuid_buf;
+			break;
 		case 'i':
 			index = strtoul(optarg, NULL, 0);
 			break;
@@ -596,14 +606,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();
 		return -1;
 	}
 
-	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) {
 		printf("Creating firmware capsule failed\n");
 		return -1;
-- 
2.33.0

Re: [PATCH v3 4/5] tools: mkeficapsule: allow for specifying GUID explicitly

[Heinrich Schuchardt]

On 8/31/21 4:46 AM, 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>
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

> ---
>   doc/develop/uefi/uefi.rst |  4 ++--
>   doc/mkeficapsule.1        | 19 +++++++++++-----
>   tools/mkeficapsule.c      | 46 ++++++++++++++++++++++++---------------
>   3 files changed, 43 insertions(+), 26 deletions(-)
>
> diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> index 5ccb455da984..c23fc2125fb2 100644
> --- a/doc/develop/uefi/uefi.rst
> +++ b/doc/develop/uefi/uefi.rst
> @@ -359,8 +359,8 @@ To generate and sign the capsule file::
>         --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] \
> +      <binary_blob> <capsule_file_name>
>
>   Place the capsule generated in the above step on the EFI System
>   Partition under the EFI/UpdateCapsule directory
> diff --git a/doc/mkeficapsule.1 b/doc/mkeficapsule.1
> index 7c2341160ea4..ab2aa3719744 100644
> --- a/doc/mkeficapsule.1
> +++ b/doc/mkeficapsule.1
> @@ -5,7 +5,7 @@ mkeficapsule \- Generate EFI capsule file for U-Boot
>
>   .SH SYNOPSIS
>   .B mkeficapsule
> -.RB [\fIoptions\fP] " \fIcapsule-file\fP"
> +.RB [\fIoptions\fP] " \fIimage-blob\fP \fIcapsule-file\fP"
>
>   .SH "DESCRIPTION"
>   The
> @@ -21,7 +21,7 @@ Optionally, a capsule file can be signed with a given private key.
>   In this case, the update will be authenticated by verifying the signature
>   before applying.
>
> -\fBmkeficapsule\fP supports two different format of image files:
> +\fBmkeficapsule\fP takes any type of image files, including:
>   .TP
>   .I raw image
>   format is a single binary blob of any type of firmware.
> @@ -33,15 +33,22 @@ is the same as used in the new \fIuImage\fP format and allows for
>   multiple binary blobs in a single capsule file.
>   This type of image file can be generated by \fBmkimage\fP.
>
> +If you want to use other types than above two, you should explicitly
> +specify a guid for the FMP driver.
> +
>   .SH "OPTIONS"
>
>   .TP
> -.BI "-f, --fit \fIfit-image-file\fP"
> -Specify a FIT image file
> +.BI "-f, --fit
> +Indicate that the blob is a FIT image file
> +
> +.TP
> +.BI "-r, --raw
> +Indicate that the blob is a raw image file
>
>   .TP
> -.BI "-r, --raw \fIraw-image-file\fP"
> -Specify a raw image file
> +.BI "-g, --guid \fIguid-string\fP"
> +Specify guid for image blob type
>
>   .TP
>   .BI "-i, --index \fIindex\fP"
> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> index 090fb02de68d..c730cf468d2b 100644
> --- a/tools/mkeficapsule.c
> +++ b/tools/mkeficapsule.c
> @@ -14,7 +14,7 @@
>
>   #include <sys/stat.h>
>   #include <sys/types.h>
> -
> +#include <uuid/uuid.h>
>   #include <linux/kconfig.h>
>   #ifdef CONFIG_TOOLS_LIBCRYPTO
>   #include <openssl/asn1.h>
> @@ -51,14 +51,15 @@ efi_guid_t efi_guid_image_type_uboot_raw =
>   efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
>
>   #ifdef CONFIG_TOOLS_LIBCRYPTO
> -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";
>   #else
> -static const char *opts_short = "f:r:i:I:v:h";
> +static const char *opts_short = "frg:i:I:v:h";
>   #endif
>
>   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'},
>   #ifdef CONFIG_TOOLS_LIBCRYPTO
> @@ -73,11 +74,12 @@ static struct option options[] = {
>
>   static void print_usage(void)
>   {
> -	printf("Usage: %s [options] <output file>\n"
> +	printf("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"
>   #ifdef CONFIG_TOOLS_LIBCRYPTO
> @@ -524,14 +526,13 @@ err_1:
>    */
>   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;
> @@ -546,21 +547,30 @@ int main(int argc, char **argv)
>
>   		switch (c) {
>   		case 'f':
> -			if (file) {
> -				printf("Image already specified\n");
> +			if (guid) {
> +				printf("Image type already specified\n");
>   				return -1;
>   			}
> -			file = optarg;
>   			guid = &efi_guid_image_type_uboot_fit;
>   			break;
>   		case 'r':
> -			if (file) {
> -				printf("Image already specified\n");
> +			if (guid) {
> +				printf("Image type already specified\n");
>   				return -1;
>   			}
> -			file = optarg;
>   			guid = &efi_guid_image_type_uboot_raw;
>   			break;
> +		case 'g':
> +			if (guid) {
> +				printf("Image type already specified\n");
> +				return -1;
> +			}
> +			if (uuid_parse(optarg, uuid_buf)) {
> +				printf("Wrong guid format\n");
> +				return -1;
> +			}
> +			guid = (efi_guid_t *)uuid_buf;
> +			break;
>   		case 'i':
>   			index = strtoul(optarg, NULL, 0);
>   			break;
> @@ -596,14 +606,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();
>   		return -1;
>   	}
>
> -	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) {
>   		printf("Creating firmware capsule failed\n");
>   		return -1;
>

[PATCH v3 5/5] test/py: efi_capsule: align with the syntax change of mkeficapsule

[AKASHI Takahiro]

Modify command line arguments at mkeficapsule as the syntax was
a bit modified in the previous commit.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 test/py/tests/test_efi_capsule/conftest.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/py/tests/test_efi_capsule/conftest.py b/test/py/tests/test_efi_capsule/conftest.py
index 6ad5608cd71c..8b5368c11abc 100644
--- a/test/py/tests/test_efi_capsule/conftest.py
+++ b/test/py/tests/test_efi_capsule/conftest.py
@@ -50,10 +50,10 @@ def efi_capsule_data(request, u_boot_config):
         check_call('cd %s; %s/tools/mkimage -f uboot_bin_env.its uboot_bin_env.itb' %
                    (data_dir, u_boot_config.build_dir),
                    shell=True)
-        check_call('cd %s; %s/tools/mkeficapsule --fit uboot_bin_env.itb --index 1 Test01' %
+        check_call('cd %s; %s/tools/mkeficapsule --index 1 --fit uboot_bin_env.itb Test01' %
                    (data_dir, u_boot_config.build_dir),
                    shell=True)
-        check_call('cd %s; %s/tools/mkeficapsule --raw u-boot.bin.new --index 1 Test02' %
+        check_call('cd %s; %s/tools/mkeficapsule --index 1 --raw u-boot.bin.new Test02' %
                    (data_dir, u_boot_config.build_dir),
                    shell=True)
 
-- 
2.33.0

Re: [PATCH v3 5/5] test/py: efi_capsule: align with the syntax change of mkeficapsule

[Heinrich Schuchardt]

On 8/31/21 4:46 AM, AKASHI Takahiro wrote:
> Modify command line arguments at mkeficapsule as the syntax was
> a bit modified in the previous commit.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   test/py/tests/test_efi_capsule/conftest.py | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/test/py/tests/test_efi_capsule/conftest.py b/test/py/tests/test_efi_capsule/conftest.py
> index 6ad5608cd71c..8b5368c11abc 100644
> --- a/test/py/tests/test_efi_capsule/conftest.py
> +++ b/test/py/tests/test_efi_capsule/conftest.py
> @@ -50,10 +50,10 @@ def efi_capsule_data(request, u_boot_config):
>           check_call('cd %s; %s/tools/mkimage -f uboot_bin_env.its uboot_bin_env.itb' %
>                      (data_dir, u_boot_config.build_dir),
>                      shell=True)
> -        check_call('cd %s; %s/tools/mkeficapsule --fit uboot_bin_env.itb --index 1 Test01' %
> +        check_call('cd %s; %s/tools/mkeficapsule --index 1 --fit uboot_bin_env.itb Test01' %
>                      (data_dir, u_boot_config.build_dir),
>                      shell=True)
> -        check_call('cd %s; %s/tools/mkeficapsule --raw u-boot.bin.new --index 1 Test02' %
> +        check_call('cd %s; %s/tools/mkeficapsule --index 1 --raw u-boot.bin.new Test02' %
>                      (data_dir, u_boot_config.build_dir),
>                      shell=True)
>
>

Should one of the tsts use a GUID (introduced in patch 4)?

Best regards

Heinrich

Re: [PATCH v3 5/5] test/py: efi_capsule: align with the syntax change of mkeficapsule

[AKASHI Takahiro]

On Tue, Aug 31, 2021 at 08:10:31AM +0200, Heinrich Schuchardt wrote:
> On 8/31/21 4:46 AM, AKASHI Takahiro wrote:
> > Modify command line arguments at mkeficapsule as the syntax was
> > a bit modified in the previous commit.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >   test/py/tests/test_efi_capsule/conftest.py | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/test/py/tests/test_efi_capsule/conftest.py b/test/py/tests/test_efi_capsule/conftest.py
> > index 6ad5608cd71c..8b5368c11abc 100644
> > --- a/test/py/tests/test_efi_capsule/conftest.py
> > +++ b/test/py/tests/test_efi_capsule/conftest.py
> > @@ -50,10 +50,10 @@ def efi_capsule_data(request, u_boot_config):
> >           check_call('cd %s; %s/tools/mkimage -f uboot_bin_env.its uboot_bin_env.itb' %
> >                      (data_dir, u_boot_config.build_dir),
> >                      shell=True)
> > -        check_call('cd %s; %s/tools/mkeficapsule --fit uboot_bin_env.itb --index 1 Test01' %
> > +        check_call('cd %s; %s/tools/mkeficapsule --index 1 --fit uboot_bin_env.itb Test01' %
> >                      (data_dir, u_boot_config.build_dir),
> >                      shell=True)
> > -        check_call('cd %s; %s/tools/mkeficapsule --raw u-boot.bin.new --index 1 Test02' %
> > +        check_call('cd %s; %s/tools/mkeficapsule --index 1 --raw u-boot.bin.new Test02' %
> >                      (data_dir, u_boot_config.build_dir),
> >                      shell=True)
> > 
> > 
> 
> Should one of the tsts use a GUID (introduced in patch 4)?

Good point. After adding an extra test, I found a bug :)
uuid_parse() in libuuid is used to convert a string
into a guid value, but uuid and guid have different formats
of representation and we can't use the return value from
uuid_parse() as it is.

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 

Re: [PATCH v3 0/5] efi_loader: capsule: improve capsule authentication support

[Heinrich Schuchardt]

On 8/31/21 4:46 AM, AKASHI Takahiro wrote:
> # This patch set is basically the same as v2, but removed authentication-
> # related pytest because the discussion about where to save a public
> # key is still going on. The command, mkeficapule, is independent from
> # the result.
>
> As I proposed and discussed in [1] and [2], I have made a couple of
> improvements on the current implementation of capsule update in this
> patch set.
>
> * add signing feature to mkeficapsule
> * add "--guid" option to mkeficapsule
> * add man page of mkeficapsule

Thanks a lot for getting around the requirement to use an EDK II tool.

My main concern is that there should be only mkeficapsule version.

If signed and unsigned capsules are of interest, mkeficapsule should
support both.

Best regards

Heinrich

>
> [1] https://lists.denx.de/pipermail/u-boot/2021-April/447918.html
> [2] https://lists.denx.de/pipermail/u-boot/2021-July/455292.html
>
> Prerequisite patches
> ====================
> None
>
> Test
> ====
> * locally passed the pytest which is included in this patch series
>    on sandbox built.
>
> Todo
> ====
> * add capsule authentication pytest
>
> Changes
> =======
> v3 (Aug 31, 2021)
> * rebased on v2021.10-rc3
> * remove pytest-related patches
> * add function descriptions in mkeficapsule.c
> * correct format specifiers in printf()
> * let main() return 0 or -1 only
> * update doc/develop/uefi/uefi.rst for syntax change of mkeficapsule
>
> v2 (July 28, 2021)
> * rebased on v2021.10-rc*
> * removed dependency on target's configuration
> * removed fdtsig.sh and others
> * add man page
> * update the UEFI document
> * add dedicate defconfig for testing on sandbox
> * add gitlab CI support
> * add "--guid" option to mkeficapsule
>    (yet rather RFC)
>
> Initial release (May 12, 2021)
> * based on v2021.07-rc2
>
> AKASHI Takahiro (5):
>    tools: mkeficapsule: add firmwware image signing
>    tools: mkeficapsule: add man page
>    doc: update UEFI document for usage of mkeficapsule
>    tools: mkeficapsule: allow for specifying GUID explicitly
>    test/py: efi_capsule: align with the syntax change of mkeficapsule
>
>   MAINTAINERS                                |   1 +
>   doc/develop/uefi/uefi.rst                  |  31 +-
>   doc/mkeficapsule.1                         |  98 +++++
>   test/py/tests/test_efi_capsule/conftest.py |   4 +-
>   tools/Kconfig                              |   7 +
>   tools/Makefile                             |   8 +-
>   tools/mkeficapsule.c                       | 471 +++++++++++++++++++--
>   7 files changed, 554 insertions(+), 66 deletions(-)
>   create mode 100644 doc/mkeficapsule.1
>