From 533a806405f328dbbee156c23e340f78c7019944 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 5 Feb 2026 09:30:48 +0100 Subject: [PATCH 1/4] pkwrite: test: factor common part into helper func MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_pkwrite.function | 75 ++++++++++++------------ 1 file changed, 39 insertions(+), 36 deletions(-) diff --git a/tests/suites/test_suite_pkwrite.function b/tests/suites/test_suite_pkwrite.function index 491bc489aa..a07c5dec2a 100644 --- a/tests/suites/test_suite_pkwrite.function +++ b/tests/suites/test_suite_pkwrite.function @@ -66,15 +66,46 @@ static int pk_write_any_key(mbedtls_pk_context *pk, unsigned char **p, return 0; } +static void pk_write_check_context(mbedtls_pk_context *key, + int is_public_key, int is_der, + unsigned char *check_buf, size_t check_buf_len) +{ + unsigned char *buf = NULL; + size_t buf_len; + unsigned char *start_buf; + int expected_result; + + TEST_CALLOC(buf, check_buf_len); + + start_buf = buf; + buf_len = check_buf_len; + if (is_der) { + expected_result = MBEDTLS_ERR_ASN1_BUF_TOO_SMALL; + } else { + expected_result = MBEDTLS_ERR_BASE64_BUFFER_TOO_SMALL; + } + /* Intentionally pass a wrong size for the provided output buffer and check + * that the writing functions fails as expected. */ + for (size_t i = 1; i < buf_len; i++) { + TEST_EQUAL(pk_write_any_key(key, &start_buf, &i, is_public_key, + is_der), expected_result); + } + TEST_EQUAL(pk_write_any_key(key, &start_buf, &buf_len, is_public_key, + is_der), 0); + + TEST_MEMORY_COMPARE(start_buf, buf_len, check_buf, check_buf_len); + +exit: + mbedtls_free(buf); +} + + static void pk_write_check_common(char *key_file, int is_public_key, int is_der) { mbedtls_pk_context key; mbedtls_pk_init(&key); - unsigned char *buf = NULL; unsigned char *check_buf = NULL; - unsigned char *start_buf; - size_t buf_len, check_buf_len; - int expected_result; + size_t check_buf_len; #if defined(MBEDTLS_USE_PSA_CRYPTO) mbedtls_svc_key_id_t opaque_id = MBEDTLS_SVC_KEY_ID_INIT; psa_key_attributes_t key_attr = PSA_KEY_ATTRIBUTES_INIT; @@ -100,8 +131,6 @@ static void pk_write_check_common(char *key_file, int is_public_key, int is_der) } TEST_ASSERT(check_buf_len > 0); - TEST_CALLOC(buf, check_buf_len); - if (is_public_key) { TEST_EQUAL(mbedtls_pk_parse_public_keyfile(&key, key_file), 0); } else { @@ -109,28 +138,12 @@ static void pk_write_check_common(char *key_file, int is_public_key, int is_der) mbedtls_test_rnd_std_rand, NULL), 0); } - start_buf = buf; - buf_len = check_buf_len; - if (is_der) { - expected_result = MBEDTLS_ERR_ASN1_BUF_TOO_SMALL; - } else { - expected_result = MBEDTLS_ERR_BASE64_BUFFER_TOO_SMALL; - } - /* Intentionally pass a wrong size for the provided output buffer and check - * that the writing functions fails as expected. */ - for (size_t i = 1; i < buf_len; i++) { - TEST_EQUAL(pk_write_any_key(&key, &start_buf, &i, is_public_key, - is_der), expected_result); - } - TEST_EQUAL(pk_write_any_key(&key, &start_buf, &buf_len, is_public_key, - is_der), 0); - - TEST_MEMORY_COMPARE(start_buf, buf_len, check_buf, check_buf_len); + pk_write_check_context(&key, is_public_key, is_der, + check_buf, check_buf_len); #if defined(MBEDTLS_USE_PSA_CRYPTO) /* Verify that pk_write works also for opaque private keys */ if (!is_public_key) { - memset(buf, 0, check_buf_len); /* Turn the key PK context into an opaque one. * Note: set some practical usage for the key to make get_psa_attributes() happy. */ TEST_EQUAL(mbedtls_pk_get_psa_attributes(&key, PSA_KEY_USAGE_SIGN_MESSAGE, &key_attr), 0); @@ -138,18 +151,9 @@ static void pk_write_check_common(char *key_file, int is_public_key, int is_der) mbedtls_pk_free(&key); mbedtls_pk_init(&key); TEST_EQUAL(mbedtls_pk_setup_opaque(&key, opaque_id), 0); - start_buf = buf; - buf_len = check_buf_len; - /* Intentionally pass a wrong size for the provided output buffer and check - * that the writing functions fails as expected. */ - for (size_t i = 1; i < buf_len; i++) { - TEST_EQUAL(pk_write_any_key(&key, &start_buf, &i, is_public_key, - is_der), expected_result); - } - TEST_EQUAL(pk_write_any_key(&key, &start_buf, &buf_len, is_public_key, - is_der), 0); - TEST_MEMORY_COMPARE(start_buf, buf_len, check_buf, check_buf_len); + pk_write_check_context(&key, is_public_key, is_der, + check_buf, check_buf_len); } #endif /* MBEDTLS_USE_PSA_CRYPTO */ @@ -157,7 +161,6 @@ exit: #if defined(MBEDTLS_USE_PSA_CRYPTO) psa_destroy_key(opaque_id); #endif /* MBEDTLS_USE_PSA_CRYPTO */ - mbedtls_free(buf); mbedtls_free(check_buf); mbedtls_pk_free(&key); USE_PSA_DONE(); From 56503ba3402f1f6a09c537b93d1860d2b7b761df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 5 Feb 2026 10:08:49 +0100 Subject: [PATCH 2/4] pkwrite: tests: test that DER writes at the end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_pkwrite.function | 47 ++++++++++++++---------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/tests/suites/test_suite_pkwrite.function b/tests/suites/test_suite_pkwrite.function index a07c5dec2a..8e84825c64 100644 --- a/tests/suites/test_suite_pkwrite.function +++ b/tests/suites/test_suite_pkwrite.function @@ -71,29 +71,38 @@ static void pk_write_check_context(mbedtls_pk_context *key, unsigned char *check_buf, size_t check_buf_len) { unsigned char *buf = NULL; - size_t buf_len; - unsigned char *start_buf; - int expected_result; + int expected_error = is_der ? + MBEDTLS_ERR_ASN1_BUF_TOO_SMALL : + MBEDTLS_ERR_BASE64_BUFFER_TOO_SMALL; - TEST_CALLOC(buf, check_buf_len); + /* Test with: + * - buffer too small (all sizes) + * - buffer exactly the right size + * - buffer a bit larger - DER functions should write to the end of the + * buffer, and we can only tell the difference with a larger buffer + */ + for (size_t buf_size = 1; buf_size <= check_buf_len + 2; buf_size++) { + mbedtls_free(buf); + buf = NULL; + TEST_CALLOC(buf, buf_size); - start_buf = buf; - buf_len = check_buf_len; - if (is_der) { - expected_result = MBEDTLS_ERR_ASN1_BUF_TOO_SMALL; - } else { - expected_result = MBEDTLS_ERR_BASE64_BUFFER_TOO_SMALL; - } - /* Intentionally pass a wrong size for the provided output buffer and check - * that the writing functions fails as expected. */ - for (size_t i = 1; i < buf_len; i++) { - TEST_EQUAL(pk_write_any_key(key, &start_buf, &i, is_public_key, + unsigned char *start_buf = buf; + size_t out_len = buf_size; + int expected_result = buf_size < check_buf_len ? expected_error : 0; + mbedtls_test_set_step(buf_size); + + TEST_EQUAL(pk_write_any_key(key, &start_buf, &out_len, is_public_key, is_der), expected_result); - } - TEST_EQUAL(pk_write_any_key(key, &start_buf, &buf_len, is_public_key, - is_der), 0); - TEST_MEMORY_COMPARE(start_buf, buf_len, check_buf, check_buf_len); + if (expected_result == 0) { + TEST_MEMORY_COMPARE(start_buf, out_len, check_buf, check_buf_len); + + if (is_der) { + /* Data should be at the end of the buffer */ + TEST_ASSERT(start_buf + out_len == buf + buf_size); + } + } + } exit: mbedtls_free(buf); From 20118b65bd28e5cfa3acaf0f749e0f3830c13f5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 5 Feb 2026 10:47:23 +0100 Subject: [PATCH 3/4] pkwrite: RSA: avoid large stack buffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the default build, it was 2363 bytes which is a lot to put on the stack for constrained devices. Fortunately we already have a large enough buffer at hand: the user-provided output buffer. Use it. Signed-off-by: Manuel Pégourié-Gonnard --- library/pkwrite.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/library/pkwrite.c b/library/pkwrite.c index 2a698448be..ff6c0bfb44 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -64,22 +64,24 @@ static int pk_write_rsa_der(unsigned char **p, unsigned char *buf, { #if defined(MBEDTLS_USE_PSA_CRYPTO) if (mbedtls_pk_get_type(pk) == MBEDTLS_PK_OPAQUE) { - uint8_t tmp[PSA_EXPORT_KEY_PAIR_MAX_SIZE]; - size_t tmp_len = 0; + psa_status_t status; + size_t buf_size = (size_t) (*p - buf); + size_t key_len = 0; - if (psa_export_key(pk->priv_id, tmp, sizeof(tmp), &tmp_len) != PSA_SUCCESS) { - return MBEDTLS_ERR_PK_BAD_INPUT_DATA; - } - /* Ensure there's enough space in the provided buffer before copying data into it. */ - if (tmp_len > (size_t) (*p - buf)) { - mbedtls_platform_zeroize(tmp, sizeof(tmp)); + status = psa_export_key(pk->priv_id, buf, buf_size, &key_len); + if (status == PSA_ERROR_BUFFER_TOO_SMALL) { return MBEDTLS_ERR_ASN1_BUF_TOO_SMALL; + } else if (status != PSA_SUCCESS) { + return PSA_PK_RSA_TO_MBEDTLS_ERR(status); } - *p -= tmp_len; - memcpy(*p, tmp, tmp_len); - mbedtls_platform_zeroize(tmp, sizeof(tmp)); - return (int) tmp_len; + /* We wrote to the beginning of the buffer while + * we were supposed to write to its end. */ + *p -= key_len; + memmove(*p, buf, key_len); + mbedtls_platform_zeroize(buf, *p - buf); + + return (int) key_len; } #endif /* MBEDTLS_USE_PSA_CRYPTO */ return mbedtls_rsa_write_key(mbedtls_pk_rsa(*pk), buf, p); From 6617ab467feb7f4222d2930559c2463587f656e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 5 Feb 2026 12:22:12 +0100 Subject: [PATCH 4/4] pkwrite: tests: make helper more robust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_pkwrite.function | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/tests/suites/test_suite_pkwrite.function b/tests/suites/test_suite_pkwrite.function index 8e84825c64..691125e416 100644 --- a/tests/suites/test_suite_pkwrite.function +++ b/tests/suites/test_suite_pkwrite.function @@ -66,10 +66,11 @@ static int pk_write_any_key(mbedtls_pk_context *pk, unsigned char **p, return 0; } -static void pk_write_check_context(mbedtls_pk_context *key, - int is_public_key, int is_der, - unsigned char *check_buf, size_t check_buf_len) +static int pk_write_check_context(mbedtls_pk_context *key, + int is_public_key, int is_der, + unsigned char *check_buf, size_t check_buf_len) { + int ret = -1; unsigned char *buf = NULL; int expected_error = is_der ? MBEDTLS_ERR_ASN1_BUF_TOO_SMALL : @@ -104,8 +105,11 @@ static void pk_write_check_context(mbedtls_pk_context *key, } } + ret = 0; + exit: mbedtls_free(buf); + return ret; } @@ -147,8 +151,10 @@ static void pk_write_check_common(char *key_file, int is_public_key, int is_der) mbedtls_test_rnd_std_rand, NULL), 0); } - pk_write_check_context(&key, is_public_key, is_der, - check_buf, check_buf_len); + if (pk_write_check_context(&key, is_public_key, is_der, + check_buf, check_buf_len) != 0) { + goto exit; + } #if defined(MBEDTLS_USE_PSA_CRYPTO) /* Verify that pk_write works also for opaque private keys */ @@ -161,8 +167,10 @@ static void pk_write_check_common(char *key_file, int is_public_key, int is_der) mbedtls_pk_init(&key); TEST_EQUAL(mbedtls_pk_setup_opaque(&key, opaque_id), 0); - pk_write_check_context(&key, is_public_key, is_der, - check_buf, check_buf_len); + if (pk_write_check_context(&key, is_public_key, is_der, + check_buf, check_buf_len) != 0) { + goto exit; + } } #endif /* MBEDTLS_USE_PSA_CRYPTO */