[edk2-devel] [PATCH 2/5] MdeModulePkg/SecurityLockAuditDebugLib: Add lib instance
Wu, Hao A
hao.a.wu at intel.com
Tue Jul 23 03:26:27 UTC 2019
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek at redhat.com]
> Sent: Tuesday, July 23, 2019 4:34 AM
> To: devel at edk2.groups.io; Gao, Zhichao
> Cc: Bret Barkelew; Wang, Jian J; Wu, Hao A; Ni, Ray; Zeng, Star; Gao, Liming;
> Sean Brogan; Michael Turner
> Subject: Re: [edk2-devel] [PATCH 2/5]
> MdeModulePkg/SecurityLockAuditDebugLib: Add lib instance
>
> On 07/22/19 06:02, Gao, Zhichao wrote:
> > From: Bret Barkelew <Bret.Barkelew at microsoft.com>
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2006
> >
> > Add the instance of SecurityLockAuditLib. This instance
> > has one interface SecurityLockReportEvent to log hardware
> > and software security locks info.
> >
> > Cc: Jian J Wang <jian.j.wang at intel.com>
> > Cc: Hao A Wu <hao.a.wu at intel.com>
> > Cc: Ray Ni <ray.ni at intel.com>
> > Cc: Star Zeng <star.zeng at intel.com>
> > Cc: Liming gao <liming.gao at intel.com>
> > Cc: Sean Brogan <sean.brogan at microsoft.com>
> > Cc: Michael Turner <Michael.Turner at microsoft.com>
> > Cc: Bret Barkelew <Bret.Barkelew at microsoft.com>
> > Signed-off-by: Zhichao Gao <zhichao.gao at intel.com>
> > ---
> > .../SecurityLockAuditDebugLib.c | 53 +++++++++++++++++++
> > .../SecurityLockAuditDebugLib.inf | 29 ++++++++++
> > 2 files changed, 82 insertions(+)
> > create mode 100644
> MdeModulePkg/Library/SecurityLockAuditDebugLib/SecurityLockAuditDebu
> gLib.c
> > create mode 100644
> MdeModulePkg/Library/SecurityLockAuditDebugLib/SecurityLockAuditDebu
> gLib.inf
> >
> > diff --git
> a/MdeModulePkg/Library/SecurityLockAuditDebugLib/SecurityLockAuditDe
> bugLib.c
> b/MdeModulePkg/Library/SecurityLockAuditDebugLib/SecurityLockAuditDe
> bugLib.c
> > new file mode 100644
> > index 0000000000..c1872bc023
> > --- /dev/null
> > +++
> b/MdeModulePkg/Library/SecurityLockAuditDebugLib/SecurityLockAuditDe
> bugLib.c
> > @@ -0,0 +1,53 @@
> > +/** @file
> > + This library implements the necessary functions
> > + to log hardware and software security locks for post-processing
> > +
> > + Copyright (c) 2018, Microsoft Corporation
> > +
> > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#include <Base.h>
> > +#include <Library/SecurityLockAuditLib.h>
> > +#include <Library/DebugLib.h>
> > +
> > +//
> > +// Used to look up lock name from LOCK_TYPE enum
> > +//
> > +CHAR8* mLockName[] = {
> > + "SOFTWARE_LOCK",
> > + "HARDWARE_LOCK"
> > +};
> > +
> > +
> > +/**
> > + Function for security Lock event logging and reporting
> > +
> > + @param[in] Module GUID of calling module
> > + @param[in] Function Name of calling function
> > + @param[in] LockEventText Debug message explaining what is
> locked
> > + @param[in] LockType Enumerated lock type for differentiation
> > +
> > +**/
> > +VOID
> > +EFIAPI
> > +SecurityLockReportEvent (
> > + IN GUID *Module,
> > + IN CONST CHAR8 *Function,
> > + IN CONST CHAR8 *LockEventText,
> > + IN LOCK_TYPE LockType
> > + )
> > +{
> > + UINTN LockTypeIndex;
> > + UINTN LockNameCount;
> > +
> > + LockTypeIndex = (UINTN)LockType;
> > + LockNameCount = sizeof (mLockName) / sizeof (mLockName[0]);
> > +
> > + if (LockTypeIndex < LockNameCount) {
> > + DEBUG ((DEBUG_ERROR, "SecurityLock::LockType: %a, Module: %g,
> Function: %a, Output: %a\n", mLockName[LockTypeIndex], Module,
> Function, LockEventText));
> > + } else {
> > + DEBUG ((DEBUG_ERROR, "SecurityLock::LockType: %d, Module: %g,
> Function: %a, Output: %a\n", LockType, Module, Function, LockEventText));
> > + }
> > +}
>
> I disagree with this implementation. Security log messages are
> important, but they are not necessarily errors. DEBUG_ERROR should be
> used for errors, preferably for such that cannot be recovered from
> (DEBUG_WARN is OK for recoverable errors).
Agree that using 'DEBUG_ERROR' is improper here.
>
> If DEBUG_INFO is deemed too "quiet" for logging security events, then we
> should introduce a new bit value for the debug bitmask, such as
> DEBUG_SECURITY. We have a number of "special purpose" bits already:
>
> - DEBUG_LOAD
> - DEBUG_FS
> - DEBUG_POOL
> - DEBUG_PAGE
> - DEBUG_DISPATCH
> - etc
>
> and I think we have room for DEBUG_SECURITY.
Not sure if 'DEBUG_EVENT' can be used in this case.
I think this bit is added for event-related services in the UEFI/PI core
system. However, I do not see any usage of 'DEBUG_EVENT' or 'EFI_D_EVENT'
(at least within the edk2 repository). So I am thinking whether we can
extend this bit to match this case here.
I am also fine with adding a new bit.
Best Regards,
Hao Wu
>
> (1) And then, the log mask in this library instance should be
>
> DEBUG_SECURITY | DEBUG_INFO
>
> I believe.
>
> Thanks
> Laszlo
>
>
>
> > diff --git
> a/MdeModulePkg/Library/SecurityLockAuditDebugLib/SecurityLockAuditDe
> bugLib.inf
> b/MdeModulePkg/Library/SecurityLockAuditDebugLib/SecurityLockAuditDe
> bugLib.inf
> > new file mode 100644
> > index 0000000000..b641016087
> > --- /dev/null
> > +++
> b/MdeModulePkg/Library/SecurityLockAuditDebugLib/SecurityLockAuditDe
> bugLib.inf
> > @@ -0,0 +1,29 @@
> > +## @file
> > +#
> > +# Library that implements logging and reporting for security locks
> > +# Using DebugLib
> > +#
> > +#
> > +# Copyright (c) 2018, Microsoft Corporation
> > +#
> > +# SPDX-License-Identifier: BSD-2-Clause-Patent
> > +##
> > +
> > +[Defines]
> > + INF_VERSION = 0x00010005
> > + BASE_NAME = SecurityLockAuditDebugLib
> > + FILE_GUID = 459d0456-d6be-458e-9cc8-e9b21745f9aa
> > + MODULE_TYPE = BASE
> > + VERSION_STRING = 1.0
> > + LIBRARY_CLASS = SecurityLockAuditLib
> > +
> > +[Sources.common]
> > + SecurityLockAuditDebugLib.c
> > +
> > +[Packages]
> > + MdePkg/MdePkg.dec
> > + MdeModulePkg/MdeModulePkg.dec
> > +
> > +[LibraryClasses]
> > + BaseLib
> > + DebugLib
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#44198): https://edk2.groups.io/g/devel/message/44198
Mute This Topic: https://groups.io/mt/32555407/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list