Sorry, fix typo: 2. With reason above, I feel that adding comment in the code might not be the best idea, because it is so simple that it will easily introduce misunderstanding and confusing.
> -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, > Jiewen > Sent: Saturday, December 17, 2022 11:10 AM > To: Gerd Hoffmann <kra...@redhat.com>; devel@edk2.groups.io > Cc: Justen, Jordan L <jordan.l.jus...@intel.com>; Ard Biesheuvel > <ardb+tianoc...@kernel.org>; Oliver Steffen <ostef...@redhat.com>; Pawel > Polawski <ppola...@redhat.com> > Subject: Re: [edk2-devel] [PATCH 1/1] > OvmfPkg/QemuFlashFvbServicesRuntimeDxe: add security warning > > 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 <kra...@redhat.com> > > Sent: Friday, December 16, 2022 6:12 PM > > To: devel@edk2.groups.io > > Cc: Justen, Jordan L <jordan.l.jus...@intel.com>; Gerd Hoffmann > > <kra...@redhat.com>; Ard Biesheuvel <ardb+tianoc...@kernel.org>; > Oliver > > Steffen <ostef...@redhat.com>; Pawel Polawski <ppola...@redhat.com>; > > Yao, Jiewen <jiewen....@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 <kra...@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 > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#97522): https://edk2.groups.io/g/devel/message/97522 Mute This Topic: https://groups.io/mt/95707152/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
