| Message ID | 20220705054815.30318-5-takahiro.akashi@linaro.org |
|---|---|
| State | Accepted |
| Commit | 634f6b2fb1056021fba603ccb7488d1864787576 |
| Headers | show |
| Series | efi_loader: fix a verification process issue in secure boot | expand |
On 7/5/22 07:48, AKASHI Takahiro wrote: > At the last step of PE image authentication, an image's hash value must be > compared with a message digest stored as the content (of SpcPeImageData type) > of pkcs7's contentInfo. > > Fixes: commit 4540dabdcaca ("efi_loader: image_loader: support image authentication") > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > lib/efi_loader/Kconfig | 1 + > lib/efi_loader/efi_image_loader.c | 62 ++++++++++++++++++++++++++++++- > 2 files changed, 61 insertions(+), 2 deletions(-) > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > index e2a1a5a69a24..e3f2402d0e8e 100644 > --- a/lib/efi_loader/Kconfig > +++ b/lib/efi_loader/Kconfig > @@ -366,6 +366,7 @@ config EFI_SECURE_BOOT > select X509_CERTIFICATE_PARSER > select PKCS7_MESSAGE_PARSER > select PKCS7_VERIFY > + select MSCODE_PARSER > select EFI_SIGNATURE_SUPPORT > 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 fe8e4a89082c..eaf75a5803d4 100644 > --- a/lib/efi_loader/efi_image_loader.c > +++ b/lib/efi_loader/efi_image_loader.c > @@ -16,6 +16,7 @@ > #include <malloc.h> > #include <pe.h> > #include <sort.h> > +#include <crypto/mscode.h> > #include <crypto/pkcs7_parser.h> > #include <linux/err.h> > > @@ -516,6 +517,51 @@ err: > } > > #ifdef CONFIG_EFI_SECURE_BOOT > +/** > + * efi_image_verify_digest - verify image's message digest > + * @regs: Array of memory regions to digest > + * @msg: Signature in pkcs7 structure > + * > + * @regs contains all the data in a PE image to digest. Calculate > + * a hash value based on @regs and compare it with a messaged digest > + * in the content (SpcPeImageData) of @msg's contentInfo. > + * > + * Return: true if verified, false if not > + */ > +static bool efi_image_verify_digest(struct efi_image_regions *regs, > + struct pkcs7_message *msg) > +{ > + struct pefile_context ctx; > + void *hash; > + int hash_len, ret; > + > + const void *data; > + size_t data_len; > + size_t asn1hdrlen; > + > + /* get pkcs7's contentInfo */ > + ret = pkcs7_get_content_data(msg, &data, &data_len, &asn1hdrlen); > + if (ret < 0 || !data) > + return false; > + > + /* parse data and retrieve a message digest into ctx */ > + ret = mscode_parse(&ctx, data, data_len, asn1hdrlen); > + if (ret < 0) > + return false; > + > + /* calculate a hash value of PE image */ > + hash = NULL; > + if (!efi_hash_regions(regs->reg, regs->num, &hash, ctx.digest_algo, > + &hash_len)) > + return false; > + > + /* match the digest */ > + if (ctx.digest_len != hash_len || memcmp(ctx.digest, hash, hash_len)) > + return false; > + > + return true; > +} > + > /** > * efi_image_authenticate() - verify a signature of signed image > * @efi: Pointer to image > @@ -645,6 +691,9 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) > } > > /* > + * verify signatures in pkcs7's signedInfos which are > + * to authenticate the integrity of pkcs7's contentInfo. > + * > * NOTE: > * UEFI specification defines two signature types possible > * in signature database: > @@ -677,12 +726,21 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) > } > > /* try white-list */ > - if (efi_signature_verify(regs, msg, db, dbx)) { > + if (!efi_signature_verify(regs, msg, db, dbx)) { > + log_debug("Signature was not verified by \"db\"\n"); Shouldn't this be log_err()? > + continue; > + } > + > + /* > + * now calculate an image's hash value and compare it with > + * a messaged digest embedded in pkcs7's contentInfo > + */ > + if (efi_image_verify_digest(regs, msg)) { > ret = true; > continue; > } > > - log_debug("Signature was not verified by \"db\"\n"); ditto? Or does the caller write an error message somewhere? Best regards Heinrich > + log_debug("Message digest doesn't match\n"); > } > >
Heinrich, On Tue, Jul 05, 2022 at 05:29:07PM +0200, Heinrich Schuchardt wrote: > On 7/5/22 07:48, AKASHI Takahiro wrote: > > At the last step of PE image authentication, an image's hash value must be > > compared with a message digest stored as the content (of SpcPeImageData type) > > of pkcs7's contentInfo. > > > > Fixes: commit 4540dabdcaca ("efi_loader: image_loader: support image authentication") > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > lib/efi_loader/Kconfig | 1 + > > lib/efi_loader/efi_image_loader.c | 62 ++++++++++++++++++++++++++++++- > > 2 files changed, 61 insertions(+), 2 deletions(-) > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index e2a1a5a69a24..e3f2402d0e8e 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -366,6 +366,7 @@ config EFI_SECURE_BOOT > > select X509_CERTIFICATE_PARSER > > select PKCS7_MESSAGE_PARSER > > select PKCS7_VERIFY > > + select MSCODE_PARSER > > select EFI_SIGNATURE_SUPPORT > > 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 fe8e4a89082c..eaf75a5803d4 100644 > > --- a/lib/efi_loader/efi_image_loader.c > > +++ b/lib/efi_loader/efi_image_loader.c > > @@ -16,6 +16,7 @@ > > #include <malloc.h> > > #include <pe.h> > > #include <sort.h> > > +#include <crypto/mscode.h> > > #include <crypto/pkcs7_parser.h> > > #include <linux/err.h> > > > > @@ -516,6 +517,51 @@ err: > > } > > > > #ifdef CONFIG_EFI_SECURE_BOOT > > +/** > > + * efi_image_verify_digest - verify image's message digest > > + * @regs: Array of memory regions to digest > > + * @msg: Signature in pkcs7 structure > > + * > > + * @regs contains all the data in a PE image to digest. Calculate > > + * a hash value based on @regs and compare it with a messaged digest > > + * in the content (SpcPeImageData) of @msg's contentInfo. > > + * > > + * Return: true if verified, false if not > > + */ > > +static bool efi_image_verify_digest(struct efi_image_regions *regs, > > + struct pkcs7_message *msg) > > +{ > > + struct pefile_context ctx; > > + void *hash; > > + int hash_len, ret; > > + > > + const void *data; > > + size_t data_len; > > + size_t asn1hdrlen; > > + > > + /* get pkcs7's contentInfo */ > > + ret = pkcs7_get_content_data(msg, &data, &data_len, &asn1hdrlen); > > + if (ret < 0 || !data) > > + return false; > > + > > + /* parse data and retrieve a message digest into ctx */ > > + ret = mscode_parse(&ctx, data, data_len, asn1hdrlen); > > + if (ret < 0) > > + return false; > > + > > + /* calculate a hash value of PE image */ > > + hash = NULL; > > + if (!efi_hash_regions(regs->reg, regs->num, &hash, ctx.digest_algo, > > + &hash_len)) > > + return false; > > + > > + /* match the digest */ > > + if (ctx.digest_len != hash_len || memcmp(ctx.digest, hash, hash_len)) > > + return false; > > + > > + return true; > > +} > > + > > /** > > * efi_image_authenticate() - verify a signature of signed image > > * @efi: Pointer to image > > @@ -645,6 +691,9 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) > > } > > > > /* > > + * verify signatures in pkcs7's signedInfos which are > > + * to authenticate the integrity of pkcs7's contentInfo. > > + * > > * NOTE: > > * UEFI specification defines two signature types possible > > * in signature database: > > @@ -677,12 +726,21 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) > > } > > > > /* try white-list */ > > - if (efi_signature_verify(regs, msg, db, dbx)) { > > + if (!efi_signature_verify(regs, msg, db, dbx)) { > > + log_debug("Signature was not verified by \"db\"\n"); > > Shouldn't this be log_err()? I think that I have already explained my idea behind here in my previous reply. > > + continue; > > + } > > + > > + /* > > + * now calculate an image's hash value and compare it with > > + * a messaged digest embedded in pkcs7's contentInfo > > + */ > > + if (efi_image_verify_digest(regs, msg)) { > > ret = true; > > continue; > > } > > > > - log_debug("Signature was not verified by \"db\"\n"); > > ditto? > > Or does the caller write an error message somewhere? Yes: efi_load_pe() ... /* Authenticate an image */ if (efi_image_authenticate(efi, efi_size)) { handle->auth_status = EFI_IMAGE_AUTH_PASSED; } else { handle->auth_status = EFI_IMAGE_AUTH_FAILED; log_err("Image not authenticated\n"); } -Takahiro Akashi > Best regards > > Heinrich > > > + log_debug("Message digest doesn't match\n"); > > } > > > > >
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index e2a1a5a69a24..e3f2402d0e8e 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -366,6 +366,7 @@ config EFI_SECURE_BOOT select X509_CERTIFICATE_PARSER select PKCS7_MESSAGE_PARSER select PKCS7_VERIFY + select MSCODE_PARSER select EFI_SIGNATURE_SUPPORT 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 fe8e4a89082c..eaf75a5803d4 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -16,6 +16,7 @@ #include <malloc.h> #include <pe.h> #include <sort.h> +#include <crypto/mscode.h> #include <crypto/pkcs7_parser.h> #include <linux/err.h> @@ -516,6 +517,51 @@ err: } #ifdef CONFIG_EFI_SECURE_BOOT +/** + * efi_image_verify_digest - verify image's message digest + * @regs: Array of memory regions to digest + * @msg: Signature in pkcs7 structure + * + * @regs contains all the data in a PE image to digest. Calculate + * a hash value based on @regs and compare it with a messaged digest + * in the content (SpcPeImageData) of @msg's contentInfo. + * + * Return: true if verified, false if not + */ +static bool efi_image_verify_digest(struct efi_image_regions *regs, + struct pkcs7_message *msg) +{ + struct pefile_context ctx; + void *hash; + int hash_len, ret; + + const void *data; + size_t data_len; + size_t asn1hdrlen; + + /* get pkcs7's contentInfo */ + ret = pkcs7_get_content_data(msg, &data, &data_len, &asn1hdrlen); + if (ret < 0 || !data) + return false; + + /* parse data and retrieve a message digest into ctx */ + ret = mscode_parse(&ctx, data, data_len, asn1hdrlen); + if (ret < 0) + return false; + + /* calculate a hash value of PE image */ + hash = NULL; + if (!efi_hash_regions(regs->reg, regs->num, &hash, ctx.digest_algo, + &hash_len)) + return false; + + /* match the digest */ + if (ctx.digest_len != hash_len || memcmp(ctx.digest, hash, hash_len)) + return false; + + return true; +} + /** * efi_image_authenticate() - verify a signature of signed image * @efi: Pointer to image @@ -645,6 +691,9 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) } /* + * verify signatures in pkcs7's signedInfos which are + * to authenticate the integrity of pkcs7's contentInfo. + * * NOTE: * UEFI specification defines two signature types possible * in signature database: @@ -677,12 +726,21 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) } /* try white-list */ - if (efi_signature_verify(regs, msg, db, dbx)) { + if (!efi_signature_verify(regs, msg, db, dbx)) { + log_debug("Signature was not verified by \"db\"\n"); + continue; + } + + /* + * now calculate an image's hash value and compare it with + * a messaged digest embedded in pkcs7's contentInfo + */ + if (efi_image_verify_digest(regs, msg)) { ret = true; continue; } - log_debug("Signature was not verified by \"db\"\n"); + log_debug("Message digest doesn't match\n"); }
At the last step of PE image authentication, an image's hash value must be compared with a message digest stored as the content (of SpcPeImageData type) of pkcs7's contentInfo. Fixes: commit 4540dabdcaca ("efi_loader: image_loader: support image authentication") Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- lib/efi_loader/Kconfig | 1 + lib/efi_loader/efi_image_loader.c | 62 ++++++++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 2 deletions(-)