Re: [PATCH v7 03/17] efi_loader: add signature database parser
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
> > ---
> > 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 indicat
Re: [PATCH v7 03/17] efi_loader: add signature database parser
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
> ---
> 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).
Please, send a follow-up patch.
Best regards
Heinrich
> + debug("Certificate is too small\n");
> + goto err;
> + }
> +
> +
[PATCH v7 03/17] efi_loader: add signature database parser
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
---
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) {
+ 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 sign
