public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Yao, Jiewen" <jiewen.yao@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"Yao, Jiewen" <jiewen.yao@intel.com>,
	Laszlo Ersek <lersek@redhat.com>,
	"Zurcher, Christopher J" <christopher.j.zurcher@intel.com>
Cc: "Wang, Jian J" <jian.j.wang@intel.com>,
	"Lu, XiaoyuX" <xiaoyux.lu@intel.com>
Subject: Re: [edk2-devel] [PATCH v3 1/3] CryptoPkg/BaseCryptLib: Add EVP (Envelope) Digest interface
Date: Wed, 16 Sep 2020 12:07:36 +0000	[thread overview]
Message-ID: <CY4PR11MB1288333636B3B9577D2B40E38C210@CY4PR11MB1288.namprd11.prod.outlook.com> (raw)

I overlooked the behavior that NULL DigestValue will take side effect to free Context.

Hi Christopher
May I understand why we need free the context?
This does not align with other existing HASH or HMAC functions.

Thank you
Yao Jiewen

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Wednesday, September 16, 2020 7:45 PM
> To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io; Zurcher,
> Christopher J <christopher.j.zurcher@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Subject: Re: [edk2-devel] [PATCH v3 1/3] CryptoPkg/BaseCryptLib: Add EVP
> (Envelope) Digest interface
> 
> Hi Laszlo
> I disagree to put OPTIONAL for DigestValue, just because NULL is checked.
> If we need follow this, then we need add OPTIONAL to almost all pointer, which
> is unnecessary.
> 
> > +BOOLEAN
> > +EFIAPI
> > +EvpMdFinal (
> > +  IN OUT  VOID   *EvpMdContext,
> > +  OUT     UINT8  *DigestValue
> 
> (3) DigestValue should be marked OPTIONAL in my opinion, as NULL is
> deliberately permitted (for just freeing the context).
> 
> 
> 
> 
> > -----Original Message-----
> > From: Laszlo Ersek <lersek@redhat.com>
> > Sent: Wednesday, September 16, 2020 7:07 PM
> > To: devel@edk2.groups.io; Zurcher, Christopher J
> > <christopher.j.zurcher@intel.com>
> > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>;
> > Lu, XiaoyuX <xiaoyux.lu@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v3 1/3] CryptoPkg/BaseCryptLib: Add EVP
> > (Envelope) Digest interface
> >
> > Hello Christopher,
> >
> > On 09/16/20 02:59, Zurcher, Christopher J wrote:
> > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2545
> > >
> > > The EVP interface should be used in place of discrete digest function
> > > calls.
> > >
> > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> > > Signed-off-by: Christopher J Zurcher <christopher.j.zurcher@intel.com>
> > > ---
> > >  CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf         |   1 +
> > >  CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf          |   1 +
> > >  CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf      |   1 +
> > >  CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf          |   1 +
> > >  CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf |   1 +
> > >  CryptoPkg/Include/Library/BaseCryptLib.h                | 129 ++++++++++
> > >  CryptoPkg/Library/BaseCryptLib/Evp/CryptEvpMd.c         | 257
> > ++++++++++++++++++++
> > >  CryptoPkg/Library/BaseCryptLib/Evp/CryptEvpMdNull.c     | 128 ++++++++++
> > >  CryptoPkg/Library/BaseCryptLibNull/Evp/CryptEvpMdNull.c | 128
> ++++++++++
> > >  9 files changed, 647 insertions(+)
> >
> > I agree that "CryptoPkg/Library/BaseCryptLibNull/Evp/CryptEvpMdNull.c"
> > is necessary. (If I understand correctly, that file was missing from
> > your v2 posting.)
> >
> > But "CryptoPkg/Library/BaseCryptLib/Evp/CryptEvpMdNull.c" seems
> > superfluous. This file is never referenced in the INF files.
> >
> > The point of this file would be to allow *some* of the Base / Pei /
> > Runtime / Smm instances to "stub out" the EVP MD functions (i.e. provide
> > only stub implementations). But this isn't what's happening -- all of
> > the Base / Pei / Runtime / Smm instances are getting the real deal
> > ("CryptoPkg/Library/BaseCryptLib/Evp/CryptEvpMd.c").
> >
> > (1) So I think "CryptoPkg/Library/BaseCryptLib/Evp/CryptEvpMdNull.c"
> > should be dropped. Only the Null instance of the library needs null
> > versions of the new functions. The Base / Pei / Runtime / Smm instances
> > don't.
> >
> > >
> > > diff --git a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> > b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> > > index 4aae2aba95..3968f29412 100644
> > > --- a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> > > +++ b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> > > @@ -50,6 +50,7 @@
> > >    Pk/CryptAuthenticode.c
> > >    Pk/CryptTs.c
> > >    Pem/CryptPem.c
> > > +  Evp/CryptEvpMd.c
> > >
> > >    SysCall/CrtWrapper.c
> > >    SysCall/TimerWrapper.c
> > > diff --git a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
> > b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
> > > index dc28e3a11d..d0b91716d0 100644
> > > --- a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
> > > +++ b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
> > > @@ -57,6 +57,7 @@
> > >    Pk/CryptTsNull.c
> > >    Pem/CryptPemNull.c
> > >    Rand/CryptRandNull.c
> > > +  Evp/CryptEvpMd.c
> > >
> > >    SysCall/CrtWrapper.c
> > >    SysCall/ConstantTimeClock.c
> > > diff --git a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> > b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> > > index 5005beed02..9f3accd35b 100644
> > > --- a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> > > +++ b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> > > @@ -56,6 +56,7 @@
> > >    Pk/CryptAuthenticodeNull.c
> > >    Pk/CryptTsNull.c
> > >    Pem/CryptPem.c
> > > +  Evp/CryptEvpMd.c
> > >
> > >    SysCall/CrtWrapper.c
> > >    SysCall/TimerWrapper.c
> > > diff --git a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> > b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> > > index 91ec3e03bf..420623cdc6 100644
> > > --- a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> > > +++ b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> > > @@ -54,6 +54,7 @@
> > >    Pk/CryptAuthenticodeNull.c
> > >    Pk/CryptTsNull.c
> > >    Pem/CryptPem.c
> > > +  Evp/CryptEvpMd.c
> > >
> > >    SysCall/CrtWrapper.c
> > >    SysCall/ConstantTimeClock.c
> > > diff --git a/CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf
> > b/CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf
> > > index 689af4fedd..542ac2e2e1 100644
> > > --- a/CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf
> > > +++ b/CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf
> > > @@ -50,6 +50,7 @@
> > >    Pk/CryptTsNull.c
> > >    Pem/CryptPemNull.c
> > >    Rand/CryptRandNull.c
> > > +  Evp/CryptEvpMdNull.c
> > >
> > >  [Packages]
> > >    MdePkg/MdePkg.dec
> > > diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h
> > b/CryptoPkg/Include/Library/BaseCryptLib.h
> > > index ae9bde9e37..5e1b408b54 100644
> > > --- a/CryptoPkg/Include/Library/BaseCryptLib.h
> > > +++ b/CryptoPkg/Include/Library/BaseCryptLib.h
> > > @@ -1012,6 +1012,135 @@ HmacSha256Final (
> > >    OUT     UINT8  *HmacValue
> > >    );
> > >
> > >
> >
> +//==============================================================
> > =======================
> > > +//    EVP (Envelope) Primitive
> > >
> >
> +//==============================================================
> > =======================
> > > +
> > > +/**
> > > +  Allocates and initializes one EVP_MD_CTX context for subsequent EVP_MD
> > use.
> > > +
> > > +  If DigestName is NULL, then return FALSE.
> > > +
> > > +  @param[in]    DigestName    Pointer to the digest name as a NULL-
> > terminated ASCII string.
> > > +                              Valid digest names are:
> > > +                              MD5, SHA1, SHA224, SHA256, SHA384, SHA512
> > > +                              SHA3-224, SHA3-256, SHA3-384, SHA3-512
> > > +                              SM3
> > > +
> > > +  @return  Pointer to the EVP_MD_CTX context that has been allocated and
> > initialized.
> > > +           If DigestName is invalid, returns NULL.
> > > +           If the allocations fails, returns NULL.
> > > +           If initialization fails, returns NULL.
> > > +
> > > +**/
> > > +VOID *
> > > +EFIAPI
> > > +EvpMdInit (
> > > +  IN  CONST CHAR8   *DigestName
> > > +  );
> > > +
> > > +/**
> > > +  Makes a copy of an existing EVP_MD context.
> > > +
> > > +  If EvpMdContext is NULL, then return FALSE.
> > > +  If NewEvpMdContext is NULL, then return FALSE.
> > > +
> > > +  @param[in]  EvpMdContext     Pointer to EVP_MD context being copied.
> > > +  @param[out] NewEvpMdContext  Pointer to new EVP_MD context.
> > > +
> > > +  @retval TRUE   EVP_MD context copy succeeded.
> > > +  @retval FALSE  EVP_MD context copy failed.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +EvpMdDuplicate (
> > > +  IN  CONST VOID    *EvpMdContext,
> > > +  OUT VOID          *NewEvpMdContext
> > > +  );
> > > +
> > > +/**
> > > +  Digests the input data and updates EVP_MD context.
> > > +
> > > +  This function performs EVP 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.
> > > +  EVP_MD context should be already correctly initialized by EvpMdInit(), and
> > should not
> > > +  be finalized by EvpMdFinal(). Behavior with invalid context is undefined.
> > > +
> > > +  If EvpMdContext is NULL, then return FALSE.
> > > +  If Data is NULL and DataSize is not zero, return FALSE.
> > > +
> > > +  @param[in, out]  EvpMdContext       Pointer to the EVP_MD context.
> > > +  @param[in]       Data               Pointer to the buffer containing the data to
> be
> > digested.
> > > +  @param[in]       DataSize           Size of Data buffer in bytes.
> > > +
> > > +  @retval TRUE   EVP data digest succeeded.
> > > +  @retval FALSE  EVP data digest failed.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +EvpMdUpdate (
> > > +  IN OUT  VOID        *EvpMdContext,
> > > +  IN      CONST VOID  *Data,
> > > +  IN      UINTN       DataSize
> > > +  );
> > > +
> > > +/**
> > > +  Completes computation of the EVP digest value.
> > > +  Releases the specified EVP_MD_CTX context.
> > > +
> > > +  This function completes EVP hash computation and retrieves the digest
> > value into
> > > +  the specified memory. After this function has been called, the EVP context
> > cannot
> > > +  be used again.
> > > +  EVP context should be already correctly initialized by EvpMdInit(), and
> > should
> > > +  not be finalized by EvpMdFinal(). Behavior with invalid EVP context is
> > undefined.
> > > +
> > > +  If EvpMdContext is NULL, then return FALSE.
> > > +  If DigestValue is NULL, free the Context then return FALSE.
> > > +
> > > +  @param[in, out]  EvpMdContext   Pointer to the EVP context.
> > > +  @param[out]      Digest         Pointer to a buffer that receives the EVP
> digest
> > value.
> >
> > (2) Please extend the comment on Digest with the following:
> >
> >   The caller is responsible for providing enough storage for the digest
> >   algorithm selected with EvpMdInit(). Providing EVP_MAX_MD_SIZE bytes
> >   will suffice for storing the digest regardless of the algorithm chosen
> >   in EvpMdInit().
> >
> > (EVP_MAX_MD_SIZE is a public OpenSSL macro and I think we should openly
> > advertise it to consumers in edk2.)
> >
> > > +
> > > +  @retval TRUE   EVP digest computation succeeded.
> > > +  @retval FALSE  EVP digest computation failed.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +EvpMdFinal (
> > > +  IN OUT  VOID   *EvpMdContext,
> > > +  OUT     UINT8  *DigestValue
> >
> > (3) DigestValue should be marked OPTIONAL in my opinion, as NULL is
> > deliberately permitted (for just freeing the context).
> >
> > > +  );
> > > +
> > > +/**
> > > +  Computes the message digest of an input data buffer.
> > > +
> > > +  This function performs the message digest of a given data buffer, and
> places
> > > +  the digest value into the specified memory.
> > > +
> > > +  If DigestName is NULL, return FALSE.
> > > +  If Data is NULL and DataSize is not zero, return FALSE.
> > > +  If HashValue is NULL, return FALSE.
> > > +
> > > +  @param[in]    DigestName    Pointer to the digest name.
> > > +  @param[in]    Data          Pointer to the buffer containing the data to be
> > hashed.
> > > +  @param[in]    DataSize      Size of Data buffer in bytes.
> > > +  @param[out]   HashValue     Pointer to a buffer that receives the digest
> > value.
> > > +
> > > +  @retval TRUE   Digest computation succeeded.
> > > +  @retval FALSE  Digest computation failed.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +EvpMdHashAll (
> > > +  IN  CONST CHAR8   *DigestName,
> > > +  IN  CONST VOID    *Data,
> > > +  IN  UINTN         DataSize,
> > > +  OUT UINT8         *HashValue
> > > +  );
> > > +
> > >
> >
> //===============================================================
> > ======================
> > >  //    Symmetric Cryptography Primitive
> > >
> >
> //===============================================================
> > ======================
> > > diff --git a/CryptoPkg/Library/BaseCryptLib/Evp/CryptEvpMd.c
> > b/CryptoPkg/Library/BaseCryptLib/Evp/CryptEvpMd.c
> > > new file mode 100644
> > > index 0000000000..b2770a9186
> > > --- /dev/null
> > > +++ b/CryptoPkg/Library/BaseCryptLib/Evp/CryptEvpMd.c
> > > @@ -0,0 +1,257 @@
> > > +/** @file
> > > +  EVP MD Wrapper Implementation for OpenSSL.
> > > +
> > > +Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> > > +SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +
> > > +**/
> > > +
> > > +#include "InternalCryptLib.h"
> > > +#include <openssl/evp.h>
> > > +
> > > +/**
> > > +  Allocates and initializes one EVP_MD_CTX context for subsequent EVP_MD
> > use.
> > > +
> > > +  If DigestName is NULL, then return FALSE.
> > > +
> > > +  @param[in]    DigestName    Pointer to the digest name as a NULL-
> > terminated ASCII string.
> > > +                              Valid digest names are:
> > > +                              MD5, SHA1, SHA224, SHA256, SHA384, SHA512
> > > +                              SHA3-224, SHA3-256, SHA3-384, SHA3-512
> > > +                              SM3
> > > +
> > > +  @return  Pointer to the EVP_MD_CTX context that has been allocated and
> > initialized.
> > > +           If DigestName is invalid, returns NULL.
> > > +           If the allocations fails, returns NULL.
> > > +           If initialization fails, returns NULL.
> > > +
> > > +**/
> > > +VOID *
> > > +EFIAPI
> > > +EvpMdInit (
> > > +  IN  CONST CHAR8   *DigestName
> > > +  )
> > > +{
> > > +  EVP_MD    *Digest;
> > > +  VOID      *EvpMdContext;
> > > +
> > > +  //
> > > +  // Check input parameters.
> > > +  //
> > > +  if (DigestName == NULL) {
> > > +    return NULL;
> > > +  }
> > > +
> > > +  //
> > > +  // Allocate EVP_MD_CTX Context
> > > +  //
> > > +  EvpMdContext = EVP_MD_CTX_new ();
> > > +  if (EvpMdContext == NULL) {
> > > +    return NULL;
> > > +  }
> > > +
> > > +  Digest = EVP_get_digestbyname (DigestName);
> >
> > I think this may not compile with gcc (and correctly so). The pointer
> > returned by EVP_get_digestbyname() is const-qualified, but with the
> > assignment, we're throwing away the const-ness.
> >
> > (4) Please const-qualify the "Digest" local pointer.
> >
> > > +  if (Digest == NULL) {
> > > +    return NULL;
> > > +  }
> >
> > (5) This is a memory leak I believe; "EvpMdContext" is leaked.
> >
> > For keeping the control flow simple, consider moving
> > EVP_get_digestbyname() above EVP_MD_CTX_new().
> >
> > > +
> > > +  //
> > > +  // Initialize Context
> > > +  //
> > > +  if (EVP_DigestInit_ex (EvpMdContext, Digest, NULL) != 1) {
> > > +    EVP_MD_CTX_free (EvpMdContext);
> > > +    return NULL;
> > > +  }
> > > +
> > > +  return EvpMdContext;
> > > +}
> > > +
> > > +/**
> > > +  Makes a copy of an existing EVP_MD context.
> > > +
> > > +  If EvpMdContext is NULL, then return FALSE.
> > > +  If NewEvpMdContext is NULL, then return FALSE.
> > > +
> > > +  @param[in]  EvpMdContext     Pointer to EVP_MD context being copied.
> > > +  @param[out] NewEvpMdContext  Pointer to new EVP_MD context.
> > > +
> > > +  @retval TRUE   EVP_MD context copy succeeded.
> > > +  @retval FALSE  EVP_MD context copy failed.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +EvpMdDuplicate (
> > > +  IN  CONST VOID    *EvpMdContext,
> > > +  OUT VOID          *NewEvpMdContext
> > > +  )
> > > +{
> > > +  //
> > > +  // Check input parameters.
> > > +  //
> > > +  if (EvpMdContext == NULL || NewEvpMdContext == NULL) {
> > > +    return FALSE;
> > > +  }
> > > +
> > > +  if (EVP_MD_CTX_copy (NewEvpMdContext, EvpMdContext) != 1) {
> > > +    return FALSE;
> > > +  }
> > > +
> > > +  return TRUE;
> > > +}
> >
> > (6) Can you please confirm that the caller is supposed to initialize
> > "NewEvpMdContext" with EvpMdInit() first, before calling EvpMdDuplicate()?
> >
> > > +
> > > +/**
> > > +  Digests the input data and updates EVP_MD context.
> > > +
> > > +  This function performs EVP 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.
> > > +  EVP_MD context should be already correctly initialized by EvpMdInit(), and
> > should not
> > > +  be finalized by EvpMdFinal(). Behavior with invalid context is undefined.
> > > +
> > > +  If EvpMdContext is NULL, then return FALSE.
> > > +  If Data is NULL and DataSize is not zero, return FALSE.
> > > +
> > > +  @param[in, out]  EvpMdContext       Pointer to the EVP_MD context.
> > > +  @param[in]       Data               Pointer to the buffer containing the data to
> be
> > digested.
> > > +  @param[in]       DataSize           Size of Data buffer in bytes.
> > > +
> > > +  @retval TRUE   EVP data digest succeeded.
> > > +  @retval FALSE  EVP data digest failed.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +EvpMdUpdate (
> > > +  IN OUT  VOID        *EvpMdContext,
> > > +  IN      CONST VOID  *Data,
> > > +  IN      UINTN       DataSize
> > > +  )
> > > +{
> > > +  //
> > > +  // Check input parameters.
> > > +  //
> > > +  if (EvpMdContext == NULL) {
> > > +    return FALSE;
> > > +  }
> > > +
> > > +  //
> > > +  // Check invalid parameters, in case only DataLength was checked in
> > OpenSSL
> > > +  //
> > > +  if (Data == NULL && DataSize != 0) {
> > > +    return FALSE;
> > > +  }
> > > +
> > > +  //
> > > +  // OpenSSL EVP digest update
> > > +  //
> > > +  if (EVP_DigestUpdate (EvpMdContext, Data, DataSize) != 1) {
> > > +    return FALSE;
> > > +  }
> > > +
> > > +  return TRUE;
> > > +}
> > > +
> > > +/**
> > > +  Completes computation of the EVP digest value.
> > > +  Releases the specified EVP_MD_CTX context.
> > > +
> > > +  This function completes EVP hash computation and retrieves the digest
> > value into
> > > +  the specified memory. After this function has been called, the EVP context
> > cannot
> > > +  be used again.
> > > +  EVP context should be already correctly initialized by EvpMdInit(), and
> > should
> > > +  not be finalized by EvpMdFinal(). Behavior with invalid EVP context is
> > undefined.
> > > +
> > > +  If EvpMdContext is NULL, then return FALSE.
> > > +  If DigestValue is NULL, free the Context then return FALSE.
> > > +
> > > +  @param[in, out]  EvpMdContext   Pointer to the EVP context.
> > > +  @param[out]      Digest         Pointer to a buffer that receives the EVP
> digest
> > value.
> > > +
> > > +  @retval TRUE   EVP digest computation succeeded.
> > > +  @retval FALSE  EVP digest computation failed.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +EvpMdFinal (
> > > +  IN OUT  VOID   *EvpMdContext,
> > > +  OUT     UINT8  *DigestValue
> > > +  )
> > > +{
> > > +  UINT32    Length;
> > > +  BOOLEAN   ReturnValue;
> > > +
> > > +  ReturnValue = TRUE;
> > > +
> > > +  //
> > > +  // Check input parameters.
> > > +  //
> > > +  if (EvpMdContext == NULL) {
> > > +    return FALSE;
> > > +  }
> > > +  if (DigestValue == NULL) {
> > > +    EVP_MD_CTX_free (EvpMdContext);
> > > +    return FALSE;
> > > +  }
> > > +
> > > +  //
> > > +  // OpenSSL EVP digest finalization
> > > +  //
> > > +  if (EVP_DigestFinal_ex (EvpMdContext, DigestValue, &Length) != 1) {
> > > +    ReturnValue = FALSE;
> > > +  }
> >
> >
> > (7) I suggest dropping the "Length" local variable. EVP_DigestFinal_ex()
> > deals fine with the third parameter being NULL, according to the docs
> > (and the code).
> >
> >
> > > +
> > > +  //
> > > +  // Free OpenSSL EVP_MD_CTX Context
> > > +  //
> > > +  EVP_MD_CTX_free (EvpMdContext);
> > > +
> > > +  return ReturnValue;
> > > +}
> > > +
> > > +/**
> > > +  Computes the message digest of an input data buffer.
> > > +
> > > +  This function performs the message digest of a given data buffer, and
> places
> > > +  the digest value into the specified memory.
> > > +
> > > +  If DigestName is NULL, return FALSE.
> > > +  If Data is NULL and DataSize is not zero, return FALSE.
> > > +  If HashValue is NULL, return FALSE.
> > > +
> > > +  @param[in]    DigestName    Pointer to the digest name.
> > > +  @param[in]    Data          Pointer to the buffer containing the data to be
> > hashed.
> > > +  @param[in]    DataSize      Size of Data buffer in bytes.
> > > +  @param[out]   HashValue     Pointer to a buffer that receives the digest
> > value.
> > > +
> > > +  @retval TRUE   Digest computation succeeded.
> > > +  @retval FALSE  Digest computation failed.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +EvpMdHashAll (
> > > +  IN  CONST CHAR8   *DigestName,
> > > +  IN  CONST VOID    *Data,
> > > +  IN  UINTN         DataSize,
> > > +  OUT UINT8         *HashValue
> > > +  )
> > > +{
> > > +  BOOLEAN   Result;
> > > +  VOID      *EvpMdContext;
> > > +
> > > +  EvpMdContext = EvpMdInit (DigestName);
> > > +  if (EvpMdContext == NULL) {
> > > +    return FALSE;
> > > +  }
> > > +
> > > +  Result = EvpMdUpdate (EvpMdContext, Data, DataSize);
> > > +  if (Result == FALSE) {
> >
> > (8) Style: please write (!Result).
> >
> >
> > > +    EvpMdFinal (EvpMdContext, NULL);
> > > +    return FALSE;
> > > +  }
> > > +
> > > +  Result = EvpMdFinal (EvpMdContext, HashValue);
> > > +
> > > +  return Result;
> > > +}
> > > diff --git a/CryptoPkg/Library/BaseCryptLib/Evp/CryptEvpMdNull.c
> > b/CryptoPkg/Library/BaseCryptLib/Evp/CryptEvpMdNull.c
> > > new file mode 100644
> > > index 0000000000..038f63801f
> > > --- /dev/null
> > > +++ b/CryptoPkg/Library/BaseCryptLib/Evp/CryptEvpMdNull.c
> > > @@ -0,0 +1,128 @@
> > > +/** @file
> > > +  EVP MD Wrapper Null Library.
> > > +
> > > +Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> > > +SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +
> > > +**/
> > > +
> > > +#include "InternalCryptLib.h"
> > > +
> > > +/**
> > > +  Allocates and initializes one EVP_MD_CTX context for subsequent EVP_MD
> > use.
> > > +
> > > +  Return FALSE to indicate this interface is not supported.
> > > +
> > > +  @param[in]    DigestName    Pointer to the digest name as a NULL-
> > terminated ASCII string.
> > > +                              Valid digest names are:
> > > +                              MD5, SHA1, SHA224, SHA256, SHA384, SHA512
> > > +                              SHA3-224, SHA3-256, SHA3-384, SHA3-512
> > > +                              SM3
> > > +
> > > +  @return NULL  This interface is not supported.
> > > +
> > > +**/
> > > +VOID *
> > > +EFIAPI
> > > +EvpMdInit (
> > > +  IN  CONST CHAR8   *DigestName
> > > +  )
> > > +{
> > > +  ASSERT (FALSE);
> > > +  return NULL;
> > > +}
> > > +
> > > +/**
> > > +  Makes a copy of an existing EVP_MD context.
> > > +
> > > +  Return FALSE to indicate this interface is not supported.
> > > +
> > > +  @param[in]  EvpMdContext     Pointer to EVP_MD context being copied.
> > > +  @param[out] NewEvpMdContext  Pointer to new EVP_MD context.
> > > +
> > > +  @retval FALSE  This interface is not supported.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +EvpMdDuplicate (
> > > +  IN  CONST VOID    *EvpMdContext,
> > > +  OUT VOID          *NewEvpMdContext
> > > +  )
> > > +{
> > > +  ASSERT (FALSE);
> > > +  return FALSE;
> > > +}
> > > +
> > > +/**
> > > +  Digests the input data and updates EVP_MD context.
> > > +
> > > +  Return FALSE to indicate this interface is not supported.
> > > +
> > > +  @param[in, out]  EvpMdContext       Pointer to the EVP_MD context.
> > > +  @param[in]       Data               Pointer to the buffer containing the data to
> be
> > digested.
> > > +  @param[in]       DataSize           Size of Data buffer in bytes.
> > > +
> > > +  @retval FALSE  This interface is not supported.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +EvpMdUpdate (
> > > +  IN OUT  VOID        *EvpMdContext,
> > > +  IN      CONST VOID  *Data,
> > > +  IN      UINTN       DataSize
> > > +  )
> > > +{
> > > +  ASSERT (FALSE);
> > > +  return FALSE;
> > > +}
> > > +
> > > +/**
> > > +  Completes computation of the EVP digest value.
> > > +  Releases the specified EVP_MD_CTX context.
> > > +
> > > +  Return FALSE to indicate this interface is not supported.
> > > +
> > > +  @param[in, out]  EvpMdContext   Pointer to the EVP context.
> > > +  @param[out]      Digest         Pointer to a buffer that receives the EVP
> digest
> > value.
> > > +
> > > +  @retval FALSE  This interface is not supported.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +EvpMdFinal (
> > > +  IN OUT  VOID   *EvpMdContext,
> > > +  OUT     UINT8  *DigestValue
> > > +  )
> > > +{
> > > +  ASSERT (FALSE);
> > > +  return FALSE;
> > > +}
> > > +
> > > +/**
> > > +  Computes the message digest of an input data buffer.
> > > +
> > > +  Return FALSE to indicate this interface is not supported.
> > > +
> > > +  @param[in]    DigestName    Pointer to the digest name.
> > > +  @param[in]    Data          Pointer to the buffer containing the data to be
> > hashed.
> > > +  @param[in]    DataSize      Size of Data buffer in bytes.
> > > +  @param[out]   HashValue     Pointer to a buffer that receives the digest
> > value.
> > > +
> > > +  @retval FALSE  This interface is not supported.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +EvpMdHashAll (
> > > +  IN  CONST CHAR8   *DigestName,
> > > +  IN  CONST VOID    *Data,
> > > +  IN  UINTN         DataSize,
> > > +  OUT UINT8         *HashValue
> > > +  )
> > > +{
> > > +  ASSERT (FALSE);
> > > +  return FALSE;
> > > +}
> > > diff --git a/CryptoPkg/Library/BaseCryptLibNull/Evp/CryptEvpMdNull.c
> > b/CryptoPkg/Library/BaseCryptLibNull/Evp/CryptEvpMdNull.c
> > > new file mode 100644
> > > index 0000000000..038f63801f
> > > --- /dev/null
> > > +++ b/CryptoPkg/Library/BaseCryptLibNull/Evp/CryptEvpMdNull.c
> > > @@ -0,0 +1,128 @@
> > > +/** @file
> > > +  EVP MD Wrapper Null Library.
> > > +
> > > +Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> > > +SPDX-License-Identifier: BSD-2-Clause-Patent
> > > +
> > > +**/
> > > +
> > > +#include "InternalCryptLib.h"
> > > +
> > > +/**
> > > +  Allocates and initializes one EVP_MD_CTX context for subsequent EVP_MD
> > use.
> > > +
> > > +  Return FALSE to indicate this interface is not supported.
> > > +
> > > +  @param[in]    DigestName    Pointer to the digest name as a NULL-
> > terminated ASCII string.
> > > +                              Valid digest names are:
> > > +                              MD5, SHA1, SHA224, SHA256, SHA384, SHA512
> > > +                              SHA3-224, SHA3-256, SHA3-384, SHA3-512
> > > +                              SM3
> > > +
> > > +  @return NULL  This interface is not supported.
> > > +
> > > +**/
> > > +VOID *
> > > +EFIAPI
> > > +EvpMdInit (
> > > +  IN  CONST CHAR8   *DigestName
> > > +  )
> > > +{
> > > +  ASSERT (FALSE);
> > > +  return NULL;
> > > +}
> > > +
> > > +/**
> > > +  Makes a copy of an existing EVP_MD context.
> > > +
> > > +  Return FALSE to indicate this interface is not supported.
> > > +
> > > +  @param[in]  EvpMdContext     Pointer to EVP_MD context being copied.
> > > +  @param[out] NewEvpMdContext  Pointer to new EVP_MD context.
> > > +
> > > +  @retval FALSE  This interface is not supported.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +EvpMdDuplicate (
> > > +  IN  CONST VOID    *EvpMdContext,
> > > +  OUT VOID          *NewEvpMdContext
> > > +  )
> > > +{
> > > +  ASSERT (FALSE);
> > > +  return FALSE;
> > > +}
> > > +
> > > +/**
> > > +  Digests the input data and updates EVP_MD context.
> > > +
> > > +  Return FALSE to indicate this interface is not supported.
> > > +
> > > +  @param[in, out]  EvpMdContext       Pointer to the EVP_MD context.
> > > +  @param[in]       Data               Pointer to the buffer containing the data to
> be
> > digested.
> > > +  @param[in]       DataSize           Size of Data buffer in bytes.
> > > +
> > > +  @retval FALSE  This interface is not supported.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +EvpMdUpdate (
> > > +  IN OUT  VOID        *EvpMdContext,
> > > +  IN      CONST VOID  *Data,
> > > +  IN      UINTN       DataSize
> > > +  )
> > > +{
> > > +  ASSERT (FALSE);
> > > +  return FALSE;
> > > +}
> > > +
> > > +/**
> > > +  Completes computation of the EVP digest value.
> > > +  Releases the specified EVP_MD_CTX context.
> > > +
> > > +  Return FALSE to indicate this interface is not supported.
> > > +
> > > +  @param[in, out]  EvpMdContext   Pointer to the EVP context.
> > > +  @param[out]      Digest         Pointer to a buffer that receives the EVP
> digest
> > value.
> > > +
> > > +  @retval FALSE  This interface is not supported.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +EvpMdFinal (
> > > +  IN OUT  VOID   *EvpMdContext,
> > > +  OUT     UINT8  *DigestValue
> > > +  )
> > > +{
> > > +  ASSERT (FALSE);
> > > +  return FALSE;
> > > +}
> > > +
> > > +/**
> > > +  Computes the message digest of an input data buffer.
> > > +
> > > +  Return FALSE to indicate this interface is not supported.
> > > +
> > > +  @param[in]    DigestName    Pointer to the digest name.
> > > +  @param[in]    Data          Pointer to the buffer containing the data to be
> > hashed.
> > > +  @param[in]    DataSize      Size of Data buffer in bytes.
> > > +  @param[out]   HashValue     Pointer to a buffer that receives the digest
> > value.
> > > +
> > > +  @retval FALSE  This interface is not supported.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +EFIAPI
> > > +EvpMdHashAll (
> > > +  IN  CONST CHAR8   *DigestName,
> > > +  IN  CONST VOID    *Data,
> > > +  IN  UINTN         DataSize,
> > > +  OUT UINT8         *HashValue
> > > +  )
> > > +{
> > > +  ASSERT (FALSE);
> > > +  return FALSE;
> > > +}
> > >
> >
> > Thanks,
> > Laszlo
> 
> 
> 


             reply	other threads:[~2020-09-16 12:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-16 12:07 Yao, Jiewen [this message]
2020-09-16 12:39 ` [edk2-devel] [PATCH v3 1/3] CryptoPkg/BaseCryptLib: Add EVP (Envelope) Digest interface Laszlo Ersek
2020-09-16 12:56   ` Yao, Jiewen
2020-09-16 15:27     ` Laszlo Ersek
2020-09-16 13:31   ` Laszlo Ersek
  -- strict thread matches above, loose matches on Subject: below --
2020-09-16  0:58 [PATCH v3 0/3] " Zurcher, Christopher J
2020-09-16  0:59 ` [PATCH v3 1/3] " Zurcher, Christopher J
2020-09-16 11:06   ` [edk2-devel] " Laszlo Ersek
2020-09-16 11:44     ` Yao, Jiewen
2020-09-16 13:35       ` Laszlo Ersek
2020-09-16 14:17         ` Yao, Jiewen
2020-09-16 16:04           ` Laszlo Ersek
2020-09-17  1:21             ` Zurcher, Christopher J
2020-09-17  1:33               ` Yao, Jiewen
2020-09-17  1:44                 ` Zurcher, Christopher 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=CY4PR11MB1288333636B3B9577D2B40E38C210@CY4PR11MB1288.namprd11.prod.outlook.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