Skip to content

Relax input restrictions for key derivation #276

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions include/psa/crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -3229,9 +3229,12 @@ psa_status_t psa_key_derivation_set_capacity(
* Refer to the documentation of each key derivation or key agreement
* algorithm for information.
*
* This function passes direct inputs. Some inputs must be passed as keys
* using psa_key_derivation_input_key() instead of this function. Refer to
* the documentation of individual step types for information.
* This function passes direct inputs, which is usually correct for
* non-secret inputs. To pass a secret input, which should be in a key
* object, call psa_key_derivation_input_key() instead of this function.
* Refer to the documentation of individual step types
* (`PSA_KEY_DERIVATION_INPUT_xxx` values of type ::psa_key_derivation_step_t)
* for more information.
*
* If this function returns an error status, the operation enters an error
* state and must be aborted by calling psa_key_derivation_abort().
Expand Down Expand Up @@ -3274,10 +3277,13 @@ psa_status_t psa_key_derivation_input_bytes(
* Refer to the documentation of each key derivation or key agreement
* algorithm for information.
*
* This function passes key inputs. Some inputs must be passed as keys
* of the appropriate type using this function, while others must be
* passed as direct inputs using psa_key_derivation_input_bytes(). Refer to
* the documentation of individual step types for information.
* This function obtains input from a key object, which is usually correct for
* secret inputs or for non-secret personalization strings kept in the key
* store. To pass a non-secret parameter which is not in the key store,
* call psa_key_derivation_input_bytes() instead of this function.
* Refer to the documentation of individual step types
* (`PSA_KEY_DERIVATION_INPUT_xxx` values of type ::psa_key_derivation_step_t)
* for more information.
*
* If this function returns an error status, the operation enters an error
* state and must be aborted by calling psa_key_derivation_abort().
Expand All @@ -3298,7 +3304,8 @@ psa_status_t psa_key_derivation_input_bytes(
* \retval #PSA_ERROR_INVALID_ARGUMENT
* \c step is not compatible with the operation's algorithm.
* \retval #PSA_ERROR_INVALID_ARGUMENT
* \c step does not allow key inputs.
* \c step does not allow key inputs of the given type
* or does not allow key inputs at all.
* \retval #PSA_ERROR_INSUFFICIENT_MEMORY
* \retval #PSA_ERROR_COMMUNICATION_FAILURE
* \retval #PSA_ERROR_HARDWARE_FAILURE
Expand Down Expand Up @@ -3368,6 +3375,8 @@ psa_status_t psa_key_derivation_input_key(
* \c private_key.
* \retval #PSA_ERROR_NOT_SUPPORTED
* \c alg is not supported or is not a key derivation algorithm.
* \retval #PSA_ERROR_INVALID_ARGUMENT
* \c step does not allow an input resulting from a key agreement.
* \retval #PSA_ERROR_INSUFFICIENT_MEMORY
* \retval #PSA_ERROR_COMMUNICATION_FAILURE
* \retval #PSA_ERROR_HARDWARE_FAILURE
Expand Down Expand Up @@ -3518,6 +3527,11 @@ psa_status_t psa_key_derivation_output_bytes(
* In all cases, the data that is read is discarded from the operation.
* The operation's capacity is decreased by the number of bytes read.
*
* For algorithms that take an input step #PSA_KEY_DERIVATION_INPUT_SECRET,
* the input to that step must be provided with psa_key_derivation_input_key().
* Future versions of this specification may include additional restrictions
* on the derived key based on the attributes and strength of the secret key.
*
* \param[in] attributes The attributes for the new key.
* \param[in,out] operation The key derivation operation object to read from.
* \param[out] handle On success, a handle to the newly created key.
Expand All @@ -3540,6 +3554,9 @@ psa_status_t psa_key_derivation_output_bytes(
* implementation in general or in this particular location.
* \retval #PSA_ERROR_INVALID_ARGUMENT
* The provided key attributes are not valid for the operation.
* \retval #PSA_ERROR_NOT_PERMITTED
* The #PSA_KEY_DERIVATION_INPUT_SECRET input was not provided through
* a key.
* \retval #PSA_ERROR_BAD_STATE
* The operation state is not valid (it must be active and completed
* all required input steps).
Expand Down
3 changes: 2 additions & 1 deletion include/psa/crypto_struct.h
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ typedef struct psa_tls12_prf_key_derivation_s
struct psa_key_derivation_s
{
psa_algorithm_t alg;
unsigned int can_output_key : 1;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This leaves 31 bits unused. #281

size_t capacity;
union
{
Expand All @@ -268,7 +269,7 @@ struct psa_key_derivation_s
};

/* This only zeroes out the first byte in the union, the rest is unspecified. */
#define PSA_KEY_DERIVATION_OPERATION_INIT {0, 0, {0}}
#define PSA_KEY_DERIVATION_OPERATION_INIT {0, 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;
Expand Down
22 changes: 17 additions & 5 deletions include/psa/crypto_values.h
Original file line number Diff line number Diff line change
Expand Up @@ -1618,31 +1618,43 @@

/** A secret input for key derivation.
*
* This must be a key of type #PSA_KEY_TYPE_DERIVE.
* This should be a key of type #PSA_KEY_TYPE_DERIVE
* (passed to psa_key_derivation_input_key())
* or the shared secret resulting from a key agreement
* (obtained via psa_key_derivation_key_agreement()).
*
* The secret can also be a direct input (passed to
* key_derivation_input_bytes()). In this case, the derivation operation
* may not be used to derive keys: the operation will only allow
* psa_key_derivation_output_bytes(), not psa_key_derivation_output_key().
*/
#define PSA_KEY_DERIVATION_INPUT_SECRET ((psa_key_derivation_step_t)0x0101)

/** A label for key derivation.
*
* This must be a direct input.
* This should be a direct input.
* It can also be a key of type #PSA_KEY_TYPE_RAW_DATA.
*/
#define PSA_KEY_DERIVATION_INPUT_LABEL ((psa_key_derivation_step_t)0x0201)

/** A salt for key derivation.
*
* This must be a direct input.
* This should be a direct input.
* It can also be a key of type #PSA_KEY_TYPE_RAW_DATA.
*/
#define PSA_KEY_DERIVATION_INPUT_SALT ((psa_key_derivation_step_t)0x0202)

/** An information string for key derivation.
*
* This must be a direct input.
* This should be a direct input.
* It can also be a key of type #PSA_KEY_TYPE_RAW_DATA.
*/
#define PSA_KEY_DERIVATION_INPUT_INFO ((psa_key_derivation_step_t)0x0203)

/** A seed for key derivation.
*
* This must be a direct input.
* This should be a direct input.
* It can also be a key of type #PSA_KEY_TYPE_RAW_DATA.
*/
#define PSA_KEY_DERIVATION_INPUT_SEED ((psa_key_derivation_step_t)0x0204)

Expand Down
73 changes: 58 additions & 15 deletions library/psa_crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -4787,6 +4787,9 @@ psa_status_t psa_key_derivation_output_key( const psa_key_attributes_t *attribut
if( psa_get_key_bits( attributes ) == 0 )
return( PSA_ERROR_INVALID_ARGUMENT );

if( ! operation->can_output_key )
return( PSA_ERROR_NOT_PERMITTED );

status = psa_start_key_creation( PSA_KEY_CREATION_DERIVE,
attributes, handle, &slot, &driver );
#if defined(MBEDTLS_PSA_CRYPTO_SE_C)
Expand Down Expand Up @@ -5076,15 +5079,54 @@ static psa_status_t psa_tls12_prf_psk_to_ms_input(
}
#endif /* MBEDTLS_MD_C */

/** Check whether the given key type is acceptable for the given
* input step of a key derivation.
*
* Secret inputs must have the type #PSA_KEY_TYPE_DERIVE.
* Non-secret inputs must have the type #PSA_KEY_TYPE_RAW_DATA.
* Both secret and non-secret inputs can alternatively have the type
* #PSA_KEY_TYPE_NONE, which is never the type of a key object, meaning
* that the input was passed as a buffer rather than via a key object.
*/
static int psa_key_derivation_check_input_type(
psa_key_derivation_step_t step,
psa_key_type_t key_type )
{
switch( step )
{
case PSA_KEY_DERIVATION_INPUT_SECRET:
if( key_type == PSA_KEY_TYPE_DERIVE )
return( PSA_SUCCESS );
if( key_type == PSA_KEY_TYPE_NONE )
return( PSA_SUCCESS );
break;
case PSA_KEY_DERIVATION_INPUT_LABEL:
case PSA_KEY_DERIVATION_INPUT_SALT:
case PSA_KEY_DERIVATION_INPUT_INFO:
case PSA_KEY_DERIVATION_INPUT_SEED:
if( key_type == PSA_KEY_TYPE_RAW_DATA )
return( PSA_SUCCESS );
if( key_type == PSA_KEY_TYPE_NONE )
return( PSA_SUCCESS );
break;
}
return( PSA_ERROR_INVALID_ARGUMENT );
}

static psa_status_t psa_key_derivation_input_internal(
psa_key_derivation_operation_t *operation,
psa_key_derivation_step_t step,
psa_key_type_t key_type,
const uint8_t *data,
size_t data_length )
{
psa_status_t status;
psa_algorithm_t kdf_alg = psa_key_derivation_get_kdf_alg( operation );

status = psa_key_derivation_check_input_type( step, key_type );
if( status != PSA_SUCCESS )
goto exit;

#if defined(MBEDTLS_MD_C)
if( PSA_ALG_IS_HKDF( kdf_alg ) )
{
Expand All @@ -5111,6 +5153,7 @@ static psa_status_t psa_key_derivation_input_internal(
return( PSA_ERROR_BAD_STATE );
}

exit:
if( status != PSA_SUCCESS )
psa_key_derivation_abort( operation );
return( status );
Expand All @@ -5122,10 +5165,8 @@ psa_status_t psa_key_derivation_input_bytes(
const uint8_t *data,
size_t data_length )
{
if( step == PSA_KEY_DERIVATION_INPUT_SECRET )
return( PSA_ERROR_INVALID_ARGUMENT );

return( psa_key_derivation_input_internal( operation, step,
PSA_KEY_TYPE_NONE,
data, data_length ) );
}

Expand All @@ -5136,23 +5177,23 @@ psa_status_t psa_key_derivation_input_key(
{
psa_key_slot_t *slot;
psa_status_t status;

status = psa_get_transparent_key( handle, &slot,
PSA_KEY_USAGE_DERIVE,
operation->alg );
if( status != PSA_SUCCESS )
{
psa_key_derivation_abort( operation );
return( status );
if( slot->attr.type != PSA_KEY_TYPE_DERIVE )
return( PSA_ERROR_INVALID_ARGUMENT );
/* Don't allow a key to be used as an input that is usually public.
* This is debatable. It's ok from a cryptographic perspective to
* use secret material as an input that is usually public. However
* the material should be dedicated to a particular input step,
* otherwise this may allow the key to be used in an unintended way
* and leak values derived from the key. So be conservative. */
if( step != PSA_KEY_DERIVATION_INPUT_SECRET )
return( PSA_ERROR_INVALID_ARGUMENT );
}

/* Passing a key object as a SECRET input unlocks the permission
* to output to a key object. */
if( step == PSA_KEY_DERIVATION_INPUT_SECRET )
operation->can_output_key = 1;

return( psa_key_derivation_input_internal( operation,
step,
step, slot->attr.type,
slot->data.raw.data,
slot->data.raw.bytes ) );
}
Expand Down Expand Up @@ -5265,8 +5306,10 @@ static psa_status_t psa_key_agreement_internal( psa_key_derivation_operation_t *
goto exit;

/* Step 2: set up the key derivation to generate key material from
* the shared secret. */
* the shared secret. A shared secret is permitted wherever a key
* of type DERIVE is permitted. */
status = psa_key_derivation_input_internal( operation, step,
PSA_KEY_TYPE_DERIVE,
shared_secret,
shared_secret_length );

Expand Down
Loading