Merge pull request #276 from gilles-peskine-arm/psa-key_derivation-relax_inputs

Relax input restrictions for key derivation

Author: gilles-peskine-arm

include/psa/crypto.h

@@ -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().
@@ -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().
@@ -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
@@ -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
@@ -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.
@@ -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).

include/psa/crypto_struct.h

@@ -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;
     size_t capacity;
     union
     {
@@ -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;

include/psa/crypto_values.h

@@ -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)
 

library/psa_crypto.c

@@ -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)
@@ -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 ) )
     {
@@ -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 );
@@ -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 ) );
 }
 
@@ -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 ) );
 }
@@ -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 );