From: "Heng Luo" <heng.luo@intel.com>
To: "Li, Yi1" <yi1.li@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Tan, Ming" <ming.tan@intel.com>
Subject: Re: [staging/crypto-new-api PATCH] CryptoPkg: Fix issues from crypto code review.
Date: Fri, 15 Jul 2022 02:13:16 +0000 [thread overview]
Message-ID: <SN6PR11MB275288CED8A13326C4F8BBA7938B9@SN6PR11MB2752.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20220715020711.1190-1-yi1.li@intel.com>
Reviewed-by: Heng Luo <heng.luo@intel.com>
> -----Original Message-----
> From: Li, Yi1 <yi1.li@intel.com>
> Sent: Friday, July 15, 2022 10:07 AM
> To: devel@edk2.groups.io
> Cc: Li, Yi1 <yi1.li@intel.com>; Tan, Ming <ming.tan@intel.com>; Luo, Heng
> <heng.luo@intel.com>
> Subject: [staging/crypto-new-api PATCH] CryptoPkg: Fix issues from crypto code
> review.
>
> Details:
> 1. Some APIs need more detail comment.
> 2. Correct BnRShift() param order.
> 3. Remove unsecure ECC curve from GroupToNid().
> 4. Add full public key validating procedures to EcDhDeriveSecret().
>
> Cc: Ming Tan <ming.tan@intel.com>
> Cc: Heng Luo <heng.luo@intel.com>
> Signed-off-by: Yi Li <yi1.li@intel.com>
>
> ---
> CryptoPkg/Driver/Crypto.c | 31 ++++++++++++++++++++---------
> --
> CryptoPkg/Include/Library/BaseCryptLib.h | 31
> ++++++++++++++++++++-----------
> CryptoPkg/Library/BaseCryptLib/Bn/CryptBn.c | 7 ++++---
> CryptoPkg/Library/BaseCryptLib/Bn/CryptBnNull.c | 4 +++-
> CryptoPkg/Library/BaseCryptLib/Ec/CryptEc.c | 61
> ++++++++++++++++++++++++++++++++++---------------------------
> CryptoPkg/Library/BaseCryptLib/Ec/CryptEcNull.c | 27
> +++++++++++++++++----------
> CryptoPkg/Library/BaseCryptLibNull/Bn/CryptBnNull.c | 4 +++-
> CryptoPkg/Library/BaseCryptLibNull/Ec/CryptEcNull.c | 27
> +++++++++++++++++----------
> CryptoPkg/Library/BaseCryptLibOnProtocolPpi/CryptLib.c | 31
> ++++++++++++++++++++-----------
> CryptoPkg/Private/Protocol/Crypto.h | 31 ++++++++++++++++++++--
> ---------
> 10 files changed, 158 insertions(+), 96 deletions(-)
>
> diff --git a/CryptoPkg/Driver/Crypto.c b/CryptoPkg/Driver/Crypto.c index
> de422b7f53..10a0ce8800 100644
> --- a/CryptoPkg/Driver/Crypto.c
> +++ b/CryptoPkg/Driver/Crypto.c
> @@ -4962,7 +4962,6 @@ CryptoServiceBigNumValueOne (
> @param[out] BnRes The result.
>
> @retval EFI_SUCCESS On success.
> - @retval EFI_OUT_OF_RESOURCES In case of internal allocation failures.
> @retval EFI_PROTOCOL_ERROR Otherwise.
> **/
> EFI_STATUS
> @@ -5051,6 +5050,9 @@ CryptoServiceBigNumContextFree (
>
> @param[in] Bn Big number to set.
> @param[in] Val Value to set.
> +
> + @retval EFI_SUCCESS On success.
> + @retval EFI_PROTOCOL_ERROR Otherwise.
> **/
> EFI_STATUS
> EFIAPI
> @@ -5092,7 +5094,7 @@ CryptoServiceBigNumAddMod (
> using EcGroupFree() function.
>
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
>
> @retval EcGroup object On success.
> @retval NULL On failure.
> @@ -5114,8 +5116,8 @@ CryptoServiceEcGroupInit (
>
> @param[in] EcGroup EC group object.
> @param[out] BnPrime Group prime number.
> - @param[out] BnA A coofecient.
> - @param[out] BnB B coofecient.
> + @param[out] BnA A coefficient.
> + @param[out] BnB B coefficient.
> @param[in] BnCtx BN context.
>
> @retval EFI_SUCCESS On success.
> @@ -5426,13 +5428,14 @@ CryptoServiceEcPointSetCompressedCoordinates (
> /**
> Generate a key using ECDH algorithm. Please note, this function uses
> pseudo random number generator. The caller must make sure RandomSeed()
> - funtion was properly called before.
> + function was properly called before.
>
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
> @param[out] PKey Pointer to an object that will hold the ECDH key.
>
> @retval EFI_SUCCESS On success.
> + @retval EFI_UNSUPPORTED ECC group not supported.
> @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> @@ -5466,8 +5469,9 @@ CryptoServiceEcDhKeyFree (
> @param[in] PKey ECDH Key object.
> @param[out] EcPoint Properly initialized EC Point to hold the public key.
>
> - @retval EFI_SUCCESS On success.
> - @retval EFI_PROTOCOL_ERROR On failure.
> + @retval EFI_SUCCESS On success.
> + @retval EFI_INVALID_PARAMETER EcPoint should be initialized properly.
> + @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> EFIAPI
> @@ -5484,15 +5488,20 @@ CryptoServiceEcDhGetPubKey (
>
> @param[in] PKey ECDH Key object.
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
> @param[in] EcPointPublic Peer public key.
> @param[out] SecretSize On success, holds secret size.
> @param[out] Secret On success, holds the derived secret.
> Should be freed by caller using FreePool()
> function.
>
> - @retval EFI_SUCCESS On success.
> - @retval EFI_PROTOCOL_ERROR On failure.
> + @retval EFI_SUCCESS On success.
> + @retval EFI_UNSUPPORTED ECC group not supported.
> + @retval EFI_INVALID_PARAMETER One or more of the following conditions is
> TRUE:
> + Secret is NULL.
> + SecretSize is NULL.
> + Public key in EcPointPublic is invalid.
> + @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> EFIAPI
> diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h
> b/CryptoPkg/Include/Library/BaseCryptLib.h
> index 8fcb496c40..0de9f0739e 100644
> --- a/CryptoPkg/Include/Library/BaseCryptLib.h
> +++ b/CryptoPkg/Include/Library/BaseCryptLib.h
> @@ -2723,7 +2723,6 @@ BigNumValueOne (
> @param[out] BnRes The result.
>
> @retval EFI_SUCCESS On success.
> - @retval EFI_OUT_OF_RESOURCES In case of internal allocation failures.
> @retval EFI_PROTOCOL_ERROR Otherwise.
> **/
> EFI_STATUS
> @@ -2797,6 +2796,9 @@ BigNumContextFree (
>
> @param[in] Bn Big number to set.
> @param[in] Val Value to set.
> +
> + @retval EFI_SUCCESS On success.
> + @retval EFI_PROTOCOL_ERROR Otherwise.
> **/
> EFI_STATUS
> EFIAPI
> @@ -2832,7 +2834,7 @@ BigNumAddMod (
> using EcGroupFree() function.
>
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
>
> @retval EcGroup object On success.
> @retval NULL On failure.
> @@ -2851,8 +2853,8 @@ EcGroupInit (
>
> @param[in] EcGroup EC group object.
> @param[out] BnPrime Group prime number.
> - @param[out] BnA A coofecient.
> - @param[out] BnB B coofecient.
> + @param[out] BnA A coefficient.
> + @param[out] BnB B coefficient.
> @param[in] BnCtx BN context.
>
> @retval EFI_SUCCESS On success.
> @@ -3121,13 +3123,14 @@ EcPointSetCompressedCoordinates (
> /**
> Generate a key using ECDH algorithm. Please note, this function uses
> pseudo random number generator. The caller must make sure RandomSeed()
> - funtion was properly called before.
> + function was properly called before.
>
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
> @param[out] PKey Pointer to an object that will hold the ECDH key.
>
> @retval EFI_SUCCESS On success.
> + @retval EFI_UNSUPPORTED ECC group not supported.
> @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> @@ -3155,8 +3158,9 @@ EcDhKeyFree (
> @param[in] PKey ECDH Key object.
> @param[out] EcPoint Properly initialized EC Point to hold the public key.
>
> - @retval EFI_SUCCESS On success.
> - @retval EFI_PROTOCOL_ERROR On failure.
> + @retval EFI_SUCCESS On success.
> + @retval EFI_INVALID_PARAMETER EcPoint should be initialized properly.
> + @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> EFIAPI
> @@ -3170,15 +3174,20 @@ EcDhGetPubKey (
>
> @param[in] PKey ECDH Key object.
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
> @param[in] EcPointPublic Peer public key.
> @param[out] SecretSize On success, holds secret size.
> @param[out] Secret On success, holds the derived secret.
> Should be freed by caller using FreePool()
> function.
>
> - @retval EFI_SUCCESS On success.
> - @retval EFI_PROTOCOL_ERROR On failure.
> + @retval EFI_SUCCESS On success.
> + @retval EFI_UNSUPPORTED ECC group not supported.
> + @retval EFI_INVALID_PARAMETER One or more of the following conditions is
> TRUE:
> + Secret is NULL.
> + SecretSize is NULL.
> + Public key in EcPointPublic is invalid.
> + @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> EFIAPI
> diff --git a/CryptoPkg/Library/BaseCryptLib/Bn/CryptBn.c
> b/CryptoPkg/Library/BaseCryptLib/Bn/CryptBn.c
> index 3e43492a56..b6411cd541 100644
> --- a/CryptoPkg/Library/BaseCryptLib/Bn/CryptBn.c
> +++ b/CryptoPkg/Library/BaseCryptLib/Bn/CryptBn.c
> @@ -442,7 +442,6 @@ BigNumValueOne (
> @param[out] BnRes The result.
>
> @retval EFI_SUCCESS On success.
> - @retval EFI_OUT_OF_RESOURCES In case of internal allocation failures.
> @retval EFI_PROTOCOL_ERROR Otherwise.
> **/
> EFI_STATUS
> @@ -453,8 +452,7 @@ BigNumRShift (
> OUT VOID *BnRes
> )
> {
> - // BN_rshift() does not modify the first argument, so we remove const.
> - if (BN_rshift ((BIGNUM *)Bn, BnRes, (int)n) == 1) {
> + if (BN_rshift (BnRes, Bn, (int)n) == 1) {
> return EFI_SUCCESS;
> } else {
> return EFI_PROTOCOL_ERROR;
> @@ -547,6 +545,9 @@ BigNumContextFree (
>
> @param[in] Bn Big number to set.
> @param[in] Val Value to set.
> +
> + @retval EFI_SUCCESS On success.
> + @retval EFI_PROTOCOL_ERROR Otherwise.
> **/
> EFI_STATUS
> EFIAPI
> diff --git a/CryptoPkg/Library/BaseCryptLib/Bn/CryptBnNull.c
> b/CryptoPkg/Library/BaseCryptLib/Bn/CryptBnNull.c
> index 4a27433a0e..4d2fa039df 100644
> --- a/CryptoPkg/Library/BaseCryptLib/Bn/CryptBnNull.c
> +++ b/CryptoPkg/Library/BaseCryptLib/Bn/CryptBnNull.c
> @@ -395,7 +395,6 @@ BigNumValueOne (
> @param[out] BnRes The result.
>
> @retval EFI_SUCCESS On success.
> - @retval EFI_OUT_OF_RESOURCES In case of internal allocation failures.
> @retval EFI_PROTOCOL_ERROR Otherwise.
> **/
> EFI_STATUS
> @@ -487,6 +486,9 @@ BigNumContextFree (
>
> @param[in] Bn Big number to set.
> @param[in] Val Value to set.
> +
> + @retval EFI_SUCCESS On success.
> + @retval EFI_PROTOCOL_ERROR Otherwise.
> **/
> EFI_STATUS
> EFIAPI
> diff --git a/CryptoPkg/Library/BaseCryptLib/Ec/CryptEc.c
> b/CryptoPkg/Library/BaseCryptLib/Ec/CryptEc.c
> index 4d1aab8d32..90d1b8bce7 100644
> --- a/CryptoPkg/Library/BaseCryptLib/Ec/CryptEc.c
> +++ b/CryptoPkg/Library/BaseCryptLib/Ec/CryptEc.c
> @@ -21,13 +21,13 @@
> #include <openssl/ec.h>
>
> /**
> - Temp comment.
> + Return the Nid of certain ECC group.
>
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
>
> - @retval EcGroup object On success.
> - @retval NULL On failure.
> + @retval !=-1 On success.
> + @retval -1 ECC group not supported.
> **/
> STATIC
> INT32
> @@ -47,12 +47,6 @@ GroupToNid (
> case 21:
> Nid = NID_secp521r1;
> break;
> - case 25:
> - Nid = NID_X9_62_prime192v1;
> - break;
> - case 26:
> - Nid = NID_secp224r1;
> - break;
> default:
> return -1;
> }
> @@ -66,7 +60,7 @@ GroupToNid (
> using EcGroupFree() function.
>
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
>
> @retval EcGroup object On success.
> @retval NULL On failure.
> @@ -96,8 +90,8 @@ EcGroupInit (
>
> @param[in] EcGroup EC group object.
> @param[out] BnPrime Group prime number.
> - @param[out] BnA A coofecient.
> - @param[out] BnB B coofecient.
> + @param[out] BnA A coefficient.
> + @param[out] BnB B coefficient.
> @param[in] BnCtx BN context.
>
> @retval EFI_SUCCESS On success.
> @@ -218,7 +212,7 @@ EcPointGetAffineCoordinates (
> )
> {
> return EC_POINT_get_affine_coordinates (EcGroup, EcPoint, BnX, BnY, BnCtx) ?
> - EFI_SUCCESS : EFI_INVALID_PARAMETER;
> + EFI_SUCCESS : EFI_PROTOCOL_ERROR;
> }
>
> /**
> @@ -244,7 +238,7 @@ EcPointSetAffineCoordinates (
> )
> {
> return EC_POINT_set_affine_coordinates (EcGroup, EcPoint, BnX, BnY, BnCtx) ?
> - EFI_SUCCESS : EFI_INVALID_PARAMETER;
> + EFI_SUCCESS : EFI_PROTOCOL_ERROR;
> }
>
> /**
> @@ -271,7 +265,7 @@ EcPointAdd (
> )
> {
> return EC_POINT_add (EcGroup, EcPointResult, EcPointA, EcPointB, BnCtx) ?
> - EFI_SUCCESS : EFI_INVALID_PARAMETER;
> + EFI_SUCCESS : EFI_PROTOCOL_ERROR;
> }
>
> /**
> @@ -298,7 +292,7 @@ EcPointMul (
> )
> {
> return EC_POINT_mul (EcGroup, EcPointResult, NULL, EcPoint, BnPScalar,
> BnCtx) ?
> - EFI_SUCCESS : EFI_INVALID_PARAMETER;
> + EFI_SUCCESS : EFI_PROTOCOL_ERROR;
> }
>
> /**
> @@ -320,7 +314,7 @@ EcPointInvert (
> )
> {
> return EC_POINT_invert (EcGroup, EcPoint, BnCtx) ?
> - EFI_SUCCESS : EFI_INVALID_PARAMETER;
> + EFI_SUCCESS : EFI_PROTOCOL_ERROR;
> }
>
> /**
> @@ -414,19 +408,20 @@ EcPointSetCompressedCoordinates (
> )
> {
> return EC_POINT_set_compressed_coordinates (EcGroup, EcPoint, BnX, YBit,
> BnCtx) ?
> - EFI_SUCCESS : EFI_INVALID_PARAMETER;
> + EFI_SUCCESS : EFI_PROTOCOL_ERROR;
> }
>
> /**
> Generate a key using ECDH algorithm. Please note, this function uses
> pseudo random number generator. The caller must make sure RandomSeed()
> - funtion was properly called before.
> + function was properly called before.
>
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
> @param[out] PKey Pointer to an object that will hold the ECDH key.
>
> @retval EFI_SUCCESS On success.
> + @retval EFI_UNSUPPORTED ECC group not supported.
> @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> @@ -508,8 +503,9 @@ EcDhKeyFree (
> @param[in] PKey ECDH Key object.
> @param[out] EcPoint Properly initialized EC Point to hold the public key.
>
> - @retval EFI_SUCCESS On success.
> - @retval EFI_PROTOCOL_ERROR On failure.
> + @retval EFI_SUCCESS On success.
> + @retval EFI_INVALID_PARAMETER EcPoint should be initialized properly.
> + @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> EFIAPI
> @@ -553,15 +549,21 @@ out:
>
> @param[in] PKey ECDH Key object.
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> - @param[in] EcPointPublic Peer public key.
> + Description" attribute registry for RFC 2409).
> + @param[in] EcPointPublic Peer public key. Certain sanity checks on the key
> + will be performed to confirm that it is valid.
> @param[out] SecretSize On success, holds secret size.
> @param[out] Secret On success, holds the derived secret.
> Should be freed by caller using FreePool()
> function.
>
> - @retval EFI_SUCCESS On success.
> - @retval EFI_PROTOCOL_ERROR On failure.
> + @retval EFI_SUCCESS On success.
> + @retval EFI_UNSUPPORTED ECC group not supported.
> + @retval EFI_INVALID_PARAMETER One or more of the following conditions is
> TRUE:
> + Secret is NULL.
> + SecretSize is NULL.
> + Public key in EcPointPublic is invalid.
> + @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> EFIAPI
> @@ -605,6 +607,11 @@ EcDhDeriveSecret (
> goto fail;
> }
>
> + if (!EC_KEY_check_key (EcKey)) {
> + Status = EFI_INVALID_PARAMETER;
> + goto fail;
> + }
> +
> Ctx = EVP_PKEY_CTX_new (PKey, NULL);
> if ((Ctx == NULL) || (EVP_PKEY_derive_init (Ctx) != 1) ||
> (EVP_PKEY_derive_set_peer (Ctx, PeerKey) != 1) || diff --git
> a/CryptoPkg/Library/BaseCryptLib/Ec/CryptEcNull.c
> b/CryptoPkg/Library/BaseCryptLib/Ec/CryptEcNull.c
> index 2d7e5db464..e7fe378095 100644
> --- a/CryptoPkg/Library/BaseCryptLib/Ec/CryptEcNull.c
> +++ b/CryptoPkg/Library/BaseCryptLib/Ec/CryptEcNull.c
> @@ -15,7 +15,7 @@
> using EcGroupFree() function.
>
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
>
> @retval EcGroup object On success.
> @retval NULL On failure.
> @@ -38,8 +38,8 @@ EcGroupInit (
>
> @param[in] EcGroup EC group object.
> @param[out] BnPrime Group prime number.
> - @param[out] BnA A coofecient.
> - @param[out] BnB B coofecient.
> + @param[out] BnA A coefficient.
> + @param[out] BnB B coefficient.
> @param[in] BnCtx BN context.
>
> @retval EFI_SUCCESS On success.
> @@ -362,13 +362,14 @@ EcPointSetCompressedCoordinates (
> /**
> Generate a key using ECDH algorithm. Please note, this function uses
> pseudo random number generator. The caller must make sure RandomSeed()
> - funtion was properly called before.
> + function was properly called before.
>
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
> @param[out] PKey Pointer to an object that will hold the ECDH key.
>
> @retval EFI_SUCCESS On success.
> + @retval EFI_UNSUPPORTED ECC group not supported.
> @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> @@ -403,8 +404,9 @@ EcDhKeyFree (
> @param[in] PKey ECDH Key object.
> @param[out] EcPoint Properly initialized EC Point to hold the public key.
>
> - @retval EFI_SUCCESS On success.
> - @retval EFI_PROTOCOL_ERROR On failure.
> + @retval EFI_SUCCESS On success.
> + @retval EFI_INVALID_PARAMETER EcPoint should be initialized properly.
> + @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> EFIAPI
> @@ -422,15 +424,20 @@ EcDhGetPubKey (
>
> @param[in] PKey ECDH Key object.
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
> @param[in] EcPointPublic Peer public key.
> @param[out] SecretSize On success, holds secret size.
> @param[out] Secret On success, holds the derived secret.
> Should be freed by caller using FreePool()
> function.
>
> - @retval EFI_SUCCESS On success.
> - @retval EFI_PROTOCOL_ERROR On failure.
> + @retval EFI_SUCCESS On success.
> + @retval EFI_UNSUPPORTED ECC group not supported.
> + @retval EFI_INVALID_PARAMETER One or more of the following conditions is
> TRUE:
> + Secret is NULL.
> + SecretSize is NULL.
> + Public key in EcPointPublic is invalid.
> + @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> EFIAPI
> diff --git a/CryptoPkg/Library/BaseCryptLibNull/Bn/CryptBnNull.c
> b/CryptoPkg/Library/BaseCryptLibNull/Bn/CryptBnNull.c
> index 4a27433a0e..4d2fa039df 100644
> --- a/CryptoPkg/Library/BaseCryptLibNull/Bn/CryptBnNull.c
> +++ b/CryptoPkg/Library/BaseCryptLibNull/Bn/CryptBnNull.c
> @@ -395,7 +395,6 @@ BigNumValueOne (
> @param[out] BnRes The result.
>
> @retval EFI_SUCCESS On success.
> - @retval EFI_OUT_OF_RESOURCES In case of internal allocation failures.
> @retval EFI_PROTOCOL_ERROR Otherwise.
> **/
> EFI_STATUS
> @@ -487,6 +486,9 @@ BigNumContextFree (
>
> @param[in] Bn Big number to set.
> @param[in] Val Value to set.
> +
> + @retval EFI_SUCCESS On success.
> + @retval EFI_PROTOCOL_ERROR Otherwise.
> **/
> EFI_STATUS
> EFIAPI
> diff --git a/CryptoPkg/Library/BaseCryptLibNull/Ec/CryptEcNull.c
> b/CryptoPkg/Library/BaseCryptLibNull/Ec/CryptEcNull.c
> index 2d7e5db464..e7fe378095 100644
> --- a/CryptoPkg/Library/BaseCryptLibNull/Ec/CryptEcNull.c
> +++ b/CryptoPkg/Library/BaseCryptLibNull/Ec/CryptEcNull.c
> @@ -15,7 +15,7 @@
> using EcGroupFree() function.
>
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
>
> @retval EcGroup object On success.
> @retval NULL On failure.
> @@ -38,8 +38,8 @@ EcGroupInit (
>
> @param[in] EcGroup EC group object.
> @param[out] BnPrime Group prime number.
> - @param[out] BnA A coofecient.
> - @param[out] BnB B coofecient.
> + @param[out] BnA A coefficient.
> + @param[out] BnB B coefficient.
> @param[in] BnCtx BN context.
>
> @retval EFI_SUCCESS On success.
> @@ -362,13 +362,14 @@ EcPointSetCompressedCoordinates (
> /**
> Generate a key using ECDH algorithm. Please note, this function uses
> pseudo random number generator. The caller must make sure RandomSeed()
> - funtion was properly called before.
> + function was properly called before.
>
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
> @param[out] PKey Pointer to an object that will hold the ECDH key.
>
> @retval EFI_SUCCESS On success.
> + @retval EFI_UNSUPPORTED ECC group not supported.
> @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> @@ -403,8 +404,9 @@ EcDhKeyFree (
> @param[in] PKey ECDH Key object.
> @param[out] EcPoint Properly initialized EC Point to hold the public key.
>
> - @retval EFI_SUCCESS On success.
> - @retval EFI_PROTOCOL_ERROR On failure.
> + @retval EFI_SUCCESS On success.
> + @retval EFI_INVALID_PARAMETER EcPoint should be initialized properly.
> + @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> EFIAPI
> @@ -422,15 +424,20 @@ EcDhGetPubKey (
>
> @param[in] PKey ECDH Key object.
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
> @param[in] EcPointPublic Peer public key.
> @param[out] SecretSize On success, holds secret size.
> @param[out] Secret On success, holds the derived secret.
> Should be freed by caller using FreePool()
> function.
>
> - @retval EFI_SUCCESS On success.
> - @retval EFI_PROTOCOL_ERROR On failure.
> + @retval EFI_SUCCESS On success.
> + @retval EFI_UNSUPPORTED ECC group not supported.
> + @retval EFI_INVALID_PARAMETER One or more of the following conditions is
> TRUE:
> + Secret is NULL.
> + SecretSize is NULL.
> + Public key in EcPointPublic is invalid.
> + @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> EFIAPI
> diff --git a/CryptoPkg/Library/BaseCryptLibOnProtocolPpi/CryptLib.c
> b/CryptoPkg/Library/BaseCryptLibOnProtocolPpi/CryptLib.c
> index 548116abb4..0410067c9d 100644
> --- a/CryptoPkg/Library/BaseCryptLibOnProtocolPpi/CryptLib.c
> +++ b/CryptoPkg/Library/BaseCryptLibOnProtocolPpi/CryptLib.c
> @@ -4069,7 +4069,6 @@ BigNumValueOne (
> @param[out] BnRes The result.
>
> @retval EFI_SUCCESS On success.
> - @retval EFI_OUT_OF_RESOURCES In case of internal allocation failures.
> @retval EFI_PROTOCOL_ERROR Otherwise.
> **/
> EFI_STATUS
> @@ -4158,6 +4157,9 @@ BigNumContextFree (
>
> @param[in] Bn Big number to set.
> @param[in] Val Value to set.
> +
> + @retval EFI_SUCCESS On success.
> + @retval EFI_PROTOCOL_ERROR Otherwise.
> **/
> EFI_STATUS
> EFIAPI
> @@ -4199,7 +4201,7 @@ BigNumAddMod (
> using EcGroupFree() function.
>
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
>
> @retval EcGroup object On success.
> @retval NULL On failure.
> @@ -4221,8 +4223,8 @@ EcGroupInit (
>
> @param[in] EcGroup EC group object.
> @param[out] BnPrime Group prime number.
> - @param[out] BnA A coofecient.
> - @param[out] BnB B coofecient.
> + @param[out] BnA A coefficient.
> + @param[out] BnB B coefficient.
> @param[in] BnCtx BN context.
>
> @retval EFI_SUCCESS On success.
> @@ -4533,13 +4535,14 @@ EcPointSetCompressedCoordinates (
> /**
> Generate a key using ECDH algorithm. Please note, this function uses
> pseudo random number generator. The caller must make sure RandomSeed()
> - funtion was properly called before.
> + function was properly called before.
>
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
> @param[out] PKey Pointer to an object that will hold the ECDH key.
>
> @retval EFI_SUCCESS On success.
> + @retval EFI_UNSUPPORTED ECC group not supported.
> @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> @@ -4573,8 +4576,9 @@ EcDhKeyFree (
> @param[in] PKey ECDH Key object.
> @param[out] EcPoint Properly initialized EC Point to hold the public key.
>
> - @retval EFI_SUCCESS On success.
> - @retval EFI_PROTOCOL_ERROR On failure.
> + @retval EFI_SUCCESS On success.
> + @retval EFI_INVALID_PARAMETER EcPoint should be initialized properly.
> + @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> EFIAPI
> @@ -4591,15 +4595,20 @@ EcDhGetPubKey (
>
> @param[in] PKey ECDH Key object.
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409).
> + Description" attribute registry for RFC 2409).
> @param[in] EcPointPublic Peer public key.
> @param[out] SecretSize On success, holds secret size.
> @param[out] Secret On success, holds the derived secret.
> Should be freed by caller using FreePool()
> function.
>
> - @retval EFI_SUCCESS On success.
> - @retval EFI_PROTOCOL_ERROR On failure.
> + @retval EFI_SUCCESS On success.
> + @retval EFI_UNSUPPORTED ECC group not supported.
> + @retval EFI_INVALID_PARAMETER One or more of the following conditions is
> TRUE:
> + Secret is NULL.
> + SecretSize is NULL.
> + Public key in EcPointPublic is invalid.
> + @retval EFI_PROTOCOL_ERROR On failure.
> **/
> EFI_STATUS
> EFIAPI
> diff --git a/CryptoPkg/Private/Protocol/Crypto.h
> b/CryptoPkg/Private/Protocol/Crypto.h
> index 1b31714d77..1cf5d18cc3 100644
> --- a/CryptoPkg/Private/Protocol/Crypto.h
> +++ b/CryptoPkg/Private/Protocol/Crypto.h
> @@ -3863,7 +3863,6 @@ CONST VOID *
> @param[out] BnRes The result, such that (BnA * BnB) % BnM.
>
> @retval EFI_SUCCESS On success.
> - @retval EFI_OUT_OF_RESOURCES In case of internal allocation failures.
> @retval EFI_PROTOCOL_ERROR Otherwise.
> **/
> typedef
> @@ -3935,6 +3934,9 @@ VOID
>
> @param[in] Bn Big number to set.
> @param[in] Val Value to set.
> +
> + @retval EFI_SUCCESS On success.
> + @retval EFI_PROTOCOL_ERROR Otherwise.
> **/
> typedef
> EFI_STATUS
> @@ -3970,7 +3972,7 @@ EFI_STATUS
> using EcGroupFree() function.
>
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409)
> + Description" attribute registry for RFC 2409)
>
> @retval EcGroup object On success
> @retval NULL On failure
> @@ -3989,8 +3991,8 @@ VOID *
>
> @param[in] EcGroup EC group object
> @param[out] BnPrime Group prime number
> - @param[out] BnA A coofecient
> - @param[out] BnB B coofecient
> + @param[out] BnA A coefficient
> + @param[out] BnB B coefficient
> @param[in] BnCtx BN context
>
> @retval EFI_SUCCESS On success
> @@ -4260,13 +4262,14 @@ EFI_STATUS
> /**
> Generate a key using ECDH algorithm. Please note, this function uses
> pseudo random number generator. The caller must make sure RandomSeed()
> - funtion was properly called before.
> + function was properly called before.
>
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409)
> + Description" attribute registry for RFC 2409)
> @param[out] PKey Pointer to an object that will hold the ECDH key
>
> @retval EFI_SUCCESS On success
> + @retval EFI_UNSUPPORTED ECC group not supported.
> @retval EFI_PROTOCOL_ERROR On failure **/ typedef @@ -4294,8 +4297,9
> @@ VOID
> @param[in] PKey ECDH Key object
> @param[out] EcPoint Properly initialized EC Point to hold the public key
>
> - @retval EFI_SUCCESS On success
> - @retval EFI_PROTOCOL_ERROR On failure
> + @retval EFI_SUCCESS On success
> + @retval EFI_INVALID_PARAMETER EcPoint should be initialized properly.
> + @retval EFI_PROTOCOL_ERROR On failure
> **/
> typedef
> EFI_STATUS
> @@ -4309,15 +4313,20 @@ EFI_STATUS
>
> @param[in] PKey ECDH Key object
> @param[in] Group Identifying number for the ECC group (IANA "Group
> - Description" attribute registrty for RFC 2409)
> + Description" attribute registry for RFC
> + 2409)
> @param[in] EcPointPublic Peer public key
> @param[out] SecretSize On success, holds secret size
> @param[out] Secret On success, holds the derived secret
> Should be freed by caller using FreePool()
> function.
>
> - @retval EFI_SUCCESS On success
> - @retval EFI_PROTOCOL_ERROR On failure
> + @retval EFI_SUCCESS On success.
> + @retval EFI_UNSUPPORTED ECC group not supported.
> + @retval EFI_INVALID_PARAMETER One or more of the following conditions is
> TRUE:
> + Secret is NULL.
> + SecretSize is NULL.
> + Public key in EcPointPublic is invalid.
> + @retval EFI_PROTOCOL_ERROR On failure.
> **/
> typedef
> EFI_STATUS
> --
> 2.31.1.windows.1
next prev parent reply other threads:[~2022-07-15 2:13 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-15 2:07 [staging/crypto-new-api PATCH] CryptoPkg: Fix issues from crypto code review yi1 li
2022-07-15 2:13 ` Heng Luo [this message]
2022-07-15 3:31 ` Tan, Ming
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=SN6PR11MB275288CED8A13326C4F8BBA7938B9@SN6PR11MB2752.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox