From: ross.philipson@oracle.com To: Eric Biggers <ebiggers@kernel.org>, Andy Lutomirski <luto@kernel.org> Cc: Andrew Cooper <andrew.cooper3@citrix.com>, Ard Biesheuvel <ardb@kernel.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, the arch/x86 maintainers <x86@kernel.org>, linux-integrity@vger.kernel.org, linux-doc@vger.kernel.org, Linux Crypto Mailing List <linux-crypto@vger.kernel.org>, kexec@lists.infradead.org, linux-efi@vger.kernel.org, dpsmith@apertussolutions.com, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, "H. Peter Anvin" <hpa@zytor.com>, Dave Hansen <dave.hansen@linux.intel.com>, Matthew Garrett <mjg59@srcf.ucam.org>, James.Bottomley@hansenpartnership.com, peterhuewe@gmx.de, jarkko@kernel.org, Jason Gunthorpe <jgg@ziepe.ca>, "luto@amacapital.net" <luto@amacapital.net>, Arvind Sankar <nivedita@alum.mit.edu>, Herbert Xu <herbert@gondor.apana.org.au>, davem@davemloft.net, kanth.ghatraju@oracle.com, trenchboot-devel@googlegroups.com Subject: Re: [PATCH v8 06/15] x86: Add early SHA support for Secure Launch early measurements Date: Wed, 3 Apr 2024 21:55:41 -0700 [thread overview] Message-ID: <8b6285e8-bb00-4541-b054-ee01c86d367a@oracle.com> (raw) In-Reply-To: <20240403235635.GA24248@quark.localdomain> On 4/3/24 4:56 PM, Eric Biggers wrote: > On Wed, Apr 03, 2024 at 09:32:02AM -0700, Andy Lutomirski wrote: >> On Fri, Feb 23, 2024, at 10:30 AM, Eric Biggers wrote: >>> On Fri, Feb 23, 2024 at 06:20:27PM +0000, Andrew Cooper wrote: >>>> On 23/02/2024 5:54 pm, Eric Biggers wrote: >>>>> On Fri, Feb 23, 2024 at 04:42:11PM +0000, Andrew Cooper wrote: >>>>>> Yes, and I agree. We're not looking to try and force this in with >>>>>> underhand tactics. >>>>>> >>>>>> But a blind "nack to any SHA-1" is similarly damaging in the opposite >>>>>> direction. >>>>>> >>>>> Well, reviewers have said they'd prefer that SHA-1 not be included and given >>>>> some thoughtful reasons for that. But also they've given suggestions on how to >>>>> make the SHA-1 support more palatable, such as splitting it into a separate >>>>> patch and giving it a proper justification. >>>>> >>>>> All suggestions have been ignored. >>>> >>>> The public record demonstrates otherwise. >>>> >>>> But are you saying that you'd be happy if the commit message read >>>> something more like: >>>> >>>> ---8<--- >>>> 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. >>>> --- >>> >>> Please take some time to read through the comments that reviewers have left on >>> previous versions of the patchset. >> >> So I went and read through the old comments, and I'm lost. In brief summary: >> >> If the hardware+firmware only supports SHA-1, then some reviewers would prefer >> Linux not to support DRTM. I personally think this is a bit silly, but it's >> not entirely unreasonable. Maybe it should be a config option? >> >> If the hardware+firmware does support SHA-256, then it sounds (to me, reading >> this -- I haven't dug into the right spec pages) that, for optimal security, >> something still needs to effectively turn SHA-1 *off* at runtime by capping >> the event log properly. And that requires computing a SHA-1 hash. And, to be >> clear, (a) this is only on systems that already support SHA-256 and that we >> should support and (b) *not* doing so leaves us potentially more vulnerable to >> SHA-1 attacks than doing so. And no SHA-256-supporting tooling will actually >> be compromised by a SHA-1 compromise if we cap the event log. >> >> So is there a way forward? Just saying "read through the comments" seems like >> a dead end. >> > > It seems there may be a justification for some form of SHA-1 support in this > feature. As I've said, the problem is that it's not explained in the patchset > itself. Rather, it just talks about "SHA" and pretends like SHA-1 and SHA-2 are > basically the same. In fact, SHA-1 differs drastically from SHA-2 in terms of > security. SHA-1 support should be added in a separate patch, with a clearly > explained rationale *in the patch itself* for the SHA-1 support *specifically*. For the record, we were never trying to "pretend" or obfuscate the use of SHA-1. It was simply expedient to put the hash algorithm changes in one patch. We have now separated the patches for clarity and will add any text that explains our use and/or explain the issues with its use. We went back through the comments and tried to address everything that came up about the use of SHA-1. We will review it all again before posting another patch set. Thank you for your feedback. Ross > > - Eric _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
WARNING: multiple messages have this Message-ID (diff)
From: ross.philipson@oracle.com To: Eric Biggers <ebiggers@kernel.org>, Andy Lutomirski <luto@kernel.org> Cc: Andrew Cooper <andrew.cooper3@citrix.com>, Ard Biesheuvel <ardb@kernel.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, the arch/x86 maintainers <x86@kernel.org>, linux-integrity@vger.kernel.org, linux-doc@vger.kernel.org, Linux Crypto Mailing List <linux-crypto@vger.kernel.org>, kexec@lists.infradead.org, linux-efi@vger.kernel.org, dpsmith@apertussolutions.com, Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, "H. Peter Anvin" <hpa@zytor.com>, Dave Hansen <dave.hansen@linux.intel.com>, Matthew Garrett <mjg59@srcf.ucam.org>, James.Bottomley@hansenpartnership.com, peterhuewe@gmx.de, jarkko@kernel.org, Jason Gunthorpe <jgg@ziepe.ca>, "luto@amacapital.net" <luto@amacapital.net>, Arvind Sankar <nivedita@alum.mit.edu>, Herbert Xu <herbert@gondor.apana.org.au>, davem@davemloft.net, kanth.ghatraju@oracle.com, trenchboot-devel@googlegroups.com Subject: Re: [PATCH v8 06/15] x86: Add early SHA support for Secure Launch early measurements Date: Wed, 3 Apr 2024 21:55:41 -0700 [thread overview] Message-ID: <8b6285e8-bb00-4541-b054-ee01c86d367a@oracle.com> (raw) In-Reply-To: <20240403235635.GA24248@quark.localdomain> On 4/3/24 4:56 PM, Eric Biggers wrote: > On Wed, Apr 03, 2024 at 09:32:02AM -0700, Andy Lutomirski wrote: >> On Fri, Feb 23, 2024, at 10:30 AM, Eric Biggers wrote: >>> On Fri, Feb 23, 2024 at 06:20:27PM +0000, Andrew Cooper wrote: >>>> On 23/02/2024 5:54 pm, Eric Biggers wrote: >>>>> On Fri, Feb 23, 2024 at 04:42:11PM +0000, Andrew Cooper wrote: >>>>>> Yes, and I agree. We're not looking to try and force this in with >>>>>> underhand tactics. >>>>>> >>>>>> But a blind "nack to any SHA-1" is similarly damaging in the opposite >>>>>> direction. >>>>>> >>>>> Well, reviewers have said they'd prefer that SHA-1 not be included and given >>>>> some thoughtful reasons for that. But also they've given suggestions on how to >>>>> make the SHA-1 support more palatable, such as splitting it into a separate >>>>> patch and giving it a proper justification. >>>>> >>>>> All suggestions have been ignored. >>>> >>>> The public record demonstrates otherwise. >>>> >>>> But are you saying that you'd be happy if the commit message read >>>> something more like: >>>> >>>> ---8<--- >>>> 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. >>>> --- >>> >>> Please take some time to read through the comments that reviewers have left on >>> previous versions of the patchset. >> >> So I went and read through the old comments, and I'm lost. In brief summary: >> >> If the hardware+firmware only supports SHA-1, then some reviewers would prefer >> Linux not to support DRTM. I personally think this is a bit silly, but it's >> not entirely unreasonable. Maybe it should be a config option? >> >> If the hardware+firmware does support SHA-256, then it sounds (to me, reading >> this -- I haven't dug into the right spec pages) that, for optimal security, >> something still needs to effectively turn SHA-1 *off* at runtime by capping >> the event log properly. And that requires computing a SHA-1 hash. And, to be >> clear, (a) this is only on systems that already support SHA-256 and that we >> should support and (b) *not* doing so leaves us potentially more vulnerable to >> SHA-1 attacks than doing so. And no SHA-256-supporting tooling will actually >> be compromised by a SHA-1 compromise if we cap the event log. >> >> So is there a way forward? Just saying "read through the comments" seems like >> a dead end. >> > > It seems there may be a justification for some form of SHA-1 support in this > feature. As I've said, the problem is that it's not explained in the patchset > itself. Rather, it just talks about "SHA" and pretends like SHA-1 and SHA-2 are > basically the same. In fact, SHA-1 differs drastically from SHA-2 in terms of > security. SHA-1 support should be added in a separate patch, with a clearly > explained rationale *in the patch itself* for the SHA-1 support *specifically*. For the record, we were never trying to "pretend" or obfuscate the use of SHA-1. It was simply expedient to put the hash algorithm changes in one patch. We have now separated the patches for clarity and will add any text that explains our use and/or explain the issues with its use. We went back through the comments and tried to address everything that came up about the use of SHA-1. We will review it all again before posting another patch set. Thank you for your feedback. Ross > > - Eric
next prev parent reply other threads:[~2024-04-04 4:56 UTC|newest] Thread overview: 116+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-02-14 22:18 [PATCH v8 00/15] x86: Trenchboot secure dynamic launch Linux kernel support Ross Philipson 2024-02-14 22:18 ` Ross Philipson 2024-02-14 22:18 ` [PATCH v8 01/15] x86/boot: Place kernel_info at a fixed offset Ross Philipson 2024-02-14 22:18 ` Ross Philipson 2024-02-15 7:56 ` Ard Biesheuvel 2024-02-15 7:56 ` Ard Biesheuvel 2024-02-15 10:56 ` Daniel Kiper 2024-02-15 10:56 ` Daniel Kiper 2024-03-21 13:45 ` Daniel P. Smith 2024-03-21 13:45 ` Daniel P. Smith 2024-03-22 14:18 ` H. Peter Anvin 2024-03-22 14:18 ` H. Peter Anvin 2024-03-23 1:33 ` Daniel P. Smith 2024-03-23 1:33 ` Daniel P. Smith 2024-02-14 22:18 ` [PATCH v8 02/15] Documentation/x86: Secure Launch kernel documentation Ross Philipson 2024-02-14 22:18 ` Ross Philipson 2024-02-14 22:18 ` [PATCH v8 03/15] x86: Secure Launch Kconfig Ross Philipson 2024-02-14 22:18 ` Ross Philipson 2024-02-15 7:59 ` Ard Biesheuvel 2024-02-15 7:59 ` Ard Biesheuvel 2024-02-15 22:20 ` ross.philipson 2024-02-15 22:20 ` ross.philipson 2024-02-14 22:18 ` [PATCH v8 04/15] x86: Secure Launch Resource Table header file Ross Philipson 2024-02-14 22:18 ` Ross Philipson 2024-02-15 8:08 ` Ard Biesheuvel 2024-02-15 8:08 ` Ard Biesheuvel 2024-02-22 2:03 ` Andrew Cooper 2024-02-22 2:03 ` Andrew Cooper 2024-02-22 2:10 ` ross.philipson 2024-02-22 2:10 ` ross.philipson 2024-02-22 17:49 ` ross.philipson 2024-02-22 17:49 ` ross.philipson 2024-03-29 22:38 ` Kim Phillips 2024-03-29 22:38 ` Kim Phillips 2024-03-29 22:38 ` Kim Phillips 2024-03-29 22:38 ` Kim Phillips 2024-03-29 22:38 ` Kim Phillips 2024-03-29 22:38 ` Kim Phillips 2024-04-01 18:25 ` ross.philipson 2024-04-01 18:25 ` ross.philipson 2024-02-14 22:18 ` [PATCH v8 05/15] x86: Secure Launch main " Ross Philipson 2024-02-14 22:18 ` Ross Philipson 2024-02-14 22:18 ` [PATCH v8 06/15] x86: Add early SHA support for Secure Launch early measurements Ross Philipson 2024-02-14 22:18 ` Ross Philipson 2024-02-15 8:17 ` Ard Biesheuvel 2024-02-15 8:17 ` Ard Biesheuvel 2024-02-22 3:04 ` Andrew Cooper 2024-02-22 3:04 ` Andrew Cooper 2024-02-22 9:34 ` Ard Biesheuvel 2024-02-22 9:34 ` Ard Biesheuvel 2024-02-22 12:30 ` Andrew Cooper 2024-02-22 12:30 ` Andrew Cooper 2024-02-23 9:27 ` Ard Biesheuvel 2024-02-23 9:27 ` Ard Biesheuvel 2024-02-23 16:42 ` Andrew Cooper 2024-02-23 16:42 ` Andrew Cooper 2024-02-23 17:54 ` Eric Biggers 2024-02-23 17:54 ` Eric Biggers 2024-02-23 18:20 ` Andrew Cooper 2024-02-23 18:20 ` Andrew Cooper 2024-02-23 18:30 ` Eric Biggers 2024-02-23 18:30 ` Eric Biggers 2024-04-03 16:32 ` Andy Lutomirski 2024-04-03 16:32 ` Andy Lutomirski 2024-04-03 23:56 ` Eric Biggers 2024-04-03 23:56 ` Eric Biggers 2024-04-04 4:55 ` ross.philipson [this message] 2024-04-04 4:55 ` ross.philipson 2024-04-04 14:55 ` Jarkko Sakkinen 2024-04-04 14:55 ` Jarkko Sakkinen 2024-02-14 22:18 ` [PATCH v8 07/15] x86: Secure Launch kernel early boot stub Ross Philipson 2024-02-14 22:18 ` Ross Philipson 2024-02-15 8:29 ` Ard Biesheuvel 2024-02-15 8:29 ` Ard Biesheuvel 2024-02-15 22:26 ` ross.philipson 2024-02-15 22:26 ` ross.philipson 2024-02-14 22:18 ` [PATCH v8 08/15] x86: Secure Launch kernel late " Ross Philipson 2024-02-14 22:18 ` Ross Philipson 2024-02-14 22:18 ` [PATCH v8 09/15] x86: Secure Launch SMP bringup support Ross Philipson 2024-02-14 22:18 ` Ross Philipson 2024-02-14 22:18 ` [PATCH v8 10/15] kexec: Secure Launch kexec SEXIT support Ross Philipson 2024-02-14 22:18 ` Ross Philipson 2024-02-14 22:18 ` [PATCH v8 11/15] reboot: Secure Launch SEXIT support on reboot paths Ross Philipson 2024-02-14 22:18 ` Ross Philipson 2024-02-14 22:18 ` [PATCH v8 12/15] tpm: Add ability to set the preferred locality the TPM chip uses Ross Philipson 2024-02-14 22:18 ` Ross Philipson 2024-02-14 22:18 ` [PATCH v8 13/15] tpm: Add sysfs interface to allow setting and querying the preferred locality Ross Philipson 2024-02-14 22:18 ` Ross Philipson 2024-02-14 22:18 ` [PATCH v8 14/15] x86: Secure Launch late initcall platform module Ross Philipson 2024-02-14 22:18 ` Ross Philipson 2024-02-15 8:40 ` Ard Biesheuvel 2024-02-15 8:40 ` Ard Biesheuvel 2024-02-22 13:57 ` Daniel P. Smith 2024-02-22 13:57 ` Daniel P. Smith 2024-02-23 9:36 ` Ard Biesheuvel 2024-02-23 9:36 ` Ard Biesheuvel 2024-03-21 14:11 ` Daniel P. Smith 2024-03-21 14:11 ` Daniel P. Smith 2024-02-16 1:53 ` kernel test robot 2024-02-16 1:53 ` kernel test robot 2024-02-17 7:53 ` kernel test robot 2024-02-17 7:53 ` kernel test robot 2024-02-14 22:18 ` [PATCH v8 15/15] x86: EFI stub DRTM launch support for Secure Launch Ross Philipson 2024-02-14 22:18 ` Ross Philipson 2024-02-15 9:01 ` Ard Biesheuvel 2024-02-15 9:01 ` Ard Biesheuvel 2024-02-21 20:17 ` ross.philipson 2024-02-21 20:17 ` ross.philipson 2024-02-21 20:37 ` H. Peter Anvin 2024-02-21 20:37 ` H. Peter Anvin 2024-02-21 23:24 ` Ard Biesheuvel 2024-02-21 23:24 ` Ard Biesheuvel 2024-02-17 7:31 ` kernel test robot 2024-02-17 7:31 ` kernel test robot 2024-02-17 20:06 ` kernel test robot 2024-02-17 20:06 ` kernel test robot
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=8b6285e8-bb00-4541-b054-ee01c86d367a@oracle.com \ --to=ross.philipson@oracle.com \ --cc=James.Bottomley@hansenpartnership.com \ --cc=andrew.cooper3@citrix.com \ --cc=ardb@kernel.org \ --cc=bp@alien8.de \ --cc=dave.hansen@linux.intel.com \ --cc=davem@davemloft.net \ --cc=dpsmith@apertussolutions.com \ --cc=ebiggers@kernel.org \ --cc=herbert@gondor.apana.org.au \ --cc=hpa@zytor.com \ --cc=jarkko@kernel.org \ --cc=jgg@ziepe.ca \ --cc=kanth.ghatraju@oracle.com \ --cc=kexec@lists.infradead.org \ --cc=linux-crypto@vger.kernel.org \ --cc=linux-doc@vger.kernel.org \ --cc=linux-efi@vger.kernel.org \ --cc=linux-integrity@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=luto@amacapital.net \ --cc=luto@kernel.org \ --cc=mingo@redhat.com \ --cc=mjg59@srcf.ucam.org \ --cc=nivedita@alum.mit.edu \ --cc=peterhuewe@gmx.de \ --cc=tglx@linutronix.de \ --cc=trenchboot-devel@googlegroups.com \ --cc=x86@kernel.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.