public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: AKASHI Takahiro <takahiro.akashi@linaro.org>
To: u-boot@lists.denx.de
Subject: [PATCH v2 08/17] efi_loader: signature: fix a size check against revocation list
Date: Wed, 8 Jul 2020 10:30:24 +0900	[thread overview]
Message-ID: <20200708013024.GA18146@laputa> (raw)
In-Reply-To: <20200708011238.GA16575@laputa>

On Wed, Jul 08, 2020 at 10:12:38AM +0900, AKASHI Takahiro wrote:
> On Fri, Jul 03, 2020 at 01:00:21PM +0200, Heinrich Schuchardt wrote:
> > On 09.06.20 07:09, AKASHI Takahiro wrote:
> > > Since the size check against an entry in efi_search_siglist() is
> > > incorrect, this function will never find out a to-be-matched certificate
> > > and its associated revocation time in the signature list.
> > >
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > ---
> > >  lib/efi_loader/efi_signature.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
> > > index a05c75472721..f22dc151971f 100644
> > > --- a/lib/efi_loader/efi_signature.c
> > > +++ b/lib/efi_loader/efi_signature.c
> > > @@ -434,10 +434,11 @@ static bool efi_search_siglist(struct x509_certificate *cert,
> > >  		 *	time64_t revocation_time;
> > >  		 * };
> > >  		 */
> > > -		if ((sig_data->size == SHA256_SUM_LEN) &&
> > > -		    !memcmp(sig_data->data, hash, SHA256_SUM_LEN)) {
> > > +		if ((sig_data->size >= SHA256_SUM_LEN + sizeof(time64_t)) &&
> > > +		    !memcmp(sig_data->data, msg, SHA256_SUM_LEN)) {
> > >  			memcpy(revoc_time, sig_data->data + SHA256_SUM_LEN,
> > >  			       sizeof(*revoc_time));
> > > +			EFI_PRINT("revocation time: %llu\n", *revoc_time);
> > 
> > *revoc_time is of type __s64. So this must be %lld. Cf.
> > 
> > include/linux/time.h:156
> 
> I know that because I added the definition.
> Interestingly, linux added another type, timeu64_t, later on
> to avoid an overflow in some calculation.
> 
> While I don't think the current format is harmful, I will change it.

Oops, I have already changed the format to "0x%llx" locally.
I will stick to this.


> -Takahiro Akashi
> 
> 
> > Best regards
> > 
> > Heinrich
> > 
> > >  			found = true;
> > >  			goto out;
> > >  		}
> > >
> > 

  reply	other threads:[~2020-07-08  1:30 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-09  5:09 [PATCH v2 00/17] efi_loader: rework/improve UEFI secure boot code AKASHI Takahiro
2020-06-09  5:09 ` [PATCH v2 01/17] efi_loader: change efi objects initialization order AKASHI Takahiro
2020-07-03 10:29   ` Heinrich Schuchardt
2020-06-09  5:09 ` [PATCH v2 02/17] Revert "test: stabilize test_efi_secboot" AKASHI Takahiro
2020-07-03 10:30   ` Heinrich Schuchardt
2020-06-09  5:09 ` [PATCH v2 03/17] efi_loader: signature: replace debug to EFI_PRINT AKASHI Takahiro
2020-07-03 10:30   ` Heinrich Schuchardt
2020-06-09  5:09 ` [PATCH v2 04/17] efi_loader: variable: " AKASHI Takahiro
2020-06-09  5:09 ` [PATCH v2 05/17] efi_loader: image_loader: " AKASHI Takahiro
2020-07-03 10:38   ` Heinrich Schuchardt
2020-06-09  5:09 ` [PATCH v2 06/17] efi_loader: image_loader: add a check against certificate type of authenticode AKASHI Takahiro
2020-07-03 10:56   ` Heinrich Schuchardt
2020-07-08  1:08     ` AKASHI Takahiro
2020-06-09  5:09 ` [PATCH v2 07/17] efi_loader: image_loader: retrieve authenticode only if it exists AKASHI Takahiro
2020-06-09  5:09 ` [PATCH v2 08/17] efi_loader: signature: fix a size check against revocation list AKASHI Takahiro
2020-07-03 11:00   ` Heinrich Schuchardt
2020-07-08  1:12     ` AKASHI Takahiro
2020-07-08  1:30       ` AKASHI Takahiro [this message]
2020-06-09  5:09 ` [PATCH v2 09/17] efi_loader: signature: make efi_hash_regions more generic AKASHI Takahiro
2020-07-03 11:08   ` Heinrich Schuchardt
2020-07-08  1:22     ` AKASHI Takahiro
2020-06-09  5:09 ` [PATCH v2 10/17] efi_loader: image_loader: verification for all signatures should pass AKASHI Takahiro
2020-06-09  7:14   ` Heinrich Schuchardt
2020-06-09  5:09 ` [PATCH v2 11/17] efi_loader: image_loader: add digest-based verification for signed image AKASHI Takahiro
2020-06-09  5:09 ` [PATCH v2 12/17] test/py: efi_secboot: remove all "re.search" AKASHI Takahiro
2020-07-03 15:52   ` Heinrich Schuchardt
2020-06-09  5:09 ` [PATCH v2 13/17] test/py: efi_secboot: fix test case 1g of test_authvar AKASHI Takahiro
2020-07-03 16:08   ` Heinrich Schuchardt
2020-06-09  5:09 ` [PATCH v2 14/17] test/py: efi_secboot: split "signed image" test case-1 into two cases AKASHI Takahiro
2020-07-03 16:14   ` Heinrich Schuchardt
2020-06-09  5:09 ` [PATCH v2 15/17] test/py: efi_secboot: add a test against certificate revocation AKASHI Takahiro
2020-06-09  5:09 ` [PATCH v2 16/17] test/py: efi_secboot: add a test for multiple signatures AKASHI Takahiro
2020-06-09  5:09 ` [PATCH v2 17/17] test/py: efi_secboot: add a test for verifying with digest of signed image AKASHI Takahiro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200708013024.GA18146@laputa \
    --to=takahiro.akashi@linaro.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox