From ab43997f44b03c2299e3c56f168a0b74c60a6d9f Mon Sep 17 00:00:00 2001 From: Jaeden Amero Date: Fri, 15 Feb 2019 14:12:05 +0000 Subject: [PATCH] psa: Disallow use of invalid cipher contexts Ensure that when doing cipher operations out of order, PSA_ERROR_BAD_STATE is returned as documented in crypto.h and the PSA Crypto specification. --- library/psa_crypto.c | 6 + tests/suites/test_suite_psa_crypto.data | 4 + tests/suites/test_suite_psa_crypto.function | 161 ++++++++++++++++++++ 3 files changed, 171 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 9bfe8d28c7..4075c658fa 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3073,6 +3073,12 @@ psa_status_t psa_cipher_update( psa_cipher_operation_t *operation, psa_status_t status; int ret; size_t expected_output_size; + + if( operation->alg == 0 ) + { + return( PSA_ERROR_BAD_STATE ); + } + if( ! PSA_ALG_IS_STREAM_CIPHER( operation->alg ) ) { /* Take the unprocessed partial block left over from previous diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 489389a847..098e3f925b 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -926,6 +926,10 @@ depends_on:MBEDTLS_ARC4_C:MBEDTLS_CIPHER_MODE_CTR # Either INVALID_ARGUMENT or NOT_SUPPORTED would be reasonable here cipher_setup:PSA_KEY_TYPE_ARC4:"000102030405060708090a0b0c0d0e0f":PSA_ALG_CTR:PSA_ERROR_NOT_SUPPORTED +PSA cipher: bad order function calls +depends_on:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC +cipher_bad_order: + PSA symmetric encrypt: AES-CBC-nopad, 16 bytes, good depends_on:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC cipher_encrypt:PSA_ALG_CBC_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"6bc1bee22e409f96e93d7e117393172a":"a076ec9dfbe47d52afc357336f20743b":PSA_SUCCESS diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 37b4d8d699..d1364b923b 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -2447,6 +2447,9 @@ exit: /* BEGIN_CASE */ void cipher_operation_init( ) { + const uint8_t input[1] = { 0 }; + unsigned char output[1] = { 0 }; + size_t output_length; /* 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 @@ -2457,6 +2460,23 @@ void cipher_operation_init( ) memset( &zero, 0, sizeof( zero ) ); + /* A freshly-initialized cipher operation should not be usable. */ + TEST_EQUAL( psa_cipher_update( &func, + input, sizeof( input ), + output, sizeof( output ), + &output_length ), + PSA_ERROR_BAD_STATE ); + TEST_EQUAL( psa_cipher_update( &init, + input, sizeof( input ), + output, sizeof( output ), + &output_length ), + PSA_ERROR_BAD_STATE ); + TEST_EQUAL( psa_cipher_update( &zero, + input, sizeof( input ), + output, sizeof( output ), + &output_length ), + PSA_ERROR_BAD_STATE ); + /* A default cipher operation should be abortable without error. */ PSA_ASSERT( psa_cipher_abort( &func ) ); PSA_ASSERT( psa_cipher_abort( &init ) ); @@ -2497,6 +2517,147 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void cipher_bad_order( ) +{ + psa_key_handle_t handle = 0; + psa_key_type_t key_type = PSA_KEY_TYPE_AES; + psa_algorithm_t alg = PSA_ALG_CBC_PKCS7; + psa_key_policy_t policy = PSA_KEY_POLICY_INIT; + psa_cipher_operation_t operation = PSA_CIPHER_OPERATION_INIT; + unsigned char iv[PSA_BLOCK_CIPHER_BLOCK_SIZE(PSA_KEY_TYPE_AES)] = { 0 }; + const uint8_t key[] = { + 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, + 0xaa, 0xaa, 0xaa, 0xaa }; + const uint8_t text[] = { + 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, + 0xbb, 0xbb, 0xbb, 0xbb }; + uint8_t buffer[PSA_BLOCK_CIPHER_BLOCK_SIZE(PSA_KEY_TYPE_AES)] = { 0 }; + size_t length = 0; + + PSA_ASSERT( psa_crypto_init( ) ); + PSA_ASSERT( psa_allocate_key( &handle ) ); + psa_key_policy_set_usage( &policy, + PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT, + alg ); + PSA_ASSERT( psa_set_key_policy( handle, &policy ) ); + PSA_ASSERT( psa_import_key( handle, key_type, + key, sizeof(key) ) ); + + + /* Generate an IV without calling setup beforehand. */ + TEST_EQUAL( psa_cipher_generate_iv( &operation, + buffer, sizeof( buffer ), + &length ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_cipher_abort( &operation ) ); + + /* Generate an IV twice in a row. */ + PSA_ASSERT( psa_cipher_encrypt_setup( &operation, handle, alg ) ); + PSA_ASSERT( psa_cipher_generate_iv( &operation, + buffer, sizeof( buffer ), + &length ) ); + TEST_EQUAL( psa_cipher_generate_iv( &operation, + buffer, sizeof( buffer ), + &length ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_cipher_abort( &operation ) ); + + /* Generate an IV after it's already set. */ + PSA_ASSERT( psa_cipher_encrypt_setup( &operation, handle, alg ) ); + PSA_ASSERT( psa_cipher_set_iv( &operation, + iv, sizeof( iv ) ) ); + TEST_EQUAL( psa_cipher_generate_iv( &operation, + buffer, sizeof( buffer ), + &length ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_cipher_abort( &operation ) ); + + /* Set an IV without calling setup beforehand. */ + TEST_EQUAL( psa_cipher_set_iv( &operation, + iv, sizeof( iv ) ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_cipher_abort( &operation ) ); + + /* Set an IV after it's already set. */ + PSA_ASSERT( psa_cipher_encrypt_setup( &operation, handle, alg ) ); + PSA_ASSERT( psa_cipher_set_iv( &operation, + iv, sizeof( iv ) ) ); + TEST_EQUAL( psa_cipher_set_iv( &operation, + iv, sizeof( iv ) ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_cipher_abort( &operation ) ); + + /* Set an IV after it's already generated. */ + PSA_ASSERT( psa_cipher_encrypt_setup( &operation, handle, alg ) ); + PSA_ASSERT( psa_cipher_generate_iv( &operation, + buffer, sizeof( buffer ), + &length ) ); + TEST_EQUAL( psa_cipher_set_iv( &operation, + iv, sizeof( iv ) ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_cipher_abort( &operation ) ); + + /* Call update without calling setup beforehand. */ + TEST_EQUAL( psa_cipher_update( &operation, + text, sizeof( text ), + buffer, sizeof( buffer ), + &length ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_cipher_abort( &operation ) ); + + /* Call update without an IV where an IV is required. */ + TEST_EQUAL( psa_cipher_update( &operation, + text, sizeof( text ), + buffer, sizeof( buffer ), + &length ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_cipher_abort( &operation ) ); + + /* Call update after finish. */ + PSA_ASSERT( psa_cipher_encrypt_setup( &operation, handle, alg ) ); + PSA_ASSERT( psa_cipher_set_iv( &operation, + iv, sizeof( iv ) ) ); + PSA_ASSERT( psa_cipher_finish( &operation, + buffer, sizeof( buffer ), &length ) ); + TEST_EQUAL( psa_cipher_update( &operation, + text, sizeof( text ), + buffer, sizeof( buffer ), + &length ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_cipher_abort( &operation ) ); + + /* Call finish without calling setup beforehand. */ + TEST_EQUAL( psa_cipher_finish( &operation, + buffer, sizeof( buffer ), &length ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_cipher_abort( &operation ) ); + + /* Call finish without an IV where an IV is required. */ + PSA_ASSERT( psa_cipher_encrypt_setup( &operation, handle, alg ) ); + /* Not calling update means we are encrypting an empty buffer, which is OK + * for cipher modes with padding. */ + TEST_EQUAL( psa_cipher_finish( &operation, + buffer, sizeof( buffer ), &length ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_cipher_abort( &operation ) ); + + /* Call finish twice in a row. */ + PSA_ASSERT( psa_cipher_encrypt_setup( &operation, handle, alg ) ); + PSA_ASSERT( psa_cipher_set_iv( &operation, + iv, sizeof( iv ) ) ); + PSA_ASSERT( psa_cipher_finish( &operation, + buffer, sizeof( buffer ), &length ) ); + TEST_EQUAL( psa_cipher_finish( &operation, + buffer, sizeof( buffer ), &length ), + PSA_ERROR_BAD_STATE ); + PSA_ASSERT( psa_cipher_abort( &operation ) ); + +exit: + mbedtls_psa_crypto_free( ); +} +/* END_CASE */ + /* BEGIN_CASE */ void cipher_encrypt( int alg_arg, int key_type_arg, data_t *key,