Re: [PATCH V3] MdeModulePkg: Variable: Update PCR[7] measure for new TCG spec

["Yao, Jiewen" <jiewen.yao@intel.com>]

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: Zhang, Chao B
> Sent: Sunday, January 22, 2017 12:54 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>;
> Zhang, Chao B <chao.b.zhang@intel.com>
> Subject: [PATCH V3] MdeModulePkg: Variable: Update PCR[7] measure for new
> TCG spec
> 
> Measure DBT into PCR[7] when it is updated between initial measure and
> ExitBootService. Measure "SecureBoot" change after PK update.
> Spec version : TCG PC Client PFP 00.37.
> http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific
> _Platform_Profile_for_TPM_2p0_Systems_v21.pdf
> 
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Yao Jiewen <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
> ---
>  .../Universal/Variable/RuntimeDxe/Measurement.c    | 82
> +++++++++++++++++++++-
>  .../Universal/Variable/RuntimeDxe/VariableDxe.c    | 19 ++++-
>  .../Variable/RuntimeDxe/VariableRuntimeDxe.inf     | 10 ++-
>  .../Variable/RuntimeDxe/VariableSmmRuntimeDxe.c    | 19 ++++-
>  .../Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf  |  7 +-
>  5 files changed, 128 insertions(+), 9 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
> index 2f92fae..309521f 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Measure TrEE required variable.
> 
> -Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be found
> at
> @@ -36,8 +36,16 @@ VARIABLE_TYPE  mVariableType[] = {
>    {EFI_KEY_EXCHANGE_KEY_NAME,    &gEfiGlobalVariableGuid},
>    {EFI_IMAGE_SECURITY_DATABASE,  &gEfiImageSecurityDatabaseGuid},
>    {EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid},
> +  {EFI_IMAGE_SECURITY_DATABASE2, &gEfiImageSecurityDatabaseGuid},
>  };
> 
> +//
> +// "SecureBoot" may update following PK Del/Add
> +//  Cache its value to detect value update
> +//
> +UINT8       *mSecureBootVarData    = NULL;
> +UINTN       mSecureBootVarDataSize = 0;
> +
>  /**
>    This function will return if this variable is SecureBootPolicy Variable.
> 
> @@ -251,5 +259,77 @@ SecureBootHook (
>      FreePool (VariableData);
>    }
> 
> +  //
> +  // "SecureBoot" is 8bit & read-only. It can only be changed according to PK
> update
> +  //
> +  if ((StrCmp (VariableName, EFI_PLATFORM_KEY_NAME) == 0) &&
> +       CompareGuid (VendorGuid, &gEfiGlobalVariableGuid)) {
> +     Status = InternalGetVariable (
> +                EFI_SECURE_BOOT_MODE_NAME,
> +                &gEfiGlobalVariableGuid,
> +                &VariableData,
> +                &VariableDataSize
> +                );
> +     if (EFI_ERROR (Status)) {
> +       return;
> +     }
> +
> +     //
> +     // If PK update is successful. "SecureBoot" shall always exist ever since
> variable write service is ready
> +     //
> +     ASSERT(mSecureBootVarData != NULL);
> +
> +     if (CompareMem(mSecureBootVarData, VariableData,
> VariableDataSize) != 0) {
> +       FreePool(mSecureBootVarData);
> +       mSecureBootVarData     = VariableData;
> +       mSecureBootVarDataSize = VariableDataSize;
> +
> +       DEBUG((DEBUG_INFO, "%s variable updated according to PK change.
> Remeasure the value!\n", EFI_SECURE_BOOT_MODE_NAME));
> +       Status = MeasureVariable (
> +                  EFI_SECURE_BOOT_MODE_NAME,
> +                  &gEfiGlobalVariableGuid,
> +                  mSecureBootVarData,
> +                  mSecureBootVarDataSize
> +                  );
> +       DEBUG ((DEBUG_INFO, "MeasureBootPolicyVariable - %r\n", Status));
> +     } else {
> +       //
> +       // "SecureBoot" variable is not changed
> +       //
> +       FreePool(VariableData);
> +     }
> +  }
> +
>    return ;
>  }
> +
> +/**
> +  Some Secure Boot Policy Variable may update following other variable
> changes(SecureBoot follows PK change, etc).
> +  Record their initial State when variable write service is ready.
> +
> +**/
> +VOID
> +EFIAPI
> +RecordSecureBootPolicyVarData(
> +  VOID
> +  )
> +{
> +  EFI_STATUS Status;
> +
> +  //
> +  // Record initial "SecureBoot" variable value.
> +  // It is used to detect SecureBoot variable change in SecureBootHook.
> +  //
> +  Status = InternalGetVariable (
> +             EFI_SECURE_BOOT_MODE_NAME,
> +             &gEfiGlobalVariableGuid,
> +             (VOID **)&mSecureBootVarData,
> +             &mSecureBootVarDataSize
> +             );
> +  if (EFI_ERROR(Status)) {
> +    //
> +    // Read could fail when Auth Variable solution is not supported
> +    //
> +    DEBUG((DEBUG_INFO, "RecordSecureBootPolicyVarData GetVariable %s
> Status %x\n", EFI_SECURE_BOOT_MODE_NAME, Status));
> +  }
> +}
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> index 3d3cd24..fe1b2b5 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> @@ -3,7 +3,7 @@
>    and volatile storage space and install variable architecture protocol.
> 
>  Copyright (C) 2013, Red Hat, Inc.
> -Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>  (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
> @@ -32,6 +32,17 @@ EDKII_VAR_CHECK_PROTOCOL            mVarCheck
> = { VarCheckRegis
> 
> VarCheckVariablePropertyGet };
> 
>  /**
> +  Some Secure Boot Policy Variable may update following other variable
> changes(SecureBoot follows PK change, etc).
> +  Record their initial State when variable write service is ready.
> +
> +**/
> +VOID
> +EFIAPI
> +RecordSecureBootPolicyVarData(
> +  VOID
> +  );
> +
> +/**
>    Return TRUE if ExitBootServices () has been called.
> 
>    @retval TRUE If ExitBootServices () has been called.
> @@ -415,6 +426,12 @@ FtwNotificationEvent (
>    }
> 
>    //
> +  // Some Secure Boot Policy Var (SecureBoot, etc) updates following other
> +  // Secure Boot Policy Variable change. Record their initial value.
> +  //
> +  RecordSecureBootPolicyVarData();
> +
> +  //
>    // Install the Variable Write Architectural protocol.
>    //
>    Status = gBS->InstallProtocolInterface (
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> index 6214966..bc24a25 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> @@ -9,7 +9,7 @@
>  #  This external input must be validated carefully to avoid security issues such
> as
>  #  buffer overflow or integer overflow.
>  #
> -# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>  # This program and the accompanying materials
>  # are licensed and made available under the terms and conditions of the BSD
> License
>  # which accompanies this distribution. The full text of the license may be found
> at
> @@ -94,6 +94,9 @@
>    ## SOMETIMES_PRODUCES   ## Variable:L"PlatformLang"
>    ## SOMETIMES_CONSUMES   ## Variable:L"Lang"
>    ## SOMETIMES_PRODUCES   ## Variable:L"Lang"
> +  ## SOMETIMES_CONSUMES   ## Variable:L"PK"
> +  ## SOMETIMES_CONSUMES   ## Variable:L"KEK"
> +  ## SOMETIMES_CONSUMES   ## Variable:L"SecureBoot"
>    gEfiGlobalVariableGuid
> 
>    gEfiMemoryOverwriteControlDataGuid            ##
> SOMETIMES_CONSUMES   ## Variable:L"MemoryOverwriteRequestControl"
> @@ -108,8 +111,9 @@
>    ## SOMETIMES_PRODUCES   ## Variable:L"VarErrorFlag"
>    gEdkiiVarErrorFlagGuid
> 
> -  ## SOMETIMES_CONSUMES   ## Variable:L"DB"
> -  ## SOMETIMES_CONSUMES   ## Variable:L"DBX"
> +  ## SOMETIMES_CONSUMES   ## Variable:L"db"
> +  ## SOMETIMES_CONSUMES   ## Variable:L"dbx"
> +  ## SOMETIMES_CONSUMES   ## Variable:L"dbt"
>    gEfiImageSecurityDatabaseGuid
> 
>  [Pcd]
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
> index 0a076ae..e209d54 100644
> ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
> @@ -13,7 +13,7 @@
> 
>    InitCommunicateBuffer() is really function to check the variable data size.
> 
> -Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be found
> at
> @@ -71,6 +71,17 @@ SecureBootHook (
>    );
> 
>  /**
> +  Some Secure Boot Policy Variable may update following other variable
> changes(SecureBoot follows PK change, etc).
> +  Record their initial State when variable write service is ready.
> +
> +**/
> +VOID
> +EFIAPI
> +RecordSecureBootPolicyVarData(
> +  VOID
> +  );
> +
> +/**
>    Acquires lock only at boot time. Simply returns at runtime.
> 
>    This is a temperary function that will be removed when
> @@ -1079,6 +1090,12 @@ SmmVariableWriteReady (
>      return;
>    }
> 
> +  //
> +  // Some Secure Boot Policy Var (SecureBoot, etc) updates following other
> +  // Secure Boot Policy Variable change.  Record their initial value.
> +  //
> +  RecordSecureBootPolicyVarData();
> +
>    Status = gBS->InstallProtocolInterface (
>                    &mHandle,
>                    &gEfiVariableWriteArchProtocolGuid,
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.in
> f
> index 82ddb00..9975f5a 100644
> ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.in
> f
> @@ -13,7 +13,7 @@
>  #  may not be modified without authorization. If platform fails to protect these
> resources,
>  #  the authentication service provided in this driver will be broken, and the
> behavior is undefined.
>  #
> -# Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
>  # This program and the accompanying materials
>  # are licensed and made available under the terms and conditions of the BSD
> License
>  # which accompanies this distribution. The full text of the license may be found
> at
> @@ -81,8 +81,9 @@
>    ## SOMETIMES_CONSUMES   ## Variable:L"SecureBoot"
>    gEfiGlobalVariableGuid
> 
> -  ## SOMETIMES_CONSUMES   ## Variable:L"DB"
> -  ## SOMETIMES_CONSUMES   ## Variable:L"DBX"
> +  ## SOMETIMES_CONSUMES   ## Variable:L"db"
> +  ## SOMETIMES_CONSUMES   ## Variable:L"dbx"
> +  ## SOMETIMES_CONSUMES   ## Variable:L"dbt"
>    gEfiImageSecurityDatabaseGuid
> 
>  [Depex]
> --
> 1.9.5.msysgit.1