diff --git a/.travis.yml b/.travis.yml index 8789c444b4..9ba98ab8b9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -32,12 +32,6 @@ jobs: script: - tests/scripts/all.sh -k 'test_depends_*' 'build_key_exchanges' - - name: macOS - os: osx - compiler: clang - script: - - tests/scripts/all.sh -k test_default_out_of_box - - name: Windows os: windows script: diff --git a/ChangeLog.d/fix-ssl-cf-hmac-alt.txt b/ChangeLog.d/fix-ssl-cf-hmac-alt.txt new file mode 100644 index 0000000000..e77613f03d --- /dev/null +++ b/ChangeLog.d/fix-ssl-cf-hmac-alt.txt @@ -0,0 +1,5 @@ +Bugfix + * Fix a regression introduced in 2.16.8 which broke (D)TLS CBC ciphersuites + (when the encrypt-then-MAC extension is not in use) with some ALT + implementations of the underlying hash (SHA-1, SHA-256, SHA-384), causing + the affected side to wrongly reject valid messages. Fixes #4118. diff --git a/library/nist_kw.c b/library/nist_kw.c index 8341ff1303..278b7e91ab 100644 --- a/library/nist_kw.c +++ b/library/nist_kw.c @@ -219,8 +219,6 @@ int mbedtls_nist_kw_wrap( mbedtls_nist_kw_context *ctx, uint64_t t = 0; unsigned char outbuff[KW_SEMIBLOCK_LENGTH * 2]; unsigned char inbuff[KW_SEMIBLOCK_LENGTH * 2]; - unsigned char *R2 = output + KW_SEMIBLOCK_LENGTH; - unsigned char *A = output; *out_len = 0; /* @@ -296,6 +294,9 @@ int mbedtls_nist_kw_wrap( mbedtls_nist_kw_context *ctx, } else { + unsigned char *R2 = output + KW_SEMIBLOCK_LENGTH; + unsigned char *A = output; + /* * Do the wrapping function W, as defined in RFC 3394 section 2.2.1 */ @@ -359,7 +360,7 @@ static int unwrap( mbedtls_nist_kw_context *ctx, uint64_t t = 0; unsigned char outbuff[KW_SEMIBLOCK_LENGTH * 2]; unsigned char inbuff[KW_SEMIBLOCK_LENGTH * 2]; - unsigned char *R = output + ( semiblocks - 2 ) * KW_SEMIBLOCK_LENGTH; + unsigned char *R = NULL; *out_len = 0; if( semiblocks < MIN_SEMIBLOCKS_COUNT ) @@ -369,6 +370,7 @@ static int unwrap( mbedtls_nist_kw_context *ctx, memcpy( A, input, KW_SEMIBLOCK_LENGTH ); memmove( output, input + KW_SEMIBLOCK_LENGTH, ( semiblocks - 1 ) * KW_SEMIBLOCK_LENGTH ); + R = output + ( semiblocks - 2 ) * KW_SEMIBLOCK_LENGTH; /* Calculate intermediate values */ for( t = s; t >= 1; t-- ) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index a00b439ef4..1a0794afa6 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1895,6 +1895,9 @@ int mbedtls_ssl_cf_hmac( MD_CHK( mbedtls_md_update( ctx, data + offset, 1 ) ); } + /* The context needs to finish() before it starts() again */ + MD_CHK( mbedtls_md_finish( ctx, aux_out ) ); + /* Now compute HASH(okey + inner_hash) */ MD_CHK( mbedtls_md_starts( ctx ) ); MD_CHK( mbedtls_md_update( ctx, okey, block_size ) ); diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index ceac6703ff..ae207fd042 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -160,6 +160,66 @@ typedef enum } \ } while( 0 ) +/** Evaluate two expressions and fail the test case if they have different + * values. + * + * \param expr1 An expression to evaluate. + * \param expr2 The expected value of \p expr1. This can be any + * expression, but it is typically a constant. + */ +#define TEST_EQUAL( expr1, expr2 ) \ + TEST_ASSERT( ( expr1 ) == ( expr2 ) ) + +/** Allocate memory dynamically and fail the test case if this fails. + * The allocated memory will be filled with zeros. + * + * You must set \p pointer to \c NULL before calling this macro and + * put `mbedtls_free( pointer )` in the test's cleanup code. + * + * If \p length is zero, the resulting \p pointer will be \c NULL. + * This is usually what we want in tests since API functions are + * supposed to accept null pointers when a buffer size is zero. + * + * This macro expands to an instruction, not an expression. + * It may jump to the \c exit label. + * + * \param pointer An lvalue where the address of the allocated buffer + * will be stored. + * This expression may be evaluated multiple times. + * \param length Number of elements to allocate. + * This expression may be evaluated multiple times. + * + */ +#define ASSERT_ALLOC( pointer, length ) \ + do \ + { \ + TEST_ASSERT( ( pointer ) == NULL ); \ + if( ( length ) != 0 ) \ + { \ + ( pointer ) = mbedtls_calloc( sizeof( *( pointer ) ), \ + ( length ) ); \ + TEST_ASSERT( ( pointer ) != NULL ); \ + } \ + } \ + while( 0 ) + +/** Allocate memory dynamically. If the allocation fails, skip the test case. + * + * This macro behaves like #ASSERT_ALLOC, except that if the allocation + * fails, it marks the test as skipped rather than failed. + */ +#define ASSERT_ALLOC_WEAK( pointer, length ) \ + do \ + { \ + TEST_ASSERT( ( pointer ) == NULL ); \ + if( ( length ) != 0 ) \ + { \ + ( pointer ) = mbedtls_calloc( sizeof( *( pointer ) ), \ + ( length ) ); \ + TEST_ASSUME( ( pointer ) != NULL ); \ + } \ + } \ + while( 0 ) /** Compare two buffers and fail the test case if they differ. * * This macro expands to an instruction, not an expression. diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index 9c90e9c2a5..3e901f7ddb 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -1,6 +1,25 @@ /* BEGIN_HEADER */ #include "mbedtls/ecp.h" +/* Backported from Mbed TLS 2.x for test dependencies. */ +#if defined(MBEDTLS_ECP_DP_SECP192R1_ENABLED) || \ + defined(MBEDTLS_ECP_DP_SECP224R1_ENABLED) || \ + defined(MBEDTLS_ECP_DP_SECP256R1_ENABLED) || \ + defined(MBEDTLS_ECP_DP_SECP384R1_ENABLED) || \ + defined(MBEDTLS_ECP_DP_SECP521R1_ENABLED) || \ + defined(MBEDTLS_ECP_DP_BP256R1_ENABLED) || \ + defined(MBEDTLS_ECP_DP_BP384R1_ENABLED) || \ + defined(MBEDTLS_ECP_DP_BP512R1_ENABLED) || \ + defined(MBEDTLS_ECP_DP_SECP192K1_ENABLED) || \ + defined(MBEDTLS_ECP_DP_SECP224K1_ENABLED) || \ + defined(MBEDTLS_ECP_DP_SECP256K1_ENABLED) +#define MBEDTLS_ECP_SHORT_WEIERSTRASS_ENABLED +#endif +#if defined(MBEDTLS_ECP_DP_CURVE25519_ENABLED) || \ + defined(MBEDTLS_ECP_DP_CURVE448_ENABLED) +#define MBEDTLS_ECP_MONTGOMERY_ENABLED +#endif + #define ECP_PF_UNKNOWN -1 #define ECP_PT_RESET( x ) \ diff --git a/tests/suites/test_suite_pkparse.data b/tests/suites/test_suite_pkparse.data index cd842cf8bd..d4a0e44219 100644 --- a/tests/suites/test_suite_pkparse.data +++ b/tests/suites/test_suite_pkparse.data @@ -989,7 +989,7 @@ depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_BP512R1_ENABLED pk_parse_public_keyfile_ec:"data_files/ec_bp512_pub.pem":0 Parse EC Key #1 (SEC1 DER) -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED +depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED pk_parse_keyfile_ec:"data_files/ec_prv.sec1.der":"NULL":0 Parse EC Key #2 (SEC1 PEM) @@ -1005,15 +1005,15 @@ depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED pk_parse_keyfile_ec:"data_files/ec_prv.pk8.der":"NULL":0 Parse EC Key #4a (PKCS8 DER, no public key) -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED pk_parse_keyfile_ec:"data_files/ec_prv.pk8nopub.der":"NULL":0 Parse EC Key #4b (PKCS8 DER, no public key, with parameters) -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED pk_parse_keyfile_ec:"data_files/ec_prv.pk8nopubparam.der":"NULL":0 Parse EC Key #4c (PKCS8 DER, with parameters) -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED pk_parse_keyfile_ec:"data_files/ec_prv.pk8param.der":"NULL":0 Parse EC Key #5 (PKCS8 PEM) @@ -1069,7 +1069,7 @@ depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_BP512R1_ENABLED pk_parse_keyfile_ec:"data_files/ec_bp512_prv.pem":"NULL":0 Parse EC Key #15 (SEC1 DER, secp256k1, SpecifiedECDomain) -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256K1_ENABLED:MBEDTLS_PK_PARSE_EC_EXTENDED +depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256K1_ENABLED:MBEDTLS_PK_PARSE_EC_EXTENDED pk_parse_keyfile_ec:"data_files/ec_prv.specdom.der":"NULL":0 Key ASN1 (No data)