[edk2-devel] [PATCH RESEND v1 5/7] MdePkg/AesLib: Definition for AES library class interface
PierreGondois
pierre.gondois at arm.com
Fri Jul 1 13:58:46 UTC 2022
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 <pierre.gondois at arm.com>
>> Sent: Friday, July 1, 2022 5:49 PM
>> To: Yao, Jiewen <jiewen.yao at intel.com>; devel at edk2.groups.io
>> Cc: Sami Mujawar <sami.mujawar at arm.com>; Leif Lindholm
>> <quic_llindhol at quicinc.com>; Ard Biesheuvel <ardb+tianocore at kernel.org>;
>> Rebecca Cran <rebecca at bsdio.com>; Kinney, Michael D
>> <michael.d.kinney at intel.com>; Gao, Liming <gaoliming at byosoft.com.cn>;
>> Edward Pickup <Edward.Pickup at arm.com>
>> 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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
>>>> PierreGondois
>>>> Sent: Thursday, June 30, 2022 3:14 AM
>>>> To: devel at edk2.groups.io
>>>> Cc: Sami Mujawar <sami.mujawar at arm.com>; Leif Lindholm
>>>> <quic_llindhol at quicinc.com>; Ard Biesheuvel <ardb+tianocore at kernel.org>;
>>>> Rebecca Cran <rebecca at bsdio.com>; Kinney, Michael D
>>>> <michael.d.kinney at intel.com>; Gao, Liming <gaoliming at byosoft.com.cn>;
>>>> Edward Pickup <Edward.Pickup at arm.com>
>>>> Subject: [edk2-devel] [PATCH RESEND v1 5/7] MdePkg/AesLib: Definition for
>> AES
>>>> library class interface
>>>>
>>>> From: Pierre Gondois <Pierre.Gondois at arm.com>
>>>>
>>>> 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 <pierre.gondois at arm.com>
>>>> ---
>>>> 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.<BR>
>>>> +
>>>> + 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 at edk2.groups.io
>>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub [jiewen.yao at intel.com]
>>>> -=-=-=-=-=-=
>>>>
>>>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#90981): https://edk2.groups.io/g/devel/message/90981
Mute This Topic: https://groups.io/mt/92072168/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list