Extend measured boot tests

[m-iwanicki]

No description provided.

[m-iwanicki]

@SergiiDmytruk
Test logs on Protectli vp2420: log.tar.gz
Tested on commit 11a910f

==============================================================================
Measured-Boot
==============================================================================

Checking if tpm2-tools is installed...

Package tpm2-tools is installed
MBO001.001 Measured Boot support (Ubuntu 20.04) :: Check whether M... | PASS |
------------------------------------------------------------------------------
MBO002.001 Check if event log PCRs match actual values (Ubuntu 22.... | PASS |
------------------------------------------------------------------------------
MBO003.001 Changing Secure Boot certificate changes only PCR-7 :: ... | PASS |
------------------------------------------------------------------------------
MBO004.001 Changing Dasharo network boot settings changes only PCR... | SKIP |
Tests in Dasharo Networking Menu are not supported
------------------------------------------------------------------------------
MBO004.002 Changing Dasharo USB settings changes only PCR-1 :: Che... | FAIL |
D5F66150C90F85FE8E6D254847D5BB5E464885DAB2F3F495950B7A875F5C3DAD != C0E4FFF832B03B2BC3C7FE604046C2B0806BB21EF58FABAA593268A35B4F5685
------------------------------------------------------------------------------
MBO004.003 Changing Dasharo APU settings changes only PCR-1 :: Che... | SKIP |
Tests in Dasharo APU Menu are not supported
------------------------------------------------------------------------------
MBO005.001 Flashing firmware and reset to defaults results in same... | PASS |
------------------------------------------------------------------------------
MBO005.002 Multiple reset to defaults results in identical measure... | PASS |
------------------------------------------------------------------------------
MBO005.003 Identical configuration results in identical measuremen... | FAIL |
C541A3033EC12521178523052186C490463178BF3022FDF5CC6D5A2E55CD6992 != 6B9A48A2045AA66C33114F17EDCF7DEF12E2E184898DBC9FDA8ADEDC8B2CCE3A
------------------------------------------------------------------------------
MBO005.004 Identical configuration after reset results in identica... | FAIL |
C541A3033EC12521178523052186C490463178BF3022FDF5CC6D5A2E55CD6992 != 6B9A48A2045AA66C33114F17EDCF7DEF12E2E184898DBC9FDA8ADEDC8B2CCE3A
------------------------------------------------------------------------------
Measured-Boot                                                         | FAIL |
10 tests, 5 passed, 3 failed, 2 skipped
==============================================================================

Couple fails in log.html you can see that:

• MBO004.002 - had different PCR-8
• MBO005.003 & .004 - had different PCR-1

MBO004.002 might have something to do with the fact that test disables USB Mass Storage and platform had connected bootable USB? If yes then other test that enables/disables network boot might have similar problems?

[SergiiDmytruk]

MBO004.002 might have something to do with the fact that test disables USB Mass Storage and platform had connected bootable USB?

PCR-8 shouldn't be touched by firmware. But GRUB could be extending a module related to USB (docs) or maybe it enumerates available drives. I don't see an obvious cause in the log. If there is no more neutral option to use, could exclude PCR-8 or check only PCR-0..7 after comparing event logs to confirm the cause of the failure.

If yes then other test that enables/disables network boot might have similar problems?

Not necessarily, this might not apply to network boot. Depends on what's the actual cause.

[m-iwanicki]

If there is no more neutral option to use

Tried to find option that would be available in most platforms. Maybe lock BIOS boot medium would be better

I don't see an obvious cause in the log

In the eventlog there is couple events with different hd number:

+ grub_cmd: [ -z (hd1,gpt6)/boot/grub -a -f (hd1,gpt6)/boot/grub/custom.cfg ]
- grub_cmd: [ -z (hd2,gpt6)/boot/grub -a -f (hd2,gpt6)/boot/grub/custom.cfg ]

So i guess I would have to change it.

[m-iwanicki]

@SergiiDmytruk From what I read in pcr-measurements.md EV_EFI_VARIABLE_DRIVER_CONFIG should only be measured if Secure Boot is on?
While this test passed I guess that's because of shim so even with SB enabled it'll still succeed but for wrong reasons?
I'm not sure how to test this otherwise.
I could boot DTS but booting via iPXE doesn't work with SB enabled (at least on QEMU) so I guess it'd have to be via USB with DTS that can be booted when SB is enabled. That would probably limit platforms on which this test can be done.

[SergiiDmytruk]

Actually both old and new version of pcr-measurements.md seem to be wrong... Those variables are unconditionally measured for TPM 2.0 during boot, they are also measured when they are set regardless of TPM version. I was testing with TPM 1.2, saw unexpected event logs and confused myself. Maybe TCG doesn't require those measurement for TPM 1.2 (https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_PFP_r1p05_v23_pub.pdf might apply only to TPM 2.0). I guess adding them wouldn't hurt if it will help with tests and measurements on setting variables might already be "unnecessary" for TPM 1.2 case.

[m-iwanicki]

@SergiiDmytruk Rebased and changed USB test to use SMM write protection.
Now the only test that fails is MBO002.001 Check if event log PCRs match actual values tested on newest binary from Dasharo/coreboot#517 on Protectli v2410. Weird, because this test passed before.

==============================================================================
Measured-Boot
==============================================================================

Checking if tpm2-tools is installed...

Package tpm2-tools is not installed

Installing required package (tpm2-tools)...

Required package (tpm2-tools) installed successfully
MBO001.001 Measured Boot support :: Check whether Measured Boot is... | PASS |
------------------------------------------------------------------------------
MBO002.001 Check if event log PCRs match actual values :: Check wh... | FAIL |
'0x0cb9f33ccb957f8b53944617dfe746bb4788be97f576b913b67adb021ed8e072' does not contain '55a2caae3e70018dfa2d6c6571de2ad2fb98ea2099d64e0f667ab7a441618553'
------------------------------------------------------------------------------
MBO003.001 Changing Secure Boot certificate changes only PCR-7 :: ... | PASS |
------------------------------------------------------------------------------
MBO004.001 Changing Dasharo network boot settings changes only PCR... | SKIP |
Tests in Dasharo Networking Menu are not supported
------------------------------------------------------------------------------
MBO004.002 Changing Dasharo security settings changes only PCR-1 :... | FAIL |
67D3ECDCD7430B68FBAF990F885ABDF6D1B359F0 == 67D3ECDCD7430B68FBAF990F885ABDF6D1B359F0
------------------------------------------------------------------------------
MBO004.003 Changing Dasharo APU settings changes only PCR-1 :: Che... | SKIP |
Tests in Dasharo APU Menu are not supported
------------------------------------------------------------------------------
MBO005.001 Flashing firmware and reset to defaults results in same... | PASS |
------------------------------------------------------------------------------
MBO005.002 Multiple reset to defaults results in identical measure... | PASS |
------------------------------------------------------------------------------
MBO006.001 Identical configuration results in identical measuremen... | PASS |
------------------------------------------------------------------------------
MBO006.002 Identical configuration after reset results in identica... | PASS |
------------------------------------------------------------------------------
Measured-Boot                                                         | FAIL |
10 tests, 6 passed, 2 failed, 2 skipped
==============================================================================

log.tar.gz

MBO004.002 was tested on current fw that's why it failed, after retesting on your version:

MBO002.001 Check if event log PCRs match actual values :: Check wh... | FAIL |
'0x5398290bdf51282424884831e17e1f730ee6c8f5' does not contain 'b2a83b0ebf2f8374299a5b2bdfc31ea955ad7236'
------------------------------------------------------------------------------
MBO004.002 Changing Dasharo security settings changes only PCR-1 :... | PASS |
------------------------------------------------------------------------------
Measured-Boot                                                         | FAIL |
2 tests, 1 passed, 1 failed
==============================================================================

MBO002.001 fails when comparing pcr-sha1/2 pcr.

[SergiiDmytruk]

MBO002.001 fails when comparing pcr-sha1/2 pcr.

That's because tpm2_eventlog doesn't know that it should skip 0100... log entries. I referred to them initially as "fake digests" and later commented in Dasharo/docs#838 (comment) that we might want to not add such entries in EDK so tests wouldn't need to take them into account, but the conclusion is that they stay. I see two ways to address this:

1. Allow some PCR banks to not match if 0100... is present in event log (can happen only for TPM2).
2. Implement computing expected PCR values that skips such fake digests. Iterating over YAML (output of tpm2_eventlog) in Python and performing PCR extend operation (concatenation of current PCR value (all zeros initially) and a digest followed by hashing of the result) sounds doable. I don't think there is anything already available to do this (like an option for tpm2_eventlog).

[pietrushnic]

@SergiiDmytruk, shouldn't we bring such issues to the attention of people who have 10+ years of experience with TCG?

[SergiiDmytruk]

@pietrushnic, the issue is caused by coreboot extending only a single PCR bank and EDK having to deal with it somehow, TCG isn't at fault here. Initial version of upstreamed coreboot patches that add TCG logs allowed extending multiple banks, but one of the reviewers didn't want to "complicate the code", so that part got dropped.

[macpijan]

As author of this comment, do you have a suggestion on how the test scenario should look like @krystian-hebel ?

I think we are discussing the MBO002.001 scenario right now.

[krystian-hebel]

I would have to see the event log to be sure, but to me it looks that scenario is good, it is coreboot that doesn't abide by the TCG PC Client Platform Firmware Profile Specification, namely 3.3.1 Collection and Reporting of Measurements:

4. Unless TPM is hidden, integrity measurements made by the Platform Firmware that are extended MUST be
   extended into all allocated banks for a PCR, see Section 10.1 Introduction.

Section 10.1 is about the event log (so it applies to edk2 rather than coreboot), which includes:

8. There SHALL be a Hash algorithm in the TCG PCClientPCREvent structure for all allocated PCR banks.

So edk2 does the sane thing here, and mismatch between PCR and event log shows that something fishy is going on.

There are two possible solutions I can think of, neither of them involves changes to the test:

• make coreboot work with multiple allocated banks (makes upstream unhappy), or
• enforce that only one PCR bank can be allocated at any given time (decreases security against hash collisions).

[pietrushnic]

make coreboot work with multiple allocated banks (makes upstream unhappy)

When did we start to worry about that? Is the whole upstream happy about Dasharo's existence?

[krystian-hebel]

make coreboot work with multiple allocated banks (makes upstream unhappy)

When did we start to worry about that? Is the whole upstream happy about Dasharo's existence?

https://docs.dasharo.com/newcomers/#dasharo-contribution

Since Dasharo is based on coreboot and edk2, it's best to contribute directly in the upstream if possible.

Or in our internal documentation: http://wiki.3mdeb.com/products/EX018-dasharo-release-process/#3-implement

[pietrushnic]

@krystian-hebel I have no idea what precisely you refer to. Can you be more specific? Ideally, you would quote a sentence, which means we have to make upstream happy because I can't see such a sentence in the document. So maybe we should change the newcomer documentation you are sticking to so it reflects the correct approach because not first time perfect is the enemy of good.

[SergiiDmytruk]

After a discussion with @macpijan I've created Dasharo/dasharo-issues#982 to move this forward. The plan is to limit PCR banks to only one for now (to not postpone landing MeasuredBoot improvements) and make coreboot extend all banks at some later point (the issues gives pointers how to do that).