Attention is currently required from: Michał Żygowski, Maciej Pijanowski, Christian Walter, Sergii Dmytruk.
Krystian Hebel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68747 )
Change subject: [WIP] security/tpm: add TPM log format as per 1.2 spec ......................................................................
Patch Set 2:
(6 comments)
File src/security/tpm/tpm12_log_serialized.h:
https://review.coreboot.org/c/coreboot/+/68747/comment/bf690000_1122c882 PS2, Line 11: TCPA_DIGEST_MAX_LENGTH `TCPA_DIGEST_LENGTH`, there can be no other.
https://review.coreboot.org/c/coreboot/+/68747/comment/56194302_45edbd97 PS2, Line 13: #define TCPA_PCR_HASH_LEN 10 What is this for?
https://review.coreboot.org/c/coreboot/+/68747/comment/27f1d3e2_93581649 PS2, Line 22: char name[TCPA_PCR_HASH_NAME]; Specification doesn't precise that this is a name, just data. Smaller measured blocks may even be copied here, e.g. `ascii_bios_measurements` for my PC with vendor firmware looks like this:
``` 5 50d327b20ea5f890635a11d8fff2dffbeaa867bb 80000002 [] 5 35c5f66a7073aac2392cf3f71e0e8ddb9569298b 80000002 [] 0 d9be6524a5f5047db5866813acf3277892a7a30a 04 [����] 1 d9be6524a5f5047db5866813acf3277892a7a30a 04 [����] 2 d9be6524a5f5047db5866813acf3277892a7a30a 04 [����] 3 d9be6524a5f5047db5866813acf3277892a7a30a 04 [����] 4 d9be6524a5f5047db5866813acf3277892a7a30a 04 [����] 5 d9be6524a5f5047db5866813acf3277892a7a30a 04 [����] 6 d9be6524a5f5047db5866813acf3277892a7a30a 04 [����] 7 d9be6524a5f5047db5866813acf3277892a7a30a 04 [����] 5 b6ae9742d3936a4291cfed8df775bc4657e368c0 80000007 [] ```
For better compatibility we should use `uint8_t`.
Also, this field should have variable length.
https://review.coreboot.org/c/coreboot/+/68747/comment/938e12c3_e17e5a9c PS2, Line 43: uint16_t max_entries; Since fields are not fixed size (in general), instead of `{max,num}_entries` it would be better to keep full size in bytes and offset at which next entry should be added. `tcpa_cbmem_init()` would have to be changed accordingly, as well as code for adding new entries.
File src/security/tpm/tspi.h:
https://review.coreboot.org/c/coreboot/+/68747/comment/36dd7481_429bb5be PS2, Line 13: #include "tpm12_log_serialized.h" May be worth moving to `commonlib`
File src/security/tpm/tspi/log-tpm12.c:
https://review.coreboot.org/c/coreboot/+/68747/comment/04115d44_4106ca19 PS2, Line 5: then `than`
-
Krystian Hebel (Code Review)