From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=150.165.85.253; helo=mta-zimbra.lsd.ufcg.edu.br; envelope-from=ricardo@lsd.ufcg.edu.br; receiver=edk2-devel@lists.01.org Received: from mta-zimbra.lsd.ufcg.edu.br (mta-zimbra.lsd.ufcg.edu.br [150.165.85.253]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id D93A6210C6458 for ; Fri, 3 Aug 2018 06:39:33 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by mta-zimbra.lsd.ufcg.edu.br (Postfix) with ESMTP id 2BDBF3A2DAB; Fri, 3 Aug 2018 10:39:31 -0300 (-03) Received: from mta-zimbra.lsd.ufcg.edu.br ([127.0.0.1]) by localhost (mta-zimbra.lsd.ufcg.edu.br [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id zFp5ms5C7a2S; Fri, 3 Aug 2018 10:39:29 -0300 (-03) Received: from localhost (localhost [127.0.0.1]) by mta-zimbra.lsd.ufcg.edu.br (Postfix) with ESMTP id 1F6093A2DDD; Fri, 3 Aug 2018 10:39:29 -0300 (-03) X-Virus-Scanned: amavisd-new at mta-zimbra.lsd.ufcg.edu.br Received: from mta-zimbra.lsd.ufcg.edu.br ([127.0.0.1]) by localhost (mta-zimbra.lsd.ufcg.edu.br [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id 7_cVLj11081R; Fri, 3 Aug 2018 10:39:28 -0300 (-03) Received: from mta-zimbra.lsd.ufcg.edu.br (mta-zimbra.lsd.ufcg.edu.br [150.165.85.253]) by mta-zimbra.lsd.ufcg.edu.br (Postfix) with ESMTP id AC15B3A2DAB; Fri, 3 Aug 2018 10:39:28 -0300 (-03) Date: Fri, 3 Aug 2018 10:39:28 -0300 (BRT) From: Ricardo =?utf-8?Q?Ara=C3=BAjo?= Reply-To: Ricardo =?utf-8?Q?Ara=C3=BAjo?= To: "Zhang, Chao B" Cc: Laszlo Ersek , =?utf-8?Q?Marc-Andr=C3=A9?= Lureau , edk2-devel@lists.01.org, "Gao, Liming" , "Zeng, Star" Message-ID: <1029983813.116236.1533303568344.JavaMail.zimbra@lsd.ufcg.edu.br> In-Reply-To: References: <551258016.93465.1533144825411.JavaMail.zimbra@lsd.ufcg.edu.br> <1169526465.93534.1533145845454.JavaMail.zimbra@lsd.ufcg.edu.br> <00859174-589e-92d8-5f97-dcf9d735424c@redhat.com> <23c3c4c3-461f-7e6a-a8ca-0574021c1ccf@redhat.com> MIME-Version: 1.0 X-Originating-IP: [10.30.0.39] X-Mailer: Zimbra 8.6.0_GA_1153 (ZimbraWebClient - GC68 (Linux)/8.6.0_GA_1153) Thread-Topic: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 with OVMF Thread-Index: AdQqBSqZ9dVG8it6RN6+fCOtT6CWegAGoPcAACgIXaBDl/8tww== X-Content-Filtered-By: Mailman/MimeDel 2.1.27 Subject: Re: Missing boot related measurements at TPM 2.0 PCRs 0-7 with OVMF X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 03 Aug 2018 13:39:35 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi folks, sorry for the delay!=20 I've just applied the patch and got the same error message and empty PCRs.= =20 Regards,=20 Ricardo Ara=C3=BAjo Santos -=20 www.lsd.ufcg.edu.br/~ricardo=20 De: "Zhang, Chao B" =20 Para: "Laszlo Ersek" , "Ricardo Ara=C3=BAjo" , "Marc-Andr=C3=A9 Lureau" =20 Cc: edk2-devel@lists.01.org, "Gao, Liming" , "Zeng, S= tar" =20 Enviadas: Quinta-feira, 2 de agosto de 2018 21:22:18=20 Assunto: RE: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 w= ith OVMF=20 Tks Lazslo. And please make sure PcdLib is correctly lined in OVMF=20 From: Laszlo Ersek [mailto:lersek@redhat.com]=20 Sent: Thursday, August 2, 2018 9:14 PM=20 To: Zhang, Chao B ; Ricardo Ara=C3=BAjo ; Marc-Andr=C3=A9 Lureau =20 Cc: edk2-devel@lists.01.org; Gao, Liming ; Zeng, Star= =20 Subject: Re: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 w= ith OVMF=20 On 08/02/18 04:04, Zhang, Chao B wrote:=20 > Hi Laszlo & Ricardo=20 > The patch was intended to reduce the time to read TPM interface ID regist= er. The interface type should not change within boot cycle according to PTP= spec.=20 > I agree to add some ASSERT after PCDSetxxsS.=20 > But It is a core solution without platform change as PCD has been configu= red as DYN, DYNEx in DEC. I don=E2=80=99t know why you meet Set Failure=20 > In OVMF. Here, I include PCD expert to explain this.=20 As far as I recall, dynamic PCDs have never behaved as actually settable=20 for me unless I added dynamic defaults for them in the OVMF DSC files.=20 I never really researched why this was the case, I just accepted that=20 the dynamic defaults were apparently necessary.=20 Let's wait for Ricardo's response. Perhaps my analysis / suspicion were=20 incorrect and it's not actually the "dynamism" of the PCD that's missing=20 for OVMF. Ricardo's answer will tell us if there's another issue.=20 Thanks=20 Laszlo=20 > From: Laszlo Ersek [ mailto:lersek@redhat.com ]=20 > Sent: Thursday, August 2, 2018 5:49 AM=20 > To: Ricardo Ara=C3=BAjo < ricardo@lsd.ufcg.edu.br >; Zhang, Chao B < chao= .b.zhang@intel.com >; Marc-Andr=C3=A9 Lureau < marcandre.lureau@redhat.com = >=20 > Cc: edk2-devel@lists.01.org=20 > Subject: Re: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7= with OVMF=20 >=20 > On 08/01/18 19:50, Ricardo Ara=C3=BAjo wrote:=20 >> The commit I was referring to is:=20 >> https://github.com/tianocore/edk2/commit/f15cb995bb3880b77e15afe6facd3da= 05e599a17=20 >>=20 >> Regards,=20 >>=20 >> Ricardo Araujo -=20 >> www.lsd.ufcg.edu.br/~ricardo=20 >>=20 >> ----- Mensagem original -----=20 >> De: "Ricardo Ara=C3=BAjo" < ricardo@lsd.ufcg.edu.br>=20 >> Para: edk2-devel@lists.01.org=20 >> Enviadas: Quarta-feira, 1 de agosto de 2018 14:33:45=20 >> Assunto: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7=20 >> with OVMF=20 >>=20 >> Hi everyone,=20 >>=20 >> I'm using OVMF with a simulated TPM 2.0 (from=20 >> https://github.com/stefanberger/swtpm ) and I noticed lately that PCRs= =20 >> 0-7 are zeroed after booting the vm (ubuntu 18.04) and the only=20 >> message related to this in dmesg is:=20 >>=20 >> [ 2.286690] tpm_tis 00:06: 2.0 TPM (device-id 0x1, rev-id 1)=20 >> [ 2.303753] tpm tpm0: A TPM error (256) occurred continue selftest=20 >> [ 2.314199] tpm tpm0: starting up the TPM manually=20 >>=20 >> I found this started to happen after this commit , previous commits to= =20 >> that are showing boot time measurements on PCR 0-7 normally and the=20 >> error message is gone. Has anyone experienced the same behavior? I=20 >> followed the instructions here for building OVMF but I added the=20 >> parameters -D TPM2_ENABLE=3DTRUE -D SECURE_BOOT_ENABLE=3DTRUE -D=20 >> HTTP_BOOT_ENABLE=3DTRUE. Is there anything else I need to add to enable= =20 >> these measurements?=20 >>=20 >> Regards,=20 >>=20 >> Ricardo Araujo=20 >> www.lsd.ufcg.edu.br/~ricardo=20 >=20 > Thank you for the bug report. It looks like a regression to me, but the= =20 > details aren't immediately clear.=20 >=20 > Adding Marc-Andr=C3=A9 who contributed the TPM enablement for OVMF, and C= hao=20 > Zhang who authored the commit in question.=20 >=20 > If I recall correctly, in OVMF we decided to never cache the TPM type=20 > but always detect it. I could be remembering wrong though. See commit=20 > 6cf1880fb5b6 ("OvmfPkg: add customized Tcg2ConfigPei clone",=20 > 2018-03-09).=20 >=20 > Chao Zhang: can you please explain what additional requirements are=20 > presented for a platform by commit f15cb995bb38? In OVMF we use a=20 > customized Tcg2ConfigPei module (see the commit above).=20 >=20 >=20 > Oh wait, I suspect what's wrong. I believe there are two bugs in commit= =20 > f15cb995bb38 ("SecurityPkg: Cache TPM interface type info", 2018-06-25):= =20 >=20 > * Bug#1:=20 >=20 > Commit f15cb995bb38 introduces a new PCD, called=20 > "PcdActiveTpmInterfaceType", in section [PcdsDynamic, PcdsDynamicEx] of= =20 > "SecurityPkg.dec", and makes core modules from SecurityPkg dependent on= =20 > it.=20 >=20 > Obviously this means that platforms are required to provide a Dynamic=20 > Default for the new PCD in their DSC files, if they include those core=20 > modules from SecurityPkg, otherwise the PCD won't actually behave=20 > dynamically -- "set" operations will fail, and "get" operations will=20 > just return the central default from the SecurityPkg.dec file. As a=20 > result, the cached TPM type will always be wrong (it will look like=20 > "undetected", 0xFF).=20 >=20 > This could have been avoided by grepping all "*dsc*" files in the edk2=20 > tree for references to the SecurityPkg module INF files that were about= =20 > to receive a dependency on the PCD. Such as:=20 >=20 > git grep -l -F \=20 > -e SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf \=20 > --or -e SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf \= =20 > --or -e SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf \=20 > --or -e SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf \=20 > '*dsc*'=20 >=20 > This would have listed all platforms in-tree that were going to depend=20 > on the new dynamic PCD via inclusion of the affected SecurityPkg=20 > modules.=20 >=20 > Running this command now, I get the following output:=20 >=20 > OvmfPkg/OvmfPkgIa32.dsc=20 > OvmfPkg/OvmfPkgIa32X64.dsc=20 > OvmfPkg/OvmfPkgX64.dsc=20 > SecurityPkg/SecurityPkg.dsc=20 >=20 > Open source hygiene dictates that modifications to infrastructure code=20 > or otherwise central code be accompanied by necessary updates to *ALL*=20 > in-tree subsystems that depend on said core code. (Out-of-tree=20 > subsystems are a different matter.) It's OK if a single contributor=20 > cannot test every single platform -- but we can still use the mailing=20 > list and the bug tracker for raising the issue and expose the new=20 > dependency for platforms that we can't test, but see as affected.=20 >=20 > Ricardo, Marc-Andr=C3=A9: does the following patch work for you guys=20 > (build-tested only):=20 >=20 >> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc=20 >> index a28b511d5c2f..b0153f66b710 100644=20 >> --- a/OvmfPkg/OvmfPkgIa32.dsc=20 >> +++ b/OvmfPkg/OvmfPkgIa32.dsc=20 >> @@ -579,6 +579,7 @@ [PcdsDynamicDefault]=20 >>=20 >> !if $(TPM2_ENABLE) =3D=3D TRUE=20 >> gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00= , 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}= =20 >> + gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType|0xFF=20 >> !endif=20 >>=20 >> ########################################################################= ########=20 >> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc=20 >> index 115d0c01ff5c..fcce846ab9a5 100644=20 >> --- a/OvmfPkg/OvmfPkgIa32X64.dsc=20 >> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc=20 >> @@ -587,6 +587,7 @@ [PcdsDynamicDefault]=20 >>=20 >> !if $(TPM2_ENABLE) =3D=3D TRUE=20 >> gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00= , 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}= =20 >> + gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType|0xFF=20 >> !endif=20 >>=20 >> ########################################################################= ########=20 >> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc=20 >> index 362eb789c712..3eda1b3013f7 100644=20 >> --- a/OvmfPkg/OvmfPkgX64.dsc=20 >> +++ b/OvmfPkg/OvmfPkgX64.dsc=20 >> @@ -586,6 +586,7 @@ [PcdsDynamicDefault]=20 >>=20 >> !if $(TPM2_ENABLE) =3D=3D TRUE=20 >> gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00= , 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}= =20 >> + gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType|0xFF=20 >> !endif=20 >>=20 >> ########################################################################= ########=20 >=20 > If it works, I'll submit it later as a standalone patch.=20 >=20 >=20 > * Bug#2:=20 >=20 > The PcdSet8S() calls added by commit f15cb995bb38 are not error-checked;= =20 > their return values are ignored.=20 >=20 > Honestly, if we ignore the return values of PcdSetXxxS() calls, then it= =20 > has been a wasted effort to introduce those "safe" APIs in the first=20 > place, in commit 9a3558419509. At the bare minimum, an=20 > ASSERT_RETURN_ERROR() should be added after every invocation.=20 >=20 > I've filed the following TianoCore BZ about Bug#2 now:=20 >=20 > https://bugzilla.tianocore.org/show_bug.cgi?id=3D1070=20 >=20 > Thanks=20 > Laszlo=20 >=20