On 10/16/17 18:57, Zmuda, Wojciech wrote:
> Hello,
>
> I'd like to ask you for help with understanding Secure Boot policy
> mechanism, specifically DENY_EXECUTE_ON_SECURITY_VIOLATION for
> PcdOptionRomImageVerificationPolicy. The OptionROM in my setup is
> loaded while, in my opinion, it should be rejected.
>
> I'm testing the following scenario: Secure Boot is enabled with my own
> PK and KEK enrolled, but with no db, to make sure nothing unsigned or
> signed by somebody else but me can be executed. A PCIe card with
> OptionROM (some EBC code) is installed.
> gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy is
> set to 0x04 in my platform's package. What I expect, is the OptionROM
> execution being denied, as it is not signed by my certificate. What I
> observe, on the other hand, is a message on the console, that no EBC
> interpreter is found, which suggest, that the OptionROM is loaded,
> just fails at the execution of EBC. The same message is printed when
> Secure Boot is disabled.
>
> I tried to understand the code by stepping through it in the DS-5. The
> following part of
> SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> seems suspicious to me:
>
> SecurityStatus = gSecurity->FileAuthenticationState (
> gSecurity,
> AuthenticationStatus,
> OriginalFilePath
> );
> }
This code is not from "DxeImageVerificationLib.c"; it is from
CoreLoadImageCommon() in "MdeModulePkg/Core/Dxe/Image/Image.c". With
more context:
} else if ((gSecurity != NULL) && (OriginalFilePath != NULL)) {
//
// Verify the Authentication Status through the Security Architectural
Protocol
//
SecurityStatus = gSecurity->FileAuthenticationState (
gSecurity,
AuthenticationStatus,
OriginalFilePath
);
}
>
> //
> // Check Security Status.
> //
> if (EFI_ERROR (SecurityStatus) && SecurityStatus != EFI_SECURITY_VIOLATION)
> {
> if (SecurityStatus == EFI_ACCESS_DENIED) {
> //
> // Image was not loaded because the platform policy prohibits the image
> from being loaded.
> // It's the only place we could meet EFI_ACCESS_DENIED.
> //
> *ImageHandle = NULL;
> }
> Status = SecurityStatus;
> Image = NULL;
> goto Done;
> }
>
> The return code of gSecurity->FileAuthenticationState () (which is
> implemented in
> MdeModulePkg/Core/Dxe/Image/Image.c:DxeImageVerificationHandler ()),
> is EFI_SECURITY_VIOLATION. Such return code skips this if-statement,
> that prevents the image to be loaded. According to the comment in the
> if-statement: for the policy to be respected,
> gSecurity->FileAuthenticationState () should return EFI_ACCESS_DENIED.
No, that's a different kind of "platform policy".
The PCD that you mention is indeed platform policy, but only for
DxeImageVerificationLib. The platform policy being referred-to in this
comment is on the level of the gSecurity->FileAuthenticationState()
function call; and for that, we have to review the type definition of
the
EFI_SECURITY_ARCH_PROTOCOL.FileAuthenticationState
member function. The type name for this member function is
EFI_SECURITY_FILE_AUTHENTICATION_STATE, and it is defined in
"MdePkg/Include/Protocol/Security.h".
I will not quote the entire description from said header file, just the
part that explains the difference:
> @retval EFI_SECURITY_VIOLATION The file specified by File did not
> authenticate, and
> the platform policy dictates that File should
> be placed
> in the untrusted state. A file may be
> promoted from
> the untrusted to the trusted state at a
> future time
> with a call to the Trust() DXE Service.
> @retval EFI_ACCESS_DENIED The file specified by File did not
> authenticate, and
> the platform policy dictates that File should
> not be
> used for any purpose.
Back to your email:
On 10/16/17 18:57, Zmuda, Wojciech wrote:
> That being said, I stepped through DxeImageVerificationHandler (). The
> PCD with OptionROM policy is checked correctly. The function handles
> ALWAYS_EXECUTE and NEVER_EXECUTE policies properly and hangs on
> QUERY_USER_ and ALLOW_EXECUTE_ON_SECURITY_VIOLATION. This seems fine,
> however there is no code handling the
> DENY_EXECUTE_ON_SECURITY_VIOLATION (0x04) case. Stepping through this
> function shows that the image to be loaded cannot be found in the db
> (correct, as there's no db). Then, the function jumps to its very end
> and returns EFI_SECURITY_VIOLATION, which skips the aforementioned
> if-statement:
>
> Done:
> if (Status != EFI_SUCCESS) {
> //
> // Policy decides to defer or reject the image; add its information in
> image executable information table.
> //
> NameStr = ConvertDevicePathToText (File, FALSE, TRUE);
> AddImageExeInfo (Action, NameStr, File, SignatureList, SignatureListSize);
> if (NameStr != NULL) {
> DEBUG((EFI_D_INFO, "The image doesn't pass verification: %s\n",
> NameStr));
> FreePool(NameStr);
> }
> Status = EFI_SECURITY_VIOLATION;
> }
>
> if (SignatureList != NULL) {
> FreePool (SignatureList);
> }
>
> return Status;
> }
This quote is indeed from
"SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c",
and indeed the EFI_SECURITY_VIOLATION value returned by it causes the
"if" statement you quoted from "MdeModulePkg/Core/Dxe/Image/Image.c" to
be skipped.
However, that's not the whole story. In
"MdeModulePkg/Core/Dxe/Image/Image.c", in the CoreLoadImageCommon()
function, track the "SecurityStatus" variable from the point where the
EFI_ACCESS_DENIED branch is *not* taken, to the end of the function.
Near the end of the function, "Status" is EFI_SUCCESS, and thus
"SecurityStatus" will be assigned to it (and the function will
ultimately return EFI_SECURITY_VIOLATION):
Done:
//
// All done accessing the source file
// If we allocated the Source buffer, free it
//
if (FHand.FreeBuffer) {
CoreFreePool (FHand.Source);
}
if (OriginalFilePath != InputFilePath) {
CoreFreePool (OriginalFilePath);
}
//
// There was an error. If there's an Image structure, free it
//
if (EFI_ERROR (Status)) {
if (Image != NULL) {
CoreUnloadAndCloseImage (Image, (BOOLEAN)(DstBuffer == 0));
Image = NULL;
}
} else if (EFI_ERROR (SecurityStatus)) {
Status = SecurityStatus;
}
//
// Track the return status from LoadImage.
//
if (Image != NULL) {
Image->LoadImageStatus = Status;
}
return Status;
}
Back to your email:
On 10/16/17 18:57, Zmuda, Wojciech wrote:
> Is there anything I'm doing wrong trying to prevent untrusted
> OptionROM execution? If my understanding is correct, my case should
> make DxeImageVerificationHandler () return EFI_ACCESS_DENIED here. For
> example, in the snippet above, Status should be set to
> EFI_ACCESS_DENIED if Policy == DENY_EXECUTE_ON_SECURITY_VIOLATION.
To me it seems like everything is working by design. The image is loaded
and the security violation is recorded. Please see the documentation of
the EFI_SECURITY_VIOLATION return code in the UEFI-2.7 spec, under
EFI_BOOT_SERVICES.LoadImage():
Image was loaded and an ImageHandle was created with a valid
EFI_LOADED_IMAGE_PROTOCOL.. However, the current platform policy
specifies that the image should not be started.
(This section also has a pointer to "35.1.5 Deferred Execution".)
I believe the misunderstanding comes from the facts that
(a) the message
CoreLoadPeImage: There is no EBC interpreter for an EBC image.
is printed by the CoreLoadPeImage() function, and
(b) that CoreLoadImageCommon() calls CoreLoadPeImage() *between* the
EFI_ACCESS_DENIED check that you thought should have been taken -- which
is not the case -- and the final setting of Status to
EFI_SECURITY_VIOLATION (from "SecurityStatus"), which actually *would*
occur, if you had an EBC driver included in your platform firmware.
In other words, CoreLoadImageCommon() identifies (and prepares to
return) the EFI_SECURITY_VIOLATION status code, based on
PcdOptionRomImageVerificationPolicy. However, before the function could
finish processing (and return this error code), it comes across a more
severe error -- no EBC interpreter -- which takes priority both with and
without the Secure Boot operational mode.
If you want to be completely sure, I suggest including the EBC
interpreter in your platform, and retesting.
(Based on commit 81d9f86f8a71 ("ArmVirtPkg: enable EBC interpreter for
AArch64", 2016-07-29), adding the EBC interpreter should be easy.)
Again, to me it looks like everything is working by design.
Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
