[SeaBIOS] Saving a few bytes across a reboot

[Laszlo Ersek]

On 02/08/18 16:52, Marc-André Lureau wrote:
> Hi
>
> On Wed, Feb 7, 2018 at 6:21 PM, Laszlo Ersek <lersek at redhat.com>
> wrote:
>> On 02/07/18 17:44, Stefan Berger wrote:
>>> On 02/07/2018 10:50 AM, Laszlo Ersek wrote:
>>
>>>> OK, but if the OS is allowed to modify this set of "queued
>>>> operations", then what protection is expected of SMM? Whether you
>>>> can modify the TPM directly, or queue random commands for it at
>>>> libery, what's the difference?
>>>
>>>
>>> On the OS level it is presumably an operation that is reserved to
>>> the admin to queue the operation.
>>>
>>> I am not that familiar with UEFI and who is allowed to run code
>>> there and what code it can execute. But UEFI seems to lock the
>>> variable that holds that PPI code that tells it what to do after
>>> next reboot. So presumably a UEFI module cannot modify that variable
>>> but can only read it (and hopefully not manipulate NVRAM directly).
>>> If PPI was implemented through a memory location where the code gets
>>> written to it could do that likely easily (unless memory protections
>>> are setup by UEFI, which I don't know), cause a reset and have UEFI
>>> execute on that code.
>>
>> This makes sense... but then it doesn't make sense :)
>>
>> Assume that the variable is indeed "locked" (so that random UEFI
>> drivers / apps cannot rewrite it using the UEFI variable service).
>> Then,
>>
>> - if the lock is enforced in SMM, then the variable will be locked
>>   from the OS as well, not just from 3rd party UEFI apps, so no PPI
>>   operations can ever be queued,
>>
>> - if the lock is "simulated" in ACPI or in non-SMM firmare code (= in
>>   the "runtime DXE driver" layer), then the lock can be circumvented
>>   by both 3rd party UEFI apps and the OS.
>
>
> Regarding security of PPI pending operations, the spec clearly says
> "The location for tracking the pending PPI
> operation, including the tracking of necessary PLATFORM RESET
> operations, does not need to be a secure or trusted location." (9.9
> p.32)

Indeed, this is what the spec writes.

In edk2, I found one variable that seems related, it's called
"PhysicalPresenceFlags", in the 0F6499B1-E9AD-493D-B9C2-2F90815C6CBC
variable namespace:

[SecurityPkg/Include/Guid/PhysicalPresenceData.h]

> //
> // This variable is used to save TPM Management Flags and corresponding operations.
> // It should be protected from malicious software (e.g. Set it as read-only variable).
> //
> #define PHYSICAL_PRESENCE_FLAGS_VARIABLE  L"PhysicalPresenceFlags"
> typedef struct {
>   UINT8   PPFlags;
> } EFI_PHYSICAL_PRESENCE_FLAGS;

In
"SecurityPkg/Library/DxeTcgPhysicalPresenceLib/DxeTcgPhysicalPresenceLib.c",
the TcgPhysicalPresenceLibProcessRequest() function is introduced like
this:

> /**
>   Check and execute the pending TPM request and Lock TPM.
>
>   The TPM request may come from OS or BIOS. This API will display request information and wait
>   for user confirmation if TPM request exists. The TPM request will be sent to TPM device after
>   the TPM request is confirmed, and one or more reset may be required to make TPM request to
>   take effect. At last, it will lock TPM to prevent TPM state change by malware.
>
>   This API should be invoked after console in and console out are all ready as they are required
>   to display request information and get user input to confirm the request. This API should also
>   be invoked as early as possible as TPM is locked in this function.
>
> **/

Part of the function:

>   //
>   // This flags variable controls whether physical presence is required for TPM command.
>   // It should be protected from malicious software. We set it as read-only variable here.
>   //
>   Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, NULL, (VOID **)&VariableLockProtocol);
>   if (!EFI_ERROR (Status)) {
>     Status = VariableLockProtocol->RequestToLock (
>                                      VariableLockProtocol,
>                                      PHYSICAL_PRESENCE_FLAGS_VARIABLE,
>                                      &gEfiPhysicalPresenceGuid
>                                      );
>     if (EFI_ERROR (Status)) {
>       DEBUG ((EFI_D_ERROR, "[TPM] Error when lock variable %s, Status = %r\n", PHYSICAL_PRESENCE_FLAGS_VARIABLE, Status));
>       ASSERT_EFI_ERROR (Status);
>     }
>   }

When the SMM driver stack is used, the variable lock service is composed
of two halves (as expected):

- "MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c"
  composes a request buffer, with the
  SMM_VARIABLE_FUNCTION_LOCK_VARIABLE command, and raises an SMI,

- "MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c" and
  "MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c" set the
  (internal) property VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY on the
  variable, in SMM code.

Thus the variable lock is enforced in SMM, and the OS cannot use the
UEFI variable services to modify the variable. (It could be possible
that the ACPI method enters SMM with different parameters, and then SMM
code modifies the variable *without* going through the outer variable
services.) I don't know.

On 02/08/18 16:52, Marc-André Lureau wrote:

> I assume this is because the user has to confirm the pending operation
> in pre-os console, so if some attacker wanted to clear the TPM, the
> user would have to confirm it (same for other operation of flags
> manipulation). That may not be the best security design, but at least,
> the user could be in control.
>
> How does the UEFI runtime variable services verify the authenticity of
> the requests? Or does it only check a request validity?

I didn't find any particular verification for this variable.

Thanks
Laszlo