From ba519b94a58199c76462b228ab178eb82bb590e0 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Sun, 9 Dec 2018 20:58:36 +0000 Subject: [PATCH 01/12] Add parameter validation to SHA-512 module --- ChangeLog | 2 ++ include/mbedtls/error.h | 2 +- include/mbedtls/sha512.h | 1 + library/sha512.c | 23 +++++++++++++++++++++++ 4 files changed, 27 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 66a8ce92f9..1c2614a6f1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -41,6 +41,8 @@ API Changes mbedtls_ctr_drbg_update() -> mbedtls_ctr_drbg_update_ret() mbedtls_hmac_drbg_update() -> mbedtls_hmac_drbg_update_ret() * Extend ECDH interface to enable alternative implementations. + * Add validation checks for input parameters to functions in the SHA-512 + module. New deprecations * Deprecate mbedtls_ctr_drbg_update and mbedtls_hmac_drbg_update diff --git a/include/mbedtls/error.h b/include/mbedtls/error.h index 0c38889878..851be1b6c0 100644 --- a/include/mbedtls/error.h +++ b/include/mbedtls/error.h @@ -76,7 +76,7 @@ * RIPEMD160 1 0x0031-0x0031 * SHA1 1 0x0035-0x0035 * SHA256 1 0x0037-0x0037 - * SHA512 1 0x0039-0x0039 + * SHA512 1 0x0039-0x0039 0x0075-0x0075 * CHACHA20 3 0x0051-0x0055 * POLY1305 3 0x0057-0x005B * CHACHAPOLY 2 0x0054-0x0056 diff --git a/include/mbedtls/sha512.h b/include/mbedtls/sha512.h index 020f95de61..257e8d43f8 100644 --- a/include/mbedtls/sha512.h +++ b/include/mbedtls/sha512.h @@ -37,6 +37,7 @@ /* MBEDTLS_ERR_SHA512_HW_ACCEL_FAILED is deprecated and should not be used. */ #define MBEDTLS_ERR_SHA512_HW_ACCEL_FAILED -0x0039 /**< SHA-512 hardware accelerator failed */ +#define MBEDTLS_ERR_SHA512_BAD_INPUT_DATA -0x0075 /**< Invalid input data. */ #ifdef __cplusplus extern "C" { diff --git a/library/sha512.c b/library/sha512.c index a9440e8af5..7a99170c97 100644 --- a/library/sha512.c +++ b/library/sha512.c @@ -88,8 +88,14 @@ } #endif /* PUT_UINT64_BE */ +#define MBEDTLS_SHA512_VALIDATE_RET(cond) \ + MBEDTLS_VALIDATE_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA, cond ) +#define MBEDTLS_SHA512_VALIDATE(cond) MBEDTLS_VALIDATE( cond ) + void mbedtls_sha512_init( mbedtls_sha512_context *ctx ) { + MBEDTLS_SHA512_VALIDATE( ctx != NULL ); + memset( ctx, 0, sizeof( mbedtls_sha512_context ) ); } @@ -104,6 +110,9 @@ void mbedtls_sha512_free( mbedtls_sha512_context *ctx ) void mbedtls_sha512_clone( mbedtls_sha512_context *dst, const mbedtls_sha512_context *src ) { + MBEDTLS_SHA512_VALIDATE( dst != NULL ); + MBEDTLS_SHA512_VALIDATE( src != NULL ); + *dst = *src; } @@ -112,6 +121,8 @@ void mbedtls_sha512_clone( mbedtls_sha512_context *dst, */ int mbedtls_sha512_starts_ret( mbedtls_sha512_context *ctx, int is384 ) { + MBEDTLS_SHA512_VALIDATE_RET( ctx != NULL ); + ctx->total[0] = 0; ctx->total[1] = 0; @@ -209,6 +220,9 @@ int mbedtls_internal_sha512_process( mbedtls_sha512_context *ctx, uint64_t temp1, temp2, W[80]; uint64_t A, B, C, D, E, F, G, H; + MBEDTLS_SHA512_VALIDATE_RET( ctx != NULL ); + MBEDTLS_SHA512_VALIDATE_RET( (const unsigned char *)data != NULL ); + #define SHR(x,n) (x >> n) #define ROTR(x,n) (SHR(x,n) | (x << (64 - n))) @@ -297,6 +311,9 @@ int mbedtls_sha512_update_ret( mbedtls_sha512_context *ctx, if( ilen == 0 ) return( 0 ); + MBEDTLS_SHA512_VALIDATE_RET( ctx != NULL ); + MBEDTLS_SHA512_VALIDATE_RET( input != NULL ); + left = (unsigned int) (ctx->total[0] & 0x7F); fill = 128 - left; @@ -351,6 +368,9 @@ int mbedtls_sha512_finish_ret( mbedtls_sha512_context *ctx, unsigned used; uint64_t high, low; + MBEDTLS_SHA512_VALIDATE_RET( ctx != NULL ); + MBEDTLS_SHA512_VALIDATE_RET( (unsigned char *)output != NULL ); + /* * Add padding: 0x80 then 0x00 until 16 bytes remain for the length */ @@ -427,6 +447,9 @@ int mbedtls_sha512_ret( const unsigned char *input, int ret; mbedtls_sha512_context ctx; + MBEDTLS_SHA512_VALIDATE_RET( ilen == 0 || input != NULL ); + MBEDTLS_SHA512_VALIDATE_RET( (unsigned char *)output != NULL ); + mbedtls_sha512_init( &ctx ); if( ( ret = mbedtls_sha512_starts_ret( &ctx, is384 ) ) != 0 ) From 863d48396531855bbd0bd4544b8655976ac2f7ea Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Sun, 9 Dec 2018 20:58:52 +0000 Subject: [PATCH 02/12] Add MBEDTLS_ERR_SHA512_BAD_INPUT_DATA to error.{h,c} --- library/error.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/error.c b/library/error.c index eabee9e21b..3251af06ce 100644 --- a/library/error.c +++ b/library/error.c @@ -865,6 +865,8 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen ) #if defined(MBEDTLS_SHA512_C) if( use_ret == -(MBEDTLS_ERR_SHA512_HW_ACCEL_FAILED) ) mbedtls_snprintf( buf, buflen, "SHA512 - SHA-512 hardware accelerator failed" ); + if( use_ret == -(MBEDTLS_ERR_SHA512_BAD_INPUT_DATA) ) + mbedtls_snprintf( buf, buflen, "SHA512 - Invalid input data" ); #endif /* MBEDTLS_SHA512_C */ #if defined(MBEDTLS_THREADING_C) From ff1052e6b074204e1aa7e449ad23f65de99ccfff Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Mon, 10 Dec 2018 10:28:10 +0000 Subject: [PATCH 03/12] Document valid function params for SHA-512 functions --- include/mbedtls/sha512.h | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/sha512.h b/include/mbedtls/sha512.h index 257e8d43f8..28b4998202 100644 --- a/include/mbedtls/sha512.h +++ b/include/mbedtls/sha512.h @@ -37,7 +37,7 @@ /* MBEDTLS_ERR_SHA512_HW_ACCEL_FAILED is deprecated and should not be used. */ #define MBEDTLS_ERR_SHA512_HW_ACCEL_FAILED -0x0039 /**< SHA-512 hardware accelerator failed */ -#define MBEDTLS_ERR_SHA512_BAD_INPUT_DATA -0x0075 /**< Invalid input data. */ +#define MBEDTLS_ERR_SHA512_BAD_INPUT_DATA -0x0075 /**< SHA-512 input data was malformed. */ #ifdef __cplusplus extern "C" { @@ -72,6 +72,7 @@ mbedtls_sha512_context; * \brief This function initializes a SHA-512 context. * * \param ctx The SHA-512 context to initialize. + * Must not be \c NULL. */ void mbedtls_sha512_init( mbedtls_sha512_context *ctx ); @@ -86,7 +87,9 @@ void mbedtls_sha512_free( mbedtls_sha512_context *ctx ); * \brief This function clones the state of a SHA-512 context. * * \param dst The destination context. + * Must not be \c NULL. * \param src The context to clone. + * Must not be \c NULL. */ void mbedtls_sha512_clone( mbedtls_sha512_context *dst, const mbedtls_sha512_context *src ); @@ -96,6 +99,7 @@ void mbedtls_sha512_clone( mbedtls_sha512_context *dst, * calculation. * * \param ctx The SHA-512 context to initialize. + * Must not be \c NULL. * \param is384 Determines which function to use: * 0: Use SHA-512, or 1: Use SHA-384. * @@ -108,7 +112,9 @@ int mbedtls_sha512_starts_ret( mbedtls_sha512_context *ctx, int is384 ); * SHA-512 checksum calculation. * * \param ctx The SHA-512 context. + * Must not be \c NULL. * \param input The buffer holding the input data. + * Must not be \c NULL if \p ilen is greater than 0. * \param ilen The length of the input data. * * \return \c 0 on success. @@ -123,7 +129,9 @@ int mbedtls_sha512_update_ret( mbedtls_sha512_context *ctx, * internal use only. * * \param ctx The SHA-512 context. + * Must not be \c NULL. * \param output The SHA-384 or SHA-512 checksum result. + * Must not be \c NULL. * * \return \c 0 on success. */ @@ -135,7 +143,9 @@ int mbedtls_sha512_finish_ret( mbedtls_sha512_context *ctx, * the ongoing SHA-512 computation. * * \param ctx The SHA-512 context. + * Must not be \c NULL. * \param data The buffer holding one block of data. + * Must not be \c NULL. * * \return \c 0 on success. */ @@ -154,6 +164,7 @@ int mbedtls_internal_sha512_process( mbedtls_sha512_context *ctx, * \deprecated Superseded by mbedtls_sha512_starts_ret() in 2.7.0 * * \param ctx The SHA-512 context to initialize. + * Must not be \c NULL. * \param is384 Determines which function to use: * 0: Use SHA-512, or 1: Use SHA-384. */ @@ -167,7 +178,9 @@ MBEDTLS_DEPRECATED void mbedtls_sha512_starts( mbedtls_sha512_context *ctx, * \deprecated Superseded by mbedtls_sha512_update_ret() in 2.7.0. * * \param ctx The SHA-512 context. + * Must not be \c NULL. * \param input The buffer holding the data. + * Must not be \c NULL if \p ilen is greater than 0. * \param ilen The length of the input data. */ MBEDTLS_DEPRECATED void mbedtls_sha512_update( mbedtls_sha512_context *ctx, @@ -181,7 +194,9 @@ MBEDTLS_DEPRECATED void mbedtls_sha512_update( mbedtls_sha512_context *ctx, * \deprecated Superseded by mbedtls_sha512_finish_ret() in 2.7.0. * * \param ctx The SHA-512 context. + * Must not be \c NULL. * \param output The SHA-384 or SHA-512 checksum result. + * Must not be \c NULL. */ MBEDTLS_DEPRECATED void mbedtls_sha512_finish( mbedtls_sha512_context *ctx, unsigned char output[64] ); @@ -194,7 +209,9 @@ MBEDTLS_DEPRECATED void mbedtls_sha512_finish( mbedtls_sha512_context *ctx, * \deprecated Superseded by mbedtls_internal_sha512_process() in 2.7.0. * * \param ctx The SHA-512 context. + * Must not be \c NULL. * \param data The buffer holding one block of data. + * Must not be \c NULL. */ MBEDTLS_DEPRECATED void mbedtls_sha512_process( mbedtls_sha512_context *ctx, @@ -214,8 +231,10 @@ MBEDTLS_DEPRECATED void mbedtls_sha512_process( * output = SHA-512(input buffer). * * \param input The buffer holding the input data. + * Must not be \c NULL if \p ilen is greater than 0. * \param ilen The length of the input data. * \param output The SHA-384 or SHA-512 checksum result. + * Must not be \c NULL. * \param is384 Determines which function to use: * 0: Use SHA-512, or 1: Use SHA-384. * @@ -245,8 +264,10 @@ int mbedtls_sha512_ret( const unsigned char *input, * \deprecated Superseded by mbedtls_sha512_ret() in 2.7.0 * * \param input The buffer holding the data. + * Must not be \c NULL if \p ilen is greater than 0. * \param ilen The length of the input data. * \param output The SHA-384 or SHA-512 checksum result. + * Must not be \c NULL. * \param is384 Determines which function to use: * 0: Use SHA-512, or 1: Use SHA-384. */ From b5c99f5c72d637ad9017e894381cb1875464dad5 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 18 Dec 2018 15:29:32 +0000 Subject: [PATCH 04/12] Improve documentation of SHA-512 parameter preconditions --- include/mbedtls/sha512.h | 113 +++++++++++++++++++++------------------ 1 file changed, 60 insertions(+), 53 deletions(-) diff --git a/include/mbedtls/sha512.h b/include/mbedtls/sha512.h index 28b4998202..93f9646ea9 100644 --- a/include/mbedtls/sha512.h +++ b/include/mbedtls/sha512.h @@ -71,25 +71,26 @@ mbedtls_sha512_context; /** * \brief This function initializes a SHA-512 context. * - * \param ctx The SHA-512 context to initialize. - * Must not be \c NULL. + * \param ctx The SHA-512 context to initialize. This must + * not be \c NULL. */ void mbedtls_sha512_init( mbedtls_sha512_context *ctx ); /** * \brief This function clears a SHA-512 context. * - * \param ctx The SHA-512 context to clear. + * \param ctx The SHA-512 context to clear. This may be \c NULL, + * in which case this function does nothing. If it + * is not \c NULL, it must point to an initialized + * SHA-512 context. */ void mbedtls_sha512_free( mbedtls_sha512_context *ctx ); /** * \brief This function clones the state of a SHA-512 context. * - * \param dst The destination context. - * Must not be \c NULL. - * \param src The context to clone. - * Must not be \c NULL. + * \param dst The destination context. This must be initialized. + * \param src The context to clone. This must be initialized. */ void mbedtls_sha512_clone( mbedtls_sha512_context *dst, const mbedtls_sha512_context *src ); @@ -98,12 +99,12 @@ void mbedtls_sha512_clone( mbedtls_sha512_context *dst, * \brief This function starts a SHA-384 or SHA-512 checksum * calculation. * - * \param ctx The SHA-512 context to initialize. - * Must not be \c NULL. - * \param is384 Determines which function to use: - * 0: Use SHA-512, or 1: Use SHA-384. + * \param ctx The SHA-512 context to use. This must be initialized. + * \param is384 Determines which function to use. This must be + * either \c for SHA-512, or \c 1 for SHA-384. * * \return \c 0 on success. + * \return A negative error code on failure. */ int mbedtls_sha512_starts_ret( mbedtls_sha512_context *ctx, int is384 ); @@ -111,13 +112,15 @@ int mbedtls_sha512_starts_ret( mbedtls_sha512_context *ctx, int is384 ); * \brief This function feeds an input buffer into an ongoing * SHA-512 checksum calculation. * - * \param ctx The SHA-512 context. - * Must not be \c NULL. - * \param input The buffer holding the input data. - * Must not be \c NULL if \p ilen is greater than 0. - * \param ilen The length of the input data. + * \param ctx The SHA-512 context. This must be initialized + * and have a hash operation started. + * \param input The buffer holding the input data. This must + * be a readable buffer of length \p ilen Bytes. + * It must not be \c NULL. + * \param ilen The length of the input data \p input in Bytes. * * \return \c 0 on success. + * \return A negative error code on failure. */ int mbedtls_sha512_update_ret( mbedtls_sha512_context *ctx, const unsigned char *input, @@ -128,12 +131,13 @@ int mbedtls_sha512_update_ret( mbedtls_sha512_context *ctx, * the result to the output buffer. This function is for * internal use only. * - * \param ctx The SHA-512 context. - * Must not be \c NULL. + * \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. - * Must not be \c NULL. + * This must be a writable buffer of length \c 64 Bytes. * * \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] ); @@ -142,12 +146,13 @@ int mbedtls_sha512_finish_ret( mbedtls_sha512_context *ctx, * \brief This function processes a single data block within * the ongoing SHA-512 computation. * - * \param ctx The SHA-512 context. - * Must not be \c NULL. - * \param data The buffer holding one block of data. - * Must not be \c NULL. + * \param ctx The SHA-512 context. This must be initialized + * and have a hash operation started. + * \param data The buffer holding one block of data. This + * must be a readable buffer of length \c 128 Bytes. * * \return \c 0 on success. + * \return A negative error code on failure. */ int mbedtls_internal_sha512_process( mbedtls_sha512_context *ctx, const unsigned char data[128] ); @@ -163,10 +168,9 @@ int mbedtls_internal_sha512_process( mbedtls_sha512_context *ctx, * * \deprecated Superseded by mbedtls_sha512_starts_ret() in 2.7.0 * - * \param ctx The SHA-512 context to initialize. - * Must not be \c NULL. - * \param is384 Determines which function to use: - * 0: Use SHA-512, or 1: Use SHA-384. + * \param ctx The SHA-512 context to use. This must be initialized. + * \param is384 Determines which function to use. This must be either + * \c 0 for SHA-512 or \c 1 for SHA-384. */ MBEDTLS_DEPRECATED void mbedtls_sha512_starts( mbedtls_sha512_context *ctx, int is384 ); @@ -177,11 +181,11 @@ MBEDTLS_DEPRECATED void mbedtls_sha512_starts( mbedtls_sha512_context *ctx, * * \deprecated Superseded by mbedtls_sha512_update_ret() in 2.7.0. * - * \param ctx The SHA-512 context. - * Must not be \c NULL. - * \param input The buffer holding the data. - * Must not be \c NULL if \p ilen is greater than 0. - * \param ilen The length of the input data. + * \param ctx The SHA-512 context. This must be initialized + * and have a hash operation started. + * \param input The buffer holding the data. This must be a readable + * buffer of length \p ilen Bytes. It must not be \c NULL. + * \param ilen The length of the input data \p input in Bytes. */ MBEDTLS_DEPRECATED void mbedtls_sha512_update( mbedtls_sha512_context *ctx, const unsigned char *input, @@ -193,10 +197,10 @@ MBEDTLS_DEPRECATED void mbedtls_sha512_update( mbedtls_sha512_context *ctx, * * \deprecated Superseded by mbedtls_sha512_finish_ret() in 2.7.0. * - * \param ctx The SHA-512 context. - * Must not be \c NULL. - * \param output The SHA-384 or SHA-512 checksum result. - * Must not be \c NULL. + * \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 size \c 64 Bytes. */ MBEDTLS_DEPRECATED void mbedtls_sha512_finish( mbedtls_sha512_context *ctx, unsigned char output[64] ); @@ -208,10 +212,10 @@ MBEDTLS_DEPRECATED void mbedtls_sha512_finish( mbedtls_sha512_context *ctx, * * \deprecated Superseded by mbedtls_internal_sha512_process() in 2.7.0. * - * \param ctx The SHA-512 context. - * Must not be \c NULL. - * \param data The buffer holding one block of data. - * Must not be \c NULL. + * \param ctx The SHA-512 context. This must be initialized and + * have a hash operation started. + * \param data The buffer holding one block of data. This must be + * a readable buffer of length \c 128 Bytes. */ MBEDTLS_DEPRECATED void mbedtls_sha512_process( mbedtls_sha512_context *ctx, @@ -230,15 +234,17 @@ MBEDTLS_DEPRECATED void mbedtls_sha512_process( * The SHA-512 result is calculated as * output = SHA-512(input buffer). * - * \param input The buffer holding the input data. - * Must not be \c NULL if \p ilen is greater than 0. - * \param ilen The length of the input data. + * \param input The buffer holding the input data. This must be + * a readable buffer of length \p ilen Bytes. It + * must not be \c NULL. + * \param ilen The length of the input data \p input in Bytes. * \param output The SHA-384 or SHA-512 checksum result. - * Must not be \c NULL. - * \param is384 Determines which function to use: - * 0: Use SHA-512, or 1: Use SHA-384. + * This must be a writable buffer of length \c 64 Bytes. + * \param is384 Determines which function to use. This must be either + * \c 0 for SHA-512, or \c 1 for SHA-384. * * \return \c 0 on success. + * \return A negative error code on failure. */ int mbedtls_sha512_ret( const unsigned char *input, size_t ilen, @@ -263,13 +269,14 @@ int mbedtls_sha512_ret( const unsigned char *input, * * \deprecated Superseded by mbedtls_sha512_ret() in 2.7.0 * - * \param input The buffer holding the data. - * Must not be \c NULL if \p ilen is greater than 0. - * \param ilen The length of the input data. - * \param output The SHA-384 or SHA-512 checksum result. - * Must not be \c NULL. - * \param is384 Determines which function to use: - * 0: Use SHA-512, or 1: Use SHA-384. + * \param input The buffer holding the data. This must be a + * readable buffer of length \p ilen Bytes. It + * must not be \c NULL. + * \param ilen The length of the input data \p input in Bytes. + * \param output The SHA-384 or SHA-512 checksum result. This must + * be a writable buffer of length \c 64 Bytes. + * \param is384 Determines which function to use. This must be eiher + * \c 0 for SHA-512, or \c 1 for SHA-384. */ MBEDTLS_DEPRECATED void mbedtls_sha512( const unsigned char *input, size_t ilen, From 686c9a0e8d3132b1d7c36fdd9e6e0829507bd022 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 18 Dec 2018 15:33:14 +0000 Subject: [PATCH 05/12] Test SHA-512 parameter validation --- tests/suites/test_suite_shax.data | 6 +++ tests/suites/test_suite_shax.function | 56 +++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/tests/suites/test_suite_shax.data b/tests/suites/test_suite_shax.data index ee8074dc08..6958a02cf5 100644 --- a/tests/suites/test_suite_shax.data +++ b/tests/suites/test_suite_shax.data @@ -95,6 +95,12 @@ SHA-256 Test Vector NIST CAVS #7 depends_on:MBEDTLS_SHA256_C mbedtls_sha256:"8390cf0be07661cc7669aac54ce09a37733a629d45f5d983ef201f9b2d13800e555d9b1097fec3b783d7a50dcb5e2b644b96a1e9463f177cf34906bf388f366db5c2deee04a30e283f764a97c3b377a034fefc22c259214faa99babaff160ab0aaa7e2ccb0ce09c6b32fe08cbc474694375aba703fadbfa31cf685b30a11c57f3cf4edd321e57d3ae6ebb1133c8260e75b9224fa47a2bb205249add2e2e62f817491482ae152322be0900355cdcc8d42a98f82e961a0dc6f537b7b410eff105f59673bfb787bf042aa071f7af68d944d27371c64160fe9382772372516c230c1f45c0d6b6cca7f274b394da9402d3eafdf733994ec58ab22d71829a98399574d4b5908a447a5a681cb0dd50a31145311d92c22a16de1ead66a5499f2dceb4cae694772ce90762ef8336afec653aa9b1a1c4820b221136dfce80dce2ba920d88a530c9410d0a4e0358a3a11052e58dd73b0b179ef8f56fe3b5a2d117a73a0c38a1392b6938e9782e0d86456ee4884e3c39d4d75813f13633bc79baa07c0d2d555afbf207f52b7dca126d015aa2b9873b3eb065e90b9b065a5373fe1fb1b20d594327d19fba56cb81e7b6696605ffa56eba3c27a438697cc21b201fd7e09f18deea1b3ea2f0d1edc02df0e20396a145412cd6b13c32d2e605641c948b714aec30c0649dc44143511f35ab0fd5dd64c34d06fe86f3836dfe9edeb7f08cfc3bd40956826356242191f99f53473f32b0cc0cf9321d6c92a112e8db90b86ee9e87cc32d0343db01e32ce9eb782cb24efbbbeb440fe929e8f2bf8dfb1550a3a2e742e8b455a3e5730e9e6a7a9824d17acc0f72a7f67eae0f0970f8bde46dcdefaed3047cf807e7f00a42e5fd11d40f5e98533d7574425b7d2bc3b3845c443008b58980e768e464e17cc6f6b3939eee52f713963d07d8c4abf02448ef0b889c9671e2f8a436ddeeffcca7176e9bf9d1005ecd377f2fa67c23ed1f137e60bf46018a8bd613d038e883704fc26e798969df35ec7bbc6a4fe46d8910bd82fa3cded265d0a3b6d399e4251e4d8233daa21b5812fded6536198ff13aa5a1cd46a5b9a17a4ddc1d9f85544d1d1cc16f3df858038c8e071a11a7e157a85a6a8dc47e88d75e7009a8b26fdb73f33a2a70f1e0c259f8f9533b9b8f9af9288b7274f21baeec78d396f8bacdcc22471207d9b4efccd3fedc5c5a2214ff5e51c553f35e21ae696fe51e8df733a8e06f50f419e599e9f9e4b37ce643fc810faaa47989771509d69a110ac916261427026369a21263ac4460fb4f708f8ae28599856db7cb6a43ac8e03d64a9609807e76c5f312b9d1863bfa304e8953647648b4f4ab0ed995e":"4109cdbec3240ad74cc6c37f39300f70fede16e21efc77f7865998714aad0b5e" +SHA-512 Invalid parameters +sha512_invalid_param: + +SHA-512 Valid parameters +sha512_valid_param: + SHA-384 Test Vector NIST CAVS #1 depends_on:MBEDTLS_SHA512_C sha384:"":"38b060a751ac96384cd9327eb1b1e36a21fdb71114be07434c0cc7bf63f6e1da274edebfe76f65fbd51ad2f14898b95b" diff --git a/tests/suites/test_suite_shax.function b/tests/suites/test_suite_shax.function index 147ae0e1ff..a5a7b68d55 100644 --- a/tests/suites/test_suite_shax.function +++ b/tests/suites/test_suite_shax.function @@ -46,6 +46,62 @@ void mbedtls_sha256( data_t * src_str, data_t * hex_hash_string ) } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_SHA256_C */ +void sha512_valid_param( ) +{ + TEST_VALID_PARAM( mbedtls_sha512_free( NULL ) ); +} +/* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_SHA512_C:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */ +void sha512_invalid_param( ) +{ + mbedtls_sha512_context ctx; + unsigned char buf[64] = { 0 }; + size_t const buflen = sizeof( buf ); + int valid_type = 0; + int invalid_type = 42; + + TEST_INVALID_PARAM( mbedtls_sha512_init( NULL ) ); + + TEST_INVALID_PARAM( mbedtls_sha512_clone( NULL, &ctx ) ); + TEST_INVALID_PARAM( mbedtls_sha512_clone( &ctx, NULL ) ); + + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA, + mbedtls_sha512_starts_ret( NULL, valid_type ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA, + mbedtls_sha512_starts_ret( &ctx, invalid_type ) ); + + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA, + mbedtls_sha512_update_ret( NULL, buf, buflen ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA, + mbedtls_sha512_update_ret( &ctx, NULL, buflen ) ); + + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA, + mbedtls_sha512_finish_ret( NULL, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA, + mbedtls_sha512_finish_ret( &ctx, NULL ) ); + + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA, + mbedtls_internal_sha512_process( NULL, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA, + mbedtls_internal_sha512_process( &ctx, NULL ) ); + + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA, + mbedtls_sha512_ret( NULL, buflen, + buf, valid_type ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA, + mbedtls_sha512_ret( buf, buflen, + NULL, valid_type ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA, + mbedtls_sha512_ret( buf, buflen, + buf, invalid_type ) ); + +exit: + return; +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_SHA512_C */ void sha384( data_t * src_str, data_t * hex_hash_string ) { From ca6f4585c726766d24c4368de26daa1364b30f39 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 18 Dec 2018 15:37:22 +0000 Subject: [PATCH 06/12] Fix parameter validation in SHA-512 module --- include/mbedtls/sha512.h | 17 +++++++---------- library/sha512.c | 12 +++++++----- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/include/mbedtls/sha512.h b/include/mbedtls/sha512.h index 93f9646ea9..bf40e4b045 100644 --- a/include/mbedtls/sha512.h +++ b/include/mbedtls/sha512.h @@ -116,8 +116,7 @@ int mbedtls_sha512_starts_ret( mbedtls_sha512_context *ctx, int is384 ); * and have a hash operation started. * \param input The buffer holding the input data. This must * be a readable buffer of length \p ilen Bytes. - * It must not be \c NULL. - * \param ilen The length of the input data \p input in Bytes. + * \param ilen The length of the input data in Bytes. * * \return \c 0 on success. * \return A negative error code on failure. @@ -184,8 +183,8 @@ MBEDTLS_DEPRECATED void mbedtls_sha512_starts( mbedtls_sha512_context *ctx, * \param ctx The SHA-512 context. This must be initialized * and have a hash operation started. * \param input The buffer holding the data. This must be a readable - * buffer of length \p ilen Bytes. It must not be \c NULL. - * \param ilen The length of the input data \p input in Bytes. + * buffer of length \p ilen Bytes. + * \param ilen The length of the input data in Bytes. */ MBEDTLS_DEPRECATED void mbedtls_sha512_update( mbedtls_sha512_context *ctx, const unsigned char *input, @@ -235,9 +234,8 @@ MBEDTLS_DEPRECATED void mbedtls_sha512_process( * output = SHA-512(input buffer). * * \param input The buffer holding the input data. This must be - * a readable buffer of length \p ilen Bytes. It - * must not be \c NULL. - * \param ilen The length of the input data \p input in Bytes. + * 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. * \param is384 Determines which function to use. This must be either @@ -270,9 +268,8 @@ int mbedtls_sha512_ret( const unsigned char *input, * \deprecated Superseded by mbedtls_sha512_ret() in 2.7.0 * * \param input The buffer holding the data. This must be a - * readable buffer of length \p ilen Bytes. It - * must not be \c NULL. - * \param ilen The length of the input data \p input in Bytes. + * 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. * \param is384 Determines which function to use. This must be eiher diff --git a/library/sha512.c b/library/sha512.c index 7a99170c97..8260f32a65 100644 --- a/library/sha512.c +++ b/library/sha512.c @@ -89,8 +89,8 @@ #endif /* PUT_UINT64_BE */ #define MBEDTLS_SHA512_VALIDATE_RET(cond) \ - MBEDTLS_VALIDATE_RET( MBEDTLS_ERR_SHA512_BAD_INPUT_DATA, cond ) -#define MBEDTLS_SHA512_VALIDATE(cond) MBEDTLS_VALIDATE( cond ) + MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_SHA512_BAD_INPUT_DATA ) +#define MBEDTLS_SHA512_VALIDATE(cond) MBEDTLS_INTERNAL_VALIDATE( cond ) void mbedtls_sha512_init( mbedtls_sha512_context *ctx ) { @@ -122,6 +122,7 @@ void mbedtls_sha512_clone( mbedtls_sha512_context *dst, int mbedtls_sha512_starts_ret( mbedtls_sha512_context *ctx, int is384 ) { MBEDTLS_SHA512_VALIDATE_RET( ctx != NULL ); + MBEDTLS_SHA512_VALIDATE_RET( is384 == 0 || is384 == 1 ); ctx->total[0] = 0; ctx->total[1] = 0; @@ -308,12 +309,12 @@ int mbedtls_sha512_update_ret( mbedtls_sha512_context *ctx, size_t fill; unsigned int left; + MBEDTLS_SHA512_VALIDATE_RET( ctx != NULL ); + MBEDTLS_SHA512_VALIDATE_RET( ilen == 0 || input != NULL ); + if( ilen == 0 ) return( 0 ); - MBEDTLS_SHA512_VALIDATE_RET( ctx != NULL ); - MBEDTLS_SHA512_VALIDATE_RET( input != NULL ); - left = (unsigned int) (ctx->total[0] & 0x7F); fill = 128 - left; @@ -447,6 +448,7 @@ int mbedtls_sha512_ret( const unsigned char *input, int ret; mbedtls_sha512_context ctx; + MBEDTLS_SHA512_VALIDATE_RET( is384 == 0 || is384 == 1 ); MBEDTLS_SHA512_VALIDATE_RET( ilen == 0 || input != NULL ); MBEDTLS_SHA512_VALIDATE_RET( (unsigned char *)output != NULL ); From 4fbd4bf442467c57d03f7340948a6d374634227d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 18 Dec 2018 16:37:43 +0000 Subject: [PATCH 07/12] Fix guard in SHA-512 tests --- tests/suites/test_suite_shax.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_shax.function b/tests/suites/test_suite_shax.function index a5a7b68d55..ead6d9e313 100644 --- a/tests/suites/test_suite_shax.function +++ b/tests/suites/test_suite_shax.function @@ -46,7 +46,7 @@ void mbedtls_sha256( data_t * src_str, data_t * hex_hash_string ) } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_SHA256_C */ +/* BEGIN_CASE depends_on:MBEDTLS_SHA512_C */ void sha512_valid_param( ) { TEST_VALID_PARAM( mbedtls_sha512_free( NULL ) ); From 38e15d49f359e27a633d642920e789f9a0663d3b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 18 Dec 2018 17:54:00 +0000 Subject: [PATCH 08/12] Don't declare MBEDTLS-namespace identifiers in sha512.c --- library/sha512.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/library/sha512.c b/library/sha512.c index 8260f32a65..e7b2c5093c 100644 --- a/library/sha512.c +++ b/library/sha512.c @@ -88,13 +88,13 @@ } #endif /* PUT_UINT64_BE */ -#define MBEDTLS_SHA512_VALIDATE_RET(cond) \ +#define SHA512_VALIDATE_RET(cond) \ MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_SHA512_BAD_INPUT_DATA ) -#define MBEDTLS_SHA512_VALIDATE(cond) MBEDTLS_INTERNAL_VALIDATE( cond ) +#define SHA512_VALIDATE(cond) MBEDTLS_INTERNAL_VALIDATE( cond ) void mbedtls_sha512_init( mbedtls_sha512_context *ctx ) { - MBEDTLS_SHA512_VALIDATE( ctx != NULL ); + SHA512_VALIDATE( ctx != NULL ); memset( ctx, 0, sizeof( mbedtls_sha512_context ) ); } @@ -110,8 +110,8 @@ void mbedtls_sha512_free( mbedtls_sha512_context *ctx ) void mbedtls_sha512_clone( mbedtls_sha512_context *dst, const mbedtls_sha512_context *src ) { - MBEDTLS_SHA512_VALIDATE( dst != NULL ); - MBEDTLS_SHA512_VALIDATE( src != NULL ); + SHA512_VALIDATE( dst != NULL ); + SHA512_VALIDATE( src != NULL ); *dst = *src; } @@ -121,8 +121,8 @@ void mbedtls_sha512_clone( mbedtls_sha512_context *dst, */ int mbedtls_sha512_starts_ret( mbedtls_sha512_context *ctx, int is384 ) { - MBEDTLS_SHA512_VALIDATE_RET( ctx != NULL ); - MBEDTLS_SHA512_VALIDATE_RET( is384 == 0 || is384 == 1 ); + SHA512_VALIDATE_RET( ctx != NULL ); + SHA512_VALIDATE_RET( is384 == 0 || is384 == 1 ); ctx->total[0] = 0; ctx->total[1] = 0; @@ -221,8 +221,8 @@ int mbedtls_internal_sha512_process( mbedtls_sha512_context *ctx, uint64_t temp1, temp2, W[80]; uint64_t A, B, C, D, E, F, G, H; - MBEDTLS_SHA512_VALIDATE_RET( ctx != NULL ); - MBEDTLS_SHA512_VALIDATE_RET( (const unsigned char *)data != NULL ); + SHA512_VALIDATE_RET( ctx != NULL ); + SHA512_VALIDATE_RET( (const unsigned char *)data != NULL ); #define SHR(x,n) (x >> n) #define ROTR(x,n) (SHR(x,n) | (x << (64 - n))) @@ -309,8 +309,8 @@ int mbedtls_sha512_update_ret( mbedtls_sha512_context *ctx, size_t fill; unsigned int left; - MBEDTLS_SHA512_VALIDATE_RET( ctx != NULL ); - MBEDTLS_SHA512_VALIDATE_RET( ilen == 0 || input != NULL ); + SHA512_VALIDATE_RET( ctx != NULL ); + SHA512_VALIDATE_RET( ilen == 0 || input != NULL ); if( ilen == 0 ) return( 0 ); @@ -369,8 +369,8 @@ int mbedtls_sha512_finish_ret( mbedtls_sha512_context *ctx, unsigned used; uint64_t high, low; - MBEDTLS_SHA512_VALIDATE_RET( ctx != NULL ); - MBEDTLS_SHA512_VALIDATE_RET( (unsigned char *)output != NULL ); + SHA512_VALIDATE_RET( ctx != NULL ); + SHA512_VALIDATE_RET( (unsigned char *)output != NULL ); /* * Add padding: 0x80 then 0x00 until 16 bytes remain for the length @@ -448,9 +448,9 @@ int mbedtls_sha512_ret( const unsigned char *input, int ret; mbedtls_sha512_context ctx; - MBEDTLS_SHA512_VALIDATE_RET( is384 == 0 || is384 == 1 ); - MBEDTLS_SHA512_VALIDATE_RET( ilen == 0 || input != NULL ); - MBEDTLS_SHA512_VALIDATE_RET( (unsigned char *)output != NULL ); + SHA512_VALIDATE_RET( is384 == 0 || is384 == 1 ); + SHA512_VALIDATE_RET( ilen == 0 || input != NULL ); + SHA512_VALIDATE_RET( (unsigned char *)output != NULL ); mbedtls_sha512_init( &ctx ); From 3f2d1ef1694d9c329a9817c483d6c588976aeac4 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 18 Dec 2018 18:41:40 +0000 Subject: [PATCH 09/12] Fix typo in SHA512 documentation --- include/mbedtls/sha512.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/sha512.h b/include/mbedtls/sha512.h index bf40e4b045..34a90b51ee 100644 --- a/include/mbedtls/sha512.h +++ b/include/mbedtls/sha512.h @@ -272,7 +272,7 @@ int mbedtls_sha512_ret( const unsigned char *input, * \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. - * \param is384 Determines which function to use. This must be eiher + * \param is384 Determines which function to use. This must be either * \c 0 for SHA-512, or \c 1 for SHA-384. */ MBEDTLS_DEPRECATED void mbedtls_sha512( const unsigned char *input, From 9994e0d7cff6d185c958491dba72b50b6ddbd459 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 19 Dec 2018 09:55:40 +0000 Subject: [PATCH 10/12] Regenerate errors.c --- library/error.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/error.c b/library/error.c index 3251af06ce..ab7bfb88a2 100644 --- a/library/error.c +++ b/library/error.c @@ -866,7 +866,7 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen ) if( use_ret == -(MBEDTLS_ERR_SHA512_HW_ACCEL_FAILED) ) mbedtls_snprintf( buf, buflen, "SHA512 - SHA-512 hardware accelerator failed" ); if( use_ret == -(MBEDTLS_ERR_SHA512_BAD_INPUT_DATA) ) - mbedtls_snprintf( buf, buflen, "SHA512 - Invalid input data" ); + mbedtls_snprintf( buf, buflen, "SHA512 - SHA-512 input data was malformed" ); #endif /* MBEDTLS_SHA512_C */ #if defined(MBEDTLS_THREADING_C) From bb186f89fce29f6e0ad5587588529c273d925eda Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 19 Dec 2018 10:27:24 +0000 Subject: [PATCH 11/12] Weaken preconditions for mbedtls[_internal]_sha512_process() --- include/mbedtls/sha512.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/sha512.h b/include/mbedtls/sha512.h index 34a90b51ee..7b26cf5cc3 100644 --- a/include/mbedtls/sha512.h +++ b/include/mbedtls/sha512.h @@ -145,8 +145,7 @@ int mbedtls_sha512_finish_ret( mbedtls_sha512_context *ctx, * \brief This function processes a single data block within * the ongoing SHA-512 computation. * - * \param ctx The SHA-512 context. This must be initialized - * and have a hash operation started. + * \param ctx The SHA-512 context. This must be initialized. * \param data The buffer holding one block of data. This * must be a readable buffer of length \c 128 Bytes. * @@ -211,8 +210,7 @@ MBEDTLS_DEPRECATED void mbedtls_sha512_finish( mbedtls_sha512_context *ctx, * * \deprecated Superseded by mbedtls_internal_sha512_process() in 2.7.0. * - * \param ctx The SHA-512 context. This must be initialized and - * have a hash operation started. + * \param ctx The SHA-512 context. This must be initialized. * \param data The buffer holding one block of data. This must be * a readable buffer of length \c 128 Bytes. */ From c756049dc32d9845ec6b5c1f7b6b8b8467ce0b43 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 20 Dec 2018 10:23:39 +0000 Subject: [PATCH 12/12] Move SHA512_VALIDATE[_RET] outside of MBEDTLS_SHA512_ALT guard Somehow, mbedtls_sha512_ret() is defined even if MBEDTLS_SHA512_ALT is set, and it is using SHA512_VALIDATE_RET. The documentation should be enhanced to indicate that MBEDTLS_SHA512_ALT does _not_ replace the entire module, but only the core SHA-512 functions. --- library/sha512.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/sha512.c b/library/sha512.c index e7b2c5093c..941ecda762 100644 --- a/library/sha512.c +++ b/library/sha512.c @@ -55,6 +55,10 @@ #endif /* MBEDTLS_PLATFORM_C */ #endif /* MBEDTLS_SELF_TEST */ +#define SHA512_VALIDATE_RET(cond) \ + MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_SHA512_BAD_INPUT_DATA ) +#define SHA512_VALIDATE(cond) MBEDTLS_INTERNAL_VALIDATE( cond ) + #if !defined(MBEDTLS_SHA512_ALT) /* @@ -88,10 +92,6 @@ } #endif /* PUT_UINT64_BE */ -#define SHA512_VALIDATE_RET(cond) \ - MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_SHA512_BAD_INPUT_DATA ) -#define SHA512_VALIDATE(cond) MBEDTLS_INTERNAL_VALIDATE( cond ) - void mbedtls_sha512_init( mbedtls_sha512_context *ctx ) { SHA512_VALIDATE( ctx != NULL );