[coreboot] SPI controller and Lock bits

Youness Alaoui kakaroto at kakaroto.homelinux.net
Thu Sep 27 22:29:08 CEST 2018


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.work at 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.zhao at 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.huber at 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 at coreboot.org
>> https://mail.coreboot.org/mailman/listinfo/coreboot



More information about the coreboot mailing list