From c8b786561232ad8bb97b2fdd3d02d7acec06eec1 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 26 Apr 2023 12:24:26 +0200 Subject: [PATCH 1/9] test: align ec_pub public keyfile with its ec_prv.sec1 counterpart This change affects: - both PEM and DER files, since they contain the same public key only in different formats - "ec_pub.comp.pem" since it's the same as "ec_pub.pem" but in compressed format The makefile was also updated accordingly to reflect these dependencies. Signed-off-by: Valerio Setti --- tests/data_files/Makefile | 3 +++ tests/data_files/ec_pub.comp.pem | 4 ++-- tests/data_files/ec_pub.der | Bin 75 -> 75 bytes tests/data_files/ec_pub.pem | 4 ++-- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/data_files/Makefile b/tests/data_files/Makefile index 47370b49e9..c008030792 100644 --- a/tests/data_files/Makefile +++ b/tests/data_files/Makefile @@ -935,6 +935,9 @@ ec_prv.pk8param.pem: ec_prv.pk8param.der $(OPENSSL) pkey -in $< -inform DER -out $@ all_final += ec_prv.pk8param.pem +ec_pub.der: ec_prv.sec1.der + $(OPENSSL) pkey -in $< -inform DER -outform DER -pubout -out $@ + ec_prv.sec1.comp.pem: ec_prv.sec1.pem $(OPENSSL) ec -in $< -out $@ -conv_form compressed all_final += ec_prv.sec1.comp.pem diff --git a/tests/data_files/ec_pub.comp.pem b/tests/data_files/ec_pub.comp.pem index a3742a3120..55fac08430 100644 --- a/tests/data_files/ec_pub.comp.pem +++ b/tests/data_files/ec_pub.comp.pem @@ -1,4 +1,4 @@ -----BEGIN PUBLIC KEY----- -MDEwEwYHKoZIzj0CAQYIKoZIzj0DAQEDGgACvHl9s65/COw9SWtPtBGz9iClWKUB -4CIt +MDEwEwYHKoZIzj0CAQYIKoZIzj0DAQEDGgADUXW83zCjcPOdU5PmEnKI2AFntfS0 +t3bG -----END PUBLIC KEY----- diff --git a/tests/data_files/ec_pub.der b/tests/data_files/ec_pub.der index 74c5951f60c2c13c29369f85c95958c4af70dc3c..e4e59158a897650220a9650b7e30beaaf00fdbba 100644 GIT binary patch delta 53 zcmV-50LuSMOOP8dQFXlEFr#quol}$M5^{*x0cW-Jw6}J~boa*dRJYP31}quXE*J>OB delta 53 zcmV-50LuSMOOP8dym@`Iu73#ZJxOa%v=OuRAf;HP0pKDn2+!c$itLLTd-6-^DN)Q@ Lp}xB$bdZ|hmr)u- diff --git a/tests/data_files/ec_pub.pem b/tests/data_files/ec_pub.pem index d677d27eb5..d54dc944cd 100644 --- a/tests/data_files/ec_pub.pem +++ b/tests/data_files/ec_pub.pem @@ -1,4 +1,4 @@ -----BEGIN PUBLIC KEY----- -MEkwEwYHKoZIzj0CAQYIKoZIzj0DAQEDMgAEvHl9s65/COw9SWtPtBGz9iClWKUB -4CItCM/g3Irsixp78kvpKVHMW6G+uyR0kJrg +MEkwEwYHKoZIzj0CAQYIKoZIzj0DAQEDMgAEUXW83zCjcPOdU5PmEnKI2AFntfS0 +t3bGdPfG81S30iQGLB9oVLWnrw/leOryWPAn -----END PUBLIC KEY----- From 2c5052647646ee52e525fd454c785db0d37fbbef Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 26 Apr 2023 13:08:56 +0200 Subject: [PATCH 2/9] pk: fix: clear buffer holding raw EC private key on exit Signed-off-by: Valerio Setti --- library/pk.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/pk.c b/library/pk.c index 2516eed6fe..04c5e405e9 100644 --- a/library/pk.c +++ b/library/pk.c @@ -908,6 +908,8 @@ int mbedtls_pk_wrap_as_opaque(mbedtls_pk_context *pk, return PSA_PK_TO_MBEDTLS_ERR(status); } + mbedtls_platform_zeroize(d, sizeof(d)); + /* make PK context wrap the key slot */ mbedtls_pk_free(pk); mbedtls_pk_init(pk); From 1751341b682a89bdc5761d7a15d6232c3dbcbd11 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 26 Apr 2023 14:48:43 +0200 Subject: [PATCH 3/9] test: add test function for public key derivation starting from private one Data test cases are also included in the commit. Signed-off-by: Valerio Setti --- tests/suites/test_suite_pkwrite.data | 20 +++++++ tests/suites/test_suite_pkwrite.function | 75 ++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/tests/suites/test_suite_pkwrite.data b/tests/suites/test_suite_pkwrite.data index a339cdbb0c..778352f64b 100644 --- a/tests/suites/test_suite_pkwrite.data +++ b/tests/suites/test_suite_pkwrite.data @@ -93,3 +93,23 @@ pk_write_key_check:"data_files/ec_bp512_prv.pem":TEST_PEM Private key write check EC Brainpool 512 bits (DER) depends_on:MBEDTLS_ECP_LIGHT:MBEDTLS_ECP_DP_BP512R1_ENABLED pk_write_key_check:"data_files/ec_bp512_prv.der":TEST_DER + +Derive public key RSA +depends_on:MBEDTLS_RSA_C +pk_write_check_public_key_derivation:"data_files/server1.key":"data_files/server1.pubkey.der" + +Derive public key RSA 4096 +depends_on:MBEDTLS_RSA_C +pk_write_check_public_key_derivation:"data_files/rsa4096_prv.der":"data_files/rsa4096_pub.der" + +Derive public key EC 192 bits +depends_on:MBEDTLS_ECP_LIGHT:MBEDTLS_ECP_DP_SECP192R1_ENABLED +pk_write_check_public_key_derivation:"data_files/ec_prv.sec1.der":"data_files/ec_pub.der" + +Derive public key EC 521 bits +depends_on:MBEDTLS_ECP_LIGHT:MBEDTLS_ECP_DP_SECP521R1_ENABLED +pk_write_check_public_key_derivation:"data_files/ec_521_prv.der":"data_files/ec_521_pub.der" + +Derive public key EC Brainpool 512 bits +depends_on:MBEDTLS_ECP_LIGHT:MBEDTLS_ECP_DP_BP512R1_ENABLED +pk_write_check_public_key_derivation:"data_files/ec_bp512_prv.der":"data_files/ec_bp512_pub.der" diff --git a/tests/suites/test_suite_pkwrite.function b/tests/suites/test_suite_pkwrite.function index 804e9a77eb..c5668a0b39 100644 --- a/tests/suites/test_suite_pkwrite.function +++ b/tests/suites/test_suite_pkwrite.function @@ -2,6 +2,7 @@ #include "mbedtls/pk.h" #include "mbedtls/pem.h" #include "mbedtls/oid.h" +#include "psa/crypto_sizes.h" typedef enum { TEST_PEM, @@ -124,3 +125,77 @@ void pk_write_key_check(char *key_file, int is_der) goto exit; /* make the compiler happy */ } /* END_CASE */ + +/* BEGIN_CASE */ +void pk_write_check_public_key_derivation(char *priv_key_file, + char *pub_key_file) +{ + mbedtls_pk_context priv_key, pub_key; + uint8_t derived_key_raw[PSA_EXPORT_PUBLIC_KEY_MAX_SIZE]; + uint8_t *derived_key_start; + size_t derived_key_len = 0; + uint8_t pub_key_raw[PSA_EXPORT_PUBLIC_KEY_MAX_SIZE]; + uint8_t *pub_key_start; + size_t pub_key_len = 0; +#if defined(MBEDTLS_USE_PSA_CRYPTO) + mbedtls_svc_key_id_t opaque_key_id = MBEDTLS_SVC_KEY_ID_INIT; +#endif /* MBEDTLS_USE_PSA_CRYPTO */ + + mbedtls_pk_init(&priv_key); + mbedtls_pk_init(&pub_key); + USE_PSA_INIT(); + + memset(derived_key_raw, 0, sizeof(derived_key_raw)); + memset(pub_key_raw, 0, sizeof(pub_key_raw)); + + TEST_EQUAL(mbedtls_pk_parse_keyfile(&priv_key, priv_key_file, NULL, + mbedtls_test_rnd_std_rand, NULL), 0); + TEST_EQUAL(mbedtls_pk_parse_public_keyfile(&pub_key, pub_key_file), 0); + + /* mbedtls_pk_write_pubkey() writes data backward in the provided buffer, + * i.e. derived_key_raw, so we place derived_key_start at the end of it + * and it will be updated accordingly on return. + * The same holds for pub_key_raw and pub_key_start below.*/ + derived_key_start = derived_key_raw + sizeof(derived_key_raw); + TEST_LE_U(1, mbedtls_pk_write_pubkey(&derived_key_start, + derived_key_raw, &priv_key)); + derived_key_len = sizeof(derived_key_raw) - + (derived_key_start - derived_key_raw); + + + pub_key_start = pub_key_raw + sizeof(pub_key_raw); + TEST_LE_U(1, mbedtls_pk_write_pubkey(&pub_key_start, + pub_key_raw, &pub_key)); + pub_key_len = sizeof(pub_key_raw) - + (pub_key_start - pub_key_raw); + + ASSERT_COMPARE(derived_key_start, derived_key_len, + pub_key_start, pub_key_len); + +#if defined(MBEDTLS_USE_PSA_CRYPTO) + mbedtls_platform_zeroize(derived_key_raw, sizeof(derived_key_raw)); + derived_key_len = 0; + + TEST_EQUAL(mbedtls_pk_wrap_as_opaque(&priv_key, &opaque_key_id, + PSA_ALG_NONE, PSA_KEY_USAGE_EXPORT, + PSA_ALG_NONE), 0); + + derived_key_start = derived_key_raw + sizeof(derived_key_raw); + TEST_LE_U(1, mbedtls_pk_write_pubkey(&derived_key_start, + derived_key_raw, &priv_key)); + derived_key_len = sizeof(derived_key_raw) - + (derived_key_start - derived_key_raw); + + ASSERT_COMPARE(derived_key_start, derived_key_len, + pub_key_start, pub_key_len); +#endif /* MBEDTLS_USE_PSA_CRYPTO */ + +exit: +#if defined(MBEDTLS_USE_PSA_CRYPTO) + psa_destroy_key(opaque_key_id); +#endif /* MBEDTLS_USE_PSA_CRYPTO */ + mbedtls_pk_free(&pub_key); + mbedtls_pk_free(&priv_key); + USE_PSA_DONE(); +} +/* END_CASE */ From d860a79029fd9d74b441478ac8d61fd7b4a83c09 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Thu, 27 Apr 2023 09:41:09 +0200 Subject: [PATCH 4/9] test: fix wrong private key file Signed-off-by: Valerio Setti --- tests/suites/test_suite_pkwrite.data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_pkwrite.data b/tests/suites/test_suite_pkwrite.data index 778352f64b..a14b42b9e6 100644 --- a/tests/suites/test_suite_pkwrite.data +++ b/tests/suites/test_suite_pkwrite.data @@ -96,7 +96,7 @@ pk_write_key_check:"data_files/ec_bp512_prv.der":TEST_DER Derive public key RSA depends_on:MBEDTLS_RSA_C -pk_write_check_public_key_derivation:"data_files/server1.key":"data_files/server1.pubkey.der" +pk_write_check_public_key_derivation:"data_files/server1.key.der":"data_files/server1.pubkey.der" Derive public key RSA 4096 depends_on:MBEDTLS_RSA_C From 8820b57b6e5fecd78e55e7aa804eadf500ebeb4c Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Thu, 27 Apr 2023 10:03:08 +0200 Subject: [PATCH 5/9] test: fix makefile for ec_pub.[der/pem] generation Signed-off-by: Valerio Setti --- tests/data_files/Makefile | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/data_files/Makefile b/tests/data_files/Makefile index c008030792..257903ce80 100644 --- a/tests/data_files/Makefile +++ b/tests/data_files/Makefile @@ -935,8 +935,9 @@ ec_prv.pk8param.pem: ec_prv.pk8param.der $(OPENSSL) pkey -in $< -inform DER -out $@ all_final += ec_prv.pk8param.pem -ec_pub.der: ec_prv.sec1.der - $(OPENSSL) pkey -in $< -inform DER -outform DER -pubout -out $@ +ec_pub.pem: ec_prv.sec1.der + $(OPENSSL) pkey -in $< -inform DER -outform PEM -pubout -out $@ +all_final += ec_pub.pem ec_prv.sec1.comp.pem: ec_prv.sec1.pem $(OPENSSL) ec -in $< -out $@ -conv_form compressed From 2d814990267b4f9b1bb509d81c9f699ea3bace71 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Thu, 27 Apr 2023 10:05:03 +0200 Subject: [PATCH 6/9] pk: fix position for mbedtls_platform_zeroize Signed-off-by: Valerio Setti --- library/pk.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/pk.c b/library/pk.c index 04c5e405e9..4e2f218032 100644 --- a/library/pk.c +++ b/library/pk.c @@ -904,12 +904,11 @@ int mbedtls_pk_wrap_as_opaque(mbedtls_pk_context *pk, /* import private key into PSA */ status = psa_import_key(&attributes, d, d_len, key); + mbedtls_platform_zeroize(d, sizeof(d)); if (status != PSA_SUCCESS) { return PSA_PK_TO_MBEDTLS_ERR(status); } - mbedtls_platform_zeroize(d, sizeof(d)); - /* make PK context wrap the key slot */ mbedtls_pk_free(pk); mbedtls_pk_init(pk); From 84554e983025e079bb6bdc6fd467db7c81b6c552 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Thu, 27 Apr 2023 10:06:45 +0200 Subject: [PATCH 7/9] test: use better naming for the newly introduced test function Signed-off-by: Valerio Setti --- tests/suites/test_suite_pkwrite.data | 10 +++++----- tests/suites/test_suite_pkwrite.function | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/suites/test_suite_pkwrite.data b/tests/suites/test_suite_pkwrite.data index a14b42b9e6..34bf0da853 100644 --- a/tests/suites/test_suite_pkwrite.data +++ b/tests/suites/test_suite_pkwrite.data @@ -96,20 +96,20 @@ pk_write_key_check:"data_files/ec_bp512_prv.der":TEST_DER Derive public key RSA depends_on:MBEDTLS_RSA_C -pk_write_check_public_key_derivation:"data_files/server1.key.der":"data_files/server1.pubkey.der" +pk_write_public_from_private:"data_files/server1.key.der":"data_files/server1.pubkey.der" Derive public key RSA 4096 depends_on:MBEDTLS_RSA_C -pk_write_check_public_key_derivation:"data_files/rsa4096_prv.der":"data_files/rsa4096_pub.der" +pk_write_public_from_private:"data_files/rsa4096_prv.der":"data_files/rsa4096_pub.der" Derive public key EC 192 bits depends_on:MBEDTLS_ECP_LIGHT:MBEDTLS_ECP_DP_SECP192R1_ENABLED -pk_write_check_public_key_derivation:"data_files/ec_prv.sec1.der":"data_files/ec_pub.der" +pk_write_public_from_private:"data_files/ec_prv.sec1.der":"data_files/ec_pub.der" Derive public key EC 521 bits depends_on:MBEDTLS_ECP_LIGHT:MBEDTLS_ECP_DP_SECP521R1_ENABLED -pk_write_check_public_key_derivation:"data_files/ec_521_prv.der":"data_files/ec_521_pub.der" +pk_write_public_from_private:"data_files/ec_521_prv.der":"data_files/ec_521_pub.der" Derive public key EC Brainpool 512 bits depends_on:MBEDTLS_ECP_LIGHT:MBEDTLS_ECP_DP_BP512R1_ENABLED -pk_write_check_public_key_derivation:"data_files/ec_bp512_prv.der":"data_files/ec_bp512_pub.der" +pk_write_public_from_private:"data_files/ec_bp512_prv.der":"data_files/ec_bp512_pub.der" diff --git a/tests/suites/test_suite_pkwrite.function b/tests/suites/test_suite_pkwrite.function index c5668a0b39..28684622bd 100644 --- a/tests/suites/test_suite_pkwrite.function +++ b/tests/suites/test_suite_pkwrite.function @@ -127,7 +127,7 @@ void pk_write_key_check(char *key_file, int is_der) /* END_CASE */ /* BEGIN_CASE */ -void pk_write_check_public_key_derivation(char *priv_key_file, +void pk_write_public_from_private(char *priv_key_file, char *pub_key_file) { mbedtls_pk_context priv_key, pub_key; From f5451717af843d95f7ed4f7d2154dfa7b464332b Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Thu, 27 Apr 2023 10:52:57 +0200 Subject: [PATCH 8/9] test: optimize code for pk_write_public_from_private() Signed-off-by: Valerio Setti --- tests/suites/test_suite_pkwrite.function | 56 ++++++++---------------- 1 file changed, 18 insertions(+), 38 deletions(-) diff --git a/tests/suites/test_suite_pkwrite.function b/tests/suites/test_suite_pkwrite.function index 28684622bd..7fdda80f24 100644 --- a/tests/suites/test_suite_pkwrite.function +++ b/tests/suites/test_suite_pkwrite.function @@ -127,74 +127,54 @@ void pk_write_key_check(char *key_file, int is_der) /* END_CASE */ /* BEGIN_CASE */ -void pk_write_public_from_private(char *priv_key_file, - char *pub_key_file) +void pk_write_public_from_private(char *priv_key_file, char *pub_key_file) { - mbedtls_pk_context priv_key, pub_key; - uint8_t derived_key_raw[PSA_EXPORT_PUBLIC_KEY_MAX_SIZE]; - uint8_t *derived_key_start; + mbedtls_pk_context priv_key; + uint8_t *derived_key_raw = NULL; size_t derived_key_len = 0; - uint8_t pub_key_raw[PSA_EXPORT_PUBLIC_KEY_MAX_SIZE]; - uint8_t *pub_key_start; + uint8_t *pub_key_raw = NULL; size_t pub_key_len = 0; #if defined(MBEDTLS_USE_PSA_CRYPTO) mbedtls_svc_key_id_t opaque_key_id = MBEDTLS_SVC_KEY_ID_INIT; #endif /* MBEDTLS_USE_PSA_CRYPTO */ mbedtls_pk_init(&priv_key); - mbedtls_pk_init(&pub_key); USE_PSA_INIT(); - memset(derived_key_raw, 0, sizeof(derived_key_raw)); - memset(pub_key_raw, 0, sizeof(pub_key_raw)); - TEST_EQUAL(mbedtls_pk_parse_keyfile(&priv_key, priv_key_file, NULL, mbedtls_test_rnd_std_rand, NULL), 0); - TEST_EQUAL(mbedtls_pk_parse_public_keyfile(&pub_key, pub_key_file), 0); + TEST_EQUAL(mbedtls_pk_load_file(pub_key_file, &pub_key_raw, + &pub_key_len), 0); - /* mbedtls_pk_write_pubkey() writes data backward in the provided buffer, - * i.e. derived_key_raw, so we place derived_key_start at the end of it - * and it will be updated accordingly on return. - * The same holds for pub_key_raw and pub_key_start below.*/ - derived_key_start = derived_key_raw + sizeof(derived_key_raw); - TEST_LE_U(1, mbedtls_pk_write_pubkey(&derived_key_start, - derived_key_raw, &priv_key)); - derived_key_len = sizeof(derived_key_raw) - - (derived_key_start - derived_key_raw); + derived_key_len = pub_key_len; + ASSERT_ALLOC(derived_key_raw, derived_key_len); + TEST_LE_U(1, mbedtls_pk_write_pubkey_der(&priv_key, derived_key_raw, + derived_key_len)); - pub_key_start = pub_key_raw + sizeof(pub_key_raw); - TEST_LE_U(1, mbedtls_pk_write_pubkey(&pub_key_start, - pub_key_raw, &pub_key)); - pub_key_len = sizeof(pub_key_raw) - - (pub_key_start - pub_key_raw); - - ASSERT_COMPARE(derived_key_start, derived_key_len, - pub_key_start, pub_key_len); + ASSERT_COMPARE(derived_key_raw, derived_key_len, + pub_key_raw, pub_key_len); #if defined(MBEDTLS_USE_PSA_CRYPTO) mbedtls_platform_zeroize(derived_key_raw, sizeof(derived_key_raw)); - derived_key_len = 0; TEST_EQUAL(mbedtls_pk_wrap_as_opaque(&priv_key, &opaque_key_id, PSA_ALG_NONE, PSA_KEY_USAGE_EXPORT, PSA_ALG_NONE), 0); - derived_key_start = derived_key_raw + sizeof(derived_key_raw); - TEST_LE_U(1, mbedtls_pk_write_pubkey(&derived_key_start, - derived_key_raw, &priv_key)); - derived_key_len = sizeof(derived_key_raw) - - (derived_key_start - derived_key_raw); + TEST_LE_U(1, mbedtls_pk_write_pubkey_der(&priv_key, derived_key_raw, + derived_key_len)); - ASSERT_COMPARE(derived_key_start, derived_key_len, - pub_key_start, pub_key_len); + ASSERT_COMPARE(derived_key_raw, derived_key_len, + pub_key_raw, pub_key_len); #endif /* MBEDTLS_USE_PSA_CRYPTO */ exit: #if defined(MBEDTLS_USE_PSA_CRYPTO) psa_destroy_key(opaque_key_id); #endif /* MBEDTLS_USE_PSA_CRYPTO */ - mbedtls_pk_free(&pub_key); + mbedtls_free(derived_key_raw); + mbedtls_free(pub_key_raw); mbedtls_pk_free(&priv_key); USE_PSA_DONE(); } From 9a855f21aa610b5f1f8aafce90d279fc5d3524ff Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Thu, 27 Apr 2023 12:07:23 +0200 Subject: [PATCH 9/9] test: check for exact length of returned pub key Signed-off-by: Valerio Setti --- tests/suites/test_suite_pkwrite.function | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_pkwrite.function b/tests/suites/test_suite_pkwrite.function index 7fdda80f24..c148c8a848 100644 --- a/tests/suites/test_suite_pkwrite.function +++ b/tests/suites/test_suite_pkwrite.function @@ -149,8 +149,8 @@ void pk_write_public_from_private(char *priv_key_file, char *pub_key_file) derived_key_len = pub_key_len; ASSERT_ALLOC(derived_key_raw, derived_key_len); - TEST_LE_U(1, mbedtls_pk_write_pubkey_der(&priv_key, derived_key_raw, - derived_key_len)); + TEST_EQUAL(mbedtls_pk_write_pubkey_der(&priv_key, derived_key_raw, + derived_key_len), pub_key_len); ASSERT_COMPARE(derived_key_raw, derived_key_len, pub_key_raw, pub_key_len); @@ -162,8 +162,8 @@ void pk_write_public_from_private(char *priv_key_file, char *pub_key_file) PSA_ALG_NONE, PSA_KEY_USAGE_EXPORT, PSA_ALG_NONE), 0); - TEST_LE_U(1, mbedtls_pk_write_pubkey_der(&priv_key, derived_key_raw, - derived_key_len)); + TEST_EQUAL(mbedtls_pk_write_pubkey_der(&priv_key, derived_key_raw, + derived_key_len), pub_key_len); ASSERT_COMPARE(derived_key_raw, derived_key_len, pub_key_raw, pub_key_len);