From 4a6a55cae364e955cd324bfc91685f0628e1a63a Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Fri, 28 Jun 2019 14:14:02 +0200 Subject: [PATCH 1/3] Fix handling of md failure The failure of mbedtls_md was not checked in one place. This could have led to an incorrect computation if a hardware accelerator failed. In most cases this would have led to the key exchange failing, so the impact would have been a hard-to-diagnose error reported in the wrong place. If the two sides of the key exchange failed in the same way with an output from mbedtls_md that was independent of the input, this could have led to an apparently successful key exchange with a predictable key, thus a glitching md accelerator could have caused a security vulnerability. --- library/ecjpake.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ecjpake.c b/library/ecjpake.c index b276514e8..1845c936a 100644 --- a/library/ecjpake.c +++ b/library/ecjpake.c @@ -226,7 +226,7 @@ static int ecjpake_hash( const mbedtls_md_info_t *md_info, p += id_len; /* Compute hash */ - mbedtls_md( md_info, buf, p - buf, hash ); + MBEDTLS_MPI_CHK( mbedtls_md( md_info, buf, p - buf, hash ) ); /* Turn it into an integer mod n */ MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( h, hash, From ad1836af58a33e82eafa7b059a7ecadd544ee8fb Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Fri, 28 Jun 2019 14:17:04 +0200 Subject: [PATCH 2/3] Add a test for mlaformed ECJPAKE context --- tests/suites/test_suite_ecjpake.data | 3 +++ tests/suites/test_suite_ecjpake.function | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/tests/suites/test_suite_ecjpake.data b/tests/suites/test_suite_ecjpake.data index 84c99c985..ffa59e546 100644 --- a/tests/suites/test_suite_ecjpake.data +++ b/tests/suites/test_suite_ecjpake.data @@ -4,6 +4,9 @@ ecjpake_invalid_param: ECJPAKE selftest ecjpake_selftest: +ECJPAKE fail read corrupt MD +read_bad_md:"41047ea6e3a4487037a9e0dbd79262b2cc273e779930fc18409ac5361c5fe669d702e147790aeb4ce7fd6575ab0f6c7fd1c335939aa863ba37ec91b7e32bb013bb2b410409f85b3d20ebd7885ce464c08d056d6428fe4dd9287aa365f131f4360ff386d846898bc4b41583c2a5197f65d78742746c12a5ec0a4ffe2f270a750a1d8fb51620934d74eb43e54df424fd96306c0117bf131afabf90a9d33d1198d905193735144104190a07700ffa4be6ae1d79ee0f06aeb544cd5addaabedf70f8623321332c54f355f0fbfec783ed359e5d0bf7377a0fc4ea7ace473c9c112b41ccd41ac56a56124104360a1cea33fce641156458e0a4eac219e96831e6aebc88b3f3752f93a0281d1bf1fb106051db9694a8d6e862a5ef1324a3d9e27894f1ee4f7c59199965a8dd4a2091847d2d22df3ee55faa2a3fb33fd2d1e055a07a7c61ecfb8d80ec00c2c9eb12" + ECJPAKE round one: client, valid read_round_one:MBEDTLS_ECJPAKE_CLIENT:"41047ea6e3a4487037a9e0dbd79262b2cc273e779930fc18409ac5361c5fe669d702e147790aeb4ce7fd6575ab0f6c7fd1c335939aa863ba37ec91b7e32bb013bb2b410409f85b3d20ebd7885ce464c08d056d6428fe4dd9287aa365f131f4360ff386d846898bc4b41583c2a5197f65d78742746c12a5ec0a4ffe2f270a750a1d8fb51620934d74eb43e54df424fd96306c0117bf131afabf90a9d33d1198d905193735144104190a07700ffa4be6ae1d79ee0f06aeb544cd5addaabedf70f8623321332c54f355f0fbfec783ed359e5d0bf7377a0fc4ea7ace473c9c112b41ccd41ac56a56124104360a1cea33fce641156458e0a4eac219e96831e6aebc88b3f3752f93a0281d1bf1fb106051db9694a8d6e862a5ef1324a3d9e27894f1ee4f7c59199965a8dd4a2091847d2d22df3ee55faa2a3fb33fd2d1e055a07a7c61ecfb8d80ec00c2c9eb12":0 diff --git a/tests/suites/test_suite_ecjpake.function b/tests/suites/test_suite_ecjpake.function index d26729522..38f190de2 100644 --- a/tests/suites/test_suite_ecjpake.function +++ b/tests/suites/test_suite_ecjpake.function @@ -236,6 +236,27 @@ void ecjpake_selftest( ) } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA256_C */ +void read_bad_md( data_t *msg ) +{ + mbedtls_ecjpake_context corrupt_ctx; + const unsigned char * pw = NULL; + const size_t pw_len = 0; + int any_role = MBEDTLS_ECJPAKE_CLIENT; + + mbedtls_ecjpake_init( &corrupt_ctx ); + TEST_ASSERT( mbedtls_ecjpake_setup( &corrupt_ctx, any_role, + MBEDTLS_MD_SHA256, MBEDTLS_ECP_DP_SECP256R1, pw, pw_len ) == 0 ); + corrupt_ctx.md_info = NULL; + + TEST_ASSERT( mbedtls_ecjpake_read_round_one( &corrupt_ctx, msg->x, + msg->len ) == MBEDTLS_ERR_MD_BAD_INPUT_DATA ); + +exit: + mbedtls_ecjpake_free( &corrupt_ctx ); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA256_C */ void read_round_one( int role, data_t * msg, int ref_ret ) { From 50be358479472b3affcab8cf05b0ecb3a0cc6225 Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Wed, 10 Jul 2019 11:43:23 +0200 Subject: [PATCH 3/3] Add a change log entry --- ChangeLog | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ChangeLog b/ChangeLog index 9702af207..a9c853edf 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,11 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS x.x.x branch released xxxx-xx-xx +Security + * Fix a missing error detection in ECJPAKE. This could have caused a + predictable shared secret if a hardware accelerator failed and the other + side of the key exchange had a similar bug. + Bugfix * Fix to allow building test suites with any warning that detects unused functions. Fixes #1628.