From 252ef28dac1eca278b409a05636b587ea0d29f2f Mon Sep 17 00:00:00 2001 From: Jaeden Amero Date: Fri, 15 Feb 2019 14:05:35 +0000 Subject: [PATCH] psa: Disallow use of invalid MAC contexts Ensure that when doing MAC operations out of order, PSA_ERROR_BAD_STATE is returned as documented in crypto.h and the PSA Crypto specification. --- library/psa_crypto.c | 10 ++ tests/suites/test_suite_psa_crypto.data | 4 + tests/suites/test_suite_psa_crypto.function | 129 ++++++++++++++++++++ 3 files changed, 143 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index ad7367b9c..9bfe8d28c 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2238,6 +2238,11 @@ psa_status_t psa_mac_sign_finish( psa_mac_operation_t *operation, { psa_status_t status; + if( operation->alg == 0 ) + { + return( PSA_ERROR_BAD_STATE ); + } + /* Fill the output buffer with something that isn't a valid mac * (barring an attack on the mac and deliberately-crafted input), * in case the caller doesn't check the return status properly. */ @@ -2276,6 +2281,11 @@ psa_status_t psa_mac_verify_finish( psa_mac_operation_t *operation, uint8_t actual_mac[PSA_MAC_MAX_SIZE]; psa_status_t status; + if( operation->alg == 0 ) + { + return( PSA_ERROR_BAD_STATE ); + } + if( operation->is_sign ) { status = PSA_ERROR_BAD_STATE; diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index d9dd9ef48..489389a84 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -705,6 +705,10 @@ depends_on:MBEDTLS_CMAC_C # Either INVALID_ARGUMENT or NOT_SUPPORTED would be reasonable here mac_setup:PSA_KEY_TYPE_HMAC:"000102030405060708090a0b0c0d0e0f":PSA_ALG_CMAC:PSA_ERROR_NOT_SUPPORTED +PSA MAC: bad order function calls +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +mac_bad_order: + PSA MAC sign: RFC4231 Test case 1 - HMAC-SHA-224 depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C mac_sign:PSA_KEY_TYPE_HMAC:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":PSA_ALG_HMAC(PSA_ALG_SHA_224):"4869205468657265":"896fb1128abbdf196832107cd49df33f47b4b1169912ba4f53684b22" diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 929d1b268..37b4d8d69 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -2168,6 +2168,8 @@ exit: /* BEGIN_CASE */ void mac_operation_init( ) { + const uint8_t input[1] = { 0 }; + /* Test each valid way of initializing the object, except for `= {0}`, as * Clang 5 complains when `-Wmissing-field-initializers` is used, even * though it's OK by the C standard. We could test for this, but we'd need @@ -2178,6 +2180,17 @@ void mac_operation_init( ) memset( &zero, 0, sizeof( zero ) ); + /* A freshly-initialized MAC operation should not be usable. */ + TEST_EQUAL( psa_mac_update( &func, + input, sizeof( input ) ), + PSA_ERROR_BAD_STATE ); + TEST_EQUAL( psa_mac_update( &init, + input, sizeof( input ) ), + PSA_ERROR_BAD_STATE ); + TEST_EQUAL( psa_mac_update( &zero, + input, sizeof( input ) ), + PSA_ERROR_BAD_STATE ); + /* A default MAC operation should be abortable without error. */ PSA_ASSERT( psa_mac_abort( &func ) ); PSA_ASSERT( psa_mac_abort( &init ) ); @@ -2220,6 +2233,122 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void mac_bad_order( ) +{ + psa_key_handle_t handle = 0; + psa_key_type_t key_type = PSA_KEY_TYPE_HMAC; + psa_algorithm_t alg = PSA_ALG_HMAC(PSA_ALG_SHA_256); + const uint8_t key[] = { + 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, + 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, + 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa }; + psa_key_policy_t policy = PSA_KEY_POLICY_INIT; + psa_mac_operation_t operation = PSA_MAC_OPERATION_INIT; + uint8_t sign_mac[PSA_MAC_MAX_SIZE + 10] = { 0 }; + size_t sign_mac_length = 0; + const uint8_t input[] = { 0xbb, 0xbb, 0xbb, 0xbb }; + const uint8_t verify_mac[] = { + 0x74, 0x65, 0x93, 0x8c, 0xeb, 0x1d, 0xb3, 0x76, 0x5a, 0x38, 0xe7, 0xdd, + 0x85, 0xc5, 0xad, 0x4f, 0x07, 0xe7, 0xd5, 0xb2, 0x64, 0xf0, 0x1a, 0x1a, + 0x2c, 0xf9, 0x18, 0xca, 0x59, 0x7e, 0x5d, 0xf6 }; + + PSA_ASSERT( psa_crypto_init( ) ); + PSA_ASSERT( psa_allocate_key( &handle ) ); + psa_key_policy_set_usage( &policy, + PSA_KEY_USAGE_SIGN | PSA_KEY_USAGE_VERIFY, + alg ); + PSA_ASSERT( psa_set_key_policy( handle, &policy ) ); + + PSA_ASSERT( psa_import_key( handle, key_type, + key, sizeof(key) ) ); + + /* Call update without calling setup beforehand. */ + TEST_EQUAL( psa_mac_update( &operation, input, sizeof( input ) ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_mac_abort( &operation ) ); + + /* Call sign finish without calling setup beforehand. */ + TEST_EQUAL( psa_mac_sign_finish( &operation, sign_mac, sizeof( sign_mac ), + &sign_mac_length), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_mac_abort( &operation ) ); + + /* Call verify finish without calling setup beforehand. */ + TEST_EQUAL( psa_mac_verify_finish( &operation, + verify_mac, sizeof( verify_mac ) ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_mac_abort( &operation ) ); + + /* Call update after sign finish. */ + PSA_ASSERT( psa_mac_sign_setup( &operation, + handle, alg ) ); + PSA_ASSERT( psa_mac_update( &operation, input, sizeof( input ) ) ); + PSA_ASSERT( psa_mac_sign_finish( &operation, + sign_mac, sizeof( sign_mac ), + &sign_mac_length ) ); + TEST_EQUAL( psa_mac_update( &operation, input, sizeof( input ) ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_mac_abort( &operation ) ); + + /* Call update after verify finish. */ + PSA_ASSERT( psa_mac_verify_setup( &operation, + handle, alg ) ); + PSA_ASSERT( psa_mac_update( &operation, input, sizeof( input ) ) ); + PSA_ASSERT( psa_mac_verify_finish( &operation, + verify_mac, sizeof( verify_mac ) ) ); + TEST_EQUAL( psa_mac_update( &operation, input, sizeof( input ) ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_mac_abort( &operation ) ); + + /* Call sign finish twice in a row. */ + PSA_ASSERT( psa_mac_sign_setup( &operation, + handle, alg ) ); + PSA_ASSERT( psa_mac_update( &operation, input, sizeof( input ) ) ); + PSA_ASSERT( psa_mac_sign_finish( &operation, + sign_mac, sizeof( sign_mac ), + &sign_mac_length ) ); + TEST_EQUAL( psa_mac_sign_finish( &operation, + sign_mac, sizeof( sign_mac ), + &sign_mac_length ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_mac_abort( &operation ) ); + + /* Call verify finish twice in a row. */ + PSA_ASSERT( psa_mac_verify_setup( &operation, + handle, alg ) ); + PSA_ASSERT( psa_mac_update( &operation, input, sizeof( input ) ) ); + PSA_ASSERT( psa_mac_verify_finish( &operation, + verify_mac, sizeof( verify_mac ) ) ); + TEST_EQUAL( psa_mac_verify_finish( &operation, + verify_mac, sizeof( verify_mac ) ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_mac_abort( &operation ) ); + + /* Setup sign but try verify. */ + PSA_ASSERT( psa_mac_sign_setup( &operation, + handle, alg ) ); + PSA_ASSERT( psa_mac_update( &operation, input, sizeof( input ) ) ); + TEST_EQUAL( psa_mac_verify_finish( &operation, + verify_mac, sizeof( verify_mac ) ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_mac_abort( &operation ) ); + + /* Setup verify but try sign. */ + PSA_ASSERT( psa_mac_verify_setup( &operation, + handle, alg ) ); + PSA_ASSERT( psa_mac_update( &operation, input, sizeof( input ) ) ); + TEST_EQUAL( psa_mac_sign_finish( &operation, + sign_mac, sizeof( sign_mac ), + &sign_mac_length ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_mac_abort( &operation ) ); + +exit: + mbedtls_psa_crypto_free( ); +} +/* END_CASE */ + /* BEGIN_CASE */ void mac_sign( int key_type_arg, data_t *key,