Currently a EFI_DEVICE_ERROR is returned if `TPM_RC_HANDLE` is the
return code from a TPM2_NV_ReadPublic command. However, in the TCG
TPM Library Part 3: Commands specification, `TPM_RC_HANDLE` is a
return code if:
1. An Index does not exist that corresponds to the handle
(TPM_RC_HANDLE)
2. The hierarchy associated with the existing NV Index is not
enabled (TPM_RC_HANDLE)
Therefore, return EFI_NOT_FOUND in this case, since that more
precisely allows a caller to identify this condition and act on
it as opposed to a more generic device error.
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
This reverts commit ba6a8eb045.
PR https://github.com/tianocore/edk2/pull/11307 introduced a
logic change that caused regressions in FV verification on
some platforms. This PR is being reverted to restore the prior
logic.
The Code QL fixes in https://github.com/tianocore/edk2/pull/11307
can be resubmitted without the logic change along with one
commit for each type of Code QL issue being addressed.
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
First, If there are multiple devices in DeviceList and are going to
remove the first device in the DeviceList, the DeviceList will be
cleared up with setting to NULL.
This is not the expected behavior, as it should keep the rest of the
devices in the DeviceList. DeviceList should point to the next device,
Dev->Next.
Second, there is a potential infinite while loop if TmpDev->Next not
equal to Dev. TmpDev should point to next device.
Signed-off-by: Hunter Chang <hunter.chang@intel.com>
Add initial unit test for DxeImageVerificationHandler to
validate signature verification bypass for selected image types.
Signed-off-by: Alexander Gryanko <xpahos@gmail.com>
To support TPM2 devices that operate over the FF-A specification using CRB
in the Tcg2Pei PEIM, add the Tcg2ConfigFfaPei PEIM to
detect the presence of such TPM2 devices.
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
To support measurement and extend PCR in PeilessSec with
TPM device using FF-A over CRB, add Tpm2DeviceSecLibFfa for PeilessSec.
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
This is preparation patch to add Tpm2DeviceSecLibFfa for SEC
used in PeilessSec.
In SEC phase, DynamicPcd used for cacahing TPM2 information
couldn't be used.
To resolve this, writes wrapper functions to get TPM2 information
so that in the wrapper functions used in SEC wouldn't use the
related DyanmicPcd.
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
Makes changes to comply with alerts raised by CodeQL.
Most of the issues here fall into the following two categories:
1. Potential use of uninitialized pointer.
2. Inconsistent integer width in comparison.
Co-authored-by: Taylor Beebe <31827475+TaylorBeebe@users.noreply.github.com>
Co-authored-by: kenlautner <85201046+kenlautner@users.noreply.github.com>
Co-authored-by: Bret Barkelew <bret@corthon.com>
Signed-off-by: Doug Flick <dougflick@microsoft.com>
When DEBUG_SECURITY was added, not all prints in
Tpm2DeviceLibDTpmDump.c were updated. This commit updates
the remaining prints.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
The Event3 memory comparison is technically correct since the
definitions of the struct types are the same. The extended
bodies of the events are different. The Event2 size guard
for the Event3 comparison should be split to use the Event3
in its sizeof for better clarity.
The large single condition makes the function difficult to
understand, so the combined logic is split into different
conditional statements.
Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
[ardb: whitespace fixes]
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
This commit adds additional dumping logic to Tpm2DeviceLibDTpm,
print at DEBUG_SECURITY to aid in TPM debugging.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
Co-authored-by: Bret Barkelew <brbarkel@microsoft.com>
The TPM code is currently very noisy (e.g. in a sample platform,
4,000 of the 5,700 lines printed to the serial port at DEBUG_INFO
level were from the TPM code). For TPM debugging, this is very
critical information, but for most builds it simply spams the logs
and slows down the build.
This commit moves the event log and PCR dumping to log at
DEBUG_SECURITY level.
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
The TCG code is very noisy when a TPM is connected. This
commit downgrades some prints to verbose and removes some
others that do not have value (such as function enter and
exit prints).
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
CC_MR_INDEX_0_MRTD indicates the MRTD register. This register is NOT
extendable in TDVF. So the return value of TdxMeasurementMapPcrToMrIndex
must be checked.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Ceping Sun <cepingx.sun@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3267
Adds two new helper functions.
Currently, a function exists in Tpm2CommandLib to copy a digest list
to a buffer. A function to perform the inverse operation - copying
from a buffer to a digest list is added.
A function is also added to compute the total digest size for a given
hash algorithm mask.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Some of the spdmlib crypto functions return 'false' in
functions that return a pointer to indicate a null
return. false is mapped to FALSE to cover other usages
to return a boolean value.
Add -Wno-non-literal-null-conversion for CLANGPDB and
CLANGDWARF to ignore these types of errors from CLANG
builds within this one library build that uses the
spdmlib git submodule.
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Fix CLANG 20.1.0 enum conversion errors
Address implied conversion between enum types by using
the enum type from spdmlib and remove the enum types
that are never used after this update.
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
This change introduces a `Tpm2InstanceLibFfa` library to support TPM over
FF-A and works with Tpm2DeviceLibRouter* libraries.
The implementation follows the TPM over FF-A spec v1.0 BET:
https://developer.arm.com/documentation/den0138/latest/
The change is tested on QEMU SBSA virtual platform and proprietary
hardware platforms.
Signed-off-by: Kun Qin <kun.qin@microsoft.com>
This change introduces a `Tpm2DeviceLibFfa` library to support TPM over
FF-A.
The implementation follows the TPM over FF-A spec v1.0 BET:
https://developer.arm.com/documentation/den0138/latest/
The change is tested on QEMU SBSA virtual platform and proprietary
hardware platforms.
Co-authored-by: Raymond Diaz <raymonddiaz@microsoft.com>
Signed-off-by: Kun Qin <kun.qin@microsoft.com>
TPM over FF-A is a mechanism enabling the normal world to communicate
with TPM devices offered as a FF-A service in the secure world.
This update introduces a header file containing definitions from the TPM
over FF-A specification, as detailed in the following documentation:
https://developer.arm.com/documentation/den0138/latest/
Signed-off-by: Kun Qin <kun.qin@microsoft.com>
This change adds a new driver Tcg2AcpiFfa. It will publish the TPM2 and
the corresponding SSDT table that is responsible for supporting the
physical presence interface through ASL methods during OS runtime.
Co-authored-by: Raymond Diaz <raymonddiaz@microsoft.com>
Signed-off-by: Kun Qin <kun.qin@microsoft.com>
This change adds a new library instance of SmmTcg2PhysicalPresenceLib. It
will directly check on the PCD value instead of relying on the HOB value,
which will require change on the TFA/SPMC side.
Signed-off-by: Kun Qin <kun.qin@microsoft.com>
This change adds a new driver Tcg2StandaloneMmArm. It will register an
MMI handler that is responsible for supporting the physical presence
interface from ASL methods during OS runtime.
Platforms need to expose the PPI ACPI function GUID in the Standalone MM
secure partition.
Signed-off-by: Kun Qin <kun.qin@microsoft.com>
This change adds a GUID for the physical presence interface. This is
defined in TCG Physical Presence Interface v1.30, Rev. 00.52: Section
8.1 ACPI Functions.
Signed-off-by: Kun Qin <kun.qin@microsoft.com>
Improve OpalDriverGetDriverDeviceName function that gets device name
through the component name protocol. Currently the function searches for
all handles (as controller handle) to find the right GetControllerName
service for the child handle. The update improves the way to get device
name and supports better performance (e.g. 1681(μs) -> 3(μs) for 1 NVMe
device). This can prevent a compatibility issue for GetControllerName
service of some drivers, which is not flexible for handle parameter
information (e.g. it was found that an EFI driver caused an exception
error/hang when GetControllerName service for the driver is called in
OpalDriverGetDeviceNameByProtocol function).
Signed-off-by: Phil Noh <Phil.Noh@amd.com>
AmdSvsmLib currently doesn't check if SNP enabled, thus using AmdSvsmLib
may errantly cause the caller code to believe SVSM is present. This
leads to boot failure on non-SNP enabled VMs.
We use the PcdConfidentialComputingGuestAttr since it remains valid
after MpInitLib runs which invalidates PcdSevEsWorkArea's cached
sev-status msr which we use to check for SNP enabled in other places.
The added functions ConfidentialComputingGuestHas() and
AmdMemEncryptionAttrCheck() are copied from MpLib.c, which is intended
to be replaced later on with a more minimal library perhaps in MdePkg to
cleanup some of the circular dependencies currently surrounding SvsmLib.
Signed-off-by: Jacob Xu <jacobhxu@google.com>
Signed-off-by: Oliver Steffen <osteffen@redhat.com>
Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
This commit adds the ability to skip the setting the Dbx variable if
the Default being provided is less than the size of the
EFI_SIGNATURE_LIST structure. This is to prevent the
setting of an invalid DBX which would cause the system to fail to boot.
Additionally, this can be used to signal that setting the DBX
should leave DBX undefined for Platforms that want to let the OS
be the sole servicer of the DBX.
Breakdown of the math is as follows:
1. **`sizeof(EFI_SIGNATURE_LIST)`**:
- This is the size of the `EFI_SIGNATURE_LIST` structure itself,
which includes:
- `EFI_GUID SignatureType` (16 bytes)
- `UINT32 SignatureListSize` (4 bytes)
- `UINT32 SignatureHeaderSize` (4 bytes)
- `UINT32 SignatureSize` (4 bytes)
- Total: `16 + 4 + 4 + 4 = 28 bytes`
2. **`SignatureHeaderSize`**:
- This is the size of the optional signature header. If no header is
provided, this value is `0`.
3. **`SignatureSize`**:
- This is the size of each `EFI_SIGNATURE_DATA` entry. For an empty
list, this value is `0`.
The total size of an empty `EFI_SIGNATURE_LIST` is:
```c
sizeof(EFI_SIGNATURE_LIST) + SignatureHeaderSize
```
1. **No Signature Header**:
- If `SignatureHeaderSize = 0`, the size is:
```c
28 + 0 = 28 bytes
```
2. **With a Signature Header**:
- If `SignatureHeaderSize = 16` (example size for a header), the
size is:
```c
28 + 16 = 44 bytes
```
- **Minimum Size**: `28 bytes` (if `SignatureHeaderSize = 0`).
- **Additional Size**: Add the value of `SignatureHeaderSize` if a
header is included.
Signed-off-by: Doug Flick <dougflick@microsoft.com>
RsaGetPublicKeyFromX509 allocates memory for RsaContext parameter
and the memory allocated earlier is not necessary
Signed-off-by: Anbazhagan Baraneedharan <anbazhagan@hp.com>
This commit updates the SecurityFixes.yaml file to include
information about the CVE-2024-38797 vulnerability.
Signed-off-by: Doug Flick <DougFlick@microsoft.com>
Namely:
(1) The TWO_BYTE_ENCODE check is independent of Index. If it evalutes
to TRUE for Index==0, then it will evaluate to TRUE for all other
Index values as well. As a result, the (Index == HASHALG_MAX)
condition will fire after the loop, and we'll return
EFI_UNSUPPORTED.
While this is correct, functionally speaking, it is wasteful to
keep re-checking TWO_BYTE_ENCODE in the loop body. The check
should be made at the top of the function, and EFI_UNSUPPORTED
should be returned at once, if appropriate.
(2) If the hash algorithm selected by Index has such a large OID that
the OID comparison cannot even be performed (because AuthDataSize
is not large enough for containing the OID in question, starting
at offset 32), then the function returns EFI_UNSUPPORTED at once.
This is bogus; this case should simply be treated as an OID
mismatch, and the loop should advance to the next Index value /
hash algorithm candidate. A remaining hash algo may have a shorter
OID and yield an OID match.
Signed-off-by: Doug Flick <DougFlick@microsoft.com>
Namely:
(1) The TWO_BYTE_ENCODE check is independent of Index. If it evalutes
to TRUE for Index==0, then it will evaluate to TRUE for all other
Index values as well. As a result, the (Index == HASHALG_MAX)
condition will fire after the loop, and we'll return
EFI_UNSUPPORTED.
While this is correct, functionally speaking, it is wasteful to
keep re-checking TWO_BYTE_ENCODE in the loop body. The check
should be made at the top of the function, and EFI_UNSUPPORTED
should be returned at once, if appropriate.
(2) If the hash algorithm selected by Index has such a large OID that
the OID comparison cannot even be performed (because AuthDataSize
is not large enough for containing the OID in question, starting
at offset 32), then the function returns EFI_UNSUPPORTED at once.
This is bogus; this case should simply be treated as an OID
mismatch, and the loop should advance to the next Index value /
hash algorithm candidate. A remaining hash algo may have a shorter
OID and yield an OID match.
Signed-off-by: Doug Flick <DougFlick@microsoft.com>
In HashPeImageByType(), the hash of PE/COFF image is calculated.
This function may get untrusted input.
Inside this function, the following code verifies the loaded image has
the correct format, by reading the second byte of the buffer.
```c
if ((*(AuthData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) {
...
}
```
The input image is not trusted and that may not have the second byte to
read. So this poses an out of bound read error.
With below fix we are assuring that we don't do out of bound read. i.e,
we make sure that AuthDataSize is greater than 1.
```c
if (AuthDataSize > 1
&& (*(AuthData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE){
...
}
```
AuthDataSize size is verified before reading the second byte.
So if AuthDataSize is less than 2, the second byte will not be read, and
the out of bound read situation won't occur.
Tested the patch on real platform with and without TPM connected and
verified image is booting fine.
Authored-by: Raj AlwinX Selvaraj <Alw...@intel.com>
Signed-off-by: Doug Flick <DougFlick@microsoft.com>
Add the SecTpmMeasurementLib to support
TpmMeasurementAndLogData in Sec phase.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
Signed-off-by: Ceping Sun <cepingx.sun@intel.com>
SEV-SNP provides a feature known as VM Privilege Level (VMPL), which
allows for services to be run in the guest at different privilege
levels. By running at VMPL0 (most privileged VM level), the SVSM can be
used to provide privileged services, e.g. a virtual TPM, for the guest
rather than trust such services from the hypervisor.
This patch adds a DTpm driver to communicate with a virtual TPM running
in the SVSM. The driver follows the vTPM protocol documented in the SVSM
specification.
SVSM vTPM functionality is available as new device and instance
libraries, which can be consumed optionally, keeping changes to the
regular TPM implementation minimal.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Co-authored-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
Signed-off-by: Oliver Steffen <osteffen@redhat.com>
A some of functions implemented in Tpm2Ptp.c are forward declared in a
couple of places. To clean this up, introduce a header that contains
these declarations in a central place and use it instead.
Signed-off-by: Oliver Steffen <osteffen@redhat.com>
Fix some spelling/grammar mistakes in the documentation comments.
Suggested-by: Dionna Glaze <dionnaglaze@google.com>
Signed-off-by: Oliver Steffen <osteffen@redhat.com>
REF:https://github.com/tianocore/edk2/issues/10529
The `RngPei` PEIM can be used if RNG should be provided over a
dynamic binary interface to other PEIMs on a platform.
Tested with:
- SecurityPkg CI
- Verify RNG linked with RngLib is executed as expected
- Verify random numbers are generated successfully with a valid
RngLib
Integration instructions:
Use the `RngPei` module if a platform needs to produce
`gEfiRngPpiGuid`.
The platform should usually link a different `RngLib` instance to
`RngPei` than other PEIMs that may use the RNG PPI produced since
`RngPei` is responsible for producing the PPI.
For example, a `RngLib` instance that uses the rdrand instruction
may be linked against `RngPei` and a `RngLib` instance that uses
the RNG PPI may be linked against other PEIMs.
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
ArmTrng.c: In function 'GenerateEntropy':
ArmTrng.c:40:15: error: 'Status' may be used uninitialized
in this function [-Werror=maybe-uninitialized]
EFI_STATUS Status;
^~~~~~
cc1: all warnings being treated as errors
Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
Currently "Delete Signature" Setup page lists enrolled signatures and each
signature is shown with signature GUID (prompt) and type (help). It is
possible for some signatures to be shown with same signature GUID and
type. In this case, it is difficult to identify the target signature to
delete. The update enhances help information to distinguish signatures.
Signed-off-by: Phil Noh <Phil.Noh@amd.com>
BaseTools has a limitation that modules in FVs that are force rebased
must have the same file and section alignment. This is intended for
XIP modules.
VS2019 and previous VS toolchains did not set 4k section alignment,
but VS2022 does, in order for memory protections to be applied to
images. This causes issues when building SEC and PEI modules on
VS2022 as the file alignment is 0x20 but the section alignment
is 0x1000, so BaseTools will fail to generate the FV. One option
is to set the file alignment to 0x1000 for all of these files, but
that is a large waste of space and is not feasible on some platforms
that have limited flash space. The other option is to selectively
set 0x20 as the section alignment for SEC and PEI modules, which is
the approach GCC ARM/AARCH64 took.
This is only an issue for building 64-bit PEI on x86 currently, as
other architectures are not supported by VS2022 in edk2 yet. For IA32,
the section alignment is set to 0x20 and so it matches the file
alignment, however x64 PEI uses the X64 DLINK flags which have 0x1000
set. For other architectures that don't have the PEI/DXE architecture
split, this is also an issue.
This commit is required to use VS2022 as the default CI in edk2, as
OvmfPkgX64.dsc will fail to build. Any platform with 64-bit PEI also
requires this.
This commit also updates CryptoPkg.dsc and SecurityPkg.dsc as they
are setting custom section alignments.
Continuous-integration-options: PatchCheck.ignore-multi-package
Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
PeiTpmMeasurementLib is updated to support both TCG measurement and
CC Measurement. gEfiPeiMasterBootModePpiGuid is removed from [Depex]
because it is not needed for the library.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
Signed-off-by: Ceping Sun <cepingx.sun@intel.com>
gEdkiiCcPpi is designed to support CC measurement in PEI phase.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
Signed-off-by: Ceping Sun <cepingx.sun@intel.com>
