On 7/1/22 18:11, Yao, Jiewen wrote:
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
PierreGondois
Sent: Friday, July 1, 2022 11:23 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
On 7/1/22 16:40, Yao, Jiewen wrote:
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.
I don't think this option is possible as the interface definition would be in
ArmPkg,
making MdePkg dependent on ArmPkg.
[Jiewen] Why MdePkg depends on ArmPkg???
MdePkg only have library API. Why your DrbgLib.h includes AES information?
If so, I would recommend you need fix the DrbgLib.h.
Yes right, there would be indeed no dependency between the MdePkg and ArmPkg,
the above case is perfectly correct.
(or)
DrbgLib.h (interface) -> MdePkg.
DrbgLibAes (instance) -> ArmPkg. (you can put AES implementation here
directly, without AesLib.h)
I agree this option is possible, but I think it would be inefficient as the
only Arm
(or arch)
specific parts of the DrbgLib are:
- the Trng implementation
- the Aes implementation
Both are defined as libraries used by the DrbgLib. The rest of the DrbgLib code
is
common to all architectures.
The above explains how/why the DrbgLib is modularized. If the DrbgLib was put
in the SecurityPkg (I think this would fit), there would be no need to have the
AesLib in the MdePkg. Would the distribution below fit for you ?
DrbgLib.h (interface) -> SecurityPkg
DrbgLib (instance) -> SecurityPkg (note: DrbgLibAes != DrbgLib)
AesLib.h (interface) -> CryptoPkg
AesLib (instance) -> ArmPkg or CryptoPkg
[Jiewen] I have expressed my concern on AesLib.h public API definition, if it
is in MdePkg, or CryptoPkg.
In firmware, most program just wants to get a Random value. We already have
RngLib and BaseCryptoLib.
I think it is enough for the consumer. Adding more public APIs just confuses
people.
For producer, you want to build multiple layers, that is fine.
I would suggest to not expose such complexity to the consumer.
It could be limited in your internal implementation.
So far, I feel it is an overdesign to expose AesLib.h, because I don’t see the
use other use case besides DrbgLib.
Even if you want to add AES instruction to BaseCryptoLib, you can add the ARM
version directly. I still don’t see the value to have AesLib.h.
To continue the discussion on one thread, please see the answer to:
https://edk2.groups.io/g/devel/message/91009
Regards,
Pierre
Thank you
Yao Jiewen
Regards,
Pierre
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.gond...@arm.com>
Sent: Friday, July 1, 2022 9:59 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 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.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 (#91029): https://edk2.groups.io/g/devel/message/91029
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]
-=-=-=-=-=-=-=-=-=-=-=-
