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.39115.1656683968408015864 for ; Fri, 01 Jul 2022 06:59:28 -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 3D8AF113E; Fri, 1 Jul 2022 06:59:28 -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 E826A3F792; Fri, 1 Jul 2022 06:59:25 -0700 (PDT) Message-ID: Date: Fri, 1 Jul 2022 15:58:46 +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: 7bit 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 function. > 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/f966093f5bb88e6fccac8e0b9eeca6c73aef0c35/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/AesCore.c#L215 [3] https://github.com/tianocore/edk2/blob/f966093f5bb88e6fccac8e0b9eeca6c73aef0c35/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 understand 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: Definition 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 cryptolib? - >> https://github.com/tianocore/edk2/blob/master/CryptoPkg/Include/Library/Bas >> eCryptLib.h#L1091 >>> >>> Currently, we have AES_CBC. We are going to add AES_GCM in near future. >>> >> >> 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/OpensslLib/ >> X64, can ARM use the similar model? >>> >> >> We also need to have a look at this. However this might be a bit more >> difficult if we want to avoid Openssl license. >> >>> 3) Do you have chance to take a look if this interface is good enough 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: Definition for >> AES >>>> library class interface >>>> >>>> From: Pierre Gondois >>>> >>>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3970 >>>> >>>> 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 = 4: >>>> + Number of columns (32-bit words) comprising the State >>>> + - Nr = 10, 12, or 14: >>>> + Number of rounds. >>>> + */ >>>> +#define AES_MAX_KEYLENGTH_U32 (4 * (14 + 1)) >>>> + >>>> +/** A context holding information to for AES encryption/decryption. >>>> + */ >>>> +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.AARCH64] >>>> ## @libraryclass Provides services to generate random number. >>>> # >>>> -- >>>> 2.25.1 >>>> >>>> >>>> >>>> -=-=-=-=-=-= >>>> 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] >>>> -=-=-=-=-=-= >>>> >>>