[PATCH 1/2] efi_loader: signature: correct a behavior against multiple signatures
AKASHI Takahiro
takahiro.akashi at linaro.org
Fri Aug 14 07:39:23 CEST 2020
Under the current implementation, all the signatures, if any, in
a signed image must be verified before loading it.
Meanwhile, UEFI specification v2.8b section 32.5.3.3 says,
Multiple signatures are allowed to exist in the binary’s certificate
table (as per PE/COFF Section “Attribute Certificate Table”). Only
one hash or signature is required to be present in db in order to pass
validation, so long as neither the SHA-256 hash of the binary nor any
present signature is reflected in dbx.
This patch makes the semantics of signature verification compliant with
the specification mentioned above.
Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
Reported-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
---
include/efi_loader.h | 9 ++--
lib/efi_loader/efi_image_loader.c | 33 +++++++-------
lib/efi_loader/efi_signature.c | 76 +++----------------------------
3 files changed, 30 insertions(+), 88 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index df8dc377257c..ae01e909b56c 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -770,13 +770,16 @@ struct pkcs7_message;
bool efi_signature_lookup_digest(struct efi_image_regions *regs,
struct efi_signature_store *db);
-bool efi_signature_verify_one(struct efi_image_regions *regs,
- struct pkcs7_message *msg,
- struct efi_signature_store *db);
bool efi_signature_verify(struct efi_image_regions *regs,
struct pkcs7_message *msg,
struct efi_signature_store *db,
struct efi_signature_store *dbx);
+static inline bool efi_signature_verify_one(struct efi_image_regions *regs,
+ struct pkcs7_message *msg,
+ struct efi_signature_store *db)
+{
+ return efi_signature_verify(regs, msg, db, NULL);
+}
bool efi_signature_check_signers(struct pkcs7_message *msg,
struct efi_signature_store *dbx);
diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
index 832bce939403..eea42cc20436 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -546,6 +546,11 @@ static bool efi_image_authenticate(void *efi, size_t efi_size)
goto err;
}
+ if (efi_signature_lookup_digest(regs, dbx)) {
+ EFI_PRINT("Image's digest was found in \"dbx\"\n");
+ goto err;
+ }
+
/*
* go through WIN_CERTIFICATE list
* NOTE:
@@ -553,10 +558,9 @@ static bool efi_image_authenticate(void *efi, size_t efi_size)
* in PE header, or as pkcs7 SignerInfo's in SignedData.
* So the verification policy here is:
* - Success if, at least, one of signatures is verified
- * - unless
- * any of signatures is rejected explicitly, or
- * none of digest algorithms are supported
+ * - unless signature is rejected explicitly with its digest.
*/
+
for (wincert = wincerts, wincerts_end = (u8 *)wincerts + wincerts_len;
(u8 *)wincert < wincerts_end;
wincert = (WIN_CERTIFICATE *)
@@ -627,32 +631,29 @@ static bool efi_image_authenticate(void *efi, size_t efi_size)
/* try black-list first */
if (efi_signature_verify_one(regs, msg, dbx)) {
EFI_PRINT("Signature was rejected by \"dbx\"\n");
- goto err;
+ continue;
}
if (!efi_signature_check_signers(msg, dbx)) {
EFI_PRINT("Signer(s) in \"dbx\"\n");
- goto err;
- }
-
- if (efi_signature_lookup_digest(regs, dbx)) {
- EFI_PRINT("Image's digest was found in \"dbx\"\n");
- goto err;
+ continue;
}
/* try white-list */
- if (efi_signature_verify(regs, msg, db, dbx))
- continue;
+ if (efi_signature_verify(regs, msg, db, dbx)) {
+ ret = true;
+ break;
+ }
debug("Signature was not verified by \"db\"\n");
- if (efi_signature_lookup_digest(regs, db))
- continue;
+ if (efi_signature_lookup_digest(regs, db)) {
+ ret = true;
+ break;
+ }
debug("Image's digest was not found in \"db\" or \"dbx\"\n");
- goto err;
}
- ret = true;
err:
efi_sigstore_free(db);
diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
index 7b33a4010fe8..a8da2496360d 100644
--- a/lib/efi_loader/efi_signature.c
+++ b/lib/efi_loader/efi_signature.c
@@ -175,6 +175,8 @@ static bool efi_lookup_certificate(struct x509_certificate *cert,
if (IS_ERR_OR_NULL(cert_tmp))
continue;
+ EFI_PRINT("%s: against %s\n", __func__,
+ cert_tmp->subject);
reg[0].data = cert_tmp->tbs;
reg[0].size = cert_tmp->tbs_size;
if (!efi_hash_regions(reg, 1, &hash_tmp, NULL))
@@ -267,7 +269,7 @@ out:
* protocol at this time and any image will be unconditionally revoked
* when this match occurs.
*
- * Return: true if check passed, false otherwise.
+ * Return: true if check passed (not found), false otherwise.
*/
static bool efi_signature_check_revocation(struct pkcs7_signed_info *sinfo,
struct x509_certificate *cert,
@@ -337,70 +339,6 @@ out:
return !revoked;
}
-/**
- * efi_signature_verify_one - verify signatures with database
- * @regs: List of regions to be authenticated
- * @msg: Signature
- * @db: Signature database
- *
- * All the signature pointed to by @msg against image pointed to by @regs
- * will be verified by signature database pointed to by @db.
- *
- * Return: true if verification for one of signatures passed, false
- * otherwise
- */
-bool efi_signature_verify_one(struct efi_image_regions *regs,
- struct pkcs7_message *msg,
- struct efi_signature_store *db)
-{
- struct pkcs7_signed_info *sinfo;
- struct x509_certificate *signer;
- bool verified = false;
- int ret;
-
- EFI_PRINT("%s: Enter, %p, %p, %p\n", __func__, regs, msg, db);
-
- if (!db)
- goto out;
-
- if (!db->sig_data_list)
- goto out;
-
- EFI_PRINT("%s: Verify signed image with db\n", __func__);
- for (sinfo = msg->signed_infos; sinfo; sinfo = sinfo->next) {
- EFI_PRINT("Signed Info: digest algo: %s, pkey algo: %s\n",
- sinfo->sig->hash_algo, sinfo->sig->pkey_algo);
-
- EFI_PRINT("Verifying certificate chain\n");
- signer = NULL;
- ret = pkcs7_verify_one(msg, sinfo, &signer);
- if (ret == -ENOPKG)
- continue;
-
- if (ret < 0 || !signer)
- goto out;
-
- if (sinfo->blacklisted)
- continue;
-
- EFI_PRINT("Verifying last certificate in chain\n");
- if (signer->self_signed) {
- if (efi_lookup_certificate(signer, db)) {
- verified = true;
- goto out;
- }
- } else if (efi_verify_certificate(signer, db, NULL)) {
- verified = true;
- goto out;
- }
- EFI_PRINT("Valid certificate not in db\n");
- }
-
-out:
- EFI_PRINT("%s: Exit, verified: %d\n", __func__, verified);
- return verified;
-}
-
/*
* efi_signature_verify - verify signatures with db and dbx
* @regs: List of regions to be authenticated
@@ -463,7 +401,7 @@ bool efi_signature_verify(struct efi_image_regions *regs,
if (efi_lookup_certificate(signer, db))
if (efi_signature_check_revocation(sinfo,
signer, dbx))
- continue;
+ break;
} else if (efi_verify_certificate(signer, db, &root)) {
bool check;
@@ -471,13 +409,13 @@ bool efi_signature_verify(struct efi_image_regions *regs,
dbx);
x509_free_certificate(root);
if (check)
- continue;
+ break;
}
EFI_PRINT("Certificate chain didn't reach trusted CA\n");
- goto out;
}
- verified = true;
+ if (sinfo)
+ verified = true;
out:
EFI_PRINT("%s: Exit, verified: %d\n", __func__, verified);
return verified;
--
2.28.0
More information about the U-Boot
mailing list