From da252bed3c561fed9cbd114527b37396f23d82f5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 5 Nov 2019 16:23:49 +0100 Subject: [PATCH 01/11] Define a constant for the maximum signature size from pk_sign() Based on the buffer size used in the pk_sign sample program, this is MBEDTLS_MPI_MAX_SIZE. --- include/mbedtls/pk.h | 14 ++++++++++++-- programs/pkey/pk_sign.c | 2 +- programs/pkey/pk_verify.c | 2 +- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index d750004d56..a511778078 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -101,6 +101,11 @@ typedef struct mbedtls_pk_rsassa_pss_options } mbedtls_pk_rsassa_pss_options; +/** + * \brief Maximum size of a signature made by mbedtls_pk_sign(). + */ +#define MBEDTLS_PK_SIGNATURE_MAX_SIZE MBEDTLS_MPI_MAX_SIZE + /** * \brief Types for interfacing with the debug module */ @@ -442,8 +447,13 @@ int mbedtls_pk_verify_ext( mbedtls_pk_type_t type, const void *options, * \param md_alg Hash algorithm used (see notes) * \param hash Hash of the message to sign * \param hash_len Hash length or 0 (see notes) - * \param sig Place to write the signature - * \param sig_len Number of bytes written + * \param sig Place to write the signature. + * It must have enough room for the signature. + * #MBEDTLS_PK_SIGNATURE_MAX_SIZE is always enough. + * You may use a smaller buffer if it is large enough + * given the key type. + * \param sig_len On successful return, + * the number of bytes written to \p sig. * \param f_rng RNG function * \param p_rng RNG parameter * diff --git a/programs/pkey/pk_sign.c b/programs/pkey/pk_sign.c index 47a098a1a1..79fb27376e 100644 --- a/programs/pkey/pk_sign.c +++ b/programs/pkey/pk_sign.c @@ -70,7 +70,7 @@ int main( int argc, char *argv[] ) mbedtls_entropy_context entropy; mbedtls_ctr_drbg_context ctr_drbg; unsigned char hash[32]; - unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; + unsigned char buf[MBEDTLS_PK_SIGNATURE_MAX_SIZE]; char filename[512]; const char *pers = "mbedtls_pk_sign"; size_t olen = 0; diff --git a/programs/pkey/pk_verify.c b/programs/pkey/pk_verify.c index a6bfe3f297..72caf71399 100644 --- a/programs/pkey/pk_verify.c +++ b/programs/pkey/pk_verify.c @@ -65,7 +65,7 @@ int main( int argc, char *argv[] ) size_t i; mbedtls_pk_context pk; unsigned char hash[32]; - unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; + unsigned char buf[MBEDTLS_PK_SIGNATURE_MAX_SIZE]; char filename[512]; mbedtls_pk_init( &pk ); From f85e4e67bd9a9b69a70741c4ccf1d155271a3c1b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 12 Nov 2019 11:08:23 +0100 Subject: [PATCH 02/11] test_suite_pk: fix use of sig_len without initialization In pk_sign_verify, if mbedtls_pk_sign() failed, sig_len was passed to mbedtls_pk_verify_restartable() without having been initialized. This worked only because in the only test case that expects signature to fail, the verify implementation doesn't look at sig_len before failing for the expected reason. The value of sig_len if sign() fails is undefined, so set sig_len to something sensible. --- tests/suites/test_suite_pk.function | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index b349075225..0050db7bee 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -926,6 +926,8 @@ void pk_sign_verify( int type, int sign_ret, int verify_ret ) TEST_ASSERT( mbedtls_pk_sign_restartable( &pk, MBEDTLS_MD_SHA256, hash, sizeof hash, sig, &sig_len, rnd_std_rand, NULL, rs_ctx ) == sign_ret ); + if( sign_ret != 0 ) + sig_len = MBEDTLS_PK_SIGNATURE_MAX_SIZE; TEST_ASSERT( mbedtls_pk_verify( &pk, MBEDTLS_MD_SHA256, hash, sizeof hash, sig, sig_len ) == verify_ret ); @@ -945,6 +947,8 @@ void pk_sign_verify( int type, int sign_ret, int verify_ret ) TEST_ASSERT( mbedtls_pk_sign( &pk, MBEDTLS_MD_SHA256, hash, sizeof hash, sig, &sig_len, rnd_std_rand, NULL ) == sign_ret ); + if( sign_ret != 0 ) + sig_len = MBEDTLS_PK_SIGNATURE_MAX_SIZE; TEST_ASSERT( mbedtls_pk_verify_restartable( &pk, MBEDTLS_MD_SHA256, hash, sizeof hash, sig, sig_len, rs_ctx ) == verify_ret ); From eba088a8ac585a038efccc4d21a44df1b21d1d73 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 5 Nov 2019 16:32:32 +0100 Subject: [PATCH 03/11] test_suite_pk: check the signature size after pk_sign Add a check that the signature size from pk_sign is less than the documented maximum size. Reduce the stack consumption in pk_sign_verify. --- tests/suites/test_suite_pk.function | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 0050db7bee..a7c0368c4b 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -900,8 +900,9 @@ exit: void pk_sign_verify( int type, int sign_ret, int verify_ret ) { mbedtls_pk_context pk; - unsigned char hash[50], sig[5000]; size_t sig_len; + unsigned char hash[MBEDTLS_MD_MAX_SIZE]; + unsigned char sig[MBEDTLS_PK_SIGNATURE_MAX_SIZE]; void *rs_ctx = NULL; #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) mbedtls_pk_restart_ctx ctx; @@ -926,7 +927,9 @@ void pk_sign_verify( int type, int sign_ret, int verify_ret ) TEST_ASSERT( mbedtls_pk_sign_restartable( &pk, MBEDTLS_MD_SHA256, hash, sizeof hash, sig, &sig_len, rnd_std_rand, NULL, rs_ctx ) == sign_ret ); - if( sign_ret != 0 ) + if( sign_ret == 0 ) + TEST_ASSERT( sig_len <= MBEDTLS_PK_SIGNATURE_MAX_SIZE ); + else sig_len = MBEDTLS_PK_SIGNATURE_MAX_SIZE; TEST_ASSERT( mbedtls_pk_verify( &pk, MBEDTLS_MD_SHA256, @@ -947,7 +950,9 @@ void pk_sign_verify( int type, int sign_ret, int verify_ret ) TEST_ASSERT( mbedtls_pk_sign( &pk, MBEDTLS_MD_SHA256, hash, sizeof hash, sig, &sig_len, rnd_std_rand, NULL ) == sign_ret ); - if( sign_ret != 0 ) + if( sign_ret == 0 ) + TEST_ASSERT( sig_len <= MBEDTLS_PK_SIGNATURE_MAX_SIZE ); + else sig_len = MBEDTLS_PK_SIGNATURE_MAX_SIZE; TEST_ASSERT( mbedtls_pk_verify_restartable( &pk, MBEDTLS_MD_SHA256, From e48fe55c24ed92fdafe066bd9526cc3e2ce6c37b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 5 Nov 2019 16:42:13 +0100 Subject: [PATCH 04/11] test_suite_pk: pk_genkey: support a variable key size or curve No intended behavior change. --- tests/suites/test_suite_pk.data | 30 +++++++++++++-------------- tests/suites/test_suite_pk.function | 32 +++++++++++++++++++++-------- 2 files changed, 38 insertions(+), 24 deletions(-) diff --git a/tests/suites/test_suite_pk.data b/tests/suites/test_suite_pk.data index ea5fc4f22b..e1334d9c85 100644 --- a/tests/suites/test_suite_pk.data +++ b/tests/suites/test_suite_pk.data @@ -8,21 +8,21 @@ PK write valid parameters depends_on:MBEDTLS_RSA_C valid_parameters_pkwrite:"308204a20201000282010100a9021f3d406ad555538bfd36ee82652e15615e89bfb8e84590dbee881652d3f143504796125964876bfd2be046f973beddcf92e1915bed66a06f8929794580d0836ad54143775f397c09044782b0573970eda3ec15191ea8330847c10542a9fd4cc3b4dfdd061f4d1051406773130f40f86d81255f0ab153c6307e1539acf95aee7f929ea6055be7139785b52392d9d42406d50925897507dda61a8f3f0919bead652c64eb959bdcfe415e17a6da6c5b69cc02ba142c16249c4adccdd0f7526773f12da023fd7ef431ca2d70ca890b04db2ea64f706e9ecebd5889e253599e6e5a9265e2883f0c9419a3dde5e89d9513ed29dbab7012dc5aca6b17ab528254b10203010001028201001689f5e89142ae18a6ffb0513715a4b0b4a13b9e5b3729a2bd62d738c6e15cea7bf3a4d85ab2193a0628c9452bb1f0c1af8b132789df1c95e72778bf5330f5b0d915d242d5e0818e85001ed5fa93d1ce13455deb0a15438562e8e3c8d60ec1e4c9ebff9f2b36b9cde9332cc79f0d17a7ae79cc1353cd75409ad9b4b6d7ee3d82af6f3207656cf2ac98947c15c398db0cebf8dc3eef5398269480cdd09411b960273ae3f364da09af849f24aa87346c58618ea91d9d6cd1d3932c80dbfc1f0a4166a9036911999ca27761079f0ce02db02c1c909ff9b4278578d7bb1b54b2b7082fc9e864b6b394e331c0d11a9a68255565b6dd477f4119c5809839520700711102818100d7db987ad86de6a9b0749fb5da80bacde3bebd72dcc83f60a27db74f927ac3661386577bfce5b4a00ad024682401d6aad29713c8e223b53415305ca07559821099b187fdd1bad3dc4dec9da96f5fa6128331e8f7d89f1e1a788698d1a27256dc7cd392f04e531a9e38e7265bf4fd7eec01e7835e9b1a0dd8923e440381be1c2702818100c87025fff7a493c623404966fbc8b32ed164ca620ad1a0ad11ef42fd12118456017856a8b42e5d4ad36104e9dc9f8a2f3003c3957ffddb20e2f4e3fc3cf2cdddae01f57a56de4fd24b91ab6d3e5cc0e8af0473659594a6bbfdaacf958f19c8d508eac12d8977616af6877106288093d37904a139220c1bc278ea56edc086976702818043e708685c7cf5fa9b4f948e1856366d5e1f3a694f9a8e954f884c89f3823ac5798ee12657bfcaba2dac9c47464c6dc2fecc17a531be19da706fee336bb6e47b645dbc71d3eff9856bddeb1ac9b644ffbdd58d7ba9e1240f1faaf797ba8a4d58becbaf85789e1bd979fcfccc209d3db7f0416bc9eef09b3a6d86b8ce8199d4310281804f4b86ccffe49d0d8ace98fb63ea9f708b284ba483d130b6a75cb76cb4e4372d6b41774f20912319420ca4cbfc1b25a8cb5f01d6381f6ebc50ed3ef08010327f5ba2acc1ac7220b3fa6f7399314db2879b0db0b5647abd87abb01295815a5b086491b2c0d81c616ed67ef8a8ce0727f446711d7323d4147b5828a52143c43b4b028180540756beba83c20a0bda11d6dec706a71744ff28090cec079dffb507d82828038fe657f61496a20317f779cb683ce8196c29a6fe28839a282eef4de57773be56808b0c3e2ac7747e2b200b2fbf20b55258cd24622a1ce0099de098ab0855106ae087f08b0c8c346d81619400c1b4838e33ed9ff90f05db8fccf8fb7ab881ca12" -PK utils: RSA +PK utils: RSA 512-bit depends_on:MBEDTLS_RSA_C:MBEDTLS_GENPRIME -pk_utils:MBEDTLS_PK_RSA:512:64:"RSA" +pk_utils:MBEDTLS_PK_RSA:512:512:64:"RSA" -PK utils: ECKEY +PK utils: ECKEY SECP192R1 depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED -pk_utils:MBEDTLS_PK_ECKEY:192:24:"EC" +pk_utils:MBEDTLS_PK_ECKEY:MBEDTLS_ECP_DP_SECP192R1:192:24:"EC" -PK utils: ECKEY_DH +PK utils: ECKEY_DH SECP192R1 depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED -pk_utils:MBEDTLS_PK_ECKEY_DH:192:24:"EC_DH" +pk_utils:MBEDTLS_PK_ECKEY_DH:MBEDTLS_ECP_DP_SECP192R1:192:24:"EC_DH" -PK utils: ECDSA +PK utils: ECDSA SECP192R1 depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED -pk_utils:MBEDTLS_PK_ECDSA:192:24:"ECDSA" +pk_utils:MBEDTLS_PK_ECDSA:MBEDTLS_ECP_DP_SECP192R1:192:24:"ECDSA" PK PSA utilities: setup/free, info functions, unsupported operations pk_psa_utils: @@ -83,21 +83,21 @@ EC(DSA) verify test vector: good, bitlen(s) = 247 depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED pk_ec_test_vec:MBEDTLS_PK_ECKEY:MBEDTLS_ECP_DP_SECP256R1:"0437cc56d976091e5a723ec7592dff206eee7cf9069174d0ad14b5f768225962924ee500d82311ffea2fd2345d5d16bd8a88c26b770d55cd8a2a0efa01c8b4edff":"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855":"30430220685a6994daa6a14e4411b5267edc2a00beee907f2dddd956b2a5a1df791c15f8021f675db4538c000c734489ac737fddd5a739c5a23cd6c6eceea70c286ca4fac9":0 -ECDSA sign-verify +ECDSA sign-verify: SECP192R1 depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED -pk_sign_verify:MBEDTLS_PK_ECDSA:0:0 +pk_sign_verify:MBEDTLS_PK_ECDSA:MBEDTLS_ECP_DP_SECP192R1:0:0 -EC(DSA) sign-verify +EC(DSA) sign-verify: SECP192R1 depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED -pk_sign_verify:MBEDTLS_PK_ECKEY:0:0 +pk_sign_verify:MBEDTLS_PK_ECKEY:MBEDTLS_ECP_DP_SECP192R1:0:0 -EC_DH (no) sign-verify +EC_DH (no) sign-verify: SECP192R1 depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED -pk_sign_verify:MBEDTLS_PK_ECKEY_DH:MBEDTLS_ERR_PK_TYPE_MISMATCH:MBEDTLS_ERR_PK_TYPE_MISMATCH +pk_sign_verify:MBEDTLS_PK_ECKEY_DH:MBEDTLS_ECP_DP_SECP192R1:MBEDTLS_ERR_PK_TYPE_MISMATCH:MBEDTLS_ERR_PK_TYPE_MISMATCH RSA sign-verify depends_on:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:MBEDTLS_GENPRIME -pk_sign_verify:MBEDTLS_PK_RSA:0:0 +pk_sign_verify:MBEDTLS_PK_RSA:512:0:0 RSA encrypt test vector depends_on:MBEDTLS_PKCS1_V15 diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index a7c0368c4b..ccf173632f 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -27,13 +27,27 @@ static int rnd_std_rand( void *rng_state, unsigned char *output, size_t len ); #define RSA_KEY_SIZE 512 #define RSA_KEY_LEN 64 -static int pk_genkey( mbedtls_pk_context *pk ) +/** Generate a key of the desired type. + * + * \param pk The PK object to fill. It must have been initialized + * with mbedtls_pk_setup(). + * \param parameter - For RSA keys, the key size in bits. + * - For EC keys, the curve (\c MBEDTLS_ECP_DP_xxx). + * + * \return The status from the underlying type-specific key + * generation function. + * \return -1 if the key type is not recognized. + */ +static int pk_genkey( mbedtls_pk_context *pk, int parameter ) { ((void) pk); + (void) parameter; #if defined(MBEDTLS_RSA_C) && defined(MBEDTLS_GENPRIME) if( mbedtls_pk_get_type( pk ) == MBEDTLS_PK_RSA ) - return mbedtls_rsa_gen_key( mbedtls_pk_rsa( *pk ), rnd_std_rand, NULL, RSA_KEY_SIZE, 3 ); + return mbedtls_rsa_gen_key( mbedtls_pk_rsa( *pk ), + rnd_std_rand, NULL, + parameter, 3 ); #endif #if defined(MBEDTLS_ECP_C) if( mbedtls_pk_get_type( pk ) == MBEDTLS_PK_ECKEY || @@ -42,7 +56,7 @@ static int pk_genkey( mbedtls_pk_context *pk ) { int ret; if( ( ret = mbedtls_ecp_group_load( &mbedtls_pk_ec( *pk )->grp, - MBEDTLS_ECP_DP_SECP192R1 ) ) != 0 ) + parameter ) ) != 0 ) return( ret ); return mbedtls_ecp_gen_keypair( &mbedtls_pk_ec( *pk )->grp, &mbedtls_pk_ec( *pk )->d, @@ -608,18 +622,18 @@ void invalid_parameters( ) /* END_CASE */ /* BEGIN_CASE */ -void pk_utils( int type, int size, int len, char * name ) +void pk_utils( int type, int parameter, int bitlen, int len, char * name ) { mbedtls_pk_context pk; mbedtls_pk_init( &pk ); TEST_ASSERT( mbedtls_pk_setup( &pk, mbedtls_pk_info_from_type( type ) ) == 0 ); - TEST_ASSERT( pk_genkey( &pk ) == 0 ); + TEST_ASSERT( pk_genkey( &pk, parameter ) == 0 ); TEST_ASSERT( (int) mbedtls_pk_get_type( &pk ) == type ); TEST_ASSERT( mbedtls_pk_can_do( &pk, type ) ); - TEST_ASSERT( mbedtls_pk_get_bitlen( &pk ) == (unsigned) size ); + TEST_ASSERT( mbedtls_pk_get_bitlen( &pk ) == (unsigned) bitlen ); TEST_ASSERT( mbedtls_pk_get_len( &pk ) == (unsigned) len ); TEST_ASSERT( strcmp( mbedtls_pk_get_name( &pk), name ) == 0 ); @@ -897,7 +911,7 @@ exit: /* END_CASE */ /* BEGIN_CASE depends_on:MBEDTLS_SHA256_C */ -void pk_sign_verify( int type, int sign_ret, int verify_ret ) +void pk_sign_verify( int type, int parameter, int sign_ret, int verify_ret ) { mbedtls_pk_context pk; size_t sig_len; @@ -922,7 +936,7 @@ void pk_sign_verify( int type, int sign_ret, int verify_ret ) memset( sig, 0, sizeof sig ); TEST_ASSERT( mbedtls_pk_setup( &pk, mbedtls_pk_info_from_type( type ) ) == 0 ); - TEST_ASSERT( pk_genkey( &pk ) == 0 ); + TEST_ASSERT( pk_genkey( &pk, parameter ) == 0 ); TEST_ASSERT( mbedtls_pk_sign_restartable( &pk, MBEDTLS_MD_SHA256, hash, sizeof hash, sig, &sig_len, @@ -1162,7 +1176,7 @@ void pk_rsa_alt( ) /* Initiliaze PK RSA context with random key */ TEST_ASSERT( mbedtls_pk_setup( &rsa, mbedtls_pk_info_from_type( MBEDTLS_PK_RSA ) ) == 0 ); - TEST_ASSERT( pk_genkey( &rsa ) == 0 ); + TEST_ASSERT( pk_genkey( &rsa, RSA_KEY_SIZE ) == 0 ); /* Extract key to the raw rsa context */ TEST_ASSERT( mbedtls_rsa_copy( &raw, mbedtls_pk_rsa( rsa ) ) == 0 ); From a719db8b04ac3291af1ba49bd3166b1cbe7f70fa Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 5 Nov 2019 16:48:35 +0100 Subject: [PATCH 05/11] Add pk_utils and pk_sign tests with different curves This reveals that MBEDTLS_PK_SIGNATURE_MAX_SIZE is too small. --- tests/suites/test_suite_pk.data | 40 +++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/suites/test_suite_pk.data b/tests/suites/test_suite_pk.data index e1334d9c85..caa4c77769 100644 --- a/tests/suites/test_suite_pk.data +++ b/tests/suites/test_suite_pk.data @@ -20,10 +20,30 @@ PK utils: ECKEY_DH SECP192R1 depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED pk_utils:MBEDTLS_PK_ECKEY_DH:MBEDTLS_ECP_DP_SECP192R1:192:24:"EC_DH" +PK utils: ECKEY_DH Curve25519 +depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_CURVE25519_ENABLED +pk_utils:MBEDTLS_PK_ECKEY_DH:MBEDTLS_ECP_DP_CURVE25519:255:32:"EC_DH" + +PK utils: ECKEY_DH Curve448 +depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_CURVE448_ENABLED +pk_utils:MBEDTLS_PK_ECKEY_DH:MBEDTLS_ECP_DP_CURVE448:448:56:"EC_DH" + PK utils: ECDSA SECP192R1 depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED pk_utils:MBEDTLS_PK_ECDSA:MBEDTLS_ECP_DP_SECP192R1:192:24:"ECDSA" +PK utils: ECDSA SECP256R1 +depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +pk_utils:MBEDTLS_PK_ECDSA:MBEDTLS_ECP_DP_SECP256R1:256:32:"ECDSA" + +PK utils: ECDSA SECP384R1 +depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED +pk_utils:MBEDTLS_PK_ECDSA:MBEDTLS_ECP_DP_SECP384R1:384:48:"ECDSA" + +PK utils: ECDSA SECP521R1 +depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP521R1_ENABLED +pk_utils:MBEDTLS_PK_ECDSA:MBEDTLS_ECP_DP_SECP521R1:521:66:"ECDSA" + PK PSA utilities: setup/free, info functions, unsupported operations pk_psa_utils: @@ -87,6 +107,26 @@ ECDSA sign-verify: SECP192R1 depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED pk_sign_verify:MBEDTLS_PK_ECDSA:MBEDTLS_ECP_DP_SECP192R1:0:0 +ECDSA sign-verify: SECP256R1 +depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED +pk_sign_verify:MBEDTLS_PK_ECDSA:MBEDTLS_ECP_DP_SECP256R1:0:0 + +ECDSA sign-verify: SECP384R1 +depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED +pk_sign_verify:MBEDTLS_PK_ECDSA:MBEDTLS_ECP_DP_SECP384R1:0:0 + +ECDSA sign-verify: SECP521R1 +depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP521R1_ENABLED +pk_sign_verify:MBEDTLS_PK_ECDSA:MBEDTLS_ECP_DP_SECP521R1:0:0 + +ECDSA sign-verify: BP256R1 +depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_BP256R1_ENABLED +pk_sign_verify:MBEDTLS_PK_ECDSA:MBEDTLS_ECP_DP_BP256R1:0:0 + +ECDSA sign-verify: BP512R1 +depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_BP512R1_ENABLED +pk_sign_verify:MBEDTLS_PK_ECDSA:MBEDTLS_ECP_DP_BP512R1:0:0 + EC(DSA) sign-verify: SECP192R1 depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED pk_sign_verify:MBEDTLS_PK_ECKEY:MBEDTLS_ECP_DP_SECP192R1:0:0 From b22a24b23f7807fa406c61aa64a4d4fbbde18ffe Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 5 Nov 2019 16:56:39 +0100 Subject: [PATCH 06/11] Fix MBEDTLS_PK_SIGNATURE_MAX_SIZE to account for ECDSA The original definition of MBEDTLS_PK_SIGNATURE_MAX_SIZE only took RSA into account. An ECDSA signature may be larger than the maximum possible RSA signature size, depending on build options; for example this is the case with config-suite-b.h. --- include/mbedtls/pk.h | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index a511778078..2fdc4c1fc0 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -104,7 +104,37 @@ typedef struct mbedtls_pk_rsassa_pss_options /** * \brief Maximum size of a signature made by mbedtls_pk_sign(). */ +/* This fallback value is used if there is no software signature support. + * This is possible even if check_config.h is included, for example if + * MBEDTLS_ECDH_C is enabled but neither MBEDTLS_ECDSA_C nor MBEDTLS_RSA_C. + * Use MBEDTLS_MPI_MAX_SIZE which is the maximum size than an RSA-alt + * implementation can produce, assuming that MBEDTLS_MPI_MAX_SIZE is set + * correctly. This is not necessarily the best choice of size and it may + * change in future versions. */ #define MBEDTLS_PK_SIGNATURE_MAX_SIZE MBEDTLS_MPI_MAX_SIZE +#if defined(MBEDTLS_RSA_C) && \ + MBEDTLS_MPI_MAX_SIZE > MBEDTLS_PK_SIGNATURE_MAX_SIZE +#undef MBEDTLS_PK_SIGNATURE_MAX_SIZE +#define MBEDTLS_PK_SIGNATURE_MAX_SIZE MBEDTLS_MPI_MAX_SIZE +#endif +#if defined(MBEDTLS_ECDSA_C) && \ + MBEDTLS_ECDSA_MAX_LEN > MBEDTLS_PK_SIGNATURE_MAX_SIZE +#undef MBEDTLS_PK_SIGNATURE_MAX_SIZE +#define MBEDTLS_PK_SIGNATURE_MAX_SIZE MBEDTLS_ECDSA_MAX_LEN +#endif +#if defined(MBEDTLS_USE_PSA_CRYPTO) && \ + PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE + 11 > MBEDTLS_PK_SIGNATURE_MAX_SIZE +/* PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE is the maximum size of a signature made + * through the PSA API in the PSA representation. + * The Mbed TLS representation is different for ECDSA signatures: + * PSA uses the raw concatenation of r and s, + * whereas Mbed TLS uses the ASN.1 representation (SEQUENCE of two INTEGERs). + * Add the overhead of ASN.1: up to (1+2) + 2 * (1+2+1) for the + * types, lengths (represented by up to 2 bytes), and potential leading + * zeros of the INTEGERs and the SEQUENCE. */ +#undef MBEDTLS_PK_SIGNATURE_MAX_SIZE +#define MBEDTLS_PK_SIGNATURE_MAX_SIZE ( PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE + 11 ) +#endif /** * \brief Types for interfacing with the debug module From f48d6f232092bc09e64c239f8bc511059f42c0f9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 5 Nov 2019 17:31:36 +0100 Subject: [PATCH 07/11] Add sanity checks for the mbedtls_pk_sign output size mbedtls_pk_sign does not take the size of its output buffer as a parameter. We guarantee that MBEDTLS_PK_SIGNATURE_MAX_SIZE is enough. For RSA and ECDSA signatures made in software, this is ensured by the way MBEDTLS_PK_SIGNATURE_MAX_SIZE is defined at compile time. For signatures made through RSA-alt and PSA, this is not guaranteed robustly at compile time, but we can test it at runtime, so do that. --- library/pk_wrap.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 5a699c030b..7ffb2c0c9b 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -774,6 +774,8 @@ static int rsa_alt_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, #endif /* SIZE_MAX > UINT_MAX */ *sig_len = rsa_alt->key_len_func( rsa_alt->key ); + if( *sig_len > MBEDTLS_PK_SIGNATURE_MAX_SIZE ) + return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); return( rsa_alt->sign_func( rsa_alt->key, f_rng, p_rng, MBEDTLS_RSA_PRIVATE, md_alg, (unsigned int) hash_len, hash, sig ) ); @@ -1017,6 +1019,8 @@ static int pk_opaque_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, return( mbedtls_psa_err_translate_pk( status ) ); buf_len = MBEDTLS_ECDSA_MAX_SIG_LEN( psa_get_key_bits( &attributes ) ); psa_reset_key_attributes( &attributes ); + if( *sig_len > MBEDTLS_PK_SIGNATURE_MAX_SIZE ) + return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); /* make the signature */ status = psa_asymmetric_sign( *key, alg, hash, hash_len, From 2975571ff560dd9863f77c2771c365ec6b90cf4a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 8 Nov 2019 15:49:40 +0100 Subject: [PATCH 08/11] Fix ECDSA case in PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE was taking the maximum ECDSA key size as the ECDSA signature size. Fix it to use the actual maximum size of an ECDSA signature. --- include/psa/crypto_sizes.h | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/include/psa/crypto_sizes.h b/include/psa/crypto_sizes.h index bcca72482f..33322472a7 100644 --- a/include/psa/crypto_sizes.h +++ b/include/psa/crypto_sizes.h @@ -247,21 +247,6 @@ */ #define PSA_ALG_TLS12_PSK_TO_MS_MAX_PSK_LEN 128 -/** \def PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE - * - * Maximum size of an asymmetric signature. - * - * This macro must expand to a compile-time constant integer. This value - * should be the maximum size of a MAC supported by the implementation, - * in bytes, and must be no smaller than this maximum. - */ -#define PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE \ - PSA_BITS_TO_BYTES( \ - PSA_VENDOR_RSA_MAX_KEY_BITS > PSA_VENDOR_ECC_MAX_CURVE_BITS ? \ - PSA_VENDOR_RSA_MAX_KEY_BITS : \ - PSA_VENDOR_ECC_MAX_CURVE_BITS \ - ) - /** The maximum size of a block cipher supported by the implementation. */ #define PSA_MAX_BLOCK_CIPHER_BLOCK_SIZE 16 @@ -457,6 +442,22 @@ PSA_KEY_TYPE_IS_ECC(key_type) ? PSA_ECDSA_SIGNATURE_SIZE(key_bits) : \ ((void)alg, 0)) +#define PSA_VENDOR_ECDSA_SIGNATURE_MAX_SIZE \ + PSA_ECDSA_SIGNATURE_SIZE(PSA_VENDOR_ECC_MAX_CURVE_BITS) + +/** \def PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE + * + * Maximum size of an asymmetric signature. + * + * This macro must expand to a compile-time constant integer. This value + * should be the maximum size of a signature supported by the implementation, + * in bytes, and must be no smaller than this maximum. + */ +#define PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE \ + (PSA_BITS_TO_BYTES(PSA_VENDOR_RSA_MAX_KEY_BITS) > PSA_VENDOR_ECDSA_SIGNATURE_MAX_SIZE ? \ + PSA_BITS_TO_BYTES(PSA_VENDOR_RSA_MAX_KEY_BITS) : \ + PSA_VENDOR_ECDSA_SIGNATURE_MAX_SIZE) + /** Sufficient output buffer size for psa_asymmetric_encrypt(). * * This macro returns a sufficient buffer size for a ciphertext produced using From 5460565be448bd573a9b4db8d7f5e6e1f7e0ef32 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 8 Nov 2019 16:24:16 +0100 Subject: [PATCH 09/11] Fix errors in the definition of MBEDTLS_PK_SIGNATURE_MAX_SIZE The initial value for the max calculation needs to be 0. The fallback needs to come last. With the old code, the value was never smaller than the fallback. For RSA_ALT, use MPI_MAX_SIZE. Only use this if RSA_ALT is enabled. For PSA, check PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE, and separately check the special case of ECDSA where PSA and mbedtls have different representations for the signature. --- include/mbedtls/pk.h | 45 ++++++++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 2fdc4c1fc0..d0d7ac0f82 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -104,37 +104,54 @@ typedef struct mbedtls_pk_rsassa_pss_options /** * \brief Maximum size of a signature made by mbedtls_pk_sign(). */ -/* This fallback value is used if there is no software signature support. - * This is possible even if check_config.h is included, for example if - * MBEDTLS_ECDH_C is enabled but neither MBEDTLS_ECDSA_C nor MBEDTLS_RSA_C. - * Use MBEDTLS_MPI_MAX_SIZE which is the maximum size than an RSA-alt - * implementation can produce, assuming that MBEDTLS_MPI_MAX_SIZE is set - * correctly. This is not necessarily the best choice of size and it may - * change in future versions. */ -#define MBEDTLS_PK_SIGNATURE_MAX_SIZE MBEDTLS_MPI_MAX_SIZE -#if defined(MBEDTLS_RSA_C) && \ +/* We need to set MBEDTLS_PK_SIGNATURE_MAX_SIZE to the maximum signature + * size among the supported signature types. Do it by starting at 0, + * then incrementally increasing to be large enough for each supported + * signature mechanism. + * + * The resulting value can be 0, for example if MBEDTLS_ECDH_C is enabled + * (which allows the pk module to be included) but neither MBEDTLS_ECDSA_C + * nor MBEDTLS_RSA_C nor any opaque signature mechanism (PSA or RSA_ALT). + */ +#define MBEDTLS_PK_SIGNATURE_MAX_SIZE 0 + +#if ( defined(MBEDTLS_RSA_C) || defined(MBEDTLS_PK_RSA_ALT_SUPPORT) ) && \ MBEDTLS_MPI_MAX_SIZE > MBEDTLS_PK_SIGNATURE_MAX_SIZE +/* For RSA, the signature can be as large as the bignum module allows. + * For RSA_ALT, the signature size is not necessarily tied to what the + * bignum module can do, but in the absence of any specific setting, + * we use that (rsa_alt_sign_wrap in pk_wrap will check). */ #undef MBEDTLS_PK_SIGNATURE_MAX_SIZE #define MBEDTLS_PK_SIGNATURE_MAX_SIZE MBEDTLS_MPI_MAX_SIZE #endif + #if defined(MBEDTLS_ECDSA_C) && \ MBEDTLS_ECDSA_MAX_LEN > MBEDTLS_PK_SIGNATURE_MAX_SIZE +/* For ECDSA, the ecdsa module exports a constant for the maximum + * signature size. */ #undef MBEDTLS_PK_SIGNATURE_MAX_SIZE #define MBEDTLS_PK_SIGNATURE_MAX_SIZE MBEDTLS_ECDSA_MAX_LEN #endif -#if defined(MBEDTLS_USE_PSA_CRYPTO) && \ - PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE + 11 > MBEDTLS_PK_SIGNATURE_MAX_SIZE + +#if defined(MBEDTLS_USE_PSA_CRYPTO) +#if PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE > MBEDTLS_PK_SIGNATURE_MAX_SIZE /* PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE is the maximum size of a signature made - * through the PSA API in the PSA representation. - * The Mbed TLS representation is different for ECDSA signatures: + * through the PSA API in the PSA representation. */ +#undef MBEDTLS_PK_SIGNATURE_MAX_SIZE +#define MBEDTLS_PK_SIGNATURE_MAX_SIZE PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE +#endif + +#if PSA_VENDOR_ECDSA_SIGNATURE_MAX_SIZE + 11 > MBEDTLS_PK_SIGNATURE_MAX_SIZE +/* The Mbed TLS representation is different for ECDSA signatures: * PSA uses the raw concatenation of r and s, * whereas Mbed TLS uses the ASN.1 representation (SEQUENCE of two INTEGERs). * Add the overhead of ASN.1: up to (1+2) + 2 * (1+2+1) for the * types, lengths (represented by up to 2 bytes), and potential leading * zeros of the INTEGERs and the SEQUENCE. */ #undef MBEDTLS_PK_SIGNATURE_MAX_SIZE -#define MBEDTLS_PK_SIGNATURE_MAX_SIZE ( PSA_ASYMMETRIC_SIGNATURE_MAX_SIZE + 11 ) +#define MBEDTLS_PK_SIGNATURE_MAX_SIZE ( PSA_VENDOR_ECDSA_SIGNATURE_MAX_SIZE + 11 ) #endif +#endif /* defined(MBEDTLS_USE_PSA_CRYPTO) */ /** * \brief Types for interfacing with the debug module From 5bcb24b56ec39069c75c747555c3d3c259b84f2c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 8 Nov 2019 17:33:29 +0100 Subject: [PATCH 10/11] Fix output buffer length check in pk_opaque_sign_wrap --- library/pk_wrap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 7ffb2c0c9b..702c3bbb41 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -1019,7 +1019,7 @@ static int pk_opaque_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, return( mbedtls_psa_err_translate_pk( status ) ); buf_len = MBEDTLS_ECDSA_MAX_SIG_LEN( psa_get_key_bits( &attributes ) ); psa_reset_key_attributes( &attributes ); - if( *sig_len > MBEDTLS_PK_SIGNATURE_MAX_SIZE ) + if( buf_len > MBEDTLS_PK_SIGNATURE_MAX_SIZE ) return( MBEDTLS_ERR_PK_BAD_INPUT_DATA ); /* make the signature */ From 9db14fa478abd45993e1cbf0eafcd75776275e65 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 8 Nov 2019 18:37:19 +0100 Subject: [PATCH 11/11] Update the documentation of mbedtls_pk_sign_restartable() Clarify the documentation regarding the signature size. Also fix minor niggles about references to mbedtls_pk_sign(). --- include/mbedtls/pk.h | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index d0d7ac0f82..6343563348 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -531,16 +531,21 @@ int mbedtls_pk_sign( mbedtls_pk_context *ctx, mbedtls_md_type_t md_alg, * * \param ctx The PK context to use. It must have been set up * with a private key. - * \param md_alg Hash algorithm used (see notes) + * \param md_alg Hash algorithm used (see notes for mbedtls_pk_sign()) * \param hash Hash of the message to sign - * \param hash_len Hash length or 0 (see notes) - * \param sig Place to write the signature - * \param sig_len Number of bytes written + * \param hash_len Hash length or 0 (see notes for mbedtls_pk_sign()) + * \param sig Place to write the signature. + * It must have enough room for the signature. + * #MBEDTLS_PK_SIGNATURE_MAX_SIZE is always enough. + * You may use a smaller buffer if it is large enough + * given the key type. + * \param sig_len On successful return, + * the number of bytes written to \p sig. * \param f_rng RNG function * \param p_rng RNG parameter * \param rs_ctx Restart context (NULL to disable restart) * - * \return See \c mbedtls_pk_sign(), or + * \return See \c mbedtls_pk_sign(). * \return #MBEDTLS_ERR_ECP_IN_PROGRESS if maximum number of * operations was reached: see \c mbedtls_ecp_set_max_ops(). */