Re: [PATCH v8 06/15] x86: Add early SHA support for Secure Launch early measurements

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Ard Biesheuvel <ardb@kernel.org>,
	Ross Philipson <ross.philipson@oracle.com>
Cc: 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, dpsmith@apertussolutions.com,
	tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
	hpa@zytor.com, dave.hansen@linux.intel.com, 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, kanth.ghatraju@oracle.com,
	trenchboot-devel@googlegroups.com,
	Eric Biggers <ebiggers@kernel.org>
Subject: Re: [PATCH v8 06/15] x86: Add early SHA support for Secure Launch early measurements
Date: Thu, 22 Feb 2024 03:04:55 +0000	[thread overview]
Message-ID: <98ad92bb-ef17-4c15-88ba-252db2a2e738@citrix.com> (raw)
In-Reply-To: <CAMj1kXEmMBY_jc0uM5UgZbuZ3-C7NPKzg5AScaunyu9XzLgzZA@mail.gmail.com>

On 15/02/2024 8:17 am, Ard Biesheuvel wrote:
> On Wed, 14 Feb 2024 at 23:31, Ross Philipson <ross.philipson@oracle.com> wrote:
>> From: "Daniel P. Smith" <dpsmith@apertussolutions.com>
>>
>> The SHA algorithms are necessary to measure configuration information into
>> the TPM as early as possible before using the values. This implementation
>> uses the established approach of #including the SHA libraries directly in
>> the code since the compressed kernel is not uncompressed at this point.
>>
>> The SHA 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 sha256 code and allow it to be pulled into the
>> setup kernel in the same manner as sha256 is.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
> We have had some discussions about this, and you really need to
> capture the justification in the commit log for introducing new code
> that implements an obsolete and broken hashing algorithm.
>
> SHA-1 is broken and should no longer be used for anything. Introducing
> new support for a highly complex boot security feature, and then
> relying on SHA-1 in the implementation makes this whole effort seem
> almost futile, *unless* you provide some rock solid reasons here why
> this is still safe.
>
> If the upshot would be that some people are stuck with SHA-1 so they
> won't be able to use this feature, then I'm not convinced we should
> obsess over that.

To be absolutely crystal clear here.

The choice of hash algorithm(s) are determined by the OEM and the
platform, not by Linux.

Failing to (at least) cap a PCR in a bank which the OEM/platform left
active is a security vulnerability.  It permits the unsealing of secrets
if an attacker can replay a good set of measurements into an unused bank.

The only way to get rid of the requirement for SHA-1 here is to lobby
the IHVs/OEMs, or perhaps the TCG, to produce/spec a platform where the
SHA-1 banks can be disabled.  There are no known such platforms in the
market today, to the best of our knowledge.

~Andrew