Laszlo,
> -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Thursday, May 09, 2019 10:02 PM > To: devel@edk2.groups.io; Lu, XiaoyuX <xiaoyux...@intel.com> > Cc: Wang, Jian J <jian.j.w...@intel.com>; Ye, Ting <ting...@intel.com> > Subject: Re: [edk2-devel] [PATCH v2 6/6] CryptoPkg/BaseCryptLib: Make > HMAC_CTX size backward compatible > > On 05/09/19 07:23, Xiaoyu lu wrote: > > From: Xiaoyu Lu <xiaoyux...@intel.com> > > > > Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1089 > > > > OpenSSL internally redefines the size of HMAC_CTX at > > crypto/hmac/hmac_lcl.h(OpenSSL commit e0810e35). > > In my review at <https://edk2.groups.io/g/devel/message/39837>, I *also* > asked for referencing <https://github.com/openssl/openssl/pull/4338>, > because that PULL request discussion provides the rationale for the > change. Well, whatever. > > > We should not use it directly and should remove relevant > > functions(Hmac*GetContextSize). > > In the same review, in bullet (2), I asked that the v2 commit message > please reference the *new* TianoCore BZ -- the one that tracks the > HMAC_xxx_CTX_SIZE / Hmac*GetContextSize() removal. > > Jiaxin said in <https://edk2.groups.io/g/devel/message/39840> that he'd > file such a BZ. Do we have that BZ now? Can we please mention it in the > commit message? > My apology. I forgot to file one. Just entered one https://bugzilla.tianocore.org/show_bug.cgi?id=1792 Regards, Jian > > > > But for compatiblility, still updated HMAC_*_CTX_SIZE. > > > > Cc: Jian J Wang <jian.j.w...@intel.com> > > Cc: Ting Ye <ting...@intel.com> > > Signed-off-by: Xiaoyu Lu <xiaoyux...@intel.com> > > --- > > CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c | 8 ++++++-- > > CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c | 9 +++++++-- > > CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c | 8 ++++++-- > > 3 files changed, 19 insertions(+), 6 deletions(-) > > > > diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c > b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c > > index 3134806..3a90e03 100644 > > --- a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c > > +++ b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c > > @@ -9,8 +9,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > #include "InternalCryptLib.h" > > #include <openssl/hmac.h> > > > > -#define HMAC_MD5_CTX_SIZE sizeof(void *) * 4 + sizeof(unsigned int) + \ > > - sizeof(unsigned char) * HMAC_MAX_MD_CBLOCK > > +/** > > + 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. > > In my review linked above, I asked for the comment to be removed. I > guess you disagreed, ultimately. > > I agree that the new comment is acceptable. However, it does not conform > to the edk2 coding style. We should use the // format, for comments like > these. > > Summary: > - please file the new BZ, and mention it too, in the commit message > - please clean up the comment style. > > With both of those fixed, you can add > > Reviewed-by: Laszlo Ersek <ler...@redhat.com> > > Thanks > Laszlo > > > diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c > b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c > > index bbe3df4..a8e8d44 100644 > > --- a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c > > +++ b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c > > @@ -9,8 +9,13 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > #include "InternalCryptLib.h" > > #include <openssl/hmac.h> > > > > -#define HMAC_SHA1_CTX_SIZE sizeof(void *) * 4 + sizeof(unsigned int) + \ > > - sizeof(unsigned char) * HMAC_MAX_MD_CBLOCK > > +/** > > + 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. > > diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c > b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c > > index ac9084f..fec60b1 100644 > > --- a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c > > +++ b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c > > @@ -9,8 +9,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > #include "InternalCryptLib.h" > > #include <openssl/hmac.h> > > > > -#define HMAC_SHA256_CTX_SIZE sizeof(void *) * 4 + sizeof(unsigned int) + > > \ > > - sizeof(unsigned char) * HMAC_MAX_MD_CBLOCK > > +/** > > + 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. > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#40364): https://edk2.groups.io/g/devel/message/40364 Mute This Topic: https://groups.io/mt/31552213/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
