Skip to content

Commit 72f8348

Browse files
PataterPatater
authored
Merge pull request #72 from gilles-peskine-arm/psa-fix_setup_cleanup
Fix missing cleanup in psa_cipher_setup
2 parents e23a693 + 9e0a4a5 commit 72f8348

File tree

2 files changed

+220
-62
lines changed

2 files changed

+220
-62
lines changed

‎library/psa_crypto.c

+16-17
Original file line numberDiff line numberDiff line change
@@ -2902,8 +2902,8 @@ static psa_status_t psa_cipher_setup( psa_cipher_operation_t *operation,
29022902
psa_algorithm_t alg,
29032903
mbedtls_operation_t cipher_operation )
29042904
{
2905-
int ret = MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE;
2906-
psa_status_t status;
2905+
int ret = 0;
2906+
psa_status_t status = PSA_ERROR_GENERIC_ERROR;
29072907
psa_key_slot_t *slot;
29082908
size_t key_bits;
29092909
const mbedtls_cipher_info_t *cipher_info = NULL;
@@ -2923,19 +2923,19 @@ static psa_status_t psa_cipher_setup( psa_cipher_operation_t *operation,
29232923

29242924
status = psa_get_key_from_slot( handle, &slot, usage, alg);
29252925
if( status != PSA_SUCCESS )
2926-
return( status );
2926+
goto exit;
29272927
key_bits = psa_get_key_bits( slot );
29282928

29292929
cipher_info = mbedtls_cipher_info_from_psa( alg, slot->type, key_bits, NULL );
29302930
if( cipher_info == NULL )
2931-
return( PSA_ERROR_NOT_SUPPORTED );
2931+
{
2932+
status = PSA_ERROR_NOT_SUPPORTED;
2933+
goto exit;
2934+
}
29322935

29332936
ret = mbedtls_cipher_setup( &operation->ctx.cipher, cipher_info );
29342937
if( ret != 0 )
2935-
{
2936-
psa_cipher_abort( operation );
2937-
return( mbedtls_to_psa_error( ret ) );
2938-
}
2938+
goto exit;
29392939

29402940
#if defined(MBEDTLS_DES_C)
29412941
if( slot->type == PSA_KEY_TYPE_DES && key_bits == 128 )
@@ -2956,10 +2956,7 @@ static psa_status_t psa_cipher_setup( psa_cipher_operation_t *operation,
29562956
(int) key_bits, cipher_operation );
29572957
}
29582958
if( ret != 0 )
2959-
{
2960-
psa_cipher_abort( operation );
2961-
return( mbedtls_to_psa_error( ret ) );
2962-
}
2959+
goto exit;
29632960

29642961
#if defined(MBEDTLS_CIPHER_MODE_WITH_PADDING)
29652962
switch( alg )
@@ -2978,10 +2975,7 @@ static psa_status_t psa_cipher_setup( psa_cipher_operation_t *operation,
29782975
break;
29792976
}
29802977
if( ret != 0 )
2981-
{
2982-
psa_cipher_abort( operation );
2983-
return( mbedtls_to_psa_error( ret ) );
2984-
}
2978+
goto exit;
29852979
#endif //MBEDTLS_CIPHER_MODE_WITH_PADDING
29862980

29872981
operation->key_set = 1;
@@ -2992,7 +2986,12 @@ static psa_status_t psa_cipher_setup( psa_cipher_operation_t *operation,
29922986
operation->iv_size = PSA_BLOCK_CIPHER_BLOCK_SIZE( slot->type );
29932987
}
29942988

2995-
return( PSA_SUCCESS );
2989+
exit:
2990+
if( status == 0 )
2991+
status = mbedtls_to_psa_error( ret );
2992+
if( status != 0 )
2993+
psa_cipher_abort( operation );
2994+
return( status );
29962995
}
29972996

29982997
psa_status_t psa_cipher_encrypt_setup( psa_cipher_operation_t *operation,

‎tests/suites/test_suite_psa_crypto.function

+204-45
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,89 @@
1414
/** An invalid export length that will never be set by psa_export_key(). */
1515
static const size_t INVALID_EXPORT_LENGTH = ~0U;
1616

17+
/* A hash algorithm that is known to be supported.
18+
*
19+
* This is used in some smoke tests.
20+
*/
21+
#if defined(MBEDTLS_MD2_C)
22+
#define KNOWN_SUPPORTED_HASH_ALG PSA_ALG_MD2
23+
#elif defined(MBEDTLS_MD4_C)
24+
#define KNOWN_SUPPORTED_HASH_ALG PSA_ALG_MD4
25+
#elif defined(MBEDTLS_MD5_C)
26+
#define KNOWN_SUPPORTED_HASH_ALG PSA_ALG_MD5
27+
/* MBEDTLS_RIPEMD160_C omitted. This is necessary for the sake of
28+
* exercise_signature_key() because Mbed TLS doesn't support RIPEMD160
29+
* in RSA PKCS#1v1.5 signatures. A RIPEMD160-only configuration would be
30+
* implausible anyway. */
31+
#elif defined(MBEDTLS_SHA1_C)
32+
#define KNOWN_SUPPORTED_HASH_ALG PSA_ALG_SHA_1
33+
#elif defined(MBEDTLS_SHA256_C)
34+
#define KNOWN_SUPPORTED_HASH_ALG PSA_ALG_SHA_256
35+
#elif defined(MBEDTLS_SHA512_C)
36+
#define KNOWN_SUPPORTED_HASH_ALG PSA_ALG_SHA_384
37+
#elif defined(MBEDTLS_SHA3_C)
38+
#define KNOWN_SUPPORTED_HASH_ALG PSA_ALG_SHA3_256
39+
#else
40+
#undef KNOWN_SUPPORTED_HASH_ALG
41+
#endif
42+
43+
/* A block cipher that is known to be supported.
44+
*
45+
* For simplicity's sake, stick to block ciphers with 16-byte blocks.
46+
*/
47+
#if defined(MBEDTLS_AES_C)
48+
#define KNOWN_SUPPORTED_BLOCK_CIPHER PSA_KEY_TYPE_AES
49+
#elif defined(MBEDTLS_ARIA_C)
50+
#define KNOWN_SUPPORTED_BLOCK_CIPHER PSA_KEY_TYPE_ARIA
51+
#elif defined(MBEDTLS_CAMELLIA_C)
52+
#define KNOWN_SUPPORTED_BLOCK_CIPHER PSA_KEY_TYPE_CAMELLIA
53+
#undef KNOWN_SUPPORTED_BLOCK_CIPHER
54+
#endif
55+
56+
/* A MAC mode that is known to be supported.
57+
*
58+
* It must either be HMAC with #KNOWN_SUPPORTED_HASH_ALG or
59+
* a block cipher-based MAC with #KNOWN_SUPPORTED_BLOCK_CIPHER.
60+
*
61+
* This is used in some smoke tests.
62+
*/
63+
#if defined(KNOWN_SUPPORTED_HASH_ALG)
64+
#define KNOWN_SUPPORTED_MAC_ALG ( PSA_ALG_HMAC( KNOWN_SUPPORTED_HASH_ALG ) )
65+
#define KNOWN_SUPPORTED_MAC_KEY_TYPE PSA_KEY_TYPE_HMAC
66+
#elif defined(KNOWN_SUPPORTED_BLOCK_CIPHER) && defined(MBEDTLS_CMAC_C)
67+
#define KNOWN_SUPPORTED_MAC_ALG PSA_ALG_CMAC
68+
#define KNOWN_SUPPORTED_MAC_KEY_TYPE KNOWN_SUPPORTED_BLOCK_CIPHER
69+
#else
70+
#undef KNOWN_SUPPORTED_MAC_ALG
71+
#undef KNOWN_SUPPORTED_MAC_KEY_TYPE
72+
#endif
73+
74+
/* A cipher algorithm and key type that are known to be supported.
75+
*
76+
* This is used in some smoke tests.
77+
*/
78+
#if defined(KNOWN_SUPPORTED_BLOCK_CIPHER) && defined(MBEDTLS_CIPHER_MODE_CTR)
79+
#define KNOWN_SUPPORTED_BLOCK_CIPHER_ALG PSA_ALG_CTR
80+
#elif defined(KNOWN_SUPPORTED_BLOCK_CIPHER) && defined(MBEDTLS_CIPHER_MODE_CBC)
81+
#define KNOWN_SUPPORTED_BLOCK_CIPHER_ALG PSA_ALG_CBC_NO_PADDING
82+
#elif defined(KNOWN_SUPPORTED_BLOCK_CIPHER) && defined(MBEDTLS_CIPHER_MODE_CFB)
83+
#define KNOWN_SUPPORTED_BLOCK_CIPHER_ALG PSA_ALG_CFB
84+
#elif defined(KNOWN_SUPPORTED_BLOCK_CIPHER) && defined(MBEDTLS_CIPHER_MODE_OFB)
85+
#define KNOWN_SUPPORTED_BLOCK_CIPHER_ALG PSA_ALG_OFB
86+
#else
87+
#undef KNOWN_SUPPORTED_BLOCK_CIPHER_ALG
88+
#endif
89+
#if defined(KNOWN_SUPPORTED_BLOCK_CIPHER_ALG)
90+
#define KNOWN_SUPPORTED_CIPHER_ALG KNOWN_SUPPORTED_BLOCK_CIPHER_ALG
91+
#define KNOWN_SUPPORTED_CIPHER_KEY_TYPE KNOWN_SUPPORTED_BLOCK_CIPHER
92+
#elif defined(MBEDTLS_RC4_C)
93+
#define KNOWN_SUPPORTED_CIPHER_ALG PSA_ALG_RC4
94+
#define KNOWN_SUPPORTED_CIPHER_KEY_TYPE PSA_KEY_TYPE_RC4
95+
#else
96+
#undef KNOWN_SUPPORTED_CIPHER_ALG
97+
#undef KNOWN_SUPPORTED_CIPHER_KEY_TYPE
98+
#endif
99+
17100
/** Test if a buffer contains a constant byte value.
18101
*
19102
* `mem_is_char(buffer, c, size)` is true after `memset(buffer, c, size)`.
@@ -120,6 +203,74 @@ static int construct_fake_rsa_key( unsigned char *buffer,
120203
return( len );
121204
}
122205

206+
int exercise_mac_setup( psa_key_type_t key_type,
207+
const unsigned char *key_bytes,
208+
size_t key_length,
209+
psa_algorithm_t alg,
210+
psa_mac_operation_t *operation,
211+
psa_status_t *status )
212+
{
213+
psa_key_handle_t handle = 0;
214+
psa_key_policy_t policy = PSA_KEY_POLICY_INIT;
215+
216+
PSA_ASSERT( psa_allocate_key( &handle ) );
217+
psa_key_policy_set_usage( &policy, PSA_KEY_USAGE_SIGN, alg );
218+
PSA_ASSERT( psa_set_key_policy( handle, &policy ) );
219+
PSA_ASSERT( psa_import_key( handle, key_type, key_bytes, key_length ) );
220+
221+
*status = psa_mac_sign_setup( operation, handle, alg );
222+
/* Whether setup succeeded or failed, abort must succeed. */
223+
PSA_ASSERT( psa_mac_abort( operation ) );
224+
/* If setup failed, reproduce the failure, so that the caller can
225+
* test the resulting state of the operation object. */
226+
if( *status != PSA_SUCCESS )
227+
{
228+
TEST_EQUAL( psa_mac_sign_setup( operation, handle, alg ),
229+
*status );
230+
}
231+
232+
psa_destroy_key( handle );
233+
return( 1 );
234+
235+
exit:
236+
psa_destroy_key( handle );
237+
return( 0 );
238+
}
239+
240+
int exercise_cipher_setup( psa_key_type_t key_type,
241+
const unsigned char *key_bytes,
242+
size_t key_length,
243+
psa_algorithm_t alg,
244+
psa_cipher_operation_t *operation,
245+
psa_status_t *status )
246+
{
247+
psa_key_handle_t handle = 0;
248+
psa_key_policy_t policy = PSA_KEY_POLICY_INIT;
249+
250+
PSA_ASSERT( psa_allocate_key( &handle ) );
251+
psa_key_policy_set_usage( &policy, PSA_KEY_USAGE_ENCRYPT, alg );
252+
PSA_ASSERT( psa_set_key_policy( handle, &policy ) );
253+
PSA_ASSERT( psa_import_key( handle, key_type, key_bytes, key_length ) );
254+
255+
*status = psa_cipher_encrypt_setup( operation, handle, alg );
256+
/* Whether setup succeeded or failed, abort must succeed. */
257+
PSA_ASSERT( psa_cipher_abort( operation ) );
258+
/* If setup failed, reproduce the failure, so that the caller can
259+
* test the resulting state of the operation object. */
260+
if( *status != PSA_SUCCESS )
261+
{
262+
TEST_EQUAL( psa_cipher_encrypt_setup( operation, handle, alg ),
263+
*status );
264+
}
265+
266+
psa_destroy_key( handle );
267+
return( 1 );
268+
269+
exit:
270+
psa_destroy_key( handle );
271+
return( 0 );
272+
}
273+
123274
static int exercise_mac_key( psa_key_handle_t handle,
124275
psa_key_usage_t usage,
125276
psa_algorithm_t alg )
@@ -287,26 +438,13 @@ static int exercise_signature_key( psa_key_handle_t handle,
287438
/* If the policy allows signing with any hash, just pick one. */
288439
if( PSA_ALG_IS_HASH_AND_SIGN( alg ) && hash_alg == PSA_ALG_ANY_HASH )
289440
{
290-
#if defined(MBEDTLS_MD2_C)
291-
hash_alg = PSA_ALG_MD2;
292-
#elif defined(MBEDTLS_MD4_C)
293-
hash_alg = PSA_ALG_MD4;
294-
#elif defined(MBEDTLS_MD5_C)
295-
hash_alg = PSA_ALG_MD5;
296-
/* MBEDTLS_RIPEMD160_C omitted because Mbed TLS doesn't
297-
* support it in RSA PKCS#1v1.5 signatures. */
298-
#elif defined(MBEDTLS_SHA1_C)
299-
hash_alg = PSA_ALG_SHA_1;
300-
#elif defined(MBEDTLS_SHA256_C)
301-
hash_alg = PSA_ALG_SHA_256;
302-
#elif defined(MBEDTLS_SHA512_C)
303-
hash_alg = PSA_ALG_SHA_384;
304-
#elif defined(MBEDTLS_SHA3_C)
305-
hash_alg = PSA_ALG_SHA3_256;
441+
#if defined(KNOWN_SUPPORTED_HASH_ALG)
442+
hash_alg = KNOWN_SUPPORTED_HASH_ALG;
443+
alg ^= PSA_ALG_ANY_HASH ^ hash_alg;
306444
#else
307445
test_fail( "No hash algorithm for hash-and-sign testing", __LINE__, __FILE__ );
446+
return( 1 );
308447
#endif
309-
alg ^= PSA_ALG_ANY_HASH ^ hash_alg;
310448
}
311449

312450
if( usage & PSA_KEY_USAGE_SIGN )
@@ -1988,9 +2126,22 @@ void hash_setup( int alg_arg,
19882126
PSA_ASSERT( psa_crypto_init( ) );
19892127

19902128
status = psa_hash_setup( &operation, alg );
1991-
psa_hash_abort( &operation );
19922129
TEST_EQUAL( status, expected_status );
19932130

2131+
/* Whether setup succeeded or failed, abort must succeed. */
2132+
PSA_ASSERT( psa_hash_abort( &operation ) );
2133+
2134+
/* If setup failed, reproduce the failure, so as to
2135+
* test the resulting state of the operation object. */
2136+
if( status != PSA_SUCCESS )
2137+
TEST_EQUAL( psa_hash_setup( &operation, alg ), status );
2138+
2139+
/* Now the operation object should be reusable. */
2140+
#if defined(KNOWN_SUPPORTED_HASH_ALG)
2141+
PSA_ASSERT( psa_hash_setup( &operation, KNOWN_SUPPORTED_HASH_ALG ) );
2142+
PSA_ASSERT( psa_hash_abort( &operation ) );
2143+
#endif
2144+
19942145
exit:
19952146
mbedtls_psa_crypto_free( );
19962147
}
@@ -2266,31 +2417,34 @@ void mac_setup( int key_type_arg,
22662417
int alg_arg,
22672418
int expected_status_arg )
22682419
{
2269-
psa_key_handle_t handle = 0;
22702420
psa_key_type_t key_type = key_type_arg;
22712421
psa_algorithm_t alg = alg_arg;
22722422
psa_status_t expected_status = expected_status_arg;
22732423
psa_mac_operation_t operation = PSA_MAC_OPERATION_INIT;
2274-
psa_key_policy_t policy = PSA_KEY_POLICY_INIT;
2275-
psa_status_t status;
2424+
psa_status_t status = PSA_ERROR_GENERIC_ERROR;
2425+
#if defined(KNOWN_SUPPORTED_MAC_ALG)
2426+
const uint8_t smoke_test_key_data[16] = "kkkkkkkkkkkkkkkk";
2427+
#endif
22762428

22772429
PSA_ASSERT( psa_crypto_init( ) );
22782430

2279-
PSA_ASSERT( psa_allocate_key( &handle ) );
2280-
psa_key_policy_set_usage( &policy,
2281-
PSA_KEY_USAGE_SIGN | PSA_KEY_USAGE_VERIFY,
2282-
alg );
2283-
PSA_ASSERT( psa_set_key_policy( handle, &policy ) );
2284-
2285-
PSA_ASSERT( psa_import_key( handle, key_type,
2286-
key->x, key->len ) );
2287-
2288-
status = psa_mac_sign_setup( &operation, handle, alg );
2289-
psa_mac_abort( &operation );
2431+
if( ! exercise_mac_setup( key_type, key->x, key->len, alg,
2432+
&operation, &status ) )
2433+
goto exit;
22902434
TEST_EQUAL( status, expected_status );
22912435

2436+
/* The operation object should be reusable. */
2437+
#if defined(KNOWN_SUPPORTED_MAC_ALG)
2438+
if( ! exercise_mac_setup( KNOWN_SUPPORTED_MAC_KEY_TYPE,
2439+
smoke_test_key_data,
2440+
sizeof( smoke_test_key_data ),
2441+
KNOWN_SUPPORTED_MAC_ALG,
2442+
&operation, &status ) )
2443+
goto exit;
2444+
TEST_EQUAL( status, PSA_SUCCESS );
2445+
#endif
2446+
22922447
exit:
2293-
psa_destroy_key( handle );
22942448
mbedtls_psa_crypto_free( );
22952449
}
22962450
/* END_CASE */
@@ -2560,29 +2714,34 @@ void cipher_setup( int key_type_arg,
25602714
int alg_arg,
25612715
int expected_status_arg )
25622716
{
2563-
psa_key_handle_t handle = 0;
25642717
psa_key_type_t key_type = key_type_arg;
25652718
psa_algorithm_t alg = alg_arg;
25662719
psa_status_t expected_status = expected_status_arg;
25672720
psa_cipher_operation_t operation = PSA_CIPHER_OPERATION_INIT;
2568-
psa_key_policy_t policy = PSA_KEY_POLICY_INIT;
25692721
psa_status_t status;
2722+
#if defined(KNOWN_SUPPORTED_MAC_ALG)
2723+
const uint8_t smoke_test_key_data[16] = "kkkkkkkkkkkkkkkk";
2724+
#endif
25702725

25712726
PSA_ASSERT( psa_crypto_init( ) );
25722727

2573-
PSA_ASSERT( psa_allocate_key( &handle ) );
2574-
psa_key_policy_set_usage( &policy, PSA_KEY_USAGE_ENCRYPT, alg );
2575-
PSA_ASSERT( psa_set_key_policy( handle, &policy ) );
2576-
2577-
PSA_ASSERT( psa_import_key( handle, key_type,
2578-
key->x, key->len ) );
2579-
2580-
status = psa_cipher_encrypt_setup( &operation, handle, alg );
2581-
psa_cipher_abort( &operation );
2728+
if( ! exercise_cipher_setup( key_type, key->x, key->len, alg,
2729+
&operation, &status ) )
2730+
goto exit;
25822731
TEST_EQUAL( status, expected_status );
25832732

2733+
/* The operation object should be reusable. */
2734+
#if defined(KNOWN_SUPPORTED_CIPHER_ALG)
2735+
if( ! exercise_cipher_setup( KNOWN_SUPPORTED_CIPHER_KEY_TYPE,
2736+
smoke_test_key_data,
2737+
sizeof( smoke_test_key_data ),
2738+
KNOWN_SUPPORTED_CIPHER_ALG,
2739+
&operation, &status ) )
2740+
goto exit;
2741+
TEST_EQUAL( status, PSA_SUCCESS );
2742+
#endif
2743+
25842744
exit:
2585-
psa_destroy_key( handle );
25862745
mbedtls_psa_crypto_free( );
25872746
}
25882747
/* END_CASE */

0 commit comments

Comments
 (0)