Hi,
> > (a) the SecureBoot variable is not present (EFI_NOT_FOUND) according to
> > the return value, or
>
> @Maintainers Would there be any objection to drop this and skip the SB checks
> only when explicitly disabled?
> Please explicitly respond even if not, so we don't end up with everyone
> silently agreeing, but forgetting about the patch after. Thanks! :)
I hold back v2, waiting for an answer here.
> > - if (*SecureBoot == SECURE_BOOT_MODE_DISABLE) {
> > - FreePool (SecureBoot);
> > + if ((VarStatus == EFI_SUCCESS) && (SecureBoot ==
> > SECURE_BOOT_MODE_DISABLE)) {
>
> I would check the attributes here as well. They should be BS | RT, but
> explicitly not NV. This would force the SB checks in case a malicious
> actor somehow managed to store a persistent disable-value variable (be
> that a bug, physical access, or other means).
Like this (incremental fixup)?
Do we have macros for variable attribute checking?
Havn't seen anything while skimming variable-related headers ...
take care,
Gerd
diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index f29a27e5a053..79c784f77ac8 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1688,6 +1688,7 @@ DxeImageVerificationHandler (
EFI_STATUS HashStatus;
EFI_STATUS DbStatus;
EFI_STATUS VarStatus;
+ UINT32 VarAttr;
BOOLEAN IsFound;
SignatureList = NULL;
@@ -1745,7 +1746,7 @@ DxeImageVerificationHandler (
}
SecureBootSize = sizeof (SecureBoot);
- VarStatus = gRT->GetVariable (EFI_SECURE_BOOT_MODE_NAME,
&gEfiGlobalVariableGuid, NULL, &SecureBootSize, &SecureBoot);
+ VarStatus = gRT->GetVariable (EFI_SECURE_BOOT_MODE_NAME,
&gEfiGlobalVariableGuid, &VarAttr, &SecureBootSize, &SecureBoot);
//
// Skip verification if SecureBoot variable doesn't exist.
//
@@ -1756,7 +1757,12 @@ DxeImageVerificationHandler (
//
// Skip verification if SecureBoot is disabled but not AuditMode
//
- if ((VarStatus == EFI_SUCCESS) && (SecureBoot == SECURE_BOOT_MODE_DISABLE)) {
+ if ((VarStatus == EFI_SUCCESS) &&
+ !(VarAttr & EFI_VARIABLE_NON_VOLATILE) &&
+ (VarAttr & EFI_VARIABLE_BOOTSERVICE_ACCESS) &&
+ (VarAttr & EFI_VARIABLE_RUNTIME_ACCESS) &&
+ (SecureBoot == SECURE_BOOT_MODE_DISABLE))
+ {
return EFI_SUCCESS;
}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100535): https://edk2.groups.io/g/devel/message/100535
Mute This Topic: https://groups.io/mt/97265237/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
