Re: [edk2-devel] [PATCH v2 02/10] UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib library instance

[Ni, Ray]

Comments below starting with [Ray]

Thanks,
Ray


From: Wu, Jiaxin 
Sent: Tuesday, April 16, 2024 20:58
To: Ni, Ray ; devel@edk2.groups.io 
Cc: Zeng, Star ; Gerd Hoffmann ; Kumar, 
Rahul R 
Subject: RE: [PATCH v2 02/10] UefiCpuPkg/SmmRelocationLib: Add SmmRelocationLib 
library instance



 /**
@@ -30,11 +30,12 @@ SemaphoreHook (
 {
   SMRAM_SAVE_STATE_MAP  *CpuState;

   mRebasedFlag = RebasedFlag;

-  CpuState  = (SMRAM_SAVE_STATE_MAP 
*)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);
+  CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + 
SMRAM_SAVE_STATE_MAP_OFFSET);



[Ray.1] This change is unnecessary.

[Jiaxin.1] it’s not only move the code to the new lib, but also do some 
adaptation. For example here: the code style change to PASS the CI.

If no this change, can’t pass the CI check.

[Ray] Original code should also pass uncrustify checker. I guess you added one 
blank line above so the alignment of "=" is not required.




[Ray.4] Can you evaluate what extra changes are required if allowing the lib 
runs in flash area? Basically all global variables cannot be modified at 
runtime.

[Jiaxin.4] The Lib needs to depend on the MP service PPI, it shall be called 
during post-mem phase, global variables can’t be used?

[Ray] A PEIM could run in flash in post-mem phase. If you pass the values 
across routines through parameters, most of the global variables can be avoided.


+  UINTN   SmramRanges;

[Ray.6] No need SmramRanges.

[Jiaxin.6] replace it with DescriptorBlock ->NumberOfSmmReservedRegions 
directly?

[Ray] yes.


+  //
+  // Increase the number of SMRAM descriptors by 1 to make room for the 
ALLOCATED descriptor of size EFI_PAGE_SIZE
+  //
+  NewDescriptorBlock->NumberOfSmmReservedRegions = (UINT32)(SmramRanges + 1);

[Ray.9] NewBlock->NumberOfSmmReservedRegions++;

[Jiaxin.9] Agree since we copied the DescriptorBlock to NewDescriptorBlock 
first. But I still think original is more easy to understand.

[Ray] As long as you remove local variable SmramRanges, I am fine.



+
+DEBUG ((DEBUG_INFO, "CreateSmmBaseHob - 
SmmBaseHobData[%d]->ProcessorIndex: %03x\n", HobCount, 
SmmBaseHobData->ProcessorIndex));
+DEBUG ((DEBUG_INFO, "CreateSmmBaseHob - 
SmmBaseHobData[%d]->NumberOfProcessors: %03x\n", HobCount, 
SmmBaseHobData->NumberOfProcessors));
+for (Index = 0; Index < SmmBaseHobData->NumberOfProcessors; Index++) {
+  //
+  // Calculate the new SMBASE address
+  //
+  SmmBaseHobData->SmBase[Index] = SmBaseForAllCpus[Index + CpuCount];

[Ray.12] Please re-organize the code so that SmBaseForAllCpus array is not 
needed. What we need is only the Cpu0SmBase and TileSize.

[Jiaxin.12] do you mean calculate the value here? --> (UINTN)(Cpu0SmBase)+ 
Index * TileSize - SMM_HANDLER_OFFSET ?

I init the smbase value in the function of InitSmBaseForAllCpus(), the value 
will be used in both ConfigureSmBase & CreateSmmBaseHob.

How about the ConfigureSmBase function during SmmRelocateBases()? What’s reason 
you think SmBaseForAllCpus is not good?

[Ray] I just want to avoid unnecessary memory allocation.




+
+  //
+  // Patch SMI stack for SMM base relocation
+  // Note: No need allocate stack for all CPUs since the relocation
+  // occurs serially for each CPU
+  //
+  SmmStackSize = EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (PcdGet32 
(PcdCpuSmmStackSize)));

[Ray.15] PcdCpuSmmStackSize is configured by platform as only platform knows 
what kind of SMI handlers will run in SMM env.

But in this case, all code is provided by this lib and stack size should be 
fixed, maybe simply 1 page is enough.

[Jiaxin.15] agree, do you prefer this change as another patch or I just update 
the hard code value directly in this patch?

[Ray] If the code is inherited from PiSmmCpuDxeSmm driver, you can do that 
change in another patch.

+  //
+  // Retrieve the Processor Info for all CPUs
+  //
+  mProcessorInfo = (EFI_PROCESSOR_INFORMATION *)AllocatePool (sizeof 
(EFI_PROCESSOR_INFORMATION) * mMaxNumberOfCpus);

[Ray.16] mProcessorInfo is needed when programming the new SMBASE. Then can you 
just put the new SMBASE for every CPU in the stack top, or just patch the value 
in the code region?

You can do that in a new patch.

[Jiaxin.16] why need such behavior? I only allocate one stack for all cpus with 
existing design. The reason see below as I explained:

[Ray] My purpose is to avoid global variables.



+  //
+  // Initialize the SmBase for all CPUs
+  //
+  Status = InitSmBaseForAllCpus ();

[Ray.17] mSmBaseForAllCpus global variable is not needed. We only need 
Cpu0Smbase and TileSize and the two can be local variables and passed to 
further routines through parameters.

[Jiaxin.17] let me remove the mSmBaseForAllCpus global variable. But I still 
think it’s not good to calculate value in different 2 places again and again 
(ConfigureSmBase & CreateSmmBaseHob).

[Ray] I agree with you regarding 

Re: [edk2-devel] [PATCH V1 0/5] Move Tdx specific lib from SecurityPkg to OvmfPkg

[Yao, Jiewen]

I have merged this one https://github.com/tianocore/edk2/pull/5566

Hi Gerd
If you prefer that we move all TDX / SEV specific component to IntelTdx and 
AmdSev, I am OK with that.

Personally, I like your idea. Please submit Bugzilla and work on it, if you 
would like to.

Thank you
Yao, Jiewen


> -Original Message-
> From: Yao, Jiewen
> Sent: Tuesday, April 16, 2024 11:40 PM
> To: Gerd Hoffmann ; devel@edk2.groups.io; Xu, Min M
> 
> Cc: Ard Biesheuvel 
> Subject: RE: [edk2-devel] [PATCH V1 0/5] Move Tdx specific lib from 
> SecurityPkg
> to OvmfPkg
> 
> Yeah, I also considered that before. But after look at current code 
> structure, I give
> up.
> 
> Since following SEV component are NOT in AmdSev directory (especially the TCG
> one), I do not see a strong reason to put them to IntelTdx dir.
> https://github.com/tianocore/edk2/tree/master/OvmfPkg/AmdSevDxe
> https://github.com/tianocore/edk2/tree/master/OvmfPkg/Tcg/TpmMmioSevDec
> ryptPei
> https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/BaseMemEncr
> yptSevLib
> 
> I think we can follow the existing code structure in this patch set.
> 
> Thank you
> Yao, Jiewen
> 
> 
> > -Original Message-
> > From: Gerd Hoffmann 
> > Sent: Tuesday, April 16, 2024 6:16 PM
> > To: devel@edk2.groups.io; Xu, Min M 
> > Cc: Ard Biesheuvel ; Yao, Jiewen
> > 
> > Subject: Re: [edk2-devel] [PATCH V1 0/5] Move Tdx specific lib from 
> > SecurityPkg
> > to OvmfPkg
> >
> > On Mon, Apr 15, 2024 at 03:55:49PM +0800, Min Xu wrote:
> > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4752
> > >
> > > HashLibTdx and TdTcg2Dxe are designed for Intel TDX enlightened OVMF.
> > > They're more reasonable to be put in OvmfPkg than in SecurityPkg.
> > >
> > > SecTpmMeasurementLibTdx is not used anymore. So it is deleted in this
> > > patch-set.
> > >
> >
> > >  rename {SecurityPkg => OvmfPkg}/Library/HashLibTdx/HashLibTdx.c (100%)
> > >  rename {SecurityPkg => OvmfPkg}/Library/HashLibTdx/HashLibTdx.inf (100%)
> > >  rename {SecurityPkg => OvmfPkg}/Tcg/TdTcg2Dxe/MeasureBootPeCoff.c
> > (100%)
> > >  rename {SecurityPkg => OvmfPkg}/Tcg/TdTcg2Dxe/TdTcg2Dxe.c (100%)
> > >  rename {SecurityPkg => OvmfPkg}/Tcg/TdTcg2Dxe/TdTcg2Dxe.inf (100%)
> >
> > Better place them in OvmfPkg/IntelTdx ?
> >
> > Otherwise looks fine to me.
> >
> > take care,
> >   Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117892): https://edk2.groups.io/g/devel/message/117892
Mute This Topic: https://groups.io/mt/105531957/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 21/26] OvmfPkg/LoongArchVirt: Add FdtQemuFwCfgLib

[Chao Li]

Hi Gerd,

Part 2 has been be merged, I'm separating  this Lib into two serve the 
PEI stage and DXE stage.


Currently, This DXE library uses three global variables, and when I 
simulate the no-mmio version: MmioLib.c + Dxe.c + Pei.c, I can abstract 
some helper functions as the public functions in Mmio version.


Do you mind if I replace these three vaiables with three dynamically 
typed PCDs? If so, the PEI and DXE stage libraries can using  some of 
the same APIs.



Thanks,
Chao
On 2024/3/21 20:27, Chao Li wrote:


Hi Gerd,

On 2024/3/21 19:39, Gerd Hoffmann wrote:

   Hi,


QemuFwCfgLibMmio.inf is looks like a DXE stage library, while this patch is
the PEI stage library we are dicussing.

I have tow plans:

*Plan A:* Keep this library under LoongArchQemuVirt.

*Plan  B:* Create a new INF named QemuFwCfgPeiLibMmio.inf under
OvmfPkg/Library/QemuFwCfgLib/, which will obtain the resources from FDT, and
store them in the HOB or dynamic PCD.

Which one do you like? I'm leaning toward B because more people will be
served if it's under OvmfPkg/Library.

Yes, Plan (b) is better.  Also try avoid code duplication.  The existing
code can be splitted into two files.  Move the code which works in DXE
only (i.e. the bits using FdtClientProtocol to find the fw_cfg mmio
address, maybe more) to QemuFwCfgLibMmioDxe.c, keep the code which can
work for both PEI and DXE in QemuFwCfgLibMmio.c.  Add
QemuFwCfgLibMmioPei.c for the PEI-specific code.

The ioport version of the library uses the same approach with
QemuFwCfgLib.c + QemuFwCfgDxe.c + QemuFwCfgPei.c
OK, I'm going to do the plan B next and commit it with the next 
version, which will probably be called "Part 3 patch set, enable 
LoongArchVirtQemu in OvmfPkg".

take care,
   Gerd










-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117891): https://edk2.groups.io/g/devel/message/117891
Mute This Topic: https://groups.io/mt/104859897/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] MdeModulePkg: Fix MAT SplitRecord() Logic introduce one bug and will cause SUT reset when boot to windows

[Taylor Beebe]

Hi Yanbo,

Can you confirm that the following resolves the issue you're seeing?

[PATCH v1] MdeModulePkg: Fixup MAT Attributes After Splitting EFI Memory 
Map (groups.io) 


-Taylor

On 4/15/2024 6:17 PM, Taylor Beebe wrote:

On 4/15/2024 3:57 AM, Bi, Dandan wrote:

Hi Taylor,

With this patch, MAT contains some entries with Attribute - 0x8000, 
doesn't have EFI_MEMORY_RO or EFI_MEMORY_XP.
After revert this patch, don't see such entries in MAT.

a. MAT with this patch:
Entry (0x609E4268)
   Type  - 0x5
   PhysicalStart - 0x769CF000
   VirtualStart  - 0x
   NumberOfPages - 0x0016
   Attribute - 0x8000
Entry (0x609E4298)
   Type  - 0x5
   PhysicalStart - 0x769E5000
   VirtualStart  - 0x
   NumberOfPages - 0x0001
   Attribute - 0x80004000
Entry (0x609E42C8)
   Type  - 0x5
   PhysicalStart - 0x769E6000
   VirtualStart  - 0x
   NumberOfPages - 0x0002
   Attribute - 0x8002

b. MAT without this patch:
Entry (0x609E4268)
   Type  - 0x5
   PhysicalStart - 0x769CF000
   VirtualStart  - 0x
   NumberOfPages - 0x0017
   Attribute - 0x80004000
Entry (0x609E4298)
   Type  - 0x5
   PhysicalStart - 0x769E6000
   VirtualStart  - 0x
   NumberOfPages - 0x0002
   Attribute - 0x8002

1. For example, when OldRecord in old memory map with:
 Type - 0x0005
 Attribute - 0x800F
 PhysicalStart - 0x769CF000
 PhysicalStart is smaller than ImageBase 0x769E5000, with this patch, it 
will create a new memory descriptor entry for range 0x769CF000~0x769E5000 and 
without EFI_MEMORY_RO or EFI_MEMORY_XP Attribute.
 Then it will only contain EFI_MEMORY_RUNTIME Attribute in MAT as doing  
MemoryAttributesEntry->Attribute &= 
(EFI_MEMORY_RO|EFI_MEMORY_XP|EFI_MEMORY_RUNTIME); when install MAT.
 It seems not aligned with UEFI Spec " The only valid bits for Attribute field 
currently are EFI_MEMORY_RO ,EFI_MEMORY_XP , plus EFI_MEMORY_RUNTIME "?
 Could you please help double check? Thanks.
Agreed that this is currently the behaviour and that the range should 
have a restrictive access attribute. More below.

2. In function SetNewRecord, it semes already cover the DATA entry before the 
CODE and the DATA entry after the CODE.
 And old SplitRecord function without this patch, also has the entry to 
cover the reaming range of this record if no more image covered by this range.
 Why do we still need this patch? Could you please help explain? Thanks.


GetMemoryMap() will merge adjacent descriptors which have the same 
type and attributes. This means that a single EfiRuntimeServicesCode 
descriptor within the memory map returned by CoreGetMemoryMap() could 
describe memory with the following layout (NOTE: this layout is odd 
but needs to be handled to fulfill the SplitTable() contract):


-
Some EfiRuntimeServicesCode memory
-
Runtime Image DATA Section
-
Runtime Image CODE Section
-
Runtime Image DATA Section
-
Some EfiRuntimeServicesCode memory
-

In this possible layout, the pre-patch logic would assume that the 
regions before and after the image were part of the image's data 
sections and would mark them as EFI_MEMORY_XP. The post-patch logic 
does not mark them with any access attributes which is fine but the 
DXE MAT logic needs to walk the memory map returned by SplitTable() to 
add access attributes to runtime regions which don't have any.


In your example, because PhysicalStart < ImageBase and 
0769CF000-0x769E5000 is EfiRuntimeServicesCode, the access attribute 
should technically be EFI_MEMORY_RO. It's likely that 
0769CF000-0x769E5000is just the unused memory bucket and so it might 
be best to mark it both EFI_MEMORY_RO and EFI_MEMORY_XP.


*An open question to the community:* Are there cases where runtime 
executable code is in a buffer separate from a loaded runtime image? 
Can the EfiRuntimeServicesCode memory regions not part of an image be 
specified in the MAT as both EFI_MEMORY_RO and EFI_MEMORY_XP, or even 
dropped entirely?


Thanks :)
-Taylor



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117890): https://edk2.groups.io/g/devel/message/117890
Mute This Topic: https://groups.io/mt/105477564/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v1] MdeModulePkg: Fixup MAT Attributes After Splitting EFI Memory Map

[Taylor Beebe]

The Memory Attributes Table is generated by fetching the EFI
memory map and splitting entries which contain loaded
images so DATA and CODE sections have separate descriptors.
The splitting is done via a call to SplitTable() which
marks image DATA sections with the EFI_MEMORY_XP attribute
and CODE sections with the EFI_MEMORY_RO attribute when
splitting. After this process, there may still be
EfiRuntimeServicesCode regions which did not have their
attributes set because they are not part of loaded images.

This patch updates the MAT EnforceMemoryMapAttribute logic
to set the access attributes of runtime memory regions
which are not part of loaded images (have not had their
access attributes set).

Cc: Liming Gao 
Signed-off-by: Taylor Beebe 
---
 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 29 ++--
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c 
b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
index e9343a2c4e..1d9f935db0 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
@@ -425,8 +425,8 @@ MergeMemoryMap (
 }
 
 /**
-  Enforce memory map attributes.
-  This function will set 
EfiRuntimeServicesData/EfiMemoryMappedIO/EfiMemoryMappedIOPortSpace to be 
EFI_MEMORY_XP.
+  Walk the memory map and set 
EfiRuntimeServicesData/EfiMemoryMappedIO/EfiMemoryMappedIOPortSpace
+  to EFI_MEMORY_XP and EfiRuntimeServicesCode to EFI_MEMORY_RO.
 
   @param  MemoryMap  A pointer to the buffer in which firmware 
places
  the current memory map.
@@ -447,18 +447,19 @@ EnforceMemoryMapAttribute (
   MemoryMapEntry = MemoryMap;
   MemoryMapEnd   = (EFI_MEMORY_DESCRIPTOR *)((UINT8 *)MemoryMap + 
MemoryMapSize);
   while ((UINTN)MemoryMapEntry < (UINTN)MemoryMapEnd) {
-switch (MemoryMapEntry->Type) {
-  case EfiRuntimeServicesCode:
-// do nothing
-break;
-  case EfiRuntimeServicesData:
-  case EfiMemoryMappedIO:
-  case EfiMemoryMappedIOPortSpace:
-MemoryMapEntry->Attribute |= EFI_MEMORY_XP;
-break;
-  case EfiReservedMemoryType:
-  case EfiACPIMemoryNVS:
-break;
+if ((MemoryMapEntry->Attribute & EFI_MEMORY_ACCESS_MASK) == 0) {
+  switch (MemoryMapEntry->Type) {
+case EfiRuntimeServicesCode:
+  MemoryMapEntry->Attribute |= EFI_MEMORY_RO;
+  break;
+case EfiRuntimeServicesData:
+case EfiMemoryMappedIO:
+case EfiMemoryMappedIOPortSpace:
+  MemoryMapEntry->Attribute |= EFI_MEMORY_XP;
+  break;
+default:
+  break;
+  }
 }
 
 MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);
-- 
2.40.1.vfs.0.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117889): https://edk2.groups.io/g/devel/message/117889
Mute This Topic: https://groups.io/mt/105570114/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] NetworkPkg/WifiConnectionManagerDxe: Update UI according to UEFI spec

[Michael D Kinney]

Merged: https://github.com/tianocore/edk2/pull/5487



> -Original Message-
> From: Kinney, Michael D 
> Sent: Monday, April 15, 2024 10:32 PM
> To: devel@edk2.groups.io; Clark-williams, Zachary  willi...@intel.com>; Tan, Ming 
> Cc: Kasbekar, Saloni ; Bi, Dandan
> ; POLUDOV, FELIX ; Liu, Liqi
> ; Kinney, Michael D 
> Subject: RE: [edk2-devel] [PATCH] NetworkPkg/WifiConnectionManagerDxe:
> Update UI according to UEFI spec
> 
> Acked-by: Michael D Kinney 
> 
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of
> Clark-
> > williams, Zachary
> > Sent: Wednesday, April 10, 2024 9:03 AM
> > To: Tan, Ming ; devel@edk2.groups.io
> > Cc: Kasbekar, Saloni ; Bi, Dandan
> > ; POLUDOV, FELIX ; Liu, Liqi
> > 
> > Subject: Re: [edk2-devel] [PATCH]
> NetworkPkg/WifiConnectionManagerDxe:
> > Update UI according to UEFI spec
> >
> > Reviewed-by: Zachary Clark-Williams  willi...@intel.com>
> >
> > -Original Message-
> > From: Tan, Ming 
> > Sent: Thursday, March 21, 2024 6:16 PM
> > To: devel@edk2.groups.io
> > Cc: Kasbekar, Saloni ; Clark-williams,
> > Zachary ; Bi, Dandan
> > ; POLUDOV, FELIX ; Liu, Liqi
> > 
> > Subject: [PATCH] NetworkPkg/WifiConnectionManagerDxe: Update UI
> > according to UEFI spec
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4736
> >
> > In UEFI_Spec_2_10_Aug29.pdf page 1694 section 35.5.4 for
> > EFI_BROWSER_ACTION_FORM_OPEN:
> > NOTE: EFI_FORM_BROWSER2_PROTOCOL.BrowserCallback() cannot be used
> with
> > this browser action because question values have not been retrieved
> > yet.
> >
> > So should not call HiiGetBrowserData() and HiiSetBrowserData() in
> > FORM_OPEN call back function.
> >
> > Now use wifi list key and enroll cert key instead of the connect
> > action key, move wifi info display from open action to close action.
> >
> > Cc: Saloni Kasbekar 
> > Cc: Zachary Clark-williams 
> > Cc: Dandan Bi 
> > Cc: Felix Polyudov 
> > Signed-off-by: Liqi Liu 
> > Signed-off-by: Ming Tan 
> > ---
> >   PR: https://github.com/tianocore/edk2/pull/5487
> >
> >  .../WifiConnectionMgrHiiConfigAccess.c| 113 ---
> --
> > -
> >  1 file changed, 74 insertions(+), 39 deletions(-)
> >
> > diff --git
> >
> a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAccess
> > .c
> >
> b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAccess
> > .c
> > index 431fcbb33a..f242bdf056 100644
> > ---
> >
> a/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAccess
> > .c
> > +++
> > b/NetworkPkg/WifiConnectionManagerDxe/WifiConnectionMgrHiiConfigAcce
> > +++ ss.c
> > @@ -1412,7 +1412,9 @@ WifiMgrDxeHiiConfigAccessCallback (
> >  return EFI_OUT_OF_RESOURCES;   } -  HiiGetBrowserData
> > (, mVendorStorageName, BufferSize, (UINT8
> > *)IfrNvData);+  if (Action != EFI_BROWSER_ACTION_FORM_OPEN) {+
> > HiiGetBrowserData (, mVendorStorageName,
> > BufferSize, (UINT8 *)IfrNvData);+  }if (Action ==
> > EFI_BROWSER_ACTION_FORM_OPEN) { switch (QuestionId) {@@ -1438,43
> > +1440,6 @@ WifiMgrDxeHiiConfigAccessCallback (
> >   break; -  case KEY_CONNECT_ACTION:--if
> (Private-
> > >CurrentNic->UserSelectedProfile == NULL) {-  break;-
> > }--Profile = Private->CurrentNic->UserSelectedProfile;--
> > //-// Enter the network connection configuration page-
> > // Recovery from restored data-//-if (HiiSetString
> > (Private->RegisteredHandle, STRING_TOKEN (STR_SSID), Profile->SSId,
> > NULL) == 0) {-  return EFI_OUT_OF_RESOURCES;-}--
> > IfrNvData->SecurityType = Profile->SecurityType;-if
> > (HiiSetString (-  Private->RegisteredHandle,-
> > STRING_TOKEN (STR_SECURITY_TYPE),-
> > mSecurityType[IfrNvData->SecurityType],-  NULL-
> > ) == 0)-{-  return EFI_OUT_OF_RESOURCES;-}--
> > if ((IfrNvData->SecurityType == SECURITY_TYPE_WPA2_ENTERPRISE) ||-
> > (IfrNvData->SecurityType == SECURITY_TYPE_WPA3_ENTERPRISE))-
> {-
> > IfrNvData->EapAuthMethod   = Profile->EapAuthMethod;-
> > IfrNvData->EapSecondAuthMethod = Profile->EapSecondAuthMethod;-
> > StrCpyS (IfrNvData->EapIdentity, EAP_IDENTITY_SIZE, Profile-
> > >EapIdentity);-}--break;-   case
> > KEY_ENROLLED_CERT_NAME:  if (Private->CurrentNic-
> > >UserSelectedProfile == NULL) {@@ -1524,6 +1489,43 @@
> > WifiMgrDxeHiiConfigAccessCallback (
> >  }   } else if (Action == EFI_BROWSER_ACTION_FORM_CLOSE) {
> > switch (QuestionId) {+  case KEY_EAP_ENROLL_CERT_FROM_FILE:++
> > if (Private->CurrentNic->UserSelectedProfile == NULL) {+
> > break;+}++Profile = Private->CurrentNic-
> > >UserSelectedProfile;++//+// Enter the network
> > connection configuration page+// Recovery from restored
> data+
> > //+if (HiiSetString (Private->RegisteredHandle, STRING_TOKEN
> > (STR_SSID), Profile->SSId, NULL) == 0) {+  return
> > EFI_OUT_OF_RESOURCES;+}++

Re: [edk2-devel] [PATCH V1 0/5] Move Tdx specific lib from SecurityPkg to OvmfPkg

[Yao, Jiewen]

Yeah, I also considered that before. But after look at current code structure, 
I give up.

Since following SEV component are NOT in AmdSev directory (especially the TCG 
one), I do not see a strong reason to put them to IntelTdx dir.
https://github.com/tianocore/edk2/tree/master/OvmfPkg/AmdSevDxe
https://github.com/tianocore/edk2/tree/master/OvmfPkg/Tcg/TpmMmioSevDecryptPei
https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/BaseMemEncryptSevLib

I think we can follow the existing code structure in this patch set.

Thank you
Yao, Jiewen


> -Original Message-
> From: Gerd Hoffmann 
> Sent: Tuesday, April 16, 2024 6:16 PM
> To: devel@edk2.groups.io; Xu, Min M 
> Cc: Ard Biesheuvel ; Yao, Jiewen
> 
> Subject: Re: [edk2-devel] [PATCH V1 0/5] Move Tdx specific lib from 
> SecurityPkg
> to OvmfPkg
> 
> On Mon, Apr 15, 2024 at 03:55:49PM +0800, Min Xu wrote:
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4752
> >
> > HashLibTdx and TdTcg2Dxe are designed for Intel TDX enlightened OVMF.
> > They're more reasonable to be put in OvmfPkg than in SecurityPkg.
> >
> > SecTpmMeasurementLibTdx is not used anymore. So it is deleted in this
> > patch-set.
> >
> 
> >  rename {SecurityPkg => OvmfPkg}/Library/HashLibTdx/HashLibTdx.c (100%)
> >  rename {SecurityPkg => OvmfPkg}/Library/HashLibTdx/HashLibTdx.inf (100%)
> >  rename {SecurityPkg => OvmfPkg}/Tcg/TdTcg2Dxe/MeasureBootPeCoff.c
> (100%)
> >  rename {SecurityPkg => OvmfPkg}/Tcg/TdTcg2Dxe/TdTcg2Dxe.c (100%)
> >  rename {SecurityPkg => OvmfPkg}/Tcg/TdTcg2Dxe/TdTcg2Dxe.inf (100%)
> 
> Better place them in OvmfPkg/IntelTdx ?
> 
> Otherwise looks fine to me.
> 
> take care,
>   Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117887): https://edk2.groups.io/g/devel/message/117887
Mute This Topic: https://groups.io/mt/105531957/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 2/4] uefi-sct/SctPkg: TCG2 Protocol: use OFFSET_OF for computing offsets

[Stuart Yoder]

Use compiler-independent OFFSET_OF macro defined from Base.h instead
of the GCC specific __builtin_offsetof

Signed-off-by: Stuart Yoder 
---
 
uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTestConformance.c
 | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTestConformance.c
 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTestConformance.c
index 0122458a89fe..cba1ec1b9e2c 100644
--- 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTestConformance.c
+++ 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTestConformance.c
@@ -25,8 +25,7 @@ Abstract:
 --*/
 
 #include "TCG2ProtocolBBTest.h"
-
-#define offsetof(st, m) __builtin_offsetof(st, m)
+#include 
 
 /**
  *  @brief Entrypoint for GetCapability() Function Test.
@@ -475,7 +474,7 @@ BBTestGetCapabilityConformanceTestCheckpoint4 (
 
   // set size of struct to be up to and including the ManufacturerID
   // (this acts like a client with a 1.0 version of the struct)
-  BootServiceCap.Size = offsetof(EFI_TCG2_BOOT_SERVICE_CAPABILITY, 
NumberOfPcrBanks);
+  BootServiceCap.Size = OFFSET_OF(EFI_TCG2_BOOT_SERVICE_CAPABILITY, 
NumberOfPcrBanks);
 
   Status = TCG2->GetCapability (
TCG2,
@@ -494,7 +493,7 @@ BBTestGetCapabilityConformanceTestCheckpoint4 (
   }
 
   // Verify returned Size equals the size of EFI_TCG2_BOOT_SERVICE_CAPABILITY 
up to and including the ManufacturerID field.
-  if (BootServiceCap.Size != offsetof(EFI_TCG2_BOOT_SERVICE_CAPABILITY, 
NumberOfPcrBanks)) {
+  if (BootServiceCap.Size != OFFSET_OF(EFI_TCG2_BOOT_SERVICE_CAPABILITY, 
NumberOfPcrBanks)) {
 StandardLib->RecordMessage (
  StandardLib,
  EFI_VERBOSE_LEVEL_DEFAULT,
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117884): https://edk2.groups.io/g/devel/message/117884
Mute This Topic: https://groups.io/mt/105558007/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 3/4] uefi-sct/SctPkg: TCG2 Protocol: #pragma pack cleanup

[Stuart Yoder]

Fix compiler warning by adding #pragma pack() to close a pragma
section.  Also delete extraneous #pragma pack(1).

Signed-off-by: Stuart Yoder 
Reviewed-by: Heinrich Schuchardt 
---
 
uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h
 | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h
 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h
index deba13f21804..95307b7fa50f 100644
--- 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h
+++ 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h
@@ -79,7 +79,6 @@ typedef struct {
   UINT8  buffer[TEST_STRING_LEN];
 } TPM2B_MAX_BUFFER;
 
-#pragma pack(1)
 // TPM2B_DIGEST as defined in Table 73 of TPM Library Spec Part 2: Structures
 typedef struct {
   UINT16 size;
@@ -110,7 +109,7 @@ typedef struct {
   TPM2B_DIGEST data;
   TPMT_TK_HASHCHECK validation;
 } TPM2_HASH_RESPONSE;
-#pragma
+#pragma pack()
 
 EFI_STATUS
 EFIAPI
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117885): https://edk2.groups.io/g/devel/message/117885
Mute This Topic: https://groups.io/mt/105558008/21656
Mute #pragma:https://edk2.groups.io/g/devel/mutehashtag/pragma
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 4/4] uefi-sct/SctPkg: TCG2 Protocol: clean up type conversion warnings

[Stuart Yoder]

The VS2015x86 build encountered errors due to type conversion
warnings.  Resolve these by adding casts and refactoring.

Signed-off-by: Stuart Yoder 
---
 
uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTestConformance.c
 | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTestConformance.c
 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTestConformance.c
index cba1ec1b9e2c..2a3cd6aa4f23 100644
--- 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTestConformance.c
+++ 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTestConformance.c
@@ -639,7 +639,7 @@ BBTestHashLogExtendEventConformanceTestCheckpoint1 (
   EFI_TCG2_EVENT*EfiTcgEvent;
   const CHAR8   *EventData = "TCG2 Protocol Test";
   const CHAR8   *Str = "The quick brown fox jumps over 
the lazy dog";
-  UINT32EfiTcgEventSize = 
sizeof(EFI_TCG2_EVENT) + SctAsciiStrLen(EventData);
+  UINT32EfiTcgEventSize = 
(UINT32)(sizeof(EFI_TCG2_EVENT) + SctAsciiStrLen(EventData));
 
   DataToHash =  (EFI_PHYSICAL_ADDRESS)Str;
   DataToHashLen = SctAsciiStrLen(Str);
@@ -654,7 +654,7 @@ BBTestHashLogExtendEventConformanceTestCheckpoint1 (
   EfiTcgEvent->Header.HeaderVersion = 1;
   EfiTcgEvent->Header.EventType = EV_POST_CODE;
   EfiTcgEvent->Header.PCRIndex = 16;
-  EfiTcgEvent->Size = EfiTcgEvent->Header.HeaderSize + 
SctAsciiStrLen(EventData);
+  EfiTcgEvent->Size = EfiTcgEvent->Header.HeaderSize + 
(UINT32)SctAsciiStrLen(EventData);
 
   // Ensure HashLogExtendEvent returns Invalid Parameter when passing in NULL 
DataToHash pointer
   // EFI Protocol Spec Section 6.6.5 #1
@@ -710,7 +710,7 @@ BBTestHashLogExtendEventConformanceTestCheckpoint1 (
 
   // Ensure HashLogExtendEvent returns Invalid Parameter when passed in 
EventSize < HeaderSize + sizeof(UINT32)
   // EFI Protocol Spec Section 6.6.5 #2
-  EfiTcgEvent->Size = EfiTcgEvent->Header.HeaderSize + sizeof(UINT32) - 1;
+  EfiTcgEvent->Size = EfiTcgEvent->Header.HeaderSize + (UINT32)sizeof(UINT32) 
- 1;
 
   Status = TCG2->HashLogExtendEvent (
TCG2,
@@ -739,7 +739,7 @@ BBTestHashLogExtendEventConformanceTestCheckpoint1 (
   // Ensure HashLogExtendEvent returns Invalid Parameter when passing in PCR 
Index > 23
   // EFI Protocol Spec Section 6.6.5 #3
   EfiTcgEvent->Header.PCRIndex = 24;
-  EfiTcgEvent->Size = EfiTcgEvent->Header.HeaderSize + 
SctAsciiStrLen(EventData);
+  EfiTcgEvent->Size = EfiTcgEvent->Header.HeaderSize + 
(UINT32)SctAsciiStrLen(EventData);
 
   Status = TCG2->HashLogExtendEvent (
TCG2,
@@ -782,7 +782,7 @@ BBTestHashLogExtendEventConformanceTestCheckpoint2 (
   UINT64DataToHashLen;
   const CHAR8   *Str = "The quick brown fox jumps over 
the lazy dog";
   const CHAR8   *EventData = "TCG2 Protocol Test";
-  UINT32 EfiTcgEventSize = sizeof(EFI_TCG2_EVENT) + SctAsciiStrLen(EventData);
+  UINT32 EfiTcgEventSize = (UINT32)(sizeof(EFI_TCG2_EVENT) + 
SctAsciiStrLen(EventData));
 
   DataToHash = (EFI_PHYSICAL_ADDRESS)Str;
   DataToHashLen = SctAsciiStrLen(Str);
@@ -797,7 +797,7 @@ BBTestHashLogExtendEventConformanceTestCheckpoint2 (
   EfiTcgEvent->Header.HeaderVersion = 1;
   EfiTcgEvent->Header.EventType = EV_POST_CODE;
   EfiTcgEvent->Header.PCRIndex = 16;
-  EfiTcgEvent->Size = EfiTcgEvent->Header.HeaderSize + 
SctAsciiStrLen(EventData);
+  EfiTcgEvent->Size = EfiTcgEvent->Header.HeaderSize + 
(UINT32)SctAsciiStrLen(EventData);
 
   // Perform HashLogExtendEvent over test buffer to PCR 16
   Status = TCG2->HashLogExtendEvent (
@@ -991,9 +991,7 @@ BBTestGetEventLogConformanceTestCheckpoint2 (
   }
 
   // Verify EventLog Signature
-  Status = SctCompareMem(EventLogHeaderSpecEvent->signature, signature, 
sizeof(signature));
-
-  if (Status != EFI_SUCCESS) {
+  if (SctCompareMem(EventLogHeaderSpecEvent->signature, signature, 
sizeof(signature) != 0)) {
 StandardLib->RecordMessage (
  StandardLib,
  EFI_VERBOSE_LEVEL_DEFAULT,
@@ -1076,7 +1074,7 @@ BBTestSubmitCommandConformanceTestCheckpoint1 (
   CommandInput.Tag = SctSwapBytes16(ST_NO_SESSIONS);
   CommandInput.CommandSize = SctSwapBytes32(sizeof(TPM2_HASH_COMMAND));
   CommandInput.CommandCode = SctSwapBytes32(TPM_CC_Hash);
-  CommandInput.data.size = SctSwapBytes16(SctAsciiStrLen(Str));
+  CommandInput.data.size = SctSwapBytes16((UINT16)SctAsciiStrLen(Str));
   SctAsciiStrCpy((CHAR8 *)CommandInput.data.buffer, Str);
   CommandInput.hashAlg = SctSwapBytes16(TPM_ALG_SHA256);
   CommandInput.hierarchy = SctSwapBytes32(TPM_RH_NULL);
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages 

[edk2-devel] [PATCH v2 0/4] TCG2 protocol clean up

[Stuart Yoder]

This patch series cleans up some issues found when building edk2-test with
a non-GCC compiler:
  -TPMT_HA struct had an error due to incorrect use of C flexible array member
  -compute struct member offsets using OFFSET_OF, which is not GCC specific
  -clean up of #pragma pack in one file
  -resolve type conversion warnings

Patches are in github here:
https://github.com/stuyod01/edk2-test/tree/tcg2-cleanup

Version 2
  -add SM3 hash type to TPM2.h
  -resolve type conversion warnings

Stuart Yoder (4):
  uefi-sct/SctPkg: TCG2 Protocol: correct definition of TPMT_HA struct
  uefi-sct/SctPkg: TCG2 Protocol: use OFFSET_OF for computing offsets
  uefi-sct/SctPkg: TCG2 Protocol: #pragma pack cleanup
  uefi-sct/SctPkg: TCG2 Protocol: clean up type conversion warnings

 
uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h
|  3 +--
 uefi-sct/SctPkg/UEFI/Protocol/TCG2.h   
  | 17 +++--
 
uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTestConformance.c
 | 25 +---
 3 files changed, 27 insertions(+), 18 deletions(-)

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117882): https://edk2.groups.io/g/devel/message/117882
Mute This Topic: https://groups.io/mt/105558005/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 1/4] uefi-sct/SctPkg: TCG2 Protocol: correct definition of TPMT_HA struct

[Stuart Yoder]

The TPMT_HA struct defining event log hash algorithms was cut/pasted
from the TCG EFI Protocol specification which used a C struct
with a flexible array member as the last element.  This is incorrect
because TPMT_HA itself is used as an array element, and thus can't
be variable size.

Because the size of hash algorithms varies, this should have been
defined as a union of the sizes of supported hash algorithms.  This is
how is it done in the TPM Library specfication and in EDK2.

In addition, added the SM3 hash type to align with the TPM Library
spec.

Signed-off-by: Stuart Yoder 
---
 uefi-sct/SctPkg/UEFI/Protocol/TCG2.h | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/uefi-sct/SctPkg/UEFI/Protocol/TCG2.h 
b/uefi-sct/SctPkg/UEFI/Protocol/TCG2.h
index a83a84c33134..01ef81be8793 100644
--- a/uefi-sct/SctPkg/UEFI/Protocol/TCG2.h
+++ b/uefi-sct/SctPkg/UEFI/Protocol/TCG2.h
@@ -50,7 +50,12 @@ Abstract:
 
 #define EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 0x0002
 
-#define HASH_NUMBER 0x04
+#define HASH_NUMBER 0x05
+#define SHA1_DIGEST_SIZE20
+#define SHA256_DIGEST_SIZE  32
+#define SHA384_DIGEST_SIZE  48
+#define SHA512_DIGEST_SIZE  64
+#define SM3_256_DIGEST_SIZE 32
 
 typedef struct _EFI_TCG2_PROTOCOL EFI_TCG2_PROTOCOL;
 
@@ -117,9 +122,17 @@ typedef struct tdEFI_TCG2_EVENT {
   UINT8 Event[];
 } EFI_TCG2_EVENT;
 
+typedef union {
+  UINT8 sha1[SHA1_DIGEST_SIZE];
+  UINT8 sha256[SHA256_DIGEST_SIZE];
+  UINT8 sm3_256[SM3_256_DIGEST_SIZE];
+  UINT8 sha384[SHA384_DIGEST_SIZE];
+  UINT8 sha512[SHA512_DIGEST_SIZE];
+} TPMU_HA;
+
 typedef struct {
   UINT16 hashAlg;
-  UINT8  digest[];
+  TPMU_HAdigest;
 } TPMT_HA;
 
 typedef struct tdTPML_DIGEST_VALUES {
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117883): https://edk2.groups.io/g/devel/message/117883
Mute This Topic: https://groups.io/mt/105558006/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Cancelled Event: TianoCore Bug Triage - APAC / NAMO - Tuesday, April 16, 2024 #cal-cancelled

[Group Notification]

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Groups.io Inc//Groups.io Calendar//EN
METHOD:CANCEL
REFRESH-INTERVAL;VALUE=DURATION:PT1H
X-PUBLISHED-TTL:PT1H
CALSCALE:GREGORIAN
BEGIN:VTIMEZONE
TZID:America/Los_Angeles
LAST-MODIFIED:20231222T233358Z
TZURL:https://www.tzurl.org/zoneinfo-outlook/America/Los_Angeles
X-LIC-LOCATION:America/Los_Angeles
BEGIN:DAYLIGHT
TZNAME:PDT
TZOFFSETFROM:-0800
TZOFFSETTO:-0700
DTSTART:19700308T02
RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=2SU
END:DAYLIGHT
BEGIN:STANDARD
TZNAME:PST
TZOFFSETFROM:-0700
TZOFFSETTO:-0800
DTSTART:19701101T02
RRULE:FREQ=YEARLY;BYMONTH=11;BYDAY=1SU
END:STANDARD
END:VTIMEZONE
BEGIN:VEVENT
X-GIOIDS:Event:2159781 
UID:mlda.1580078539586725120.r...@groups.io
DTSTAMP:20240416T134939Z
ORGANIZER;CN=Liming Gao;SENT-BY="mailto:gaolim...@byosoft.com.cn":mailto:
 gaolim...@byosoft.com.cn
DTSTART:20240417T013000Z
DTEND:20240417T023000Z
SUMMARY:TianoCore Bug Triage - APAC / NAMO
DESCRIPTION:TianoCore Bug Triage - APAC / NAMO\n\nHosted by Liming Gao\n\
 n
 \n\nMicrosoft Teams meeting\n\n*Join on your computer or mobile a
 pp*\n\nClick here to join the meeting ( https://teams.microsoft.com/l/mee
 tup-join/19%3ameeting_OTk1YzJhN2UtOGQwNi00NjY4LWEwMTktY2JiODRlYTY1NmY0%40
 thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255
 d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d )\n\n*Jo
 in with a video conferencing device*\n\nte...@conf.intel.com\n\nVideo Con
 ference ID: 116 062 094 0\n\nAlternate VTC dialing instructions ( https:/
 /conf.intel.com/teams/?conf=1160620940=teams=conf.intel.com=te
 st_call )\n\n*Or call in (audio only)*\n\n+1 916-245-6934\,\,77463821# ( 
 tel:+19162456934\,\,77463821# ) United States\, Sacramento\n\nPhone Confe
 rence ID: 774 638 21#\n\nFind a local number ( https://dialin.teams.micro
 soft.com/d195d438-2daa-420e-b9ea-da26f9d1d6d5?id=77463821 ) | Reset PIN (
  https://mysettings.lync.com/pstnconferencing )\n\nLearn More ( https://a
 ka.ms/JoinTeamsMeeting ) | Meeting options ( https://teams.microsoft.com/
 meetingOptions/?organizerId=b286b53a-1218-4db3-bfc9-3d4c5aa7669e
 =46c98d88-e344-4ed4-8496-4ed7712e255d=19_meeting_OTUyZTg2NjgtNDh
 lNS00ODVlLTllYTUtYzg1OTNjNjdiZjFh@thread.v2=0=en-US )
LOCATION:https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTk1YzJhN
 2UtOGQwNi00NjY4LWEwMTktY2JiODRlYTY1NmY0%40thread.v2/0?context=%7b%22Tid%2
 2%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-
 1242-431b-9a51-92cd01a5df3c%22%7d
SEQUENCE:1
STATUS:CANCELLED
END:VEVENT
END:VCALENDAR


invite.ics
Description: application/ics

[edk2-devel] DxeSmmReadyToLockProtocol interface doesn't get installed

[memristor2 via groups.io]

Hello everyone,
I am developing a platform based on MinPlatformPkg everything works fine up 
until the BDS phase.
To be more specific what's happening is that inside the BdsDxe module when the 
BdsEntry function calls the platform-dependent 
"PlatformBootManagerBeforeConsole" function (Inside MinPlatformPkg) and 
eventually the ExitPmAuth() function, the execution seems to get stuck on this 
part of the code:
Handle = NULL;
Status = gBS->InstallProtocolInterface (
,
,
EFI_NATIVE_INTERFACE,
NULL );
I do not have a single clue why this is happening especially since it doesn't 
return and simply hangs. The only thing that would come to my mind would be 
that its dependencies aren't met which is not true in this case since other 
drivers also have this depex and get dispatched with no problem and if this was 
the case the module wouldn't even execute at the first place.
Any ideas are extremely appreciated.
Thanks,
Memristor

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117879): https://edk2.groups.io/g/devel/message/117879
Mute This Topic: https://groups.io/mt/10635/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 03/13] UefiCpuPkg/SmmRelocationLib: Add library instance for OVMF

[Wu, Jiaxin]

Hi Gerd,

> > I don't the background why AMD and OVMF has such difference. Maybe
> OVFM doesn't not support the MSR "EFER_ADDRESS".
> 
> It surely does, it's a rather essential MSR for x64 CPUs.
> 
> > > The SmBase configuration for OVMF looks suspicious to me.  I'm
> wondering
> > > whenever the OVMF code actually works in Ia32 builds ...
> 
> Tested OvmfPkg/OvmfPkgIa32.dsc with SMM_REQUIRE == TRUE (master
> branch).
> Doesn't boot.  The difference above (where the OVMF code does not
> consider the 32bit case) could very well explain why 32bit support is
> broken.  Switching to the AMD code might actually fix that.
> 
> Is the SmmRelocationLib approach supposed to work with mixed mode
> firmware where PEI is running in ia32 mode and dxe/smm is running
> in x64 mode (i.e. OvmfPkg/OvmfPkgIa32X64.dsc)?
> 

Yes, I passed the test on the both OvmfPkgIa32X64 & OvmfPkgX64 for SMM support.

It does has the problem on OvmfPkgIa32 for smm support (same as master). I did 
quick check, it's not only the CpuSaveState->x86 or CpuSaveState->x64 structure 
issue, but also has some problem to handler the smi hook return. We can handle 
this problem in another topic. 

Thanks,
Jiaxin

 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117878): https://edk2.groups.io/g/devel/message/117878
Mute This Topic: https://groups.io/mt/105441992/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 03/13] UefiCpuPkg/SmmRelocationLib: Add library instance for OVMF

[Gerd Hoffmann]

  Hi,

> > > 2) Existing SmBase configuration is different between the AMD & OVMF.
> > > OVMF:
> > >  AmdCpuState->x64.SMBASE = (UINT32)mSmBaseForAllCpus[CpuIndex];
> > >
> > > AMD:
> > >  if ((CpuSaveState->x86.SMMRevId & 0x) == 0) {
> > > CpuSaveState->x86.SMBASE = (UINT32)mSmBaseForAllCpus[CpuIndex];
> > >   } else {
> > > CpuSaveState->x64.SMBASE = (UINT32)mSmBaseForAllCpus[CpuIndex];
> > >   }

> I don't the background why AMD and OVMF has such difference. Maybe OVFM 
> doesn't not support the MSR "EFER_ADDRESS".

It surely does, it's a rather essential MSR for x64 CPUs.

> > The SmBase configuration for OVMF looks suspicious to me.  I'm wondering
> > whenever the OVMF code actually works in Ia32 builds ...

Tested OvmfPkg/OvmfPkgIa32.dsc with SMM_REQUIRE == TRUE (master branch).
Doesn't boot.  The difference above (where the OVMF code does not
consider the 32bit case) could very well explain why 32bit support is
broken.  Switching to the AMD code might actually fix that.

Is the SmmRelocationLib approach supposed to work with mixed mode
firmware where PEI is running in ia32 mode and dxe/smm is running
in x64 mode (i.e. OvmfPkg/OvmfPkgIa32X64.dsc)?

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117877): https://edk2.groups.io/g/devel/message/117877
Mute This Topic: https://groups.io/mt/105441992/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 04/13] UefiCpuPkg/SmmRelocationLib: Add library instance for AMD

[Abdul Lateef Attar via groups.io]

Reviewed-by: Abdul Lateef Attar 

On 10-04-2024 19:27, Jiaxin Wu wrote:

Caution: This message originated from an External Source. Use proper caution 
when opening attachments, clicking links, or responding.


Due to the definition difference of SMRAM Save State,
SmmBase config in SMRAM Save State for AMD is also different.

This patch provides the AmdSmmRelocationLib library instance
to handle the SMRAM Save State difference.

Cc: Abdul Lateef Attar
Cc: Abner Chang
Cc: Ray Ni
Cc: Zeng Star
Cc: Gerd Hoffmann
Cc: Rahul Kumar
Signed-off-by: Jiaxin Wu
---
  .../SmmRelocationLib/AmdSmmRelocationLib.inf   |  61 
  .../SmmRelocationLib/AmdSmramSaveStateConfig.c | 109 +
  2 files changed, 170 insertions(+)
  create mode 100644 UefiCpuPkg/Library/SmmRelocationLib/AmdSmmRelocationLib.inf
  create mode 100644 
UefiCpuPkg/Library/SmmRelocationLib/AmdSmramSaveStateConfig.c

diff --git a/UefiCpuPkg/Library/SmmRelocationLib/AmdSmmRelocationLib.inf 
b/UefiCpuPkg/Library/SmmRelocationLib/AmdSmmRelocationLib.inf
new file mode 100644
index 00..710cd1948b
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmRelocationLib/AmdSmmRelocationLib.inf
@@ -0,0 +1,61 @@
+## @file
+# SMM Relocation Lib for each processor.
+#
+# This Lib produces the SMM_BASE_HOB in HOB database which tells
+# the PiSmmCpuDxeSmm driver (runs at a later phase) about the new
+# SMBASE for each processor. PiSmmCpuDxeSmm driver installs the
+# SMI handler at the SMM_BASE_HOB.SmBase[Index]+0x8000 for processor
+# Index.
+#
+# Copyright (c) 2024, Intel Corporation. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = SmmRelocationLib
+  FILE_GUID  = 65C74DCD-0D09-494A-8BFF-A64226EB8054
+  MODULE_TYPE= PEIM
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = SmmRelocationLib
+
+[Sources]
+  InternalSmmRelocationLib.h
+  AmdSmramSaveStateConfig.c
+  SmmRelocationLib.c
+
+[Sources.Ia32]
+  Ia32/Semaphore.c
+  Ia32/SmmInit.nasm
+
+[Sources.X64]
+  X64/Semaphore.c
+  X64/SmmInit.nasm
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  CpuExceptionHandlerLib
+  DebugLib
+  HobLib
+  LocalApicLib
+  MemoryAllocationLib
+  PcdLib
+  PeiServicesLib
+
+[Guids]
+  gSmmBaseHobGuid   ## HOB ALWAYS_PRODUCED
+  gEfiSmmSmramMemoryGuid## CONSUMES
+
+[Pcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize ## CONSUMES
+
+[FeaturePcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugSupport## 
CONSUMES
diff --git a/UefiCpuPkg/Library/SmmRelocationLib/AmdSmramSaveStateConfig.c 
b/UefiCpuPkg/Library/SmmRelocationLib/AmdSmramSaveStateConfig.c
new file mode 100644
index 00..fbcf347f9b
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmRelocationLib/AmdSmramSaveStateConfig.c
@@ -0,0 +1,109 @@
+/** @file
+  Config SMRAM Save State for SmmBases Relocation.
+
+  Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.
+  Copyright (c) 2024, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#include "InternalSmmRelocationLib.h"
+#include 
+
+/**
+  This function configures the SmBase on the currently executing CPU.
+
+  @param[in] CpuIndex The index of the CPU.
+  @param[in,out] CpuState Pointer to SMRAM Save State Map for the
+  currently executing CPU. On out, SmBase 
is
+  updated to the new value.
+
+**/
+VOID
+EFIAPI
+ConfigureSmBase (
+  IN UINTN CpuIndex,
+  IN OUT SMRAM_SAVE_STATE_MAP  *CpuState
+  )
+{
+  AMD_SMRAM_SAVE_STATE_MAP  *AmdCpuState;
+
+  AmdCpuState = (AMD_SMRAM_SAVE_STATE_MAP *)CpuState;
+
+  if (mSmmSaveStateRegisterLma == EFI_MM_SAVE_STATE_REGISTER_LMA_32BIT) {
+AmdCpuState->x86.SMBASE = (UINT32)mSmBaseForAllCpus[CpuIndex];
+  } else {
+AmdCpuState->x64.SMBASE = (UINT32)mSmBaseForAllCpus[CpuIndex];
+  }
+}
+
+/**
+  This function updates the SMRAM save state on the currently executing CPU
+  to resume execution at a specific address after an RSM instruction.  This
+  function must evaluate the SMRAM save state to determine the execution mode
+  the RSM instruction resumes and update the resume execution address with
+  either NewInstructionPointer32 or NewInstructionPoint.  The auto HALT restart
+  flag in the SMRAM save state must always be cleared.  This function returns
+  the value of the instruction pointer from the SMRAM save state that was
+  replaced.  If this function returns 0, then the SMRAM save state was not
+  modified.
+
+  This function is called during the very first SMI on each CPU after
+  

Re: [edk2-devel] [PATCH V1 0/5] Move Tdx specific lib from SecurityPkg to OvmfPkg

[Gerd Hoffmann]

On Mon, Apr 15, 2024 at 03:55:49PM +0800, Min Xu wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4752
> 
> HashLibTdx and TdTcg2Dxe are designed for Intel TDX enlightened OVMF.
> They're more reasonable to be put in OvmfPkg than in SecurityPkg.
> 
> SecTpmMeasurementLibTdx is not used anymore. So it is deleted in this
> patch-set.
> 

>  rename {SecurityPkg => OvmfPkg}/Library/HashLibTdx/HashLibTdx.c (100%)
>  rename {SecurityPkg => OvmfPkg}/Library/HashLibTdx/HashLibTdx.inf (100%)
>  rename {SecurityPkg => OvmfPkg}/Tcg/TdTcg2Dxe/MeasureBootPeCoff.c (100%)
>  rename {SecurityPkg => OvmfPkg}/Tcg/TdTcg2Dxe/TdTcg2Dxe.c (100%)
>  rename {SecurityPkg => OvmfPkg}/Tcg/TdTcg2Dxe/TdTcg2Dxe.inf (100%)

Better place them in OvmfPkg/IntelTdx ?

Otherwise looks fine to me.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117875): https://edk2.groups.io/g/devel/message/117875
Mute This Topic: https://groups.io/mt/105531957/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 03/13] UefiCpuPkg/SmmRelocationLib: Add library instance for OVMF

[Wu, Jiaxin]

> 
> On Mon, Apr 15, 2024 at 01:04:58PM +, Wu, Jiaxin wrote:
> > Hi Gred,
> >
> > Because:
> > 1) The mode of the CPU check is different between the AMD & OVMF.
> > OVMF:
> > CpuSaveState->x86.SMMRevId & 0X
> >
> > AMD:
> >  LMAValue = (UINT32)AsmReadMsr64 (EFER_ADDRESS) & LMA
> >
> > 2) Existing SmBase configuration is different between the AMD & OVMF.
> > OVMF:
> >  AmdCpuState->x64.SMBASE = (UINT32)mSmBaseForAllCpus[CpuIndex];
> >
> > AMD:
> >  if ((CpuSaveState->x86.SMMRevId & 0x) == 0) {
> > CpuSaveState->x86.SMBASE = (UINT32)mSmBaseForAllCpus[CpuIndex];
> >   } else {
> > CpuSaveState->x64.SMBASE = (UINT32)mSmBaseForAllCpus[CpuIndex];
> >   }
> >
> > This series patch won't change the existing implementation code logic, so, 
> > we
> need override one version for OVMF.
> 
> The real question is why do these differences exist and are they
> actually needed.
> 
> I'd expect the CPU mode check return identical results.

I don't the background why AMD and OVMF has such difference. Maybe OVFM doesn't 
not support the MSR "EFER_ADDRESS".

> 
> The SmBase configuration for OVMF looks suspicious to me.  I'm wondering
> whenever the OVMF code actually works in Ia32 builds ...
> 

Yes, I will fix it in the next version. Currently, please build it with 
SMM_REQUIRE == TRUE. In the next version, I will make sure all pass the check.

> take care,
>   Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117874): https://edk2.groups.io/g/devel/message/117874
Mute This Topic: https://groups.io/mt/105441992/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 00/10] Add SmmRelocationLib

[Wu, Jiaxin]

Yes, because I moved the SmmRelocationLib within the condition of SMM_REQUIRE 
== TRUE in the version 2 patch. It should be always included since the 
PlatformPei need consume it.

I will fix it in the next version patch. It won't impact the code review & test 
with SMM_REQUIRE == TRUE build.

Thanks,
Jiaxin 


> -Original Message-
> From: Gerd Hoffmann 
> Sent: Tuesday, April 16, 2024 3:31 PM
> To: devel@edk2.groups.io; Wu, Jiaxin 
> Cc: Ni, Ray ; Zeng, Star ; Kumar,
> Rahul R ; Dong, Guo ;
> Rhodes, Sean ; Lu, James ;
> Guo, Gua ; Ard Biesheuvel
> ; Yao, Jiewen ; Abdul
> Lateef Attar ; Abner Chang
> ; Tom Lendacky 
> Subject: Re: [edk2-devel] [PATCH v2 00/10] Add SmmRelocationLib
> 
> On Mon, Apr 15, 2024 at 09:30:11PM +0800, Wu, Jiaxin wrote:
> > Intel plans to separate the smbase relocation logic from
> > PiSmmCpuDxeSmm driver, and the related behavior will be
> > moved to the new interface defined by the SmmRelocationLib
> > class.
> >
> > The SmmRelocationLib class provides the SmmRelocationInit()
> > interface for platform to do the smbase relocation, which
> > shall provide below 2 functionalities:
> > 1. Relocate smbases for each processor.
> > 2. Create the gSmmBaseHobGuid HOB.
> >
> > With SmmRelocationLib, PiSmmCpuDxeSmm driver (which runs at
> > a later phase) can be simplfied as below for SMM init:
> > 1. Consume the gSmmBaseHobGuid HOB for the relocated smbases
> > for each Processor.
> > 2. Execute the early SMM Init.
> 
> How this was tested?
> I can't even build this (without -D SMM_REQUIRE=TRUE).
> 
> /home/kraxel/projects/edk2/OvmfPkg/OvmfPkgX64.dsc(...): error 4000:
> Instance of library class [SmmRelocationLib] is not found
>   in
> [/home/kraxel/projects/edk2/OvmfPkg/PlatformPei/PlatformPei.inf] [X64]
>   consumed by module
> [/home/kraxel/projects/edk2/OvmfPkg/PlatformPei/PlatformPei.inf]
> 
> I doubt it passes CI.
> 
> take care,
>   Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117873): https://edk2.groups.io/g/devel/message/117873
Mute This Topic: https://groups.io/mt/105535804/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/9] Add DeviceSecurity feature based on PFP 1.06 spec

[Yao, Jiewen]

Hi Wenxing
I just realized that this libspdm submodule does NOT use the latest tag.

Since DMTF release 3.3.0 for libspdm 
https://github.com/DMTF/libspdm/releases/tag/3.3.0, I recommend we update to 
the latest one.

Thank you
Yao, Jiewen

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Yao, Jiewen
> Sent: Tuesday, April 16, 2024 5:26 PM
> To: Hou, Wenxing ; Kinney, Michael D
> ; devel@edk2.groups.io
> Cc: Sean Brogan ; Joey Vagedes
> ; Liming Gao ; Andrew
> Fish ; Liu, Zhiguang ; Kumar, Rahul R
> 
> Subject: Re: [edk2-devel] [PATCH 0/9] Add DeviceSecurity feature based on PFP
> 1.06 spec
> 
> Reviewed-by: Jiewen Yao 
> 
> > -Original Message-
> > From: Hou, Wenxing 
> > Sent: Monday, April 15, 2024 10:08 AM
> > To: Kinney, Michael D ; devel@edk2.groups.io
> > Cc: Sean Brogan ; Joey Vagedes
> > ; Liming Gao ; Andrew
> > Fish ; Liu, Zhiguang ; Kumar, Rahul
> R
> > ; Yao, Jiewen 
> > Subject: RE: [PATCH 0/9] Add DeviceSecurity feature based on PFP 1.06 spec
> >
> > Hi Mike,
> >
> > I have submitted PATCH v3, which updated the Readme.rst for libspdm
> submodule
> > license.  And I have added Leif.
> > Please review the PATCH v3.
> >
> > For your second feedback, I have investigate the situation.
> >
> > If we use 'git submodule update --init' to clone the submodule, the
> > mbedtls/openssl/cmocka in libspdm will not  be cloned due to the absence of
> the
> > '--recursive' option.
> > And it will not affect the build and use of DeviceSecurity.
> >
> >
> > Thanks,
> > Wenxing
> >
> >
> > -Original Message-
> > From: Kinney, Michael D 
> > Sent: Tuesday, April 9, 2024 11:14 PM
> > To: Hou, Wenxing ; devel@edk2.groups.io
> > Cc: Sean Brogan ; Joey Vagedes
> > ; Liming Gao ; Andrew
> > Fish ; Liu, Zhiguang ; Kumar, Rahul
> R
> > ; Yao, Jiewen ; Kinney,
> > Michael D 
> > Subject: RE: [PATCH 0/9] Add DeviceSecurity feature based on PFP 1.06 spec
> >
> > +Leif
> >
> > Adding a new submodule requires review by the stewards to review the license
> > and the health and support of the submodule project.
> >
> > The top level Readme also requires updates.  It lists all the submodules and
> > licenses used. Please update this series with the Readme changes.
> >
> > https://github.com/tianocore/edk2?tab=readme-ov-file#license-details
> >
> > I also notice that libspdm has its own .gitmodules file that pulls in more
> > submodules.
> >
> > [submodule "os_stub/openssllib/openssl"]
> > path = os_stub/openssllib/openssl
> > url = https://github.com/openssl/openssl
> > [submodule "os_stub/mbedtlslib/mbedtls"]
> > path = os_stub/mbedtlslib/mbedtls
> > url = https://github.com/ARMmbed/mbedtls
> > [submodule "unit_test/cmockalib/cmocka"]
> > path = unit_test/cmockalib/cmocka
> > url = https://git.cryptomilk.org/projects/cmocka.git
> >
> >
> > edk2 already had openssl and mbedtls as submodules, does this mean that
> > openssl and mbedtls will be cloned twice in 2 different locations now?
> >
> > The edk2 project had issues with the stability of the cmocka server and 
> > changed
> > to a tianocore mirror of the cmocka submodule to improve CI stability. This 
> > is
> > another submodule that will be cloned twice and may reintroduce the 
> > potential
> > for CI stability issues.
> >
> > Thanks,
> >
> > Mike
> >
> > > -Original Message-
> > > From: Hou, Wenxing 
> > > Sent: Monday, April 1, 2024 7:31 PM
> > > To: devel@edk2.groups.io
> > > Cc: Sean Brogan ; Joey Vagedes
> > > ; Kinney, Michael D
> > > ; Liming Gao ;
> > > Andrew Fish ; Liu, Zhiguang ;
> > > Kumar, Rahul R ; Yao, Jiewen
> > > 
> > > Subject: [PATCH 0/9] Add DeviceSecurity feature based on PFP 1.06 spec
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2479
> > >
> > > In PFP spec 1.06, platform firmware records the device certificate and
> > > device measurement for each SPDM responder.
> > > This PATCH set implement the DeviceSecurityLib to support spdm device
> > > Authentication and Measurement.
> > >
> > > Libspdm as submodule is to support DeviceSecurity feature:
> > > https://github.com/DMTF/libspdm
> > >
> > > TCG PFP spec 1.06:
> > > https://trustedcomputinggroup.org/resource/pc-client-specific-
> > > platform-firmware-profile-specification/
> > >
> > > The POC branch:
> > > https://github.com/tianocore/edk2-staging/tree/DeviceSecurity
> > >
> > > And the PATCH set has passed the EDKII CI:
> > > https://github.com/tianocore/edk2/pull/5508
> > >
> > > Cc: Sean Brogan 
> > > Cc: Joey Vagedes 
> > > Cc: Michael D Kinney 
> > > Cc: Liming Gao 
> > > Cc: Andrew Fish 
> > > Cc: Zhiguang Liu 
> > > Cc: Rahul Kumar 
> > > Cc: Jiewen Yao 
> > > Signed-off-by: Wenxing Hou 
> > >
> > > Wenxing Hou (9):
> > >   MdePkg: Add SPDM1.2 support.
> > >   MdePkg: Add TCG PFP 1.06 support.
> > >   MdePkg: Add devAuthBoot GlobalVariable
> > >   MdeModulePkg/Variable: Add TCG SPDM device measurement update
> > >   SecurityPkg: Add TCG PFP 1.06 support.
> > >   SecurityPkg: add DeviceSecurity 

Re: [edk2-devel] [PATCH 0/9] Add DeviceSecurity feature based on PFP 1.06 spec

[Yao, Jiewen]

Reviewed-by: Jiewen Yao 

> -Original Message-
> From: Hou, Wenxing 
> Sent: Monday, April 15, 2024 10:08 AM
> To: Kinney, Michael D ; devel@edk2.groups.io
> Cc: Sean Brogan ; Joey Vagedes
> ; Liming Gao ; Andrew
> Fish ; Liu, Zhiguang ; Kumar, Rahul R
> ; Yao, Jiewen 
> Subject: RE: [PATCH 0/9] Add DeviceSecurity feature based on PFP 1.06 spec
> 
> Hi Mike,
> 
> I have submitted PATCH v3, which updated the Readme.rst for libspdm submodule
> license.  And I have added Leif.
> Please review the PATCH v3.
> 
> For your second feedback, I have investigate the situation.
> 
> If we use 'git submodule update --init' to clone the submodule, the
> mbedtls/openssl/cmocka in libspdm will not  be cloned due to the absence of 
> the
> '--recursive' option.
> And it will not affect the build and use of DeviceSecurity.
> 
> 
> Thanks,
> Wenxing
> 
> 
> -Original Message-
> From: Kinney, Michael D 
> Sent: Tuesday, April 9, 2024 11:14 PM
> To: Hou, Wenxing ; devel@edk2.groups.io
> Cc: Sean Brogan ; Joey Vagedes
> ; Liming Gao ; Andrew
> Fish ; Liu, Zhiguang ; Kumar, Rahul R
> ; Yao, Jiewen ; Kinney,
> Michael D 
> Subject: RE: [PATCH 0/9] Add DeviceSecurity feature based on PFP 1.06 spec
> 
> +Leif
> 
> Adding a new submodule requires review by the stewards to review the license
> and the health and support of the submodule project.
> 
> The top level Readme also requires updates.  It lists all the submodules and
> licenses used. Please update this series with the Readme changes.
> 
> https://github.com/tianocore/edk2?tab=readme-ov-file#license-details
> 
> I also notice that libspdm has its own .gitmodules file that pulls in more
> submodules.
> 
> [submodule "os_stub/openssllib/openssl"]
> path = os_stub/openssllib/openssl
> url = https://github.com/openssl/openssl
> [submodule "os_stub/mbedtlslib/mbedtls"]
> path = os_stub/mbedtlslib/mbedtls
> url = https://github.com/ARMmbed/mbedtls
> [submodule "unit_test/cmockalib/cmocka"]
> path = unit_test/cmockalib/cmocka
> url = https://git.cryptomilk.org/projects/cmocka.git
> 
> 
> edk2 already had openssl and mbedtls as submodules, does this mean that
> openssl and mbedtls will be cloned twice in 2 different locations now?
> 
> The edk2 project had issues with the stability of the cmocka server and 
> changed
> to a tianocore mirror of the cmocka submodule to improve CI stability. This is
> another submodule that will be cloned twice and may reintroduce the potential
> for CI stability issues.
> 
> Thanks,
> 
> Mike
> 
> > -Original Message-
> > From: Hou, Wenxing 
> > Sent: Monday, April 1, 2024 7:31 PM
> > To: devel@edk2.groups.io
> > Cc: Sean Brogan ; Joey Vagedes
> > ; Kinney, Michael D
> > ; Liming Gao ;
> > Andrew Fish ; Liu, Zhiguang ;
> > Kumar, Rahul R ; Yao, Jiewen
> > 
> > Subject: [PATCH 0/9] Add DeviceSecurity feature based on PFP 1.06 spec
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2479
> >
> > In PFP spec 1.06, platform firmware records the device certificate and
> > device measurement for each SPDM responder.
> > This PATCH set implement the DeviceSecurityLib to support spdm device
> > Authentication and Measurement.
> >
> > Libspdm as submodule is to support DeviceSecurity feature:
> > https://github.com/DMTF/libspdm
> >
> > TCG PFP spec 1.06:
> > https://trustedcomputinggroup.org/resource/pc-client-specific-
> > platform-firmware-profile-specification/
> >
> > The POC branch:
> > https://github.com/tianocore/edk2-staging/tree/DeviceSecurity
> >
> > And the PATCH set has passed the EDKII CI:
> > https://github.com/tianocore/edk2/pull/5508
> >
> > Cc: Sean Brogan 
> > Cc: Joey Vagedes 
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Cc: Andrew Fish 
> > Cc: Zhiguang Liu 
> > Cc: Rahul Kumar 
> > Cc: Jiewen Yao 
> > Signed-off-by: Wenxing Hou 
> >
> > Wenxing Hou (9):
> >   MdePkg: Add SPDM1.2 support.
> >   MdePkg: Add TCG PFP 1.06 support.
> >   MdePkg: Add devAuthBoot GlobalVariable
> >   MdeModulePkg/Variable: Add TCG SPDM device measurement update
> >   SecurityPkg: Add TCG PFP 1.06 support.
> >   SecurityPkg: add DeviceSecurity support
> >   .pytool/CISettings.py: add libspdm submodule.
> >   .gitmodule: Add libspdm submodule for EDKII
> >   SecurityPkg: Add libspdm submodule
> >
> >  .gitmodules   |3 +
> >  .pytool/CISettings.py |2 +
> >  MdeModulePkg/MdeModulePkg.dec |5 +
> >  .../Variable/RuntimeDxe/Measurement.c |   38 +-
> >  .../RuntimeDxe/VariableRuntimeDxe.inf |3 +
> >  .../RuntimeDxe/VariableSmmRuntimeDxe.inf  |3 +
> >  MdePkg/Include/Guid/GlobalVariable.h  |8 +-
> >  MdePkg/Include/Guid/ImageAuthentication.h |5 +-
> >  MdePkg/Include/IndustryStandard/Spdm.h| 1112
> > -
> >  .../IndustryStandard/UefiTcgPlatform.h|  186 ++-
> >  .../OsStub/CryptlibWrapper/CryptlibWrapper.c  |  970 ++
> >  

Re: [edk2-devel] [PATCH V1 0/5] Move Tdx specific lib from SecurityPkg to OvmfPkg

[Yao, Jiewen]

Reviewed-by: Jiewen Yao 

> -Original Message-
> From: Xu, Min M 
> Sent: Monday, April 15, 2024 3:59 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel ; Yao, Jiewen
> ; Gerd Hoffmann 
> Subject: RE: [PATCH V1 0/5] Move Tdx specific lib from SecurityPkg to OvmfPkg
> 
> The code is at: https://github.com/mxu9/edk2/tree/move_tdx.v1
> 
> > -Original Message-
> > From: Xu, Min M 
> > Sent: Monday, April 15, 2024 3:56 PM
> > To: devel@edk2.groups.io
> > Cc: Xu, Min M ; Ard Biesheuvel
> > ; Yao, Jiewen ; Gerd
> > Hoffmann 
> > Subject: [PATCH V1 0/5] Move Tdx specific lib from SecurityPkg to OvmfPkg
> >
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4752
> >
> > HashLibTdx and TdTcg2Dxe are designed for Intel TDX enlightened OVMF.
> > They're more reasonable to be put in OvmfPkg than in SecurityPkg.
> >
> > SecTpmMeasurementLibTdx is not used anymore. So it is deleted in this
> > patch-set.
> >
> > Cc: Ard Biesheuvel 
> > Cc: Jiewen Yao 
> > Cc: Gerd Hoffmann 
> > Signed-off-by: Min Xu 
> >
> > Min M Xu (5):
> >   Security/SecTpmMeasurementLibTdx: Delete unused
> > SecTpmMeasurementLibTdx
> >   OmvfPkg/HashLibTdx: Add HashLibTdx
> >   OvmfPkg/TdTcg2Dxe: Add TdTcg2Dxe
> >   OvmfPkg: Update TdTcg2Dxe path in OvmfPkgX64 and IntelTdxX64.dsc
> >   SecurityPkg: Delete TdTcg2Dxe and HashLibTdx in SecurityPkg
> >
> >  OvmfPkg/IntelTdx/IntelTdxX64.dsc  |   4 +-
> >  OvmfPkg/IntelTdx/IntelTdxX64.fdf  |   2 +-
> >  .../Library/HashLibTdx/HashLibTdx.c   |   0
> >  .../Library/HashLibTdx/HashLibTdx.inf |   0
> >  OvmfPkg/OvmfPkgX64.dsc|   4 +-
> >  OvmfPkg/OvmfPkgX64.fdf|   2 +-
> >  .../Tcg/TdTcg2Dxe/MeasureBootPeCoff.c |   0
> >  .../Tcg/TdTcg2Dxe/TdTcg2Dxe.c |   0
> >  .../Tcg/TdTcg2Dxe/TdTcg2Dxe.inf   |   0
> >  .../SecTpmMeasurementLibTdx.c | 175 --
> >  .../SecTpmMeasurementLibTdx.inf   |  34 
> >  SecurityPkg/SecurityPkg.dsc   |  16 --
> >  12 files changed, 6 insertions(+), 231 deletions(-)  rename {SecurityPkg =>
> > OvmfPkg}/Library/HashLibTdx/HashLibTdx.c (100%)  rename {SecurityPkg =>
> > OvmfPkg}/Library/HashLibTdx/HashLibTdx.inf (100%)  rename {SecurityPkg =>
> > OvmfPkg}/Tcg/TdTcg2Dxe/MeasureBootPeCoff.c (100%)  rename {SecurityPkg
> > => OvmfPkg}/Tcg/TdTcg2Dxe/TdTcg2Dxe.c (100%)  rename {SecurityPkg =>
> > OvmfPkg}/Tcg/TdTcg2Dxe/TdTcg2Dxe.inf (100%)  delete mode 100644
> > SecurityPkg/Library/SecTpmMeasurementLib/SecTpmMeasurementLibTdx.c
> >  delete mode 100644
> > SecurityPkg/Library/SecTpmMeasurementLib/SecTpmMeasurementLibTdx.inf
> >
> > --
> > 2.44.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117870): https://edk2.groups.io/g/devel/message/117870
Mute This Topic: https://groups.io/mt/105531957/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v5 0/2] SecurityPkg/OpalPasswordDxe: Update according to UEFI spec

[Yao, Jiewen]

Merged https://github.com/tianocore/edk2/pull/5563

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Cindy Kuo
> Sent: Tuesday, April 16, 2024 1:03 PM
> To: devel@edk2.groups.io
> Cc: Kuo, CindyX 
> Subject: [edk2-devel] [PATCH v5 0/2] SecurityPkg/OpalPasswordDxe: Update
> according to UEFI spec
> 
> For opalHii current design, it will display all NVME disks when the user 
> enters TCG
> Drive Management dynamically.
> Also, the related disk info form will be created along with the disks.
> These actions will call get/set browser to refresh the display, which is not 
> allowed
> in ACTION_FORM_OPEN callback function.
> 
> To meet UEFI 2.9 spec, a latency issue will be observed if the browser 
> callback
> action changes from ACTION_FORM_OPEN to ACTION_RETRIEVE.
> The NVNE disks will not be displayed when the user enters the formset at the 
> first
> time. Revisit the formset can see the update.
> So need to force reparsing the IFR binary when RETRIEVE.
> 
> v2:
> Format code with Uncrustify.
> 
> v3:
> Code refine based on comments from Dandan and Tina.
> 
> v4:
> Split solution into two patches as different purpose.
> 
> v5:
> Update commit message.
> 
> Cindy Kuo (2):
>   SecurityPkg/OpalPasswordDxe: Change callback action to meet UEFI spec
>   SecurityPkg/OpalPasswordDxe: Force reparsing IFR binary when RETRIEVE
> 
>  .../Tcg/Opal/OpalPassword/OpalDriver.h|  1 +
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c   | 84 ---
>  .../Tcg/Opal/OpalPassword/OpalHiiFormValues.h |  6 ++
>  .../Tcg/Opal/OpalPassword/OpalPasswordDxe.inf |  1 +
>  .../Opal/OpalPassword/OpalPasswordForm.vfr|  8 +-
>  5 files changed, 87 insertions(+), 13 deletions(-)
> 
> --
> 2.44.0.windows.1
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117869): https://edk2.groups.io/g/devel/message/117869
Mute This Topic: https://groups.io/mt/105551557/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] ACPI table generators and ConfigurationManagerProtocol

[Marcin Juszkiewicz]

W dniu 16.04.2024 o 09:38, Marcin Juszkiewicz via groups.io pisze:
And if someone considers using ConfigurationManager for their platform: 
please reconsider it as it may not be worth your time.


Wrote blog post about my adventure with CM:

https://marcin.juszkiewicz.com.pl/2024/04/16/configurationmanager-in-edk2-just-say-no/


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117868): https://edk2.groups.io/g/devel/message/117868
Mute This Topic: https://groups.io/mt/105481311/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3 00/24] Provide SEV-SNP support for running under an SVSM

[Ard Biesheuvel]

(cc'ing the stewards)

On Fri, 12 Apr 2024 at 17:14, Tom Lendacky  wrote:
>
> On 4/12/24 10:05, Ard Biesheuvel wrote:
> > On Fri, 12 Apr 2024 at 16:02, Tom Lendacky  wrote:
> >>
> >> Re-pinging the list/maintainers, again. I need reviews from the
> >> maintainers on patches #2, #7, #10, #11 and #13.
> >>
> >> Once I get final approval, I'll submit the change to edk2-platforms for
> >> the new library as a reply to this series for a quick review.
> >>
> >
> > So this is MdePkg and UefiCpuPkg, right?
>
> MdePkg and UefiPayloadPkg.
>
> >
> > @Liming, Mike, Ray: please let me know if you are ok with these
>
> Ray already reviewed/acked the UefiCpuPkg patches.
>
> Looking for
> MdePkg: Liming, Mike or Zhiguang
> UefiPayloadPkg: Gua, Guo, James or Sean
>

Given the lack of objections by these maintainers, I will go ahead and
merge this series tomorrow.

> > changes, or whether there are any objections.
> >
> > I intend to merge this somewhere next week unless there are issues raised.
>
> I'll submit the edk2-platforms series (2 patches, one for Platform/AMD and
> one for Platform/Intel) on Monday, just to have out there.
>

Noted. I will merge this in two phases, and merge the [trivial]
edk2-platforms pieces in between to preserve some notion of
monotonicity  between the repos.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117867): https://edk2.groups.io/g/devel/message/117867
Mute This Topic: https://groups.io/mt/104810672/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 6/9] CryptoPkg: Add Pkcs5 functions based on Mbedtls

[Wenxing Hou]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

PBKDF2 Key Derivation Function Wrapper Implementation over MbedTLS.

Cc: Jiewen Yao 
Cc: Yi Li 
Signed-off-by: Wenxing Hou 
---
 .../BaseCryptLibMbedTls/Pk/CryptPkcs5Pbkdf2.c | 100 ++
 1 file changed, 100 insertions(+)
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs5Pbkdf2.c

diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs5Pbkdf2.c 
b/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs5Pbkdf2.c
new file mode 100644
index 00..94f1fcfa3b
--- /dev/null
+++ b/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs5Pbkdf2.c
@@ -0,0 +1,100 @@
+/** @file
+  PBKDF2 Key Derivation Function Wrapper Implementation over MbedTLS.
+
+Copyright (c) 2024, Intel Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "InternalCryptLib.h"
+#include 
+
+/**
+  Derives a key from a password using a salt and iteration count, based on 
PKCS#5 v2.0
+  password based encryption key derivation function PBKDF2, as specified in 
RFC 2898.
+
+  If Password or Salt or OutKey is NULL, then return FALSE.
+  If the hash algorithm could not be determined, then return FALSE.
+
+  @param[in]  PasswordLength  Length of input password in bytes.
+  @param[in]  PasswordPointer to the array for the password.
+  @param[in]  SaltLength  Size of the Salt in bytes.
+  @param[in]  SaltPointer to the Salt.
+  @param[in]  IterationCount  Number of iterations to perform. Its value 
should be
+  greater than or equal to 1.
+  @param[in]  DigestSize  Size of the message digest to be used (eg. 
SHA256_DIGEST_SIZE).
+  NOTE: DigestSize will be used to determine the 
hash algorithm.
+Only SHA1_DIGEST_SIZE or 
SHA256_DIGEST_SIZE is supported.
+  @param[in]  KeyLength   Size of the derived key buffer in bytes.
+  @param[out] OutKey  Pointer to the output derived key buffer.
+
+  @retval  TRUE   A key was derived successfully.
+  @retval  FALSE  One of the pointers was NULL or one of the sizes was too 
large.
+  @retval  FALSE  The hash algorithm could not be determined from the digest 
size.
+  @retval  FALSE  The key derivation operation failed.
+
+**/
+BOOLEAN
+EFIAPI
+Pkcs5HashPassword (
+  IN UINTNPasswordLength,
+  IN CONST CHAR8  *Password,
+  IN UINTNSaltLength,
+  IN CONST UINT8  *Salt,
+  IN UINTNIterationCount,
+  IN UINTNDigestSize,
+  IN UINTNKeyLength,
+  OUT UINT8   *OutKey
+  )
+{
+  mbedtls_md_type_t  HashAlg;
+
+  //
+  // Parameter Checking.
+  //
+  if ((Password == NULL) || (Salt == NULL) || (OutKey == NULL)) {
+return FALSE;
+  }
+
+  if ((PasswordLength == 0) || (PasswordLength > INT_MAX) ||
+  (SaltLength == 0) || (SaltLength > INT_MAX) ||
+  (KeyLength == 0) || (KeyLength > INT_MAX) ||
+  (IterationCount < 1) || (IterationCount > INT_MAX))
+  {
+return FALSE;
+  }
+
+  //
+  // Make sure the digest algorithm is supported.
+  //
+  switch (DigestSize) {
+case SHA1_DIGEST_SIZE:
+  HashAlg = MBEDTLS_MD_SHA1;
+  break;
+case SHA256_DIGEST_SIZE:
+  HashAlg = MBEDTLS_MD_SHA256;
+  break;
+default:
+  return FALSE;
+  break;
+  }
+
+  //
+  // Perform password-based key derivation routines.
+  //
+  if (mbedtls_pkcs5_pbkdf2_hmac_ext (
+HashAlg,
+(CONST UINT8 *)Password,
+(int)PasswordLength,
+(CONST UINT8 *)Salt,
+(int)SaltLength,
+(int)IterationCount,
+(int)KeyLength,
+(UINT8 *)OutKey
+) != 0)
+  {
+return FALSE;
+  } else {
+return TRUE;
+  }
+}
-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117863): https://edk2.groups.io/g/devel/message/117863
Mute This Topic: https://groups.io/mt/105552837/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 9/9] CryptoPkg: Add ImageTimestampVerify based on Mbedtls

[Wenxing Hou]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

Timestamp Countersignature Verification implementaion based on Mbedtls.

Cc: Jiewen Yao 
Cc: Yi Li 
Signed-off-by: Wenxing Hou 
---
 .../Library/BaseCryptLibMbedTls/Pk/CryptTs.c  | 381 ++
 1 file changed, 381 insertions(+)
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptTs.c

diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptTs.c 
b/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptTs.c
new file mode 100644
index 00..d3fa205f9c
--- /dev/null
+++ b/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptTs.c
@@ -0,0 +1,381 @@
+/** @file
+  RFC3161 Timestamp Countersignature Verification Wrapper Implementation which 
does
+  not provide real capabilities.
+
+Copyright (c) 2024, Intel Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "InternalCryptLib.h"
+#include 
+
+//
+// OID ASN.1 Value for SPC_RFC3161_OBJID ("1.3.6.1.4.1.311.3.3.1")
+//
+GLOBAL_REMOVE_IF_UNREFERENCED const UINT8  mSpcRFC3161OidValue[] = {
+  0x2b, 0x06, 0x01, 0x04, 0x01, 0x82, 0x37, 0x03, 0x03, 0x01
+};
+
+/**
+  Convert ASN.1 GeneralizedTime to EFI Time.
+
+  @param[in]  Ptr  Pointer to the ASN.1 GeneralizedTime to be 
converted.
+  @param[out] EfiTime  Return the corresponding EFI Time.
+
+  @retval  TRUE   The time conversion succeeds.
+  @retval  FALSE  Invalid parameters.
+
+**/
+STATIC
+BOOLEAN
+ConvertAsn1TimeToEfiTime (
+  IN  UINT8 *Ptr,
+  OUT EFI_TIME  *EfiTime
+  )
+{
+  CONST CHAR8  *Str;
+  UINTNIndex;
+
+  if ((Ptr == NULL) || (EfiTime == NULL)) {
+return FALSE;
+  }
+
+  Str = (CONST CHAR8 *)Ptr;
+  SetMem (EfiTime, sizeof (EFI_TIME), 0);
+
+  Index = 0;
+
+  /* four digit year */
+  EfiTime->Year  = (Str[Index++] - '0') * 1000;
+  EfiTime->Year += (Str[Index++] - '0') * 100;
+  EfiTime->Year += (Str[Index++] - '0') * 10;
+  EfiTime->Year += (Str[Index++] - '0');
+  if ((EfiTime->Year < 1900) || (EfiTime->Year > )) {
+return FALSE;
+  }
+
+  EfiTime->Month  = (Str[Index++] - '0') * 10;
+  EfiTime->Month += (Str[Index++] - '0');
+  if ((EfiTime->Month < 1) || (EfiTime->Month > 12)) {
+return FALSE;
+  }
+
+  EfiTime->Day  = (Str[Index++] - '0') * 10;
+  EfiTime->Day += (Str[Index++] - '0');
+  if ((EfiTime->Day < 1) || (EfiTime->Day > 31)) {
+return FALSE;
+  }
+
+  EfiTime->Hour  = (Str[Index++] - '0') * 10;
+  EfiTime->Hour += (Str[Index++] - '0');
+  if (EfiTime->Hour > 23) {
+return FALSE;
+  }
+
+  EfiTime->Minute  = (Str[Index++] - '0') * 10;
+  EfiTime->Minute += (Str[Index++] - '0');
+  if (EfiTime->Minute > 59) {
+return FALSE;
+  }
+
+  EfiTime->Second  = (Str[Index++] - '0') * 10;
+  EfiTime->Second += (Str[Index++] - '0');
+  if (EfiTime->Second > 59) {
+return FALSE;
+  }
+
+  /* Note: we did not adjust the time based on time zone information */
+
+  return TRUE;
+}
+
+/**
+  Verifies the validity of a RFC3161 Timestamp CounterSignature embedded in 
PE/COFF Authenticode
+  signature.
+
+  Return FALSE to indicate this interface is not supported.
+
+  @param[in]  AuthData Pointer to the Authenticode Signature retrieved 
from signed
+   PE/COFF image to be verified.
+  @param[in]  DataSize Size of the Authenticode Signature in bytes.
+  @param[in]  TsaCert  Pointer to a trusted/root TSA certificate encoded 
in DER, which
+   is used for TSA certificate chain verification.
+  @param[in]  CertSize Size of the trusted certificate in bytes.
+  @param[out] SigningTime  Return the time of timestamp generation time if the 
timestamp
+   signature is valid.
+
+  @retval  FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+ImageTimestampVerify (
+  IN  CONST UINT8  *AuthData,
+  IN  UINTNDataSize,
+  IN  CONST UINT8  *TsaCert,
+  IN  UINTNCertSize,
+  OUT EFI_TIME *SigningTime
+  )
+{
+  BOOLEAN  Status;
+  UINT8*Ptr;
+  UINT8*End;
+  INT32Len;
+  UINTNObjLen;
+  UINT8*TempPtr;
+
+  //
+  // Initializations
+  //
+  if (SigningTime != NULL) {
+SetMem (SigningTime, sizeof (EFI_TIME), 0);
+  }
+
+  //
+  // Input Parameters Checking.
+  //
+  if ((AuthData == NULL) || (TsaCert == NULL)) {
+return FALSE;
+  }
+
+  if ((DataSize > INT_MAX) || (CertSize > INT_MAX)) {
+return FALSE;
+  }
+
+  Ptr = (UINT8 *)(UINTN)AuthData;
+  Len = (UINT32)DataSize;
+  End = Ptr + Len;
+
+  // ContentInfo
+  if (mbedtls_asn1_get_tag (, End, , MBEDTLS_ASN1_CONSTRUCTED | 
MBEDTLS_ASN1_SEQUENCE) != 0) {
+return FALSE;
+  }
+
+  // ContentType
+  if (mbedtls_asn1_get_tag (, End, , MBEDTLS_ASN1_OID) != 0) {
+return FALSE;
+  }
+
+  Ptr += ObjLen;
+  // content
+  if (mbedtls_asn1_get_tag (, End, , MBEDTLS_ASN1_CONSTRUCTED | 
MBEDTLS_ASN1_CONTEXT_SPECIFIC) != 0) {
+return FALSE;
+  }
+
+  End = Ptr + ObjLen;
+  // signedData
+  if (mbedtls_asn1_get_tag (, End, , MBEDTLS_ASN1_CONSTRUCTED | 

[edk2-devel] [PATCH 8/9] CryptoPkg: Add AuthenticodeVerify based on Mbedtls

[Wenxing Hou]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

Implement AuthenticodeVerify based on Mbedtls.

Cc: Jiewen Yao 
Cc: Yi Li 
Signed-off-by: Wenxing Hou 
---
 .../Pk/CryptAuthenticode.c| 214 ++
 1 file changed, 214 insertions(+)
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptAuthenticode.c

diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptAuthenticode.c 
b/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptAuthenticode.c
new file mode 100644
index 00..9d8301b2c0
--- /dev/null
+++ b/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptAuthenticode.c
@@ -0,0 +1,214 @@
+/** @file
+  Authenticode Portable Executable Signature Verification which does not 
provide
+  real capabilities.
+
+Copyright (c) 2024, Intel Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "InternalCryptLib.h"
+#include 
+
+//
+// OID ASN.1 Value for SPC_INDIRECT_DATA_OBJID
+//
+GLOBAL_REMOVE_IF_UNREFERENCED const UINT8  mSpcIndirectOidValue[] = {
+  0x2B, 0x06, 0x01, 0x04, 0x01, 0x82, 0x37, 0x02, 0x01, 0x04
+};
+
+/**
+  Verifies the validity of a PE/COFF Authenticode Signature as described in 
"Windows
+  Authenticode Portable Executable Signature Format".
+
+  Return FALSE to indicate this interface is not supported.
+
+  @param[in]  AuthData Pointer to the Authenticode Signature retrieved 
from signed
+   PE/COFF image to be verified.
+  @param[in]  DataSize Size of the Authenticode Signature in bytes.
+  @param[in]  TrustedCert  Pointer to a trusted/root certificate encoded in 
DER, which
+   is used for certificate chain verification.
+  @param[in]  CertSize Size of the trusted certificate in bytes.
+  @param[in]  ImageHashPointer to the original image file hash value. The 
procedure
+   for calculating the image hash value is described 
in Authenticode
+   specification.
+  @param[in]  HashSize Size of Image hash value in bytes.
+
+  @retval FALSE  This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+AuthenticodeVerify (
+  IN CONST UINT8  *AuthData,
+  IN UINTNDataSize,
+  IN CONST UINT8  *TrustedCert,
+  IN UINTNCertSize,
+  IN CONST UINT8  *ImageHash,
+  IN UINTNHashSize
+  )
+{
+  BOOLEAN  Status;
+  CONST UINT8  *OrigAuthData;
+  UINT8*SpcIndirectDataContent;
+  UINT8Asn1Byte;
+  UINTNContentSize;
+  CONST UINT8  *SpcIndirectDataOid;
+  UINT8*Ptr;
+  UINT8*End;
+  INT32Len;
+  UINTNObjLen;
+
+  OrigAuthData = AuthData;
+
+  //
+  // Check input parameters.
+  //
+  if ((AuthData == NULL) || (TrustedCert == NULL) || (ImageHash == NULL)) {
+return FALSE;
+  }
+
+  if ((DataSize > INT_MAX) || (CertSize > INT_MAX) || (HashSize > INT_MAX)) {
+return FALSE;
+  }
+
+  if (DataSize <= HashSize) {
+return FALSE;
+  }
+
+  Ptr = (UINT8 *)(UINTN)AuthData;
+  Len = (UINT32)DataSize;
+  End = Ptr + Len;
+
+  // ContentInfo
+  if (mbedtls_asn1_get_tag (, End, , MBEDTLS_ASN1_CONSTRUCTED | 
MBEDTLS_ASN1_SEQUENCE) != 0) {
+return FALSE;
+  }
+
+  // ContentType
+  if (mbedtls_asn1_get_tag (, End, , MBEDTLS_ASN1_OID) != 0) {
+return FALSE;
+  }
+
+  Ptr += ObjLen;
+  // content
+  if (mbedtls_asn1_get_tag (, End, , MBEDTLS_ASN1_CONSTRUCTED | 
MBEDTLS_ASN1_CONTEXT_SPECIFIC) != 0) {
+return FALSE;
+  }
+
+  End = Ptr + ObjLen;
+  // signedData
+  if (mbedtls_asn1_get_tag (, End, , MBEDTLS_ASN1_CONSTRUCTED | 
MBEDTLS_ASN1_SEQUENCE) != 0) {
+return FALSE;
+  }
+
+  // version
+  if (mbedtls_asn1_get_tag (, End, , MBEDTLS_ASN1_INTEGER) != 0) {
+return FALSE;
+  }
+
+  Ptr += ObjLen;
+  // digestAlgo
+  if (mbedtls_asn1_get_tag (, End, , MBEDTLS_ASN1_CONSTRUCTED | 
MBEDTLS_ASN1_SET) != 0) {
+return FALSE;
+  }
+
+  Ptr += ObjLen;
+
+  // encapContentInfo
+  if (mbedtls_asn1_get_tag (, End, , MBEDTLS_ASN1_CONSTRUCTED | 
MBEDTLS_ASN1_SEQUENCE) != 0) {
+return FALSE;
+  }
+
+  End = Ptr + ObjLen;
+  // eContentType
+  if (mbedtls_asn1_get_tag (, End, , MBEDTLS_ASN1_OID) != 0) {
+return FALSE;
+  }
+
+  Status = FALSE;
+
+  SpcIndirectDataOid = Ptr;
+  if ((ObjLen != sizeof (mSpcIndirectOidValue)) ||
+  (CompareMem (
+ SpcIndirectDataOid,
+ mSpcIndirectOidValue,
+ sizeof (mSpcIndirectOidValue)
+ ) != 0))
+  {
+//
+// Un-matched SPC_INDIRECT_DATA_OBJID.
+//
+goto _Exit;
+  }
+
+  Ptr += ObjLen;
+  // eContent
+  if (mbedtls_asn1_get_tag (, End, , MBEDTLS_ASN1_CONSTRUCTED | 
MBEDTLS_ASN1_CONTEXT_SPECIFIC) != 0) {
+return FALSE;
+  }
+
+  SpcIndirectDataContent = Ptr;
+
+  //
+  // Retrieve the SEQUENCE data size from ASN.1-encoded SpcIndirectDataContent.
+  //
+  Asn1Byte = *(SpcIndirectDataContent + 1);
+
+  if ((Asn1Byte & 0x80) == 0) {
+//
+// Short Form of Length Encoding (Length < 128)
+//
+ContentSize = (UINTN)(Asn1Byte 

[edk2-devel] [PATCH 7/9] CryptoPkg: Add more RSA related functions based on Mbedtls

[Wenxing Hou]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

Implement more RSA functions such as RsaPkcs1Sign based Mbedlts.

Cc: Jiewen Yao 
Cc: Yi Li 
Signed-off-by: Wenxing Hou 
---
 .../BaseCryptLibMbedTls/Pk/CryptPkcs1Oaep.c   | 278 ++
 .../BaseCryptLibMbedTls/Pk/CryptRsaExt.c  | 346 ++
 .../BaseCryptLibMbedTls/Pk/CryptRsaPssSign.c  | 137 +++
 3 files changed, 761 insertions(+)
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs1Oaep.c
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptRsaExt.c
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptRsaPssSign.c

diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs1Oaep.c 
b/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs1Oaep.c
new file mode 100644
index 00..61ccdd78e6
--- /dev/null
+++ b/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs1Oaep.c
@@ -0,0 +1,278 @@
+/** @file
+  This file contains UEFI wrapper functions for RSA PKCS1v2 OAEP encryption 
routines.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  Copyright (c) 2024, Intel Corporation. All rights reserved.
+
+**/
+
+#include "InternalCryptLib.h"
+#include 
+#include 
+#include 
+
+/**
+  Encrypts a blob using PKCS1v2 (RSAES-OAEP) schema. On success, will return 
the
+  encrypted message in a newly allocated buffer.
+
+  Things that can cause a failure include:
+  - X509 key size does not match any known key size.
+  - Fail to parse X509 certificate.
+  - Fail to allocate an intermediate buffer.
+  - Null pointer provided for a non-optional parameter.
+  - Data size is too large for the provided key size (max size is a function 
of key size
+and hash digest size).
+
+  @param[in]  PublicKey   A pointer to the DER-encoded X509 
certificate that
+  will be used to encrypt the data.
+  @param[in]  PublicKeySize   Size of the X509 cert buffer.
+  @param[in]  InData  Data to be encrypted.
+  @param[in]  InDataSize  Size of the data buffer.
+  @param[in]  PrngSeed[Optional] If provided, a pointer to a 
random seed buffer
+  to be used when initializing the PRNG. NULL 
otherwise.
+  @param[in]  PrngSeedSize[Optional] If provided, size of the random 
seed buffer.
+  0 otherwise.
+  @param[out] EncryptedData   Pointer to an allocated buffer containing 
the encrypted
+  message.
+  @param[out] EncryptedDataSize   Size of the encrypted message buffer.
+
+  @retval TRUEEncryption was successful.
+  @retval FALSE   Encryption failed.
+
+**/
+BOOLEAN
+EFIAPI
+Pkcs1v2Encrypt (
+  IN CONST UINT8  *PublicKey,
+  IN UINTNPublicKeySize,
+  IN UINT8*InData,
+  IN UINTNInDataSize,
+  IN CONST UINT8  *PrngSeed OPTIONAL,
+  IN UINTNPrngSeedSize OPTIONAL,
+  OUT UINT8   **EncryptedData,
+  OUT UINTN   *EncryptedDataSize
+  )
+{
+  BOOLEAN  Result;
+  UINT32   Ret;
+  UINT8*OutData;
+  mbedtls_x509_crt CertContext;
+  mbedtls_rsa_context  RsaContext;
+
+  //
+  // Check input parameters.
+  //
+  if ((PublicKey == NULL) || (InData == NULL) ||
+  (EncryptedData == NULL) || (EncryptedDataSize == NULL))
+  {
+return FALSE;
+  }
+
+  //
+  // Check public key size.
+  //
+  if (PublicKeySize > UINT_MAX) {
+//
+// Public key size is too large for implementation.
+//
+return FALSE;
+  }
+
+  *EncryptedData = NULL;
+  *EncryptedDataSize = 0;
+  Result = FALSE;
+  OutData= NULL;
+
+  mbedtls_x509_crt_init ();
+
+  if (mbedtls_x509_crt_parse_der (, PublicKey, 
(UINT32)PublicKeySize) != 0) {
+goto _Exit;
+  }
+
+  if (mbedtls_pk_get_type () != MBEDTLS_PK_RSA) {
+goto _Exit;
+  }
+
+  mbedtls_rsa_init ();
+  if (mbedtls_rsa_set_padding (, MBEDTLS_RSA_PKCS_V21, 
MBEDTLS_MD_NONE) != 0) {
+goto _Exit;
+  }
+
+  Ret = mbedtls_rsa_copy (, mbedtls_pk_rsa (CertContext.pk));
+  if (Ret != 0) {
+goto _Exit;
+  }
+
+  *EncryptedDataSize = RsaContext.len;
+
+  //
+  // Allocate a buffer for the output data.
+  //
+  OutData = AllocateZeroPool (*EncryptedDataSize);
+  if (OutData == NULL) {
+//
+// Fail to allocate the output buffer.
+//
+goto _Exit;
+  }
+
+  Ret = mbedtls_rsa_pkcs1_encrypt (
+  ,
+  MbedtlsRand,
+  NULL,
+  InDataSize,
+  InData,
+  OutData
+  );
+  if (Ret != 0) {
+FreePool (OutData);
+OutData = NULL;
+goto _Exit;
+  }
+
+  *EncryptedData = OutData;
+  Result = TRUE;
+
+_Exit:
+  //
+  // Release Resources
+  //
+  if ( != NULL) {
+mbedtls_x509_crt_free ();
+  }
+
+  if ( != NULL) {
+mbedtls_rsa_free ();
+  }
+
+  return Result;
+}
+
+/**
+  Encrypts a blob using PKCS1v2 (RSAES-OAEP) schema. On success, will return 
the
+  encrypted message in a 

[edk2-devel] [PATCH 5/9] CryptoPkg: Add Pkcs7 related functions based on Mbedtls

[Wenxing Hou]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

Because the current Mbedlts pkcs7 library doesn't support
authenticatedAttributes:
Mbed-TLS/mbedtls@bb82ab7
and only support 0 or 1 certificates in Signed data:
tianocore/edk2-staging@9c5b26b

The patch implement Pkcs7 by low Mbedtls Api.
And the implementation has pass unit_tes and integration test.

Cc: Jiewen Yao 
Cc: Yi Li 
Signed-off-by: Wenxing Hou 
---
 .../BaseCryptLibMbedTls/InternalCryptLib.h|   33 +
 .../Pk/CryptPkcs7Internal.h   |   20 +-
 .../BaseCryptLibMbedTls/Pk/CryptPkcs7Sign.c   |  615 
 .../Pk/CryptPkcs7VerifyBase.c |  113 ++
 .../Pk/CryptPkcs7VerifyCommon.c   | 1315 +
 .../Pk/CryptPkcs7VerifyEku.c  |  689 +
 6 files changed, 2773 insertions(+), 12 deletions(-)
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7Sign.c
 create mode 100644 
CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7VerifyBase.c
 create mode 100644 
CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7VerifyCommon.c
 create mode 100644 
CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7VerifyEku.c

diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/InternalCryptLib.h 
b/CryptoPkg/Library/BaseCryptLibMbedTls/InternalCryptLib.h
index e2c7e42ecb..1b9742c166 100644
--- a/CryptoPkg/Library/BaseCryptLibMbedTls/InternalCryptLib.h
+++ b/CryptoPkg/Library/BaseCryptLibMbedTls/InternalCryptLib.h
@@ -38,4 +38,37 @@ MbedtlsRand (
   UINT8  *output,
   UINTN  len
   );
+
+/**
+  Check input P7Data is a wrapped ContentInfo structure or not. If not 
construct
+  a new structure to wrap P7Data.
+
+  Caution: This function may receive untrusted input.
+  UEFI Authenticated Variable is external input, so this function will do basic
+  check for PKCS#7 data structure.
+
+  @param[in]  P7Data   Pointer to the PKCS#7 message to verify.
+  @param[in]  P7Length Length of the PKCS#7 message in bytes.
+  @param[out] WrapFlag If TRUE P7Data is a ContentInfo structure, otherwise
+   return FALSE.
+  @param[out] WrapData If return status of this function is TRUE:
+   1) when WrapFlag is TRUE, pointer to P7Data.
+   2) when WrapFlag is FALSE, pointer to a new 
ContentInfo
+   structure. It's caller's responsibility to free this
+   buffer.
+  @param[out] WrapDataSize Length of ContentInfo structure in bytes.
+
+  @retval TRUE The operation is finished successfully.
+  @retval FALSEThe operation is failed due to lack of resources.
+
+**/
+BOOLEAN
+WrapPkcs7Data (
+  IN  CONST UINT8  *P7Data,
+  IN  UINTNP7Length,
+  OUT BOOLEAN  *WrapFlag,
+  OUT UINT8**WrapData,
+  OUT UINTN*WrapDataSize
+  );
+
 #endif
diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7Internal.h 
b/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7Internal.h
index 207f493cbb..d4bdb0abf7 100644
--- a/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7Internal.h
+++ b/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7Internal.h
@@ -4,7 +4,7 @@
 
   RFC 2315 - PKCS #7: Cryptographic Message Syntax Version 1.5
 
-Copyright (c) 2023, Intel Corporation. All rights reserved.
+Copyright (c) 2023-2024, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -31,10 +31,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define MBEDTLS_OID_PKCS7_DIGESTED_DATA  MBEDTLS_OID_PKCS7 "\x05"
 #define MBEDTLS_OID_PKCS7_ENCRYPTED_DATA MBEDTLS_OID_PKCS7 "\x06"
 
-typedef mbedtls_asn1_buf MBEDTLSPKCS7BUF;
-typedef mbedtls_asn1_named_data  MBEDTLSPKCS7NAME;
-typedef mbedtls_asn1_sequenceMBEDTLSPKCS7SEQUENCE;
-
 ///
 /// PKCS7 SignerInfo type
 /// https://tools.ietf.org/html/rfc2315#section-9.2
@@ -48,8 +44,8 @@ typedef struct MbedtlsPkcs7SignerInfo {
   mbedtls_x509_buf SigAlgIdentifier;
   mbedtls_x509_buf AuthAttr;
   mbedtls_x509_buf Sig;
-  struct MBEDTLSPKCS7SIGNERINFO*Next;
-} MBEDTLSPKCS7SIGNERINFO;
+  struct MbedtlsPkcs7SignerInfo*Next;
+} MbedtlsPkcs7SignerInfo;
 
 ///
 /// PKCS7 signed data attached data format
@@ -57,7 +53,7 @@ typedef struct MbedtlsPkcs7SignerInfo {
 typedef struct MbedtlsPkcs7Data {
   mbedtls_asn1_bufOid;
   mbedtls_asn1_bufData;
-} MBEDTLSPKCS7DATA;
+} MbedtlsPkcs7Data;
 
 ///
 /// Signed Data
@@ -66,18 +62,18 @@ typedef struct MbedtlsPkcs7Data {
 typedef struct MbedtlsPkcs7SignedData {
   INT32Version;
   mbedtls_asn1_buf DigestAlgorithms;
-  struct MBEDTLSPKCS7DATA  ContentInfo;
+  struct MbedtlsPkcs7Data  ContentInfo;
   mbedtls_x509_crt Certificates;
   mbedtls_x509_crl Crls;
   struct MbedtlsPkcs7SignerInfoSignerInfos;
-} MBEDTLSPKCS7SIGNEDDATA;
+} 

[edk2-devel] [PATCH 4/9] CryptoPkg: Add X509 functions based on Mbedtls

[Wenxing Hou]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

X.509 Certificate Handler Wrapper Implementation over MbedTLS.

Cc: Jiewen Yao 
Cc: Yi Li 
Signed-off-by: Wenxing Hou 
---
 .../BaseCryptLibMbedTls/Pk/CryptX509.c| 1924 +
 1 file changed, 1924 insertions(+)
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptX509.c

diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptX509.c 
b/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptX509.c
new file mode 100644
index 00..791fe78cd2
--- /dev/null
+++ b/CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptX509.c
@@ -0,0 +1,1924 @@
+/** @file
+  X.509 Certificate Handler Wrapper Implementation over MbedTLS.
+
+Copyright (c) 2024, Intel Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "InternalCryptLib.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+///
+/// OID
+///
+STATIC CONST UINT8  OID_commonName[] = {
+  0x55, 0x04, 0x03
+};
+STATIC CONST UINT8  OID_organizationName[] = {
+  0x55, 0x04, 0x0A
+};
+STATIC CONST UINT8  OID_extKeyUsage[] = {
+  0x55, 0x1D, 0x25
+};
+STATIC CONST UINT8  OID_BasicConstraints[] = {
+  0x55, 0x1D, 0x13
+};
+
+/* Profile for backward compatibility. Allows RSA 1024, unlike the default
+   profile. */
+STATIC mbedtls_x509_crt_profile  gCompatProfile =
+{
+  /* Hashes from SHA-256 and above. Note that this selection
+   * should be aligned with ssl_preset_default_hashes in ssl_tls.c. */
+  MBEDTLS_X509_ID_FLAG (MBEDTLS_MD_SHA256) |
+  MBEDTLS_X509_ID_FLAG (MBEDTLS_MD_SHA384) |
+  MBEDTLS_X509_ID_FLAG (MBEDTLS_MD_SHA512),
+  0xFFF,   /* Any PK alg*/
+
+  /* Curves at or above 128-bit security level. Note that this selection
+   * should be aligned with ssl_preset_default_curves in ssl_tls.c. */
+  MBEDTLS_X509_ID_FLAG (MBEDTLS_ECP_DP_SECP256R1) |
+  MBEDTLS_X509_ID_FLAG (MBEDTLS_ECP_DP_SECP384R1) |
+  MBEDTLS_X509_ID_FLAG (MBEDTLS_ECP_DP_SECP521R1) |
+  MBEDTLS_X509_ID_FLAG (MBEDTLS_ECP_DP_BP256R1) |
+  MBEDTLS_X509_ID_FLAG (MBEDTLS_ECP_DP_BP384R1) |
+  MBEDTLS_X509_ID_FLAG (MBEDTLS_ECP_DP_BP512R1) |
+  0,
+  1024,
+};
+
+/**
+  Construct a X509 object from DER-encoded certificate data.
+
+  If Cert is NULL, then return FALSE.
+  If SingleX509Cert is NULL, then return FALSE.
+
+  @param[in]  CertPointer to the DER-encoded certificate data.
+  @param[in]  CertSizeThe size of certificate data in bytes.
+  @param[out] SingleX509Cert  The generated X509 object.
+
+  @retval TRUEThe X509 object generation succeeded.
+  @retval FALSE   The operation failed.
+
+**/
+BOOLEAN
+EFIAPI
+X509ConstructCertificate (
+  IN CONST UINT8  *Cert,
+  IN UINTNCertSize,
+  OUT UINT8   **SingleX509Cert
+  )
+{
+  mbedtls_x509_crt  *MbedTlsCert;
+  INT32 Ret;
+
+  if ((Cert == NULL) || (SingleX509Cert == NULL) || (CertSize == 0)) {
+return FALSE;
+  }
+
+  MbedTlsCert = AllocateZeroPool (sizeof (mbedtls_x509_crt));
+  if (MbedTlsCert == NULL) {
+return FALSE;
+  }
+
+  mbedtls_x509_crt_init (MbedTlsCert);
+
+  *SingleX509Cert = (UINT8 *)(VOID *)MbedTlsCert;
+  Ret = mbedtls_x509_crt_parse_der (MbedTlsCert, Cert, CertSize);
+
+  return Ret == 0;
+}
+
+/**
+  Construct a X509 stack object from a list of DER-encoded certificate data.
+
+  If X509Stack is NULL, then return FALSE.
+  If this interface is not supported, then return FALSE.
+
+  @param[in, out]  X509Stack  On input, pointer to an existing or NULL X509 
stack object.
+  On output, pointer to the X509 stack object with 
new
+  inserted X509 certificate.
+  @param[in]   Args   VA_LIST marker for the variable argument list.
+  A list of DER-encoded single certificate data 
followed
+  by certificate size. A NULL terminates the list. 
The
+  pairs are the arguments to 
X509ConstructCertificate().
+
+  @retval TRUEThe X509 stack construction succeeded.
+  @retval FALSE   The construction operation failed.
+  @retval FALSE   This interface is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+X509ConstructCertificateStackV (
+  IN OUT UINT8  **X509Stack,
+  IN VA_LISTArgs
+  )
+{
+  UINT8 *Cert;
+  UINTN CertSize;
+  INT32 Index;
+  INT32 Ret;
+  mbedtls_x509_crt  *Crt;
+
+  if (X509Stack == NULL) {
+return FALSE;
+  }
+
+  Ret = 0;
+  Crt = (mbedtls_x509_crt *)*X509Stack;
+  if (Crt == NULL) {
+Crt = AllocateZeroPool (sizeof (mbedtls_x509_crt));
+if (Crt == NULL) {
+  return FALSE;
+}
+
+mbedtls_x509_crt_init (Crt);
+*X509Stack = (UINT8 *)Crt;
+  }
+
+  for (Index = 0; ; Index++) {
+//
+// If Cert is NULL, then it is the end of the list.
+//
+Cert = VA_ARG (Args, UINT8 *);
+if (Cert == NULL) {
+  break;
+}
+

[edk2-devel] [PATCH 3/9] CryptoPkg: Add Pem APIs based on Mbedtls

[Wenxing Hou]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

Implement Pem API based on Mbedtls.

Cc: Jiewen Yao 
Cc: Yi Li 
Signed-off-by: Wenxing Hou 
---
 .../BaseCryptLibMbedTls/Pem/CryptPem.c| 138 ++
 1 file changed, 138 insertions(+)
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pem/CryptPem.c

diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/Pem/CryptPem.c 
b/CryptoPkg/Library/BaseCryptLibMbedTls/Pem/CryptPem.c
new file mode 100644
index 00..def1478b8b
--- /dev/null
+++ b/CryptoPkg/Library/BaseCryptLibMbedTls/Pem/CryptPem.c
@@ -0,0 +1,138 @@
+/** @file
+  PEM (Privacy Enhanced Mail) Format Handler Wrapper Implementation over 
MbedTLS.
+
+Copyright (c) 2024, Intel Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "InternalCryptLib.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+  Retrieve the RSA Private Key from the password-protected PEM key data.
+
+  @param[in]  PemData  Pointer to the PEM-encoded key data to be retrieved.
+  @param[in]  PemSize  Size of the PEM key data in bytes.
+  @param[in]  Password NULL-terminated passphrase used for encrypted PEM 
key data.
+  @param[out] RsaContext   Pointer to new-generated RSA context which contain 
the retrieved
+   RSA private key component. Use RsaFree() function 
to free the
+   resource.
+
+  If PemData is NULL, then return FALSE.
+  If RsaContext is NULL, then return FALSE.
+
+  @retval  TRUE   RSA Private Key was retrieved successfully.
+  @retval  FALSE  Invalid PEM key data or incorrect password.
+
+**/
+BOOLEAN
+EFIAPI
+RsaGetPrivateKeyFromPem (
+  IN   CONST UINT8  *PemData,
+  IN   UINTNPemSize,
+  IN   CONST CHAR8  *Password,
+  OUT  VOID **RsaContext
+  )
+{
+  INT32Ret;
+  mbedtls_pk_context   pk;
+  mbedtls_rsa_context  *rsa;
+  UINT8*NewPemData;
+  UINTNPasswordLen;
+
+  if ((PemData == NULL) || (RsaContext == NULL) || (PemSize > INT_MAX)) {
+return FALSE;
+  }
+
+  NewPemData = NULL;
+  if (PemData[PemSize - 1] != 0) {
+NewPemData = AllocateZeroPool (PemSize + 1);
+if (NewPemData == NULL) {
+  return FALSE;
+}
+
+CopyMem (NewPemData, PemData, PemSize + 1);
+NewPemData[PemSize] = 0;
+PemData = NewPemData;
+PemSize+= 1;
+  }
+
+  mbedtls_pk_init ();
+
+  if (Password != NULL) {
+PasswordLen = AsciiStrLen (Password);
+  } else {
+PasswordLen = 0;
+  }
+
+  Ret = mbedtls_pk_parse_key (, PemData, PemSize, (CONST UINT8 *)Password, 
PasswordLen, NULL, NULL);
+
+  if (NewPemData != NULL) {
+FreePool (NewPemData);
+NewPemData = NULL;
+  }
+
+  if (Ret != 0) {
+mbedtls_pk_free ();
+return FALSE;
+  }
+
+  if (mbedtls_pk_get_type () != MBEDTLS_PK_RSA) {
+mbedtls_pk_free ();
+return FALSE;
+  }
+
+  rsa = RsaNew ();
+  if (rsa == NULL) {
+mbedtls_pk_free ();
+return FALSE;
+  }
+
+  Ret = mbedtls_rsa_copy (rsa, mbedtls_pk_rsa (pk));
+  if (Ret != 0) {
+RsaFree (rsa);
+mbedtls_pk_free ();
+return FALSE;
+  }
+
+  mbedtls_pk_free ();
+
+  *RsaContext = rsa;
+  return TRUE;
+}
+
+/**
+  Retrieve the EC Private Key from the password-protected PEM key data.
+
+  @param[in]  PemData  Pointer to the PEM-encoded key data to be retrieved.
+  @param[in]  PemSize  Size of the PEM key data in bytes.
+  @param[in]  Password NULL-terminated passphrase used for encrypted PEM 
key data.
+  @param[out] EcContextPointer to new-generated EC DSA context which 
contain the retrieved
+   EC private key component. Use EcFree() function to 
free the
+   resource.
+
+  If PemData is NULL, then return FALSE.
+  If EcContext is NULL, then return FALSE.
+
+  @retval  TRUE   EC Private Key was retrieved successfully.
+  @retval  FALSE  Invalid PEM key data or incorrect password.
+
+**/
+BOOLEAN
+EFIAPI
+EcGetPrivateKeyFromPem (
+  IN   CONST UINT8  *PemData,
+  IN   UINTNPemSize,
+  IN   CONST CHAR8  *Password,
+  OUT  VOID **EcContext
+  )
+{
+  ASSERT (FALSE);
+  return FALSE;
+}
-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117860): https://edk2.groups.io/g/devel/message/117860
Mute This Topic: https://groups.io/mt/105552833/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 2/9] CryptoPkg: Add rand function for BaseCryptLibMbedTls

[Wenxing Hou]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

Add rand function for BaseCryptLibMbedTls.

Cc: Jiewen Yao 
Cc: Yi Li 
Signed-off-by: Wenxing Hou 
---
 CryptoPkg/CryptoPkg.ci.yaml   |   1 +
 .../BaseCryptLibMbedTls/InternalCryptLib.h|  16 +++
 .../BaseCryptLibMbedTls/Rand/CryptRand.c  | 105 ++
 .../BaseCryptLibMbedTls/Rand/CryptRandTsc.c   | 105 ++
 4 files changed, 227 insertions(+)
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Rand/CryptRand.c
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Rand/CryptRandTsc.c

diff --git a/CryptoPkg/CryptoPkg.ci.yaml b/CryptoPkg/CryptoPkg.ci.yaml
index b601bcf85c..2fbc021c84 100644
--- a/CryptoPkg/CryptoPkg.ci.yaml
+++ b/CryptoPkg/CryptoPkg.ci.yaml
@@ -45,6 +45,7 @@
 "Library/Include/stdint.h",
 "Library/Include/stubs-32.h",
 "Library/Include/inttypes.h",
+"Library/BaseCryptLibMbedTls/InternalCryptLib.h",
 # These directories contain auto-generated OpenSSL content
 "Library/OpensslLib",
 "Library/IntrinsicLib",
diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/InternalCryptLib.h 
b/CryptoPkg/Library/BaseCryptLibMbedTls/InternalCryptLib.h
index 039aa32028..e2c7e42ecb 100644
--- a/CryptoPkg/Library/BaseCryptLibMbedTls/InternalCryptLib.h
+++ b/CryptoPkg/Library/BaseCryptLibMbedTls/InternalCryptLib.h
@@ -22,4 +22,20 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 //
 #include 
 
+/**
+  The MbedTLS function f_rng, which MbedtlsRand implements.
+
+  @param[in]   rng_state Not used, just for compatibility with mbedlts.
+  @param[out]  output  Pointer to buffer to receive random value.
+  @param[in]   lenSize of random bytes to generate.
+
+  @retval 0  Pseudorandom byte stream generated successfully.
+  @retval Non-0  Pseudorandom number generator fails to generate due to lack 
of entropy.
+**/
+INTN
+MbedtlsRand (
+  VOID   *rng_state,
+  UINT8  *output,
+  UINTN  len
+  );
 #endif
diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/Rand/CryptRand.c 
b/CryptoPkg/Library/BaseCryptLibMbedTls/Rand/CryptRand.c
new file mode 100644
index 00..081b413740
--- /dev/null
+++ b/CryptoPkg/Library/BaseCryptLibMbedTls/Rand/CryptRand.c
@@ -0,0 +1,105 @@
+/** @file
+  Pseudorandom Number Generator Wrapper Implementation over MbedTLS.
+
+Copyright (c) 2024, Intel Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "InternalCryptLib.h"
+#include 
+
+/**
+  Sets up the seed value for the pseudorandom number generator.
+
+  This function sets up the seed value for the pseudorandom number generator.
+  If Seed is not NULL, then the seed passed in is used.
+  If Seed is NULL, then default seed is used.
+
+  @param[in]  Seed  Pointer to seed value.
+If NULL, default seed is used.
+  @param[in]  SeedSize  Size of seed value.
+If Seed is NULL, this parameter is ignored.
+
+  @retval TRUE   Pseudorandom number generator has enough entropy for random 
generation.
+  @retval FALSE  Pseudorandom number generator does not have enough entropy 
for random generation.
+
+**/
+BOOLEAN
+EFIAPI
+RandomSeed (
+  IN  CONST  UINT8  *Seed  OPTIONAL,
+  IN  UINTN SeedSize
+  )
+{
+  return TRUE;
+}
+
+/**
+  Generates a pseudorandom byte stream of the specified size.
+
+  If Output is NULL, then return FALSE.
+
+  @param[out]  Output  Pointer to buffer to receive random value.
+  @param[in]   SizeSize of random bytes to generate.
+
+  @retval TRUE   Pseudorandom byte stream generated successfully.
+  @retval FALSE  Pseudorandom number generator fails to generate due to lack 
of entropy.
+
+**/
+BOOLEAN
+EFIAPI
+RandomBytes (
+  OUT  UINT8  *Output,
+  IN   UINTN  Size
+  )
+{
+  BOOLEAN  Ret;
+  UINT64   TempRand;
+
+  Ret = FALSE;
+
+  while (Size > 0) {
+// Use RngLib to get random number
+Ret = GetRandomNumber64 ();
+
+if (!Ret) {
+  return Ret;
+}
+
+if (Size >= sizeof (TempRand)) {
+  *((UINT64 *)Output) = TempRand;
+  Output += sizeof (UINT64);
+  Size   -= sizeof (TempRand);
+} else {
+  CopyMem (Output, , Size);
+  Size = 0;
+}
+  }
+
+  return Ret;
+}
+
+/**
+  The MbedTLS function f_rng, which MbedtlsRand implements.
+
+  @param[in]   rng_state Not used, just for compatibility with mbedlts.
+  @param[out]  output  Pointer to buffer to receive random value.
+  @param[in]   lenSize of random bytes to generate.
+
+  @retval 0  Pseudorandom byte stream generated successfully.
+  @retval Non-0  Pseudorandom number generator fails to generate due to lack 
of entropy.
+**/
+INTN
+MbedtlsRand (
+  VOID   *rng_state,
+  UINT8  *output,
+  UINTN  len
+  )
+{
+  BOOLEAN  Result;
+
+  Result = RandomBytes (output, len);
+
+  return Result ? 0 : -1;
+}
diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/Rand/CryptRandTsc.c 

[edk2-devel] [PATCH 1/9] CryptoPkg: Add AeadAesGcm based on Mbedtls

[Wenxing Hou]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

AeadAesGcm implementation based on Mbedtls.

Cc: Jiewen Yao 
Cc: Yi Li 
Signed-off-by: Wenxing Hou 
---
 .../Cipher/CryptAeadAesGcm.c  | 227 ++
 1 file changed, 227 insertions(+)
 create mode 100644 
CryptoPkg/Library/BaseCryptLibMbedTls/Cipher/CryptAeadAesGcm.c

diff --git a/CryptoPkg/Library/BaseCryptLibMbedTls/Cipher/CryptAeadAesGcm.c 
b/CryptoPkg/Library/BaseCryptLibMbedTls/Cipher/CryptAeadAesGcm.c
new file mode 100644
index 00..73104e42fa
--- /dev/null
+++ b/CryptoPkg/Library/BaseCryptLibMbedTls/Cipher/CryptAeadAesGcm.c
@@ -0,0 +1,227 @@
+/** @file
+  AEAD (AES-GCM) Wrapper Implementation over MbedTLS.
+
+  RFC 5116 - An Interface and Algorithms for Authenticated Encryption
+  NIST SP800-38d - Cipher Modes of Operation: Galois / Counter Mode(GCM) and 
GMAC
+
+Copyright (c) 2024, Intel Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "InternalCryptLib.h"
+#include 
+
+/**
+  Performs AEAD AES-GCM authenticated encryption on a data buffer and 
additional authenticated data (AAD).
+
+  IvSize must be 12, otherwise FALSE is returned.
+  KeySize must be 16, 24 or 32, otherwise FALSE is returned.
+  TagSize must be 12, 13, 14, 15, 16, otherwise FALSE is returned.
+
+  @param[in]   Key Pointer to the encryption key.
+  @param[in]   KeySize Size of the encryption key in bytes.
+  @param[in]   Iv  Pointer to the IV value.
+  @param[in]   IvSize  Size of the IV value in bytes.
+  @param[in]   AData   Pointer to the additional authenticated data (AAD).
+  @param[in]   ADataSize   Size of the additional authenticated data (AAD) in 
bytes.
+  @param[in]   DataIn  Pointer to the input data buffer to be encrypted.
+  @param[in]   DataInSize  Size of the input data buffer in bytes.
+  @param[out]  TagOut  Pointer to a buffer that receives the 
authentication tag output.
+  @param[in]   TagSize Size of the authentication tag in bytes.
+  @param[out]  DataOut Pointer to a buffer that receives the encryption 
output.
+  @param[out]  DataOutSize Size of the output data buffer in bytes.
+
+  @retval TRUE   AEAD AES-GCM authenticated encryption succeeded.
+  @retval FALSE  AEAD AES-GCM authenticated encryption failed.
+
+**/
+BOOLEAN
+EFIAPI
+AeadAesGcmEncrypt (
+  IN   CONST UINT8  *Key,
+  IN   UINTNKeySize,
+  IN   CONST UINT8  *Iv,
+  IN   UINTNIvSize,
+  IN   CONST UINT8  *AData,
+  IN   UINTNADataSize,
+  IN   CONST UINT8  *DataIn,
+  IN   UINTNDataInSize,
+  OUT  UINT8*TagOut,
+  IN   UINTNTagSize,
+  OUT  UINT8*DataOut,
+  OUT  UINTN*DataOutSize
+  )
+{
+  mbedtls_gcm_context  ctx;
+  INT32Ret;
+
+  if (DataInSize > INT_MAX) {
+return FALSE;
+  }
+
+  if (ADataSize > INT_MAX) {
+return FALSE;
+  }
+
+  if (IvSize != 12) {
+return FALSE;
+  }
+
+  switch (KeySize) {
+case 16:
+case 24:
+case 32:
+  break;
+default:
+  return FALSE;
+  }
+
+  if ((TagSize != 12) && (TagSize != 13) && (TagSize != 14) && (TagSize != 15) 
&& (TagSize != 16)) {
+return FALSE;
+  }
+
+  if (DataOutSize != NULL) {
+if ((*DataOutSize > INT_MAX) || (*DataOutSize < DataInSize)) {
+  return FALSE;
+}
+  }
+
+  mbedtls_gcm_init ();
+
+  Ret = mbedtls_gcm_setkey (, MBEDTLS_CIPHER_ID_AES, Key, (UINT32)(KeySize 
* 8));
+  if (Ret != 0) {
+return FALSE;
+  }
+
+  Ret = mbedtls_gcm_crypt_and_tag (
+  ,
+  MBEDTLS_GCM_ENCRYPT,
+  (UINT32)DataInSize,
+  Iv,
+  (UINT32)IvSize,
+  AData,
+  (UINT32)ADataSize,
+  DataIn,
+  DataOut,
+  TagSize,
+  TagOut
+  );
+  mbedtls_gcm_free ();
+  if (Ret != 0) {
+return FALSE;
+  }
+
+  if (DataOutSize != NULL) {
+*DataOutSize = DataInSize;
+  }
+
+  return TRUE;
+}
+
+/**
+  Performs AEAD AES-GCM authenticated decryption on a data buffer and 
additional authenticated data (AAD).
+
+  IvSize must be 12, otherwise FALSE is returned.
+  KeySize must be 16, 24 or 32, otherwise FALSE is returned.
+  TagSize must be 12, 13, 14, 15, 16, otherwise FALSE is returned.
+  If additional authenticated data verification fails, FALSE is returned.
+
+  @param[in]   Key Pointer to the encryption key.
+  @param[in]   KeySize Size of the encryption key in bytes.
+  @param[in]   Iv  Pointer to the IV value.
+  @param[in]   IvSize  Size of the IV value in bytes.
+  @param[in]   AData   Pointer to the additional authenticated data (AAD).
+  @param[in]   ADataSize   Size of the additional authenticated data (AAD) in 
bytes.
+  @param[in]   DataIn  Pointer to the input data buffer to be decrypted.
+  @param[in]   DataInSize  Size of the input data buffer in bytes.
+  @param[in]   Tag Pointer to a buffer that contains the 
authentication tag.
+  @param[in]   TagSize 

[edk2-devel] [PATCH 0/9] Add more crypt APIs based on Mbedtls

[Wenxing Hou]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

Add AeadAesGcm/Pem(only RSA)/X509(only RSA)/More RSA/PKCS5
/pKCS7/Authenticode/Timestamp implementation based on Mbedtls.

The patch has passed the EDKII CI check:
https://github.com/tianocore/edk2/pull/5552

And the patch has passed unit_test in EDKII and integration test for platform.
And the patch hass passed the fuzz test:
https://github.com/tianocore/edk2-staging/commit/4f19398053c92e4f7791d468a184530b6ab89128

Cc: Jiewen Yao 
Cc: Yi Li 
Signed-off-by: Wenxing Hou 

Wenxing Hou (9):
  CryptoPkg: Add AeadAesGcm based on Mbedtls
  CryptoPkg: Add rand function for BaseCryptLibMbedTls
  CryptoPkg: Add Pem APIs based on Mbedtls
  CryptoPkg: Add X509 functions based on Mbedtls
  CryptoPkg: Add Pkcs7 related functions based on Mbedtls
  CryptoPkg: Add Pkcs5 functions based on Mbedtls
  CryptoPkg: Add more RSA related functions based on Mbedtls
  CryptoPkg: Add AuthenticodeVerify based on Mbedtls
  CryptoPkg: Add ImageTimestampVerify based on Mbedtls

 CryptoPkg/CryptoPkg.ci.yaml   |1 +
 .../Cipher/CryptAeadAesGcm.c  |  227 ++
 .../BaseCryptLibMbedTls/InternalCryptLib.h|   49 +
 .../BaseCryptLibMbedTls/Pem/CryptPem.c|  138 ++
 .../Pk/CryptAuthenticode.c|  214 ++
 .../BaseCryptLibMbedTls/Pk/CryptPkcs1Oaep.c   |  278 +++
 .../BaseCryptLibMbedTls/Pk/CryptPkcs5Pbkdf2.c |  100 +
 .../Pk/CryptPkcs7Internal.h   |   20 +-
 .../BaseCryptLibMbedTls/Pk/CryptPkcs7Sign.c   |  615 ++
 .../Pk/CryptPkcs7VerifyBase.c |  113 +
 .../Pk/CryptPkcs7VerifyCommon.c   | 1315 +++
 .../Pk/CryptPkcs7VerifyEku.c  |  689 ++
 .../BaseCryptLibMbedTls/Pk/CryptRsaExt.c  |  346 +++
 .../BaseCryptLibMbedTls/Pk/CryptRsaPssSign.c  |  137 ++
 .../Library/BaseCryptLibMbedTls/Pk/CryptTs.c  |  381 
 .../BaseCryptLibMbedTls/Pk/CryptX509.c| 1924 +
 .../BaseCryptLibMbedTls/Rand/CryptRand.c  |  105 +
 .../BaseCryptLibMbedTls/Rand/CryptRandTsc.c   |  105 +
 18 files changed, 6745 insertions(+), 12 deletions(-)
 create mode 100644 
CryptoPkg/Library/BaseCryptLibMbedTls/Cipher/CryptAeadAesGcm.c
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pem/CryptPem.c
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptAuthenticode.c
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs1Oaep.c
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs5Pbkdf2.c
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7Sign.c
 create mode 100644 
CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7VerifyBase.c
 create mode 100644 
CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7VerifyCommon.c
 create mode 100644 
CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7VerifyEku.c
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptRsaExt.c
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptRsaPssSign.c
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptTs.c
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptX509.c
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Rand/CryptRand.c
 create mode 100644 CryptoPkg/Library/BaseCryptLibMbedTls/Rand/CryptRandTsc.c

-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117857): https://edk2.groups.io/g/devel/message/117857
Mute This Topic: https://groups.io/mt/105552830/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] ACPI table generators and ConfigurationManagerProtocol

[Marcin Juszkiewicz]

W dniu 12.04.2024 o 14:43, Marcin Juszkiewicz via groups.io pisze:

W dniu 8.10.2023 o 19:25, Marcin Juszkiewicz via groups.io pisze:

I hate replying to myself after half a year of not seeing any answer. So 
added some people from repo to Cc:


Can someone point me to documentation on how to use ACPI table 
generators and ConfigurationManagerProtocol?


https://github.com/tianocore/edk2/blob/master/DynamicTablesPkg/Readme.md 
tells the story and points to presentation from 2018 with some pictures 
in it.


Looks like the only documentation is the code itself. Is checkout of 
some old tree and looking change by change how it was done on some 
platform the easiest way to find out how to use it?


The more I look at */ConfigurationManager.[ch] the more I do not like 
the amount of static structures there. All those 
EDKII_PLATFORM_REPOSITORY_INFO covering 1000 lines looks like "how the 
hell I will find what to change in other piece of code".


Any doc pointers? Recommendations (other than "change your job")?


I have spent two days on trying ConfigurationManager and finally gave 
up. The idea behind may look interesting but implementation of it is 
terrible. And there is no documentation for it.


Edk2_Platform_Repo_Info struct full of other structs can be one thousand 
lines long. Then are functions to query those structs - which can take 
almost another thousand lines.


For hardware where amount of system components is known and does not 
change it may be tempting. For those like sbsa-ref/QemuSbsa it means 
extra code to fill struct of structs with hardware details (cpu nodes, 
gic nodes, cpu topology/hierarchy/cache info etc).


So I am going back to how "my" platform is done. Probably will learn 
AmlLib to create DSDT from C code.



And if someone considers using ConfigurationManager for their platform: 
please reconsider it as it may not be worth your time.



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117856): https://edk2.groups.io/g/devel/message/117856
Mute This Topic: https://groups.io/mt/105481311/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/1] BaseTool/Ecc: Fix incorrect parsing of variable initialisation

[PierreGondois]

Tested-by: Pierre Gondois 

On 4/15/24 20:59, levi.yun wrote:

If a global variable is initialised using a macro with multiple
arguments, ECC incorrectly parses the statement and reports the
macro arguments as variable declarations.

Example: In the following statement:
   STATIC INT WrongVariable = MACRO_VERSION(1, 0), NextVariable;
The logic in the ECC function GetIdentifierList() interprets the
above statement as declaration of three variables:
 1. 'WrongVariable = MACRO_VERSION(1,'
 2. '0)'
 3. 'NextVariable'
Following which NamingConventionCheckVariableName() reports an
error for "0)" stating an incorrect variable declaration as
below:
"ERROR - *The variable name [0)] does not follow the rules"

This patch fixes the parsing logic so that scenarios with macro
initialisations are handled correctly.

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
Cc: Sami Mujawar 
Cc: Pierre Gondois 
Signed-off-by: levi.yun 
---

The changes can be seen at 
https://github.com/LeviYeoReum/edk2/tree/levi/3057_fix_false_on_ecc_v2

  BaseTools/Source/Python/Ecc/c.py | 23 ++--
  1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/Ecc/c.py b/BaseTools/Source/Python/Ecc/c.py
index 
61ad084fcc5b85b5a2194afd8bb1a4b4b65fdaee..71dc0fcf884ee3d45a527f20844b697958df366c
 100644
--- a/BaseTools/Source/Python/Ecc/c.py
+++ b/BaseTools/Source/Python/Ecc/c.py
@@ -182,8 +182,27 @@ def GetIdentifierList():
  continue
  
  if var.Declarator.find('{') == -1:

-for decl in var.Declarator.split(','):
-DeclList = decl.split('=')
+DeclText = var.Declarator
+while (len(DeclText) > 0):
+AllocatorPos = DeclText.find('=')
+SplitPos = DeclText.find(',')
+
+if (SplitPos == -1):
+SplitPos = len(DeclText)
+elif (SplitPos > AllocatorPos):
+NextAllcatorPos = DeclText.find('=', AllocatorPos + 1)
+if (NextAllcatorPos == -1):
+NextAllcatorPos = len(DeclText)
+ParPos = DeclText.rfind(')', SplitPos, NextAllcatorPos)
+if (ParPos != -1):
+SplitPos = DeclText.find(',', ParPos)
+if (SplitPos == -1):
+SplitPos = ParPos + 1
+
+SubDeclText = DeclText[:SplitPos]
+DeclText = DeclText[SplitPos + 1:]
+
+DeclList = SubDeclText.split('=')
  Name = DeclList[0].strip()
  if ArrayPattern.match(Name):
  LSBPos = var.Declarator.find('[')
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117855): https://edk2.groups.io/g/devel/message/117855
Mute This Topic: https://groups.io/mt/105542888/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 1/1] MdeModulePkg: Warn if out of space when writing variables

[Gerd Hoffmann]

On Mon, Apr 15, 2024 at 09:46:37PM +0200, Oliver Steffen wrote:
> Emit a DEBUG_ERROR message if there is not enough flash variable left to
> write/update a variable. This condition is currently not logged
> appropriately in all cases, given that full variable store can easily
> render the system unbootable.
> This new message helps identifying this condition.
> 
> Cc: Bob Feng 
> Cc: Gerd Hoffmann 
> Cc: Jiewen Yao 
> Cc: Laszlo Ersek 
> Cc: Liming Gao 
> Cc: Rahul Kumar 
> Cc: Rebecca Cran 
> Cc: Yuwei Chen 
> 
> Signed-off-by: Oliver Steffen 
> Reviewed-by: Laszlo Ersek 

Reviewed-by: Gerd Hoffmann 

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117854): https://edk2.groups.io/g/devel/message/117854
Mute This Topic: https://groups.io/mt/105543779/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 03/13] UefiCpuPkg/SmmRelocationLib: Add library instance for OVMF

[Gerd Hoffmann]

On Mon, Apr 15, 2024 at 01:04:58PM +, Wu, Jiaxin wrote:
> Hi Gred,
> 
> Because:
> 1) The mode of the CPU check is different between the AMD & OVMF.
> OVMF: 
> CpuSaveState->x86.SMMRevId & 0X
> 
> AMD:
>  LMAValue = (UINT32)AsmReadMsr64 (EFER_ADDRESS) & LMA
> 
> 2) Existing SmBase configuration is different between the AMD & OVMF.
> OVMF:
>  AmdCpuState->x64.SMBASE = (UINT32)mSmBaseForAllCpus[CpuIndex];
> 
> AMD:   
>  if ((CpuSaveState->x86.SMMRevId & 0x) == 0) {
> CpuSaveState->x86.SMBASE = (UINT32)mSmBaseForAllCpus[CpuIndex];
>   } else {
> CpuSaveState->x64.SMBASE = (UINT32)mSmBaseForAllCpus[CpuIndex];
>   }
> 
> This series patch won't change the existing implementation code logic, so, we 
> need override one version for OVMF.

The real question is why do these differences exist and are they
actually needed.

I'd expect the CPU mode check return identical results.

The SmBase configuration for OVMF looks suspicious to me.  I'm wondering
whenever the OVMF code actually works in Ia32 builds ...

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117853): https://edk2.groups.io/g/devel/message/117853
Mute This Topic: https://groups.io/mt/105441992/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 00/10] Add SmmRelocationLib

[Gerd Hoffmann]

On Mon, Apr 15, 2024 at 09:30:11PM +0800, Wu, Jiaxin wrote:
> Intel plans to separate the smbase relocation logic from
> PiSmmCpuDxeSmm driver, and the related behavior will be
> moved to the new interface defined by the SmmRelocationLib
> class.
> 
> The SmmRelocationLib class provides the SmmRelocationInit()
> interface for platform to do the smbase relocation, which
> shall provide below 2 functionalities:
> 1. Relocate smbases for each processor.
> 2. Create the gSmmBaseHobGuid HOB.
> 
> With SmmRelocationLib, PiSmmCpuDxeSmm driver (which runs at
> a later phase) can be simplfied as below for SMM init:
> 1. Consume the gSmmBaseHobGuid HOB for the relocated smbases
> for each Processor.
> 2. Execute the early SMM Init.

How this was tested?
I can't even build this (without -D SMM_REQUIRE=TRUE).

/home/kraxel/projects/edk2/OvmfPkg/OvmfPkgX64.dsc(...): error 4000: Instance of 
library class [SmmRelocationLib] is not found
in [/home/kraxel/projects/edk2/OvmfPkg/PlatformPei/PlatformPei.inf] 
[X64]
consumed by module 
[/home/kraxel/projects/edk2/OvmfPkg/PlatformPei/PlatformPei.inf]

I doubt it passes CI.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117852): https://edk2.groups.io/g/devel/message/117852
Mute This Topic: https://groups.io/mt/105535804/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 3/3] uefi-sct/SctPkg: TCG2 Protocol: #pragma pack cleanup

[Heinrich Schuchardt]

On 4/16/24 00:23, Stuart Yoder wrote:

Fix compiler warning by adding #pragma pack() to close a pragma
section.  Also delete extraneous #pragma pack(1).

Signed-off-by: Stuart Yoder 


Reviewed-by: Heinrich Schuchardt 


---
  
uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h
 | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h
 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h
index deba13f21804..95307b7fa50f 100644
--- 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h
+++ 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/TCG2/BlackBoxTest/TCG2ProtocolBBTest.h
@@ -79,7 +79,6 @@ typedef struct {
UINT8  buffer[TEST_STRING_LEN];
  } TPM2B_MAX_BUFFER;
  
-#pragma pack(1)

  // TPM2B_DIGEST as defined in Table 73 of TPM Library Spec Part 2: Structures
  typedef struct {
UINT16 size;
@@ -110,7 +109,7 @@ typedef struct {
TPM2B_DIGEST data;
TPMT_TK_HASHCHECK validation;
  } TPM2_HASH_RESPONSE;
-#pragma
+#pragma pack()
  
  EFI_STATUS

  EFIAPI




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117851): https://edk2.groups.io/g/devel/message/117851
Mute This Topic: https://groups.io/mt/105546457/21656
Mute #pragma:https://edk2.groups.io/g/devel/mutehashtag/pragma
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/3] uefi-sct/SctPkg: TCG2 Protocol: correct definition of TPMT_HA struct

[Heinrich Schuchardt]

On 4/16/24 00:23, Stuart Yoder wrote:

The TPMT_HA struct defining event log hash algorithms was cut/pasted
from the TCG EFI Protocol specification which used a C struct
with a flexible array member as the last element.  This is incorrect
because TPMT_HA itself is used as an array element, and thus can't
be variable size.

Because the size of hash algorithms varies, this should have been
defined as a union of the sizes of supported hash algorithms.  This is
how is it done in the TPM Library specfication and in EDK2.

Signed-off-by: Stuart Yoder 
---
  uefi-sct/SctPkg/UEFI/Protocol/TCG2.h | 13 -
  1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/uefi-sct/SctPkg/UEFI/Protocol/TCG2.h 
b/uefi-sct/SctPkg/UEFI/Protocol/TCG2.h
index a83a84c33134..e42b8b347c05 100644
--- a/uefi-sct/SctPkg/UEFI/Protocol/TCG2.h
+++ b/uefi-sct/SctPkg/UEFI/Protocol/TCG2.h
@@ -51,6 +51,10 @@ Abstract:
  #define EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 0x0002
  
  #define HASH_NUMBER 0x04

+#define SHA1_DIGEST_SIZE20
+#define SHA256_DIGEST_SIZE  32


We should follow the TCG EFI Protocol Specification and support ShangMi 
3 in the test too.


#define SM3_256_DIGEST_SIZE  32


+#define SHA384_DIGEST_SIZE  48
+#define SHA512_DIGEST_SIZE  64
  
  typedef struct _EFI_TCG2_PROTOCOL EFI_TCG2_PROTOCOL;
  
@@ -117,9 +121,16 @@ typedef struct tdEFI_TCG2_EVENT {

UINT8 Event[];
  } EFI_TCG2_EVENT;
  
+typedef union {

+  UINT8 sha1[SHA1_DIGEST_SIZE];
+  UINT8 sha256[SHA256_DIGEST_SIZE];


UINT8 sm3_256[SM3_256_DIGEST_SIZE];

Best regards

Heinrich


+  UINT8 sha384[SHA384_DIGEST_SIZE];
+  UINT8 sha512[SHA512_DIGEST_SIZE];
+} TPMU_HA;
+
  typedef struct {
UINT16 hashAlg;
-  UINT8  digest[];
+  TPMU_HAdigest;
  } TPMT_HA;
  
  typedef struct tdTPML_DIGEST_VALUES {




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117850): https://edk2.groups.io/g/devel/message/117850
Mute This Topic: https://groups.io/mt/105546455/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-