From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web09.40136.1656688990166402104 for ; Fri, 01 Jul 2022 08:23:10 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: pierre.gondois@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E4996113E; Fri, 1 Jul 2022 08:23:09 -0700 (PDT) Received: from [192.168.1.11] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 757133F66F; Fri, 1 Jul 2022 08:23:07 -0700 (PDT) Message-ID: Date: Fri, 1 Jul 2022 17:22:38 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [edk2-devel] [PATCH RESEND v1 5/7] MdePkg/AesLib: Definition for AES library class interface To: "Yao, Jiewen" , "devel@edk2.groups.io" Cc: Sami Mujawar , Leif Lindholm , Ard Biesheuvel , Rebecca Cran , "Kinney, Michael D" , "Gao, Liming" , Edward Pickup References: <20220629191355.2618844-1-Pierre.Gondois@arm.com> <20220629191355.2618844-6-Pierre.Gondois@arm.com> <499390b1-0916-2f25-3831-b1e2d406bc49@arm.com> From: "PierreGondois" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable On 7/1/22 16:40, Yao, Jiewen wrote: > Please allow me to clarify my understanding: >=20 > 1) You want to promote DrbgLib to MdePkg. -- That is a different topic.= We should discuss that in other thread. > Now, let=E2=80=99s assume it is OK. >=20 > 2) You want to use AES as an implementation for DrbgLib. > That is also reasonable. >=20 > Please note: MdePkg only requires the library interface to be self-cont= ained. But not the library instance. >=20 > Assuming you are working on ARM solution. It is legal that: > DrbgLib.h (interface) -> MdePkg. > AesLib.h (interface) -> ArmPkg > AesLib (instance) -> ArmPkg > DrbgLibAes (instance) -> ArmPkg. I don't think this option is possible as the interface definition would b= e in ArmPkg, making MdePkg dependent on ArmPkg. >=20 > (or) > DrbgLib.h (interface) -> MdePkg. > DrbgLibAes (instance) -> ArmPkg. (you can put AES implementation here d= irectly, without AesLib.h) I agree this option is possible, but I think it would be inefficient as t= he only Arm (or arch) specific parts of the DrbgLib are: - the Trng implementation - the Aes implementation Both are defined as libraries used by the DrbgLib. The rest of the DrbgLi= b code is common to all architectures. The above explains how/why the DrbgLib is modularized. If the DrbgLib was= put in the SecurityPkg (I think this would fit), there would be no need to ha= ve the AesLib in the MdePkg. Would the distribution below fit for you ? DrbgLib.h (interface) -> SecurityPkg DrbgLib (instance) -> SecurityPkg (note: DrbgLibAes !=3D DrbgLib) AesLib.h (interface) -> CryptoPkg AesLib (instance) -> ArmPkg or CryptoPkg Regards, Pierre >=20 > I don=E2=80=99t see the need put AesLib.h to MdePkg. > And I don=E2=80=99t have comment for ArmPkg. >=20 > Thank you > Yao Jiewen >=20 >=20 >> -----Original Message----- >> From: Pierre Gondois >> Sent: Friday, July 1, 2022 9:59 PM >> To: Yao, Jiewen ; devel@edk2.groups.io >> Cc: Sami Mujawar ; Leif Lindholm >> ; Ard Biesheuvel ; >> Rebecca Cran ; Kinney, Michael D >> ; Gao, Liming ; >> Edward Pickup >> Subject: Re: [edk2-devel] [PATCH RESEND v1 5/7] MdePkg/AesLib: Definit= ion for >> AES library class interface >> >> Hello Jiewen, >> >> On 7/1/22 13:55, Yao, Jiewen wrote: >>> I have two concern: >>> >>> 1) I am worried that this API might be misused. Usually, a crypto API= should be >> secure enough to avoid misuse. For example, if a program wants to use = AES >> encryption, it must NOT use this AES API. Instead it must use AES_CCB = + MAC or >> AES_GCM. (or equivalent) >>> I doubt if this is right direction to expose this publicly in MdePkg. >>> >>> 2) I am not sure how this API will be used in CryptoLib. >>> Ideally, an EDKII program should use crypto lib API for any crypto fu= nction. >>> However, I do not understand how that is done. >>> >> >> The reason the AesLib was put in MdePkg: >> - The DrbgLib was thought to be generic enough to be in MdePkg >> (this is arguable). >> - The MdePkg must be self-contained (i.e. not use libraries/modules >> defined in other packages). Thus if an AesLib is created, it must = be >> in the MdePkg. >> I don't mind moving the DrbgLib (and the AesLib) to another package if >> this is the common agreement. >> >> Why a single block AesLib should be created: >> - The DrbgLib requires to have Aes single block encryption. A software >> implementation of Aes is also available (and used) at [2] in the >> SecurityPkg. This implementation is limited to a module scope. >> Thus, there is a need create a common library for this. >> - I agree that this AesLib should not be mistaken with something else >> (cf your comment about AES_CCB + MAC or AES_GCM). However, the new >> interface needed is for a single block encryption. So adding these >> new functions to: >> CryptoPkg/Include/Library/BaseCryptLib.h >> won't make it safer. >> >> Please let me know if there are still concerns, >> Regards, >> Pierre >> >> Note: >> The functions in AesLib are equivalent to the ones in [4]. >> >> [1] https://edk2.groups.io/g/devel/files/Designs/2021/0116/EDKII%20- >> %20Proposed%20update%20to%20RNG%20implementation.pdf >> [2] >> https://github.com/tianocore/edk2/blob/f966093f5bb88e6fccac8e0b9eeca6c= 73 >> aef0c35/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/AesCore.c#L215 >> [3] >> https://github.com/tianocore/edk2/blob/f966093f5bb88e6fccac8e0b9eeca6c= 73 >> aef0c35/CryptoPkg/Include/Library/BaseCryptLib.h#L1128 >> [4] https://github.com/openssl/openssl/blob/master/crypto/aes/aes_core= .c >> >> >>> >>> I think it is good idea to enable ARM AES hardware accelerator. >>> And I would like to see a total solution. >>> >>> It will be great, if you also submit the cryptopkg patch to help me u= nderstand >> how to achieve that. >>> >>> Thank you >>> Yao Jiewen >>> >>> >>>> -----Original Message----- >>>> From: Pierre Gondois >>>> Sent: Friday, July 1, 2022 5:49 PM >>>> To: Yao, Jiewen ; devel@edk2.groups.io >>>> Cc: Sami Mujawar ; Leif Lindholm >>>> ; Ard Biesheuvel ; >>>> Rebecca Cran ; Kinney, Michael D >>>> ; Gao, Liming = ; >>>> Edward Pickup >>>> Subject: Re: [edk2-devel] [PATCH RESEND v1 5/7] MdePkg/AesLib: Defin= ition >> for >>>> AES library class interface >>>> >>>> Hello Yao, >>>> >>>> On 6/30/22 02:29, Yao, Jiewen wrote: >>>>> Hi >>>>> 1) Would you please educate me, how this library be used in cryptol= ib? - >>>> >> https://github.com/tianocore/edk2/blob/master/CryptoPkg/Include/Librar= y/Bas >>>> eCryptLib.h#L1091 >>>>> >>>>> Currently, we have AES_CBC. We are going to add AES_GCM in near fut= ure. >>>>> >>>> >>>> We are currently looking forward to do that. Just to be sure, the >>>> AesInit() function pointed above is for AesCbcEncrypt(), which can >>>> encrypt a buffer. >>>> The AesInitCtx() in this file is for a single block encryption. So >>>> there should be nothing preventing from implementing CBC (or other) >>>> encryption based on the Aes block encryption added by this patch-set= . >>>> >>>>> 2) For Intel AES_NI, we added support in OpensslLib directly - >>>> >> https://github.com/tianocore/edk2/tree/master/CryptoPkg/Library/Openss= lLib/ >>>> X64, can ARM use the similar model? >>>>> >>>> >>>> We also need to have a look at this. However this might be a bit mor= e >>>> difficult if we want to avoid Openssl license. >>>> >>>>> 3) Do you have chance to take a look if this interface is good enou= gh to >>>> implement Intel AES_NI instruction? >>>>> >>>> >>>> We have not looked at the AES_NI instruction, but the interface >>>> definition should be generic enough to accept any implementation. >>>> Please tell us if you think this requires modification. >>>> >>>> Regards, >>>> Pierre >>>> >>>>> Thank you >>>>> Yao Jiewen >>>>> >>>>>> -----Original Message----- >>>>>> From: devel@edk2.groups.io On Behalf Of >>>>>> PierreGondois >>>>>> Sent: Thursday, June 30, 2022 3:14 AM >>>>>> To: devel@edk2.groups.io >>>>>> Cc: Sami Mujawar ; Leif Lindholm >>>>>> ; Ard Biesheuvel >> ; >>>>>> Rebecca Cran ; Kinney, Michael D >>>>>> ; Gao, Liming ; >>>>>> Edward Pickup >>>>>> Subject: [edk2-devel] [PATCH RESEND v1 5/7] MdePkg/AesLib: Definit= ion >> for >>>> AES >>>>>> library class interface >>>>>> >>>>>> From: Pierre Gondois >>>>>> >>>>>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3970 >>>>>> >>>>>> The FIPS PUB 197: "Advanced Encryption Standard (AES)" >>>>>> details the AES algorithm. Add a library to allow >>>>>> different architecture specific implementations. >>>>>> >>>>>> Signed-off-by: Pierre Gondois >>>>>> --- >>>>>> MdePkg/Include/Library/AesLib.h | 104 >>>> ++++++++++++++++++++++++++++++++ >>>>>> MdePkg/MdePkg.dec | 4 ++ >>>>>> 2 files changed, 108 insertions(+) >>>>>> create mode 100644 MdePkg/Include/Library/AesLib.h >>>>>> >>>>>> diff --git a/MdePkg/Include/Library/AesLib.h >>>> b/MdePkg/Include/Library/AesLib.h >>>>>> new file mode 100644 >>>>>> index 000000000000..bc3408bb249b >>>>>> --- /dev/null >>>>>> +++ b/MdePkg/Include/Library/AesLib.h >>>>>> @@ -0,0 +1,104 @@ >>>>>> +/** @file >>>>>> + AES library. >>>>>> + >>>>>> + Copyright (c) 2022, Arm Limited. All rights reserved.
>>>>>> + >>>>>> + SPDX-License-Identifier: BSD-2-Clause-Patent >>>>>> + >>>>>> + @par Reference(s): >>>>>> + - FIPS 197 November 26, 2001: >>>>>> + Specification for the ADVANCED ENCRYPTION STANDARD (AES) >>>>>> +**/ >>>>>> + >>>>>> +#ifndef AES_LIB_H_ >>>>>> +#define AES_LIB_H_ >>>>>> + >>>>>> +/// Key size in bytes. >>>>>> +#define AES_KEY_SIZE_128 16 >>>>>> +#define AES_KEY_SIZE_192 24 >>>>>> +#define AES_KEY_SIZE_256 32 >>>>>> +#define AES_BLOCK_SIZE 16 >>>>>> + >>>>>> +/* >>>>>> + The Key Expansion generates a total of Nb (Nr + 1) words with: >>>>>> + - Nb =3D 4: >>>>>> + Number of columns (32-bit words) comprising the State >>>>>> + - Nr =3D 10, 12, or 14: >>>>>> + Number of rounds. >>>>>> + */ >>>>>> +#define AES_MAX_KEYLENGTH_U32 (4 * (14 + 1)) >>>>>> + >>>>>> +/** A context holding information to for AES encryption/decryptio= n. >>>>>> + */ >>>>>> +typedef struct { >>>>>> + /// Expanded encryption key. >>>>>> + UINT32 ExpEncKey[AES_MAX_KEYLENGTH_U32]; >>>>>> + /// Expanded decryption key. >>>>>> + UINT32 ExpDecKey[AES_MAX_KEYLENGTH_U32]; >>>>>> + /// Key size, in bytes. >>>>>> + /// Must be one of 16|24|32. >>>>>> + UINT32 KeySize; >>>>>> +} AES_CTX; >>>>>> + >>>>>> +/** Encrypt an AES block. >>>>>> + >>>>>> + Buffers are little-endian. Overlapping is not checked. >>>>>> + >>>>>> + @param [in] AesCtx AES context. >>>>>> + AesCtx is initialized with AesInitCtx ()= . >>>>>> + @param [in] InBlock Input Block. The block to cipher. >>>>>> + @param [out] OutBlock Output Block. The ciphered block. >>>>>> + >>>>>> + @retval RETURN_SUCCESS Success. >>>>>> + @retval RETURN_INVALID_PARAMETER Invalid parameter. >>>>>> + @retval RETURN_UNSUPPORTED Unsupported. >>>>>> +**/ >>>>>> +RETURN_STATUS >>>>>> +EFIAPI >>>>>> +AesEncrypt ( >>>>>> + IN AES_CTX *AesCtx, >>>>>> + IN UINT8 CONST *InBlock, >>>>>> + OUT UINT8 *OutBlock >>>>>> + ); >>>>>> + >>>>>> +/** Decrypt an AES block. >>>>>> + >>>>>> + Buffers are little-endian. Overlapping is not checked. >>>>>> + >>>>>> + @param [in] AesCtx AES context. >>>>>> + AesCtx is initialized with AesInitCtx ()= . >>>>>> + @param [in] InBlock Input Block. The block to de-cipher. >>>>>> + @param [out] OutBlock Output Block. The de-ciphered block. >>>>>> + >>>>>> + @retval RETURN_SUCCESS Success. >>>>>> + @retval RETURN_INVALID_PARAMETER Invalid parameter. >>>>>> + @retval RETURN_UNSUPPORTED Unsupported. >>>>>> +**/ >>>>>> +RETURN_STATUS >>>>>> +EFIAPI >>>>>> +AesDecrypt ( >>>>>> + IN AES_CTX *AesCtx, >>>>>> + IN UINT8 CONST *InBlock, >>>>>> + OUT UINT8 *OutBlock >>>>>> + ); >>>>>> + >>>>>> +/** Initialize an AES_CTX structure. >>>>>> + >>>>>> + @param [in] Key AES key. Buffer of KeySize bytes. >>>>>> + The buffer is little endian. >>>>>> + @param [in] KeySize Size of the key. Must be one of 128= |192|256. >>>>>> + @param [in, out] AesCtx AES context to initialize. >>>>>> + >>>>>> + @retval RETURN_SUCCESS Success. >>>>>> + @retval RETURN_INVALID_PARAMETER Invalid parameter. >>>>>> + @retval RETURN_UNSUPPORTED Unsupported. >>>>>> +**/ >>>>>> +RETURN_STATUS >>>>>> +EFIAPI >>>>>> +AesInitCtx ( >>>>>> + IN UINT8 *Key, >>>>>> + IN UINT32 KeySize, >>>>>> + IN OUT AES_CTX *AesCtx >>>>>> + ); >>>>>> + >>>>>> +#endif // AES_LIB_H_ >>>>>> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec >>>>>> index 7ff26e22f915..078ae9323ba6 100644 >>>>>> --- a/MdePkg/MdePkg.dec >>>>>> +++ b/MdePkg/MdePkg.dec >>>>>> @@ -280,6 +280,10 @@ [LibraryClasses] >>>>>> # >>>>>> TrngLib|Include/Library/TrngLib.h >>>>>> >>>>>> + ## @libraryclass Provides AES encryption/decryption services. >>>>>> + # >>>>>> + AesLib|Include/Library/AesLib.h >>>>>> + >>>>>> [LibraryClasses.IA32, LibraryClasses.X64, LibraryClasses.AARCH= 64] >>>>>> ## @libraryclass Provides services to generate random numb= er. >>>>>> # >>>>>> -- >>>>>> 2.25.1 >>>>>> >>>>>> >>>>>> >>>>>> -=3D-=3D-=3D-=3D-=3D-=3D >>>>>> Groups.io Links: You receive all messages sent to this group. >>>>>> View/Reply Online (#90895): >> https://edk2.groups.io/g/devel/message/90895 >>>>>> Mute This Topic: https://groups.io/mt/92072168/1772286 >>>>>> Group Owner: devel+owner@edk2.groups.io >>>>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub >> [jiewen.yao@intel.com] >>>>>> -=3D-=3D-=3D-=3D-=3D-=3D >>>>>> >>>>>