From 733676b97894cda2bc2bb565f32361e80a41df04 Mon Sep 17 00:00:00 2001 From: Nicholas Wilson Date: Sat, 14 Nov 2015 13:09:01 +0000 Subject: [PATCH 01/15] Allow test suites to be run on Windows For a start, they don't even compile with Visual Studio due to strcasecmp being missing. Secondly, on Windows Perl scripts aren't executable and have to be run using the Perl interpreter directly; thankfully CMake is able to find cygwin Perl straight away without problems. --- tests/CMakeLists.txt | 7 ++++++- tests/suites/helpers.function | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1cca81830..23eb2a432 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -10,6 +10,11 @@ if(ENABLE_ZLIB_SUPPORT) set(libs ${libs} ${ZLIB_LIBRARIES}) endif(ENABLE_ZLIB_SUPPORT) +find_package(Perl) +if(NOT PERL_FOUND) + message(FATAL_ERROR "Cannot build test suites without Perl") +endif() + function(add_test_suite suite_name) if(ARGV1) set(data_name ${ARGV1}) @@ -19,7 +24,7 @@ function(add_test_suite suite_name) add_custom_command( OUTPUT test_suite_${data_name}.c - COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/scripts/generate_code.pl ${CMAKE_CURRENT_SOURCE_DIR}/suites test_suite_${suite_name} test_suite_${data_name} + COMMAND ${PERL_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/scripts/generate_code.pl ${CMAKE_CURRENT_SOURCE_DIR}/suites test_suite_${suite_name} test_suite_${data_name} DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/scripts/generate_code.pl mbedtls suites/helpers.function suites/main_test.function suites/test_suite_${suite_name}.function suites/test_suite_${data_name}.data ) diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index 8f681dbd4..6af918cad 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -15,6 +15,8 @@ #ifdef _MSC_VER #include typedef UINT32 uint32_t; +#define strncasecmp _strnicmp +#define strcasecmp _stricmp #else #include #endif From 6c8edca2d41580e1ca9dcaf8fd4fba70670a19c9 Mon Sep 17 00:00:00 2001 From: James Cowgill Date: Thu, 17 Dec 2015 01:40:26 +0000 Subject: [PATCH 02/15] Fix build errors on x32 by using the generic 'add' instruction On x32 systems, pointers are 4-bytes wide and are therefore stored in %e?x registers (instead of %r?x registers). These registers must be accessed using "addl" instead of "addq", however the GNU assembler will acccept the generic "add" instruction and determine the correct opcode based on the registers passed to it. --- library/aesni.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/aesni.c b/library/aesni.c index 83a5868bd..1ca3c3ef5 100644 --- a/library/aesni.c +++ b/library/aesni.c @@ -100,7 +100,7 @@ int mbedtls_aesni_crypt_ecb( mbedtls_aes_context *ctx, asm( "movdqu (%3), %%xmm0 \n\t" // load input "movdqu (%1), %%xmm1 \n\t" // load round key 0 "pxor %%xmm1, %%xmm0 \n\t" // round 0 - "addq $16, %1 \n\t" // point to next round key + "add $16, %1 \n\t" // point to next round key "subl $1, %0 \n\t" // normal rounds = nr - 1 "test %2, %2 \n\t" // mode? "jz 2f \n\t" // 0 = decrypt @@ -108,7 +108,7 @@ int mbedtls_aesni_crypt_ecb( mbedtls_aes_context *ctx, "1: \n\t" // encryption loop "movdqu (%1), %%xmm1 \n\t" // load round key AESENC xmm1_xmm0 "\n\t" // do round - "addq $16, %1 \n\t" // point to next round key + "add $16, %1 \n\t" // point to next round key "subl $1, %0 \n\t" // loop "jnz 1b \n\t" "movdqu (%1), %%xmm1 \n\t" // load round key @@ -118,7 +118,7 @@ int mbedtls_aesni_crypt_ecb( mbedtls_aes_context *ctx, "2: \n\t" // decryption loop "movdqu (%1), %%xmm1 \n\t" AESDEC xmm1_xmm0 "\n\t" // do round - "addq $16, %1 \n\t" + "add $16, %1 \n\t" "subl $1, %0 \n\t" "jnz 2b \n\t" "movdqu (%1), %%xmm1 \n\t" // load round key From 21e402a3aef100414546ac77a8093ea7c0e917c0 Mon Sep 17 00:00:00 2001 From: James Cowgill Date: Thu, 17 Dec 2015 01:51:09 +0000 Subject: [PATCH 03/15] Fix segfault on x32 by using better register constraints in bn_mul.h On x32, pointers are only 4-bytes wide and need to be loaded using the "movl" instruction instead of "movq" to avoid loading garbage into the register. The MULADDC routines for x86-64 are adjusted to work on x32 as well by getting gcc to load all the registers for us in advance (and storing them later) by using better register constraints. The b, c, D and S constraints correspond to the rbx, rcx, rdi and rsi registers respectively. --- include/mbedtls/bn_mul.h | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/include/mbedtls/bn_mul.h b/include/mbedtls/bn_mul.h index 5408d4146..71dd672c2 100644 --- a/include/mbedtls/bn_mul.h +++ b/include/mbedtls/bn_mul.h @@ -162,10 +162,6 @@ #define MULADDC_INIT \ asm( \ - "movq %3, %%rsi \n\t" \ - "movq %4, %%rdi \n\t" \ - "movq %5, %%rcx \n\t" \ - "movq %6, %%rbx \n\t" \ "xorq %%r8, %%r8 \n\t" #define MULADDC_CORE \ @@ -181,12 +177,9 @@ "addq $8, %%rdi \n\t" #define MULADDC_STOP \ - "movq %%rcx, %0 \n\t" \ - "movq %%rdi, %1 \n\t" \ - "movq %%rsi, %2 \n\t" \ - : "=m" (c), "=m" (d), "=m" (s) \ - : "m" (s), "m" (d), "m" (c), "m" (b) \ - : "rax", "rcx", "rdx", "rbx", "rsi", "rdi", "r8" \ + : "+c" (c), "+D" (d), "+S" (s) \ + : "b" (b) \ + : "rax", "rdx", "r8" \ ); #endif /* AMD64 */ From 00b78a9c54ab2c5c2941d395fd3202272d148b88 Mon Sep 17 00:00:00 2001 From: Alexey Skalozub Date: Wed, 13 Jan 2016 17:39:58 +0200 Subject: [PATCH 04/15] Move K inside MBEDTLS_SHA512_PROCESS_ALT block It is used only by `mbedtls_sha512_process()`, and in case `MBEDTLS_SHA512_PROCESS_ALT` is defined, it still cannot be reused because of `static` declaration. --- library/sha512.c | 95 ++++++++++++++++++++++++------------------------ 1 file changed, 48 insertions(+), 47 deletions(-) diff --git a/library/sha512.c b/library/sha512.c index af610bb43..0f9e1e535 100644 --- a/library/sha512.c +++ b/library/sha512.c @@ -89,53 +89,6 @@ static void mbedtls_zeroize( void *v, size_t n ) { } #endif /* PUT_UINT64_BE */ -/* - * Round constants - */ -static const uint64_t K[80] = -{ - UL64(0x428A2F98D728AE22), UL64(0x7137449123EF65CD), - UL64(0xB5C0FBCFEC4D3B2F), UL64(0xE9B5DBA58189DBBC), - UL64(0x3956C25BF348B538), UL64(0x59F111F1B605D019), - UL64(0x923F82A4AF194F9B), UL64(0xAB1C5ED5DA6D8118), - UL64(0xD807AA98A3030242), UL64(0x12835B0145706FBE), - UL64(0x243185BE4EE4B28C), UL64(0x550C7DC3D5FFB4E2), - UL64(0x72BE5D74F27B896F), UL64(0x80DEB1FE3B1696B1), - UL64(0x9BDC06A725C71235), UL64(0xC19BF174CF692694), - UL64(0xE49B69C19EF14AD2), UL64(0xEFBE4786384F25E3), - UL64(0x0FC19DC68B8CD5B5), UL64(0x240CA1CC77AC9C65), - UL64(0x2DE92C6F592B0275), UL64(0x4A7484AA6EA6E483), - UL64(0x5CB0A9DCBD41FBD4), UL64(0x76F988DA831153B5), - UL64(0x983E5152EE66DFAB), UL64(0xA831C66D2DB43210), - UL64(0xB00327C898FB213F), UL64(0xBF597FC7BEEF0EE4), - UL64(0xC6E00BF33DA88FC2), UL64(0xD5A79147930AA725), - UL64(0x06CA6351E003826F), UL64(0x142929670A0E6E70), - UL64(0x27B70A8546D22FFC), UL64(0x2E1B21385C26C926), - UL64(0x4D2C6DFC5AC42AED), UL64(0x53380D139D95B3DF), - UL64(0x650A73548BAF63DE), UL64(0x766A0ABB3C77B2A8), - UL64(0x81C2C92E47EDAEE6), UL64(0x92722C851482353B), - UL64(0xA2BFE8A14CF10364), UL64(0xA81A664BBC423001), - UL64(0xC24B8B70D0F89791), UL64(0xC76C51A30654BE30), - UL64(0xD192E819D6EF5218), UL64(0xD69906245565A910), - UL64(0xF40E35855771202A), UL64(0x106AA07032BBD1B8), - UL64(0x19A4C116B8D2D0C8), UL64(0x1E376C085141AB53), - UL64(0x2748774CDF8EEB99), UL64(0x34B0BCB5E19B48A8), - UL64(0x391C0CB3C5C95A63), UL64(0x4ED8AA4AE3418ACB), - UL64(0x5B9CCA4F7763E373), UL64(0x682E6FF3D6B2B8A3), - UL64(0x748F82EE5DEFB2FC), UL64(0x78A5636F43172F60), - UL64(0x84C87814A1F0AB72), UL64(0x8CC702081A6439EC), - UL64(0x90BEFFFA23631E28), UL64(0xA4506CEBDE82BDE9), - UL64(0xBEF9A3F7B2C67915), UL64(0xC67178F2E372532B), - UL64(0xCA273ECEEA26619C), UL64(0xD186B8C721C0C207), - UL64(0xEADA7DD6CDE0EB1E), UL64(0xF57D4F7FEE6ED178), - UL64(0x06F067AA72176FBA), UL64(0x0A637DC5A2C898A6), - UL64(0x113F9804BEF90DAE), UL64(0x1B710B35131C471B), - UL64(0x28DB77F523047D84), UL64(0x32CAAB7B40C72493), - UL64(0x3C9EBE0A15C9BEBC), UL64(0x431D67C49C100D4C), - UL64(0x4CC5D4BECB3E42B6), UL64(0x597F299CFC657E2A), - UL64(0x5FCB6FAB3AD6FAEC), UL64(0x6C44198C4A475817) -}; - void mbedtls_sha512_init( mbedtls_sha512_context *ctx ) { memset( ctx, 0, sizeof( mbedtls_sha512_context ) ); @@ -192,6 +145,54 @@ void mbedtls_sha512_starts( mbedtls_sha512_context *ctx, int is384 ) } #if !defined(MBEDTLS_SHA512_PROCESS_ALT) + +/* + * Round constants + */ +static const uint64_t K[80] = +{ + UL64(0x428A2F98D728AE22), UL64(0x7137449123EF65CD), + UL64(0xB5C0FBCFEC4D3B2F), UL64(0xE9B5DBA58189DBBC), + UL64(0x3956C25BF348B538), UL64(0x59F111F1B605D019), + UL64(0x923F82A4AF194F9B), UL64(0xAB1C5ED5DA6D8118), + UL64(0xD807AA98A3030242), UL64(0x12835B0145706FBE), + UL64(0x243185BE4EE4B28C), UL64(0x550C7DC3D5FFB4E2), + UL64(0x72BE5D74F27B896F), UL64(0x80DEB1FE3B1696B1), + UL64(0x9BDC06A725C71235), UL64(0xC19BF174CF692694), + UL64(0xE49B69C19EF14AD2), UL64(0xEFBE4786384F25E3), + UL64(0x0FC19DC68B8CD5B5), UL64(0x240CA1CC77AC9C65), + UL64(0x2DE92C6F592B0275), UL64(0x4A7484AA6EA6E483), + UL64(0x5CB0A9DCBD41FBD4), UL64(0x76F988DA831153B5), + UL64(0x983E5152EE66DFAB), UL64(0xA831C66D2DB43210), + UL64(0xB00327C898FB213F), UL64(0xBF597FC7BEEF0EE4), + UL64(0xC6E00BF33DA88FC2), UL64(0xD5A79147930AA725), + UL64(0x06CA6351E003826F), UL64(0x142929670A0E6E70), + UL64(0x27B70A8546D22FFC), UL64(0x2E1B21385C26C926), + UL64(0x4D2C6DFC5AC42AED), UL64(0x53380D139D95B3DF), + UL64(0x650A73548BAF63DE), UL64(0x766A0ABB3C77B2A8), + UL64(0x81C2C92E47EDAEE6), UL64(0x92722C851482353B), + UL64(0xA2BFE8A14CF10364), UL64(0xA81A664BBC423001), + UL64(0xC24B8B70D0F89791), UL64(0xC76C51A30654BE30), + UL64(0xD192E819D6EF5218), UL64(0xD69906245565A910), + UL64(0xF40E35855771202A), UL64(0x106AA07032BBD1B8), + UL64(0x19A4C116B8D2D0C8), UL64(0x1E376C085141AB53), + UL64(0x2748774CDF8EEB99), UL64(0x34B0BCB5E19B48A8), + UL64(0x391C0CB3C5C95A63), UL64(0x4ED8AA4AE3418ACB), + UL64(0x5B9CCA4F7763E373), UL64(0x682E6FF3D6B2B8A3), + UL64(0x748F82EE5DEFB2FC), UL64(0x78A5636F43172F60), + UL64(0x84C87814A1F0AB72), UL64(0x8CC702081A6439EC), + UL64(0x90BEFFFA23631E28), UL64(0xA4506CEBDE82BDE9), + UL64(0xBEF9A3F7B2C67915), UL64(0xC67178F2E372532B), + UL64(0xCA273ECEEA26619C), UL64(0xD186B8C721C0C207), + UL64(0xEADA7DD6CDE0EB1E), UL64(0xF57D4F7FEE6ED178), + UL64(0x06F067AA72176FBA), UL64(0x0A637DC5A2C898A6), + UL64(0x113F9804BEF90DAE), UL64(0x1B710B35131C471B), + UL64(0x28DB77F523047D84), UL64(0x32CAAB7B40C72493), + UL64(0x3C9EBE0A15C9BEBC), UL64(0x431D67C49C100D4C), + UL64(0x4CC5D4BECB3E42B6), UL64(0x597F299CFC657E2A), + UL64(0x5FCB6FAB3AD6FAEC), UL64(0x6C44198C4A475817) +}; + void mbedtls_sha512_process( mbedtls_sha512_context *ctx, const unsigned char data[128] ) { int i; From d19ea90f11c657db2c1a4232a40c460eb1a92400 Mon Sep 17 00:00:00 2001 From: Attila Molnar Date: Tue, 26 Jan 2016 11:39:26 +0100 Subject: [PATCH 05/15] Fix handle leak in mbedtls_platform_entropy_poll() on Windows on error --- library/entropy_poll.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/entropy_poll.c b/library/entropy_poll.c index 25a27bef3..01bd58efc 100644 --- a/library/entropy_poll.c +++ b/library/entropy_poll.c @@ -61,7 +61,10 @@ int mbedtls_platform_entropy_poll( void *data, unsigned char *output, size_t len } if( CryptGenRandom( provider, (DWORD) len, output ) == FALSE ) + { + CryptReleaseContext( provider, 0 ); return( MBEDTLS_ERR_ENTROPY_SOURCE_FAILED ); + } CryptReleaseContext( provider, 0 ); *olen = len; From 2cc69fffcf431085f18f4e59c3c5188297f97b87 Mon Sep 17 00:00:00 2001 From: Nicholas Wilson Date: Wed, 13 Apr 2016 11:44:29 +0100 Subject: [PATCH 06/15] Shut up a clang-analyzer warning The function appears to be safe, since grow() is called with sensible arguments in previous functions. Ideally Clang would be clever enough to realise this. Even if N has size MBEDTLS_MPI_MAX_LIMBS, which will cause the grow to fail, the affected lines in montmul won't be reached. Having this sanity check can hardly hurt though. --- library/bignum.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/library/bignum.c b/library/bignum.c index 7841bea43..81af57d5a 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1542,12 +1542,15 @@ static void mpi_montg_init( mbedtls_mpi_uint *mm, const mbedtls_mpi *N ) /* * Montgomery multiplication: A = A * B * R^-1 mod N (HAC 14.36) */ -static void mpi_montmul( mbedtls_mpi *A, const mbedtls_mpi *B, const mbedtls_mpi *N, mbedtls_mpi_uint mm, +static int mpi_montmul( mbedtls_mpi *A, const mbedtls_mpi *B, const mbedtls_mpi *N, mbedtls_mpi_uint mm, const mbedtls_mpi *T ) { size_t i, n, m; mbedtls_mpi_uint u0, u1, *d; + if( T->n < N->n + 1 || T->p == NULL ) + return( MBEDTLS_ERR_MPI_BAD_INPUT_DATA ); + memset( T->p, 0, T->n * ciL ); d = T->p; @@ -1575,12 +1578,14 @@ static void mpi_montmul( mbedtls_mpi *A, const mbedtls_mpi *B, const mbedtls_mpi else /* prevent timing attacks */ mpi_sub_hlp( n, A->p, T->p ); + + return( 0 ); } /* * Montgomery reduction: A = A * R^-1 mod N */ -static void mpi_montred( mbedtls_mpi *A, const mbedtls_mpi *N, mbedtls_mpi_uint mm, const mbedtls_mpi *T ) +static int mpi_montred( mbedtls_mpi *A, const mbedtls_mpi *N, mbedtls_mpi_uint mm, const mbedtls_mpi *T ) { mbedtls_mpi_uint z = 1; mbedtls_mpi U; @@ -1588,7 +1593,7 @@ static void mpi_montred( mbedtls_mpi *A, const mbedtls_mpi *N, mbedtls_mpi_uint U.n = U.s = (int) z; U.p = &z; - mpi_montmul( A, &U, N, mm, T ); + return( mpi_montmul( A, &U, N, mm, T ) ); } /* @@ -1665,13 +1670,13 @@ int mbedtls_mpi_exp_mod( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi else MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &W[1], A ) ); - mpi_montmul( &W[1], &RR, N, mm, &T ); + MBEDTLS_MPI_CHK( mpi_montmul( &W[1], &RR, N, mm, &T ) ); /* * X = R^2 * R^-1 mod N = R mod N */ MBEDTLS_MPI_CHK( mbedtls_mpi_copy( X, &RR ) ); - mpi_montred( X, N, mm, &T ); + MBEDTLS_MPI_CHK( mpi_montred( X, N, mm, &T ) ); if( wsize > 1 ) { @@ -1684,7 +1689,7 @@ int mbedtls_mpi_exp_mod( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &W[j], &W[1] ) ); for( i = 0; i < wsize - 1; i++ ) - mpi_montmul( &W[j], &W[j], N, mm, &T ); + MBEDTLS_MPI_CHK( mpi_montmul( &W[j], &W[j], N, mm, &T ) ); /* * W[i] = W[i - 1] * W[1] @@ -1694,7 +1699,7 @@ int mbedtls_mpi_exp_mod( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi MBEDTLS_MPI_CHK( mbedtls_mpi_grow( &W[i], N->n + 1 ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &W[i], &W[i - 1] ) ); - mpi_montmul( &W[i], &W[1], N, mm, &T ); + MBEDTLS_MPI_CHK( mpi_montmul( &W[i], &W[1], N, mm, &T ) ); } } @@ -1731,7 +1736,7 @@ int mbedtls_mpi_exp_mod( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi /* * out of window, square X */ - mpi_montmul( X, X, N, mm, &T ); + MBEDTLS_MPI_CHK( mpi_montmul( X, X, N, mm, &T ) ); continue; } @@ -1749,12 +1754,12 @@ int mbedtls_mpi_exp_mod( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi * X = X^wsize R^-1 mod N */ for( i = 0; i < wsize; i++ ) - mpi_montmul( X, X, N, mm, &T ); + MBEDTLS_MPI_CHK( mpi_montmul( X, X, N, mm, &T ) ); /* * X = X * W[wbits] R^-1 mod N */ - mpi_montmul( X, &W[wbits], N, mm, &T ); + MBEDTLS_MPI_CHK( mpi_montmul( X, &W[wbits], N, mm, &T ) ); state--; nbits = 0; @@ -1767,18 +1772,18 @@ int mbedtls_mpi_exp_mod( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi */ for( i = 0; i < nbits; i++ ) { - mpi_montmul( X, X, N, mm, &T ); + MBEDTLS_MPI_CHK( mpi_montmul( X, X, N, mm, &T ) ); wbits <<= 1; if( ( wbits & ( one << wsize ) ) != 0 ) - mpi_montmul( X, &W[1], N, mm, &T ); + MBEDTLS_MPI_CHK( mpi_montmul( X, &W[1], N, mm, &T ) ); } /* * X = A^E * R * R^-1 mod N = A^E mod N */ - mpi_montred( X, N, mm, &T ); + MBEDTLS_MPI_CHK( mpi_montred( X, N, mm, &T ) ); if( neg ) { From 409401c044da02c84cb3ec5f4edb01d79a1f16a7 Mon Sep 17 00:00:00 2001 From: Nicholas Wilson Date: Wed, 13 Apr 2016 11:48:25 +0100 Subject: [PATCH 07/15] Shut up a few clang-analyze warnings about use of uninitialized variables The functions are all safe, Clang just isn't clever enough to realise it. --- library/pkcs12.c | 2 +- library/rsa.c | 19 +++++++++++++++++-- programs/hash/generic_sum.c | 2 +- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/library/pkcs12.c b/library/pkcs12.c index 7023b9dbc..c603a1357 100644 --- a/library/pkcs12.c +++ b/library/pkcs12.c @@ -93,7 +93,7 @@ static int pkcs12_pbe_derive_key_iv( mbedtls_asn1_buf *pbe_params, mbedtls_md_ty unsigned char *key, size_t keylen, unsigned char *iv, size_t ivlen ) { - int ret, iterations; + int ret, iterations = 0; mbedtls_asn1_buf salt; size_t i; unsigned char unipwd[PKCS12_MAX_PWDLEN * 2 + 2]; diff --git a/library/rsa.c b/library/rsa.c index fba68ddfc..60559e2ac 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -797,7 +797,12 @@ int mbedtls_rsa_rsaes_pkcs1_v15_decrypt( mbedtls_rsa_context *ctx, int ret; size_t ilen, pad_count = 0, i; unsigned char *p, bad, pad_done = 0; +#ifdef __clang_analyzer__ + /* Shut up Clang, mbedtls_rsa_public/private writes to this */ + unsigned char buf[MBEDTLS_MPI_MAX_SIZE] = { }; +#else unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; +#endif if( mode == MBEDTLS_RSA_PRIVATE && ctx->padding != MBEDTLS_RSA_PKCS_V15 ) return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); @@ -1175,13 +1180,18 @@ int mbedtls_rsa_rsassa_pss_verify_ext( mbedtls_rsa_context *ctx, int ret; size_t siglen; unsigned char *p; - unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; unsigned char result[MBEDTLS_MD_MAX_SIZE]; unsigned char zeros[8]; unsigned int hlen; size_t slen, msb; const mbedtls_md_info_t *md_info; mbedtls_md_context_t md_ctx; +#ifdef __clang_analyzer__ + /* Shut up Clang, mbedtls_rsa_public/private writes to this */ + unsigned char buf[MBEDTLS_MPI_MAX_SIZE] = { }; +#else + unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; +#endif if( mode == MBEDTLS_RSA_PRIVATE && ctx->padding != MBEDTLS_RSA_PKCS_V21 ) return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); @@ -1320,10 +1330,15 @@ int mbedtls_rsa_rsassa_pkcs1_v15_verify( mbedtls_rsa_context *ctx, int ret; size_t len, siglen, asn1_len; unsigned char *p, *end; - unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; mbedtls_md_type_t msg_md_alg; const mbedtls_md_info_t *md_info; mbedtls_asn1_buf oid; +#ifdef __clang_analyzer__ + /* Shut up Clang, mbedtls_rsa_public/private writes to this */ + unsigned char buf[MBEDTLS_MPI_MAX_SIZE] = { }; +#else + unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; +#endif if( mode == MBEDTLS_RSA_PRIVATE && ctx->padding != MBEDTLS_RSA_PKCS_V15 ) return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); diff --git a/programs/hash/generic_sum.c b/programs/hash/generic_sum.c index f071d311e..7805a79bc 100644 --- a/programs/hash/generic_sum.c +++ b/programs/hash/generic_sum.c @@ -83,7 +83,7 @@ static int generic_check( const mbedtls_md_info_t *md_info, char *filename ) int nb_err1, nb_err2; int nb_tot1, nb_tot2; unsigned char sum[MBEDTLS_MD_MAX_SIZE]; - char buf[MBEDTLS_MD_MAX_SIZE * 2 + 1], line[1024]; + char buf[MBEDTLS_MD_MAX_SIZE * 2 + 1] = { }, line[1024]; char diff; if( ( f = fopen( filename, "rb" ) ) == NULL ) From daf534dcf921895604c2a73a767d7ce9a6de1906 Mon Sep 17 00:00:00 2001 From: Nicholas Wilson Date: Wed, 13 Apr 2016 11:50:33 +0100 Subject: [PATCH 08/15] Remove a dead store to silence clang-analyze --- library/ssl_cli.c | 1 - 1 file changed, 1 deletion(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 52ddf9a92..d1ef7dd35 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -265,7 +265,6 @@ static void ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_ECP_C) for( grp_id = ssl->conf->curve_list; *grp_id != MBEDTLS_ECP_DP_NONE; grp_id++ ) { - info = mbedtls_ecp_curve_info_from_grp_id( *grp_id ); #else for( info = mbedtls_ecp_curve_list(); info->grp_id != MBEDTLS_ECP_DP_NONE; info++ ) { From 5d5e421d089bf02488f5a0f06ae3a727ff9aadd7 Mon Sep 17 00:00:00 2001 From: Nicholas Wilson Date: Wed, 13 Apr 2016 11:51:05 +0100 Subject: [PATCH 09/15] Refactor slightly to silence a clang-analyze warning Since the buffer is used in a few places, it seems Clang isn't clever enough to realise that the first byte is never touched. So, even though the function has a correct null check for ssl->handshake, Clang complains. Pulling the handshake type out into its own variable is enough for Clang's analysis to kick in though. --- library/ssl_tls.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 1c44b7ddb..2e41bcad2 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2708,7 +2708,7 @@ void mbedtls_ssl_send_flight_completed( mbedtls_ssl_context *ssl ) */ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl ) { - int ret, done = 0; + int ret, done = 0, out_msg_type; size_t len = ssl->out_msglen; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write record" ) ); @@ -2724,7 +2724,9 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl ) #endif if( ssl->out_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE ) { - if( ssl->out_msg[0] != MBEDTLS_SSL_HS_HELLO_REQUEST && + out_msg_type = ssl->out_msg[0]; + + if( out_msg_type != MBEDTLS_SSL_HS_HELLO_REQUEST && ssl->handshake == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); @@ -2751,7 +2753,7 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl ) len += 8; /* Write message_seq and update it, except for HelloRequest */ - if( ssl->out_msg[0] != MBEDTLS_SSL_HS_HELLO_REQUEST ) + if( out_msg_type != MBEDTLS_SSL_HS_HELLO_REQUEST ) { ssl->out_msg[4] = ( ssl->handshake->out_msg_seq >> 8 ) & 0xFF; ssl->out_msg[5] = ( ssl->handshake->out_msg_seq ) & 0xFF; @@ -2769,7 +2771,7 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_SSL_PROTO_DTLS */ - if( ssl->out_msg[0] != MBEDTLS_SSL_HS_HELLO_REQUEST ) + if( out_msg_type != MBEDTLS_SSL_HS_HELLO_REQUEST ) ssl->handshake->update_checksum( ssl, ssl->out_msg, len ); } From 42d47f0fb5414bc5cb07e0353303a6d81297dda3 Mon Sep 17 00:00:00 2001 From: Nicholas Wilson Date: Wed, 13 Apr 2016 11:53:27 +0100 Subject: [PATCH 10/15] Silence a clang-analyze warning The check is already effectively performed later in the function, but implicitly, so Clang's analysis fail to notice the functions are in fact safe. Pulling the check up to the top helps Clang to verify the behaviour. --- library/x509_csr.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/x509_csr.c b/library/x509_csr.c index f8c45f8d2..603d06b64 100644 --- a/library/x509_csr.c +++ b/library/x509_csr.c @@ -104,7 +104,7 @@ int mbedtls_x509_csr_parse_der( mbedtls_x509_csr *csr, /* * Check for valid input */ - if( csr == NULL || buf == NULL ) + if( csr == NULL || buf == NULL || buflen == 0 ) return( MBEDTLS_ERR_X509_BAD_INPUT_DATA ); mbedtls_x509_csr_init( csr ); @@ -274,14 +274,14 @@ int mbedtls_x509_csr_parse( mbedtls_x509_csr *csr, const unsigned char *buf, siz /* * Check for valid input */ - if( csr == NULL || buf == NULL ) + if( csr == NULL || buf == NULL || buflen == 0 ) return( MBEDTLS_ERR_X509_BAD_INPUT_DATA ); #if defined(MBEDTLS_PEM_PARSE_C) mbedtls_pem_init( &pem ); /* Avoid calling mbedtls_pem_read_buffer() on non-null-terminated string */ - if( buflen == 0 || buf[buflen - 1] != '\0' ) + if( buf[buflen - 1] != '\0' ) ret = MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT; else ret = mbedtls_pem_read_buffer( &pem, From 699d7193a18afbc72699ca955827a9855e2523b9 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Tue, 10 May 2016 21:16:54 +0100 Subject: [PATCH 11/15] Disables backtrace config from basic-build-test.sh The configuration MBEDTLS_MEMORY_BACKTRACE is intended for debug and is not necessary for test coverage. Because it causes timing problems in some tests the configuration has been removed as it's not present in equivalent tests in the all.sh test script. --- tests/scripts/basic-build-test.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/scripts/basic-build-test.sh b/tests/scripts/basic-build-test.sh index d13a8e4ed..d961230ed 100755 --- a/tests/scripts/basic-build-test.sh +++ b/tests/scripts/basic-build-test.sh @@ -39,6 +39,7 @@ fi export CFLAGS=' --coverage -g3 -O0 ' make clean scripts/config.pl full +scripts/config.pl unset MBEDTLS_MEMORY_BACKTRACE make From 71c7ac55973b409205f75a08cc45429538050bbf Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Tue, 10 May 2016 23:47:30 +0100 Subject: [PATCH 12/15] Corrects incorrectly named function in ctr_drbg.c comment --- library/ctr_drbg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/ctr_drbg.c b/library/ctr_drbg.c index 6962d68b9..386f8adb0 100644 --- a/library/ctr_drbg.c +++ b/library/ctr_drbg.c @@ -67,8 +67,8 @@ void mbedtls_ctr_drbg_init( mbedtls_ctr_drbg_context *ctx ) } /* - * Non-public function wrapped by mbedtls_ctr_drbg_init(). Necessary to allow NIST - * tests to succeed (which require known length fixed entropy) + * Non-public function wrapped by mbedtls_ctr_drbg_seed(). Necessary to allow + * NIST tests to succeed (which require known length fixed entropy) */ int mbedtls_ctr_drbg_seed_entropy_len( mbedtls_ctr_drbg_context *ctx, From 17ddff5eafeef9947c68f799e43515860da072e3 Mon Sep 17 00:00:00 2001 From: Embedthis Software Date: Thu, 10 Sep 2015 11:45:13 -0700 Subject: [PATCH 13/15] Fix single threaded builds --- include/mbedtls/threading.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/mbedtls/threading.h b/include/mbedtls/threading.h index c39cbf24d..b416d478a 100644 --- a/include/mbedtls/threading.h +++ b/include/mbedtls/threading.h @@ -81,6 +81,7 @@ void mbedtls_threading_set_alt( void (*mutex_init)( mbedtls_threading_mutex_t * void mbedtls_threading_free_alt( void ); #endif /* MBEDTLS_THREADING_ALT */ +#if defined(MBEDTLS_THREADING_C) /* * The function pointers for mutex_init, mutex_free, mutex_ and mutex_unlock * @@ -96,6 +97,7 @@ extern int (*mbedtls_mutex_unlock)( mbedtls_threading_mutex_t *mutex ); */ extern mbedtls_threading_mutex_t mbedtls_threading_readdir_mutex; extern mbedtls_threading_mutex_t mbedtls_threading_gmtime_mutex; +#endif #ifdef __cplusplus } From e049ccd40595961da7e1dfb448923aea1f84f715 Mon Sep 17 00:00:00 2001 From: Paul Bakker Date: Tue, 10 May 2016 16:17:27 +0100 Subject: [PATCH 14/15] Add end guard comment --- include/mbedtls/threading.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/threading.h b/include/mbedtls/threading.h index b416d478a..b0c34ecc7 100644 --- a/include/mbedtls/threading.h +++ b/include/mbedtls/threading.h @@ -97,7 +97,7 @@ extern int (*mbedtls_mutex_unlock)( mbedtls_threading_mutex_t *mutex ); */ extern mbedtls_threading_mutex_t mbedtls_threading_readdir_mutex; extern mbedtls_threading_mutex_t mbedtls_threading_gmtime_mutex; -#endif +#endif /* MBEDTLS_THREADING_C */ #ifdef __cplusplus } From 2dd49d1e47dad8bbd98d27694334d1e82d802690 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Wed, 11 May 2016 23:15:58 +0100 Subject: [PATCH 15/15] Reverts change in commit daf534d Commit daf534d from PR #457 breaks the build. This may reintroduce a clang-analyse warning, but this is the wrong fix for that. The fix removed a call to mbedtls_ecp_curve_info_from_grp_id() to find the curve info. This fix adds that back in. --- library/ssl_cli.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 509484e36..cd39db027 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -267,6 +267,7 @@ static void ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_ECP_C) for( grp_id = ssl->conf->curve_list; *grp_id != MBEDTLS_ECP_DP_NONE; grp_id++ ) { + info = mbedtls_ecp_curve_info_from_grp_id( *grp_id ); #else for( info = mbedtls_ecp_curve_list(); info->grp_id != MBEDTLS_ECP_DP_NONE; info++ ) {