Re: [edk2-devel] [PATCH V2 0/4] CryptoPkg: add more X509 functions.

["Sean" <spbrogan@outlook.com>]

[-- Attachment #1: Type: text/plain, Size: 4858 bytes --]

Thanks for the context

I am very glad to see you all adding unit tests with the feature enablement.

Thanks
Sean
________________________________
From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Monday, October 10, 2022 5:36:21 PM
To: Sean Brogan <spbrogan@outlook.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Zhang, Qi1 <qi1.zhang@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
Subject: RE: [edk2-devel] [PATCH V2 0/4] CryptoPkg: add more X509 functions.

HI Sean
You are right that the purpose is NOT to expose *all* APIs. Our purpose is still to export *necessary* APIs only.

This X.509 is for SPDM support. (https://www.dmtf.org/dsp/DSP0274). The BIOS may need collect the device identity (certificate) and validate the integrity of the certificate chain.

As such, the BIOS will check the device certificate based upon the definition in SPDM specification.
For example, SPDM 1.2.1 (https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_1.2.1.pdf) - 10.8.2 SPDM certificate requirements and recommendations, Table 33 — Required fields, Table 34 — Optional fields.


To summarize our recent change:
1) ECC in OpensslLib is to support more TLS cipher suite
2) ECC (EC-DH), BigNumber and TLS new APIs in BaseCryptoLib are to support WPA3
3) SHA-384 (HMAC/HKDF), ECDSA, AEAD (AES-GCM) and X.509 new APIs in BaseCryptoLib are for SPDM.

All functions are associated with PCD family bit. For old platform, if this new feature is not required, they can just be turned off.
We also evaluated to implement same API with other crypto lib, such as mbedtls. It is also feasible. As such, those APIs are openssl-agnostic.

Thank you
Yao Jiewen

> -----Original Message-----
> From: Sean Brogan <spbrogan@outlook.com>
> Sent: Tuesday, October 11, 2022 5:01 AM
> To: devel@edk2.groups.io; Zhang, Qi1 <qi1.zhang@intel.com>
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu@intel.com>; Jiang,
> Guomin <guomin.jiang@intel.com>
> Subject: Re: [edk2-devel] [PATCH V2 0/4] CryptoPkg: add more X509
> functions.
>
> Can you provide some context as to why we need to make all these x509
> functions external?
>
> BaseCryptLib was intended to simplify crypto usage and not be a full
> featured crypto library interface.
>
> At some point we might as well just open up the openssl export table and
> wrap that in a dynamically generated protocol/ppi.
>
> If this is intended to make an Edk2 crypto library api that is
> implementation agnostic but full featured then maybe you could do as Tls
> did which was create your own usage specific API/wrapper. Then CryptoPkg
> API surface will increase but it doesn't have to all be in one
> monolithic library.
>
>
> Thanks
>
> Sean
>
>
>
>
> On 10/10/2022 4:32 AM, Qi Zhang wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4082
> >
> > This patch serial is to add more CryptoX509 functions.
> >
> > Tested by:
> > 1. https://github.com/tianocore/edk2-staging/tree/DeviceSecurity.
> > 2. Unit test: CryptoPkg/Test/UnitTest/Library/BaseCryptLib/X509Tests.c
> >
> > Review PR: https://github.com/tianocore/edk2/pull/3380.
> >
> > V2 change: rename X509SetDateTime() to X509FormatDateTime().
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Xiaoyu Lu <xiaoyu1.lu@intel.com>
> > Cc: Guomin Jiang <guomin.jiang@intel.com>
> > Signed-off-by: Qi Zhang <qi1.zhang@intel.com>
> >
> > Qi Zhang (4):
> >    CryptoPkg: add new X509 function definition.
> >    CryptoPkg: add new X509 function.
> >    CryptoPkg: add new X509 function to Crypto Service.
> >    CryptoPkg: add Unit Test for X509 new function.
> >
> >   CryptoPkg/Driver/Crypto.c                     |  432 ++++++-
> >   CryptoPkg/Include/Library/BaseCryptLib.h      |  374 ++++++
> >   .../Pcd/PcdCryptoServiceFamilyEnable.h        |   34 +-
> >   CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 1036
> +++++++++++++++++
> >   .../Library/BaseCryptLib/Pk/CryptX509Null.c   |  429 +++++++
> >   .../BaseCryptLibNull/Pk/CryptX509Null.c       |  429 +++++++
> >   .../BaseCryptLibOnProtocolPpi/CryptLib.c      |  415 +++++++
> >   CryptoPkg/Private/Protocol/Crypto.h           |  390 +++++++
> >   .../BaseCryptLib/BaseCryptLibUnitTests.c      |    1 +
> >   .../Library/BaseCryptLib/TestBaseCryptLib.h   |    4 +
> >   .../BaseCryptLib/TestBaseCryptLibHost.inf     |    1 +
> >   .../BaseCryptLib/TestBaseCryptLibShell.inf    |    1 +
> >   .../UnitTest/Library/BaseCryptLib/X509Tests.c |  631 ++++++++++
> >   13 files changed, 4166 insertions(+), 11 deletions(-)
> >   create mode 100644
> CryptoPkg/Test/UnitTest/Library/BaseCryptLib/X509Tests.c
> >

[-- Attachment #2: Type: text/html, Size: 7435 bytes --]