[coreboot] SPI controller and Lock bits

[Youness Alaoui]

Hi,

I'm trying to add a way to lock the SPI flash to be read-only via
software *after* coreboot boots. The scenario is basically with using
Heads, you could authenticate to it (with a yubikey/nitrokey/librem
key) then be able to flash a new rom (update your BIOS), but once you
boot an OS, Heads would first lock the flash so it can't be written
to.
This should add some security to avoid any malware writing to the
flash, or someone booting into a USB stick and using that to flash a
malicious BIOS, but still gives the user the freedom of updating their
flash whenever they want to.

The problem is that I can't make the flash read-only because the SPI
interface is already locked down by coreboot (see
src/soc/intel/skylake/lockdown.c and
src/soc/intel/common/block/fast_spi/fast_spi.c).

There's a couple of things happening here :
First, the FLOCKDN bit is set which prevents us from enabling the
write protection. the BIOS Interface lock down is controlled by the
chipset_lockdown config variable, but the FLOCKDN is not behind a
config variable.
The second thing is that if I wanted to use the protected ranges
feature to lock specific regions, they are all getting locked using
the discrete lock register even while being unused. The locking of the
protected ranges was added in this change :
https://review.coreboot.org/c/coreboot/+/21064 and it passed without
notice among the move that the commit was supposed to do.

The commit states that the lockdown is meant to "support platform
security guidelines", but I think that this is not actually good
because coreboot leaves everything read-write and locks down the
registers so we can't make it read-only. I think that the security
guidelines would say to disable the write protection before locking
the registers down.

Either way, I'm going to need to add a way to make this configurable,
so my main questions here are :
- Should I create a new config variable to decide on whether or not to
lock the spi registers and another one to decide on whether or not to
lock the protected ranges ?
- Should I make the chipset_lockdown (currently used for locking the
BIOS CNTL register from LPC and SPI controllers) into an OR-ed flags
variable where we can say : chipset_lockdown = LOCKDOWN_COREBOOT |
LOCKDOWN_SPI | LOCKDOWN_PROTECTED_RANGES ?
- Should I make a single new config variable to decide what to
lockdown (LPC_BIOS, SPI_BIOS, SPI_BAR, SPI_PROTECTED_RANGES) and only
set them if the CHIPSET_LOCKDOWN_COREBOOT is set ? And if
chipset_lockdown is set to CHIPSET_LOCKDOWN_FSP not lockdown anything
at all ?
- Do we want to keep the protected ranges locked down at all, have
them configurable or completely remove that as I don't see the point
in using the discrete lock register ?

Once I see a consensus on what's the best way to move forward, I'll
implement it and push it for review.

Note: I think these only affect hardware sequencing though so I assume
someone could always use software sequencing to do the writes. As long
as the FLOCKDN bit isn't set though, I could remove all write-related
opcodes from the software sequencing register, which would also
prevent someone using swseq to do writes.

Thanks,
Youness.