Skip to content
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
2 changes: 2 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,8 @@ endif()
# drivers, that are appended in `drivers/CMakeLists.txt`.
#
set(TF_PSA_CRYPTO_PRIVATE_INCLUDE_DIRS
# psa_crypto_driver_wrappers.h is generated, so it may be in the build tree.
${CMAKE_CURRENT_BINARY_DIR}/core
${CMAKE_CURRENT_SOURCE_DIR}/core
${CMAKE_CURRENT_SOURCE_DIR}/dispatch
${CMAKE_CURRENT_SOURCE_DIR}/extras
Expand Down
183 changes: 182 additions & 1 deletion drivers/pqcp/src/psa_crypto_mldsa.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,189 @@

#if defined(MBEDTLS_PSA_CRYPTO_C) && defined(TF_PSA_CRYPTO_PQCP_MLDSA_ENABLED)

#include <psa/crypto.h>
#include "psa_crypto_mldsa.h"
#include "wrap_mldsa_native.h"
#include <mbedtls/platform_util.h>

int tf_psa_crypto_pqcp_driver_exists = 1;
/* The size of an ML-DSA seed in bytes.
* The PSA API uses the seed as the private key.
* (Some other ML-DSA interfaces use the "expanded secret", which is
* derived from the seed, as the private key.)
*/
#define SEED_SIZE 32

/* For now, hard-coded values for MLDSA-87 */
#define TF_PSA_CRYPTO_MLDSA_EXPANDED_SECRET_MAX_SIZE MLDSA87_SECRETKEYBYTES
#define TF_PSA_CRYPTO_MLDSA_PUBLIC_KEY_MAX_SIZE MLDSA87_PUBLICKEYBYTES
#define TF_PSA_CRYPTO_MLDSA_SIGNATURE_MAX_SIZE MLDSA87_BYTES

static psa_status_t pqcp_to_psa_error(int ret)
{
/* At the time of writing, mldsa-native has very few documented error
* conditions: only invalid signature on verification, and self-test
* failure. But this will change when we update mldsa-native
* with support for heap allocation of intermediate values.
*/
if (ret == 0) {
return PSA_SUCCESS;
} else {
/* Not really hardware, but this is the fallback error code for
* something unexpectedly going wrong in a driver. */
return PSA_ERROR_HARDWARE_FAILURE;
}
}

static psa_status_t seed_to_public_key(
size_t bits,
const uint8_t *key_buffer, size_t key_buffer_size,
uint8_t *data, size_t data_size, size_t *data_length)
{
if (key_buffer_size != SEED_SIZE) {
return PSA_ERROR_INVALID_ARGUMENT;
}

if (bits != 87) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

minor: use of magic numbers

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is cryptography. Some magic numbers are very much baked into the specification of the algorithm. Here the best we can do is #define PARAMETER_SET_87 87.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, defining it as a constant would be better as it'll make the code more readable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I'm looking at the code again and I really don't see a way in which having

    if (bits != PARAMETER_SET_87) {
        /* Other parameter sets are not supported yet. */
        return PSA_ERROR_NOT_SUPPORTED;
    }

in this file would make the code more readable. The string 87 is baked into a lot of function names and constant names. The role of 87 in this piece of code is clear in context. I just don't see how adding PARAMETER_SET_ here, and requiring the reader to double check the value of PARAMETER_SET_87, could make the code more readable rather than less.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So, if I'm honest I don't have a strong opinion so I'll leave it up to you and you're right in this case the argument for/against is quite marginal. So it's a very minor point. I guess the pros are we have 87 a few times through the patch, so if we repurposed this code for something that wasn't 87 or 87 was wrong in some way we could easily change them (though suspect that's unlikely in this case). Also by giving it a readable name we can provide traceability back to where it came from, I guess in this is just the number of bits in the key? Which admittedly is not a massive thing to work out. As you point out the cons are that you actually end up with more code, particularly on that line.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

87 is the same number (or more accurately the same substring) that appears in the names of mldsa-native functions and in some macros (MLDSA87_PUBLICKEYBYTES, tf_psa_crypto_pqcp_mldsa87_verify, etc.).

We already know that at some point in the future (likely not soon), we'll want to support the other ML-DSA parameter sets (44 and 65). Then the code will need to be more complicated to allow for up to three different bits values, each of which can be independently enabled in the compile-time configuration, each of which requiring to call a different function. That will require checking pretty much every line of the code to make sure we're calling the right functions, using the right buffer sizes, etc. Using a symbolic constant for 87 where it appears as a number will not help us at all.

/* Other parameter sets are not supported yet. */
return PSA_ERROR_NOT_SUPPORTED;
}

size_t public_key_length = MLDSA87_PUBLICKEYBYTES;
if (data_size < public_key_length) {
return PSA_ERROR_BUFFER_TOO_SMALL;
}

/* Beyond this point, we must go through the cleanup code. */
uint8_t secret[TF_PSA_CRYPTO_MLDSA_EXPANDED_SECRET_MAX_SIZE];

int ret = tf_psa_crypto_pqcp_mldsa87_keypair_internal(data,
secret,
key_buffer);
if (ret != 0) {
goto cleanup;
}
ret = 0;
*data_length = public_key_length;

cleanup:
mbedtls_platform_zeroize(secret, sizeof(secret));
return pqcp_to_psa_error(ret);
}

psa_status_t tf_psa_crypto_mldsa_export_public_key(
const psa_key_attributes_t *attributes,
const uint8_t *key_buffer, size_t key_buffer_size,
uint8_t *data, size_t data_size, size_t *data_length)
{
*data_length = 0; /* Safe default */

if (psa_get_key_type(attributes) != PSA_KEY_TYPE_ML_DSA_KEY_PAIR) {
return PSA_ERROR_NOT_SUPPORTED;
}

return seed_to_public_key(psa_get_key_bits(attributes),
key_buffer, key_buffer_size,
data, data_size, data_length);
}

psa_status_t tf_psa_crypto_mldsa_sign_message(
const psa_key_attributes_t *attributes,
const uint8_t *key_buffer, size_t key_buffer_size,
psa_algorithm_t alg,
const uint8_t *message, size_t message_length,
uint8_t *signature, size_t signature_size, size_t *signature_length)
{
*signature_length = 0; /* Safe default */

if (alg != PSA_ALG_DETERMINISTIC_ML_DSA) {
return PSA_ERROR_NOT_SUPPORTED;
}

if (psa_get_key_type(attributes) != PSA_KEY_TYPE_ML_DSA_KEY_PAIR) {
return PSA_ERROR_INVALID_ARGUMENT;
}
if (psa_get_key_bits(attributes) != 87) {
/* Other parameter sets are not supported yet. */
return PSA_ERROR_NOT_SUPPORTED;
}
size_t actual_signature_length = MLDSA87_BYTES;

if (key_buffer_size != SEED_SIZE) {
return PSA_ERROR_INVALID_ARGUMENT;
}

if (signature_size < actual_signature_length) {
return PSA_ERROR_BUFFER_TOO_SMALL;
}

/* Beyond this point, we must go through the cleanup code. */
uint8_t secret[TF_PSA_CRYPTO_MLDSA_EXPANDED_SECRET_MAX_SIZE];
uint8_t public[TF_PSA_CRYPTO_MLDSA_PUBLIC_KEY_MAX_SIZE];

int ret = tf_psa_crypto_pqcp_mldsa87_keypair_internal(public,
secret,
key_buffer);
if (ret != 0) {
goto cleanup;
}

const uint8_t prefix[2] = { 0, 0 }; // pure ML-DSA with empty context
const size_t prefix_length = sizeof(prefix);
const uint8_t rnd[MLDSA_RNDBYTES] = { 0 };

ret = tf_psa_crypto_pqcp_mldsa87_signature_internal(signature,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure if this is by design or something missed, however it appears here you are capturing a return and then not returning it? As far as I can tell the called functions have error cases that we presumably should be taking note of, is there a reason we ignore them and return success?

signature_length,
message, message_length,
prefix, prefix_length,
rnd,
secret,
0);
ret = 0;

cleanup:
mbedtls_platform_zeroize(secret, sizeof(secret));
return pqcp_to_psa_error(ret);
}

psa_status_t tf_psa_crypto_mldsa_verify_message(
const psa_key_attributes_t *attributes,
const uint8_t *key_buffer, size_t key_buffer_size,
psa_algorithm_t alg,
const uint8_t *message, size_t message_length,
const uint8_t *signature, size_t signature_length)
{
if (!PSA_ALG_IS_ML_DSA(alg)) {
return PSA_ERROR_NOT_SUPPORTED;
}

if (psa_get_key_type(attributes) != PSA_KEY_TYPE_ML_DSA_PUBLIC_KEY) {
return PSA_ERROR_INVALID_ARGUMENT;
}
if (psa_get_key_bits(attributes) != 87) {
/* Other parameter sets are not supported yet. */
return PSA_ERROR_NOT_SUPPORTED;
}
if (key_buffer_size != MLDSA87_PUBLICKEYBYTES) {
return PSA_ERROR_INVALID_ARGUMENT;
}

if (signature_length != MLDSA87_BYTES) {
return PSA_ERROR_INVALID_SIGNATURE;
}

int ret = tf_psa_crypto_pqcp_mldsa87_verify(signature, signature_length,
message, message_length,
NULL, 0,
key_buffer);
if (ret == 0) {
return PSA_SUCCESS;
} else {
/* At the time of writing, invalid signature is the only possible
* error condition. But this will change when we update mldsa-native
* with support for heap allocation of intermediate values.
*/
return PSA_ERROR_INVALID_SIGNATURE;
}
}

#endif /* MBEDTLS_PSA_CRYPTO_C && TF_PSA_CRYPTO_PQCP_MLDSA_ENABLED */
132 changes: 131 additions & 1 deletion drivers/pqcp/src/psa_crypto_mldsa.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,136 @@

#include <psa/crypto.h>

extern int tf_psa_crypto_pqcp_driver_exists;
/* Define macros for key types and algorithms here in a private header,
* rather than in a public header, because ML-DSA is not yet supported
* through the API. In particular, the size macros in <psa/crypto_sizes.h>
* do not yet take ML-DSA into account.
*/

/** The type of an ML-DSA key pair.
*
* It is represented as just the 32-byte seed.
*
* The `bits` attribute of the key indicates the parameter set:
* 44, 56 or 87.
*/
#define PSA_KEY_TYPE_ML_DSA_KEY_PAIR ((psa_key_type_t) 0x7002)

/** The type of an ML-DSA public key.
*
* The `bits` attribute of the key indicates the parameter set:
* 44, 56 or 87.
*/
#define PSA_KEY_TYPE_ML_DSA_PUBLIC_KEY ((psa_key_type_t) 0x6002)

/** Whether the key type is an ML-DSA key (key pair or public key). */
#define PSA_KEY_TYPE_IS_ML_DSA(type) \
((type) == PSA_KEY_TYPE_ML_DSA_PUBLIC_KEY || \
(type) == PSA_KEY_TYPE_ML_DSA_KEY_PAIR)

/** Hedged pure ML-DSA (without pre-hashing). */
#define PSA_ALG_ML_DSA ((psa_algorithm_t) 0x06004400)

/** Deterministic pure ML-DSA (without pre-hashing). */
#define PSA_ALG_DETERMINISTIC_ML_DSA ((psa_algorithm_t) 0x06004500)

/** Whether the given algorithm is a pure ML-DSA algorithm
* (without pre-hashing).
*/
#define PSA_ALG_IS_ML_DSA(alg) \
((alg) == PSA_ALG_DETERMINISTIC_ML_DSA || \
(alg) == PSA_ALG_ML_DSA)

/** Export the public key of an ML-DSA key pair.
*
* \param[in] attributes The key attributes.
* \param[in] key_buffer The key material. This must be a key pair
* in the standard representation, i.e.
* just the 32-byte seed.
* \param key_buffer_size The size of \p key_buffer, in bytes.
* \param[out] data On success, the exported key.
* \param data_size The size of \p data, in bytes.
* \param[out] data_length On success, the length of the data written
* to \p data.
*
* \retval 0
* Success.
* \retval #PSA_ERROR_NOT_SUPPORTED
* The key type or size registered in \p attributes is not supported.
* \retval #PSA_ERROR_INVALID_ARGUMENT
* The key material is invalid.
* \retval #PSA_ERROR_BUFFER_TOO_SMALL
* \p data_size is too small.
*/
psa_status_t tf_psa_crypto_mldsa_export_public_key(
const psa_key_attributes_t *attributes,
const uint8_t *key_buffer, size_t key_buffer_size,
uint8_t *data, size_t data_size, size_t *data_length);
Comment thread
bjwtaylor marked this conversation as resolved.

/** Sign a message using pure-ML-DSA (without pre-hashing).
*
* \param[in] attributes The key attributes.
* \param[in] key_buffer The key material. This must be a key pair
* in the standard representation, i.e.
* just the 32-byte seed.
* \param key_buffer_size The size of \p key_buffer, in bytes.
* \param alg The algorithm:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The docs currently advertise support for #PSA_ALG_ML_DSA or #PSA_ALG_DETERMINISTIC_ML_DSA, however on line drivers/pqcp/src/psa_crypto_mldsa.c:86 we appear to use psa_get_key_type(attributes) != PSA_KEY_TYPE_ML_DSA_KEY_PAIR rejecting PSA_ALG_ML_DSA. Is it the case that this is planned for future? In which case is this just the API definition and maybe we just need a note this is planned and not implemented?

* #PSA_ALG_ML_DSA (not implemented yet) or
* #PSA_ALG_DETERMINISTIC_ML_DSA.
* \param[in] message The message to sign.
* \param message_length The length of \p message, in bytes.
* \param[out] signature On success, the exported key.
* \param signature_size The size of \p signature, in bytes.
* \param[out] signature_length On success, the length of the data written
* to \p signature.
*
* \retval 0
* Success.
* \retval #PSA_ERROR_NOT_SUPPORTED
* The key type or size registered in \p attributes is not supported,
* or the algorithm is not supported.
* \retval #PSA_ERROR_INVALID_ARGUMENT
* The key material is invalid, or the key type is invalid for the
* given algorithm.
* \retval #PSA_ERROR_BUFFER_TOO_SMALL
* \p signature_size is too small.
*/
psa_status_t tf_psa_crypto_mldsa_sign_message(
const psa_key_attributes_t *attributes,
const uint8_t *key_buffer, size_t key_buffer_size,
psa_algorithm_t alg,
const uint8_t *message, size_t message_length,
uint8_t *signature, size_t signature_size, size_t *signature_length);

/** Verify a message using pure-ML-DSA (without pre-hashing).
*
* \param[in] attributes The key attributes.
* \param[in] key_buffer The key material. This must be a public key
* in the standard representation.
* \param key_buffer_size The size of \p key_buffer, in bytes.
* \param alg The algorithm:
* #PSA_ALG_ML_DSA (not implemented yet) or
* #PSA_ALG_DETERMINISTIC_ML_DSA.
* \param[in] message The message to verify.
* \param message_length The length of \p message, in bytes.
* \param[out] signature The signature to verify.
* \param signature_length The length of \p signature, in bytes.
*
* \retval 0
* Success.
* \retval #PSA_ERROR_NOT_SUPPORTED
* The key type or size registered in \p attributes is not supported,
* or the algorithm is not supported.
* \retval #PSA_ERROR_INVALID_ARGUMENT
* The key material is invalid.
* \retval #PSA_ERROR_INVALID_SIGNATURE
* The signature is not valid for this message.
*/
psa_status_t tf_psa_crypto_mldsa_verify_message(
const psa_key_attributes_t *attributes,
const uint8_t *key_buffer, size_t key_buffer_size,
psa_algorithm_t alg,
const uint8_t *message, size_t message_length,
const uint8_t *signature, size_t signature_length);

#endif /* "psa_crypto_mldsa.h" */
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,18 @@ static inline psa_status_t psa_driver_wrapper_sign_message(
return( status );
#endif /* PSA_CRYPTO_DRIVER_TEST */
#endif /* PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT */

#if defined(TF_PSA_CRYPTO_PQCP_MLDSA_ENABLED)
if (PSA_ALG_IS_ML_DSA(alg)) {
status = tf_psa_crypto_mldsa_sign_message(
attributes,
key_buffer, key_buffer_size,
alg,
input, input_length,
signature, signature_size, signature_length);
return status;
}
#endif
break;

/* Add cases for opaque driver here */
Expand Down Expand Up @@ -202,6 +214,18 @@ static inline psa_status_t psa_driver_wrapper_verify_message(
return( status );
#endif /* PSA_CRYPTO_DRIVER_TEST */
#endif /* PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT */

#if defined(TF_PSA_CRYPTO_PQCP_MLDSA_ENABLED)
if (PSA_ALG_IS_ML_DSA(alg)) {
status = tf_psa_crypto_mldsa_verify_message(
attributes,
key_buffer, key_buffer_size,
alg,
input, input_length,
signature, signature_length);
return status;
}
#endif
break;

/* Add cases for opaque driver here */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,16 @@ data_length
{% endwith -%}
#endif /* PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT */
/* Fell through, meaning no accelerator supports this operation */

#if defined(TF_PSA_CRYPTO_PQCP_MLDSA_ENABLED)
if (psa_get_key_type(attributes) == PSA_KEY_TYPE_ML_DSA_KEY_PAIR) {
status = tf_psa_crypto_mldsa_export_public_key(
attributes,
key_buffer, key_buffer_size,
data, data_size, data_length);
return status;
}
#endif
return( psa_export_public_key_internal( attributes,
key_buffer,
key_buffer_size,
Expand Down
Loading