From e62ba399ffbe412b174aa63309785185aaa03508 Mon Sep 17 00:00:00 2001 From: "wtchang%redhat.com" Date: Sat, 21 Jan 2006 02:14:46 +0000 Subject: [PATCH] Bugzilla Bug 320589: miscellaneous code cleanup: distinguish between the length of the field size and the length of the base point order. Report better error codes. In ECDSA_VerifyDigest, removed unnecessary local variables and be lenient in the signature lengths we accept. r=relyea,nelsonb git-svn-id: svn://10.0.0.236/trunk@187942 18797224-902f-48f8-a5cc-f745e15eee43 --- mozilla/security/nss/lib/freebl/ec.c | 105 ++++++++++++--------------- 1 file changed, 48 insertions(+), 57 deletions(-) diff --git a/mozilla/security/nss/lib/freebl/ec.c b/mozilla/security/nss/lib/freebl/ec.c index 5a75dc9e331..a133da7461e 100644 --- a/mozilla/security/nss/lib/freebl/ec.c +++ b/mozilla/security/nss/lib/freebl/ec.c @@ -53,7 +53,7 @@ PRBool ec_point_at_infinity(SECItem *pointP) { - int i; + unsigned int i; for (i = 1; i < pointP->len; i++) { if (pointP->data[i] != 0x00) return PR_FALSE; @@ -614,7 +614,8 @@ ECDSA_SignDigestWithSeed(ECPrivateKey *key, SECItem *signature, ECParams *ecParams = NULL; SECItem localDigest; SECItem kGpoint = { siBuffer, NULL, 0}; - int len = 0; + int flen = 0; /* length in bytes of the field size */ + unsigned olen; /* length in bytes of the base point order */ #if EC_DEBUG char mpstr[256]; @@ -636,17 +637,18 @@ ECDSA_SignDigestWithSeed(ECPrivateKey *key, SECItem *signature, } ecParams = &(key->ecParams); - len = (ecParams->fieldID.size + 7) >> 3; - if (signature->len < 2*len) { - PORT_SetError(SEC_ERROR_INVALID_ARGS); + flen = (ecParams->fieldID.size + 7) >> 3; + olen = ecParams->order.len; + if (signature->len < 2*olen) { + PORT_SetError(SEC_ERROR_OUTPUT_LEN); goto cleanup; } - /* ec defines signing with a larger digest than the keysize - * as signing with a truncated digest */ + /* In the definition of EC signing, digests are truncated + * to the length of the EC base point order */ localDigest = *digest; - if (localDigest.len > len) { - localDigest.len = len; + if (localDigest.len > olen) { + localDigest.len = olen; } CHECK_MPI_OK( mp_init(&x1) ); @@ -677,8 +679,8 @@ ECDSA_SignDigestWithSeed(ECPrivateKey *key, SECItem *signature, ** ** Compute kG */ - kGpoint.len = 2*len + 1; - kGpoint.data = PORT_Alloc(2*len + 1); + kGpoint.len = 2*flen + 1; + kGpoint.data = PORT_Alloc(2*flen + 1); if ((kGpoint.data == NULL) || (ec_points_mul(ecParams, &k, NULL, NULL, &kGpoint) != SECSuccess)) @@ -690,7 +692,7 @@ ECDSA_SignDigestWithSeed(ECPrivateKey *key, SECItem *signature, ** Extract the x co-ordinate of kG into x1 */ CHECK_MPI_OK( mp_read_unsigned_octets(&x1, kGpoint.data + 1, - (mp_size) len) ); + (mp_size) flen) ); /* ** ANSI X9.62, Section 5.3.3, Step 2 @@ -757,9 +759,9 @@ ECDSA_SignDigestWithSeed(ECPrivateKey *key, SECItem *signature, ** ** Signature is tuple (r, s) */ - CHECK_MPI_OK( mp_to_fixlen_octets(&r, signature->data, len) ); - CHECK_MPI_OK( mp_to_fixlen_octets(&s, signature->data + len, len) ); - signature->len = 2*len; + CHECK_MPI_OK( mp_to_fixlen_octets(&r, signature->data, olen) ); + CHECK_MPI_OK( mp_to_fixlen_octets(&s, signature->data + olen, olen) ); + signature->len = 2*olen; rv = SECSuccess; err = MP_OKAY; @@ -772,7 +774,7 @@ cleanup: mp_clear(&n); if (kGpoint.data) { - PORT_ZFree(kGpoint.data, 2*len + 1); + PORT_ZFree(kGpoint.data, 2*flen + 1); } if (err) { @@ -864,17 +866,15 @@ ECDSA_VerifyDigest(ECPublicKey *key, const SECItem *signature, #ifdef NSS_ENABLE_ECC mp_int r_, s_; /* tuple (r', s') is received signature) */ mp_int c, u1, u2, v; /* intermediate values used in verification */ - mp_int x1, y1; - mp_int x2, y2; + mp_int x1; mp_int n; mp_err err = MP_OKAY; - PRArenaPool *arena = NULL; ECParams *ecParams = NULL; SECItem localDigest; - SECItem pointA = { siBuffer, NULL, 0 }; - SECItem pointB = { siBuffer, NULL, 0 }; SECItem pointC = { siBuffer, NULL, 0 }; - int len; + int slen; /* length in bytes of a half signature (r or s) */ + int flen; /* length in bytes of the field size */ + unsigned olen; /* length in bytes of the base point order */ #if EC_DEBUG char mpstr[256]; @@ -889,9 +889,6 @@ ECDSA_VerifyDigest(ECPublicKey *key, const SECItem *signature, MP_DIGITS(&u1) = 0; MP_DIGITS(&u2) = 0; MP_DIGITS(&x1) = 0; - MP_DIGITS(&y1) = 0; - MP_DIGITS(&x2) = 0; - MP_DIGITS(&y2) = 0; MP_DIGITS(&v) = 0; MP_DIGITS(&n) = 0; @@ -901,30 +898,23 @@ ECDSA_VerifyDigest(ECPublicKey *key, const SECItem *signature, goto cleanup; } - - ecParams = &(key->ecParams); - len = (ecParams->fieldID.size + 7) >> 3; - if (signature->len < 2*len) { + if (signature->len == 0 || signature->len%2 != 0) { PORT_SetError(SEC_ERROR_INVALID_ARGS); goto cleanup; } + slen = signature->len/2; + ecParams = &(key->ecParams); + flen = (ecParams->fieldID.size + 7) >> 3; + olen = ecParams->order.len; - - /* ec defines signing with a larger digest than the keysize - * as signing with a truncated digest */ + /* truncate digest to the length of the base point order */ localDigest = *digest; - if (localDigest.len > len) { - localDigest.len = len; + if (localDigest.len > olen) { + localDigest.len = olen; } - /* Initialize an arena for pointA, pointB and pointC */ - if ((arena = PORT_NewArena(NSS_FREEBL_DEFAULT_CHUNKSIZE)) == NULL) - goto cleanup; - - SECITEM_AllocItem(arena, &pointA, 2*len + 1); - SECITEM_AllocItem(arena, &pointB, 2*len + 1); - SECITEM_AllocItem(arena, &pointC, 2*len + 1); - if (pointA.data == NULL || pointB.data == NULL || pointC.data == NULL) + SECITEM_AllocItem(NULL, &pointC, 2*flen + 1); + if (pointC.data == NULL) goto cleanup; CHECK_MPI_OK( mp_init(&r_) ); @@ -933,17 +923,14 @@ ECDSA_VerifyDigest(ECPublicKey *key, const SECItem *signature, CHECK_MPI_OK( mp_init(&u1) ); CHECK_MPI_OK( mp_init(&u2) ); CHECK_MPI_OK( mp_init(&x1) ); - CHECK_MPI_OK( mp_init(&y1) ); - CHECK_MPI_OK( mp_init(&x2) ); - CHECK_MPI_OK( mp_init(&y2) ); CHECK_MPI_OK( mp_init(&v) ); CHECK_MPI_OK( mp_init(&n) ); /* ** Convert received signature (r', s') into MPI integers. */ - CHECK_MPI_OK( mp_read_unsigned_octets(&r_, signature->data, len) ); - CHECK_MPI_OK( mp_read_unsigned_octets(&s_, signature->data + len, len) ); + CHECK_MPI_OK( mp_read_unsigned_octets(&r_, signature->data, slen) ); + CHECK_MPI_OK( mp_read_unsigned_octets(&s_, signature->data + slen, slen) ); /* ** ANSI X9.62, Section 5.4.2, Steps 1 and 2 @@ -952,8 +939,10 @@ ECDSA_VerifyDigest(ECPublicKey *key, const SECItem *signature, */ SECITEM_TO_MPINT(ecParams->order, &n); if (mp_cmp_z(&r_) <= 0 || mp_cmp_z(&s_) <= 0 || - mp_cmp(&r_, &n) >= 0 || mp_cmp(&s_, &n) >= 0) + mp_cmp(&r_, &n) >= 0 || mp_cmp(&s_, &n) >= 0) { + PORT_SetError(SEC_ERROR_BAD_SIGNATURE); goto cleanup; /* will return rv == SECFailure */ + } /* ** ANSI X9.62, Section 5.4.2, Step 3 @@ -996,13 +985,18 @@ ECDSA_VerifyDigest(ECPublicKey *key, const SECItem *signature, ** Here, A = u1.G B = u2.Q and C = A + B ** If the result, C, is the point at infinity, reject the signature */ - if ((ec_points_mul(ecParams, &u1, &u2, &key->publicValue, &pointC) == SECFailure) || - ec_point_at_infinity(&pointC)) { - rv = SECFailure; - goto cleanup; + if (ec_points_mul(ecParams, &u1, &u2, &key->publicValue, &pointC) + != SECSuccess) { + rv = SECFailure; + goto cleanup; + } + if (ec_point_at_infinity(&pointC)) { + PORT_SetError(SEC_ERROR_BAD_SIGNATURE); + rv = SECFailure; + goto cleanup; } - CHECK_MPI_OK( mp_read_unsigned_octets(&x1, pointC.data + 1, len) ); + CHECK_MPI_OK( mp_read_unsigned_octets(&x1, pointC.data + 1, flen) ); /* ** ANSI X9.62, Section 5.4.4, Step 2 @@ -1048,13 +1042,10 @@ cleanup: mp_clear(&u1); mp_clear(&u2); mp_clear(&x1); - mp_clear(&y1); - mp_clear(&x2); - mp_clear(&y2); mp_clear(&v); mp_clear(&n); - if (arena) PORT_FreeArena(arena, PR_TRUE); + if (pointC.data) SECITEM_FreeItem(&pointC, PR_FALSE); if (err) { MP_TO_SEC_ERROR(err); rv = SECFailure;