Re: [PATCH 07/13] efi_loader: image_loader: add digest-based verification for signed image

2020-06-01 Thread AKASHI Takahiro
Heinrich,

On Sat, May 30, 2020 at 09:09:30AM +0200, Heinrich Schuchardt wrote:
> On 5/29/20 8:41 AM, AKASHI Takahiro wrote:
> > In case that a type of certificate in "db" or "dbx" is
> > EFI_CERT_X509_SHA256_GUID, it is actually not a certificate which contains
> > a public key for RSA decryption, but a digest of image to be loaded.
> > If the value matches to a value calculated from a given binary image, it is
> > granted for loading.
> >
> > With this patch, common digest check code, which used to be used for
> > unsigned image verification, will be extracted from
> > efi_signature_verify_with_sigdb() into efi_signature_lookup_digest(), and
> > extra step for digest check will be added to efi_image_authenticate().
> 
> Could you, please, add comments in the code describing this process flow.

All the necessary code is contained in efi_signature_lookup_digest(),
but I'll add some comments in efi_image_authenticate().

Thanks,
-Takahiro Akashi


> Best regards
> 
> Heinrich


Re: [PATCH 07/13] efi_loader: image_loader: add digest-based verification for signed image

2020-05-30 Thread Heinrich Schuchardt
On 5/29/20 8:41 AM, AKASHI Takahiro wrote:
> In case that a type of certificate in "db" or "dbx" is
> EFI_CERT_X509_SHA256_GUID, it is actually not a certificate which contains
> a public key for RSA decryption, but a digest of image to be loaded.
> If the value matches to a value calculated from a given binary image, it is
> granted for loading.
>
> With this patch, common digest check code, which used to be used for
> unsigned image verification, will be extracted from
> efi_signature_verify_with_sigdb() into efi_signature_lookup_digest(), and
> extra step for digest check will be added to efi_image_authenticate().

Could you, please, add comments in the code describing this process flow.

Best regards

Heinrich


[PATCH 07/13] efi_loader: image_loader: add digest-based verification for signed image

2020-05-29 Thread AKASHI Takahiro
In case that a type of certificate in "db" or "dbx" is
EFI_CERT_X509_SHA256_GUID, it is actually not a certificate which contains
a public key for RSA decryption, but a digest of image to be loaded.
If the value matches to a value calculated from a given binary image, it is
granted for loading.

With this patch, common digest check code, which used to be used for
unsigned image verification, will be extracted from
efi_signature_verify_with_sigdb() into efi_signature_lookup_digest(), and
extra step for digest check will be added to efi_image_authenticate().

Signed-off-by: AKASHI Takahiro 
---
 include/efi_loader.h  |   2 +
 lib/efi_loader/efi_image_loader.c |  27 +--
 lib/efi_loader/efi_signature.c| 122 +++---
 3 files changed, 84 insertions(+), 67 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 2cbd52e273d4..30470bc35ce2 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -761,6 +761,8 @@ struct efi_signature_store {
 struct x509_certificate;
 struct pkcs7_message;
 
+bool efi_signature_lookup_digest(struct efi_image_regions *regs,
+struct efi_signature_store *db);
 bool efi_signature_verify_with_sigdb(struct efi_image_regions *regs,
 struct pkcs7_message *msg,
 struct efi_signature_store *db,
diff --git a/lib/efi_loader/efi_image_loader.c 
b/lib/efi_loader/efi_image_loader.c
index 33ffb43f3886..c5849b4afebd 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -446,16 +446,16 @@ static bool efi_image_unsigned_authenticate(struct 
efi_image_regions *regs)
}
 
/* try black-list first */
-   if (efi_signature_verify_with_sigdb(regs, NULL, dbx, NULL)) {
-   debug("Image is not signed and rejected by \"dbx\"\n");
+   if (efi_signature_lookup_digest(regs, dbx)) {
+   debug("Image is not signed and its digest found in \"dbx\"\n");
goto out;
}
 
/* try white-list */
-   if (efi_signature_verify_with_sigdb(regs, NULL, db, NULL))
+   if (efi_signature_lookup_digest(regs, db))
ret = true;
else
-   debug("Image is not signed and not found in \"db\" or 
\"dbx\"\n");
+   debug("Image is not signed and its digest not found in \"db\" 
or \"dbx\"\n");
 
 out:
efi_sigstore_free(db);
@@ -612,11 +612,22 @@ static bool efi_image_authenticate(void *efi, size_t 
efi_size)
goto err;
}
 
+   if (efi_signature_lookup_digest(regs, dbx)) {
+   debug("Image's digest was found in \"dbx\"\n");
+   goto err;
+   }
+
/* try white-list */
-   if (!efi_signature_verify_with_sigdb(regs, msg, db, dbx)) {
-   debug("Signature was not verified by \"db\"\n");
-   goto err;
-   }
+   if (efi_signature_verify_with_sigdb(regs, msg, db, dbx))
+   continue;
+
+   debug("Signature was not verified by \"db\"\n");
+
+   if (efi_signature_lookup_digest(regs, db))
+   continue;
+
+   debug("Image's digest was not found in \"db\" or \"dbx\"\n");
+   goto err;
}
ret = true;
 
diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
index ab5687040a38..d2ded5cfac78 100644
--- a/lib/efi_loader/efi_signature.c
+++ b/lib/efi_loader/efi_signature.c
@@ -196,6 +196,68 @@ out:
return verified;
 }
 
+/**
+ * efi_signature_lookup_digest - search for an image's digest in sigdb
+ * @regs:  List of regions to be authenticated
+ * @db:Signature database for trusted certificates
+ *
+ * A message digest of image pointed to by @regs is calculated and
+ * its hash value is compared to entries in signature database pointed
+ * to by @db.
+ *
+ * Return: true if found, false if not
+ */
+bool efi_signature_lookup_digest(struct efi_image_regions *regs,
+struct efi_signature_store *db)
+{
+   struct efi_signature_store *siglist;
+   struct efi_sig_data *sig_data;
+   void *hash = NULL;
+   size_t size = 0;
+   bool found = false;
+
+   debug("%s: Enter, %p, %p\n", __func__, regs, db);
+
+   if (!regs || !db || !db->sig_data_list)
+   goto out;
+
+   for (siglist = db; siglist; siglist = siglist->next) {
+   /* TODO: support other hash algorithms */
+   if (guidcmp(>sig_type, _guid_sha256)) {
+   debug("Digest algorithm is not supported: %pUl\n",
+ >sig_type);
+   break;
+   }
+
+   if (!efi_hash_regions(regs->reg, regs->num, , )) {
+   debug("Digesting an