[M] Change in coreboot[master]: cpu/intel/microcode: Implement microcode info caching inside cbmem

[Julius Werner (Code Review)]

Attention is currently required from: Tarun Tuli, Subrata Banik, Reka Norman, Kapil Porwal, Arthur Heymans, Lean Sheng Tan.

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74873 )

Change subject: cpu/intel/microcode: Implement microcode info caching inside cbmem
......................................................................

Patch Set 4:

(2 comments)

Patchset:

PS3:
> > > > > > > I assume this patch is in response to b/242473942? And your time savings aren't really f […]

Arthur is right, splitting the file is the only viable way to solve this. (See also some previous discussions about this approach in b/242473942.)

File src/cpu/intel/microcode/microcode.c:

https://review.coreboot.org/c/coreboot/+/74873/comment/e29e53a2_0028557a 
PS3, Line 305: 		info->microcode_ptr = (uintptr_t)find_cbfs_microcode();
> > Is this just stashing a pointer to MMIO flash across reboots? That sounds like a bad idea. […]

No, that can't work. First of all, you're not actually (as far as I can tell) computing a checksum, you're just checking what written in a checksum field in a header structure. If an attacker wants to craft a malicious microcode blob they can just put the right checksum there and then put completely different code in the rest of the file. Also, I don't even know what kind of checksum algorithm this is (it doesn't look like we even have code to compute it in coreboot) -- the word "checksum" usually implies something not cryptographically safe (meaning a sufficiently dedicated attacker can forge it) like a CRC.

Second, you're also just putting that checksum in that same unprotected CBMEM area in memory, right next to the CBFS pointer that we are worried an attacker might change. So they could, you know, just change that checksum field as well to whatever matches their malicious microcode blob.

Fundamentally this problem cannot be solved that way because the time you're trying to save is the time we spend on verification itself. So even if you found a secure way to pass that checksum to the next boot, and you used a cryptographically secure checksum (e.g. SHA-256 hash), and you added the code to actually verify that checksum on the next boot... then that would again take the exact same 20ms you're trying to save here, because you're doing the exact same thing that CBFS verification normally does (computing a SHA-256 hash of the microcode blob). We already have the hash from CBFS metadata, that's not the part that costs time... the part that costs time is the one that you can't skip without breaking the security model.