generate/derive key ext: pass method_data_length rather than method_length
Instead of passing the size of the whole structure, just pass the data
length and let the implementation worry about adding the size of the
structure. The intent with passing the structure size was to allow
the client code in a client-server implementation to know nothing
about the structure and just copy the bytes to the server. But that was not
really a useful consideration since the application has to know the
structure layout, so it has to be available in the client implementation's
headers. Passing the method data length makes life simpler for everyone by
not having to worry about possible padding at the end of the structure, and
removes a potential error condition
(method_length < sizeof(psa_key_generation_method_t)).
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
diff --git a/include/psa/crypto.h b/include/psa/crypto.h
index d6ad969..da1418f 100644
--- a/include/psa/crypto.h
+++ b/include/psa/crypto.h
@@ -3704,8 +3704,7 @@
* \note This function is equivalent to calling
* psa_key_derivation_output_key_ext()
* with the method #PSA_KEY_GENERATION_METHOD_INIT and
- * `method_length == sizeof(psa_key_generation_method_t)`
- * (i.e. `method->flags == 0` and `method->data` is empty).
+ * `method_data_length == 0` (i.e. `method->data` is empty).
*
* \param[in] attributes The attributes for the new key.
* If the key type to be created is
@@ -3777,17 +3776,14 @@
* \param[in,out] operation The key derivation operation object to read from.
* \param[in] method Customization parameters for the key generation.
* When this is #PSA_KEY_GENERATION_METHOD_INIT
- * with \p method_length =
- * `sizeof(psa_key_generation_method_t)`,
+ * with \p method_data_length = 0,
* this function is equivalent to
* psa_key_derivation_output_key().
* Mbed TLS currently only supports the default
* method, i.e. #PSA_KEY_GENERATION_METHOD_INIT,
* for all key types.
- * \param method_length Length of \p method in bytes.
- * This must be
- * `sizeof(psa_key_generation_method_t) + n`
- * where `n` is the size of `method->data` in bytes.
+ * \param method_data_length
+ * Length of `method.data` in bytes.
* \param[out] key On success, an identifier for the newly created
* key. For persistent keys, this is the key
* identifier defined in \p attributes.
@@ -3834,7 +3830,7 @@
const psa_key_attributes_t *attributes,
psa_key_derivation_operation_t *operation,
const psa_key_generation_method_t *method,
- size_t method_length,
+ size_t method_data_length,
mbedtls_svc_key_id_t *key);
/** Compare output data from a key derivation operation to an expected value.
@@ -4093,8 +4089,7 @@
*
* \note This function is equivalent to calling psa_generate_key_ext()
* with the method #PSA_KEY_GENERATION_METHOD_INIT and
- * `method_length == sizeof(psa_key_generation_method_t)`
- * (i.e. `method->flags == 0` and `method->data` is empty).
+ * `method_data_length == 0` (i.e. `method->data` is empty).
*
* \param[in] attributes The attributes for the new key.
* \param[out] key On success, an identifier for the newly created
@@ -4144,14 +4139,11 @@
* \param[in] attributes The attributes for the new key.
* \param[in] method Customization parameters for the key generation.
* When this is #PSA_KEY_GENERATION_METHOD_INIT
- * with \p method_length =
- * `sizeof(psa_key_generation_method_t)`,
+ * with \p method_data_length = 0,
* this function is equivalent to
* psa_key_derivation_output_key().
- * \param method_length Length of \p method in bytes.
- * This must be
- * `sizeof(psa_key_generation_method_t) + n`
- * where `n` is the size of `method->data` in bytes.
+ * \param method_data_length
+ * Length of `method.data` in bytes.
* \param[out] key On success, an identifier for the newly created
* key. For persistent keys, this is the key
* identifier defined in \p attributes.
@@ -4182,7 +4174,7 @@
*/
psa_status_t psa_generate_key_ext(const psa_key_attributes_t *attributes,
const psa_key_generation_method_t *method,
- size_t method_length,
+ size_t method_data_length,
mbedtls_svc_key_id_t *key);
/**@}*/
diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h
index b5e942e..248caa2 100644
--- a/include/psa/crypto_struct.h
+++ b/include/psa/crypto_struct.h
@@ -233,7 +233,7 @@
*
* Calling psa_generate_key_ext() or psa_key_derivation_output_key_ext()
* with `method=PSA_KEY_GENERATION_METHOD_INIT` and
- * `method_length=sizeof(psa_key_generation_method_t)` is equivalent to
+ * `method_data_length == 0` is equivalent to
* calling psa_generate_key() or psa_key_derivation_output_key()
* respectively.
*/
diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index 98823de..3c328c4 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -6027,12 +6027,12 @@
static int psa_key_generation_method_is_default(
const psa_key_generation_method_t *method,
- size_t method_length)
+ size_t method_data_length)
{
- if (method_length != sizeof(*method)) {
+ if (method->flags != 0) {
return 0;
}
- if (method->flags != 0) {
+ if (method_data_length != 0) {
return 0;
}
return 1;
@@ -6042,7 +6042,7 @@
const psa_key_attributes_t *attributes,
psa_key_derivation_operation_t *operation,
const psa_key_generation_method_t *method,
- size_t method_length,
+ size_t method_data_length,
mbedtls_svc_key_id_t *key)
{
psa_status_t status;
@@ -6057,10 +6057,7 @@
return PSA_ERROR_INVALID_ARGUMENT;
}
- if (method_length < sizeof(*method)) {
- return PSA_ERROR_INVALID_ARGUMENT;
- }
- if (!psa_key_generation_method_is_default(method, method_length)) {
+ if (!psa_key_generation_method_is_default(method, method_data_length)) {
return PSA_ERROR_INVALID_ARGUMENT;
}
@@ -6100,10 +6097,9 @@
psa_key_derivation_operation_t *operation,
mbedtls_svc_key_id_t *key)
{
- return psa_key_derivation_output_key_ext(
- attributes, operation,
- &default_method, sizeof(default_method),
- key);
+ return psa_key_derivation_output_key_ext(attributes, operation,
+ &default_method, 0,
+ key);
}
@@ -7501,7 +7497,7 @@
psa_status_t psa_generate_key_internal(
const psa_key_attributes_t *attributes,
- const psa_key_generation_method_t *method, size_t method_length,
+ const psa_key_generation_method_t *method, size_t method_data_length,
uint8_t *key_buffer, size_t key_buffer_size, size_t *key_buffer_length)
{
psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
@@ -7509,7 +7505,7 @@
/* Only used for RSA */
(void) method;
- (void) method_length;
+ (void) method_data_length;
if ((attributes->domain_parameters == NULL) &&
(attributes->domain_parameters_size != 0)) {
@@ -7536,9 +7532,8 @@
* that mbedtls_psa_rsa_generate_key() gets e via a new
* parameter instead. */
psa_key_attributes_t override_attributes = *attributes;
- if (method_length > sizeof(*method)) {
- override_attributes.domain_parameters_size =
- method_length - offsetof(psa_key_generation_method_t, data);
+ if (method_data_length != 0) {
+ override_attributes.domain_parameters_size = method_data_length;
override_attributes.domain_parameters = (uint8_t *) &method->data;
}
return mbedtls_psa_rsa_generate_key(&override_attributes,
@@ -7575,7 +7570,7 @@
psa_status_t psa_generate_key_ext(const psa_key_attributes_t *attributes,
const psa_key_generation_method_t *method,
- size_t method_length,
+ size_t method_data_length,
mbedtls_svc_key_id_t *key)
{
psa_status_t status;
@@ -7596,10 +7591,6 @@
return PSA_ERROR_INVALID_ARGUMENT;
}
- if (method_length < sizeof(*method)) {
- return PSA_ERROR_INVALID_ARGUMENT;
- }
-
#if defined(PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_GENERATE)
if (attributes->core.type == PSA_KEY_TYPE_RSA_KEY_PAIR) {
if (method->flags != 0) {
@@ -7607,7 +7598,7 @@
}
} else
#endif
- if (!psa_key_generation_method_is_default(method, method_length)) {
+ if (!psa_key_generation_method_is_default(method, method_data_length)) {
return PSA_ERROR_INVALID_ARGUMENT;
}
@@ -7648,7 +7639,7 @@
}
status = psa_driver_wrapper_generate_key(attributes,
- method, method_length,
+ method, method_data_length,
slot->key.data, slot->key.bytes,
&slot->key.bytes);
if (status != PSA_SUCCESS) {
@@ -7670,7 +7661,7 @@
mbedtls_svc_key_id_t *key)
{
return psa_generate_key_ext(attributes,
- &default_method, sizeof(default_method),
+ &default_method, 0,
key);
}
diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h
index 99aea9d..3a9b02d 100644
--- a/library/psa_crypto_core.h
+++ b/library/psa_crypto_core.h
@@ -405,8 +405,7 @@
* \param[in] attributes The attributes for the key to generate.
* \param[in] method The generation method from
* psa_generate_key_ext().
- * This can be \c NULL if \p method_length is 0.
- * \param method_length The size of \p method in bytes.
+ * \param method_data_length The size of `method.data` in bytes.
* \param[out] key_buffer Buffer where the key data is to be written.
* \param[in] key_buffer_size Size of \p key_buffer in bytes.
* \param[out] key_buffer_length On success, the number of bytes written in
@@ -422,7 +421,7 @@
*/
psa_status_t psa_generate_key_internal(const psa_key_attributes_t *attributes,
const psa_key_generation_method_t *method,
- size_t method_length,
+ size_t method_data_length,
uint8_t *key_buffer,
size_t key_buffer_size,
size_t *key_buffer_length);
diff --git a/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.h.jinja b/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.h.jinja
index e6d827b..b1a952b 100644
--- a/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.h.jinja
+++ b/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.h.jinja
@@ -731,7 +731,7 @@
static inline psa_status_t psa_driver_wrapper_generate_key(
const psa_key_attributes_t *attributes,
- const psa_key_generation_method_t *method, size_t method_length,
+ const psa_key_generation_method_t *method, size_t method_data_length,
uint8_t *key_buffer, size_t key_buffer_size, size_t *key_buffer_length )
{
psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
@@ -797,7 +797,7 @@
/* Software fallback */
status = psa_generate_key_internal(
- attributes, method, method_length,
+ attributes, method, method_data_length,
key_buffer, key_buffer_size, key_buffer_length );
break;
diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data
index 2d0cf2c..1a0f08b 100644
--- a/tests/suites/test_suite_psa_crypto.data
+++ b/tests/suites/test_suite_psa_crypto.data
@@ -6874,14 +6874,6 @@
depends_on:PSA_WANT_ALG_HKDF:PSA_WANT_ALG_SHA_256:PSA_WANT_KEY_TYPE_AES
derive_key_ext:PSA_ALG_HKDF(PSA_ALG_SHA_256):"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":"000102030405060708090a0b0c":"f0f1f2f3f4f5f6f7f8f9":PSA_KEY_TYPE_AES:128:0:"":PSA_SUCCESS:"3cb25f25faacd57a90434f64d0362f2a"
-PSA key derivation: null method -> AES-128
-depends_on:PSA_WANT_ALG_HKDF:PSA_WANT_ALG_SHA_256:PSA_WANT_KEY_TYPE_AES
-derive_key_ext:PSA_ALG_HKDF(PSA_ALG_SHA_256):"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":"000102030405060708090a0b0c":"f0f1f2f3f4f5f6f7f8f9":PSA_KEY_TYPE_AES:128:-offsetof(psa_key_generation_method_t, data):"":PSA_ERROR_INVALID_ARGUMENT:""
-
-PSA key derivation: method too short by 1 -> AES-128
-depends_on:PSA_WANT_ALG_HKDF:PSA_WANT_ALG_SHA_256:PSA_WANT_KEY_TYPE_AES
-derive_key_ext:PSA_ALG_HKDF(PSA_ALG_SHA_256):"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":"000102030405060708090a0b0c":"f0f1f2f3f4f5f6f7f8f9":PSA_KEY_TYPE_AES:128:-1:"":PSA_ERROR_INVALID_ARGUMENT:""
-
PSA key derivation: method.flags=1 -> AES-128
depends_on:PSA_WANT_ALG_HKDF:PSA_WANT_ALG_SHA_256:PSA_WANT_KEY_TYPE_AES
derive_key_ext:PSA_ALG_HKDF(PSA_ALG_SHA_256):"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":"000102030405060708090a0b0c":"f0f1f2f3f4f5f6f7f8f9":PSA_KEY_TYPE_AES:128:1:"":PSA_ERROR_INVALID_ARGUMENT:""
@@ -7481,14 +7473,6 @@
depends_on:PSA_WANT_ALG_FFDH:PSA_WANT_KEY_TYPE_DH_KEY_PAIR_GENERATE
generate_key:PSA_KEY_TYPE_DH_KEY_PAIR(PSA_DH_FAMILY_RFC7919):1024:PSA_KEY_USAGE_EXPORT:PSA_ALG_FFDH:PSA_ERROR_NOT_SUPPORTED:0
-PSA generate key ext: RSA, null method
-depends_on:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_GENERATE
-generate_key_ext:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_VENDOR_RSA_GENERATE_MIN_KEY_BITS:PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT:0:-offsetof(psa_key_generation_method_t, data):"":PSA_ERROR_INVALID_ARGUMENT
-
-PSA generate key ext: RSA, method too short by 1
-depends_on:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_GENERATE
-generate_key_ext:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_VENDOR_RSA_GENERATE_MIN_KEY_BITS:PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT:0:-1:"":PSA_ERROR_INVALID_ARGUMENT
-
PSA generate key ext: RSA, method.flags=1
depends_on:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_GENERATE
generate_key_ext:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_VENDOR_RSA_GENERATE_MIN_KEY_BITS:PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT:0:1:"":PSA_ERROR_INVALID_ARGUMENT
@@ -7545,10 +7529,6 @@
depends_on:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_GENERATE
generate_key_ext:PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_VENDOR_RSA_GENERATE_MIN_KEY_BITS:PSA_KEY_USAGE_ENCRYPT | PSA_KEY_USAGE_DECRYPT:0:0:"02":PSA_ERROR_INVALID_ARGUMENT
-PSA generate key ext: ECC, null method
-depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_GENERATE:PSA_WANT_ECC_SECP_R1_256:PSA_WANT_ALG_ECDH
-generate_key_ext:PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):256:PSA_KEY_USAGE_DERIVE:PSA_ALG_ECDH:-offsetof(psa_key_generation_method_t, data):"":PSA_ERROR_INVALID_ARGUMENT
-
PSA generate key ext: ECC, flags=0
depends_on:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_GENERATE:PSA_WANT_ECC_SECP_R1_256:PSA_WANT_ALG_ECDH
generate_key_ext:PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):256:PSA_KEY_USAGE_DERIVE:PSA_ALG_ECDH:0:"":PSA_SUCCESS
diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function
index 34baa1b..e946f4e 100644
--- a/tests/suites/test_suite_psa_crypto.function
+++ b/tests/suites/test_suite_psa_crypto.function
@@ -1310,21 +1310,25 @@
#endif /* PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_GENERATE */
static int setup_key_generation_method(psa_key_generation_method_t **method,
- size_t *method_length,
- int64_t flags_arg,
+ size_t *method_data_length,
+ int flags_arg,
const data_t *method_data)
{
- if (flags_arg >= 0) {
- *method_length = sizeof(**method) + method_data->len;
- *method = mbedtls_calloc(1, *method_length);
- TEST_ASSERT(*method != NULL);
- (*method)->flags = (uint32_t) flags_arg;
- memcpy((*method)->data, method_data->x, method_data->len);
- } else if (sizeof(**method) + flags_arg > 0) {
- *method_length = sizeof(**method) + flags_arg;
- *method = mbedtls_calloc(1, *method_length);
- TEST_ASSERT(*method != NULL);
- }
+ *method_data_length = method_data->len;
+ /* If there are N bytes of padding at the end of
+ * psa_key_generation_method_t, then it's enough to allocate
+ * MIN(sizeof(psa_key_generation_method_t),
+ * offsetof(psa_key_generation_method_t, data) + method_data_length).
+ *
+ * For simplicity, here, we allocate up to N more bytes than necessary.
+ * In practice, the current layout of psa_key_generation_method_t
+ * makes padding extremely unlikely, so we don't worry about testing
+ * that the library code doesn't try to access these extra N bytes.
+ */
+ *method = mbedtls_calloc(1, sizeof(**method) + *method_data_length);
+ TEST_ASSERT(*method != NULL);
+ (*method)->flags = (uint32_t) flags_arg;
+ memcpy((*method)->data, method_data->x, method_data->len);
return 1;
exit:
return 0;
@@ -9335,7 +9339,7 @@
data_t *input1,
data_t *input2,
int key_type_arg, int bits_arg,
- int64_t flags_arg, /*negative for truncated method*/
+ int flags_arg,
data_t *method_data,
psa_status_t expected_status,
data_t *expected_export)
@@ -9346,7 +9350,7 @@
const psa_key_type_t key_type = key_type_arg;
const size_t bits = bits_arg;
psa_key_generation_method_t *method = NULL;
- size_t method_length = 0;
+ size_t method_data_length = 0;
psa_key_derivation_operation_t operation = PSA_KEY_DERIVATION_OPERATION_INIT;
const size_t export_buffer_size =
PSA_EXPORT_KEY_OUTPUT_SIZE(key_type, bits);
@@ -9376,13 +9380,13 @@
psa_set_key_algorithm(&derived_attributes, 0);
psa_set_key_type(&derived_attributes, key_type);
psa_set_key_bits(&derived_attributes, bits);
- if (!setup_key_generation_method(&method, &method_length,
+ if (!setup_key_generation_method(&method, &method_data_length,
flags_arg, method_data)) {
goto exit;
}
TEST_EQUAL(psa_key_derivation_output_key_ext(&derived_attributes, &operation,
- method, method_length,
+ method, method_data_length,
&derived_key),
expected_status);
@@ -9924,7 +9928,7 @@
int bits_arg,
int usage_arg,
int alg_arg,
- int64_t flags_arg, /*negative for truncated method*/
+ int flags_arg,
data_t *method_data,
int expected_status_arg)
{
@@ -9936,7 +9940,7 @@
psa_status_t expected_status = expected_status_arg;
psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT;
psa_key_generation_method_t *method = NULL;
- size_t method_length = 0;
+ size_t method_data_length = 0;
psa_key_attributes_t got_attributes = PSA_KEY_ATTRIBUTES_INIT;
PSA_ASSERT(psa_crypto_init());
@@ -9946,14 +9950,14 @@
psa_set_key_type(&attributes, type);
psa_set_key_bits(&attributes, bits);
- if (!setup_key_generation_method(&method, &method_length,
+ if (!setup_key_generation_method(&method, &method_data_length,
flags_arg, method_data)) {
goto exit;
}
/* Generate a key */
psa_status_t status = psa_generate_key_ext(&attributes,
- method, method_length,
+ method, method_data_length,
&key);
TEST_EQUAL(status, expected_status);
@@ -9997,11 +10001,6 @@
psa_key_generation_method_t zero;
memset(&zero, 0, sizeof(zero));
- /* In order for sizeof(psa_key_generation_method_t) to mean
- * empty data, there must not be any padding in the structure:
- * the size of the structure must be the offset of the data field. */
- TEST_EQUAL(sizeof(zero), offsetof(psa_key_generation_method_t, data));
-
TEST_EQUAL(func.flags, 0);
TEST_EQUAL(init.flags, 0);
TEST_EQUAL(zero.flags, 0);