From 6bf5f063973fd82f4b378a5ef05a357338f6874d Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Thu, 22 Feb 2018 09:21:48 +1000 Subject: [PATCH] Fixes from code review --- wolfcrypt/src/asn.c | 94 ++++++++++++++++++----------------------- wolfcrypt/src/ecc.c | 40 +++++++++++------- wolfcrypt/test/test.c | 46 +++++++++++++++++++- wolfssl/wolfcrypt/ecc.h | 4 ++ 4 files changed, 114 insertions(+), 70 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 47337114e..88083370d 100755 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -11025,12 +11025,13 @@ int wc_EccPrivateKeyDecode(const byte* input, word32* inOutIdx, ecc_key* key, #ifdef WOLFSSL_CUSTOM_CURVES static void ByteToHex(byte n, char* str) { - static char hex[] = { '0', '1', '2', '3', '4', '5', '6', '7', - '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; + static const char hexChar[] = { '0', '1', '2', '3', '4', '5', '6', '7', + '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; - str[0] = hex[n >> 4]; - str[1] = hex[n & 0xf]; + str[0] = hexChar[n >> 4]; + str[1] = hexChar[n & 0xf]; } + static int ASNToHexString(const byte* input, word32* inOutIdx, char** out, word32 inSz, void* heap, int heapType) { @@ -11047,7 +11048,7 @@ static int ASNToHexString(const byte* input, word32* inOutIdx, char** out, return ASN_PARSE_E; } - str = XMALLOC(len * 2 + 1, heap, heapType); + str = (char*)XMALLOC(len * 2 + 1, heap, heapType); for (i=0; iheap, + DYNAMIC_TYPE_ECC_BUFFER); + if (curve == NULL) ret = MEMORY_E; if (ret == 0) { - XMEMSET(params, 0, sizeof(*params)); - params->name = "Custom"; - params->id = ECC_CURVE_CUSTOM; + XMEMSET(curve, 0, sizeof(*curve)); + curve->name = "Custom"; + curve->id = ECC_CURVE_CUSTOM; if (GetSequence(input, inOutIdx, &length, inSz) < 0) ret = ASN_PARSE_E; @@ -11108,22 +11110,22 @@ int wc_EccPublicKeyDecode(const byte* input, word32* inOutIdx, } if (ret == 0) { SkipObjectId(input, inOutIdx, inSz); - ret = ASNToHexString(input, inOutIdx, (char**)¶ms->prime, inSz, - NULL, DYNAMIC_TYPE_ECC_BUFFER); + ret = ASNToHexString(input, inOutIdx, (char**)&curve->prime, inSz, + key->heap, DYNAMIC_TYPE_ECC_BUFFER); } if (ret == 0) { - params->size = (int)XSTRLEN(params->prime) / 2; + curve->size = (int)XSTRLEN(curve->prime) / 2; if (GetSequence(input, inOutIdx, &length, inSz) < 0) ret = ASN_PARSE_E; } if (ret == 0) { - ret = ASNToHexString(input, inOutIdx, (char**)¶ms->Af, inSz, - NULL, DYNAMIC_TYPE_ECC_BUFFER); + ret = ASNToHexString(input, inOutIdx, (char**)&curve->Af, inSz, + key->heap, DYNAMIC_TYPE_ECC_BUFFER); } if (ret == 0) { - ret = ASNToHexString(input, inOutIdx, (char**)¶ms->Bf, inSz, - NULL, DYNAMIC_TYPE_ECC_BUFFER); + ret = ASNToHexString(input, inOutIdx, (char**)&curve->Bf, inSz, + key->heap, DYNAMIC_TYPE_ECC_BUFFER); } if (ret == 0) { if (input[*inOutIdx] == ASN_BIT_STRING) { @@ -11133,56 +11135,42 @@ int wc_EccPublicKeyDecode(const byte* input, word32* inOutIdx, } } if (ret == 0) { - ret = ASNToHexString(input, inOutIdx, (char**)&point, inSz, NULL, - DYNAMIC_TYPE_ECC_BUFFER); + ret = ASNToHexString(input, inOutIdx, (char**)&point, inSz, + key->heap, DYNAMIC_TYPE_ECC_BUFFER); } if (ret == 0) { - params->Gx = XMALLOC(params->size * 2 + 2, NULL, + curve->Gx = (const char*)XMALLOC(curve->size * 2 + 2, key->heap, DYNAMIC_TYPE_ECC_BUFFER); - params->Gy = XMALLOC(params->size * 2 + 2, NULL, + curve->Gy = (const char*)XMALLOC(curve->size * 2 + 2, key->heap, DYNAMIC_TYPE_ECC_BUFFER); - if (params->Gx == NULL || params->Gy == NULL) + if (curve->Gx == NULL || curve->Gy == NULL) ret = MEMORY_E; } if (ret == 0) { - XMEMCPY((char*)params->Gx, point + 2, params->size * 2); - XMEMCPY((char*)params->Gy, point + params->size * 2 + 2, - params->size * 2); - ((char*)params->Gx)[params->size * 2] = '\0'; - ((char*)params->Gy)[params->size * 2] = '\0'; - XFREE(point, NULL, DYNAMIC_TYPE_ECC_BUFFER); - ret = ASNToHexString(input, inOutIdx, (char**)¶ms->order, inSz, - NULL, DYNAMIC_TYPE_ECC_BUFFER); + XMEMCPY((char*)curve->Gx, point + 2, curve->size * 2); + XMEMCPY((char*)curve->Gy, point + curve->size * 2 + 2, + curve->size * 2); + ((char*)curve->Gx)[curve->size * 2] = '\0'; + ((char*)curve->Gy)[curve->size * 2] = '\0'; + XFREE(point, key->heap, DYNAMIC_TYPE_ECC_BUFFER); + ret = ASNToHexString(input, inOutIdx, (char**)&curve->order, inSz, + key->heap, DYNAMIC_TYPE_ECC_BUFFER); } if (ret == 0) { - params->cofactor = GetInteger7Bit(input, inOutIdx, inSz); + curve->cofactor = GetInteger7Bit(input, inOutIdx, inSz); - params->oid = NULL; - params->oidSz = 0; - params->oidSum = 0; + curve->oid = NULL; + curve->oidSz = 0; + curve->oidSum = 0; - if (wc_ecc_set_custom_curve(key, params) < 0) { + if (wc_ecc_set_custom_curve(key, curve) < 0) { ret = ASN_PARSE_E; } key->deallocSet = 1; - params = NULL; - } - if (params != NULL) { - if (params->prime != NULL) - XFREE((void*)params->prime, NULL, DYNAMIC_TYPE_ECC_BUFFER); - if (params->Af != NULL) - XFREE((void*)params->Af, NULL, DYNAMIC_TYPE_ECC_BUFFER); - if (params->Bf != NULL) - XFREE((void*)params->Bf, NULL, DYNAMIC_TYPE_ECC_BUFFER); - if (params->order != NULL) - XFREE((void*)params->order, NULL, DYNAMIC_TYPE_ECC_BUFFER); - if (params->Gx != NULL) - XFREE((void*)params->Gx, NULL, DYNAMIC_TYPE_ECC_BUFFER); - if (params->Gy != NULL) - XFREE((void*)params->Gy, NULL, DYNAMIC_TYPE_ECC_BUFFER); - - XFREE((void*)params, NULL, DYNAMIC_TYPE_ECC_BUFFER); + curve = NULL; } + if (curve != NULL) + wc_ecc_free_curve(curve, key->heap); if (ret < 0) return ret; diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index d4ad8b6d1..1a879b055 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -3869,6 +3869,28 @@ int wc_ecc_sign_hash_ex(const byte* in, word32 inlen, WC_RNG* rng, #endif /* WOLFSSL_ATECC508A */ #endif /* HAVE_ECC_SIGN */ +#ifdef WOLFSSL_CUSTOM_CURVES +void wc_ecc_free_curve(const ecc_set_type* curve, void* heap) +{ + if (curve->prime != NULL) + XFREE((void*)curve->prime, heap, DYNAMIC_TYPE_ECC_BUFFER); + if (curve->Af != NULL) + XFREE((void*)curve->Af, heap, DYNAMIC_TYPE_ECC_BUFFER); + if (curve->Bf != NULL) + XFREE((void*)curve->Bf, heap, DYNAMIC_TYPE_ECC_BUFFER); + if (curve->order != NULL) + XFREE((void*)curve->order, heap, DYNAMIC_TYPE_ECC_BUFFER); + if (curve->Gx != NULL) + XFREE((void*)curve->Gx, heap, DYNAMIC_TYPE_ECC_BUFFER); + if (curve->Gy != NULL) + XFREE((void*)curve->Gy, heap, DYNAMIC_TYPE_ECC_BUFFER); + + XFREE((void*)curve, heap, DYNAMIC_TYPE_ECC_BUFFER); + + (void)heap; +} +#endif /* WOLFSSL_CUSTOM_CURVES */ + /** Free an ECC key from memory key The key you wish to free @@ -3897,22 +3919,8 @@ int wc_ecc_free(ecc_key* key) #endif /* WOLFSSL_ATECC508A */ #ifdef WOLFSSL_CUSTOM_CURVES - if (key->deallocSet && key->dp != NULL) { - if (key->dp->prime != NULL) - XFREE((void*)key->dp->prime, NULL, DYNAMIC_TYPE_ECC_BUFFER); - if (key->dp->Af != NULL) - XFREE((void*)key->dp->Af, NULL, DYNAMIC_TYPE_ECC_BUFFER); - if (key->dp->Bf != NULL) - XFREE((void*)key->dp->Bf, NULL, DYNAMIC_TYPE_ECC_BUFFER); - if (key->dp->order != NULL) - XFREE((void*)key->dp->order, NULL, DYNAMIC_TYPE_ECC_BUFFER); - if (key->dp->Gx != NULL) - XFREE((void*)key->dp->Gx, NULL, DYNAMIC_TYPE_ECC_BUFFER); - if (key->dp->Gy != NULL) - XFREE((void*)key->dp->Gy, NULL, DYNAMIC_TYPE_ECC_BUFFER); - - XFREE((void*)key->dp, NULL, DYNAMIC_TYPE_ECC_BUFFER); - } + if (key->deallocSet && key->dp != NULL) + wc_ecc_free_curve(key->dp, key->heap); #endif return 0; diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 45678923d..29f586fe8 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -14368,9 +14368,45 @@ done: #endif /* WOLFSSL_CERT_EXT */ #ifdef WOLFSSL_CUSTOM_CURVES +static const byte eccKeyExplicitCurve[] = { + 0x30, 0x81, 0xf5, 0x30, 0x81, 0xae, 0x06, 0x07, + 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, 0x01, 0x30, + 0x81, 0xa2, 0x02, 0x01, 0x01, 0x30, 0x2c, 0x06, + 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x01, 0x01, + 0x02, 0x21, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xff, + 0xff, 0xfc, 0x2f, 0x30, 0x06, 0x04, 0x01, 0x00, + 0x04, 0x01, 0x07, 0x04, 0x41, 0x04, 0x79, 0xbe, + 0x66, 0x7e, 0xf9, 0xdc, 0xbb, 0xac, 0x55, 0xa0, + 0x62, 0x95, 0xce, 0x87, 0x0b, 0x07, 0x02, 0x9b, + 0xfc, 0xdb, 0x2d, 0xce, 0x28, 0xd9, 0x59, 0xf2, + 0x81, 0x5b, 0x16, 0xf8, 0x17, 0x98, 0x48, 0x3a, + 0xda, 0x77, 0x26, 0xa3, 0xc4, 0x65, 0x5d, 0xa4, + 0xfb, 0xfc, 0x0e, 0x11, 0x08, 0xa8, 0xfd, 0x17, + 0xb4, 0x48, 0xa6, 0x85, 0x54, 0x19, 0x9c, 0x47, + 0xd0, 0x8f, 0xfb, 0x10, 0xd4, 0xb8, 0x02, 0x21, + 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xfe, 0xba, 0xae, 0xdc, 0xe6, 0xaf, 0x48, 0xa0, + 0x3b, 0xbf, 0xd2, 0x5e, 0x8c, 0xd0, 0x36, 0x41, + 0x41, 0x02, 0x01, 0x01, 0x03, 0x42, 0x00, 0x04, + 0x3c, 0x4c, 0xc9, 0x5e, 0x2e, 0xa2, 0x3d, 0x49, + 0xcc, 0x5b, 0xff, 0x4f, 0xc9, 0x2e, 0x1d, 0x4a, + 0xc6, 0x21, 0xf6, 0xf3, 0xe6, 0x0b, 0x4f, 0xa9, + 0x9d, 0x74, 0x99, 0xdd, 0x97, 0xc7, 0x6e, 0xbe, + 0x14, 0x2b, 0x39, 0x9d, 0x63, 0xc7, 0x97, 0x0d, + 0x45, 0x25, 0x40, 0x30, 0x77, 0x05, 0x76, 0x88, + 0x38, 0x96, 0x29, 0x7d, 0x9c, 0xe1, 0x50, 0xbe, + 0xac, 0xf0, 0x1d, 0x86, 0xf4, 0x2f, 0x65, 0x0b +}; + static int ecc_test_custom_curves(WC_RNG* rng) { - int ret; + int ret; + word32 inOutIdx; + ecc_key key; /* test use of custom curve - using BRAINPOOLP256R1 for test */ const word32 ecc_oid_brainpoolp256r1_sum = 104; @@ -14417,6 +14453,14 @@ static int ecc_test_custom_curves(WC_RNG* rng) } #endif + inOutIdx = 0; + ret = wc_EccPublicKeyDecode(eccKeyExplicitCurve, &inOutIdx, &key, + sizeof(eccKeyExplicitCurve)); + if (ret != 0) + return -6715; + + wc_ecc_free(&key); + return ret; } #endif /* WOLFSSL_CUSTOM_CURVES */ diff --git a/wolfssl/wolfcrypt/ecc.h b/wolfssl/wolfcrypt/ecc.h index d8babe1ce..d41b1fff5 100644 --- a/wolfssl/wolfcrypt/ecc.h +++ b/wolfssl/wolfcrypt/ecc.h @@ -397,6 +397,10 @@ WOLFSSL_API int wc_ecc_init(ecc_key* key); WOLFSSL_API int wc_ecc_init_ex(ecc_key* key, void* heap, int devId); +#ifdef WOLFSSL_CUSTOM_CURVES +WOLFSSL_LOCAL +void wc_ecc_free_curve(const ecc_set_type* curve, void* heap); +#endif WOLFSSL_API int wc_ecc_free(ecc_key* key); WOLFSSL_API