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

From: "Eric W. Biederman" <ebiederm@xmission.com>
To: Eric Biggers <ebiggers@kernel.org>
Cc: Ross Philipson <ross.philipson@oracle.com>,
	linux-kernel@vger.kernel.org,  x86@kernel.org,
	linux-integrity@vger.kernel.org,  linux-doc@vger.kernel.org,
	linux-crypto@vger.kernel.org,  kexec@lists.infradead.org,
	linux-efi@vger.kernel.org,  iommu@lists.linux-foundation.org,
	dpsmith@apertussolutions.com,  tglx@linutronix.de,
	 mingo@redhat.com, bp@alien8.de,  hpa@zytor.com,
	 dave.hansen@linux.intel.com, ardb@kernel.org,
	 mjg59@srcf.ucam.org, James.Bottomley@hansenpartnership.com,
	 peterhuewe@gmx.de, jarkko@kernel.org,  jgg@ziepe.ca,
	 luto@amacapital.net, nivedita@alum.mit.edu,
	 herbert@gondor.apana.org.au, davem@davemloft.net,
	 corbet@lwn.net,  dwmw2@infradead.org, baolu.lu@linux.intel.com,
	 kanth.ghatraju@oracle.com, andrew.cooper3@citrix.com,
	 trenchboot-devel@googlegroups.com
Subject: Re: [PATCH v9 06/19] x86: Add early SHA-1 support for Secure Launch early measurements
Date: Fri, 31 May 2024 08:54:21 -0500	[thread overview]
Message-ID: <874jaegk8i.fsf@email.froward.int.ebiederm.org> (raw)
In-Reply-To: <20240531021656.GA1502@sol.localdomain> (Eric Biggers's message of "Thu, 30 May 2024 19:16:56 -0700")

Eric Biggers <ebiggers@kernel.org> writes:

> On Thu, May 30, 2024 at 06:03:18PM -0700, Ross Philipson wrote:
>> From: "Daniel P. Smith" <dpsmith@apertussolutions.com>
>> 
>> 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@apertussolutions.com>
>> Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
>
> 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?

No offense to your Trenchboot project but my gut says that anything new
relying on SHA-1 is probably a bad joke at this point.

Firmware can most definitely be upgraded and if the goal is a more
secure boot the usual backwards compatibility concerns for supporting
old firmware really don't apply.

Perhaps hide all of the SHA-1 stuff behind CONFIG_TRENCHBOOT_PROTOTYPE
or something like that to make it clear that SHA-1 is not appropriate
for any kind of production deployment and is only good for prototyping
your implementation until properly implemented firmware comes along.

Eric

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec