From e4875e015fe68bfbcdd924b55e3b91968c47c1e6 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sun, 23 Jul 2017 10:19:29 +0100 Subject: [PATCH 1/7] 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 b1bc19ea7..bdf3db8c2 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 7fdabd3c6404fa8b4969e084833d9afa8605448c Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sun, 23 Jul 2017 10:22:45 +0100 Subject: [PATCH 2/7] 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 833eef565..42630cb37 100644 --- a/tests/suites/test_suite_entropy.data +++ b/tests/suites/test_suite_entropy.data @@ -31,10 +31,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 Entropy self test From 276d530abee44ae539073db012a21f860623cae6 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sun, 23 Jul 2017 10:24:22 +0100 Subject: [PATCH 3/7] 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 1c9cfc5b3..3491eced3 100755 --- a/tests/scripts/generate_code.pl +++ b/tests/scripts/generate_code.pl @@ -194,7 +194,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) { @@ -205,7 +205,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( 0 ); +#else + return( 1 ); +#endif + } +END + } + else + { + $dep_check_code .= << "END"; if( strcmp( str, "$key" ) == 0 ) { #if defined($key) @@ -215,6 +231,7 @@ while( my ($key, $value) = each(%case_deps) ) #endif } END + } } # Make mapping code From 66580d284dd6afeaa337b4a5c28fc0ce262afd79 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 8 Sep 2017 10:06:41 +0100 Subject: [PATCH 4/7] Add internal macro ENTROPY_HAVE_STRONG indicating strong entropy This commit adds the macro ENTROPY_HAVE_STRONG to the helper test file tests/suites/helpers.function to be able to make tests depend on the presence of strong entropy. --- library/entropy.c | 3 +++ tests/suites/helpers.function | 12 ++++++++++++ 2 files changed, 15 insertions(+) diff --git a/library/entropy.c b/library/entropy.c index 3622ad280..b45384dbe 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -60,6 +60,9 @@ void mbedtls_entropy_init( mbedtls_entropy_context *ctx ) { memset( ctx, 0, sizeof(mbedtls_entropy_context) ); + /* Reminder: Update ENTROPY_HAVE_STRONG in the test files + * when adding more strong entropy sources here. */ + #if defined(MBEDTLS_THREADING_C) mbedtls_mutex_init( &ctx->mutex ); #endif diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index 6af918cad..b80831ee5 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -55,6 +55,18 @@ typedef UINT32 uint32_t; } #endif +/* 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) ) ) +#define ENTROPY_HAVE_STRONG +#endif + static int unhexify( unsigned char *obuf, const char *ibuf ) { unsigned char c, c2; From 7968ad9c31f94a33c7620b3ca391208eb3c612e2 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 8 Sep 2017 10:10:03 +0100 Subject: [PATCH 5/7] Guard some tests by presence of strong entropy --- tests/suites/test_suite_entropy.function | 8 ++++---- tests/suites/test_suite_rsa.function | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/suites/test_suite_entropy.function b/tests/suites/test_suite_entropy.function index 3b739cce9..19796a3be 100644 --- a/tests/suites/test_suite_entropy.function +++ b/tests/suites/test_suite_entropy.function @@ -40,7 +40,7 @@ static int entropy_dummy_source( void *data, unsigned char *output, * END_DEPENDENCIES */ -/* BEGIN_CASE depends_on:MBEDTLS_FS_IO */ +/* BEGIN_CASE depends_on:MBEDTLS_FS_IO:ENTROPY_HAVE_STRONG */ void entropy_seed_file( char *path, int ret ) { mbedtls_entropy_context ctx; @@ -80,7 +80,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE */ +/* BEGIN_CASE depends_on:ENTROPY_HAVE_STRONG */ void entropy_func_len( int len, int ret ) { mbedtls_entropy_context ctx; @@ -141,7 +141,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE */ +/* BEGIN_CASE depends_on:ENTROPY_HAVE_STRONG */ void entropy_threshold( int threshold, int chunk_size, int result ) { mbedtls_entropy_context ctx; @@ -172,7 +172,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_SELF_TEST */ +/* BEGIN_CASE depends_on:MBEDTLS_SELF_TEST:ENTROPY_HAVE_STRONG */ void entropy_selftest( ) { TEST_ASSERT( mbedtls_entropy_self_test( 0 ) == 0 ); diff --git a/tests/suites/test_suite_rsa.function b/tests/suites/test_suite_rsa.function index bdf3db8c2..181eb0df6 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:ENTROPY_HAVE_STRONG */ void mbedtls_rsa_gen_key( int nrbits, int exponent, int result) { mbedtls_rsa_context ctx; From d2cc7ce4cb3303a113e57bfed062188fe6466fdc Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 8 Sep 2017 10:47:33 +0100 Subject: [PATCH 6/7] Correct definition of ENTROPY_HAVE_STRONG Mbed TLS 2.1 doesn't have MBEDTLS_TEST_NULL_ENTROPY macro. --- tests/suites/helpers.function | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index b80831ee5..5c20f81ca 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -59,11 +59,10 @@ typedef UINT32 uint32_t; /* 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) ) ) +#if !defined(MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES) && \ + ( !defined(MBEDTLS_NO_PLATFORM_ENTROPY) || \ + defined(MBEDTLS_HAVEGE_C) || \ + defined(MBEDTLS_ENTROPY_HARDWARE_ALT) ) #define ENTROPY_HAVE_STRONG #endif From 026d18aefa7e615e8ed7c3393efb8c8961835d0d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 28 Nov 2017 17:40:56 +0100 Subject: [PATCH 7/7] Add ChangeLog entry --- ChangeLog | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog b/ChangeLog index 1d06476d7..e62235d68 100644 --- a/ChangeLog +++ b/ChangeLog @@ -42,6 +42,7 @@ Bugfix encoded X509 CSRs. The overflow would enable maliciously constructed CSRs to bypass the version verification check. Found by Peng Li/Yueh-Hsun Lin, KNOX Security, Samsung Research America + * Fix bugs in RSA test suite under MBEDTLS_NO_PLATFORM_ENTROPY. #1023 #1024 Changes * Avoid shadowing of time and index functions through mbed TLS function