[PATCH v2 1/9] tools: mkeficapsule: add firmwware image signing

AKASHI Takahiro takahiro.akashi at linaro.org
Mon Aug 2 05:30:04 CEST 2021


Heinrich,

On Sun, Aug 01, 2021 at 11:21:55AM +0200, Heinrich Schuchardt wrote:
> On 7/27/21 11:10 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:
> >    -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 at linaro.org>
> > ---
> >   tools/Kconfig        |   7 +
> >   tools/Makefile       |   8 +-
> >   tools/mkeficapsule.c | 332 +++++++++++++++++++++++++++++++++++++++----
> >   3 files changed, 316 insertions(+), 31 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 bae3f95c4995..af8536489652 100644
> > --- a/tools/Makefile
> > +++ b/tools/Makefile
> > @@ -245,8 +245,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")
> 
> Please, add package pkg-config to the build dependencies enumerated in
> doc/build/gcc.rst.

Already there: libssl-dev

> > +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..798706c7b5f7 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,195 @@ 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);
> >   }
> > 
> 
> Please, add Sphinx documentation.

I added the man page. Why need another different format of doc?
I think that doc/develop/uefi/uefi.rst gives you an enough
introduction to the command.
(Although the doc will have to been updated due to the latest change
of mkeficapsule syntax that I'm also proposing.)

Actually, there is no sphinx doc for any host tools.

> > +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
> > +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;
> > +}
> > +
> 
> Please, add Sphinx documentation for all functions.

Yeah, but "all" is a too forcible requirement.
There is no source code of host tools to meet the requirement.
Actually, most don't have even one description of a function.

I think that, if it is a really required element in the source,
it should be explicitly described somewhere under "doc". 

> > +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;
> > +}
> > +
> > +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;
> > +}
> > +
> > +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;
> > +}
> > +
> > +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
> > +
> >   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;
> > @@ -78,6 +280,7 @@ static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
> >   	printf("\tbin: %s\n\ttype: %pUl\n", bin, guid);
> >   	printf("\tindex: %ld\n\tinstance: %ld\n", index, instance);
> 
> [tools/mkeficapsule.c:281] (warning) %ld in format string (no. 1)
> requires 'long' but the argument type is 'unsigned long'.

Will fix it.

-Takahiro Akashi


> Best regards
> 
> Heinrich
> 
> >   #endif
> > +	auth_context.sig_size = 0;
> > 
> >   	g = fopen(bin, "r");
> >   	if (!g) {
> > @@ -93,11 +296,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 +332,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 +354,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 +370,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 +425,25 @@ err_1:
> >   	return -1;
> >   }
> > 
> > -/*
> > - * Usage:
> > - *   $ mkeficapsule -f <firmware binary> <output file>
> > - */
> >   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,26 +470,44 @@ 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) {
> > +	/* check necessary parameters */
> > +	if ((argc != optind + 1) || !file ||
> > +	    ((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)
> > -			< 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);
> >   	}
> > 
> 


More information about the U-Boot mailing list