[edk2-devel] [PATCH RESEND v1 5/7] MdePkg/AesLib: Definition for AES library class interface
Yao, Jiewen
jiewen.yao at intel.com
Fri Jul 1 14:40:09 UTC 2022
Please allow me to clarify my understanding:
1) You want to promote DrbgLib to MdePkg. -- That is a different topic. We should discuss that in other thread.
Now, let’s assume it is OK.
2) You want to use AES as an implementation for DrbgLib.
That is also reasonable.
Please note: MdePkg only requires the library interface to be self-contained. But not the library instance.
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.
(or)
DrbgLib.h (interface) -> MdePkg.
DrbgLibAes (instance) -> ArmPkg. (you can put AES implementation here directly, without AesLib.h)
I don’t see the need put AesLib.h to MdePkg.
And I don’t have comment for ArmPkg.
Thank you
Yao Jiewen
> -----Original Message-----
> From: Pierre Gondois <pierre.gondois at arm.com>
> Sent: Friday, July 1, 2022 9:59 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 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/f966093f5bb88e6fccac8e0b9eeca6c73
> aef0c35/SecurityPkg/RandomNumberGenerator/RngDxe/Rand/AesCore.c#L215
> [3]
> https://github.com/tianocore/edk2/blob/f966093f5bb88e6fccac8e0b9eeca6c73
> 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 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 (#90985): https://edk2.groups.io/g/devel/message/90985
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