Re: [edk2-devel] [PATCH 3/3] CryptoPkg/BaseCryptLib: updata HMAC_ctx size

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

Hi Xiaoyu,

Small typos: "update" in subject.
Maybe better described as "Make HMAC_CTX_SIZE backward compatible"?


On 4/29/19 10:15 AM, Xiaoyu lu wrote:
> From: Xiaoyu Lu <xiaoyux.lu@intel.com>
> 
> Openssl internally redefines the size of HMAC_CTX,

"OpenSSL"?

> but there is no external definition.
> So add an additional nubmer.

"number"

> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ting Ye <ting.ye@intel.com>

Can you add the reference?

"Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1089"

> Signed-off-by: Xiaoyu Lu <xiaoyux.lu@intel.com>
> ---
>  CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c    | 11 ++++++++++-
>  CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c   | 12 ++++++++++--
>  CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c | 12 ++++++++++--
>  3 files changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c
> index 3134806..3ffb8e2 100644
> --- a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c
> +++ b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c
> @@ -9,8 +9,17 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include "InternalCryptLib.h"
>  #include <openssl/hmac.h>
>  
> +//
> +// NOTE: HMAC_MAX_MD_CBLOCK is deprecated.
> +//       #define HMAC_MAX_MD_CBLOCK 128
> +//       Openssl redefines the size of HMAC_CTX at crypto/hmac/hmac_lcl.h

OpenSSL

> +//       #define HMAC_MAX_MD_CBLOCK_SIZE     144
> +//       But we need to compatible with previous API.
> +//       So fix it with correct size 144-128 = 16.
> +//
>  #define HMAC_MD5_CTX_SIZE    sizeof(void *) * 4 + sizeof(unsigned int) + \
> -                             sizeof(unsigned char) * HMAC_MAX_MD_CBLOCK
> +                             sizeof(unsigned char) * (HMAC_MAX_MD_CBLOCK + 16)

Can you put this expression between parenthesis? (and the other ones).

> +
>  
>  /**
>    Retrieves the size, in bytes, of the context buffer required for HMAC-MD5 operations.
> diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c
> index bbe3df4..e59602e 100644
> --- a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c
> +++ b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c
> @@ -9,8 +9,16 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include "InternalCryptLib.h"
>  #include <openssl/hmac.h>
>  
> -#define HMAC_SHA1_CTX_SIZE   sizeof(void *) * 4 + sizeof(unsigned int) + \
> -                             sizeof(unsigned char) * HMAC_MAX_MD_CBLOCK
> +//
> +// NOTE: HMAC_MAX_MD_CBLOCK is deprecated.
> +//       #define HMAC_MAX_MD_CBLOCK 128
> +//       Openssl redefines the size of HMAC_CTX at crypto/hmac/hmac_lcl.h

Ditto,

> +//       #define HMAC_MAX_MD_CBLOCK_SIZE     144
> +//       But we need to compatible with previous API.
> +//       So fix it with correct size 144-128 = 16.
> +//
> +#define  HMAC_SHA1_CTX_SIZE   sizeof(void *) * 4 + sizeof(unsigned int) + \
> +                             sizeof(unsigned char) * (HMAC_MAX_MD_CBLOCK + 16)
>  
>  /**
>    Retrieves the size, in bytes, of the context buffer required for HMAC-SHA1 operations.
> diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c
> index ac9084f..8d0570b 100644
> --- a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c
> +++ b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c
> @@ -9,8 +9,16 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include "InternalCryptLib.h"
>  #include <openssl/hmac.h>
>  
> -#define HMAC_SHA256_CTX_SIZE   sizeof(void *) * 4 + sizeof(unsigned int) + \
> -                               sizeof(unsigned char) * HMAC_MAX_MD_CBLOCK
> +//
> +// NOTE: HMAC_MAX_MD_CBLOCK is deprecated.
> +//       #define HMAC_MAX_MD_CBLOCK 128
> +//       Openssl redefines the size of HMAC_CTX at crypto/hmac/hmac_lcl.h

Ditto.

Thanks!

> +//       #define HMAC_MAX_MD_CBLOCK_SIZE     144
> +//       But we need to compatible with previous API.
> +//       So fix it with correct size 144-128 = 16.
> +//
> +#define HMAC_SHA256_CTX_SIZE    sizeof(void *) * 4 + sizeof(unsigned int) + \
> +                             sizeof(unsigned char) * (HMAC_MAX_MD_CBLOCK + 16)
>  
>  /**
>    Retrieves the size, in bytes, of the context buffer required for HMAC-SHA256 operations.
>