Add configuration IgnoreFiles for package config files.
So users can rely on this to skip license conflict for
some generated files.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2793
In current implementation TPM2_ChangeEPS command is always available
in the TPM2 operation pull down list in TCG2 Configuration, which
is confusing when the command is not supported by specific TPM chip.
As a user experience improvement, TPM2_ChangeEPS command should be
removed from the list when it is not supported.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Qi Zhang <qi1.zhang@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Tpm2GetCapabilityIsCommandImplemented
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2793
check if the commad is supported by comparing the command code with
command index.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Qi Zhang <qi1.zhang@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
When we allocate pool to save rebased the PEIMs, the address will change
randomly, therefore the hash will change and result PCR0 change as well.
To avoid this, we save the raw PEIMs and use it to calculate hash.
The TcgPei calculate the hash and it use the Migrated FV Info.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Qi Zhang <qi1.zhang@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
When we allocate pool to save rebased the PEIMs, the address will change
randomly, therefore the hash will change and result PCR0 change as well.
To avoid this, we save the raw PEIMs and use it to calculate hash.
The Tcg2Pei calculate the hash and it use the Migrated FV Info.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Qi Zhang <qi1.zhang@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
And do some code clean with updated function
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2841
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Qi Zhang <qi1.zhang@intel.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
https://bugzilla.tianocore.org/show_bug.cgi?id=2713
In current code, If TCG2_PHYSICAL_PRESENCE_FLAGS_VARIABLE variable
is not exist, code will get default value from two places.
This fix is to make the default value comes from the PCD
gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags
Signed-off-by: Maggie Chu <maggie.chu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2652
According to the File description, the File is optional and can be NULL.
Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2652
According to the File description, The File is optional and can be NULL.
Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2652
DxeTpm2MeasureBootHandler() and DxeTpmMeasureBootHandler() functions may
receive a FileBuffer argument that is not associated with any particular
device path (e.g., because the UEFI image has not been loaded from any
particular device path).
Therefore rejecting (File==NULL) at the top of the function is invalid.
Fixes: 4b026f0d5a
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2416
Replaces the hardcoded value of 0xB2 with a PCD for the SMI port access
operation region. This allows platforms to customize the IO port value if
necessary.
Cc: Kun Qin <Kun.Qin@microsoft.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Guomin Jiang <guomin.jiang@intel.com>
Reviewed-by: Bret Barkelew <bret.barkelew@microsoft.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2416
Adds a PCD to SecurityPkg used to specify the SW SMI Command port. This
value may vary across platforms and is current hardcoded to 0xB2 in
package code.
Cc: Kun Qin <Kun.Qin@microsoft.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Guomin Jiang <guomin.jiang@intel.com>
Reviewed-by: Bret Barkelew <bret.barkelew@microsoft.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1959
Replaces usage of the linked list iteration macros defined in
HddPasswordDxe.h with the common definition in BaseLib.h.
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Reviewed-by: Bret Barkelew <bret.barkelew@microsoft.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2594
Add null version of VariableKeyLib instance. The full version should be
provided by platforms which supports key generator.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Nishant C Mistry <nishant.c.mistry@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2594
Add null version of RpmcLib instance. The full version should be provided
by platform which supports RPMC device.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Nishant C Mistry <nishant.c.mistry@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2594
RpmcLib.h and VariableKeyLib.h are header files required to access RPMC
device and Key generator from platform. They will be used to ensure the
integrity and confidentiality of NV variables.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Nishant C Mistry <nishant.c.mistry@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
IsCertHashFoundInDatabase() is actually used only for searching dbx,
according to the function logic, its comments and its use cases. Changing
it to IsCertHashFoundInDbx to avoid confusion.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608
To avoid false-negative issue in check hash against dbx, both error
condition (as return value) and check result (as out parameter) of
IsSignatureFoundInDatabase() are added. So the caller of this function
will know exactly if a failure is caused by a black list hit or
other error happening, and enforce a more secure operation to prevent
secure boot from being bypassed. For a white list check (db), there's
no such necessity.
All intermediate results inside this function will be checked and
returned immediately upon any failure or error, like out-of-resource,
hash calculation error or certificate retrieval failure.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608
If the second GetVariable() call for "dbx" fails, in IsForbiddenByDbx(),
we have to free Data. Jump to "Done" for that.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608
All intermediate results inside this function will be checked and
returned immediately upon any failure or error, like out-of-resource,
hash calculation error or certificate retrieval failure.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608
To avoid false-negative issue in check hash against dbx, both error
condition (as return value) and check result (as out parameter) of
IsCertHashFoundInDatabase() are added. So the caller of this function
will know exactly if a failure is caused by a black list hit or
other error happening, and enforce a more secure operation to prevent
secure boot from being bypassed. For a white list check (db), there's
no such necessity.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608
The dbx fetching code inside the while/for-loop causes code hard to
understand. Since there's no need to get dbx more than once, this patch
simplify the code logic by moving related code to be outside the while-
loop. db fetching code is also refined accordingly to reduce the indent
level of code.
More comments are also added or refined to explain more details.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608
In timestamp check after the cert is found in db, the original code jumps
to 'Done' if any error happens in fetching dbx variable. At any of the
jump, VerifyStatus equals to TRUE, which means allowed-by-db. This should
not be allowed except to EFI_NOT_FOUND case (meaning dbx doesn't exist),
because it could be used to bypass timestamp check.
This patch add code to change VerifyStatus to FALSE in the case of memory
allocation failure and dbx fetching failure to avoid potential bypass
issue.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608
Normally two times of calling gRT->GetVariable() are needed to get
the data of a variable: get the variable size by passing zero variable
size, and then allocate enough memory and pass the correct variable size
and buffer.
But in the inner loop in IsAllowedByDb(), the DbxDataSize was not
initialized to zero before calling gRT->GetVariable(). It won't cause
problem if dbx does not exist. But it will give wrong result if dbx
exists and the DbxDataSize happens to be a small enough value. In this
situation, EFI_BUFFER_TOO_SMALL will be returned. Then the result check
code followed will jump to 'Done', which is not correct because it's
actually the value expected.
if (Status == EFI_BUFFER_TOO_SMALL) {
goto Done;
}
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
In case the signers' certificate stack, retrieved from the PE/COFF image's
Authenticode blob, has zero elements (=there are zero signer certificates),
then we should consider the image forbidden by DBX, not accepted by DBX.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1608
Pointer HashCtx used in IsCertHashFoundInDatabase() is not freed inside
the while-loop, if it will run more than once.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Fix few typos in comments and documentation.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Antoine Coeur <coeur@gmx.fr>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
Message-Id: <20200207010831.9046-67-philmd@redhat.com>
Fix various typos in documentation, comments and debug strings.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Antoine Coeur <coeur@gmx.fr>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
Message-Id: <20200207010831.9046-66-philmd@redhat.com>
[lersek@redhat.com: replace EFI_D_xxx w/ DEBUG_xxx to shut up PatchCheck]
Fix various typos in comments and documentation.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Antoine Coeur <coeur@gmx.fr>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
Message-Id: <20200207010831.9046-65-philmd@redhat.com>
Fix few typos in documentation, comments and debug strings.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Antoine Coeur <coeur@gmx.fr>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
Message-Id: <20200207010831.9046-64-philmd@redhat.com>
Fix few typos in comments and documentation.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Antoine Coeur <coeur@gmx.fr>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
Message-Id: <20200207010831.9046-63-philmd@redhat.com>
The DxeTpmMeasureBootHandler and DxeTpm2MeasureBootHandler handlers
are SECURITY2_FILE_AUTHENTICATION_HANDLER prototype. This prototype
can not return EFI_INVALID_PARAMETER.
The prototype documentation states it returns EFI_ACCESS_DENIED if:
"The file specified by File and FileBuffer did not authenticate,
and the platform policy dictates that the DXE Foundation may not
use File."
Correct the documentation, and add a early check, returning
EFI_ACCESS_DENIED when File is NULL.
Noticed while reviewing commit 6d57592740.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2472
Replace the ASSERT with the error code return in the TpmPhysicalPresence
and GetTpmCapability.
Add missing error checking after call TpmPhysicalPresence in
TcgPhysicalPresenceLibProcessRequest.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
In DxeImageVerificationHandler(), we should return EFI_SECURITY_VIOLATION
for a rejected image only if the platform sets
DEFER_EXECUTE_ON_SECURITY_VIOLATION as the policy for the image's source.
Otherwise, EFI_ACCESS_DENIED must be returned.
Right now, EFI_SECURITY_VIOLATION is returned for all rejected images,
which is wrong -- it causes LoadImage() to hold on to rejected images (in
untrusted state), for further platform actions. However, if a platform
already set DENY_EXECUTE_ON_SECURITY_VIOLATION, the platform will not
expect the rejected image to stick around in memory (regardless of its
untrusted state).
Therefore, adhere to the platform policy in the return value of the
DxeImageVerificationHandler() function.
Furthermore, according to "32.4.2 Image Execution Information Table" in
the UEFI v2.8 spec, and considering that edk2 only supports (AuditMode==0)
at the moment:
> When AuditMode==0, if the image's signature is not found in the
> authorized database, or is found in the forbidden database, the image
> will not be started and instead, information about it will be placed in
> this table.
we have to store an EFI_IMAGE_EXECUTION_INFO record in both the "defer"
case and the "deny" case. Thus, the AddImageExeInfo() call is not being
made conditional on (Policy == DEFER_EXECUTE_ON_SECURITY_VIOLATION); the
documentation is updated instead.
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129
Fixes: 5db28a6753
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200116190705.18816-12-lersek@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
[lersek@redhat.com: push with Mike's R-b due to Chinese New Year
Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid
<d3fbb76dabed4e1987c512c328c82810@intel.com>]
It makes no sense to call AddImageExeInfo() with (Signature == NULL) and
(SignatureSize > 0). AddImageExeInfo() does not crash in such a case -- it
avoids the CopyMem() call --, but it creates an invalid
EFI_IMAGE_EXECUTION_INFO record. Namely, the
"EFI_IMAGE_EXECUTION_INFO.InfoSize" field includes "SignatureSize", but
the actual signature bytes are not filled in.
Document and ASSERT() this condition in AddImageExeInfo().
In DxeImageVerificationHandler(), zero out "SignatureListSize" if we set
"SignatureList" to NULL due to AllocateZeroPool() failure.
(Another approach could be to avoid calling AddImageExeInfo() completely,
in case AllocateZeroPool() fails. Unfortunately, the UEFI v2.8 spec does
not seem to state clearly whether a signature is mandatory in
EFI_IMAGE_EXECUTION_INFO, if the "Action" field is
EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED or EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND.
For now, the EFI_IMAGE_EXECUTION_INFO addition logic is not changed; we
only make sure that the record we add is not malformed.)
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200116190705.18816-11-lersek@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
[lersek@redhat.com: push with Mike's R-b due to Chinese New Year
Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid
<d3fbb76dabed4e1987c512c328c82810@intel.com>]
"FileBuffer" is a non-optional input (pointer) parameter to
DxeImageVerificationHandler(). Normally, when an edk2 function receives a
NULL argument for such a parameter, we return EFI_INVALID_PARAMETER or
RETURN_INVALID_PARAMETER. However, those don't conform to the
SECURITY2_FILE_AUTHENTICATION_HANDLER prototype.
Return EFI_ACCESS_DENIED when "FileBuffer" is NULL; it means that no image
has been loaded.
This patch does not change the control flow in the function, it only
changes the "Status" outcome from API-incompatible error codes to
EFI_ACCESS_DENIED, under some circumstances.
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129
Fixes: 570b3d1a72
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200116190705.18816-10-lersek@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
[lersek@redhat.com: push with Mike's R-b due to Chinese New Year
Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid
<d3fbb76dabed4e1987c512c328c82810@intel.com>]
The "Status" variable is set to EFI_ACCESS_DENIED at the top of the
function. Then it is overwritten with EFI_SECURITY_VIOLATION under the
"Failed" (earlier: "Done") label. We finally return "Status".
The above covers the complete usage of "Status" in
DxeImageVerificationHandler(). Remove the variable, and simply return
EFI_SECURITY_VIOLATION in the end.
This patch is a no-op, regarding behavior.
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200116190705.18816-9-lersek@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
[lersek@redhat.com: push with Mike's R-b due to Chinese New Year
Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid
<d3fbb76dabed4e1987c512c328c82810@intel.com>]
Before the "Done" label at the end of DxeImageVerificationHandler(), we
now have a single access to "Status": we set "Status" to EFI_ACCESS_DENIED
at the top of the function. Therefore, the (Status != EFI_SUCCESS)
condition is always true under the "Done" label.
Accordingly, unnest the AddImageExeInfo() call dependent on that
condition, remove the condition, and also rename the "Done" label to
"Failed".
Functionally, this patch is a no-op. It's easier to review with:
git show -b -W
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200116190705.18816-8-lersek@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
[lersek@redhat.com: replace EFI_D_INFO w/ DEBUG_INFO for PatchCheck.py]
[lersek@redhat.com: push with Mike's R-b due to Chinese New Year
Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid
<d3fbb76dabed4e1987c512c328c82810@intel.com>]
After the final "IsVerified" check, we set "Status" to EFI_ACCESS_DENIED.
This is superfluous, as "Status" already carries EFI_ACCESS_DENIED value
there, from the top of the function. Remove the assignment.
Functionally, this change is a no-op.
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200116190705.18816-7-lersek@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
[lersek@redhat.com: push with Mike's R-b due to Chinese New Year
Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid
<d3fbb76dabed4e1987c512c328c82810@intel.com>]
A SECURITY2_FILE_AUTHENTICATION_HANDLER function is not expected to return
EFI_OUT_OF_RESOURCES. We should only return EFI_SUCCESS,
EFI_SECURITY_VIOLATION, or EFI_ACCESS_DENIED.
In case we run out of memory while preparing "SignatureList" for
AddImageExeInfo(), we should simply stick with the EFI_ACCESS_DENIED value
that is already in "Status" -- from just before the "Action" condition --,
and not suppress it with EFI_OUT_OF_RESOURCES.
This patch does not change the control flow in the function, it only
changes the "Status" outcome from API-incompatible error codes to
EFI_ACCESS_DENIED, under some circumstances.
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129
Fixes: 570b3d1a72
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200116190705.18816-6-lersek@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
[lersek@redhat.com: push with Mike's R-b due to Chinese New Year
Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid
<d3fbb76dabed4e1987c512c328c82810@intel.com>]
Inside the "for" loop that scans the signatures of the image, we call
HashPeImageByType(), and assign its return value to "Status".
Beyond the immediate retval check, this assignment is useless (never
consumed). That's because a subsequent access to "Status" may only be one
of the following:
- the "Status" assignment when we call HashPeImageByType() in the next
iteration of the loop,
- the "Status = EFI_ACCESS_DENIED" assignment right after the final
"IsVerified" check.
To make it clear that the assignment is only useful for the immediate
HashPeImageByType() retval check, introduce a specific helper variable,
called "HashStatus".
This patch is a no-op, functionally.
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200116190705.18816-5-lersek@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
[lersek@redhat.com: push with Mike's R-b due to Chinese New Year
Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid
<d3fbb76dabed4e1987c512c328c82810@intel.com>]
Shenglei Zhang
Qi Zhang
Guomin Jiang
Jiewen Yao
Maggie Chu
Michael Kubacki
Abner Chang
Jian J Wang
Laszlo Ersek
GuoMinJ
Antoine Coeur
Philippe Mathieu-Daudé
Zhichao Gao