From e02e02f203e3f71e04b53e95fcd7c535940b48aa Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 13 May 2021 00:22:35 +0200 Subject: [PATCH 1/4] Change sha512 output type from an array to a pointer The output parameter of mbedtls_sha512_finish_ret and mbedtls_sha512_ret now has a pointer type rather than array type. This removes spurious warnings in some compilers when outputting a SHA-384 hash into a 48-byte buffer. Signed-off-by: Gilles Peskine --- ChangeLog.d/sha512-output-type.txt | 5 +++++ docs/3.0-migration-guide.d/sha512-output-type.md | 8 ++++++++ include/mbedtls/sha512.h | 10 ++++++---- library/sha512.c | 4 ++-- 4 files changed, 21 insertions(+), 6 deletions(-) create mode 100644 ChangeLog.d/sha512-output-type.txt create mode 100644 docs/3.0-migration-guide.d/sha512-output-type.md diff --git a/ChangeLog.d/sha512-output-type.txt b/ChangeLog.d/sha512-output-type.txt new file mode 100644 index 0000000000..e29557c9d5 --- /dev/null +++ b/ChangeLog.d/sha512-output-type.txt @@ -0,0 +1,5 @@ +API changes + * The output parameter of mbedtls_sha512_finish_ret and mbedtls_sha512_ret + now has a pointer type rather than array type. This removes spurious + warnings in some compilers when outputting a SHA-384 hash into a + 48-byte buffer. diff --git a/docs/3.0-migration-guide.d/sha512-output-type.md b/docs/3.0-migration-guide.d/sha512-output-type.md new file mode 100644 index 0000000000..5a7d2053c4 --- /dev/null +++ b/docs/3.0-migration-guide.d/sha512-output-type.md @@ -0,0 +1,8 @@ +SHA-512 output type change +-------------------------- + +The output parameter of `mbedtls_sha512_finish_ret()` and `mbedtls_sha512_ret()` now has a pointer type rather than array type. This makes no difference in terms of C semantics, but removes spurious warnings in some compilers when outputting a SHA-384 hash into a 48-byte buffer. + +This makes no difference to a vast majority of applications. If your code takes a pointer to one of these functions, you may need to change the type of the pointer. + +Alternative implementations of the SHA512 module must adjust their functions' prototype accordingly. diff --git a/include/mbedtls/sha512.h b/include/mbedtls/sha512.h index 56cefe1bd0..2852273140 100644 --- a/include/mbedtls/sha512.h +++ b/include/mbedtls/sha512.h @@ -134,13 +134,14 @@ int mbedtls_sha512_update_ret( mbedtls_sha512_context *ctx, * \param ctx The SHA-512 context. This must be initialized * and have a hash operation started. * \param output The SHA-384 or SHA-512 checksum result. - * This must be a writable buffer of length \c 64 Bytes. + * This must be a writable buffer of length \c 64 bytes + * for SHA-512, 48 bytes for SHA-384. * * \return \c 0 on success. * \return A negative error code on failure. */ int mbedtls_sha512_finish_ret( mbedtls_sha512_context *ctx, - unsigned char output[64] ); + unsigned char *output ); /** * \brief This function processes a single data block within @@ -171,7 +172,8 @@ int mbedtls_internal_sha512_process( mbedtls_sha512_context *ctx, * a readable buffer of length \p ilen Bytes. * \param ilen The length of the input data in Bytes. * \param output The SHA-384 or SHA-512 checksum result. - * This must be a writable buffer of length \c 64 Bytes. + * This must be a writable buffer of length \c 64 bytes + * for SHA-512, 48 bytes for SHA-384. * \param is384 Determines which function to use. This must be either * \c 0 for SHA-512, or \c 1 for SHA-384. * @@ -184,7 +186,7 @@ int mbedtls_internal_sha512_process( mbedtls_sha512_context *ctx, */ int mbedtls_sha512_ret( const unsigned char *input, size_t ilen, - unsigned char output[64], + unsigned char *output, int is384 ); #if defined(MBEDTLS_SELF_TEST) diff --git a/library/sha512.c b/library/sha512.c index 75306298fd..7d53731d08 100644 --- a/library/sha512.c +++ b/library/sha512.c @@ -380,7 +380,7 @@ int mbedtls_sha512_update_ret( mbedtls_sha512_context *ctx, * SHA-512 final digest */ int mbedtls_sha512_finish_ret( mbedtls_sha512_context *ctx, - unsigned char output[64] ) + unsigned char *output ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned used; @@ -453,7 +453,7 @@ int mbedtls_sha512_finish_ret( mbedtls_sha512_context *ctx, */ int mbedtls_sha512_ret( const unsigned char *input, size_t ilen, - unsigned char output[64], + unsigned char *output, int is384 ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; From 3e3a6789d12571000df91f4e5ef3549a6cd5733c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 13 May 2021 00:26:17 +0200 Subject: [PATCH 2/4] Remove a kludge for the output size of mbedtls_sha512_finish_ret Remove a kludge to avoid a warning in GCC 11 when calling mbedtls_sha512_finish_ret with a 48-byte output buffer. This is correct since we're calculating SHA-384. When mbedtls_sha512_finish_ret's output parameter was declared as a 64-byte array, GCC 11 -Wstringop-overflow emitted a well-meaning, but inaccurate buffer overflow warning, which we tried to work around (successfully with beta releases but unsuccessfully with GCC 11.1.0 as released). Now that the output parameter is declared as a pointer, no workaround is necessary. Signed-off-by: Gilles Peskine --- library/ssl_tls.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index bc2f269a9c..bae9ed70ca 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2897,8 +2897,6 @@ static void ssl_calc_finished_tls_sha256( #if defined(MBEDTLS_SHA512_C) -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 ) { @@ -2957,13 +2955,7 @@ 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 d7b3d9247602fe5d5015a759d4f3867f28ac22a8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 13 May 2021 00:45:25 +0200 Subject: [PATCH 3/4] Change sha256 output type from an array to a pointer The output parameter of mbedtls_sha256_finish_ret and mbedtls_sha256_ret now has a pointer type rather than array type. This removes spurious warnings in some compilers when outputting a SHA-224 hash into a 28-byte buffer. Signed-off-by: Gilles Peskine --- ChangeLog.d/sha512-output-type.txt | 9 +++++---- docs/3.0-migration-guide.d/sha512-output-type.md | 6 +++--- include/mbedtls/sha256.h | 12 +++++++----- library/sha256.c | 4 ++-- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/ChangeLog.d/sha512-output-type.txt b/ChangeLog.d/sha512-output-type.txt index e29557c9d5..eabc67df70 100644 --- a/ChangeLog.d/sha512-output-type.txt +++ b/ChangeLog.d/sha512-output-type.txt @@ -1,5 +1,6 @@ API changes - * The output parameter of mbedtls_sha512_finish_ret and mbedtls_sha512_ret - now has a pointer type rather than array type. This removes spurious - warnings in some compilers when outputting a SHA-384 hash into a - 48-byte buffer. + * The output parameter of mbedtls_sha512_finish_ret, mbedtls_sha512_ret, + mbedtls_sha256_finish_ret and mbedtls_sha256_ret now has a pointer type + rather than array type. This removes spurious warnings in some compilers + when outputting a SHA-384 or SHA-224 hash into a buffer of exactly + the hash size. diff --git a/docs/3.0-migration-guide.d/sha512-output-type.md b/docs/3.0-migration-guide.d/sha512-output-type.md index 5a7d2053c4..c62a881598 100644 --- a/docs/3.0-migration-guide.d/sha512-output-type.md +++ b/docs/3.0-migration-guide.d/sha512-output-type.md @@ -1,8 +1,8 @@ -SHA-512 output type change +SHA-512 and SHA-256 output type change -------------------------- -The output parameter of `mbedtls_sha512_finish_ret()` and `mbedtls_sha512_ret()` now has a pointer type rather than array type. This makes no difference in terms of C semantics, but removes spurious warnings in some compilers when outputting a SHA-384 hash into a 48-byte buffer. +The output parameter of `mbedtls_sha256_finish_ret()`, `mbedtls_sha256_ret()`, `mbedtls_sha512_finish_ret()`, `mbedtls_sha512_ret()` now has a pointer type rather than array type. This makes no difference in terms of C semantics, but removes spurious warnings in some compilers when outputting a SHA-384 hash into a 48-byte buffer or a SHA-224 hash into a 28-byte buffer. This makes no difference to a vast majority of applications. If your code takes a pointer to one of these functions, you may need to change the type of the pointer. -Alternative implementations of the SHA512 module must adjust their functions' prototype accordingly. +Alternative implementations of the SHA256 and SHA512 modules must adjust their functions' prototype accordingly. diff --git a/include/mbedtls/sha256.h b/include/mbedtls/sha256.h index 9b8d91d1ca..1100869520 100644 --- a/include/mbedtls/sha256.h +++ b/include/mbedtls/sha256.h @@ -127,13 +127,14 @@ int mbedtls_sha256_update_ret( mbedtls_sha256_context *ctx, * \param ctx The SHA-256 context. This must be initialized * and have a hash operation started. * \param output The SHA-224 or SHA-256 checksum result. - * This must be a writable buffer of length \c 32 Bytes. + * This must be a writable buffer of length \c 32 bytes + * for SHA-256, 28 bytes for SHA-224. * * \return \c 0 on success. * \return A negative error code on failure. */ int mbedtls_sha256_finish_ret( mbedtls_sha256_context *ctx, - unsigned char output[32] ); + unsigned char *output ); /** * \brief This function processes a single data block within @@ -163,14 +164,15 @@ int mbedtls_internal_sha256_process( mbedtls_sha256_context *ctx, * \param input The buffer holding the data. This must be a readable * buffer of length \p ilen Bytes. * \param ilen The length of the input data in Bytes. - * \param output The SHA-224 or SHA-256 checksum result. This must - * be a writable buffer of length \c 32 Bytes. + * \param output The SHA-224 or SHA-256 checksum result. + * This must be a writable buffer of length \c 32 bytes + * for SHA-256, 28 bytes for SHA-224. * \param is224 Determines which function to use. This must be * either \c 0 for SHA-256, or \c 1 for SHA-224. */ int mbedtls_sha256_ret( const unsigned char *input, size_t ilen, - unsigned char output[32], + unsigned char *output, int is224 ); #if defined(MBEDTLS_SELF_TEST) diff --git a/library/sha256.c b/library/sha256.c index a94f325e8b..36ab0c1aa8 100644 --- a/library/sha256.c +++ b/library/sha256.c @@ -332,7 +332,7 @@ int mbedtls_sha256_update_ret( mbedtls_sha256_context *ctx, * SHA-256 final digest */ int mbedtls_sha256_finish_ret( mbedtls_sha256_context *ctx, - unsigned char output[32] ) + unsigned char *output ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; uint32_t used; @@ -401,7 +401,7 @@ int mbedtls_sha256_finish_ret( mbedtls_sha256_context *ctx, */ int mbedtls_sha256_ret( const unsigned char *input, size_t ilen, - unsigned char output[32], + unsigned char *output, int is224 ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; From 96d6e087175e650c90e93a2d686fbca1e4ec1194 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 18 May 2021 20:06:04 +0200 Subject: [PATCH 4/4] Make the formatting of numbers consistent Signed-off-by: Gilles Peskine --- include/mbedtls/sha256.h | 4 ++-- include/mbedtls/sha512.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/sha256.h b/include/mbedtls/sha256.h index 1100869520..22c2c7d7eb 100644 --- a/include/mbedtls/sha256.h +++ b/include/mbedtls/sha256.h @@ -128,7 +128,7 @@ int mbedtls_sha256_update_ret( mbedtls_sha256_context *ctx, * and have a hash operation started. * \param output The SHA-224 or SHA-256 checksum result. * This must be a writable buffer of length \c 32 bytes - * for SHA-256, 28 bytes for SHA-224. + * for SHA-256, \c 28 bytes for SHA-224. * * \return \c 0 on success. * \return A negative error code on failure. @@ -166,7 +166,7 @@ int mbedtls_internal_sha256_process( mbedtls_sha256_context *ctx, * \param ilen The length of the input data in Bytes. * \param output The SHA-224 or SHA-256 checksum result. * This must be a writable buffer of length \c 32 bytes - * for SHA-256, 28 bytes for SHA-224. + * for SHA-256, \c 28 bytes for SHA-224. * \param is224 Determines which function to use. This must be * either \c 0 for SHA-256, or \c 1 for SHA-224. */ diff --git a/include/mbedtls/sha512.h b/include/mbedtls/sha512.h index 2852273140..ef1fa22231 100644 --- a/include/mbedtls/sha512.h +++ b/include/mbedtls/sha512.h @@ -135,7 +135,7 @@ int mbedtls_sha512_update_ret( mbedtls_sha512_context *ctx, * and have a hash operation started. * \param output The SHA-384 or SHA-512 checksum result. * This must be a writable buffer of length \c 64 bytes - * for SHA-512, 48 bytes for SHA-384. + * for SHA-512, \c 48 bytes for SHA-384. * * \return \c 0 on success. * \return A negative error code on failure. @@ -173,7 +173,7 @@ int mbedtls_internal_sha512_process( mbedtls_sha512_context *ctx, * \param ilen The length of the input data in Bytes. * \param output The SHA-384 or SHA-512 checksum result. * This must be a writable buffer of length \c 64 bytes - * for SHA-512, 48 bytes for SHA-384. + * for SHA-512, \c 48 bytes for SHA-384. * \param is384 Determines which function to use. This must be either * \c 0 for SHA-512, or \c 1 for SHA-384. *