From 1ecf92c364d00abbaf3a52e887760f648949c600 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 3bc2029a337aafdb24e7fed24d6089668573c376 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 274f521b9ab35278cfcb0d850aad4bb37276366b 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 06c631859cb5e567cae07c3e7ae4087494ea71f2 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 683632b78ec5c37408d33e778b31e61c03e043ab 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 07b103fe07848ce77c7f8e8a16334eb1b5a88625 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 99af2f0dd1717b929e6d045dd6016fe12bb45481 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 7d51255ca755c014b636d2c8c8909891794af7fa 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 276cb64e6c141bb88b4315ba8b88b05d50a818f9 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 35a7ff93664761644ea100c4f1c32ed037c97bbd 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 fe8607350c8c934140e0ccf7454fbb3684d9086c 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 f127e6080e907d0743edde8747e1530fe2b108ed 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 615530728f9737faf3d2673a1ef68a9c59f9d148 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 1e48ebd306c76a3e70aadc25b0ef05350faa6c21 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 f4427678ae05b451604400db9834017fab570fd4 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 29a1325b0d1ea24b3ed9ffd0576a1ed91b48f343 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 261456221224bc2ada3ce86363dc9523e4a75973 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 72d94be0deb7ce363dd85cfb2379c6715ce5917d 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 e31411a8149370291a47ff48b4991a77412c020c 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: