Date   

Re: [PATCH EDK2 v2 1/1] SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

Yao, Jiewen
 

Apology that I did not say clearly.
I mean you should not modify any code to perform an attack.

I am not asking you to exploit the system. Attack here just means: to cause system hang or buffer overflow. That is enough.
But you cannot modify code to remove any existing checker.

Thank you
Yao Jiewen

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of wenyi,xie
via groups.io
Sent: Friday, August 28, 2020 2:18 PM
To: Yao, Jiewen <jiewen.yao@...>; devel@edk2.groups.io; Laszlo Ersek
<lersek@...>; Wang, Jian J <jian.j.wang@...>
Cc: songdongkuang@...; Mathews, John <john.mathews@...>
Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

Hi,Jiewen,

I don't really get the meaning "create a case that bypass all checks in PeCoffLib",
do you mean I need to create a PE file that can bypass all check in PeCoffLib
without modify any
code and then cause the problem in DxeImageVerificationLib, or just modify
some code in PeCoffLib to bypass check instead of removing the calling of
PeCoffLoaderGetImageInfo. Would
you mind explaining a little more specifically? As far as I tried, it's really hard to
reproduce the issue without touching any code.

Thanks
Wenyi

On 2020/8/28 11:50, Yao, Jiewen wrote:
HI Wenyi
Thank you very much to take time to reproduce.

I am particular interested in below:
"As PE file is modified, function PeCoffLoaderGetImageInfo will return
error, so I have to remove it so that for loop can be tested in
DxeImageVerificationHandler."

By design, the PeCoffLib should catch illegal PE/COFF image and return error.
(even it cannot catch all, it should catch most ones).
Other PE/COFF parser may rely on the checker in PeCoffLib and *no need* to
duplicate all checkers.
As such, DxeImageVerificationLib (and other PeCoff consumer) just need
checks what has passed the check in PeCoffLib.

I don’t think you should remove the checker. If people can remove the checker,
I am sure the rest code will be vulnerable, according to the current design.
Could you please to create a case that bypass all checks in PeCoffLib, then run
into DxeImageVerificationLib and cause the problem? That would be more
valuable for us.

Thank you
Yao Jiewen

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
wenyi,xie
via groups.io
Sent: Friday, August 28, 2020 11:18 AM
To: Laszlo Ersek <lersek@...>; Wang, Jian J
<jian.j.wang@...>;
devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@...>
Cc: songdongkuang@...; Mathews, John
<john.mathews@...>
Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

Hi,Laszlo and everyone,

These days I tried to reproduce the issue,and made some progress. I think
there are two way to cause overflow from a mathematical point of view,
1.As Laszlo analysed before, if WinCertificate->dwLength is large enough,
close
to MAX_UINT32, then (WinCertificate->dwLength + ALIGN_SIZE
(WinCertificate-
dwLength)) will cause overflow.
2.(WinCertificate->dwLength + ALIGN_SIZE (WinCertificate->dwLength)) is
good,
OffSet is good, but OffSet += (WinCertificate->dwLength + ALIGN_SIZE
(WinCertificate->dwLength)) cause overflow.

Here I choose the second way to reproduce the issue, I choose a PE file and
sign
it with my own db certificate. Then I use binary edit tool to modify the PE file
like
below,

1.change SecDataDir->Size from 0x5F8 to 0xFFFF1FFF
2.change WinCertificate->dwLength from 0x5F1 to 0xFFFF1FFE
SecDataDir->VirtualAddress in my PE is 0xe000 and no need to change.

As PE file is modified, function PeCoffLoaderGetImageInfo will return error,
so I
have to remove it so that for loop can be tested in
DxeImageVerificationHandler.
The log is as below,

SecDataDir->VirtualAddress=0xE000, SecDataDir->Size=0xFFFF1FFF.
(First Loop)
OffSet=0xE000.
WinCertificate->dwLength=0xFFFF1FFE, ALIGN_SIZE (WinCertificate-
dwLength)=0x2.
(Second Loop)
OffSet=0x0.
WinCertificate->dwLength=0x5A4D, ALIGN_SIZE (WinCertificate-
dwLength)=0x3.
(Third Loop)
OffSet=0x5A50.
WinCertificate->dwLength=0x9024, ALIGN_SIZE (WinCertificate-
dwLength)=0x4.
(Forth Loop)
OffSet=0xEA78.
WinCertificate->dwLength=0xAFAFAFAF, ALIGN_SIZE (WinCertificate-
dwLength)=0x1.
(Fifth Loop)
OffSet=0xAFB09A28.

As I modify SecDataDir->Size and WinCertificate->dwLength, so in first loop
the
condition check whether the Security Data Directory has enough room left
for
"WinCertificate->dwLength" is ok.

if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof
(WIN_CERTIFICATE) ||
(SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < WinCertificate-
dwLength) {
break;
}

In the beginning of second loop, WinCertificate->dwLength + ALIGN_SIZE
(WinCertificate->dwLength) is 0xFFFF2000, offset is 0xE000

OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-
dwLength))

Offset now is 0 and overflow happens. So even if my PE only have one
signature,
the for loop is still going untill exception happens.


I can't reproduce the issue using the first way, because if WinCertificate-
dwLength is close to MAX_UINT32, it means SecDataDir->Size should also
close
to MAX_UINT32, or the condition check
"(SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < WinCertificate-
dwLength" will break. But if SecDataDir->Size is very large, SecDataDir-
VirtualAddress have to be smaller than 8 bytes,
because SecDataDir->VirtualAddress + SecDataDir->Size < MAX_UINT32.
SecDataDir->VirtualAddress is the beginning address of the signature, and
before
SecDataDir->VirtualAddress is the content of origin PE file, I think it's
impossible
that the size of PE file is only 8 bytes.

As I changed the one line code in DxeImageVerificationHandler to reproduce
the
issue, I'm not sure whether it's ok.

Thanks
Wenyi

On 2020/8/19 17:26, Laszlo Ersek wrote:
On 08/18/20 17:18, Mathews, John wrote:
I dug up the original report details. This was noted as a concern during a
source code inspection. There was no demonstration of how it might be
triggered.

" There is an integer overflow vulnerability in the
DxeImageVerificationHandler function when
parsing the PE files attribute certificate table. In cases where
WinCertificate-
dwLength is
sufficiently large, it's possible to overflow Offset back to 0 causing an
endless
loop."

The recommendation was to add stricter checking of "Offset" and the
embedded length fields of certificate data
before using them.
Thanks for checking!

Laszlo




-----Original Message-----
From: Laszlo Ersek <lersek@...>
Sent: Tuesday, August 18, 2020 1:59 AM
To: Wang, Jian J <jian.j.wang@...>; devel@edk2.groups.io; Yao,
Jiewen <jiewen.yao@...>; xiewenyi2@...
Cc: huangming23@...; songdongkuang@...; Mathews,
John <john.mathews@...>
Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

On 08/18/20 04:10, Wang, Jian J wrote:
Laszlo,

My apologies for the slow response. I'm not the original reporter but
just the BZ submitter. And I didn't do deep analysis to this issue.
The issues was reported from one internal team. Add John in loop to see
if
he knows more about it or not.

My superficial understanding on such issue is that, if there's
"potential" issue in theory and hard to reproduce, it's still worthy
of using an alternative way to replace the original implementation
with no "potential" issue at all. Maybe we don't have to prove old way is
something wrong but must prove that the new way is really safe.

I agree, thanks.

It would be nice to hear more from the internal team about the originally
reported (even if hard-to-trigger) issue.

Thanks!
Laszlo


Regards,
Jian

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
Laszlo
Ersek
Sent: Tuesday, August 18, 2020 12:53 AM
To: Yao, Jiewen <jiewen.yao@...>; devel@edk2.groups.io;
xiewenyi2@...; Wang, Jian J <jian.j.wang@...>
Cc: huangming23@...; songdongkuang@...
Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

Hi Jiewen,

On 08/14/20 10:53, Yao, Jiewen wrote:
To Jiewen,
Sorry, I don't have environment to reproduce the issue.
Please help me understand, if you don’t have environment to
reproduce the
issue, how do you guarantee that your patch does fix the problem and
we don’t have any other vulnerabilities?

The original bug report in
<https://bugzilla.tianocore.org/show_bug.cgi?id=2215#c0> is seriously
lacking. It does not go into detail about the alleged integer overflow.
It does not quote the code, does not explain the control flow, does
not identify the exact edk2 commit at which the vulnerability exists.

The bug report also does not offer a reproducer.

Additionally, the exact statement that the bug report does make,
namely

it's possible to overflow Offset back to 0 causing an endless loop

is wrong (as far as I can tell anyway). It is not "OffSet" that can
be overflowed to zero, but the *addend* that is added to OffSet can
be overflowed to zero. Therefore the infinite loop will arise because
OffSet remains stuck at its present value, and not because OffSet
will be re-set to zero.

For the reasons, we can only speculate as to what the actual problem
is, unless Jian decides to join the discussion and clarifies what he
had in mind originally.

My understanding (or even "reconstruction") of the vulnerability is
described above, and in the patches that I proposed.

We can write a patch based on code analysis. It's possible to
identify integer overflows based on code analysis, and it's possible
to verify the correctness of fixes by code review. Obviously testing
is always good, but many times, constructing reproducers for such
issues that were found by code review, is difficult and time
consuming. We can say that we don't fix vulnerabilities without
reproducers, or we can say that we make an effort to fix them even if
all we have is code analysis (and not a reproducer).

So the above paragraph concerns "correctness". Regarding
"completeness", I guarantee you that this patch does not fix *all*
problems related to PE parsing. (See the other BZ tickets.) It does
fix *one* issue with PE parsing. We can say that we try to fix such
issues gradually (give different CVE numbers to different issues, and
address them one at a time), or we can say that we rewrite PE parsing
from the ground up.
(BTW: I have seriously attempted that in the past, and I gave up,
because the PE format is FUBAR.)

In summary:

- the problem statement is unclear,

- it seems like there is indeed an integer overflow problem in the
SecDataDir parsing loop, but it's uncertain whether the bug reporter
had exactly that in mind

- PE parsing is guaranteed to have other vulnerabilities elsewhere in
edk2, but I'm currently unaware of other such issues in
DxeImageVerificationLib specifically

- even if there are other such problems (in DxeImageVerificationLib
or elswehere), fixing this bug that we know about is likely
worthwhile

- for many such bugs, constructing a reproducer is difficult and time
consuming; code analysis, and *regression-testing* are frequently the
only tools we have. That doesn't mean we should ignore this class of
bugs.

(Fixing integer overflows retro-actively is more difficult than
writing overflow-free code in the first place, but that ship has
sailed; so we can only fight these bugs incrementally now, unless we
can rewrite PE parsing with a new data structure from the ground up.
Again I tried that and gave up, because the spec is not public, and
what I did manage to learn about PE, showed that it was insanely
over-engineered. I'm not saying that other binary / executable
formats are better, of course.)

Please check out my patches (inlined elsewhere in this thread), and
comment whether you'd like me to post them to the list as a
standalone series.

Jian: it wouldn't hurt if you commented as well.

Thanks
Laszlo

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
wenyi,xie
via groups.io
Sent: Friday, August 14, 2020 3:54 PM
To: Laszlo Ersek <lersek@...>; devel@edk2.groups.io; Yao,
Jiewen <jiewen.yao@...>; Wang, Jian J
<jian.j.wang@...>
Cc: huangming23@...; songdongkuang@...
Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

To Laszlo,
Thank you for your detailed description, I agree with what you
analyzed and
I'm
OK with your patches, it's
correct and much simpler.

To Jiewen,
Sorry, I don't have environment to reproduce the issue.

Thanks
Wenyi

On 2020/8/14 2:50, Laszlo Ersek wrote:
On 08/13/20 13:55, Wenyi Xie wrote:
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2215

There is an integer overflow vulnerability in
DxeImageVerificationHandler function when parsing the PE files
attribute certificate table. In cases where
WinCertificate->dwLength is sufficiently large, it's possible to
overflow Offset back to 0 causing an endless loop.

Check offset inbetween VirtualAddress and VirtualAddress + Size.
Using SafeintLib to do offset addition with result check.

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Laszlo Ersek <lersek@...>
Signed-off-by: Wenyi Xie <xiewenyi2@...>
---

SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.inf
|
1 +

SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.h
|
1 +

SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
|
111 +++++++++++---------
3 files changed, 63 insertions(+), 50 deletions(-)

diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.inf
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.inf
index 1e1a639857e0..a7ac4830b3d4 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.inf
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.inf
@@ -53,6 +53,7 @@ [LibraryClasses]
SecurityManagementLib
PeCoffLib
TpmMeasurementLib
+ SafeIntLib

[Protocols]
gEfiFirmwareVolume2ProtocolGuid ##
SOMETIMES_CONSUMES
diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.h
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.h
index 17955ff9774c..060273917d5d 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.h
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.h
@@ -23,6 +23,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/DevicePathLib.h> #include
<Library/SecurityManagementLib.h> #include <Library/PeCoffLib.h>
+#include <Library/SafeIntLib.h>
#include <Protocol/FirmwareVolume2.h> #include
<Protocol/DevicePath.h> #include <Protocol/BlockIo.h> diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
index 36b87e16d53d..dbc03e28c05b 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib
.c
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
@@ -1658,6 +1658,10 @@ DxeImageVerificationHandler (
EFI_STATUS HashStatus;
EFI_STATUS DbStatus;
BOOLEAN IsFound;
+ UINT32 AlignedLength;
+ UINT32 Result;
+ EFI_STATUS AddStatus;
+ BOOLEAN IsAuthDataAssigned;

SignatureList = NULL;
SignatureListSize = 0;
@@ -1667,6 +1671,7 @@ DxeImageVerificationHandler (
Action = EFI_IMAGE_EXECUTION_AUTH_UNTESTED;
IsVerified = FALSE;
IsFound = FALSE;
+ Result = 0;

//
// Check the image type and get policy setting.
@@ -1850,9 +1855,10 @@ DxeImageVerificationHandler (
// The first certificate starts at offset
(SecDataDir->VirtualAddress) from
the
start of the file.
//
for (OffSet = SecDataDir->VirtualAddress;
- OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
- OffSet += (WinCertificate->dwLength + ALIGN_SIZE
(WinCertificate-
dwLength))) {
+ (OffSet >= SecDataDir->VirtualAddress) && (OffSet <
+ (SecDataDir-
VirtualAddress + SecDataDir->Size));) {
+ IsAuthDataAssigned = FALSE;
WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
+ AlignedLength = WinCertificate->dwLength + ALIGN_SIZE
(WinCertificate-
dwLength);

I disagree with this patch.

The primary reason for my disagreement is that the bug report
<https://bugzilla.tianocore.org/show_bug.cgi?id=2215#c0> is
inexact, and so this patch tries to fix the wrong thing.

With edk2 master at commit 65904cdbb33c, it is *not* possible to
overflow the OffSet variable to zero with "WinCertificate-
dwLength"
*purely*, and cause an endless loop. Note that we have (at commit
65904cdbb33c):

for (OffSet = SecDataDir->VirtualAddress;
OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
OffSet += (WinCertificate->dwLength + ALIGN_SIZE
(WinCertificate-
dwLength))) {
WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet)
<= sizeof
(WIN_CERTIFICATE) ||
(SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <
WinCertificate-
dwLength) {
break;
}

The last sub-condition checks whether the Security Data Directory
has enough room left for "WinCertificate->dwLength". If not, then
we break out of the loop.

If we *do* have enough room, that is:

(SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) >=
WinCertificate-
dwLength

then we have (by adding OffSet to both sides):

SecDataDir->VirtualAddress + SecDataDir->Size >= OffSet +
WinCertificate- dwLength

The left hand side is a known-good UINT32, and so incrementing
OffSet (a
UINT32) *solely* by "WinCertificate->dwLength" (also a UINT32)
does not cause an overflow.

Instead, the problem is with the alignment. The "if" statement
checks whether we have enough room for "dwLength", but then
"OffSet" is advanced by "dwLength" *aligned up* to the next
multiple of 8. And that may indeed cause various overflows.

Now, the main problem with the present patch is that it does not
fix one of those overflows. Namely, consider that "dwLength" is
very close to
MAX_UINT32 (or even think it's exactly MAX_UINT32). Then aligning
it up to the next multiple of 8 will yield 0. In other words,
"AlignedLength"
will be zero.

And when that happens, there's going to be an infinite loop just
the
same: "OffSet" will not be zero, but it will be *stuck*. The
SafeUint32Add() call at the bottom will succeed, but it will not
change the value of "OffSet".

More at the bottom.


if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet)
<= sizeof
(WIN_CERTIFICATE) ||
(SecDataDir->VirtualAddress + SecDataDir->Size - OffSet)
<
WinCertificate->dwLength) {
break;
@@ -1872,6 +1878,8 @@ DxeImageVerificationHandler (
}
AuthData = PkcsCertData->CertData;
AuthDataSize = PkcsCertData->Hdr.dwLength -
sizeof(PkcsCertData-
Hdr);
+ IsAuthDataAssigned = TRUE;
+ HashStatus = HashPeImageByType (AuthData, AuthDataSize);
} else if (WinCertificate->wCertificateType ==
WIN_CERT_TYPE_EFI_GUID)
{
//
// The certificate is formatted as
WIN_CERTIFICATE_UEFI_GUID which
is
described in UEFI Spec.
@@ -1880,72 +1888,75 @@ DxeImageVerificationHandler (
if (WinCertUefiGuid->Hdr.dwLength <=
OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData)) {
break;
}
- if (!CompareGuid (&WinCertUefiGuid->CertType,
&gEfiCertPkcs7Guid))
{
- continue;
+ if (CompareGuid (&WinCertUefiGuid->CertType,
+ &gEfiCertPkcs7Guid))
{
+ AuthData = WinCertUefiGuid->CertData;
+ AuthDataSize = WinCertUefiGuid->Hdr.dwLength -
OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData);
+ IsAuthDataAssigned = TRUE;
+ HashStatus = HashPeImageByType (AuthData, AuthDataSize);
}
- AuthData = WinCertUefiGuid->CertData;
- AuthDataSize = WinCertUefiGuid->Hdr.dwLength -
OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData);
} else {
if (WinCertificate->dwLength < sizeof (WIN_CERTIFICATE)) {
break;
}
- continue;
}

- HashStatus = HashPeImageByType (AuthData, AuthDataSize);
- if (EFI_ERROR (HashStatus)) {
- continue;
- }
-
- //
- // Check the digital signature against the revoked certificate in
forbidden
database (dbx).
- //
- if (IsForbiddenByDbx (AuthData, AuthDataSize)) {
- Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED;
- IsVerified = FALSE;
- break;
- }
-
- //
- // Check the digital signature against the valid certificate in
allowed
database (db).
- //
- if (!IsVerified) {
- if (IsAllowedByDb (AuthData, AuthDataSize)) {
- IsVerified = TRUE;
+ if (IsAuthDataAssigned && !EFI_ERROR (HashStatus)) {
+ //
+ // Check the digital signature against the revoked
+ certificate in
forbidden
database (dbx).
+ //
+ if (IsForbiddenByDbx (AuthData, AuthDataSize)) {
+ Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED;
+ IsVerified = FALSE;
+ break;
}
- }

- //
- // Check the image's hash value.
- //
- DbStatus = IsSignatureFoundInDatabase (
- EFI_IMAGE_SECURITY_DATABASE1,
- mImageDigest,
- &mCertType,
- mImageDigestSize,
- &IsFound
- );
- if (EFI_ERROR (DbStatus) || IsFound) {
- Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND;
- DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is
signed
but %s
hash of image is found in DBX.\n", mHashTypeStr));
- IsVerified = FALSE;
- break;
- }
+ //
+ // Check the digital signature against the valid
+ certificate in allowed
database (db).
+ //
+ if (!IsVerified) {
+ if (IsAllowedByDb (AuthData, AuthDataSize)) {
+ IsVerified = TRUE;
+ }
+ }

- if (!IsVerified) {
+ //
+ // Check the image's hash value.
+ //
DbStatus = IsSignatureFoundInDatabase (
- EFI_IMAGE_SECURITY_DATABASE,
+ EFI_IMAGE_SECURITY_DATABASE1,
mImageDigest,
&mCertType,
mImageDigestSize,
&IsFound
);
- if (!EFI_ERROR (DbStatus) && IsFound) {
- IsVerified = TRUE;
- } else {
- DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is
signed
but
signature is not allowed by DB and %s hash of image is not found in
DB/DBX.\n",
mHashTypeStr));
+ if (EFI_ERROR (DbStatus) || IsFound) {
+ Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND;
+ DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is
+ signed
but %s hash of image is found in DBX.\n", mHashTypeStr));
+ IsVerified = FALSE;
+ break;
}
+
+ if (!IsVerified) {
+ DbStatus = IsSignatureFoundInDatabase (
+ EFI_IMAGE_SECURITY_DATABASE,
+ mImageDigest,
+ &mCertType,
+ mImageDigestSize,
+ &IsFound
+ );
+ if (!EFI_ERROR (DbStatus) && IsFound) {
+ IsVerified = TRUE;
+ } else {
+ DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is
+ signed
but
signature is not allowed by DB and %s hash of image is not found in
DB/DBX.\n",
mHashTypeStr));
+ }
+ }
+ }
+
+ AddStatus = SafeUint32Add (OffSet, AlignedLength, &Result);
+ if (EFI_ERROR (AddStatus)) {
+ break;
}
+ OffSet = Result;
}

if (OffSet != (SecDataDir->VirtualAddress + SecDataDir->Size))
{
There are other (smaller) reasons why I dislike this patch:

- The "IsAuthDataAssigned" variable is superfluous; we could use
the existent "AuthData" variable (with a NULL-check and a
NULL-assignment) similarly.

- The patch complicates / reorganizes the control flow needlessly.
This complication originates from placing the checked "OffSet"
increment at the bottom of the loop, which then requires the
removal of all the "continue" statements. But we don't need to
check-and-increment at the bottom. We can keep the increment
inside the "for" statement, only extend the *existent* room check
(which I've quoted) to take the alignment into account as well. If
there is enough room for the alignment in the security data
directory, then that guarantees there won't be a UINT32 overflow
either.

All in all, I'm proposing the following three patches instead. The
first two patches are preparation, the last patch is the fix.

Patch#1:

From 11af0a104d34d39bf1b1aab256428ae4edbddd77 Mon Sep 17
00:00:00
2001
From: Laszlo Ersek <lersek@...>
Date: Thu, 13 Aug 2020 19:11:39 +0200
Subject: [PATCH 1/3] SecurityPkg/DxeImageVerificationLib: extract
SecDataDirEnd, SecDataDirLeft

The following two quantities:

SecDataDir->VirtualAddress + SecDataDir->Size
SecDataDir->VirtualAddress + SecDataDir->Size - OffSet

are used multiple times in DxeImageVerificationHandler().
Introduce helper variables for them: "SecDataDirEnd" and
"SecDataDirLeft", respectively.
This saves us multiple calculations and significantly simplifies the
code.

Note that all three summands above have type UINT32, therefore
the new variables are also of type UINT32.

This patch does not change behavior.

(Note that the code already handles the case when the

SecDataDir->VirtualAddress + SecDataDir->Size

UINT32 addition overflows -- namely, in that case, the
certificate loop is never entered, and the corruption check right
after the loop fires.)

Signed-off-by: Laszlo Ersek <lersek@...>
---

SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c |
12
++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
index 36b87e16d53d..8761980c88aa 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib
.c
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
@@ -1652,6 +1652,8 @@ DxeImageVerificationHandler (
UINT8 *AuthData;
UINTN AuthDataSize;
EFI_IMAGE_DATA_DIRECTORY *SecDataDir;
+ UINT32 SecDataDirEnd;
+ UINT32 SecDataDirLeft;
UINT32 OffSet;
CHAR16 *NameStr;
RETURN_STATUS PeCoffStatus;
@@ -1849,12 +1851,14 @@ DxeImageVerificationHandler (
// "Attribute Certificate Table".
// The first certificate starts at offset
(SecDataDir->VirtualAddress) from
the
start of the file.
//
+ SecDataDirEnd = SecDataDir->VirtualAddress + SecDataDir->Size;
for (OffSet = SecDataDir->VirtualAddress;
- OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
+ OffSet < SecDataDirEnd;
OffSet += (WinCertificate->dwLength + ALIGN_SIZE
(WinCertificate-
dwLength))) {
WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
- if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <=
sizeof
(WIN_CERTIFICATE) ||
- (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <
WinCertificate->dwLength) {
+ SecDataDirLeft = SecDataDirEnd - OffSet;
+ if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE) ||
+ SecDataDirLeft < WinCertificate->dwLength) {
break;
}

@@ -1948,7 +1952,7 @@ DxeImageVerificationHandler (
}
}

- if (OffSet != (SecDataDir->VirtualAddress + SecDataDir->Size))
{
+ if (OffSet != SecDataDirEnd) {
//
// The Size in Certificate Table or the attribute
certificate table is
corrupted.
//
--
2.19.1.3.g30247aa5d201
Patch#2:

From 72012c065a53582f7df695e7b9730c45f49226c6 Mon Sep 17
00:00:00
2001
From: Laszlo Ersek <lersek@...>
Date: Thu, 13 Aug 2020 19:19:06 +0200
Subject: [PATCH 2/3] SecurityPkg/DxeImageVerificationLib: assign
WinCertificate after size check

Currently the (SecDataDirLeft <= sizeof (WIN_CERTIFICATE)) check
only guards the de-referencing of the "WinCertificate" pointer.
It does not guard the calculation of hte pointer itself:

WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);

This is wrong; if we don't know for sure that we have enough room
for a WIN_CERTIFICATE, then even creating such a pointer, not
just de-referencing it, may invoke undefined behavior.

Move the pointer calculation after the size check.

Signed-off-by: Laszlo Ersek <lersek@...>
---

SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c |
8
+++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
index 8761980c88aa..461ed7cfb5ac 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib
.c
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
@@ -1855,10 +1855,12 @@ DxeImageVerificationHandler (
for (OffSet = SecDataDir->VirtualAddress;
OffSet < SecDataDirEnd;
OffSet += (WinCertificate->dwLength + ALIGN_SIZE
(WinCertificate-
dwLength))) {
- WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
SecDataDirLeft = SecDataDirEnd - OffSet;
- if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE) ||
- SecDataDirLeft < WinCertificate->dwLength) {
+ if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE)) {
+ break;
+ }
+ WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
+ if (SecDataDirLeft < WinCertificate->dwLength) {
break;
}

--
2.19.1.3.g30247aa5d201
Patch#3:

From 0bbba15b84f8f9f2cdc770a89f418aaec6cfb31e Mon Sep 17
00:00:00
2001
From: Laszlo Ersek <lersek@...>
Date: Thu, 13 Aug 2020 19:34:33 +0200
Subject: [PATCH 3/3] SecurityPkg/DxeImageVerificationLib: catch
alignment
overflow (CVE-2019-14562)

The DxeImageVerificationHandler() function currently checks
whether "SecDataDir" has enough room for
"WinCertificate->dwLength". However,
for
advancing "OffSet", "WinCertificate->dwLength" is aligned to the
next multiple of 8. If "WinCertificate->dwLength" is large
enough, the alignment will return 0, and "OffSet" will be stuck at
the
same value.

Check whether "SecDataDir" has room left for both
"WinCertificate->dwLength" and the alignment.

Signed-off-by: Laszlo Ersek <lersek@...>
---

SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c |
4
+++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
index 461ed7cfb5ac..e38eb981b7a0 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib
.c
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
@@ -1860,7 +1860,9 @@ DxeImageVerificationHandler (
break;
}
WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
- if (SecDataDirLeft < WinCertificate->dwLength) {
+ if (SecDataDirLeft < WinCertificate->dwLength ||
+ (SecDataDirLeft - WinCertificate->dwLength <
+ ALIGN_SIZE (WinCertificate->dwLength))) {
break;
}

--
2.19.1.3.g30247aa5d201
If Wenyi and the reviewers are OK with these patches, I can submit
them as a standalone patch series.

Note that I do not have any reproducer for the issue; the best
testing that I could offer would be some light-weight Secure Boot
regression tests.

Thanks
Laszlo


.



.



Re: [PATCH] MdeModulePkg/Library: change TpmMeasurementLibNull to BASE library.

Qi Zhang
 

Hi, Jian & Hao

Could you please review this change as well? Thanks!

Qi Zhang

-----Original Message-----
From: Yao, Jiewen <jiewen.yao@...>
Sent: Friday, August 28, 2020 2:17 PM
To: Zhang, Qi1 <qi1.zhang@...>; devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>
Subject: RE: [PATCH] MdeModulePkg/Library: change TpmMeasurementLibNull
to BASE library.

Reviewed-by: Jiewen Yao <Jiewen.yao@...>

-----Original Message-----
From: Zhang, Qi1 <qi1.zhang@...>
Sent: Friday, August 28, 2020 2:15 PM
To: devel@edk2.groups.io
Cc: Zhang, Qi1 <qi1.zhang@...>; Wang, Jian J
<jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>; Yao, Jiewen
<jiewen.yao@...>
Subject: [PATCH] MdeModulePkg/Library: change TpmMeasurementLibNull
to
BASE library.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2940

TpmMeasurementLib includes DxeTpmMeasurementLib and
PeiTpmMeasurementLib.
So need to change TpmMeasurementLibNull to BASE library to avoid build
error in some platform.

Signed-off-by: Qi Zhang <qi1.zhang@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Jiewen Yao <jiewen.yao@...>
---
.../Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c | 4 +++-
.../Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf | 6
+++---
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git
a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.
c
b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.
c
index b9c5b68de8..ee3be62fc6 100644
---
a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.
c
+++
b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.
c
@@ -1,11 +1,13 @@
/** @file

This library is used by other modules to measure data to TPM.



-Copyright (c) 2015, Intel Corporation. All rights reserved. <BR>

+Copyright (c) 2015-2020, Intel Corporation. All rights reserved. <BR>

SPDX-License-Identifier: BSD-2-Clause-Patent



**/



+#include <Uefi/UefiBaseType.h>

+

/**

Tpm measure and log data, and extend the measurement result into a
specific PCR.



diff --git
a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.i
n
f
b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.i
n
f
index 61abcfa2ec..1db2c0d6a7 100644
---
a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.i
n
f
+++
b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.i
n
f
@@ -1,7 +1,7 @@
## @file

# Provides NULL TPM measurement function.

#

-# Copyright (c) 2015 - 2018, Intel Corporation. All rights
reserved.<BR>

+# Copyright (c) 2015 - 2020, Intel Corporation. All rights
+reserved.<BR>

# SPDX-License-Identifier: BSD-2-Clause-Patent

#

##

@@ -10,9 +10,9 @@
INF_VERSION = 0x00010005

BASE_NAME = TpmMeasurementLibNull

FILE_GUID = 6DFD6E9F-9278-48D8-8F45-B6CFF2C2B69C

- MODULE_TYPE = UEFI_DRIVER

+ MODULE_TYPE = BASE

VERSION_STRING = 1.0

- LIBRARY_CLASS = TpmMeasurementLib|DXE_DRIVER
DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER

+ LIBRARY_CLASS = TpmMeasurementLib

MODULE_UNI_FILE = TpmMeasurementLibNull.uni



#

--
2.26.2.windows.1


Re: [PATCH 2/3] Platform/Intel/KabylakeOpenBoardPkg: add ibrary for Fsp measurement.

Chiu, Chasel
 

Reviewed-by: Chasel Chiu <chasel.chiu@...>

-----Original Message-----
From: Zhang, Qi1 <qi1.zhang@...>
Sent: Friday, August 28, 2020 2:33 PM
To: devel@edk2.groups.io
Cc: Zhang, Qi1 <qi1.zhang@...>; Chiu, Chasel <chasel.chiu@...>;
Desimone, Nathaniel L <nathaniel.l.desimone@...>; Jeremy Soller
<jeremy@...>
Subject: [PATCH 2/3] Platform/Intel/KabylakeOpenBoardPkg: add ibrary for
Fsp measurement.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2939

Signed-off-by: Qi Zhang <qi1.zhang@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Jeremy Soller <jeremy@...>
---
Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc | 2
++
.../Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc | 2
++
2 files changed, 4 insertions(+)

diff --git
a/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc
b/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc
index 862e6a6655..34d645be7e 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc
+++ b/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc
@@ -172,6 +172,8 @@
!if $(TARGET) == DEBUG


TestPointCheckLib|$(PLATFORM_PACKAGE)/Test/Library/TestPointCheckLib/P
eiTestPointCheckLib.inf

!endif

+
FspMeasurementLib|IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/
BaseFspMeasurementLib.inf

+
TcgEventLogRecordLib|SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLo
gRecordLib.inf



#######################################

# Board Package

diff --git
a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc
b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc
index 0b30da8f96..fdfaaa0cda 100644
---
a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc
+++
b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc
@@ -213,6 +213,8 @@
!endif


SetCacheMtrrLib|$(PLATFORM_PACKAGE)/Library/SetCacheMtrrLib/SetCache
MtrrLibNull.inf


ReportCpuHobLib|$(PLATFORM_PACKAGE)/PlatformInit/Library/ReportCpuH
obLib/ReportCpuHobLib.inf

+
FspMeasurementLib|IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/
BaseFspMeasurementLib.inf

+
TcgEventLogRecordLib|SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLo
gRecordLib.inf



#######################################

# Board Package

--
2.26.2.windows.1


Re: [PATCH 0/3] add ibrary for Fsp measurement to OpenBoardPkg.

Qi Zhang
 

Hi, Liming

I also request these serial patches to catch stable tag 202008. Thanks!

-----Original Message-----
From: Zhang, Qi1 <qi1.zhang@...>
Sent: Friday, August 28, 2020 2:33 PM
To: devel@edk2.groups.io
Cc: Zhang, Qi1 <qi1.zhang@...>; Chiu, Chasel <chasel.chiu@...>;
Yao, Jiewen <jiewen.yao@...>; Desimone, Nathaniel L
<nathaniel.l.desimone@...>; Chaganty, Rangasai V
<rangasai.v.chaganty@...>; Kethi Reddy, Deepika
<deepika.kethi.reddy@...>; Esakkithevar, Kathappan
<kathappan.esakkithevar@...>; Jeremy Soller <jeremy@...>
Subject: [PATCH 0/3] add ibrary for Fsp measurement to OpenBoardPkg.

These patches also depends on one fix of edk2:
https://bugzilla.tianocore.org/show_bug.cgi?id=2939.

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Cc: Deepika Kethi Reddy <deepika.kethi.reddy@...>
Cc: Kathappan Esakkithevar <kathappan.esakkithevar@...>
Cc: Jeremy Soller <jeremy@...>

Qi Zhang (3):
Platform/Intel/CometlakeOpenBoardPkg: add ibrary for Fsp measurement.
Platform/Intel/KabylakeOpenBoardPkg: add ibrary for Fsp measurement.
Platform/Intel/WhiskeylakeOpenBoardPkg: add ibrary for Fsp
measurement.

.../Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.dsc | 2 ++
Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc | 2 ++
.../Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc | 2 ++
.../Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc | 2 ++
.../WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.dsc | 2 ++
5 files changed, 10 insertions(+)

--
2.26.2.windows.1


Re: [PATCH 3/3] Platform/Intel/WhiskeylakeOpenBoardPkg: add ibrary for Fsp measurement.

Chiu, Chasel
 

Reviewed-by: Chasel Chiu <chasel.chiu@...>

-----Original Message-----
From: Zhang, Qi1 <qi1.zhang@...>
Sent: Friday, August 28, 2020 2:33 PM
To: devel@edk2.groups.io
Cc: Zhang, Qi1 <qi1.zhang@...>; Chiu, Chasel <chasel.chiu@...>;
Desimone, Nathaniel L <nathaniel.l.desimone@...>
Subject: [PATCH 3/3] Platform/Intel/WhiskeylakeOpenBoardPkg: add ibrary
for Fsp measurement.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2939

Signed-off-by: Qi Zhang <qi1.zhang@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
---
.../Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc | 2
++
.../WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.dsc | 2
++
2 files changed, 4 insertions(+)

diff --git
a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc
b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc
index fb493973e2..ab02a2ef59 100644
---
a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc
+++
b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc
@@ -173,6 +173,8 @@
!endif


SetCacheMtrrLib|$(PLATFORM_PACKAGE)/Library/SetCacheMtrrLib/SetCache
MtrrLibNull.inf


ReportCpuHobLib|$(PLATFORM_PACKAGE)/PlatformInit/Library/ReportCpuH
obLib/ReportCpuHobLib.inf

+
FspMeasurementLib|IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/
BaseFspMeasurementLib.inf

+
TcgEventLogRecordLib|SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLo
gRecordLib.inf



#######################################

# Board Package

diff --git
a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardP
kg.dsc
b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardP
kg.dsc
index 9a1f107faf..0a87a3d4b2 100644
---
a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardP
kg.dsc
+++
b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardP
kg.dsc
@@ -173,6 +173,8 @@
!endif


SetCacheMtrrLib|$(PLATFORM_PACKAGE)/Library/SetCacheMtrrLib/SetCache
MtrrLibNull.inf


ReportCpuHobLib|$(PLATFORM_PACKAGE)/PlatformInit/Library/ReportCpuH
obLib/ReportCpuHobLib.inf

+
FspMeasurementLib|IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/
BaseFspMeasurementLib.inf

+
TcgEventLogRecordLib|SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLo
gRecordLib.inf



#######################################

# Board Package

--
2.26.2.windows.1


Re: [PATCH 1/3] Platform/Intel/CometlakeOpenBoardPkg: add ibrary for Fsp measurement.

Chiu, Chasel
 

Reviewed-by: Chasel Chiu <chasel.chiu@...>

-----Original Message-----
From: Zhang, Qi1 <qi1.zhang@...>
Sent: Friday, August 28, 2020 2:33 PM
To: devel@edk2.groups.io
Cc: Zhang, Qi1 <qi1.zhang@...>; Chiu, Chasel <chasel.chiu@...>;
Desimone, Nathaniel L <nathaniel.l.desimone@...>; Chaganty,
Rangasai V <rangasai.v.chaganty@...>; Kethi Reddy, Deepika
<deepika.kethi.reddy@...>; Esakkithevar, Kathappan
<kathappan.esakkithevar@...>
Subject: [PATCH 1/3] Platform/Intel/CometlakeOpenBoardPkg: add ibrary for
Fsp measurement.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2939

Signed-off-by: Qi Zhang <qi1.zhang@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Cc: Deepika Kethi Reddy <deepika.kethi.reddy@...>
Cc: Kathappan Esakkithevar <kathappan.esakkithevar@...>
---
.../Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.dsc | 2
++
1 file changed, 2 insertions(+)

diff --git
a/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.d
sc
b/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.d
sc
index 2d9dcb139f..4ea797c550 100644
---
a/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.d
sc
+++
b/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.d
sc
@@ -173,6 +173,8 @@
!endif


SetCacheMtrrLib|$(PLATFORM_PACKAGE)/Library/SetCacheMtrrLib/SetCache
MtrrLibNull.inf


ReportCpuHobLib|$(PLATFORM_PACKAGE)/PlatformInit/Library/ReportCpuH
obLib/ReportCpuHobLib.inf

+
FspMeasurementLib|IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/
BaseFspMeasurementLib.inf

+
TcgEventLogRecordLib|SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLo
gRecordLib.inf



#######################################

# Board Package

--
2.26.2.windows.1


Re: [PATCH] MdeModulePkg/Library: change TpmMeasurementLibNull to BASE library.

Qi Zhang
 

Yes. This fix is for build error of intel OpenBoardPkg in edk2-platform .

-----Original Message-----
From: gaoliming <gaoliming@...>
Sent: Friday, August 28, 2020 2:31 PM
To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@...>; Zhang, Qi1
<qi1.zhang@...>
Cc: Wang, Jian J <jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>
Subject: 回复: [edk2-devel] [PATCH] MdeModulePkg/Library: change
TpmMeasurementLibNull to BASE library.

Qi:
This is a bug fix. Do you request to catch it into this stable tag 202008?

Thanks
Liming
-----邮件原件-----
发件人: bounce+27952+64729+4905953+8761045@groups.io
<bounce+27952+64729+4905953+8761045@groups.io> 代表 Yao, Jiewen
发送时间: 2020年8月28日 14:17
收件人: Zhang, Qi1 <qi1.zhang@...>; devel@edk2.groups.io
抄送: Wang, Jian J <jian.j.wang@...>; Wu, Hao A
<hao.a.wu@...>
主题: Re: [edk2-devel] [PATCH] MdeModulePkg/Library: change
TpmMeasurementLibNull to BASE library.

Reviewed-by: Jiewen Yao <Jiewen.yao@...>

-----Original Message-----
From: Zhang, Qi1 <qi1.zhang@...>
Sent: Friday, August 28, 2020 2:15 PM
To: devel@edk2.groups.io
Cc: Zhang, Qi1 <qi1.zhang@...>; Wang, Jian J
<jian.j.wang@...>;
Wu, Hao A <hao.a.wu@...>; Yao, Jiewen <jiewen.yao@...>
Subject: [PATCH] MdeModulePkg/Library: change TpmMeasurementLibNull
to
BASE library.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2940

TpmMeasurementLib includes DxeTpmMeasurementLib and
PeiTpmMeasurementLib.
So need to change TpmMeasurementLibNull to BASE library to avoid
build error in some platform.

Signed-off-by: Qi Zhang <qi1.zhang@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Jiewen Yao <jiewen.yao@...>
---
.../Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c | 4
+++-
.../Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf | 6
+++---
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git
a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu
ll.c
b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu
ll.c
index b9c5b68de8..ee3be62fc6 100644
---
a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu
ll.c
+++
b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu
ll.c
@@ -1,11 +1,13 @@
/** @file

This library is used by other modules to measure data to TPM.



-Copyright (c) 2015, Intel Corporation. All rights reserved. <BR>

+Copyright (c) 2015-2020, Intel Corporation. All rights reserved.
+<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent



**/



+#include <Uefi/UefiBaseType.h>

+

/**

Tpm measure and log data, and extend the measurement result into
a
specific
PCR.



diff --git
a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu
ll.in
f
b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu
ll.in
f
index 61abcfa2ec..1db2c0d6a7 100644
---
a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu
ll.in
f
+++
b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu
ll.in
f
@@ -1,7 +1,7 @@
## @file

# Provides NULL TPM measurement function.

#

-# Copyright (c) 2015 - 2018, Intel Corporation. All rights
reserved.<BR>

+# Copyright (c) 2015 - 2020, Intel Corporation. All rights
reserved.<BR>

# SPDX-License-Identifier: BSD-2-Clause-Patent

#

##

@@ -10,9 +10,9 @@
INF_VERSION = 0x00010005

BASE_NAME = TpmMeasurementLibNull

FILE_GUID =
6DFD6E9F-9278-48D8-8F45-B6CFF2C2B69C

- MODULE_TYPE = UEFI_DRIVER

+ MODULE_TYPE = BASE

VERSION_STRING = 1.0

- LIBRARY_CLASS =
TpmMeasurementLib|DXE_DRIVER
DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION
UEFI_DRIVER

+ LIBRARY_CLASS = TpmMeasurementLib

MODULE_UNI_FILE = TpmMeasurementLibNull.uni



#

--
2.26.2.windows.1


[PATCH 3/3] Platform/Intel/WhiskeylakeOpenBoardPkg: add ibrary for Fsp measurement.

Qi Zhang
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2939

Signed-off-by: Qi Zhang <qi1.zhang@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
---
.../Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc | 2 ++
.../WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.dsc | 2 ++
2 files changed, 4 insertions(+)

diff --git a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.d=
sc b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc
index fb493973e2..ab02a2ef59 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc
@@ -173,6 +173,8 @@
!endif=0D
SetCacheMtrrLib|$(PLATFORM_PACKAGE)/Library/SetCacheMtrrLib/SetCacheMtrr=
LibNull.inf=0D
ReportCpuHobLib|$(PLATFORM_PACKAGE)/PlatformInit/Library/ReportCpuHobLib=
/ReportCpuHobLib.inf=0D
+ FspMeasurementLib|IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/Base=
FspMeasurementLib.inf=0D
+ TcgEventLogRecordLib|SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLo=
gRecordLib.inf=0D
=0D
#######################################=0D
# Board Package=0D
diff --git a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoa=
rdPkg.dsc b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoar=
dPkg.dsc
index 9a1f107faf..0a87a3d4b2 100644
--- a/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.d=
sc
+++ b/Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.d=
sc
@@ -173,6 +173,8 @@
!endif=0D
SetCacheMtrrLib|$(PLATFORM_PACKAGE)/Library/SetCacheMtrrLib/SetCacheMtrr=
LibNull.inf=0D
ReportCpuHobLib|$(PLATFORM_PACKAGE)/PlatformInit/Library/ReportCpuHobLib=
/ReportCpuHobLib.inf=0D
+ FspMeasurementLib|IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/Base=
FspMeasurementLib.inf=0D
+ TcgEventLogRecordLib|SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLo=
gRecordLib.inf=0D
=0D
#######################################=0D
# Board Package=0D
--=20
2.26.2.windows.1


[PATCH 2/3] Platform/Intel/KabylakeOpenBoardPkg: add ibrary for Fsp measurement.

Qi Zhang
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2939

Signed-off-by: Qi Zhang <qi1.zhang@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Jeremy Soller <jeremy@...>
---
Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc | 2 ++
.../Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc | 2 ++
2 files changed, 4 insertions(+)

diff --git a/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.ds=
c b/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc
index 862e6a6655..34d645be7e 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc
+++ b/Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc
@@ -172,6 +172,8 @@
!if $(TARGET) =3D=3D DEBUG=0D
TestPointCheckLib|$(PLATFORM_PACKAGE)/Test/Library/TestPointCheckLib/Pei=
TestPointCheckLib.inf=0D
!endif=0D
+ FspMeasurementLib|IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/Base=
FspMeasurementLib.inf=0D
+ TcgEventLogRecordLib|SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLo=
gRecordLib.inf=0D
=0D
#######################################=0D
# Board Package=0D
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.=
dsc b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc
index 0b30da8f96..fdfaaa0cda 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc
+++ b/Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc
@@ -213,6 +213,8 @@
!endif=0D
SetCacheMtrrLib|$(PLATFORM_PACKAGE)/Library/SetCacheMtrrLib/SetCacheMtrr=
LibNull.inf=0D
ReportCpuHobLib|$(PLATFORM_PACKAGE)/PlatformInit/Library/ReportCpuHobLib=
/ReportCpuHobLib.inf=0D
+ FspMeasurementLib|IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/Base=
FspMeasurementLib.inf=0D
+ TcgEventLogRecordLib|SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLo=
gRecordLib.inf=0D
=0D
#######################################=0D
# Board Package=0D
--=20
2.26.2.windows.1


[PATCH 1/3] Platform/Intel/CometlakeOpenBoardPkg: add ibrary for Fsp measurement.

Qi Zhang
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2939

Signed-off-by: Qi Zhang <qi1.zhang@...>
Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Cc: Deepika Kethi Reddy <deepika.kethi.reddy@...>
Cc: Kathappan Esakkithevar <kathappan.esakkithevar@...>
---
.../Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.dsc | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPk=
g.dsc b/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.dsc
index 2d9dcb139f..4ea797c550 100644
--- a/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.dsc
+++ b/Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.dsc
@@ -173,6 +173,8 @@
!endif=0D
SetCacheMtrrLib|$(PLATFORM_PACKAGE)/Library/SetCacheMtrrLib/SetCacheMtrr=
LibNull.inf=0D
ReportCpuHobLib|$(PLATFORM_PACKAGE)/PlatformInit/Library/ReportCpuHobLib=
/ReportCpuHobLib.inf=0D
+ FspMeasurementLib|IntelFsp2WrapperPkg/Library/BaseFspMeasurementLib/Base=
FspMeasurementLib.inf=0D
+ TcgEventLogRecordLib|SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLo=
gRecordLib.inf=0D
=0D
#######################################=0D
# Board Package=0D
--=20
2.26.2.windows.1


[PATCH 0/3] add ibrary for Fsp measurement to OpenBoardPkg.

Qi Zhang
 

These patches also depends on one fix of edk2:
https://bugzilla.tianocore.org/show_bug.cgi?id=2939.

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Cc: Deepika Kethi Reddy <deepika.kethi.reddy@...>
Cc: Kathappan Esakkithevar <kathappan.esakkithevar@...>
Cc: Jeremy Soller <jeremy@...>

Qi Zhang (3):
Platform/Intel/CometlakeOpenBoardPkg: add ibrary for Fsp measurement.
Platform/Intel/KabylakeOpenBoardPkg: add ibrary for Fsp measurement.
Platform/Intel/WhiskeylakeOpenBoardPkg: add ibrary for Fsp
measurement.

.../Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.dsc | 2 ++
Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc | 2 ++
.../Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc | 2 ++
.../Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc | 2 ++
.../WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.dsc | 2 ++
5 files changed, 10 insertions(+)

--
2.26.2.windows.1


回复: [edk2-devel] [PATCH] MdeModulePkg/Library: change TpmMeasurementLibNull to BASE library.

gaoliming
 

Qi:
This is a bug fix. Do you request to catch it into this stable tag 202008?

Thanks
Liming
-----邮件原件-----
发件人: bounce+27952+64729+4905953+8761045@groups.io
<bounce+27952+64729+4905953+8761045@groups.io> 代表 Yao, Jiewen
发送时间: 2020年8月28日 14:17
收件人: Zhang, Qi1 <qi1.zhang@...>; devel@edk2.groups.io
抄送: Wang, Jian J <jian.j.wang@...>; Wu, Hao A
<hao.a.wu@...>
主题: Re: [edk2-devel] [PATCH] MdeModulePkg/Library: change
TpmMeasurementLibNull to BASE library.

Reviewed-by: Jiewen Yao <Jiewen.yao@...>

-----Original Message-----
From: Zhang, Qi1 <qi1.zhang@...>
Sent: Friday, August 28, 2020 2:15 PM
To: devel@edk2.groups.io
Cc: Zhang, Qi1 <qi1.zhang@...>; Wang, Jian J
<jian.j.wang@...>;
Wu, Hao A <hao.a.wu@...>; Yao, Jiewen <jiewen.yao@...>
Subject: [PATCH] MdeModulePkg/Library: change TpmMeasurementLibNull
to
BASE library.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2940

TpmMeasurementLib includes DxeTpmMeasurementLib and
PeiTpmMeasurementLib.
So need to change TpmMeasurementLibNull to BASE library to avoid build
error in some platform.

Signed-off-by: Qi Zhang <qi1.zhang@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Jiewen Yao <jiewen.yao@...>
---
.../Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c | 4
+++-
.../Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf | 6
+++---
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git
a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu
ll.c
b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu
ll.c
index b9c5b68de8..ee3be62fc6 100644
---
a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu
ll.c
+++
b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu
ll.c
@@ -1,11 +1,13 @@
/** @file

This library is used by other modules to measure data to TPM.



-Copyright (c) 2015, Intel Corporation. All rights reserved. <BR>

+Copyright (c) 2015-2020, Intel Corporation. All rights reserved. <BR>

SPDX-License-Identifier: BSD-2-Clause-Patent



**/



+#include <Uefi/UefiBaseType.h>

+

/**

Tpm measure and log data, and extend the measurement result into a
specific
PCR.



diff --git
a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu
ll.in
f
b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu
ll.in
f
index 61abcfa2ec..1db2c0d6a7 100644
---
a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu
ll.in
f
+++
b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu
ll.in
f
@@ -1,7 +1,7 @@
## @file

# Provides NULL TPM measurement function.

#

-# Copyright (c) 2015 - 2018, Intel Corporation. All rights
reserved.<BR>

+# Copyright (c) 2015 - 2020, Intel Corporation. All rights
reserved.<BR>

# SPDX-License-Identifier: BSD-2-Clause-Patent

#

##

@@ -10,9 +10,9 @@
INF_VERSION = 0x00010005

BASE_NAME = TpmMeasurementLibNull

FILE_GUID =
6DFD6E9F-9278-48D8-8F45-B6CFF2C2B69C

- MODULE_TYPE = UEFI_DRIVER

+ MODULE_TYPE = BASE

VERSION_STRING = 1.0

- LIBRARY_CLASS =
TpmMeasurementLib|DXE_DRIVER
DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION
UEFI_DRIVER

+ LIBRARY_CLASS = TpmMeasurementLib

MODULE_UNI_FILE = TpmMeasurementLibNull.uni



#

--
2.26.2.windows.1


Re: [PATCH EDK2 v2 1/1] SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

wenyi,xie
 

Hi,Jiewen,

I don't really get the meaning "create a case that bypass all checks in PeCoffLib", do you mean I need to create a PE file that can bypass all check in PeCoffLib without modify any
code and then cause the problem in DxeImageVerificationLib, or just modify some code in PeCoffLib to bypass check instead of removing the calling of PeCoffLoaderGetImageInfo. Would
you mind explaining a little more specifically? As far as I tried, it's really hard to reproduce the issue without touching any code.

Thanks
Wenyi

On 2020/8/28 11:50, Yao, Jiewen wrote:
HI Wenyi
Thank you very much to take time to reproduce.

I am particular interested in below:
"As PE file is modified, function PeCoffLoaderGetImageInfo will return error, so I have to remove it so that for loop can be tested in DxeImageVerificationHandler."

By design, the PeCoffLib should catch illegal PE/COFF image and return error. (even it cannot catch all, it should catch most ones).
Other PE/COFF parser may rely on the checker in PeCoffLib and *no need* to duplicate all checkers.
As such, DxeImageVerificationLib (and other PeCoff consumer) just need checks what has passed the check in PeCoffLib.

I don’t think you should remove the checker. If people can remove the checker, I am sure the rest code will be vulnerable, according to the current design.
Could you please to create a case that bypass all checks in PeCoffLib, then run into DxeImageVerificationLib and cause the problem? That would be more valuable for us.

Thank you
Yao Jiewen

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of wenyi,xie
via groups.io
Sent: Friday, August 28, 2020 11:18 AM
To: Laszlo Ersek <lersek@...>; Wang, Jian J <jian.j.wang@...>;
devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@...>
Cc: songdongkuang@...; Mathews, John <john.mathews@...>
Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

Hi,Laszlo and everyone,

These days I tried to reproduce the issue,and made some progress. I think
there are two way to cause overflow from a mathematical point of view,
1.As Laszlo analysed before, if WinCertificate->dwLength is large enough, close
to MAX_UINT32, then (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-
dwLength)) will cause overflow.
2.(WinCertificate->dwLength + ALIGN_SIZE (WinCertificate->dwLength)) is good,
OffSet is good, but OffSet += (WinCertificate->dwLength + ALIGN_SIZE
(WinCertificate->dwLength)) cause overflow.

Here I choose the second way to reproduce the issue, I choose a PE file and sign
it with my own db certificate. Then I use binary edit tool to modify the PE file like
below,

1.change SecDataDir->Size from 0x5F8 to 0xFFFF1FFF
2.change WinCertificate->dwLength from 0x5F1 to 0xFFFF1FFE
SecDataDir->VirtualAddress in my PE is 0xe000 and no need to change.

As PE file is modified, function PeCoffLoaderGetImageInfo will return error, so I
have to remove it so that for loop can be tested in DxeImageVerificationHandler.
The log is as below,

SecDataDir->VirtualAddress=0xE000, SecDataDir->Size=0xFFFF1FFF.
(First Loop)
OffSet=0xE000.
WinCertificate->dwLength=0xFFFF1FFE, ALIGN_SIZE (WinCertificate-
dwLength)=0x2.
(Second Loop)
OffSet=0x0.
WinCertificate->dwLength=0x5A4D, ALIGN_SIZE (WinCertificate-
dwLength)=0x3.
(Third Loop)
OffSet=0x5A50.
WinCertificate->dwLength=0x9024, ALIGN_SIZE (WinCertificate-
dwLength)=0x4.
(Forth Loop)
OffSet=0xEA78.
WinCertificate->dwLength=0xAFAFAFAF, ALIGN_SIZE (WinCertificate-
dwLength)=0x1.
(Fifth Loop)
OffSet=0xAFB09A28.

As I modify SecDataDir->Size and WinCertificate->dwLength, so in first loop the
condition check whether the Security Data Directory has enough room left for
"WinCertificate->dwLength" is ok.

if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof
(WIN_CERTIFICATE) ||
(SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < WinCertificate-
dwLength) {
break;
}

In the beginning of second loop, WinCertificate->dwLength + ALIGN_SIZE
(WinCertificate->dwLength) is 0xFFFF2000, offset is 0xE000

OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate->dwLength))

Offset now is 0 and overflow happens. So even if my PE only have one signature,
the for loop is still going untill exception happens.


I can't reproduce the issue using the first way, because if WinCertificate-
dwLength is close to MAX_UINT32, it means SecDataDir->Size should also close
to MAX_UINT32, or the condition check
"(SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < WinCertificate-
dwLength" will break. But if SecDataDir->Size is very large, SecDataDir-
VirtualAddress have to be smaller than 8 bytes,
because SecDataDir->VirtualAddress + SecDataDir->Size < MAX_UINT32.
SecDataDir->VirtualAddress is the beginning address of the signature, and before
SecDataDir->VirtualAddress is the content of origin PE file, I think it's impossible
that the size of PE file is only 8 bytes.

As I changed the one line code in DxeImageVerificationHandler to reproduce the
issue, I'm not sure whether it's ok.

Thanks
Wenyi

On 2020/8/19 17:26, Laszlo Ersek wrote:
On 08/18/20 17:18, Mathews, John wrote:
I dug up the original report details. This was noted as a concern during a
source code inspection. There was no demonstration of how it might be
triggered.

" There is an integer overflow vulnerability in the
DxeImageVerificationHandler function when
parsing the PE files attribute certificate table. In cases where WinCertificate-
dwLength is
sufficiently large, it's possible to overflow Offset back to 0 causing an endless
loop."

The recommendation was to add stricter checking of "Offset" and the
embedded length fields of certificate data
before using them.
Thanks for checking!

Laszlo




-----Original Message-----
From: Laszlo Ersek <lersek@...>
Sent: Tuesday, August 18, 2020 1:59 AM
To: Wang, Jian J <jian.j.wang@...>; devel@edk2.groups.io; Yao,
Jiewen <jiewen.yao@...>; xiewenyi2@...
Cc: huangming23@...; songdongkuang@...; Mathews,
John <john.mathews@...>
Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

On 08/18/20 04:10, Wang, Jian J wrote:
Laszlo,

My apologies for the slow response. I'm not the original reporter but
just the BZ submitter. And I didn't do deep analysis to this issue.
The issues was reported from one internal team. Add John in loop to see if
he knows more about it or not.

My superficial understanding on such issue is that, if there's
"potential" issue in theory and hard to reproduce, it's still worthy
of using an alternative way to replace the original implementation
with no "potential" issue at all. Maybe we don't have to prove old way is
something wrong but must prove that the new way is really safe.

I agree, thanks.

It would be nice to hear more from the internal team about the originally
reported (even if hard-to-trigger) issue.

Thanks!
Laszlo


Regards,
Jian

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
Ersek
Sent: Tuesday, August 18, 2020 12:53 AM
To: Yao, Jiewen <jiewen.yao@...>; devel@edk2.groups.io;
xiewenyi2@...; Wang, Jian J <jian.j.wang@...>
Cc: huangming23@...; songdongkuang@...
Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

Hi Jiewen,

On 08/14/20 10:53, Yao, Jiewen wrote:
To Jiewen,
Sorry, I don't have environment to reproduce the issue.
Please help me understand, if you don’t have environment to
reproduce the
issue, how do you guarantee that your patch does fix the problem and
we don’t have any other vulnerabilities?

The original bug report in
<https://bugzilla.tianocore.org/show_bug.cgi?id=2215#c0> is seriously
lacking. It does not go into detail about the alleged integer overflow.
It does not quote the code, does not explain the control flow, does
not identify the exact edk2 commit at which the vulnerability exists.

The bug report also does not offer a reproducer.

Additionally, the exact statement that the bug report does make,
namely

it's possible to overflow Offset back to 0 causing an endless loop

is wrong (as far as I can tell anyway). It is not "OffSet" that can
be overflowed to zero, but the *addend* that is added to OffSet can
be overflowed to zero. Therefore the infinite loop will arise because
OffSet remains stuck at its present value, and not because OffSet
will be re-set to zero.

For the reasons, we can only speculate as to what the actual problem
is, unless Jian decides to join the discussion and clarifies what he
had in mind originally.

My understanding (or even "reconstruction") of the vulnerability is
described above, and in the patches that I proposed.

We can write a patch based on code analysis. It's possible to
identify integer overflows based on code analysis, and it's possible
to verify the correctness of fixes by code review. Obviously testing
is always good, but many times, constructing reproducers for such
issues that were found by code review, is difficult and time
consuming. We can say that we don't fix vulnerabilities without
reproducers, or we can say that we make an effort to fix them even if
all we have is code analysis (and not a reproducer).

So the above paragraph concerns "correctness". Regarding
"completeness", I guarantee you that this patch does not fix *all*
problems related to PE parsing. (See the other BZ tickets.) It does
fix *one* issue with PE parsing. We can say that we try to fix such
issues gradually (give different CVE numbers to different issues, and
address them one at a time), or we can say that we rewrite PE parsing
from the ground up.
(BTW: I have seriously attempted that in the past, and I gave up,
because the PE format is FUBAR.)

In summary:

- the problem statement is unclear,

- it seems like there is indeed an integer overflow problem in the
SecDataDir parsing loop, but it's uncertain whether the bug reporter
had exactly that in mind

- PE parsing is guaranteed to have other vulnerabilities elsewhere in
edk2, but I'm currently unaware of other such issues in
DxeImageVerificationLib specifically

- even if there are other such problems (in DxeImageVerificationLib
or elswehere), fixing this bug that we know about is likely
worthwhile

- for many such bugs, constructing a reproducer is difficult and time
consuming; code analysis, and *regression-testing* are frequently the
only tools we have. That doesn't mean we should ignore this class of bugs.

(Fixing integer overflows retro-actively is more difficult than
writing overflow-free code in the first place, but that ship has
sailed; so we can only fight these bugs incrementally now, unless we
can rewrite PE parsing with a new data structure from the ground up.
Again I tried that and gave up, because the spec is not public, and
what I did manage to learn about PE, showed that it was insanely
over-engineered. I'm not saying that other binary / executable
formats are better, of course.)

Please check out my patches (inlined elsewhere in this thread), and
comment whether you'd like me to post them to the list as a
standalone series.

Jian: it wouldn't hurt if you commented as well.

Thanks
Laszlo

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
wenyi,xie
via groups.io
Sent: Friday, August 14, 2020 3:54 PM
To: Laszlo Ersek <lersek@...>; devel@edk2.groups.io; Yao,
Jiewen <jiewen.yao@...>; Wang, Jian J <jian.j.wang@...>
Cc: huangming23@...; songdongkuang@...
Subject: Re: [edk2-devel] [PATCH EDK2 v2 1/1]
SecurityPkg/DxeImageVerificationLib:Enhanced verification of Offset

To Laszlo,
Thank you for your detailed description, I agree with what you
analyzed and
I'm
OK with your patches, it's
correct and much simpler.

To Jiewen,
Sorry, I don't have environment to reproduce the issue.

Thanks
Wenyi

On 2020/8/14 2:50, Laszlo Ersek wrote:
On 08/13/20 13:55, Wenyi Xie wrote:
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2215

There is an integer overflow vulnerability in
DxeImageVerificationHandler function when parsing the PE files
attribute certificate table. In cases where
WinCertificate->dwLength is sufficiently large, it's possible to
overflow Offset back to 0 causing an endless loop.

Check offset inbetween VirtualAddress and VirtualAddress + Size.
Using SafeintLib to do offset addition with result check.

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Laszlo Ersek <lersek@...>
Signed-off-by: Wenyi Xie <xiewenyi2@...>
---

SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.inf
|
1 +

SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.h
|
1 +

SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
|
111 +++++++++++---------
3 files changed, 63 insertions(+), 50 deletions(-)

diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.inf
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.inf
index 1e1a639857e0..a7ac4830b3d4 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.inf
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.inf
@@ -53,6 +53,7 @@ [LibraryClasses]
SecurityManagementLib
PeCoffLib
TpmMeasurementLib
+ SafeIntLib

[Protocols]
gEfiFirmwareVolume2ProtocolGuid ## SOMETIMES_CONSUMES
diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.h
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.h
index 17955ff9774c..060273917d5d 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.h
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.h
@@ -23,6 +23,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/DevicePathLib.h> #include
<Library/SecurityManagementLib.h> #include <Library/PeCoffLib.h>
+#include <Library/SafeIntLib.h>
#include <Protocol/FirmwareVolume2.h> #include
<Protocol/DevicePath.h> #include <Protocol/BlockIo.h> diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
index 36b87e16d53d..dbc03e28c05b 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib
.c
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
@@ -1658,6 +1658,10 @@ DxeImageVerificationHandler (
EFI_STATUS HashStatus;
EFI_STATUS DbStatus;
BOOLEAN IsFound;
+ UINT32 AlignedLength;
+ UINT32 Result;
+ EFI_STATUS AddStatus;
+ BOOLEAN IsAuthDataAssigned;

SignatureList = NULL;
SignatureListSize = 0;
@@ -1667,6 +1671,7 @@ DxeImageVerificationHandler (
Action = EFI_IMAGE_EXECUTION_AUTH_UNTESTED;
IsVerified = FALSE;
IsFound = FALSE;
+ Result = 0;

//
// Check the image type and get policy setting.
@@ -1850,9 +1855,10 @@ DxeImageVerificationHandler (
// The first certificate starts at offset
(SecDataDir->VirtualAddress) from
the
start of the file.
//
for (OffSet = SecDataDir->VirtualAddress;
- OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
- OffSet += (WinCertificate->dwLength + ALIGN_SIZE
(WinCertificate-
dwLength))) {
+ (OffSet >= SecDataDir->VirtualAddress) && (OffSet <
+ (SecDataDir-
VirtualAddress + SecDataDir->Size));) {
+ IsAuthDataAssigned = FALSE;
WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
+ AlignedLength = WinCertificate->dwLength + ALIGN_SIZE
(WinCertificate-
dwLength);

I disagree with this patch.

The primary reason for my disagreement is that the bug report
<https://bugzilla.tianocore.org/show_bug.cgi?id=2215#c0> is
inexact, and so this patch tries to fix the wrong thing.

With edk2 master at commit 65904cdbb33c, it is *not* possible to
overflow the OffSet variable to zero with "WinCertificate->dwLength"
*purely*, and cause an endless loop. Note that we have (at commit
65904cdbb33c):

for (OffSet = SecDataDir->VirtualAddress;
OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
OffSet += (WinCertificate->dwLength + ALIGN_SIZE
(WinCertificate-
dwLength))) {
WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet)
<= sizeof
(WIN_CERTIFICATE) ||
(SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <
WinCertificate-
dwLength) {
break;
}

The last sub-condition checks whether the Security Data Directory
has enough room left for "WinCertificate->dwLength". If not, then
we break out of the loop.

If we *do* have enough room, that is:

(SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) >=
WinCertificate-
dwLength

then we have (by adding OffSet to both sides):

SecDataDir->VirtualAddress + SecDataDir->Size >= OffSet +
WinCertificate- dwLength

The left hand side is a known-good UINT32, and so incrementing
OffSet (a
UINT32) *solely* by "WinCertificate->dwLength" (also a UINT32)
does not cause an overflow.

Instead, the problem is with the alignment. The "if" statement
checks whether we have enough room for "dwLength", but then
"OffSet" is advanced by "dwLength" *aligned up* to the next
multiple of 8. And that may indeed cause various overflows.

Now, the main problem with the present patch is that it does not
fix one of those overflows. Namely, consider that "dwLength" is
very close to
MAX_UINT32 (or even think it's exactly MAX_UINT32). Then aligning
it up to the next multiple of 8 will yield 0. In other words,
"AlignedLength"
will be zero.

And when that happens, there's going to be an infinite loop just
the
same: "OffSet" will not be zero, but it will be *stuck*. The
SafeUint32Add() call at the bottom will succeed, but it will not
change the value of "OffSet".

More at the bottom.


if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet)
<= sizeof
(WIN_CERTIFICATE) ||
(SecDataDir->VirtualAddress + SecDataDir->Size - OffSet)
<
WinCertificate->dwLength) {
break;
@@ -1872,6 +1878,8 @@ DxeImageVerificationHandler (
}
AuthData = PkcsCertData->CertData;
AuthDataSize = PkcsCertData->Hdr.dwLength -
sizeof(PkcsCertData-
Hdr);
+ IsAuthDataAssigned = TRUE;
+ HashStatus = HashPeImageByType (AuthData, AuthDataSize);
} else if (WinCertificate->wCertificateType ==
WIN_CERT_TYPE_EFI_GUID)
{
//
// The certificate is formatted as
WIN_CERTIFICATE_UEFI_GUID which
is
described in UEFI Spec.
@@ -1880,72 +1888,75 @@ DxeImageVerificationHandler (
if (WinCertUefiGuid->Hdr.dwLength <=
OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData)) {
break;
}
- if (!CompareGuid (&WinCertUefiGuid->CertType,
&gEfiCertPkcs7Guid))
{
- continue;
+ if (CompareGuid (&WinCertUefiGuid->CertType,
+ &gEfiCertPkcs7Guid))
{
+ AuthData = WinCertUefiGuid->CertData;
+ AuthDataSize = WinCertUefiGuid->Hdr.dwLength -
OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData);
+ IsAuthDataAssigned = TRUE;
+ HashStatus = HashPeImageByType (AuthData, AuthDataSize);
}
- AuthData = WinCertUefiGuid->CertData;
- AuthDataSize = WinCertUefiGuid->Hdr.dwLength -
OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData);
} else {
if (WinCertificate->dwLength < sizeof (WIN_CERTIFICATE)) {
break;
}
- continue;
}

- HashStatus = HashPeImageByType (AuthData, AuthDataSize);
- if (EFI_ERROR (HashStatus)) {
- continue;
- }
-
- //
- // Check the digital signature against the revoked certificate in
forbidden
database (dbx).
- //
- if (IsForbiddenByDbx (AuthData, AuthDataSize)) {
- Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED;
- IsVerified = FALSE;
- break;
- }
-
- //
- // Check the digital signature against the valid certificate in
allowed
database (db).
- //
- if (!IsVerified) {
- if (IsAllowedByDb (AuthData, AuthDataSize)) {
- IsVerified = TRUE;
+ if (IsAuthDataAssigned && !EFI_ERROR (HashStatus)) {
+ //
+ // Check the digital signature against the revoked
+ certificate in
forbidden
database (dbx).
+ //
+ if (IsForbiddenByDbx (AuthData, AuthDataSize)) {
+ Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED;
+ IsVerified = FALSE;
+ break;
}
- }

- //
- // Check the image's hash value.
- //
- DbStatus = IsSignatureFoundInDatabase (
- EFI_IMAGE_SECURITY_DATABASE1,
- mImageDigest,
- &mCertType,
- mImageDigestSize,
- &IsFound
- );
- if (EFI_ERROR (DbStatus) || IsFound) {
- Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND;
- DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is
signed
but %s
hash of image is found in DBX.\n", mHashTypeStr));
- IsVerified = FALSE;
- break;
- }
+ //
+ // Check the digital signature against the valid
+ certificate in allowed
database (db).
+ //
+ if (!IsVerified) {
+ if (IsAllowedByDb (AuthData, AuthDataSize)) {
+ IsVerified = TRUE;
+ }
+ }

- if (!IsVerified) {
+ //
+ // Check the image's hash value.
+ //
DbStatus = IsSignatureFoundInDatabase (
- EFI_IMAGE_SECURITY_DATABASE,
+ EFI_IMAGE_SECURITY_DATABASE1,
mImageDigest,
&mCertType,
mImageDigestSize,
&IsFound
);
- if (!EFI_ERROR (DbStatus) && IsFound) {
- IsVerified = TRUE;
- } else {
- DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is
signed
but
signature is not allowed by DB and %s hash of image is not found in
DB/DBX.\n",
mHashTypeStr));
+ if (EFI_ERROR (DbStatus) || IsFound) {
+ Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND;
+ DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is
+ signed
but %s hash of image is found in DBX.\n", mHashTypeStr));
+ IsVerified = FALSE;
+ break;
}
+
+ if (!IsVerified) {
+ DbStatus = IsSignatureFoundInDatabase (
+ EFI_IMAGE_SECURITY_DATABASE,
+ mImageDigest,
+ &mCertType,
+ mImageDigestSize,
+ &IsFound
+ );
+ if (!EFI_ERROR (DbStatus) && IsFound) {
+ IsVerified = TRUE;
+ } else {
+ DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is
+ signed
but
signature is not allowed by DB and %s hash of image is not found in
DB/DBX.\n",
mHashTypeStr));
+ }
+ }
+ }
+
+ AddStatus = SafeUint32Add (OffSet, AlignedLength, &Result);
+ if (EFI_ERROR (AddStatus)) {
+ break;
}
+ OffSet = Result;
}

if (OffSet != (SecDataDir->VirtualAddress + SecDataDir->Size))
{
There are other (smaller) reasons why I dislike this patch:

- The "IsAuthDataAssigned" variable is superfluous; we could use
the existent "AuthData" variable (with a NULL-check and a
NULL-assignment) similarly.

- The patch complicates / reorganizes the control flow needlessly.
This complication originates from placing the checked "OffSet"
increment at the bottom of the loop, which then requires the
removal of all the "continue" statements. But we don't need to
check-and-increment at the bottom. We can keep the increment
inside the "for" statement, only extend the *existent* room check
(which I've quoted) to take the alignment into account as well. If
there is enough room for the alignment in the security data
directory, then that guarantees there won't be a UINT32 overflow
either.

All in all, I'm proposing the following three patches instead. The
first two patches are preparation, the last patch is the fix.

Patch#1:

From 11af0a104d34d39bf1b1aab256428ae4edbddd77 Mon Sep 17
00:00:00
2001
From: Laszlo Ersek <lersek@...>
Date: Thu, 13 Aug 2020 19:11:39 +0200
Subject: [PATCH 1/3] SecurityPkg/DxeImageVerificationLib: extract
SecDataDirEnd, SecDataDirLeft

The following two quantities:

SecDataDir->VirtualAddress + SecDataDir->Size
SecDataDir->VirtualAddress + SecDataDir->Size - OffSet

are used multiple times in DxeImageVerificationHandler().
Introduce helper variables for them: "SecDataDirEnd" and
"SecDataDirLeft", respectively.
This saves us multiple calculations and significantly simplifies the code.

Note that all three summands above have type UINT32, therefore
the new variables are also of type UINT32.

This patch does not change behavior.

(Note that the code already handles the case when the

SecDataDir->VirtualAddress + SecDataDir->Size

UINT32 addition overflows -- namely, in that case, the
certificate loop is never entered, and the corruption check right
after the loop fires.)

Signed-off-by: Laszlo Ersek <lersek@...>
---

SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c |
12
++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
index 36b87e16d53d..8761980c88aa 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib
.c
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
@@ -1652,6 +1652,8 @@ DxeImageVerificationHandler (
UINT8 *AuthData;
UINTN AuthDataSize;
EFI_IMAGE_DATA_DIRECTORY *SecDataDir;
+ UINT32 SecDataDirEnd;
+ UINT32 SecDataDirLeft;
UINT32 OffSet;
CHAR16 *NameStr;
RETURN_STATUS PeCoffStatus;
@@ -1849,12 +1851,14 @@ DxeImageVerificationHandler (
// "Attribute Certificate Table".
// The first certificate starts at offset
(SecDataDir->VirtualAddress) from
the
start of the file.
//
+ SecDataDirEnd = SecDataDir->VirtualAddress + SecDataDir->Size;
for (OffSet = SecDataDir->VirtualAddress;
- OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
+ OffSet < SecDataDirEnd;
OffSet += (WinCertificate->dwLength + ALIGN_SIZE
(WinCertificate-
dwLength))) {
WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
- if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <=
sizeof
(WIN_CERTIFICATE) ||
- (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <
WinCertificate->dwLength) {
+ SecDataDirLeft = SecDataDirEnd - OffSet;
+ if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE) ||
+ SecDataDirLeft < WinCertificate->dwLength) {
break;
}

@@ -1948,7 +1952,7 @@ DxeImageVerificationHandler (
}
}

- if (OffSet != (SecDataDir->VirtualAddress + SecDataDir->Size))
{
+ if (OffSet != SecDataDirEnd) {
//
// The Size in Certificate Table or the attribute
certificate table is
corrupted.
//
--
2.19.1.3.g30247aa5d201
Patch#2:

From 72012c065a53582f7df695e7b9730c45f49226c6 Mon Sep 17
00:00:00
2001
From: Laszlo Ersek <lersek@...>
Date: Thu, 13 Aug 2020 19:19:06 +0200
Subject: [PATCH 2/3] SecurityPkg/DxeImageVerificationLib: assign
WinCertificate after size check

Currently the (SecDataDirLeft <= sizeof (WIN_CERTIFICATE)) check
only guards the de-referencing of the "WinCertificate" pointer.
It does not guard the calculation of hte pointer itself:

WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);

This is wrong; if we don't know for sure that we have enough room
for a WIN_CERTIFICATE, then even creating such a pointer, not
just de-referencing it, may invoke undefined behavior.

Move the pointer calculation after the size check.

Signed-off-by: Laszlo Ersek <lersek@...>
---

SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c |
8
+++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
index 8761980c88aa..461ed7cfb5ac 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib
.c
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
@@ -1855,10 +1855,12 @@ DxeImageVerificationHandler (
for (OffSet = SecDataDir->VirtualAddress;
OffSet < SecDataDirEnd;
OffSet += (WinCertificate->dwLength + ALIGN_SIZE
(WinCertificate-
dwLength))) {
- WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
SecDataDirLeft = SecDataDirEnd - OffSet;
- if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE) ||
- SecDataDirLeft < WinCertificate->dwLength) {
+ if (SecDataDirLeft <= sizeof (WIN_CERTIFICATE)) {
+ break;
+ }
+ WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
+ if (SecDataDirLeft < WinCertificate->dwLength) {
break;
}

--
2.19.1.3.g30247aa5d201
Patch#3:

From 0bbba15b84f8f9f2cdc770a89f418aaec6cfb31e Mon Sep 17
00:00:00
2001
From: Laszlo Ersek <lersek@...>
Date: Thu, 13 Aug 2020 19:34:33 +0200
Subject: [PATCH 3/3] SecurityPkg/DxeImageVerificationLib: catch
alignment
overflow (CVE-2019-14562)

The DxeImageVerificationHandler() function currently checks
whether "SecDataDir" has enough room for
"WinCertificate->dwLength". However,
for
advancing "OffSet", "WinCertificate->dwLength" is aligned to the
next multiple of 8. If "WinCertificate->dwLength" is large
enough, the alignment will return 0, and "OffSet" will be stuck at the
same value.

Check whether "SecDataDir" has room left for both
"WinCertificate->dwLength" and the alignment.

Signed-off-by: Laszlo Ersek <lersek@...>
---

SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c |
4
+++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
index 461ed7cfb5ac..e38eb981b7a0 100644
---
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib
.c
+++
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL
ib.c
@@ -1860,7 +1860,9 @@ DxeImageVerificationHandler (
break;
}
WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
- if (SecDataDirLeft < WinCertificate->dwLength) {
+ if (SecDataDirLeft < WinCertificate->dwLength ||
+ (SecDataDirLeft - WinCertificate->dwLength <
+ ALIGN_SIZE (WinCertificate->dwLength))) {
break;
}

--
2.19.1.3.g30247aa5d201
If Wenyi and the reviewers are OK with these patches, I can submit
them as a standalone patch series.

Note that I do not have any reproducer for the issue; the best
testing that I could offer would be some light-weight Secure Boot
regression tests.

Thanks
Laszlo


.



.


Re: [PATCH] MdeModulePkg/Library: change TpmMeasurementLibNull to BASE library.

Yao, Jiewen
 

Reviewed-by: Jiewen Yao <Jiewen.yao@...>

-----Original Message-----
From: Zhang, Qi1 <qi1.zhang@...>
Sent: Friday, August 28, 2020 2:15 PM
To: devel@edk2.groups.io
Cc: Zhang, Qi1 <qi1.zhang@...>; Wang, Jian J <jian.j.wang@...>;
Wu, Hao A <hao.a.wu@...>; Yao, Jiewen <jiewen.yao@...>
Subject: [PATCH] MdeModulePkg/Library: change TpmMeasurementLibNull to
BASE library.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2940

TpmMeasurementLib includes DxeTpmMeasurementLib and
PeiTpmMeasurementLib.
So need to change TpmMeasurementLibNull to BASE library to avoid build
error in some platform.

Signed-off-by: Qi Zhang <qi1.zhang@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Jiewen Yao <jiewen.yao@...>
---
.../Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c | 4 +++-
.../Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf | 6 +++---
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git
a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c
b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c
index b9c5b68de8..ee3be62fc6 100644
---
a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c
+++
b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c
@@ -1,11 +1,13 @@
/** @file

This library is used by other modules to measure data to TPM.



-Copyright (c) 2015, Intel Corporation. All rights reserved. <BR>

+Copyright (c) 2015-2020, Intel Corporation. All rights reserved. <BR>

SPDX-License-Identifier: BSD-2-Clause-Patent



**/



+#include <Uefi/UefiBaseType.h>

+

/**

Tpm measure and log data, and extend the measurement result into a specific
PCR.



diff --git
a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.in
f
b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.in
f
index 61abcfa2ec..1db2c0d6a7 100644
---
a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.in
f
+++
b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.in
f
@@ -1,7 +1,7 @@
## @file

# Provides NULL TPM measurement function.

#

-# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>

+# Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.<BR>

# SPDX-License-Identifier: BSD-2-Clause-Patent

#

##

@@ -10,9 +10,9 @@
INF_VERSION = 0x00010005

BASE_NAME = TpmMeasurementLibNull

FILE_GUID = 6DFD6E9F-9278-48D8-8F45-B6CFF2C2B69C

- MODULE_TYPE = UEFI_DRIVER

+ MODULE_TYPE = BASE

VERSION_STRING = 1.0

- LIBRARY_CLASS = TpmMeasurementLib|DXE_DRIVER
DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER

+ LIBRARY_CLASS = TpmMeasurementLib

MODULE_UNI_FILE = TpmMeasurementLibNull.uni



#

--
2.26.2.windows.1


[PATCH] MdeModulePkg/Library: change TpmMeasurementLibNull to BASE library.

Qi Zhang
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2940

TpmMeasurementLib includes DxeTpmMeasurementLib and PeiTpmMeasurementLib.
So need to change TpmMeasurementLibNull to BASE library to avoid build
error in some platform.

Signed-off-by: Qi Zhang <qi1.zhang@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Jiewen Yao <jiewen.yao@...>
---
.../Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c | 4 +++-
.../Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf | 6 +++---
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu=
ll.c b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c
index b9c5b68de8..ee3be62fc6 100644
--- a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c
+++ b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.c
@@ -1,11 +1,13 @@
/** @file=0D
This library is used by other modules to measure data to TPM.=0D
=0D
-Copyright (c) 2015, Intel Corporation. All rights reserved. <BR>=0D
+Copyright (c) 2015-2020, Intel Corporation. All rights reserved. <BR>=0D
SPDX-License-Identifier: BSD-2-Clause-Patent=0D
=0D
**/=0D
=0D
+#include <Uefi/UefiBaseType.h>=0D
+=0D
/**=0D
Tpm measure and log data, and extend the measurement result into a speci=
fic PCR.=0D
=0D
diff --git a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNu=
ll.inf b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.i=
nf
index 61abcfa2ec..1db2c0d6a7 100644
--- a/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
+++ b/MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
@@ -1,7 +1,7 @@
## @file=0D
# Provides NULL TPM measurement function.=0D
#=0D
-# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>=0D
+# Copyright (c) 2015 - 2020, Intel Corporation. All rights reserved.<BR>=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
##=0D
@@ -10,9 +10,9 @@
INF_VERSION =3D 0x00010005=0D
BASE_NAME =3D TpmMeasurementLibNull=0D
FILE_GUID =3D 6DFD6E9F-9278-48D8-8F45-B6CFF2C2B69C=
=0D
- MODULE_TYPE =3D UEFI_DRIVER=0D
+ MODULE_TYPE =3D BASE=0D
VERSION_STRING =3D 1.0=0D
- LIBRARY_CLASS =3D TpmMeasurementLib|DXE_DRIVER DXE_RUNT=
IME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER=0D
+ LIBRARY_CLASS =3D TpmMeasurementLib=0D
MODULE_UNI_FILE =3D TpmMeasurementLibNull.uni=0D
=0D
#=0D
--=20
2.26.2.windows.1


[PATCH v7 05/14] OvmfPkg: Add VariablePolicy engine to OvmfPkg platform

Bret Barkelew
 

https://bugzilla.tianocore.org/show_bug.cgi?id=3D2522

Cc: Jordan Justen <jordan.l.justen@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Bret Barkelew <brbarkel@...>
Signed-off-by: Bret Barkelew <brbarkel@...>
Reviewed-by: Laszlo Ersek <lersek@...>
---
OvmfPkg/OvmfPkgIa32.dsc | 5 +++++
OvmfPkg/OvmfPkgIa32X64.dsc | 5 +++++
OvmfPkg/OvmfPkgX64.dsc | 5 +++++
OvmfPkg/OvmfXen.dsc | 4 ++++
4 files changed, 19 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 133a9a93c071..f9867167b070 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -3,6 +3,7 @@
#=0D
# Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR>=
=0D
# (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>=0D
+# Copyright (c) Microsoft Corporation.=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -197,6 +198,8 @@ [LibraryClasses]
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLib=
Null.inf=0D
!endif=0D
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf=0D
+ VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyL=
ib.inf=0D
+ VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Var=
iablePolicyHelperLib.inf=0D
=0D
=0D
#=0D
@@ -336,6 +339,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf=0D
PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf=0D
QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf=
=0D
+ VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyL=
ibRuntimeDxe.inf=0D
=0D
[LibraryClasses.common.UEFI_DRIVER]=0D
PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf=0D
@@ -963,6 +967,7 @@ [Components]
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf {=0D
<LibraryClasses>=0D
NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf=0D
+ NULL|MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf=0D
}=0D
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf=0D
=0D
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 338c38db29b5..440b60c758d3 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -3,6 +3,7 @@
#=0D
# Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR>=
=0D
# (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>=0D
+# Copyright (c) Microsoft Corporation.=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -201,6 +202,8 @@ [LibraryClasses]
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLib=
Null.inf=0D
!endif=0D
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf=0D
+ VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyL=
ib.inf=0D
+ VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Var=
iablePolicyHelperLib.inf=0D
=0D
=0D
#=0D
@@ -340,6 +343,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf=0D
PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf=0D
QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf=
=0D
+ VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyL=
ibRuntimeDxe.inf=0D
=0D
[LibraryClasses.common.UEFI_DRIVER]=0D
PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf=0D
@@ -978,6 +982,7 @@ [Components.X64]
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf {=0D
<LibraryClasses>=0D
NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf=0D
+ NULL|MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf=0D
}=0D
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf=0D
=0D
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index b80710fbdca4..3098f5b48f65 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -3,6 +3,7 @@
#=0D
# Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR>=
=0D
# (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>=0D
+# Copyright (c) Microsoft Corporation.=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -201,6 +202,8 @@ [LibraryClasses]
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLib=
Null.inf=0D
!endif=0D
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf=0D
+ VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyL=
ib.inf=0D
+ VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Var=
iablePolicyHelperLib.inf=0D
=0D
=0D
#=0D
@@ -340,6 +343,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf=0D
PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf=0D
QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf=
=0D
+ VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyL=
ibRuntimeDxe.inf=0D
=0D
[LibraryClasses.common.UEFI_DRIVER]=0D
PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf=0D
@@ -974,6 +978,7 @@ [Components]
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf {=0D
<LibraryClasses>=0D
NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf=0D
+ NULL|MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf=0D
}=0D
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf=0D
=0D
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 37b63a874067..1b1857fb74fd 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -4,6 +4,7 @@
# Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR>=
=0D
# (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>=0D
# Copyright (c) 2019, Citrix Systems, Inc.=0D
+# Copyright (c) Microsoft Corporation.=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -182,6 +183,8 @@ [LibraryClasses]
=0D
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLib=
Null.inf=0D
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf=0D
+ VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyL=
ib.inf=0D
+ VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Var=
iablePolicyHelperLib.inf=0D
=0D
=0D
#=0D
@@ -290,6 +293,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf=0D
PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf=0D
QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf=
=0D
+ VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyL=
ibRuntimeDxe.inf=0D
=0D
[LibraryClasses.common.UEFI_DRIVER]=0D
PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf=0D
--=20
2.28.0.windows.1


[PATCH v7 14/14] MdeModulePkg: Add a shell-based functional test for VariablePolicy

Bret Barkelew
 

https://bugzilla.tianocore.org/show_bug.cgi?id=3D2522

To verify that VariablePolicy is correctly integrated
on platforms, add a Shell-based functional test to
confirm expected behavior.

NOTE: This test assumes that VariablePolicy is built
with PcdAllowVariablePolicyEnforcementDisable set to
TRUE.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Liming Gao <liming.gao@...>
Cc: Bret Barkelew <brbarkel@...>
Signed-off-by: Bret Barkelew <brbarkel@...>
---
MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyFuncTe=
stApp.c | 2226 ++++++++++++++++++++
MdeModulePkg/MdeModulePkg.ci.yaml =
| 4 +-
MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/Readme.md =
| 55 +
MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyFuncTe=
stApp.inf | 47 +
MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyTestAu=
thVar.h | 128 ++
5 files changed, 2459 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/Variable=
PolicyFuncTestApp.c b/MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp=
/VariablePolicyFuncTestApp.c
new file mode 100644
index 000000000000..c2b28e4b642b
--- /dev/null
+++ b/MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyF=
uncTestApp.c
@@ -0,0 +1,2226 @@
+/** @file=0D
+UEFI Shell based application for unit testing the Variable Policy Protocol=
.=0D
+=0D
+Copyright (c) Microsoft Corporation.=0D
+SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+**/=0D
+=0D
+#include <Uefi.h>=0D
+#include <Library/UefiLib.h>=0D
+#include <Library/PrintLib.h>=0D
+#include <Library/DebugLib.h>=0D
+#include <Library/MemoryAllocationLib.h>=0D
+#include <Library/UnitTestLib.h>=0D
+#include <Library/UefiBootServicesTableLib.h>=0D
+#include <Protocol/VariablePolicy.h>=0D
+#include <Library/VariablePolicyHelperLib.h>=0D
+#include <Library/UefiRuntimeServicesTableLib.h>=0D
+#include <Library/BaseMemoryLib.h>=0D
+#include <Library/MemoryAllocationLib.h>=0D
+=0D
+#include "VariablePolicyTestAuthVar.h"=0D
+=0D
+// TODO: Need to add to the UnitTestFrameworkPkg=0D
+// #include <Library/UnitTestBootLib.h>=0D
+=0D
+#define UNIT_TEST_APP_NAME "Variable Policy Unit Test Application"=
=0D
+#define UNIT_TEST_APP_VERSION "0.1"=0D
+=0D
+// TODO: Need to add to the UnitTestFrameworkPkg=0D
+UNIT_TEST_FRAMEWORK_HANDLE=0D
+GetActiveFrameworkHandle (=0D
+ VOID=0D
+ );=0D
+=0D
+EDKII_VARIABLE_POLICY_PROTOCOL *mVarPol =3D NULL;=0D
+=0D
+=0D
+EFI_GUID mTestNamespaceGuid1 =3D { 0x3b389299, 0xabaf, 0x433b, { 0xa4, 0xa=
9, 0x23, 0xc8, 0x44, 0x02, 0xfc, 0xad } };=0D
+EFI_GUID mTestNamespaceGuid2 =3D { 0x4c49a3aa, 0xbcb0, 0x544c, { 0xb5, 0xb=
a, 0x34, 0xd9, 0x55, 0x13, 0x0d, 0xbe } };=0D
+EFI_GUID mTestNamespaceGuid3 =3D { 0x5d5ab4bb, 0xcdc1, 0x655d, { 0xc6, 0xc=
b, 0x45, 0xea, 0x66, 0x24, 0x1e, 0xcf } };=0D
+=0D
+#define TEST_AUTH_VAR_NAME L"DummyAuthVar"=0D
+EFI_GUID mTestAuthNamespaceGuid =3D { 0xb6c5a2c6, 0x3ece, 0x4b9b, { 0x8c, =
0xc8, 0x96, 0xd8, 0xd9, 0xca, 0xd3, 0x4e } };=0D
+=0D
+/**=0D
+ Prerequisite for most test cases.=0D
+**/=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+LocateVarPolicyPreReq (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+=0D
+ if (mVarPol =3D=3D NULL) {=0D
+ Status =3D gBS->LocateProtocol (&gEdkiiVariablePolicyProtocolGuid,=0D
+ NULL,=0D
+ (VOID **) &mVarPol);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+ UT_ASSERT_NOT_NULL (mVarPol);=0D
+ }=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // LocateVarPolicyPreReq=0D
+=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+VarPolicyEnabledPreReq (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ BOOLEAN State;=0D
+=0D
+ UT_ASSERT_EQUAL(LocateVarPolicyPreReq(Context), UNIT_TEST_PASSED);=0D
+ Status =3D mVarPol->IsVariablePolicyEnabled (&State);=0D
+ UT_ASSERT_NOT_EFI_ERROR(Status);=0D
+ UT_ASSERT_TRUE(State);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+}=0D
+=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+VarPolicyDisabledPreReq (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ BOOLEAN State;=0D
+=0D
+ UT_ASSERT_EQUAL(LocateVarPolicyPreReq(Context), UNIT_TEST_PASSED);=0D
+ Status =3D mVarPol->IsVariablePolicyEnabled (&State);=0D
+ UT_ASSERT_NOT_EFI_ERROR(Status);=0D
+ UT_ASSERT_FALSE(State);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+}=0D
+=0D
+/**=0D
+ Getting Started tests.=0D
+**/=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+CheckVpEnabled (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ BOOLEAN State;=0D
+=0D
+ Status =3D mVarPol->IsVariablePolicyEnabled (&State);=0D
+=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+ UT_ASSERT_EQUAL (State, TRUE);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // CheckVpEnabled=0D
+=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+CheckVpRevision (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ UT_ASSERT_NOT_EQUAL (mVarPol->Revision, 0);=0D
+ UT_LOG_INFO ("VP Revision: 0x%x\n", mVarPol->Revision);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // CheckVpRevision=0D
+=0D
+/**=0D
+ NOTE: Copied from SecureBootConfigImpl.c, then modified.=0D
+=0D
+ Create a time based data payload by concatenating the EFI_VARIABLE_AUTHE=
NTICATION_2=0D
+ descriptor with the input data. NO authentication is required in this fu=
nction.=0D
+=0D
+ @param[in, out] DataSize On input, the size of Data buffer in by=
tes.=0D
+ On output, the size of data returned in=
Data=0D
+ buffer in bytes.=0D
+ @param[in, out] Data On input, Pointer to data buffer to be =
wrapped or=0D
+ pointer to NULL to wrap an empty payloa=
d.=0D
+ On output, Pointer to the new payload d=
ate buffer allocated from pool,=0D
+ it's caller's responsibility to free th=
e memory when finish using it.=0D
+ @param[in] Time [Optional] If provided, will be used as=
the timestamp for the payload.=0D
+ If NULL, a new timestamp will be genera=
ted using GetTime().=0D
+=0D
+ @retval EFI_SUCCESS Create time based payload successfully.=
=0D
+ @retval EFI_OUT_OF_RESOURCES There are not enough memory resources t=
o create time based payload.=0D
+ @retval EFI_INVALID_PARAMETER The parameter is invalid.=0D
+ @retval Others Unexpected error happens.=0D
+=0D
+**/=0D
+STATIC=0D
+EFI_STATUS=0D
+CreateEmptyTimeBasedPayload (=0D
+ IN OUT UINTN *DataSize,=0D
+ IN OUT UINT8 **Data,=0D
+ IN EFI_TIME *Time OPTIONAL=0D
+ )=0D
+{=0D
+ UINT8 *NewData;=0D
+ UINT8 *Payload;=0D
+ UINTN PayloadSize;=0D
+ EFI_VARIABLE_AUTHENTICATION_2 *DescriptorData;=0D
+ UINTN DescriptorSize;=0D
+ EFI_TIME NewTime;=0D
+=0D
+ if (Data =3D=3D NULL || DataSize =3D=3D NULL) {=0D
+ DEBUG((DEBUG_ERROR, "CreateEmptyTimeBasedPayload(), invalid arg\n"));=
=0D
+ return EFI_INVALID_PARAMETER;=0D
+ }=0D
+=0D
+ //=0D
+ // In Setup mode or Custom mode, the variable does not need to be signed=
but the=0D
+ // parameters to the SetVariable() call still need to be prepared as aut=
henticated=0D
+ // variable. So we create EFI_VARIABLE_AUTHENTICATED_2 descriptor withou=
t certificate=0D
+ // data in it.=0D
+ //=0D
+ Payload =3D *Data;=0D
+ PayloadSize =3D *DataSize;=0D
+=0D
+ DescriptorSize =3D OFFSET_OF (EFI_VARIABLE_AUTHENTICATION_2, AuthInfo=
) + OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData);=0D
+ NewData =3D (UINT8*) AllocateZeroPool (DescriptorSize + PayloadSize);=0D
+ if (NewData =3D=3D NULL) {=0D
+ DEBUG((DEBUG_ERROR, "CreateEmptyTimeBasedPayload() Out of resources.\n=
"));=0D
+ return EFI_OUT_OF_RESOURCES;=0D
+ }=0D
+=0D
+ if ((Payload !=3D NULL) && (PayloadSize !=3D 0)) {=0D
+ CopyMem (NewData + DescriptorSize, Payload, PayloadSize);=0D
+ }=0D
+=0D
+ DescriptorData =3D (EFI_VARIABLE_AUTHENTICATION_2 *) (NewData);=0D
+=0D
+ //=0D
+ // Use or create the timestamp.=0D
+ //=0D
+ // If Time is NULL, create a new timestamp.=0D
+ if (Time =3D=3D NULL)=0D
+ {=0D
+ NewTime.Year =3D 9999;=0D
+ NewTime.Month =3D 12;=0D
+ NewTime.Day =3D 31;=0D
+ NewTime.Hour =3D 23;=0D
+ NewTime.Minute =3D 59;=0D
+ NewTime.Second =3D 59;=0D
+ NewTime.Pad1 =3D 0;=0D
+ NewTime.Nanosecond =3D 0;=0D
+ NewTime.TimeZone =3D 0;=0D
+ NewTime.Daylight =3D 0;=0D
+ NewTime.Pad2 =3D 0;=0D
+ Time =3D &NewTime; // Use the new timestamp.=0D
+ }=0D
+ CopyMem (&DescriptorData->TimeStamp, Time, sizeof (EFI_TIME));=0D
+=0D
+ DescriptorData->AuthInfo.Hdr.dwLength =3D OFFSET_OF (WIN_CERTIFI=
CATE_UEFI_GUID, CertData);=0D
+ DescriptorData->AuthInfo.Hdr.wRevision =3D 0x0200;=0D
+ DescriptorData->AuthInfo.Hdr.wCertificateType =3D WIN_CERT_TYPE_EFI_GUID=
;=0D
+ CopyGuid (&DescriptorData->AuthInfo.CertType, &gEfiCertPkcs7Guid);=0D
+=0D
+ if (Payload !=3D NULL) {=0D
+ FreePool(Payload);=0D
+ }=0D
+=0D
+ *DataSize =3D DescriptorSize + PayloadSize;=0D
+ *Data =3D NewData;=0D
+ return EFI_SUCCESS;=0D
+}=0D
+=0D
+/**=0D
+ NoLock Policy tests.=0D
+**/=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestMinSizeNoLock (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT8 Value1;=0D
+ UINT32 Value2;=0D
+ UINT8 *Buffer;=0D
+=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"MinSizeNoLockVar",=0D
+ 4,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Try to write a var that is smaller than minsize=0D
+ //=0D
+ Value1 =3D 0x12;=0D
+ Status =3D gRT->SetVariable (L"MinSizeNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BO=
OTSERVICE_ACCESS),=0D
+ sizeof (Value1),=0D
+ &Value1);=0D
+ UT_ASSERT_TRUE ((Status =3D=3D EFI_WRITE_PROTECTED) || (Status =3D=3D EF=
I_INVALID_PARAMETER));=0D
+=0D
+ //=0D
+ // Try to write a var of size that matches minsize=0D
+ //=0D
+ Value2 =3D 0xa1b2c3d4;=0D
+ Status =3D gRT->SetVariable (L"MinSizeNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BO=
OTSERVICE_ACCESS),=0D
+ sizeof (Value2),=0D
+ &Value2);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Should be able to delete the var=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"MinSizeNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Try to write a var of size larger than minsize=0D
+ //=0D
+ Buffer =3D AllocateZeroPool (40);=0D
+ UT_ASSERT_NOT_NULL (Buffer);=0D
+ Status =3D gRT->SetVariable (L"MinSizeNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BO=
OTSERVICE_ACCESS),=0D
+ 40,=0D
+ Buffer);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Delete the variable=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"MinSizeNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ FreePool (Buffer);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestMinSizeNoLock=0D
+=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestMaxSizeNoLock (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT8 Value1;=0D
+ UINT32 Value2;=0D
+ UINT8 *Buffer;=0D
+=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"MaxSizeNoLockVar",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ 4,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Try to write a var that is smaller than maxsize=0D
+ //=0D
+ Value1 =3D 0x34;=0D
+ Status =3D gRT->SetVariable (L"MaxSizeNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BO=
OTSERVICE_ACCESS),=0D
+ sizeof (Value1),=0D
+ &Value1);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Should be able to delete the var=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"MaxSizeNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Try to write a var of size that matches maxsize=0D
+ //=0D
+ Value2 =3D 0xa1b2c3d4;=0D
+ Status =3D gRT->SetVariable (L"MaxSizeNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BO=
OTSERVICE_ACCESS),=0D
+ sizeof (Value2),=0D
+ &Value2);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Should be able to delete the var=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"MaxSizeNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Try to write a var of size larger than maxsize=0D
+ //=0D
+ Buffer =3D AllocateZeroPool (40);=0D
+ UT_ASSERT_NOT_NULL (Buffer);=0D
+ Status =3D gRT->SetVariable (L"MaxSizeNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BO=
OTSERVICE_ACCESS),=0D
+ 40,=0D
+ Buffer);=0D
+ UT_ASSERT_TRUE ((Status =3D=3D EFI_WRITE_PROTECTED) || (Status =3D=3D EF=
I_INVALID_PARAMETER));=0D
+=0D
+ FreePool (Buffer);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestMaxSizeNoLock=0D
+=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestMustHaveAttrNoLock (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT8 Value;=0D
+=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"MustHaveAttrNoLockVar",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_=
VARIABLE_BOOTSERVICE_ACCESS),=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Try to write a var that doesn't have the must-have attributes=0D
+ //=0D
+ Value =3D 0x56;=0D
+ Status =3D gRT->SetVariable (L"MustHaveAttrNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ EFI_VARIABLE_BOOTSERVICE_ACCESS,=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_TRUE ((Status =3D=3D EFI_WRITE_PROTECTED) || (Status =3D=3D EF=
I_INVALID_PARAMETER));=0D
+=0D
+ //=0D
+ // Try to write a var that has exactly the required attributes=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"MustHaveAttrNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BO=
OTSERVICE_ACCESS),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Should be able to delete the var=0D
+ // NOTE: some implementations of VP will require the musthave attributes=
to be passed even when deleting=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"MustHaveAttrNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Try to write a var that has the required attributes and one extra att=
ribute=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"MustHaveAttrNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BO=
OTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Should be able to delete the var=0D
+ // NOTE: some implementations of VP will require the musthave attributes=
to be passed even when deleting=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"MustHaveAttrNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestMustHaveAttrNoLock=0D
+=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestCantHaveAttrNoLock (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT8 Value;=0D
+=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"CantHaveAttrNoLockVar",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ EFI_VARIABLE_NON_VOLATILE,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Try to write a var that has a can't have attr=0D
+ //=0D
+ Value =3D 0x78;=0D
+ Status =3D gRT->SetVariable (L"CantHaveAttrNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_TRUE ((Status =3D=3D EFI_WRITE_PROTECTED) || (Status =3D=3D EF=
I_INVALID_PARAMETER));=0D
+=0D
+ //=0D
+ // Try to write a var that satisfies the can't have requirement=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"CantHaveAttrNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ EFI_VARIABLE_BOOTSERVICE_ACCESS,=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Should be able to delete the var=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"CantHaveAttrNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestCantHaveAttrNoLock=0D
+=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestMaxSizeNamespaceNoLock (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT8 Value1;=0D
+ UINT32 Value2;=0D
+ UINT8 *Buffer;=0D
+=0D
+ //=0D
+ // Register a namespace-wide policy limiting max size to 4 bytes=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid2,=0D
+ NULL,=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ 4,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Try to write a var that is smaller than maxsize=0D
+ //=0D
+ Value1 =3D 0x34;=0D
+ Status =3D gRT->SetVariable (L"MaxSizeNoLockVar",=0D
+ &mTestNamespaceGuid2,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BO=
OTSERVICE_ACCESS),=0D
+ sizeof (Value1),=0D
+ &Value1);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Should be able to delete the var=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"MaxSizeNoLockVar",=0D
+ &mTestNamespaceGuid2,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Try to write a var of size that matches maxsize=0D
+ //=0D
+ Value2 =3D 0xa1b2c3d4;=0D
+ Status =3D gRT->SetVariable (L"MaxSizeNoLockVar",=0D
+ &mTestNamespaceGuid2,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BO=
OTSERVICE_ACCESS),=0D
+ sizeof (Value2),=0D
+ &Value2);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Should be able to delete the var=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"MaxSizeNoLockVar",=0D
+ &mTestNamespaceGuid2,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Try to write a var of size larger than maxsize=0D
+ //=0D
+ Buffer =3D AllocateZeroPool (40);=0D
+ UT_ASSERT_NOT_NULL (Buffer);=0D
+ Status =3D gRT->SetVariable (L"MaxSizeNoLockVar",=0D
+ &mTestNamespaceGuid2,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BO=
OTSERVICE_ACCESS),=0D
+ 40,=0D
+ Buffer);=0D
+ UT_ASSERT_TRUE ((Status =3D=3D EFI_WRITE_PROTECTED) || (Status =3D=3D EF=
I_INVALID_PARAMETER));=0D
+=0D
+ FreePool (Buffer);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestMaxSizeNamespaceNoLock=0D
+=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestMustHaveAttrWildcardNoLock (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT8 Value;=0D
+=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"MustHaveAttrWildcardNoLockVar###=
#",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_=
VARIABLE_BOOTSERVICE_ACCESS),=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Try to write a var that doesn't have the must-have attributes=0D
+ //=0D
+ Value =3D 0x56;=0D
+ Status =3D gRT->SetVariable (L"MustHaveAttrWildcardNoLockVar1573",=0D
+ &mTestNamespaceGuid1,=0D
+ EFI_VARIABLE_BOOTSERVICE_ACCESS,=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_TRUE ((Status =3D=3D EFI_WRITE_PROTECTED) || (Status =3D=3D EF=
I_INVALID_PARAMETER));=0D
+=0D
+ //=0D
+ // Try to write a var that has exactly the required attributes=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"MustHaveAttrWildcardNoLockVar1234",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BO=
OTSERVICE_ACCESS),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Should be able to delete the var=0D
+ // NOTE: some implementations of VP will require the musthave attributes=
to be passed even when deleting=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"MustHaveAttrWildcardNoLockVar1234",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Try to write a var that has the required attributes and one extra att=
ribute=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"MustHaveAttrWildcardNoLockVar5612",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BO=
OTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Should be able to delete the var=0D
+ // NOTE: some implementations of VP will require the musthave attributes=
to be passed even when deleting=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"MustHaveAttrWildcardNoLockVar5612",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestMustHaveAttrWildcardNoLock=0D
+=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestPolicyprioritizationNoLock (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT8 Value8;=0D
+ UINT16 Value16;=0D
+ UINT32 Value32;=0D
+ UINT64 Value64;=0D
+=0D
+ //=0D
+ // Register a policy targeting the specific var=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid3,=0D
+ L"PolicyPriorityTestVar123",=0D
+ 8, // min size of UINT64=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Register a policy with wildcards in the name=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid3,=0D
+ L"PolicyPriorityTestVar###",=0D
+ 4, // min size of UINT32=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Register a policy with wildcards in the name=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid3,=0D
+ NULL,=0D
+ 2, // min size of UINT16=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // The idea is that the most specific policy is applied:=0D
+ // For varname "TestVar", the namespace-wide one should apply: UINT16 =
minimum=0D
+ // For varname "PolicyPriorityTestVar567" the wildcard policy should a=
pply: UINT32 minimum=0D
+ // For varname "PolicyPriorityTestVar123" the var-specific policy shou=
ld apply: UINT64 minimum=0D
+ //=0D
+=0D
+ //=0D
+ // Let's confirm the namespace-wide policy enforcement=0D
+ //=0D
+ Value8 =3D 0x78;=0D
+ Status =3D gRT->SetVariable (L"TestVar",=0D
+ &mTestNamespaceGuid3,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value8),=0D
+ &Value8);=0D
+ UT_ASSERT_TRUE ((Status =3D=3D EFI_WRITE_PROTECTED) || (Status =3D=3D EF=
I_INVALID_PARAMETER));=0D
+=0D
+ Value16 =3D 0x6543;=0D
+ Status =3D gRT->SetVariable (L"TestVar",=0D
+ &mTestNamespaceGuid3,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value16),=0D
+ &Value16);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Let's confirm the wildcard policy enforcement=0D
+ //=0D
+ Value16 =3D 0xabba;=0D
+ Status =3D gRT->SetVariable (L"PolicyPriorityTestVar567",=0D
+ &mTestNamespaceGuid3,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value16),=0D
+ &Value16);=0D
+ UT_ASSERT_TRUE ((Status =3D=3D EFI_WRITE_PROTECTED) || (Status =3D=3D EF=
I_INVALID_PARAMETER));=0D
+=0D
+ Value32 =3D 0xfedcba98;=0D
+ Status =3D gRT->SetVariable (L"PolicyPriorityTestVar567",=0D
+ &mTestNamespaceGuid3,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value32),=0D
+ &Value32);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Let's confirm the var-specific policy enforcement=0D
+ //=0D
+ Value32 =3D 0x8d3f627c;=0D
+ Status =3D gRT->SetVariable (L"PolicyPriorityTestVar123",=0D
+ &mTestNamespaceGuid3,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value32),=0D
+ &Value32);=0D
+ UT_ASSERT_TRUE ((Status =3D=3D EFI_WRITE_PROTECTED) || (Status =3D=3D EF=
I_INVALID_PARAMETER));=0D
+=0D
+ Value64 =3D 0xbebecdcdafaf6767;=0D
+ Status =3D gRT->SetVariable (L"PolicyPriorityTestVar123",=0D
+ &mTestNamespaceGuid3,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value64),=0D
+ &Value64);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestPolicyprioritizationNoLock=0D
+=0D
+/**=0D
+ LockNow Policy tests.=0D
+**/=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestExistingVarLockNow (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT8 Value;=0D
+=0D
+ //=0D
+ // Write a var that we'll protect next=0D
+ //=0D
+ Value =3D 0x78;=0D
+ Status =3D gRT->SetVariable (L"ExistingLockNowVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Register a LockNow policy targeting the var=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"ExistingLockNowVar",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_LOCK_NOW);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Attempt to modify the locked var=0D
+ //=0D
+ Value =3D 0xA5;=0D
+ Status =3D gRT->SetVariable (L"ExistingLockNowVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_STATUS_EQUAL (Status, EFI_WRITE_PROTECTED);=0D
+=0D
+ //=0D
+ // Attempt to delete the locked var=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"ExistingLockNowVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_STATUS_EQUAL (Status, EFI_WRITE_PROTECTED);=0D
+=0D
+ //=0D
+ // This variable is deleted in final cleanup.=0D
+ //=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestExistingVarLockNow=0D
+=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestNonexistentVarLockNow (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT8 Value;=0D
+ UINTN Size;=0D
+=0D
+ //=0D
+ // Make sure the variable we're about to create the policy for doesn't e=
xist=0D
+ //=0D
+ Size =3D 0;=0D
+ Status =3D gRT->GetVariable (L"NonexistentLockNowVar",=0D
+ &mTestNamespaceGuid1,=0D
+ NULL,=0D
+ &Size,=0D
+ NULL);=0D
+ UT_ASSERT_STATUS_EQUAL (Status, EFI_NOT_FOUND);=0D
+=0D
+ //=0D
+ // Register a LockNow policy targeting the var=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"NonexistentLockNowVar",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_LOCK_NOW);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Attempt to create the locked var=0D
+ //=0D
+ Value =3D 0xA5;=0D
+ Status =3D gRT->SetVariable (L"NonexistentLockNowVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_STATUS_EQUAL (Status, EFI_WRITE_PROTECTED);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestNonexistentVarLockNow=0D
+=0D
+/**=0D
+ LockOnCreate Policy tests.=0D
+**/=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestExistingVarLockOnCreate (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT8 Value;=0D
+=0D
+ //=0D
+ // Write a var that we'll protect later=0D
+ //=0D
+ Value =3D 0x78;=0D
+ Status =3D gRT->SetVariable (L"ExistingLockOnCreateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Register a LockNow policy targeting the var=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"ExistingLockOnCreateVar",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_LOCK_ON_CREAT=
E);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Attempt to modify the locked var=0D
+ //=0D
+ Value =3D 0xA5;=0D
+ Status =3D gRT->SetVariable (L"ExistingLockOnCreateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_STATUS_EQUAL (Status, EFI_WRITE_PROTECTED);=0D
+=0D
+ //=0D
+ // Attempt to delete the locked var=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"ExistingLockOnCreateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_STATUS_EQUAL (Status, EFI_WRITE_PROTECTED);=0D
+=0D
+ //=0D
+ // This variable is deleted in final cleanup.=0D
+ //=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestExistingVarLockOnCreate=0D
+=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestNonexistentVarLockOnCreate (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT8 Value1;=0D
+ UINT32 Value2;=0D
+ UINTN Size;=0D
+=0D
+ //=0D
+ // Make sure the variable we're about to create the policy for doesn't e=
xist=0D
+ //=0D
+ Size =3D 0;=0D
+ Status =3D gRT->GetVariable (L"NonexistentLockOnCreateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ NULL,=0D
+ &Size,=0D
+ NULL);=0D
+ UT_ASSERT_STATUS_EQUAL (Status, EFI_NOT_FOUND);=0D
+=0D
+ //=0D
+ // Register a LockOnCreate policy targeting the var=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"NonexistentLockOnCreateVar",=0D
+ 2, // min size of 2 bytes, UINT16+=
=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ EFI_VARIABLE_RUNTIME_ACCESS, // mu=
st have RT attr=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_LOCK_ON_CREAT=
E);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Attempt to create the var, but smaller than min size=0D
+ //=0D
+ Value1 =3D 0xA5;=0D
+ Status =3D gRT->SetVariable (L"NonexistentLockOnCreateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE),=0D
+ sizeof (Value1),=0D
+ &Value1);=0D
+ UT_ASSERT_TRUE ((Status =3D=3D EFI_WRITE_PROTECTED) || (Status =3D=3D EF=
I_INVALID_PARAMETER));=0D
+=0D
+ //=0D
+ // Now let's make sure attribute req is enforced=0D
+ //=0D
+ Value2 =3D 0x43218765;=0D
+ Status =3D gRT->SetVariable (L"NonexistentLockOnCreateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value2),=0D
+ &Value2);=0D
+ UT_ASSERT_TRUE ((Status =3D=3D EFI_WRITE_PROTECTED) || (Status =3D=3D EF=
I_INVALID_PARAMETER));=0D
+=0D
+ //=0D
+ // Now let's create a valid variable=0D
+ //=0D
+ Value2 =3D 0x43218765;=0D
+ Status =3D gRT->SetVariable (L"NonexistentLockOnCreateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE),=0D
+ sizeof (Value2),=0D
+ &Value2);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Let's make sure we can't modify it=0D
+ //=0D
+ Value2 =3D 0xa5a5b6b6;=0D
+ Status =3D gRT->SetVariable (L"NonexistentLockOnCreateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE),=0D
+ sizeof (Value2),=0D
+ &Value2);=0D
+ UT_ASSERT_STATUS_EQUAL (Status, EFI_WRITE_PROTECTED);=0D
+=0D
+ //=0D
+ // Finally, let's make sure we can't delete it=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"NonexistentLockOnCreateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_STATUS_EQUAL (Status, EFI_WRITE_PROTECTED);=0D
+=0D
+ //=0D
+ // This variable is deleted in final cleanup.=0D
+ //=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestNonexistentVarLockOnCreate=0D
+=0D
+/**=0D
+ LockOnVarState Policy tests.=0D
+**/=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestLockOnVarStateBeforeCreate (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINTN Size;=0D
+ UINT8 Value;=0D
+=0D
+ //=0D
+ // First of all, let's make sure the var we're trying to protect doesn't=
exist=0D
+ //=0D
+ Size =3D 0;=0D
+ Status =3D gRT->GetVariable (L"NonexistentLockOnVarStateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ NULL,=0D
+ &Size,=0D
+ NULL);=0D
+ UT_ASSERT_STATUS_EQUAL (Status, EFI_NOT_FOUND);=0D
+=0D
+ //=0D
+ // Good, now let's create a policy=0D
+ //=0D
+ Status =3D RegisterVarStateVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"NonexistentLockOnVarStateVar"=
,=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ &mTestNamespaceGuid1,=0D
+ L"Trigger1",=0D
+ 0x7E);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Now we write the trigger var=0D
+ //=0D
+ Value =3D 0x7E;=0D
+ Status =3D gRT->SetVariable (L"Trigger1",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Ok, now we attempt to write a var protected by the trigger=0D
+ //=0D
+ Value =3D 0xFA;=0D
+ Status =3D gRT->SetVariable (L"NonexistentLockOnVarStateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_STATUS_EQUAL (Status, EFI_WRITE_PROTECTED);=0D
+=0D
+ //=0D
+ // Let's modify the trigger var and "untrigger" the policy=0D
+ //=0D
+ Value =3D 0x38;=0D
+ Status =3D gRT->SetVariable (L"Trigger1",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Now we should be able to create the var targeted by the policy=0D
+ //=0D
+ Value =3D 0x23;=0D
+ Status =3D gRT->SetVariable (L"NonexistentLockOnVarStateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Cleanup: delete the trigger and the protected var=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"Trigger1",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ Status =3D gRT->SetVariable (L"NonexistentLockOnVarStateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestLockOnVarStateBeforeCreate=0D
+=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestLockOnVarStateAfterCreate (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT8 Value;=0D
+=0D
+ //=0D
+ // Let's create a policy=0D
+ //=0D
+ Status =3D RegisterVarStateVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"ExistingLockOnVarStateVar",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ &mTestNamespaceGuid1,=0D
+ L"Trigger2",=0D
+ 0x5C);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Should be able to write targeted var since the policy isn't active ye=
t.=0D
+ //=0D
+ Value =3D 0x17;=0D
+ Status =3D gRT->SetVariable (L"ExistingLockOnVarStateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Let's modify the var to make sure the policy isn't acting like a lock=
-on-create one=0D
+ //=0D
+ Value =3D 0x30;=0D
+ Status =3D gRT->SetVariable (L"ExistingLockOnVarStateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Now we trigger the policy=0D
+ //=0D
+ Value =3D 0x5C;=0D
+ Status =3D gRT->SetVariable (L"Trigger2",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Let's now verify the variable is protected=0D
+ //=0D
+ Value =3D 0xB9;=0D
+ Status =3D gRT->SetVariable (L"ExistingLockOnVarStateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_STATUS_EQUAL (Status, EFI_WRITE_PROTECTED);=0D
+=0D
+ //=0D
+ // Ok, to clean up, we need to remove the trigger var, so delete it, and=
then delete the target var=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"Trigger2",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ Status =3D gRT->SetVariable (L"ExistingLockOnVarStateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestLockOnVarStateAfterCreate=0D
+=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestLockOnVarStateInvalidLargeTrigger (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT16 Value;=0D
+=0D
+ //=0D
+ // First let's create a variable policy=0D
+ //=0D
+ Status =3D RegisterVarStateVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"InvalidLargeTriggerLockOnVarS=
tateVar",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ &mTestNamespaceGuid1,=0D
+ L"Trigger3",=0D
+ 0x5C);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Now attempt to trigger the lock but with a variable larger than one b=
yte=0D
+ //=0D
+ Value =3D 0x8085;=0D
+ Status =3D gRT->SetVariable (L"Trigger3",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Should still be able to create the targeted var=0D
+ //=0D
+ Value =3D 0x1234;=0D
+ Status =3D gRT->SetVariable (L"InvalidLargeTriggerLockOnVarStateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Let's clean up by deleting the invalid trigger and the targeted var=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"Trigger3",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ Status =3D gRT->SetVariable (L"InvalidLargeTriggerLockOnVarStateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestLockOnVarStateInvalidLargeTrigger=0D
+=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestLockOnVarStateWrongValueTrigger (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT8 Value;=0D
+=0D
+ //=0D
+ // First let's create a variable policy=0D
+ //=0D
+ Status =3D RegisterVarStateVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"WrongValueTriggerLockOnVarSta=
teVar",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ &mTestNamespaceGuid1,=0D
+ L"Trigger4",=0D
+ 0xCA);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Now attempt to trigger the lock but with a wrong value=0D
+ //=0D
+ Value =3D 0x80;=0D
+ Status =3D gRT->SetVariable (L"Trigger4",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Should still be able to create the targeted var=0D
+ //=0D
+ Value =3D 0x14;=0D
+ Status =3D gRT->SetVariable (L"WrongValueTriggerLockOnVarStateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Let's clean up by deleting the invalid trigger and the targeted var=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"Trigger4",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ Status =3D gRT->SetVariable (L"WrongValueTriggerLockOnVarStateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestLockOnVarStateWrongValueTrigger=0D
+=0D
+/**=0D
+ Invalid policy tests.=0D
+**/=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestInvalidAttributesPolicy (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+=0D
+ //=0D
+ // The only must/can't have attributes supported by VPE are NV, BS, and =
RT. They are 1, 2, and 4, respectively.=0D
+ // Let's try some bits higher than that?=0D
+ //=0D
+=0D
+ //=0D
+ // Trying must have attribute 0x8 which is EFI_VARIABLE_HARDWARE_ERROR_R=
ECORD=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"InvalidMustHaveAttributesPolicyV=
ar1",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ EFI_VARIABLE_HARDWARE_ERROR_RECORD=
,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ UT_LOG_INFO ("Setting must have attr to EFI_VARIABLE_HARDWARE_ERROR_RECO=
RD returned %r\n", Status);=0D
+=0D
+ //=0D
+ // Let's try 0x10 - EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS, a deprecate=
d attribute=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"InvalidMustHaveAttributesPolicyV=
ar2",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ EFI_VARIABLE_AUTHENTICATED_WRITE_A=
CCESS,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ UT_LOG_INFO ("Setting must have attr to EFI_VARIABLE_AUTHENTICATED_WRITE=
_ACCESS returned %r\n", Status);=0D
+=0D
+ //=0D
+ // Let's try 0x20 - EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"InvalidMustHaveAttributesPolicyV=
ar3",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ EFI_VARIABLE_TIME_BASED_AUTHENTICA=
TED_WRITE_ACCESS,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ UT_LOG_INFO ("Setting must have attr to EFI_VARIABLE_TIME_BASED_AUTHENTI=
CATED_WRITE_ACCESS returned %r\n", Status);=0D
+=0D
+ //=0D
+ // Let's try something wild, like 0x4000=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"InvalidMustHaveAttributesPolicyV=
ar4",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ 0x4000,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ UT_LOG_INFO ("Setting must have attr to 0x4000 returned %r\n", Status);=
=0D
+=0D
+ //=0D
+ // Now repeat the same tests, but for the can't-have param=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"InvalidCantHaveAttributesPolicyV=
ar1",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ EFI_VARIABLE_HARDWARE_ERROR_RECORD=
,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ UT_LOG_INFO ("Setting cant have attr to EFI_VARIABLE_HARDWARE_ERROR_RECO=
RD returned %r\n", Status);=0D
+=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"InvalidCantHaveAttributesPolicyV=
ar2",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ EFI_VARIABLE_AUTHENTICATED_WRITE_A=
CCESS,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ UT_LOG_INFO ("Setting cant have attr to EFI_VARIABLE_AUTHENTICATED_WRITE=
_ACCESS returned %r\n", Status);=0D
+=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"InvalidCantHaveAttributesPolicyV=
ar3",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ EFI_VARIABLE_TIME_BASED_AUTHENTICA=
TED_WRITE_ACCESS,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ UT_LOG_INFO ("Setting cant have attr to EFI_VARIABLE_TIME_BASED_AUTHENTI=
CATED_WRITE_ACCESS returned %r\n", Status);=0D
+=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"InvalidCantHaveAttributesPolicyV=
ar4",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ 0x4000,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ UT_LOG_INFO ("Setting cant have attr to 0x4000 returned %r\n", Status);=
=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestInvalidAttributesPolicy=0D
+=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestLargeMinSizePolicy (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+=0D
+ //=0D
+ // Let's set the min size to 2GB and see what happens=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"LargeMinSizeInvalidPolicyVar",=0D
+ 0x80000000,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+=0D
+ UT_LOG_INFO ("Setting min size to 0x80000000 returned %r\n", Status);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestLargeMinSizePolicy=0D
+=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestZeroMaxSizePolicy (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+=0D
+ //=0D
+ // Let's set the max size to 0 and see what happens=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"ZeroMinSizeInvalidPolicyVar",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ 0,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK);=0D
+ //UT_ASSERT_NOT_EQUAL (Status, EFI_SUCCESS); // this fails on QC. Real b=
ug? Do we care?=0D
+ UT_LOG_INFO ("Setting max size to 0 returned %r\n", Status);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestZeroMaxSizePolicy=0D
+=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestInvalidPolicyTypePolicy (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+=0D
+ //=0D
+ // Let's set policy type to an invalid value and see what happens=0D
+ // Valid ones are:=0D
+ // VARIABLE_POLICY_TYPE_NO_LOCK 0=0D
+ // VARIABLE_POLICY_TYPE_LOCK_NOW 1=0D
+ // VARIABLE_POLICY_TYPE_LOCK_ON_CREATE 2=0D
+ // VARIABLE_POLICY_TYPE_LOCK_ON_VAR_STATE 3=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"InvalidPolicyTypePolicyVar",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ 4);=0D
+ UT_ASSERT_NOT_EQUAL (Status, EFI_SUCCESS);=0D
+=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"InvalidPolicyTypePolicyVar",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ 147);=0D
+ UT_ASSERT_NOT_EQUAL (Status, EFI_SUCCESS);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestInvalidPolicyTypePolicy=0D
+=0D
+/**=0D
+ Test dumping policy.=0D
+**/=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestDumpPolicy (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT8* Buffer;=0D
+ UINT32 Size;=0D
+=0D
+ //=0D
+ // First let's call DumpVariablePolicy with null buffer to get size=0D
+ //=0D
+ Size =3D 0;=0D
+ Status =3D mVarPol->DumpVariablePolicy (NULL, &Size);=0D
+ UT_ASSERT_STATUS_EQUAL (Status, EFI_BUFFER_TOO_SMALL);=0D
+=0D
+ //=0D
+ // Now we allocate the buffer for the dump=0D
+ //=0D
+ Buffer =3D NULL;=0D
+ Buffer =3D AllocatePool (Size);=0D
+ UT_ASSERT_NOT_NULL (Buffer);=0D
+=0D
+ //=0D
+ // Now we get the dump. In this test we will not analyze the dump.=0D
+ //=0D
+ Status =3D mVarPol->DumpVariablePolicy (Buffer, &Size);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestDumpPolicy=0D
+=0D
+/**=0D
+ Test policy version.=0D
+**/=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+TestPolicyVersion (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ VARIABLE_POLICY_ENTRY *NewEntry;=0D
+=0D
+ //=0D
+ // Create the new entry using a helper lib=0D
+ //=0D
+ NewEntry =3D NULL;=0D
+ Status =3D CreateBasicVariablePolicy (&mTestNamespaceGuid1,=0D
+ L"PolicyVersionTestNoLockVar",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ 4, // max size of 4 bytes=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_NO_LOCK,=0D
+ &NewEntry);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ NewEntry->Version =3D 0x1234;=0D
+ Status =3D mVarPol->RegisterVariablePolicy (NewEntry);=0D
+ UT_LOG_INFO ("Registering policy entry with an unknown version status: %=
r\n", Status);=0D
+=0D
+ FreePool (NewEntry);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // TestPolicyVersion=0D
+=0D
+/**=0D
+ Lock Policy Tests.=0D
+**/=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+LockPolicyEngineTests (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT16 Value;=0D
+ UINT64 Value64;=0D
+ BOOLEAN State;=0D
+=0D
+ //=0D
+ // First let's register a policy that we'll test after VPE lock=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"BeforeVpeLockNoLockPolicyVar",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ 4, // max size of 4 bytes=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_LOCK_ON_CREAT=
E);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Now, lock VPE!=0D
+ //=0D
+ Status =3D mVarPol->LockVariablePolicy ();=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // See if we can lock it again?=0D
+ //=0D
+ Status =3D mVarPol->LockVariablePolicy ();=0D
+ UT_LOG_INFO ("Locking VPE for second time returned %r\n", Status);=0D
+=0D
+ //=0D
+ // Let's confirm one of the policies from prior test suites is still enf=
orced=0D
+ // Attempt to delete a locked var=0D
+ //=0D
+ Status =3D gRT->SetVariable (L"ExistingLockNowVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_ASSERT_STATUS_EQUAL (Status, EFI_WRITE_PROTECTED);=0D
+=0D
+ //=0D
+ // We'll make sure the policy from earlier in this test case is actively=
filtering out by size=0D
+ //=0D
+ Value64 =3D 0x3829fed212345678;=0D
+ Status =3D gRT->SetVariable (L"BeforeVpeLockNoLockPolicyVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BO=
OTSERVICE_ACCESS),=0D
+ sizeof (Value64),=0D
+ &Value64);=0D
+ UT_ASSERT_TRUE ((Status =3D=3D EFI_WRITE_PROTECTED) || (Status =3D=3D EF=
I_INVALID_PARAMETER));=0D
+=0D
+ //=0D
+ // Let's create the variable from the policy now=0D
+ //=0D
+ Value =3D 0x323f;=0D
+ Status =3D gRT->SetVariable (L"BeforeVpeLockNoLockPolicyVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BO=
OTSERVICE_ACCESS),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Now confirm that the var is locked after creation=0D
+ //=0D
+ Value =3D 0x1212;=0D
+ Status =3D gRT->SetVariable (L"BeforeVpeLockNoLockPolicyVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BO=
OTSERVICE_ACCESS),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_STATUS_EQUAL (Status, EFI_WRITE_PROTECTED);=0D
+=0D
+ //=0D
+ // Let's attempt to register a new policy, it should fail=0D
+ //=0D
+ Status =3D RegisterBasicVariablePolicy (mVarPol,=0D
+ &mTestNamespaceGuid1,=0D
+ L"AfterVpeLockNowPolicyVar",=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_LOCK_NOW);=0D
+ UT_ASSERT_NOT_EQUAL (Status, EFI_SUCCESS);=0D
+=0D
+ //=0D
+ // Make sure VPE is enabled=0D
+ //=0D
+ Status =3D mVarPol->IsVariablePolicyEnabled (&State);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+ UT_ASSERT_EQUAL (State, TRUE);=0D
+=0D
+ //=0D
+ // Finally, make sure we can't disable VPE=0D
+ //=0D
+ Status =3D mVarPol->DisableVariablePolicy ();=0D
+ UT_ASSERT_NOT_EQUAL (Status, EFI_SUCCESS);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // LockPolicyEngineTests=0D
+=0D
+/**=0D
+ Save context and reboot after the lock policy test suite.=0D
+**/=0D
+STATIC=0D
+VOID=0D
+EFIAPI=0D
+SaveStateAndReboot (=0D
+ VOID=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+=0D
+ //=0D
+ // Now, save all the data associated with this framework.=0D
+ // TODO: Need to add to the UnitTestFrameworkPkg=0D
+ Status =3D SaveFrameworkState( NULL, 0 );=0D
+=0D
+ //=0D
+ // If we're all good, let's book...=0D
+ if (!EFI_ERROR( Status ))=0D
+ {=0D
+ //=0D
+ // Next, we want to update the BootNext variable to USB=0D
+ // so that we have a fighting chance of coming back here.=0D
+ //=0D
+ // TODO: Need to add to the UnitTestFrameworkPkg=0D
+ // SetBootNextDevice();=0D
+=0D
+ //=0D
+ // Reset=0D
+ DEBUG(( DEBUG_INFO, "%a - Rebooting! Launch this test again once boote=
d.\n", __FUNCTION__ ));=0D
+ gRT->ResetSystem( EfiResetCold, EFI_SUCCESS, 0, NULL );=0D
+ DEBUG(( DEBUG_ERROR, "%a - Unit test failed to quit! Framework can no =
longer be used!\n", __FUNCTION__ ));=0D
+=0D
+ //=0D
+ // We REALLY shouldn't be here.=0D
+ Status =3D EFI_ABORTED;=0D
+ }=0D
+=0D
+ return;=0D
+} // SaveContextAndReboot=0D
+=0D
+STATIC=0D
+VOID=0D
+IgnoreContextAndReboot (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ // Just a wrapper for prototype reuse.=0D
+ SaveStateAndReboot();=0D
+}=0D
+=0D
+/**=0D
+ Disable policy tests.=0D
+**/=0D
+UNIT_TEST_STATUS=0D
+EFIAPI=0D
+DisablePolicyEngineTests (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ BOOLEAN State;=0D
+ UINT8 Value;=0D
+=0D
+ //=0D
+ // First, we disable the variable policy=0D
+ //=0D
+ Status =3D mVarPol->DisableVariablePolicy ();=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ //=0D
+ // Confirm it's disabled=0D
+ //=0D
+ Status =3D mVarPol->IsVariablePolicyEnabled (&State);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+ UT_ASSERT_EQUAL (State, FALSE);=0D
+=0D
+ //=0D
+ // Try locking it?=0D
+ //=0D
+ Status =3D mVarPol->LockVariablePolicy ();=0D
+ UT_LOG_INFO ("Locking VP after disabling it status: %r\n", Status);=0D
+=0D
+ //=0D
+ // Try modifying the var from TestExistingVarLockNow=0D
+ //=0D
+ Value =3D 0xB5;=0D
+ Status =3D gRT->SetVariable (L"ExistingLockNowVar",=0D
+ &mTestNamespaceGuid1,=0D
+ (EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIAB=
LE_NON_VOLATILE),=0D
+ sizeof (Value),=0D
+ &Value);=0D
+ UT_ASSERT_NOT_EFI_ERROR (Status);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+} // DisablePolicyEngineTests=0D
+=0D
+//=0D
+// Pre-Disable Setup and Test for Authenticated Variables=0D
+//=0D
+UNIT_TEST_STATUS=0D
+TestAuthVarPart1 (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT32 Data;=0D
+ UINTN DataSize;=0D
+ UINT8 *DeleteData;=0D
+=0D
+ // First, we need to create our dummy Authenticated Variable.=0D
+ Status =3D gRT->SetVariable(TEST_AUTH_VAR_NAME,=0D
+ &mTestAuthNamespaceGuid,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOT=
SERVICE_ACCESS |=0D
+ EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_T=
IME_BASED_AUTHENTICATED_WRITE_ACCESS),=0D
+ mTestAuthVarPayloadSize,=0D
+ &mTestAuthVarPayload[0]);=0D
+ UT_ASSERT_NOT_EFI_ERROR(Status);=0D
+=0D
+ // Prove that we created it.=0D
+ DataSize =3D sizeof(Data);=0D
+ Status =3D gRT->GetVariable(TEST_AUTH_VAR_NAME,=0D
+ &mTestAuthNamespaceGuid,=0D
+ NULL,=0D
+ &DataSize,=0D
+ &Data);=0D
+ UT_ASSERT_NOT_EFI_ERROR(Status);=0D
+ UT_ASSERT_EQUAL(Data, 0xDEADBEEF);=0D
+=0D
+ // Prove that we cannot delete it.=0D
+ DeleteData =3D NULL;=0D
+ DataSize =3D 0;=0D
+ Status =3D CreateEmptyTimeBasedPayload(&DataSize, &DeleteData, NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR(Status);=0D
+ Status =3D gRT->SetVariable(TEST_AUTH_VAR_NAME,=0D
+ &mTestAuthNamespaceGuid,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOT=
SERVICE_ACCESS |=0D
+ EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_T=
IME_BASED_AUTHENTICATED_WRITE_ACCESS),=0D
+ DataSize,=0D
+ (VOID*)DeleteData);=0D
+ UT_ASSERT_STATUS_EQUAL(Status, EFI_SECURITY_VIOLATION);=0D
+ FreePool(DeleteData);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+}=0D
+=0D
+//=0D
+// Post-Disable Test for Authenticated Variables=0D
+//=0D
+UNIT_TEST_STATUS=0D
+TestAuthVarPart2 (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UINT32 Data;=0D
+ UINTN DataSize;=0D
+ UINT8 *DeleteData;=0D
+=0D
+ // Prove that it exists.=0D
+ DataSize =3D sizeof(Data);=0D
+ Status =3D gRT->GetVariable(TEST_AUTH_VAR_NAME,=0D
+ &mTestAuthNamespaceGuid,=0D
+ NULL,=0D
+ &DataSize,=0D
+ &Data);=0D
+ UT_ASSERT_NOT_EFI_ERROR(Status);=0D
+ UT_ASSERT_EQUAL(Data, 0xDEADBEEF);=0D
+=0D
+ // Prove that we can delete it.=0D
+ DeleteData =3D NULL;=0D
+ DataSize =3D 0;=0D
+ Status =3D CreateEmptyTimeBasedPayload(&DataSize, &DeleteData, NULL);=0D
+ UT_ASSERT_NOT_EFI_ERROR(Status);=0D
+ Status =3D gRT->SetVariable(TEST_AUTH_VAR_NAME,=0D
+ &mTestAuthNamespaceGuid,=0D
+ (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOT=
SERVICE_ACCESS |=0D
+ EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_T=
IME_BASED_AUTHENTICATED_WRITE_ACCESS),=0D
+ DataSize,=0D
+ (VOID*)DeleteData);=0D
+ UT_ASSERT_NOT_EFI_ERROR(Status);=0D
+ FreePool(DeleteData);=0D
+=0D
+ // Prove that we deleted it.=0D
+ DataSize =3D sizeof(Data);=0D
+ Status =3D gRT->GetVariable(TEST_AUTH_VAR_NAME,=0D
+ &mTestAuthNamespaceGuid,=0D
+ NULL,=0D
+ &DataSize,=0D
+ &Data);=0D
+ UT_ASSERT_STATUS_EQUAL(Status, EFI_NOT_FOUND);=0D
+=0D
+ return UNIT_TEST_PASSED;=0D
+}=0D
+=0D
+/**=0D
+ Final Cleanup: delete some variables earlier test cases created.=0D
+**/=0D
+STATIC=0D
+VOID=0D
+EFIAPI=0D
+FinalCleanup (=0D
+ IN UNIT_TEST_CONTEXT Context=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+=0D
+ Status =3D gRT->SetVariable (L"ExistingLockNowVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_LOG_INFO ("Delete ExistingLockNowVar status: %r\n", Status);=0D
+=0D
+ Status =3D gRT->SetVariable (L"ExistingLockOnCreateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_LOG_INFO ("Delete ExistingLockOnCreateVar status: %r\n", Status);=0D
+=0D
+ Status =3D gRT->SetVariable (L"NonexistentLockOnCreateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_LOG_INFO ("Delete NonexistentLockOnCreateVar status: %r\n", Status);=
=0D
+=0D
+ Status =3D gRT->SetVariable (L"NonexistentLockNowVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_LOG_INFO ("Delete NonexistentLockNowVar status: %r\n", Status);=0D
+=0D
+ Status =3D gRT->SetVariable (L"CantHaveAttrNoLockVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_LOG_INFO ("Delete CantHaveAttrNoLockVar status: %r\n", Status);=0D
+=0D
+ Status =3D gRT->SetVariable (L"NonexistentLockOnVarStateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_LOG_INFO ("Delete NonexistentLockOnVarStateVar status: %r\n", Status)=
;=0D
+=0D
+ Status =3D gRT->SetVariable (L"ExistingLockOnVarStateVar",=0D
+ &mTestNamespaceGuid1,=0D
+ 0,=0D
+ 0,=0D
+ NULL);=0D
+ UT_LOG_INFO ("Delete ExistingLockOnVarStateVar status: %r\n", Status);=0D
+} // FinalCleanup=0D
+=0D
+/**=0D
+=0D
+ Main fuction sets up the unit test environment.=0D
+=0D
+**/=0D
+EFI_STATUS=0D
+EFIAPI=0D
+UefiMain (=0D
+ IN EFI_HANDLE ImageHandle,=0D
+ IN EFI_SYSTEM_TABLE* SystemTable)=0D
+{=0D
+ EFI_STATUS Status;=0D
+ UNIT_TEST_FRAMEWORK_HANDLE Framework;=0D
+ UNIT_TEST_SUITE_HANDLE GettingStartedTestSuite;=0D
+ UNIT_TEST_SUITE_HANDLE NoLockPoliciesTestSuite;=0D
+ UNIT_TEST_SUITE_HANDLE LockNowPoliciesTestSuite;=0D
+ UNIT_TEST_SUITE_HANDLE LockOnCreatePoliciesTestSuite;=0D
+ UNIT_TEST_SUITE_HANDLE LockOnVarStatePoliciesTestSuite;=0D
+ UNIT_TEST_SUITE_HANDLE InvalidPoliciesTestSuite;=0D
+ UNIT_TEST_SUITE_HANDLE DumpPolicyTestSuite;=0D
+ UNIT_TEST_SUITE_HANDLE PolicyVersionTestSuite;=0D
+ UNIT_TEST_SUITE_HANDLE LockPolicyTestSuite;=0D
+ UNIT_TEST_SUITE_HANDLE DisablePolicyTestSuite;=0D
+=0D
+ Framework =3D NULL;=0D
+ GettingStartedTestSuite =3D NULL;=0D
+ NoLockPoliciesTestSuite =3D NULL;=0D
+ LockNowPoliciesTestSuite =3D NULL;=0D
+ LockOnCreatePoliciesTestSuite =3D NULL;=0D
+ LockOnVarStatePoliciesTestSuite =3D NULL;=0D
+ InvalidPoliciesTestSuite =3D NULL;=0D
+ DumpPolicyTestSuite =3D NULL;=0D
+ PolicyVersionTestSuite =3D NULL;=0D
+ LockPolicyTestSuite =3D NULL;=0D
+ DisablePolicyTestSuite =3D NULL;=0D
+=0D
+ DEBUG ((DEBUG_INFO, "%a v%a\n", UNIT_TEST_APP_NAME, UNIT_TEST_APP_VERSIO=
N));=0D
+=0D
+ //=0D
+ // Start setting up the test framework for running the tests.=0D
+ //=0D
+ Status =3D InitUnitTestFramework (&Framework, UNIT_TEST_APP_NAME, gEfiCa=
llerBaseName, UNIT_TEST_APP_VERSION);=0D
+ if (EFI_ERROR (Status)) {=0D
+ DEBUG ((DEBUG_ERROR, "Failed in InitUnitTestFramework. Status =3D %r\n=
", Status));=0D
+ goto EXIT;=0D
+ }=0D
+=0D
+ //=0D
+ // Test suite 1: Getting Started. Get VP protocol, check state, log revi=
sion=0D
+ //=0D
+ Status =3D CreateUnitTestSuite (&GettingStartedTestSuite, Framework, "Ge=
tting Started", "Common.VP.GettingStarted", NULL, NULL);=0D
+ if (EFI_ERROR (Status)) {=0D
+ DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for the Getting St=
arted Test Suite\n"));=0D
+ Status =3D EFI_OUT_OF_RESOURCES;=0D
+ goto EXIT;=0D
+ }=0D
+ AddTestCase (GettingStartedTestSuite, "Confirm VP is enabled", "Common.V=
P.GettingStarted.CheckVpEnabled", CheckVpEnabled, LocateVarPolicyPreReq, NU=
LL, NULL);=0D
+ AddTestCase (GettingStartedTestSuite, "Check VP revision", "Common.VP.Ge=
ttingStarted.CheckVpRevision", CheckVpRevision, LocateVarPolicyPreReq, NULL=
, NULL);=0D
+=0D
+ //=0D
+ // Test suite 2: Test NoLock Policies=0D
+ //=0D
+ Status =3D CreateUnitTestSuite (&NoLockPoliciesTestSuite, Framework, "Ex=
ercise NoLock Policies", "Common.VP.NoLockPolicies", NULL, NULL);=0D
+ if (EFI_ERROR (Status)) {=0D
+ DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for the NoLock Pol=
icies Test Suite\n"));=0D
+ Status =3D EFI_OUT_OF_RESOURCES;=0D
+ goto EXIT;=0D
+ }=0D
+ AddTestCase (NoLockPoliciesTestSuite, "Test Min Size enforcement in NoLo=
ck policy", "Common.VP.NoLockPolicies.TestMinSizeNoLock", TestMinSizeNoLock=
, LocateVarPolicyPreReq, NULL, NULL);=0D
+ AddTestCase (NoLockPoliciesTestSuite, "Test Max Size enforcement in NoLo=
ck policy", "Common.VP.NoLockPolicies.TestMaxSizeNoLock", TestMaxSizeNoLock=
, LocateVarPolicyPreReq, NULL, NULL);=0D
+ AddTestCase (NoLockPoliciesTestSuite, "Test Must Have Attribute enforcem=
ent in NoLock policy", "Common.VP.NoLockPolicies.TestMustHaveAttrNoLock", T=
estMustHaveAttrNoLock, LocateVarPolicyPreReq, NULL, NULL);=0D
+ AddTestCase (NoLockPoliciesTestSuite, "Test Can't Have Attribute enforce=
ment in NoLock policy", "Common.VP.NoLockPolicies.TestCantHaveAttrNoLock", =
TestCantHaveAttrNoLock, LocateVarPolicyPreReq, NULL, NULL);=0D
+ AddTestCase (NoLockPoliciesTestSuite, "Test Max Size enforcement in NoLo=
ck policy for entire namespace", "Common.VP.NoLockPolicies.TestMaxSizeNames=
paceNoLock", TestMaxSizeNamespaceNoLock, LocateVarPolicyPreReq, NULL, NULL)=
;=0D
+ AddTestCase (NoLockPoliciesTestSuite, "Test Must Have Attribute enforcem=
ent in NoLock policy with wildcards", "Common.VP.NoLockPolicies.TestMustHav=
eAttrWildcardNoLock", TestMustHaveAttrWildcardNoLock, LocateVarPolicyPreReq=
, NULL, NULL);=0D
+ AddTestCase (NoLockPoliciesTestSuite, "Test policy prioritization betwee=
n namespace-wide, wildcard, and var-specific policies", "Common.VP.NoLockPo=
licies.TestPolicyprioritizationNoLock", TestPolicyprioritizationNoLock, Loc=
ateVarPolicyPreReq, NULL, NULL);=0D
+=0D
+ //=0D
+ // Test suite 3: Test LockNow policies=0D
+ //=0D
+ Status =3D CreateUnitTestSuite (&LockNowPoliciesTestSuite, Framework, "E=
xercise LockNow Policies", "Common.VP.LockNowPolicies", NULL, NULL);=0D
+ if (EFI_ERROR (Status)) {=0D
+ DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for the LockNow Po=
licies Test Suite\n"));=0D
+ Status =3D EFI_OUT_OF_RESOURCES;=0D
+ goto EXIT;=0D
+ }=0D
+ AddTestCase (LockNowPoliciesTestSuite, "Test LockNow policy for a pre-ex=
isting variable", "Common.VP.LockNowPolicies.TestExistingVarLockNow", TestE=
xistingVarLockNow, LocateVarPolicyPreReq, NULL, NULL);=0D
+ AddTestCase (LockNowPoliciesTestSuite, "Test LockNow policy for a nonexi=
stent variable", "Common.VP.LockNowPolicies.TestNonexistentVarLockNow", Tes=
tNonexistentVarLockNow, LocateVarPolicyPreReq, NULL, NULL);=0D
+=0D
+ //=0D
+ // Test suite 4: Test LockOnCreate policies=0D
+ //=0D
+ Status =3D CreateUnitTestSuite (&LockOnCreatePoliciesTestSuite, Framewor=
k, "Exercise LockOnCreate Policies", "Common.VP.LockOnCreate", NULL, NULL);=
=0D
+ if (EFI_ERROR (Status))=0D
+ {=0D
+ DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for the LockOnCrea=
te Policies Test Suite\n"));=0D
+ Status =3D EFI_OUT_OF_RESOURCES;=0D
+ goto EXIT;=0D
+ }=0D
+ AddTestCase (LockOnCreatePoliciesTestSuite, "Test LockOnCreate policy fo=
r a pre-existing variable", "Common.VP.LockOnCreate.TestExistingVarLockOnCr=
eate", TestExistingVarLockOnCreate, LocateVarPolicyPreReq, NULL, NULL);=0D
+ AddTestCase (LockOnCreatePoliciesTestSuite, "Test LockOnCreate policy fo=
r a nonexistent variable", "Common.VP.LockOnCreate.TestNonexistentVarLockOn=
Create", TestNonexistentVarLockOnCreate, LocateVarPolicyPreReq, NULL, NULL)=
;=0D
+=0D
+ //=0D
+ // Test suite 5: Test LockOnVarState policies=0D
+ //=0D
+ Status =3D CreateUnitTestSuite (&LockOnVarStatePoliciesTestSuite, Framew=
ork, "Exercise LockOnVarState Policies", "Common.VP.LockOnVarState", NULL, =
NULL);=0D
+ if (EFI_ERROR (Status)) {=0D
+ DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for the LockOnVarS=
tate Policies Test Suite\n"));=0D
+ Status =3D EFI_OUT_OF_RESOURCES;=0D
+ goto EXIT;=0D
+ }=0D
+ AddTestCase (LockOnVarStatePoliciesTestSuite, "Test LockOnVarState polic=
y for a nonexistent variable", "Common.VP.LockOnVarState.TestLockOnVarState=
BeforeCreate", TestLockOnVarStateBeforeCreate, LocateVarPolicyPreReq, NULL,=
NULL);=0D
+ AddTestCase (LockOnVarStatePoliciesTestSuite, "Test LockOnVarState polic=
y for a pre-existing variable", "Common.VP.LockOnVarState.TestLockOnVarStat=
eAfterCreate", TestLockOnVarStateAfterCreate, LocateVarPolicyPreReq, NULL, =
NULL);=0D
+ AddTestCase (LockOnVarStatePoliciesTestSuite, "Test LockOnVarState polic=
y triggered by invalid-size variable", "Common.VP.LockOnVarState.TestLockOn=
VarStateInvalidLargeTrigger", TestLockOnVarStateInvalidLargeTrigger, Locate=
VarPolicyPreReq, NULL, NULL);=0D
+ AddTestCase (LockOnVarStatePoliciesTestSuite, "Test LockOnVarState polic=
y triggered by invalid-value variable", "Common.VP.LockOnVarState.TestLockO=
nVarStateWrongValueTrigger", TestLockOnVarStateWrongValueTrigger, LocateVar=
PolicyPreReq, NULL, NULL);=0D
+=0D
+ //=0D
+ // Test suite 6: Test registering invalid policies=0D
+ //=0D
+ Status =3D CreateUnitTestSuite (&InvalidPoliciesTestSuite, Framework, "A=
ttempt registering invalid policies", "Common.VP.InvalidPolicies", NULL, NU=
LL);=0D
+ if (EFI_ERROR (Status)) {=0D
+ DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for the Invalid Po=
licies Test Suite\n"));=0D
+ Status =3D EFI_OUT_OF_RESOURCES;=0D
+ goto EXIT;=0D
+ }=0D
+ AddTestCase (InvalidPoliciesTestSuite, "Test policy with invalid must-ha=
ve attributes", "Common.VP.InvalidPolicies.TestInvalidAttributesPolicy", Te=
stInvalidAttributesPolicy, LocateVarPolicyPreReq, NULL, NULL);=0D
+ AddTestCase (InvalidPoliciesTestSuite, "Test policy with invalid attribu=
tes", "Common.VP.InvalidPolicies.TestLargeMinSizePolicy", TestLargeMinSizeP=
olicy, LocateVarPolicyPreReq, NULL, NULL);=0D
+ AddTestCase (InvalidPoliciesTestSuite, "Test policy with invalid attribu=
tes", "Common.VP.InvalidPolicies.TestZeroMaxSizePolicy", TestZeroMaxSizePol=
icy, LocateVarPolicyPreReq, NULL, NULL);=0D
+ AddTestCase (InvalidPoliciesTestSuite, "Test policy with invalid type", =
"Common.VP.InvalidPolicies.TestInvalidPolicyTypePolicy", TestInvalidPolicyT=
ypePolicy, LocateVarPolicyPreReq, NULL, NULL);=0D
+=0D
+ //=0D
+ // Test suite 7: Test dumping the policy=0D
+ //=0D
+ Status =3D CreateUnitTestSuite (&DumpPolicyTestSuite, Framework, "Attemp=
t dumping policy", "Common.VP.DumpPolicy", NULL, NULL);=0D
+ if (EFI_ERROR (Status)) {=0D
+ DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for the Dump Polic=
y Test Suite\n"));=0D
+ Status =3D EFI_OUT_OF_RESOURCES;=0D
+ goto EXIT;=0D
+ }=0D
+ AddTestCase (DumpPolicyTestSuite, "Test dumping policy", "Common.VP.Dump=
Policy.TestDumpPolicy", TestDumpPolicy, LocateVarPolicyPreReq, NULL, NULL);=
=0D
+=0D
+ //=0D
+ // Test suite 8: Test policy version=0D
+ //=0D
+ Status =3D CreateUnitTestSuite (&PolicyVersionTestSuite, Framework, "Use=
non-zero policy version", "Common.VP.PolicyVersion", NULL, NULL);=0D
+ if (EFI_ERROR (Status)) {=0D
+ DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for the Policy Ver=
sion Test Suite\n"));=0D
+ Status =3D EFI_OUT_OF_RESOURCES;=0D
+ goto EXIT;=0D
+ }=0D
+ AddTestCase (PolicyVersionTestSuite, "Test policy version", "Common.VP.D=
umpPolicy.TestPolicyVersion", TestPolicyVersion, LocateVarPolicyPreReq, NUL=
L, NULL);=0D
+=0D
+ //=0D
+ // Test suite 9: Lock VPE and test implications=0D
+ //=0D
+ Status =3D CreateUnitTestSuite (&LockPolicyTestSuite, Framework, "Lock p=
olicy, test it", "Common.VP.LockPolicyTests", NULL, NULL);=0D
+ if (EFI_ERROR (Status)) {=0D
+ DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for the Lock Polic=
y Test Suite\n"));=0D
+ Status =3D EFI_OUT_OF_RESOURCES;=0D
+ goto EXIT;=0D
+ }=0D
+ AddTestCase (LockPolicyTestSuite, "Test locking policy", "Common.VP.Lock=
PolicyTests.LockPolicyEngineTests", LockPolicyEngineTests, LocateVarPolicyP=
reReq, NULL, NULL);=0D
+ AddTestCase (LockPolicyTestSuite, "Test locking policy", "Common.VP.Lock=
PolicyTests.LockPolicyEngineTests", LockPolicyEngineTests, LocateVarPolicyP=
reReq, IgnoreContextAndReboot, NULL);=0D
+=0D
+ //=0D
+ // Test suite 10: Disable var policy and confirm expected behavior=0D
+ //=0D
+ Status =3D CreateUnitTestSuite (&DisablePolicyTestSuite, Framework, "Dis=
able policy, test it", "Common.VP.DisablePolicyTests", NULL, NULL);=0D
+ if (EFI_ERROR (Status)) {=0D
+ DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for the Disable Po=
licy Test Suite\n"));=0D
+ Status =3D EFI_OUT_OF_RESOURCES;=0D
+ goto EXIT;=0D
+ }=0D
+ AddTestCase (DisablePolicyTestSuite, "Confirm VP is enabled", "Common.VP=
.DisablePolicyTests.CheckVpEnabled", CheckVpEnabled, LocateVarPolicyPreReq,=
NULL, NULL);=0D
+ AddTestCase (DisablePolicyTestSuite, "Test LockNow policy for a pre-exis=
ting variable", "Common.VP.DisablePolicyTests.TestExistingVarLockNow", Test=
ExistingVarLockNow, LocateVarPolicyPreReq, NULL, NULL);=0D
+ AddTestCase (DisablePolicyTestSuite, "Test AuthVar protection while Vari=
ablePolicy is enabled", "Common.VP.DisablePolicyTests.TestAuthVar1", TestAu=
thVarPart1, VarPolicyEnabledPreReq, NULL, NULL);=0D
+ AddTestCase (DisablePolicyTestSuite, "Test disabling policy", "Common.VP=
.DisablePolicyTests.DisablePolicyEngineTests", DisablePolicyEngineTests, Lo=
cateVarPolicyPreReq, NULL, NULL);=0D
+ AddTestCase (DisablePolicyTestSuite, "Test AuthVar protection while Vari=
ablePolicy is disabled", "Common.VP.DisablePolicyTests.TestAuthVar2", TestA=
uthVarPart2, VarPolicyDisabledPreReq, FinalCleanup, NULL);=0D
+=0D
+ //=0D
+ // Execute the tests.=0D
+ //=0D
+ Status =3D RunAllTestSuites (Framework);=0D
+=0D
+EXIT:=0D
+ if (Framework !=3D NULL) {=0D
+ FreeUnitTestFramework (Framework);=0D
+ }=0D
+=0D
+ return Status;=0D
+} // UefiMain=0D
diff --git a/MdeModulePkg/MdeModulePkg.ci.yaml b/MdeModulePkg/MdeModulePkg.=
ci.yaml
index 20d53fc5a5fa..a1beee9f4aab 100644
--- a/MdeModulePkg/MdeModulePkg.ci.yaml
+++ b/MdeModulePkg/MdeModulePkg.ci.yaml
@@ -53,7 +53,9 @@
"UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec"=0D
],=0D
# For UEFI shell based apps=0D
- "AcceptableDependencies-UEFI_APPLICATION":[],=0D
+ "AcceptableDependencies-UEFI_APPLICATION":[=0D
+ "UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec"=0D
+ ],=0D
"IgnoreInf": []=0D
},=0D
=0D
diff --git a/MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/Readme.m=
d b/MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/Readme.md
new file mode 100644
index 000000000000..804ad4173a5f
--- /dev/null
+++ b/MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/Readme.md
@@ -0,0 +1,55 @@
+# variable Policy Unit Tests=0D
+=0D
+## &#x1F539; Copyright=0D
+Copyright (C) Microsoft Corporation. All rights reserved.=0D
+SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+=0D
+## About This Test=0D
+This test verifies functionality of the Variable Policy Protocol by regist=
ering various variable policies and exercising them, as well as tests locki=
ng the policy, disabling it, and dumping the policy entries.=0D
+=0D
+Only policies that are created as a part of this test will be tested.=0D
+1. Try getting test context, if empty then get VP protocol, confirm that V=
P is not disabled by calling IsVariablePolicyEnabled. Log VP revision.=0D
+2. "No lock" policies:=0D
+ * check minsize enforcement=0D
+ * check maxsize enforcement=0D
+ * check musthave attr enforcement=0D
+ * check canthave attr enforcement=0D
+ * check one of the above with empty string policy i.e. name wildcard=0D
+ * check another one of the above with a "#" containing policy string=0D
+ * check policy prioritization by having a namespace-wide policy, a pol=
icy with a # wildcard, and a one-var specific policy and testing which one =
is enforced=0D
+3. "Lock now" policies (means if the var doesn't exist, it won't be create=
d; if one exists, it can't be updated):=0D
+ * test a policy for an already existing variable, verify we can't writ=
e into that variable=0D
+ * create a policy for a non-existing variable and attempt to register =
such var=0D
+4. "Lock on create" policies (means the var can still be created, but no u=
pdates later, existing vars can't be updated):=0D
+ * create a var, lock it with LockOnCreate, attempt to update its conte=
nts=0D
+ * create LockOnCreate VP, attempt to create var with invalid size, the=
n invalid attr, then create valid var, attempt to update its contents=0D
+5. "Lock on var state" policies (means the var protected by this policy ca=
n't be created or updated once the trigger is set)=0D
+ * create VP, trigger lock with a valid var, attempt to create a locked=
var, then modify the trigger var, create locked var=0D
+ * create VP, create targeted var, modify it, trigger lock, attempt to =
modify var=0D
+ * create VP, trigger lock with invalid (larger than one byte) var, see=
if VPE allows creation of the locked var (it should allow)=0D
+ * create VP, set locking var with wrong value, see if VPE allows creat=
ion of the locked var (should allow)=0D
+6. Attempt registering invalid policy entries=0D
+ * invalid required and banned attributes=0D
+ * large min size - let's say 2GB=0D
+ * max size equal to 0=0D
+ * invalid policy type=0D
+7. Exercise dumping policy. No need to check the validity of the dump blob=
.=0D
+8. Test registering a policy with a random version.=0D
+9. Lock VPE, make sure old policies are enforced, new ones can't be regist=
ered.=0D
+ * Register a LockOnCreate policy=0D
+ * Lock VPE=0D
+ * Test locking it again.=0D
+ * Verify one of the prior policies is enforced=0D
+ * Make sure we can create variables even if those are protected by Loc=
kOnCreate policy, after locking the VPE=0D
+ * Attempt to register new policies=0D
+ * Make sure can't disable VPE=0D
+ * Cleanup: save context and reboot=0D
+10. Disable variable policy and try some things=0D
+ * Locate Variable Policy Protocol=0D
+ * Make sure VP is enabled=0D
+ * Register a policy=0D
+ * Disable VPE=0D
+ * Call IsVariablePolicyEnabled to confirm it's disabled.=0D
+ * Make sure can't lock policy=0D
+ * Make sure the policy from a is no longer enforced=0D
+ * Final cleanup: delete vars that were created in some earlier test su=
ites=0D
diff --git a/MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/Variable=
PolicyFuncTestApp.inf b/MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestA=
pp/VariablePolicyFuncTestApp.inf
new file mode 100644
index 000000000000..bfbac406b504
--- /dev/null
+++ b/MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyF=
uncTestApp.inf
@@ -0,0 +1,47 @@
+## @file=0D
+# Uefi Shell based Application that unit tests the Variable Policy Protoco=
l=0D
+#=0D
+# Copyright (c) Microsoft Corporation.=0D
+# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+##=0D
+=0D
+[Defines]=0D
+ INF_VERSION =3D 0x00010005=0D
+ BASE_NAME =3D VariablePolicyFuncTestApp=0D
+ FILE_GUID =3D B653C4C3-3FCC-4B6C-8051-5F692AEAECBA=
=0D
+ MODULE_TYPE =3D UEFI_APPLICATION=0D
+ VERSION_STRING =3D 1.0=0D
+ ENTRY_POINT =3D UefiMain=0D
+=0D
+#=0D
+# The following information is for reference only and not required by the =
build tools.=0D
+#=0D
+# VALID_ARCHITECTURES =3D X64 AARCH64=0D
+#=0D
+=0D
+[Sources]=0D
+ VariablePolicyFuncTestApp.c=0D
+ VariablePolicyTestAuthVar.h=0D
+=0D
+[Packages]=0D
+ MdePkg/MdePkg.dec=0D
+ MdeModulePkg/MdeModulePkg.dec=0D
+ UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec=0D
+=0D
+[LibraryClasses]=0D
+ UefiApplicationEntryPoint=0D
+ BaseLib=0D
+ BaseMemoryLib=0D
+ UnitTestLib=0D
+ UnitTestBootLib=0D
+ PrintLib=0D
+ UefiBootServicesTableLib=0D
+ UefiRuntimeServicesTableLib=0D
+ MemoryAllocationLib=0D
+ VariablePolicyHelperLib=0D
+=0D
+[Guids]=0D
+ gEfiCertPkcs7Guid=0D
+=0D
+[Protocols]=0D
+ gEdkiiVariablePolicyProtocolGuid=0D
diff --git a/MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/Variable=
PolicyTestAuthVar.h b/MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp=
/VariablePolicyTestAuthVar.h
new file mode 100644
index 000000000000..c90310226827
--- /dev/null
+++ b/MdeModulePkg/Test/ShellTest/VariablePolicyFuncTestApp/VariablePolicyT=
estAuthVar.h
@@ -0,0 +1,128 @@
+/** @file -- VarPolicyTestAuthVar.h=0D
+Payload to be used to create an Authenticated Variable for testing.=0D
+=0D
+Copyright (c) Microsoft Corporation.=0D
+SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+=0D
+**/=0D
+=0D
+#ifndef _VAR_POLICY_TEST_AUTH_VAR_H_=0D
+#define _VAR_POLICY_TEST_AUTH_VAR_H_=0D
+=0D
+UINT8 mTestAuthVarPayload[] =3D {=0D
+ // EFI_VARIABLE_AUTHENTICATION_2=0D
+ // Timestamp=0D
+ 0xE4, 0x07, 0x08, 0x15, 0x0D, 0x1E, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, =
0x00, 0x00, 0x00, 0x00,=0D
+ // AuthInfo (WIN_CERTIFICATE_UEFI_GUID)=0D
+ // Hdr (WIN_CERTIFICATE)=0D
+ // dwLength=0D
+ 0x45, 0x05, 0x00, 0x00,=0D
+ // wRevision=0D
+ 0x00, 0x02,=0D
+ // wCertificateType=0D
+ // (WIN_CERT_TYPE_EFI_GUID)=0D
+ 0xF1, 0x0E,=0D
+ // CertType=0D
+ // (gEfiCertPkcs7Guid)=0D
+ 0x9D, 0xD2, 0xAF, 0x4A, 0xDF, 0x68, 0xEE, 0x49, 0x8A, 0xA9, 0x34, 0x7D, =
0x37, 0x56, 0x65, 0xA7,=0D
+ // CertData (Packed SignedData Signature)=0D
+ // Digest Buffer Was...=0D
+ // Name (DummyAuthVar)=0D
+ // 44 00 75 00 6D 00 6D 00 79 00 41 00 75 00 74 00 68 00 56 00 61=
00 72 00=0D
+ // Vendor Guid (mTestAuthNamespaceGuid)=0D
+ // C6 A2 C5 B6 CE 3E 9B 4B 8C C8 96 D8 D9 CA D3 4E=0D
+ // Attributes (NV + BS + RT, TimeAuth)=0D
+ // 27 00 00 00=0D
+ // Timestamp=0D
+ // E4 07 08 15 0D 1E 00 00 00 00 00 00 00 00 00 00=0D
+ // Data (0xDEADBEEF)=0D
+ // EF BE AD DE=0D
+ 0x30, 0x82, 0x05, 0x29, 0x02, 0x01, 0x01, 0x31, 0x0F, 0x30, 0x0D, 0x06, =
0x09, 0x60, 0x86, 0x48,=0D
+ 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, 0x05, 0x00, 0x30, 0x0B, 0x06, 0x09, =
0x2A, 0x86, 0x48, 0x86,=0D
+ 0xF7, 0x0D, 0x01, 0x07, 0x01, 0xA0, 0x82, 0x03, 0x82, 0x30, 0x82, 0x03, =
0x7E, 0x30, 0x82, 0x02,=0D
+ 0x66, 0xA0, 0x03, 0x02, 0x01, 0x02, 0x02, 0x10, 0x5A, 0xAE, 0x85, 0xA8, =
0x61, 0x6E, 0x80, 0xA3,=0D
+ 0x4D, 0x11, 0x69, 0x06, 0xC3, 0xFE, 0x2D, 0x89, 0x30, 0x0D, 0x06, 0x09, =
0x2A, 0x86, 0x48, 0x86,=0D
+ 0xF7, 0x0D, 0x01, 0x01, 0x0B, 0x05, 0x00, 0x30, 0x3F, 0x31, 0x3D, 0x30, =
0x3B, 0x06, 0x03, 0x55,=0D
+ 0x04, 0x03, 0x1E, 0x34, 0x00, 0x50, 0x00, 0x41, 0x00, 0x4C, 0x00, 0x49, =
0x00, 0x4E, 0x00, 0x44,=0D
+ 0x00, 0x52, 0x00, 0x4F, 0x00, 0x4D, 0x00, 0x45, 0x00, 0x5F, 0x00, 0x53, =
0x00, 0x65, 0x00, 0x6C,=0D
+ 0x00, 0x66, 0x00, 0x68, 0x00, 0x6F, 0x00, 0x73, 0x00, 0x74, 0x00, 0x5F, =
0x00, 0x53, 0x00, 0x69,=0D
+ 0x00, 0x67, 0x00, 0x6E, 0x00, 0x65, 0x00, 0x72, 0x30, 0x20, 0x17, 0x0D, =
0x30, 0x30, 0x30, 0x31,=0D
+ 0x30, 0x31, 0x30, 0x37, 0x30, 0x30, 0x30, 0x30, 0x5A, 0x18, 0x0F, 0x32, =
0x39, 0x39, 0x39, 0x31,=0D
+ 0x32, 0x33, 0x31, 0x30, 0x37, 0x30, 0x30, 0x30, 0x30, 0x5A, 0x30, 0x3F, =
0x31, 0x3D, 0x30, 0x3B,=0D
+ 0x06, 0x03, 0x55, 0x04, 0x03, 0x1E, 0x34, 0x00, 0x50, 0x00, 0x41, 0x00, =
0x4C, 0x00, 0x49, 0x00,=0D
+ 0x4E, 0x00, 0x44, 0x00, 0x52, 0x00, 0x4F, 0x00, 0x4D, 0x00, 0x45, 0x00, =
0x5F, 0x00, 0x53, 0x00,=0D
+ 0x65, 0x00, 0x6C, 0x00, 0x66, 0x00, 0x68, 0x00, 0x6F, 0x00, 0x73, 0x00, =
0x74, 0x00, 0x5F, 0x00,=0D
+ 0x53, 0x00, 0x69, 0x00, 0x67, 0x00, 0x6E, 0x00, 0x65, 0x00, 0x72, 0x30, =
0x82, 0x01, 0x22, 0x30,=0D
+ 0x0D, 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x01, 0x01, =
0x05, 0x00, 0x03, 0x82,=0D
+ 0x01, 0x0F, 0x00, 0x30, 0x82, 0x01, 0x0A, 0x02, 0x82, 0x01, 0x01, 0x00, =
0xC9, 0xA2, 0x80, 0xE7,=0D
+ 0x3A, 0x0B, 0x3E, 0xCF, 0xEE, 0x0E, 0x22, 0x65, 0xF5, 0x03, 0xD2, 0x6A, =
0x99, 0xBF, 0x5F, 0x48,=0D
+ 0xF4, 0xC0, 0xD3, 0x19, 0xE7, 0x6B, 0x09, 0xFC, 0x0C, 0xB0, 0x3B, 0x69, =
0x3A, 0x07, 0x6F, 0x36,=0D
+ 0x57, 0xF6, 0x63, 0xAF, 0x6B, 0x7B, 0x30, 0x55, 0xD5, 0xE9, 0xF4, 0xDE, =
0x89, 0xE3, 0x5F, 0xA1,=0D
+ 0x71, 0x13, 0x3E, 0x84, 0x5D, 0x46, 0x9F, 0x78, 0xA9, 0x5B, 0xA5, 0x46, =
0x3B, 0x38, 0x4F, 0x00,=0D
+ 0x06, 0x63, 0x0E, 0x7A, 0x0A, 0x93, 0xE7, 0x36, 0x87, 0xCC, 0x47, 0xBD, =
0xFB, 0x0A, 0x5D, 0x45,=0D
+ 0x9C, 0xC4, 0x1B, 0xE6, 0x9E, 0xCB, 0xAB, 0xF9, 0x20, 0x11, 0xEF, 0x03, =
0xCA, 0x9F, 0xE9, 0x29,=0D
+ 0x1A, 0x05, 0xF8, 0xB3, 0x46, 0xB0, 0x3D, 0xFD, 0x88, 0x7C, 0x82, 0x0E, =
0x3C, 0x6F, 0xEA, 0x5B,=0D
+ 0xFF, 0xA8, 0xA4, 0xE0, 0x40, 0x2B, 0x25, 0xE8, 0x59, 0x46, 0xEE, 0xDB, =
0x4B, 0x5F, 0x02, 0xB3,=0D
+ 0x21, 0x33, 0x47, 0x2E, 0xD5, 0x66, 0x79, 0xF3, 0x79, 0x93, 0x18, 0x75, =
0x94, 0x4A, 0x01, 0xCF,=0D
+ 0x59, 0x86, 0xF4, 0x8B, 0x35, 0xBD, 0xA4, 0x58, 0xA4, 0x76, 0x89, 0x77, =
0x55, 0x55, 0xB1, 0xE4,=0D
+ 0x00, 0x09, 0x78, 0xF3, 0x29, 0x5B, 0xC0, 0xED, 0xD6, 0x68, 0x7E, 0xDB, =
0xAA, 0x9F, 0x4E, 0xFE,=0D
+ 0x67, 0x41, 0x4E, 0x6C, 0xC8, 0xDD, 0x52, 0xD6, 0xA5, 0x8A, 0x8A, 0x56, =
0x50, 0x51, 0x27, 0x29,=0D
+ 0x2B, 0xD3, 0x1B, 0x4D, 0xCE, 0x93, 0x76, 0x8E, 0x55, 0x53, 0x55, 0x30, =
0x10, 0xF5, 0xF9, 0x6C,=0D
+ 0xAE, 0xDA, 0xBA, 0xAC, 0x36, 0x79, 0x11, 0x02, 0xD0, 0x24, 0x07, 0xA6, =
0xD1, 0x56, 0xCB, 0xEC,=0D
+ 0x81, 0x29, 0xA8, 0xC1, 0x2E, 0x9D, 0x9B, 0xF9, 0xE9, 0xF4, 0x55, 0x74, =
0xA0, 0x52, 0x87, 0x49,=0D
+ 0x4F, 0xAC, 0x71, 0xFF, 0x30, 0x12, 0x24, 0xDD, 0x6D, 0x50, 0x5C, 0x7D, =
0x02, 0x03, 0x01, 0x00,=0D
+ 0x01, 0xA3, 0x74, 0x30, 0x72, 0x30, 0x70, 0x06, 0x03, 0x55, 0x1D, 0x01, =
0x04, 0x69, 0x30, 0x67,=0D
+ 0x80, 0x10, 0x0E, 0xB2, 0xFB, 0xDC, 0xD5, 0xAB, 0xCC, 0xB4, 0x3B, 0x46, =
0x1B, 0x60, 0x18, 0xFD,=0D
+ 0xDE, 0x74, 0xA1, 0x41, 0x30, 0x3F, 0x31, 0x3D, 0x30, 0x3B, 0x06, 0x03, =
0x55, 0x04, 0x03, 0x1E,=0D
+ 0x34, 0x00, 0x50, 0x00, 0x41, 0x00, 0x4C, 0x00, 0x49, 0x00, 0x4E, 0x00, =
0x44, 0x00, 0x52, 0x00,=0D
+ 0x4F, 0x00, 0x4D, 0x00, 0x45, 0x00, 0x5F, 0x00, 0x53, 0x00, 0x65, 0x00, =
0x6C, 0x00, 0x66, 0x00,=0D
+ 0x68, 0x00, 0x6F, 0x00, 0x73, 0x00, 0x74, 0x00, 0x5F, 0x00, 0x53, 0x00, =
0x69, 0x00, 0x67, 0x00,=0D
+ 0x6E, 0x00, 0x65, 0x00, 0x72, 0x82, 0x10, 0x5A, 0xAE, 0x85, 0xA8, 0x61, =
0x6E, 0x80, 0xA3, 0x4D,=0D
+ 0x11, 0x69, 0x06, 0xC3, 0xFE, 0x2D, 0x89, 0x30, 0x0D, 0x06, 0x09, 0x2A, =
0x86, 0x48, 0x86, 0xF7,=0D
+ 0x0D, 0x01, 0x01, 0x0B, 0x05, 0x00, 0x03, 0x82, 0x01, 0x01, 0x00, 0xB5, =
0xA2, 0xD0, 0x1B, 0x70,=0D
+ 0x24, 0xC2, 0xE8, 0x64, 0xCD, 0xF1, 0xE9, 0x97, 0x9E, 0xA7, 0xC1, 0x86, =
0x92, 0x06, 0x2F, 0x8F,=0D
+ 0x33, 0x64, 0x0A, 0xB9, 0x2B, 0x77, 0xE2, 0x70, 0x82, 0xDE, 0x06, 0xD3, =
0x69, 0x8E, 0xB4, 0x69,=0D
+ 0xF1, 0x6B, 0x59, 0x5E, 0x68, 0x5F, 0xB4, 0xFA, 0x30, 0xC3, 0xB6, 0xA1, =
0x72, 0x1A, 0xD4, 0x01,=0D
+ 0xED, 0x69, 0x4A, 0x96, 0x0F, 0x1C, 0xC3, 0x6F, 0x80, 0x0B, 0xE5, 0xD4, =
0x46, 0xBE, 0x27, 0x9D,=0D
+ 0xDE, 0x68, 0xB3, 0xA1, 0x93, 0xC3, 0x1A, 0x47, 0x20, 0x7A, 0x87, 0x80, =
0x13, 0x85, 0x1E, 0x46,=0D
+ 0x01, 0x42, 0x6A, 0x68, 0x46, 0xE2, 0x77, 0x3D, 0x2E, 0x50, 0xA1, 0x96, =
0x23, 0x83, 0x03, 0xD1,=0D
+ 0x57, 0xDD, 0xC6, 0x63, 0x59, 0xB7, 0x1A, 0x49, 0xA2, 0xC9, 0x44, 0x8D, =
0xC7, 0x81, 0x18, 0xE8,=0D
+ 0x52, 0x3A, 0x74, 0x32, 0xD3, 0xE6, 0x6D, 0x54, 0x9F, 0xC9, 0x87, 0x1C, =
0xBC, 0x81, 0xEB, 0x6D,=0D
+ 0x5D, 0x58, 0xF7, 0x91, 0x81, 0x5B, 0xB0, 0x86, 0xB4, 0x06, 0xE7, 0x19, =
0x44, 0xE9, 0x24, 0x28,=0D
+ 0xF5, 0x42, 0x7A, 0x7A, 0x28, 0x94, 0x3E, 0x70, 0x61, 0x1B, 0x68, 0x8D, =
0xA9, 0x48, 0x3A, 0xFE,=0D
+ 0x7D, 0xB5, 0x29, 0x10, 0xCE, 0xD6, 0xC1, 0xFF, 0x16, 0xDF, 0x90, 0x94, =
0x16, 0xC8, 0xFA, 0x9E,=0D
+ 0x52, 0x49, 0xE5, 0xC3, 0xF5, 0x8C, 0x87, 0xC2, 0x93, 0x3D, 0x3D, 0x27, =
0x23, 0x37, 0xC3, 0xDA,=0D
+ 0x55, 0x92, 0x12, 0xE9, 0x1F, 0xEB, 0x32, 0xB5, 0xD8, 0x30, 0xD6, 0xC0, =
0x23, 0x45, 0xBB, 0x06,=0D
+ 0xBC, 0x11, 0xA6, 0xA3, 0x47, 0x82, 0x04, 0xCB, 0xAA, 0x98, 0xCA, 0xF9, =
0x00, 0x0E, 0xD3, 0xC3,=0D
+ 0x09, 0xF6, 0x21, 0x4C, 0x90, 0xE0, 0x78, 0x08, 0xAE, 0x8F, 0xB1, 0x7D, =
0x62, 0x3F, 0x6A, 0x1E,=0D
+ 0xD6, 0xF1, 0x8E, 0xEE, 0xFD, 0x49, 0x04, 0xDE, 0x14, 0x9C, 0x7B, 0x31, =
0x82, 0x01, 0x7E, 0x30,=0D
+ 0x82, 0x01, 0x7A, 0x02, 0x01, 0x01, 0x30, 0x53, 0x30, 0x3F, 0x31, 0x3D, =
0x30, 0x3B, 0x06, 0x03,=0D
+ 0x55, 0x04, 0x03, 0x1E, 0x34, 0x00, 0x50, 0x00, 0x41, 0x00, 0x4C, 0x00, =
0x49, 0x00, 0x4E, 0x00,=0D
+ 0x44, 0x00, 0x52, 0x00, 0x4F, 0x00, 0x4D, 0x00, 0x45, 0x00, 0x5F, 0x00, =
0x53, 0x00, 0x65, 0x00,=0D
+ 0x6C, 0x00, 0x66, 0x00, 0x68, 0x00, 0x6F, 0x00, 0x73, 0x00, 0x74, 0x00, =
0x5F, 0x00, 0x53, 0x00,=0D
+ 0x69, 0x00, 0x67, 0x00, 0x6E, 0x00, 0x65, 0x00, 0x72, 0x02, 0x10, 0x5A, =
0xAE, 0x85, 0xA8, 0x61,=0D
+ 0x6E, 0x80, 0xA3, 0x4D, 0x11, 0x69, 0x06, 0xC3, 0xFE, 0x2D, 0x89, 0x30, =
0x0D, 0x06, 0x09, 0x60,=0D
+ 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, 0x05, 0x00, 0x30, 0x0D, =
0x06, 0x09, 0x2A, 0x86,=0D
+ 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x01, 0x01, 0x05, 0x00, 0x04, 0x82, 0x01, =
0x00, 0xA6, 0x06, 0xE7,=0D
+ 0x46, 0x7E, 0xFB, 0x4A, 0xA7, 0x25, 0x2F, 0x52, 0x1D, 0xBC, 0x5C, 0x41, =
0x3B, 0xD3, 0x13, 0x50,=0D
+ 0xCE, 0x5F, 0xE2, 0x4B, 0x31, 0xED, 0x28, 0x5E, 0xF5, 0x36, 0xBD, 0x1C, =
0x38, 0xA1, 0xB6, 0x45,=0D
+ 0x7C, 0xFD, 0xAB, 0x7B, 0x0C, 0xBF, 0x06, 0x06, 0xBB, 0x95, 0x5E, 0x47, =
0x10, 0x7C, 0xD8, 0x10,=0D
+ 0x76, 0x74, 0x81, 0x2D, 0x40, 0x3A, 0xD0, 0xF4, 0x15, 0x9D, 0xDF, 0x44, =
0x2B, 0xA4, 0xCD, 0xF7,=0D
+ 0x44, 0x77, 0x9F, 0x35, 0x46, 0xD3, 0x30, 0x67, 0x44, 0x33, 0xF4, 0x7B, =
0xB6, 0xC0, 0xE4, 0xA2,=0D
+ 0xAD, 0xDF, 0xAF, 0x56, 0x41, 0xA3, 0x0D, 0x76, 0x36, 0xB9, 0x7E, 0x29, =
0x49, 0x17, 0x43, 0xAF,=0D
+ 0xB0, 0xA0, 0xC0, 0xF1, 0xE1, 0xE6, 0xCA, 0x62, 0x9F, 0x3E, 0x9D, 0x6C, =
0x63, 0x03, 0xF6, 0xDF,=0D
+ 0x84, 0x32, 0xB1, 0x01, 0x0C, 0x12, 0x83, 0x52, 0x13, 0x2F, 0xAE, 0xBC, =
0x79, 0xB7, 0x75, 0xF6,=0D
+ 0x10, 0x20, 0xFC, 0x7A, 0x13, 0x92, 0xF7, 0x87, 0x50, 0xF5, 0x9C, 0xD9, =
0xE4, 0xEA, 0x4C, 0x3D,=0D
+ 0x31, 0xED, 0x7F, 0xA6, 0x6C, 0x58, 0xAD, 0x6C, 0x31, 0xAF, 0xC4, 0x64, =
0xAE, 0x11, 0xBF, 0x72,=0D
+ 0xF5, 0xAA, 0x69, 0xB4, 0x76, 0xDB, 0x73, 0x8F, 0x8C, 0x3E, 0x23, 0x4A, =
0x2D, 0xB7, 0x65, 0x65,=0D
+ 0x10, 0xA8, 0xC6, 0x52, 0x14, 0xE2, 0xC6, 0x2B, 0x07, 0xCE, 0x45, 0x58, =
0x6F, 0x92, 0x78, 0xAA,=0D
+ 0xB5, 0xE9, 0x76, 0x39, 0x8A, 0x17, 0xE3, 0x0B, 0xA5, 0x12, 0x0F, 0x2A, =
0xC1, 0xCE, 0xC5, 0x4F,=0D
+ 0xD8, 0xA7, 0xD1, 0x7C, 0x3F, 0xE3, 0x23, 0x9B, 0x53, 0x56, 0x18, 0x28, =
0x66, 0xC7, 0xB3, 0x04,=0D
+ 0x38, 0xE3, 0x40, 0xCC, 0xB2, 0x18, 0xA8, 0xC7, 0x11, 0xE1, 0x67, 0xD8, =
0xBF, 0xBE, 0x8D, 0x2A,=0D
+ 0x75, 0x00, 0x96, 0x8F, 0x7F, 0x80, 0xCF, 0xDB, 0xF0, 0x0D, 0xB5, 0x8D, =
0x73,=0D
+ // Data=0D
+ 0xEF, 0xBE, 0xAD, 0xDE=0D
+};=0D
+UINTN mTestAuthVarPayloadSize =3D sizeof(mTestAuthVarPayload);=0D
+=0D
+#endif // _VAR_POLICY_TEST_AUTH_VAR_H_=0D
--=20
2.28.0.windows.1


[PATCH v7 13/14] MdeModulePkg: Drop VarLock from RuntimeDxe variable driver

Bret Barkelew
 

https://bugzilla.tianocore.org/show_bug.cgi?id=3D2522

Now that everything should be moved to
VariablePolicy, drop support for the
deprecated VarLock SMI interface and
associated functions from variable RuntimeDxe.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Liming Gao <liming.gao@...>
Cc: Bret Barkelew <brbarkel@...>
Signed-off-by: Bret Barkelew <brbarkel@...>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c | 49=
+-------------
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock.c | 71=
++++++++++++++++++++
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf | 1=
+
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf | 1=
+
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf | 1=
+
5 files changed, 75 insertions(+), 48 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c b/MdeMod=
ulePkg/Universal/Variable/RuntimeDxe/VarCheck.c
index f15219df5eb8..486d85b022e1 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c
@@ -3,60 +3,13 @@
and variable lock protocol based on VarCheckLib.=0D
=0D
Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>=0D
+Copyright (c) Microsoft Corporation.=0D
SPDX-License-Identifier: BSD-2-Clause-Patent=0D
=0D
**/=0D
=0D
#include "Variable.h"=0D
=0D
-/**=0D
- Mark a variable that will become read-only after leaving the DXE phase o=
f execution.=0D
- Write request coming from SMM environment through EFI_SMM_VARIABLE_PROTO=
COL is allowed.=0D
-=0D
- @param[in] This The VARIABLE_LOCK_PROTOCOL instance.=0D
- @param[in] VariableName A pointer to the variable name that will be mad=
e read-only subsequently.=0D
- @param[in] VendorGuid A pointer to the vendor GUID that will be made =
read-only subsequently.=0D
-=0D
- @retval EFI_SUCCESS The variable specified by the VariableName=
and the VendorGuid was marked=0D
- as pending to be read-only.=0D
- @retval EFI_INVALID_PARAMETER VariableName or VendorGuid is NULL.=0D
- Or VariableName is an empty string.=0D
- @retval EFI_ACCESS_DENIED EFI_END_OF_DXE_EVENT_GROUP_GUID or EFI_EVE=
NT_GROUP_READY_TO_BOOT has=0D
- already been signaled.=0D
- @retval EFI_OUT_OF_RESOURCES There is not enough resource to hold the l=
ock request.=0D
-**/=0D
-EFI_STATUS=0D
-EFIAPI=0D
-VariableLockRequestToLock (=0D
- IN CONST EDKII_VARIABLE_LOCK_PROTOCOL *This,=0D
- IN CHAR16 *VariableName,=0D
- IN EFI_GUID *VendorGuid=0D
- )=0D
-{=0D
- EFI_STATUS Status;=0D
- VAR_CHECK_VARIABLE_PROPERTY Property;=0D
-=0D
- AcquireLockOnlyAtBootTime (&mVariableModuleGlobal->VariableGlobal.Variab=
leServicesLock);=0D
-=0D
- Status =3D VarCheckLibVariablePropertyGet (VariableName, VendorGuid, &Pr=
operty);=0D
- if (!EFI_ERROR (Status)) {=0D
- Property.Property |=3D VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY;=0D
- } else {=0D
- Property.Revision =3D VAR_CHECK_VARIABLE_PROPERTY_REVISION;=0D
- Property.Property =3D VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY;=0D
- Property.Attributes =3D 0;=0D
- Property.MinSize =3D 1;=0D
- Property.MaxSize =3D MAX_UINTN;=0D
- }=0D
- Status =3D VarCheckLibVariablePropertySet (VariableName, VendorGuid, &Pr=
operty);=0D
-=0D
- DEBUG ((EFI_D_INFO, "[Variable] Lock: %g:%s %r\n", VendorGuid, VariableN=
ame, Status));=0D
-=0D
- ReleaseLockOnlyAtBootTime (&mVariableModuleGlobal->VariableGlobal.Variab=
leServicesLock);=0D
-=0D
- return Status;=0D
-}=0D
-=0D
/**=0D
Register SetVariable check handler.=0D
=0D
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstT=
oLock.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLo=
ck.c
new file mode 100644
index 000000000000..1f7f0b7ef06c
--- /dev/null
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequstToLock.c
@@ -0,0 +1,71 @@
+/** @file -- VariableLockRequstToLock.c=0D
+Temporary location of the RequestToLock shim code while=0D
+projects are moved to VariablePolicy. Should be removed when deprecated.=0D
+=0D
+Copyright (c) Microsoft Corporation.=0D
+SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+=0D
+**/=0D
+=0D
+#include <Uefi.h>=0D
+=0D
+#include <Library/DebugLib.h>=0D
+#include <Library/MemoryAllocationLib.h>=0D
+=0D
+#include <Protocol/VariableLock.h>=0D
+=0D
+#include <Protocol/VariablePolicy.h>=0D
+#include <Library/VariablePolicyLib.h>=0D
+#include <Library/VariablePolicyHelperLib.h>=0D
+=0D
+=0D
+/**=0D
+ DEPRECATED. THIS IS ONLY HERE AS A CONVENIENCE WHILE PORTING.=0D
+ Mark a variable that will become read-only after leaving the DXE phase o=
f execution.=0D
+ Write request coming from SMM environment through EFI_SMM_VARIABLE_PROTO=
COL is allowed.=0D
+=0D
+ @param[in] This The VARIABLE_LOCK_PROTOCOL instance.=0D
+ @param[in] VariableName A pointer to the variable name that will be mad=
e read-only subsequently.=0D
+ @param[in] VendorGuid A pointer to the vendor GUID that will be made =
read-only subsequently.=0D
+=0D
+ @retval EFI_SUCCESS The variable specified by the VariableName=
and the VendorGuid was marked=0D
+ as pending to be read-only.=0D
+ @retval EFI_INVALID_PARAMETER VariableName or VendorGuid is NULL.=0D
+ Or VariableName is an empty string.=0D
+ @retval EFI_ACCESS_DENIED EFI_END_OF_DXE_EVENT_GROUP_GUID or EFI_EVE=
NT_GROUP_READY_TO_BOOT has=0D
+ already been signaled.=0D
+ @retval EFI_OUT_OF_RESOURCES There is not enough resource to hold the l=
ock request.=0D
+**/=0D
+EFI_STATUS=0D
+EFIAPI=0D
+VariableLockRequestToLock (=0D
+ IN CONST EDKII_VARIABLE_LOCK_PROTOCOL *This,=0D
+ IN CHAR16 *VariableName,=0D
+ IN EFI_GUID *VendorGuid=0D
+ )=0D
+{=0D
+ EFI_STATUS Status;=0D
+ VARIABLE_POLICY_ENTRY *NewPolicy;=0D
+=0D
+ NewPolicy =3D NULL;=0D
+ Status =3D CreateBasicVariablePolicy( VendorGuid,=0D
+ VariableName,=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_LOCK_NOW,=0D
+ &NewPolicy );=0D
+ if (!EFI_ERROR( Status )) {=0D
+ Status =3D RegisterVariablePolicy( NewPolicy );=0D
+ }=0D
+ if (EFI_ERROR( Status )) {=0D
+ DEBUG(( DEBUG_ERROR, "%a - Failed to lock variable %s! %r\n", __FUNCTI=
ON__, VariableName, Status ));=0D
+ ASSERT_EFI_ERROR( Status );=0D
+ }=0D
+ if (NewPolicy !=3D NULL) {=0D
+ FreePool( NewPolicy );=0D
+ }=0D
+=0D
+ return Status;=0D
+}=0D
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.=
inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
index 8debc560e6dc..3005e9617423 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
@@ -49,6 +49,7 @@ [Sources]
VarCheck.c=0D
VariableExLib.c=0D
SpeculationBarrierDxe.c=0D
+ VariableLockRequstToLock.c=0D
=0D
[Packages]=0D
MdePkg/MdePkg.dec=0D
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf b/M=
deModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
index bbc8d2080193..26fbad97339f 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
@@ -58,6 +58,7 @@ [Sources]
VariableExLib.c=0D
TcgMorLockSmm.c=0D
SpeculationBarrierSmm.c=0D
+ VariableLockRequstToLock.c=0D
=0D
[Packages]=0D
MdePkg/MdePkg.dec=0D
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneM=
m.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
index 62f2f9252f43..7c6fdf4d65fd 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
@@ -58,6 +58,7 @@ [Sources]
VariableExLib.c=0D
TcgMorLockSmm.c=0D
SpeculationBarrierSmm.c=0D
+ VariableLockRequstToLock.c=0D
=0D
[Packages]=0D
MdePkg/MdePkg.dec=0D
--=20
2.28.0.windows.1


[PATCH v7 12/14] MdeModulePkg: Change TCG MOR variables to use VariablePolicy

Bret Barkelew
 

https://bugzilla.tianocore.org/show_bug.cgi?id=3D2522

These were previously using VarLock, which is
being deprecated.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Liming Gao <liming.gao@...>
Cc: Bret Barkelew <brbarkel@...>
Signed-off-by: Bret Barkelew <brbarkel@...>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c | 52 +=
+++++++++++++------
MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c | 52 +=
++++++++++++++-----
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf | 2 +
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf | 1 +
4 files changed, 82 insertions(+), 25 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c b/M=
deModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c
index e7accf4ed806..b85f08c48c11 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c
@@ -5,6 +5,7 @@
MOR lock control unsupported.=0D
=0D
Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>=0D
+Copyright (c) Microsoft Corporation.=0D
SPDX-License-Identifier: BSD-2-Clause-Patent=0D
=0D
**/=0D
@@ -17,7 +18,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/BaseMemoryLib.h>=0D
#include "Variable.h"=0D
=0D
-extern EDKII_VARIABLE_LOCK_PROTOCOL mVariableLock;=0D
+#include <Protocol/VariablePolicy.h>=0D
+#include <Library/VariablePolicyHelperLib.h>=0D
=0D
/**=0D
This service is an MOR/MorLock checker handler for the SetVariable().=0D
@@ -77,11 +79,6 @@ MorLockInit (
NULL // Data=0D
);=0D
=0D
- //=0D
- // Need set this variable to be read-only to prevent other module set it=
.=0D
- //=0D
- VariableLockRequestToLock (&mVariableLock, MEMORY_OVERWRITE_REQUEST_CONT=
ROL_LOCK_NAME, &gEfiMemoryOverwriteRequestControlLockGuid);=0D
-=0D
//=0D
// The MOR variable can effectively improve platform security only when =
the=0D
// MorLock variable protects the MOR variable. In turn MorLock cannot be=
made=0D
@@ -99,11 +96,6 @@ MorLockInit (
0, // DataSize=0D
NULL // Data=0D
);=0D
- VariableLockRequestToLock (=0D
- &mVariableLock,=0D
- MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME,=0D
- &gEfiMemoryOverwriteControlDataGuid=0D
- );=0D
=0D
return EFI_SUCCESS;=0D
}=0D
@@ -118,7 +110,39 @@ MorLockInitAtEndOfDxe (
VOID=0D
)=0D
{=0D
- //=0D
- // Do nothing.=0D
- //=0D
+ EFI_STATUS Status;=0D
+ EDKII_VARIABLE_POLICY_PROTOCOL *VariablePolicy;=0D
+=0D
+ // First, we obviously need to locate the VariablePolicy protocol.=0D
+ Status =3D gBS->LocateProtocol( &gEdkiiVariablePolicyProtocolGuid, NULL,=
(VOID**)&VariablePolicy );=0D
+ if (EFI_ERROR( Status )) {=0D
+ DEBUG(( DEBUG_ERROR, "%a - Could not locate VariablePolicy protocol! %=
r\n", __FUNCTION__, Status ));=0D
+ return;=0D
+ }=0D
+=0D
+ // If we're successful, go ahead and set the policies to protect the tar=
get variables.=0D
+ Status =3D RegisterBasicVariablePolicy( VariablePolicy,=0D
+ &gEfiMemoryOverwriteRequestControl=
LockGuid,=0D
+ MEMORY_OVERWRITE_REQUEST_CONTROL_L=
OCK_NAME,=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_LOCK_NOW );=0D
+ if (EFI_ERROR( Status )) {=0D
+ DEBUG(( DEBUG_ERROR, "%a - Could not lock variable %s! %r\n", __FUNCTI=
ON__, MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME, Status ));=0D
+ }=0D
+ Status =3D RegisterBasicVariablePolicy( VariablePolicy,=0D
+ &gEfiMemoryOverwriteControlDataGui=
d,=0D
+ MEMORY_OVERWRITE_REQUEST_VARIABLE_=
NAME,=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_LOCK_NOW );=0D
+ if (EFI_ERROR( Status )) {=0D
+ DEBUG(( DEBUG_ERROR, "%a - Could not lock variable %s! %r\n", __FUNCTI=
ON__, MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME, Status ));=0D
+ }=0D
+=0D
+ return;=0D
}=0D
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c b/M=
deModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c
index 085f82035f4b..ee37942a6b0c 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c
@@ -19,7 +19,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include "Variable.h"=0D
=0D
#include <Protocol/VariablePolicy.h>=0D
-=0D
+#include <Library/VariablePolicyHelperLib.h>=0D
#include <Library/VariablePolicyLib.h>=0D
=0D
typedef struct {=0D
@@ -422,6 +422,8 @@ MorLockInitAtEndOfDxe (
{=0D
UINTN MorSize;=0D
EFI_STATUS MorStatus;=0D
+ EFI_STATUS Status;=0D
+ VARIABLE_POLICY_ENTRY *NewPolicy;=0D
=0D
if (!mMorLockInitializationRequired) {=0D
//=0D
@@ -494,11 +496,25 @@ MorLockInitAtEndOfDxe (
// The MOR variable is absent; the platform firmware does not support it=
.=0D
// Lock the variable so that no other module may create it.=0D
//=0D
- VariableLockRequestToLock (=0D
- NULL, // This=0D
- MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME,=0D
- &gEfiMemoryOverwriteControlDataGuid=0D
- );=0D
+ NewPolicy =3D NULL;=0D
+ Status =3D CreateBasicVariablePolicy( &gEfiMemoryOverwriteControlDataGui=
d,=0D
+ MEMORY_OVERWRITE_REQUEST_VARIABLE_NA=
ME,=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_LOCK_NOW,=0D
+ &NewPolicy );=0D
+ if (!EFI_ERROR( Status )) {=0D
+ Status =3D RegisterVariablePolicy( NewPolicy );=0D
+ }=0D
+ if (EFI_ERROR( Status )) {=0D
+ DEBUG(( DEBUG_ERROR, "%a - Failed to lock variable %s! %r\n", __FUNCTI=
ON__, MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME, Status ));=0D
+ ASSERT_EFI_ERROR( Status );=0D
+ }=0D
+ if (NewPolicy !=3D NULL) {=0D
+ FreePool( NewPolicy );=0D
+ }=0D
=0D
//=0D
// Delete the MOR Control Lock variable too (should it exists for some=0D
@@ -514,9 +530,23 @@ MorLockInitAtEndOfDxe (
);=0D
mMorLockPassThru =3D FALSE;=0D
=0D
- VariableLockRequestToLock (=0D
- NULL, // This=0D
- MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME,=0D
- &gEfiMemoryOverwriteRequestControlLockGuid=0D
- );=0D
+ NewPolicy =3D NULL;=0D
+ Status =3D CreateBasicVariablePolicy( &gEfiMemoryOverwriteRequestControl=
LockGuid,=0D
+ MEMORY_OVERWRITE_REQUEST_CONTROL_LOC=
K_NAME,=0D
+ VARIABLE_POLICY_NO_MIN_SIZE,=0D
+ VARIABLE_POLICY_NO_MAX_SIZE,=0D
+ VARIABLE_POLICY_NO_MUST_ATTR,=0D
+ VARIABLE_POLICY_NO_CANT_ATTR,=0D
+ VARIABLE_POLICY_TYPE_LOCK_NOW,=0D
+ &NewPolicy );=0D
+ if (!EFI_ERROR( Status )) {=0D
+ Status =3D RegisterVariablePolicy( NewPolicy );=0D
+ }=0D
+ if (EFI_ERROR( Status )) {=0D
+ DEBUG(( DEBUG_ERROR, "%a - Failed to lock variable %s! %r\n", __FUNCTI=
ON__, MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME, Status ));=0D
+ ASSERT_EFI_ERROR( Status );=0D
+ }=0D
+ if (NewPolicy !=3D NULL) {=0D
+ FreePool( NewPolicy );=0D
+ }=0D
}=0D
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.=
inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
index 48ac167906f7..8debc560e6dc 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
@@ -71,6 +71,7 @@ [LibraryClasses]
AuthVariableLib=0D
VarCheckLib=0D
VariablePolicyLib=0D
+ VariablePolicyHelperLib=0D
=0D
[Protocols]=0D
gEfiFirmwareVolumeBlockProtocolGuid ## CONSUMES=0D
@@ -80,6 +81,7 @@ [Protocols]
gEfiVariableWriteArchProtocolGuid ## PRODUCES=0D
gEfiVariableArchProtocolGuid ## PRODUCES=0D
gEdkiiVariableLockProtocolGuid ## PRODUCES=0D
+ gEdkiiVariablePolicyProtocolGuid ## CONSUMES=0D
gEdkiiVarCheckProtocolGuid ## PRODUCES=0D
=0D
[Guids]=0D
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneM=
m.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
index d8f480be27cc..62f2f9252f43 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
@@ -76,6 +76,7 @@ [LibraryClasses]
SynchronizationLib=0D
VarCheckLib=0D
VariablePolicyLib=0D
+ VariablePolicyHelperLib=0D
=0D
[Protocols]=0D
gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES=0D
--=20
2.28.0.windows.1


[PATCH v7 11/14] SecurityPkg: Allow VariablePolicy state to delete authenticated variables

Bret Barkelew
 

https://bugzilla.tianocore.org/show_bug.cgi?id=3D2522

Causes AuthService to check
IsVariablePolicyEnabled() before enforcing
write protections to allow variable deletion
when policy engine is disabled.

Only allows deletion, not modification.

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Chao Zhang <chao.b.zhang@...>
Cc: Bret Barkelew <brbarkel@...>
Signed-off-by: Bret Barkelew <brbarkel@...>
---
SecurityPkg/Library/AuthVariableLib/AuthService.c | 22 +++++++++++++=
+++----
SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf | 2 ++
2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c b/SecurityPk=
g/Library/AuthVariableLib/AuthService.c
index 2f60331f2c04..aca9a5620c28 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
@@ -19,12 +19,16 @@
to verify the signature.=0D
=0D
Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>=0D
+Copyright (c) Microsoft Corporation.=0D
SPDX-License-Identifier: BSD-2-Clause-Patent=0D
=0D
**/=0D
=0D
#include "AuthServiceInternal.h"=0D
=0D
+#include <Protocol/VariablePolicy.h>=0D
+#include <Library/VariablePolicyLib.h>=0D
+=0D
//=0D
// Public Exponent of RSA Key.=0D
//=0D
@@ -217,9 +221,12 @@ NeedPhysicallyPresent(
IN EFI_GUID *VendorGuid=0D
)=0D
{=0D
- if ((CompareGuid (VendorGuid, &gEfiSecureBootEnableDisableGuid) && (StrC=
mp (VariableName, EFI_SECURE_BOOT_ENABLE_NAME) =3D=3D 0))=0D
- || (CompareGuid (VendorGuid, &gEfiCustomModeEnableGuid) && (StrCmp (Va=
riableName, EFI_CUSTOM_MODE_NAME) =3D=3D 0))) {=0D
- return TRUE;=0D
+ // If the VariablePolicy engine is disabled, allow deletion of any authe=
nticated variables.=0D
+ if (IsVariablePolicyEnabled()) {=0D
+ if ((CompareGuid (VendorGuid, &gEfiSecureBootEnableDisableGuid) && (St=
rCmp (VariableName, EFI_SECURE_BOOT_ENABLE_NAME) =3D=3D 0))=0D
+ || (CompareGuid (VendorGuid, &gEfiCustomModeEnableGuid) && (StrCmp (=
VariableName, EFI_CUSTOM_MODE_NAME) =3D=3D 0))) {=0D
+ return TRUE;=0D
+ }=0D
}=0D
=0D
return FALSE;=0D
@@ -842,7 +849,8 @@ ProcessVariable (
&OrgVariableInfo=0D
);=0D
=0D
- if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable (OrgVariableInfo.Attri=
butes, Data, DataSize, Attributes) && UserPhysicalPresent()) {=0D
+ // If the VariablePolicy engine is disabled, allow deletion of any authe=
nticated variables.=0D
+ if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable (OrgVariableInfo.Attri=
butes, Data, DataSize, Attributes) && (UserPhysicalPresent() || !IsVariable=
PolicyEnabled())) {=0D
//=0D
// Allow the delete operation of common authenticated variable(AT or A=
W) at user physical presence.=0D
//=0D
@@ -1960,6 +1968,12 @@ VerifyTimeBasedPayload (
=0D
CopyMem (Buffer, PayloadPtr, PayloadSize);=0D
=0D
+ // If the VariablePolicy engine is disabled, allow deletion of any authe=
nticated variables.=0D
+ if (PayloadSize =3D=3D 0 && (Attributes & EFI_VARIABLE_APPEND_WRITE) =3D=
=3D 0 && !IsVariablePolicyEnabled()) {=0D
+ VerifyStatus =3D TRUE;=0D
+ goto Exit;=0D
+ }=0D
+=0D
if (AuthVarType =3D=3D AuthVarTypePk) {=0D
//=0D
// Verify that the signature has been made with the current Platform K=
ey (no chaining for PK).=0D
diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf b/Secu=
rityPkg/Library/AuthVariableLib/AuthVariableLib.inf
index 8d4ce14df494..8eadeebcebd7 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
+++ b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
@@ -3,6 +3,7 @@
#=0D
# Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>=
=0D
# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>=0D
+# Copyright (c) Microsoft Corporation.=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -41,6 +42,7 @@ [LibraryClasses]
MemoryAllocationLib=0D
BaseCryptLib=0D
PlatformSecureLib=0D
+ VariablePolicyLib=0D
=0D
[Guids]=0D
## CONSUMES ## Variable:L"SetupMode"=0D
--=20
2.28.0.windows.1