Large key buffer allocated on stack in pk.c causes issues on constrained devices

Ashish285 commented

2025-09-24 23:08:04 -05:00

(Migrated from github.com)

Description

In drivers/builtin/src/pk.c#L727, a key buffer is allocated directly on the stack.

unsigned char data[PSA_KEY_EXPORT_MAX_SIZE];

On edge devices with limited stack memory, this can cause stack overflows or severely limit available stack space for other operations.

Suggested enhancement

Consider allocating this buffer from the heap or using a caller-provided buffer.
If a fixed buffer is required, make it configurable at compile time (e.g., MBEDTLS_PSA_KEY_BUFFER_ALLOC).

### Description In [drivers/builtin/src/pk.c#L727](https://github.com/Mbed-TLS/TF-PSA-Crypto/blob/development/drivers/builtin/src/pk.c#L727), a key buffer is allocated directly on the stack. ``` unsigned char data[PSA_KEY_EXPORT_MAX_SIZE]; ``` On edge devices with limited stack memory, this can cause stack overflows or severely limit available stack space for other operations. ### Suggested enhancement * Consider allocating this buffer from the heap or using a caller-provided buffer. * If a fixed buffer is required, make it configurable at compile time (e.g., MBEDTLS_PSA_KEY_BUFFER_ALLOC).

mpg commented

2025-09-26 04:55:11 -05:00

(Migrated from github.com)

Hi @Ashish285 and thanks for your report!

You're making a good point. When we added this buffer it was only used for ECC keys, which are always relatively small, so I think a stack buffer was appropriate at that time. Later we started using it for RSA keys as well, making the maximum size much larger, and we didn't revisit the location of this buffer.

Consider allocating this buffer from the heap or using a caller-provided buffer.

I think that's probably our best option. PK already relies on heap allocations quite heavily - until 3.6 it directly made allocations, and even now it indirectly makes a lot of them via bignum under the hood. So I don't think it makes sense to avoid a heap allocation here.

If a fixed buffer is required, make it configurable at compile time (e.g., MBEDTLS_PSA_KEY_BUFFER_ALLOC)

I don't think a fixed buffer is required. However, regarding compile-time configuration, note that if your application is only using ECC keys, you'll want to make sure support for RSA keys (PSA_WANT_KEY_TYPE_RSA_PUBLIC) is disabled in your config. This should make the size of this buffer more reasonable.

Hi @Ashish285 and thanks for your report! You're making a good point. When we added this buffer it was only used for ECC keys, which are always relatively small, so I think a stack buffer was appropriate at that time. Later we started using it for RSA keys as well, making the maximum size much larger, and we didn't revisit the location of this buffer. > Consider allocating this buffer from the heap or using a caller-provided buffer. I think that's probably our best option. PK already relies on heap allocations quite heavily - until 3.6 it directly made allocations, and even now it indirectly makes a lot of them via bignum under the hood. So I don't think it makes sense to avoid a heap allocation here. > If a fixed buffer is required, make it configurable at compile time (e.g., MBEDTLS_PSA_KEY_BUFFER_ALLOC) I don't think a fixed buffer is required. However, regarding compile-time configuration, note that if your application is only using ECC keys, you'll want to make sure support for RSA keys (`PSA_WANT_KEY_TYPE_RSA_PUBLIC`) is disabled in your config. This should make the size of this buffer more reasonable.

gilles-peskine-arm commented

2025-09-26 05:05:46 -05:00

(Migrated from github.com)

The library has users with no heap, and users with a small stack. It's hard to satisfy both...

We have a compile-time option MBEDTLS_PSA_STATIC_KEY_SLOTS which removes all uses of malloc() in the PSA key store (at the expense of more global memory consumption and a few limitations). We could generalize this option to mean “don't use malloc() in places where it's avoidable”, so that could affect PK too.

until 3.6 it directly made allocations, and even now it indirectly makes a lot of them via bignum under the hood. So I don't think it makes sense to avoid a heap allocation here.

Not when you only use ECC and all the crypto is done by a driver, which was the practical scenario that our keenest heapless user got to work.

The library has users with no heap, and users with a small stack. It's hard to satisfy both... We have a compile-time option `MBEDTLS_PSA_STATIC_KEY_SLOTS` which removes all uses of `malloc()` in the PSA key store (at the expense of more global memory consumption and a few limitations). We could generalize this option to mean “don't use `malloc()` in places where it's avoidable”, so that could affect PK too. > until 3.6 it directly made allocations, and even now it indirectly makes a lot of them via bignum under the hood. So I don't think it makes sense to avoid a heap allocation here. Not when you only use ECC and all the crypto is done by a driver, which was the practical scenario that our keenest heapless user got to work.

gilles-peskine-arm commented

2025-09-26 05:07:51 -05:00

(Migrated from github.com)

We're overdue for adding PQC support. It's likely that PK will start supporting ML-DSA in 2026, and you really don't want an ML-DSA public key on the stack. Or even just to consume the size of the largest ML-DSA key in all PK objects even when you happen to be doing ECC. So that will be another reason to make the buffer heap-allocated, at least in most configurations.

mpg commented

2025-09-26 05:31:13 -05:00

(Migrated from github.com)

I was on the fence treating that as a bug or even regression from 3.6 to 1.0: some use cases worked in 3.6 and no longer work in 1.0 (and that was not intentional, unlike all the removals).

mpg commented

2025-10-02 02:10:15 -05:00

(Migrated from github.com)

Sharing some thoughts I had about this.

One approach could be to stop storing public keys serialized in the PK context, but instead keep them in the PSA key store like we do for private keys. The primary motivation for not using the key store was that is used to not be dynamic and some use cases (loading 100+ X.509 trusted roots) would make it full - that's no longer an issue now. However there was a secondary motivation which is that a serialized key has no policy, which allows using whatever algorithm we want with the public key. If we were storing public keys in the PSA key store, we'd has to be prepared to export and re-import them with a different alg in some cases, which is only moving the problem of the buffer to a different place.

Another approach we could take is to keep storing public keys serialized in the PK context, but make the buffer static only when ECC is the only key type, and dynamic otherwise. Something like

#if !defined(PSA_WANT_KEY_TYPE_RSA_PUBLIC) // && !defined(PSA_WANT_KEY_TYPE_ML_DSA_PUBLIC) in the future
#define MBEDTLS_PK_STATIC_PUB_RAW
#endif

typedef struct mbedtls_pk_context {
    // [...]
#if defined(MBEDTLS_PK_STATIC_PUB_RAW)
    uint8_t MBEDTLS_PRIVATE(pub_raw)[MBEDTLS_PK_MAX_PUBKEY_RAW_LEN];
#else
    uint8_t *MBEDTLS_PRIVATE(pub_raw);
#endif
    size_t MBEDTLS_PRIVATE(pub_raw_len);
} mbedtls_pk_context;

I think this could give users the best of both worlds at a reasonable cost.

Thoughts?

Sharing some thoughts I had about this. One approach could be to stop storing public keys serialized in the PK context, but instead keep them in the PSA key store like we do for private keys. The primary motivation for not using the key store was that is used to not be dynamic and some use cases (loading 100+ X.509 trusted roots) would make it full - that's no longer an issue now. However there was a secondary motivation which is that a serialized key has no policy, which allows using whatever algorithm we want with the public key. If we were storing public keys in the PSA key store, we'd has to be prepared to export and re-import them with a different alg in some cases, which is only moving the problem of the buffer to a different place. Another approach we could take is to keep storing public keys serialized in the PK context, but make the buffer static only when ECC is the only key type, and dynamic otherwise. Something like ```c #if !defined(PSA_WANT_KEY_TYPE_RSA_PUBLIC) // && !defined(PSA_WANT_KEY_TYPE_ML_DSA_PUBLIC) in the future #define MBEDTLS_PK_STATIC_PUB_RAW #endif typedef struct mbedtls_pk_context { // [...] #if defined(MBEDTLS_PK_STATIC_PUB_RAW) uint8_t MBEDTLS_PRIVATE(pub_raw)[MBEDTLS_PK_MAX_PUBKEY_RAW_LEN]; #else uint8_t *MBEDTLS_PRIVATE(pub_raw); #endif size_t MBEDTLS_PRIVATE(pub_raw_len); } mbedtls_pk_context; ``` I think this could give users the best of both worlds at a reasonable cost. Thoughts?

mpg commented

2025-10-03 02:17:09 -05:00

(Migrated from github.com)

As discussed elsewhere, we're handling this as a bug, but not a severe one. We'll try to fix in 1.1 but are not making promises.

mpg commented

2026-01-27 05:33:11 -06:00

(Migrated from github.com)

As was pointed out in private discussions, there are multiple buffers being discussed here:

The original report is about the PSA_KEY_EXPORT_MAX_SIZE-sized buffer in export_import_into_psa() (permanent link, used by the public function mbedtls_pk_import_into_psa().
I didn't pay close enough attention to the original report and thought it was about the buffer that jumped to my mind: the MBEDTLS_PK_MAX_PUBKEY_RAW_LEN-size buffer in the structure mbedtls_pk_context (permanent link) so my comments were about that one.

Sorry for the confusion.

Moving forward, I'm trying to get a complete list of large stack buffers we a have in PK. I used two complementary approaches, comparing their results. The first relies on GCC's -fstack-size option:

% gcc -Iinclude -Icore -I drivers/builtin/include -Os -fstack-usage -c drivers/builtin/src/pk*.c
% egrep -h '[^:]([0-9]{4}|[5-9][0-9]{2})' pk*.su | awk '{print $2" "$1}' | sort -rn
2480 drivers/builtin/src/pk.c:909:12:copy_from_psa
2432 drivers/builtin/src/pk.c:717:21:export_import_into_psa
2416 drivers/builtin/src/pkwrite.c:50:12:pk_write_rsa_der.isra
1136 drivers/builtin/src/pk.c:820:12:import_public_into_psa.constprop
1088 drivers/builtin/src/pkwrite.c:111:12:pk_write_ec_pubkey
576 drivers/builtin/src/pkwrite.c:71:12:pk_write_rsa_pubkey
576 drivers/builtin/src/pk_wrap.c:132:12:rsa_check_pair_wrap
528 drivers/builtin/src/pkparse.c:381:12:pk_use_ecparams

The second approach is based on looking for patterns in the source code:

% egrep -n '\[(PSA.*KEY.*MAX|.*PK_MAX)' drivers/builtin/src/pk* include/mbedtls/**/pk*.h
drivers/builtin/src/pk.c:721:    unsigned char key_buffer[PSA_EXPORT_KEY_PAIR_MAX_SIZE];
drivers/builtin/src/pk.c:825:    unsigned char key_buffer[PSA_EXPORT_PUBLIC_KEY_MAX_SIZE];
drivers/builtin/src/pk.c:918:    unsigned char exp_key[PSA_EXPORT_KEY_PAIR_OR_PUBLIC_MAX_SIZE];
drivers/builtin/src/pk_wrap.c:135:    uint8_t exp_pub_key[MBEDTLS_PK_MAX_RSA_PUBKEY_RAW_LEN];
drivers/builtin/src/pk_wrap.c:249:    unsigned char key[MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN];
drivers/builtin/src/pkwrite.c:53:    uint8_t tmp[PSA_EXPORT_KEY_PAIR_MAX_SIZE];
drivers/builtin/src/pkwrite.c:74:    unsigned char tmp_key[MBEDTLS_PK_MAX_RSA_PUBKEY_RAW_LEN];
drivers/builtin/src/pkwrite.c:115:    uint8_t buf[PK_MAX_EC_PUBLIC_KEY_SIZE];
drivers/builtin/src/pkwrite.c:144:    unsigned char tmp[PK_MAX_EC_KEY_PAIR_SIZE];
include/mbedtls/pk.h:151:    uint8_t MBEDTLS_PRIVATE(pub_raw)[MBEDTLS_PK_MAX_PUBKEY_RAW_LEN];

The results are quite consistent. The source-based approach finds a few buffers that are smaller than the threeshold used of the compiler-based approach, and also finds the buffer in the structure (the one I had in mind). The compiler-based approach identifies pk_use_ecparams() as using a lot of stack space, but looking at the source code, that doesn't seem to be related to a large buffer for key material, so I'm excluding this function from further analysis (also, I strongly suspect this function stack usage can be brought down by disabling MBEDTLS_PK_PARSE_EC_EXTENDED).

As was pointed out in private discussions, there are multiple buffers being discussed here: 1. The original report is about the `PSA_KEY_EXPORT_MAX_SIZE`-sized buffer in `export_import_into_psa()` ([permanent link](https://github.com/Mbed-TLS/TF-PSA-Crypto/blob/29cc9f70b8e5c92f2bf47a6a29519c0d7376a469/drivers/builtin/src/pk.c#L721), used by the public function `mbedtls_pk_import_into_psa()`. 2. I didn't pay close enough attention to the original report and thought it was about the buffer that jumped to my mind: the `MBEDTLS_PK_MAX_PUBKEY_RAW_LEN`-size buffer in the structure `mbedtls_pk_context` ([permanent link](https://github.com/Mbed-TLS/TF-PSA-Crypto/blob/29cc9f70b8e5c92f2bf47a6a29519c0d7376a469/include/mbedtls/pk.h#L151)) so my comments were about that one. Sorry for the confusion. Moving forward, I'm trying to get a complete list of large stack buffers we a have in PK. I used two complementary approaches, comparing their results. The first relies on GCC's `-fstack-size` option: ``` % gcc -Iinclude -Icore -I drivers/builtin/include -Os -fstack-usage -c drivers/builtin/src/pk*.c % egrep -h '[^:]([0-9]{4}|[5-9][0-9]{2})' pk*.su | awk '{print $2" "$1}' | sort -rn 2480 drivers/builtin/src/pk.c:909:12:copy_from_psa 2432 drivers/builtin/src/pk.c:717:21:export_import_into_psa 2416 drivers/builtin/src/pkwrite.c:50:12:pk_write_rsa_der.isra 1136 drivers/builtin/src/pk.c:820:12:import_public_into_psa.constprop 1088 drivers/builtin/src/pkwrite.c:111:12:pk_write_ec_pubkey 576 drivers/builtin/src/pkwrite.c:71:12:pk_write_rsa_pubkey 576 drivers/builtin/src/pk_wrap.c:132:12:rsa_check_pair_wrap 528 drivers/builtin/src/pkparse.c:381:12:pk_use_ecparams ``` The second approach is based on looking for patterns in the source code: ``` % egrep -n '\[(PSA.*KEY.*MAX|.*PK_MAX)' drivers/builtin/src/pk* include/mbedtls/**/pk*.h drivers/builtin/src/pk.c:721: unsigned char key_buffer[PSA_EXPORT_KEY_PAIR_MAX_SIZE]; drivers/builtin/src/pk.c:825: unsigned char key_buffer[PSA_EXPORT_PUBLIC_KEY_MAX_SIZE]; drivers/builtin/src/pk.c:918: unsigned char exp_key[PSA_EXPORT_KEY_PAIR_OR_PUBLIC_MAX_SIZE]; drivers/builtin/src/pk_wrap.c:135: uint8_t exp_pub_key[MBEDTLS_PK_MAX_RSA_PUBKEY_RAW_LEN]; drivers/builtin/src/pk_wrap.c:249: unsigned char key[MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN]; drivers/builtin/src/pkwrite.c:53: uint8_t tmp[PSA_EXPORT_KEY_PAIR_MAX_SIZE]; drivers/builtin/src/pkwrite.c:74: unsigned char tmp_key[MBEDTLS_PK_MAX_RSA_PUBKEY_RAW_LEN]; drivers/builtin/src/pkwrite.c:115: uint8_t buf[PK_MAX_EC_PUBLIC_KEY_SIZE]; drivers/builtin/src/pkwrite.c:144: unsigned char tmp[PK_MAX_EC_KEY_PAIR_SIZE]; include/mbedtls/pk.h:151: uint8_t MBEDTLS_PRIVATE(pub_raw)[MBEDTLS_PK_MAX_PUBKEY_RAW_LEN]; ``` The results are quite consistent. The source-based approach finds a few buffers that are smaller than the threeshold used of the compiler-based approach, and also finds the buffer in the structure (the one I had in mind). The compiler-based approach identifies `pk_use_ecparams()` as using a lot of stack space, but looking at the source code, that doesn't seem to be related to a large buffer for key material, so I'm excluding this function from further analysis (also, I strongly suspect this function stack usage can be brought down by disabling `MBEDTLS_PK_PARSE_EC_EXTENDED`).

mpg commented

2026-01-27 06:00:25 -06:00

(Migrated from github.com)

Here's a more detailed analysis of the large buffers for key material identified by the source-based approach, ranked from largest to smallest. (For reference, line numbers are in today's development, which is 29cc9f70b8.)

2363 all-types keypair

drivers/builtin/src/pk.c:721:    unsigned char key_buffer[PSA_EXPORT_KEY_PAIR_MAX_SIZE];
    export_import_into_psa() -> copy_into_psa() -> import_pair_into_psa() ->
    mbedtls_pk_import_into_psa()

The one originally reported.

2363 all-types keypair or public

drivers/builtin/src/pk.c:918:    unsigned char exp_key[PSA_EXPORT_KEY_PAIR_OR_PUBLIC_MAX_SIZE];
   copy_from_psa() ->
   mbedtls_pk_copy_from_psa(), mbedtls_pk_copy_public_from_psa()

Very similar to the previous one.

2363 RSA keypair

drivers/builtin/src/pkwrite.c:53:    uint8_t tmp[PSA_EXPORT_KEY_PAIR_MAX_SIZE];
    pk_write_rsa_der() ->
    mbedtls_pk_write_key_der()

Note: could write directly to the output buffer I think. Edit: being done in https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/684

528 ~~1024~~ all-types public ~~(wrongly includes FFDH, should be 528)~~

drivers/builtin/src/pk.c:825:    unsigned char key_buffer[PSA_EXPORT_PUBLIC_KEY_MAX_SIZE];
    import_public_into_psa() ->
    mbedtls_pk_import_into_psa()

Note: useless now, could directly access pk->pub_raw instead, see https://github.com/Mbed-TLS/mbedtls/issues/10583. Edit: in the meantime, size brought down to 528 by #679

133 ~~1024~~ EC public ~~(wrongly includes RSA and FFDH, should be 133)~~

drivers/builtin/src/pkwrite.c:115:    uint8_t buf[PK_MAX_EC_PUBLIC_KEY_SIZE];
    pk_write_ec_pubkey() -> pk_write_ec_der() ->
    mbedtls_pk_write_pubkey(), mbedtls_pk_write_key_der()

Note: useless now, could directly access pk->pub_raw instead, see https://github.com/Mbed-TLS/mbedtls/issues/10583.
Edit: in the meantime, size brought down to 133 by #679

528 all-types public

include/mbedtls/pk.h:151:    uint8_t MBEDTLS_PRIVATE(pub_raw)[MBEDTLS_PK_MAX_PUBKEY_RAW_LEN];

Structure, used in a number of places. (The one I had in mind.)

~~528 RSA public~~

drivers/builtin/src/pk_wrap.c:135:    uint8_t exp_pub_key[MBEDTLS_PK_MAX_RSA_PUBKEY_RAW_LEN];
    rsa_check_pair_wrap ->
    mbedtls_pk_check_pair

Note: being removed by #580 - edit: merged, so this buffer is now gone.

528 RSA public

drivers/builtin/src/pkwrite.c:74:    unsigned char tmp_key[MBEDTLS_PK_MAX_RSA_PUBKEY_RAW_LEN];
    pk_write_rsa_pubkey() ->
    mbedtls_pk_write_pubkey()

Note: useless now, could directly access pk->pub_raw instead, see https://github.com/Mbed-TLS/mbedtls/issues/10583.

133 ECC public

drivers/builtin/src/pk_wrap.c:249:    unsigned char key[MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN];
    ecdsa_opaque_verify_wrap() ->
    mbedtls_pk_verify(), mbedtls_pk_verify_ext()

Note: useless now, could directly access pk->pub_raw instead, see https://github.com/Mbed-TLS/mbedtls/issues/10583.

66 EC keypair

drivers/builtin/src/pkwrite.c:144:    unsigned char tmp[PK_MAX_EC_KEY_PAIR_SIZE];
    pk_write_ec_private() -> pk_write_ec_rfc8410_der(), pk_write_ec_der() ->
    mbedtls_pk_write_key_der()

Note: could possibly write directly to the output buffer?

Clearly the last two are harmless and will be excluded from further analysis.

Just looking at the sizes, there is a large gap between the first three (2368 bytes, able to hold an RSA keypair) and the next ones, which should be 528 bytes (able to hold an RSA public key (or an ECC public key or keypair)) if we hadn't made silly mistakes while defining our macros.

Among the first three (2368 bytes), I'd say the first two are more worrying than the third one: that's because they must cover all types supported by PK, which means their size will grow when we add support for ML-DSA (whose key sizes go up to 4896 for private keys, 2592 for public keys). By contrast, the 3rd one is purely about RSA, which means it isn't growing when adding support for ML-DSA (or other PQ schemes), and the price is only paid when you actually use RSA keys.

Then in the 528-bytes group (assuming we fix the silly mistakes in macro definitions), we can make the same distinction. Further, we can disregard those that could just directly access pub_raw instead - which quite logically would leave us with the pub_raw field in the mbedtls_pk_context structure as the only one in that category.

I'll think about possible solutions and come back later.

Here's a more detailed analysis of the large buffers for key material identified by the source-based approach, ranked from largest to smallest. (For reference, line numbers are in today's development, which is 29cc9f70b8e5c92f2bf47a6a29519c0d7376a469.) **2363 all-types keypair** ``` drivers/builtin/src/pk.c:721: unsigned char key_buffer[PSA_EXPORT_KEY_PAIR_MAX_SIZE]; export_import_into_psa() -> copy_into_psa() -> import_pair_into_psa() -> mbedtls_pk_import_into_psa() ``` The one originally reported. **2363 all-types keypair or public** ``` drivers/builtin/src/pk.c:918: unsigned char exp_key[PSA_EXPORT_KEY_PAIR_OR_PUBLIC_MAX_SIZE]; copy_from_psa() -> mbedtls_pk_copy_from_psa(), mbedtls_pk_copy_public_from_psa() ``` Very similar to the previous one. **2363 RSA keypair** ``` drivers/builtin/src/pkwrite.c:53: uint8_t tmp[PSA_EXPORT_KEY_PAIR_MAX_SIZE]; pk_write_rsa_der() -> mbedtls_pk_write_key_der() ``` Note: could write directly to the output buffer I think. Edit: being done in https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/684 **528 ~1024~ all-types public ~(wrongly includes FFDH, should be 528)~** ``` drivers/builtin/src/pk.c:825: unsigned char key_buffer[PSA_EXPORT_PUBLIC_KEY_MAX_SIZE]; import_public_into_psa() -> mbedtls_pk_import_into_psa() ``` Note: useless now, could directly access `pk->pub_raw` instead, see https://github.com/Mbed-TLS/mbedtls/issues/10583. Edit: in the meantime, size brought down to 528 by #679 **133 ~1024~ EC public ~(wrongly includes RSA and FFDH, should be 133)~** ``` drivers/builtin/src/pkwrite.c:115: uint8_t buf[PK_MAX_EC_PUBLIC_KEY_SIZE]; pk_write_ec_pubkey() -> pk_write_ec_der() -> mbedtls_pk_write_pubkey(), mbedtls_pk_write_key_der() ``` Note: useless now, could directly access `pk->pub_raw` instead, see https://github.com/Mbed-TLS/mbedtls/issues/10583. Edit: in the meantime, size brought down to 133 by #679 **528 all-types public** ``` include/mbedtls/pk.h:151: uint8_t MBEDTLS_PRIVATE(pub_raw)[MBEDTLS_PK_MAX_PUBKEY_RAW_LEN]; ``` Structure, used in a number of places. (The one I had in mind.) **~~528 RSA public~~** ``` drivers/builtin/src/pk_wrap.c:135: uint8_t exp_pub_key[MBEDTLS_PK_MAX_RSA_PUBKEY_RAW_LEN]; rsa_check_pair_wrap -> mbedtls_pk_check_pair ``` Note: being removed by #580 - edit: merged, so this buffer is now gone. **528 RSA public** ``` drivers/builtin/src/pkwrite.c:74: unsigned char tmp_key[MBEDTLS_PK_MAX_RSA_PUBKEY_RAW_LEN]; pk_write_rsa_pubkey() -> mbedtls_pk_write_pubkey() ``` Note: useless now, could directly access `pk->pub_raw` instead, see https://github.com/Mbed-TLS/mbedtls/issues/10583. **133 ECC public** ``` drivers/builtin/src/pk_wrap.c:249: unsigned char key[MBEDTLS_PK_MAX_EC_PUBKEY_RAW_LEN]; ecdsa_opaque_verify_wrap() -> mbedtls_pk_verify(), mbedtls_pk_verify_ext() ``` Note: useless now, could directly access `pk->pub_raw` instead, see https://github.com/Mbed-TLS/mbedtls/issues/10583. **66 EC keypair** ``` drivers/builtin/src/pkwrite.c:144: unsigned char tmp[PK_MAX_EC_KEY_PAIR_SIZE]; pk_write_ec_private() -> pk_write_ec_rfc8410_der(), pk_write_ec_der() -> mbedtls_pk_write_key_der() ``` Note: could possibly write directly to the output buffer? Clearly the last two are harmless and will be excluded from further analysis. Just looking at the sizes, there is a large gap between the first three (2368 bytes, able to hold an RSA keypair) and the next ones, which should be 528 bytes (able to hold an RSA public key (or an ECC public key or keypair)) if we hadn't made silly mistakes while defining our macros. Among the first three (2368 bytes), I'd say the first two are more worrying than the third one: that's because they must cover all types supported by PK, which means their size will grow when we add support for ML-DSA (whose [key sizes](https://nvlpubs.nist.gov/nistpubs/fips/nist.fips.204.pdf#page=26) go up to 4896 for private keys, 2592 for public keys). By contrast, the 3rd one is purely about RSA, which means it isn't growing when adding support for ML-DSA (or other PQ schemes), and the price is only paid when you actually use RSA keys. Then in the 528-bytes group (assuming we fix the silly mistakes in macro definitions), we can make the same distinction. Further, we can disregard those that could just directly access `pub_raw` instead - which quite logically would leave us with the `pub_raw` field in the `mbedtls_pk_context` structure as the only one in that category. I'll think about possible solutions and come back later.

mpg commented

2026-02-05 05:25:55 -06:00

(Migrated from github.com)

I'll think about possible solutions and come back later.

Most of the large stack buffers identified are now being addressed by other PRs or other issues - I've edited the previous comment with links. The only remaining ones are:

The two 2363-byte buffers in mbedtls_pk_import_into_psa(), mbedtls_pk_write_pubkey(), mbedtls_pk_write_key_der() which are very similar in nature.
The 528-byte buffer in the mbedtls_pk_context structure.

While (2) will be problematic in the future (when we add support for ML-DSA with its large public keys), I suggest we consider it out of scope for the present issue, and consider it resolved when the last 2k+ stack buffers are removed.

> I'll think about possible solutions and come back later. Most of the large stack buffers identified are now being addressed by other PRs or other issues - I've edited the previous comment with links. The only remaining ones are: 1. The two 2363-byte buffers in `mbedtls_pk_import_into_psa()`, `mbedtls_pk_write_pubkey()`, `mbedtls_pk_write_key_der()` which are very similar in nature. 2. The 528-byte buffer in the `mbedtls_pk_context` structure. While (2) will be problematic in the future (when we add support for ML-DSA with its large public keys), I suggest we consider it out of scope for the present issue, and consider it resolved when the last 2k+ stack buffers are removed.

mpg commented

2026-02-10 03:57:56 -06:00

(Migrated from github.com)

I'll think about possible solutions and come back later.

I found it easier to express the proposal as a patch than in English, so here it is: ~~#684~~ edit: #687

The library has users with no heap, and users with a small stack. It's hard to satisfy both...

[me wrongly saying PK requires a heap anyway]
Not when you only use ECC and all the crypto is done by a driver, which was the practical scenario that our keenest heapless user got to work.

So, my proposal preserves the fact that we're not using the heap when ECC is the only key type enabled in PK. We only use the heap when RSA keys are enabled (and later ML-DSA of course). We never officially supported driver-only RSA (though it should mostly work by now) so I don't think using the heap for RSA in PK is a regression.

@gilles-peskine-arm I'd appreciate if you could take a look at the design of ~~#684~~ edit: #687.

> I'll think about possible solutions and come back later. I found it easier to express the proposal as a patch than in English, so here it is: ~#684~ edit: #687 > The library has users with no heap, and users with a small stack. It's hard to satisfy both... > > > [me wrongly saying PK requires a heap anyway] > Not when you only use ECC and all the crypto is done by a driver, which was the practical scenario that our keenest heapless user got to work. So, my proposal preserves the fact that we're not using the heap when ECC is the only key type enabled in PK. We only use the heap when RSA keys are enabled (and later ML-DSA of course). We never officially supported driver-only RSA (though it should mostly work by now) so I don't think using the heap for RSA in PK is a regression. @gilles-peskine-arm I'd appreciate if you could take a look at the design of ~#684~ edit: #687.

Rows
Columns

Large key buffer allocated on stack in pk.c causes issues on constrained devices #476

Description

Suggested enhancement