Re: [PATCH 1/1] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: add security warning

["Yao, Jiewen" <jiewen.yao@intel.com>]

Hi Gerd
I would like to clarify a couple of things:

1) "Using these builds with writable flash is not secure."

Whenever we say "secure" or "not secure", we need align the threat model at first.
What component is trusted? Which is not trusted? Who is adversary? With which capability? Under which attack scenario? 

Sometimes, we also say: "UEFI secure boot is not secure", because it cannot resist the offline hardware attack the flash chip.
We only say "UEFI secure boot can resist the system software attack."

Also many time, we need debate if DOS attack is in scope or not.

If we are going to say something like that, we need a full description. Just saying: "not secure" is not enough.

2) With reason above, I feel that adding comment in the code might not be the best idea, because it is too simple to introduce misunderstanding and confusing.
Can we add better description in readme? Such as https://github.com/tianocore/edk2/blob/master/OvmfPkg/README

3) What is definition of "stateless secure boot configuration" ?
What does you mean "stateless"? Do you mean "SMM_REQUIRE=FALSE" or something else?
Then why not call it as simple as "secure boot without SMM" ?
I don't understand how "SMM_REQUIRE=FALSE" will contribute "stateless".

I hope we can clarify the terminology if we choose 2).

4) What is the purpose of "Log a warning" ?
Is that to tell people, DON'T DO IT?
Or is that to tell people, you may play with it by yourself, but don't use it a production?
Or something else?

I think we need give a clear answer after we clarify the threat model.
Otherwise, a WARNING just adds confusing, IMHO.

Thank you
Yao Jiewen


> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Friday, December 16, 2022 6:12 PM
> To: devel@edk2.groups.io
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Oliver
> Steffen <osteffen@redhat.com>; Pawel Polawski <ppolawsk@redhat.com>;
> Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH 1/1] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: add
> security warning
> 
> OVMF builds in stateless secure boot configuration
> (SECURE_BOOT_ENABLE=TRUE + SMM_REQUIRE=FALSE) are expected to use
> the
> emulated variable store (EmuVariableFvbRuntimeDxe) with the store being
> re-initialized on each reset (see PlatformInitEmuVariableNvStore())
> 
> Using these builds with writable flash is not secure.  Log a warning
> message saying so in case we find such a configuration.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
> b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
> index 61e1f2e196e5..ab7154685424 100644
> --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
> +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServiceDxe.c
> @@ -57,6 +57,11 @@ InstallProtocolInterfaces (
>                      NULL
>                      );
>      ASSERT_EFI_ERROR (Status);
> + #ifdef SECURE_BOOT_FEATURE_ENABLED
> +    DEBUG ((DEBUG_WARN, "This build is configured for stateless secure
> boot.\n"));
> +    DEBUG ((DEBUG_WARN, "Using this build with writable flash is NOT
> secure.\n"));
> +    // should we ASSERT(0) here?
> + #endif
>    } else if (IsDevicePathEnd (FvbDevice->DevicePath)) {
>      //
>      // Device already exists, so reinstall the FVB protocol
> --
> 2.38.1