If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Use Horner's rule to calculate the elliptic curve polynomial in ec_GFp_validate_point

RESOLVED FIXED in 3.14.3

Status

NSS
Libraries
P2
minor
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Wan-Teh Chang, Assigned: Wan-Teh Chang)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Created attachment 708380 [details] [diff] [review]
Patch

The attached patch uses Horner's rule to calculate the
x^3 + a*x + b polynomial for elliptic curves over the
prime field.

This saves one group->meth->field_mul operation.
Attachment #708380 - Flags: review?(agl)
Attachment #708380 - Flags: feedback?(douglas)
(Assignee)

Comment 1

5 years ago
Created attachment 708383 [details] [diff] [review]
Patch with a local tmp variable (for reference only)

mp_add, mp_sub, and mp_mul all allow an in/out argument.
I take advantage of this in my patch.

This adds no overhead to mp_add and mp_sub. But to support
an in/out argument, mp_mul needs to copy the in/out argument
to an internal temp variable.

So, if we don't use an in/out argument with group->meth->field_mul,
we can avoid this copying inside mp_mul (which is eventually
called). Since we are copying only 32 - 65 bytes for P-256
and P-521 curves, this copying seems inexpensive, in exchange
for code clarity.

If you prefer this patch, please let me know.

Comment 2

5 years ago
Comment on attachment 708380 [details] [diff] [review]
Patch

I'm happy with either form of this patch (that is, with the additional |tmp| variable or not.)
Attachment #708380 - Flags: review?(agl) → review+
(Assignee)

Comment 3

5 years ago
Created attachment 708916 [details] [diff] [review]
Patch with a local tmp variable, v2

I checked in the patch that uses a local tmp variable. But
I removed the comment to avoid clutter in the code.

Checking in ecp_aff.c;
/cvsroot/mozilla/security/nss/lib/freebl/ecl/ecp_aff.c,v  <--  ecp_aff.c
new revision: 1.5; previous revision: 1.4
done
Attachment #708380 - Attachment is obsolete: true
Attachment #708383 - Attachment is obsolete: true
Attachment #708380 - Flags: feedback?(douglas)
Attachment #708916 - Flags: checked-in+
(Assignee)

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 4

5 years ago
Looks good to me!
You need to log in before you can comment on or make changes to this bug.