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.gond...@arm.com>
Sent: Friday, July 1, 2022 5:49 PM
To: Yao, Jiewen <jiewen....@intel.com>; devel@edk2.groups.io
Cc: Sami Mujawar <sami.muja...@arm.com>; Leif Lindholm
<quic_llind...@quicinc.com>; Ard Biesheuvel <ardb+tianoc...@kernel.org>;
Rebecca Cran <rebe...@bsdio.com>; Kinney, Michael D
<michael.d.kin...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>;
Edward Pickup <edward.pic...@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.muja...@arm.com>; Leif Lindholm
<quic_llind...@quicinc.com>; Ard Biesheuvel <ardb+tianoc...@kernel.org>;
Rebecca Cran <rebe...@bsdio.com>; Kinney, Michael D
<michael.d.kin...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>;
Edward Pickup <edward.pic...@arm.com>
Subject: [edk2-devel] [PATCH RESEND v1 5/7] MdePkg/AesLib: Definition for
AES
library class interface

From: Pierre Gondois <pierre.gond...@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.gond...@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+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [jiewen....@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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to