Change in coreboot[master]: src/security/vboot: Set up secure counter space in TPM NVRAM

[Julius Werner (Code Review)]

Attention is currently required from: Andrey Pronin, Raul Rangel, Paul Menzel, Yu-Ping Wu, Karthik Ramasubramanian.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59476 )

Change subject: src/security/vboot: Set up secure counter space in TPM NVRAM
......................................................................

Patch Set 2:

(4 comments)

File src/security/vboot/antirollback.h:

https://review.coreboot.org/c/coreboot/+/59476/comment/ba541d38_744b0acf 
PS2, Line 30: #define HASH_NV_SIZE                    VB2_SHA256_DIGEST_SIZE
I would insert them here so the spaces are listed in ascending order. (In fact, I don't know why we picked such different indices for the ZTE spaces... Andrey, do you remember? Is there any significance to those indices or can you just pick whatever number? Maybe we should  put these secure counters aside in their own area as well, so that if we find out that we need more in the future it's easy to add some (and the directly following indices won't be taken up by other unrelated spaces that get added later yet).

https://review.coreboot.org/c/coreboot/+/59476/comment/e3682459_4113ad0d 
PS2, Line 39: SECURE_COUNTER1_NV_INDEX
> If you go this route, I would say put a comment saying that they are CR50 specific.

Well, they aren't really (I think?), counters are part of the TPM 2.0 standard.

https://review.coreboot.org/c/coreboot/+/59476/comment/560ba2d1_5e1dc35f 
PS2, Line 42: #define SECURE_COUNTER4_NV_INDEX	0x1012
I would maybe go a bit further and just define this as SECURE_COUNTER_NV_INDEX(n) (0x... + n), and a SECURE_COUNTER_NUM 4. That makes your for-loop cleaner to write.

File src/security/vboot/secure_counter.h:

https://review.coreboot.org/c/coreboot/+/59476/comment/7354f847_9ad35db7 
PS1, Line 14: 	SECURE_COUNTER4_NV_INDEX,		/* 0x1012 */
> Sorry for prematurely saying done. I moved it to antirollback.h. […]

No, I think the counters should be specifically named after their purpose. "counter" is just a general TPM concept (and they're all "secure" in the same way), we can create as many as we want for those, and in fact we already have some (for ZTE and soon for CB:59097). But I don't think you want coreboot to just create a range of generic counters that the OS can then decide what to do with, so that one platform uses the same indices for Widevine and another for something completely different... because then what happens if the platform that decided to use these same counters for something else eventually decides that it wants to start doing the Widevine stuff as well (and reuse the same userland code from those other platforms that originally did it)? Then you have a mess on your hands.

I think the right way to organize this is that here, in this header, we sort of have a unique registry for TPM space indices where every purpose that any OS running on top of coreboot could ever want to use a TPM space for is assigned its own unique index, and we make sure there are no overlaps. The index space is big enough so we shouldn't run out (and since the extra code is all guarded by compile time flags it's not really a "bloat" problem either). Each of those indices should only be used for one specific purpose (according to one specific "API"), and that way we won't get nasty clashes when platforms later try to mix and match some things together that previous platforms did with the TPM.