[PATCH] lib/crypto: Enable more algorithms in cert verification

[Ilias Apalodimas]

\> >
> > No that's not doable.  Things like EFI_TCG2 protocol needs that since
> > we use a sha1 in the tcg eventlog.
>
> I simply wonder why you can trust SHA1 in PCR/event log while you don't
> trust it in secure boot.

You don't trust the PCRs in the eventlog.  The eventlog is a human
readable form of the events which extended the PCRs to that state.  In
order to verify the eventlog you need to replay it and compare it
against the hardware PCRs.  The number and nature of hashing
algorithms is configurable but PCR bank configuration is not (yet)
supported in U-Boot.  However the TPMv2 (the only ones we support on
TCG2) never use sha1 exclusively.  The usual config for hardware is
sha1,256 (or for swtpm sha1,256,384,512).  I don't remember if
switching a TPMv2 into 'sha1 only' is permitted by the spec,  but even
if it is, that's a very very bad idea since it defeats the purpose of
a v2 hardware in the first place.  In any case we can apply similar
restrictions on the TCG code when we add the PCR bank config options.

Hope that clears it up
/Ilias

> -Takahiro Akashi
>
> > I've looked at the code a bit more
> > and not adding in db looks either bad or hard to reason about, since
> > we do have different storage backends(i.e efi variables in RPMB via
> > standaloneMM).  So one way to do this without affecting the generic
> > crypto code is
> >
> > bool efi_signature_verify(struct efi_image_regions *regs,
> >                 if (ret < 0 || !signer)
> >                         goto out;
> >
> > +               if (!strcmp(signer->sig->hash_algo, "sha1")) {
> > +                       pr_err("SHA1 support is disabled for EFI\n");
> > +                       goto out;
> > +               }
> > +
> >                 if (sinfo->blacklisted)
> >                         goto out;
> >
> > Cheers
> > /Ilias
> >
> > > -Takahiro Akashi
> > >
> > > > Cheers
> > > > /Ilias
> > > > >
> > > > > Best regards
> > > > >
> > > > > Heinrich
> > > > >
> > > > > >
> > > > > > Regards
> > > > > > /Ilias
> > > > > >>
> > > > > >> Best regards
> > > > > >>
> > > > > >> Heinrich
> > > > > >>
> > > > > >>>>
> > > > > >>>> I'm not sure that this naming rule, in particular the latter part, will
> > > > > >>>> always hold in the future while all the existing algo's observe it.
> > > > > >>>> (Maybe we need some note somewhere?)
> > > > > >>>
> > > > > >>> The if a few lines below will shield us and return -EINVAL.  How about
> > > > > >>> adding an error message there?
> > > > > >>>
> > > > > >>> Cheers
> > > > > >>> /Ilias
> > > > > >>>>
> > > > > >>>> -Takahiro Akashi
> > > > > >>>>
> > > > > >>>>> +
> > > > > >>>>> + if (ret >= sizeof(algo))
> > > > > >>>>> +         return -EINVAL;
> > > > > >>>>> +
> > > > > >>>>> + info.checksum = image_get_checksum_algo((const char *)algo);
> > > > > >>>>> + info.name = (const char *)algo;
> > > > > >>>>>           info.crypto = image_get_crypto_algo(info.name);
> > > > > >>>>> - if (IS_ERR(info.checksum) || IS_ERR(info.crypto))
> > > > > >>>>> + if (!info.checksum || !info.crypto)
> > > > > >>>>>                   return -ENOPKG;
> > > > > >>>>>
> > > > > >>>>>           info.key = pkey->key;
> > > > > >>>>> --
> > > > > >>>>> 2.30.2
> > > > > >>>>>
> > > > > >>
> > > > >