Re: [edk2-devel] [PATCH v2 3/4] OvmfPkg: Enable physical presence interface for TPM 1.2

["Stefan Berger" <stefanb@linux.ibm.com>]

On 11/8/21 09:43, Stefan Berger wrote:
> On 11/8/21 07:13, Yao, Jiewen wrote:
>
>> The PPFlag variable MUST to be locked to prevent malicious modification.
>> Otherwise, anyone can change the PP configuration without 
>> confirmation from end user.
>
> That change by an attacker could presumably only  be done via UEFI 
> shell/command line? How do I display the variables? I tried with 
> 'dmpstore PhysicalPresenceFlags' (TPM 1.2) or 'dumpstore 
> Tcg2PhysicalPresenceFlags' but I don't see them. I don't see them with 
> 'dmpstore -b', either, but I see them both on Linux in 
> /sys/firmware/efi/efivars.
>
> Under Linux I can remove the (Tcg2)PhysicalPresenceFlags after 'chattr 
> -i' on the file and then an 'rm'. Is it a concern for this particular 
> variable if root can do this?
>
> Since this is old/outdated, what is a new API for it?
>
>    Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, 
> NULL, (VOID **)&VariableLockProtocol);
>    if (!EFI_ERROR (Status)) {
>      Status = VariableLockProtocol->RequestToLock (
>
> Thanks.
>
It seems that the following makes this a read-only variable for root on 
Linux as well. It doesn't make it appear with 'dmpstrore -b'. Is this 
now the correct solution?


diff --git a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h 
b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
index 2bec637f75..fc2abdb96c 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
+++ b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
@@ -30,6 +30,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent

  #include <Guid/AuthenticatedVariableFormat.h>
  #include <Guid/ImageAuthentication.h>
+#include <Guid/PhysicalPresenceData.h>

  #define TWO_BYTE_ENCODE       0x82

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c 
b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
index 122b3b0bf4..888977efa7 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
@@ -89,6 +89,17 @@ VARIABLE_ENTRY_PROPERTY mAuthVarEntry[] = {
        MAX_UINTN
      }
    },
+  {
+    &gEfiPhysicalPresenceGuid,
+    PHYSICAL_PRESENCE_FLAGS_VARIABLE,
+    {
+      VAR_CHECK_VARIABLE_PROPERTY_REVISION,
+      VAR_CHECK_VARIABLE_PROPERTY_READ_ONLY,
+      VARIABLE_ATTRIBUTE_NV_BS,
+      sizeof (EFI_PHYSICAL_PRESENCE_FLAGS),
+      MAX_UINTN
+    }
+  }
  };

  VOID **mAuthVarAddressPointer[9];
diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf 
b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
index 8eadeebceb..d0ced0792c 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
+++ b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
@@ -75,6 +75,10 @@
    ## PRODUCES            ## Variable:L"certdbv"
    gEfiCertDbGuid

+  ## CONSUMES            ## Variable:L"PhysicalPresenceFlags"
+  ## PRODUCES            ## Variable:L"PhysicalPresenceFlags"
+  gEfiPhysicalPresenceGuid
+
    ## CONSUMES            ## Variable:L"VendorKeysNv"
    ## PRODUCES            ## Variable:L"VendorKeysNv"
    gEfiVendorKeysNvGuid


    Stefan