From f147b016742d8e989427270dcbbf9e66d51a538f Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 21 Jul 2017 10:50:12 -0700 Subject: [PATCH 1/4] =?UTF-8?q?Fixes=20for=20using=20`WOLFSSL=5FCUSTOM=5FC?= =?UTF-8?q?URVES`=20and=20`wc=5Fecc=5Fset=5Fcustom=5Fcurve`.=20Fixes=20res?= =?UTF-8?q?olves=20issue=20with=20`->dp`=20and=20`->idx`=20getting=20reset?= =?UTF-8?q?=20which=20caused=20curve=20parameters=20to=20not=20be=20set=20?= =?UTF-8?q?correctly.=20Proper=20sequence=20for=20using=20custom=20curves?= =?UTF-8?q?=20is=20=E2=80=98wc=5Fecc=5Finit`,=20`wc=5Fecc=5Fset=5Fcustom?= =?UTF-8?q?=5Fcurve`=20then=20`wc=5Fecc=5Fmake=5Fkey=5Fex(=E2=80=A6,=20ECC?= =?UTF-8?q?=5FCUSTOM=5FIDX)=E2=80=99=20or=20`wc=5Fecc=5Fimport=5Fx963=5Fex?= =?UTF-8?q?(=E2=80=A6,=20ECC=5FCUSTOM=5FIDX)`.=20Test=20case=20and=20examp?= =?UTF-8?q?le=20to=20follow=20shortly.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- wolfcrypt/src/ecc.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 3fc2f5038..bd3d117fa 100755 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -1245,6 +1245,10 @@ int wc_ecc_set_curve(ecc_key* key, int keysize, int curve_id) if (key->idx != ECC_CUSTOM_IDX) { int x; + /* default values */ + key->idx = 0; + key->dp = NULL; + /* find ecc_set based on curve_id or key size */ for (x = 0; ecc_sets[x].size != 0; x++) { if (curve_id > ECC_CURVE_DEF) { @@ -2969,6 +2973,12 @@ static int wc_ecc_gen_k(WC_RNG* rng, int size, mp_int* k, mp_int* order) } #endif /* !WOLFSSL_ATECC508A */ +static INLINE void wc_ecc_reset(ecc_key* key) +{ + /* make sure required key variables are reset */ + key->state = ECC_STATE_NONE; +} + int wc_ecc_make_key_ex(WC_RNG* rng, int keysize, ecc_key* key, int curve_id) { int err; @@ -2981,10 +2991,8 @@ int wc_ecc_make_key_ex(WC_RNG* rng, int keysize, ecc_key* key, int curve_id) return BAD_FUNC_ARG; } - /* make sure required key variables are reset */ - key->state = ECC_STATE_NONE; - key->idx = 0; - key->dp = NULL; + /* make sure required variables are reset */ + wc_ecc_reset(key); err = wc_ecc_set_curve(key, keysize, curve_id); if (err != 0) { @@ -4777,7 +4785,6 @@ int wc_ecc_import_x963_ex(const byte* in, word32 inLen, ecc_key* key, #ifndef WOLFSSL_ATECC508A int compressed = 0; #endif /* !WOLFSSL_ATECC508A */ - void* heap; if (in == NULL || key == NULL) return BAD_FUNC_ARG; @@ -4787,9 +4794,8 @@ int wc_ecc_import_x963_ex(const byte* in, word32 inLen, ecc_key* key, return ECC_BAD_ARG_E; } - heap = key->heap; /* save heap */ - XMEMSET(key, 0, sizeof(ecc_key)); - key->heap = heap; /* restore heap */ + /* make sure required variables are reset */ + wc_ecc_reset(key); #ifdef WOLFSSL_ATECC508A /* TODO: Implement equiv call to ATECC508A */ @@ -5086,18 +5092,14 @@ int wc_ecc_import_private_key_ex(const byte* priv, word32 privSz, /* public optional, NULL if only importing private */ if (pub != NULL) { - ret = wc_ecc_import_x963_ex(pub, pubSz, key, curve_id); - - } else { - + } + else { if (key == NULL || priv == NULL) return BAD_FUNC_ARG; - /* make sure required key variables are reset */ - key->state = ECC_STATE_NONE; - key->idx = 0; - key->dp = NULL; + /* make sure required variables are reset */ + wc_ecc_reset(key); /* set key size */ ret = wc_ecc_set_curve(key, privSz, curve_id); @@ -5236,16 +5238,14 @@ static int wc_ecc_import_raw_private(ecc_key* key, const char* qx, const char* qy, const char* d, int curve_id) { int err = MP_OKAY; - void* heap; /* if d is NULL, only import as public key using Qx,Qy */ if (key == NULL || qx == NULL || qy == NULL) { return BAD_FUNC_ARG; } - heap = key->heap; /* save heap */ - XMEMSET(key, 0, sizeof(ecc_key)); - key->heap = heap; /* restore heap */ + /* make sure required variables are reset */ + wc_ecc_reset(key); /* set curve type and index */ err = wc_ecc_set_curve(key, 0, curve_id); From 5180cf4cce6dde46237b9b6f62eb266049caa822 Mon Sep 17 00:00:00 2001 From: David Garske Date: Mon, 24 Jul 2017 14:43:38 -0700 Subject: [PATCH 2/4] Fix ECC sign with custom curves so the custom params (dp) are passed to public key used for sign. --- wolfcrypt/src/ecc.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index bd3d117fa..3a894b37b 100755 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -3538,6 +3538,13 @@ int wc_ecc_sign_hash_ex(const byte* in, word32 inlen, WC_RNG* rng, /* don't use async for key, since we don't support async return here */ if (wc_ecc_init_ex(&pubkey, key->heap, INVALID_DEVID) == MP_OKAY) { + #ifdef WOLFSSL_CUSTOM_CURVES + /* if custom curve, apply params to pubkey */ + if (key->idx == ECC_CUSTOM_IDX) { + wc_ecc_set_custom_curve(&pubkey, key->dp); + } + #endif + for (;;) { if (++loop_check > 64) { err = RNG_FAILURE_E; From 33e214ffc1409639f8a9fddaab9fe056996c6598 Mon Sep 17 00:00:00 2001 From: David Garske Date: Mon, 24 Jul 2017 16:43:56 -0700 Subject: [PATCH 3/4] Fix to allow ECC set curve size with curve_id == 0. Added wolfCrypt tests / example for using `wc_ecc_set_custom_curve` for `BRAINPOOLP256R1`. --- wolfcrypt/src/ecc.c | 2 +- wolfcrypt/test/test.c | 90 ++++++++++++++++++++++++++++++++----------- 2 files changed, 69 insertions(+), 23 deletions(-) diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 3a894b37b..8d58685e8 100755 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -1233,7 +1233,7 @@ const char* wc_ecc_get_name(int curve_id) int wc_ecc_set_curve(ecc_key* key, int keysize, int curve_id) { - if (keysize <= 0 && curve_id <= 0) { + if (keysize <= 0 && curve_id < 0) { return BAD_FUNC_ARG; } diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 08c7e9574..bda6f4eb0 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -43,11 +43,7 @@ #include #include -#ifdef WOLFSSL_TEST_CERT - #include -#else - #include -#endif +#include #include #include #include @@ -10109,7 +10105,7 @@ done: #endif /* WOLFSSL_KEY_GEN */ static int ecc_test_curve_size(WC_RNG* rng, int keySize, int testVerifyCount, - int curve_id) + int curve_id, const ecc_set_type* dp) { DECLARE_VAR(sharedA, byte, ECC_SHARED_SIZE, HEAP_HINT); DECLARE_VAR(sharedB, byte, ECC_SHARED_SIZE, HEAP_HINT); @@ -10129,6 +10125,7 @@ static int ecc_test_curve_size(WC_RNG* rng, int keySize, int testVerifyCount, ecc_key userA, userB, pubKey; (void)testVerifyCount; + (void)dp; XMEMSET(&userA, 0, sizeof(ecc_key)); XMEMSET(&userB, 0, sizeof(ecc_key)); @@ -10144,6 +10141,14 @@ static int ecc_test_curve_size(WC_RNG* rng, int keySize, int testVerifyCount, if (ret != 0) goto done; +#ifdef WOLFSSL_CUSTOM_CURVES + if (dp) { + wc_ecc_set_custom_curve(&userA, dp); + wc_ecc_set_custom_curve(&userB, dp); + wc_ecc_set_custom_curve(&pubKey, dp); + } +#endif + ret = wc_ecc_make_key_ex(rng, keySize, &userA, curve_id); #if defined(WOLFSSL_ASYNC_CRYPT) ret = wc_AsyncWait(ret, &userA.asyncDev, WC_ASYNC_FLAG_CALL_AGAIN); @@ -10386,7 +10391,7 @@ static int ecc_test_curve(WC_RNG* rng, int keySize) { int ret; - ret = ecc_test_curve_size(rng, keySize, ECC_TEST_VERIFY_COUNT, ECC_CURVE_DEF); + ret = ecc_test_curve_size(rng, keySize, ECC_TEST_VERIFY_COUNT, ECC_CURVE_DEF, NULL); if (ret < 0) { if (ret == ECC_CURVE_OID_E) { /* ignore error for curves not found */ @@ -10983,6 +10988,59 @@ done: } #endif /* WOLFSSL_CERT_EXT */ +#ifdef WOLFSSL_CUSTOM_CURVES +static int ecc_test_custom_curves(WC_RNG* rng) +{ + int ret; + + /* test use of custom curve - using BRAINPOOLP256R1 for test */ + const ecc_oid_t ecc_oid_brainpoolp256r1[] = { + 0x2B,0x24,0x03,0x03,0x02,0x08,0x01,0x01,0x07 + }; + const ecc_set_type ecc_dp_brainpool256r1 = { + 32, /* size/bytes */ + ECC_BRAINPOOLP256R1, /* ID */ + "BRAINPOOLP256R1", /* curve name */ + "A9FB57DBA1EEA9BC3E660A909D838D726E3BF623D52620282013481D1F6E5377", /* prime */ + "7D5A0975FC2C3057EEF67530417AFFE7FB8055C126DC5C6CE94A4B44F330B5D9", /* A */ + "26DC5C6CE94A4B44F330B5D9BBD77CBF958416295CF7E1CE6BCCDC18FF8C07B6", /* B */ + "A9FB57DBA1EEA9BC3E660A909D838D718C397AA3B561A6F7901E0E82974856A7", /* order */ + "8BD2AEB9CB7E57CB2C4B482FFC81B7AFB9DE27E1E3BD23C23A4453BD9ACE3262", /* Gx */ + "547EF835C3DAC4FD97F8461A14611DC9C27745132DED8E545C1D54C72F046997", /* Gy */ + ecc_oid_brainpoolp256r1, /* oid/oidSz */ + sizeof(ecc_oid_brainpoolp256r1) / sizeof(ecc_oid_t), + ECC_BRAINPOOLP256R1_OID, /* oid sum */ + 1, /* cofactor */ + }; + + ret = ecc_test_curve_size(rng, 0, ECC_TEST_VERIFY_COUNT, ECC_CURVE_DEF, + &ecc_dp_brainpool256r1); + if (ret != 0) { + printf("ECC test for custom curve failed! %d\n", ret); + return ret; + } + + #if defined(HAVE_ECC_BRAINPOOL) || defined(HAVE_ECC_KOBLITZ) + { + int curve_id; + #ifdef HAVE_ECC_BRAINPOOL + curve_id = ECC_BRAINPOOLP256R1; + #else + curve_id = ECC_SECP256K1; + #endif + /* Test and demonstrate use of non-SECP curve */ + ret = ecc_test_curve_size(rng, 0, ECC_TEST_VERIFY_COUNT, curve_id, NULL); + if (ret < 0) { + printf("ECC test for curve_id %d failed! %d\n", curve_id, ret); + return ret; + } + } + #endif + + return ret; +} +#endif /* WOLFSSL_CUSTOM_CURVES */ + int ecc_test(void) { int ret; @@ -11081,22 +11139,10 @@ int ecc_test(void) #endif /* HAVE_ECC521 */ #if defined(WOLFSSL_CUSTOM_CURVES) - #if defined(HAVE_ECC_BRAINPOOL) || defined(HAVE_ECC_KOBLITZ) - { - int curve_id; - #ifdef HAVE_ECC_BRAINPOOL - curve_id = ECC_BRAINPOOLP256R1; - #else - curve_id = ECC_SECP256K1; - #endif - /* Test and demonstrate use of non-SECP curve */ - ret = ecc_test_curve_size(&rng, 0, ECC_TEST_VERIFY_COUNT, curve_id); - if (ret < 0) { - printf("ecc_test_curve_size: type %d: failed!: %d\n", curve_id, ret); - goto done; - } + ret = ecc_test_custom_curves(&rng); + if (ret != 0) { + goto done; } - #endif #endif #ifdef HAVE_ECC_CDH From 08488b52b73b3be5f4769431894f745616cea473 Mon Sep 17 00:00:00 2001 From: David Garske Date: Mon, 24 Jul 2017 21:04:18 -0700 Subject: [PATCH 4/4] Fix for wolfCrypt test custom curve test not setting `wc_ecc_set_custom_curve`before calling `wc_ecc_import_x963_ex`. Fix for using `ECC_CACHE_CURVE` option and `wc_ecc_set_custom_curve`. Added error checking for `wc_ecc_set_custom_curve` calls. Reverted ASN header change in test.c. --- wolfcrypt/src/ecc.c | 12 +++++++++++- wolfcrypt/test/test.c | 36 ++++++++++++++++++++++++++++-------- wolfssl/wolfcrypt/ecc.h | 4 ++++ 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 8d58685e8..f4e9ac9ff 100755 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -929,6 +929,15 @@ const ecc_set_type ecc_sets[] = { }, #endif /* !NO_ECC_SECP */ #endif /* ECC521 */ +#if defined(WOLFSSL_CUSTOM_CURVES) && defined(ECC_CACHE_CURVE) + /* place holder for custom curve index for cache */ + { + 1, /* non-zero */ + ECC_CURVE_CUSTOM, + NULL, NULL, NULL, NULL, NULL, NULL, NULL, + NULL, 0, 0, 0 + }, +#endif { 0, -1, NULL, NULL, NULL, NULL, NULL, NULL, NULL, @@ -2506,7 +2515,8 @@ int wc_ecc_get_curve_idx_from_name(const char* curveName) len = (word32)XSTRLEN(curveName); for (curve_idx = 0; ecc_sets[curve_idx].size != 0; curve_idx++) { - if (XSTRNCASECMP(ecc_sets[curve_idx].name, curveName, len) == 0) { + if (ecc_sets[curve_idx].name && + XSTRNCASECMP(ecc_sets[curve_idx].name, curveName, len) == 0) { break; } } diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index bda6f4eb0..b31c41bd5 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -43,7 +43,11 @@ #include #include -#include +#ifdef WOLFSSL_TEST_CERT + #include +#else + #include +#endif #include #include #include @@ -10142,10 +10146,13 @@ static int ecc_test_curve_size(WC_RNG* rng, int keySize, int testVerifyCount, goto done; #ifdef WOLFSSL_CUSTOM_CURVES - if (dp) { - wc_ecc_set_custom_curve(&userA, dp); - wc_ecc_set_custom_curve(&userB, dp); - wc_ecc_set_custom_curve(&pubKey, dp); + if (dp != NULL) { + ret = wc_ecc_set_custom_curve(&userA, dp); + if (ret != 0) + goto done; + ret = wc_ecc_set_custom_curve(&userB, dp); + if (ret != 0) + goto done; } #endif @@ -10232,6 +10239,12 @@ static int ecc_test_curve_size(WC_RNG* rng, int keySize, int testVerifyCount, goto done; #ifdef HAVE_ECC_KEY_IMPORT + #ifdef WOLFSSL_CUSTOM_CURVES + if (dp != NULL) { + ret = wc_ecc_set_custom_curve(&pubKey, dp); + if (ret != 0) goto done; + } + #endif ret = wc_ecc_import_x963_ex(exportBuf, x, &pubKey, curve_id); if (ret != 0) goto done; @@ -10259,10 +10272,16 @@ static int ecc_test_curve_size(WC_RNG* rng, int keySize, int testVerifyCount, if (ret != 0) goto done; wc_ecc_free(&pubKey); + ret = wc_ecc_init_ex(&pubKey, HEAP_HINT, devId); if (ret != 0) goto done; - + #ifdef WOLFSSL_CUSTOM_CURVES + if (dp != NULL) { + ret = wc_ecc_set_custom_curve(&pubKey, dp); + if (ret != 0) goto done; + } + #endif ret = wc_ecc_import_x963_ex(exportBuf, x, &pubKey, curve_id); if (ret != 0) goto done; @@ -10994,12 +11013,13 @@ static int ecc_test_custom_curves(WC_RNG* rng) int ret; /* test use of custom curve - using BRAINPOOLP256R1 for test */ + const word32 ecc_oid_brainpoolp256r1_sum = 104; const ecc_oid_t ecc_oid_brainpoolp256r1[] = { 0x2B,0x24,0x03,0x03,0x02,0x08,0x01,0x01,0x07 }; const ecc_set_type ecc_dp_brainpool256r1 = { 32, /* size/bytes */ - ECC_BRAINPOOLP256R1, /* ID */ + ECC_CURVE_CUSTOM, /* ID */ "BRAINPOOLP256R1", /* curve name */ "A9FB57DBA1EEA9BC3E660A909D838D726E3BF623D52620282013481D1F6E5377", /* prime */ "7D5A0975FC2C3057EEF67530417AFFE7FB8055C126DC5C6CE94A4B44F330B5D9", /* A */ @@ -11009,7 +11029,7 @@ static int ecc_test_custom_curves(WC_RNG* rng) "547EF835C3DAC4FD97F8461A14611DC9C27745132DED8E545C1D54C72F046997", /* Gy */ ecc_oid_brainpoolp256r1, /* oid/oidSz */ sizeof(ecc_oid_brainpoolp256r1) / sizeof(ecc_oid_t), - ECC_BRAINPOOLP256R1_OID, /* oid sum */ + ecc_oid_brainpoolp256r1_sum, /* oid sum */ 1, /* cofactor */ }; diff --git a/wolfssl/wolfcrypt/ecc.h b/wolfssl/wolfcrypt/ecc.h index df29e7168..a146d6a53 100644 --- a/wolfssl/wolfcrypt/ecc.h +++ b/wolfssl/wolfcrypt/ecc.h @@ -158,6 +158,10 @@ typedef enum ecc_curve_id { #ifdef HAVE_X448 ECC_X448, #endif + +#ifdef WOLFSSL_CUSTOM_CURVES + ECC_CURVE_CUSTOM, +#endif } ecc_curve_id; #ifdef HAVE_OID_ENCODING