Re: [edk2] [PATCH v4 0/4] Introduce CapsuleApp patch v4

2019-01-30 Thread Chen, Chen A
Hi Jian.

The interface CharToUpper() is used to convert just one character, But the 
UpperCaseString is used to convert a string.

I think it's no need to call CharToUpper repeatedly in UpperCaseString function 
to convert a string.

Regards,
Chen

-Original Message-
From: Wang, Jian J 
Sent: Thursday, January 31, 2019 10:48 AM
To: Chen, Chen A ; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v4 0/4] Introduce CapsuleApp patch v4

Chen,

I just noticed that there's another patch series in which a new interface 
CharToUpper() is introduced in BaseLib which can be used to replace your local 
equivalent one UpperCaseString(). I would suggest to hold your patch for a 
while and use the new interface instead once that patch series is pushed. Sorry 
for the late remind.

Regards,
Jian

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Chen A Chen
> Sent: Thursday, January 31, 2019 10:34 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH v4 0/4] Introduce CapsuleApp patch v4
> 
> No functionality change.
> Fix ECC and code style issue.
> Already pass CR process.
> https://git-amr-7.devtools.intel.com/gerrit/#/c/39023/
> 
> Chen A Chen (4):
>   MdePkg/UefiSpec.h: Add definition to support Capsule-on-Disk feature
>   MdeModulePkg/CapsuleApp: Add a function used to get next DevicePath
>   MdeModulePkg/CapsuleApp: Add functions to support Capsule-on-Disk
>   MdeModulePkg/CapsuleApp: Enhance CapsuleApp to support 
> Capsule-on-Disk
> 
>  MdeModulePkg/Application/CapsuleApp/CapsuleApp.c   | 160 +++-
>  MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf |   8 +
>  MdeModulePkg/Application/CapsuleApp/CapsuleDump.c  | 538
> +-
>  .../Application/CapsuleApp/CapsuleOnDisk.c | 808
> +
>  MdeModulePkg/Include/Library/UefiBootManagerLib.h  |  21 +-
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c   |  24 +-
>  MdeModulePkg/MdeModulePkg.dsc  |   1 +
>  MdePkg/Include/Uefi/UefiSpec.h |   5 +
>  8 files changed, 1546 insertions(+), 19 deletions(-)  create mode 
> 100644 MdeModulePkg/Application/CapsuleApp/CapsuleOnDisk.c
> 
> --
> 2.16.2.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 2/2] MdePkg/UefiGpt.h: Add new definition for enable GPT support

2019-01-24 Thread Chen, Chen A



-Original Message-
From: Wu, Hao A 
Sent: Friday, January 25, 2019 3:27 PM
To: Chen, Chen A ; Gao, Liming ; 
edk2-devel@lists.01.org
Cc: Kinney, Michael D ; Zhang, Chao B 

Subject: RE: [edk2] [PATCH 2/2] MdePkg/UefiGpt.h: Add new definition for enable 
GPT support

> -Original Message-
> From: Chen, Chen A
> Sent: Friday, January 25, 2019 3:16 PM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Kinney, Michael D; Zhang, Chao B; Gao, Liming
> Subject: RE: [edk2] [PATCH 2/2] MdePkg/UefiGpt.h: Add new definition 
> for enable GPT support
> 
> 
> 
> -Original Message-
> From: Wu, Hao A
> Sent: Friday, January 25, 2019 11:04 AM
> To: Chen, Chen A ; edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Zhang, Chao B 
> ; Gao, Liming 
> Subject: RE: [edk2] [PATCH 2/2] MdePkg/UefiGpt.h: Add new definition 
> for enable GPT support
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> > Of Chen A Chen
> > Sent: Thursday, January 17, 2019 10:03 AM
> > To: edk2-devel@lists.01.org
> > Cc: Kinney, Michael D; Zhang, Chao B; Gao, Liming
> > Subject: [edk2] [PATCH 2/2] MdePkg/UefiGpt.h: Add new definition for 
> > enable GPT support
> >
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1470
> > This two new definitions are defined for GPT in FatPei diver.
> >
> > Cc: Liming Gao 
> > Cc: Michael D Kinney 
> > Cc: Zhang Chao B 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Chen A Chen 
> > ---
> >  MdePkg/Include/Uefi/UefiGpt.h | 16 
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/MdePkg/Include/Uefi/UefiGpt.h 
> > b/MdePkg/Include/Uefi/UefiGpt.h index f635b05390..8665c8cbc9 100644
> > --- a/MdePkg/Include/Uefi/UefiGpt.h
> > +++ b/MdePkg/Include/Uefi/UefiGpt.h
> > @@ -24,9 +24,25 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND,
> > EITHER EXPRESS OR IMPLIED.
> >  /// EFI Partition Table Signature: "EFI PART".
> >  ///
> >  #define EFI_PTAB_HEADER_ID  SIGNATURE_64 ('E','F','I',' 
> > ','P','A','R','T')
> > +///
> > +/// Minimum bytes reserve for EFI entry array buffer.
> > +///
> > +#define GPT_PART_ENTRY_MIN_SIZE 16384
> 
> May I know where this definition comes from?
> Does it come from the UEFI spec?
> 
> Chen: The MACRO is not explicitly defined in UEFI Spec, But UEFI Spec 
> specifies a minimum value for GPT entry araray.

Does it comes from the below content within the UEFI spec?

> A minimum of 16,384 bytes of space must be reserved for the GPT 
> Partition Entry Array.

If so, I am not sure whether 'EFI_' prefix should be added before 
'GPT_PART_ENTRY_MIN_SIZE'.

Liming, could you help to confirm?

Chen: I have no strong opinion on ' GPT_PART_ENTRY_MIN_SIZE' and 
'EFI_GPT_PART_ENTRY_MIN_SIZE'. 

> 
> >
> >  #pragma pack(1)
> >
> > +///
> > +/// MBR Partition Entry
> > +///
> > +typedef struct {
> > +  UINT8   BootIndicator;
> > +  UINT8   StartingCHS[3];
> > +  UINT8   OSType;
> > +  UINT8   EndingCHS[3];
> > +  UINT32  StartingLBA;
> > +  UINT32  SizeInLBA;
> > +} MBR_PARTITION_ENTRY;
> > +
> 
> What about using the 'MBR_PARTITION_RECORD' definition within 
> edk2/MdePkg/Include/IndustryStandard/Mbr.h
> 
> and thus get rid of adding this one?
> 
> Chen: This structure defined as the following format in Mbr.h
> 
> typedef struct {
>   ..
>   UINT8 StartingLBA[4];
>   UINT8 SizeInLBA[4];
> } MBR_PARTITION_RECORD;
> 
> For StartingLBA, this field is represented the LBA, so I think it 
> should be
> UINT32 type not an array type.

I do not get your point, what prevents you from getting the LBA information for 
the byte array?

Chen: Yes, GPT DXE driver use UNPACK_UINT32 macro to extract UINT32 from 
array. But I thought use UINT32 type get the value more directly.

Best Regards,
Hao Wu

> 
> The same meaning is also for SizeInLba.
> 
> Best Regards,
> Hao Wu
> 
> >  ///
> >  /// GPT Partition Table Header.
> >  ///
> > --
> > 2.16.2.windows.1
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 2/2] MdePkg/UefiGpt.h: Add new definition for enable GPT support

2019-01-24 Thread Chen, Chen A



-Original Message-
From: Wu, Hao A 
Sent: Friday, January 25, 2019 11:04 AM
To: Chen, Chen A ; edk2-devel@lists.01.org
Cc: Kinney, Michael D ; Zhang, Chao B 
; Gao, Liming 
Subject: RE: [edk2] [PATCH 2/2] MdePkg/UefiGpt.h: Add new definition for enable 
GPT support

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Chen A Chen
> Sent: Thursday, January 17, 2019 10:03 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D; Zhang, Chao B; Gao, Liming
> Subject: [edk2] [PATCH 2/2] MdePkg/UefiGpt.h: Add new definition for 
> enable GPT support
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1470
> This two new definitions are defined for GPT in FatPei diver.
> 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> Cc: Zhang Chao B 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chen A Chen 
> ---
>  MdePkg/Include/Uefi/UefiGpt.h | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/MdePkg/Include/Uefi/UefiGpt.h 
> b/MdePkg/Include/Uefi/UefiGpt.h index f635b05390..8665c8cbc9 100644
> --- a/MdePkg/Include/Uefi/UefiGpt.h
> +++ b/MdePkg/Include/Uefi/UefiGpt.h
> @@ -24,9 +24,25 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, 
> EITHER EXPRESS OR IMPLIED.
>  /// EFI Partition Table Signature: "EFI PART".
>  ///
>  #define EFI_PTAB_HEADER_ID  SIGNATURE_64 ('E','F','I',' 
> ','P','A','R','T')
> +///
> +/// Minimum bytes reserve for EFI entry array buffer.
> +///
> +#define GPT_PART_ENTRY_MIN_SIZE 16384

May I know where this definition comes from?
Does it come from the UEFI spec?

Chen: The MACRO is not explicitly defined in UEFI Spec, But UEFI Spec specifies 
a minimum value for GPT entry araray.

> 
>  #pragma pack(1)
> 
> +///
> +/// MBR Partition Entry
> +///
> +typedef struct {
> +  UINT8   BootIndicator;
> +  UINT8   StartingCHS[3];
> +  UINT8   OSType;
> +  UINT8   EndingCHS[3];
> +  UINT32  StartingLBA;
> +  UINT32  SizeInLBA;
> +} MBR_PARTITION_ENTRY;
> +

What about using the 'MBR_PARTITION_RECORD' definition within 
edk2/MdePkg/Include/IndustryStandard/Mbr.h

and thus get rid of adding this one?

Chen: This structure defined as the following format in Mbr.h

typedef struct {
  ..
  UINT8 StartingLBA[4];
  UINT8 SizeInLBA[4];
} MBR_PARTITION_RECORD;

For StartingLBA, this field is represented the LBA, so I think it should be 
UINT32 type not an array type.

The same meaning is also for SizeInLba.

Best Regards,
Hao Wu

>  ///
>  /// GPT Partition Table Header.
>  ///
> --
> 2.16.2.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 3/3] FatPkg: Add GPT check in FatPei to support Capsule-on-Disk feature.

2019-01-23 Thread Chen, Chen A
Hi Ray and Hao

Do you have any comments for this patch?

-Original Message-
From: Zhang, Chao B 
Sent: Wednesday, January 23, 2019 2:22 PM
To: Chen, Chen A ; edk2-devel@lists.01.org
Cc: Ni, Ray 
Subject: RE: [PATCH 3/3] FatPkg: Add GPT check in FatPei to support 
Capsule-on-Disk feature.

Reviewed-by : Chao Zhang 

-Original Message-
From: Chen, Chen A
Sent: Thursday, January 17, 2019 10:03 AM
To: edk2-devel@lists.01.org
Cc: Chen, Chen A ; Ni, Ray ; Zhang, 
Chao B 
Subject: [PATCH 3/3] FatPkg: Add GPT check in FatPei to support Capsule-on-Disk 
feature.

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1470
This feature is used for finding GPT partition, follow the following step to 
check.
1) Check Protective MBR.
2) Check GPT primary/backup header.
3) Check GPT primary/backup entry array.

Cc: Ruiyu Ni 
Cc: Zhang Chao B 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chen A Chen 
---
 FatPkg/FatPei/FatLitePeim.h |   1 +
 FatPkg/FatPei/FatPei.inf|   3 +
 FatPkg/FatPei/Gpt.c | 546 
 3 files changed, 550 insertions(+)
 create mode 100644 FatPkg/FatPei/Gpt.c

diff --git a/FatPkg/FatPei/FatLitePeim.h b/FatPkg/FatPei/FatLitePeim.h index 
fbf887da5f..afb429c56e 100644
--- a/FatPkg/FatPei/FatLitePeim.h
+++ b/FatPkg/FatPei/FatLitePeim.h
@@ -27,6 +27,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/FatPkg/FatPei/FatPei.inf b/FatPkg/FatPei/FatPei.inf index 
829e87fe92..dd0869f7cd 100644
--- a/FatPkg/FatPei/FatPei.inf
+++ b/FatPkg/FatPei/FatPei.inf
@@ -31,6 +31,7 @@
 
 [Sources]
   Mbr.c
+  Gpt.c
   Eltorito.c
   Part.c
   FatLiteApi.c
@@ -49,6 +50,7 @@
 [LibraryClasses]
   PcdLib
   BaseMemoryLib
+  MemoryAllocationLib
   PeimEntryPoint
   BaseLib
   DebugLib
@@ -61,6 +63,7 @@
   gRecoveryOnFatIdeDiskGuid   ## SOMETIMES_CONSUMES   ## 
UNDEFINED
   gRecoveryOnFatFloppyDiskGuid## SOMETIMES_CONSUMES   ## 
UNDEFINED
   gRecoveryOnFatNvmeDiskGuid  ## SOMETIMES_CONSUMES   ## 
UNDEFINED
+  gEfiPartTypeUnusedGuid  ## SOMETIMES_CONSUMES   ## 
UNDEFINED
 
 
 [Ppis]
diff --git a/FatPkg/FatPei/Gpt.c b/FatPkg/FatPei/Gpt.c new file mode 100644 
index 00..d1f4c1c8b5
--- /dev/null
+++ b/FatPkg/FatPei/Gpt.c
@@ -0,0 +1,546 @@
+/** @file
+  Routines supporting partition discovery and
+  logical device reading
+
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+
+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 http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, 
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+#include 
+#include 
+#include "FatLitePeim.h"
+
+//
+// Assumption: 'a' and 'blocksize' are all UINT32 or UINT64.
+// If 'a' and 'blocksize' are not the same type, should use DivU64xU32 to 
calculate.
+//
+#define EFI_SIZE_TO_BLOCKS(a, blocksize)  (((a) / (blocksize)) + (((a) 
+% (blocksize)) ? 1 : 0))
+
+//
+// GPT Partition Entry Status
+//
+typedef struct {
+  BOOLEAN OutOfRange;
+  BOOLEAN Overlap;
+  BOOLEAN OsSpecific;
+} EFI_PARTITION_ENTRY_STATUS;
+
+/**
+  Check if the CRC field in the Partition table header is valid
+
+  @param[in]  BlockIo Parent BlockIo interface
+  @param[in]  DiskIo  Disk Io Protocol.
+  @param[in]  PartHeader  Partition table header structure
+
+  @retval TRUE  the CRC is valid
+  @retval FALSE the CRC is invalid
+
+**/
+BOOLEAN
+PartitionCheckGptHeaderCRC (
+  IN  EFI_PARTITION_TABLE_HEADER  *PartHeader
+  )
+{
+  UINT32  GptHdrCrc;
+  UINT32  Crc;
+
+  GptHdrCrc = PartHeader->Header.CRC32;
+
+  //
+  // Set CRC field to zero when doing calcuation  //
+  PartHeader->Header.CRC32 = 0;
+
+  Crc = CalculateCrc32 (PartHeader, PartHeader->Header.HeaderSize);
+
+  //
+  // Restore Header CRC
+  //
+  PartHeader->Header.CRC32 = GptHdrCrc;
+
+  return (GptHdrCrc == Crc);
+}
+
+
+/**
+  Check if the CRC field in the Partition table header is valid
+  for Partition entry array.
+
+  @param[in]  BlockIo Parent BlockIo interface
+  @param[in]  DiskIo  Disk Io Protocol.
+  @param[in]  PartHeader  Partition table header structure
+
+  @retval TRUE  the CRC is valid
+  @retval FALSE the CRC is invalid
+
+**/
+BOOLEAN
+PartitionCheckGptEntryArrayCRC (
+  IN  EFI_PARTITION_TABLE_HEADER *PartHeader,
+  IN  EFI_PARTITION_ENTRY*PartEntry
+  )
+{
+  UINT32  Crc;
+  UINTN   Size;
+
+  Size = (UINTN)MultU64x32(PartHeader->NumberOfPartitionEntries,
+ PartHeader->SizeOfPartitionEntry);

Re: [edk2] [PATCH 1/3] IntelFrameworkModulePkg: Remove the missing PalLib in DSC file.

2018-09-24 Thread Chen, Chen A
Ok

-Original Message-
From: Gao, Liming 
Sent: Tuesday, September 25, 2018 9:16 AM
To: Chen, Chen A ; edk2-devel@lists.01.org
Cc: Kinney, Michael D 
Subject: RE: [PATCH 1/3] IntelFrameworkModulePkg: Remove the missing PalLib in 
DSC file.

Chen:
  Please update patch title and commit message to follow Star comments. 
 
 Reviewed-by: Liming Gao 

Thanks
Liming
>-Original Message-
>From: Chen, Chen A
>Sent: Friday, September 21, 2018 9:00 AM
>To: edk2-devel@lists.01.org
>Cc: Chen, Chen A ; Gao, Liming 
>; Kinney, Michael D 
>Subject: [PATCH 1/3] IntelFrameworkModulePkg: Remove the missing PalLib 
>in DSC file.
>
>The PalLib will remove in MdePkg, so remove this lib from DSC file.
>
>Cc: Liming Gao 
>Cc: Michael D Kinney 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Chen A Chen 
>---
> IntelFrameworkModulePkg/IntelFrameworkModulePkg.dsc | 1 -
> 1 file changed, 1 deletion(-)
>
>diff --git a/IntelFrameworkModulePkg/IntelFrameworkModulePkg.dsc
>b/IntelFrameworkModulePkg/IntelFrameworkModulePkg.dsc
>index 84e1d890b5..894c5340a0 100644
>--- a/IntelFrameworkModulePkg/IntelFrameworkModulePkg.dsc
>+++ b/IntelFrameworkModulePkg/IntelFrameworkModulePkg.dsc
>@@ -75,7 +75,6 @@
>
>DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTable
>Lib.inf
>   UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
>   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
>-  PalLib|MdePkg/Library/BasePalLibNull/BasePalLibNull.inf
>
> [LibraryClasses.common.PEIM]
>   HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>--
>2.16.2.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 2/3] MdeModulePkg: Remove the missing PalLib in DSC file.

2018-09-24 Thread Chen, Chen A
Good suggest for me.

-Original Message-
From: Zeng, Star 
Sent: Friday, September 21, 2018 11:48 AM
To: Chen, Chen A ; edk2-devel@lists.01.org
Cc: Dong, Eric ; Kinney, Michael D 
; Zeng, Star 
Subject: RE: [PATCH 2/3] MdeModulePkg: Remove the missing PalLib in DSC file.

As I remember, I raised comment about removing the PalLib in MdeModulePkg.dsc 
at https://lists.01.org/pipermail/edk2-devel/2018-June/026079.html.

I'd like suggest updating the title and commit message a little.

For title: How about "MdeModulePkg: Remove PalLib in dsc which was missed at 
de00522" ?

For commit message: How about like below?

The PalLib is IPF specific and will be removed from MdePkg.
So this patch removes PalLib in MdeModulePkg.dsc which was missed at 
de005223b77c473d45c9c8a11147f6968325f73e.

With them accepted, Reviewed-by: Star Zeng .


Thanks,
Star
-Original Message-----
From: Chen, Chen A
Sent: Friday, September 21, 2018 9:00 AM
To: edk2-devel@lists.01.org
Cc: Chen, Chen A ; Zeng, Star ; 
Dong, Eric ; Kinney, Michael D 
Subject: [PATCH 2/3] MdeModulePkg: Remove the missing PalLib in DSC file.

The PalLib will remove in MdePkg, so remove this lib from DSC file.

Cc: Star Zeng 
Cc: Eric Dong 
Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chen A Chen 
---
 MdeModulePkg/MdeModulePkg.dsc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc 
index 8a81ea141f..3ff3b1213c 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -79,7 +79,6 @@
   SerialPortLib|MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
-  PalLib|MdePkg/Library/BasePalLibNull/BasePalLibNull.inf
   
CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
   
FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
   #
--
2.16.2.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] SecurityPkg:AuthVariableLib:Implement ECR1707 for Private Auth Variable

2017-10-12 Thread Chen, Chen A
Reviewed-by: Chen A Chen 

-Original Message-
From: Zhang, Chao B 
Sent: Thursday, October 12, 2017 5:14 PM
To: edk2-devel@lists.01.org
Cc: Long, Qin ; Chen, Chen A ; 
Zhang, Chao B 
Subject: [PATCH] SecurityPkg:AuthVariableLib:Implement ECR1707 for Private Auth 
Variable

ECR1707 for UEFI2.7 clarified certificate management rule for private 
time-based AuthVariable.Trusted cert rule changed from whole signer's 
certificate stack to top-level issuer cert tbscertificate + SignerCert CN for 
better management compatibility.
Hash is used to reduce storage overhead.

Cc: Long Qin 
Cc: Chen Chen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang 
---
 SecurityPkg/Library/AuthVariableLib/AuthService.c | 208 ++
 1 file changed, 171 insertions(+), 37 deletions(-)

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c 
b/SecurityPkg/Library/AuthVariableLib/AuthService.c
index a37ec0b..7188ff6 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
@@ -1530,6 +1530,85 @@ AuthServiceInternalCompareTimeStamp (  }
 
 /**
+  Calculate SHA256 digest of SignerCert CommonName + ToplevelCert 
+ tbsCertificate  SignerCert and ToplevelCert are inside the signer certificate 
chain.
+
+  @param[in]  SignerCert  A pointer to SignerCert data.
+  @param[in]  SignerCertSize  Length of SignerCert data.
+  @param[in]  TopLevelCertA pointer to TopLevelCert data.
+  @param[in]  TopLevelCertSizeLength of TopLevelCert data.
+  @param[out] Sha256Digest   Sha256 digest calculated.
+
+  @return EFI_ABORTED  Digest process failed.
+  @return EFI_SUCCESS  SHA256 Digest is succesfully calculated.
+
+**/
+EFI_STATUS
+CalculatePrivAuthVarSignChainSHA256Digest(
+  IN UINT8*SignerCert,
+  IN UINTNSignerCertSize,
+  IN UINT8*TopLevelCert,
+  IN UINTNTopLevelCertSize,
+  OUTUINT8*Sha256Digest
+  )
+{
+  UINT8   *TbsCert;
+  UINTN   TbsCertSize;
+  UINT8   CertCommonName[128];
+  UINTN   CertCommonNameSize;
+  BOOLEAN CryptoStatus;
+  EFI_STATUS  Status;
+
+  CertCommonNameSize = sizeof(CertCommonName);
+
+  //
+  // Get SignerCert CommonName
+  //
+  Status = X509GetCommonName(SignerCert, SignerCertSize, 
+ CertCommonName, &CertCommonNameSize);  if (EFI_ERROR(Status)) {
+DEBUG((DEBUG_INFO, "%a Get SignerCert CommonName failed with status %x\n", 
__FUNCTION__, Status));
+return EFI_ABORTED;
+  }
+
+  //
+  // Get TopLevelCert tbsCertificate
+  //
+  if (!X509GetTBSCert(TopLevelCert, TopLevelCertSize, &TbsCert, &TbsCertSize)) 
{
+DEBUG((DEBUG_INFO, "%a Get Top-level Cert tbsCertificate failed!\n", 
__FUNCTION__));
+return EFI_ABORTED;
+  }
+
+  //
+  // Digest SignerCert CN + TopLevelCert tbsCertificate  //  ZeroMem 
+ (Sha256Digest, SHA256_DIGEST_SIZE);  CryptoStatus = Sha256Init 
+ (mHashCtx);  if (!CryptoStatus) {
+return EFI_ABORTED;
+  }
+
+  //
+  // '\0' is forced in CertCommonName. No overflow issue  //  
+ CryptoStatus = Sha256Update (mHashCtx, CertCommonName, 
+ AsciiStrLen((CHAR8 *)CertCommonName));  if (!CryptoStatus) {
+return EFI_ABORTED;
+  }
+
+  CryptoStatus = Sha256Update (mHashCtx, TbsCert, TbsCertSize);  if 
+ (!CryptoStatus) {
+return EFI_ABORTED;
+  }
+
+  CryptoStatus  = Sha256Final (mHashCtx, Sha256Digest);  if 
+ (!CryptoStatus) {
+return EFI_ABORTED;
+  }
+
+  return EFI_SUCCESS;
+}
+
+/**
   Find matching signer's certificates for common authenticated variable
   by corresponding VariableName and VendorGuid from "certdb" or "certdbv".
 
@@ -1872,13 +1951,16 @@ DeleteCertsFromDb (
 /**
   Insert signer's certificates for common authenticated variable with 
VariableName
   and VendorGuid in AUTH_CERT_DB_DATA to "certdb" or "certdbv" according to
-  time based authenticated variable attributes.
+  time based authenticated variable attributes. CertData is the SHA256 
+ digest of  SignerCert CommonName + TopLevelCert tbsCertificate.
 
-  @param[in]  VariableName   Name of authenticated Variable.
-  @param[in]  VendorGuid Vendor GUID of authenticated Variable.
-  @param[in]  Attributes Attributes of authenticated variable.
-  @param[in]  CertData   Pointer to signer's certificates.
-  @param[in]  CertDataSize   Length of CertData in bytes.
+  @param[in]  VariableName  Name of authenticated Variable.
+  @param[in]  VendorGuidVendor GUID of authenticated Variable.
+  @param[in]  AttributesAttributes of authenticated variable.
+  @param[in]  SignerCertSigner certificate data.
+  @param[in]  SignerCertSizeLength of signer certificate.
+  @param[in]  TopLevelCert  Top-level certificate data.
+  @param[in]  TopLeve