From 20678b2ae2b4c13044e4cba48221b2348e6c2ec7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 22 Oct 2018 12:11:15 +0200 Subject: [PATCH 01/19] Skeleton for PK_OPAQUE_PSA --- include/mbedtls/pk.h | 23 +++++++++++++++++++++++ include/mbedtls/pk_internal.h | 4 ++++ library/pk.c | 23 +++++++++++++++++++++++ library/pk_wrap.c | 27 +++++++++++++++++++++++++++ 4 files changed, 77 insertions(+) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index df3a03c7c..3a35afba7 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -45,6 +45,10 @@ #include "ecdsa.h" #endif +#if defined(MBEDTLS_USE_PSA_CRYPTO) +#include "psa/crypto.h" +#endif + #if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \ !defined(inline) && !defined(__cplusplus) #define inline __inline @@ -83,6 +87,7 @@ typedef enum { MBEDTLS_PK_ECDSA, MBEDTLS_PK_RSA_ALT, MBEDTLS_PK_RSASSA_PSS, + MBEDTLS_PK_OPAQUE_PSA, } mbedtls_pk_type_t; /** @@ -234,6 +239,24 @@ void mbedtls_pk_restart_free( mbedtls_pk_restart_ctx *ctx ); */ int mbedtls_pk_setup( mbedtls_pk_context *ctx, const mbedtls_pk_info_t *info ); +#if defined(MBEDTLS_USE_PSA_CRYPTO) +/** + * \brief Initialize a PK context to wrap a PSA key slot. + * + * \param ctx Context to initialize. Must be empty (type NONE). + * \param key PSA key slot to wrap. + * + * \return 0 on success, + * MBEDTLS_ERR_PK_BAD_INPUT_DATA on invalid input, + * MBEDTLS_ERR_PK_ALLOC_FAILED on allocation failure. + * + * \note This function replaces mbedtls_pk_setup() for contexts + * that wrap a (possibly opaque) PSA key slot instead of + * storing and manipulating the key material directly. + */ +int mbedtls_pk_setup_psa( mbedtls_pk_context *ctx, const psa_key_slot_t key ); +#endif /* MBEDTLS_USE_PSA_CRYPTO */ + #if defined(MBEDTLS_PK_RSA_ALT_SUPPORT) /** * \brief Initialize an RSA-alt context diff --git a/include/mbedtls/pk_internal.h b/include/mbedtls/pk_internal.h index 48b7a5f7b..7288e9b32 100644 --- a/include/mbedtls/pk_internal.h +++ b/include/mbedtls/pk_internal.h @@ -135,4 +135,8 @@ extern const mbedtls_pk_info_t mbedtls_ecdsa_info; extern const mbedtls_pk_info_t mbedtls_rsa_alt_info; #endif +#if defined(MBEDTLS_USE_PSA_CRYPTO) +extern const mbedtls_pk_info_t mbedtls_pk_opaque_psa_info; +#endif + #endif /* MBEDTLS_PK_WRAP_H */ diff --git a/library/pk.c b/library/pk.c index e0e8dbad2..cb6e1587a 100644 --- a/library/pk.c +++ b/library/pk.c @@ -139,6 +139,29 @@ int mbedtls_pk_setup( mbedtls_pk_context *ctx, const mbedtls_pk_info_t *info ) return( 0 ); } +#if defined(MBEDTLS_USE_PSA_CRYPTO) +/* + * Initialise a PSA-wrapping context + */ +int mbedtls_pk_setup_psa( mbedtls_pk_context *ctx, const psa_key_slot_t key ) +{ + const mbedtls_pk_info_t * const info = &mbedtls_pk_opaque_psa_info; + + if( ctx == NULL || ctx->pk_info != NULL ) + return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); + + if( ( ctx->pk_ctx = info->ctx_alloc_func() ) == NULL ) + return( MBEDTLS_ERR_PK_ALLOC_FAILED ); + + /* coming soon: remember key */ + (void) key; + + ctx->pk_info = info; + + return( 0 ); +} +#endif /* MBEDTLS_USE_PSA_CRYPTO */ + #if defined(MBEDTLS_PK_RSA_ALT_SUPPORT) /* * Initialize an RSA-alt context diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 87806be33..4885c49ac 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -716,4 +716,31 @@ const mbedtls_pk_info_t mbedtls_rsa_alt_info = { #endif /* MBEDTLS_PK_RSA_ALT_SUPPORT */ +#if defined(MBEDTLS_USE_PSA_CRYPTO) + +const mbedtls_pk_info_t mbedtls_pk_opaque_psa_info = { + MBEDTLS_PK_OPAQUE_PSA, + "Opaque (PSA)", + NULL, /* coming soon: bitlen */ + NULL, /* coming soon: can_do */ + NULL, /* verify - will be done later */ + NULL, /* coming soon: sign */ +#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) + NULL, /* restartable verify - not relevant */ + NULL, /* restartable sign - not relevant */ +#endif + NULL, /* decrypt - will be done later */ + NULL, /* encrypt - will be done later */ + NULL, /* check_pair - could be done later or left NULL */ + NULL, /* coming soon: alloc */ + NULL, /* coming soon: free */ +#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) + NULL, /* restart alloc - not relevant */ + NULL, /* restart free - not relevant */ +#endif + NULL, /* debug - could be done later, or even left NULL */ +}; + +#endif /* MBEDTLS_USE_PSA_CRYPTO */ + #endif /* MBEDTLS_PK_C */ From eaeb7b23ffc160707f44d1ca4b4d12aef000c2dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 24 Oct 2018 12:37:44 +0200 Subject: [PATCH 02/19] Clarify return value of pk_check_pair() --- include/mbedtls/pk.h | 6 +++++- library/pk.c | 6 ++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 3a35afba7..d70e54650 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -503,7 +503,11 @@ int mbedtls_pk_encrypt( mbedtls_pk_context *ctx, * \param pub Context holding a public key. * \param prv Context holding a private (and public) key. * - * \return 0 on success or MBEDTLS_ERR_PK_BAD_INPUT_DATA + * \return \c 0 on success (keys were checked and match each other). + * \return #MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE if the keys could not + * be checked - in that case they may or may not match. + * \return #MBEDTLS_ERR_PK_BAD_INPUT_DATA if a context is invalid. + * \return Another non-zero value if the keys do not match. */ int mbedtls_pk_check_pair( const mbedtls_pk_context *pub, const mbedtls_pk_context *prv ); diff --git a/library/pk.c b/library/pk.c index cb6e1587a..b2f681242 100644 --- a/library/pk.c +++ b/library/pk.c @@ -456,12 +456,14 @@ int mbedtls_pk_encrypt( mbedtls_pk_context *ctx, int mbedtls_pk_check_pair( const mbedtls_pk_context *pub, const mbedtls_pk_context *prv ) { if( pub == NULL || pub->pk_info == NULL || - prv == NULL || prv->pk_info == NULL || - prv->pk_info->check_pair_func == NULL ) + prv == NULL || prv->pk_info == NULL ) { return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); } + if( prv->pk_info->check_pair_func == NULL ) + return( MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE ); + if( prv->pk_info->type == MBEDTLS_PK_RSA_ALT ) { if( pub->pk_info->type != MBEDTLS_PK_RSA ) From 7b5fe041f1be3a4ddd7c6d99b24314e9794c2db8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 31 Oct 2018 09:57:45 +0100 Subject: [PATCH 03/19] Implement alloc/free wrappers for pk_opaque_psa --- library/pk.c | 7 ++++--- library/pk_wrap.c | 19 +++++++++++++++++-- tests/suites/test_suite_pk.data | 3 +++ tests/suites/test_suite_pk.function | 19 +++++++++++++++++++ 4 files changed, 43 insertions(+), 5 deletions(-) diff --git a/library/pk.c b/library/pk.c index b2f681242..331ed6c76 100644 --- a/library/pk.c +++ b/library/pk.c @@ -146,6 +146,7 @@ int mbedtls_pk_setup( mbedtls_pk_context *ctx, const mbedtls_pk_info_t *info ) int mbedtls_pk_setup_psa( mbedtls_pk_context *ctx, const psa_key_slot_t key ) { const mbedtls_pk_info_t * const info = &mbedtls_pk_opaque_psa_info; + psa_key_slot_t *pk_ctx; if( ctx == NULL || ctx->pk_info != NULL ) return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); @@ -153,11 +154,11 @@ int mbedtls_pk_setup_psa( mbedtls_pk_context *ctx, const psa_key_slot_t key ) if( ( ctx->pk_ctx = info->ctx_alloc_func() ) == NULL ) return( MBEDTLS_ERR_PK_ALLOC_FAILED ); - /* coming soon: remember key */ - (void) key; - ctx->pk_info = info; + pk_ctx = (psa_key_slot_t *) ctx->pk_ctx; + *pk_ctx = key; + return( 0 ); } #endif /* MBEDTLS_USE_PSA_CRYPTO */ diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 4885c49ac..0e12d05c2 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -718,6 +718,21 @@ const mbedtls_pk_info_t mbedtls_rsa_alt_info = { #if defined(MBEDTLS_USE_PSA_CRYPTO) +static void *pk_psa_alloc_wrap( void ) +{ + void *ctx = mbedtls_calloc( 1, sizeof( psa_key_slot_t ) ); + + /* no _init() function to call, an calloc() already zeroized */ + + return( ctx ); +} + +static void pk_psa_free_wrap( void *ctx ) +{ + mbedtls_platform_zeroize( ctx, sizeof( psa_key_slot_t ) ); + mbedtls_free( ctx ); +} + const mbedtls_pk_info_t mbedtls_pk_opaque_psa_info = { MBEDTLS_PK_OPAQUE_PSA, "Opaque (PSA)", @@ -732,8 +747,8 @@ const mbedtls_pk_info_t mbedtls_pk_opaque_psa_info = { NULL, /* decrypt - will be done later */ NULL, /* encrypt - will be done later */ NULL, /* check_pair - could be done later or left NULL */ - NULL, /* coming soon: alloc */ - NULL, /* coming soon: free */ + pk_psa_alloc_wrap, + pk_psa_free_wrap, #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) NULL, /* restart alloc - not relevant */ NULL, /* restart free - not relevant */ diff --git a/tests/suites/test_suite_pk.data b/tests/suites/test_suite_pk.data index 478cde7be..417670d80 100644 --- a/tests/suites/test_suite_pk.data +++ b/tests/suites/test_suite_pk.data @@ -14,6 +14,9 @@ PK utils: ECDSA depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED pk_utils:MBEDTLS_PK_ECDSA:192:24:"ECDSA" +PK PSA utils +pk_psa_utils: + RSA verify test vector #1 (good) depends_on:MBEDTLS_SHA1_C:MBEDTLS_PKCS1_V15 pk_rsa_verify_test_vec:"206ef4bf396c6087f8229ef196fd35f37ccb8de5efcdb238f20d556668f114257a11fbe038464a67830378e62ae9791453953dac1dbd7921837ba98e84e856eb80ed9487e656d0b20c28c8ba5e35db1abbed83ed1c7720a97701f709e3547a4bfcabca9c89c57ad15c3996577a0ae36d7c7b699035242f37954646c1cd5c08ac":MBEDTLS_MD_SHA1:1024:16:"e28a13548525e5f36dccb24ecb7cc332cc689dfd64012604c9c7816d72a16c3f5fcdc0e86e7c03280b1c69b586ce0cd8aec722cc73a5d3b730310bf7dfebdc77ce5d94bbc369dc18a2f7b07bd505ab0f82224aef09fdc1e5063234255e0b3c40a52e9e8ae60898eb88a766bdd788fe9493d8fd86bcdd2884d5c06216c65469e5":16:"3":"5abc01f5de25b70867ff0c24e222c61f53c88daf42586fddcd56f3c4588f074be3c328056c063388688b6385a8167957c6e5355a510e005b8a851d69c96b36ec6036644078210e5d7d326f96365ee0648882921492bc7b753eb9c26cdbab37555f210df2ca6fec1b25b463d38b81c0dcea202022b04af5da58aa03d77be949b7":0 diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 4813f71f7..d95dbc9b3 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -69,6 +69,25 @@ size_t mbedtls_rsa_key_len_func( void *ctx ) * END_DEPENDENCIES */ +/* BEGIN_CASE depends_on:MBEDTLS_USE_PSA_CRYPTO */ +void pk_psa_utils( ) +{ + mbedtls_pk_context pk; + const char * const name = "Opaque (PSA)"; + + mbedtls_pk_init( &pk ); + + TEST_ASSERT( mbedtls_pk_setup_psa( &pk, 0 ) == 0 ); + + TEST_ASSERT( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_OPAQUE_PSA ); + TEST_ASSERT( strcmp( mbedtls_pk_get_name( &pk), name ) == 0 ); + +exit: + mbedtls_pk_free( &pk ); +} +/* END_CASE */ + + /* BEGIN_CASE */ void pk_utils( int type, int size, int len, char * name ) { From 01a12c49aa057c83383ac2fc1dd32489672089c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 31 Oct 2018 10:28:01 +0100 Subject: [PATCH 04/19] Add key generation to opaque test function While at it, clarify who's responsible for destroying the underlying key. That can't be us because some keys cannot be destroyed and we wouldn't know. So let's leave that up to the caller. --- include/mbedtls/pk.h | 11 ++++++++ tests/suites/test_suite_pk.function | 42 ++++++++++++++++++++++++++--- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index d70e54650..b481e437b 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -208,6 +208,11 @@ void mbedtls_pk_init( mbedtls_pk_context *ctx ); /** * \brief Free a mbedtls_pk_context + * + * \note For contexts that have been set up with + * mbedtls_pk_setup_psa(), this does not free the underlying + * key slot and you still need to call psa_destroy_key() + * independently if you want to destroy that key. */ void mbedtls_pk_free( mbedtls_pk_context *ctx ); @@ -246,6 +251,12 @@ int mbedtls_pk_setup( mbedtls_pk_context *ctx, const mbedtls_pk_info_t *info ); * \param ctx Context to initialize. Must be empty (type NONE). * \param key PSA key slot to wrap. * + * \note The wrapped key slot must remain valid as long as the + * wrapping PK context is in use, that is at least between + * the point this function is called and the point + * mbedtls_pk_free() is called on this context. The wrapped + * key slot might then be independently used or destroyed. + * * \return 0 on success, * MBEDTLS_ERR_PK_BAD_INPUT_DATA on invalid input, * MBEDTLS_ERR_PK_ALLOC_FAILED on allocation failure. diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index d95dbc9b3..64f1fec42 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -62,6 +62,34 @@ size_t mbedtls_rsa_key_len_func( void *ctx ) return( ((const mbedtls_rsa_context *) ctx)->len ); } #endif /* MBEDTLS_RSA_C */ + +#if defined(MBEDTLS_USE_PSA_CRYPTO) + +#include "mbedtls/psa_util.h" + +#define PK_PSA_INVALID_SLOT 0 /* guaranteed invalid */ + +/* + * Generate a key in a free key slot and return this key slot, + * or PK_PSA_INVALID_SLOT if no slot was available. + */ +psa_key_slot_t pk_psa_genkey( void ) +{ + psa_key_slot_t key; + + const int curve = PSA_ECC_CURVE_SECP256R1; + const psa_key_type_t type = PSA_KEY_TYPE_ECC_KEYPAIR(curve); + const size_t bits = 256; + + if( PSA_SUCCESS != mbedtls_psa_get_free_key_slot( &key ) ) + return( PK_PSA_INVALID_SLOT ); + + if( PSA_SUCCESS != psa_generate_key( key, type, bits, NULL, 0 ) ) + return( PK_PSA_INVALID_SLOT ); + + return( key ); +} +#endif /* MBEDTLS_USE_PSA_CRYPTO */ /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -69,21 +97,29 @@ size_t mbedtls_rsa_key_len_func( void *ctx ) * END_DEPENDENCIES */ -/* BEGIN_CASE depends_on:MBEDTLS_USE_PSA_CRYPTO */ +/* BEGIN_CASE depends_on:MBEDTLS_USE_PSA_CRYPTO:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED */ void pk_psa_utils( ) { mbedtls_pk_context pk; const char * const name = "Opaque (PSA)"; + psa_key_slot_t key; mbedtls_pk_init( &pk ); - TEST_ASSERT( mbedtls_pk_setup_psa( &pk, 0 ) == 0 ); + key = pk_psa_genkey(); + TEST_ASSERT( key != 0 ); + + TEST_ASSERT( mbedtls_pk_setup_psa( &pk, key ) == 0 ); TEST_ASSERT( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_OPAQUE_PSA ); TEST_ASSERT( strcmp( mbedtls_pk_get_name( &pk), name ) == 0 ); -exit: + /* test that freeing the context does not destroy the key */ mbedtls_pk_free( &pk ); + TEST_ASSERT( PSA_SUCCESS == psa_destroy_key( key ) ); + +exit: + mbedtls_pk_free( &pk ); /* redundant except upon error */ } /* END_CASE */ From 0184b3c69b8bd1891ff6b2cf711b5ddf63254eef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 31 Oct 2018 10:36:51 +0100 Subject: [PATCH 05/19] Add support for get_(bit)len on opaque keys --- library/pk_wrap.c | 13 ++++++++++++- tests/suites/test_suite_pk.function | 7 ++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 0e12d05c2..75a49a15c 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -733,10 +733,21 @@ static void pk_psa_free_wrap( void *ctx ) mbedtls_free( ctx ); } +static size_t pk_psa_get_bitlen( const void *ctx ) +{ + const psa_key_slot_t *key = (const psa_key_slot_t *) ctx; + size_t bits; + + if( PSA_SUCCESS != psa_get_key_information( *key, NULL, &bits ) ) + return( 0 ); + + return( bits ); +} + const mbedtls_pk_info_t mbedtls_pk_opaque_psa_info = { MBEDTLS_PK_OPAQUE_PSA, "Opaque (PSA)", - NULL, /* coming soon: bitlen */ + pk_psa_get_bitlen, NULL, /* coming soon: can_do */ NULL, /* verify - will be done later */ NULL, /* coming soon: sign */ diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 64f1fec42..8f6abf59e 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -101,9 +101,11 @@ psa_key_slot_t pk_psa_genkey( void ) void pk_psa_utils( ) { mbedtls_pk_context pk; - const char * const name = "Opaque (PSA)"; psa_key_slot_t key; + const char * const name = "Opaque (PSA)"; + const size_t bitlen = 256; /* harcoded in genkey() */ + mbedtls_pk_init( &pk ); key = pk_psa_genkey(); @@ -114,6 +116,9 @@ void pk_psa_utils( ) TEST_ASSERT( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_OPAQUE_PSA ); TEST_ASSERT( strcmp( mbedtls_pk_get_name( &pk), name ) == 0 ); + TEST_ASSERT( mbedtls_pk_get_bitlen( &pk ) == bitlen ); + TEST_ASSERT( mbedtls_pk_get_len( &pk ) == bitlen / 8 ); + /* test that freeing the context does not destroy the key */ mbedtls_pk_free( &pk ); TEST_ASSERT( PSA_SUCCESS == psa_destroy_key( key ) ); From 920c063bad6ca3bc27cdef40ba3d3ddd647cde74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 31 Oct 2018 10:57:29 +0100 Subject: [PATCH 06/19] Implement can_do for opaque ECC keypairs Unfortunately the can_do wrapper does not receive the key context as an argument, so it cannot check psa_get_key_information(). Later we might want to change our internal structures to fix this, but for now we'll just restrict opaque PSA keys to be ECDSA keypairs, as this is the only thing we need for now. It also simplifies testing a bit (no need to test each key type). --- include/mbedtls/pk.h | 14 ++++++++++---- library/pk.c | 8 ++++++++ library/pk_wrap.c | 11 ++++++++++- tests/suites/test_suite_pk.function | 10 ++++++++++ 4 files changed, 38 insertions(+), 5 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index b481e437b..3f640931f 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -249,7 +249,7 @@ int mbedtls_pk_setup( mbedtls_pk_context *ctx, const mbedtls_pk_info_t *info ); * \brief Initialize a PK context to wrap a PSA key slot. * * \param ctx Context to initialize. Must be empty (type NONE). - * \param key PSA key slot to wrap. + * \param key PSA key slot to wrap - must hold an ECC keypair. * * \note The wrapped key slot must remain valid as long as the * wrapping PK context is in use, that is at least between @@ -257,13 +257,19 @@ int mbedtls_pk_setup( mbedtls_pk_context *ctx, const mbedtls_pk_info_t *info ); * mbedtls_pk_free() is called on this context. The wrapped * key slot might then be independently used or destroyed. * - * \return 0 on success, - * MBEDTLS_ERR_PK_BAD_INPUT_DATA on invalid input, - * MBEDTLS_ERR_PK_ALLOC_FAILED on allocation failure. + * \return \c 0 on success, + * \return #MBEDTLS_ERR_PK_BAD_INPUT_DATA on invalid input + * (context already used, invalid key slot) + * \return #MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE if the key is not an + * ECC keypair, + * \return #MBEDTLS_ERR_PK_ALLOC_FAILED on allocation failure. * * \note This function replaces mbedtls_pk_setup() for contexts * that wrap a (possibly opaque) PSA key slot instead of * storing and manipulating the key material directly. + * + * \note This function is currently only available for ECC keypair. + * Support for other key types will be added later. */ int mbedtls_pk_setup_psa( mbedtls_pk_context *ctx, const psa_key_slot_t key ); #endif /* MBEDTLS_USE_PSA_CRYPTO */ diff --git a/library/pk.c b/library/pk.c index 331ed6c76..f65b2eed7 100644 --- a/library/pk.c +++ b/library/pk.c @@ -147,10 +147,18 @@ int mbedtls_pk_setup_psa( mbedtls_pk_context *ctx, const psa_key_slot_t key ) { const mbedtls_pk_info_t * const info = &mbedtls_pk_opaque_psa_info; psa_key_slot_t *pk_ctx; + psa_key_type_t type; if( ctx == NULL || ctx->pk_info != NULL ) return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); + if( PSA_SUCCESS != psa_get_key_information( key, &type, NULL ) ) + return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); + + /* Current implementation of can_do() relies on this. */ + if( ! PSA_KEY_TYPE_IS_ECC_KEYPAIR( type ) ) + return( MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE) ; + if( ( ctx->pk_ctx = info->ctx_alloc_func() ) == NULL ) return( MBEDTLS_ERR_PK_ALLOC_FAILED ); diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 75a49a15c..d01694c69 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -744,11 +744,20 @@ static size_t pk_psa_get_bitlen( const void *ctx ) return( bits ); } +static int pk_psa_can_do( mbedtls_pk_type_t type ) +{ + /* For now opaque PSA keys can only wrap ECC keypairs, + * as checked by setup_psa(). + * Also, ECKEY_DH does not really make sense with the current API. */ + return( type == MBEDTLS_PK_ECKEY || + type == MBEDTLS_PK_ECDSA ); +} + const mbedtls_pk_info_t mbedtls_pk_opaque_psa_info = { MBEDTLS_PK_OPAQUE_PSA, "Opaque (PSA)", pk_psa_get_bitlen, - NULL, /* coming soon: can_do */ + pk_psa_can_do, NULL, /* verify - will be done later */ NULL, /* coming soon: sign */ #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 8f6abf59e..3beff380f 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -108,6 +108,12 @@ void pk_psa_utils( ) mbedtls_pk_init( &pk ); + TEST_ASSERT( mbedtls_pk_setup_psa( &pk, 0 ) == + MBEDTLS_ERR_PK_BAD_INPUT_DATA ); + + mbedtls_pk_free( &pk ); + mbedtls_pk_init( &pk ); + key = pk_psa_genkey(); TEST_ASSERT( key != 0 ); @@ -119,6 +125,10 @@ void pk_psa_utils( ) TEST_ASSERT( mbedtls_pk_get_bitlen( &pk ) == bitlen ); TEST_ASSERT( mbedtls_pk_get_len( &pk ) == bitlen / 8 ); + TEST_ASSERT( mbedtls_pk_can_do( &pk, MBEDTLS_PK_ECKEY ) == 1 ); + TEST_ASSERT( mbedtls_pk_can_do( &pk, MBEDTLS_PK_ECDSA ) == 1 ); + TEST_ASSERT( mbedtls_pk_can_do( &pk, MBEDTLS_PK_RSA ) == 0 ); + /* test that freeing the context does not destroy the key */ mbedtls_pk_free( &pk ); TEST_ASSERT( PSA_SUCCESS == psa_destroy_key( key ) ); From d97390e97d648d34d8d064c26c0685aa1ad12bbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 31 Oct 2018 11:14:36 +0100 Subject: [PATCH 07/19] Add tests for unsupported operations/functions --- tests/suites/test_suite_pk.function | 30 ++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 3beff380f..1edc04eb2 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -100,13 +100,19 @@ psa_key_slot_t pk_psa_genkey( void ) /* BEGIN_CASE depends_on:MBEDTLS_USE_PSA_CRYPTO:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED */ void pk_psa_utils( ) { - mbedtls_pk_context pk; + mbedtls_pk_context pk, pk2; psa_key_slot_t key; const char * const name = "Opaque (PSA)"; const size_t bitlen = 256; /* harcoded in genkey() */ + mbedtls_md_type_t md_alg = MBEDTLS_MD_NONE; + unsigned char b1[1], b2[1]; + size_t len; + mbedtls_pk_debug_item dbg; + mbedtls_pk_init( &pk ); + mbedtls_pk_init( &pk2 ); TEST_ASSERT( mbedtls_pk_setup_psa( &pk, 0 ) == MBEDTLS_ERR_PK_BAD_INPUT_DATA ); @@ -129,12 +135,34 @@ void pk_psa_utils( ) TEST_ASSERT( mbedtls_pk_can_do( &pk, MBEDTLS_PK_ECDSA ) == 1 ); TEST_ASSERT( mbedtls_pk_can_do( &pk, MBEDTLS_PK_RSA ) == 0 ); + /* unsupported operations: verify, decrypt, encrypt */ + TEST_ASSERT( mbedtls_pk_verify( &pk, md_alg, + b1, sizeof( b1), b2, sizeof( b2 ) ) + == MBEDTLS_ERR_PK_TYPE_MISMATCH ); + TEST_ASSERT( mbedtls_pk_decrypt( &pk, b1, sizeof( b1 ), + b2, &len, sizeof( b2 ), + NULL, NULL ) + == MBEDTLS_ERR_PK_TYPE_MISMATCH ); + TEST_ASSERT( mbedtls_pk_encrypt( &pk, b1, sizeof( b1 ), + b2, &len, sizeof( b2 ), + NULL, NULL ) + == MBEDTLS_ERR_PK_TYPE_MISMATCH ); + + /* unsupported functions: check_pair, debug */ + TEST_ASSERT( mbedtls_pk_setup( &pk2, + mbedtls_pk_info_from_type( MBEDTLS_PK_ECKEY ) ) == 0 ); + TEST_ASSERT( mbedtls_pk_check_pair( &pk, &pk2 ) + == MBEDTLS_ERR_PK_TYPE_MISMATCH ); + TEST_ASSERT( mbedtls_pk_debug( &pk, &dbg ) + == MBEDTLS_ERR_PK_TYPE_MISMATCH ); + /* test that freeing the context does not destroy the key */ mbedtls_pk_free( &pk ); TEST_ASSERT( PSA_SUCCESS == psa_destroy_key( key ) ); exit: mbedtls_pk_free( &pk ); /* redundant except upon error */ + mbedtls_pk_free( &pk2 ); } /* END_CASE */ From 3686771dfa08d76f998ffeeebc2a38b5ab0dd4b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 31 Oct 2018 16:22:49 +0100 Subject: [PATCH 08/19] Implement pk_sign() for opaque ECDSA keys --- library/pk_wrap.c | 113 +++++++++++++++++++++++++++- tests/suites/test_suite_pk.data | 3 + tests/suites/test_suite_pk.function | 61 +++++++++++++++ 3 files changed, 176 insertions(+), 1 deletion(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index d01694c69..47f39d7e7 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -41,10 +41,18 @@ #include "mbedtls/ecdsa.h" #endif +#if defined(MBEDTLS_USE_PSA_CRYPTO) +#include "mbedtls/asn1write.h" +#endif + #if defined(MBEDTLS_PK_RSA_ALT_SUPPORT) #include "mbedtls/platform_util.h" #endif +#if defined(MBEDTLS_USE_PSA_CRYPTO) +#include "mbedtls/psa_util.h" +#endif + #if defined(MBEDTLS_PLATFORM_C) #include "mbedtls/platform.h" #else @@ -753,13 +761,116 @@ static int pk_psa_can_do( mbedtls_pk_type_t type ) type == MBEDTLS_PK_ECDSA ); } +/* Like mbedtls_asn1_write_mpi, but from a buffer */ +static int asn1_write_mpibuf( unsigned char **p, unsigned char *start, + const unsigned char *src, size_t slen ) +{ + int ret; + size_t len = 0; + + if( (size_t)( *p - start ) < slen ) + return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); + + len = slen; + *p -= len; + memcpy( *p, src, len ); + + if( **p & 0x80 ) + { + if( *p - start < 1 ) + return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); + + *--(*p) = 0x00; + len += 1; + } + + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( p, start, len ) ); + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( p, start, MBEDTLS_ASN1_INTEGER ) ); + + return( (int) len ); +} + +/* Transcode signature from PSA format to ASN.1 sequence. + * See ecdsa_signature_to_asn1 in ecdsa.c. + * + * [in] sig: the signature in PSA format + * [in/out] sig_len: signature length pre- and post-transcoding + * [out] dst: the signature in ASN.1 format + */ +static int pk_ecdsa_sig_asn1_from_psa( const unsigned char *sig, size_t *sig_len, + unsigned char *dst ) +{ + int ret; + unsigned char buf[MBEDTLS_ECDSA_MAX_LEN]; + unsigned char *p = buf + sizeof( buf ); + size_t len = 0; + const size_t mpi_len = *sig_len / 2; + + MBEDTLS_ASN1_CHK_ADD( len, asn1_write_mpibuf( &p, buf, sig + mpi_len, mpi_len ) ); + MBEDTLS_ASN1_CHK_ADD( len, asn1_write_mpibuf( &p, buf, sig, mpi_len ) ); + + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &p, buf, len ) ); + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &p, buf, + MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ); + + memcpy( dst, p, len ); + *sig_len = len; + + return( 0 ); +} + +static int pk_psa_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, + const unsigned char *hash, size_t hash_len, + unsigned char *sig, size_t *sig_len, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) +{ + const psa_key_slot_t *key = (const psa_key_slot_t *) ctx; + psa_status_t status; + psa_algorithm_t alg = PSA_ALG_ECDSA( mbedtls_psa_translate_md( md_alg ) ); + /* PSA needs a buffer of know size */ + unsigned char buf[2 * MBEDTLS_ECP_MAX_BYTES]; + const size_t buf_len = sizeof( buf ); + + /* PSA has its own RNG */ + (void) f_rng; + (void) p_rng; + + status = psa_asymmetric_sign( *key, alg, hash, hash_len, + buf, buf_len, sig_len ); + + /* translate errors to best approximation */ + switch( status ) + { + case PSA_SUCCESS: + break; /* don't return now */ + case PSA_ERROR_NOT_SUPPORTED: + return( MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE ); + case PSA_ERROR_INSUFFICIENT_MEMORY: + return( MBEDTLS_ERR_PK_ALLOC_FAILED ); + case PSA_ERROR_COMMUNICATION_FAILURE: + case PSA_ERROR_HARDWARE_FAILURE: + case PSA_ERROR_TAMPERING_DETECTED: + return( MBEDTLS_ERR_PK_HW_ACCEL_FAILED ); + case PSA_ERROR_INSUFFICIENT_ENTROPY: + return( MBEDTLS_ERR_ECP_RANDOM_FAILED ); + case PSA_ERROR_BAD_STATE: + return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); + default: /* should never happen */ + return( MBEDTLS_ERR_PK_HW_ACCEL_FAILED ); + } + + pk_ecdsa_sig_asn1_from_psa( buf, sig_len, sig ); + + return( 0 ); +} + const mbedtls_pk_info_t mbedtls_pk_opaque_psa_info = { MBEDTLS_PK_OPAQUE_PSA, "Opaque (PSA)", pk_psa_get_bitlen, pk_psa_can_do, NULL, /* verify - will be done later */ - NULL, /* coming soon: sign */ + pk_psa_sign_wrap, #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) NULL, /* restartable verify - not relevant */ NULL, /* restartable sign - not relevant */ diff --git a/tests/suites/test_suite_pk.data b/tests/suites/test_suite_pk.data index 417670d80..011b1f5f6 100644 --- a/tests/suites/test_suite_pk.data +++ b/tests/suites/test_suite_pk.data @@ -188,3 +188,6 @@ pk_sign_verify_restart:MBEDTLS_PK_ECDSA:MBEDTLS_ECP_DP_SECP256R1:"C9AFA9D845BA75 ECDSA restartable sign/verify: ECKEY, max_ops=250 depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA256_C pk_sign_verify_restart:MBEDTLS_PK_ECKEY:MBEDTLS_ECP_DP_SECP256R1:"C9AFA9D845BA75166B5C215767B1D6934E50C3DB36E89B127B8A622B120F6721":"60FED4BA255A9D31C961EB74C6356D68C049B8923B61FA6CE669622E60F29FB6":"7903FE1008B8BC99A41AE9E95628BC64F2F1B20C2D7E9F5177A3C294D4462299":MBEDTLS_MD_SHA256:"test":"3045022100f1abb023518351cd71d881567b1ea663ed3efcf6c5132b354f28d3b0b7d383670220019f4113742a2b14bd25926b49c649155f267e60d3814b4c0cc84250e46f0083":250:2:64 + +PSA wrapped sign +pk_psa_sign: diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 1edc04eb2..563fa44f5 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -72,6 +72,7 @@ size_t mbedtls_rsa_key_len_func( void *ctx ) /* * Generate a key in a free key slot and return this key slot, * or PK_PSA_INVALID_SLOT if no slot was available. + * The key uses NIST P-256 and is usable for signing with SHA-256. */ psa_key_slot_t pk_psa_genkey( void ) { @@ -80,10 +81,20 @@ psa_key_slot_t pk_psa_genkey( void ) const int curve = PSA_ECC_CURVE_SECP256R1; const psa_key_type_t type = PSA_KEY_TYPE_ECC_KEYPAIR(curve); const size_t bits = 256; + psa_key_policy_t policy; + /* find a free key slot */ if( PSA_SUCCESS != mbedtls_psa_get_free_key_slot( &key ) ) return( PK_PSA_INVALID_SLOT ); + /* set up policy on key slot */ + psa_key_policy_init( &policy ); + psa_key_policy_set_usage( &policy, PSA_KEY_USAGE_SIGN, + PSA_ALG_ECDSA(PSA_ALG_SHA_256) ); + if( PSA_SUCCESS != psa_set_key_policy( key, &policy ) ) + return( PK_PSA_INVALID_SLOT ); + + /* generate key */ if( PSA_SUCCESS != psa_generate_key( key, type, bits, NULL, 0 ) ) return( PK_PSA_INVALID_SLOT ); @@ -760,3 +771,53 @@ exit: mbedtls_pk_free( &rsa ); mbedtls_pk_free( &alt ); } /* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_SHA256_C:MBEDTLS_USE_PSA_CRYPTO:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED */ +void pk_psa_sign( ) +{ + mbedtls_pk_context pk; + psa_key_slot_t key; + unsigned char hash[50], sig[100], pkey[100]; + size_t sig_len, klen = 0; + + /* + * This tests making signatures with a wrapped PSA key: + * - generate a fresh PSA key + * - wrap it in a PK context and make a signature this way + * - extract the public key + * - parse it to a PK context and verify the signature this way + */ + + mbedtls_pk_init( &pk ); + + memset( hash, 0x2a, sizeof hash ); + memset( sig, 0, sizeof sig ); + memset( pkey, 0, sizeof pkey ); + + key = pk_psa_genkey(); + TEST_ASSERT( key != 0 ); + + TEST_ASSERT( mbedtls_pk_setup_psa( &pk, key ) == 0 ); + + TEST_ASSERT( mbedtls_pk_sign( &pk, MBEDTLS_MD_SHA256, + hash, sizeof hash, sig, &sig_len, + NULL, NULL ) == 0 ); + + mbedtls_pk_free( &pk ); + + TEST_ASSERT( PSA_SUCCESS == psa_export_public_key( + key, pkey, sizeof( pkey ), &klen ) ); + TEST_ASSERT( PSA_SUCCESS == psa_destroy_key( key ) ); + + mbedtls_pk_init( &pk ); + + TEST_ASSERT( mbedtls_pk_parse_public_key( &pk, pkey, klen ) == 0 ); + + + TEST_ASSERT( mbedtls_pk_verify( &pk, MBEDTLS_MD_SHA256, + hash, sizeof hash, sig, sig_len ) == 0 ); + +exit: + mbedtls_pk_free( &pk ); +} +/* END_CASE */ From 69baf7098466e0a65eb5d2f6f37fc809e274a1a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 6 Nov 2018 09:34:30 +0100 Subject: [PATCH 09/19] Align names to use "opaque" only everywhere It's better for names in the API to describe the "what" (opaque keys) rather than the "how" (using PSA), at least since we don't intend to have multiple function doing the same "what" in different ways in the foreseeable future. --- include/mbedtls/pk.h | 6 +++--- include/mbedtls/pk_internal.h | 2 +- library/pk.c | 4 ++-- library/pk_wrap.c | 26 +++++++++++++------------- tests/suites/test_suite_pk.function | 10 +++++----- 5 files changed, 24 insertions(+), 24 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 3f640931f..001dcca6d 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -87,7 +87,7 @@ typedef enum { MBEDTLS_PK_ECDSA, MBEDTLS_PK_RSA_ALT, MBEDTLS_PK_RSASSA_PSS, - MBEDTLS_PK_OPAQUE_PSA, + MBEDTLS_PK_OPAQUE, } mbedtls_pk_type_t; /** @@ -210,7 +210,7 @@ void mbedtls_pk_init( mbedtls_pk_context *ctx ); * \brief Free a mbedtls_pk_context * * \note For contexts that have been set up with - * mbedtls_pk_setup_psa(), this does not free the underlying + * mbedtls_pk_setup_opaque(), this does not free the underlying * key slot and you still need to call psa_destroy_key() * independently if you want to destroy that key. */ @@ -271,7 +271,7 @@ int mbedtls_pk_setup( mbedtls_pk_context *ctx, const mbedtls_pk_info_t *info ); * \note This function is currently only available for ECC keypair. * Support for other key types will be added later. */ -int mbedtls_pk_setup_psa( mbedtls_pk_context *ctx, const psa_key_slot_t key ); +int mbedtls_pk_setup_opaque( mbedtls_pk_context *ctx, const psa_key_slot_t key ); #endif /* MBEDTLS_USE_PSA_CRYPTO */ #if defined(MBEDTLS_PK_RSA_ALT_SUPPORT) diff --git a/include/mbedtls/pk_internal.h b/include/mbedtls/pk_internal.h index 7288e9b32..fc9ba13fe 100644 --- a/include/mbedtls/pk_internal.h +++ b/include/mbedtls/pk_internal.h @@ -136,7 +136,7 @@ extern const mbedtls_pk_info_t mbedtls_rsa_alt_info; #endif #if defined(MBEDTLS_USE_PSA_CRYPTO) -extern const mbedtls_pk_info_t mbedtls_pk_opaque_psa_info; +extern const mbedtls_pk_info_t mbedtls_pk_opaque_info; #endif #endif /* MBEDTLS_PK_WRAP_H */ diff --git a/library/pk.c b/library/pk.c index f65b2eed7..c34ab7e02 100644 --- a/library/pk.c +++ b/library/pk.c @@ -143,9 +143,9 @@ int mbedtls_pk_setup( mbedtls_pk_context *ctx, const mbedtls_pk_info_t *info ) /* * Initialise a PSA-wrapping context */ -int mbedtls_pk_setup_psa( mbedtls_pk_context *ctx, const psa_key_slot_t key ) +int mbedtls_pk_setup_opaque( mbedtls_pk_context *ctx, const psa_key_slot_t key ) { - const mbedtls_pk_info_t * const info = &mbedtls_pk_opaque_psa_info; + const mbedtls_pk_info_t * const info = &mbedtls_pk_opaque_info; psa_key_slot_t *pk_ctx; psa_key_type_t type; diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 47f39d7e7..e576f7334 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -726,7 +726,7 @@ const mbedtls_pk_info_t mbedtls_rsa_alt_info = { #if defined(MBEDTLS_USE_PSA_CRYPTO) -static void *pk_psa_alloc_wrap( void ) +static void *pk_opaque_alloc_wrap( void ) { void *ctx = mbedtls_calloc( 1, sizeof( psa_key_slot_t ) ); @@ -735,13 +735,13 @@ static void *pk_psa_alloc_wrap( void ) return( ctx ); } -static void pk_psa_free_wrap( void *ctx ) +static void pk_opaque_free_wrap( void *ctx ) { mbedtls_platform_zeroize( ctx, sizeof( psa_key_slot_t ) ); mbedtls_free( ctx ); } -static size_t pk_psa_get_bitlen( const void *ctx ) +static size_t pk_opaque_get_bitlen( const void *ctx ) { const psa_key_slot_t *key = (const psa_key_slot_t *) ctx; size_t bits; @@ -752,7 +752,7 @@ static size_t pk_psa_get_bitlen( const void *ctx ) return( bits ); } -static int pk_psa_can_do( mbedtls_pk_type_t type ) +static int pk_opaque_can_do( mbedtls_pk_type_t type ) { /* For now opaque PSA keys can only wrap ECC keypairs, * as checked by setup_psa(). @@ -819,7 +819,7 @@ static int pk_ecdsa_sig_asn1_from_psa( const unsigned char *sig, size_t *sig_len return( 0 ); } -static int pk_psa_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, +static int pk_opaque_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, const unsigned char *hash, size_t hash_len, unsigned char *sig, size_t *sig_len, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) @@ -864,13 +864,13 @@ static int pk_psa_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, return( 0 ); } -const mbedtls_pk_info_t mbedtls_pk_opaque_psa_info = { - MBEDTLS_PK_OPAQUE_PSA, - "Opaque (PSA)", - pk_psa_get_bitlen, - pk_psa_can_do, +const mbedtls_pk_info_t mbedtls_pk_opaque_info = { + MBEDTLS_PK_OPAQUE, + "Opaque", + pk_opaque_get_bitlen, + pk_opaque_can_do, NULL, /* verify - will be done later */ - pk_psa_sign_wrap, + pk_opaque_sign_wrap, #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) NULL, /* restartable verify - not relevant */ NULL, /* restartable sign - not relevant */ @@ -878,8 +878,8 @@ const mbedtls_pk_info_t mbedtls_pk_opaque_psa_info = { NULL, /* decrypt - will be done later */ NULL, /* encrypt - will be done later */ NULL, /* check_pair - could be done later or left NULL */ - pk_psa_alloc_wrap, - pk_psa_free_wrap, + pk_opaque_alloc_wrap, + pk_opaque_free_wrap, #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) NULL, /* restart alloc - not relevant */ NULL, /* restart free - not relevant */ diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 563fa44f5..bf87b2b0d 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -114,7 +114,7 @@ void pk_psa_utils( ) mbedtls_pk_context pk, pk2; psa_key_slot_t key; - const char * const name = "Opaque (PSA)"; + const char * const name = "Opaque"; const size_t bitlen = 256; /* harcoded in genkey() */ mbedtls_md_type_t md_alg = MBEDTLS_MD_NONE; @@ -125,7 +125,7 @@ void pk_psa_utils( ) mbedtls_pk_init( &pk ); mbedtls_pk_init( &pk2 ); - TEST_ASSERT( mbedtls_pk_setup_psa( &pk, 0 ) == + TEST_ASSERT( mbedtls_pk_setup_opaque( &pk, 0 ) == MBEDTLS_ERR_PK_BAD_INPUT_DATA ); mbedtls_pk_free( &pk ); @@ -134,9 +134,9 @@ void pk_psa_utils( ) key = pk_psa_genkey(); TEST_ASSERT( key != 0 ); - TEST_ASSERT( mbedtls_pk_setup_psa( &pk, key ) == 0 ); + TEST_ASSERT( mbedtls_pk_setup_opaque( &pk, key ) == 0 ); - TEST_ASSERT( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_OPAQUE_PSA ); + TEST_ASSERT( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_OPAQUE ); TEST_ASSERT( strcmp( mbedtls_pk_get_name( &pk), name ) == 0 ); TEST_ASSERT( mbedtls_pk_get_bitlen( &pk ) == bitlen ); @@ -797,7 +797,7 @@ void pk_psa_sign( ) key = pk_psa_genkey(); TEST_ASSERT( key != 0 ); - TEST_ASSERT( mbedtls_pk_setup_psa( &pk, key ) == 0 ); + TEST_ASSERT( mbedtls_pk_setup_opaque( &pk, key ) == 0 ); TEST_ASSERT( mbedtls_pk_sign( &pk, MBEDTLS_MD_SHA256, hash, sizeof hash, sig, &sig_len, From 392dc045c95626ff3719308dc6a4ae72dc5c4ba0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 13 Nov 2018 10:48:23 +0100 Subject: [PATCH 10/19] Improve documentation of mbedtls_pk_setup_opaque() --- include/mbedtls/pk.h | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 001dcca6d..57a7005a5 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -248,8 +248,13 @@ int mbedtls_pk_setup( mbedtls_pk_context *ctx, const mbedtls_pk_info_t *info ); /** * \brief Initialize a PK context to wrap a PSA key slot. * - * \param ctx Context to initialize. Must be empty (type NONE). - * \param key PSA key slot to wrap - must hold an ECC keypair. + * \note This function replaces mbedtls_pk_setup() for contexts + * that wrap a (possibly opaque) PSA key slot instead of + * storing and manipulating the key material directly. + * + * \param ctx The context to initialize. It must be empty (type NONE). + * \param key The PSA key slot to wrap, which must hold an ECC key pair + * (see notes below). * * \note The wrapped key slot must remain valid as long as the * wrapping PK context is in use, that is at least between @@ -257,19 +262,16 @@ int mbedtls_pk_setup( mbedtls_pk_context *ctx, const mbedtls_pk_info_t *info ); * mbedtls_pk_free() is called on this context. The wrapped * key slot might then be independently used or destroyed. * - * \return \c 0 on success, + * \note This function is currently only available for ECC key + * pairs (that is, ECC keys containing private key material). + * Support for other key types may be added later. + * + * \return \c 0 on success. * \return #MBEDTLS_ERR_PK_BAD_INPUT_DATA on invalid input - * (context already used, invalid key slot) + * (context already used, invalid key slot). * \return #MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE if the key is not an - * ECC keypair, + * ECC key pair. * \return #MBEDTLS_ERR_PK_ALLOC_FAILED on allocation failure. - * - * \note This function replaces mbedtls_pk_setup() for contexts - * that wrap a (possibly opaque) PSA key slot instead of - * storing and manipulating the key material directly. - * - * \note This function is currently only available for ECC keypair. - * Support for other key types will be added later. */ int mbedtls_pk_setup_opaque( mbedtls_pk_context *ctx, const psa_key_slot_t key ); #endif /* MBEDTLS_USE_PSA_CRYPTO */ From 2f2b396b7a2b3999894569843894febc6b313d9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 12 Nov 2018 15:06:57 +0100 Subject: [PATCH 11/19] Add new macro to detemine ECDSA signature length Revived from a previous PR by Gilles, see: https://github.com/ARMmbed/mbedtls/pull/1293/files#diff-568ef321d275f2035b8b26a70ee9af0bR71 This will be useful in eliminating temporary stack buffers for transcoding the signature: in order to do that in place we need to be able to make assumptions about the size of the output buffer, which this macro will provide. (See next commit.) --- include/mbedtls/ecdsa.h | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/include/mbedtls/ecdsa.h b/include/mbedtls/ecdsa.h index 4057828d4..5245c6ee3 100644 --- a/include/mbedtls/ecdsa.h +++ b/include/mbedtls/ecdsa.h @@ -35,25 +35,30 @@ #include "ecp.h" #include "md.h" -/* - * RFC-4492 page 20: +/** + * \brief Maximum ECDSA signature size for a given curve bit size * + * \param bits Curve size in bits + * \return Maximum signature size in bytes + * + * \note This macro returns a compile-time constant if its argument + * is one. It may evaluate its argument multiple times. + */ +/* * Ecdsa-Sig-Value ::= SEQUENCE { * r INTEGER, * s INTEGER * } * - * Size is at most - * 1 (tag) + 1 (len) + 1 (initial 0) + ECP_MAX_BYTES for each of r and s, - * twice that + 1 (tag) + 2 (len) for the sequence - * (assuming ECP_MAX_BYTES is less than 126 for r and s, - * and less than 124 (total len <= 255) for the sequence) + * For each of r and s, the value (V) may include an extra initial "0" bit. */ -#if MBEDTLS_ECP_MAX_BYTES > 124 -#error "MBEDTLS_ECP_MAX_BYTES bigger than expected, please fix MBEDTLS_ECDSA_MAX_LEN" -#endif +#define MBEDTLS_ECDSA_MAX_SIG_LEN( bits ) \ + ( /*T,L of SEQUENCE*/ ( ( bits ) >= 61 * 8 ? 3 : 2 ) + \ + /*T,L of r,s*/ 2 * ( ( ( bits ) >= 127 * 8 ? 3 : 2 ) + \ + /*V of r,s*/ ( ( bits ) + 8 ) / 8 ) ) + /** The maximal size of an ECDSA signature in Bytes. */ -#define MBEDTLS_ECDSA_MAX_LEN ( 3 + 2 * ( 3 + MBEDTLS_ECP_MAX_BYTES ) ) +#define MBEDTLS_ECDSA_MAX_LEN MBEDTLS_ECDSA_MAX_SIG_LEN( MBEDTLS_ECP_MAX_BITS ) #ifdef __cplusplus extern "C" { From d8454bc5158d3e2e23fba4faf65f3c7093be7367 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 13 Nov 2018 10:32:00 +0100 Subject: [PATCH 12/19] Get rid of large stack buffers in PSA sign wrapper --- library/pk_wrap.c | 180 ++++++++++++++++++++++++++-------------------- 1 file changed, 101 insertions(+), 79 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index e576f7334..e8b26db56 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -761,88 +761,13 @@ static int pk_opaque_can_do( mbedtls_pk_type_t type ) type == MBEDTLS_PK_ECDSA ); } -/* Like mbedtls_asn1_write_mpi, but from a buffer */ -static int asn1_write_mpibuf( unsigned char **p, unsigned char *start, - const unsigned char *src, size_t slen ) +/* translate PSA errors to best PK approximation */ +static int pk_err_from_psa( psa_status_t status ) { - int ret; - size_t len = 0; - - if( (size_t)( *p - start ) < slen ) - return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); - - len = slen; - *p -= len; - memcpy( *p, src, len ); - - if( **p & 0x80 ) - { - if( *p - start < 1 ) - return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); - - *--(*p) = 0x00; - len += 1; - } - - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( p, start, len ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( p, start, MBEDTLS_ASN1_INTEGER ) ); - - return( (int) len ); -} - -/* Transcode signature from PSA format to ASN.1 sequence. - * See ecdsa_signature_to_asn1 in ecdsa.c. - * - * [in] sig: the signature in PSA format - * [in/out] sig_len: signature length pre- and post-transcoding - * [out] dst: the signature in ASN.1 format - */ -static int pk_ecdsa_sig_asn1_from_psa( const unsigned char *sig, size_t *sig_len, - unsigned char *dst ) -{ - int ret; - unsigned char buf[MBEDTLS_ECDSA_MAX_LEN]; - unsigned char *p = buf + sizeof( buf ); - size_t len = 0; - const size_t mpi_len = *sig_len / 2; - - MBEDTLS_ASN1_CHK_ADD( len, asn1_write_mpibuf( &p, buf, sig + mpi_len, mpi_len ) ); - MBEDTLS_ASN1_CHK_ADD( len, asn1_write_mpibuf( &p, buf, sig, mpi_len ) ); - - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &p, buf, len ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &p, buf, - MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ); - - memcpy( dst, p, len ); - *sig_len = len; - - return( 0 ); -} - -static int pk_opaque_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, - const unsigned char *hash, size_t hash_len, - unsigned char *sig, size_t *sig_len, - int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) -{ - const psa_key_slot_t *key = (const psa_key_slot_t *) ctx; - psa_status_t status; - psa_algorithm_t alg = PSA_ALG_ECDSA( mbedtls_psa_translate_md( md_alg ) ); - /* PSA needs a buffer of know size */ - unsigned char buf[2 * MBEDTLS_ECP_MAX_BYTES]; - const size_t buf_len = sizeof( buf ); - - /* PSA has its own RNG */ - (void) f_rng; - (void) p_rng; - - status = psa_asymmetric_sign( *key, alg, hash, hash_len, - buf, buf_len, sig_len ); - - /* translate errors to best approximation */ switch( status ) { case PSA_SUCCESS: - break; /* don't return now */ + return( 0 ); case PSA_ERROR_NOT_SUPPORTED: return( MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE ); case PSA_ERROR_INSUFFICIENT_MEMORY: @@ -858,12 +783,109 @@ static int pk_opaque_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, default: /* should never happen */ return( MBEDTLS_ERR_PK_HW_ACCEL_FAILED ); } +} - pk_ecdsa_sig_asn1_from_psa( buf, sig_len, sig ); +/* + * Like mbedtls_asn1_write_mpi(), but from a buffer. + * + * p: pointer to the end of the output buffer + * start: start of the output buffer, and also of the mpi to write at the end + * n_len: length ot the mpi to read from start + */ +static int asn1_write_mpibuf( unsigned char **p, unsigned char *start, + size_t n_len ) +{ + int ret; + size_t len = 0; + + if( (size_t)( *p - start ) < n_len ) + return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); + + len = n_len; + *p -= len; + memmove( *p, start, len ); + + /* if the msb is 1, ASN.1 requires that we prepend a 0. + * we're never called with n_len == 0, so we can always read back a byte */ + if( **p & 0x80 ) + { + if( *p - start < 1 ) + return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); + + *--(*p) = 0x00; + len += 1; + } + + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( p, start, len ) ); + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( p, start, + MBEDTLS_ASN1_INTEGER ) ); + + return( (int) len ); +} + +/* Transcode signature from PSA format to ASN.1 sequence. + * See ecdsa_signature_to_asn1 in ecdsa.c, but with byte buffers instead of + * MPIs, and in-place. + * + * [in/out] sig: the signature pre- and post-transcoding + * [in/out] sig_len: signature length pre- and post-transcoding + * [int] buf_len: the available size the in/out buffer + */ +static int pk_ecdsa_sig_asn1_from_psa( unsigned char *sig, size_t *sig_len, + size_t buf_len ) +{ + int ret; + size_t len = 0; + const size_t rs_len = *sig_len / 2; + unsigned char *p = sig + buf_len; + + MBEDTLS_ASN1_CHK_ADD( len, asn1_write_mpibuf( &p, sig + rs_len, rs_len ) ); + MBEDTLS_ASN1_CHK_ADD( len, asn1_write_mpibuf( &p, sig, rs_len ) ); + + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &p, sig, len ) ); + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &p, sig, + MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ); + + memmove( sig, p, len ); + *sig_len = len; return( 0 ); } +static int pk_opaque_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, + const unsigned char *hash, size_t hash_len, + unsigned char *sig, size_t *sig_len, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) +{ + const psa_key_slot_t *key = (const psa_key_slot_t *) ctx; + psa_algorithm_t alg = PSA_ALG_ECDSA( mbedtls_psa_translate_md( md_alg ) ); + size_t bits, buf_len; + psa_status_t status; + + /* PSA has its own RNG */ + (void) f_rng; + (void) p_rng; + + /* PSA needs an output buffer of known size, but our API doesn't provide + * that information. Assume that the buffer is large enough for a + * maximal-length signature with that key (otherwise the application is + * buggy anyway). */ + status = psa_get_key_information( *key, NULL, &bits ); + if( status != PSA_SUCCESS ) + return( pk_err_from_psa( status ) ); + + buf_len = MBEDTLS_ECDSA_MAX_SIG_LEN( bits ); + + /* make the signature */ + status = psa_asymmetric_sign( *key, alg, hash, hash_len, + sig, buf_len, sig_len ); + if( status != PSA_SUCCESS ) + return( pk_err_from_psa( status ) ); + + /* transcode it to ASN.1 sequence */ + return( pk_ecdsa_sig_asn1_from_psa( sig, sig_len, buf_len ) ); +} + const mbedtls_pk_info_t mbedtls_pk_opaque_info = { MBEDTLS_PK_OPAQUE, "Opaque", From 509aff111f3b6e26adc7a2bab6da05c10fadd4bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 15 Nov 2018 12:17:38 +0100 Subject: [PATCH 13/19] Improve documentation of an internal function --- library/pk_wrap.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index e8b26db56..762dbfb91 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -786,11 +786,13 @@ static int pk_err_from_psa( psa_status_t status ) } /* - * Like mbedtls_asn1_write_mpi(), but from a buffer. + * Simultaneously convert and move raw MPI from the beginning of a buffer + * to an ASN.1 MPI at the end of the buffer. + * See also mbedtls_asn1_write_mpi(). * * p: pointer to the end of the output buffer * start: start of the output buffer, and also of the mpi to write at the end - * n_len: length ot the mpi to read from start + * n_len: length of the mpi to read from start */ static int asn1_write_mpibuf( unsigned char **p, unsigned char *start, size_t n_len ) From 45013a1d5440140b36443562bc628b9a3933a8df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 16 Nov 2018 10:09:11 +0100 Subject: [PATCH 14/19] Fix a compliance issue in signature encoding The issue is not present in the normal path because asn1write_mpi() does it automatically, but we're not using that here... --- library/pk_wrap.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 762dbfb91..5e8360225 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -807,8 +807,16 @@ static int asn1_write_mpibuf( unsigned char **p, unsigned char *start, *p -= len; memmove( *p, start, len ); + /* ASN.1 DER encoding requires minimal length, so skip leading 0s. + * Neither r nor s can be 0, so we can assume len > 0 at all times. */ + while( **p == 0x00 ) + { + ++(*p); + --len; + } + /* if the msb is 1, ASN.1 requires that we prepend a 0. - * we're never called with n_len == 0, so we can always read back a byte */ + * Neither r nor s can be 0, so we can assume len > 0 at all times. */ if( **p & 0x80 ) { if( *p - start < 1 ) From 9a5a77ba7ce90c89c49af9a730115e26b7d63a4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 16 Nov 2018 10:15:09 +0100 Subject: [PATCH 15/19] Use shared function for error translation --- library/pk_wrap.c | 28 ++-------------------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 5e8360225..301d2266f 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -761,30 +761,6 @@ static int pk_opaque_can_do( mbedtls_pk_type_t type ) type == MBEDTLS_PK_ECDSA ); } -/* translate PSA errors to best PK approximation */ -static int pk_err_from_psa( psa_status_t status ) -{ - switch( status ) - { - case PSA_SUCCESS: - return( 0 ); - case PSA_ERROR_NOT_SUPPORTED: - return( MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE ); - case PSA_ERROR_INSUFFICIENT_MEMORY: - return( MBEDTLS_ERR_PK_ALLOC_FAILED ); - case PSA_ERROR_COMMUNICATION_FAILURE: - case PSA_ERROR_HARDWARE_FAILURE: - case PSA_ERROR_TAMPERING_DETECTED: - return( MBEDTLS_ERR_PK_HW_ACCEL_FAILED ); - case PSA_ERROR_INSUFFICIENT_ENTROPY: - return( MBEDTLS_ERR_ECP_RANDOM_FAILED ); - case PSA_ERROR_BAD_STATE: - return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); - default: /* should never happen */ - return( MBEDTLS_ERR_PK_HW_ACCEL_FAILED ); - } -} - /* * Simultaneously convert and move raw MPI from the beginning of a buffer * to an ASN.1 MPI at the end of the buffer. @@ -882,7 +858,7 @@ static int pk_opaque_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, * buggy anyway). */ status = psa_get_key_information( *key, NULL, &bits ); if( status != PSA_SUCCESS ) - return( pk_err_from_psa( status ) ); + return( mbedtls_psa_err_translate_pk( status ) ); buf_len = MBEDTLS_ECDSA_MAX_SIG_LEN( bits ); @@ -890,7 +866,7 @@ static int pk_opaque_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, status = psa_asymmetric_sign( *key, alg, hash, hash_len, sig, buf_len, sig_len ); if( status != PSA_SUCCESS ) - return( pk_err_from_psa( status ) ); + return( mbedtls_psa_err_translate_pk( status ) ); /* transcode it to ASN.1 sequence */ return( pk_ecdsa_sig_asn1_from_psa( sig, sig_len, buf_len ) ); From 59eecb0e9eae6a5168b145cb880b022b4b5991e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 16 Nov 2018 10:54:54 +0100 Subject: [PATCH 16/19] Guard against PSA generating invalid signature The goal is not to double-check everything PSA does, but to ensure that it anything goes wrong, we fail cleanly rather than by overwriting a buffer. --- library/pk_wrap.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 301d2266f..3af17d398 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -784,13 +784,18 @@ static int asn1_write_mpibuf( unsigned char **p, unsigned char *start, memmove( *p, start, len ); /* ASN.1 DER encoding requires minimal length, so skip leading 0s. - * Neither r nor s can be 0, so we can assume len > 0 at all times. */ - while( **p == 0x00 ) + * Neither r nor s should be 0, but as a failsafe measure, still detect + * that rather than overflowing the buffer in case of a PSA error. */ + while( len > 0 && **p == 0x00 ) { ++(*p); --len; } + /* this is only reached if the signature was invalid */ + if( len == 0 ) + return( MBEDTLS_ERR_PK_HW_ACCEL_FAILED ); + /* if the msb is 1, ASN.1 requires that we prepend a 0. * Neither r nor s can be 0, so we can assume len > 0 at all times. */ if( **p & 0x80 ) From 347a00e07e56c566cb66c0b391d592fb2fed7937 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 19 Nov 2018 12:25:37 +0100 Subject: [PATCH 17/19] Add test utility function: wrap_as_opaque() The new function is not tested here, but will be in a subsequent PR. --- include/mbedtls/pk.h | 25 +++++++++++++++++ library/pk.c | 65 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 57a7005a5..862065eed 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -740,6 +740,31 @@ int mbedtls_pk_write_pubkey( unsigned char **p, unsigned char *start, int mbedtls_pk_load_file( const char *path, unsigned char **buf, size_t *n ); #endif +#if defined(MBEDTLS_USE_PSA_CRYPTO) +/** + * \brief Turn an EC key into an Opaque one + * + * \warning This is a temporary utility function for tests. It might + * change or be removed at any time without notice. + * + * \note Only ECDSA keys are supported so far. Signing with the + * specified hash is the only allowed use of that key. + * + * \param pk Input: the EC key to transfer to a PSA key slot. + * Output: a PK context wrapping that PSA key slot. + * \param slot Output: the chosen slot for storing the key. + * It's the caller's responsibility to destroy that slot + * after calling mbedtls_pk_free() on the PK context. + * \param hash_alg The hash algorithm to allow for use with that key. + * + * \return \c 0 if successful. + * \return An Mbed TLS error code otherwise. + */ +int mbedtls_pk_wrap_as_opaque( mbedtls_pk_context *pk, + psa_key_slot_t *slot, + psa_algorithm_t hash_alg ); +#endif /* MBEDTLS_USE_PSA_CRYPTO */ + #ifdef __cplusplus } #endif diff --git a/library/pk.c b/library/pk.c index c34ab7e02..989ed095b 100644 --- a/library/pk.c +++ b/library/pk.c @@ -41,6 +41,10 @@ #include "mbedtls/ecdsa.h" #endif +#if defined(MBEDTLS_USE_PSA_CRYPTO) +#include "mbedtls/psa_util.h" +#endif + #include #include @@ -535,4 +539,65 @@ mbedtls_pk_type_t mbedtls_pk_get_type( const mbedtls_pk_context *ctx ) return( ctx->pk_info->type ); } +#if defined(MBEDTLS_USE_PSA_CRYPTO) +/* + * Load the key to a PSA key slot, + * then turn the PK context into a wrapper for that key slot. + * + * Currently only works for EC private keys. + */ +int mbedtls_pk_wrap_as_opaque( mbedtls_pk_context *pk, + psa_key_slot_t *slot, + psa_algorithm_t hash_alg ) +{ +#if !defined(MBEDTLS_ECP_C) + return( MBEDTLS_ERR_PK_TYPE_MISMATCH ); +#else + psa_key_slot_t key; + const mbedtls_ecp_keypair *ec; + unsigned char d[MBEDTLS_ECP_MAX_BYTES]; + size_t d_len; + psa_ecc_curve_t curve_id; + psa_key_type_t key_type; + psa_key_policy_t policy; + int ret; + + /* export the private key material in the format PSA wants */ + if( mbedtls_pk_get_type( pk ) != MBEDTLS_PK_ECKEY ) + return( MBEDTLS_ERR_PK_TYPE_MISMATCH ); + + ec = mbedtls_pk_ec( *pk ); + d_len = ( ec->grp.nbits + 7 ) / 8; + if( ( ret = mbedtls_mpi_write_binary( &ec->d, d, d_len ) ) != 0 ) + return( ret ); + + curve_id = mbedtls_ecp_curve_info_from_grp_id( ec->grp.id )->tls_id; + + /* find a free key slot */ + if( PSA_SUCCESS != mbedtls_psa_get_free_key_slot( &key ) ) + return( MBEDTLS_ERR_PK_HW_ACCEL_FAILED ); + + /* set policy */ + psa_key_policy_init( &policy ); + psa_key_policy_set_usage( &policy, PSA_KEY_USAGE_SIGN, + PSA_ALG_ECDSA(hash_alg) ); + if( PSA_SUCCESS != psa_set_key_policy( key, &policy ) ) + return( MBEDTLS_ERR_PK_HW_ACCEL_FAILED ); + + /* import private key in slot */ + key_type = PSA_KEY_TYPE_ECC_KEYPAIR(curve_id); + if( PSA_SUCCESS != psa_import_key( key, key_type, d, d_len ) ) + return( MBEDTLS_ERR_PK_HW_ACCEL_FAILED ); + + /* remember slot number to be destroyed later by caller */ + *slot = key; + + /* make PK context wrap the key slot */ + mbedtls_pk_free( pk ); + mbedtls_pk_init( pk ); + + return( mbedtls_pk_setup_opaque( pk, key ) ); +#endif /* MBEDTLS_ECP_C */ +} +#endif /* MBEDTLS_USE_PSA_CRYPTO */ #endif /* MBEDTLS_PK_C */ From fa9a1ca9671274fbc6568d807d492cbb6edf7330 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 19 Nov 2018 12:39:27 +0100 Subject: [PATCH 18/19] Improve description of a test --- tests/suites/test_suite_pk.data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_pk.data b/tests/suites/test_suite_pk.data index 011b1f5f6..049750268 100644 --- a/tests/suites/test_suite_pk.data +++ b/tests/suites/test_suite_pk.data @@ -14,7 +14,7 @@ PK utils: ECDSA depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED pk_utils:MBEDTLS_PK_ECDSA:192:24:"ECDSA" -PK PSA utils +PK PSA utilities: setup/free, info functions, unsupported operations pk_psa_utils: RSA verify test vector #1 (good) From 23a1ccd23f56a4fec944be644066c778e626fec2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 22 Nov 2018 12:21:20 +0100 Subject: [PATCH 19/19] Fix test that wasn't actually effective psa_destroy_key() returns success even if the slot is empty. --- tests/suites/test_suite_pk.function | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index bf87b2b0d..37cf5c569 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -169,6 +169,7 @@ void pk_psa_utils( ) /* test that freeing the context does not destroy the key */ mbedtls_pk_free( &pk ); + TEST_ASSERT( PSA_SUCCESS == psa_get_key_information( key, NULL, NULL ) ); TEST_ASSERT( PSA_SUCCESS == psa_destroy_key( key ) ); exit: