From 45ad306fbf0b66c0396d0be442c620293c63c531 Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Fri, 11 Aug 2023 15:03:51 +0800 Subject: [PATCH 1/7] pkwrite.c: save stack usage for pk_write_pubkey_pem mbedtls_pk_write_pubkey_pem would allocate 2086 bytes in writing a DER encoded RSA public key. To save stack usage significantly, we use heap memory instead. Signed-off-by: Yanray Wang --- library/pkwrite.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/library/pkwrite.c b/library/pkwrite.c index 439428cff..511e22251 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -759,20 +759,27 @@ int mbedtls_pk_write_key_der(const mbedtls_pk_context *key, unsigned char *buf, int mbedtls_pk_write_pubkey_pem(const mbedtls_pk_context *key, unsigned char *buf, size_t size) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - unsigned char output_buf[PUB_DER_MAX_BYTES]; + unsigned char *output_buf = NULL; + output_buf = calloc(1, PUB_DER_MAX_BYTES); + if (output_buf == NULL) { + return MBEDTLS_ERR_PK_ALLOC_FAILED; + } size_t olen = 0; if ((ret = mbedtls_pk_write_pubkey_der(key, output_buf, - sizeof(output_buf))) < 0) { + PUB_DER_MAX_BYTES)) < 0) { + free(output_buf); return ret; } if ((ret = mbedtls_pem_write_buffer(PEM_BEGIN_PUBLIC_KEY, PEM_END_PUBLIC_KEY, - output_buf + sizeof(output_buf) - ret, + output_buf + PUB_DER_MAX_BYTES - ret, ret, buf, size, &olen)) != 0) { + free(output_buf); return ret; } + free(output_buf); return 0; } From c84086e55c96a44dd63dd508a5929be1788854d1 Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Fri, 11 Aug 2023 15:33:07 +0800 Subject: [PATCH 2/7] pkwrite.c: save stack usage for pk_write_key_pem mbedtls_pk_write_key_pem would allocate 5679 bytes in writing a DER encoded RSA private key. To save stack usage significantly, we use heap memory instead. Signed-off-by: Yanray Wang --- library/pkwrite.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/library/pkwrite.c b/library/pkwrite.c index 511e22251..f3acec751 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -786,7 +786,11 @@ int mbedtls_pk_write_pubkey_pem(const mbedtls_pk_context *key, unsigned char *bu int mbedtls_pk_write_key_pem(const mbedtls_pk_context *key, unsigned char *buf, size_t size) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - unsigned char output_buf[PRV_DER_MAX_BYTES]; + unsigned char *output_buf = NULL; + output_buf = calloc(1, PRV_DER_MAX_BYTES); + if (output_buf == NULL) { + return MBEDTLS_ERR_PK_ALLOC_FAILED; + } const char *begin, *end; size_t olen = 0; #if defined(MBEDTLS_PK_HAVE_ECC_KEYS) @@ -799,7 +803,8 @@ int mbedtls_pk_write_key_pem(const mbedtls_pk_context *key, unsigned char *buf, int is_rsa_opaque = 0; #endif - if ((ret = mbedtls_pk_write_key_der(key, output_buf, sizeof(output_buf))) < 0) { + if ((ret = mbedtls_pk_write_key_der(key, output_buf, PRV_DER_MAX_BYTES)) < 0) { + free(output_buf); return ret; } @@ -843,14 +848,19 @@ int mbedtls_pk_write_key_pem(const mbedtls_pk_context *key, unsigned char *buf, } } else #endif /* MBEDTLS_PK_HAVE_ECC_KEYS */ - return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; + { + free(output_buf); + return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; + } if ((ret = mbedtls_pem_write_buffer(begin, end, - output_buf + sizeof(output_buf) - ret, + output_buf + PRV_DER_MAX_BYTES - ret, ret, buf, size, &olen)) != 0) { + free(output_buf); return ret; } + free(output_buf); return 0; } #endif /* MBEDTLS_PEM_WRITE_C */ From 7226df07577183b59d566709d7caf2d2089609aa Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Fri, 11 Aug 2023 15:52:09 +0800 Subject: [PATCH 3/7] pkwrite.c: add a cleanup label to save code size Signed-off-by: Yanray Wang --- library/pkwrite.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/library/pkwrite.c b/library/pkwrite.c index f3acec751..cc58f46e9 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -768,19 +768,19 @@ int mbedtls_pk_write_pubkey_pem(const mbedtls_pk_context *key, unsigned char *bu if ((ret = mbedtls_pk_write_pubkey_der(key, output_buf, PUB_DER_MAX_BYTES)) < 0) { - free(output_buf); - return ret; + goto cleanup; } if ((ret = mbedtls_pem_write_buffer(PEM_BEGIN_PUBLIC_KEY, PEM_END_PUBLIC_KEY, output_buf + PUB_DER_MAX_BYTES - ret, ret, buf, size, &olen)) != 0) { - free(output_buf); - return ret; + goto cleanup; } + ret = 0; +cleanup: free(output_buf); - return 0; + return ret; } int mbedtls_pk_write_key_pem(const mbedtls_pk_context *key, unsigned char *buf, size_t size) @@ -804,8 +804,7 @@ int mbedtls_pk_write_key_pem(const mbedtls_pk_context *key, unsigned char *buf, #endif if ((ret = mbedtls_pk_write_key_der(key, output_buf, PRV_DER_MAX_BYTES)) < 0) { - free(output_buf); - return ret; + goto cleanup; } #if defined(MBEDTLS_USE_PSA_CRYPTO) @@ -849,19 +848,20 @@ int mbedtls_pk_write_key_pem(const mbedtls_pk_context *key, unsigned char *buf, } else #endif /* MBEDTLS_PK_HAVE_ECC_KEYS */ { - free(output_buf); - return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; + ret = MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; + goto cleanup; } if ((ret = mbedtls_pem_write_buffer(begin, end, output_buf + PRV_DER_MAX_BYTES - ret, ret, buf, size, &olen)) != 0) { - free(output_buf); - return ret; + goto cleanup; } + ret = 0; +cleanup: free(output_buf); - return 0; + return ret; } #endif /* MBEDTLS_PEM_WRITE_C */ From 0882828b5162f1a4e3e9776dc5ea8105650de314 Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Fri, 11 Aug 2023 16:15:14 +0800 Subject: [PATCH 4/7] pkwrite: add Changelog entry Signed-off-by: Yanray Wang --- ChangeLog.d/pkwrite-pem-use-heap.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 ChangeLog.d/pkwrite-pem-use-heap.txt diff --git a/ChangeLog.d/pkwrite-pem-use-heap.txt b/ChangeLog.d/pkwrite-pem-use-heap.txt new file mode 100644 index 000000000..d2e7129ec --- /dev/null +++ b/ChangeLog.d/pkwrite-pem-use-heap.txt @@ -0,0 +1,4 @@ +Changes + * Use heap memory to allocate DER encoded RSA public/private key. + This reduces stack usage significantly for writing a public/private + key to a PEM string. From 08d5f46c83d0d7fcfbadd0e879db7da015046a88 Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Mon, 21 Aug 2023 15:15:19 +0800 Subject: [PATCH 5/7] pkwrite.c: call calloc and free properly Signed-off-by: Yanray Wang --- library/pkwrite.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/pkwrite.c b/library/pkwrite.c index cc58f46e9..225cde90d 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -760,7 +760,7 @@ int mbedtls_pk_write_pubkey_pem(const mbedtls_pk_context *key, unsigned char *bu { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char *output_buf = NULL; - output_buf = calloc(1, PUB_DER_MAX_BYTES); + output_buf = mbedtls_calloc(1, PUB_DER_MAX_BYTES); if (output_buf == NULL) { return MBEDTLS_ERR_PK_ALLOC_FAILED; } @@ -779,7 +779,7 @@ int mbedtls_pk_write_pubkey_pem(const mbedtls_pk_context *key, unsigned char *bu ret = 0; cleanup: - free(output_buf); + mbedtls_free(output_buf); return ret; } @@ -787,7 +787,7 @@ int mbedtls_pk_write_key_pem(const mbedtls_pk_context *key, unsigned char *buf, { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char *output_buf = NULL; - output_buf = calloc(1, PRV_DER_MAX_BYTES); + output_buf = mbedtls_calloc(1, PRV_DER_MAX_BYTES); if (output_buf == NULL) { return MBEDTLS_ERR_PK_ALLOC_FAILED; } @@ -860,7 +860,7 @@ int mbedtls_pk_write_key_pem(const mbedtls_pk_context *key, unsigned char *buf, ret = 0; cleanup: - free(output_buf); + mbedtls_free(output_buf); return ret; } #endif /* MBEDTLS_PEM_WRITE_C */ From edbab91bf81eeafecca44697c0a25242a1b5f00f Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Mon, 21 Aug 2023 15:17:43 +0800 Subject: [PATCH 6/7] pkwrite.c: write ChangeLog accurately The heap memory is used for both RSA and EC keys. So removing `RSA` in the ChangeLog. Signed-off-by: Yanray Wang --- ChangeLog.d/pkwrite-pem-use-heap.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog.d/pkwrite-pem-use-heap.txt b/ChangeLog.d/pkwrite-pem-use-heap.txt index d2e7129ec..11db7b6b0 100644 --- a/ChangeLog.d/pkwrite-pem-use-heap.txt +++ b/ChangeLog.d/pkwrite-pem-use-heap.txt @@ -1,4 +1,4 @@ Changes - * Use heap memory to allocate DER encoded RSA public/private key. + * Use heap memory to allocate DER encoded public/private key. This reduces stack usage significantly for writing a public/private key to a PEM string. From 044eb16379ffc00d984f22ce38ab49223ac059a9 Mon Sep 17 00:00:00 2001 From: Yanray Wang Date: Mon, 28 Aug 2023 10:35:39 +0800 Subject: [PATCH 7/7] pkwrite: zeroize buf containing info of private key Signed-off-by: Yanray Wang --- library/pkwrite.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/pkwrite.c b/library/pkwrite.c index 225cde90d..4e625292c 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -860,7 +860,7 @@ int mbedtls_pk_write_key_pem(const mbedtls_pk_context *key, unsigned char *buf, ret = 0; cleanup: - mbedtls_free(output_buf); + mbedtls_zeroize_and_free(output_buf, PRV_DER_MAX_BYTES); return ret; } #endif /* MBEDTLS_PEM_WRITE_C */