From 7e8e57c6d1192d7cf867166935a0d66e6e71743f Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sun, 23 Jul 2017 10:19:29 +0100 Subject: [PATCH 1/9] Initialize RSA context in RSA test suite before first potentially failing operation The function `mbedtls_rsa_gen_key` from `test_suite_rsa.function` initialized a stack allocated RSA context only after seeding the CTR DRBG. If the latter operation failed, the cleanup code tried to free the uninitialized RSA context, potentially resulting in a segmentation fault. Fixes one aspect of #1023. --- tests/suites/test_suite_rsa.function | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/suites/test_suite_rsa.function b/tests/suites/test_suite_rsa.function index d48bc8595..e9ae1bf96 100644 --- a/tests/suites/test_suite_rsa.function +++ b/tests/suites/test_suite_rsa.function @@ -667,13 +667,12 @@ void mbedtls_rsa_gen_key( int nrbits, int exponent, int result) const char *pers = "test_suite_rsa"; mbedtls_ctr_drbg_init( &ctr_drbg ); - mbedtls_entropy_init( &entropy ); + mbedtls_rsa_init ( &ctx, 0, 0 ); + TEST_ASSERT( mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, &entropy, (const unsigned char *) pers, strlen( pers ) ) == 0 ); - mbedtls_rsa_init( &ctx, 0, 0 ); - TEST_ASSERT( mbedtls_rsa_gen_key( &ctx, mbedtls_ctr_drbg_random, &ctr_drbg, nrbits, exponent ) == result ); if( result == 0 ) { From 1b841cc9bf8f4756938946cce312f4dbff8bd87a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sun, 23 Jul 2017 10:22:45 +0100 Subject: [PATCH 2/9] Correct typo in entropy test suite data --- tests/suites/test_suite_entropy.data | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_entropy.data b/tests/suites/test_suite_entropy.data index e0dfae32a..5cff39984 100644 --- a/tests/suites/test_suite_entropy.data +++ b/tests/suites/test_suite_entropy.data @@ -34,10 +34,10 @@ entropy_threshold:16:2:8 Entropy threshold #2 entropy_threshold:32:1:32 -Entropy thershold #3 +Entropy threshold #3 entropy_threshold:16:0:MBEDTLS_ERR_ENTROPY_SOURCE_FAILED -Entropy thershold #4 +Entropy threshold #4 entropy_threshold:1024:1:MBEDTLS_ERR_ENTROPY_SOURCE_FAILED Check NV seed standard IO From 910f662cd7e804b5ecff9abb97d0e216122a675d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sun, 23 Jul 2017 10:23:24 +0100 Subject: [PATCH 3/9] Increase readability of verbose test suite output --- tests/suites/main_test.function | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/suites/main_test.function b/tests/suites/main_test.function index a7bb41de3..fe49bdfd8 100644 --- a/tests/suites/main_test.function +++ b/tests/suites/main_test.function @@ -432,24 +432,24 @@ int main(int argc, const char *argv[]) if( unmet_dep_count > 0 || ret == DISPATCH_UNSUPPORTED_SUITE ) { total_skipped++; - mbedtls_fprintf( stdout, "----\n" ); + mbedtls_fprintf( stdout, "----" ); if( 1 == option_verbose && ret == DISPATCH_UNSUPPORTED_SUITE ) { - mbedtls_fprintf( stdout, " Test Suite not enabled" ); + mbedtls_fprintf( stdout, "\n Test Suite not enabled" ); } if( 1 == option_verbose && unmet_dep_count > 0 ) { - mbedtls_fprintf( stdout, " Unmet dependencies: " ); + mbedtls_fprintf( stdout, "\n Unmet dependencies: " ); for( i = 0; i < unmet_dep_count; i++ ) { mbedtls_fprintf(stdout, "%s ", unmet_dependencies[i]); free(unmet_dependencies[i]); } - mbedtls_fprintf( stdout, "\n" ); } + mbedtls_fprintf( stdout, "\n" ); fflush( stdout ); unmet_dep_count = 0; From 75efa792013d00bc35fab91e28cc0ebd29a86f71 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sun, 23 Jul 2017 10:23:43 +0100 Subject: [PATCH 4/9] Adapt generic test suite file to coding standard --- tests/suites/main_test.function | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/suites/main_test.function b/tests/suites/main_test.function index fe49bdfd8..5d1e9ecf0 100644 --- a/tests/suites/main_test.function +++ b/tests/suites/main_test.function @@ -396,7 +396,7 @@ int main(int argc, const char *argv[]) break; cnt = parse_arguments( buf, strlen(buf), params ); } - + // If there are no unmet dependencies execute the test if( unmet_dep_count == 0 ) { @@ -462,22 +462,22 @@ int main(int argc, const char *argv[]) else if( ret == DISPATCH_INVALID_TEST_DATA ) { mbedtls_fprintf( stderr, "FAILED: FATAL PARSE ERROR\n" ); - fclose(file); + fclose( file ); mbedtls_exit( 2 ); } else total_errors++; - if( ( ret = get_line( file, buf, sizeof(buf) ) ) != 0 ) + if( ( ret = get_line( file, buf, sizeof( buf ) ) ) != 0 ) break; - if( strlen(buf) != 0 ) + if( strlen( buf ) != 0 ) { mbedtls_fprintf( stderr, "Should be empty %d\n", - (int) strlen(buf) ); + (int) strlen( buf ) ); return( 1 ); } } - fclose(file); + fclose( file ); /* In case we encounter early end of file */ for( i = 0; i < unmet_dep_count; i++ ) @@ -508,4 +508,3 @@ int main(int argc, const char *argv[]) return( total_errors != 0 ); } - From f058f34b5a892e73c0fe465e3180feab4659080a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sun, 23 Jul 2017 10:24:22 +0100 Subject: [PATCH 5/9] Support negative dependencies in test cases The entropy test suite uses a negative dependency "depends_on:!CONFIG_FLAG" for one of its tests. This kind of dependency (running a test only if some configuration flag is not defined) is currently not supported and instead results in the respective test case being dropped. This commit adds support for negative dependencies in test cases. --- tests/scripts/generate_code.pl | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/scripts/generate_code.pl b/tests/scripts/generate_code.pl index 84e949dfa..f803a803d 100755 --- a/tests/scripts/generate_code.pl +++ b/tests/scripts/generate_code.pl @@ -312,7 +312,7 @@ END # and make check code my $dep_check_code; -my @res = $test_data =~ /^depends_on:([\w:]+)/msg; +my @res = $test_data =~ /^depends_on:([!:\w]+)/msg; my %case_deps; foreach my $deps (@res) { @@ -323,7 +323,23 @@ foreach my $deps (@res) } while( my ($key, $value) = each(%case_deps) ) { - $dep_check_code .= << "END"; + if( substr($key, 0, 1) eq "!" ) + { + my $key = substr($key, 1); + $dep_check_code .= << "END"; + if( strcmp( str, "!$key" ) == 0 ) + { +#if !defined($key) + return( DEPENDENCY_SUPPORTED ); +#else + return( DEPENDENCY_NOT_SUPPORTED ); +#endif + } +END + } + else + { + $dep_check_code .= << "END"; if( strcmp( str, "$key" ) == 0 ) { #if defined($key) @@ -333,6 +349,7 @@ while( my ($key, $value) = each(%case_deps) ) #endif } END + } } # Make mapping code From c6deafc0d495b3e80cd42cf8de451960f6e2190d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sun, 23 Jul 2017 14:06:42 +0100 Subject: [PATCH 6/9] Omit RSA key generation test if no strong entropy is present The RSA key generation test needs strong entropy to succeed. This commit captures the presence of a strong entropy source in a preprocessor flag and only runs the key generation test if that flag is set. --- include/mbedtls/entropy.h | 10 ++++++++++ library/entropy.c | 3 +++ tests/suites/test_suite_entropy.data | 8 ++++++-- tests/suites/test_suite_rsa.function | 2 +- 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/entropy.h b/include/mbedtls/entropy.h index 747aca4df..b374b34ec 100644 --- a/include/mbedtls/entropy.h +++ b/include/mbedtls/entropy.h @@ -55,6 +55,16 @@ #define MBEDTLS_ERR_ENTROPY_NO_STRONG_SOURCE -0x003D /**< No strong sources have been added to poll. */ #define MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR -0x003F /**< Read/write error in file. */ +/* Indicates whether at least one standard strong entropy source is enabled. */ +#if defined(MBEDTLS_TEST_NULL_ENTROPY) || \ + ( !defined(MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES) && \ + ( !defined(MBEDTLS_NO_PLATFORM_ENTROPY) || \ + defined(MBEDTLS_HAVEGE_C) || \ + defined(MBEDTLS_ENTROPY_HARDWARE_ALT) || \ + defined(ENTROPY_NV_SEED) ) ) +#define MBEDTLS_ENTROPY_HAVE_STRONG +#endif + /** * \name SECTION: Module settings * diff --git a/library/entropy.c b/library/entropy.c index d4d1b27b7..4de168250 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -83,6 +83,9 @@ void mbedtls_entropy_init( mbedtls_entropy_context *ctx ) mbedtls_havege_init( &ctx->havege_data ); #endif + /* Reminder: Update MBEDTLS_ENTROPY_HAVE_STRONG when + * adding more strong entropy sources here. */ + #if defined(MBEDTLS_TEST_NULL_ENTROPY) mbedtls_entropy_add_source( ctx, mbedtls_null_entropy_poll, NULL, 1, MBEDTLS_ENTROPY_SOURCE_STRONG ); diff --git a/tests/suites/test_suite_entropy.data b/tests/suites/test_suite_entropy.data index 5cff39984..bf9ce49ed 100644 --- a/tests/suites/test_suite_entropy.data +++ b/tests/suites/test_suite_entropy.data @@ -52,10 +52,14 @@ entropy_nv_seed:"000000000000000000000000000000000000000000000000000000000000000 Check NV seed manually #3 entropy_nv_seed:"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" -Entropy self test -depends_on:!MBEDTLS_TEST_NULL_ENTROPY +Entropy self test (with strong entropy) +depends_on:!MBEDTLS_TEST_NULL_ENTROPY:MBEDTLS_ENTROPY_HAVE_STRONG entropy_selftest:0 +Entropy self test (without strong entropy) +depends_on:!MBEDTLS_TEST_NULL_ENTROPY:!MBEDTLS_ENTROPY_HAVE_STRONG +entropy_selftest:1 + Entropy self test (MBEDTLS_TEST_NULL_ENTROPY) depends_on:MBEDTLS_TEST_NULL_ENTROPY entropy_selftest:1 diff --git a/tests/suites/test_suite_rsa.function b/tests/suites/test_suite_rsa.function index e9ae1bf96..f64e1a73a 100644 --- a/tests/suites/test_suite_rsa.function +++ b/tests/suites/test_suite_rsa.function @@ -658,7 +658,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_CTR_DRBG_C:MBEDTLS_ENTROPY_C */ +/* BEGIN_CASE depends_on:MBEDTLS_CTR_DRBG_C:MBEDTLS_ENTROPY_C:MBEDTLS_ENTROPY_HAVE_STRONG */ void mbedtls_rsa_gen_key( int nrbits, int exponent, int result) { mbedtls_rsa_context ctx; From 47deec488f8da931ee82961d47c5e6eb9ffb94c4 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 24 Jul 2017 12:27:09 +0100 Subject: [PATCH 7/9] Move flag indicating presence of strong entropy to test code --- include/mbedtls/entropy.h | 10 ---------- library/entropy.c | 4 ++-- tests/suites/helpers.function | 16 +++++++++++++++- tests/suites/test_suite_entropy.data | 8 ++------ tests/suites/test_suite_entropy.function | 6 +++--- tests/suites/test_suite_rsa.function | 1 + 6 files changed, 23 insertions(+), 22 deletions(-) diff --git a/include/mbedtls/entropy.h b/include/mbedtls/entropy.h index b374b34ec..747aca4df 100644 --- a/include/mbedtls/entropy.h +++ b/include/mbedtls/entropy.h @@ -55,16 +55,6 @@ #define MBEDTLS_ERR_ENTROPY_NO_STRONG_SOURCE -0x003D /**< No strong sources have been added to poll. */ #define MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR -0x003F /**< Read/write error in file. */ -/* Indicates whether at least one standard strong entropy source is enabled. */ -#if defined(MBEDTLS_TEST_NULL_ENTROPY) || \ - ( !defined(MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES) && \ - ( !defined(MBEDTLS_NO_PLATFORM_ENTROPY) || \ - defined(MBEDTLS_HAVEGE_C) || \ - defined(MBEDTLS_ENTROPY_HARDWARE_ALT) || \ - defined(ENTROPY_NV_SEED) ) ) -#define MBEDTLS_ENTROPY_HAVE_STRONG -#endif - /** * \name SECTION: Module settings * diff --git a/library/entropy.c b/library/entropy.c index 4de168250..10449b8d0 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -83,8 +83,8 @@ void mbedtls_entropy_init( mbedtls_entropy_context *ctx ) mbedtls_havege_init( &ctx->havege_data ); #endif - /* Reminder: Update MBEDTLS_ENTROPY_HAVE_STRONG when - * adding more strong entropy sources here. */ + /* Reminder: Update MBEDTLS_ENTROPY_HAVE_STRONG in the test files + * when adding more strong entropy sources here. */ #if defined(MBEDTLS_TEST_NULL_ENTROPY) mbedtls_entropy_add_source( ctx, mbedtls_null_entropy_poll, NULL, diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index 63815df85..39cd3c768 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -102,6 +102,21 @@ typedef UINT32 uint32_t; static int test_errors = 0; +/*----------------------------------------------------------------------------*/ +/* Helper flags for complex dependencies */ + +/* Indicates whether we expect mbedtls_entropy_init + * to initialize some strong entropy source. */ +#if defined(MBEDTLS_TEST_NULL_ENTROPY) || \ + ( !defined(MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES) && \ + ( !defined(MBEDTLS_NO_PLATFORM_ENTROPY) || \ + defined(MBEDTLS_HAVEGE_C) || \ + defined(MBEDTLS_ENTROPY_HARDWARE_ALT) || \ + defined(ENTROPY_NV_SEED) ) ) +#define MBEDTLS_ENTROPY_HAVE_STRONG +#endif + + /*----------------------------------------------------------------------------*/ /* Helper Functions */ @@ -401,4 +416,3 @@ static void test_fail( const char *test, int line_no, const char* filename ) mbedtls_fprintf( stdout, " %s\n at line %d, %s\n", test, line_no, filename ); } - diff --git a/tests/suites/test_suite_entropy.data b/tests/suites/test_suite_entropy.data index bf9ce49ed..5cff39984 100644 --- a/tests/suites/test_suite_entropy.data +++ b/tests/suites/test_suite_entropy.data @@ -52,14 +52,10 @@ entropy_nv_seed:"000000000000000000000000000000000000000000000000000000000000000 Check NV seed manually #3 entropy_nv_seed:"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" -Entropy self test (with strong entropy) -depends_on:!MBEDTLS_TEST_NULL_ENTROPY:MBEDTLS_ENTROPY_HAVE_STRONG +Entropy self test +depends_on:!MBEDTLS_TEST_NULL_ENTROPY entropy_selftest:0 -Entropy self test (without strong entropy) -depends_on:!MBEDTLS_TEST_NULL_ENTROPY:!MBEDTLS_ENTROPY_HAVE_STRONG -entropy_selftest:1 - Entropy self test (MBEDTLS_TEST_NULL_ENTROPY) depends_on:MBEDTLS_TEST_NULL_ENTROPY entropy_selftest:1 diff --git a/tests/suites/test_suite_entropy.function b/tests/suites/test_suite_entropy.function index 97a21bc18..7983c767e 100644 --- a/tests/suites/test_suite_entropy.function +++ b/tests/suites/test_suite_entropy.function @@ -163,7 +163,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_ENTROPY_HAVE_STRONG */ void entropy_func_len( int len, int ret ) { mbedtls_entropy_context ctx; @@ -224,7 +224,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_ENTROPY_HAVE_STRONG */ void entropy_threshold( int threshold, int chunk_size, int result ) { mbedtls_entropy_context ctx; @@ -377,7 +377,7 @@ void entropy_nv_seed( char *read_seed_str ) } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_SELF_TEST */ +/* BEGIN_CASE depends_on:MBEDTLS_ENTROPY_HAVE_STRONG:MBEDTLS_SELF_TEST */ void entropy_selftest( int result ) { TEST_ASSERT( mbedtls_entropy_self_test( 1 ) == result ); diff --git a/tests/suites/test_suite_rsa.function b/tests/suites/test_suite_rsa.function index f64e1a73a..f41b14cc3 100644 --- a/tests/suites/test_suite_rsa.function +++ b/tests/suites/test_suite_rsa.function @@ -8,6 +8,7 @@ #include "mbedtls/sha512.h" #include "mbedtls/entropy.h" #include "mbedtls/ctr_drbg.h" + /* END_HEADER */ /* BEGIN_DEPENDENCIES From d4a872ee678ece252f052087564eb69c62871782 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 7 Sep 2017 08:09:33 +0100 Subject: [PATCH 8/9] Rename internal MBEDTLS_ENTROPY_HAVE_STRONG to ENTROPY_HAVE_STRONG This commit renames the test-only flag MBEDTLS_ENTROPY_HAVE_STRONG to ENTROPY_HAVE_STRONG to make it more transparent that it's an internal flag, and also to content the testscript tests/scripts/check-names.pl which previously complained about the macro occurring in a comment in `entropy.c` without being defined in a library file. --- library/entropy.c | 2 +- tests/suites/helpers.function | 2 +- tests/suites/test_suite_entropy.function | 6 +++--- tests/suites/test_suite_rsa.function | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/library/entropy.c b/library/entropy.c index 10449b8d0..7c0915676 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -83,7 +83,7 @@ void mbedtls_entropy_init( mbedtls_entropy_context *ctx ) mbedtls_havege_init( &ctx->havege_data ); #endif - /* Reminder: Update MBEDTLS_ENTROPY_HAVE_STRONG in the test files + /* Reminder: Update ENTROPY_HAVE_STRONG in the test files * when adding more strong entropy sources here. */ #if defined(MBEDTLS_TEST_NULL_ENTROPY) diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index 39cd3c768..d36746789 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -113,7 +113,7 @@ static int test_errors = 0; defined(MBEDTLS_HAVEGE_C) || \ defined(MBEDTLS_ENTROPY_HARDWARE_ALT) || \ defined(ENTROPY_NV_SEED) ) ) -#define MBEDTLS_ENTROPY_HAVE_STRONG +#define ENTROPY_HAVE_STRONG #endif diff --git a/tests/suites/test_suite_entropy.function b/tests/suites/test_suite_entropy.function index 7983c767e..2bab796d1 100644 --- a/tests/suites/test_suite_entropy.function +++ b/tests/suites/test_suite_entropy.function @@ -163,7 +163,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_ENTROPY_HAVE_STRONG */ +/* BEGIN_CASE depends_on:ENTROPY_HAVE_STRONG */ void entropy_func_len( int len, int ret ) { mbedtls_entropy_context ctx; @@ -224,7 +224,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_ENTROPY_HAVE_STRONG */ +/* BEGIN_CASE depends_on:ENTROPY_HAVE_STRONG */ void entropy_threshold( int threshold, int chunk_size, int result ) { mbedtls_entropy_context ctx; @@ -377,7 +377,7 @@ void entropy_nv_seed( char *read_seed_str ) } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_ENTROPY_HAVE_STRONG:MBEDTLS_SELF_TEST */ +/* BEGIN_CASE depends_on:ENTROPY_HAVE_STRONG:MBEDTLS_SELF_TEST */ void entropy_selftest( int result ) { TEST_ASSERT( mbedtls_entropy_self_test( 1 ) == result ); diff --git a/tests/suites/test_suite_rsa.function b/tests/suites/test_suite_rsa.function index f41b14cc3..270e2d989 100644 --- a/tests/suites/test_suite_rsa.function +++ b/tests/suites/test_suite_rsa.function @@ -659,7 +659,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_CTR_DRBG_C:MBEDTLS_ENTROPY_C:MBEDTLS_ENTROPY_HAVE_STRONG */ +/* BEGIN_CASE depends_on:MBEDTLS_CTR_DRBG_C:MBEDTLS_ENTROPY_C:ENTROPY_HAVE_STRONG */ void mbedtls_rsa_gen_key( int nrbits, int exponent, int result) { mbedtls_rsa_context ctx; From d742b748388e550e2f5ba3f03c6aa10fc3a7dc6f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 28 Nov 2017 17:40:56 +0100 Subject: [PATCH 9/9] Add ChangeLog entry --- ChangeLog | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog b/ChangeLog index 40b4fae44..a65b7ae87 100644 --- a/ChangeLog +++ b/ChangeLog @@ -13,6 +13,7 @@ Bugfix Found by redplait #590 * Add MBEDTLS_MPI_CHK to check for error value of mbedtls_mpi_fill_random. Reported and fix suggested by guidovranken in #740 + * Fix bugs in RSA test suite under MBEDTLS_NO_PLATFORM_ENTROPY. #1023 #1024 Features * Add the functions mbedtls_platform_setup() and mbedtls_platform_teardown()