PSA_VENDOR_RSA_MAX_KEY_BITS too low and not configurable #202

New issue

Open

opened 2025-03-14 08:08:43 -05:00 by CodingMarkus · 3 comments

CodingMarkus commented

2025-03-14 08:08:43 -05:00

(Migrated from github.com)

#define PSA_VENDOR_RSA_MAX_KEY_BITS 4096u

Because of this hardcoded define in code, MbedTLS 3.6.2 currently cannot process any TLS handshake involving certificates larger than 4096 bits. I know several server and client certs that are bigger than 4096. To me this limit is not meaningful at all and also not easily alterable through an external config define, so the only way we can alter this limit is by patching the code prior to building, which should never be required.

As a bare minimum, I would expect this to be configurable, similar to

#ifdef CONFIG_PSA_VENDOR_RSA_MAX_KEY_BITS
#define PSA_VENDOR_RSA_MAX_KEY_BITS CONFIG_PSA_VENDOR_RSA_MAX_KEY_BITS
#else  
#define PSA_VENDOR_RSA_MAX_KEY_BITS 4096u
#endif

#ifndef CONFIG_PSA_VENDOR_RSA_MAX_KEY_BITS
#define PSA_VENDOR_RSA_MAX_KEY_BITS 4096u
#endif

It is configurable in MbedTLS and there the default is 8192

#if !defined(MBEDTLS_MPI_MAX_SIZE)
/*
 * Maximum size of MPIs allowed in bits and bytes for user-MPIs.
 * ( Default: 512 bytes => 4096 bits, Maximum tested: 2048 bytes => 16384 bits )
 *
 * Note: Calculations can temporarily result in larger MPIs. So the number
 * of limbs required (MBEDTLS_MPI_MAX_LIMBS) is higher.
 */
#define MBEDTLS_MPI_MAX_SIZE                              1024     /**< Maximum number of bytes for usable MPIs. */
#endif /* !MBEDTLS_MPI_MAX_SIZE */

But altering that value won't help when enabling PSA because then the TLS handshake will fail in psa_cryto_rsa.c in function mbedtls_psa_rsa_load_representation()

    /* The size of an RSA key doesn't have to be a multiple of 8. Mbed TLS
     * supports non-byte-aligned key sizes, but not well. For example,
     * mbedtls_rsa_get_len() returns the key size in bytes, not in bits. */
    bits = PSA_BYTES_TO_BITS(mbedtls_rsa_get_len(*p_rsa));
    if (bits > PSA_VENDOR_RSA_MAX_KEY_BITS) {
        status = PSA_ERROR_NOT_SUPPORTED;
        goto exit;
    }

``` #define PSA_VENDOR_RSA_MAX_KEY_BITS 4096u ``` Because of this hardcoded define in code, MbedTLS 3.6.2 currently cannot process any TLS handshake involving certificates larger than 4096 bits. I know several server and client certs that are bigger than 4096. To me this limit is not meaningful at all and also not easily alterable through an external config define, so the only way we can alter this limit is by patching the code prior to building, which should never be required. As a bare minimum, I would expect this to be configurable, similar to ``` #ifdef CONFIG_PSA_VENDOR_RSA_MAX_KEY_BITS #define PSA_VENDOR_RSA_MAX_KEY_BITS CONFIG_PSA_VENDOR_RSA_MAX_KEY_BITS #else #define PSA_VENDOR_RSA_MAX_KEY_BITS 4096u #endif ``` or ``` #ifndef CONFIG_PSA_VENDOR_RSA_MAX_KEY_BITS #define PSA_VENDOR_RSA_MAX_KEY_BITS 4096u #endif ``` It is configurable in MbedTLS and there the default is 8192 ``` #if !defined(MBEDTLS_MPI_MAX_SIZE) /* * Maximum size of MPIs allowed in bits and bytes for user-MPIs. * ( Default: 512 bytes => 4096 bits, Maximum tested: 2048 bytes => 16384 bits ) * * Note: Calculations can temporarily result in larger MPIs. So the number * of limbs required (MBEDTLS_MPI_MAX_LIMBS) is higher. */ #define MBEDTLS_MPI_MAX_SIZE 1024 /**< Maximum number of bytes for usable MPIs. */ #endif /* !MBEDTLS_MPI_MAX_SIZE */ ``` But altering that value won't help when enabling PSA because then the TLS handshake will fail in `psa_cryto_rsa.c` in function `mbedtls_psa_rsa_load_representation()` ``` /* The size of an RSA key doesn't have to be a multiple of 8. Mbed TLS * supports non-byte-aligned key sizes, but not well. For example, * mbedtls_rsa_get_len() returns the key size in bytes, not in bits. */ bits = PSA_BYTES_TO_BITS(mbedtls_rsa_get_len(*p_rsa)); if (bits > PSA_VENDOR_RSA_MAX_KEY_BITS) { status = PSA_ERROR_NOT_SUPPORTED; goto exit; } ```

gilles-peskine-arm commented

2025-03-14 11:07:29 -05:00

(Migrated from github.com)

I agree, this value should be configurable. We just never got around to making it a configuration option.

There's no exact match in the legacy API: MBEDTLS_MPI_MAX_SIZE applies to all mechanisms, including Diffie-Hellman. (If there had been, PSA_VENDOR_RSA_MAX_KEY_BITS would just have been an alias for the legacy equivalent.)

I'm not sure if PSA_VENDOR_RSA_MAX_KEY_BITS is the right name for a configuration option: it doesn't match our usual naming patterns. But option names are likely to be a bit of a mess in TF-PSA-Crypto 1 anyway, because it looks like we're heading for different naming patterns based on whether the option was inherited from Mbed TLS ≤3.6 or created after the split.

Would you want this added to the 3.6 LTS branch as well? That would be reasonable.

I agree, this value should be configurable. We just never got around to making it a configuration option. There's no exact match in the legacy API: `MBEDTLS_MPI_MAX_SIZE` applies to all mechanisms, including Diffie-Hellman. (If there had been, `PSA_VENDOR_RSA_MAX_KEY_BITS` would just have been an alias for the legacy equivalent.) I'm not sure if `PSA_VENDOR_RSA_MAX_KEY_BITS` is the right name for a configuration option: it doesn't match our usual naming patterns. But option names are likely to be a bit of a mess in TF-PSA-Crypto 1 anyway, because it looks like we're heading for different naming patterns based on whether the option was inherited from Mbed TLS ≤3.6 or created after the split. Would you want this added to the 3.6 LTS branch as well? That would be reasonable.

gilles-peskine-arm commented

2025-03-14 11:10:20 -05:00

(Migrated from github.com)

Our team's agenda is very full. Would you be willing to propose a patch? We'd want negative test cases that checks that attempts to import or generate a larger key fails. These test cases should fail, at least in one configuration, due to the enforced limit and not due to MBEDTLS_MPI_MAX_SIZE. That would be the case in the default configuration if we keep 4096 as the default value.

Our team's agenda is very full. Would you be willing to propose a patch? We'd want negative test cases that checks that attempts to import or generate a larger key fails. These test cases should fail, at least in one configuration, due to the enforced limit and not due to `MBEDTLS_MPI_MAX_SIZE`. That would be the case in the default configuration if we keep 4096 as the default value.

gilles-peskine-arm commented

2025-03-14 11:14:32 -05:00

(Migrated from github.com)

Ideally, from a security perspective there should be a different value for the different RSA operations: verify is just about memory usage and should preferably be unconstrained except via MBEDTLS_MPI_MAX_SIZE (which is more about memory usage), sign is more expensive, and generate is very expensive. But having different configurable maximum values increases the complexity, so I'm not so keen on it. But maybe at least this should be a PSA_VENDOR_RSA_MAX_KEY_PAIR_BITS, not applicable to public keys?

By the way, the default value is a compromise between allowing reasonable sizes, and limiting resource usage on low-end embedded devices (which are what this library is primarily designed for). 4096 bits was probably a better compromise in 2018 than it is in 2025. Low-end embedded devices are increasingly unlikely to do anything with RSA other than signature verification anyway. Ideally, from a security perspective there should be a different value for the different RSA operations: verify is just about memory usage and should preferably be unconstrained except via `MBEDTLS_MPI_MAX_SIZE` (which is more about memory usage), sign is more expensive, and generate is very expensive. But having different configurable maximum values increases the complexity, so I'm not so keen on it. But maybe at least this should be a `PSA_VENDOR_RSA_MAX_KEY_PAIR_BITS`, not applicable to public keys?

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-Mbed-TLS/TF-PSA-Crypto#202

No description provided.

Rows
Columns