On 07/22/19 06:02, Gao, Zhichao wrote: > From: Bret Barkelew <bret.barke...@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.w...@intel.com> > Cc: Hao A Wu <hao.a...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Star Zeng <star.z...@intel.com> > Cc: Liming gao <liming....@intel.com> > Cc: Sean Brogan <sean.bro...@microsoft.com> > Cc: Michael Turner <michael.tur...@microsoft.com> > Cc: Bret Barkelew <bret.barke...@microsoft.com> > Signed-off-by: Zhichao Gao <zhichao....@intel.com> > --- > .../SecurityLockAuditDebugLib.c | 53 +++++++++++++++++++ > .../SecurityLockAuditDebugLib.inf | 29 ++++++++++ > 2 files changed, 82 insertions(+) > create mode 100644 > MdeModulePkg/Library/SecurityLockAuditDebugLib/SecurityLockAuditDebugLib.c > create mode 100644 > MdeModulePkg/Library/SecurityLockAuditDebugLib/SecurityLockAuditDebugLib.inf > > diff --git > a/MdeModulePkg/Library/SecurityLockAuditDebugLib/SecurityLockAuditDebugLib.c > b/MdeModulePkg/Library/SecurityLockAuditDebugLib/SecurityLockAuditDebugLib.c > new file mode 100644 > index 0000000000..c1872bc023 > --- /dev/null > +++ > b/MdeModulePkg/Library/SecurityLockAuditDebugLib/SecurityLockAuditDebugLib.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). 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. (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/SecurityLockAuditDebugLib.inf > > b/MdeModulePkg/Library/SecurityLockAuditDebugLib/SecurityLockAuditDebugLib.inf > new file mode 100644 > index 0000000000..b641016087 > --- /dev/null > +++ > b/MdeModulePkg/Library/SecurityLockAuditDebugLib/SecurityLockAuditDebugLib.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 (#44154): https://edk2.groups.io/g/devel/message/44154 Mute This Topic: https://groups.io/mt/32555407/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
