Skip to content
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

Add PSA interruptible key agreement APIs #9490

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
a2891a9
Add PSA interuptable key agreement APIs
waleed-elmelegy-arm Aug 6, 2024
a7fc0a6
Add PSA interruptible key agreement tests
waleed-elmelegy-arm Aug 20, 2024
96a5c01
Add changelog entry for interuptible psa key agreement APIs
waleed-elmelegy-arm Aug 20, 2024
4cef20d
Fix everest build issue
waleed-elmelegy-arm Aug 22, 2024
26136ec
Improve interruptible key agreement implementation
waleed-elmelegy-arm Sep 4, 2024
57fb2a6
Refactor interuptible key agreement testing helper function
waleed-elmelegy-arm Sep 4, 2024
93be7a1
Refactor PSA key agreement API implementation
waleed-elmelegy-arm Sep 24, 2024
3783aca
Improve key agreement iop basic testing
waleed-elmelegy-arm Sep 24, 2024
d610d18
Fix codestyle in psa iop key agreement driver wrapper APIs
waleed-elmelegy-arm Sep 24, 2024
fa4eb35
Improve psa iop key agreement changelog message
waleed-elmelegy-arm Sep 24, 2024
a98aeaf
Fix iop key agreement struct initilaization error on some platforms
waleed-elmelegy-arm Sep 24, 2024
8422138
Fix possible error in initalizing key agreement iop struct
waleed-elmelegy-arm Sep 25, 2024
86e518b
Remove interuptible key agreement driver interface
waleed-elmelegy-arm Oct 30, 2024
18df1c5
Refactor and improve interuptible key agreement builtin implementation
waleed-elmelegy-arm Oct 30, 2024
cd721b9
Add a common key agreement parameter validation function across iop a…
waleed-elmelegy-arm Oct 30, 2024
280e225
Add small fixes to iop key agreement APIs
waleed-elmelegy-arm Oct 30, 2024
f840b3a
Add compile time initilaizers to ECDH and bignum structs
waleed-elmelegy-arm Nov 1, 2024
e980fbe
Fix codestyle in ECDH compile time initilaizers
waleed-elmelegy-arm Nov 1, 2024
97041ed
Fix Documentation issue in mbedtls_psa_key_agreement_iop_setup()
waleed-elmelegy-arm Nov 1, 2024
a4d0fd1
Fix a typo and a mistake in ECDH conext compile time initalizer
waleed-elmelegy-arm Nov 4, 2024
7817da0
Improve and fix compile initializers for ECDH/ECP
waleed-elmelegy-arm Nov 6, 2024
947afa0
Remove designated initializers from ECDH compile time initializers
waleed-elmelegy-arm Nov 7, 2024
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
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ if(ENABLE_TESTING OR ENABLE_PROGRAMS)
PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include
PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/tf-psa-crypto/include
PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/tf-psa-crypto/drivers/builtin/include
PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/tf-psa-crypto/drivers/everest/include
PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/library
PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/tf-psa-crypto/core
PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/tf-psa-crypto/drivers/builtin/src)
Expand Down
4 changes: 4 additions & 0 deletions ChangeLog.d/add-psa-iop-key-agreement.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Features
* Add an interruptible version of key agreement to the PSA interface.
See psa_key_agreement_iop_setup() and related functions.

1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
set(libs
${mbedtls_target}
${everest_target}
${CMAKE_THREAD_LIBS_INIT}
)

Expand Down
4 changes: 2 additions & 2 deletions tests/include/test/psa_test_wrappers.h
Original file line number Diff line number Diff line change
Expand Up @@ -475,13 +475,13 @@ psa_status_t mbedtls_test_wrap_psa_key_agreement_iop_abort(

psa_status_t mbedtls_test_wrap_psa_key_agreement_iop_complete(
psa_key_agreement_iop_t *arg0_operation,
psa_key_id_t *arg1_key);
mbedtls_svc_key_id_t *arg1_key);
#define psa_key_agreement_iop_complete(arg0_operation, arg1_key) \
mbedtls_test_wrap_psa_key_agreement_iop_complete(arg0_operation, arg1_key)

psa_status_t mbedtls_test_wrap_psa_key_agreement_iop_setup(
psa_key_agreement_iop_t *arg0_operation,
psa_key_id_t arg1_private_key,
mbedtls_svc_key_id_t arg1_private_key,
const uint8_t *arg2_peer_key,
size_t arg3_peer_key_length,
psa_algorithm_t arg4_alg,
Expand Down
36 changes: 36 additions & 0 deletions tests/psa-client-server/psasim/src/psa_sim_serialise.c
Original file line number Diff line number Diff line change
Expand Up @@ -1624,6 +1624,42 @@ int psasim_deserialise_mbedtls_svc_key_id_t(uint8_t **pos,
return 1;
}

size_t psasim_serialise_psa_key_agreement_iop_t_needs(
psa_key_agreement_iop_t value)
{
return sizeof(value);
}

int psasim_serialise_psa_key_agreement_iop_t(uint8_t **pos,
size_t *remaining,
psa_key_agreement_iop_t value)
{
if (*remaining < sizeof(value)) {
return 0;
}

memcpy(*pos, &value, sizeof(value));
*pos += sizeof(value);

return 1;
}

int psasim_deserialise_psa_key_agreement_iop_t(uint8_t **pos,
size_t *remaining,
psa_key_agreement_iop_t *value)
{
if (*remaining < sizeof(*value)) {
return 0;
}

memcpy(value, *pos, sizeof(*value));

*pos += sizeof(*value);
*remaining -= sizeof(*value);

return 1;
}

void psa_sim_serialize_reset(void)
{
memset(hash_operation_handles, 0,
Expand Down
43 changes: 43 additions & 0 deletions tests/psa-client-server/psasim/src/psa_sim_serialise.h
Original file line number Diff line number Diff line change
Expand Up @@ -1301,3 +1301,46 @@ int psasim_serialise_mbedtls_svc_key_id_t(uint8_t **pos,
int psasim_deserialise_mbedtls_svc_key_id_t(uint8_t **pos,
size_t *remaining,
mbedtls_svc_key_id_t *value);

/** Return how much buffer space is needed by \c psasim_serialise_psa_key_agreement_iop_t()
* to serialise a `psa_key_agreement_iop_t`.
*
* \param value The value that will be serialised into the buffer
* (needed in case some serialisations are value-
* dependent).
*
* \return The number of bytes needed in the buffer by
* \c psasim_serialise_psa_key_agreement_iop_t() to serialise
* the given value.
*/
size_t psasim_serialise_psa_key_agreement_iop_t_needs(
psa_key_agreement_iop_t value);

/** Serialise a `psa_key_agreement_iop_t` into a buffer.
*
* \param pos[in,out] Pointer to a `uint8_t *` holding current position
* in the buffer.
* \param remaining[in,out] Pointer to a `size_t` holding number of bytes
* remaining in the buffer.
* \param value The value to serialise into the buffer.
*
* \return \c 1 on success ("okay"), \c 0 on error.
*/
int psasim_serialise_psa_key_agreement_iop_t(uint8_t **pos,
size_t *remaining,
psa_key_agreement_iop_t value);

/** Deserialise a `psa_key_agreement_iop_t` from a buffer.
*
* \param pos[in,out] Pointer to a `uint8_t *` holding current position
* in the buffer.
* \param remaining[in,out] Pointer to a `size_t` holding number of bytes
* remaining in the buffer.
* \param value Pointer to a `psa_key_agreement_iop_t` to receive the value
* deserialised from the buffer.
*
* \return \c 1 on success ("okay"), \c 0 on error.
*/
int psasim_deserialise_psa_key_agreement_iop_t(uint8_t **pos,
size_t *remaining,
psa_key_agreement_iop_t *value);
3 changes: 2 additions & 1 deletion tests/psa-client-server/psasim/src/psa_sim_serialise.pl
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@
psa_key_derivation_operation_t
psa_sign_hash_interruptible_operation_t
psa_verify_hash_interruptible_operation_t
mbedtls_svc_key_id_t);
mbedtls_svc_key_id_t
psa_key_agreement_iop_t);

grep(s/-/ /g, @types);

Expand Down
1 change: 1 addition & 0 deletions tests/scripts/test_psa_constant_names.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ def main():
parser.add_argument('--include', '-I',
action='append', default=['tf-psa-crypto/include',
'tf-psa-crypto/drivers/builtin/include',
'tf-psa-crypto/drivers/everest/include',
'include'],
help='Directory for header files')
parser.add_argument('--keep-c',
Expand Down
4 changes: 2 additions & 2 deletions tests/src/psa_test_wrappers.c
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,7 @@ psa_status_t mbedtls_test_wrap_psa_key_agreement_iop_abort(
/* Wrapper for psa_key_agreement_iop_complete */
psa_status_t mbedtls_test_wrap_psa_key_agreement_iop_complete(
psa_key_agreement_iop_t *arg0_operation,
psa_key_id_t *arg1_key)
mbedtls_svc_key_id_t *arg1_key)
{
psa_status_t status = (psa_key_agreement_iop_complete)(arg0_operation, arg1_key);
return status;
Expand All @@ -844,7 +844,7 @@ psa_status_t mbedtls_test_wrap_psa_key_agreement_iop_complete(
/* Wrapper for psa_key_agreement_iop_setup */
psa_status_t mbedtls_test_wrap_psa_key_agreement_iop_setup(
psa_key_agreement_iop_t *arg0_operation,
psa_key_id_t arg1_private_key,
mbedtls_svc_key_id_t arg1_private_key,
const uint8_t *arg2_peer_key,
size_t arg3_peer_key_length,
psa_algorithm_t arg4_alg,
Expand Down
181 changes: 176 additions & 5 deletions tf-psa-crypto/core/psa_crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -7732,6 +7732,24 @@ psa_status_t psa_raw_key_agreement(psa_algorithm_t alg,
return (status == PSA_SUCCESS) ? unlock_status : status;
}

static psa_status_t validate_key_agreement_params(const psa_key_attributes_t *attributes,
psa_algorithm_t alg)
{
psa_key_type_t key_type;

if (!PSA_ALG_IS_RAW_KEY_AGREEMENT(alg)) {
return PSA_ERROR_INVALID_ARGUMENT;
}

key_type = psa_get_key_type(attributes);
if (key_type != PSA_KEY_TYPE_DERIVE && key_type != PSA_KEY_TYPE_RAW_DATA
&& key_type != PSA_KEY_TYPE_HMAC && key_type != PSA_KEY_TYPE_PASSWORD) {
return PSA_ERROR_INVALID_ARGUMENT;
}

return PSA_SUCCESS;
}

psa_status_t psa_key_agreement(mbedtls_svc_key_id_t private_key,
const uint8_t *peer_key,
size_t peer_key_length,
Expand All @@ -7742,14 +7760,12 @@ psa_status_t psa_key_agreement(mbedtls_svc_key_id_t private_key,
psa_status_t status;
uint8_t shared_secret[PSA_RAW_KEY_AGREEMENT_OUTPUT_MAX_SIZE];
size_t shared_secret_len;
psa_key_type_t key_type;

*key = MBEDTLS_SVC_KEY_ID_INIT;

key_type = psa_get_key_type(attributes);
if (key_type != PSA_KEY_TYPE_DERIVE && key_type != PSA_KEY_TYPE_RAW_DATA
&& key_type != PSA_KEY_TYPE_HMAC && key_type != PSA_KEY_TYPE_PASSWORD) {
return PSA_ERROR_INVALID_ARGUMENT;
status = validate_key_agreement_params(attributes, alg);
if (status != PSA_SUCCESS) {
return status;
}

status = psa_raw_key_agreement(alg, private_key, peer_key, peer_key_length, shared_secret,
Expand All @@ -7764,6 +7780,161 @@ psa_status_t psa_key_agreement(mbedtls_svc_key_id_t private_key,
return status;
}

#if defined(MBEDTLS_ECP_RESTARTABLE) && \
defined(MBEDTLS_PSA_BUILTIN_ALG_ECDH)

static psa_status_t psa_key_agreement_iop_abort_internal(psa_key_agreement_iop_t *operation)
{
psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;

if (operation->id == 0) {
return PSA_SUCCESS;
}

status = mbedtls_psa_key_agreement_iop_abort(&operation->mbedtls_ctx);

operation->id = 0;

return status;
}
#endif

uint32_t psa_key_agreement_iop_get_num_ops(
Copy link
Contributor

Choose a reason for hiding this comment

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

Many users don't care about interruptible operations and don't want to pay for the code size. This is a preexisting issue with signature. For new interruptible code we should do it right from the start, it's easier than fixing it later.

Copy link
Member

Choose a reason for hiding this comment

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

As agreed, we will do this later, further discussion on this in #7029

psa_key_agreement_iop_t *operation)
{
#if defined(MBEDTLS_ECP_RESTARTABLE) && \
defined(MBEDTLS_PSA_BUILTIN_ALG_ECDH)
return operation->num_ops;
#else
(void) operation;
return 0;
#endif
}

psa_status_t psa_key_agreement_iop_setup(
psa_key_agreement_iop_t *operation,
mbedtls_svc_key_id_t private_key,
const uint8_t *peer_key,
size_t peer_key_length,
psa_algorithm_t alg,
const psa_key_attributes_t *attributes)
{
#if defined(MBEDTLS_ECP_RESTARTABLE) && \
defined(MBEDTLS_PSA_BUILTIN_ALG_ECDH)
gilles-peskine-arm marked this conversation as resolved.
Show resolved Hide resolved
psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED;
psa_key_slot_t *slot = NULL;

if (operation->id != 0 || operation->error_occurred) {
return PSA_ERROR_BAD_STATE;
}

status = validate_key_agreement_params(attributes, alg);
if (status != PSA_SUCCESS) {
operation->error_occurred = 1;
return status;
}

status = psa_get_and_lock_transparent_key_slot_with_policy(
private_key, &slot, PSA_KEY_USAGE_DERIVE, alg);
if (status != PSA_SUCCESS) {
goto exit;
}

operation->attributes = *attributes;

operation->num_ops = 0;

/* To be removed later when driver dispatch is added. */
operation->id = PSA_CRYPTO_MBED_TLS_DRIVER_ID;

status = mbedtls_psa_key_agreement_iop_setup(&operation->mbedtls_ctx,
&slot->attr, slot->key.data,
slot->key.bytes, peer_key,
peer_key_length);

operation->num_ops = mbedtls_psa_key_agreement_iop_get_num_ops(&operation->mbedtls_ctx);

exit:
unlock_status = psa_unregister_read_under_mutex(slot);
if (status != PSA_SUCCESS) {
operation->error_occurred = 1;
gilles-peskine-arm marked this conversation as resolved.
Show resolved Hide resolved
psa_key_agreement_iop_abort_internal(operation);
return status;
}
if (unlock_status != PSA_SUCCESS) {
operation->error_occurred = 1;
}
return unlock_status;
gilles-peskine-arm marked this conversation as resolved.
Show resolved Hide resolved
#else
(void) operation;
(void) private_key;
(void) peer_key;
(void) peer_key_length;
(void) alg;
(void) attributes;
return PSA_ERROR_NOT_SUPPORTED;
#endif
}

psa_status_t psa_key_agreement_iop_complete(
psa_key_agreement_iop_t *operation,
mbedtls_svc_key_id_t *key)
{
#if defined(MBEDTLS_ECP_RESTARTABLE) && \
defined(MBEDTLS_PSA_BUILTIN_ALG_ECDH)

if (operation->id == 0 || operation->error_occurred) {
return PSA_ERROR_BAD_STATE;
}

psa_status_t status;
uint8_t intermediate_key[PSA_RAW_KEY_AGREEMENT_OUTPUT_MAX_SIZE];
size_t key_len = 0;

status = mbedtls_psa_key_agreement_iop_complete(&operation->mbedtls_ctx, intermediate_key,
sizeof(intermediate_key),
&key_len);

operation->num_ops = mbedtls_psa_key_agreement_iop_get_num_ops(&operation->mbedtls_ctx);

if (status == PSA_SUCCESS) {
status = psa_import_key(&operation->attributes, intermediate_key,
key_len, key);
}

if (status != PSA_SUCCESS && status != PSA_OPERATION_INCOMPLETE) {
operation->error_occurred = 1;
gilles-peskine-arm marked this conversation as resolved.
Show resolved Hide resolved
psa_key_agreement_iop_abort_internal(operation);
}
mbedtls_platform_zeroize(intermediate_key, sizeof(intermediate_key));
return status;
#else
(void) operation;
(void) key;
return PSA_ERROR_BAD_STATE;
#endif
}

psa_status_t psa_key_agreement_iop_abort(
psa_key_agreement_iop_t *operation)
{
#if defined(MBEDTLS_ECP_RESTARTABLE) && \
defined(MBEDTLS_PSA_BUILTIN_ALG_ECDH)
psa_status_t status;

status = psa_key_agreement_iop_abort_internal(operation);
Copy link
Contributor

Choose a reason for hiding this comment

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

If operation->id == 0 (no driver assigned, e.g. a freshly initialized object), the driver dispatch layer returns an error. But calling the API abort function on a freshly initialized object is correct, so should return PSA_SUCCESS.


operation->num_ops = 0;
operation->error_occurred = 0;

return status;
#else
(void) operation;
return PSA_SUCCESS;
#endif
}

/****************************************************************/
/* Random generation */
/****************************************************************/
Expand Down
Loading