Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add need merge mbedtls(AesGcm/Pem(only RSA)/X509(not EC)/RSA/PKCS5/7/TS/Auth) #5552

Closed
wants to merge 11 commits into from
Closed

Add need merge mbedtls(AesGcm/Pem(only RSA)/X509(not EC)/RSA/PKCS5/7/TS/Auth) #5552

wants to merge 11 commits into from

Conversation

Wenxing-hou
Copy link
Member

No description provided.

@Wenxing-hou Wenxing-hou force-pushed the add_need_merge_mbedtls branch 10 times, most recently from 4050c90 to 9ec285d Compare April 19, 2024 11:27
liyi77
liyi77 reviewed Apr 22, 2024
View reviewed changes
CryptoPkg/CryptoPkg.ci.yaml Outdated Show resolved Hide resolved
liyi77
liyi77 reviewed Apr 22, 2024
View reviewed changes
CryptoPkg/Library/BaseCryptLibMbedTls/Cipher/CryptAeadAesGcm.c Outdated Show resolved Hide resolved
CryptoPkg/Library/BaseCryptLibMbedTls/Pem/CryptPem.c Outdated Show resolved Hide resolved
CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7Sign.c Outdated Show resolved Hide resolved
CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7VerifyCommon.c Outdated Show resolved Hide resolved
CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7VerifyCommon.c Outdated Show resolved Hide resolved
CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptPkcs7VerifyCommon.c Outdated
// Check whether input P7Data is a wrapped ContentInfo structure or not.
//
Wrapped = FALSE;
if ((P7Data[4] == 0x06) && (P7Data[5] == 0x09)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Magic number, is there any define in Mbedtls?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I have changed the hardcode numbers.

CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptRsaExt.c Outdated Show resolved Hide resolved
CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptX509.c Outdated Show resolved Hide resolved
CryptoPkg/Library/BaseCryptLibMbedTls/Rand/CryptRand.c Outdated Show resolved Hide resolved
CryptoPkg/Library/BaseCryptLibMbedTls/Cipher/CryptAeadAesGcm.c Outdated
OUT UINTN *DataOutSize
)
{
mbedtls_gcm_context ctx;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

format issue ctx.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I have fixed all the format issue.

@Wenxing-hou Wenxing-hou force-pushed the add_need_merge_mbedtls branch 2 times, most recently from b606d11 to 9e08e39 Compare April 22, 2024 13:14
@SergiiDmytruk SergiiDmytruk mentioned this pull request May 1, 2024
Open
@Wenxing-hou Wenxing-hou force-pushed the add_need_merge_mbedtls branch 3 times, most recently from c129734 to bde5eeb Compare May 5, 2024 15:20
liyi77
liyi77 reviewed May 6, 2024
View reviewed changes
CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptRsaExt.c
IN VOID *RsaContext,
IN CONST UINT8 *MessageHash,
IN UINTN HashSize,
OUT UINT8 *Signature,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should check Signature == NULL

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I have added the check. And the unit_test also passed.

CryptoPkg/Library/BaseCryptLibMbedTls/Pk/CryptX509.c Outdated
if (Ret == 0) {
return TRUE;
} else {
if ((mbedtls_x509_crt *)*X509Stack == NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meaningless check, *X509Stack always be non-NULL when need to free.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I have changed the logic. Thanks.

@Wenxing-hou Wenxing-hou force-pushed the add_need_merge_mbedtls branch 4 times, most recently from e6557e8 to 8da4c4a Compare May 8, 2024 06:27
@Wenxing-hou
CryptoPkg: Add AeadAesGcm based on Mbedtls
201cd33 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

AeadAesGcm implementation based on Mbedtls.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Yi Li <yi1.li@intel.com>
Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>
@Wenxing-hou
CryptoPkg: Add rand function for BaseCryptLibMbedTls
38bcbca 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

Add rand function for BaseCryptLibMbedTls.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Yi Li <yi1.li@intel.com>
Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>
@Wenxing-hou
CryptoPkg: Add Pem APIs based on Mbedtls
f7eab62 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

Implement Pem API based on Mbedtls.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Yi Li <yi1.li@intel.com>
Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>
@Wenxing-hou
CryptoPkg: Add X509 functions based on Mbedtls
632de4c 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

X.509 Certificate Handler Wrapper Implementation over MbedTLS.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Yi Li <yi1.li@intel.com>
Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>
@Wenxing-hou
CryptoPkg: Add Pkcs7 related functions based on Mbedtls
c5de418 
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 <jiewen.yao@intel.com>
Cc: Yi Li <yi1.li@intel.com>
Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>
@Wenxing-hou
CryptoPkg: Add Pkcs5 functions based on Mbedtls
04bf0d5 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

PBKDF2 Key Derivation Function Wrapper Implementation over MbedTLS.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Yi Li <yi1.li@intel.com>
Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>
@Wenxing-hou
CryptoPkg: Add more RSA related functions based on Mbedtls
a9914e0 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

Implement more RSA functions such as RsaPkcs1Sign based Mbedlts.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Yi Li <yi1.li@intel.com>
Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>
@Wenxing-hou
CryptoPkg: Add AuthenticodeVerify based on Mbedtls
f054ae2 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

Implement AuthenticodeVerify based on Mbedtls.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Yi Li <yi1.li@intel.com>
Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>
@Wenxing-hou
CryptoPkg: Add ImageTimestampVerify based on Mbedtls
6c68c35 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

Timestamp Countersignature Verification implementaion based on Mbedtls.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Yi Li <yi1.li@intel.com>
Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>
@Wenxing-hou
CryptoPkg: Update *.inf in BaseCryptLibMbedTls
cb5762d 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

Update all *.inf in BaseCryptLibMbedTls based on new implementation.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Yi Li <yi1.li@intel.com>
Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>
@Wenxing-hou
Add SHA3/SM3 functions with openssl for Mbedtls
d3ad1b2 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4177

Because the Mbedlts 3.3.0 doesn't have SHA3 and Sm3, the SHA3 and Sm3
implementaion based on Openssl.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Yi Li <yi1.li@intel.com>
Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liyi77 liyi77 liyi77 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Wenxing-hou @liyi77