Mbed-TLS / mbedtls

An open source, portable, easy to use, readable and flexible TLS library, and reference implementation of the PSA Cryptography API. Releases are on a varying cadence, typically around 3 - 6 months between releases.
https://www.trustedfirmware.org/projects/mbed-tls/
Other
5.57k stars 2.61k forks source link

Do not use arrays in function parameter types #4452

Open gilles-peskine-arm opened 3 years ago

gilles-peskine-arm commented 3 years ago

Context and rationale

A few functions in the API have a parameter of array type. Array types are equivalent to pointers in function parameters as far as the C language is concerned: the array size is purely indicative. However, static analyzers including compilers don't always see it that way and may treat the array size as meaningful.

See https://github.com/ARMmbed/mbedtls/issues/4130 for why this is a problem. Popular compilers are increasingly likely to complain that mbedtls_sha512_finish should not output to a 48-byte buffer, even when it's calculating a SHA-384 hash, because its output is declared as unsigned char[64].

Proposal

Change array types in API function parameters to pointers.

Work items for 3.0

Work items for 3.x

Work items for 4.0

Since low-level crypto APIs are becoming private, only one public headers still has array parameters: ssl.h. Let's remove those.

typedef void mbedtls_ssl_export_keys_t(void *p_expkey,
                                       mbedtls_ssl_key_export_type type,
                                       const unsigned char *secret,
                                       size_t secret_len,
                                       const unsigned char client_random[32],
                                       const unsigned char server_random[32],
                                       mbedtls_tls_prf_types tls_prf_type);
int mbedtls_ssl_get_own_cid(mbedtls_ssl_context *ssl,
                            int *enabled,
                            unsigned char own_cid[MBEDTLS_SSL_CID_OUT_LEN_MAX],
                            size_t *own_cid_len);
int mbedtls_ssl_get_peer_cid(mbedtls_ssl_context *ssl,
                             int *enabled,
                             unsigned char peer_cid[MBEDTLS_SSL_CID_OUT_LEN_MAX],
                             size_t *peer_cid_len);
void mbedtls_ssl_conf_renegotiation_period(mbedtls_ssl_config *conf,
                                           const unsigned char period[8]);
gilles-peskine-arm commented 3 years ago

https://github.com/ARMmbed/mbedtls/pull/4506 has done the critical part, which is the sha512 functions. I'm leaving this task open for all the other array parameters, but this is low-importance since all the remaining ones actually do have a fixed size so they aren't causing trouble, and we may well want to stop here.