The EC_POINTs are still allocated (for now), but everything else fits on
the stack nicely, which saves a lot of fiddling with cleanup and
allocations.
Change-Id: Ib8480737ecc97e6b40b2c05f217cd8d3dc82cb72
Reviewed-on: https://boringssl-review.googlesource.com/25150
Reviewed-by: Adam Langley <agl@google.com>
This is to simplify clearing unnecessary mallocs out of ec_wNAF_mul, and
perhaps to use it in tuned variable-time multiplication functions.
Change-Id: Ic390d2e8e20d0ee50f3643830a582e94baebba95
Reviewed-on: https://boringssl-review.googlesource.com/25149
Reviewed-by: Adam Langley <agl@google.com>
This cuts out another total_num-length array and simplifies things.
Leading zeros at the front of the schedule don't do anything, so it's
easier to just produce a fixed-length one. (I'm also hoping to
ultimately reuse this function in //third_party/fiat/p256.c and get the
best of both worlds for ECDSA verification; tuned field arithmetic
operations, precomputed table, and variable-time multiply.)
Change-Id: I771f4ff7dcfdc3ee0eff8d9038d6dc9a0be3d4e0
Reviewed-on: https://boringssl-review.googlesource.com/25148
Reviewed-by: Adam Langley <agl@google.com>
Note this switches from walking BN_num_bits to the full bit length of
the scalar. But that can only cause it to add a few extra zeros to the
front of the schedule, which r_is_at_infinity will skip over.
Change-Id: I91e087c9c03505566b68f75fb37dfb53db467652
Reviewed-on: https://boringssl-review.googlesource.com/25147
Reviewed-by: Adam Langley <agl@google.com>
This appears to be pointless. Before, we would have a 50% chance of
doing an inversion at each non-zero bit but the first
(r_is_at_infinity), plus a 50% chance of doing an inversion at the end.
Now we would have a 50% chance of doing an inversion at each non-zero
bit. That's the same number of coin flips.
Change-Id: I8158fd48601cb041188826d4f68ac1a31a6fbbbc
Reviewed-on: https://boringssl-review.googlesource.com/25146
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The optimization for wsize = 1 only kicks in for 19-bit primes. The
cases for b >= 800 and cannot happen due to EC_MAX_SCALAR_BYTES.
Change-Id: If5ca908563f027172cdf31c9a22342152fecd12f
Reviewed-on: https://boringssl-review.googlesource.com/25145
Reviewed-by: Adam Langley <agl@google.com>
Simplify things slightly. The probability of the scalar being small
enough to go down a window size is astronomically small. (2^-186 for
P-256 and 2^-84 for P-384.)
Change-Id: Ie879f0b06bcfd1e6e6e3bf3f54e0d7d6567525a4
Reviewed-on: https://boringssl-review.googlesource.com/25144
Reviewed-by: Adam Langley <agl@google.com>
None of the asymmetric crypto we inherented from OpenSSL is
constant-time because of BIGNUM. BIGNUM chops leading zeros off the
front of everything, so we end up leaking information about the first
word, in theory. BIGNUM functions additionally tend to take the full
range of inputs and then call into BN_nnmod at various points.
All our secret values should be acted on in constant-time, but k in
ECDSA is a particularly sensitive value. So, ecdsa_sign_setup, in an
attempt to mitigate the BIGNUM leaks, would add a couple copies of the
order.
This does not work at all. k is used to compute two values: k^-1 and kG.
The first operation when computing k^-1 is to call BN_nnmod if k is out
of range. The entry point to our tuned constant-time curve
implementations is to call BN_nnmod if the scalar has too many bits,
which this causes. The result is both corrections are immediately undone
but cause us to do more variable-time work in the meantime.
Replace all these computations around k with the word-based functions
added in the various preceding CLs. In doing so, replace the BN_mod_mul
calls (which internally call BN_nnmod) with Montgomery reduction. We can
avoid taking k^-1 out of Montgomery form, which combines nicely with
Brian Smith's trick in 3426d10119. Along
the way, we avoid some unnecessary mallocs.
BIGNUM still affects the private key itself, as well as the EC_POINTs.
But this should hopefully be much better now. Also it's 10% faster:
Before:
Did 15000 ECDSA P-224 signing operations in 1069117us (14030.3 ops/sec)
Did 18000 ECDSA P-256 signing operations in 1053908us (17079.3 ops/sec)
Did 1078 ECDSA P-384 signing operations in 1087853us (990.9 ops/sec)
Did 473 ECDSA P-521 signing operations in 1069835us (442.1 ops/sec)
After:
Did 16000 ECDSA P-224 signing operations in 1064799us (15026.3 ops/sec)
Did 19000 ECDSA P-256 signing operations in 1007839us (18852.2 ops/sec)
Did 1078 ECDSA P-384 signing operations in 1079413us (998.7 ops/sec)
Did 484 ECDSA P-521 signing operations in 1083616us (446.7 ops/sec)
Change-Id: I2a25e90fc99dac13c0616d0ea45e125a4bd8cca1
Reviewed-on: https://boringssl-review.googlesource.com/23075
Reviewed-by: Adam Langley <agl@google.com>
crypto/{asn1,x509,x509v3,pem} were skipped as they are still OpenSSL
style.
Change-Id: I3cd9a60e1cb483a981aca325041f3fbce294247c
Reviewed-on: https://boringssl-review.googlesource.com/19504
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The names in the P-224 code collided with the P-256 code and thus many
of the functions and constants in the P-224 code have been prefixed.
Change-Id: I6bcd304640c539d0483d129d5eaf1702894929a8
Reviewed-on: https://boringssl-review.googlesource.com/15847
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
