[PATCH v7 03/17] efi_loader: add signature database parser

AKASHI Takahiro takahiro.akashi at linaro.org
Mon Apr 20 04:19:29 CEST 2020


Heinrich,

On Fri, Apr 17, 2020 at 08:05:41PM +0200, Heinrich Schuchardt wrote:
> On 4/14/20 4:51 AM, AKASHI Takahiro wrote:
> > efi_signature_parse_sigdb() is a helper function will be used to parse
> > signature database variable and instantiate a signature store structure
> > in later patches.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> > ---
> >  include/efi_loader.h           |   3 +
> >  lib/efi_loader/efi_signature.c | 226 +++++++++++++++++++++++++++++++++
> >  2 files changed, 229 insertions(+)
> >
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index 8cf85d2fb7e2..fea2ead02e93 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -750,6 +750,9 @@ bool efi_signature_verify_with_sigdb(struct efi_image_regions *regs,
> >  efi_status_t efi_image_region_add(struct efi_image_regions *regs,
> >  				  const void *start, const void *end,
> >  				  int nocheck);
> > +
> > +void efi_sigstore_free(struct efi_signature_store *sigstore);
> > +struct efi_signature_store *efi_sigstore_parse_sigdb(u16 *name);
> >  #endif /* CONFIG_EFI_SECURE_BOOT */
> >
> >  #else /* CONFIG_IS_ENABLED(EFI_LOADER) */
> > diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
> > index 23dac94c0593..2d1f38980e5f 100644
> > --- a/lib/efi_loader/efi_signature.c
> > +++ b/lib/efi_loader/efi_signature.c
> > @@ -580,4 +580,230 @@ efi_status_t efi_image_region_add(struct efi_image_regions *regs,
> >
> >  	return EFI_SUCCESS;
> >  }
> > +
> > +/**
> > + * efi_sigstore_free - free signature store
> > + * @sigstore:	Pointer to signature store structure
> > + *
> > + * Feee all the memories held in signature store and itself,
> > + * which were allocated by efi_sigstore_parse_sigdb().
> > + */
> > +void efi_sigstore_free(struct efi_signature_store *sigstore)
> > +{
> > +	struct efi_signature_store *sigstore_next;
> > +	struct efi_sig_data *sig_data, *sig_data_next;
> > +
> > +	while (sigstore) {
> > +		sigstore_next = sigstore->next;
> > +
> > +		sig_data = sigstore->sig_data_list;
> > +		while (sig_data) {
> > +			sig_data_next = sig_data->next;
> > +			free(sig_data->data);
> > +			free(sig_data);
> > +			sig_data = sig_data_next;
> > +		}
> > +
> > +		free(sigstore);
> > +		sigstore = sigstore_next;
> > +	}
> > +}
> > +
> > +/**
> > + * efi_sigstore_parse_siglist - parse a signature list
> > + * @name:	Pointer to signature list
> > + *
> > + * Parse signature list and instantiate a signature store structure.
> > + * Signature database is a simple concatenation of one or more
> > + * signature list(s).
> > + *
> > + * Return:	Pointer to signature store on success, NULL on error
> > + */
> > +static struct efi_signature_store *
> > +efi_sigstore_parse_siglist(struct efi_signature_list *esl)
> > +{
> > +	struct efi_signature_store *siglist = NULL;
> > +	struct efi_sig_data *sig_data, *sig_data_next;
> > +	struct efi_signature_data *esd;
> > +	size_t left;
> > +
> > +	/*
> > +	 * UEFI specification defines certificate types:
> > +	 *   for non-signed images,
> > +	 *	EFI_CERT_SHA256_GUID
> > +	 *	EFI_CERT_RSA2048_GUID
> > +	 *	EFI_CERT_RSA2048_SHA256_GUID
> > +	 *	EFI_CERT_SHA1_GUID
> > +	 *	EFI_CERT_RSA2048_SHA_GUID
> > +	 *	EFI_CERT_SHA224_GUID
> > +	 *	EFI_CERT_SHA384_GUID
> > +	 *	EFI_CERT_SHA512_GUID
> > +	 *
> > +	 *   for signed images,
> > +	 *	EFI_CERT_X509_GUID
> > +	 *	NOTE: Each certificate will normally be in a separate
> > +	 *	EFI_SIGNATURE_LIST as the size may vary depending on
> > +	 *	its algo's.
> > +	 *
> > +	 *   for timestamp revocation of certificate,
> > +	 *	EFI_CERT_X509_SHA512_GUID
> > +	 *	EFI_CERT_X509_SHA256_GUID
> > +	 *	EFI_CERT_X509_SHA384_GUID
> > +	 */
> > +
> > +	if (esl->signature_list_size
> > +			<= (sizeof(*esl) + esl->signature_header_size)) {
> > +		debug("Siglist in wrong format\n");
> > +		return NULL;
> > +	}
> > +
> > +	/* Create a head */
> > +	siglist = calloc(sizeof(*siglist), 1);
> > +	if (!siglist) {
> > +		debug("Out of memory\n");
> > +		goto err;
> > +	}
> > +	memcpy(&siglist->sig_type, &esl->signature_type, sizeof(efi_guid_t));
> > +
> > +	/* Go through the list */
> > +	sig_data_next = NULL;
> > +	left = esl->signature_list_size
> > +			- (sizeof(*esl) + esl->signature_header_size);
> > +	esd = (struct efi_signature_data *)
> > +			((u8 *)esl + sizeof(*esl) + esl->signature_header_size);
> > +
> > +	while ((left > 0) && left >= esl->signature_size) {
> > +		/* Signature must exist if there is remaining data. */
> > +		if (left < esl->signature_size) {
> 
> This code is unreachable (as indicated by cppcheck).

Right, but I think the check (and an error message) is valuable just in case
that a given signature database might be somehow corrupted.
So I will fix 'while' statement.

Thanks,
-Takahiro Akashi


> Please, send a follow-up patch.
> 
> Best regards
> 
> Heinrich
> 
> > +			debug("Certificate is too small\n");
> > +			goto err;
> > +		}
> > +
> > +		sig_data = calloc(esl->signature_size
> > +					- sizeof(esd->signature_owner), 1);
> > +		if (!sig_data) {
> > +			debug("Out of memory\n");
> > +			goto err;
> > +		}
> > +
> > +		/* Append signature data */
> > +		memcpy(&sig_data->owner, &esd->signature_owner,
> > +		       sizeof(efi_guid_t));
> > +		sig_data->size = esl->signature_size
> > +					- sizeof(esd->signature_owner);
> > +		sig_data->data = malloc(sig_data->size);
> > +		if (!sig_data->data) {
> > +			debug("Out of memory\n");
> > +			goto err;
> > +		}
> > +		memcpy(sig_data->data, esd->signature_data, sig_data->size);
> > +
> > +		sig_data->next = sig_data_next;
> > +		sig_data_next = sig_data;
> > +
> > +		/* Next */
> > +		esd = (struct efi_signature_data *)
> > +				((u8 *)esd + esl->signature_size);
> > +		left -= esl->signature_size;
> > +	}
> > +	siglist->sig_data_list = sig_data_next;
> > +
> > +	return siglist;
> > +
> > +err:
> > +	efi_sigstore_free(siglist);
> > +
> > +	return NULL;
> > +}
> > +
> > +/**
> > + * efi_sigstore_parse_sigdb - parse a signature database variable
> > + * @name:	Variable's name
> > + *
> > + * Read in a value of signature database variable pointed to by
> > + * @name, parse it and instantiate a signature store structure.
> > + *
> > + * Return:	Pointer to signature store on success, NULL on error
> > + */
> > +struct efi_signature_store *efi_sigstore_parse_sigdb(u16 *name)
> > +{
> > +	struct efi_signature_store *sigstore = NULL, *siglist;
> > +	struct efi_signature_list *esl;
> > +	const efi_guid_t *vendor;
> > +	void *db;
> > +	efi_uintn_t db_size;
> > +	efi_status_t ret;
> > +
> > +	if (!u16_strcmp(name, L"PK") || !u16_strcmp(name, L"KEK")) {
> > +		vendor = &efi_global_variable_guid;
> > +	} else if (!u16_strcmp(name, L"db") || !u16_strcmp(name, L"dbx")) {
> > +		vendor = &efi_guid_image_security_database;
> > +	} else {
> > +		debug("unknown signature database, %ls\n", name);
> > +		return NULL;
> > +	}
> > +
> > +	/* retrieve variable data */
> > +	db_size = 0;
> > +	ret = EFI_CALL(efi_get_variable(name, vendor, NULL, &db_size, NULL));
> > +	if (ret == EFI_NOT_FOUND) {
> > +		debug("variable, %ls, not found\n", name);
> > +		sigstore = calloc(sizeof(*sigstore), 1);
> > +		return sigstore;
> > +	} else if (ret != EFI_BUFFER_TOO_SMALL) {
> > +		debug("Getting variable, %ls, failed\n", name);
> > +		return NULL;
> > +	}
> > +
> > +	db = malloc(db_size);
> > +	if (!db) {
> > +		debug("Out of memory\n");
> > +		return NULL;
> > +	}
> > +
> > +	ret = EFI_CALL(efi_get_variable(name, vendor, NULL, &db_size, db));
> > +	if (ret != EFI_SUCCESS) {
> > +		debug("Getting variable, %ls, failed\n", name);
> > +		goto err;
> > +	}
> > +
> > +	/* Parse siglist list */
> > +	esl = db;
> > +	while (db_size > 0) {
> > +		/* List must exist if there is remaining data. */
> > +		if (db_size < sizeof(*esl)) {
> > +			debug("variable, %ls, in wrong format\n", name);
> > +			goto err;
> > +		}
> > +
> > +		if (db_size < esl->signature_list_size) {
> > +			debug("variable, %ls, in wrong format\n", name);
> > +			goto err;
> > +		}
> > +
> > +		/* Parse a single siglist. */
> > +		siglist = efi_sigstore_parse_siglist(esl);
> > +		if (!siglist) {
> > +			debug("Parsing signature list of %ls failed\n", name);
> > +			goto err;
> > +		}
> > +
> > +		/* Append siglist */
> > +		siglist->next = sigstore;
> > +		sigstore = siglist;
> > +
> > +		/* Next */
> > +		db_size -= esl->signature_list_size;
> > +		esl = (void *)esl + esl->signature_list_size;
> > +	}
> > +	free(db);
> > +
> > +	return sigstore;
> > +
> > +err:
> > +	efi_sigstore_free(sigstore);
> > +	free(db);
> > +
> > +	return NULL;
> > +}
> >  #endif /* CONFIG_EFI_SECURE_BOOT */
> >
> 


More information about the U-Boot mailing list