On 8/12/21 9:48 AM, Marc-André Lureau wrote: > Hi On Tue, Aug 10, 2021 at 9:22 PM Stefan Berger > wrote: Import > PeiDxeTpmPlatformHierarchyLib.c from edk2-platforms. Modify it so that > ConfigureTpmPlatformHierarchy() is the only public function provided ‍ > ‍ ZjQcmQRYFpfptBannerStart > This Message Is From an External Sender > This message came from outside your organization. > ZjQcmQRYFpfptBannerEnd > Hi > > On Tue, Aug 10, 2021 at 9:22 PM Stefan Berger > > wrote: > > Import PeiDxeTpmPlatformHierarchyLib.c from edk2-platforms. Modify > it so > that ConfigureTpmPlatformHierarchy() is the only public function > provided > by this file. > > Signed-off-by: Stefan Berger > > --- >  .../Include/Library/TpmPlatformHierarchyLib.h |  27 +++ >  .../PeiDxeTpmPlatformHierarchyLib.c           | 210 > ++++++++++++++++++ >  .../PeiDxeTpmPlatformHierarchyLib.inf         |  40 ++++ >  3 files changed, 277 insertions(+) >  create mode 100644 OvmfPkg/Include/Library/TpmPlatformHierarchyLib.h >  create mode 100644 > OvmfPkg/Library/PeiDxeTpmPlatformHierarchyLib/PeiDxeTpmPlatformHierarchyLib.c >  create mode 100644 > OvmfPkg/Library/PeiDxeTpmPlatformHierarchyLib/PeiDxeTpmPlatformHierarchyLib.inf > > diff --git a/OvmfPkg/Include/Library/TpmPlatformHierarchyLib.h > b/OvmfPkg/Include/Library/TpmPlatformHierarchyLib.h > new file mode 100644 > index 0000000000..a872fa09dc > --- /dev/null > +++ b/OvmfPkg/Include/Library/TpmPlatformHierarchyLib.h > @@ -0,0 +1,27 @@ > +/** @file > +    TPM Platform Hierarchy configuration library. > + > +    This library provides functions for customizing the TPM's > Platform Hierarchy > +    Authorization Value (platformAuth) and Platform Hierarchy > Authorization > +    Policy (platformPolicy) can be defined through this function. > + > +Copyright (c) 2019, Intel Corporation. All rights reserved.
> +Copyright (c) Microsoft Corporation.
> +SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#ifndef _TPM_PLATFORM_HIERARCHY_LIB_H_ > +#define _TPM_PLATFORM_HIERARCHY_LIB_H_ > + > +/** > +   This service will perform the TPM Platform Hierarchy > configuration at the SmmReadyToLock event. > + > +**/ > +VOID > +EFIAPI > +ConfigureTpmPlatformHierarchy ( > +  VOID > +  ); > + > +#endif > diff --git > a/OvmfPkg/Library/PeiDxeTpmPlatformHierarchyLib/PeiDxeTpmPlatformHierarchyLib.c > b/OvmfPkg/Library/PeiDxeTpmPlatformHierarchyLib/PeiDxeTpmPlatformHierarchyLib.c > new file mode 100644 > index 0000000000..ba2d99bb53 > --- /dev/null > +++ > b/OvmfPkg/Library/PeiDxeTpmPlatformHierarchyLib/PeiDxeTpmPlatformHierarchyLib.c > @@ -0,0 +1,210 @@ > +/** @file > +    TPM Platform Hierarchy configuration library. > + > +    This library provides functions for customizing the TPM's > Platform Hierarchy > +    Authorization Value (platformAuth) and Platform Hierarchy > Authorization > +    Policy (platformPolicy) can be defined through this function. > + > +    Copyright (c) 2019, Intel Corporation. All rights reserved.
> +    Copyright (c) Microsoft Corporation.
> +    SPDX-License-Identifier: BSD-2-Clause-Patent > + > +    @par Specification Reference: > + > https://trustedcomputinggroup.org/resource/tcg-tpm-v2-0-provisioning-guidance/ > > +**/ > + > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +// > +// The authorization value may be no larger than the digest > produced by the hash > +//   algorithm used for context integrity. > +// > +#define      MAX_NEW_AUTHORIZATION_SIZE SHA512_DIGEST_SIZE > + > +UINT16       mAuthSize; > + > +/** > +  Generate high-quality entropy source through RDRAND. > + > +  @param[in]   Length        Size of the buffer, in bytes, to > fill with. > +  @param[out]  Entropy       Pointer to the buffer to store the > entropy data. > + > +  @retval EFI_SUCCESS        Entropy generation succeeded. > +  @retval EFI_NOT_READY      Failed to request random data. > + > +**/ > +EFI_STATUS > +EFIAPI > +RdRandGenerateEntropy ( > +  IN UINTN         Length, > +  OUT UINT8        *Entropy > +  ) > +{ > +  EFI_STATUS  Status; > +  UINTN       BlockCount; > +  UINT64      Seed[2]; > +  UINT8       *Ptr; > + > +  Status = EFI_NOT_READY; > +  BlockCount = Length / 64; > +  Ptr = (UINT8 *)Entropy; > + > +  // > +  // Generate high-quality seed for DRBG Entropy > +  // > +  while (BlockCount > 0) { > +    Status = GetRandomNumber128 (Seed); > +    if (EFI_ERROR (Status)) { > +      return Status; > +    } > +    CopyMem (Ptr, Seed, 64); > > > This looks like it's copying past the Seed buffer, which is 2 * > sizeof(u64) = 16. Ha! Thanks for looking at this. Those seem to be the pitfalls of blindly importing code from edk2-platforms. Now the question is whether to leave it broken in edk2-platforms or fix it there first before trying to import it to edk2. In the interest of time I'd rather fix it here. Obviously the BlockCount is also wrong. > > + > +    BlockCount--; > +    Ptr = Ptr + 64; > +  } > + > +  // > +  // Populate the remained data as request. > +  // > +  Status = GetRandomNumber128 (Seed); > +  if (EFI_ERROR (Status)) { > +    return Status; > +  } > +  CopyMem (Ptr, Seed, (Length % 64)); > > > And then again. > > Isn't there a better way to fill a buffer with random data in edk2? I don't know. On ARM it *looks like* the path goes down to an assembly instruction getting 64bit random number from the hardware: MdePkg/Library/BaseRngLib/AArch64/ArmRng.asm On x86 it will end up calling GenerateRandomNumberViaNist800Algorithm: MdePkg/Library/DxeRngLib/DxeRngLib.c CryptoPkg/Drvier/Crypto.c has this here: BOOLEAN EFIAPI CryptoServiceRandomSeed (   IN  CONST  UINT8  *Seed  OPTIONAL,   IN  UINTN         SeedSize   ) {   return CALL_BASECRYPTLIB (Random.Services.Seed, RandomSeed, (Seed, SeedSize) } and this one: BOOLEAN EFIAPI CryptoServiceRandomBytes (   OUT  UINT8  *Output,   IN   UINTN  Size   ) {   return CALL_BASECRYPTLIB (Random.Services.Bytes, RandomBytes, (Output, Size) } Those are pseudorandom numbers. I don't know about others. > > + > +  return Status; > +} > + > +/** > +  This function returns the maximum size of TPM2B_AUTH; this > structure is used for an authorization value > +  and limits an authValue to being no larger than the largest > digest produced by a TPM. > + > +  @param[out] AuthSize                 Tpm2 Auth size > + > +  @retval EFI_SUCCESS                  Auth size returned. > +  @retval EFI_DEVICE_ERROR             Can not return platform > auth due to device error. > + > +**/ > +EFI_STATUS > +EFIAPI > +GetAuthSize ( > +  OUT UINT16            *AuthSize > +  ) > +{ > +  EFI_STATUS            Status; > +  TPML_PCR_SELECTION    Pcrs; > +  UINTN                 Index; > +  UINT16                DigestSize; > + > +  Status = EFI_SUCCESS; > + > +  while (mAuthSize == 0) { > > > This is a bit odd, but ok. > > + > +    mAuthSize = SHA1_DIGEST_SIZE; > +    ZeroMem (&Pcrs, sizeof (TPML_PCR_SELECTION)); > +    Status = Tpm2GetCapabilityPcrs (&Pcrs); > + > +    if (EFI_ERROR (Status)) { > +      DEBUG ((DEBUG_ERROR, "Tpm2GetCapabilityPcrs fail!\n")); > +      break; > +    } > + > +    DEBUG ((DEBUG_ERROR, "Tpm2GetCapabilityPcrs - %08x\n", > Pcrs.count)); > + > +    for (Index = 0; Index < Pcrs.count; Index++) { > +      DEBUG ((DEBUG_ERROR, "alg - %x\n", > Pcrs.pcrSelections[Index].hash)); > + > +      switch (Pcrs.pcrSelections[Index].hash) { > +      case TPM_ALG_SHA1: > +        DigestSize = SHA1_DIGEST_SIZE; > +        break; > +      case TPM_ALG_SHA256: > +        DigestSize = SHA256_DIGEST_SIZE; > +        break; > +      case TPM_ALG_SHA384: > +        DigestSize = SHA384_DIGEST_SIZE; > +        break; > +      case TPM_ALG_SHA512: > +        DigestSize = SHA512_DIGEST_SIZE; > +        break; > +      case TPM_ALG_SM3_256: > +        DigestSize = SM3_256_DIGEST_SIZE; > +        break; > +      default: > +        DigestSize = SHA1_DIGEST_SIZE; > +        break; > +      } > + > +      if (DigestSize > mAuthSize) { > +        mAuthSize = DigestSize; > +      } > +    } > +    break; > +  } > + > +  *AuthSize = mAuthSize; > +  return Status; > +} > + > +/** > +  Set PlatformAuth to random value. > +**/ > +VOID > +RandomizePlatformAuth ( > +  VOID > +  ) > +{ > +  EFI_STATUS                        Status; > +  UINT16                            AuthSize; > +  UINT8                             *Rand; > +  UINTN                             RandSize; > +  TPM2B_AUTH                        NewPlatformAuth; > + > +  // > +  // Send Tpm2HierarchyChange Auth with random value to avoid > PlatformAuth being null > +  // > + > +  GetAuthSize (&AuthSize); > + > +  ZeroMem (NewPlatformAuth.buffer, AuthSize); > +  NewPlatformAuth.size = AuthSize; > + > +  // > +  // Allocate one buffer to store random data. > +  // > +  RandSize = MAX_NEW_AUTHORIZATION_SIZE; > +  Rand = AllocatePool (RandSize); > + > +  RdRandGenerateEntropy (RandSize, Rand); > +  CopyMem (NewPlatformAuth.buffer, Rand, AuthSize); > > > Why generate random data for MAX_NEW_AUTHORIZATION to only copy a > subset after? > > + > +  FreePool (Rand); > + > +  // > +  // Send Tpm2HierarchyChangeAuth command with the new Auth value > +  // > +  Status = Tpm2HierarchyChangeAuth (TPM_RH_PLATFORM, NULL, > &NewPlatformAuth); > +  DEBUG ((DEBUG_INFO, "Tpm2HierarchyChangeAuth Result: - %r\n", > Status)); > +  ZeroMem (NewPlatformAuth.buffer, AuthSize); > +  ZeroMem (Rand, RandSize); > > > Isn't Rand free at this point? Indeed! > > +} > + > +/** > +   This service defines the configuration of the Platform > Hierarchy Authorization Value (platformAuth) > +   and Platform Hierarchy Authorization Policy (platformPolicy) > + > +**/ > +VOID > +EFIAPI > +ConfigureTpmPlatformHierarchy ( > +  ) > +{ > +  RandomizePlatformAuth (); > +} > diff --git > a/OvmfPkg/Library/PeiDxeTpmPlatformHierarchyLib/PeiDxeTpmPlatformHierarchyLib.inf > b/OvmfPkg/Library/PeiDxeTpmPlatformHierarchyLib/PeiDxeTpmPlatformHierarchyLib.inf > new file mode 100644 > index 0000000000..a413e02302 > --- /dev/null > +++ > b/OvmfPkg/Library/PeiDxeTpmPlatformHierarchyLib/PeiDxeTpmPlatformHierarchyLib.inf > @@ -0,0 +1,40 @@ > +### @file > +# > +#   TPM Platform Hierarchy configuration library. > +# > +#   This library provides functions for customizing the TPM's > Platform Hierarchy > +#   Authorization Value (platformAuth) and Platform Hierarchy > Authorization > +#   Policy (platformPolicy) can be defined through this function. > +# > +# Copyright (c) 2019, Intel Corporation. All rights reserved.
> +# Copyright (c) Microsoft Corporation.
> +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +### > + > +[Defines] > +  INF_VERSION                    = 0x00010005 > +  BASE_NAME                      = PeiDxeTpmPlatformHierarchyLib > +  FILE_GUID                      = > 7794F92C-4E8E-4E57-9E4A-49A0764C7D73 > +  MODULE_TYPE                    = PEIM > +  VERSION_STRING                 = 1.0 > +  LIBRARY_CLASS                  = TpmPlatformHierarchyLib|PEIM > DXE_DRIVER > + > +[LibraryClasses] > +  BaseLib > +  BaseMemoryLib > +  DebugLib > +  MemoryAllocationLib > +  RngLib > +  Tpm2CommandLib > +  Tpm2DeviceLib > + > +[Packages] > +  MdePkg/MdePkg.dec > +  MdeModulePkg/MdeModulePkg.dec > +  SecurityPkg/SecurityPkg.dec > +  CryptoPkg/CryptoPkg.dec > + > +[Sources] > +  PeiDxeTpmPlatformHierarchyLib.c > -- > 2.31.1 > >