[edk2,edk2-platforms,v1,08/12] Hisilicon/D06: Fix SBBR-SCT AuthVar issue

Message ID 20181029033249.45363-9-ming.huang@linaro.org
State New
Headers show
Series
  • [edk2,edk2-platforms,v1,01/12] Silicon/Hisilicon/D06: Add watchdog to GTDT
Related show

Commit Message

Ming Huang Oct. 29, 2018, 3:32 a.m.
Enable secure boot to fix AuthVar issue:
RT.SetVariable - Set Invalid Time Base Auth Variable – FAILURE;
RT.SetVariable - Create one Time Base Auth Variable, the expect return
status should be EFI_SUCCESS – FAILURE.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 Silicon/Hisilicon/Hisilicon.dsc.inc | 16 ++++++++++++++++
 Platform/Hisilicon/D06/D06.dsc      |  2 +-
 2 files changed, 17 insertions(+), 1 deletion(-)

Comments

Leif Lindholm Nov. 14, 2018, 12:18 a.m. | #1
On Mon, Oct 29, 2018 at 11:32:45AM +0800, Ming Huang wrote:
> Enable secure boot to fix AuthVar issue:
> RT.SetVariable - Set Invalid Time Base Auth Variable – FAILURE;
> RT.SetVariable - Create one Time Base Auth Variable, the expect return
> status should be EFI_SUCCESS – FAILURE.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> ---
>  Silicon/Hisilicon/Hisilicon.dsc.inc | 16 ++++++++++++++++
>  Platform/Hisilicon/D06/D06.dsc      |  2 +-
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/Silicon/Hisilicon/Hisilicon.dsc.inc b/Silicon/Hisilicon/Hisilicon.dsc.inc
> index 3ac8e20232..6515c0d703 100644
> --- a/Silicon/Hisilicon/Hisilicon.dsc.inc
> +++ b/Silicon/Hisilicon/Hisilicon.dsc.inc
> @@ -89,8 +89,15 @@
>  
>    SemihostLib|ArmPkg/Library/SemihostLib/SemihostLib.inf
>  
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> +  TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> +  AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf

> +  # re-use the UserPhysicalPresent() dummy implementation from the ovmf tree
> +  PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf

The virtual machines and development boards can get away with this,
but it is not an appropriate action for a real platform.
Please implement a real PlatformSecureLib, doing a real
UserPhysicalPresent check, appropriate to the D06.

I don't expect this to happen in time for a 2018.11 Linaro release, so
you can drop it from the set. We can log the test failure as a known
issue for now.

/
    Leif

> +!else
>    TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
>    AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
> +!endif
>  
>    # BDS Libraries
>    FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
> @@ -217,6 +224,9 @@
>  !if $(TARGET) != RELEASE
>    DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
>  !endif
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> +  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> +!endif
>  
>  [LibraryClasses.AARCH64]
>    ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
> @@ -326,6 +336,12 @@
>    gEmbeddedTokenSpaceGuid.PcdTimerPeriod|10000
>    gArmTokenSpaceGuid.PcdVFPEnabled|1
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|32
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> +  # override the default values from SecurityPkg to ensure images from all sources are verified in secure boot
> +  gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04
> +  gEfiSecurityPkgTokenSpaceGuid.PcdFixedMediaImageVerificationPolicy|0x04
> +  gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
> +!endif
>  
>  [PcdsDynamicHii.common.DEFAULT]
>    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|10 # Variable: L"Timeout"
> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
> index b6ef9fedf0..8ee20342b1 100644
> --- a/Platform/Hisilicon/D06/D06.dsc
> +++ b/Platform/Hisilicon/D06/D06.dsc
> @@ -30,7 +30,7 @@
>    FLASH_DEFINITION               = Platform/Hisilicon/$(PLATFORM_NAME)/$(PLATFORM_NAME).fdf
>    DEFINE NETWORK_IP6_ENABLE      = FALSE
>    DEFINE HTTP_BOOT_ENABLE        = FALSE
> -  DEFINE SECURE_BOOT_ENABLE      = FALSE
> +  DEFINE SECURE_BOOT_ENABLE      = TRUE
>  
>  !include Silicon/Hisilicon/Hisilicon.dsc.inc
>  
> -- 
> 2.18.0
>
Ming Huang Nov. 14, 2018, 2:31 p.m. | #2
On 11/14/2018 8:18 AM, Leif Lindholm wrote:
> On Mon, Oct 29, 2018 at 11:32:45AM +0800, Ming Huang wrote:
>> Enable secure boot to fix AuthVar issue:
>> RT.SetVariable - Set Invalid Time Base Auth Variable – FAILURE;
>> RT.SetVariable - Create one Time Base Auth Variable, the expect return
>> status should be EFI_SUCCESS – FAILURE.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang <ming.huang@linaro.org>
>> ---
>>  Silicon/Hisilicon/Hisilicon.dsc.inc | 16 ++++++++++++++++
>>  Platform/Hisilicon/D06/D06.dsc      |  2 +-
>>  2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/Silicon/Hisilicon/Hisilicon.dsc.inc b/Silicon/Hisilicon/Hisilicon.dsc.inc
>> index 3ac8e20232..6515c0d703 100644
>> --- a/Silicon/Hisilicon/Hisilicon.dsc.inc
>> +++ b/Silicon/Hisilicon/Hisilicon.dsc.inc
>> @@ -89,8 +89,15 @@
>>  
>>    SemihostLib|ArmPkg/Library/SemihostLib/SemihostLib.inf
>>  
>> +!if $(SECURE_BOOT_ENABLE) == TRUE
>> +  TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
>> +  AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> 
>> +  # re-use the UserPhysicalPresent() dummy implementation from the ovmf tree
>> +  PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> 
> The virtual machines and development boards can get away with this,
> but it is not an appropriate action for a real platform.
> Please implement a real PlatformSecureLib, doing a real
> UserPhysicalPresent check, appropriate to the D06.
> 
> I don't expect this to happen in time for a 2018.11 Linaro release, so
> you can drop it from the set. We can log the test failure as a known
> issue for now.

Ok, I will drop this patch in v2.

> 
> /
>     Leif
> 
>> +!else
>>    TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
>>    AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
>> +!endif
>>  
>>    # BDS Libraries
>>    FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
>> @@ -217,6 +224,9 @@
>>  !if $(TARGET) != RELEASE
>>    DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
>>  !endif
>> +!if $(SECURE_BOOT_ENABLE) == TRUE
>> +  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>> +!endif
>>  
>>  [LibraryClasses.AARCH64]
>>    ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
>> @@ -326,6 +336,12 @@
>>    gEmbeddedTokenSpaceGuid.PcdTimerPeriod|10000
>>    gArmTokenSpaceGuid.PcdVFPEnabled|1
>>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|32
>> +!if $(SECURE_BOOT_ENABLE) == TRUE
>> +  # override the default values from SecurityPkg to ensure images from all sources are verified in secure boot
>> +  gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04
>> +  gEfiSecurityPkgTokenSpaceGuid.PcdFixedMediaImageVerificationPolicy|0x04
>> +  gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
>> +!endif
>>  
>>  [PcdsDynamicHii.common.DEFAULT]
>>    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|10 # Variable: L"Timeout"
>> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
>> index b6ef9fedf0..8ee20342b1 100644
>> --- a/Platform/Hisilicon/D06/D06.dsc
>> +++ b/Platform/Hisilicon/D06/D06.dsc
>> @@ -30,7 +30,7 @@
>>    FLASH_DEFINITION               = Platform/Hisilicon/$(PLATFORM_NAME)/$(PLATFORM_NAME).fdf
>>    DEFINE NETWORK_IP6_ENABLE      = FALSE
>>    DEFINE HTTP_BOOT_ENABLE        = FALSE
>> -  DEFINE SECURE_BOOT_ENABLE      = FALSE
>> +  DEFINE SECURE_BOOT_ENABLE      = TRUE
>>  
>>  !include Silicon/Hisilicon/Hisilicon.dsc.inc
>>  
>> -- 
>> 2.18.0
>>

Patch

diff --git a/Silicon/Hisilicon/Hisilicon.dsc.inc b/Silicon/Hisilicon/Hisilicon.dsc.inc
index 3ac8e20232..6515c0d703 100644
--- a/Silicon/Hisilicon/Hisilicon.dsc.inc
+++ b/Silicon/Hisilicon/Hisilicon.dsc.inc
@@ -89,8 +89,15 @@ 
 
   SemihostLib|ArmPkg/Library/SemihostLib/SemihostLib.inf
 
+!if $(SECURE_BOOT_ENABLE) == TRUE
+  TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
+  AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
+  # re-use the UserPhysicalPresent() dummy implementation from the ovmf tree
+  PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
+!else
   TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
   AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
+!endif
 
   # BDS Libraries
   FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
@@ -217,6 +224,9 @@ 
 !if $(TARGET) != RELEASE
   DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
 !endif
+!if $(SECURE_BOOT_ENABLE) == TRUE
+  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
+!endif
 
 [LibraryClasses.AARCH64]
   ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
@@ -326,6 +336,12 @@ 
   gEmbeddedTokenSpaceGuid.PcdTimerPeriod|10000
   gArmTokenSpaceGuid.PcdVFPEnabled|1
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|32
+!if $(SECURE_BOOT_ENABLE) == TRUE
+  # override the default values from SecurityPkg to ensure images from all sources are verified in secure boot
+  gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04
+  gEfiSecurityPkgTokenSpaceGuid.PcdFixedMediaImageVerificationPolicy|0x04
+  gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
+!endif
 
 [PcdsDynamicHii.common.DEFAULT]
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|10 # Variable: L"Timeout"
diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
index b6ef9fedf0..8ee20342b1 100644
--- a/Platform/Hisilicon/D06/D06.dsc
+++ b/Platform/Hisilicon/D06/D06.dsc
@@ -30,7 +30,7 @@ 
   FLASH_DEFINITION               = Platform/Hisilicon/$(PLATFORM_NAME)/$(PLATFORM_NAME).fdf
   DEFINE NETWORK_IP6_ENABLE      = FALSE
   DEFINE HTTP_BOOT_ENABLE        = FALSE
-  DEFINE SECURE_BOOT_ENABLE      = FALSE
+  DEFINE SECURE_BOOT_ENABLE      = TRUE
 
 !include Silicon/Hisilicon/Hisilicon.dsc.inc