Received-SPF: pass (google.com: domain of linux-crypto+bounces-3311-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <8b6285e8-bb00-4541-b054-ee01c86d367a@oracle.com>
Date: Wed, 3 Apr 2024 21:55:41 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v8 06/15] x86: Add early SHA support for Secure Launch
 early measurements
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
References: <CAMj1kXEmMBY_jc0uM5UgZbuZ3-C7NPKzg5AScaunyu9XzLgzZA@mail.gmail.com>
 <98ad92bb-ef17-4c15-88ba-252db2a2e738@citrix.com>
 <CAMj1kXFTu+bV2kQhAyu15hrYai20NcBLb4Zu8XG2Y-XjL0f+rw@mail.gmail.com>
 <1a8e69a7-89eb-4d36-94d6-0da662d8b72f@citrix.com>
 <CAMj1kXEvmGy9RJo4s8tECsFj2dufZ8jBPoJOEtkcGUoj+x2qsw@mail.gmail.com>
 <431a0b3a-47e5-4e61-a7fc-31cdf56f4e4c@citrix.com>
 <20240223175449.GA1112@sol.localdomain>
 <e641e2f1-16cf-4717-8a1f-8afac2644efe@citrix.com>
 <20240223183004.GE1112@sol.localdomain>
 <10db421c-77da-4a1c-a25e-2374a7a2ef79@app.fastmail.com>
 <20240403235635.GA24248@quark.localdomain>
Content-Language: en-US
From: ross.philipson@oracle.com
In-Reply-To: <20240403235635.GA24248@quark.localdomain>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?bk9yK0tFRUpRNkhWaVVRcjZZZHpqU2lPa250S3Y2Rk9tQ3gybGVZVHQwUHJ0?=
 =?utf-8?B?LzJHT3dPMWsxeHN4bGE5cHJXZ3Z3MkpEUml1cVlYbjA2czFLM2dDK1hHWEl5?=
 =?utf-8?B?NUFsM3NHbm9RTHVYdzBneUlZOFV1SVFHM1hiWmNoRE5PdW5DK2lPNnhJdisv?=
 =?utf-8?B?c05OeU8yRUZmdkQ1NjRya3gvQVVuQi93bmlucWJhdENReVQyMmV3Y3YwZDN6?=
 =?utf-8?B?UmFIUFpDaXVPc3Z4QlppWVJydFdqV3hRL2ZoTlVNRVJhN3lEcGdxMTRRWTlm?=
 =?utf-8?B?VzZpWExMa2hyY0twMXN5NC9TR3htQjdzM2t2b3lMdDhJVzErTWNXd1lBTmtX?=
 =?utf-8?B?TE1QQnFxdXNybC81NU83SHpmVC9qY3plSzJaSEtITGNJeHJ6NUlNWXlTQXRa?=
 =?utf-8?B?NCtTNWgyK0lPQzNycnBLZEk4Q0hoYUJoRk5Lc0FMdU43Vm40cFM0UmtDVUtV?=
 =?utf-8?B?U0RCVGxpTDFOWnhLSktoVWxVSHhIbUZLZUppRlNxRUdUeE10RnlBbmdCbUVt?=
 =?utf-8?B?VnMvaWdOQnROY1VXZURKalFDR29MN2lCSmZldTNqSnM5TjJML2MyZGFFMkxV?=
 =?utf-8?B?NGcySVorVitnMXp1R0xnNlZTK1ZKSXBVaElYSzhqMVJ1VlR3Y1NFcUlGUjBa?=
 =?utf-8?B?ZFpuNThneUQ3K3hLOGF4QVF1aCt3KzhGbEN0N3U2UUo5dkNoejF0ZnRDeGZp?=
 =?utf-8?B?YnJTdGZEdGlka0hFWFNhaXZIVFd3aGhWOHY0cmRNK2ZrK2pwaC9NZWR0djVa?=
 =?utf-8?B?eU04Z2J2UFFCdkJlSW81VTliNGNlWC9ZMUJYU01PelhNTHhvZVFUdFVHYmQ0?=
 =?utf-8?B?clhtYkpOVzVCNldmTmNaazlKVG1qZ0g5TmVXVk1RN1dSVHVKZi9SVHJEQk5K?=
 =?utf-8?B?ZTFjM3lMbEZwdzhkaGpPUjgwVitRcGNvNUdOODE4b2xNNWRVNXk4eGJ1bXlK?=
 =?utf-8?B?b00vTlZNTU1od1dPZEIwWDBzb0VuRlVZcmQySkdiT1AwSTRPNzdBUkpwMkw4?=
 =?utf-8?B?dXNpeklzMDMxOTdUN2FNS2RabnVvMWJLK1JVMHAwclREcW8zbmZvMTl1Y0xD?=
 =?utf-8?B?TEZFbDduK2pEVDUrTXhRZ29nNk9HTkszZnVSdHRmUm94WEpNL0I1Y2I5elBs?=
 =?utf-8?B?dUNIWmt6dURRR09ZQTlYNGxDemFXMmNJM0toL290cmp0M0U3TXNidWd0dFBq?=
 =?utf-8?B?TkhqTml3VlVtWExCMzdBeTZpZ0NRZG45cEFWV2w0azlVb0pLeVg1Rm1QUTUx?=
 =?utf-8?B?UmlldEVQYXF2VG5ndFNDSU1ZQTlhK2J5NlpvdW9qUDQxUTBpaERocE9FLy8r?=
 =?utf-8?B?RnNyQXl6WmF5YmxHaDZxQnRsbDNrKzh5OHFhMEFSdHJSbXJqL0lvVDhFTExO?=
 =?utf-8?B?VEhJclZGY1NTNnYyT0RxL0xmS0NvZnZGWisyUlhTK1NqTWpoVHU3QUgrRG9F?=
 =?utf-8?B?ODdFL2x4b3dTaHdFclJoZG9zeEExaU9ZMytWZ3BxY1hXZ0x6V2VqU215YzdG?=
 =?utf-8?B?M2FuTFJObmdJTDVrMW9XRFVVaURGd051OXZ5WkJpajg1a09DaXlQUkF2bWYw?=
 =?utf-8?B?WWY2cHlOVnNVKzdocmV2TS9Ta2pOY21BVWJQcVM1V1ZxU1NtOTRRb2hwVlB5?=
 =?utf-8?B?d2hBSDdWTEZkMGFiY0o3d0F5M3BFbzY5OGxZazBJRjBkM0YyeG5HVFJXQmQv?=
 =?utf-8?B?MDI2L0JZdXhOMXpFdS9KdHZ2c1d4MGdZMktTMFRRSWQybXVxY0ZCT3dFcWtC?=
 =?utf-8?B?bUVQTmR0d1ZvSkZvMGRoMyt1bUphTWxPV0tUUlB2akI0UG1XUHNSNWJ5V0J4?=
 =?utf-8?B?eXVmOVBDSHlLTkxrT3pCWUQ0NFUra21neHc4ZTNTUUorcXlxcFgxNU9aMVJE?=
 =?utf-8?B?RE15NktVU2w2YitJdmJLZ1JjU2ZhUURpZU9zNTEwNVREY0J2Y2ViZEhOVDI3?=
 =?utf-8?B?bTI2eU12Rk5Ca2t2Y0tjMlVpSnpST2Y2dk43Mk5ZdTNKQVpiZkxEQjlwejA1?=
 =?utf-8?B?WUl0VjlFUXBsazRka0pza2V0Mitmd1dYRTlUL3pEek91cEx5dm9OOWt2elRH?=
 =?utf-8?B?aUl1M0E0NEozc1BWNEhTYVVGcUhoeDNwWUUxOERCTjRmdlc2cDdycGZ5bVlo?=
 =?utf-8?B?Yy9aL0MwSmNWdzkyVFdGdThHSFJiVm5tTitPTlVmd2RDVWtkUFhIS3pSMFM1?=
 =?utf-8?B?cXc9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	ZhgedLelWmlsrj8M6t/eG4130W84/DkEcMrGgMlMprFPv/4AYoypFZyCAH7YDU1p3oeDgdJ73wKx9yj61koMxUFZZSWnD+uEA8Wk/Qe+w274zAkSmA6AbZ/tp9m1YDPNxYlWURMbMpoB+XM22LjATD5MeMyRPpeqL91KuTNNtJhmGhjRAemC5OkNxxU09/3TNE2HnFo4xxjedqcwjSZA0gBOnz+Kb0cUi1cKuIfx77ND079eIkngBmeQRakF3C7Zv7xsUsipQLHHfUlvUKvqUVivP64Fl+7zet6Wrx4Kq8lznkCGk3OnhG+V+cE3Lrv9pjKYpO3Fp7fR4cThNYhxVOqqiDWhcoRyTYlWrjxVl4ivOD/Jm0Oahsa08e0qPmaYZEAvfi+BTo83EAtcpi7dQ8J5/qCJAzPb9HRpM2m+x9giSFkj5UK2bnQgU/HU/PY4Mvn+M/mLfpX/o5tZhbE2a6seQpRJGfPjX8XCfyvJHjSutdmvXLKX4fcbYZCGHncMKMMNZNT2D1MQ+i8oIQ7RtOUou2UiO59sSbFCd2kiayF/DS9hfpNOku7ZppozXQYxfy/KCgwuZvWKnTjDzQvV22QcoAkHZvVYmq2NLwjMnrw=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f9c5bf59-fbac-480f-045b-08dc54637d52
X-MS-Exchange-CrossTenant-AuthSource: DS0PR10MB7224.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Apr 2024 04:55:46.2275
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 2jfer0S2nB0I4gX+3iQbp4MxH7k7s5kZEqTjbfmx4yDGa+D7BMT73zzGz9zGZU3i5/CO44wbdHP4LQXze6vFZo4BZd9Wh1mleku4tgMpFxo=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR10MB7683
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-04-03_26,2024-04-03_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 malwarescore=0
 suspectscore=0 adultscore=0 mlxscore=0 spamscore=0 bulkscore=0
 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2404010000 definitions=main-2404040031
X-Proofpoint-ORIG-GUID: HWal3hBvE-5RgHD68ArZoydWO36t-kh7
X-Proofpoint-GUID: HWal3hBvE-5RgHD68ArZoydWO36t-kh7

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