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>

2 files changed, 24 insertions, 85 deletions

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;