From 2630f6720df59a2084220b1c06f4730a0a330ea0 Mon Sep 17 00:00:00 2001 From: Rodrigo Dias Correa Date: Wed, 4 Nov 2020 01:55:38 -0300 Subject: [PATCH 1/7] Fix build failure on gcc-11 Function prototypes changed to use array parameters instead of pointers. Signed-off-by: Rodrigo Dias Correa --- library/ssl_tls.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 041578e68..ec890d7dc 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -680,20 +680,20 @@ static void ssl_calc_finished_ssl( mbedtls_ssl_context *, unsigned char *, int ) #endif #if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) -static void ssl_calc_verify_tls( const mbedtls_ssl_context *, unsigned char *, size_t * ); +static void ssl_calc_verify_tls( const mbedtls_ssl_context *, unsigned char [36], size_t * ); static void ssl_calc_finished_tls( mbedtls_ssl_context *, unsigned char *, int ); #endif #if defined(MBEDTLS_SSL_PROTO_TLS1_2) #if defined(MBEDTLS_SHA256_C) static void ssl_update_checksum_sha256( mbedtls_ssl_context *, const unsigned char *, size_t ); -static void ssl_calc_verify_tls_sha256( const mbedtls_ssl_context *,unsigned char *, size_t * ); +static void ssl_calc_verify_tls_sha256( const mbedtls_ssl_context *,unsigned char [32], size_t * ); static void ssl_calc_finished_tls_sha256( mbedtls_ssl_context *,unsigned char *, int ); #endif #if defined(MBEDTLS_SHA512_C) static void ssl_update_checksum_sha384( mbedtls_ssl_context *, const unsigned char *, size_t ); -static void ssl_calc_verify_tls_sha384( const mbedtls_ssl_context *, unsigned char *, size_t * ); +static void ssl_calc_verify_tls_sha384( const mbedtls_ssl_context *, unsigned char [48], size_t * ); static void ssl_calc_finished_tls_sha384( mbedtls_ssl_context *, unsigned char *, int ); #endif #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ From 2c424570e2a85e21273313808e8c5efcf1cfc9d7 Mon Sep 17 00:00:00 2001 From: Rodrigo Dias Correa Date: Tue, 10 Nov 2020 01:38:00 -0300 Subject: [PATCH 2/7] Fix mismatched function parameters (prototype/definition) In GCC 11, parameters declared as arrays in function prototypes cannot be declared as pointers in the function definition. The same is true for the other way around. The definition of `mbedtls_aes_cmac_prf_128` was changed to match its public prototype in `cmac.h`. The type `output` was `unsigned char *`, now is `unsigned char [16]`. In `ssl_tls.c`, all the `ssl_calc_verify_*` variants now use pointers for the output `hash` parameter. The array parameters were removed because those functions must be compatible with the function pointer `calc_verify` (defined in `ssl_internal.h`). Signed-off-by: Rodrigo Dias Correa --- library/cmac.c | 2 +- library/ssl_tls.c | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/library/cmac.c b/library/cmac.c index 816bf13da..59ece155e 100644 --- a/library/cmac.c +++ b/library/cmac.c @@ -420,7 +420,7 @@ exit: */ int mbedtls_aes_cmac_prf_128( const unsigned char *key, size_t key_length, const unsigned char *input, size_t in_len, - unsigned char *output ) + unsigned char output[16] ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const mbedtls_cipher_info_t *cipher_info; diff --git a/library/ssl_tls.c b/library/ssl_tls.c index ec890d7dc..9d4c46228 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -680,20 +680,20 @@ static void ssl_calc_finished_ssl( mbedtls_ssl_context *, unsigned char *, int ) #endif #if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) -static void ssl_calc_verify_tls( const mbedtls_ssl_context *, unsigned char [36], size_t * ); +static void ssl_calc_verify_tls( const mbedtls_ssl_context *, unsigned char*, size_t * ); static void ssl_calc_finished_tls( mbedtls_ssl_context *, unsigned char *, int ); #endif #if defined(MBEDTLS_SSL_PROTO_TLS1_2) #if defined(MBEDTLS_SHA256_C) static void ssl_update_checksum_sha256( mbedtls_ssl_context *, const unsigned char *, size_t ); -static void ssl_calc_verify_tls_sha256( const mbedtls_ssl_context *,unsigned char [32], size_t * ); +static void ssl_calc_verify_tls_sha256( const mbedtls_ssl_context *,unsigned char*, size_t * ); static void ssl_calc_finished_tls_sha256( mbedtls_ssl_context *,unsigned char *, int ); #endif #if defined(MBEDTLS_SHA512_C) static void ssl_update_checksum_sha384( mbedtls_ssl_context *, const unsigned char *, size_t ); -static void ssl_calc_verify_tls_sha384( const mbedtls_ssl_context *, unsigned char [48], size_t * ); +static void ssl_calc_verify_tls_sha384( const mbedtls_ssl_context *, unsigned char*, size_t * ); static void ssl_calc_finished_tls_sha384( mbedtls_ssl_context *, unsigned char *, int ); #endif #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ @@ -1667,7 +1667,7 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_PROTO_SSL3) void ssl_calc_verify_ssl( const mbedtls_ssl_context *ssl, - unsigned char hash[36], + unsigned char *hash, size_t *hlen ) { mbedtls_md5_context md5; @@ -1720,7 +1720,7 @@ void ssl_calc_verify_ssl( const mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) void ssl_calc_verify_tls( const mbedtls_ssl_context *ssl, - unsigned char hash[36], + unsigned char *hash, size_t *hlen ) { mbedtls_md5_context md5; @@ -1752,7 +1752,7 @@ void ssl_calc_verify_tls( const mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_PROTO_TLS1_2) #if defined(MBEDTLS_SHA256_C) void ssl_calc_verify_tls_sha256( const mbedtls_ssl_context *ssl, - unsigned char hash[32], + unsigned char *hash, size_t *hlen ) { #if defined(MBEDTLS_USE_PSA_CRYPTO) @@ -1801,7 +1801,7 @@ void ssl_calc_verify_tls_sha256( const mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SHA512_C) void ssl_calc_verify_tls_sha384( const mbedtls_ssl_context *ssl, - unsigned char hash[48], + unsigned char *hash, size_t *hlen ) { #if defined(MBEDTLS_USE_PSA_CRYPTO) From 80448aae2c93b7bfe73a05410196e237ef2dd8e1 Mon Sep 17 00:00:00 2001 From: Rodrigo Dias Correa Date: Tue, 10 Nov 2020 02:28:50 -0300 Subject: [PATCH 3/7] Fix GCC warning about `test_snprintf` GCC 11 generated the warnings because the parameter `ret_buf` was declared as `const char[10]`, but some of the arguments provided in `run_test_snprintf` are shorter literals, like "". Now the type of `ret_buf` is `const char *`. Both implementations of `test_snprintf` were fixed. Signed-off-by: Rodrigo Dias Correa --- programs/test/selftest.c | 2 +- tests/suites/host_test.function | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/programs/test/selftest.c b/programs/test/selftest.c index 2aa379b1c..41d704073 100644 --- a/programs/test/selftest.c +++ b/programs/test/selftest.c @@ -158,7 +158,7 @@ static int calloc_self_test( int verbose ) } #endif /* MBEDTLS_SELF_TEST */ -static int test_snprintf( size_t n, const char ref_buf[10], int ref_ret ) +static int test_snprintf( size_t n, const char *ref_buf, int ref_ret ) { int ret; char buf[10] = "xxxxxxxxx"; diff --git a/tests/suites/host_test.function b/tests/suites/host_test.function index db53e9784..872a3a43a 100644 --- a/tests/suites/host_test.function +++ b/tests/suites/host_test.function @@ -335,7 +335,7 @@ static int convert_params( size_t cnt , char ** params , int * int_params_store #if defined(__GNUC__) __attribute__((__noinline__)) #endif -static int test_snprintf( size_t n, const char ref_buf[10], int ref_ret ) +static int test_snprintf( size_t n, const char *ref_buf, int ref_ret ) { int ret; char buf[10] = "xxxxxxxxx"; From eb5d014d8ec82cfebfdd008d890d0e1a114c2abc Mon Sep 17 00:00:00 2001 From: Rodrigo Dias Correa Date: Tue, 10 Nov 2020 02:51:51 -0300 Subject: [PATCH 4/7] Fix GCC warning in `ssl_calc_finished_tls_sha384` GCC 11 generated a warning because `padbuf` was too small to be used as an argument for `mbedtls_sha512_finish_ret`. The `output` parameter of `mbedtls_sha512_finish_ret` has the type `unsigned char[64]`, but `padbuf` was only 48 bytes long. Even though `ssl_calc_finished_tls_sha384` uses only 48 bytes for the hash output, the size of `padbuf` was increased to 64 bytes. Signed-off-by: Rodrigo Dias Correa --- library/ssl_tls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 9d4c46228..c69de3f1d 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3202,7 +3202,7 @@ static void ssl_calc_finished_tls_sha384( { int len = 12; const char *sender; - unsigned char padbuf[48]; + unsigned char padbuf[64]; #if defined(MBEDTLS_USE_PSA_CRYPTO) size_t hash_size; psa_hash_operation_t sha384_psa = PSA_HASH_OPERATION_INIT; From 683028a2f721980fa541e49a42cfdce3cc3f1c1d Mon Sep 17 00:00:00 2001 From: Rodrigo Dias Correa Date: Tue, 10 Nov 2020 03:17:36 -0300 Subject: [PATCH 5/7] Add changelog entry file to `ChangeLog.d` Signed-off-by: Rodrigo Dias Correa --- ChangeLog.d/bugfix_3782.txt | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 ChangeLog.d/bugfix_3782.txt diff --git a/ChangeLog.d/bugfix_3782.txt b/ChangeLog.d/bugfix_3782.txt new file mode 100644 index 000000000..25e18cb18 --- /dev/null +++ b/ChangeLog.d/bugfix_3782.txt @@ -0,0 +1,2 @@ +Bugfix + * Fix build failures on GCC 11. Fixes #3782. From d596ca8a1e34b6ef2fda293250829d014e5d33af Mon Sep 17 00:00:00 2001 From: Rodrigo Dias Correa Date: Wed, 25 Nov 2020 00:42:28 -0300 Subject: [PATCH 6/7] Fix GCC warning in `ssl_calc_finished_tls_sha384` This commit fixes the same warning fixed by baeedbf9, but without wasting RAM. By casting `mbedtls_sha512_finish_ret()`, `padbuf` could be kept 48 bytes long without triggering any warnings. Signed-off-by: Rodrigo Dias Correa --- library/ssl_tls.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index c69de3f1d..79348bd8d 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3197,12 +3197,15 @@ static void ssl_calc_finished_tls_sha256( #endif /* MBEDTLS_SHA256_C */ #if defined(MBEDTLS_SHA512_C) + +typedef int (*finish_sha384_t)(mbedtls_sha512_context*, unsigned char[48]); + static void ssl_calc_finished_tls_sha384( mbedtls_ssl_context *ssl, unsigned char *buf, int from ) { int len = 12; const char *sender; - unsigned char padbuf[64]; + unsigned char padbuf[48]; #if defined(MBEDTLS_USE_PSA_CRYPTO) size_t hash_size; psa_hash_operation_t sha384_psa = PSA_HASH_OPERATION_INIT; @@ -3255,8 +3258,14 @@ static void ssl_calc_finished_tls_sha384( MBEDTLS_SSL_DEBUG_BUF( 4, "finished sha512 state", (unsigned char *) sha512.state, sizeof( sha512.state ) ); #endif + /* + * For SHA-384, we can save 16 bytes by keeping padbuf 48 bytes long. + * However, to avoid stringop-overflow warning in gcc, we have to cast + * mbedtls_sha512_finish_ret(). + */ + finish_sha384_t finish = (finish_sha384_t)mbedtls_sha512_finish_ret; + finish( &sha512, padbuf ); - mbedtls_sha512_finish_ret( &sha512, padbuf ); mbedtls_sha512_free( &sha512 ); #endif From f06a6144e23a1a8dcc3dd749b993ad88a96063fb Mon Sep 17 00:00:00 2001 From: Rodrigo Dias Correa Date: Wed, 25 Nov 2020 07:30:26 -0300 Subject: [PATCH 7/7] Change function casting in `ssl_calc_finished_tls_sha384` `finish_sha384_t` was made more generic by using `unsigned char*` instead of `unsigned char[48]` as the second parameter. This change tries to make the function casting more robust against future improvements of gcc analysis. Signed-off-by: Rodrigo Dias Correa --- library/ssl_tls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 79348bd8d..a1a5859f0 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3198,7 +3198,7 @@ static void ssl_calc_finished_tls_sha256( #if defined(MBEDTLS_SHA512_C) -typedef int (*finish_sha384_t)(mbedtls_sha512_context*, unsigned char[48]); +typedef int (*finish_sha384_t)(mbedtls_sha512_context*, unsigned char*); static void ssl_calc_finished_tls_sha384( mbedtls_ssl_context *ssl, unsigned char *buf, int from )