public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
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


      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