From: "Wang, Jian J" <jian.j.wang@intel.com>
To: Laszlo Ersek <lersek@redhat.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Lu, XiaoyuX" <xiaoyux.lu@intel.com>
Subject: Re: [PATCH v2] CryptoPkg/BaseCryptLib: remove HmacXxxGetContextSize interface
Date: Thu, 16 Jan 2020 03:11:05 +0000 [thread overview]
Message-ID: <D827630B58408649ACB04F44C5100036259F54E7@SHSMSX107.ccr.corp.intel.com> (raw)
In-Reply-To: <fae2d23b-ac02-3dc2-9e25-ac0c4b6f0aa4@redhat.com>
Laszlo,
Thanks for the comments. I'll send v3 to include all your suggestions.
Regards,
Jian
> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Tuesday, January 14, 2020 5:45 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; devel@edk2.groups.io
> Cc: Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Subject: Re: [PATCH v2] CryptoPkg/BaseCryptLib: remove
> HmacXxxGetContextSize interface
>
> 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
prev parent reply other threads:[~2020-01-16 3:11 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
2020-01-16 3:11 ` Wang, Jian J [this message]
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=D827630B58408649ACB04F44C5100036259F54E7@SHSMSX107.ccr.corp.intel.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