From 02e79a4e4388e26a724173b5ee7b7d098e4aabad Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 7 Oct 2019 17:06:06 +0200 Subject: [PATCH 1/5] MBEDTLS_CTR_DRBG_USE_128_BIT_KEY: add selftest data In the CTR_DRBG module, add selftest data for when MBEDTLS_CTR_DRBG_USE_128_BIT_KEY is enabled. I generated the test data by running our own code. This is ok because we have other tests that ensure that the algorithm is implemented correctly. This makes programs/self/selftest pass when MBEDTLS_CTR_DRBG_USE_128_BIT_KEY is enabled. --- library/ctr_drbg.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/library/ctr_drbg.c b/library/ctr_drbg.c index 047bb2a3e1..b6fcc0203f 100644 --- a/library/ctr_drbg.c +++ b/library/ctr_drbg.c @@ -712,6 +712,15 @@ static const unsigned char nonce_pers_nopr[16] = { 0x1b, 0x54, 0xb8, 0xff, 0x06, 0x42, 0xbf, 0xf5, 0x21, 0xf1, 0x5c, 0x1c, 0x0b, 0x66, 0x5f, 0x3f }; +#if defined(MBEDTLS_CTR_DRBG_USE_128_BIT_KEY) +static const unsigned char result_pr[16] = + { 0x95, 0x3c, 0xa5, 0xbd, 0x44, 0x1, 0x34, 0xb7, + 0x13, 0x58, 0x3e, 0x6a, 0x6c, 0x7e, 0x88, 0x8a }; + +static const unsigned char result_nopr[16] = + { 0x6c, 0x25, 0x27, 0x95, 0xa3, 0x62, 0xd6, 0xdb, + 0x90, 0xfd, 0x69, 0xb5, 0x42, 0x9, 0x4b, 0x84 }; +#else /* MBEDTLS_CTR_DRBG_USE_128_BIT_KEY */ static const unsigned char result_pr[16] = { 0x34, 0x01, 0x16, 0x56, 0xb4, 0x29, 0x00, 0x8f, 0x35, 0x63, 0xec, 0xb5, 0xf2, 0x59, 0x07, 0x23 }; @@ -719,6 +728,7 @@ static const unsigned char result_pr[16] = static const unsigned char result_nopr[16] = { 0xa0, 0x54, 0x30, 0x3d, 0x8a, 0x7e, 0xa9, 0x88, 0x9d, 0x90, 0x3e, 0x07, 0x7c, 0x6f, 0x21, 0x8f }; +#endif /* MBEDTLS_CTR_DRBG_USE_128_BIT_KEY */ static size_t test_offset; static int ctr_drbg_self_test_entropy( void *data, unsigned char *buf, From bbf67b98bb2b8c0245cc6225772498c9fe6471f5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 7 Oct 2019 17:07:30 +0200 Subject: [PATCH 2/5] Remove selftest dependency in the test suite The test suites should always run self-tests for all enabled features. Otherwise we miss failing self-tests in CI runs, because we don't always run the selftest program independently. There was one spurious dependency to remove: MBEDTLS_CTR_DRBG_USE_128_BIT_KEY for ctr_drbg, which was broken but has now been fixed. --- tests/suites/test_suite_ctr_drbg.data | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/suites/test_suite_ctr_drbg.data b/tests/suites/test_suite_ctr_drbg.data index b50df2ba33..09195f04bd 100644 --- a/tests/suites/test_suite_ctr_drbg.data +++ b/tests/suites/test_suite_ctr_drbg.data @@ -1097,6 +1097,4 @@ CTR_DRBG Special Behaviours ctr_drbg_special_behaviours: CTR_DRBG self test -depends_on:!MBEDTLS_CTR_DRBG_USE_128_BIT_KEY ctr_drbg_selftest: - From 80a607171ad14ff02e9340074f259c3fa951cd8b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 7 Oct 2019 17:11:03 +0200 Subject: [PATCH 3/5] config.pl full: exclude MBEDTLS_ENTROPY_FORCE_SHA256 This is a variant toggle, not an extra feature, so it should be tested separately. We test most of the effect of MBEDTLS_ENTROPY_FORCE_SHA256 (namely, using SHA-256 in the entropy module) when we test the library with the SHA512 module disabled (which we do at least via depends-hashes.pl). This commit removes testing of the MBEDTLS_ENTROPY_FORCE_SHA256 option itself, which should be added separately. --- scripts/config.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/config.py b/scripts/config.py index db2661c92b..cb0e1c5fe8 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -168,6 +168,7 @@ def include_in_full(name): 'MBEDTLS_DEPRECATED_REMOVED', 'MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED', 'MBEDTLS_ECP_RESTARTABLE', + 'MBEDTLS_ENTROPY_FORCE_SHA256', # Variant toggle, tested separately 'MBEDTLS_HAVE_SSE2', 'MBEDTLS_MEMORY_BACKTRACE', 'MBEDTLS_MEMORY_BUFFER_ALLOC_C', From 2ef377d56de6da8ccfe249a88c05d19677a8fbcc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 7 Oct 2019 18:44:21 +0200 Subject: [PATCH 4/5] all.sh: support variable seedfile size The size of the seedfile used by the entropy module when MBEDTLS_ENTROPY_NV_SEED is enabled is 32 byte when MBEDTLS_ENTROPY_FORCE_SHA256 is enabled or MBEDTLS_SHA512_C is disabled, and 64 bytes otherwise. A larger seedfile is ok on entry (the code just grabs the first N bytes), but a smaller seedfile is not ok. Therefore, if you run a component with a 32-byte seedfile and then a component with a 64-byte seedfile, the second component fails in the unit tests (up to test_suite_entropy which erases the seedfile and creates a fresh one). This is ok up to now because we only enable MBEDTLS_ENTROPY_NV_SEED together with MBEDTLS_ENTROPY_FORCE_SHA256. But it prevents enabling MBEDTLS_ENTROPY_NV_SEED without MBEDTLS_ENTROPY_FORCE_SHA256. To fix this, unconditionally create a seedfile before each component. --- tests/scripts/all.sh | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 2567cc0dd3..3b2aef3247 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -403,12 +403,6 @@ pre_check_git () { fi } -pre_check_seedfile () { - if [ ! -f "./tests/seedfile" ]; then - dd if=/dev/urandom of=./tests/seedfile bs=32 count=1 - fi -} - pre_setup_keep_going () { failure_summary= failure_count=0 @@ -1272,7 +1266,16 @@ run_component () { cp -p "$CONFIG_H" "$CONFIG_BAK" current_component="$1" export MBEDTLS_TEST_CONFIGURATION="$current_component" + + # Unconditionally create a seedfile that's sufficiently long. + # Do this before each component, because a previous component may + # have messed it up or shortened it. + dd if=/dev/urandom of=./tests/seedfile bs=64 count=1 + + # Run the component code. "$@" + + # Restore the build tree to a clean state. cleanup } @@ -1282,7 +1285,6 @@ pre_initialize_variables pre_parse_command_line "$@" pre_check_git -pre_check_seedfile build_status=0 if [ $KEEP_GOING -eq 1 ]; then From 592f591c0df15ee1fe966f0c7bc2c3dd048912fe Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 7 Oct 2019 18:49:32 +0200 Subject: [PATCH 5/5] all.sh: test CTR_DRBG_USE_128_BIT_KEY and ENTROPY_FORCE_SHA256 Test MBEDTLS_CTR_DRBG_USE_128_BIT_KEY and MBEDTLS_ENTROPY_FORCE_SHA256 together and separately. --- tests/scripts/all.sh | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 3b2aef3247..02ca38173f 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -920,6 +920,43 @@ component_test_aes_fewer_tables_and_rom_tables () { make test } +component_test_ctr_drbg_aes_256_sha_256 () { + msg "build: full + MBEDTLS_ENTROPY_FORCE_SHA256 (ASan build)" + scripts/config.pl full + scripts/config.pl unset MBEDTLS_MEMORY_BUFFER_ALLOC_C + scripts/config.pl set MBEDTLS_ENTROPY_FORCE_SHA256 + CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . + make + + msg "test: full + MBEDTLS_ENTROPY_FORCE_SHA256 (ASan build)" + make test +} + +component_test_ctr_drbg_aes_128_sha_512 () { + msg "build: full + MBEDTLS_CTR_DRBG_USE_128_BIT_KEY (ASan build)" + scripts/config.pl full + scripts/config.pl unset MBEDTLS_MEMORY_BUFFER_ALLOC_C + scripts/config.pl set MBEDTLS_CTR_DRBG_USE_128_BIT_KEY + CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . + make + + msg "test: full + MBEDTLS_CTR_DRBG_USE_128_BIT_KEY (ASan build)" + make test +} + +component_test_ctr_drbg_aes_128_sha_256 () { + msg "build: full + MBEDTLS_CTR_DRBG_USE_128_BIT_KEY + MBEDTLS_ENTROPY_FORCE_SHA256 (ASan build)" + scripts/config.pl full + scripts/config.pl unset MBEDTLS_MEMORY_BUFFER_ALLOC_C + scripts/config.pl set MBEDTLS_CTR_DRBG_USE_128_BIT_KEY + scripts/config.pl set MBEDTLS_ENTROPY_FORCE_SHA256 + CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . + make + + msg "test: full + MBEDTLS_CTR_DRBG_USE_128_BIT_KEY + MBEDTLS_ENTROPY_FORCE_SHA256 (ASan build)" + make test +} + component_test_se_default () { msg "build: default config + MBEDTLS_PSA_CRYPTO_SE_C" scripts/config.py set MBEDTLS_PSA_CRYPTO_SE_C