Add support of PKA peripheral on stm32mp13 #7151

tbourgoi · 2024-11-25T13:52:09Z

Add support of PKA peripheral for stm32mp13.
This PR :
- fix the device tree node
- add the driver stm32-pka.c
- compile the driver by default
- enable pka on stm32mp135-dk

use st,stm32mp13-pka instead of st,stm32mp13-pka64 to be aligned with all other st compatibles. Signed-off-by: Thomas Bourgoin <[email protected]>

Adds missing RNG clock resource in PKA nodes in stm32mp13 SoC DTSI files. Signed-off-by: Etienne Carriere <[email protected]> Signed-off-by: Thomas Bourgoin <[email protected]>

Add PKA IP drivers, and add hooks in OP-TEE crypto framework to use PKA IP to do ECC process. Truncate hash during ECDSA signature according to the NIST recommendation: https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.186-5.pdf Signed-off-by: Thomas Bourgoin <[email protected]> Co-developed-by: Patrick Delaunay <[email protected]> Signed-off-by: Patrick Delaunay <[email protected]> Signed-off-by: Nicolas Toromanoff <[email protected]>

Fix regression of xtest 4006 testing TEE_ALG_SM2_PKE. PKA only support ECDSA key generation ECDH key exchange. Signed-off-by: Thomas Bourgoin <[email protected]>

Default enable PKA compilation. Enable the STM32_CRYPTO_DRIVERS if PKA is compiled. Signed-off-by: Thomas Bourgoin <[email protected]>

Sets PKA peripheral status to okay. Signed-off-by: Thomas Bourgoin <[email protected]>

core/drivers/crypto/stm32/stm32_pka.h

core/drivers/crypto/stm32/ecc.c

core/drivers/crypto/stm32/stm32_pka.c

ecc.c : swap the 2 include lines change DMSG to FMSG Use common out label stm32_pka.c : reorder _PKA_CRxx defines use upper cases for pka_op enum values update documentation for struct curve_parameters update documentation of read_eo_data() use IO_READ32_POLL_TIMEOUT, SHIFT_U32 and SHIF_U64 macros use EMSG for error messages. Add const for pka_ram offset definition Use bit_mask as parameter for pka_wait_bit() function fix documentation of read_eo_data() Use U() or ULL() insteald of U and ULL Use type enum pka_op instead of int for parameter of op in stm32_pka_configure_curve() stm32_pka.h : Fix date issue Signed-off-by: Thomas Bourgoin <[email protected]>

Use TEE_TYPE_xxx_PUBLIC_KEY instead of TEE_TYPE_xxx_KEYPAIR in stm32_alloc_publickey() Signed-off-by: Thomas Bourgoin <[email protected]>

Use fdt_reg_info() to read base address and size instead of fdt_fill_device_info(). Signed-off-by: Thomas Bourgoin <[email protected]>

core/drivers/crypto/stm32/ecc.c

core/drivers/crypto/stm32/stm32_pka.c

core/drivers/crypto/stm32/ecc.c

ecc.c : use PKA_LAST_CID instead of -1; use free_wipe() where it is relevant stm32_pka.c : change len to length fix coding style for curve parameters fix inline documentation use for loop instead of do{}while() in function is_smaller() Signed-off-by: Thomas Bourgoin <[email protected]>

etienne-lms · 2024-12-04T11:18:19Z

core/drivers/crypto/stm32/ecc.c

@@ -107,15 +108,15 @@ static TEE_Result stm32_gen_keypair(struct ecc_keypair *key, size_t size_bits)
 	pk.x.val = calloc(1, bytes);
 	pk.x.size = bytes;
 	if (!pk.x.val) {
-		free(d.val);
+		free_wipe(d.val);


At this stage, d does not hold a secret. free() is enough IMHO.

d.val has been filled (with the random value)
If free_wipe does not apply here it is useless for d.val in all below cases IMO

You're right, upon other error cases in this function, d does not hold any secret. You can restore the free() operation at line 119, but IMO you should preserve the free_wipe() operation at function exit at line 140.

etienne-lms · 2024-12-04T11:19:06Z

core/drivers/crypto/stm32/ecc.c

-	free(pk.x.val);
-	free(d.val);
+	free_wipe(pk.y.val);
+	free_wipe(pk.x.val);


pk is a public key. I don't think it deserves a wipe operation.

etienne-lms · 2024-12-04T11:20:41Z

core/drivers/crypto/stm32/stm32_pka.c

 			.size = U(24),
 		},
 		.a_sign = U(1),
-		.a = { .val = (uint8_t[]){0x03},
+		.a = {
+			.val = (uint8_t[]){0x03},


nitpicking: { 0x03 } (with space chars inside the braces)
Ditto at lines 353, 425, 509 and 614.

etienne-lms · 2024-12-04T11:25:30Z

core/drivers/crypto/stm32/stm32_pka.c

@@ -863,24 +949,20 @@ static bool is_smaller(const struct stm32_pka_bn *a,
 {
 	unsigned int i = MAX(a->size, b->size) + 1;

-	do {
+	for (; i != U(1); i--) {


Prefer with i initial value defined within the for () instruction.
Note that i shall start at MAX() without the increment previously used and stop once reaching 0:
for (i = MAX(a->size, b->size); i; i--)

etienne-lms · 2024-12-04T11:27:38Z

core/drivers/crypto/stm32/stm32_pka.c

+		while ((i < sizeof(tmp)) && (data_index >= 0)) {
+			tmp |= SHIFT_U64(data[data_index], (INT8_LEN * i));
+			i++; /* Move byte index in current (u64)tmp */
+			data_index--; /* Move to next most significant byte */


/* Move byte index in current (u64)tmp */ i++; /* Move to next most significant byte */ data_index--;

etienne-lms · 2024-12-04T11:30:50Z

core/drivers/crypto/stm32/stm32_pka.c

+		if (PM_HINT_IS_STATE(pm_hint, CONTEXT)) {
+			if (stm32_pka_reset())
+				panic();
+		}


Nitpicnking:

if (PM_HINT_IS_STATE(pm_hint, CONTEXT) && stm32_pka_reset()) panic();

etienne-lms · 2024-12-04T11:32:01Z

core/drivers/crypto/stm32/ecc.c

+	struct stm32_pka_bn sig_s = { };
+	struct stm32_pka_point pk = { };
+	enum stm32_pka_curve_id cid_from_algo = PKA_LAST_CID;
+	enum stm32_pka_curve_id cid = -1;


s/-1/PKA_LAST_CID/

etienne-lms · 2024-12-04T11:37:18Z

core/drivers/crypto/stm32/stm32_pka.c

+ * @n: Curve prime order n
+ * @n_len: Curve prime order bit size
+ */
+


remove this empty line

etienne-lms · 2024-12-04T11:38:04Z

core/drivers/crypto/stm32/stm32_pka.c

+	struct mutex *lock;
+};
+
+static struct stm32_pka_platdata pka_pdata = {};


No need to use an explicit all-zero initializer.

etienne-lms · 2024-12-04T11:40:33Z

core/drivers/crypto/stm32/stm32_pka.c

+	if (!n_len || !n || !r2modn)
+		return TEE_ERROR_BAD_PARAMETERS;
+
+	mutex_lock(pka_pdata.lock);


Would'nt you prefer to use mutex_lock(&pka_lock) straight here and everywhere the mutex is locked/unlocked?

tbourgoi and others added 6 commits November 25, 2024 14:43

dts: stm32: fix compatible name of PKA peripheral

566dfef

use st,stm32mp13-pka instead of st,stm32mp13-pka64 to be aligned with all other st compatibles. Signed-off-by: Thomas Bourgoin <[email protected]>

dts: stm32: add PKA dependency on RNG clock for stm32mp13

8b34d2c

Adds missing RNG clock resource in PKA nodes in stm32mp13 SoC DTSI files. Signed-off-by: Etienne Carriere <[email protected]> Signed-off-by: Thomas Bourgoin <[email protected]>

crypto: stm32: PKA limit driver support to ECDSA and ECDH.

24c4861

Fix regression of xtest 4006 testing TEE_ALG_SM2_PKE. PKA only support ECDSA key generation ECDH key exchange. Signed-off-by: Thomas Bourgoin <[email protected]>

plat-stm32mp1: conf: default enable PKA

3570a45

Default enable PKA compilation. Enable the STM32_CRYPTO_DRIVERS if PKA is compiled. Signed-off-by: Thomas Bourgoin <[email protected]>

dts: stm32: enable PKA on stm32mp135f-dk

60fd72a

Sets PKA peripheral status to okay. Signed-off-by: Thomas Bourgoin <[email protected]>

tbourgoi changed the title ~~Pka~~ Add support of PKA on stm32mp13 Nov 25, 2024

tbourgoi changed the title ~~Add support of PKA on stm32mp13~~ Add support of PKA peripheral on stm32mp13 Nov 25, 2024

etienne-lms reviewed Nov 26, 2024

View reviewed changes

tbourgoi added 3 commits December 2, 2024 13:42

fixup! crypto: stm32: PKA limit driver support to ECDSA and ECDH.

d1022cb

Use TEE_TYPE_xxx_PUBLIC_KEY instead of TEE_TYPE_xxx_KEYPAIR in stm32_alloc_publickey() Signed-off-by: Thomas Bourgoin <[email protected]>

fixup! crypto: stm32: use PKA IP for ECC algorithm

eacd0e0

Use fdt_reg_info() to read base address and size instead of fdt_fill_device_info(). Signed-off-by: Thomas Bourgoin <[email protected]>

etienne-lms reviewed Dec 2, 2024

View reviewed changes

etienne-lms reviewed Dec 4, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support of PKA peripheral on stm32mp13 #7151

Add support of PKA peripheral on stm32mp13 #7151

tbourgoi commented Nov 25, 2024

etienne-lms Dec 4, 2024

tbourgoi Dec 4, 2024

etienne-lms Dec 4, 2024

etienne-lms Dec 4, 2024

etienne-lms Dec 4, 2024

etienne-lms Dec 4, 2024 •

edited

Loading

etienne-lms Dec 4, 2024

etienne-lms Dec 4, 2024

etienne-lms Dec 4, 2024

etienne-lms Dec 4, 2024

etienne-lms Dec 4, 2024

etienne-lms Dec 4, 2024

Add support of PKA peripheral on stm32mp13 #7151

Are you sure you want to change the base?

Add support of PKA peripheral on stm32mp13 #7151

Conversation

tbourgoi commented Nov 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etienne-lms Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etienne-lms Dec 4, 2024 •

edited

Loading