summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAKASHI Takahiro <takahiro.akashi@linaro.org>2020-08-14 14:39:23 +0900
committerHeinrich Schuchardt <xypron.glpk@gmx.de>2020-08-14 12:28:25 +0200
commit52956e535e65c852b1f95d2ca5044cb7c4fc6bbe (patch)
tree2e7e3317e17608b7c7c4c003fa15477b52d5b7b4
parentf68a6d583578799ec2011476ebd1e10590c6eb3c (diff)
efi_loader: signature: correct a behavior against multiple signatures
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@linaro.org> Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
-rw-r--r--include/efi_loader.h9
-rw-r--r--lib/efi_loader/efi_image_loader.c33
-rw-r--r--lib/efi_loader/efi_signature.c76
3 files changed, 30 insertions, 88 deletions
diff --git a/include/efi_loader.h b/include/efi_loader.h
index b941b5e994..50a17a33ca 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -773,13 +773,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 832bce9394..eea42cc204 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 f60c0e1f79..79dee27421 100644
--- a/lib/efi_loader/efi_signature.c
+++ b/lib/efi_loader/efi_signature.c
@@ -174,6 +174,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))
@@ -266,7 +268,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,
@@ -336,70 +338,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
@@ -462,7 +400,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;
@@ -470,13 +408,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;