From 0391ea39c11e8ce2125a02db65db61ca67249566 Mon Sep 17 00:00:00 2001 From: Rodrigo Dias Correa Date: Wed, 4 Nov 2020 01:55:38 -0300 Subject: [PATCH 1/8] 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 2471600c9..82181e36f 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -614,20 +614,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( mbedtls_ssl_context *, unsigned char * ); +static void ssl_calc_verify_tls( mbedtls_ssl_context *, unsigned char [36] ); 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( mbedtls_ssl_context *,unsigned char * ); +static void ssl_calc_verify_tls_sha256( mbedtls_ssl_context *, unsigned char [32] ); 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( mbedtls_ssl_context *, unsigned char * ); +static void ssl_calc_verify_tls_sha384( mbedtls_ssl_context *, unsigned char [48] ); static void ssl_calc_finished_tls_sha384( mbedtls_ssl_context *, unsigned char *, int ); #endif #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ From 1a0c7fb38355ace2edade9a124b73e989de08f32 Mon Sep 17 00:00:00 2001 From: Rodrigo Dias Correa Date: Tue, 10 Nov 2020 01:38:00 -0300 Subject: [PATCH 2/8] 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 1a1200b52..409f67958 100644 --- a/library/cmac.c +++ b/library/cmac.c @@ -450,7 +450,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; const mbedtls_cipher_info_t *cipher_info; diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 82181e36f..90b1d1667 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -614,20 +614,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( mbedtls_ssl_context *, unsigned char [36] ); +static void ssl_calc_verify_tls( mbedtls_ssl_context *, unsigned char * ); 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( mbedtls_ssl_context *, unsigned char [32] ); +static void ssl_calc_verify_tls_sha256( mbedtls_ssl_context *, unsigned char * ); 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( mbedtls_ssl_context *, unsigned char [48] ); +static void ssl_calc_verify_tls_sha384( mbedtls_ssl_context *, unsigned char * ); static void ssl_calc_finished_tls_sha384( mbedtls_ssl_context *, unsigned char *, int ); #endif #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ @@ -1142,7 +1142,7 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_SSL_PROTO_SSL3) -void ssl_calc_verify_ssl( mbedtls_ssl_context *ssl, unsigned char hash[36] ) +void ssl_calc_verify_ssl( mbedtls_ssl_context *ssl, unsigned char *hash ) { mbedtls_md5_context md5; mbedtls_sha1_context sha1; @@ -1191,7 +1191,7 @@ void ssl_calc_verify_ssl( mbedtls_ssl_context *ssl, unsigned char hash[36] ) #endif /* MBEDTLS_SSL_PROTO_SSL3 */ #if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) -void ssl_calc_verify_tls( mbedtls_ssl_context *ssl, unsigned char hash[36] ) +void ssl_calc_verify_tls( mbedtls_ssl_context *ssl, unsigned char *hash ) { mbedtls_md5_context md5; mbedtls_sha1_context sha1; @@ -1219,7 +1219,7 @@ void ssl_calc_verify_tls( mbedtls_ssl_context *ssl, unsigned char hash[36] ) #if defined(MBEDTLS_SSL_PROTO_TLS1_2) #if defined(MBEDTLS_SHA256_C) -void ssl_calc_verify_tls_sha256( mbedtls_ssl_context *ssl, unsigned char hash[32] ) +void ssl_calc_verify_tls_sha256( mbedtls_ssl_context *ssl, unsigned char *hash ) { mbedtls_sha256_context sha256; @@ -1240,7 +1240,7 @@ void ssl_calc_verify_tls_sha256( mbedtls_ssl_context *ssl, unsigned char hash[32 #endif /* MBEDTLS_SHA256_C */ #if defined(MBEDTLS_SHA512_C) -void ssl_calc_verify_tls_sha384( mbedtls_ssl_context *ssl, unsigned char hash[48] ) +void ssl_calc_verify_tls_sha384( mbedtls_ssl_context *ssl, unsigned char *hash ) { mbedtls_sha512_context sha512; From d552630f33526b448d5555afd853ac80a4ddb7d5 Mon Sep 17 00:00:00 2001 From: Rodrigo Dias Correa Date: Tue, 10 Nov 2020 02:28:50 -0300 Subject: [PATCH 3/8] 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 9bc9a9c6b..fe35bb15a 100644 --- a/programs/test/selftest.c +++ b/programs/test/selftest.c @@ -185,7 +185,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 ca51e7b28..1178135c5 100644 --- a/tests/suites/host_test.function +++ b/tests/suites/host_test.function @@ -330,7 +330,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 56ad24cad783599a10be55ed2d712a077470687c Mon Sep 17 00:00:00 2001 From: Rodrigo Dias Correa Date: Tue, 10 Nov 2020 02:51:51 -0300 Subject: [PATCH 4/8] 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 90b1d1667..ecb76de18 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6369,7 +6369,7 @@ static void ssl_calc_finished_tls_sha384( int len = 12; const char *sender; mbedtls_sha512_context sha512; - unsigned char padbuf[48]; + unsigned char padbuf[64]; mbedtls_ssl_session *session = ssl->session_negotiate; if( !session ) From d31012eceafa84b93f85d9a75d0ae4203ae198cc Mon Sep 17 00:00:00 2001 From: Rodrigo Dias Correa Date: Tue, 10 Nov 2020 03:17:36 -0300 Subject: [PATCH 5/8] 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 671600cd44b45db9eeae4f3800e864558e81ba51 Mon Sep 17 00:00:00 2001 From: Rodrigo Dias Correa Date: Wed, 25 Nov 2020 00:42:28 -0300 Subject: [PATCH 6/8] 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 ecb76de18..d96dd8f91 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6363,13 +6363,16 @@ 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; mbedtls_sha512_context sha512; - unsigned char padbuf[64]; + unsigned char padbuf[48]; mbedtls_ssl_session *session = ssl->session_negotiate; if( !session ) @@ -6396,7 +6399,13 @@ static void ssl_calc_finished_tls_sha384( ? "client finished" : "server finished"; - mbedtls_sha512_finish_ret( &sha512, padbuf ); + /* + * 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 ); ssl->handshake->tls_prf( session->master, 48, sender, padbuf, 48, buf, len ); From 0b9bc0bd77224fa0726c6e8a4300f31dbd9347b2 Mon Sep 17 00:00:00 2001 From: Rodrigo Dias Correa Date: Wed, 25 Nov 2020 07:30:26 -0300 Subject: [PATCH 7/8] 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 d96dd8f91..45df3d487 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6364,7 +6364,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 ) From d2d0e70276b96de6cdf10d7fd0aa1dcff65e5ec2 Mon Sep 17 00:00:00 2001 From: Rodrigo Dias Correa Date: Sat, 28 Nov 2020 14:59:56 -0300 Subject: [PATCH 8/8] Move declaration to fix C90 warning "declaration-after-statement" was generated because that code was backported from the development branch, which currently uses C99. Signed-off-by: Rodrigo Dias Correa --- library/ssl_tls.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 45df3d487..c749a8611 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6373,6 +6373,12 @@ static void ssl_calc_finished_tls_sha384( const char *sender; mbedtls_sha512_context sha512; unsigned char padbuf[48]; + /* + * 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_sha384 = (finish_sha384_t)mbedtls_sha512_finish_ret; mbedtls_ssl_session *session = ssl->session_negotiate; if( !session ) @@ -6399,13 +6405,7 @@ static void ssl_calc_finished_tls_sha384( ? "client finished" : "server finished"; - /* - * 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 ); + finish_sha384( &sha512, padbuf ); ssl->handshake->tls_prf( session->master, 48, sender, padbuf, 48, buf, len );