Hi Krystian, On 17.09.24 19:42, Krystian Hebel wrote: > On 9/14/24 2:29 PM, Nico Huber wrote: >> On 14.09.24 00:03, Krystian Hebel wrote: >>>> So, what I'm suggesting is to just look for an update in a pre- >>>> defined path on the boot medium. [...] >>> This is something we were considering. The problem with that >>> approach is that coreboot can (and, in most of platforms >>> supported by us, does) lock the boot medium, either by >>> mechanism implemented by src/security/lockdown, or SMM BWP, >>> or both. The payload isn't able to remove those locks, they >>> wouldn't be of much use if it could. >> Um no, not coreboot. coreboot is free software and if it would >> do something that hinders security, we would fix it. Judging >> by the terms you are using, I assume this is about Intel FSP. >> FSP does lock things, sometimes too early, sometimes even in >> an insecure state (i.e. locking empty protection registers). I >> don't think we should allow Intel's insecurity to have such an >> influence on coreboot designs, rather we should contain it as >> much as possible, i.e. try to find solutions that are as close >> as possible to what we would do in a nicer world without FSP. > > No, it is coreboot I'm talking about. Unless CONFIG_BOOTMEDIA_LOCK_NONE > is selected, the call trace is as following: > boot_device_security_lockdown(), called by boot state machine [1] > boot_device_wp_region(), if there are any RO regions [2] > spi_flash_set_write_protected() [3] appropriate prot_ops->set_write() > [4] set_write() interacts with SPI flash directly [5] In case of SMM > BWP, this is also set by coreboot [6]. FSP isn't involved anywhere in > these processes. [1] > https://github.com/coreboot/coreboot/blob/c512585e55d3ba998c9e2b6ffc6899642e2c297c/src/security/lockdown/lockdown.c#L67 > [2] > https://github.com/coreboot/coreboot/blob/c512585e55d3ba998c9e2b6ffc6899642e2c297c/src/security/lockdown/lockdown.c#L52 > [3] > https://github.com/coreboot/coreboot/blob/c512585e55d3ba998c9e2b6ffc6899642e2c297c/src/drivers/spi/boot_device_rw_nommap.c#L108 > [4] > https://github.com/coreboot/coreboot/blob/c512585e55d3ba998c9e2b6ffc6899642e2c297c/src/drivers/spi/spi_flash.c#L653 > [5] > https://github.com/coreboot/coreboot/blob/c512585e55d3ba998c9e2b6ffc6899642e2c297c/src/drivers/spi/winbond.c#L598 > [6] > https://github.com/coreboot/coreboot/blob/c512585e55d3ba998c9e2b6ffc6899642e2c297c/src/soc/intel/common/pch/lockdown/lockdown.c#L97
Um yes, but it's all configurable, isn't it? What I miss is why you'd choose a configuration that doesn't allow you to keep your security model simple, i.e. if that is what is demanding to take the SG solution. Reading below I guess it's that you don't trust edk2 to always do the right thing on a (less locked down) update path? > >>> The only option is to not set them in the first place, which >>> means that coreboot must be aware of existence of update, >>> whatever it's form may be. Dealing with in-memory capsules >>> is much easier than adding storage and filesystem drivers >>> to coreboot, not to mention that this would be against its >>> design philosophy. >> You're mixing things that shouldn't be mixed, IMO. Absolutely >> yes, of course dealing with memory structures is easier than >> dealing with storage *in coreboot*. But we shouldn't do any >> such data processing *in coreboot*, anyway. What coreboot re- >> quires is only the control over the locking. And there *are* >> alternatives. > > What coreboot requires is control over the locking *and* > knowledge about which regions of memory to avoid, If you want the SG solution, yes. But I think that's where we are talking past each other. To me this seem like making the solution a requirement. > e.g. when > loading the payload or preparing data for it. Basically all > of the code that isn't part of S3 resume path is free to > overwrite memory that isn't otherwise reserved. Unfortunately, > this means parsing of scatter-gather lists, we weren't able > to find a way around it. > >> tl;dr this is what we came up with @secunet when Intel FSP was >> pushed into mainstream coreboot: We use an nvram flag to tell >> coreboot to lock or not[1]. The payload then has to live with >> what it gets: locked => don't update, unlocked => don't boot. > > This is similar to what we've done, except instead of using > coreboot options API, we're making the decision based on UEFI > variables related to capsules, which are read anyway to locate > the capsule data. I'm curious, though, have you found a way to > enforce "don't boot" part without depending on payload's good > behavior? No, we haven't. But we come from different worlds. For us, the payload is simple and controllable and coreboot is the too com- plex, hard to control beast (multiple exit points, boot-state hooks, dependence on too many variables...and ofc. FSP). So we trust the payload more than coreboot when it comes to security locks. The only reason for us to get coreboot involved with the flash security was that many FSP binaries lock things that they shouldn't. Nico _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-le...@coreboot.org
