From d103823ba2acbcf477e73cf7b4ba7f8fafcb165c 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 3b06feee8..a60a82288 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -499,20 +499,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 375366a197a23a506db3e11bf69a0a535442a950 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 0d8280416..3f76344a7 100644 --- a/library/cmac.c +++ b/library/cmac.c @@ -454,7 +454,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 a60a82288..725e9e7ef 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -499,20 +499,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 */ @@ -1011,7 +1011,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; @@ -1060,7 +1060,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; @@ -1088,7 +1088,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; @@ -1109,7 +1109,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 5fb1bd487d828c444121b2627f4cc5b38dc6478f 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/main_test.function | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/programs/test/selftest.c b/programs/test/selftest.c index 4ac71a8de..3a8ab7a32 100644 --- a/programs/test/selftest.c +++ b/programs/test/selftest.c @@ -180,7 +180,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/main_test.function b/tests/suites/main_test.function index 8a4137bb0..1c9804f71 100644 --- a/tests/suites/main_test.function +++ b/tests/suites/main_test.function @@ -226,7 +226,7 @@ int parse_arguments( char *buf, size_t len, char *params[50] ) return( cnt ); } -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 34018bef3dcc8172a7b94f92939c55a79d6656f4 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 725e9e7ef..8757e4659 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5411,7 +5411,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 ddcc0b79824ec1819c747e9c460a4ea49fb4a0b3 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 d7853a847d0a0bcdaa575825869e116be75c2766 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 8757e4659..70d32cce7 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5405,13 +5405,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 ) @@ -5438,7 +5441,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 f75fbab19f25b1178f632f5b93c717a6633c8007 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 70d32cce7..7f0e0753e 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5406,7 +5406,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 9c7e92b5dbc616519f6486e2328b2cd2680aabd0 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 7f0e0753e..2b1aa0110 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5415,6 +5415,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 ) @@ -5441,13 +5447,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 );