Attention is currently required from: Andrey Pronin, Julius Werner, Yu-Ping Wu. Miriam Polzer 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:
PS3: Thank you for reviewing! One question: What's the best way to test the new changes? So far I was working with pretty old device I'd probably break if I flash ToT firmware on it (I see this wasn't a good idea now). Is it safe enough to put top-of-tree firmware on a newer device that only branched recently? Or better to merge back this change to a newer firmware branch and then test there?
File src/security/vboot/antirollback.h:
https://review.coreboot.org/c/coreboot/+/59097/comment/74d26a34_5ad00a03 PS2, Line 31: #define ENT_ROLLBACK_NV_INDEX 0x100e
This should probably have COUNTER in the name (e.g. […]
Done
File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/59097/comment/b10f266e_f4d1e179 PS2, Line 113: const static TPMA_NV rw_counter_attributes = {
I would suggest adding PPREAD/PPWRITE here as well even if your design doesn't need it, just in case […]
Done
https://review.coreboot.org/c/coreboot/+/59097/comment/e4095db5_070ae9b3 PS2, Line 116: .TPMA_NV_PLATFORMCREATE = 1,
Similarly, might as well set WRITE_STCLEAR too. […]
Done
https://review.coreboot.org/c/coreboot/+/59097/comment/bbdf6216_f94fda72 PS2, Line 189: * Empty policy digest.
What's this? AFAIK you should only need a policy if TPMA_NV_POLICY_DELETE is set, which it isn't for […]
Done
https://review.coreboot.org/c/coreboot/+/59097/comment/e0d1e3d2_620e6907 PS2, Line 374:
This is where you should set up this space, guarded by if (CONFIG(CHROMEOS))
Done
File src/security/vboot/vboot_logic.c:
https://review.coreboot.org/c/coreboot/+/59097/comment/1e4ba1af_b7ad7322 PS2, Line 281: enterprise_rollback_create_counter();
Spaces should be created from factory_initialize_tpm(), not here. […]
Done
-
Miriam Polzer (Code Review)