diff --git a/ChangeLog b/ChangeLog index de3b8ad98..b4aef4958 100644 --- a/ChangeLog +++ b/ChangeLog @@ -49,6 +49,8 @@ API Changes in favour of a new generic error MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA. * Add validation checks for input parameters to functions in the CCM module. * Add validation checks for input parameters to functions in the GCM module. + * 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/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/include/mbedtls/sha1.h b/include/mbedtls/sha1.h index bcaeab5eb..38ea10b13 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 /**< SHA-1 input data was malformed. */ #ifdef __cplusplus extern "C" { @@ -79,6 +80,7 @@ mbedtls_sha1_context; * stronger message digests instead. * * \param ctx The SHA-1 context to initialize. + * This must not be \c NULL. * */ void mbedtls_sha1_init( mbedtls_sha1_context *ctx ); @@ -90,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 ); @@ -102,8 +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. - * \param src The SHA-1 context to clone from. + * \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, @@ -116,9 +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. + * \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 ); @@ -131,11 +137,14 @@ 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. + * \param ctx The SHA-1 context. This must be initialized + * and have a hash operation started. * \param input The buffer holding the input data. - * \param ilen The length of the input data. + * This must be a readable buffer of length \p ilen Bytes. + * \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, @@ -149,10 +158,13 @@ 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. - * \param output The SHA-1 checksum result. + * \param ctx The SHA-1 context to use. This must be initialized and + * have a hash operation started. + * \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] ); @@ -164,10 +176,12 @@ 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. - * \param data The data block being processed. + * \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. * * \return \c 0 on success. + * \return A negative error code on failure. * */ int mbedtls_internal_sha1_process( mbedtls_sha1_context *ctx, @@ -188,7 +202,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. + * \param ctx The SHA-1 context to initialize. This must be initialized. * */ MBEDTLS_DEPRECATED void mbedtls_sha1_starts( mbedtls_sha1_context *ctx ); @@ -203,9 +217,11 @@ 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. + * \param ctx The SHA-1 context. This must be initialized and + * have a hash operation started. * \param input The buffer holding the input data. - * \param ilen The length of the input data. + * This must be a readable buffer of length \p ilen Bytes. + * \param ilen The length of the input data \p input in Bytes. * */ MBEDTLS_DEPRECATED void mbedtls_sha1_update( mbedtls_sha1_context *ctx, @@ -222,9 +238,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. + * \param ctx The SHA-1 context. This must be initialized and + * have a hash operation started. * \param output The SHA-1 checksum result. - * + * 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] ); @@ -238,8 +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. + * \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. * */ MBEDTLS_DEPRECATED void mbedtls_sha1_process( mbedtls_sha1_context *ctx, @@ -262,10 +280,13 @@ MBEDTLS_DEPRECATED void mbedtls_sha1_process( mbedtls_sha1_context *ctx, * stronger message digests instead. * * \param input The buffer holding the input data. - * \param ilen The length of the input data. + * This must be a readable buffer of length \p ilen Bytes. + * \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. * * \return \c 0 on success. + * \return A negative error code on failure. * */ int mbedtls_sha1_ret( const unsigned char *input, @@ -294,8 +315,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. - * \param ilen The length of the input data. - * \param output The SHA-1 checksum result. + * This must be a readable buffer of length \p ilen Bytes. + * \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, diff --git a/library/error.c b/library/error.c index 3be4175cc..94f7cc7b7 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 - SHA-1 input data was malformed" ); #endif /* MBEDTLS_SHA1_C */ #if defined(MBEDTLS_SHA256_C) diff --git a/library/sha1.c b/library/sha1.c index bab6087c4..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) /* @@ -73,6 +78,8 @@ void mbedtls_sha1_init( mbedtls_sha1_context *ctx ) { + 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 ) { + SHA1_VALIDATE( dst != NULL ); + 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 ) { + 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; + 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 ); GET_UINT32_BE( W[ 2], data, 8 ); @@ -294,6 +309,9 @@ int mbedtls_sha1_update_ret( mbedtls_sha1_context *ctx, size_t fill; uint32_t left; + SHA1_VALIDATE_RET( ctx != NULL ); + SHA1_VALIDATE_RET( ilen == 0 || input != NULL ); + if( ilen == 0 ) return( 0 ); @@ -352,6 +370,9 @@ int mbedtls_sha1_finish_ret( mbedtls_sha1_context *ctx, uint32_t used; uint32_t high, low; + SHA1_VALIDATE_RET( ctx != NULL ); + 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; + SHA1_VALIDATE_RET( ilen == 0 || input != NULL ); + SHA1_VALIDATE_RET( (unsigned char *)output != NULL ); + mbedtls_sha1_init( &ctx ); if( ( ret = mbedtls_sha1_starts_ret( &ctx ) ) != 0 ) 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..c035ae971 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 depends_on:MBEDTLS_SHA1_C */ +void sha1_valid_param( ) +{ + TEST_VALID_PARAM( mbedtls_sha1_free( NULL ) ); +} +/* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_SHA1_C: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 ) {