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>

diff --git a/include/efi_loader.h b/include/efi_loader.h
index b941b5e994149500a80d8a62a380b72b3ab722ea..50a17a33ca43dd0d44fc7ee88406b34ceb55ce7c 100644 (file)
--- 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 832bce939403d2e8bb7554dd5123882e4ce3b111..eea42cc204363307157ff20188cf75e00a99366d 100644 (file)
--- 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 f60c0e1f79328fb9c05f2d4c1a38e41300aebde8..79dee27421b219193a124666e85ac96a492dec3a 100644 (file)
--- 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;