Re: [edk2-devel] [PATCH RESEND v1 5/7] MdePkg/AesLib: Definition for AES library class interface

["Yao, Jiewen" <jiewen.yao@intel.com>]

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.


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@arm.com>
> Sent: Friday, July 1, 2022 5:49 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Sami Mujawar <sami.mujawar@arm.com>; Leif Lindholm
> <quic_llindhol@quicinc.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> Rebecca Cran <rebecca@bsdio.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>;
> Edward Pickup <Edward.Pickup@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> >> PierreGondois
> >> Sent: Thursday, June 30, 2022 3:14 AM
> >> To: devel@edk2.groups.io
> >> Cc: Sami Mujawar <sami.mujawar@arm.com>; Leif Lindholm
> >> <quic_llindhol@quicinc.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> >> Rebecca Cran <rebecca@bsdio.com>; Kinney, Michael D
> >> <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>;
> >> Edward Pickup <Edward.Pickup@arm.com>
> >> Subject: [edk2-devel] [PATCH RESEND v1 5/7] MdePkg/AesLib: Definition for
> AES
> >> library class interface
> >>
> >> From: Pierre Gondois <Pierre.Gondois@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@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@edk2.groups.io
> >> Unsubscribe: https://edk2.groups.io/g/devel/unsub [jiewen.yao@intel.com]
> >> -=-=-=-=-=-=
> >>
> >