From 589057245fd6c82d4ed6b41efef8e4c9145e3da5 Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 23 Oct 2020 11:00:46 -0700 Subject: [PATCH] Improvement to ECC `wc_ecc_rs_raw_to_sig` to reduce memory use (avoid the mp_int). Additional test cases. Fixes for previous function changes. --- wolfcrypt/src/asn.c | 78 +++++++++++++++++++++++++++++++++++------ wolfcrypt/src/ecc.c | 50 +------------------------- wolfcrypt/test/test.c | 46 +++++++++++++++--------- wolfssl/wolfcrypt/asn.h | 2 ++ 4 files changed, 100 insertions(+), 76 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index abb451c43..c60fdccf5 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -15427,6 +15427,66 @@ int StoreECC_DSA_Sig(byte* out, word32* outLen, mp_int* r, mp_int* s) return 0; } +/* determine if leading bit is set and trim leading zeros */ +static int is_leading_bit_set(const byte** input, word32 *sz) +{ + int i; + word32 leadingZeroCount = 0; + const byte* tmp = *input; + byte c = 0; + for (i=0; i<(int)*sz; i++) { + c = tmp[i]; + if (c != 0) + break; + leadingZeroCount++; + } + *input += leadingZeroCount; + *sz -= leadingZeroCount; + return (c & 0x80) != 0; +} + +/* Der Encode r & s ints into out, outLen is (in/out) size */ +int StoreECC_DSA_Sig_Bin(byte* out, word32* outLen, const byte* r, word32 rLen, + const byte* s, word32 sLen) +{ + int ret; + word32 idx; + word32 headerSz = 4; /* 2*ASN_TAG + 2*LEN(ENUM) */ + int rAddLeadZero, sAddLeadZero; + + /* If the leading bit on the INTEGER is a 1, add a leading zero */ + /* Add leading zero if MSB is set */ + /* Trim leading zeros */ + rAddLeadZero = is_leading_bit_set(&r, &rLen); + sAddLeadZero = is_leading_bit_set(&s, &sLen); + + if (*outLen < (rLen + rAddLeadZero + sLen + sAddLeadZero + + headerSz + 2)) /* SEQ_TAG + LEN(ENUM) */ + return BUFFER_E; + + idx = SetSequence(rLen+rAddLeadZero + sLen+sAddLeadZero + headerSz, out); + + /* store r */ + ret = SetASNInt(rLen, rAddLeadZero ? 0x80 : 0x00, &out[idx]); + if (ret < 0) + return ret; + idx += ret; + XMEMCPY(&out[idx], r, rLen); + idx += rLen; + + /* store s */ + ret = SetASNInt(sLen, sAddLeadZero ? 0x80 : 0x00, &out[idx]); + if (ret < 0) + return ret; + idx += ret; + XMEMCPY(&out[idx], s, sLen); + idx += sLen; + + *outLen = idx; + + return 0; +} + /* Der Decode ECC-DSA Signature with R/S as unsigned bin */ int DecodeECC_DSA_Sig_Bin(const byte* sig, word32 sigLen, byte* r, word32* rLen, @@ -15546,18 +15606,18 @@ int wc_EccPrivateKeyDecode(const byte* input, word32* inOutIdx, ecc_key* key, if (GetLength(input, inOutIdx, &length, inSz) < 0) return ASN_PARSE_E; + privSz = length; - if (length > ECC_MAXSIZE) + if (privSz > ECC_MAXSIZE) return BUFFER_E; #ifdef WOLFSSL_SMALL_STACK - priv = (byte*)XMALLOC(length+1, key->heap, DYNAMIC_TYPE_TMP_BUFFER); + priv = (byte*)XMALLOC(privSz, key->heap, DYNAMIC_TYPE_TMP_BUFFER); if (priv == NULL) return MEMORY_E; #endif /* priv key */ - privSz = length; XMEMCPY(priv, &input[*inOutIdx], privSz); *inOutIdx += length; @@ -15601,13 +15661,13 @@ int wc_EccPrivateKeyDecode(const byte* input, word32* inOutIdx, ecc_key* key, if (ret == 0) { /* pub key */ pubSz = length; - if (pubSz <= 2*ECC_MAXSIZE) { + if (pubSz > 2*(ECC_MAXSIZE+1)) + ret = BUFFER_E; + else { #ifdef WOLFSSL_SMALL_STACK - pub = (byte*)XMALLOC(pubSz+1, key->heap, DYNAMIC_TYPE_TMP_BUFFER); - if (pub == NULL) { - XFREE(priv, key->heap, DYNAMIC_TYPE_TMP_BUFFER); + pub = (byte*)XMALLOC(pubSz, key->heap, DYNAMIC_TYPE_TMP_BUFFER); + if (pub == NULL) ret = MEMORY_E; - } else #endif { @@ -15616,8 +15676,6 @@ int wc_EccPrivateKeyDecode(const byte* input, word32* inOutIdx, ecc_key* key, pubData = pub; } } - else - ret = BUFFER_E; } } } diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index f97f32f5d..ddc4e92d1 100644 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -7972,59 +7972,11 @@ int wc_ecc_rs_to_sig(const char* r, const char* s, byte* out, word32* outlen) int wc_ecc_rs_raw_to_sig(const byte* r, word32 rSz, const byte* s, word32 sSz, byte* out, word32* outlen) { - int err; -#ifdef WOLFSSL_SMALL_STACK - mp_int* rtmp = NULL; - mp_int* stmp = NULL; -#else - mp_int rtmp[1]; - mp_int stmp[1]; -#endif - if (r == NULL || s == NULL || out == NULL || outlen == NULL) return ECC_BAD_ARG_E; -#ifdef WOLFSSL_SMALL_STACK - rtmp = (mp_int*)XMALLOC(sizeof(mp_int), NULL, DYNAMIC_TYPE_ECC); - if (rtmp == NULL) - return MEMORY_E; - stmp = (mp_int*)XMALLOC(sizeof(mp_int), NULL, DYNAMIC_TYPE_ECC); - if (stmp == NULL) { - XFREE(rtmp, NULL, DYNAMIC_TYPE_ECC); - return MEMORY_E; - } -#endif - - err = mp_init_multi(rtmp, stmp, NULL, NULL, NULL, NULL); - if (err != MP_OKAY) { - #ifdef WOLFSSL_SMALL_STACK - XFREE(stmp, NULL, DYNAMIC_TYPE_ECC); - XFREE(rtmp, NULL, DYNAMIC_TYPE_ECC); - #endif - return err; - } - - err = mp_read_unsigned_bin(rtmp, r, rSz); - if (err == MP_OKAY) - err = mp_read_unsigned_bin(stmp, s, sSz); - /* convert mp_ints to ECDSA sig, initializes rtmp and stmp internally */ - if (err == MP_OKAY) - err = StoreECC_DSA_Sig(out, outlen, rtmp, stmp); - - if (err == MP_OKAY) { - if (mp_iszero(rtmp) == MP_YES || mp_iszero(stmp) == MP_YES) - err = MP_ZERO_E; - } - - mp_clear(rtmp); - mp_clear(stmp); -#ifdef WOLFSSL_SMALL_STACK - XFREE(stmp, NULL, DYNAMIC_TYPE_ECC); - XFREE(rtmp, NULL, DYNAMIC_TYPE_ECC); -#endif - - return err; + return StoreECC_DSA_Sig_Bin(out, outlen, r, rSz, s, sSz); } /** diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index d3d986d79..9efa64b08 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -18297,12 +18297,14 @@ static int ecc_test_vector_item(const eccVector* vector) #ifdef WOLFSSL_SMALL_STACK ecc_key *userA = (ecc_key *)XMALLOC(sizeof *userA, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); #else - ecc_key userA_buf, *userA = &userA_buf; + ecc_key userA[1]; #endif DECLARE_VAR(sig, byte, ECC_SIG_SIZE, HEAP_HINT); #if !defined(NO_ASN) && !defined(HAVE_SELFTEST) - word32 sigRawSz; + word32 sigRawSz, rSz = MAX_ECC_BYTES, sSz = MAX_ECC_BYTES; DECLARE_VAR(sigRaw, byte, ECC_SIG_SIZE, HEAP_HINT); + DECLARE_VAR(r, byte, MAX_ECC_BYTES, HEAP_HINT); + DECLARE_VAR(s, byte, MAX_ECC_BYTES, HEAP_HINT); #endif #ifdef DECLARE_VAR_IS_HEAP_ALLOC @@ -18341,11 +18343,19 @@ static int ecc_test_vector_item(const eccVector* vector) sigRaw, &sigRawSz); if (ret != 0) goto done; - if (sigSz != sigRawSz || XMEMCMP(sig, sigRaw, sigSz) != 0) { ret = -9608; goto done; } + + ret = wc_ecc_sig_to_rs(sig, sigSz, r, &rSz, s, &sSz); + if (ret != 0) + goto done; + if (rSz != vector->rSz || XMEMCMP(r, vector->r, rSz) != 0 || + sSz != vector->sSz || XMEMCMP(s, vector->s, sSz) != 0) { + ret = -9613; + goto done; + } #endif do { @@ -18376,6 +18386,8 @@ done: #if !defined(NO_ASN) && !defined(HAVE_SELFTEST) FREE_VAR(sigRaw, HEAP_HINT); + FREE_VAR(r, HEAP_HINT); + FREE_VAR(s, HEAP_HINT); #endif FREE_VAR(sig, HEAP_HINT); @@ -18598,20 +18610,20 @@ static int ecc_test_vector(int keySize) vec.S = "019cd2c5c3f9870ecdeb9b323abdf3a98cd5e231d85c6ddc5b71ab190739f7f226e6b134ba1d5889ddeb2751dabd97911dff90c34684cdbe7bb669b6c3d22f2480c"; vec.curveName = "SECP521R1"; #ifndef NO_ASN - vec.r = (byte*)"\x00\xbd\x11\x7b\x48\x07\x71\x08\x98\xf9\xdd\x77" - "\x78\x05\x64\x85\x77\x76\x68\xf0\xe7\x8e\x6d\xdf" - "\x5b\x00\x03\x56\x12\x1e\xb7\xa2\x20\xe9\x49\x3c" - "\x7f\x9a\x57\xc0\x77\x94\x7f\x89\xac\x45\xd5\xac" - "\xb6\x66\x1b\xbc\xd1\x7a\xbb\x3f\xae\xa1\x49\xba" - "\x0a\xa3\xbb\x15\x21\xbe"; - vec.rSz = 66; - vec.s = (byte*)"\x00\x19\xcd\x2c\x5c\x3f\x98\x70\xec\xde\xb9\xb3" - "\x23\xab\xdf\x3a\x98\xcd\x5e\x23\x1d\x85\xc6\xdd" - "\xc5\xb7\x1a\xb1\x90\x73\x9f\x7f\x22\x6e\x6b\x13" - "\x4b\xa1\xd5\x88\x9d\xde\xb2\x75\x1d\xab\xd9\x79" - "\x11\xdf\xf9\x0c\x34\x68\x4c\xdb\xe7\xbb\x66\x9b" - "\x6c\x3d\x22\xf2\x48\x0c"; - vec.sSz = 66; + vec.r = (byte*)"\xbd\x11\x7b\x48\x07\x71\x08\x98\xf9\xdd\x77\x78" + "\x05\x64\x85\x77\x76\x68\xf0\xe7\x8e\x6d\xdf\x5b" + "\x00\x03\x56\x12\x1e\xb7\xa2\x20\xe9\x49\x3c\x7f" + "\x9a\x57\xc0\x77\x94\x7f\x89\xac\x45\xd5\xac\xb6" + "\x66\x1b\xbc\xd1\x7a\xbb\x3f\xae\xa1\x49\xba\x0a" + "\xa3\xbb\x15\x21\xbe"; + vec.rSz = 65; + vec.s = (byte*)"\x19\xcd\x2c\x5c\x3f\x98\x70\xec\xde\xb9\xb3\x23" + "\xab\xdf\x3a\x98\xcd\x5e\x23\x1d\x85\xc6\xdd\xc5" + "\xb7\x1a\xb1\x90\x73\x9f\x7f\x22\x6e\x6b\x13\x4b" + "\xa1\xd5\x88\x9d\xde\xb2\x75\x1d\xab\xd9\x79\x11" + "\xdf\xf9\x0c\x34\x68\x4c\xdb\xe7\xbb\x66\x9b\x6c" + "\x3d\x22\xf2\x48\x0c"; + vec.sSz = 65; #endif break; #endif /* HAVE_ECC521 */ diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 261d4b4cd..f93c5c27a 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -1162,6 +1162,8 @@ WOLFSSL_LOCAL int FlattenAltNames( byte*, word32, const DNS_entry*); /* ASN sig helpers */ WOLFSSL_LOCAL int StoreECC_DSA_Sig(byte* out, word32* outLen, mp_int* r, mp_int* s); + WOLFSSL_LOCAL int StoreECC_DSA_Sig_Bin(byte* out, word32* outLen, + const byte* r, word32 rLen, const byte* s, word32 sLen); WOLFSSL_LOCAL int DecodeECC_DSA_Sig(const byte* sig, word32 sigLen, mp_int* r, mp_int* s); WOLFSSL_LOCAL int DecodeECC_DSA_Sig_Bin(const byte* sig, word32 sigLen,