Hi Youness, On 2018-09-26 01:30 AM, Youness Alaoui wrote:
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.
Feel free to propose a new "security guideline", but document it in the tree.
A similar mechanism is already implemented on Intel: https://review.coreboot.org/#/c/coreboot/+/21327/
Feel free to make it configurable on intel/soc, too.
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 ?
The protected ranges will be used on non Chromeos devices to support vboot. A common interface is being worked on right now, but it'll take some time.
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.
Am 26.09.18 um 10:50 schrieb Patrick Rudolph:
Hi Youness, On 2018-09-26 01:30 AM, Youness Alaoui wrote:
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.
Feel free to propose a new "security guideline", but document it in the tree.
A similar mechanism is already implemented on Intel: https://review.coreboot.org/#/c/coreboot/+/21327/
Please note this is about having the whole chip protected. But not about the decision whether or not to lock this configuration. It reminds me of something, though: If you want to do such configuration in the payload, both coreboot and payload code/configuration have to be kept in sync if you have suspend-to-ram. Because coreboot has to do the same confi- guration as the payload on the resume path (where the payload is not executed).
One way would be to let coreboot decide, e.g. prepare the configuration and don't lock it, and let the payload lock. The payload could validate this configuration before locking (and issue a warning if coreboot didn't set the expected bits).
Nico
I am reading the "flash security recommendation" from PCH BIOS writer guide now, it did say strongly recommend to take those actions. The EISS feature to ensure BIOS region can only get modfiyed from SMM.
On Wed, Sep 26, 2018 at 7:01 AM Nico Huber nico.huber@secunet.com wrote:
Am 26.09.18 um 10:50 schrieb Patrick Rudolph:
Hi Youness, On 2018-09-26 01:30 AM, Youness Alaoui wrote:
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.
Feel free to propose a new "security guideline", but document it in the tree.
A similar mechanism is already implemented on Intel: https://review.coreboot.org/#/c/coreboot/+/21327/
Please note this is about having the whole chip protected. But not about the decision whether or not to lock this configuration. It reminds me of something, though: If you want to do such configuration in the payload, both coreboot and payload code/configuration have to be kept in sync if you have suspend-to-ram. Because coreboot has to do the same confi- guration as the payload on the resume path (where the payload is not executed).
One way would be to let coreboot decide, e.g. prepare the configuration and don't lock it, and let the payload lock. The payload could validate this configuration before locking (and issue a warning if coreboot didn't set the expected bits).
Nico
coreboot mailing list: coreboot@coreboot.org https://mail.coreboot.org/mailman/listinfo/coreboot
-
Lance Zhao -
Nico Huber -
Patrick Rudolph