Re: [EXTERNAL] Re: [edk2-devel] [PATCH] SecurityPkg/Tcg2Config: hide PCR Bank SHA1 checkbox

["Yao, Jiewen" <jiewen.yao@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 7446 bytes --]

Hi
I gave feedback in bugzilla.

I think the statement is confusing - "SHA1 algorithm can be disabled by build option DISABLE_SHA1_DEPRECATED_INTERFACES defined.".
Do you mean disable TPM hardware bank, or just software SHA1 algorithm ?

We can have three ways to handle SHA1.
1) Disable SHA1 bank in TPM hardware. - Then no one can see the SHA1 bank.
2) Disable SHA1 PCR via capping PCR in the TCG driver. - Then SHA1 bank can be seen. But it will be useless.
3) Ignore SHA1 PCR bank. - Then it can be seen. The value is empty. The downside is that anyone can use it, even forge it.

To me, only 1) and 2) are secure way to "disable".
But the patch seems want to use the 3). Removing it in the UI checkbox really does not do anything to "disable".
Do I misunderstand something?



Besides that, I am also not clear, that if we want to hide the SHA1.
Why not remove “Sha1Supported” field completely with DISABLE_SHA1_DEPRECATED_INTERFACES ?
Why we still need it in the data structure?

Thank you
Yao Jiewen


From: Zhang, Qi1 <qi1.zhang@intel.com>
Sent: Thursday, March 18, 2021 12:47 PM
To: Bret Barkelew <Bret.Barkelew@microsoft.com>; devel@edk2.groups.io; lersek@redhat.com; Yao, Jiewen <jiewen.yao@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
Subject: RE: [EXTERNAL] Re: [edk2-devel] [PATCH] SecurityPkg/Tcg2Config: hide PCR Bank SHA1 checkbox

Submit a new BZ https://bugzilla.tianocore.org/show_bug.cgi?id=3268 for this change.

Patch set V2 has been sent out for review.

Thanks!
Qi Zhang
From: Bret Barkelew <Bret.Barkelew@microsoft.com<mailto:Bret.Barkelew@microsoft.com>>
Sent: Thursday, March 18, 2021 4:45 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; lersek@redhat.com<mailto:lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Zhang, Qi1 <qi1.zhang@intel.com<mailto:qi1.zhang@intel.com>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Kumar, Rahul1 <rahul1.kumar@intel.com<mailto:rahul1.kumar@intel.com>>
Subject: RE: [EXTERNAL] Re: [edk2-devel] [PATCH] SecurityPkg/Tcg2Config: hide PCR Bank SHA1 checkbox

+1

- Bret

________________________________
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> on behalf of Laszlo Ersek via groups.io <lersek=redhat.com@groups.io<mailto:lersek=redhat.com@groups.io>>
Sent: Wednesday, March 17, 2021 11:28:07 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Zhang, Qi1 <qi1.zhang@intel.com<mailto:qi1.zhang@intel.com>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Kumar, Rahul1 <rahul1.kumar@intel.com<mailto:rahul1.kumar@intel.com>>
Subject: [EXTERNAL] Re: [edk2-devel] [PATCH] SecurityPkg/Tcg2Config: hide PCR Bank SHA1 checkbox

On 03/17/21 05:19, Yao, Jiewen wrote:
> Thank you Qi.
>
> i recommend we file a bugzilla on the scope of the problem

I agree.

We already have a number of BZs related to the disablement of SHA1 and MD5:

https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D1682&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Ca9f46449e57d4642ab4608d8e9727545%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637516025103613890%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Bxq4fGkZUA91NNncp%2F68zbcXSYBQcIfGbgcT4CYktKs%3D&amp;reserved=0
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2943&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Ca9f46449e57d4642ab4608d8e9727545%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637516025103613890%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=eh9ef1WSF1DuepEkiXg1x%2BmmWs77eNrkSObOS5SVB94%3D&amp;reserved=0
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3003&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Ca9f46449e57d4642ab4608d8e9727545%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637516025103613890%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=YJTXNb2GnPkkc9Y6GI%2BI9WYy8MdeujtSvpah5mWhrp0%3D&amp;reserved=0
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3021&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Ca9f46449e57d4642ab4608d8e9727545%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637516025103613890%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=pcAqk46YRGn9jS0ra5sl7Gg7i8PvqZbdewsH6hMq5YM%3D&amp;reserved=0
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3027&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Ca9f46449e57d4642ab4608d8e9727545%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637516025103613890%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=4sOMDQitxKwFfXmYFJ9ooF7hovK4vfMww0ppsHz5vjk%3D&amp;reserved=0
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3079&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Ca9f46449e57d4642ab4608d8e9727545%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637516025103613890%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=8is%2FUsgRy71E%2F9ui%2BkMwoTmp4jiw0vKlsUwJ%2BxEqfO0%3D&amp;reserved=0

We should certainly track the change for Tcg2Config too, in a new BZ.

Thanks
Laszlo

>
> After the scope is agreed, then you can send the patch.
>
> For example, I can ask why not remove the sha1supported field at all?
>
> I hope the community can reach consensus on the problem statement at first.
>
>
> thank you!
> Yao, Jiewen
>
>
>> 在 2021年3月17日，上午10:56，Zhang, Qi1 <qi1.zhang@intel.com<mailto:qi1.zhang@intel.com>> 写道：
>>
>> wrap SHA1 related by DISABLE_SHA1_DEPRECATED_INTERFACES.
>>
>> Cc: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
>> Cc: Jian J Wang <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>
>> Cc: Qi Zhang <qi1.zhang@intel.com<mailto:qi1.zhang@intel.com>>
>> Cc: Rahul Kumar <rahul1.kumar@intel.com<mailto:rahul1.kumar@intel.com>>
>> Signed-off-by: Qi Zhang <qi1.zhang@intel.com<mailto:qi1.zhang@intel.com>>
>> ---
>> SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
>> index 2946f95db0..81a4d3fa6a 100644
>> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
>> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
>> @@ -710,9 +710,11 @@ SetConfigInfo (
>>   )
>> {
>>   switch (TpmAlgHash) {
>> +#ifndef DISABLE_SHA1_DEPRECATED_INTERFACES
>>   case TPM_ALG_SHA1:
>>     Tcg2ConfigInfo->Sha1Supported = TRUE;
>>     break;
>> +#endif
>>   case TPM_ALG_SHA256:
>>     Tcg2ConfigInfo->Sha256Supported = TRUE;
>>     break;
>> --
>> 2.26.2.windows.1
>>
>
>
>
>
>





[-- Attachment #2: Type: text/html, Size: 16676 bytes --]