Re: [PATCH v9 06/19] x86: Add early SHA-1 support for Secure Launch early measurements

[Daniel P. Smith]

On 5/31/24 09:54, Eric W. Biederman wrote:
> Eric Biggers <ebiggers@xxxxxxxxxx> writes:
>
> > On Thu, May 30, 2024 at 06:03:18PM -0700, Ross Philipson wrote:
> > > From: "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> > >
> > > For better or worse, Secure Launch needs SHA-1 and SHA-256. The
> > > choice of hashes used lie with the platform firmware, not with
> > > software, and is often outside of the users control.
> > >
> > > Even if we'd prefer to use SHA-256-only, if firmware elected to start us
> > > with the SHA-1 and SHA-256 backs active, we still need SHA-1 to parse
> > > the TPM event log thus far, and deliberately cap the SHA-1 PCRs in order
> > > to safely use SHA-256 for everything else.
> > >
> > > The SHA-1 code here has its origins in the code from the main kernel:
> > >
> > > commit c4d5b9ffa31f ("crypto: sha1 - implement base layer for SHA-1")
> > >
> > > A modified version of this code was introduced to the lib/crypto/sha1.c
> > > to bring it in line with the SHA-256 code and allow it to be pulled into the
> > > setup kernel in the same manner as SHA-256 is.
> > >
> > > Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> > > Signed-off-by: Ross Philipson <ross.philipson@xxxxxxxxxx>
> >
> > Thanks.  This explanation doesn't seem to have made it into the actual code or
> > documentation.  Can you please get it into a more permanent location?
> >
> > Also, can you point to where the "deliberately cap the SHA-1 PCRs" thing happens
> > in the code?
> >
> > That paragraph is also phrased as a hypothetical, "Even if we'd prefer to use
> > SHA-256-only".  That implies that you do not, in fact, prefer SHA-256 only.  Is
> > that the case?  Sure, maybe there are situations where you *have* to use SHA-1,
> > but why would you not at least *prefer* SHA-256?
>
> Yes.  Please prefer to use SHA-256.
>
> Have you considered implementing I think it is SHA1-DC (as git has) that
> is compatible with SHA1 but blocks the known class of attacks where
> sha1 is actively broken at this point?

We are using the kernel's implementation, addressing what the kernel 
provides is beyond our efforts. Perhaps someone who is interested in 
improving the kernel's SHA1 could submit a patch implementing/replacing 
it with SHA1-DC, as I am sure the maintainers would welcome the help.

v/r,
dps