From 41ab8cb6cb76dd7e8a94c8105b2ef12d1158fc3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 14 Nov 2019 11:59:09 +0100 Subject: [PATCH 01/23] Centralize everything to EccPoint_mult_safer() This will make easier to add future counter-measures in a single place. In practice this change means that: - compute_public_key() now uses projective coordinate randomisation, which it should as this is a protection against Template Attacks for example. - mult_safer() now checks that the result is not the point at infinity, which it can as the result is indeed never expected to be that --- tinycrypt/ecc.c | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index 92906fd76..c69d42278 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -951,6 +951,12 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, } EccPoint_mult(result, point, k2[!carry], initial_Z); + + if (EccPoint_isZero(result, curve)) { + r = 0; + goto clear_and_out; + } + r = 1; clear_and_out: @@ -966,25 +972,7 @@ uECC_word_t EccPoint_compute_public_key(uECC_word_t *result, uECC_word_t *private_key, uECC_Curve curve) { - - uECC_word_t tmp1[NUM_ECC_WORDS]; - uECC_word_t tmp2[NUM_ECC_WORDS]; - uECC_word_t *p2[2] = {tmp1, tmp2}; - uECC_word_t carry; - - if (curve != uECC_secp256r1()) - return 0; - - /* Regularize the bitcount for the private key so that attackers cannot - * use a side channel attack to learn the number of leading zeros. */ - carry = regularize_k(private_key, tmp1, tmp2); - - EccPoint_mult(result, curve->G, p2[!carry], 0); - - if (EccPoint_isZero(result, curve)) { - return 0; - } - return 1; + return EccPoint_mult_safer(result, curve->G, private_key, curve); } /* Converts an integer in uECC native format to big-endian bytes. */ From e714332563449c86ad140328c145ccf27120011e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 15 Nov 2019 10:47:45 +0100 Subject: [PATCH 02/23] Add pre and post-validation to mult_safer() Validating the input is always a good idea. Validating the output protects against some fault injections that would make the result invalid. Note: valid_point() implies that the point is not zero. Adding validation to mult_safer() makes it redundant in compute_shared_secret(). --- tinycrypt/ecc.c | 9 ++++++++- tinycrypt/ecc_dh.c | 11 ----------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index c69d42278..b48083222 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -936,6 +936,11 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, if (curve != uECC_secp256r1()) return 0; + /* Protects against invalid curves attacks */ + if (uECC_valid_point(point, curve) != 0 ) { + return 0; + } + /* Regularize the bitcount for the private key so that attackers cannot use a * side channel attack to learn the number of leading zeros. */ carry = regularize_k(scalar, tmp, s); @@ -952,7 +957,9 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, EccPoint_mult(result, point, k2[!carry], initial_Z); - if (EccPoint_isZero(result, curve)) { + /* Protect against fault injections that would make the resulting + * point not lie on the intended curve */ + if (uECC_valid_point(result, curve) != 0 ) { r = 0; goto clear_and_out; } diff --git a/tinycrypt/ecc_dh.c b/tinycrypt/ecc_dh.c index f9c0a5ed1..e2f5655d6 100644 --- a/tinycrypt/ecc_dh.c +++ b/tinycrypt/ecc_dh.c @@ -158,12 +158,6 @@ int uECC_shared_secret(const uint8_t *public_key, const uint8_t *private_key, wordcount_t num_bytes = curve->num_bytes; int r; - /* Protect against invalid curve attacks */ - if (uECC_valid_public_key(public_key, curve) != 0) { - r = 0; - goto clear_and_out; - } - /* Converting buffers to correct bit order: */ uECC_vli_bytesToNative(_private, private_key, @@ -176,13 +170,8 @@ int uECC_shared_secret(const uint8_t *public_key, const uint8_t *private_key, num_bytes); r = EccPoint_mult_safer(_public, _public, _private, curve); - if (r == 0) - goto clear_and_out; - uECC_vli_nativeToBytes(secret, num_bytes, _public); - r = !EccPoint_isZero(_public, curve); -clear_and_out: /* erasing temporary buffer used to store secret: */ mbedtls_platform_zeroize(_private, sizeof(_private)); From 1c6f7eae2d90bbe6490d36308c928751605b4556 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Nov 2019 09:18:29 +0100 Subject: [PATCH 03/23] Remove function pointers from curve structure They're not needed in practice, and removing them decreases the code size slightly and provides less opportunities for an attacker. --- include/tinycrypt/ecc.h | 16 ---------------- tinycrypt/ecc.c | 12 +++++++++--- tinycrypt/ecc_dsa.c | 2 +- 3 files changed, 10 insertions(+), 20 deletions(-) diff --git a/include/tinycrypt/ecc.h b/include/tinycrypt/ecc.h index 0d1d9ec98..7d57f0f43 100644 --- a/include/tinycrypt/ecc.h +++ b/include/tinycrypt/ecc.h @@ -131,10 +131,6 @@ struct uECC_Curve_t { uECC_word_t n[NUM_ECC_WORDS]; uECC_word_t G[NUM_ECC_WORDS * 2]; uECC_word_t b[NUM_ECC_WORDS]; - void (*double_jacobian)(uECC_word_t * X1, uECC_word_t * Y1, uECC_word_t * Z1, - uECC_Curve curve); - void (*x_side)(uECC_word_t *result, const uECC_word_t *x, uECC_Curve curve); - void (*mmod_fast)(uECC_word_t *result, uECC_word_t *product); }; /* @@ -147,15 +143,6 @@ struct uECC_Curve_t { void double_jacobian_default(uECC_word_t * X1, uECC_word_t * Y1, uECC_word_t * Z1, uECC_Curve curve); -/* - * @brief Computes x^3 + ax + b. result must not overlap x. - * @param result OUT -- x^3 + ax + b - * @param x IN -- value of x - * @param curve IN -- elliptic curve - */ -void x_side_default(uECC_word_t *result, const uECC_word_t *x, - uECC_Curve curve); - /* * @brief Computes result = product % curve_p * from http://www.nsa.gov/ia/_files/nist-routines.pdf @@ -201,9 +188,6 @@ static const struct uECC_Curve_t curve_secp256r1 = { BYTES_TO_WORDS_8(BC, 86, 98, 76, 55, BD, EB, B3), BYTES_TO_WORDS_8(E7, 93, 3A, AA, D8, 35, C6, 5A) }, - &double_jacobian_default, - &x_side_default, - &vli_mmod_fast_secp256r1 }; uECC_Curve uECC_secp256r1(void); diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index b48083222..7659e5481 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -622,7 +622,13 @@ void double_jacobian_default(uECC_word_t * X1, uECC_word_t * Y1, uECC_vli_set(Y1, t4); } -void x_side_default(uECC_word_t *result, +/* + * @brief Computes x^3 + ax + b. result must not overlap x. + * @param result OUT -- x^3 + ax + b + * @param x IN -- value of x + * @param curve IN -- elliptic curve + */ +static void x_side_default(uECC_word_t *result, const uECC_word_t *x, uECC_Curve curve) { @@ -775,7 +781,7 @@ static void XYcZ_initial_double(uECC_word_t * X1, uECC_word_t * Y1, uECC_vli_set(Y2, Y1); apply_z(X1, Y1, z); - curve->double_jacobian(X1, Y1, z, curve); + double_jacobian_default(X1, Y1, z, curve); apply_z(X2, Y2, z); } @@ -1050,7 +1056,7 @@ int uECC_valid_point(const uECC_word_t *point, uECC_Curve curve) } uECC_vli_modMult_fast(tmp1, point + num_words, point + num_words); - curve->x_side(tmp2, point, curve); /* tmp2 = x^3 + ax + b */ + x_side_default(tmp2, point, curve); /* tmp2 = x^3 + ax + b */ /* Make sure that y^2 == x^3 + ax + b */ if (uECC_vli_equal(tmp1, tmp2) != 0) diff --git a/tinycrypt/ecc_dsa.c b/tinycrypt/ecc_dsa.c index 6c171c3a9..a3b91b873 100644 --- a/tinycrypt/ecc_dsa.c +++ b/tinycrypt/ecc_dsa.c @@ -280,7 +280,7 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, for (i = num_bits - 2; i >= 0; --i) { uECC_word_t index; - curve->double_jacobian(rx, ry, z, curve); + double_jacobian_default(rx, ry, z, curve); index = (!!uECC_vli_testBit(u1, i)) | ((!!uECC_vli_testBit(u2, i)) << 1); point = points[index]; From 1765933ab201ecf769977ec753c78ca6aa530189 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Nov 2019 09:27:38 +0100 Subject: [PATCH 04/23] Remove num_words member from curve structure Saves code size, and makes the curve structure simpler. --- include/tinycrypt/ecc.h | 2 -- tinycrypt/ecc.c | 8 ++++---- tinycrypt/ecc_dh.c | 6 +++--- tinycrypt/ecc_dsa.c | 2 +- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/include/tinycrypt/ecc.h b/include/tinycrypt/ecc.h index 7d57f0f43..d7ad3188f 100644 --- a/include/tinycrypt/ecc.h +++ b/include/tinycrypt/ecc.h @@ -124,7 +124,6 @@ typedef uint64_t uECC_dword_t; struct uECC_Curve_t; typedef const struct uECC_Curve_t * uECC_Curve; struct uECC_Curve_t { - wordcount_t num_words; wordcount_t num_bytes; bitcount_t num_n_bits; uECC_word_t p[NUM_ECC_WORDS]; @@ -160,7 +159,6 @@ void vli_mmod_fast_secp256r1(unsigned int *result, unsigned int *product); /* definition of curve NIST p-256: */ static const struct uECC_Curve_t curve_secp256r1 = { - NUM_ECC_WORDS, NUM_ECC_BYTES, 256, /* num_n_bits */ { BYTES_TO_WORDS_8(FF, FF, FF, FF, FF, FF, FF, FF), diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index 7659e5481..9a28b18d4 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -581,7 +581,7 @@ void double_jacobian_default(uECC_word_t * X1, uECC_word_t * Y1, /* t1 = X, t2 = Y, t3 = Z */ uECC_word_t t4[NUM_ECC_WORDS]; uECC_word_t t5[NUM_ECC_WORDS]; - wordcount_t num_words = curve->num_words; + wordcount_t num_words = NUM_ECC_WORDS; if (uECC_vli_isZero(Z1)) { return; @@ -1042,7 +1042,7 @@ int uECC_valid_point(const uECC_word_t *point, uECC_Curve curve) { uECC_word_t tmp1[NUM_ECC_WORDS]; uECC_word_t tmp2[NUM_ECC_WORDS]; - wordcount_t num_words = curve->num_words; + wordcount_t num_words = NUM_ECC_WORDS; /* The point at infinity is invalid. */ if (EccPoint_isZero(point, curve)) { @@ -1072,7 +1072,7 @@ int uECC_valid_public_key(const uint8_t *public_key, uECC_Curve curve) uECC_vli_bytesToNative(_public, public_key, curve->num_bytes); uECC_vli_bytesToNative( - _public + curve->num_words, + _public + NUM_ECC_WORDS, public_key + curve->num_bytes, curve->num_bytes); @@ -1112,7 +1112,7 @@ int uECC_compute_public_key(const uint8_t *private_key, uint8_t *public_key, uECC_vli_nativeToBytes(public_key, curve->num_bytes, _public); uECC_vli_nativeToBytes( public_key + - curve->num_bytes, curve->num_bytes, _public + curve->num_words); + curve->num_bytes, curve->num_bytes, _public + NUM_ECC_WORDS); return 1; } #else diff --git a/tinycrypt/ecc_dh.c b/tinycrypt/ecc_dh.c index e2f5655d6..b3f8f71ae 100644 --- a/tinycrypt/ecc_dh.c +++ b/tinycrypt/ecc_dh.c @@ -96,7 +96,7 @@ int uECC_make_key_with_d(uint8_t *public_key, uint8_t *private_key, _public); uECC_vli_nativeToBytes(public_key + curve->num_bytes, curve->num_bytes, - _public + curve->num_words); + _public + NUM_ECC_WORDS); /* erasing temporary buffer used to store secret: */ mbedtls_platform_memset(_private, 0, NUM_ECC_BYTES); @@ -137,7 +137,7 @@ int uECC_make_key(uint8_t *public_key, uint8_t *private_key, uECC_Curve curve) _public); uECC_vli_nativeToBytes(public_key + curve->num_bytes, curve->num_bytes, - _public + curve->num_words); + _public + NUM_ECC_WORDS); /* erasing temporary buffer that stored secret: */ mbedtls_platform_memset(_private, 0, NUM_ECC_BYTES); @@ -154,7 +154,7 @@ int uECC_shared_secret(const uint8_t *public_key, const uint8_t *private_key, uECC_word_t _public[NUM_ECC_WORDS * 2]; uECC_word_t _private[NUM_ECC_WORDS]; - wordcount_t num_words = curve->num_words; + wordcount_t num_words = NUM_ECC_WORDS; wordcount_t num_bytes = curve->num_bytes; int r; diff --git a/tinycrypt/ecc_dsa.c b/tinycrypt/ecc_dsa.c index a3b91b873..f4af4b609 100644 --- a/tinycrypt/ecc_dsa.c +++ b/tinycrypt/ecc_dsa.c @@ -219,7 +219,7 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, uECC_word_t _public[NUM_ECC_WORDS * 2]; uECC_word_t r[NUM_ECC_WORDS], s[NUM_ECC_WORDS]; - wordcount_t num_words = curve->num_words; + wordcount_t num_words = NUM_ECC_WORDS; wordcount_t num_n_words = BITS_TO_WORDS(curve->num_n_bits); if (curve != uECC_secp256r1()) From 72c1764c002138cfa256e2759b798c3c2443e199 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Nov 2019 09:34:09 +0100 Subject: [PATCH 05/23] Remove num_bytes member from curve structure Reduces code size and size of the structure. --- include/tinycrypt/ecc.h | 2 -- tinycrypt/ecc.c | 13 +++++++------ tinycrypt/ecc_dh.c | 14 +++++++------- tinycrypt/ecc_dsa.c | 16 ++++++++-------- 4 files changed, 22 insertions(+), 23 deletions(-) diff --git a/include/tinycrypt/ecc.h b/include/tinycrypt/ecc.h index d7ad3188f..0cb47d471 100644 --- a/include/tinycrypt/ecc.h +++ b/include/tinycrypt/ecc.h @@ -124,7 +124,6 @@ typedef uint64_t uECC_dword_t; struct uECC_Curve_t; typedef const struct uECC_Curve_t * uECC_Curve; struct uECC_Curve_t { - wordcount_t num_bytes; bitcount_t num_n_bits; uECC_word_t p[NUM_ECC_WORDS]; uECC_word_t n[NUM_ECC_WORDS]; @@ -159,7 +158,6 @@ void vli_mmod_fast_secp256r1(unsigned int *result, unsigned int *product); /* definition of curve NIST p-256: */ static const struct uECC_Curve_t curve_secp256r1 = { - NUM_ECC_BYTES, 256, /* num_n_bits */ { BYTES_TO_WORDS_8(FF, FF, FF, FF, FF, FF, FF, FF), BYTES_TO_WORDS_8(FF, FF, FF, FF, 00, 00, 00, 00), diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index 9a28b18d4..cfdbc4bbb 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -93,7 +93,8 @@ int uECC_curve_private_key_size(uECC_Curve curve) int uECC_curve_public_key_size(uECC_Curve curve) { - return 2 * curve->num_bytes; + (void) curve; + return 2 * NUM_ECC_BYTES; } void uECC_vli_clear(uECC_word_t *vli) @@ -1070,11 +1071,11 @@ int uECC_valid_public_key(const uint8_t *public_key, uECC_Curve curve) uECC_word_t _public[NUM_ECC_WORDS * 2]; - uECC_vli_bytesToNative(_public, public_key, curve->num_bytes); + uECC_vli_bytesToNative(_public, public_key, NUM_ECC_BYTES); uECC_vli_bytesToNative( _public + NUM_ECC_WORDS, - public_key + curve->num_bytes, - curve->num_bytes); + public_key + NUM_ECC_BYTES, + NUM_ECC_BYTES); if (memcmp(_public, curve->G, NUM_ECC_WORDS * 2) == 0) { return -4; @@ -1109,10 +1110,10 @@ int uECC_compute_public_key(const uint8_t *private_key, uint8_t *public_key, return 0; } - uECC_vli_nativeToBytes(public_key, curve->num_bytes, _public); + uECC_vli_nativeToBytes(public_key, NUM_ECC_BYTES, _public); uECC_vli_nativeToBytes( public_key + - curve->num_bytes, curve->num_bytes, _public + NUM_ECC_WORDS); + NUM_ECC_BYTES, NUM_ECC_BYTES, _public + NUM_ECC_WORDS); return 1; } #else diff --git a/tinycrypt/ecc_dh.c b/tinycrypt/ecc_dh.c index b3f8f71ae..a1d7483a9 100644 --- a/tinycrypt/ecc_dh.c +++ b/tinycrypt/ecc_dh.c @@ -92,10 +92,10 @@ int uECC_make_key_with_d(uint8_t *public_key, uint8_t *private_key, BITS_TO_BYTES(curve->num_n_bits), _private); uECC_vli_nativeToBytes(public_key, - curve->num_bytes, + NUM_ECC_BYTES, _public); - uECC_vli_nativeToBytes(public_key + curve->num_bytes, - curve->num_bytes, + uECC_vli_nativeToBytes(public_key + NUM_ECC_BYTES, + NUM_ECC_BYTES, _public + NUM_ECC_WORDS); /* erasing temporary buffer used to store secret: */ @@ -133,10 +133,10 @@ int uECC_make_key(uint8_t *public_key, uint8_t *private_key, uECC_Curve curve) BITS_TO_BYTES(curve->num_n_bits), _private); uECC_vli_nativeToBytes(public_key, - curve->num_bytes, + NUM_ECC_BYTES, _public); - uECC_vli_nativeToBytes(public_key + curve->num_bytes, - curve->num_bytes, + uECC_vli_nativeToBytes(public_key + NUM_ECC_BYTES, + NUM_ECC_BYTES, _public + NUM_ECC_WORDS); /* erasing temporary buffer that stored secret: */ @@ -155,7 +155,7 @@ int uECC_shared_secret(const uint8_t *public_key, const uint8_t *private_key, uECC_word_t _public[NUM_ECC_WORDS * 2]; uECC_word_t _private[NUM_ECC_WORDS]; wordcount_t num_words = NUM_ECC_WORDS; - wordcount_t num_bytes = curve->num_bytes; + wordcount_t num_bytes = NUM_ECC_BYTES; int r; /* Converting buffers to correct bit order: */ diff --git a/tinycrypt/ecc_dsa.c b/tinycrypt/ecc_dsa.c index f4af4b609..5c4ca1578 100644 --- a/tinycrypt/ecc_dsa.c +++ b/tinycrypt/ecc_dsa.c @@ -147,7 +147,7 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, uECC_vli_modInv(k, k, curve->n); /* k = 1 / k' */ uECC_vli_modMult(k, k, tmp, curve->n); /* k = 1 / k */ - uECC_vli_nativeToBytes(signature, curve->num_bytes, p); /* store r */ + uECC_vli_nativeToBytes(signature, NUM_ECC_BYTES, p); /* store r */ /* tmp = d: */ uECC_vli_bytesToNative(tmp, private_key, BITS_TO_BYTES(curve->num_n_bits)); @@ -159,11 +159,11 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, bits2int(tmp, message_hash, hash_size, curve); uECC_vli_modAdd(s, tmp, s, curve->n); /* s = e + r*d */ uECC_vli_modMult(s, s, k, curve->n); /* s = (e + r*d) / k */ - if (uECC_vli_numBits(s) > (bitcount_t)curve->num_bytes * 8) { + if (uECC_vli_numBits(s) > (bitcount_t)NUM_ECC_BYTES * 8) { return 0; } - uECC_vli_nativeToBytes(signature + curve->num_bytes, curve->num_bytes, s); + uECC_vli_nativeToBytes(signature + NUM_ECC_BYTES, NUM_ECC_BYTES, s); return 1; } @@ -229,11 +229,11 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, r[num_n_words - 1] = 0; s[num_n_words - 1] = 0; - uECC_vli_bytesToNative(_public, public_key, curve->num_bytes); - uECC_vli_bytesToNative(_public + num_words, public_key + curve->num_bytes, - curve->num_bytes); - uECC_vli_bytesToNative(r, signature, curve->num_bytes); - uECC_vli_bytesToNative(s, signature + curve->num_bytes, curve->num_bytes); + uECC_vli_bytesToNative(_public, public_key, NUM_ECC_BYTES); + uECC_vli_bytesToNative(_public + num_words, public_key + NUM_ECC_BYTES, + NUM_ECC_BYTES); + uECC_vli_bytesToNative(r, signature, NUM_ECC_BYTES); + uECC_vli_bytesToNative(s, signature + NUM_ECC_BYTES, NUM_ECC_BYTES); /* r, s must not be 0. */ if (uECC_vli_isZero(r) || uECC_vli_isZero(s)) { From 30833f2a07ae807f0a5718865f01af819a2f9655 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Nov 2019 09:46:52 +0100 Subject: [PATCH 06/23] Remove num_n_bits member from curve structure --- include/tinycrypt/ecc.h | 3 +-- tinycrypt/ecc.c | 5 +++-- tinycrypt/ecc_dh.c | 6 +++--- tinycrypt/ecc_dsa.c | 14 +++++++------- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/include/tinycrypt/ecc.h b/include/tinycrypt/ecc.h index 0cb47d471..5a4184503 100644 --- a/include/tinycrypt/ecc.h +++ b/include/tinycrypt/ecc.h @@ -124,7 +124,6 @@ typedef uint64_t uECC_dword_t; struct uECC_Curve_t; typedef const struct uECC_Curve_t * uECC_Curve; struct uECC_Curve_t { - bitcount_t num_n_bits; uECC_word_t p[NUM_ECC_WORDS]; uECC_word_t n[NUM_ECC_WORDS]; uECC_word_t G[NUM_ECC_WORDS * 2]; @@ -158,7 +157,7 @@ void vli_mmod_fast_secp256r1(unsigned int *result, unsigned int *product); /* definition of curve NIST p-256: */ static const struct uECC_Curve_t curve_secp256r1 = { - 256, /* num_n_bits */ { + { BYTES_TO_WORDS_8(FF, FF, FF, FF, FF, FF, FF, FF), BYTES_TO_WORDS_8(FF, FF, FF, FF, 00, 00, 00, 00), BYTES_TO_WORDS_8(00, 00, 00, 00, 00, 00, 00, 00), diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index cfdbc4bbb..c43ee725f 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -88,7 +88,8 @@ uECC_RNG_Function uECC_get_rng(void) int uECC_curve_private_key_size(uECC_Curve curve) { - return BITS_TO_BYTES(curve->num_n_bits); + (void) curve; + return BITS_TO_BYTES(NUM_ECC_BITS); } int uECC_curve_public_key_size(uECC_Curve curve) @@ -1094,7 +1095,7 @@ int uECC_compute_public_key(const uint8_t *private_key, uint8_t *public_key, uECC_vli_bytesToNative( _private, private_key, - BITS_TO_BYTES(curve->num_n_bits)); + BITS_TO_BYTES(NUM_ECC_BITS)); /* Make sure the private key is in the range [1, n-1]. */ if (uECC_vli_isZero(_private)) { diff --git a/tinycrypt/ecc_dh.c b/tinycrypt/ecc_dh.c index a1d7483a9..5c5bc1343 100644 --- a/tinycrypt/ecc_dh.c +++ b/tinycrypt/ecc_dh.c @@ -89,7 +89,7 @@ int uECC_make_key_with_d(uint8_t *public_key, uint8_t *private_key, /* Converting buffers to correct bit order: */ uECC_vli_nativeToBytes(private_key, - BITS_TO_BYTES(curve->num_n_bits), + BITS_TO_BYTES(NUM_ECC_BITS), _private); uECC_vli_nativeToBytes(public_key, NUM_ECC_BYTES, @@ -130,7 +130,7 @@ int uECC_make_key(uint8_t *public_key, uint8_t *private_key, uECC_Curve curve) /* Converting buffers to correct bit order: */ uECC_vli_nativeToBytes(private_key, - BITS_TO_BYTES(curve->num_n_bits), + BITS_TO_BYTES(NUM_ECC_BITS), _private); uECC_vli_nativeToBytes(public_key, NUM_ECC_BYTES, @@ -161,7 +161,7 @@ int uECC_shared_secret(const uint8_t *public_key, const uint8_t *private_key, /* Converting buffers to correct bit order: */ uECC_vli_bytesToNative(_private, private_key, - BITS_TO_BYTES(curve->num_n_bits)); + BITS_TO_BYTES(NUM_ECC_BITS)); uECC_vli_bytesToNative(_public, public_key, num_bytes); diff --git a/tinycrypt/ecc_dsa.c b/tinycrypt/ecc_dsa.c index 5c4ca1578..591c8808e 100644 --- a/tinycrypt/ecc_dsa.c +++ b/tinycrypt/ecc_dsa.c @@ -78,8 +78,8 @@ static uECC_RNG_Function g_rng_function = 0; static void bits2int(uECC_word_t *native, const uint8_t *bits, unsigned bits_size, uECC_Curve curve) { - unsigned num_n_bytes = BITS_TO_BYTES(curve->num_n_bits); - unsigned num_n_words = BITS_TO_WORDS(curve->num_n_bits); + unsigned num_n_bytes = BITS_TO_BYTES(NUM_ECC_BITS); + unsigned num_n_words = BITS_TO_WORDS(NUM_ECC_BITS); int shift; uECC_word_t carry; uECC_word_t *ptr; @@ -90,10 +90,10 @@ static void bits2int(uECC_word_t *native, const uint8_t *bits, uECC_vli_clear(native); uECC_vli_bytesToNative(native, bits, bits_size); - if (bits_size * 8 <= (unsigned)curve->num_n_bits) { + if (bits_size * 8 <= (unsigned)NUM_ECC_BITS) { return; } - shift = bits_size * 8 - curve->num_n_bits; + shift = bits_size * 8 - NUM_ECC_BITS; carry = 0; ptr = native + num_n_words; while (ptr-- > native) { @@ -116,7 +116,7 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, uECC_word_t tmp[NUM_ECC_WORDS]; uECC_word_t s[NUM_ECC_WORDS]; uECC_word_t p[NUM_ECC_WORDS * 2]; - wordcount_t num_n_words = BITS_TO_WORDS(curve->num_n_bits); + wordcount_t num_n_words = BITS_TO_WORDS(NUM_ECC_BITS); int r; @@ -150,7 +150,7 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, uECC_vli_nativeToBytes(signature, NUM_ECC_BYTES, p); /* store r */ /* tmp = d: */ - uECC_vli_bytesToNative(tmp, private_key, BITS_TO_BYTES(curve->num_n_bits)); + uECC_vli_bytesToNative(tmp, private_key, BITS_TO_BYTES(NUM_ECC_BITS)); s[num_n_words - 1] = 0; uECC_vli_set(s, p); @@ -220,7 +220,7 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, uECC_word_t _public[NUM_ECC_WORDS * 2]; uECC_word_t r[NUM_ECC_WORDS], s[NUM_ECC_WORDS]; wordcount_t num_words = NUM_ECC_WORDS; - wordcount_t num_n_words = BITS_TO_WORDS(curve->num_n_bits); + wordcount_t num_n_words = BITS_TO_WORDS(NUM_ECC_BITS); if (curve != uECC_secp256r1()) return 0; From 4d8777cbb6e640d258c5bd812161f4602544f391 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Nov 2019 10:02:58 +0100 Subject: [PATCH 07/23] Move p from curve structure to its own constant This removes an indirection, which both makes the code smaller and decreases the number of glitching opportunities for an attacker. --- include/tinycrypt/ecc.h | 8 +--- tinycrypt/ecc.c | 90 ++++++++++++++++++++++++----------------- tinycrypt/ecc_dsa.c | 8 ++-- 3 files changed, 58 insertions(+), 48 deletions(-) diff --git a/include/tinycrypt/ecc.h b/include/tinycrypt/ecc.h index 5a4184503..75e6e9213 100644 --- a/include/tinycrypt/ecc.h +++ b/include/tinycrypt/ecc.h @@ -124,7 +124,6 @@ typedef uint64_t uECC_dword_t; struct uECC_Curve_t; typedef const struct uECC_Curve_t * uECC_Curve; struct uECC_Curve_t { - uECC_word_t p[NUM_ECC_WORDS]; uECC_word_t n[NUM_ECC_WORDS]; uECC_word_t G[NUM_ECC_WORDS * 2]; uECC_word_t b[NUM_ECC_WORDS]; @@ -155,14 +154,11 @@ void vli_mmod_fast_secp256r1(unsigned int *result, unsigned int *product); ((num_bits + ((uECC_WORD_SIZE * 8) - 1)) / (uECC_WORD_SIZE * 8)) #define BITS_TO_BYTES(num_bits) ((num_bits + 7) / 8) +extern const uECC_word_t curve_p[NUM_ECC_WORDS]; + /* definition of curve NIST p-256: */ static const struct uECC_Curve_t curve_secp256r1 = { { - BYTES_TO_WORDS_8(FF, FF, FF, FF, FF, FF, FF, FF), - BYTES_TO_WORDS_8(FF, FF, FF, FF, 00, 00, 00, 00), - BYTES_TO_WORDS_8(00, 00, 00, 00, 00, 00, 00, 00), - BYTES_TO_WORDS_8(01, 00, 00, 00, FF, FF, FF, FF) - }, { BYTES_TO_WORDS_8(51, 25, 63, FC, C2, CA, B9, F3), BYTES_TO_WORDS_8(84, 9E, 17, A7, AD, FA, E6, BC), BYTES_TO_WORDS_8(FF, FF, FF, FF, FF, FF, FF, FF), diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index c43ee725f..6f96bd939 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -68,6 +68,14 @@ #include "mbedtls/platform_util.h" #include +/* Parameters for curve NIST P-256 aka secp256r1 */ +const uECC_word_t curve_p[NUM_ECC_WORDS] = { + BYTES_TO_WORDS_8(FF, FF, FF, FF, FF, FF, FF, FF), + BYTES_TO_WORDS_8(FF, FF, FF, FF, 00, 00, 00, 00), + BYTES_TO_WORDS_8(00, 00, 00, 00, 00, 00, 00, 00), + BYTES_TO_WORDS_8(01, 00, 00, 00, FF, FF, FF, FF) +}; + /* IMPORTANT: Make sure a cryptographically-secure PRNG is set and the platform * has access to enough entropy in order to feed the PRNG regularly. */ #if default_RNG_defined @@ -585,6 +593,8 @@ void double_jacobian_default(uECC_word_t * X1, uECC_word_t * Y1, uECC_word_t t5[NUM_ECC_WORDS]; wordcount_t num_words = NUM_ECC_WORDS; + (void) curve; + if (uECC_vli_isZero(Z1)) { return; } @@ -595,15 +605,15 @@ void double_jacobian_default(uECC_word_t * X1, uECC_word_t * Y1, uECC_vli_modMult_fast(Y1, Y1, Z1); /* t2 = y1*z1 = z3 */ uECC_vli_modMult_fast(Z1, Z1, Z1); /* t3 = z1^2 */ - uECC_vli_modAdd(X1, X1, Z1, curve->p); /* t1 = x1 + z1^2 */ - uECC_vli_modAdd(Z1, Z1, Z1, curve->p); /* t3 = 2*z1^2 */ - uECC_vli_modSub(Z1, X1, Z1, curve->p); /* t3 = x1 - z1^2 */ + uECC_vli_modAdd(X1, X1, Z1, curve_p); /* t1 = x1 + z1^2 */ + uECC_vli_modAdd(Z1, Z1, Z1, curve_p); /* t3 = 2*z1^2 */ + uECC_vli_modSub(Z1, X1, Z1, curve_p); /* t3 = x1 - z1^2 */ uECC_vli_modMult_fast(X1, X1, Z1); /* t1 = x1^2 - z1^4 */ - uECC_vli_modAdd(Z1, X1, X1, curve->p); /* t3 = 2*(x1^2 - z1^4) */ - uECC_vli_modAdd(X1, X1, Z1, curve->p); /* t1 = 3*(x1^2 - z1^4) */ + uECC_vli_modAdd(Z1, X1, X1, curve_p); /* t3 = 2*(x1^2 - z1^4) */ + uECC_vli_modAdd(X1, X1, Z1, curve_p); /* t1 = 3*(x1^2 - z1^4) */ if (uECC_vli_testBit(X1, 0)) { - uECC_word_t l_carry = uECC_vli_add(X1, X1, curve->p); + uECC_word_t l_carry = uECC_vli_add(X1, X1, curve_p); uECC_vli_rshift1(X1); X1[num_words - 1] |= l_carry << (uECC_WORD_BITS - 1); } else { @@ -612,12 +622,12 @@ void double_jacobian_default(uECC_word_t * X1, uECC_word_t * Y1, /* t1 = 3/2*(x1^2 - z1^4) = B */ uECC_vli_modMult_fast(Z1, X1, X1); /* t3 = B^2 */ - uECC_vli_modSub(Z1, Z1, t5, curve->p); /* t3 = B^2 - A */ - uECC_vli_modSub(Z1, Z1, t5, curve->p); /* t3 = B^2 - 2A = x3 */ - uECC_vli_modSub(t5, t5, Z1, curve->p); /* t5 = A - x3 */ + uECC_vli_modSub(Z1, Z1, t5, curve_p); /* t3 = B^2 - A */ + uECC_vli_modSub(Z1, Z1, t5, curve_p); /* t3 = B^2 - 2A = x3 */ + uECC_vli_modSub(t5, t5, Z1, curve_p); /* t5 = A - x3 */ uECC_vli_modMult_fast(X1, X1, t5); /* t1 = B * (A - x3) */ /* t4 = B * (A - x3) - y1^4 = y3: */ - uECC_vli_modSub(t4, X1, t4, curve->p); + uECC_vli_modSub(t4, X1, t4, curve_p); uECC_vli_set(X1, Z1); uECC_vli_set(Z1, Y1); @@ -637,10 +647,10 @@ static void x_side_default(uECC_word_t *result, uECC_word_t _3[NUM_ECC_WORDS] = {3}; /* -a = 3 */ uECC_vli_modMult_fast(result, x, x); /* r = x^2 */ - uECC_vli_modSub(result, result, _3, curve->p); /* r = x^2 - 3 */ + uECC_vli_modSub(result, result, _3, curve_p); /* r = x^2 - 3 */ uECC_vli_modMult_fast(result, result, x); /* r = x^3 - 3x */ /* r = x^3 - 3x + b: */ - uECC_vli_modAdd(result, result, curve->b, curve->p); + uECC_vli_modAdd(result, result, curve->b, curve_p); } uECC_Curve uECC_secp256r1(void) @@ -738,13 +748,13 @@ void vli_mmod_fast_secp256r1(unsigned int *result, unsigned int*product) if (carry < 0) { do { - carry += uECC_vli_add(result, result, curve_secp256r1.p); + carry += uECC_vli_add(result, result, curve_p); } while (carry < 0); } else { while (carry || - uECC_vli_cmp_unsafe(curve_secp256r1.p, result) != 1) { - carry -= uECC_vli_sub(result, result, curve_secp256r1.p); + uECC_vli_cmp_unsafe(curve_p, result) != 1) { + carry -= uECC_vli_sub(result, result, curve_p); } } } @@ -795,20 +805,22 @@ static void XYcZ_add_rnd(uECC_word_t * X1, uECC_word_t * Y1, uECC_word_t t5[NUM_ECC_WORDS]; const uECC_Curve curve = &curve_secp256r1; - uECC_vli_modSub(t5, X2, X1, curve->p); /* t5 = x2 - x1 */ + (void) curve; + + uECC_vli_modSub(t5, X2, X1, curve_p); /* t5 = x2 - x1 */ uECC_vli_modMult_rnd(t5, t5, t5, s); /* t5 = (x2 - x1)^2 = A */ uECC_vli_modMult_rnd(X1, X1, t5, s); /* t1 = x1*A = B */ uECC_vli_modMult_rnd(X2, X2, t5, s); /* t3 = x2*A = C */ - uECC_vli_modSub(Y2, Y2, Y1, curve->p); /* t4 = y2 - y1 */ + uECC_vli_modSub(Y2, Y2, Y1, curve_p); /* t4 = y2 - y1 */ uECC_vli_modMult_rnd(t5, Y2, Y2, s); /* t5 = (y2 - y1)^2 = D */ - uECC_vli_modSub(t5, t5, X1, curve->p); /* t5 = D - B */ - uECC_vli_modSub(t5, t5, X2, curve->p); /* t5 = D - B - C = x3 */ - uECC_vli_modSub(X2, X2, X1, curve->p); /* t3 = C - B */ + uECC_vli_modSub(t5, t5, X1, curve_p); /* t5 = D - B */ + uECC_vli_modSub(t5, t5, X2, curve_p); /* t5 = D - B - C = x3 */ + uECC_vli_modSub(X2, X2, X1, curve_p); /* t3 = C - B */ uECC_vli_modMult_rnd(Y1, Y1, X2, s); /* t2 = y1*(C - B) */ - uECC_vli_modSub(X2, X1, t5, curve->p); /* t3 = B - x3 */ + uECC_vli_modSub(X2, X1, t5, curve_p); /* t3 = B - x3 */ uECC_vli_modMult_rnd(Y2, Y2, X2, s); /* t4 = (y2 - y1)*(B - x3) */ - uECC_vli_modSub(Y2, Y2, Y1, curve->p); /* t4 = y3 */ + uECC_vli_modSub(Y2, Y2, Y1, curve_p); /* t4 = y3 */ uECC_vli_set(X2, t5); } @@ -835,30 +847,32 @@ static void XYcZ_addC_rnd(uECC_word_t * X1, uECC_word_t * Y1, uECC_word_t t7[NUM_ECC_WORDS]; const uECC_Curve curve = &curve_secp256r1; - uECC_vli_modSub(t5, X2, X1, curve->p); /* t5 = x2 - x1 */ + (void) curve; + + uECC_vli_modSub(t5, X2, X1, curve_p); /* t5 = x2 - x1 */ uECC_vli_modMult_rnd(t5, t5, t5, s); /* t5 = (x2 - x1)^2 = A */ uECC_vli_modMult_rnd(X1, X1, t5, s); /* t1 = x1*A = B */ uECC_vli_modMult_rnd(X2, X2, t5, s); /* t3 = x2*A = C */ - uECC_vli_modAdd(t5, Y2, Y1, curve->p); /* t5 = y2 + y1 */ - uECC_vli_modSub(Y2, Y2, Y1, curve->p); /* t4 = y2 - y1 */ + uECC_vli_modAdd(t5, Y2, Y1, curve_p); /* t5 = y2 + y1 */ + uECC_vli_modSub(Y2, Y2, Y1, curve_p); /* t4 = y2 - y1 */ - uECC_vli_modSub(t6, X2, X1, curve->p); /* t6 = C - B */ + uECC_vli_modSub(t6, X2, X1, curve_p); /* t6 = C - B */ uECC_vli_modMult_rnd(Y1, Y1, t6, s); /* t2 = y1 * (C - B) = E */ - uECC_vli_modAdd(t6, X1, X2, curve->p); /* t6 = B + C */ + uECC_vli_modAdd(t6, X1, X2, curve_p); /* t6 = B + C */ uECC_vli_modMult_rnd(X2, Y2, Y2, s); /* t3 = (y2 - y1)^2 = D */ - uECC_vli_modSub(X2, X2, t6, curve->p); /* t3 = D - (B + C) = x3 */ + uECC_vli_modSub(X2, X2, t6, curve_p); /* t3 = D - (B + C) = x3 */ - uECC_vli_modSub(t7, X1, X2, curve->p); /* t7 = B - x3 */ + uECC_vli_modSub(t7, X1, X2, curve_p); /* t7 = B - x3 */ uECC_vli_modMult_rnd(Y2, Y2, t7, s); /* t4 = (y2 - y1)*(B - x3) */ /* t4 = (y2 - y1)*(B - x3) - E = y3: */ - uECC_vli_modSub(Y2, Y2, Y1, curve->p); + uECC_vli_modSub(Y2, Y2, Y1, curve_p); uECC_vli_modMult_rnd(t7, t5, t5, s); /* t7 = (y2 + y1)^2 = F */ - uECC_vli_modSub(t7, t7, t6, curve->p); /* t7 = F - (B + C) = x3' */ - uECC_vli_modSub(t6, t7, X1, curve->p); /* t6 = x3' - B */ + uECC_vli_modSub(t7, t7, t6, curve_p); /* t7 = F - (B + C) = x3' */ + uECC_vli_modSub(t6, t7, X1, curve_p); /* t6 = x3' - B */ uECC_vli_modMult_rnd(t6, t6, t5, s); /* t6 = (y2+y1)*(x3' - B) */ /* t2 = (y2+y1)*(x3' - B) - E = y3': */ - uECC_vli_modSub(Y1, t6, Y1, curve->p); + uECC_vli_modSub(Y1, t6, Y1, curve_p); uECC_vli_set(X1, t7); } @@ -896,10 +910,10 @@ static void EccPoint_mult(uECC_word_t * result, const uECC_word_t * point, XYcZ_addC_rnd(Rx[1 - nb], Ry[1 - nb], Rx[nb], Ry[nb], ws); /* Find final 1/Z value. */ - uECC_vli_modSub(z, Rx[1], Rx[0], curve->p); /* X1 - X0 */ + uECC_vli_modSub(z, Rx[1], Rx[0], curve_p); /* X1 - X0 */ uECC_vli_modMult_fast(z, z, Ry[1 - nb]); /* Yb * (X1 - X0) */ uECC_vli_modMult_fast(z, z, point); /* xP * Yb * (X1 - X0) */ - uECC_vli_modInv(z, z, curve->p); /* 1 / (xP * Yb * (X1 - X0))*/ + uECC_vli_modInv(z, z, curve_p); /* 1 / (xP * Yb * (X1 - X0))*/ /* yP / (xP * Yb * (X1 - X0)) */ uECC_vli_modMult_fast(z, z, point + num_words); /* Xb * yP / (xP * Yb * (X1 - X0)) */ @@ -956,7 +970,7 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, /* If an RNG function was specified, get a random initial Z value to * protect against side-channel attacks such as Template SPA */ if (g_rng_function) { - if (!uECC_generate_random_int(k2[carry], curve->p, num_words)) { + if (!uECC_generate_random_int(k2[carry], curve_p, num_words)) { r = 0; goto clear_and_out; } @@ -1052,8 +1066,8 @@ int uECC_valid_point(const uECC_word_t *point, uECC_Curve curve) } /* x and y must be smaller than p. */ - if (uECC_vli_cmp_unsafe(curve->p, point) != 1 || - uECC_vli_cmp_unsafe(curve->p, point + num_words) != 1) { + if (uECC_vli_cmp_unsafe(curve_p, point) != 1 || + uECC_vli_cmp_unsafe(curve_p, point + num_words) != 1) { return -2; } diff --git a/tinycrypt/ecc_dsa.c b/tinycrypt/ecc_dsa.c index 591c8808e..fc2a3fe3e 100644 --- a/tinycrypt/ecc_dsa.c +++ b/tinycrypt/ecc_dsa.c @@ -258,9 +258,9 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, uECC_vli_set(sum + num_words, _public + num_words); uECC_vli_set(tx, curve->G); uECC_vli_set(ty, curve->G + num_words); - uECC_vli_modSub(z, sum, tx, curve->p); /* z = x2 - x1 */ + uECC_vli_modSub(z, sum, tx, curve_p); /* z = x2 - x1 */ XYcZ_add(tx, ty, sum, sum + num_words, curve); - uECC_vli_modInv(z, z, curve->p); /* z = 1/z */ + uECC_vli_modInv(z, z, curve_p); /* z = 1/z */ apply_z(sum, sum + num_words, z); /* Use Shamir's trick to calculate u1*G + u2*Q */ @@ -288,13 +288,13 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, uECC_vli_set(tx, point); uECC_vli_set(ty, point + num_words); apply_z(tx, ty, z); - uECC_vli_modSub(tz, rx, tx, curve->p); /* Z = x2 - x1 */ + uECC_vli_modSub(tz, rx, tx, curve_p); /* Z = x2 - x1 */ XYcZ_add(tx, ty, rx, ry, curve); uECC_vli_modMult_fast(z, z, tz); } } - uECC_vli_modInv(z, z, curve->p); /* Z = 1/Z */ + uECC_vli_modInv(z, z, curve_p); /* Z = 1/Z */ apply_z(rx, ry, z); /* v = x1 (mod n) */ From 356d8594d7dbfbbf8ee564c3fcb5be5ef9b7399f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Nov 2019 10:23:05 +0100 Subject: [PATCH 08/23] Move n from struct curve to its own constant --- include/tinycrypt/ecc.h | 7 +------ tinycrypt/ecc.c | 13 +++++++++---- tinycrypt/ecc_dh.c | 2 +- tinycrypt/ecc_dsa.c | 38 ++++++++++++++++++++------------------ 4 files changed, 31 insertions(+), 29 deletions(-) diff --git a/include/tinycrypt/ecc.h b/include/tinycrypt/ecc.h index 75e6e9213..dccfdf423 100644 --- a/include/tinycrypt/ecc.h +++ b/include/tinycrypt/ecc.h @@ -124,7 +124,6 @@ typedef uint64_t uECC_dword_t; struct uECC_Curve_t; typedef const struct uECC_Curve_t * uECC_Curve; struct uECC_Curve_t { - uECC_word_t n[NUM_ECC_WORDS]; uECC_word_t G[NUM_ECC_WORDS * 2]; uECC_word_t b[NUM_ECC_WORDS]; }; @@ -155,15 +154,11 @@ void vli_mmod_fast_secp256r1(unsigned int *result, unsigned int *product); #define BITS_TO_BYTES(num_bits) ((num_bits + 7) / 8) extern const uECC_word_t curve_p[NUM_ECC_WORDS]; +extern const uECC_word_t curve_n[NUM_ECC_WORDS]; /* definition of curve NIST p-256: */ static const struct uECC_Curve_t curve_secp256r1 = { { - BYTES_TO_WORDS_8(51, 25, 63, FC, C2, CA, B9, F3), - BYTES_TO_WORDS_8(84, 9E, 17, A7, AD, FA, E6, BC), - BYTES_TO_WORDS_8(FF, FF, FF, FF, FF, FF, FF, FF), - BYTES_TO_WORDS_8(00, 00, 00, 00, FF, FF, FF, FF) - }, { BYTES_TO_WORDS_8(96, C2, 98, D8, 45, 39, A1, F4), BYTES_TO_WORDS_8(A0, 33, EB, 2D, 81, 7D, 03, 77), BYTES_TO_WORDS_8(F2, 40, A4, 63, E5, E6, BC, F8), diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index 6f96bd939..daa9698a3 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -75,6 +75,12 @@ const uECC_word_t curve_p[NUM_ECC_WORDS] = { BYTES_TO_WORDS_8(00, 00, 00, 00, 00, 00, 00, 00), BYTES_TO_WORDS_8(01, 00, 00, 00, FF, FF, FF, FF) }; +const uECC_word_t curve_n[NUM_ECC_WORDS] = { + BYTES_TO_WORDS_8(51, 25, 63, FC, C2, CA, B9, F3), + BYTES_TO_WORDS_8(84, 9E, 17, A7, AD, FA, E6, BC), + BYTES_TO_WORDS_8(FF, FF, FF, FF, FF, FF, FF, FF), + BYTES_TO_WORDS_8(00, 00, 00, 00, FF, FF, FF, FF) +}; /* IMPORTANT: Make sure a cryptographically-secure PRNG is set and the platform * has access to enough entropy in order to feed the PRNG regularly. */ @@ -933,13 +939,12 @@ static uECC_word_t regularize_k(const uECC_word_t * const k, uECC_word_t *k0, wordcount_t num_n_words = NUM_ECC_WORDS; bitcount_t num_n_bits = NUM_ECC_BITS; - const uECC_Curve curve = uECC_secp256r1(); - uECC_word_t carry = uECC_vli_add(k0, k, curve->n) || + uECC_word_t carry = uECC_vli_add(k0, k, curve_n) || (num_n_bits < ((bitcount_t)num_n_words * uECC_WORD_SIZE * 8) && uECC_vli_testBit(k0, num_n_bits)); - uECC_vli_add(k1, k0, curve->n); + uECC_vli_add(k1, k0, curve_n); return carry; } @@ -1116,7 +1121,7 @@ int uECC_compute_public_key(const uint8_t *private_key, uint8_t *public_key, return 0; } - if (uECC_vli_cmp(curve->n, _private) != 1) { + if (uECC_vli_cmp(curve_n, _private) != 1) { return 0; } diff --git a/tinycrypt/ecc_dh.c b/tinycrypt/ecc_dh.c index 5c5bc1343..fc429fe47 100644 --- a/tinycrypt/ecc_dh.c +++ b/tinycrypt/ecc_dh.c @@ -123,7 +123,7 @@ int uECC_make_key(uint8_t *public_key, uint8_t *private_key, uECC_Curve curve) } /* computing modular reduction of _random (see FIPS 186.4 B.4.1): */ - uECC_vli_mmod(_private, _random, curve->n); + uECC_vli_mmod(_private, _random, curve_n); /* Computing public-key from private: */ if (EccPoint_compute_public_key(_public, _private, curve)) { diff --git a/tinycrypt/ecc_dsa.c b/tinycrypt/ecc_dsa.c index fc2a3fe3e..c22ebd032 100644 --- a/tinycrypt/ecc_dsa.c +++ b/tinycrypt/ecc_dsa.c @@ -84,6 +84,8 @@ static void bits2int(uECC_word_t *native, const uint8_t *bits, uECC_word_t carry; uECC_word_t *ptr; + (void) curve; + if (bits_size > num_n_bytes) { bits_size = num_n_bytes; } @@ -103,8 +105,8 @@ static void bits2int(uECC_word_t *native, const uint8_t *bits, } /* Reduce mod curve_n */ - if (uECC_vli_cmp_unsafe(curve->n, native) != 1) { - uECC_vli_sub(native, native, curve->n); + if (uECC_vli_cmp_unsafe(curve_n, native) != 1) { + uECC_vli_sub(native, native, curve_n); } } @@ -122,7 +124,7 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, /* Make sure 0 < k < curve_n */ if (uECC_vli_isZero(k) || - uECC_vli_cmp(curve->n, k) != 1) { + uECC_vli_cmp(curve_n, k) != 1) { return 0; } @@ -137,15 +139,15 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, uECC_vli_clear(tmp); tmp[0] = 1; } - else if (!uECC_generate_random_int(tmp, curve->n, num_n_words)) { + else if (!uECC_generate_random_int(tmp, curve_n, num_n_words)) { return 0; } /* Prevent side channel analysis of uECC_vli_modInv() to determine bits of k / the private key by premultiplying by a random number */ - uECC_vli_modMult(k, k, tmp, curve->n); /* k' = rand * k */ - uECC_vli_modInv(k, k, curve->n); /* k = 1 / k' */ - uECC_vli_modMult(k, k, tmp, curve->n); /* k = 1 / k */ + uECC_vli_modMult(k, k, tmp, curve_n); /* k' = rand * k */ + uECC_vli_modInv(k, k, curve_n); /* k = 1 / k' */ + uECC_vli_modMult(k, k, tmp, curve_n); /* k = 1 / k */ uECC_vli_nativeToBytes(signature, NUM_ECC_BYTES, p); /* store r */ @@ -154,11 +156,11 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, s[num_n_words - 1] = 0; uECC_vli_set(s, p); - uECC_vli_modMult(s, tmp, s, curve->n); /* s = r*d */ + uECC_vli_modMult(s, tmp, s, curve_n); /* s = r*d */ bits2int(tmp, message_hash, hash_size, curve); - uECC_vli_modAdd(s, tmp, s, curve->n); /* s = e + r*d */ - uECC_vli_modMult(s, s, k, curve->n); /* s = (e + r*d) / k */ + uECC_vli_modAdd(s, tmp, s, curve_n); /* s = e + r*d */ + uECC_vli_modMult(s, s, k, curve_n); /* s = (e + r*d) / k */ if (uECC_vli_numBits(s) > (bitcount_t)NUM_ECC_BYTES * 8) { return 0; } @@ -183,7 +185,7 @@ int uECC_sign(const uint8_t *private_key, const uint8_t *message_hash, } // computing k as modular reduction of _random (see FIPS 186.4 B.5.1): - uECC_vli_mmod(k, _random, curve->n); + uECC_vli_mmod(k, _random, curve_n); if (uECC_sign_with_k(private_key, message_hash, hash_size, k, signature, curve)) { @@ -241,17 +243,17 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, } /* r, s must be < n. */ - if (uECC_vli_cmp_unsafe(curve->n, r) != 1 || - uECC_vli_cmp_unsafe(curve->n, s) != 1) { + if (uECC_vli_cmp_unsafe(curve_n, r) != 1 || + uECC_vli_cmp_unsafe(curve_n, s) != 1) { return UECC_FAILURE; } /* Calculate u1 and u2. */ - uECC_vli_modInv(z, s, curve->n); /* z = 1/s */ + uECC_vli_modInv(z, s, curve_n); /* z = 1/s */ u1[num_n_words - 1] = 0; bits2int(u1, message_hash, hash_size, curve); - uECC_vli_modMult(u1, u1, z, curve->n); /* u1 = e/s */ - uECC_vli_modMult(u2, r, z, curve->n); /* u2 = r/s */ + uECC_vli_modMult(u1, u1, z, curve_n); /* u1 = e/s */ + uECC_vli_modMult(u2, r, z, curve_n); /* u2 = r/s */ /* Calculate sum = G + Q. */ uECC_vli_set(sum, _public); @@ -298,8 +300,8 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, apply_z(rx, ry, z); /* v = x1 (mod n) */ - if (uECC_vli_cmp_unsafe(curve->n, rx) != 1) { - uECC_vli_sub(rx, rx, curve->n); + if (uECC_vli_cmp_unsafe(curve_n, rx) != 1) { + uECC_vli_sub(rx, rx, curve_n); } /* Accept only if v == r. */ From a6115087a0500282eb32e3daa5809504db859195 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Nov 2019 10:29:14 +0100 Subject: [PATCH 09/23] Move G from struct curve to its own constant --- include/tinycrypt/ecc.h | 12 +----------- tinycrypt/ecc.c | 14 ++++++++++++-- tinycrypt/ecc_dsa.c | 8 ++++---- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/include/tinycrypt/ecc.h b/include/tinycrypt/ecc.h index dccfdf423..70bd5ad4f 100644 --- a/include/tinycrypt/ecc.h +++ b/include/tinycrypt/ecc.h @@ -124,7 +124,6 @@ typedef uint64_t uECC_dword_t; struct uECC_Curve_t; typedef const struct uECC_Curve_t * uECC_Curve; struct uECC_Curve_t { - uECC_word_t G[NUM_ECC_WORDS * 2]; uECC_word_t b[NUM_ECC_WORDS]; }; @@ -155,20 +154,11 @@ void vli_mmod_fast_secp256r1(unsigned int *result, unsigned int *product); extern const uECC_word_t curve_p[NUM_ECC_WORDS]; extern const uECC_word_t curve_n[NUM_ECC_WORDS]; +extern const uECC_word_t curve_G[2 * NUM_ECC_WORDS]; /* definition of curve NIST p-256: */ static const struct uECC_Curve_t curve_secp256r1 = { { - BYTES_TO_WORDS_8(96, C2, 98, D8, 45, 39, A1, F4), - BYTES_TO_WORDS_8(A0, 33, EB, 2D, 81, 7D, 03, 77), - BYTES_TO_WORDS_8(F2, 40, A4, 63, E5, E6, BC, F8), - BYTES_TO_WORDS_8(47, 42, 2C, E1, F2, D1, 17, 6B), - - BYTES_TO_WORDS_8(F5, 51, BF, 37, 68, 40, B6, CB), - BYTES_TO_WORDS_8(CE, 5E, 31, 6B, 57, 33, CE, 2B), - BYTES_TO_WORDS_8(16, 9E, 0F, 7C, 4A, EB, E7, 8E), - BYTES_TO_WORDS_8(9B, 7F, 1A, FE, E2, 42, E3, 4F) - }, { BYTES_TO_WORDS_8(4B, 60, D2, 27, 3E, 3C, CE, 3B), BYTES_TO_WORDS_8(F6, B0, 53, CC, B0, 06, 1D, 65), BYTES_TO_WORDS_8(BC, 86, 98, 76, 55, BD, EB, B3), diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index daa9698a3..9cbed3f95 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -81,6 +81,16 @@ const uECC_word_t curve_n[NUM_ECC_WORDS] = { BYTES_TO_WORDS_8(FF, FF, FF, FF, FF, FF, FF, FF), BYTES_TO_WORDS_8(00, 00, 00, 00, FF, FF, FF, FF) }; +const uECC_word_t curve_G[2 * NUM_ECC_WORDS] = { + BYTES_TO_WORDS_8(96, C2, 98, D8, 45, 39, A1, F4), + BYTES_TO_WORDS_8(A0, 33, EB, 2D, 81, 7D, 03, 77), + BYTES_TO_WORDS_8(F2, 40, A4, 63, E5, E6, BC, F8), + BYTES_TO_WORDS_8(47, 42, 2C, E1, F2, D1, 17, 6B), + BYTES_TO_WORDS_8(F5, 51, BF, 37, 68, 40, B6, CB), + BYTES_TO_WORDS_8(CE, 5E, 31, 6B, 57, 33, CE, 2B), + BYTES_TO_WORDS_8(16, 9E, 0F, 7C, 4A, EB, E7, 8E), + BYTES_TO_WORDS_8(9B, 7F, 1A, FE, E2, 42, E3, 4F) +}; /* IMPORTANT: Make sure a cryptographically-secure PRNG is set and the platform * has access to enough entropy in order to feed the PRNG regularly. */ @@ -1006,7 +1016,7 @@ uECC_word_t EccPoint_compute_public_key(uECC_word_t *result, uECC_word_t *private_key, uECC_Curve curve) { - return EccPoint_mult_safer(result, curve->G, private_key, curve); + return EccPoint_mult_safer(result, curve_G, private_key, curve); } /* Converts an integer in uECC native format to big-endian bytes. */ @@ -1097,7 +1107,7 @@ int uECC_valid_public_key(const uint8_t *public_key, uECC_Curve curve) public_key + NUM_ECC_BYTES, NUM_ECC_BYTES); - if (memcmp(_public, curve->G, NUM_ECC_WORDS * 2) == 0) { + if (memcmp(_public, curve_G, NUM_ECC_WORDS * 2) == 0) { return -4; } diff --git a/tinycrypt/ecc_dsa.c b/tinycrypt/ecc_dsa.c index c22ebd032..82e159cf2 100644 --- a/tinycrypt/ecc_dsa.c +++ b/tinycrypt/ecc_dsa.c @@ -128,7 +128,7 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, return 0; } - r = EccPoint_mult_safer(p, curve->G, k, curve); + r = EccPoint_mult_safer(p, curve_G, k, curve); if (r == 0 || uECC_vli_isZero(p)) { return 0; } @@ -258,8 +258,8 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, /* Calculate sum = G + Q. */ uECC_vli_set(sum, _public); uECC_vli_set(sum + num_words, _public + num_words); - uECC_vli_set(tx, curve->G); - uECC_vli_set(ty, curve->G + num_words); + uECC_vli_set(tx, curve_G); + uECC_vli_set(ty, curve_G + num_words); uECC_vli_modSub(z, sum, tx, curve_p); /* z = x2 - x1 */ XYcZ_add(tx, ty, sum, sum + num_words, curve); uECC_vli_modInv(z, z, curve_p); /* z = 1/z */ @@ -267,7 +267,7 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, /* Use Shamir's trick to calculate u1*G + u2*Q */ points[0] = 0; - points[1] = curve->G; + points[1] = curve_G; points[2] = _public; points[3] = sum; num_bits = smax(uECC_vli_numBits(u1), From ffd13996fd6a1a79e395c50fb79399e9c1b602d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Nov 2019 10:39:06 +0100 Subject: [PATCH 10/23] Move b from curve structure to its own constant Same motivation as for the other parameters. This is the last one, making the curve structure empty, so it's left with a dummy parameter for legal reasons. --- include/tinycrypt/ecc.h | 10 +++------- tinycrypt/ecc.c | 10 +++++++++- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/include/tinycrypt/ecc.h b/include/tinycrypt/ecc.h index 70bd5ad4f..6b88aeedd 100644 --- a/include/tinycrypt/ecc.h +++ b/include/tinycrypt/ecc.h @@ -124,7 +124,7 @@ typedef uint64_t uECC_dword_t; struct uECC_Curve_t; typedef const struct uECC_Curve_t * uECC_Curve; struct uECC_Curve_t { - uECC_word_t b[NUM_ECC_WORDS]; + unsigned char dummy; }; /* @@ -155,15 +155,11 @@ void vli_mmod_fast_secp256r1(unsigned int *result, unsigned int *product); extern const uECC_word_t curve_p[NUM_ECC_WORDS]; extern const uECC_word_t curve_n[NUM_ECC_WORDS]; extern const uECC_word_t curve_G[2 * NUM_ECC_WORDS]; +extern const uECC_word_t curve_b[NUM_ECC_WORDS]; /* definition of curve NIST p-256: */ static const struct uECC_Curve_t curve_secp256r1 = { - { - BYTES_TO_WORDS_8(4B, 60, D2, 27, 3E, 3C, CE, 3B), - BYTES_TO_WORDS_8(F6, B0, 53, CC, B0, 06, 1D, 65), - BYTES_TO_WORDS_8(BC, 86, 98, 76, 55, BD, EB, B3), - BYTES_TO_WORDS_8(E7, 93, 3A, AA, D8, 35, C6, 5A) - }, + 0 }; uECC_Curve uECC_secp256r1(void); diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index 9cbed3f95..56580f4bb 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -91,6 +91,12 @@ const uECC_word_t curve_G[2 * NUM_ECC_WORDS] = { BYTES_TO_WORDS_8(16, 9E, 0F, 7C, 4A, EB, E7, 8E), BYTES_TO_WORDS_8(9B, 7F, 1A, FE, E2, 42, E3, 4F) }; +const uECC_word_t curve_b[NUM_ECC_WORDS] = { + BYTES_TO_WORDS_8(4B, 60, D2, 27, 3E, 3C, CE, 3B), + BYTES_TO_WORDS_8(F6, B0, 53, CC, B0, 06, 1D, 65), + BYTES_TO_WORDS_8(BC, 86, 98, 76, 55, BD, EB, B3), + BYTES_TO_WORDS_8(E7, 93, 3A, AA, D8, 35, C6, 5A) +}; /* IMPORTANT: Make sure a cryptographically-secure PRNG is set and the platform * has access to enough entropy in order to feed the PRNG regularly. */ @@ -662,11 +668,13 @@ static void x_side_default(uECC_word_t *result, { uECC_word_t _3[NUM_ECC_WORDS] = {3}; /* -a = 3 */ + (void) curve; + uECC_vli_modMult_fast(result, x, x); /* r = x^2 */ uECC_vli_modSub(result, result, _3, curve_p); /* r = x^2 - 3 */ uECC_vli_modMult_fast(result, result, x); /* r = x^3 - 3x */ /* r = x^3 - 3x + b: */ - uECC_vli_modAdd(result, result, curve->b, curve_p); + uECC_vli_modAdd(result, result, curve_b, curve_p); } uECC_Curve uECC_secp256r1(void) From 677b7f6c42ddc27ed26887cdccfcd5f88ce4d687 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Nov 2019 11:28:24 +0100 Subject: [PATCH 11/23] Fix direct use of struct instead of abstract type --- library/pk.c | 4 ++-- library/ssl_cli.c | 4 ++-- library/ssl_srv.c | 2 +- library/ssl_tls.c | 4 ++-- tests/suites/test_suite_tinycrypt.function | 8 ++++---- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/library/pk.c b/library/pk.c index 9eddb61ab..9c81ccc4d 100644 --- a/library/pk.c +++ b/library/pk.c @@ -580,7 +580,7 @@ static int uecc_eckey_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, volatile int ret_fi; uint8_t signature[2*NUM_ECC_BYTES]; unsigned char *p; - const struct uECC_Curve_t * uecc_curve = uECC_secp256r1(); + uECC_Curve uecc_curve = uECC_secp256r1(); const mbedtls_uecc_keypair *keypair = (const mbedtls_uecc_keypair *) ctx; ((void) md_alg); @@ -704,7 +704,7 @@ static int uecc_eckey_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { const mbedtls_uecc_keypair *keypair = (const mbedtls_uecc_keypair *) ctx; - const struct uECC_Curve_t * uecc_curve = uECC_secp256r1(); + uECC_Curve uecc_curve = uECC_secp256r1(); int ret; /* diff --git a/library/ssl_cli.c b/library/ssl_cli.c index f7d331fba..9d2af94e8 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3567,7 +3567,7 @@ static int ssl_out_client_key_exchange_write( mbedtls_ssl_context *ssl, == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) { - const struct uECC_Curve_t * uecc_curve = uECC_secp256r1(); + uECC_Curve uecc_curve = uECC_secp256r1(); ((void) n); ((void) ret); @@ -3718,7 +3718,7 @@ static int ssl_out_client_key_exchange_write( mbedtls_ssl_context *ssl, == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK ) { #if defined(MBEDTLS_USE_TINYCRYPT) - const struct uECC_Curve_t * uecc_curve = uECC_secp256r1(); + uECC_Curve uecc_curve = uECC_secp256r1(); ((void) n); ((void) ret); diff --git a/library/ssl_srv.c b/library/ssl_srv.c index a9f983ede..d3bcd807a 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3280,7 +3280,7 @@ static int ssl_prepare_server_key_exchange( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED */ #endif /* MBEDTLS_KEY_EXCHANGE__SOME_PFS__ENABLED */ #if defined(MBEDTLS_USE_TINYCRYPT) - const struct uECC_Curve_t * uecc_curve = uECC_secp256r1(); + uECC_Curve uecc_curve = uECC_secp256r1(); #endif (void) ciphersuite_info; /* unused in some configurations */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 988c59345..bf24a98ab 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1973,7 +1973,7 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) { - const struct uECC_Curve_t * uecc_curve = uECC_secp256r1(); + uECC_Curve uecc_curve = uECC_secp256r1(); ((void) ret); if( !uECC_shared_secret( ssl->handshake->ecdh_peerkey, @@ -2170,7 +2170,7 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch size_t zlen; #if defined(MBEDTLS_USE_TINYCRYPT) - const struct uECC_Curve_t * uecc_curve = uECC_secp256r1(); + uECC_Curve uecc_curve = uECC_secp256r1(); ((void) ret); if( !uECC_shared_secret( ssl->handshake->ecdh_peerkey, diff --git a/tests/suites/test_suite_tinycrypt.function b/tests/suites/test_suite_tinycrypt.function index 664cd0862..51e3e32d5 100644 --- a/tests/suites/test_suite_tinycrypt.function +++ b/tests/suites/test_suite_tinycrypt.function @@ -21,7 +21,7 @@ void test_ecdh() uint8_t secret1[NUM_ECC_BYTES] = {0}; uint8_t secret2[NUM_ECC_BYTES] = {0}; - const struct uECC_Curve_t * curve = uECC_secp256r1(); + uECC_Curve curve = uECC_secp256r1(); uECC_set_rng( &uecc_rng_wrapper ); @@ -45,7 +45,7 @@ void test_ecdsa() uint8_t hash[NUM_ECC_BYTES] = {0}; uint8_t sig[2*NUM_ECC_BYTES] = {0}; - const struct uECC_Curve_t * curve = uECC_secp256r1(); + uECC_Curve curve = uECC_secp256r1(); uECC_set_rng( &uecc_rng_wrapper ); @@ -64,7 +64,7 @@ void ecdh_primitive_testvec( data_t * private1, data_t * xA_str, data_t * yA_str, data_t * private2, data_t * xB_str, data_t * yB_str, data_t * z_str ) { - const struct uECC_Curve_t * curve = uECC_secp256r1(); + uECC_Curve curve = uECC_secp256r1(); uint8_t public1[2*NUM_ECC_BYTES] = {0}; uint8_t public2[2*NUM_ECC_BYTES] = {0}; uint8_t secret1[NUM_ECC_BYTES] = {0}; @@ -90,7 +90,7 @@ void ecdh_primitive_testvec( data_t * private1, data_t * xA_str, void ecdsa_primitive_testvec( data_t * xQ_str, data_t * yQ_str, data_t * hash, data_t * r_str, data_t * s_str ) { - const struct uECC_Curve_t * curve = uECC_secp256r1(); + uECC_Curve curve = uECC_secp256r1(); uint8_t pub_bytes[2*NUM_ECC_BYTES] = {0}; uint8_t sig_bytes[2*NUM_ECC_BYTES] = {0}; From bc3f49011a6d97037855c6862287ec53b6da4e10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Nov 2019 11:34:43 +0100 Subject: [PATCH 12/23] Remove struct curve entirely --- include/tinycrypt/ecc.h | 16 +++++----------- tinycrypt/ecc.c | 8 +------- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/include/tinycrypt/ecc.h b/include/tinycrypt/ecc.h index 6b88aeedd..65381bd03 100644 --- a/include/tinycrypt/ecc.h +++ b/include/tinycrypt/ecc.h @@ -120,12 +120,11 @@ typedef uint64_t uECC_dword_t; #define NUM_ECC_BYTES (uECC_WORD_SIZE*NUM_ECC_WORDS) #define NUM_ECC_BITS 256 -/* structure that represents an elliptic curve (e.g. p256):*/ -struct uECC_Curve_t; -typedef const struct uECC_Curve_t * uECC_Curve; -struct uECC_Curve_t { - unsigned char dummy; -}; +/* curve identifier (for API compatility - only P-256 is supported) */ +typedef enum { + curve_invalid = 0, + curve_secp256r1 = 0xff +} uECC_Curve; /* * @brief computes doubling of point ion jacobian coordinates, in place. @@ -157,11 +156,6 @@ extern const uECC_word_t curve_n[NUM_ECC_WORDS]; extern const uECC_word_t curve_G[2 * NUM_ECC_WORDS]; extern const uECC_word_t curve_b[NUM_ECC_WORDS]; -/* definition of curve NIST p-256: */ -static const struct uECC_Curve_t curve_secp256r1 = { - 0 -}; - uECC_Curve uECC_secp256r1(void); /* diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index 56580f4bb..03645c0b8 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -679,7 +679,7 @@ static void x_side_default(uECC_word_t *result, uECC_Curve uECC_secp256r1(void) { - return &curve_secp256r1; + return curve_secp256r1; } void vli_mmod_fast_secp256r1(unsigned int *result, unsigned int*product) @@ -827,9 +827,6 @@ static void XYcZ_add_rnd(uECC_word_t * X1, uECC_word_t * Y1, { /* t1 = X1, t2 = Y1, t3 = X2, t4 = Y2 */ uECC_word_t t5[NUM_ECC_WORDS]; - const uECC_Curve curve = &curve_secp256r1; - - (void) curve; uECC_vli_modSub(t5, X2, X1, curve_p); /* t5 = x2 - x1 */ uECC_vli_modMult_rnd(t5, t5, t5, s); /* t5 = (x2 - x1)^2 = A */ @@ -869,9 +866,6 @@ static void XYcZ_addC_rnd(uECC_word_t * X1, uECC_word_t * Y1, uECC_word_t t5[NUM_ECC_WORDS]; uECC_word_t t6[NUM_ECC_WORDS]; uECC_word_t t7[NUM_ECC_WORDS]; - const uECC_Curve curve = &curve_secp256r1; - - (void) curve; uECC_vli_modSub(t5, X2, X1, curve_p); /* t5 = x2 - x1 */ uECC_vli_modMult_rnd(t5, t5, t5, s); /* t5 = (x2 - x1)^2 = A */ From be5f833c9cc4a3845d49b1a5d8314546c784c6b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Nov 2019 11:02:38 +0100 Subject: [PATCH 13/23] Remove curve parameter from (semi-)internal functions By semi-internal I mean functions that are only public because they're used in more than once compilation unit in the library (for example in ecc.c and ecc_dsa.c) but should not really be part of the public-facing API. --- include/tinycrypt/ecc.h | 11 +++++------ tinycrypt/ecc.c | 38 ++++++++++++++------------------------ tinycrypt/ecc_dsa.c | 6 +++--- 3 files changed, 22 insertions(+), 33 deletions(-) diff --git a/include/tinycrypt/ecc.h b/include/tinycrypt/ecc.h index 65381bd03..5a487436a 100644 --- a/include/tinycrypt/ecc.h +++ b/include/tinycrypt/ecc.h @@ -134,7 +134,7 @@ typedef enum { * @param curve IN -- elliptic curve */ void double_jacobian_default(uECC_word_t * X1, uECC_word_t * Y1, - uECC_word_t * Z1, uECC_Curve curve); + uECC_word_t * Z1); /* * @brief Computes result = product % curve_p @@ -265,10 +265,9 @@ uECC_word_t uECC_vli_isZero(const uECC_word_t *vli); /* * @brief Check if 'point' is the point at infinity * @param point IN -- elliptic curve point - * @param curve IN -- elliptic curve * @return if 'point' is the point at infinity, 0 otherwise. */ -uECC_word_t EccPoint_isZero(const uECC_word_t *point, uECC_Curve curve); +uECC_word_t EccPoint_isZero(const uECC_word_t *point); /* * @brief computes the sign of left - right, in constant time. @@ -313,7 +312,7 @@ void uECC_vli_modSub(uECC_word_t *result, const uECC_word_t *left, * @param curve IN -- elliptic curve */ void XYcZ_add(uECC_word_t * X1, uECC_word_t * Y1, uECC_word_t * X2, - uECC_word_t * Y2, uECC_Curve curve); + uECC_word_t * Y2); /* * @brief Computes (x1 * z^2, y1 * z^3) @@ -444,7 +443,7 @@ void uECC_vli_clear(uECC_word_t *vli); * @exception returns -2 if x or y is smaller than p, * @exception returns -3 if y^2 != x^3 + ax + b. */ -int uECC_valid_point(const uECC_word_t *point, uECC_Curve curve); +int uECC_valid_point(const uECC_word_t *point); /* * @brief Check if a public key is valid. @@ -460,7 +459,7 @@ int uECC_valid_point(const uECC_word_t *point, uECC_Curve curve); * time computing a shared secret or verifying a signature using an invalid * public key. */ -int uECC_valid_public_key(const uint8_t *public_key, uECC_Curve curve); +int uECC_valid_public_key(const uint8_t *public_key); /* * @brief Converts an integer in uECC native format to big-endian bytes. diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index 03645c0b8..0c53f9dd6 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -608,15 +608,13 @@ void uECC_vli_modInv(uECC_word_t *result, const uECC_word_t *input, /* ------ Point operations ------ */ void double_jacobian_default(uECC_word_t * X1, uECC_word_t * Y1, - uECC_word_t * Z1, uECC_Curve curve) + uECC_word_t * Z1) { /* t1 = X, t2 = Y, t3 = Z */ uECC_word_t t4[NUM_ECC_WORDS]; uECC_word_t t5[NUM_ECC_WORDS]; wordcount_t num_words = NUM_ECC_WORDS; - (void) curve; - if (uECC_vli_isZero(Z1)) { return; } @@ -663,13 +661,10 @@ void double_jacobian_default(uECC_word_t * X1, uECC_word_t * Y1, * @param curve IN -- elliptic curve */ static void x_side_default(uECC_word_t *result, - const uECC_word_t *x, - uECC_Curve curve) + const uECC_word_t *x) { uECC_word_t _3[NUM_ECC_WORDS] = {3}; /* -a = 3 */ - (void) curve; - uECC_vli_modMult_fast(result, x, x); /* r = x^2 */ uECC_vli_modSub(result, result, _3, curve_p); /* r = x^2 - 3 */ uECC_vli_modMult_fast(result, result, x); /* r = x^3 - 3x */ @@ -783,9 +778,8 @@ void vli_mmod_fast_secp256r1(unsigned int *result, unsigned int*product) } } -uECC_word_t EccPoint_isZero(const uECC_word_t *point, uECC_Curve curve) +uECC_word_t EccPoint_isZero(const uECC_word_t *point) { - (void) curve; return uECC_vli_isZero(point); } @@ -802,8 +796,7 @@ void apply_z(uECC_word_t * X1, uECC_word_t * Y1, const uECC_word_t * const Z) /* P = (x1, y1) => 2P, (x2, y2) => P' */ static void XYcZ_initial_double(uECC_word_t * X1, uECC_word_t * Y1, uECC_word_t * X2, uECC_word_t * Y2, - const uECC_word_t * const initial_Z, - uECC_Curve curve) + const uECC_word_t * const initial_Z) { uECC_word_t z[NUM_ECC_WORDS]; if (initial_Z) { @@ -817,7 +810,7 @@ static void XYcZ_initial_double(uECC_word_t * X1, uECC_word_t * Y1, uECC_vli_set(Y2, Y1); apply_z(X1, Y1, z); - double_jacobian_default(X1, Y1, z, curve); + double_jacobian_default(X1, Y1, z); apply_z(X2, Y2, z); } @@ -847,10 +840,8 @@ static void XYcZ_add_rnd(uECC_word_t * X1, uECC_word_t * Y1, } void XYcZ_add(uECC_word_t * X1, uECC_word_t * Y1, - uECC_word_t * X2, uECC_word_t * Y2, - uECC_Curve curve) + uECC_word_t * X2, uECC_word_t * Y2) { - (void) curve; XYcZ_add_rnd(X1, Y1, X2, Y2, NULL); } @@ -907,14 +898,13 @@ static void EccPoint_mult(uECC_word_t * result, const uECC_word_t * point, uECC_word_t nb; const wordcount_t num_words = NUM_ECC_WORDS; const bitcount_t num_bits = NUM_ECC_BITS + 1; /* from regularize_k */ - const uECC_Curve curve = uECC_secp256r1(); ecc_wait_state_t wait_state; ecc_wait_state_t * const ws = g_rng_function ? &wait_state : NULL; uECC_vli_set(Rx[1], point); uECC_vli_set(Ry[1], point + num_words); - XYcZ_initial_double(Rx[1], Ry[1], Rx[0], Ry[0], initial_Z, curve); + XYcZ_initial_double(Rx[1], Ry[1], Rx[0], Ry[0], initial_Z); for (i = num_bits - 2; i > 0; --i) { ecc_wait_state_reset(ws); @@ -976,7 +966,7 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, return 0; /* Protects against invalid curves attacks */ - if (uECC_valid_point(point, curve) != 0 ) { + if (uECC_valid_point(point) != 0 ) { return 0; } @@ -998,7 +988,7 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, /* Protect against fault injections that would make the resulting * point not lie on the intended curve */ - if (uECC_valid_point(result, curve) != 0 ) { + if (uECC_valid_point(result) != 0 ) { r = 0; goto clear_and_out; } @@ -1071,14 +1061,14 @@ int uECC_generate_random_int(uECC_word_t *random, const uECC_word_t *top, } -int uECC_valid_point(const uECC_word_t *point, uECC_Curve curve) +int uECC_valid_point(const uECC_word_t *point) { uECC_word_t tmp1[NUM_ECC_WORDS]; uECC_word_t tmp2[NUM_ECC_WORDS]; wordcount_t num_words = NUM_ECC_WORDS; /* The point at infinity is invalid. */ - if (EccPoint_isZero(point, curve)) { + if (EccPoint_isZero(point)) { return -1; } @@ -1089,7 +1079,7 @@ int uECC_valid_point(const uECC_word_t *point, uECC_Curve curve) } uECC_vli_modMult_fast(tmp1, point + num_words, point + num_words); - x_side_default(tmp2, point, curve); /* tmp2 = x^3 + ax + b */ + x_side_default(tmp2, point); /* tmp2 = x^3 + ax + b */ /* Make sure that y^2 == x^3 + ax + b */ if (uECC_vli_equal(tmp1, tmp2) != 0) @@ -1098,7 +1088,7 @@ int uECC_valid_point(const uECC_word_t *point, uECC_Curve curve) return 0; } -int uECC_valid_public_key(const uint8_t *public_key, uECC_Curve curve) +int uECC_valid_public_key(const uint8_t *public_key) { uECC_word_t _public[NUM_ECC_WORDS * 2]; @@ -1113,7 +1103,7 @@ int uECC_valid_public_key(const uint8_t *public_key, uECC_Curve curve) return -4; } - return uECC_valid_point(_public, curve); + return uECC_valid_point(_public); } int uECC_compute_public_key(const uint8_t *private_key, uint8_t *public_key, diff --git a/tinycrypt/ecc_dsa.c b/tinycrypt/ecc_dsa.c index 82e159cf2..0f7a9fd2f 100644 --- a/tinycrypt/ecc_dsa.c +++ b/tinycrypt/ecc_dsa.c @@ -261,7 +261,7 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, uECC_vli_set(tx, curve_G); uECC_vli_set(ty, curve_G + num_words); uECC_vli_modSub(z, sum, tx, curve_p); /* z = x2 - x1 */ - XYcZ_add(tx, ty, sum, sum + num_words, curve); + XYcZ_add(tx, ty, sum, sum + num_words); uECC_vli_modInv(z, z, curve_p); /* z = 1/z */ apply_z(sum, sum + num_words, z); @@ -282,7 +282,7 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, for (i = num_bits - 2; i >= 0; --i) { uECC_word_t index; - double_jacobian_default(rx, ry, z, curve); + double_jacobian_default(rx, ry, z); index = (!!uECC_vli_testBit(u1, i)) | ((!!uECC_vli_testBit(u2, i)) << 1); point = points[index]; @@ -291,7 +291,7 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, uECC_vli_set(ty, point + num_words); apply_z(tx, ty, z); uECC_vli_modSub(tz, rx, tx, curve_p); /* Z = x2 - x1 */ - XYcZ_add(tx, ty, rx, ry, curve); + XYcZ_add(tx, ty, rx, ry); uECC_vli_modMult_fast(z, z, tz); } } From 1a5337179f26a1e4bc8012d2f07eb862d8e59cd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Nov 2019 12:00:43 +0100 Subject: [PATCH 14/23] Remove curve parameter from public functions --- include/tinycrypt/ecc.h | 19 ++++----------- include/tinycrypt/ecc_dh.h | 6 ++--- include/tinycrypt/ecc_dsa.h | 7 +++--- library/pk.c | 6 ++--- library/pkparse.c | 3 +-- library/ssl_cli.c | 8 ++----- library/ssl_srv.c | 6 +---- library/ssl_tls.c | 8 ++----- tests/suites/test_suite_pk.function | 3 +-- tests/suites/test_suite_pkparse.function | 9 +++---- tests/suites/test_suite_tinycrypt.function | 28 +++++++++------------- tinycrypt/ecc.c | 26 ++++++-------------- tinycrypt/ecc_dh.c | 12 +++++----- tinycrypt/ecc_dsa.c | 24 +++++++------------ 14 files changed, 55 insertions(+), 110 deletions(-) diff --git a/include/tinycrypt/ecc.h b/include/tinycrypt/ecc.h index 5a487436a..d50183813 100644 --- a/include/tinycrypt/ecc.h +++ b/include/tinycrypt/ecc.h @@ -120,12 +120,6 @@ typedef uint64_t uECC_dword_t; #define NUM_ECC_BYTES (uECC_WORD_SIZE*NUM_ECC_WORDS) #define NUM_ECC_BITS 256 -/* curve identifier (for API compatility - only P-256 is supported) */ -typedef enum { - curve_invalid = 0, - curve_secp256r1 = 0xff -} uECC_Curve; - /* * @brief computes doubling of point ion jacobian coordinates, in place. * @param X1 IN/OUT -- x coordinate @@ -156,8 +150,6 @@ extern const uECC_word_t curve_n[NUM_ECC_WORDS]; extern const uECC_word_t curve_G[2 * NUM_ECC_WORDS]; extern const uECC_word_t curve_b[NUM_ECC_WORDS]; -uECC_Curve uECC_secp256r1(void); - /* * @brief Generates a random integer in the range 0 < random < top. * Both random and top have num_words words. @@ -211,14 +203,14 @@ uECC_RNG_Function uECC_get_rng(void); * @param curve IN -- elliptic curve * @return size of a private key for the curve in bytes. */ -int uECC_curve_private_key_size(uECC_Curve curve); +int uECC_curve_private_key_size(void); /* * @brief computes the size of a public key for the curve in bytes. * @param curve IN -- elliptic curve * @return the size of a public key for the curve in bytes. */ -int uECC_curve_public_key_size(uECC_Curve curve); +int uECC_curve_public_key_size(void); /* * @brief Compute the corresponding public key for a private key. @@ -228,7 +220,7 @@ int uECC_curve_public_key_size(uECC_Curve curve); * @return Returns 1 if key was computed successfully, 0 if an error occurred. */ int uECC_compute_public_key(const uint8_t *private_key, - uint8_t *public_key, uECC_Curve curve); + uint8_t *public_key); /* * @brief Compute public-key. @@ -238,7 +230,7 @@ int uECC_compute_public_key(const uint8_t *private_key, * @param curve IN -- elliptic curve */ uECC_word_t EccPoint_compute_public_key(uECC_word_t *result, - uECC_word_t *private_key, uECC_Curve curve); + uECC_word_t *private_key); /* * @brief Point multiplication algorithm using Montgomery's ladder with co-Z @@ -249,10 +241,9 @@ uECC_word_t EccPoint_compute_public_key(uECC_word_t *result, * @param result OUT -- returns scalar*point * @param point IN -- elliptic curve point * @param scalar IN -- scalar - * @param curve IN -- elliptic curve */ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, - const uECC_word_t * scalar, uECC_Curve curve); + const uECC_word_t * scalar); /* * @brief Constant-time comparison to zero - secure way to compare long integers diff --git a/include/tinycrypt/ecc_dh.h b/include/tinycrypt/ecc_dh.h index a2edb0155..61e4c52ba 100644 --- a/include/tinycrypt/ecc_dh.h +++ b/include/tinycrypt/ecc_dh.h @@ -97,7 +97,7 @@ extern "C" { * @warning A cryptographically-secure PRNG function must be set (using * uECC_set_rng()) before calling uECC_make_key(). */ -int uECC_make_key(uint8_t *p_public_key, uint8_t *p_private_key, uECC_Curve curve); +int uECC_make_key(uint8_t *p_public_key, uint8_t *p_private_key); #ifdef ENABLE_TESTS @@ -108,7 +108,7 @@ int uECC_make_key(uint8_t *p_public_key, uint8_t *p_private_key, uECC_Curve curv * uECC_make_key() function for real applications. */ int uECC_make_key_with_d(uint8_t *p_public_key, uint8_t *p_private_key, - unsigned int *d, uECC_Curve curve); + unsigned int *d); #endif /** @@ -128,7 +128,7 @@ int uECC_make_key_with_d(uint8_t *p_public_key, uint8_t *p_private_key, * order to produce a cryptographically secure symmetric key. */ int uECC_shared_secret(const uint8_t *p_public_key, const uint8_t *p_private_key, - uint8_t *p_secret, uECC_Curve curve); + uint8_t *p_secret); #ifdef __cplusplus } diff --git a/include/tinycrypt/ecc_dsa.h b/include/tinycrypt/ecc_dsa.h index 55b9d43f3..5985c7f0c 100644 --- a/include/tinycrypt/ecc_dsa.h +++ b/include/tinycrypt/ecc_dsa.h @@ -109,7 +109,7 @@ extern "C" { * attack. */ int uECC_sign(const uint8_t *p_private_key, const uint8_t *p_message_hash, - unsigned p_hash_size, uint8_t *p_signature, uECC_Curve curve); + unsigned p_hash_size, uint8_t *p_signature); #ifdef ENABLE_TESTS /* @@ -117,8 +117,7 @@ int uECC_sign(const uint8_t *p_private_key, const uint8_t *p_message_hash, * Refer to uECC_sign() function for real applications. */ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, - unsigned int hash_size, uECC_word_t *k, uint8_t *signature, - uECC_Curve curve); + unsigned int hash_size, uECC_word_t *k, uint8_t *signature) #endif /** @@ -136,7 +135,7 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, * the signature values (hash_size and signature). */ int uECC_verify(const uint8_t *p_public_key, const uint8_t *p_message_hash, - unsigned int p_hash_size, const uint8_t *p_signature, uECC_Curve curve); + unsigned int p_hash_size, const uint8_t *p_signature); #ifdef __cplusplus } diff --git a/library/pk.c b/library/pk.c index 9c81ccc4d..05ffe1c70 100644 --- a/library/pk.c +++ b/library/pk.c @@ -580,7 +580,6 @@ static int uecc_eckey_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, volatile int ret_fi; uint8_t signature[2*NUM_ECC_BYTES]; unsigned char *p; - uECC_Curve uecc_curve = uECC_secp256r1(); const mbedtls_uecc_keypair *keypair = (const mbedtls_uecc_keypair *) ctx; ((void) md_alg); @@ -591,7 +590,7 @@ static int uecc_eckey_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, return( ret ); ret_fi = uECC_verify( keypair->public_key, hash, - (unsigned) hash_len, signature, uecc_curve ); + (unsigned) hash_len, signature ); if( ret_fi == UECC_ATTACK_DETECTED ) return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); @@ -704,7 +703,6 @@ static int uecc_eckey_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { const mbedtls_uecc_keypair *keypair = (const mbedtls_uecc_keypair *) ctx; - uECC_Curve uecc_curve = uECC_secp256r1(); int ret; /* @@ -724,7 +722,7 @@ static int uecc_eckey_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, */ #define MAX_SECP256R1_ECDSA_SIG_LEN ( 3 + 2 * ( 3 + NUM_ECC_BYTES ) ) - ret = uECC_sign( keypair->private_key, hash, hash_len, sig, uecc_curve ); + ret = uECC_sign( keypair->private_key, hash, hash_len, sig ); /* TinyCrypt uses 0 to signal errors. */ if( ret == 0 ) return( MBEDTLS_ERR_PK_HW_ACCEL_FAILED ); diff --git a/library/pkparse.c b/library/pkparse.c index 4562f6547..6a2507aa3 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -986,8 +986,7 @@ static int pk_parse_key_sec1_der( mbedtls_uecc_keypair *keypair, if( !pubkey_done ) { ret = uECC_compute_public_key( keypair->private_key, - keypair->public_key, - uECC_secp256r1() ); + keypair->public_key ); if( ret == 0 ) return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT ); } diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 9d2af94e8..3a5671e64 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3567,7 +3567,6 @@ static int ssl_out_client_key_exchange_write( mbedtls_ssl_context *ssl, == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) { - uECC_Curve uecc_curve = uECC_secp256r1(); ((void) n); ((void) ret); @@ -3577,8 +3576,7 @@ static int ssl_out_client_key_exchange_write( mbedtls_ssl_context *ssl, *p++ = 2 * NUM_ECC_BYTES + 1; *p++ = 0x04; /* uncompressed point presentation */ - if( !uECC_make_key( p, ssl->handshake->ecdh_privkey, - uecc_curve ) ) + if( !uECC_make_key( p, ssl->handshake->ecdh_privkey ) ) { return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED ); } @@ -3718,7 +3716,6 @@ static int ssl_out_client_key_exchange_write( mbedtls_ssl_context *ssl, == MBEDTLS_KEY_EXCHANGE_ECDHE_PSK ) { #if defined(MBEDTLS_USE_TINYCRYPT) - uECC_Curve uecc_curve = uECC_secp256r1(); ((void) n); ((void) ret); @@ -3728,8 +3725,7 @@ static int ssl_out_client_key_exchange_write( mbedtls_ssl_context *ssl, *p++ = 2 * NUM_ECC_BYTES + 1; *p++ = 0x04; /* uncompressed point presentation */ - if( !uECC_make_key( p, ssl->handshake->ecdh_privkey, - uecc_curve ) ) + if( !uECC_make_key( p, ssl->handshake->ecdh_privkey ) ) { return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED ); } diff --git a/library/ssl_srv.c b/library/ssl_srv.c index d3bcd807a..43ca2cac3 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3279,9 +3279,6 @@ static int ssl_prepare_server_key_exchange( mbedtls_ssl_context *ssl, unsigned char *dig_signed = NULL; #endif /* MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED */ #endif /* MBEDTLS_KEY_EXCHANGE__SOME_PFS__ENABLED */ -#if defined(MBEDTLS_USE_TINYCRYPT) - uECC_Curve uecc_curve = uECC_secp256r1(); -#endif (void) ciphersuite_info; /* unused in some configurations */ #if !defined(MBEDTLS_KEY_EXCHANGE__WITH_SERVER_SIGNATURE__ENABLED) @@ -3430,8 +3427,7 @@ static int ssl_prepare_server_key_exchange( mbedtls_ssl_context *ssl, ssl->out_msglen += sizeof( ecdh_param_hdr ); if( !uECC_make_key( &ssl->out_msg[ ssl->out_msglen ], - ssl->handshake->ecdh_privkey, - uecc_curve ) ) + ssl->handshake->ecdh_privkey ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Key creation failed" ) ); return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index bf24a98ab..2fc569c2f 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1973,13 +1973,11 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) { - uECC_Curve uecc_curve = uECC_secp256r1(); ((void) ret); if( !uECC_shared_secret( ssl->handshake->ecdh_peerkey, ssl->handshake->ecdh_privkey, - ssl->handshake->premaster, - uecc_curve ) ) + ssl->handshake->premaster ) ) { return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED ); } @@ -2170,13 +2168,11 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch size_t zlen; #if defined(MBEDTLS_USE_TINYCRYPT) - uECC_Curve uecc_curve = uECC_secp256r1(); ((void) ret); if( !uECC_shared_secret( ssl->handshake->ecdh_peerkey, ssl->handshake->ecdh_privkey, - p + 2, - uecc_curve ) ) + p + 2 ) ) { return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED ); } diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 2fd48c2a4..94e10bab9 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -35,8 +35,7 @@ static int pk_genkey( mbedtls_pk_context *pk ) int ret; ret = uECC_make_key( mbedtls_pk_uecc( *pk )->public_key, - mbedtls_pk_uecc( *pk )->private_key, - uECC_secp256r1() ); + mbedtls_pk_uecc( *pk )->private_key ); if( ret == 0 ) return( -1 ); diff --git a/tests/suites/test_suite_pkparse.function b/tests/suites/test_suite_pkparse.function index a4d9466c4..5563d60bc 100644 --- a/tests/suites/test_suite_pkparse.function +++ b/tests/suites/test_suite_pkparse.function @@ -93,8 +93,7 @@ void pk_parse_public_keyfile_ec( char * key_file, int result ) TEST_ASSERT( mbedtls_ecp_check_pubkey( &eckey->grp, &eckey->Q ) == 0 ); #else uecckey = mbedtls_pk_uecc( ctx ); - TEST_ASSERT( uECC_valid_public_key( uecckey->public_key, - uECC_secp256r1() ) == 0 ); + TEST_ASSERT( uECC_valid_public_key( uecckey->public_key ) == 0 ); #endif /* MBEDTLS_USE_TINYCRYPT */ } @@ -136,11 +135,9 @@ void pk_parse_keyfile_ec( char * key_file, char * password, int result ) TEST_ASSERT( mbedtls_ecp_check_privkey( &eckey->grp, &eckey->d ) == 0 ); #else uecckey = mbedtls_pk_uecc( ctx ); - TEST_ASSERT( uECC_valid_public_key( uecckey->public_key, - uECC_secp256r1() ) == 0 ); + TEST_ASSERT( uECC_valid_public_key( uecckey->public_key ) == 0 ); TEST_ASSERT( uECC_compute_public_key( uecckey->private_key, - tmp_pubkey, - uECC_secp256r1() ) != 0 ); + tmp_pubkey ) != 0 ); TEST_ASSERT( memcmp( tmp_pubkey, uecckey->public_key, sizeof( tmp_pubkey ) ) == 0 ); #endif /* MBEDTLS_USE_TINYCRYPT */ diff --git a/tests/suites/test_suite_tinycrypt.function b/tests/suites/test_suite_tinycrypt.function index 51e3e32d5..3e41720ee 100644 --- a/tests/suites/test_suite_tinycrypt.function +++ b/tests/suites/test_suite_tinycrypt.function @@ -21,17 +21,15 @@ void test_ecdh() uint8_t secret1[NUM_ECC_BYTES] = {0}; uint8_t secret2[NUM_ECC_BYTES] = {0}; - uECC_Curve curve = uECC_secp256r1(); - uECC_set_rng( &uecc_rng_wrapper ); - TEST_ASSERT( uECC_make_key( public1, private1, curve ) != 0 ); + TEST_ASSERT( uECC_make_key( public1, private1 ) != 0 ); - TEST_ASSERT( uECC_make_key( public2, private2, curve ) != 0 ); + TEST_ASSERT( uECC_make_key( public2, private2 ) != 0 ); - TEST_ASSERT( uECC_shared_secret( public2, private1, secret1, curve ) != 0 ); + TEST_ASSERT( uECC_shared_secret( public2, private1, secret1 ) != 0 ); - TEST_ASSERT( uECC_shared_secret( public1, private2, secret2, curve ) != 0 ); + TEST_ASSERT( uECC_shared_secret( public1, private2, secret2 ) != 0 ); TEST_ASSERT( memcmp( secret1, secret2, sizeof( secret1 ) ) == 0 ); } @@ -45,17 +43,15 @@ void test_ecdsa() uint8_t hash[NUM_ECC_BYTES] = {0}; uint8_t sig[2*NUM_ECC_BYTES] = {0}; - uECC_Curve curve = uECC_secp256r1(); - uECC_set_rng( &uecc_rng_wrapper ); TEST_ASSERT( rnd_std_rand( NULL, hash, NUM_ECC_BYTES ) == 0 ); - TEST_ASSERT( uECC_make_key( public, private, curve ) != 0 ); + TEST_ASSERT( uECC_make_key( public, private ) != 0 ); - TEST_ASSERT( uECC_sign( private, hash, sizeof( hash ), sig, curve ) != 0 ); + TEST_ASSERT( uECC_sign( private, hash, sizeof( hash ), sig ) != 0 ); - TEST_ASSERT( uECC_verify( public, hash, sizeof( hash ), sig, curve ) == UECC_SUCCESS ); + TEST_ASSERT( uECC_verify( public, hash, sizeof( hash ), sig ) == UECC_SUCCESS ); } /* END_CASE */ @@ -64,7 +60,6 @@ void ecdh_primitive_testvec( data_t * private1, data_t * xA_str, data_t * yA_str, data_t * private2, data_t * xB_str, data_t * yB_str, data_t * z_str ) { - uECC_Curve curve = uECC_secp256r1(); uint8_t public1[2*NUM_ECC_BYTES] = {0}; uint8_t public2[2*NUM_ECC_BYTES] = {0}; uint8_t secret1[NUM_ECC_BYTES] = {0}; @@ -76,9 +71,9 @@ void ecdh_primitive_testvec( data_t * private1, data_t * xA_str, memcpy( public2 + NUM_ECC_BYTES, yB_str->x, yB_str->len ); // Compute shared secrets and compare to test vector secret - TEST_ASSERT( uECC_shared_secret( public2, private1->x, secret1, curve ) != 0 ); + TEST_ASSERT( uECC_shared_secret( public2, private1->x, secret1 ) != 0 ); - TEST_ASSERT( uECC_shared_secret( public1, private2->x, secret2, curve ) != 0 ); + TEST_ASSERT( uECC_shared_secret( public1, private2->x, secret2 ) != 0 ); TEST_ASSERT( memcmp( secret1, secret2, sizeof( secret1 ) ) == 0 ); TEST_ASSERT( memcmp( secret1, z_str->x, sizeof( secret1 ) ) == 0 ); @@ -90,7 +85,6 @@ void ecdh_primitive_testvec( data_t * private1, data_t * xA_str, void ecdsa_primitive_testvec( data_t * xQ_str, data_t * yQ_str, data_t * hash, data_t * r_str, data_t * s_str ) { - uECC_Curve curve = uECC_secp256r1(); uint8_t pub_bytes[2*NUM_ECC_BYTES] = {0}; uint8_t sig_bytes[2*NUM_ECC_BYTES] = {0}; @@ -100,7 +94,7 @@ void ecdsa_primitive_testvec( data_t * xQ_str, data_t * yQ_str, memcpy( sig_bytes + NUM_ECC_BYTES, s_str->x, r_str->len ); TEST_ASSERT( uECC_verify( pub_bytes, hash->x, hash->len, - sig_bytes, curve ) == UECC_SUCCESS ); + sig_bytes ) == UECC_SUCCESS ); // Alter the signature and check the verification fails for( int i = 0; i < 2*NUM_ECC_BYTES; i++ ) @@ -108,7 +102,7 @@ void ecdsa_primitive_testvec( data_t * xQ_str, data_t * yQ_str, uint8_t temp = sig_bytes[i]; sig_bytes[i] = ( sig_bytes[i] + 1 ) % 256; TEST_ASSERT( uECC_verify( pub_bytes, hash->x, hash->len, - sig_bytes, curve ) == UECC_FAILURE ); + sig_bytes ) == UECC_FAILURE ); sig_bytes[i] = temp; } diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index 0c53f9dd6..62517df5a 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -116,15 +116,13 @@ uECC_RNG_Function uECC_get_rng(void) return g_rng_function; } -int uECC_curve_private_key_size(uECC_Curve curve) +int uECC_curve_private_key_size(void) { - (void) curve; return BITS_TO_BYTES(NUM_ECC_BITS); } -int uECC_curve_public_key_size(uECC_Curve curve) +int uECC_curve_public_key_size(void) { - (void) curve; return 2 * NUM_ECC_BYTES; } @@ -672,11 +670,6 @@ static void x_side_default(uECC_word_t *result, uECC_vli_modAdd(result, result, curve_b, curve_p); } -uECC_Curve uECC_secp256r1(void) -{ - return curve_secp256r1; -} - void vli_mmod_fast_secp256r1(unsigned int *result, unsigned int*product) { unsigned int tmp[NUM_ECC_WORDS]; @@ -952,7 +945,7 @@ static uECC_word_t regularize_k(const uECC_word_t * const k, uECC_word_t *k0, } int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, - const uECC_word_t * scalar, uECC_Curve curve) + const uECC_word_t * scalar) { uECC_word_t tmp[NUM_ECC_WORDS]; uECC_word_t s[NUM_ECC_WORDS]; @@ -962,9 +955,6 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, uECC_word_t *initial_Z = 0; int r; - if (curve != uECC_secp256r1()) - return 0; - /* Protects against invalid curves attacks */ if (uECC_valid_point(point) != 0 ) { return 0; @@ -1005,10 +995,9 @@ clear_and_out: } uECC_word_t EccPoint_compute_public_key(uECC_word_t *result, - uECC_word_t *private_key, - uECC_Curve curve) + uECC_word_t *private_key) { - return EccPoint_mult_safer(result, curve_G, private_key, curve); + return EccPoint_mult_safer(result, curve_G, private_key); } /* Converts an integer in uECC native format to big-endian bytes. */ @@ -1106,8 +1095,7 @@ int uECC_valid_public_key(const uint8_t *public_key) return uECC_valid_point(_public); } -int uECC_compute_public_key(const uint8_t *private_key, uint8_t *public_key, - uECC_Curve curve) +int uECC_compute_public_key(const uint8_t *private_key, uint8_t *public_key) { uECC_word_t _private[NUM_ECC_WORDS]; @@ -1128,7 +1116,7 @@ int uECC_compute_public_key(const uint8_t *private_key, uint8_t *public_key, } /* Compute public key. */ - if (!EccPoint_compute_public_key(_public, _private, curve)) { + if (!EccPoint_compute_public_key(_public, _private)) { return 0; } diff --git a/tinycrypt/ecc_dh.c b/tinycrypt/ecc_dh.c index fc429fe47..9fe03ca9a 100644 --- a/tinycrypt/ecc_dh.c +++ b/tinycrypt/ecc_dh.c @@ -73,7 +73,7 @@ #include "mbedtls/platform_util.h" int uECC_make_key_with_d(uint8_t *public_key, uint8_t *private_key, - unsigned int *d, uECC_Curve curve) + unsigned int *d) { uECC_word_t _private[NUM_ECC_WORDS]; @@ -85,7 +85,7 @@ int uECC_make_key_with_d(uint8_t *public_key, uint8_t *private_key, mbedtls_platform_memcpy (_private, d, NUM_ECC_BYTES); /* Computing public-key from private: */ - if (EccPoint_compute_public_key(_public, _private, curve)) { + if (EccPoint_compute_public_key(_public, _private)) { /* Converting buffers to correct bit order: */ uECC_vli_nativeToBytes(private_key, @@ -106,7 +106,7 @@ int uECC_make_key_with_d(uint8_t *public_key, uint8_t *private_key, return 0; } -int uECC_make_key(uint8_t *public_key, uint8_t *private_key, uECC_Curve curve) +int uECC_make_key(uint8_t *public_key, uint8_t *private_key) { uECC_word_t _random[NUM_ECC_WORDS * 2]; @@ -126,7 +126,7 @@ int uECC_make_key(uint8_t *public_key, uint8_t *private_key, uECC_Curve curve) uECC_vli_mmod(_private, _random, curve_n); /* Computing public-key from private: */ - if (EccPoint_compute_public_key(_public, _private, curve)) { + if (EccPoint_compute_public_key(_public, _private)) { /* Converting buffers to correct bit order: */ uECC_vli_nativeToBytes(private_key, @@ -149,7 +149,7 @@ int uECC_make_key(uint8_t *public_key, uint8_t *private_key, uECC_Curve curve) } int uECC_shared_secret(const uint8_t *public_key, const uint8_t *private_key, - uint8_t *secret, uECC_Curve curve) + uint8_t *secret) { uECC_word_t _public[NUM_ECC_WORDS * 2]; @@ -169,7 +169,7 @@ int uECC_shared_secret(const uint8_t *public_key, const uint8_t *private_key, public_key + num_bytes, num_bytes); - r = EccPoint_mult_safer(_public, _public, _private, curve); + r = EccPoint_mult_safer(_public, _public, _private); uECC_vli_nativeToBytes(secret, num_bytes, _public); /* erasing temporary buffer used to store secret: */ diff --git a/tinycrypt/ecc_dsa.c b/tinycrypt/ecc_dsa.c index 0f7a9fd2f..87fa53bf3 100644 --- a/tinycrypt/ecc_dsa.c +++ b/tinycrypt/ecc_dsa.c @@ -76,7 +76,7 @@ static uECC_RNG_Function g_rng_function = 0; #endif static void bits2int(uECC_word_t *native, const uint8_t *bits, - unsigned bits_size, uECC_Curve curve) + unsigned bits_size) { unsigned num_n_bytes = BITS_TO_BYTES(NUM_ECC_BITS); unsigned num_n_words = BITS_TO_WORDS(NUM_ECC_BITS); @@ -84,8 +84,6 @@ static void bits2int(uECC_word_t *native, const uint8_t *bits, uECC_word_t carry; uECC_word_t *ptr; - (void) curve; - if (bits_size > num_n_bytes) { bits_size = num_n_bytes; } @@ -111,8 +109,7 @@ static void bits2int(uECC_word_t *native, const uint8_t *bits, } int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, - unsigned hash_size, uECC_word_t *k, uint8_t *signature, - uECC_Curve curve) + unsigned hash_size, uECC_word_t *k, uint8_t *signature) { uECC_word_t tmp[NUM_ECC_WORDS]; @@ -128,7 +125,7 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, return 0; } - r = EccPoint_mult_safer(p, curve_G, k, curve); + r = EccPoint_mult_safer(p, curve_G, k); if (r == 0 || uECC_vli_isZero(p)) { return 0; } @@ -158,7 +155,7 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, uECC_vli_set(s, p); uECC_vli_modMult(s, tmp, s, curve_n); /* s = r*d */ - bits2int(tmp, message_hash, hash_size, curve); + bits2int(tmp, message_hash, hash_size); uECC_vli_modAdd(s, tmp, s, curve_n); /* s = e + r*d */ uECC_vli_modMult(s, s, k, curve_n); /* s = (e + r*d) / k */ if (uECC_vli_numBits(s) > (bitcount_t)NUM_ECC_BYTES * 8) { @@ -170,7 +167,7 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, } int uECC_sign(const uint8_t *private_key, const uint8_t *message_hash, - unsigned hash_size, uint8_t *signature, uECC_Curve curve) + unsigned hash_size, uint8_t *signature) { uECC_word_t _random[2*NUM_ECC_WORDS]; uECC_word_t k[NUM_ECC_WORDS]; @@ -187,8 +184,7 @@ int uECC_sign(const uint8_t *private_key, const uint8_t *message_hash, // computing k as modular reduction of _random (see FIPS 186.4 B.5.1): uECC_vli_mmod(k, _random, curve_n); - if (uECC_sign_with_k(private_key, message_hash, hash_size, k, signature, - curve)) { + if (uECC_sign_with_k(private_key, message_hash, hash_size, k, signature)) { return 1; } } @@ -201,8 +197,7 @@ static bitcount_t smax(bitcount_t a, bitcount_t b) } int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, - unsigned hash_size, const uint8_t *signature, - uECC_Curve curve) + unsigned hash_size, const uint8_t *signature) { uECC_word_t u1[NUM_ECC_WORDS], u2[NUM_ECC_WORDS]; @@ -224,9 +219,6 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, wordcount_t num_words = NUM_ECC_WORDS; wordcount_t num_n_words = BITS_TO_WORDS(NUM_ECC_BITS); - if (curve != uECC_secp256r1()) - return 0; - rx[num_n_words - 1] = 0; r[num_n_words - 1] = 0; s[num_n_words - 1] = 0; @@ -251,7 +243,7 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, /* Calculate u1 and u2. */ uECC_vli_modInv(z, s, curve_n); /* z = 1/s */ u1[num_n_words - 1] = 0; - bits2int(u1, message_hash, hash_size, curve); + bits2int(u1, message_hash, hash_size); uECC_vli_modMult(u1, u1, z, curve_n); /* u1 = e/s */ uECC_vli_modMult(u2, r, z, curve_n); /* u2 = r/s */ From 2b90961b8da6e79a004e467cd9d6d5b9a60a22d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Nov 2019 13:37:00 +0100 Subject: [PATCH 15/23] Add integrity check for curve parameters We don't really need a secure hash for that, something like CRC32 would probably be enough - but we have SHA-256 handy, not CRC32, so use that for the sake of simplicity. --- include/mbedtls/check_config.h | 4 ++ include/mbedtls/config.h | 1 + tinycrypt/ecc.c | 79 ++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index fe9c5945e..ce348d906 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -110,6 +110,10 @@ #error "MBEDTLS_USE_TINYCRYPT defined, but it cannot be defined with MBEDTLS_NO_64BIT_MULTIPLICATION" #endif +#if defined(MBEDTLS_USE_TINYCRYPT) && !defined(MBEDTLS_SHA256_C) +#error "MBEDTLS_USE_TINYCRYPT defined, but not MBEDTLS_SHA256_C" +#endif + #if defined(MBEDTLS_USE_TINYCRYPT) && \ !( defined(MBEDTLS_SSL_CONF_SINGLE_EC) && \ MBEDTLS_SSL_CONF_SINGLE_EC_TLS_ID == 23 && \ diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 87012dab5..dbb84b449 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -2649,6 +2649,7 @@ * MBEDTLS_SSL_CONF_SINGLE_EC * MBEDTLS_SSL_CONF_SINGLE_EC_TLS_ID == 23 * MBEDTLS_SSL_CONF_SINGLE_UECC_GRP_ID == MBEDTLS_UECC_DP_SECP256R1 + * MBEDTLS_SHA256_C * * \see MBEDTLS_SSL_CONF_RNG * diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index 62517df5a..381beff54 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -66,6 +66,7 @@ #if defined(MBEDTLS_USE_TINYCRYPT) #include #include "mbedtls/platform_util.h" +#include "mbedtls/sha256.h" #include /* Parameters for curve NIST P-256 aka secp256r1 */ @@ -98,6 +99,73 @@ const uECC_word_t curve_b[NUM_ECC_WORDS] = { BYTES_TO_WORDS_8(E7, 93, 3A, AA, D8, 35, C6, 5A) }; +static int uECC_update_param_sha256(mbedtls_sha256_context *ctx, + const uECC_word_t val[NUM_ECC_WORDS]) +{ + uint8_t bytes[NUM_ECC_BYTES]; + + uECC_vli_nativeToBytes(bytes, NUM_ECC_BYTES, val); + return mbedtls_sha256_update_ret(ctx, bytes, NUM_ECC_BYTES); +} + +static int uECC_compute_param_sha256(unsigned char output[32]) +{ + int ret = UECC_FAILURE; + mbedtls_sha256_context ctx; + + mbedtls_sha256_init( &ctx ); + + if (mbedtls_sha256_starts_ret(&ctx, 0) != 0) { + goto exit; + } + + if (uECC_update_param_sha256(&ctx, curve_p) != 0 || + uECC_update_param_sha256(&ctx, curve_n) != 0 || + uECC_update_param_sha256(&ctx, curve_G) != 0 || + uECC_update_param_sha256(&ctx, curve_G + NUM_ECC_WORDS) != 0 || + uECC_update_param_sha256(&ctx, curve_b) != 0) + { + goto exit; + } + + if (mbedtls_sha256_finish_ret(&ctx, output) != 0) { + goto exit; + } + + ret = UECC_SUCCESS; + +exit: + mbedtls_sha256_free( &ctx ); + + return ret; +} + +/* + * Check integrity of curve parameters. + * Return 0 if everything's OK, non-zero otherwise. + */ +static int uECC_check_curve_integrity(void) +{ + unsigned char computed[32]; + unsigned char reference[32] = { + 0x2d, 0xa1, 0xa4, 0x64, 0x45, 0x28, 0x0d, 0xe1, + 0x93, 0xf9, 0x29, 0x2f, 0xac, 0x3e, 0xe2, 0x92, + 0x76, 0x0a, 0xe2, 0xbc, 0xce, 0x2a, 0xa2, 0xc6, + 0x38, 0xf2, 0x19, 0x1d, 0x76, 0x72, 0x93, 0x49, + }; + volatile unsigned char diff = 0; + unsigned char i; + + if (uECC_compute_param_sha256(computed) != UECC_SUCCESS) { + return UECC_FAILURE; + } + + for (i = 0; i < 32; i++) + diff |= computed[i] ^ reference[i]; + + return diff; +} + /* IMPORTANT: Make sure a cryptographically-secure PRNG is set and the platform * has access to enough entropy in order to feed the PRNG regularly. */ #if default_RNG_defined @@ -955,6 +1023,11 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, uECC_word_t *initial_Z = 0; int r; + /* Protect against faults modifying curve paremeters in flash */ + if (uECC_check_curve_integrity() != 0) { + return 0; + } + /* Protects against invalid curves attacks */ if (uECC_valid_point(point) != 0 ) { return 0; @@ -983,6 +1056,12 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, goto clear_and_out; } + /* Protect against faults modifying curve paremeters in flash */ + if (uECC_check_curve_integrity() != 0) { + r = 0; + goto clear_and_out; + } + r = 1; clear_and_out: From 4d6186beb0bca06096be6424daa2c618fb787c30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 25 Nov 2019 10:53:24 +0100 Subject: [PATCH 16/23] Rename ATTACK_DETECTED to FAULT_DETECTED We don't know for sure it's an attack, it could be the hardware failing randomly as well. --- include/tinycrypt/ecc.h | 2 +- library/pk.c | 2 +- tinycrypt/ecc_dsa.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/tinycrypt/ecc.h b/include/tinycrypt/ecc.h index d50183813..b19958944 100644 --- a/include/tinycrypt/ecc.h +++ b/include/tinycrypt/ecc.h @@ -88,7 +88,7 @@ extern "C" { * attacks flipping a low number of bits. */ #define UECC_SUCCESS 0 #define UECC_FAILURE 0x75555555 -#define UECC_ATTACK_DETECTED 0x7aaaaaaa +#define UECC_FAULT_DETECTED 0x7aaaaaaa /* Word size (4 bytes considering 32-bits architectures) */ #define uECC_WORD_SIZE 4 diff --git a/library/pk.c b/library/pk.c index 05ffe1c70..fbe711dc2 100644 --- a/library/pk.c +++ b/library/pk.c @@ -592,7 +592,7 @@ static int uecc_eckey_verify_wrap( void *ctx, mbedtls_md_type_t md_alg, ret_fi = uECC_verify( keypair->public_key, hash, (unsigned) hash_len, signature ); - if( ret_fi == UECC_ATTACK_DETECTED ) + if( ret_fi == UECC_FAULT_DETECTED ) return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); if( ret_fi == UECC_SUCCESS ) diff --git a/tinycrypt/ecc_dsa.c b/tinycrypt/ecc_dsa.c index 87fa53bf3..0d6683be0 100644 --- a/tinycrypt/ecc_dsa.c +++ b/tinycrypt/ecc_dsa.c @@ -304,7 +304,7 @@ int uECC_verify(const uint8_t *public_key, const uint8_t *message_hash, return UECC_SUCCESS; } else { - return UECC_ATTACK_DETECTED; + return UECC_FAULT_DETECTED; } } From 9d6a535ba113ebf839f551c1bdaff7d3df50df68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 25 Nov 2019 13:06:05 +0100 Subject: [PATCH 17/23] Return and propagate UECC_FAULT_DETECTED This commit first changes the return convention of EccPoint_mult_safer() so that it properly reports when faults are detected. Then all functions that call it need to be changed to (1) follow the same return convention and (2) properly propagate UECC_FAULT_DETECTED when it occurs. Here's the reverse call graph from EccPoint_mult_safer() to the rest of the library (where return values are translated to the MBEDTLS_ERR_ space) and test functions (where expected return values are asserted explicitly). EccPoint_mult_safer() EccPoint_compute_public_key() uECC_compute_public_key() pkparse.c tests/suites/test_suite_pkparse.function uECC_make_key_with_d() uECC_make_key() ssl_cli.c ssl_srv.c tests/suites/test_suite_pk.function tests/suites/test_suite_tinycrypt.function uECC_shared_secret() ssl_tls.c tests/suites/test_suite_tinycrypt.function uECC_sign_with_k() uECC_sign() pk.c tests/suites/test_suite_tinycrypt.function Note: in uECC_sign_with_k() a test for uECC_vli_isZero(p) is suppressed because it is redundant with a more thorough test (point validity) done at the end of EccPoint_mult_safer(). This redundancy was introduced in a previous commit but not noticed earlier. --- include/tinycrypt/ecc.h | 4 +- include/tinycrypt/ecc_dh.h | 6 +-- include/tinycrypt/ecc_dsa.h | 3 +- library/pk.c | 5 +- library/pkparse.c | 4 +- library/ssl_cli.c | 16 +++---- library/ssl_srv.c | 13 ++--- library/ssl_tls.c | 26 +++++----- tests/suites/test_suite_pk.function | 2 +- tests/suites/test_suite_pkparse.function | 2 +- tests/suites/test_suite_tinycrypt.function | 16 +++---- tinycrypt/ecc.c | 29 ++++++------ tinycrypt/ecc_dh.c | 55 ++++++++++++---------- tinycrypt/ecc_dsa.c | 27 +++++++---- 14 files changed, 112 insertions(+), 96 deletions(-) diff --git a/include/tinycrypt/ecc.h b/include/tinycrypt/ecc.h index b19958944..ebd16d6fe 100644 --- a/include/tinycrypt/ecc.h +++ b/include/tinycrypt/ecc.h @@ -217,7 +217,7 @@ int uECC_curve_public_key_size(void); * @param private_key IN -- The private key to compute the public key for * @param public_key OUT -- Will be filled in with the corresponding public key * @param curve - * @return Returns 1 if key was computed successfully, 0 if an error occurred. + * @return UECC_SUCCESS or UECC_FAILURE or UECC_FAULT_DETECTED */ int uECC_compute_public_key(const uint8_t *private_key, uint8_t *public_key); @@ -228,6 +228,7 @@ int uECC_compute_public_key(const uint8_t *private_key, * @param result OUT -- public-key * @param private_key IN -- private-key * @param curve IN -- elliptic curve + * @return UECC_SUCCESS or UECC_FAILURE or UECC_FAULT_DETECTED */ uECC_word_t EccPoint_compute_public_key(uECC_word_t *result, uECC_word_t *private_key); @@ -241,6 +242,7 @@ uECC_word_t EccPoint_compute_public_key(uECC_word_t *result, * @param result OUT -- returns scalar*point * @param point IN -- elliptic curve point * @param scalar IN -- scalar + * @return UECC_SUCCESS or UECC_FAILURE or UECC_FAULT_DETECTED */ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, const uECC_word_t * scalar); diff --git a/include/tinycrypt/ecc_dh.h b/include/tinycrypt/ecc_dh.h index 61e4c52ba..f471349e4 100644 --- a/include/tinycrypt/ecc_dh.h +++ b/include/tinycrypt/ecc_dh.h @@ -83,8 +83,7 @@ extern "C" { /** * @brief Create a public/private key pair. - * @return returns TC_CRYPTO_SUCCESS (1) if the key pair was generated successfully - * returns TC_CRYPTO_FAIL (0) if error while generating key pair + * @return UECC_SUCCESS or UECC_FAILURE or UECC_FAULT_DETECTED * * @param p_public_key OUT -- Will be filled in with the public key. Must be at * least 2 * the curve size (in bytes) long. For curve secp256r1, p_public_key @@ -114,8 +113,7 @@ int uECC_make_key_with_d(uint8_t *p_public_key, uint8_t *p_private_key, /** * @brief Compute a shared secret given your secret key and someone else's * public key. - * @return returns TC_CRYPTO_SUCCESS (1) if the shared secret was computed successfully - * returns TC_CRYPTO_FAIL (0) otherwise + * @return UECC_SUCCESS or UECC_FAILURE or UECC_FAULT_DETECTED * * @param p_secret OUT -- Will be filled in with the shared secret value. Must be * the same size as the curve size (for curve secp256r1, secret must be 32 bytes diff --git a/include/tinycrypt/ecc_dsa.h b/include/tinycrypt/ecc_dsa.h index 5985c7f0c..259cf44b3 100644 --- a/include/tinycrypt/ecc_dsa.h +++ b/include/tinycrypt/ecc_dsa.h @@ -92,8 +92,7 @@ extern "C" { /** * @brief Generate an ECDSA signature for a given hash value. - * @return returns TC_CRYPTO_SUCCESS (1) if the signature generated successfully - * returns TC_CRYPTO_FAIL (0) if an error occurred. + * @return UECC_SUCCESS or UECC_FAILURE or UECC_FAULT_DETECTED * * @param p_private_key IN -- Your private key. * @param p_message_hash IN -- The hash of the message to sign. diff --git a/library/pk.c b/library/pk.c index fbe711dc2..dfdd4d143 100644 --- a/library/pk.c +++ b/library/pk.c @@ -723,8 +723,9 @@ static int uecc_eckey_sign_wrap( void *ctx, mbedtls_md_type_t md_alg, #define MAX_SECP256R1_ECDSA_SIG_LEN ( 3 + 2 * ( 3 + NUM_ECC_BYTES ) ) ret = uECC_sign( keypair->private_key, hash, hash_len, sig ); - /* TinyCrypt uses 0 to signal errors. */ - if( ret == 0 ) + if( ret == UECC_FAULT_DETECTED ) + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + if( ret != UECC_SUCCESS ) return( MBEDTLS_ERR_PK_HW_ACCEL_FAILED ); *sig_len = 2 * NUM_ECC_BYTES; diff --git a/library/pkparse.c b/library/pkparse.c index 6a2507aa3..3f202259f 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -987,7 +987,9 @@ static int pk_parse_key_sec1_der( mbedtls_uecc_keypair *keypair, { ret = uECC_compute_public_key( keypair->private_key, keypair->public_key ); - if( ret == 0 ) + if( ret == UECC_FAULT_DETECTED ) + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + if( ret != UECC_SUCCESS ) return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT ); } diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 3a5671e64..7c03b41f9 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3568,7 +3568,6 @@ static int ssl_out_client_key_exchange_write( mbedtls_ssl_context *ssl, { ((void) n); - ((void) ret); if( (size_t)( end - p ) < 2 * NUM_ECC_BYTES + 2 ) return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); @@ -3576,10 +3575,11 @@ static int ssl_out_client_key_exchange_write( mbedtls_ssl_context *ssl, *p++ = 2 * NUM_ECC_BYTES + 1; *p++ = 0x04; /* uncompressed point presentation */ - if( !uECC_make_key( p, ssl->handshake->ecdh_privkey ) ) - { + ret = uECC_make_key( p, ssl->handshake->ecdh_privkey ); + if( ret == UECC_FAULT_DETECTED ) + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + if( ret != UECC_SUCCESS ) return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED ); - } p += 2 * NUM_ECC_BYTES; } else @@ -3717,7 +3717,6 @@ static int ssl_out_client_key_exchange_write( mbedtls_ssl_context *ssl, { #if defined(MBEDTLS_USE_TINYCRYPT) ((void) n); - ((void) ret); if( (size_t)( end - p ) < 2 * NUM_ECC_BYTES + 2 ) return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); @@ -3725,10 +3724,11 @@ static int ssl_out_client_key_exchange_write( mbedtls_ssl_context *ssl, *p++ = 2 * NUM_ECC_BYTES + 1; *p++ = 0x04; /* uncompressed point presentation */ - if( !uECC_make_key( p, ssl->handshake->ecdh_privkey ) ) - { + ret = uECC_make_key( p, ssl->handshake->ecdh_privkey ); + if( ret == UECC_FAULT_DETECTED ) + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + if( ret != UECC_SUCCESS ) return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED ); - } p += 2 * NUM_ECC_BYTES; #else /* MBEDTLS_USE_TINYCRYPT */ /* diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 43ca2cac3..1ef8f9466 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3410,6 +3410,7 @@ static int ssl_prepare_server_key_exchange( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_USE_TINYCRYPT) { + int ret; static const unsigned char ecdh_param_hdr[] = { MBEDTLS_SSL_EC_TLS_NAMED_CURVE, 0 /* high bits of secp256r1 TLS ID */, @@ -3426,12 +3427,12 @@ static int ssl_prepare_server_key_exchange( mbedtls_ssl_context *ssl, ecdh_param_hdr, sizeof( ecdh_param_hdr ) ); ssl->out_msglen += sizeof( ecdh_param_hdr ); - if( !uECC_make_key( &ssl->out_msg[ ssl->out_msglen ], - ssl->handshake->ecdh_privkey ) ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Key creation failed" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } + ret = uECC_make_key( &ssl->out_msg[ ssl->out_msglen ], + ssl->handshake->ecdh_privkey ); + if( ret == UECC_FAULT_DETECTED ) + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + if( ret != UECC_SUCCESS ) + return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED ); ssl->out_msglen += 2*NUM_ECC_BYTES; } diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 2fc569c2f..c12af9684 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1973,14 +1973,13 @@ int mbedtls_ssl_build_pms( mbedtls_ssl_context *ssl ) mbedtls_ssl_suite_get_key_exchange( ciphersuite_info ) == MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA ) { - ((void) ret); - - if( !uECC_shared_secret( ssl->handshake->ecdh_peerkey, - ssl->handshake->ecdh_privkey, - ssl->handshake->premaster ) ) - { + ret = uECC_shared_secret( ssl->handshake->ecdh_peerkey, + ssl->handshake->ecdh_privkey, + ssl->handshake->premaster ); + if( ret == UECC_FAULT_DETECTED ) + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + if( ret != UECC_SUCCESS ) return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED ); - } ssl->handshake->pmslen = NUM_ECC_BYTES; } @@ -2168,14 +2167,13 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch size_t zlen; #if defined(MBEDTLS_USE_TINYCRYPT) - ((void) ret); - - if( !uECC_shared_secret( ssl->handshake->ecdh_peerkey, - ssl->handshake->ecdh_privkey, - p + 2 ) ) - { + ret = uECC_shared_secret( ssl->handshake->ecdh_peerkey, + ssl->handshake->ecdh_privkey, + p + 2 ); + if( ret == UECC_FAULT_DETECTED ) + return( MBEDTLS_ERR_PLATFORM_FAULT_DETECTED ); + if( ret != UECC_SUCCESS ) return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED ); - } zlen = NUM_ECC_BYTES; #else /* MBEDTLS_USE_TINYCRYPT */ diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 94e10bab9..61bf95c0c 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -36,7 +36,7 @@ static int pk_genkey( mbedtls_pk_context *pk ) ret = uECC_make_key( mbedtls_pk_uecc( *pk )->public_key, mbedtls_pk_uecc( *pk )->private_key ); - if( ret == 0 ) + if( ret != UECC_SUCCESS ) return( -1 ); return( 0 ); diff --git a/tests/suites/test_suite_pkparse.function b/tests/suites/test_suite_pkparse.function index 5563d60bc..54fa7af90 100644 --- a/tests/suites/test_suite_pkparse.function +++ b/tests/suites/test_suite_pkparse.function @@ -137,7 +137,7 @@ void pk_parse_keyfile_ec( char * key_file, char * password, int result ) uecckey = mbedtls_pk_uecc( ctx ); TEST_ASSERT( uECC_valid_public_key( uecckey->public_key ) == 0 ); TEST_ASSERT( uECC_compute_public_key( uecckey->private_key, - tmp_pubkey ) != 0 ); + tmp_pubkey ) == UECC_SUCCESS ); TEST_ASSERT( memcmp( tmp_pubkey, uecckey->public_key, sizeof( tmp_pubkey ) ) == 0 ); #endif /* MBEDTLS_USE_TINYCRYPT */ diff --git a/tests/suites/test_suite_tinycrypt.function b/tests/suites/test_suite_tinycrypt.function index 3e41720ee..3247e054d 100644 --- a/tests/suites/test_suite_tinycrypt.function +++ b/tests/suites/test_suite_tinycrypt.function @@ -23,13 +23,13 @@ void test_ecdh() uECC_set_rng( &uecc_rng_wrapper ); - TEST_ASSERT( uECC_make_key( public1, private1 ) != 0 ); + TEST_ASSERT( uECC_make_key( public1, private1 ) == UECC_SUCCESS ); - TEST_ASSERT( uECC_make_key( public2, private2 ) != 0 ); + TEST_ASSERT( uECC_make_key( public2, private2 ) == UECC_SUCCESS ); - TEST_ASSERT( uECC_shared_secret( public2, private1, secret1 ) != 0 ); + TEST_ASSERT( uECC_shared_secret( public2, private1, secret1 ) == UECC_SUCCESS ); - TEST_ASSERT( uECC_shared_secret( public1, private2, secret2 ) != 0 ); + TEST_ASSERT( uECC_shared_secret( public1, private2, secret2 ) == UECC_SUCCESS ); TEST_ASSERT( memcmp( secret1, secret2, sizeof( secret1 ) ) == 0 ); } @@ -47,9 +47,9 @@ void test_ecdsa() TEST_ASSERT( rnd_std_rand( NULL, hash, NUM_ECC_BYTES ) == 0 ); - TEST_ASSERT( uECC_make_key( public, private ) != 0 ); + TEST_ASSERT( uECC_make_key( public, private ) == UECC_SUCCESS ); - TEST_ASSERT( uECC_sign( private, hash, sizeof( hash ), sig ) != 0 ); + TEST_ASSERT( uECC_sign( private, hash, sizeof( hash ), sig ) == UECC_SUCCESS ); TEST_ASSERT( uECC_verify( public, hash, sizeof( hash ), sig ) == UECC_SUCCESS ); } @@ -71,9 +71,9 @@ void ecdh_primitive_testvec( data_t * private1, data_t * xA_str, memcpy( public2 + NUM_ECC_BYTES, yB_str->x, yB_str->len ); // Compute shared secrets and compare to test vector secret - TEST_ASSERT( uECC_shared_secret( public2, private1->x, secret1 ) != 0 ); + TEST_ASSERT( uECC_shared_secret( public2, private1->x, secret1 ) == UECC_SUCCESS ); - TEST_ASSERT( uECC_shared_secret( public1, private2->x, secret2 ) != 0 ); + TEST_ASSERT( uECC_shared_secret( public1, private2->x, secret2 ) == UECC_SUCCESS ); TEST_ASSERT( memcmp( secret1, secret2, sizeof( secret1 ) ) == 0 ); TEST_ASSERT( memcmp( secret1, z_str->x, sizeof( secret1 ) ) == 0 ); diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index 381beff54..261db77e0 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -1021,16 +1021,16 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, wordcount_t num_words = NUM_ECC_WORDS; uECC_word_t carry; uECC_word_t *initial_Z = 0; - int r; + int r = UECC_FAULT_DETECTED; /* Protect against faults modifying curve paremeters in flash */ if (uECC_check_curve_integrity() != 0) { - return 0; + return UECC_FAULT_DETECTED; } - /* Protects against invalid curves attacks */ + /* Protects against invalid curve attacks */ if (uECC_valid_point(point) != 0 ) { - return 0; + return UECC_FAILURE; } /* Regularize the bitcount for the private key so that attackers cannot use a @@ -1041,7 +1041,7 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, * protect against side-channel attacks such as Template SPA */ if (g_rng_function) { if (!uECC_generate_random_int(k2[carry], curve_p, num_words)) { - r = 0; + r = UECC_FAILURE; goto clear_and_out; } initial_Z = k2[carry]; @@ -1052,17 +1052,17 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, /* Protect against fault injections that would make the resulting * point not lie on the intended curve */ if (uECC_valid_point(result) != 0 ) { - r = 0; + r = UECC_FAULT_DETECTED; goto clear_and_out; } /* Protect against faults modifying curve paremeters in flash */ if (uECC_check_curve_integrity() != 0) { - r = 0; + r = UECC_FAULT_DETECTED; goto clear_and_out; } - r = 1; + r = UECC_SUCCESS; clear_and_out: /* erasing temporary buffer used to store secret: */ @@ -1176,7 +1176,7 @@ int uECC_valid_public_key(const uint8_t *public_key) int uECC_compute_public_key(const uint8_t *private_key, uint8_t *public_key) { - + int ret; uECC_word_t _private[NUM_ECC_WORDS]; uECC_word_t _public[NUM_ECC_WORDS * 2]; @@ -1187,23 +1187,24 @@ int uECC_compute_public_key(const uint8_t *private_key, uint8_t *public_key) /* Make sure the private key is in the range [1, n-1]. */ if (uECC_vli_isZero(_private)) { - return 0; + return UECC_FAILURE; } if (uECC_vli_cmp(curve_n, _private) != 1) { - return 0; + return UECC_FAILURE; } /* Compute public key. */ - if (!EccPoint_compute_public_key(_public, _private)) { - return 0; + ret = EccPoint_compute_public_key(_public, _private); + if (ret != UECC_SUCCESS) { + return ret; } uECC_vli_nativeToBytes(public_key, NUM_ECC_BYTES, _public); uECC_vli_nativeToBytes( public_key + NUM_ECC_BYTES, NUM_ECC_BYTES, _public + NUM_ECC_WORDS); - return 1; + return UECC_SUCCESS; } #else typedef int mbedtls_dummy_tinycrypt_def; diff --git a/tinycrypt/ecc_dh.c b/tinycrypt/ecc_dh.c index 9fe03ca9a..3070ecf05 100644 --- a/tinycrypt/ecc_dh.c +++ b/tinycrypt/ecc_dh.c @@ -75,7 +75,7 @@ int uECC_make_key_with_d(uint8_t *public_key, uint8_t *private_key, unsigned int *d) { - + int ret; uECC_word_t _private[NUM_ECC_WORDS]; uECC_word_t _public[NUM_ECC_WORDS * 2]; @@ -85,30 +85,32 @@ int uECC_make_key_with_d(uint8_t *public_key, uint8_t *private_key, mbedtls_platform_memcpy (_private, d, NUM_ECC_BYTES); /* Computing public-key from private: */ - if (EccPoint_compute_public_key(_public, _private)) { - - /* Converting buffers to correct bit order: */ - uECC_vli_nativeToBytes(private_key, - BITS_TO_BYTES(NUM_ECC_BITS), - _private); - uECC_vli_nativeToBytes(public_key, - NUM_ECC_BYTES, - _public); - uECC_vli_nativeToBytes(public_key + NUM_ECC_BYTES, - NUM_ECC_BYTES, - _public + NUM_ECC_WORDS); - - /* erasing temporary buffer used to store secret: */ - mbedtls_platform_memset(_private, 0, NUM_ECC_BYTES); - - return 1; + ret = EccPoint_compute_public_key(_public, _private); + if (ret != UECC_SUCCESS) { + goto exit; } - return 0; + + /* Converting buffers to correct bit order: */ + uECC_vli_nativeToBytes(private_key, + BITS_TO_BYTES(NUM_ECC_BITS), + _private); + uECC_vli_nativeToBytes(public_key, + NUM_ECC_BYTES, + _public); + uECC_vli_nativeToBytes(public_key + NUM_ECC_BYTES, + NUM_ECC_BYTES, + _public + NUM_ECC_WORDS); + +exit: + /* erasing temporary buffer used to store secret: */ + mbedtls_platform_memset(_private, 0, NUM_ECC_BYTES); + + return ret; } int uECC_make_key(uint8_t *public_key, uint8_t *private_key) { - + int ret; uECC_word_t _random[NUM_ECC_WORDS * 2]; uECC_word_t _private[NUM_ECC_WORDS]; uECC_word_t _public[NUM_ECC_WORDS * 2]; @@ -119,14 +121,19 @@ int uECC_make_key(uint8_t *public_key, uint8_t *private_key) uECC_RNG_Function rng_function = uECC_get_rng(); if (!rng_function || !rng_function((uint8_t *)_random, 2 * NUM_ECC_WORDS*uECC_WORD_SIZE)) { - return 0; + return UECC_FAILURE; } /* computing modular reduction of _random (see FIPS 186.4 B.4.1): */ uECC_vli_mmod(_private, _random, curve_n); /* Computing public-key from private: */ - if (EccPoint_compute_public_key(_public, _private)) { + ret = EccPoint_compute_public_key(_public, _private); + /* don't try again if a fault was detected */ + if (ret == UECC_FAULT_DETECTED) { + return ret; + } + if (ret == UECC_SUCCESS) { /* Converting buffers to correct bit order: */ uECC_vli_nativeToBytes(private_key, @@ -142,10 +149,10 @@ int uECC_make_key(uint8_t *public_key, uint8_t *private_key) /* erasing temporary buffer that stored secret: */ mbedtls_platform_memset(_private, 0, NUM_ECC_BYTES); - return 1; + return UECC_SUCCESS; } } - return 0; + return UECC_FAILURE; } int uECC_shared_secret(const uint8_t *public_key, const uint8_t *private_key, diff --git a/tinycrypt/ecc_dsa.c b/tinycrypt/ecc_dsa.c index 0d6683be0..9ed6941ee 100644 --- a/tinycrypt/ecc_dsa.c +++ b/tinycrypt/ecc_dsa.c @@ -122,12 +122,12 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, /* Make sure 0 < k < curve_n */ if (uECC_vli_isZero(k) || uECC_vli_cmp(curve_n, k) != 1) { - return 0; + return UECC_FAILURE; } r = EccPoint_mult_safer(p, curve_G, k); - if (r == 0 || uECC_vli_isZero(p)) { - return 0; + if (r != UECC_SUCCESS) { + return r; } /* If an RNG function was specified, get a random number @@ -137,7 +137,7 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, tmp[0] = 1; } else if (!uECC_generate_random_int(tmp, curve_n, num_n_words)) { - return 0; + return UECC_FAILURE; } /* Prevent side channel analysis of uECC_vli_modInv() to determine @@ -159,16 +159,17 @@ int uECC_sign_with_k(const uint8_t *private_key, const uint8_t *message_hash, uECC_vli_modAdd(s, tmp, s, curve_n); /* s = e + r*d */ uECC_vli_modMult(s, s, k, curve_n); /* s = (e + r*d) / k */ if (uECC_vli_numBits(s) > (bitcount_t)NUM_ECC_BYTES * 8) { - return 0; + return UECC_FAILURE; } uECC_vli_nativeToBytes(signature + NUM_ECC_BYTES, NUM_ECC_BYTES, s); - return 1; + return UECC_SUCCESS; } int uECC_sign(const uint8_t *private_key, const uint8_t *message_hash, unsigned hash_size, uint8_t *signature) { + int r; uECC_word_t _random[2*NUM_ECC_WORDS]; uECC_word_t k[NUM_ECC_WORDS]; uECC_word_t tries; @@ -178,17 +179,23 @@ int uECC_sign(const uint8_t *private_key, const uint8_t *message_hash, uECC_RNG_Function rng_function = uECC_get_rng(); if (!rng_function || !rng_function((uint8_t *)_random, 2*NUM_ECC_WORDS*uECC_WORD_SIZE)) { - return 0; + return UECC_FAILURE; } // computing k as modular reduction of _random (see FIPS 186.4 B.5.1): uECC_vli_mmod(k, _random, curve_n); - if (uECC_sign_with_k(private_key, message_hash, hash_size, k, signature)) { - return 1; + r = uECC_sign_with_k(private_key, message_hash, hash_size, k, signature); + /* don't keep trying if a fault was detected */ + if (r == UECC_FAULT_DETECTED) { + return r; } + if (r == UECC_SUCCESS) { + return UECC_SUCCESS; + } + /* else keep trying */ } - return 0; + return UECC_FAILURE; } static bitcount_t smax(bitcount_t a, bitcount_t b) From 98e1fe07966141045d8e5e1160eab068421f8e1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 27 Nov 2019 11:57:49 +0100 Subject: [PATCH 18/23] Add flow control in uECC_vli_equal loop --- tinycrypt/ecc.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index 261db77e0..d75c4d764 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -278,11 +278,16 @@ uECC_word_t uECC_vli_equal(const uECC_word_t *left, const uECC_word_t *right) { uECC_word_t diff = 0; - wordcount_t i; + volatile int i; for (i = NUM_ECC_WORDS - 1; i >= 0; --i) { diff |= (left[i] ^ right[i]); } + + /* i should be -1 now */ + mbedtls_platform_enforce_volatile_reads(); + diff |= i ^ -1; + return diff; } From 5c3066a4f6612beef34b122e2b2be7fcd71b867b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 27 Nov 2019 12:27:48 +0100 Subject: [PATCH 19/23] Add double-checking in some critical places --- tinycrypt/ecc.c | 49 ++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index d75c4d764..e4e8e0db6 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -1027,16 +1027,31 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, uECC_word_t carry; uECC_word_t *initial_Z = 0; int r = UECC_FAULT_DETECTED; + volatile int problem; /* Protect against faults modifying curve paremeters in flash */ - if (uECC_check_curve_integrity() != 0) { + problem = -1; + problem = uECC_check_curve_integrity(); + if (problem != 0) { + return UECC_FAULT_DETECTED; + } + mbedtls_platform_enforce_volatile_reads(); + if (problem != 0) { return UECC_FAULT_DETECTED; } /* Protects against invalid curve attacks */ - if (uECC_valid_point(point) != 0 ) { + problem = -1; + problem = uECC_valid_point(point); + if (problem != 0) { + /* invalid input, can happen without fault */ return UECC_FAILURE; } + mbedtls_platform_enforce_volatile_reads(); + if (problem != 0) { + /* failure on second check means fault, though */ + return UECC_FAULT_DETECTED; + } /* Regularize the bitcount for the private key so that attackers cannot use a * side channel attack to learn the number of leading zeros. */ @@ -1056,13 +1071,27 @@ int EccPoint_mult_safer(uECC_word_t * result, const uECC_word_t * point, /* Protect against fault injections that would make the resulting * point not lie on the intended curve */ - if (uECC_valid_point(result) != 0 ) { + problem = -1; + problem = uECC_valid_point(result); + if (problem != 0) { + r = UECC_FAULT_DETECTED; + goto clear_and_out; + } + mbedtls_platform_enforce_volatile_reads(); + if (problem != 0) { r = UECC_FAULT_DETECTED; goto clear_and_out; } /* Protect against faults modifying curve paremeters in flash */ - if (uECC_check_curve_integrity() != 0) { + problem = -1; + problem = uECC_check_curve_integrity(); + if (problem != 0) { + r = UECC_FAULT_DETECTED; + goto clear_and_out; + } + mbedtls_platform_enforce_volatile_reads(); + if (problem != 0) { r = UECC_FAULT_DETECTED; goto clear_and_out; } @@ -1139,6 +1168,7 @@ int uECC_valid_point(const uECC_word_t *point) uECC_word_t tmp1[NUM_ECC_WORDS]; uECC_word_t tmp2[NUM_ECC_WORDS]; wordcount_t num_words = NUM_ECC_WORDS; + volatile uECC_word_t diff = -1u; /* The point at infinity is invalid. */ if (EccPoint_isZero(point)) { @@ -1155,10 +1185,15 @@ int uECC_valid_point(const uECC_word_t *point) x_side_default(tmp2, point); /* tmp2 = x^3 + ax + b */ /* Make sure that y^2 == x^3 + ax + b */ - if (uECC_vli_equal(tmp1, tmp2) != 0) - return -3; + diff = uECC_vli_equal(tmp1, tmp2); + if (diff == 0) { + mbedtls_platform_enforce_volatile_reads(); + if (diff == 0) { + return 0; + } + } - return 0; + return -3; } int uECC_valid_public_key(const uint8_t *public_key) From e1cb8846e7648c915b0d25abeb6862ef5bece1f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 28 Nov 2019 12:21:34 +0100 Subject: [PATCH 20/23] Add loop integrity check to curve param check Also make the reference result static const while at it. --- tinycrypt/ecc.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index e4e8e0db6..df7a6928c 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -147,14 +147,14 @@ exit: static int uECC_check_curve_integrity(void) { unsigned char computed[32]; - unsigned char reference[32] = { + static const unsigned char reference[32] = { 0x2d, 0xa1, 0xa4, 0x64, 0x45, 0x28, 0x0d, 0xe1, 0x93, 0xf9, 0x29, 0x2f, 0xac, 0x3e, 0xe2, 0x92, 0x76, 0x0a, 0xe2, 0xbc, 0xce, 0x2a, 0xa2, 0xc6, 0x38, 0xf2, 0x19, 0x1d, 0x76, 0x72, 0x93, 0x49, }; volatile unsigned char diff = 0; - unsigned char i; + volatile unsigned i; if (uECC_compute_param_sha256(computed) != UECC_SUCCESS) { return UECC_FAILURE; @@ -163,6 +163,10 @@ static int uECC_check_curve_integrity(void) for (i = 0; i < 32; i++) diff |= computed[i] ^ reference[i]; + /* i should be 32 */ + mbedtls_platform_enforce_volatile_reads(); + diff |= (unsigned char) i ^ 32; + return diff; } From 231bf5269108850224ba70419db0ec642a6a97ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 28 Nov 2019 12:22:43 +0100 Subject: [PATCH 21/23] Fix indentation level in one place --- tinycrypt/ecc_dsa.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tinycrypt/ecc_dsa.c b/tinycrypt/ecc_dsa.c index 9ed6941ee..3b5560c8a 100644 --- a/tinycrypt/ecc_dsa.c +++ b/tinycrypt/ecc_dsa.c @@ -170,9 +170,9 @@ int uECC_sign(const uint8_t *private_key, const uint8_t *message_hash, unsigned hash_size, uint8_t *signature) { int r; - uECC_word_t _random[2*NUM_ECC_WORDS]; - uECC_word_t k[NUM_ECC_WORDS]; - uECC_word_t tries; + uECC_word_t _random[2*NUM_ECC_WORDS]; + uECC_word_t k[NUM_ECC_WORDS]; + uECC_word_t tries; for (tries = 0; tries < uECC_RNG_MAX_TRIES; ++tries) { /* Generating _random uniformly at random: */ From 83d7881cec8e8d7a238a1dd9c58463b3218f1c79 Mon Sep 17 00:00:00 2001 From: Jarno Lamsa Date: Wed, 4 Dec 2019 14:40:57 +0200 Subject: [PATCH 22/23] Make VS compiler happy It doesn't seem to like using unary - to unsigned values. --- tinycrypt/ecc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index df7a6928c..d54019f54 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -1172,7 +1172,7 @@ int uECC_valid_point(const uECC_word_t *point) uECC_word_t tmp1[NUM_ECC_WORDS]; uECC_word_t tmp2[NUM_ECC_WORDS]; wordcount_t num_words = NUM_ECC_WORDS; - volatile uECC_word_t diff = -1u; + volatile uECC_word_t diff = 0xffffffff; /* The point at infinity is invalid. */ if (EccPoint_isZero(point)) { From 645896e0ea881c5b16d0383b97086d6ce0c0e68c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 5 Dec 2019 15:30:09 +0100 Subject: [PATCH 23/23] Fix undefined order of volatile access Found by the IAR compiler. While at it, make 'diff' non-volatile in uECC_check_curve_integrity(), as there is no good reason to make it volatile, and making it volatile only increases the code size and the burden of defining access ordering. --- tinycrypt/ecc.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tinycrypt/ecc.c b/tinycrypt/ecc.c index d54019f54..086555a04 100644 --- a/tinycrypt/ecc.c +++ b/tinycrypt/ecc.c @@ -153,15 +153,20 @@ static int uECC_check_curve_integrity(void) 0x76, 0x0a, 0xe2, 0xbc, 0xce, 0x2a, 0xa2, 0xc6, 0x38, 0xf2, 0x19, 0x1d, 0x76, 0x72, 0x93, 0x49, }; - volatile unsigned char diff = 0; + unsigned char diff = 0; + unsigned char tmp1, tmp2; volatile unsigned i; if (uECC_compute_param_sha256(computed) != UECC_SUCCESS) { return UECC_FAILURE; } - for (i = 0; i < 32; i++) - diff |= computed[i] ^ reference[i]; + for (i = 0; i < 32; i++) { + /* make sure the order of volatile accesses is well-defined */ + tmp1 = computed[i]; + tmp2 = reference[i]; + diff |= tmp1 ^ tmp2; + } /* i should be 32 */ mbedtls_platform_enforce_volatile_reads(); @@ -282,10 +287,13 @@ uECC_word_t uECC_vli_equal(const uECC_word_t *left, const uECC_word_t *right) { uECC_word_t diff = 0; + uECC_word_t tmp1, tmp2; volatile int i; for (i = NUM_ECC_WORDS - 1; i >= 0; --i) { - diff |= (left[i] ^ right[i]); + tmp1 = left[i]; + tmp2 = right[i]; + diff |= (tmp1 ^ tmp2); } /* i should be -1 now */