From: "Laszlo Ersek" <lersek@redhat.com>
To: Jian J Wang <jian.j.wang@intel.com>, devel@edk2.groups.io
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Subject: Re: [PATCH v2] CryptoPkg/BaseCryptLib: remove HmacXxxGetContextSize interface
Date: Tue, 14 Jan 2020 10:45:15 +0100 [thread overview]
Message-ID: <fae2d23b-ac02-3dc2-9e25-ac0c4b6f0aa4@redhat.com> (raw)
In-Reply-To: <20200114062626.1248-1-jian.j.wang@intel.com>
On 01/14/20 07:26, Jian J Wang wrote:
>> v2 changes:
>> - change HmacXxxInit to HmacXxxSetKey
>> - remove calling to HMAC_CTX_reset() since HmacXxxNew() has done it
>> already and user-supplied context is not supported any longer.
>> - update comments affected by above change
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1792
>
> Hmac(Md5|Sha1|Sha256)GetContextSize() use a deprecated macro
> HMAC_MAX_MD_CBLOCK defined in openssl. They should be dropped to
> avoid misuses in the future. For context allocation and release,
> use HmacXxxNew() and HmacXxxFree() instead.
>
> Considering that HmacXxxInit() has no use cases, remove the calling
> to memset() and HMAC_CTX_reset() to drop the support of user supplied
> context buffer. The only functionality left is to set user supplied
> key for HMAC. To avoid confusion, change the the its name to
> HmacXxxSetKey.
>
> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
> CryptoPkg/Include/Library/BaseCryptLib.h | 111 +++++-------------
> .../Library/BaseCryptLib/Hmac/CryptHmacMd5.c | 58 ++-------
> .../BaseCryptLib/Hmac/CryptHmacMd5Null.c | 28 +----
> .../Library/BaseCryptLib/Hmac/CryptHmacSha1.c | 59 ++--------
> .../BaseCryptLib/Hmac/CryptHmacSha1Null.c | 28 +----
> .../BaseCryptLib/Hmac/CryptHmacSha256.c | 58 ++-------
> .../BaseCryptLib/Hmac/CryptHmacSha256Null.c | 28 +----
> .../BaseCryptLibNull/Hmac/CryptHmacMd5Null.c | 20 ----
> .../BaseCryptLibNull/Hmac/CryptHmacSha1Null.c | 20 ----
> .../Hmac/CryptHmacSha256Null.c | 20 ----
> 10 files changed, 72 insertions(+), 358 deletions(-)
>
> diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
> index 8fe303a0b3..b93f1b9009 100644
> --- a/CryptoPkg/Include/Library/BaseCryptLib.h
> +++ b/CryptoPkg/Include/Library/BaseCryptLib.h
> @@ -1025,23 +1025,6 @@ Sm3HashAll (
> // MAC (Message Authentication Code) Primitive
> //=====================================================================================
>
> -/**
> - Retrieves the size, in bytes, of the context buffer required for HMAC-MD5 operations.
> - (NOTE: This API is deprecated.
> - Use HmacMd5New() / HmacMd5Free() for HMAC-MD5 Context operations.)
> -
> - If this interface is not supported, then return zero.
> -
> - @return The size, in bytes, of the context buffer required for HMAC-MD5 operations.
> - @retval 0 This interface is not supported.
> -
> -**/
> -UINTN
> -EFIAPI
> -HmacMd5GetContextSize (
> - VOID
> - );
> -
> /**
> Allocates and initializes one HMAC_CTX context for subsequent HMAC-MD5 use.
>
> @@ -1073,24 +1056,24 @@ HmacMd5Free (
> );
>
> /**
> - Initializes user-supplied memory pointed by HmacMd5Context as HMAC-MD5 context for
> - subsequent use.
> + Set user-supplied key for subsequent use. It must be done before any
> + calling to HmacMd5Update().
>
> If HmacMd5Context is NULL, then return FALSE.
> If this interface is not supported, then return FALSE.
>
> - @param[out] HmacMd5Context Pointer to HMAC-MD5 context being initialized.
> + @param[out] HmacMd5Context Pointer to HMAC-MD5 context.
> @param[in] Key Pointer to the user-supplied key.
> @param[in] KeySize Key size in bytes.
>
> - @retval TRUE HMAC-MD5 context initialization succeeded.
> - @retval FALSE HMAC-MD5 context initialization failed.
> + @retval TRUE Key is set succeefully.
> + @retval FALSE Key is set unsucceefully.
(1) Please fix the typos: it should be "successfully" and "unsuccessfully".
> @retval FALSE This interface is not supported.
>
> **/
> BOOLEAN
> EFIAPI
> -HmacMd5Init (
> +HmacMd5SetKey (
> OUT VOID *HmacMd5Context,
> IN CONST UINT8 *Key,
> IN UINTN KeySize
> @@ -1123,8 +1106,8 @@ HmacMd5Duplicate (
>
> This function performs HMAC-MD5 digest on a data buffer of the specified size.
> It can be called multiple times to compute the digest of long or discontinuous data streams.
> - HMAC-MD5 context should be already correctly initialized by HmacMd5Init(), and should not be
> - finalized by HmacMd5Final(). Behavior with invalid context is undefined.
> + HMAC-MD5 context should be initialized by HmacMd5New(), and should not be finalized by
> + HmacMd5Final(). Behavior with invalid context is undefined.
>
> If HmacMd5Context is NULL, then return FALSE.
> If this interface is not supported, then return FALSE.
> @@ -1152,8 +1135,8 @@ HmacMd5Update (
> This function completes HMAC-MD5 hash computation and retrieves the digest value into
> the specified memory. After this function has been called, the HMAC-MD5 context cannot
> be used again.
> - HMAC-MD5 context should be already correctly initialized by HmacMd5Init(), and should not be
> - finalized by HmacMd5Final(). Behavior with invalid HMAC-MD5 context is undefined.
> + HMAC-MD5 context should be initialized by HmacMd5New(), and should not be finalized by
> + HmacMd5Final(). Behavior with invalid HMAC-MD5 context is undefined.
>
> If HmacMd5Context is NULL, then return FALSE.
> If HmacValue is NULL, then return FALSE.
> @@ -1175,23 +1158,6 @@ HmacMd5Final (
> OUT UINT8 *HmacValue
> );
>
> -/**
> - Retrieves the size, in bytes, of the context buffer required for HMAC-SHA1 operations.
> - (NOTE: This API is deprecated.
> - Use HmacSha1New() / HmacSha1Free() for HMAC-SHA1 Context operations.)
> -
> - If this interface is not supported, then return zero.
> -
> - @return The size, in bytes, of the context buffer required for HMAC-SHA1 operations.
> - @retval 0 This interface is not supported.
> -
> -**/
> -UINTN
> -EFIAPI
> -HmacSha1GetContextSize (
> - VOID
> - );
> -
> /**
> Allocates and initializes one HMAC_CTX context for subsequent HMAC-SHA1 use.
>
> @@ -1223,24 +1189,24 @@ HmacSha1Free (
> );
>
> /**
> - Initializes user-supplied memory pointed by HmacSha1Context as HMAC-SHA1 context for
> - subsequent use.
> + Set user-supplied key for subsequent use. It must be done before any
> + calling to HmacSha1Update().
>
> If HmacSha1Context is NULL, then return FALSE.
> If this interface is not supported, then return FALSE.
>
> - @param[out] HmacSha1Context Pointer to HMAC-SHA1 context being initialized.
> + @param[out] HmacSha1Context Pointer to HMAC-SHA1 context.
> @param[in] Key Pointer to the user-supplied key.
> @param[in] KeySize Key size in bytes.
>
> - @retval TRUE HMAC-SHA1 context initialization succeeded.
> - @retval FALSE HMAC-SHA1 context initialization failed.
> + @retval TRUE The Key is set succeefully.
> + @retval FALSE The Key is set unsucceefully.
(2) Same as (1).
> @retval FALSE This interface is not supported.
>
> **/
> BOOLEAN
> EFIAPI
> -HmacSha1Init (
> +HmacSha1SetKey (
> OUT VOID *HmacSha1Context,
> IN CONST UINT8 *Key,
> IN UINTN KeySize
> @@ -1273,8 +1239,8 @@ HmacSha1Duplicate (
>
> This function performs HMAC-SHA1 digest on a data buffer of the specified size.
> It can be called multiple times to compute the digest of long or discontinuous data streams.
> - HMAC-SHA1 context should be already correctly initialized by HmacSha1Init(), and should not
> - be finalized by HmacSha1Final(). Behavior with invalid context is undefined.
> + HMAC-SHA1 context should be initialized by HmacSha1New(), and should not be finalized by
> + HmacSha1Final(). Behavior with invalid context is undefined.
>
> If HmacSha1Context is NULL, then return FALSE.
> If this interface is not supported, then return FALSE.
> @@ -1302,8 +1268,8 @@ HmacSha1Update (
> This function completes HMAC-SHA1 hash computation and retrieves the digest value into
> the specified memory. After this function has been called, the HMAC-SHA1 context cannot
> be used again.
> - HMAC-SHA1 context should be already correctly initialized by HmacSha1Init(), and should
> - not be finalized by HmacSha1Final(). Behavior with invalid HMAC-SHA1 context is undefined.
> + HMAC-SHA1 context should be initialized by HmacSha1New(), and should not be finalized
> + by HmacSha1Final(). Behavior with invalid HMAC-SHA1 context is undefined.
>
> If HmacSha1Context is NULL, then return FALSE.
> If HmacValue is NULL, then return FALSE.
> @@ -1325,23 +1291,6 @@ HmacSha1Final (
> OUT UINT8 *HmacValue
> );
>
> -/**
> - Retrieves the size, in bytes, of the context buffer required for HMAC-SHA256 operations.
> - (NOTE: This API is deprecated.
> - Use HmacSha256New() / HmacSha256Free() for HMAC-SHA256 Context operations.)
> -
> - If this interface is not supported, then return zero.
> -
> - @return The size, in bytes, of the context buffer required for HMAC-SHA256 operations.
> - @retval 0 This interface is not supported.
> -
> -**/
> -UINTN
> -EFIAPI
> -HmacSha256GetContextSize (
> - VOID
> - );
> -
> /**
> Allocates and initializes one HMAC_CTX context for subsequent HMAC-SHA256 use.
>
> @@ -1368,24 +1317,24 @@ HmacSha256Free (
> );
>
> /**
> - Initializes user-supplied memory pointed by HmacSha256Context as HMAC-SHA256 context for
> - subsequent use.
> + Set user-supplied key for subsequent use. It must be done before any
> + calling to HmacSha256Update().
>
> If HmacSha256Context is NULL, then return FALSE.
> If this interface is not supported, then return FALSE.
>
> - @param[out] HmacSha256Context Pointer to HMAC-SHA256 context being initialized.
> + @param[out] HmacSha256Context Pointer to HMAC-SHA256 context.
> @param[in] Key Pointer to the user-supplied key.
> @param[in] KeySize Key size in bytes.
>
> - @retval TRUE HMAC-SHA256 context initialization succeeded.
> - @retval FALSE HMAC-SHA256 context initialization failed.
> + @retval TRUE The Key is set succeefully.
> + @retval FALSE The Key is set unsucceefully.
(3) Same as (1).
> @retval FALSE This interface is not supported.
>
> **/
> BOOLEAN
> EFIAPI
> -HmacSha256Init (
> +HmacSha256SetKey (
> OUT VOID *HmacSha256Context,
> IN CONST UINT8 *Key,
> IN UINTN KeySize
> @@ -1418,8 +1367,8 @@ HmacSha256Duplicate (
>
> This function performs HMAC-SHA256 digest on a data buffer of the specified size.
> It can be called multiple times to compute the digest of long or discontinuous data streams.
> - HMAC-SHA256 context should be already correctly initialized by HmacSha256Init(), and should not
> - be finalized by HmacSha256Final(). Behavior with invalid context is undefined.
> + HMAC-SHA256 context should be initialized by HmacSha256New(), and should not be finalized
> + by HmacSha256Final(). Behavior with invalid context is undefined.
>
> If HmacSha256Context is NULL, then return FALSE.
> If this interface is not supported, then return FALSE.
> @@ -1447,8 +1396,8 @@ HmacSha256Update (
> This function completes HMAC-SHA256 hash computation and retrieves the digest value into
> the specified memory. After this function has been called, the HMAC-SHA256 context cannot
> be used again.
> - HMAC-SHA256 context should be already correctly initialized by HmacSha256Init(), and should
> - not be finalized by HmacSha256Final(). Behavior with invalid HMAC-SHA256 context is undefined.
> + HMAC-SHA256 context should be initialized by HmacSha256New(), and should not be finalized
> + by HmacSha256Final(). Behavior with invalid HMAC-SHA256 context is undefined.
>
> If HmacSha256Context is NULL, then return FALSE.
> If HmacValue is NULL, then return FALSE.
> diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c
> index 19e9fbeae6..f0609e61d7 100644
> --- a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c
> +++ b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c
> @@ -9,37 +9,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include "InternalCryptLib.h"
> #include <openssl/hmac.h>
>
> -//
> -// NOTE: OpenSSL redefines the size of HMAC_CTX at crypto/hmac/hmac_lcl.h
> -// #define HMAC_MAX_MD_CBLOCK_SIZE 144
> -//
> -#define HMAC_MD5_CTX_SIZE (sizeof(void *) * 4 + sizeof(unsigned int) + \
> - sizeof(unsigned char) * 144)
> -
> -/**
> - Retrieves the size, in bytes, of the context buffer required for HMAC-MD5 operations.
> - (NOTE: This API is deprecated.
> - Use HmacMd5New() / HmacMd5Free() for HMAC-MD5 Context operations.)
> -
> - @return The size, in bytes, of the context buffer required for HMAC-MD5 operations.
> -
> -**/
> -UINTN
> -EFIAPI
> -HmacMd5GetContextSize (
> - VOID
> - )
> -{
> - //
> - // Retrieves the OpenSSL HMAC-MD5 Context Size
> - // NOTE: HMAC_CTX object was made opaque in openssl-1.1.x, here we just use the
> - // fixed size as a workaround to make this API work for compatibility.
> - // We should retire HmacMd5GetContextSize() in future, and use HmacMd5New()
> - // and HmacMd5Free() for context allocation and release.
> - //
> - return (UINTN) HMAC_MD5_CTX_SIZE;
> -}
> -
> /**
> Allocates and initializes one HMAC_CTX context for subsequent HMAC-MD5 use.
>
> @@ -78,22 +47,22 @@ HmacMd5Free (
> }
>
> /**
> - Initializes user-supplied memory pointed by HmacMd5Context as HMAC-MD5 context for
> - subsequent use.
> + Set user-supplied key for subsequent use. It must be done before any
> + calling to HmacMd5Update().
>
> If HmacMd5Context is NULL, then return FALSE.
>
> - @param[out] HmacMd5Context Pointer to HMAC-MD5 context being initialized.
> + @param[out] HmacMd5Context Pointer to HMAC-MD5 context.
> @param[in] Key Pointer to the user-supplied key.
> @param[in] KeySize Key size in bytes.
>
> - @retval TRUE HMAC-MD5 context initialization succeeded.
> - @retval FALSE HMAC-MD5 context initialization failed.
> + @retval TRUE Key is set succeefully.
> + @retval FALSE Key is set unsucceefully.
(4) Same as (1).
>
> **/
> BOOLEAN
> EFIAPI
> -HmacMd5Init (
> +HmacMd5SetKey (
> OUT VOID *HmacMd5Context,
> IN CONST UINT8 *Key,
> IN UINTN KeySize
> @@ -106,13 +75,6 @@ HmacMd5Init (
> return FALSE;
> }
>
> - //
> - // OpenSSL HMAC-MD5 Context Initialization
> - //
> - memset(HmacMd5Context, 0, HMAC_MD5_CTX_SIZE);
> - if (HMAC_CTX_reset ((HMAC_CTX *)HmacMd5Context) != 1) {
> - return FALSE;
> - }
> if (HMAC_Init_ex ((HMAC_CTX *)HmacMd5Context, Key, (UINT32) KeySize, EVP_md5(), NULL) != 1) {
> return FALSE;
> }
> @@ -159,8 +121,8 @@ HmacMd5Duplicate (
>
> This function performs HMAC-MD5 digest on a data buffer of the specified size.
> It can be called multiple times to compute the digest of long or discontinuous data streams.
> - HMAC-MD5 context should be already correctly initialized by HmacMd5Init(), and should not be
> - finalized by HmacMd5Final(). Behavior with invalid context is undefined.
> + HMAC-MD5 context should be initialized by HmacMd5New(), and should not be finalized by
> + HmacMd5Final(). Behavior with invalid context is undefined.
>
> If HmacMd5Context is NULL, then return FALSE.
>
> @@ -210,8 +172,8 @@ HmacMd5Update (
> This function completes HMAC-MD5 digest computation and retrieves the digest value into
> the specified memory. After this function has been called, the HMAC-MD5 context cannot
> be used again.
> - HMAC-MD5 context should be already correctly initialized by HmacMd5Init(), and should not be
> - finalized by HmacMd5Final(). Behavior with invalid HMAC-MD5 context is undefined.
> + HMAC-MD5 context should be initialized by HmacMd5New(), and should not be finalized by
> + HmacMd5Final(). Behavior with invalid HMAC-MD5 context is undefined.
>
> If HmacMd5Context is NULL, then return FALSE.
> If HmacValue is NULL, then return FALSE.
> diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5Null.c b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5Null.c
> index 3aafed874b..9da132eeee 100644
> --- a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5Null.c
> +++ b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5Null.c
> @@ -8,26 +8,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
> #include "InternalCryptLib.h"
>
> -/**
> - Retrieves the size, in bytes, of the context buffer required for HMAC-MD5 operations.
> - (NOTE: This API is deprecated.
> - Use HmacMd5New() / HmacMd5Free() for HMAC-MD5 Context operations.)
> -
> - Return zero to indicate this interface is not supported.
> -
> - @retval 0 This interface is not supported.
> -
> -**/
> -UINTN
> -EFIAPI
> -HmacMd5GetContextSize (
> - VOID
> - )
> -{
> - ASSERT (FALSE);
> - return 0;
> -}
> -
> /**
> Allocates and initializes one HMAC_CTX context for subsequent HMAC-MD5 use.
>
> @@ -65,12 +45,12 @@ HmacMd5Free (
> }
>
> /**
> - Initializes user-supplied memory pointed by HmacMd5Context as HMAC-MD5 context for
> - subsequent use.
> + Set user-supplied key for subsequent use. It must be done before any
> + calling to HmacMd5Update().
>
> Return FALSE to indicate this interface is not supported.
>
> - @param[out] HmacMd5Context Pointer to HMAC-MD5 context being initialized.
> + @param[out] HmacMd5Context Pointer to HMAC-MD5 context.
> @param[in] Key Pointer to the user-supplied key.
> @param[in] KeySize Key size in bytes.
>
> @@ -79,7 +59,7 @@ HmacMd5Free (
> **/
> BOOLEAN
> EFIAPI
> -HmacMd5Init (
> +HmacMd5SetKey (
> OUT VOID *HmacMd5Context,
> IN CONST UINT8 *Key,
> IN UINTN KeySize
> diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c
> index 7d7df9640e..1bb5edfbd1 100644
> --- a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c
> +++ b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c
> @@ -9,38 +9,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include "InternalCryptLib.h"
> #include <openssl/hmac.h>
>
> -//
> -// NOTE: OpenSSL redefines the size of HMAC_CTX at crypto/hmac/hmac_lcl.h
> -// #define HMAC_MAX_MD_CBLOCK_SIZE 144
> -//
> -//
> -#define HMAC_SHA1_CTX_SIZE (sizeof(void *) * 4 + sizeof(unsigned int) + \
> - sizeof(unsigned char) * 144)
> -
> -/**
> - Retrieves the size, in bytes, of the context buffer required for HMAC-SHA1 operations.
> - (NOTE: This API is deprecated.
> - Use HmacSha1New() / HmacSha1Free() for HMAC-SHA1 Context operations.)
> -
> - @return The size, in bytes, of the context buffer required for HMAC-SHA1 operations.
> -
> -**/
> -UINTN
> -EFIAPI
> -HmacSha1GetContextSize (
> - VOID
> - )
> -{
> - //
> - // Retrieves the OpenSSL HMAC-SHA1 Context Size
> - // NOTE: HMAC_CTX object was made opaque in openssl-1.1.x, here we just use the
> - // fixed size as a workaround to make this API work for compatibility.
> - // We should retire HmacSha15GetContextSize() in future, and use HmacSha1New()
> - // and HmacSha1Free() for context allocation and release.
> - //
> - return (UINTN) HMAC_SHA1_CTX_SIZE;
> -}
> -
> /**
> Allocates and initializes one HMAC_CTX context for subsequent HMAC-SHA1 use.
>
> @@ -79,22 +47,22 @@ HmacSha1Free (
> }
>
> /**
> - Initializes user-supplied memory pointed by HmacSha1Context as HMAC-SHA1 context for
> - subsequent use.
> + Set user-supplied key for subsequent use. It must be done before any
> + calling to HmacSha1Update().
>
> If HmacSha1Context is NULL, then return FALSE.
>
> - @param[out] HmacSha1Context Pointer to HMAC-SHA1 context being initialized.
> + @param[out] HmacSha1Context Pointer to HMAC-SHA1 context.
> @param[in] Key Pointer to the user-supplied key.
> @param[in] KeySize Key size in bytes.
>
> - @retval TRUE HMAC-SHA1 context initialization succeeded.
> - @retval FALSE HMAC-SHA1 context initialization failed.
> + @retval TRUE The Key is set succeefully.
> + @retval FALSE The Key is set unsucceefully.
(5) Same as (1).
>
> **/
> BOOLEAN
> EFIAPI
> -HmacSha1Init (
> +HmacSha1SetKey (
> OUT VOID *HmacSha1Context,
> IN CONST UINT8 *Key,
> IN UINTN KeySize
> @@ -107,13 +75,6 @@ HmacSha1Init (
> return FALSE;
> }
>
> - //
> - // OpenSSL HMAC-SHA1 Context Initialization
> - //
> - memset(HmacSha1Context, 0, HMAC_SHA1_CTX_SIZE);
> - if (HMAC_CTX_reset ((HMAC_CTX *)HmacSha1Context) != 1) {
> - return FALSE;
> - }
> if (HMAC_Init_ex ((HMAC_CTX *)HmacSha1Context, Key, (UINT32) KeySize, EVP_sha1(), NULL) != 1) {
> return FALSE;
> }
> @@ -160,8 +121,8 @@ HmacSha1Duplicate (
>
> This function performs HMAC-SHA1 digest on a data buffer of the specified size.
> It can be called multiple times to compute the digest of long or discontinuous data streams.
> - HMAC-SHA1 context should be already correctly initialized by HmacSha1Init(), and should not
> - be finalized by HmacSha1Final(). Behavior with invalid context is undefined.
> + HMAC-SHA1 context should be initialized by HmacSha1New(), and should not be finalized by
> + HmacSha1Final(). Behavior with invalid context is undefined.
>
> If HmacSha1Context is NULL, then return FALSE.
>
> @@ -211,8 +172,8 @@ HmacSha1Update (
> This function completes HMAC-SHA1 digest computation and retrieves the digest value into
> the specified memory. After this function has been called, the HMAC-SHA1 context cannot
> be used again.
> - HMAC-SHA1 context should be already correctly initialized by HmacSha1Init(), and should
> - not be finalized by HmacSha1Final(). Behavior with invalid HMAC-SHA1 context is undefined.
> + HMAC-SHA1 context should be initialized by HmacSha1New(), and should not be finalized by
> + HmacSha1Final(). Behavior with invalid HMAC-SHA1 context is undefined.
>
> If HmacSha1Context is NULL, then return FALSE.
> If HmacValue is NULL, then return FALSE.
> diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1Null.c b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1Null.c
> index 547aa484ea..2c26e9d514 100644
> --- a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1Null.c
> +++ b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1Null.c
> @@ -8,26 +8,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
> #include "InternalCryptLib.h"
>
> -/**
> - Retrieves the size, in bytes, of the context buffer required for HMAC-SHA1 operations.
> - (NOTE: This API is deprecated.
> - Use HmacSha1New() / HmacSha1Free() for HMAC-SHA1 Context operations.)
> -
> - Return zero to indicate this interface is not supported.
> -
> - @retval 0 This interface is not supported.
> -
> -**/
> -UINTN
> -EFIAPI
> -HmacSha1GetContextSize (
> - VOID
> - )
> -{
> - ASSERT (FALSE);
> - return 0;
> -}
> -
> /**
> Allocates and initializes one HMAC_CTX context for subsequent HMAC-SHA1 use.
>
> @@ -65,12 +45,12 @@ HmacSha1Free (
> }
>
> /**
> - Initializes user-supplied memory pointed by HmacSha1Context as HMAC-SHA1 context for
> - subsequent use.
> + Set user-supplied key for subsequent use. It must be done before any
> + calling to HmacSha1Update().
>
> Return FALSE to indicate this interface is not supported.
>
> - @param[out] HmacSha1Context Pointer to HMAC-SHA1 context being initialized.
> + @param[out] HmacSha1Context Pointer to HMAC-SHA1 context.
> @param[in] Key Pointer to the user-supplied key.
> @param[in] KeySize Key size in bytes.
>
> @@ -79,7 +59,7 @@ HmacSha1Free (
> **/
> BOOLEAN
> EFIAPI
> -HmacSha1Init (
> +HmacSha1SetKey (
> OUT VOID *HmacSha1Context,
> IN CONST UINT8 *Key,
> IN UINTN KeySize
> diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c
> index f24443e745..930248e39f 100644
> --- a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c
> +++ b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c
> @@ -9,37 +9,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include "InternalCryptLib.h"
> #include <openssl/hmac.h>
>
> -//
> -// NOTE: OpenSSL redefines the size of HMAC_CTX at crypto/hmac/hmac_lcl.h
> -// #define HMAC_MAX_MD_CBLOCK_SIZE 144
> -//
> -#define HMAC_SHA256_CTX_SIZE (sizeof(void *) * 4 + sizeof(unsigned int) + \
> - sizeof(unsigned char) * 144)
> -
> -/**
> - Retrieves the size, in bytes, of the context buffer required for HMAC-SHA256 operations.
> - (NOTE: This API is deprecated.
> - Use HmacSha256New() / HmacSha256Free() for HMAC-SHA256 Context operations.)
> -
> - @return The size, in bytes, of the context buffer required for HMAC-SHA256 operations.
> -
> -**/
> -UINTN
> -EFIAPI
> -HmacSha256GetContextSize (
> - VOID
> - )
> -{
> - //
> - // Retrieves the OpenSSL HMAC-SHA256 Context Size
> - // NOTE: HMAC_CTX object was made opaque in openssl-1.1.x, here we just use the
> - // fixed size as a workaround to make this API work for compatibility.
> - // We should retire HmacSha256GetContextSize() in future, and use HmacSha256New()
> - // and HmacSha256Free() for context allocation and release.
> - //
> - return (UINTN)HMAC_SHA256_CTX_SIZE;
> -}
> -
> /**
> Allocates and initializes one HMAC_CTX context for subsequent HMAC-SHA256 use.
>
> @@ -78,22 +47,22 @@ HmacSha256Free (
> }
>
> /**
> - Initializes user-supplied memory pointed by HmacSha256Context as HMAC-SHA256 context for
> - subsequent use.
> + Set user-supplied key for subsequent use. It must be done before any
> + calling to HmacSha256Update().
>
> If HmacSha256Context is NULL, then return FALSE.
>
> - @param[out] HmacSha256Context Pointer to HMAC-SHA256 context being initialized.
> + @param[out] HmacSha256Context Pointer to HMAC-SHA256 context.
> @param[in] Key Pointer to the user-supplied key.
> @param[in] KeySize Key size in bytes.
>
> - @retval TRUE HMAC-SHA256 context initialization succeeded.
> - @retval FALSE HMAC-SHA256 context initialization failed.
> + @retval TRUE The Key is set succeefully.
> + @retval FALSE The Key is set unsucceefully.
(6) Same as (1).
>
> **/
> BOOLEAN
> EFIAPI
> -HmacSha256Init (
> +HmacSha256SetKey (
> OUT VOID *HmacSha256Context,
> IN CONST UINT8 *Key,
> IN UINTN KeySize
> @@ -106,13 +75,6 @@ HmacSha256Init (
> return FALSE;
> }
>
> - //
> - // OpenSSL HMAC-SHA256 Context Initialization
> - //
> - memset(HmacSha256Context, 0, HMAC_SHA256_CTX_SIZE);
> - if (HMAC_CTX_reset ((HMAC_CTX *)HmacSha256Context) != 1) {
> - return FALSE;
> - }
> if (HMAC_Init_ex ((HMAC_CTX *)HmacSha256Context, Key, (UINT32) KeySize, EVP_sha256(), NULL) != 1) {
> return FALSE;
> }
> @@ -159,8 +121,8 @@ HmacSha256Duplicate (
>
> This function performs HMAC-SHA256 digest on a data buffer of the specified size.
> It can be called multiple times to compute the digest of long or discontinuous data streams.
> - HMAC-SHA256 context should be already correctly initialized by HmacSha256Init(), and should not
> - be finalized by HmacSha256Final(). Behavior with invalid context is undefined.
> + HMAC-SHA256 context should be initialized by HmacSha256New(), and should not be finalized
> + by HmacSha256Final(). Behavior with invalid context is undefined.
>
> If HmacSha256Context is NULL, then return FALSE.
>
> @@ -210,8 +172,8 @@ HmacSha256Update (
> This function completes HMAC-SHA256 hash computation and retrieves the digest value into
> the specified memory. After this function has been called, the HMAC-SHA256 context cannot
> be used again.
> - HMAC-SHA256 context should be already correctly initialized by HmacSha256Init(), and should
> - not be finalized by HmacSha256Final(). Behavior with invalid HMAC-SHA256 context is undefined.
> + HMAC-SHA256 context should be initialized by HmacSha256New(), and should not be finalized
> + by HmacSha256Final(). Behavior with invalid HMAC-SHA256 context is undefined.
>
> If HmacSha256Context is NULL, then return FALSE.
> If HmacValue is NULL, then return FALSE.
> diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256Null.c b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256Null.c
> index f0a4420e27..1af625ec9f 100644
> --- a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256Null.c
> +++ b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256Null.c
> @@ -8,26 +8,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
> #include "InternalCryptLib.h"
>
> -/**
> - Retrieves the size, in bytes, of the context buffer required for HMAC-SHA256 operations.
> - (NOTE: This API is deprecated.
> - Use HmacSha256New() / HmacSha256Free() for HMAC-SHA256 Context operations.)
> -
> - Return zero to indicate this interface is not supported.
> -
> - @retval 0 This interface is not supported.
> -
> -**/
> -UINTN
> -EFIAPI
> -HmacSha256GetContextSize (
> - VOID
> - )
> -{
> - ASSERT (FALSE);
> - return 0;
> -}
> -
> /**
> Allocates and initializes one HMAC_CTX context for subsequent HMAC-SHA256 use.
>
> @@ -65,12 +45,12 @@ HmacSha256Free (
> }
>
> /**
> - Initializes user-supplied memory pointed by HmacSha256Context as HMAC-SHA256 context for
> - subsequent use.
> + Set user-supplied key for subsequent use. It must be done before any
> + calling to HmacSha256Update().
>
> Return FALSE to indicate this interface is not supported.
>
> - @param[out] HmacSha256Context Pointer to HMAC-SHA256 context being initialized.
> + @param[out] HmacSha256Context Pointer to HMAC-SHA256 context.
> @param[in] Key Pointer to the user-supplied key.
> @param[in] KeySize Key size in bytes.
>
> @@ -79,7 +59,7 @@ HmacSha256Free (
> **/
> BOOLEAN
> EFIAPI
> -HmacSha256Init (
> +HmacSha256SetKey (
> OUT VOID *HmacSha256Context,
> IN CONST UINT8 *Key,
> IN UINTN KeySize
> diff --git a/CryptoPkg/Library/BaseCryptLibNull/Hmac/CryptHmacMd5Null.c b/CryptoPkg/Library/BaseCryptLibNull/Hmac/CryptHmacMd5Null.c
> index 3aafed874b..205dc9e474 100644
> --- a/CryptoPkg/Library/BaseCryptLibNull/Hmac/CryptHmacMd5Null.c
> +++ b/CryptoPkg/Library/BaseCryptLibNull/Hmac/CryptHmacMd5Null.c
> @@ -8,26 +8,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
> #include "InternalCryptLib.h"
>
> -/**
> - Retrieves the size, in bytes, of the context buffer required for HMAC-MD5 operations.
> - (NOTE: This API is deprecated.
> - Use HmacMd5New() / HmacMd5Free() for HMAC-MD5 Context operations.)
> -
> - Return zero to indicate this interface is not supported.
> -
> - @retval 0 This interface is not supported.
> -
> -**/
> -UINTN
> -EFIAPI
> -HmacMd5GetContextSize (
> - VOID
> - )
> -{
> - ASSERT (FALSE);
> - return 0;
> -}
> -
> /**
> Allocates and initializes one HMAC_CTX context for subsequent HMAC-MD5 use.
>
(7) You missed the Init -> SetKey rename, and the according comment
changes, in this file.
> diff --git a/CryptoPkg/Library/BaseCryptLibNull/Hmac/CryptHmacSha1Null.c b/CryptoPkg/Library/BaseCryptLibNull/Hmac/CryptHmacSha1Null.c
> index 547aa484ea..542350f15a 100644
> --- a/CryptoPkg/Library/BaseCryptLibNull/Hmac/CryptHmacSha1Null.c
> +++ b/CryptoPkg/Library/BaseCryptLibNull/Hmac/CryptHmacSha1Null.c
> @@ -8,26 +8,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
> #include "InternalCryptLib.h"
>
> -/**
> - Retrieves the size, in bytes, of the context buffer required for HMAC-SHA1 operations.
> - (NOTE: This API is deprecated.
> - Use HmacSha1New() / HmacSha1Free() for HMAC-SHA1 Context operations.)
> -
> - Return zero to indicate this interface is not supported.
> -
> - @retval 0 This interface is not supported.
> -
> -**/
> -UINTN
> -EFIAPI
> -HmacSha1GetContextSize (
> - VOID
> - )
> -{
> - ASSERT (FALSE);
> - return 0;
> -}
> -
> /**
> Allocates and initializes one HMAC_CTX context for subsequent HMAC-SHA1 use.
>
(8) Same as (7).
> diff --git a/CryptoPkg/Library/BaseCryptLibNull/Hmac/CryptHmacSha256Null.c b/CryptoPkg/Library/BaseCryptLibNull/Hmac/CryptHmacSha256Null.c
> index f0a4420e27..f8074cc617 100644
> --- a/CryptoPkg/Library/BaseCryptLibNull/Hmac/CryptHmacSha256Null.c
> +++ b/CryptoPkg/Library/BaseCryptLibNull/Hmac/CryptHmacSha256Null.c
> @@ -8,26 +8,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
> #include "InternalCryptLib.h"
>
> -/**
> - Retrieves the size, in bytes, of the context buffer required for HMAC-SHA256 operations.
> - (NOTE: This API is deprecated.
> - Use HmacSha256New() / HmacSha256Free() for HMAC-SHA256 Context operations.)
> -
> - Return zero to indicate this interface is not supported.
> -
> - @retval 0 This interface is not supported.
> -
> -**/
> -UINTN
> -EFIAPI
> -HmacSha256GetContextSize (
> - VOID
> - )
> -{
> - ASSERT (FALSE);
> - return 0;
> -}
> -
> /**
> Allocates and initializes one HMAC_CTX context for subsequent HMAC-SHA256 use.
>
>
(9) Same as (7).
(10) I think this patch is very difficult to review; it is too large,
doing too many things at the same time. Here's how I propose splitting
it up:
* Patch#1:
- rename Init to SetKey, updating the lib class header and all the
library instances too,
- update all comments that refer to Init, as well,
- update the non-Null implementations of the SetKey function (removing
memset() and the HMAC_CTX_reset()).
This patch should be called:
CryptoPkg/BaseCryptLib: replace HmacXxxInit API with HmacXxxSetKey
The commit message should mention that we will no longer support
user-supplied memory for contexts, and so the Init interface is replaced
with SetKey, which will only operate on contexts created with New.
* Patch#2:
- everything else
and it should be called:
CryptoPkg/BaseCryptLib: remove HmacXxxGetContextSize interface
Thanks
Laszlo
next prev parent reply other threads:[~2020-01-14 9:45 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-14 6:26 [PATCH v2] CryptoPkg/BaseCryptLib: remove HmacXxxGetContextSize interface Wang, Jian J
2020-01-14 9:45 ` Laszlo Ersek [this message]
2020-01-16 3:11 ` Wang, Jian J
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=fae2d23b-ac02-3dc2-9e25-ac0c4b6f0aa4@redhat.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