From 217416a76e2505e8af6f384fb2b413f9df8f7508 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 88e685503b..7253c6ebfe 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -571,23 +571,30 @@ end_of_export: int mbedtls_pk_write_pubkey_pem(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; PK_VALIDATE_RET(key != NULL); PK_VALIDATE_RET(buf != NULL || size == 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 7bbca1363f7d507d9c928af9aa1a4b91386ef04a 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 7253c6ebfe..0dc61cdc45 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -601,14 +601,19 @@ int mbedtls_pk_write_pubkey_pem(mbedtls_pk_context *key, unsigned char *buf, siz int mbedtls_pk_write_key_pem(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; PK_VALIDATE_RET(key != NULL); PK_VALIDATE_RET(buf != NULL || size == 0); - 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; } @@ -624,14 +629,19 @@ int mbedtls_pk_write_key_pem(mbedtls_pk_context *key, unsigned char *buf, size_t end = PEM_END_PRIVATE_KEY_EC; } else #endif - 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 a8f00508fe63b431354a960b2df944f9fad0b845 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 0dc61cdc45..8bbd40e870 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -583,19 +583,19 @@ int mbedtls_pk_write_pubkey_pem(mbedtls_pk_context *key, unsigned char *buf, siz 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(mbedtls_pk_context *key, unsigned char *buf, size_t size) @@ -613,8 +613,7 @@ int mbedtls_pk_write_key_pem(mbedtls_pk_context *key, unsigned char *buf, size_t PK_VALIDATE_RET(buf != NULL || size == 0); 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_RSA_C) @@ -630,19 +629,20 @@ int mbedtls_pk_write_key_pem(mbedtls_pk_context *key, unsigned char *buf, size_t } else #endif { - 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 79873bcf562bf58cbd4486eb19c8a746780133bf 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 0000000000..d2e7129ec4 --- /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 b59b7c643b3ff368e0040e96e9e98ae43c0ab29f 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 8bbd40e870..f0dc718f35 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -572,7 +572,7 @@ int mbedtls_pk_write_pubkey_pem(mbedtls_pk_context *key, unsigned char *buf, siz { 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; } @@ -594,7 +594,7 @@ int mbedtls_pk_write_pubkey_pem(mbedtls_pk_context *key, unsigned char *buf, siz ret = 0; cleanup: - free(output_buf); + mbedtls_free(output_buf); return ret; } @@ -602,7 +602,7 @@ int mbedtls_pk_write_key_pem(mbedtls_pk_context *key, unsigned char *buf, size_t { 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; } @@ -641,7 +641,7 @@ int mbedtls_pk_write_key_pem(mbedtls_pk_context *key, unsigned char *buf, size_t ret = 0; cleanup: - free(output_buf); + mbedtls_free(output_buf); return ret; } #endif /* MBEDTLS_PEM_WRITE_C */ From c9d5ea9a9c37e3abbe89a6df5f4bedb6ca1119a7 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 d2e7129ec4..11db7b6b06 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 4b0b97e18bc6bb3ac2e5be0756cc1493d4b9af53 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 | 1 + 1 file changed, 1 insertion(+) diff --git a/library/pkwrite.c b/library/pkwrite.c index f0dc718f35..5e3fcc9ef8 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -641,6 +641,7 @@ int mbedtls_pk_write_key_pem(mbedtls_pk_context *key, unsigned char *buf, size_t ret = 0; cleanup: + mbedtls_platform_zeroize(output_buf, PRV_DER_MAX_BYTES); mbedtls_free(output_buf); return ret; }