Attention is currently required from: Miriam Polzer, Andrey Pronin, Yu-Ping Wu. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59097 )
Change subject: security/vboot: Add NVRAM counter for TPM 2.0 ......................................................................
Patch Set 3:
(7 comments)
Patchset:
PS2:
Could you point me to the design doc?
go/rollback-2.0
Patchset:
PS3:
Thank you for reviewing! […]
Yes, for most devices ToT firmware should work fine. You should test on ToT first and then worry about backporting once that patch lands.
Note that testing TPM factory initialization is tricky because you have to fully erase all TPM spaces manually to get it to run again. To do that you have to boot a test image in recovery mode and use manual tpmc undefinespace commands. Deleting the "firmware" space (the one created with pcr0_allowed_policy) is even more tricky, you can find instructions in b/140958855. Also be aware that the patch which added that policy only landed in October 2020, so this will only work on Chromebooks that have had their very first boot (in the factory) with firmware that included that patch.
File src/security/vboot/antirollback.h:
https://review.coreboot.org/c/coreboot/+/59097/comment/b2880549_a424f776 PS3, Line 100: uint32_t enterprise_rollback_create_counter(void); This function no longer needs to be exported now.
File src/security/vboot/secdata_mock.c:
https://review.coreboot.org/c/coreboot/+/59097/comment/c3e280b7_63e0a3e3 PS3, Line 73: vb2_error_t enterprise_rollback_create_counter(void) (...and then you won't need a mock for it...)
File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/59097/comment/072290a4_84bb3322 PS3, Line 150: .TPMA_NV_NO_DA = 1, Actually, now that I'm comparing this more with the ZTE counter space I'm also wondering why it has NV_BITS and NV_NO_DA set and whether this should have those as well. I think BITS was about something else (I think the ZTE design needed the counter to start at 0 which isn't really the case for this), but NO_DA might still be a good idea. Hopefully Andrey can offer some more guidance.
https://review.coreboot.org/c/coreboot/+/59097/comment/6ec40f60_0853b469 PS3, Line 438: NULL, 0) Soo... actually, wait a second. The only reason you're doing this whole counter business is that you want to protect against a post factum, invasive attack (which includes firmware reflashing to fake the PCR0 sealing). Then not having a policy here would be defeat that whole purpose, because the attacker in that scenario could replace the firmware with something that simply deletes and recreates this counter with platform authorization.
So I think what you actually want is to use unsatisfiable_policy here (and set TPMA_NV_POLICY_DELETE to actually make it take effect), same as the ZTE RMA bytes counter.
https://review.coreboot.org/c/coreboot/+/59097/comment/435aacb5_ff74c992 PS3, Line 540: uint32_t enterprise_rollback_create_counter(void) ...and won't need this anymore.