Hi Alamy,
In my very own opinion, `psk_key_id_t` defined as structure seems to be Mbed Crypto/MbedTLS own definition and should be transparent to applications calling `psa_open_key()`, as described in https://github.com/ARMmbed/mbedtls/blob/v2.23.0/include/psa/crypto_platform.....
TF-M provides standard PSA APIs and therefore the `psa_key_id_t` in `psa_open_key()` is `uint32_t` as defined in PSA Cryptography API v1 Beta 3. The `uint32_t` `psa_key_id_t` is defined in `interface/include/psa/crypto_types.h` and included by TF-M Crypto interface files.
Then when TF-M Crypto service invokes Mbed Crypto/MbedTLS APIs to fulfill the `psa_open_key()`, it includes Mbed Crypto/MbedTLS specific header file, in which `psa_key_id_t` is defined as a structure to support multiple client. For example, `crypto_key.c` constructs a `psa_key_id_t` structure and pass it to Mbed Crypto/MbedTLS `psa_open_key()` implementation. (https://git.trustedfirmware.org/TF-M/trusted-firmware-m.git/tree/secure_fw/p...)
It requires @Jamie Foxmailto:Jamie.Fox@arm.com and @Soby Mathewmailto:Soby.Mathew@arm.com help to provide a comprehensive answer. 😉
Best regards, Hu Ziji
From: TF-M tf-m-bounces@lists.trustedfirmware.org On Behalf Of Alamy Liu via TF-M Sent: Tuesday, September 1, 2020 1:23 AM To: tf-m@lists.trustedfirmware.org Subject: Re: [TF-M] psa_key_id_t type mismatch --- HardFault
Sorry all, this is a FALSE alarm. Although some codes still include the local header file (uint32_t), none of them use psa_key_id_t at all. Maybe the code should be more clean, but there is no run-time problem!
Sorry if it causes problems, Alamy
On Mon, Aug 31, 2020 at 8:38 AM Alamy Liu <alamy.liu@gmail.commailto:alamy.liu@gmail.com> wrote: Dear all,
While I was working on the PSoC64 platform, I hit the psa_key_id_t type mismatch problem.
The patch - 98ab441e096f enables MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER Which in terms to use the psa_key_id_t structure (psa_key_file_id_t) in <mbed-crypto/mbedtls>/include/psa/crypto_platform.h Interestingly, psa_key_id_t is also defined in <tf-m>/interface/include/psa/crypto_types.h, as a uint32_t.
So, I do the following testing I could compile the master HEAD no problem 66ee5c8861 (HEAD, origin/master, origin/HEAD) Tools: update iat-verifier README and samples
I assume the psa_key_id_t should be a structure (from mbed-crypto/mbedtls), I applied the patch below
--- a/interface/include/psa/crypto_types.h
+++ b/interface/include/psa/crypto_types.h
@@ -211,6 +211,8 @@ typedef uint8_t psa_key_persistence_t;
*/
typedef uint32_t psa_key_location_t;
+#if !defined(MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER)
+#error Should not compile this
/** Encoding of identifiers of persistent keys.
*
* - Applications may freely choose key identifiers in the range
@@ -222,6 +224,7 @@ typedef uint32_t psa_key_location_t;
*/
typedef uint32_t psa_key_id_t;
#define PSA_KEY_ID_INIT 0
+#endif Then, I notice that there are still many files that use uint32_t psa_key_id_t in the TF-M source tree.
a) It's good (lucky?) that TF-M seems to cut it cleanly so it doesn't run into problems (well, it happens on PSoC64, or I won't notice it). b) It's bad that psa_key_id_t in TF-M has two different types.
I'm not going to argue what's correct/wrong. Maybe this kind of coding could be a feature in the future. I just go with it. But I found no information to define the boundary of the two types under the <tf-m>/docs/ directory. I would like to know where the boundary is, in TF-M. In other words, Which part of the code should use the structure definition from mbedtls/mbed-crypto; which part of the code should use uint32_t ?
In my work, the problem happens when it passes psa_key_id_t as a parameter, the parent & child functions see it differently (HardFault, in my case, memory violation to other parameters). e.g.: func_a.c (structure), func_b.c (uint32_t). func_b.h ---- the type changes when it's included by func_a.c and/or func_b.c
Regards, Alamy
Hi Alamy, Just to add on to what David says, one of the roles of Crypto Service is to abstract the implementation details of the mbedTLS backend. Some of the structures if directly exposed to NS client as implemented by mbedTLS, can expose secure world details to non secure client. Hence the crypto service buffers these mbedTLS structures internally and exposes opaque handles to the NS world.
The other reason is, many of the mbedTLS types are conditionally defined, and this depends on the mbedTLS configuration (like the MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER flag). The crypto service in TF-M wants to export a constant type irrespective of mbedTLS configuration applied.
This is why some types differ between the mbedTLS and the ones exposed by CryptoService.
But you are right, it is confusing to anyone reading the code, and it could do with a better organization. I have found that the current structure does prevent any inadvertent errors while development and triggers compilation errors so it works well.
Best Regards Soby Mathew
From: David Hu David.Hu@arm.com Sent: 01 September 2020 03:55 To: Alamy Liu alamy.liu@gmail.com; Jamie Fox Jamie.Fox@arm.com; Soby Mathew Soby.Mathew@arm.com Cc: tf-m@lists.trustedfirmware.org Subject: RE: [TF-M] psa_key_id_t type mismatch --- HardFault
Hi Alamy,
In my very own opinion, `psk_key_id_t` defined as structure seems to be Mbed Crypto/MbedTLS own definition and should be transparent to applications calling `psa_open_key()`, as described in https://github.com/ARMmbed/mbedtls/blob/v2.23.0/include/psa/crypto_platform.h#L6 .
TF-M provides standard PSA APIs and therefore the `psa_key_id_t` in `psa_open_key()` is `uint32_t` as defined in PSA Cryptography API v1 Beta 3. The `uint32_t` `psa_key_id_t` is defined in `interface/include/psa/crypto_types.h` and included by TF-M Crypto interface files.
Then when TF-M Crypto service invokes Mbed Crypto/MbedTLS APIs to fulfill the `psa_open_key()`, it includes Mbed Crypto/MbedTLS specific header file, in which `psa_key_id_t` is defined as a structure to support multiple client. For example, `crypto_key.c` constructs a `psa_key_id_t` structure and pass it to Mbed Crypto/MbedTLS `psa_open_key()` implementation. (https://git.trustedfirmware.org/TF-M/trusted-firmware-m.git/tree/secure_fw/p...)
It requires @Jamie Foxmailto:Jamie.Fox@arm.com and @Soby Mathewmailto:Soby.Mathew@arm.com help to provide a comprehensive answer. 😉
Best regards, Hu Ziji
From: TF-M <tf-m-bounces@lists.trustedfirmware.orgmailto:tf-m-bounces@lists.trustedfirmware.org> On Behalf Of Alamy Liu via TF-M Sent: Tuesday, September 1, 2020 1:23 AM To: tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Subject: Re: [TF-M] psa_key_id_t type mismatch --- HardFault
Sorry all, this is a FALSE alarm. Although some codes still include the local header file (uint32_t), none of them use psa_key_id_t at all. Maybe the code should be more clean, but there is no run-time problem!
Sorry if it causes problems, Alamy
On Mon, Aug 31, 2020 at 8:38 AM Alamy Liu <alamy.liu@gmail.commailto:alamy.liu@gmail.com> wrote: Dear all,
While I was working on the PSoC64 platform, I hit the psa_key_id_t type mismatch problem.
The patch - 98ab441e096f enables MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER Which in terms to use the psa_key_id_t structure (psa_key_file_id_t) in <mbed-crypto/mbedtls>/include/psa/crypto_platform.h Interestingly, psa_key_id_t is also defined in <tf-m>/interface/include/psa/crypto_types.h, as a uint32_t.
So, I do the following testing I could compile the master HEAD no problem 66ee5c8861 (HEAD, origin/master, origin/HEAD) Tools: update iat-verifier README and samples
I assume the psa_key_id_t should be a structure (from mbed-crypto/mbedtls), I applied the patch below
--- a/interface/include/psa/crypto_types.h
+++ b/interface/include/psa/crypto_types.h
@@ -211,6 +211,8 @@ typedef uint8_t psa_key_persistence_t;
*/
typedef uint32_t psa_key_location_t;
+#if !defined(MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER)
+#error Should not compile this
/** Encoding of identifiers of persistent keys.
*
* - Applications may freely choose key identifiers in the range
@@ -222,6 +224,7 @@ typedef uint32_t psa_key_location_t;
*/
typedef uint32_t psa_key_id_t;
#define PSA_KEY_ID_INIT 0
+#endif Then, I notice that there are still many files that use uint32_t psa_key_id_t in the TF-M source tree.
a) It's good (lucky?) that TF-M seems to cut it cleanly so it doesn't run into problems (well, it happens on PSoC64, or I won't notice it). b) It's bad that psa_key_id_t in TF-M has two different types.
I'm not going to argue what's correct/wrong. Maybe this kind of coding could be a feature in the future. I just go with it. But I found no information to define the boundary of the two types under the <tf-m>/docs/ directory. I would like to know where the boundary is, in TF-M. In other words, Which part of the code should use the structure definition from mbedtls/mbed-crypto; which part of the code should use uint32_t ?
In my work, the problem happens when it passes psa_key_id_t as a parameter, the parent & child functions see it differently (HardFault, in my case, memory violation to other parameters). e.g.: func_a.c (structure), func_b.c (uint32_t). func_b.h ---- the type changes when it's included by func_a.c and/or func_b.c
Regards, Alamy
tf-m@lists.trustedfirmware.org
-
David Hu -
Soby Mathew