[v2] CryptoPkg: add more X509 functions.

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

Posted by Qi Zhang 1 year, 6 months ago

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

-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94931): https://edk2.groups.io/g/devel/message/94931
Mute This Topic: https://groups.io/mt/94234101/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Sean 1 year, 6 months ago

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
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94952): https://edk2.groups.io/g/devel/message/94952
Mute This Topic: https://groups.io/mt/94234101/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Yao, Jiewen 1 year, 6 months ago

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
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94961): https://edk2.groups.io/g/devel/message/94961
Mute This Topic: https://groups.io/mt/94234101/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Sean 1 year, 6 months ago

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
> >

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#94979): https://edk2.groups.io/g/devel/message/94979
Mute This Topic: https://groups.io/mt/94234101/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-