From f7c43b3145b2952a0bc0e5fe4584df4bf47fe67e Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Sun, 9 Dec 2018 19:12:19 +0000 Subject: [PATCH 01/13] Add parameter validation to SHA-1 --- ChangeLog | 2 ++ include/mbedtls/sha1.h | 1 + library/sha1.c | 24 ++++++++++++++++++++++++ 3 files changed, 27 insertions(+) diff --git a/ChangeLog b/ChangeLog index 66a8ce92f..39bfa795e 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-1 + module. New deprecations * Deprecate mbedtls_ctr_drbg_update and mbedtls_hmac_drbg_update diff --git a/include/mbedtls/sha1.h b/include/mbedtls/sha1.h index bcaeab5eb..96da3feee 100644 --- a/include/mbedtls/sha1.h +++ b/include/mbedtls/sha1.h @@ -42,6 +42,7 @@ /* MBEDTLS_ERR_SHA1_HW_ACCEL_FAILED is deprecated and should not be used. */ #define MBEDTLS_ERR_SHA1_HW_ACCEL_FAILED -0x0035 /**< SHA-1 hardware accelerator failed */ +#define MBEDTLS_ERR_SHA1_BAD_INPUT_DATA -0x0073 /**< Invalid input data. */ #ifdef __cplusplus extern "C" { diff --git a/library/sha1.c b/library/sha1.c index bab6087c4..e9521e391 100644 --- a/library/sha1.c +++ b/library/sha1.c @@ -71,8 +71,15 @@ } #endif +#define MBEDTLS_SHA1_VALIDATE_RET(cond) \ + MBEDTLS_VALIDATE_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA, cond ) + +#define MBEDTLS_SHA1_VALIDATE(cond) MBEDTLS_VALIDATE( cond ) + void mbedtls_sha1_init( mbedtls_sha1_context *ctx ) { + MBEDTLS_SHA1_VALIDATE( ctx != NULL ); + memset( ctx, 0, sizeof( mbedtls_sha1_context ) ); } @@ -87,6 +94,9 @@ void mbedtls_sha1_free( mbedtls_sha1_context *ctx ) void mbedtls_sha1_clone( mbedtls_sha1_context *dst, const mbedtls_sha1_context *src ) { + MBEDTLS_SHA1_VALIDATE( dst != NULL ); + MBEDTLS_SHA1_VALIDATE( src != NULL ); + *dst = *src; } @@ -95,6 +105,8 @@ void mbedtls_sha1_clone( mbedtls_sha1_context *dst, */ int mbedtls_sha1_starts_ret( mbedtls_sha1_context *ctx ) { + MBEDTLS_SHA1_VALIDATE_RET( ctx != NULL ); + ctx->total[0] = 0; ctx->total[1] = 0; @@ -120,6 +132,9 @@ int mbedtls_internal_sha1_process( mbedtls_sha1_context *ctx, { uint32_t temp, W[16], A, B, C, D, E; + MBEDTLS_SHA1_VALIDATE_RET( ctx != NULL ); + MBEDTLS_SHA1_VALIDATE_RET( (const unsigned char *)data != NULL ); + GET_UINT32_BE( W[ 0], data, 0 ); GET_UINT32_BE( W[ 1], data, 4 ); GET_UINT32_BE( W[ 2], data, 8 ); @@ -297,6 +312,9 @@ int mbedtls_sha1_update_ret( mbedtls_sha1_context *ctx, if( ilen == 0 ) return( 0 ); + MBEDTLS_SHA1_VALIDATE_RET( ctx != NULL ); + MBEDTLS_SHA1_VALIDATE_RET( input != NULL ); + left = ctx->total[0] & 0x3F; fill = 64 - left; @@ -352,6 +370,9 @@ int mbedtls_sha1_finish_ret( mbedtls_sha1_context *ctx, uint32_t used; uint32_t high, low; + MBEDTLS_SHA1_VALIDATE_RET( ctx != NULL ); + MBEDTLS_SHA1_VALIDATE_RET( (unsigned char *)output != NULL ); + /* * Add padding: 0x80 then 0x00 until 8 bytes remain for the length */ @@ -420,6 +441,9 @@ int mbedtls_sha1_ret( const unsigned char *input, int ret; mbedtls_sha1_context ctx; + MBEDTLS_SHA1_VALIDATE_RET( ilen == 0 || input != NULL ); + MBEDTLS_SHA1_VALIDATE_RET( (unsigned char *)output != NULL ); + mbedtls_sha1_init( &ctx ); if( ( ret = mbedtls_sha1_starts_ret( &ctx ) ) != 0 ) From a685d4f28dce5ad6aeba3308514b8a5b6008ca0b Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Sun, 9 Dec 2018 19:13:01 +0000 Subject: [PATCH 02/13] Add MBEDTLS_ERR_SHA1_BAD_INPUT_DATA to error.{h,c} --- include/mbedtls/error.h | 2 +- library/error.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/error.h b/include/mbedtls/error.h index 0c3888987..57bbfeb6e 100644 --- a/include/mbedtls/error.h +++ b/include/mbedtls/error.h @@ -74,7 +74,7 @@ * MD4 1 0x002D-0x002D * MD5 1 0x002F-0x002F * RIPEMD160 1 0x0031-0x0031 - * SHA1 1 0x0035-0x0035 + * SHA1 1 0x0035-0x0035 0x0073-0x0073 * SHA256 1 0x0037-0x0037 * SHA512 1 0x0039-0x0039 * CHACHA20 3 0x0051-0x0055 diff --git a/library/error.c b/library/error.c index eabee9e21..564490e58 100644 --- a/library/error.c +++ b/library/error.c @@ -855,6 +855,8 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen ) #if defined(MBEDTLS_SHA1_C) if( use_ret == -(MBEDTLS_ERR_SHA1_HW_ACCEL_FAILED) ) mbedtls_snprintf( buf, buflen, "SHA1 - SHA-1 hardware accelerator failed" ); + if( use_ret == -(MBEDTLS_ERR_SHA1_BAD_INPUT_DATA) ) + mbedtls_snprintf( buf, buflen, "SHA1 - Invalid input data" ); #endif /* MBEDTLS_SHA1_C */ #if defined(MBEDTLS_SHA256_C) From c523e011e0faeb6b4cd784a2c18c4b669de535fb Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Mon, 10 Dec 2018 10:11:47 +0000 Subject: [PATCH 03/13] Document valid function params for SHA-1 functions --- include/mbedtls/sha1.h | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/sha1.h b/include/mbedtls/sha1.h index 96da3feee..2d47f5309 100644 --- a/include/mbedtls/sha1.h +++ b/include/mbedtls/sha1.h @@ -42,7 +42,7 @@ /* MBEDTLS_ERR_SHA1_HW_ACCEL_FAILED is deprecated and should not be used. */ #define MBEDTLS_ERR_SHA1_HW_ACCEL_FAILED -0x0035 /**< SHA-1 hardware accelerator failed */ -#define MBEDTLS_ERR_SHA1_BAD_INPUT_DATA -0x0073 /**< Invalid input data. */ +#define MBEDTLS_ERR_SHA1_BAD_INPUT_DATA -0x0073 /**< SHA-1 input data was malformed. */ #ifdef __cplusplus extern "C" { @@ -80,6 +80,7 @@ mbedtls_sha1_context; * stronger message digests instead. * * \param ctx The SHA-1 context to initialize. + * Must not be \c NULL. * */ void mbedtls_sha1_init( mbedtls_sha1_context *ctx ); @@ -104,7 +105,9 @@ void mbedtls_sha1_free( mbedtls_sha1_context *ctx ); * stronger message digests instead. * * \param dst The SHA-1 context to clone to. + * Must not be \c NULL. * \param src The SHA-1 context to clone from. + * Must not be \c NULL. * */ void mbedtls_sha1_clone( mbedtls_sha1_context *dst, @@ -118,6 +121,7 @@ void mbedtls_sha1_clone( mbedtls_sha1_context *dst, * stronger message digests instead. * * \param ctx The SHA-1 context to initialize. + * Must not be \c NULL. * * \return \c 0 on success. * @@ -133,7 +137,9 @@ int mbedtls_sha1_starts_ret( mbedtls_sha1_context *ctx ); * stronger message digests instead. * * \param ctx The SHA-1 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. @@ -151,7 +157,9 @@ int mbedtls_sha1_update_ret( mbedtls_sha1_context *ctx, * stronger message digests instead. * * \param ctx The SHA-1 context. + * Must not be \c NULL. * \param output The SHA-1 checksum result. + * Must not be \c NULL. * * \return \c 0 on success. */ @@ -166,7 +174,9 @@ int mbedtls_sha1_finish_ret( mbedtls_sha1_context *ctx, * stronger message digests instead. * * \param ctx The SHA-1 context. + * Must not be \c NULL. * \param data The data block being processed. + * Must not be \c NULL. * * \return \c 0 on success. * @@ -190,6 +200,7 @@ int mbedtls_internal_sha1_process( mbedtls_sha1_context *ctx, * \deprecated Superseded by mbedtls_sha1_starts_ret() in 2.7.0. * * \param ctx The SHA-1 context to initialize. + * Must not be \c NULL. * */ MBEDTLS_DEPRECATED void mbedtls_sha1_starts( mbedtls_sha1_context *ctx ); @@ -205,7 +216,9 @@ MBEDTLS_DEPRECATED void mbedtls_sha1_starts( mbedtls_sha1_context *ctx ); * \deprecated Superseded by mbedtls_sha1_update_ret() in 2.7.0. * * \param ctx The SHA-1 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. * */ @@ -224,7 +237,9 @@ MBEDTLS_DEPRECATED void mbedtls_sha1_update( mbedtls_sha1_context *ctx, * \deprecated Superseded by mbedtls_sha1_finish_ret() in 2.7.0. * * \param ctx The SHA-1 context. + * Must not be \c NULL. * \param output The SHA-1 checksum result. + * Must not be \c NULL. * */ MBEDTLS_DEPRECATED void mbedtls_sha1_finish( mbedtls_sha1_context *ctx, @@ -240,7 +255,9 @@ MBEDTLS_DEPRECATED void mbedtls_sha1_finish( mbedtls_sha1_context *ctx, * \deprecated Superseded by mbedtls_internal_sha1_process() in 2.7.0. * * \param ctx The SHA-1 context. + * Must not be \c NULL. * \param data The data block being processed. + * Must not be \c NULL. * */ MBEDTLS_DEPRECATED void mbedtls_sha1_process( mbedtls_sha1_context *ctx, @@ -263,8 +280,10 @@ MBEDTLS_DEPRECATED void mbedtls_sha1_process( mbedtls_sha1_context *ctx, * stronger message digests instead. * * \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-1 checksum result. + * Must not be \c NULL. * * \return \c 0 on success. * @@ -295,8 +314,10 @@ int mbedtls_sha1_ret( const unsigned char *input, * \deprecated Superseded by mbedtls_sha1_ret() in 2.7.0 * * \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-1 checksum result. + * Must not be \c NULL. * */ MBEDTLS_DEPRECATED void mbedtls_sha1( const unsigned char *input, From 5359ca8a549a1ee2b99048cd56590fd355af9009 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 18 Dec 2018 11:11:18 +0000 Subject: [PATCH 04/13] Improve SHA-1 documentation --- include/mbedtls/sha1.h | 88 +++++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 40 deletions(-) diff --git a/include/mbedtls/sha1.h b/include/mbedtls/sha1.h index 2d47f5309..801759af3 100644 --- a/include/mbedtls/sha1.h +++ b/include/mbedtls/sha1.h @@ -80,7 +80,7 @@ mbedtls_sha1_context; * stronger message digests instead. * * \param ctx The SHA-1 context to initialize. - * Must not be \c NULL. + * This must not be \c NULL. * */ void mbedtls_sha1_init( mbedtls_sha1_context *ctx ); @@ -92,7 +92,10 @@ void mbedtls_sha1_init( mbedtls_sha1_context *ctx ); * constitutes a security risk. We recommend considering * stronger message digests instead. * - * \param ctx The SHA-1 context to clear. + * \param ctx The SHA-1 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-1 context. * */ void mbedtls_sha1_free( mbedtls_sha1_context *ctx ); @@ -104,10 +107,8 @@ void mbedtls_sha1_free( mbedtls_sha1_context *ctx ); * constitutes a security risk. We recommend considering * stronger message digests instead. * - * \param dst The SHA-1 context to clone to. - * Must not be \c NULL. - * \param src The SHA-1 context to clone from. - * Must not be \c NULL. + * \param dst The SHA-1 context to clone to. This must be initialized. + * \param src The SHA-1 context to clone from. This must be initialized. * */ void mbedtls_sha1_clone( mbedtls_sha1_context *dst, @@ -120,10 +121,10 @@ void mbedtls_sha1_clone( mbedtls_sha1_context *dst, * constitutes a security risk. We recommend considering * stronger message digests instead. * - * \param ctx The SHA-1 context to initialize. - * Must not be \c NULL. + * \param ctx The SHA-1 context to initialize. This must be initialized. * * \return \c 0 on success. + * \return A negative error code on failure. * */ int mbedtls_sha1_starts_ret( mbedtls_sha1_context *ctx ); @@ -136,13 +137,15 @@ int mbedtls_sha1_starts_ret( mbedtls_sha1_context *ctx ); * constitutes a security risk. We recommend considering * stronger message digests instead. * - * \param ctx The SHA-1 context. - * Must not be \c NULL. + * \param ctx The SHA-1 context. This must be initialized + * and have a hash operation started. * \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. + * This must be a readable buffer of length \p ilen Bytes. + * It may be \c NULL if \p ilen is zero. + * \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_sha1_update_ret( mbedtls_sha1_context *ctx, const unsigned char *input, @@ -156,12 +159,14 @@ int mbedtls_sha1_update_ret( mbedtls_sha1_context *ctx, * constitutes a security risk. We recommend considering * stronger message digests instead. * - * \param ctx The SHA-1 context. - * Must not be \c NULL. - * \param output The SHA-1 checksum result. - * Must not be \c NULL. + * \param ctx The SHA-1 context to use. This must be initialized and + * have a hash operation started. + * This must not be \c NULL. + * \param output The SHA-1 checksum result. This must be a writable + * buffer of length \c 20 Bytes. * * \return \c 0 on success. + * \return A negative error code on failure. */ int mbedtls_sha1_finish_ret( mbedtls_sha1_context *ctx, unsigned char output[20] ); @@ -173,12 +178,13 @@ int mbedtls_sha1_finish_ret( mbedtls_sha1_context *ctx, * constitutes a security risk. We recommend considering * stronger message digests instead. * - * \param ctx The SHA-1 context. - * Must not be \c NULL. - * \param data The data block being processed. - * Must not be \c NULL. + * \param ctx The SHA-1 context to use. This must be initialized and + * have a hash operation started. + * \param data The data block being processed. This must be a + * readable buffer of length \c 64 Bytes. * * \return \c 0 on success. + * \return A negative error code on failure. * */ int mbedtls_internal_sha1_process( mbedtls_sha1_context *ctx, @@ -199,8 +205,7 @@ int mbedtls_internal_sha1_process( mbedtls_sha1_context *ctx, * * \deprecated Superseded by mbedtls_sha1_starts_ret() in 2.7.0. * - * \param ctx The SHA-1 context to initialize. - * Must not be \c NULL. + * \param ctx The SHA-1 context to initialize. This must be initialized. * */ MBEDTLS_DEPRECATED void mbedtls_sha1_starts( mbedtls_sha1_context *ctx ); @@ -215,11 +220,12 @@ MBEDTLS_DEPRECATED void mbedtls_sha1_starts( mbedtls_sha1_context *ctx ); * * \deprecated Superseded by mbedtls_sha1_update_ret() in 2.7.0. * - * \param ctx The SHA-1 context. - * Must not be \c NULL. + * \param ctx The SHA-1 context. THis must be initialized and + * have a hash operation started. * \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. + * This must be a readable buffer of length \p ilen Bytes. + * It may be \c NULL if \p ilen is zero. + * \param ilen The length of the input data \p input in Bytes. * */ MBEDTLS_DEPRECATED void mbedtls_sha1_update( mbedtls_sha1_context *ctx, @@ -236,11 +242,10 @@ MBEDTLS_DEPRECATED void mbedtls_sha1_update( mbedtls_sha1_context *ctx, * * \deprecated Superseded by mbedtls_sha1_finish_ret() in 2.7.0. * - * \param ctx The SHA-1 context. - * Must not be \c NULL. + * \param ctx The SHA-1 context. This must be initialized and + * have a hash operation started. * \param output The SHA-1 checksum result. - * Must not be \c NULL. - * + * This must be a writable buffer of length \c 20 Bytes. */ MBEDTLS_DEPRECATED void mbedtls_sha1_finish( mbedtls_sha1_context *ctx, unsigned char output[20] ); @@ -254,10 +259,10 @@ MBEDTLS_DEPRECATED void mbedtls_sha1_finish( mbedtls_sha1_context *ctx, * * \deprecated Superseded by mbedtls_internal_sha1_process() in 2.7.0. * - * \param ctx The SHA-1 context. - * Must not be \c NULL. + * \param ctx The SHA-1 context. This must be initialized and + * have a hash operation started. * \param data The data block being processed. - * Must not be \c NULL. + * This must be a readable buffer of length \c 64 bytes. * */ MBEDTLS_DEPRECATED void mbedtls_sha1_process( mbedtls_sha1_context *ctx, @@ -280,12 +285,14 @@ MBEDTLS_DEPRECATED void mbedtls_sha1_process( mbedtls_sha1_context *ctx, * stronger message digests instead. * * \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. + * This must be a readable buffer of length \p ilen Bytes. + * It may be \c NULL if \p ilen is zero. + * \param ilen The length of the input data \p input in Bytes. * \param output The SHA-1 checksum result. - * Must not be \c NULL. + * This must be a writable buffer of length \c 20 Bytes. * * \return \c 0 on success. + * \return A negative error code on failure. * */ int mbedtls_sha1_ret( const unsigned char *input, @@ -314,10 +321,11 @@ int mbedtls_sha1_ret( const unsigned char *input, * \deprecated Superseded by mbedtls_sha1_ret() in 2.7.0 * * \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-1 checksum result. - * Must not be \c NULL. + * This must be a readable buffer of length \p ilen Bytes. + * It may be \c NULL if \p ilen is zero. + * \param ilen The length of the input data \p input in Bytes. + * \param output The SHA-1 checksum result. This must be a writable + * buffer of size \c 20 Bytes. * */ MBEDTLS_DEPRECATED void mbedtls_sha1( const unsigned char *input, From 0e24473b943792706a4ac669ac16d6444615c2a6 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 18 Dec 2018 11:22:55 +0000 Subject: [PATCH 05/13] Test parameter validation in SHA-1 module --- tests/suites/test_suite_shax.data | 6 ++++ tests/suites/test_suite_shax.function | 47 +++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/tests/suites/test_suite_shax.data b/tests/suites/test_suite_shax.data index ee8074dc0..5bd70af8a 100644 --- a/tests/suites/test_suite_shax.data +++ b/tests/suites/test_suite_shax.data @@ -1,3 +1,9 @@ +SHA-1 - Valid parameters +sha1_valid_param: + +SHA-1 - Invalid parameters +sha1_invalid_param: + # Test the operation of SHA-1 and SHA-2 SHA-1 Test Vector NIST CAVS #1 depends_on:MBEDTLS_SHA1_C diff --git a/tests/suites/test_suite_shax.function b/tests/suites/test_suite_shax.function index 147ae0e1f..263759d29 100644 --- a/tests/suites/test_suite_shax.function +++ b/tests/suites/test_suite_shax.function @@ -4,6 +4,53 @@ #include "mbedtls/sha512.h" /* END_HEADER */ +/* BEGIN_CASE */ +void sha1_valid_param( ) +{ + TEST_VALID_PARAM( mbedtls_sha1_free( NULL ) ); +} +/* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */ +void sha1_invalid_param( ) +{ + mbedtls_sha1_context ctx; + unsigned char buf[64] = { 0 }; + size_t const buflen = sizeof( buf ); + + TEST_INVALID_PARAM( mbedtls_sha1_init( NULL ) ); + + TEST_INVALID_PARAM( mbedtls_sha1_clone( NULL, &ctx ) ); + TEST_INVALID_PARAM( mbedtls_sha1_clone( &ctx, NULL ) ); + + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA, + mbedtls_sha1_starts_ret( NULL ) ); + + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA, + mbedtls_sha1_update_ret( NULL, buf, buflen ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA, + mbedtls_sha1_update_ret( &ctx, NULL, buflen ) ); + + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA, + mbedtls_sha1_finish_ret( NULL, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA, + mbedtls_sha1_finish_ret( &ctx, NULL ) ); + + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA, + mbedtls_internal_sha1_process( NULL, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA, + mbedtls_internal_sha1_process( &ctx, NULL ) ); + + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA, + mbedtls_sha1_ret( NULL, buflen, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA, + mbedtls_sha1_ret( buf, buflen, NULL ) ); + +exit: + return; +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_SHA1_C */ void mbedtls_sha1( data_t * src_str, data_t * hex_hash_string ) { From b3906d8829a44b850bd8b908f03b546d6a65aa6a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 18 Dec 2018 11:35:00 +0000 Subject: [PATCH 06/13] Minor fixes to implementation of SHA1 parameter validation --- library/sha1.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/sha1.c b/library/sha1.c index e9521e391..0eaedcfe6 100644 --- a/library/sha1.c +++ b/library/sha1.c @@ -72,9 +72,9 @@ #endif #define MBEDTLS_SHA1_VALIDATE_RET(cond) \ - MBEDTLS_VALIDATE_RET( MBEDTLS_ERR_SHA1_BAD_INPUT_DATA, cond ) + MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_SHA1_BAD_INPUT_DATA ) -#define MBEDTLS_SHA1_VALIDATE(cond) MBEDTLS_VALIDATE( cond ) +#define MBEDTLS_SHA1_VALIDATE(cond) MBEDTLS_INTERNAL_VALIDATE( cond ) void mbedtls_sha1_init( mbedtls_sha1_context *ctx ) { @@ -309,12 +309,12 @@ int mbedtls_sha1_update_ret( mbedtls_sha1_context *ctx, size_t fill; uint32_t left; + MBEDTLS_SHA1_VALIDATE_RET( ctx != NULL ); + MBEDTLS_SHA1_VALIDATE_RET( ilen == 0 || input != NULL ); + if( ilen == 0 ) return( 0 ); - MBEDTLS_SHA1_VALIDATE_RET( ctx != NULL ); - MBEDTLS_SHA1_VALIDATE_RET( input != NULL ); - left = ctx->total[0] & 0x3F; fill = 64 - left; From 9171c6e9ec495a6b33437cd148ccd00da2860f7f Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 18 Dec 2018 15:42:53 +0000 Subject: [PATCH 07/13] Leave behaviour on NULL buffers to SHA-1 unspecified for now We deal correctly with NULL being passed alongside a zero length argument, but don't have tests for it, so we shouldn't promise that it works. --- include/mbedtls/sha1.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/include/mbedtls/sha1.h b/include/mbedtls/sha1.h index 801759af3..0979dc0e3 100644 --- a/include/mbedtls/sha1.h +++ b/include/mbedtls/sha1.h @@ -141,7 +141,6 @@ int mbedtls_sha1_starts_ret( mbedtls_sha1_context *ctx ); * 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 may be \c NULL if \p ilen is zero. * \param ilen The length of the input data \p input in Bytes. * * \return \c 0 on success. @@ -224,7 +223,6 @@ MBEDTLS_DEPRECATED void mbedtls_sha1_starts( mbedtls_sha1_context *ctx ); * 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 may be \c NULL if \p ilen is zero. * \param ilen The length of the input data \p input in Bytes. * */ @@ -286,7 +284,6 @@ MBEDTLS_DEPRECATED void mbedtls_sha1_process( mbedtls_sha1_context *ctx, * * \param input The buffer holding the input data. * This must be a readable buffer of length \p ilen Bytes. - * It may be \c NULL if \p ilen is zero. * \param ilen The length of the input data \p input in Bytes. * \param output The SHA-1 checksum result. * This must be a writable buffer of length \c 20 Bytes. @@ -322,7 +319,6 @@ int mbedtls_sha1_ret( const unsigned char *input, * * \param input The buffer holding the input data. * This must be a readable buffer of length \p ilen Bytes. - * It may be \c NULL if \p ilen is zero. * \param ilen The length of the input data \p input in Bytes. * \param output The SHA-1 checksum result. This must be a writable * buffer of size \c 20 Bytes. From d22df58a56f9e97b7f3341fd0f97ff64ac125cf0 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 18 Dec 2018 16:39:45 +0000 Subject: [PATCH 08/13] Add missing guards around SHA-1 tests --- tests/suites/test_suite_shax.function | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_shax.function b/tests/suites/test_suite_shax.function index 263759d29..c035ae971 100644 --- a/tests/suites/test_suite_shax.function +++ b/tests/suites/test_suite_shax.function @@ -4,14 +4,14 @@ #include "mbedtls/sha512.h" /* END_HEADER */ -/* BEGIN_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_SHA1_C */ void sha1_valid_param( ) { TEST_VALID_PARAM( mbedtls_sha1_free( NULL ) ); } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */ +/* BEGIN_CASE depends_on:MBEDTLS_SHA1_C:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */ void sha1_invalid_param( ) { mbedtls_sha1_context ctx; From 039ccab243801744e67c3d52b30c8dcdfaabacec Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 18 Dec 2018 17:52:14 +0000 Subject: [PATCH 09/13] Don't declare MBEDTLS-namespace macros in sha1.c --- library/sha1.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/library/sha1.c b/library/sha1.c index 0eaedcfe6..8863ea385 100644 --- a/library/sha1.c +++ b/library/sha1.c @@ -71,14 +71,14 @@ } #endif -#define MBEDTLS_SHA1_VALIDATE_RET(cond) \ +#define SHA1_VALIDATE_RET(cond) \ MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_SHA1_BAD_INPUT_DATA ) -#define MBEDTLS_SHA1_VALIDATE(cond) MBEDTLS_INTERNAL_VALIDATE( cond ) +#define SHA1_VALIDATE(cond) MBEDTLS_INTERNAL_VALIDATE( cond ) void mbedtls_sha1_init( mbedtls_sha1_context *ctx ) { - MBEDTLS_SHA1_VALIDATE( ctx != NULL ); + SHA1_VALIDATE( ctx != NULL ); memset( ctx, 0, sizeof( mbedtls_sha1_context ) ); } @@ -94,8 +94,8 @@ void mbedtls_sha1_free( mbedtls_sha1_context *ctx ) void mbedtls_sha1_clone( mbedtls_sha1_context *dst, const mbedtls_sha1_context *src ) { - MBEDTLS_SHA1_VALIDATE( dst != NULL ); - MBEDTLS_SHA1_VALIDATE( src != NULL ); + SHA1_VALIDATE( dst != NULL ); + SHA1_VALIDATE( src != NULL ); *dst = *src; } @@ -105,7 +105,7 @@ void mbedtls_sha1_clone( mbedtls_sha1_context *dst, */ int mbedtls_sha1_starts_ret( mbedtls_sha1_context *ctx ) { - MBEDTLS_SHA1_VALIDATE_RET( ctx != NULL ); + SHA1_VALIDATE_RET( ctx != NULL ); ctx->total[0] = 0; ctx->total[1] = 0; @@ -132,8 +132,8 @@ int mbedtls_internal_sha1_process( mbedtls_sha1_context *ctx, { uint32_t temp, W[16], A, B, C, D, E; - MBEDTLS_SHA1_VALIDATE_RET( ctx != NULL ); - MBEDTLS_SHA1_VALIDATE_RET( (const unsigned char *)data != NULL ); + SHA1_VALIDATE_RET( ctx != NULL ); + SHA1_VALIDATE_RET( (const unsigned char *)data != NULL ); GET_UINT32_BE( W[ 0], data, 0 ); GET_UINT32_BE( W[ 1], data, 4 ); @@ -309,8 +309,8 @@ int mbedtls_sha1_update_ret( mbedtls_sha1_context *ctx, size_t fill; uint32_t left; - MBEDTLS_SHA1_VALIDATE_RET( ctx != NULL ); - MBEDTLS_SHA1_VALIDATE_RET( ilen == 0 || input != NULL ); + SHA1_VALIDATE_RET( ctx != NULL ); + SHA1_VALIDATE_RET( ilen == 0 || input != NULL ); if( ilen == 0 ) return( 0 ); @@ -370,8 +370,8 @@ int mbedtls_sha1_finish_ret( mbedtls_sha1_context *ctx, uint32_t used; uint32_t high, low; - MBEDTLS_SHA1_VALIDATE_RET( ctx != NULL ); - MBEDTLS_SHA1_VALIDATE_RET( (unsigned char *)output != NULL ); + SHA1_VALIDATE_RET( ctx != NULL ); + SHA1_VALIDATE_RET( (unsigned char *)output != NULL ); /* * Add padding: 0x80 then 0x00 until 8 bytes remain for the length @@ -441,8 +441,8 @@ int mbedtls_sha1_ret( const unsigned char *input, int ret; mbedtls_sha1_context ctx; - MBEDTLS_SHA1_VALIDATE_RET( ilen == 0 || input != NULL ); - MBEDTLS_SHA1_VALIDATE_RET( (unsigned char *)output != NULL ); + SHA1_VALIDATE_RET( ilen == 0 || input != NULL ); + SHA1_VALIDATE_RET( (unsigned char *)output != NULL ); mbedtls_sha1_init( &ctx ); From 42f783d3b73f0d64c154bbbcd2ab25cb114899ce Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 18 Dec 2018 18:39:32 +0000 Subject: [PATCH 10/13] Fix minor issues in SHA1 documentation --- include/mbedtls/sha1.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/sha1.h b/include/mbedtls/sha1.h index 0979dc0e3..2c4f0a6ae 100644 --- a/include/mbedtls/sha1.h +++ b/include/mbedtls/sha1.h @@ -160,7 +160,6 @@ int mbedtls_sha1_update_ret( mbedtls_sha1_context *ctx, * * \param ctx The SHA-1 context to use. This must be initialized and * have a hash operation started. - * This must not be \c NULL. * \param output The SHA-1 checksum result. This must be a writable * buffer of length \c 20 Bytes. * @@ -177,8 +176,8 @@ int mbedtls_sha1_finish_ret( mbedtls_sha1_context *ctx, * constitutes a security risk. We recommend considering * stronger message digests instead. * - * \param ctx The SHA-1 context to use. This must be initialized and - * have a hash operation started. + * \param ctx The SHA-1 context to use. This must be initialized + * and have a hash operation started. * \param data The data block being processed. This must be a * readable buffer of length \c 64 Bytes. * @@ -219,7 +218,7 @@ MBEDTLS_DEPRECATED void mbedtls_sha1_starts( mbedtls_sha1_context *ctx ); * * \deprecated Superseded by mbedtls_sha1_update_ret() in 2.7.0. * - * \param ctx The SHA-1 context. THis must be initialized and + * \param ctx The SHA-1 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. From 79b9e39732a168292b631c8d8678b48d92c785bb Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 18 Dec 2018 23:17:49 +0000 Subject: [PATCH 11/13] Weaken preconditions for mbedtls[_internal]_sha1_process() --- include/mbedtls/sha1.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/sha1.h b/include/mbedtls/sha1.h index 2c4f0a6ae..38ea10b13 100644 --- a/include/mbedtls/sha1.h +++ b/include/mbedtls/sha1.h @@ -176,8 +176,7 @@ int mbedtls_sha1_finish_ret( mbedtls_sha1_context *ctx, * constitutes a security risk. We recommend considering * stronger message digests instead. * - * \param ctx The SHA-1 context to use. This must be initialized - * and have a hash operation started. + * \param ctx The SHA-1 context to use. This must be initialized. * \param data The data block being processed. This must be a * readable buffer of length \c 64 Bytes. * @@ -256,8 +255,7 @@ MBEDTLS_DEPRECATED void mbedtls_sha1_finish( mbedtls_sha1_context *ctx, * * \deprecated Superseded by mbedtls_internal_sha1_process() in 2.7.0. * - * \param ctx The SHA-1 context. This must be initialized and - * have a hash operation started. + * \param ctx The SHA-1 context. This must be initialized. * \param data The data block being processed. * This must be a readable buffer of length \c 64 bytes. * From 859522a31ce0cd46252cabd47a24664b6b8a4468 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 19 Dec 2018 09:54:14 +0000 Subject: [PATCH 12/13] 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 564490e58..097260c26 100644 --- a/library/error.c +++ b/library/error.c @@ -856,7 +856,7 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen ) if( use_ret == -(MBEDTLS_ERR_SHA1_HW_ACCEL_FAILED) ) mbedtls_snprintf( buf, buflen, "SHA1 - SHA-1 hardware accelerator failed" ); if( use_ret == -(MBEDTLS_ERR_SHA1_BAD_INPUT_DATA) ) - mbedtls_snprintf( buf, buflen, "SHA1 - Invalid input data" ); + mbedtls_snprintf( buf, buflen, "SHA1 - SHA-1 input data was malformed" ); #endif /* MBEDTLS_SHA1_C */ #if defined(MBEDTLS_SHA256_C) From b3c70230d23ae8dff73e552a7f6820ab8335ea12 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 20 Dec 2018 10:18:05 +0000 Subject: [PATCH 13/13] Move SHA1_VALIDATE[_RET] outside of MBEDTLS_SHA1_ALT guard Somehow, mbedtls_sha1_ret() is defined even if MBEDTLS_SHA1_ALT is set, and it is using SHA1_VALIDATE_RET. The documentation should be enhanced to indicate that MBEDTLS_SHA1_ALT does _not_ replace the entire module, but only the core SHA-1 functions. --- library/sha1.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/sha1.c b/library/sha1.c index 8863ea385..e8d4096fb 100644 --- a/library/sha1.c +++ b/library/sha1.c @@ -46,6 +46,11 @@ #endif /* MBEDTLS_PLATFORM_C */ #endif /* MBEDTLS_SELF_TEST */ +#define SHA1_VALIDATE_RET(cond) \ + MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_SHA1_BAD_INPUT_DATA ) + +#define SHA1_VALIDATE(cond) MBEDTLS_INTERNAL_VALIDATE( cond ) + #if !defined(MBEDTLS_SHA1_ALT) /* @@ -71,11 +76,6 @@ } #endif -#define SHA1_VALIDATE_RET(cond) \ - MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_SHA1_BAD_INPUT_DATA ) - -#define SHA1_VALIDATE(cond) MBEDTLS_INTERNAL_VALIDATE( cond ) - void mbedtls_sha1_init( mbedtls_sha1_context *ctx ) { SHA1_VALIDATE( ctx != NULL );