[edk2-devel] [PATCH v1 1/1] CryptoPkg/BaseCryptoLib: Remove unnecessary key generation.

[edk2-devel] [PATCH v1 1/1] CryptoPkg/BaseCryptoLib: Remove unnecessary key generation.

[levi.yun]

When EcGenerateKey() is called with PublicKeySize set to zero or
less than the required size,
it returns the size of the required buffer with failure.
However, EcGenerateKey() generates a key and then checks
if the buffer size is insufficient.
This can be optimised by moving the public key size check
before generating the key.
Therefore, optimise to avoid unnecessary key generation.

Signed-off-by: levi.yun <yeoreum.yun@arm.com>
---
This changes can be seen at https://github.com/LeviYeoReum/edk2/tree/levi/2716_not_generate_key_on_fail_size_v1

 CryptoPkg/Library/BaseCryptLib/Pk/CryptEc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptEc.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptEc.c
index d8cc9ba0e8f968f6cbd9ac4c56018f9a4392cd0b..af67f512a22b23af3844b9bbc87dd57bcf952f04 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptEc.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptEc.c
@@ -497,16 +497,16 @@ EcGenerateKey (
   Group    = EC_KEY_get0_group (EcKey);
   HalfSize = (EC_GROUP_get_degree (Group) + 7) / 8;

+  if (*PublicKeySize < HalfSize * 2) {
+    *PublicKeySize = HalfSize * 2;
+    return FALSE;
+  }
+
   // Assume RAND_seed was called
   if (EC_KEY_generate_key (EcKey) != 1) {
     return FALSE;
   }

-  if (*PublicKeySize < HalfSize * 2) {
-    *PublicKeySize = HalfSize * 2;
-    return FALSE;
-  }
-
   *PublicKeySize = HalfSize * 2;

   EcPoint = EC_KEY_get0_public_key (EcKey);
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106962): https://edk2.groups.io/g/devel/message/106962
Mute This Topic: https://groups.io/mt/100191693/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/1] CryptoPkg/BaseCryptoLib: Remove unnecessary key generation.

[Li, Yi]

Hi,

This function has a special use case: when the input PublicKey array is NULL and size is 0, the function will generate EC keypair and update context, and fill PublicKeySize with non-zero keysize to indicate success.

(CryptEc.c  L492)
  if ((PublicKey == NULL) && (*PublicKeySize != 0)) {
    return FALSE;
  }

I recommend the below changes:

  HalfSize = (EC_GROUP_get_degree (Group) + 7) / 8;

+  if ((PublicKey != NULL) && (*PublicKeySize < HalfSize * 2)) {
+    *PublicKeySize = HalfSize * 2;
+    return FALSE;
+  }
  // Assume RAND_seed was called
  if (EC_KEY_generate_key (EcKey) != 1) {
    return FALSE;
  }

-  if (*PublicKeySize < HalfSize * 2) {
+ // If PublicKey is NULL and PublicKeySize is 0, return TRUE and fill PublicKeySize with correct Key size.
+ if (*PublicKeySize == 0) {
    *PublicKeySize = HalfSize * 2;
-   return FALSE;
+   return TRUE;
  }

Regards,
Yi 

-----Original Message-----
From: levi.yun <yeoreum.yun@arm.com> 
Sent: Monday, July 17, 2023 5:54 PM
To: devel@edk2.groups.io
Cc: yeoreum.yun@arm.com; Yao, Jiewen <jiewen.yao@intel.com>; Li, Yi1 <yi1.li@intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>; sami.mujawar@arm.com; pierre.gondois@arm.com; nd@arm.com
Subject: [PATCH v1 1/1] CryptoPkg/BaseCryptoLib: Remove unnecessary key generation.

When EcGenerateKey() is called with PublicKeySize set to zero or less than the required size, it returns the size of the required buffer with failure.
However, EcGenerateKey() generates a key and then checks if the buffer size is insufficient.
This can be optimised by moving the public key size check before generating the key.
Therefore, optimise to avoid unnecessary key generation.

Signed-off-by: levi.yun <yeoreum.yun@arm.com>
---
This changes can be seen at https://github.com/LeviYeoReum/edk2/tree/levi/2716_not_generate_key_on_fail_size_v1

 CryptoPkg/Library/BaseCryptLib/Pk/CryptEc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptEc.c b/CryptoPkg/Library/BaseCryptLib/Pk/CryptEc.c
index d8cc9ba0e8f968f6cbd9ac4c56018f9a4392cd0b..af67f512a22b23af3844b9bbc87dd57bcf952f04 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptEc.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptEc.c
@@ -497,16 +497,16 @@ EcGenerateKey (
   Group    = EC_KEY_get0_group (EcKey);
   HalfSize = (EC_GROUP_get_degree (Group) + 7) / 8;

+  if (*PublicKeySize < HalfSize * 2) {
+    *PublicKeySize = HalfSize * 2;
+    return FALSE;
+  }
+
   // Assume RAND_seed was called
   if (EC_KEY_generate_key (EcKey) != 1) {
     return FALSE;
   }

-  if (*PublicKeySize < HalfSize * 2) {
-    *PublicKeySize = HalfSize * 2;
-    return FALSE;
-  }
-
   *PublicKeySize = HalfSize * 2;

   EcPoint = EC_KEY_get0_public_key (EcKey);
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106979): https://edk2.groups.io/g/devel/message/106979
Mute This Topic: https://groups.io/mt/100191693/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/1] CryptoPkg/BaseCryptoLib: Remove unnecessary key generation.

[levi.yun]

Hi Li!

On 18/07/2023 07:03, Li, Yi1 wrote:
> Hi,
>
> This function has a special use case: when the input PublicKey array is NULL and size is 0, the function will generate EC keypair and update context, and fill PublicKeySize with non-zero keysize to indicate success.
>
> (CryptEc.c  L492)
>    if ((PublicKey == NULL) && (*PublicKeySize != 0)) {
>      return FALSE;
>    }

Thanks to make me know :)

IIUC, That special case could be used with EcGetPubKey.

>
> I recommend the below changes:
>
>    HalfSize = (EC_GROUP_get_degree (Group) + 7) / 8;
>
> +  if ((PublicKey != NULL) && (*PublicKeySize < HalfSize * 2)) {
> +    *PublicKeySize = HalfSize * 2;
> +    return FALSE;
> +  }
>    // Assume RAND_seed was called
>    if (EC_KEY_generate_key (EcKey) != 1) {
>      return FALSE;
>    }
>
> -  if (*PublicKeySize < HalfSize * 2) {
> + // If PublicKey is NULL and PublicKeySize is 0, return TRUE and fill PublicKeySize with correct Key size.
> + if (*PublicKeySize == 0) {
>      *PublicKeySize = HalfSize * 2;
> -   return FALSE;
> +   return TRUE;
>    }

Look good to me But it would be better to set key size above of If clause.

I'll send the patch again :)


Many thanks.

--------

Sincerely,

Levi.


IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106983): https://edk2.groups.io/g/devel/message/106983
Mute This Topic: https://groups.io/mt/100191693/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-