From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heinrich Schuchardt Date: Wed, 22 Jul 2020 13:00:30 +0200 Subject: [PATCH v5 6/8] efi_loader: signature: rework for intermediate certificates support In-Reply-To: <20200721103524.5956-7-takahiro.akashi@linaro.org> References: <20200721103524.5956-1-takahiro.akashi@linaro.org> <20200721103524.5956-7-takahiro.akashi@linaro.org> Message-ID: <19bd9bda-aab8-a202-e78d-9e8d397749f3@gmx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > --- > 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 > #include > #include > +#include > #include > +#include > #include This line does not exist in origin master. Which patch is missing? 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 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 > #include > @@ -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 "," > - * 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"); >