From: "Yao, Jiewen" <jiewen.yao@intel.com>
To: Laszlo Ersek <lersek@redhat.com>,
"devel@edk2.groups.io" <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
Date: Wed, 16 Sep 2020 14:17:17 +0000 [thread overview]
Message-ID: <CY4PR11MB128808CFC405504B31F2A9958C210@CY4PR11MB1288.namprd11.prod.outlook.com> (raw)
In-Reply-To: <d21a3f43-9f81-294d-6db7-37b1208ddcda@redhat.com>
Thank you Laszlo.
You forced me to read the code again and more carefully. :-)
I believe I understand why you use NULL to free the context now - to support error path.
First, I did have some new thought for EVP API. Let’s consider 3 cases, 1) Hash all data one time, 2) Hash update data multiple times, 3) Error during update.
A. In current design, the API sequence is:
1) EvpMdHashAll (Digest)
2) EvpMdInit, EvpMdUpdate, EvpMdUpdate, ... , EvpMdFinal (Digest)
3) EvpMdInit, EvpMdUpdate->ERROR, EvpMdFinal (NULL)
B. We can auto free context in EvpMdUpdate in error path - the API sequence is:
1) EvpMdHashAll (Digest)
2) EvpMdInit, EvpMdUpdate, EvpMdUpdate, ... , EvpMdFinal (Digest)
3) EvpMdInit, EvpMdUpdate->ERROR
C: We can use New/Free style - similar to HMAC
1) EvpMdHashAll (Digest)
2) EvpMdNew, EvpMdInit, EvpMdUpdate, EvpMdUpdate, ... , EvpMdFinal (Digest), EvpMdFree
3) EvpMdNew, EvpMdInit, EvpMdUpdate->ERROR, EvpMdFree
I checked below APIs:
1) openssl (EVP_MD_CTX_new, EVP_DigestInit_ex, EVP_DigestUpdate, EVP_DigestFinal_ex, EVP_MD_CTX_free)
2) mbedtls (mbedtls_sha256_init, mbedtls_sha256_starts_ret, mbedtls_sha256_update_ret, mbedtls_sha256_finish_ret, mbedtls_sha256_free)
3) BaseCryptoLib HMAC (HmacSha256New, HmacSha256SetKey, HmacSha256Update, HmacSha256Final, HmacSha256Free)
All APIs include free function to free the context, I don’t think it is a bad idea to have EvpMdFree() API.
I would like to propose option - C.
Second, I believe EVP is just something in openssl. It does not mean that we *have to* design API to follow openssl.
After rethink the size impact, I do have concern to merge all Hash implementation together in one function.
It might means nothing if the crypto library is based upon openssl.
But if the cryptolib implementation is based upon other crypto, such as Intel IPP (https://github.com/slimbootloader/slimbootloader/tree/master/BootloaderCommonPkg/Library/IppCryptoLib) or mbedTls (https://github.com/jyao1/edk2/tree/DeviceSecurity/CryptoMbedTlsPkg), then we can NOT get size benefit by separating the hash algorithm.
I would like to propose separate EvpMdxxx.
EvpMdNew -> Sha256New, Sha384New
EvpMdInit -> Sha256Init, Sha384Init
EvpMdUpdate -> Sha256Update, Sha384Update
EvpMdFinal -> Sha256Final, Sha384Final
EvpMdFree -> Sha256Free, Sha384Free
We can do similar thing with Hmac, to deprecate Sha256GetContextSize() API, and replace caller with Sha256New() / Sha384Free()
Thank you
Yao Jiewen
> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Wednesday, September 16, 2020 9:36 PM
> To: Yao, Jiewen <jiewen.yao@intel.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
>
> On 09/16/20 13:44, Yao, Jiewen wrote:
> > 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.
>
> I'm not suggesting OPTIONAL *only* because NULL is checked. I'm
> suggesting OPTIONAL because there is a specific use case related to
> that. Please see the comments on the function:
>
> "If DigestValue is NULL, free the Context then return FALSE."
>
> But, anyway, I don't insist. It's not a huge deal. Feel free to ignore
> my comment regarding OPTIONAL.
>
> Thanks
> Laszlo
>
>
> >
> >> +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
> >
next prev parent reply other threads:[~2020-09-16 14:17 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-16 0:58 [PATCH v3 0/3] CryptoPkg/BaseCryptLib: Add EVP (Envelope) Digest interface 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 [this message]
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
2020-09-16 0:59 ` [PATCH v3 2/3] CryptoPkg: Add EVP to Crypto Service driver interface Zurcher, Christopher J
2020-09-16 0:59 ` [PATCH v3 3/3] SecurityPkg/Hash2DxeCrypto: Rebase Hash2DxeCrypto onto the EVP interface Zurcher, Christopher J
2020-09-16 3:00 ` [PATCH v3 0/3] CryptoPkg/BaseCryptLib: Add EVP (Envelope) Digest interface Yao, Jiewen
-- strict thread matches above, loose matches on Subject: below --
2020-09-16 12:07 [edk2-devel] [PATCH v3 1/3] " Yao, Jiewen
2020-09-16 12:39 ` Laszlo Ersek
2020-09-16 12:56 ` Yao, Jiewen
2020-09-16 15:27 ` Laszlo Ersek
2020-09-16 13:31 ` Laszlo Ersek
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=CY4PR11MB128808CFC405504B31F2A9958C210@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