From adbec81cc4ec05a4adaeceac9a16b7f3f8b90138 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Fri, 14 Jun 2019 11:05:39 +0100 Subject: [PATCH] Remove the deprecated PSA_ALG_SELECT_RAW option This change affects the psa_key_derivation_s structure. With the buffer removed from the union, it is empty if MBEDTLS_MD_C is not defined. We can avoid undefined behaviour by adding a new dummy field that is always present or make the whole union conditional on MBEDTLS_MD_C. In this latter case the initialiser macro has to depend on MBEDTLS_MD_C as well. Furthermore the first structure would be either psa_hkdf_key_derivation_t or psa_tls12_prf_key_derivation_t both of which are very deep and would make the initialisation macro difficult to maintain, therefore we go with the first option. --- include/psa/crypto_extra.h | 3 - include/psa/crypto_struct.h | 10 ++-- library/psa_crypto.c | 73 +------------------------ tests/suites/test_suite_psa_crypto.data | 4 -- 4 files changed, 7 insertions(+), 83 deletions(-) diff --git a/include/psa/crypto_extra.h b/include/psa/crypto_extra.h index 3fc73b9d31..0ab5892265 100644 --- a/include/psa/crypto_extra.h +++ b/include/psa/crypto_extra.h @@ -283,9 +283,6 @@ psa_status_t psa_key_derivation(psa_key_derivation_operation_t *operation, size_t capacity); #endif /* PSA_PRE_1_0_KEY_DERIVATION */ -/* FIXME Deprecated. Remove this as soon as all the tests are updated. */ -#define PSA_ALG_SELECT_RAW ((psa_algorithm_t)0x31000001) - /** \addtogroup crypto_types * @{ */ diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h index e6197cb9b9..d9e9b86da3 100644 --- a/include/psa/crypto_struct.h +++ b/include/psa/crypto_struct.h @@ -277,11 +277,8 @@ struct psa_key_derivation_s size_t capacity; union { - struct - { - uint8_t *data; - size_t size; - } buffer; + /* Make the union non-empty even with no supported algorithms. */ + uint8_t dummy; #if defined(MBEDTLS_MD_C) psa_hkdf_key_derivation_t hkdf; psa_tls12_prf_key_derivation_t tls12_prf; @@ -289,7 +286,8 @@ struct psa_key_derivation_s } ctx; }; -#define PSA_KEY_DERIVATION_OPERATION_INIT {0, 0, {{0, 0}}} +/* This only zeroes out the first byte in the union, the rest is unspecified. */ +#define PSA_KEY_DERIVATION_OPERATION_INIT {0, 0, {0}} static inline struct psa_key_derivation_s psa_key_derivation_operation_init( void ) { const struct psa_key_derivation_s v = PSA_KEY_DERIVATION_OPERATION_INIT; diff --git a/library/psa_crypto.c b/library/psa_crypto.c index bd9fca585c..31520b8b17 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3865,16 +3865,6 @@ psa_status_t psa_key_derivation_abort( psa_key_derivation_operation_t *operation * nothing to do. */ } else - if( kdf_alg == PSA_ALG_SELECT_RAW ) - { - if( operation->ctx.buffer.data != NULL ) - { - mbedtls_platform_zeroize( operation->ctx.buffer.data, - operation->ctx.buffer.size ); - mbedtls_free( operation->ctx.buffer.data ); - } - } - else #if defined(MBEDTLS_MD_C) if( PSA_ALG_IS_HKDF( kdf_alg ) ) { @@ -4213,23 +4203,6 @@ psa_status_t psa_key_derivation_output_bytes( psa_key_derivation_operation_t *op } operation->capacity -= output_length; - if( kdf_alg == PSA_ALG_SELECT_RAW ) - { - /* Initially, the capacity of a selection operation is always - * the size of the buffer, i.e. `operation->ctx.buffer.size`, - * abbreviated in this comment as `size`. When the remaining - * capacity is `c`, the next bytes to serve start `c` bytes - * from the end of the buffer, i.e. `size - c` from the - * beginning of the buffer. Since `operation->capacity` was just - * decremented above, we need to serve the bytes from - * `size - operation->capacity - output_length` to - * `size - operation->capacity`. */ - size_t offset = - operation->ctx.buffer.size - operation->capacity - output_length; - memcpy( output, operation->ctx.buffer.data + offset, output_length ); - status = PSA_SUCCESS; - } - else #if defined(MBEDTLS_MD_C) if( PSA_ALG_IS_HKDF( kdf_alg ) ) { @@ -4237,16 +4210,17 @@ psa_status_t psa_key_derivation_output_bytes( psa_key_derivation_operation_t *op status = psa_key_derivation_hkdf_read( &operation->ctx.hkdf, hash_alg, output, output_length ); } + else #if defined(PSA_PRE_1_0_KEY_DERIVATION) - else if( PSA_ALG_IS_TLS12_PRF( kdf_alg ) || + if( PSA_ALG_IS_TLS12_PRF( kdf_alg ) || PSA_ALG_IS_TLS12_PSK_TO_MS( kdf_alg ) ) { status = psa_key_derivation_tls12_prf_read( &operation->ctx.tls12_prf, kdf_alg, output, output_length ); } -#endif /* PSA_PRE_1_0_KEY_DERIVATION */ else +#endif /* PSA_PRE_1_0_KEY_DERIVATION */ #endif /* MBEDTLS_MD_C */ { return( PSA_ERROR_BAD_STATE ); @@ -4509,23 +4483,6 @@ static psa_status_t psa_key_derivation_internal( /* Set operation->alg even on failure so that abort knows what to do. */ operation->alg = alg; - if( alg == PSA_ALG_SELECT_RAW ) - { - (void) salt; - if( salt_length != 0 ) - return( PSA_ERROR_INVALID_ARGUMENT ); - (void) label; - if( label_length != 0 ) - return( PSA_ERROR_INVALID_ARGUMENT ); - operation->ctx.buffer.data = mbedtls_calloc( 1, secret_length ); - if( operation->ctx.buffer.data == NULL ) - return( PSA_ERROR_INSUFFICIENT_MEMORY ); - memcpy( operation->ctx.buffer.data, secret, secret_length ); - operation->ctx.buffer.size = secret_length; - max_capacity = secret_length; - status = PSA_SUCCESS; - } - else #if defined(MBEDTLS_MD_C) if( PSA_ALG_IS_HKDF( alg ) ) { @@ -4848,25 +4805,6 @@ static psa_status_t psa_tls12_prf_input( psa_tls12_prf_key_derivation_t *prf, #endif /* PSA_PRE_1_0_KEY_DERIVATION */ #endif /* MBEDTLS_MD_C */ -static psa_status_t psa_key_derivation_input_raw( - psa_key_derivation_operation_t *operation, - const uint8_t *data, - size_t data_length ) -{ - if( operation->capacity != 0 ) - return( PSA_ERROR_INVALID_ARGUMENT ); - - operation->ctx.buffer.data = mbedtls_calloc( 1, data_length ); - if( operation->ctx.buffer.data == NULL ) - return( PSA_ERROR_INSUFFICIENT_MEMORY ); - - memcpy( operation->ctx.buffer.data, data, data_length ); - operation->ctx.buffer.size = data_length; - operation->capacity = data_length; - - return PSA_SUCCESS; -} - static psa_status_t psa_key_derivation_input_internal( psa_key_derivation_operation_t *operation, psa_key_derivation_step_t step, @@ -4876,11 +4814,6 @@ static psa_status_t psa_key_derivation_input_internal( psa_status_t status; psa_algorithm_t kdf_alg = psa_key_derivation_get_kdf_alg( operation ); - if( kdf_alg == PSA_ALG_SELECT_RAW ) - { - status = psa_key_derivation_input_raw( operation, data, data_length ); - } - else #if defined(MBEDTLS_MD_C) if( PSA_ALG_IS_HKDF( kdf_alg ) ) { diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 361308b638..d9f02715af 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -1767,10 +1767,6 @@ PSA key derivation: TLS 1.2 PRF SHA-256, good case depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C derive_setup:PSA_ALG_TLS12_PRF(PSA_ALG_SHA_256):PSA_SUCCESS -PSA key derivation: not a key derivation algorithm (selection) -depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C -derive_setup:PSA_ALG_SELECT_RAW:PSA_ERROR_INVALID_ARGUMENT - PSA key derivation: not a key derivation algorithm (HMAC) depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C derive_setup:PSA_ALG_HMAC(PSA_ALG_SHA_256):PSA_ERROR_INVALID_ARGUMENT