Thanks everyone for the responses. So far my understanding on the task at hand is : - Add a CONFIG to decide if we set FLOCKDN or not (and one to decide if we lock it on the resume path?) - Remove the chipset_lockdown devicetree config and change the code to always assume it's LOCKDOWN_COREBOOT (this applies to all platforms, right?) - remove the call to fast_spi_pr_dlock() and fix the comment above that function to be more accurate - Add .config CONFIG options or maybe optional devicetree configs for how to setup the registers so coreboot can prepare them (due to the resume path) ?
Can you please confirm the ones with question marks? Is there some rule on "add as little CONFIG options as possible" ? > AIUI, FLOCKDN always locks the PRRs. Actually from what I can see, the FLOCKDN will lock a few registers, among which it will lock the "discrete PR locks" register, and it will lock the PR 0, 1 and 2, but there's also a PRR34_LOCKDN bit to separately lock the PRR 3 and 4. So technically, I could leave FLOCKDN set, and in the payload, just set protected range in PRR3 or PRR4, then set the PRR34_LOCKDN. Unfortunately, I can't do that because the PRR3 and 4 were already locked with the discrete lock register (if I remove the call to fast_spi_pr_dlock(), it should be fine for my needs though I think, since you said swseq would also fail for protected ranges). On Thu, Sep 27, 2018 at 1:09 PM Dhaval Sharma <dvsharma.w...@gmail.com> wrote: > > "but once you boot an OS, Heads would first lock the flash so it can't be > written to'. Is this some kind of a OS driver? The typical usage of all > locking register which are write once is to have them locked up within BIOS > itself which is within the most trusted boundary. Best we could do it push > out the locking part to the latest stages but not completely out IMHO. The problem is we want to allow users to update their flash and coreboot doesn't have a "flash update utility" integrated, so it has to happen in the payload, which is why coreboot needs to not lock anything then let the payload do the locking for us instead. Heads is the linux-based payload we're using, and the idea is that Heads would lock the flash before it actually boots any OS (from HDD or from USB), this way you can only update your flash from within Heads itself and Heads will ensure that the image you're trying to flash is properly signed, or that you authenticate first before it would allow you to do that (prevents someone from booting into a live USB and flash a malicious bios). > You can have write opcodes in the menu, they won't work on protected regions. > But if you have > a partially protected flash (e.g. vboot), they are still useful. Ok, that's interesting. I was actually thinking of just completely disabling writing to any region, but maybe I can use the PRR instead. > Also note that swseq is not an option anymore since Skylake (IIRC, the > ME protects the flash-cycle go bit in its default configuration). With > only hwseq, you can only access the flash chip's first status register > which makes many security features unusable. So we have to rely on the > protected regions :-/ I hope Intel is going to fix that for future plat- > forms (or with an ME update if that could add more hwseq commands). Well, I saw the opmenu/optypes/etc.. registers still being set in skylake (their offsets were not in the datasheet though), so I figured swseq is still there, just undocumented. It might work on systems where the ME is disabled. I'd have to test. I was thinking of using swseq and add the write-register-2 opcode and use it to set SPR1 and lock the entire flash until it gets power cycled. I don't know if that would work during suspend, but if it does, it might help with the resume path at least (but would prevent updating flash after a reboot and would force user to shut down and boot). I guess I have some trial-error to do. Actually, if status register remains the same through a reboot/suspend/resume, and swseq doesn't work on skylake at all, I could just set the protect bits and prevent writing to status register on resume path. > 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). Thanks, that's a good idea, I forgot about the suspend/resume case. This implies I need coreboot to know about what config the payload will want to set. I'm not sure if that should go into a CONFIG or a devicetree config. Since it might be user-configured, it makes more sense for it to go into .config, right ? Thanks again, Youness. > > > On Thu, Sep 27, 2018 at 9:55 PM Lance Zhao <lance.z...@gmail.com> wrote: >> >> Okay, then I believe we should leave the decision on CONFIG instead of force >> lockdown blindly. As of now, that's still optional I believe. >> >> >> On Thu, Sep 27, 2018 at 3:49 AM Nico Huber <nico.hu...@secunet.com> wrote: >>> >>> Am 26.09.18 um 22:26 schrieb Lance Zhao: >>> > 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. >>> >>> The EISS bit is a highly questionable feature. It's part of the lost >>> cause of security by treating SMM more privileged than the OS. AFAIK, >>> coreboot vendors have secured flash access properly in the past without >>> SMM features and never failed [1]. OTOH, UEFI vendors often granted SMM >>> full flash access in the past and failed to secure SMM [2]. >>> >>> To my knowledge, EISS is incompatible to vboot btw. (not by design but >>> to the current implementation). >>> >>> So I "strongly recommend" to ignore Intel's SMM recommendations wrt. >>> flash access and recommend instead to never grant SMM more privileges >>> than the OS. >>> >>> Nico >>> >>> [1] At least since the introduction of SPI flash chips. Earlier there >>> were possible race conditions regarding the BIOS Write Enable bit >>> where you needed SMM for protection, or had to use the flash chip's >>> own security features. But that was before/maybe why EISS became a >>> feature. >>> [2] e.g. https://github.com/Cr4sh/ThinkPwn (the list of vulnerable >>> systems is long and incomplete) >> >> -- >> coreboot mailing list: coreboot@coreboot.org >> https://mail.coreboot.org/mailman/listinfo/coreboot -- coreboot mailing list: coreboot@coreboot.org https://mail.coreboot.org/mailman/listinfo/coreboot