[PATCH v5 6/8] efi_loader: signature: rework for intermediate certificates support
AKASHI Takahiro
takahiro.akashi at linaro.org
Wed Jul 29 04:49:51 CEST 2020
Heinrich,
On Wed, Jul 22, 2020 at 01:00:30PM +0200, Heinrich Schuchardt wrote:
> On 21.07.20 12:35, AKASHI Takahiro wrote:
> > In this commit, efi_signature_verify(with_sigdb) will be re-implemented
> > using pcks7_verify_one() in order to support certificates chain, where
> > the signer's certificate will be signed by an intermediate CA (certificate
> > authority) and the latter's certificate will also be signed by another CA
> > and so on.
> >
> > What we need to do here is to search for certificates in a signature,
> > build up a chain of certificates and verify one by one. pkcs7_verify_one()
> > handles most of these steps except the last one.
> >
> > pkcs7_verify_one() returns, if succeeded, the last certificate to verify,
> > which can be either a self-signed one or one that should be signed by one
> > of certificates in "db". Re-worked efi_signature_verify() will take care
> > of this step.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> > ---
> > include/efi_loader.h | 8 +-
> > lib/efi_loader/Kconfig | 1 +
> > lib/efi_loader/efi_image_loader.c | 2 +-
> > lib/efi_loader/efi_signature.c | 385 ++++++++++++++----------------
> > lib/efi_loader/efi_variable.c | 5 +-
> > 5 files changed, 188 insertions(+), 213 deletions(-)
> >
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index 98944640bee7..df8dc377257c 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -773,10 +773,10 @@ bool efi_signature_lookup_digest(struct efi_image_regions *regs,
> > bool efi_signature_verify_one(struct efi_image_regions *regs,
> > struct pkcs7_message *msg,
> > 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,
> > - struct efi_signature_store *dbx);
> > +bool efi_signature_verify(struct efi_image_regions *regs,
> > + struct pkcs7_message *msg,
> > + struct efi_signature_store *db,
> > + struct efi_signature_store *dbx);
> > bool efi_signature_check_signers(struct pkcs7_message *msg,
> > struct efi_signature_store *dbx);
> >
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index 6017ffe9a600..bad1a29ba804 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -205,6 +205,7 @@ config EFI_SECURE_BOOT
> > select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
> > select X509_CERTIFICATE_PARSER
> > select PKCS7_MESSAGE_PARSER
> > + select PKCS7_VERIFY
> > default n
> > help
> > Select this option to enable EFI secure boot support.
> > diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> > index d81ae8c93a52..d930811141af 100644
> > --- a/lib/efi_loader/efi_image_loader.c
> > +++ b/lib/efi_loader/efi_image_loader.c
> > @@ -642,7 +642,7 @@ static bool efi_image_authenticate(void *efi, size_t efi_size)
> > }
> >
> > /* try white-list */
> > - if (efi_signature_verify_with_sigdb(regs, msg, db, dbx))
> > + if (efi_signature_verify(regs, msg, db, dbx))
> > continue;
> >
> > debug("Signature was not verified by \"db\"\n");
> > diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
> > index 8413d83e343b..7b33a4010fe8 100644
> > --- a/lib/efi_loader/efi_signature.c
> > +++ b/lib/efi_loader/efi_signature.c
> > @@ -10,7 +10,9 @@
> > #include <image.h>
> > #include <hexdump.h>
> > #include <malloc.h>
> > +#include <crypto/pkcs7.h>
> > #include <crypto/pkcs7_parser.h>
> > +#include <crypto/public_key.h>
> > #include <crypto/x509_parser.h>
>
> This line does not exist in origin master. Which patch is missing?
The line is what you deleted when you merged this file while
I said that the header should be directly included because
this file uses an x509 parser function explicitly.
I still believe that it should.
>
> Using sbsigntool 0.9.4 and this whole series applied 'make test' fails
>
> test/py/tests/test_efi_secboot/test_authvar.py FFFFF
> test/py/tests/test_efi_secboot/test_signed.py .FF.FF
> test/py/tests/test_efi_secboot/test_signed_intca.py .FF
> test/py/tests/test_efi_secboot/test_unsigned.py ..F
I rebased v5 to your *current* efi-2020-10 and tried to reproduce
the problem but failed. All the test cases passed.
-Takahiro Akashi
> Up to patch 5 make test succeeds. I will take patches 1-5 into my next
> pull-request.
>
> Anyway we have to wait for sbsigntool 0.9.4 in the Docker image and
> Travis CI.
>
> Best regards
>
> Heinrich
>
>
> > #include <linux/compat.h>
> > #include <linux/oid_registry.h>
> > @@ -61,143 +63,6 @@ static bool efi_hash_regions(struct image_region *regs, int count,
> > return true;
> > }
> >
> > -/**
> > - * efi_hash_msg_content - calculate a hash value of contentInfo
> > - * @msg: Signature
> > - * @hash: Pointer to a pointer to buffer holding a hash value
> > - * @size: Size of buffer to be returned
> > - *
> > - * Calculate a sha256 value of contentInfo in @msg and return a value in @hash.
> > - *
> > - * Return: true on success, false on error
> > - */
> > -static bool efi_hash_msg_content(struct pkcs7_message *msg, void **hash,
> > - size_t *size)
> > -{
> > - struct image_region regtmp;
> > -
> > - regtmp.data = msg->data;
> > - regtmp.size = msg->data_len;
> > -
> > - return efi_hash_regions(®tmp, 1, hash, size);
> > -}
> > -
> > -/**
> > - * efi_signature_verify - verify a signature with a certificate
> > - * @regs: List of regions to be authenticated
> > - * @signed_info: Pointer to PKCS7's signed_info
> > - * @cert: x509 certificate
> > - *
> > - * Signature pointed to by @signed_info against image pointed to by @regs
> > - * is verified by a certificate pointed to by @cert.
> > - * @signed_info holds a signature, including a message digest which is to be
> > - * compared with a hash value calculated from @regs.
> > - *
> > - * Return: true if signature is verified, false if not
> > - */
> > -static bool efi_signature_verify(struct efi_image_regions *regs,
> > - struct pkcs7_message *msg,
> > - struct pkcs7_signed_info *ps_info,
> > - struct x509_certificate *cert)
> > -{
> > - struct image_sign_info info;
> > - struct image_region regtmp[2];
> > - void *hash;
> > - size_t size;
> > - char c;
> > - bool verified;
> > -
> > - EFI_PRINT("%s: Enter, %p, %p, %p(issuer: %s, subject: %s)\n", __func__,
> > - regs, ps_info, cert, cert->issuer, cert->subject);
> > -
> > - verified = false;
> > -
> > - memset(&info, '\0', sizeof(info));
> > - info.padding = image_get_padding_algo("pkcs-1.5");
> > - /*
> > - * Note: image_get_[checksum|crypto]_algo takes an string
> > - * argument like "<checksum>,<crypto>"
> > - * TODO: support other hash algorithms
> > - */
> > - if (!strcmp(ps_info->sig->hash_algo, "sha1")) {
> > - info.checksum = image_get_checksum_algo("sha1,rsa2048");
> > - info.name = "sha1,rsa2048";
> > - } else if (!strcmp(ps_info->sig->hash_algo, "sha256")) {
> > - info.checksum = image_get_checksum_algo("sha256,rsa2048");
> > - info.name = "sha256,rsa2048";
> > - } else {
> > - EFI_PRINT("unknown msg digest algo: %s\n",
> > - ps_info->sig->hash_algo);
> > - goto out;
> > - }
> > - info.crypto = image_get_crypto_algo(info.name);
> > -
> > - info.key = cert->pub->key;
> > - info.keylen = cert->pub->keylen;
> > -
> > - /* verify signature */
> > - EFI_PRINT("%s: crypto: %s, signature len:%x\n", __func__,
> > - info.name, ps_info->sig->s_size);
> > - if (ps_info->aa_set & (1UL << sinfo_has_message_digest)) {
> > - EFI_PRINT("%s: RSA verify authentication attribute\n",
> > - __func__);
> > - /*
> > - * NOTE: This path will be executed only for
> > - * PE image authentication
> > - */
> > -
> > - /* check if hash matches digest first */
> > - EFI_PRINT("checking msg digest first, len:0x%x\n",
> > - ps_info->msgdigest_len);
> > -
> > -#ifdef DEBUG
> > - EFI_PRINT("hash in database:\n");
> > - print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,
> > - ps_info->msgdigest, ps_info->msgdigest_len,
> > - false);
> > -#endif
> > - /* against contentInfo first */
> > - hash = NULL;
> > - if ((msg->data && efi_hash_msg_content(msg, &hash, &size)) ||
> > - /* for signed image */
> > - efi_hash_regions(regs->reg, regs->num, &hash, &size)) {
> > - /* for authenticated variable */
> > - if (ps_info->msgdigest_len != size ||
> > - memcmp(hash, ps_info->msgdigest, size)) {
> > - EFI_PRINT("Digest doesn't match\n");
> > - free(hash);
> > - goto out;
> > - }
> > -
> > - free(hash);
> > - } else {
> > - EFI_PRINT("Digesting image failed\n");
> > - goto out;
> > - }
> > -
> > - /* against digest */
> > - c = 0x31;
> > - regtmp[0].data = &c;
> > - regtmp[0].size = 1;
> > - regtmp[1].data = ps_info->authattrs;
> > - regtmp[1].size = ps_info->authattrs_len;
> > -
> > - if (!rsa_verify(&info, regtmp, 2,
> > - ps_info->sig->s, ps_info->sig->s_size))
> > - verified = true;
> > - } else {
> > - EFI_PRINT("%s: RSA verify content data\n", __func__);
> > - /* against all data */
> > - if (!rsa_verify(&info, regs->reg, regs->num,
> > - ps_info->sig->s, ps_info->sig->s_size))
> > - verified = true;
> > - }
> > -
> > -out:
> > - EFI_PRINT("%s: Exit, verified: %d\n", __func__, verified);
> > - return verified;
> > -}
> > -
> > /**
> > * efi_signature_lookup_digest - search for an image's digest in sigdb
> > * @regs: List of regions to be authenticated
> > @@ -261,61 +126,127 @@ out:
> > }
> >
> > /**
> > - * efi_signature_verify_with_list - verify a signature with signature list
> > - * @regs: List of regions to be authenticated
> > - * @msg: Signature
> > - * @signed_info: Pointer to PKCS7's signed_info
> > - * @siglist: Signature list for certificates
> > - * @valid_cert: x509 certificate that verifies this signature
> > + * efi_lookup_certificate - find a certificate within db
> > + * @msg: Signature
> > + * @db: Signature database
> > *
> > - * Signature pointed to by @signed_info against image pointed to by @regs
> > - * is verified by signature list pointed to by @siglist.
> > - * Signature database is a simple concatenation of one or more
> > - * signature list(s).
> > + * Search signature database pointed to by @db and find a certificate
> > + * pointed to by @cert.
> > *
> > - * Return: true if signature is verified, false if not
> > + * Return: true if found, false otherwise.
> > */
> > -static
> > -bool efi_signature_verify_with_list(struct efi_image_regions *regs,
> > - struct pkcs7_message *msg,
> > - struct pkcs7_signed_info *signed_info,
> > - struct efi_signature_store *siglist,
> > - struct x509_certificate **valid_cert)
> > +static bool efi_lookup_certificate(struct x509_certificate *cert,
> > + struct efi_signature_store *db)
> > {
> > - struct x509_certificate *cert;
> > + struct efi_signature_store *siglist;
> > struct efi_sig_data *sig_data;
> > - bool verified = false;
> > + struct image_region reg[1];
> > + void *hash = NULL, *hash_tmp = NULL;
> > + size_t size = 0;
> > + bool found = false;
> >
> > - EFI_PRINT("%s: Enter, %p, %p, %p, %p\n", __func__,
> > - regs, signed_info, siglist, valid_cert);
> > + EFI_PRINT("%s: Enter, %p, %p\n", __func__, cert, db);
> >
> > - if (guidcmp(&siglist->sig_type, &efi_guid_cert_x509)) {
> > - EFI_PRINT("Signature type is not supported: %pUl\n",
> > - &siglist->sig_type);
> > + if (!cert || !db || !db->sig_data_list)
> > goto out;
> > - }
> >
> > - /* go through the list */
> > - for (sig_data = siglist->sig_data_list; sig_data;
> > - sig_data = sig_data->next) {
> > - /* TODO: support owner check based on policy */
> > + /*
> > + * TODO: identify a certificate using sha256 digest
> > + * Is there any better way?
> > + */
> > + /* calculate hash of TBSCertificate */
> > + reg[0].data = cert->tbs;
> > + reg[0].size = cert->tbs_size;
> > + if (!efi_hash_regions(reg, 1, &hash, &size))
> > + goto out;
> >
> > - cert = x509_cert_parse(sig_data->data, sig_data->size);
> > - if (IS_ERR(cert)) {
> > - EFI_PRINT("Parsing x509 certificate failed\n");
> > - goto out;
> > + EFI_PRINT("%s: searching for %s\n", __func__, cert->subject);
> > + for (siglist = db; siglist; siglist = siglist->next) {
> > + /* only with x509 certificate */
> > + if (guidcmp(&siglist->sig_type, &efi_guid_cert_x509))
> > + continue;
> > +
> > + for (sig_data = siglist->sig_data_list; sig_data;
> > + sig_data = sig_data->next) {
> > + struct x509_certificate *cert_tmp;
> > +
> > + cert_tmp = x509_cert_parse(sig_data->data,
> > + sig_data->size);
> > + if (IS_ERR_OR_NULL(cert_tmp))
> > + continue;
> > +
> > + reg[0].data = cert_tmp->tbs;
> > + reg[0].size = cert_tmp->tbs_size;
> > + if (!efi_hash_regions(reg, 1, &hash_tmp, NULL))
> > + goto out;
> > +
> > + x509_free_certificate(cert_tmp);
> > +
> > + if (!memcmp(hash, hash_tmp, size)) {
> > + found = true;
> > + goto out;
> > + }
> > }
> > + }
> > +out:
> > + free(hash);
> > + free(hash_tmp);
> >
> > - verified = efi_signature_verify(regs, msg, signed_info, cert);
> > + EFI_PRINT("%s: Exit, found: %d\n", __func__, found);
> > + return found;
> > +}
> >
> > - if (verified) {
> > - if (valid_cert)
> > - *valid_cert = cert;
> > - else
> > - x509_free_certificate(cert);
> > - break;
> > +/**
> > + * efi_verify_certificate - verify certificate's signature with database
> > + * @signer: Certificate
> > + * @db: Signature database
> > + * @root: Certificate to verify @signer
> > + *
> > + * Determine if certificate pointed to by @signer may be verified
> > + * by one of certificates in signature database pointed to by @db.
> > + *
> > + * Return: true if certificate is verified, false otherwise.
> > + */
> > +static bool efi_verify_certificate(struct x509_certificate *signer,
> > + struct efi_signature_store *db,
> > + struct x509_certificate **root)
> > +{
> > + struct efi_signature_store *siglist;
> > + struct efi_sig_data *sig_data;
> > + struct x509_certificate *cert;
> > + bool verified = false;
> > + int ret;
> > +
> > + EFI_PRINT("%s: Enter, %p, %p\n", __func__, signer, db);
> > +
> > + if (!signer || !db || !db->sig_data_list)
> > + goto out;
> > +
> > + for (siglist = db; siglist; siglist = siglist->next) {
> > + /* only with x509 certificate */
> > + if (guidcmp(&siglist->sig_type, &efi_guid_cert_x509))
> > + continue;
> > +
> > + for (sig_data = siglist->sig_data_list; sig_data;
> > + sig_data = sig_data->next) {
> > + cert = x509_cert_parse(sig_data->data, sig_data->size);
> > + if (IS_ERR_OR_NULL(cert)) {
> > + EFI_PRINT("Cannot parse x509 certificate\n");
> > + continue;
> > + }
> > +
> > + ret = public_key_verify_signature(cert->pub,
> > + signer->sig);
> > + if (!ret) {
> > + verified = true;
> > + if (root)
> > + *root = cert;
> > + else
> > + x509_free_certificate(cert);
> > + goto out;
> > + }
> > + x509_free_certificate(cert);
> > }
> > - x509_free_certificate(cert);
> > }
> >
> > out:
> > @@ -423,9 +354,9 @@ bool efi_signature_verify_one(struct efi_image_regions *regs,
> > struct efi_signature_store *db)
> > {
> > struct pkcs7_signed_info *sinfo;
> > - struct efi_signature_store *siglist;
> > - struct x509_certificate *cert;
> > + struct x509_certificate *signer;
> > bool verified = false;
> > + int ret;
> >
> > EFI_PRINT("%s: Enter, %p, %p, %p\n", __func__, regs, msg, db);
> >
> > @@ -440,13 +371,29 @@ bool efi_signature_verify_one(struct efi_image_regions *regs,
> > EFI_PRINT("Signed Info: digest algo: %s, pkey algo: %s\n",
> > sinfo->sig->hash_algo, sinfo->sig->pkey_algo);
> >
> > - for (siglist = db; siglist; siglist = siglist->next)
> > - if (efi_signature_verify_with_list(regs, msg, sinfo,
> > - siglist, &cert)) {
> > + 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;
> > }
> > - EFI_PRINT("Valid certificate not in \"db\"\n");
> > + } else if (efi_verify_certificate(signer, db, NULL)) {
> > + verified = true;
> > + goto out;
> > + }
> > + EFI_PRINT("Valid certificate not in db\n");
> > }
> >
> > out:
> > @@ -454,8 +401,8 @@ out:
> > return verified;
> > }
> >
> > -/**
> > - * efi_signature_verify_with_sigdb - verify signatures with db and dbx
> > +/*
> > + * efi_signature_verify - verify signatures with db and dbx
> > * @regs: List of regions to be authenticated
> > * @msg: Signature
> > * @db: Signature database for trusted certificates
> > @@ -466,43 +413,71 @@ out:
> > *
> > * Return: true if verification for all signatures passed, false otherwise
> > */
> > -bool efi_signature_verify_with_sigdb(struct efi_image_regions *regs,
> > - struct pkcs7_message *msg,
> > - struct efi_signature_store *db,
> > - struct efi_signature_store *dbx)
> > +bool efi_signature_verify(struct efi_image_regions *regs,
> > + struct pkcs7_message *msg,
> > + struct efi_signature_store *db,
> > + struct efi_signature_store *dbx)
> > {
> > - struct pkcs7_signed_info *info;
> > - struct efi_signature_store *siglist;
> > - struct x509_certificate *cert;
> > + struct pkcs7_signed_info *sinfo;
> > + struct x509_certificate *signer, *root;
> > bool verified = false;
> > + int ret;
> >
> > EFI_PRINT("%s: Enter, %p, %p, %p, %p\n", __func__, regs, msg, db, dbx);
> >
> > if (!regs || !msg || !db || !db->sig_data_list)
> > goto out;
> >
> > - for (info = msg->signed_infos; info; info = info->next) {
> > + for (sinfo = msg->signed_infos; sinfo; sinfo = sinfo->next) {
> > EFI_PRINT("Signed Info: digest algo: %s, pkey algo: %s\n",
> > - info->sig->hash_algo, info->sig->pkey_algo);
> > + sinfo->sig->hash_algo, sinfo->sig->pkey_algo);
> >
> > - for (siglist = db; siglist; siglist = siglist->next) {
> > - if (efi_signature_verify_with_list(regs, msg, info,
> > - siglist, &cert))
> > - break;
> > - }
> > - if (!siglist) {
> > - EFI_PRINT("Valid certificate not in \"db\"\n");
> > + /*
> > + * only for authenticated variable.
> > + *
> > + * If this function is called for image,
> > + * hash calculation will be done in
> > + * pkcs7_verify_one().
> > + */
> > + if (!msg->data &&
> > + !efi_hash_regions(regs->reg, regs->num,
> > + (void **)&sinfo->sig->digest, NULL)) {
> > + EFI_PRINT("Digesting an image failed\n");
> > goto out;
> > }
> >
> > - if (!dbx || efi_signature_check_revocation(info, cert, dbx))
> > + EFI_PRINT("Verifying certificate chain\n");
> > + signer = NULL;
> > + ret = pkcs7_verify_one(msg, sinfo, &signer);
> > + if (ret == -ENOPKG)
> > continue;
> >
> > - EFI_PRINT("Certificate in \"dbx\"\n");
> > + if (ret < 0 || !signer)
> > + goto out;
> > +
> > + if (sinfo->blacklisted)
> > + goto out;
> > +
> > + EFI_PRINT("Verifying last certificate in chain\n");
> > + if (signer->self_signed) {
> > + if (efi_lookup_certificate(signer, db))
> > + if (efi_signature_check_revocation(sinfo,
> > + signer, dbx))
> > + continue;
> > + } else if (efi_verify_certificate(signer, db, &root)) {
> > + bool check;
> > +
> > + check = efi_signature_check_revocation(sinfo, root,
> > + dbx);
> > + x509_free_certificate(root);
> > + if (check)
> > + continue;
> > + }
> > +
> > + EFI_PRINT("Certificate chain didn't reach trusted CA\n");
> > goto out;
> > }
> > verified = true;
> > -
> > out:
> > EFI_PRINT("%s: Exit, verified: %d\n", __func__, verified);
> > return verified;
> > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> > index 39a848290380..6b5c5c45dc1d 100644
> > --- a/lib/efi_loader/efi_variable.c
> > +++ b/lib/efi_loader/efi_variable.c
> > @@ -241,12 +241,11 @@ static efi_status_t efi_variable_authenticate(u16 *variable,
> > }
> >
> > /* verify signature */
> > - if (efi_signature_verify_with_sigdb(regs, var_sig, truststore, NULL)) {
> > + if (efi_signature_verify(regs, var_sig, truststore, NULL)) {
> > EFI_PRINT("Verified\n");
> > } else {
> > if (truststore2 &&
> > - efi_signature_verify_with_sigdb(regs, var_sig,
> > - truststore2, NULL)) {
> > + efi_signature_verify(regs, var_sig, truststore2, NULL)) {
> > EFI_PRINT("Verified\n");
> > } else {
> > EFI_PRINT("Verifying variable's signature failed\n");
> >
>
More information about the U-Boot
mailing list