From 35acfa0f4296f82da7df1685a8fa4c94078c4208 Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Tue, 24 Nov 2020 16:09:53 +1000 Subject: [PATCH] SP ECC: check the length of public key ordinates and private key Do quick bit length check before loading the MP integers into fixed size arrays. Changed ECC to use SP key check function if SP enabled and not only with SP Math. --- wolfcrypt/src/ecc.c | 50 +++++++++++++++++-------------------- wolfcrypt/src/sp_arm32.c | 18 +++++++++++++ wolfcrypt/src/sp_arm64.c | 18 +++++++++++++ wolfcrypt/src/sp_armthumb.c | 18 +++++++++++++ wolfcrypt/src/sp_c32.c | 18 +++++++++++++ wolfcrypt/src/sp_c64.c | 18 +++++++++++++ wolfcrypt/src/sp_cortexm.c | 18 +++++++++++++ wolfcrypt/src/sp_x86_64.c | 18 +++++++++++++ 8 files changed, 149 insertions(+), 27 deletions(-) diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 6e0dd5cf56..b0d34e2f83 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -6200,10 +6200,10 @@ static int wc_ecc_check_r_s_range(ecc_key* key, mp_int* r, mp_int* s) err = MP_ZERO_E; } if ((err == 0) && (mp_cmp(r, curve->order) != MP_LT)) { - return MP_VAL; + err = MP_VAL; } if ((err == 0) && (mp_cmp(s, curve->order) != MP_LT)) { - return MP_VAL; + err = MP_VAL; } wc_ecc_curve_free(curve); @@ -7444,8 +7444,8 @@ int wc_ecc_get_generator(ecc_point* ecp, int curve_idx) /* perform sanity checks on ecc key validity, 0 on success */ int wc_ecc_check_key(ecc_key* key) { +#ifndef WOLFSSL_SP_MATH int err; -#if !defined(WOLFSSL_SP_MATH) #if !defined(WOLFSSL_ATECC508A) && !defined(WOLFSSL_ATECC608A) && \ !defined(WOLFSSL_CRYPTOCELL) mp_int* b = NULL; @@ -7458,10 +7458,27 @@ int wc_ecc_check_key(ecc_key* key) DECLARE_CURVE_SPECS(curve, 3); #endif /* USE_ECC_B_PARAM */ #endif /* WOLFSSL_ATECC508A */ +#endif /* !WOLFSSL_SP_MATH */ if (key == NULL) return BAD_FUNC_ARG; +#ifdef WOLFSSL_HAVE_SP_ECC +#ifndef WOLFSSL_SP_NO_256 + if (key->idx != ECC_CUSTOM_IDX && ecc_sets[key->idx].id == ECC_SECP256R1) { + return sp_ecc_check_key_256(key->pubkey.x, key->pubkey.y, + key->type == ECC_PRIVATEKEY ? &key->k : NULL, key->heap); + } +#endif +#ifdef WOLFSSL_SP_384 + if (key->idx != ECC_CUSTOM_IDX && ecc_sets[key->idx].id == ECC_SECP384R1) { + return sp_ecc_check_key_384(key->pubkey.x, key->pubkey.y, + key->type == ECC_PRIVATEKEY ? &key->k : NULL, key->heap); + } +#endif +#endif + +#ifndef WOLFSSL_SP_MATH #if defined(WOLFSSL_ATECC508A) || defined(WOLFSSL_ATECC608A) || \ defined(WOLFSSL_CRYPTOCELL) @@ -7561,32 +7578,11 @@ int wc_ecc_check_key(ecc_key* key) FREE_CURVE_SPECS(); + return err; #endif /* WOLFSSL_ATECC508A */ #else - if (key == NULL) - return BAD_FUNC_ARG; - - /* pubkey point cannot be at infinity */ -#ifndef WOLFSSL_SP_NO_256 - if (key->idx != ECC_CUSTOM_IDX && ecc_sets[key->idx].id == ECC_SECP256R1) { - err = sp_ecc_check_key_256(key->pubkey.x, key->pubkey.y, - key->type == ECC_PRIVATEKEY ? &key->k : NULL, key->heap); - } - else -#endif -#ifdef WOLFSSL_SP_384 - if (key->idx != ECC_CUSTOM_IDX && ecc_sets[key->idx].id == ECC_SECP384R1) { - err = sp_ecc_check_key_384(key->pubkey.x, key->pubkey.y, - key->type == ECC_PRIVATEKEY ? &key->k : NULL, key->heap); - } - else -#endif - { - err = WC_KEY_SIZE_E; - } -#endif - - return err; + return WC_KEY_SIZE_E; +#endif /* !WOLFSSL_SP_MATH */ } #ifdef HAVE_ECC_KEY_IMPORT diff --git a/wolfcrypt/src/sp_arm32.c b/wolfcrypt/src/sp_arm32.c index 8af481175f..e2841500bd 100644 --- a/wolfcrypt/src/sp_arm32.c +++ b/wolfcrypt/src/sp_arm32.c @@ -37387,6 +37387,15 @@ int sp_ecc_check_key_256(mp_int* pX, mp_int* pY, mp_int* privm, void* heap) } #endif + /* Quick check the lengs of public key ordinates and private key are in + * range. Proper check later. + */ + if ((err == MP_OKAY) && ((mp_count_bits(pX) > 256) || + (mp_count_bits(pY) > 256) || + ((privm != NULL) && (mp_count_bits(privm) > 256)))) { + err = ECC_OUT_OF_RANGE_E; + } + if (err == MP_OKAY) { #if (!defined(WOLFSSL_SP_SMALL) && !defined(WOLFSSL_SMALL_STACK)) || defined(WOLFSSL_SP_NO_MALLOC) priv = privd; @@ -46136,6 +46145,15 @@ int sp_ecc_check_key_384(mp_int* pX, mp_int* pY, mp_int* privm, void* heap) } #endif + /* Quick check the lengs of public key ordinates and private key are in + * range. Proper check later. + */ + if ((err == MP_OKAY) && ((mp_count_bits(pX) > 384) || + (mp_count_bits(pY) > 384) || + ((privm != NULL) && (mp_count_bits(privm) > 384)))) { + err = ECC_OUT_OF_RANGE_E; + } + if (err == MP_OKAY) { #if (!defined(WOLFSSL_SP_SMALL) && !defined(WOLFSSL_SMALL_STACK)) || defined(WOLFSSL_SP_NO_MALLOC) priv = privd; diff --git a/wolfcrypt/src/sp_arm64.c b/wolfcrypt/src/sp_arm64.c index 9154365a60..e79150d82d 100644 --- a/wolfcrypt/src/sp_arm64.c +++ b/wolfcrypt/src/sp_arm64.c @@ -37792,6 +37792,15 @@ int sp_ecc_check_key_256(mp_int* pX, mp_int* pY, mp_int* privm, void* heap) } #endif + /* Quick check the lengs of public key ordinates and private key are in + * range. Proper check later. + */ + if ((err == MP_OKAY) && ((mp_count_bits(pX) > 256) || + (mp_count_bits(pY) > 256) || + ((privm != NULL) && (mp_count_bits(privm) > 256)))) { + err = ECC_OUT_OF_RANGE_E; + } + if (err == MP_OKAY) { #if (!defined(WOLFSSL_SP_SMALL) && !defined(WOLFSSL_SMALL_STACK)) || defined(WOLFSSL_SP_NO_MALLOC) priv = privd; @@ -44245,6 +44254,15 @@ int sp_ecc_check_key_384(mp_int* pX, mp_int* pY, mp_int* privm, void* heap) } #endif + /* Quick check the lengs of public key ordinates and private key are in + * range. Proper check later. + */ + if ((err == MP_OKAY) && ((mp_count_bits(pX) > 384) || + (mp_count_bits(pY) > 384) || + ((privm != NULL) && (mp_count_bits(privm) > 384)))) { + err = ECC_OUT_OF_RANGE_E; + } + if (err == MP_OKAY) { #if (!defined(WOLFSSL_SP_SMALL) && !defined(WOLFSSL_SMALL_STACK)) || defined(WOLFSSL_SP_NO_MALLOC) priv = privd; diff --git a/wolfcrypt/src/sp_armthumb.c b/wolfcrypt/src/sp_armthumb.c index 5335090870..d15e2f89c4 100644 --- a/wolfcrypt/src/sp_armthumb.c +++ b/wolfcrypt/src/sp_armthumb.c @@ -22160,6 +22160,15 @@ int sp_ecc_check_key_256(mp_int* pX, mp_int* pY, mp_int* privm, void* heap) } #endif + /* Quick check the lengs of public key ordinates and private key are in + * range. Proper check later. + */ + if ((err == MP_OKAY) && ((mp_count_bits(pX) > 256) || + (mp_count_bits(pY) > 256) || + ((privm != NULL) && (mp_count_bits(privm) > 256)))) { + err = ECC_OUT_OF_RANGE_E; + } + if (err == MP_OKAY) { #if (!defined(WOLFSSL_SP_SMALL) && !defined(WOLFSSL_SMALL_STACK)) || defined(WOLFSSL_SP_NO_MALLOC) priv = privd; @@ -28978,6 +28987,15 @@ int sp_ecc_check_key_384(mp_int* pX, mp_int* pY, mp_int* privm, void* heap) } #endif + /* Quick check the lengs of public key ordinates and private key are in + * range. Proper check later. + */ + if ((err == MP_OKAY) && ((mp_count_bits(pX) > 384) || + (mp_count_bits(pY) > 384) || + ((privm != NULL) && (mp_count_bits(privm) > 384)))) { + err = ECC_OUT_OF_RANGE_E; + } + if (err == MP_OKAY) { #if (!defined(WOLFSSL_SP_SMALL) && !defined(WOLFSSL_SMALL_STACK)) || defined(WOLFSSL_SP_NO_MALLOC) priv = privd; diff --git a/wolfcrypt/src/sp_c32.c b/wolfcrypt/src/sp_c32.c index 6009b49185..1e9297ece2 100644 --- a/wolfcrypt/src/sp_c32.c +++ b/wolfcrypt/src/sp_c32.c @@ -18589,6 +18589,15 @@ int sp_ecc_check_key_256(mp_int* pX, mp_int* pY, mp_int* privm, void* heap) } #endif + /* Quick check the lengs of public key ordinates and private key are in + * range. Proper check later. + */ + if ((err == MP_OKAY) && ((mp_count_bits(pX) > 256) || + (mp_count_bits(pY) > 256) || + ((privm != NULL) && (mp_count_bits(privm) > 256)))) { + err = ECC_OUT_OF_RANGE_E; + } + if (err == MP_OKAY) { #if (!defined(WOLFSSL_SP_SMALL) && !defined(WOLFSSL_SMALL_STACK)) || defined(WOLFSSL_SP_NO_MALLOC) priv = privd; @@ -25919,6 +25928,15 @@ int sp_ecc_check_key_384(mp_int* pX, mp_int* pY, mp_int* privm, void* heap) } #endif + /* Quick check the lengs of public key ordinates and private key are in + * range. Proper check later. + */ + if ((err == MP_OKAY) && ((mp_count_bits(pX) > 384) || + (mp_count_bits(pY) > 384) || + ((privm != NULL) && (mp_count_bits(privm) > 384)))) { + err = ECC_OUT_OF_RANGE_E; + } + if (err == MP_OKAY) { #if (!defined(WOLFSSL_SP_SMALL) && !defined(WOLFSSL_SMALL_STACK)) || defined(WOLFSSL_SP_NO_MALLOC) priv = privd; diff --git a/wolfcrypt/src/sp_c64.c b/wolfcrypt/src/sp_c64.c index 19c73737b2..978fce2d72 100644 --- a/wolfcrypt/src/sp_c64.c +++ b/wolfcrypt/src/sp_c64.c @@ -18315,6 +18315,15 @@ int sp_ecc_check_key_256(mp_int* pX, mp_int* pY, mp_int* privm, void* heap) } #endif + /* Quick check the lengs of public key ordinates and private key are in + * range. Proper check later. + */ + if ((err == MP_OKAY) && ((mp_count_bits(pX) > 256) || + (mp_count_bits(pY) > 256) || + ((privm != NULL) && (mp_count_bits(privm) > 256)))) { + err = ECC_OUT_OF_RANGE_E; + } + if (err == MP_OKAY) { #if (!defined(WOLFSSL_SP_SMALL) && !defined(WOLFSSL_SMALL_STACK)) || defined(WOLFSSL_SP_NO_MALLOC) priv = privd; @@ -25116,6 +25125,15 @@ int sp_ecc_check_key_384(mp_int* pX, mp_int* pY, mp_int* privm, void* heap) } #endif + /* Quick check the lengs of public key ordinates and private key are in + * range. Proper check later. + */ + if ((err == MP_OKAY) && ((mp_count_bits(pX) > 384) || + (mp_count_bits(pY) > 384) || + ((privm != NULL) && (mp_count_bits(privm) > 384)))) { + err = ECC_OUT_OF_RANGE_E; + } + if (err == MP_OKAY) { #if (!defined(WOLFSSL_SP_SMALL) && !defined(WOLFSSL_SMALL_STACK)) || defined(WOLFSSL_SP_NO_MALLOC) priv = privd; diff --git a/wolfcrypt/src/sp_cortexm.c b/wolfcrypt/src/sp_cortexm.c index dcf59c9993..348a35b2c9 100644 --- a/wolfcrypt/src/sp_cortexm.c +++ b/wolfcrypt/src/sp_cortexm.c @@ -22166,6 +22166,15 @@ int sp_ecc_check_key_256(mp_int* pX, mp_int* pY, mp_int* privm, void* heap) } #endif + /* Quick check the lengs of public key ordinates and private key are in + * range. Proper check later. + */ + if ((err == MP_OKAY) && ((mp_count_bits(pX) > 256) || + (mp_count_bits(pY) > 256) || + ((privm != NULL) && (mp_count_bits(privm) > 256)))) { + err = ECC_OUT_OF_RANGE_E; + } + if (err == MP_OKAY) { #if (!defined(WOLFSSL_SP_SMALL) && !defined(WOLFSSL_SMALL_STACK)) || defined(WOLFSSL_SP_NO_MALLOC) priv = privd; @@ -28774,6 +28783,15 @@ int sp_ecc_check_key_384(mp_int* pX, mp_int* pY, mp_int* privm, void* heap) } #endif + /* Quick check the lengs of public key ordinates and private key are in + * range. Proper check later. + */ + if ((err == MP_OKAY) && ((mp_count_bits(pX) > 384) || + (mp_count_bits(pY) > 384) || + ((privm != NULL) && (mp_count_bits(privm) > 384)))) { + err = ECC_OUT_OF_RANGE_E; + } + if (err == MP_OKAY) { #if (!defined(WOLFSSL_SP_SMALL) && !defined(WOLFSSL_SMALL_STACK)) || defined(WOLFSSL_SP_NO_MALLOC) priv = privd; diff --git a/wolfcrypt/src/sp_x86_64.c b/wolfcrypt/src/sp_x86_64.c index 123b725239..45ebdf815c 100644 --- a/wolfcrypt/src/sp_x86_64.c +++ b/wolfcrypt/src/sp_x86_64.c @@ -23916,6 +23916,15 @@ int sp_ecc_check_key_256(mp_int* pX, mp_int* pY, mp_int* privm, void* heap) } #endif + /* Quick check the lengs of public key ordinates and private key are in + * range. Proper check later. + */ + if ((err == MP_OKAY) && ((mp_count_bits(pX) > 256) || + (mp_count_bits(pY) > 256) || + ((privm != NULL) && (mp_count_bits(privm) > 256)))) { + err = ECC_OUT_OF_RANGE_E; + } + if (err == MP_OKAY) { #if (!defined(WOLFSSL_SP_SMALL) && !defined(WOLFSSL_SMALL_STACK)) || defined(WOLFSSL_SP_NO_MALLOC) priv = privd; @@ -30911,6 +30920,15 @@ int sp_ecc_check_key_384(mp_int* pX, mp_int* pY, mp_int* privm, void* heap) } #endif + /* Quick check the lengs of public key ordinates and private key are in + * range. Proper check later. + */ + if ((err == MP_OKAY) && ((mp_count_bits(pX) > 384) || + (mp_count_bits(pY) > 384) || + ((privm != NULL) && (mp_count_bits(privm) > 384)))) { + err = ECC_OUT_OF_RANGE_E; + } + if (err == MP_OKAY) { #if (!defined(WOLFSSL_SP_SMALL) && !defined(WOLFSSL_SMALL_STACK)) || defined(WOLFSSL_SP_NO_MALLOC) priv = privd;