Commit Graph

65 Commits

Author SHA1 Message Date
Adam Langley
9edbc7ff9f Revert "Revert "Speed up ECDSA verify on x86-64.""
This reverts commit e907ed4c4b. CPUID
checks have been added so hopefully this time sticks.

Change-Id: I5e0e5b87427c1230132681f936b3c70bac8263b8
Reviewed-on: https://boringssl-review.googlesource.com/c/32924
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-11-07 23:57:22 +00:00
Adam Langley
e907ed4c4b Revert "Speed up ECDSA verify on x86-64."
This reverts commit 3d450d2844. It fails
SDE, looks like a missing CPUID check before using vector instructions.

Change-Id: I6b7dd71d9e5b1f509d2e018bd8be38c973476b4e
Reviewed-on: https://boringssl-review.googlesource.com/c/32864
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
2018-11-06 00:29:15 +00:00
David Benjamin
cfd50c63a1 Route the tuned add/dbl implementations out of EC_METHOD.
Some consumer stumbled upon EC_POINT_{add,dbl} being faster with a
"custom" P-224 curve than the built-in one and made "custom" clones to
work around this. Before the EC_FELEM refactor, EC_GFp_nistp224_method
used BN_mod_mul for all reductions in fallback point arithmetic (we
primarily support the multiplication functions and keep the low-level
point arithmetic for legacy reasons) which took quite a performance hit.

EC_FELEM fixed this, but standalone felem_{mul,sqr} calls out of
nistp224 perform a lot of reductions, rather than batching them up as
that implementation is intended. So it is still slightly faster to use a
"custom" curve.

Custom curves are the last thing we want to encourage, so just route the
tuned implementations out of EC_METHOD to close this gap. Now the
built-in implementation is always solidly faster than (or identical to)
the custom clone.  This also reduces the number of places where we mix
up tuned vs. generic implementation, which gets us closer to making
EC_POINT's representation EC_METHOD-specific.

Change-Id: I843e1101a6208eaabb56d29d342e886e523c78b4
Reviewed-on: https://boringssl-review.googlesource.com/c/32848
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-11-06 00:17:19 +00:00
Nir Drucker
3d450d2844 Speed up ECDSA verify on x86-64.
This commit improves the performance of ECDSA signature verification
(over NIST P-256 curve) for x86 platforms. The speedup is by a factor of 1.15x.
It does so by:
  1) Leveraging the fact that the verification does not need
     to run in constant time. To this end, we implemented:
    a) the function ecp_nistz256_points_mul_public in a similar way to
       the current ecp_nistz256_points_mul function by removing its constant
       time features.
    b) the Binary Extended Euclidean Algorithm (BEEU) in x86 assembly to
       replace the current modular inverse function used for the inversion.
  2) The last step in the ECDSA_verify function compares the (x) affine
     coordinate with the signature (r) value. Converting x from the Jacobian's
     representation to the affine coordinate requires to perform one inversions
     (x_affine = x * z^(-2)). We save this inversion and speed up the computations
     by instead bringing r to x (r_jacobian = r*z^2) which is faster.

The measured results are:
Before (on a Kaby Lake desktop with gcc-5):
Did 26000 ECDSA P-224 signing operations in 1002372us (25938.5 ops/sec)
Did 11000 ECDSA P-224 verify operations in 1043821us (10538.2 ops/sec)
Did 55000 ECDSA P-256 signing operations in 1017560us (54050.9 ops/sec)
Did 17000 ECDSA P-256 verify operations in 1051280us (16170.8 ops/sec)

After (on a Kaby Lake desktop with gcc-5):
Did 27000 ECDSA P-224 signing operations in 1011287us (26698.7 ops/sec)
Did 11640 ECDSA P-224 verify operations in 1076698us (10810.8 ops/sec)
Did 55000 ECDSA P-256 signing operations in 1016880us (54087.0 ops/sec)
Did 20000 ECDSA P-256 verify operations in 1038736us (19254.2 ops/sec)

Before (on a Skylake server platform with gcc-5):
Did 25000 ECDSA P-224 signing operations in 1021651us (24470.2 ops/sec)
Did 10373 ECDSA P-224 verify operations in 1046563us (9911.5 ops/sec)
Did 50000 ECDSA P-256 signing operations in 1002774us (49861.7 ops/sec)
Did 15000 ECDSA P-256 verify operations in 1006471us (14903.6 ops/sec)

After (on a Skylake server platform with gcc-5):
Did 25000 ECDSA P-224 signing operations in 1020958us (24486.8 ops/sec)
Did 10373 ECDSA P-224 verify operations in 1046359us (9913.4 ops/sec)
Did 50000 ECDSA P-256 signing operations in 1003996us (49801.0 ops/sec)
Did 18000 ECDSA P-256 verify operations in 1021604us (17619.4 ops/sec)

Developers and authors:
***************************************************************************
Nir Drucker (1,2), Shay Gueron (1,2)
(1) Amazon Web Services Inc.
(2) University of Haifa, Israel
***************************************************************************

Change-Id: Idd42a7bc40626bce974ea000b61fdb5bad33851c
Reviewed-on: https://boringssl-review.googlesource.com/c/31304
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2018-11-05 23:48:07 +00:00
Joshua Liebow-Feeser
8c7c6356e6 Support symbol prefixes
- In base.h, if BORINGSSL_PREFIX is defined, include
  boringssl_prefix_symbols.h
- In all .S files, if BORINGSSL_PREFIX is defined, include
  boringssl_prefix_symbols_asm.h
- In base.h, BSSL_NAMESPACE_BEGIN and BSSL_NAMESPACE_END are
  defined with appropriate values depending on whether
  BORINGSSL_PREFIX is defined; these macros are used in place
  of 'namespace bssl {' and '}'
- Add util/make_prefix_headers.go, which takes a list of symbols
  and auto-generates the header files mentioned above
- In CMakeLists.txt, if BORINGSSL_PREFIX and BORINGSSL_PREFIX_SYMBOLS
  are defined, run util/make_prefix_headers.go to generate header
  files
- In various CMakeLists.txt files, add "global_target" that all
  targets depend on to give us a place to hook logic that must run
  before all other targets (in particular, the header file generation
  logic)
- Document this in BUILDING.md, including the fact that it is
  the caller's responsibility to provide the symbol list and keep it
  up to date
- Note that this scheme has not been tested on Windows, and likely
  does not work on it; Windows support will need to be added in a
  future commit

Change-Id: If66a7157f46b5b66230ef91e15826b910cf979a2
Reviewed-on: https://boringssl-review.googlesource.com/31364
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-09-06 20:07:52 +00:00
Joshua Liebow-Feeser
67e64342c1 Document that ED25519_sign only fails on allocation failure
Change-Id: I45866c3a4aa98ebac51d4e554a22eb5add45002f
Reviewed-on: https://boringssl-review.googlesource.com/31404
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>
2018-08-29 18:35:12 +00:00
David Benjamin
bdc409801f Add new curve/hash ECDSA combinations from Wycheproof.
Change-Id: I7bb36c4e4108a2b7d9481ab2cafc245ea31927c0
Reviewed-on: https://boringssl-review.googlesource.com/30847
Reviewed-by: Adam Langley <agl@google.com>
2018-08-10 18:26:06 +00:00
David Benjamin
af37f84840 Add RSA-PSS tests from Wycheproof.
Along the way, split up the EVPTest Wycheproof tests into separate tests (they
shard better when running in parallel).

Change-Id: I5ee919f7ec7c35a7f2e0cc2af4142991a808a9db
Reviewed-on: https://boringssl-review.googlesource.com/30846
Reviewed-by: Adam Langley <agl@google.com>
2018-08-10 18:26:00 +00:00
David Benjamin
f84c0dad7a Use newly-sharded ECDH tests.
Also remove some transition step for a recent format change. Together, this
removes the curve hacks in the converter, which can now be purely syntactic.
The RSA ones are still a bit all over the place in terms of sharded vs
combined, so leaving that alone for now.

Change-Id: I721d6b0de388a53a39543725e366dc5b52e83561
Reviewed-on: https://boringssl-review.googlesource.com/30845
Reviewed-by: Adam Langley <agl@google.com>
2018-08-10 18:25:51 +00:00
David Benjamin
a711b53e0b Update Wycheproof test vectors.
This only updates the repository. We'll catch up with the new tests in a
subsequent commit.

Change-Id: I074a041479159ce1141af3241e7158599b648365
Reviewed-on: https://boringssl-review.googlesource.com/30844
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2018-08-10 17:56:29 +00:00
David Benjamin
42ea84b317 Update Wycheproof test vectors.
They've since added new files that split up ECDH and RSA. The former especially
could be useful. A later commit will switch to those. Along the way, fix the
aes_cmac_test.json entry in the convert_wycheproof.go which got lost at some
point.

Change-Id: I9c4a2e5fc5f3e0935482f583c5466c1b64fe325e
Reviewed-on: https://boringssl-review.googlesource.com/29686
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-07-13 20:46:20 +00:00
Adam Langley
576b637861 Move convert_wycheproof.go to util/
This file is not part of the Wycheproof project and consumers of
BoringSSL who wish to provide Wycheproof themselves (and not have
third_party/wycheproof_testvectors) need it in another location.

Change-Id: I730fe294f46a9aac77b858a91a03ee64fb8ea579
Reviewed-on: https://boringssl-review.googlesource.com/28704
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-05-22 17:16:36 +00:00
David Benjamin
62abcebb01 Add a driver for Wycheproof CMAC tests.
Change-Id: Iafe81d22647c99167ab27a5345cfa970755112ac
Reviewed-on: https://boringssl-review.googlesource.com/28465
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-05-15 21:19:42 +00:00
Martin Kreichgauer
044f637fef reformat third_party/wycheproof_testvectors/METADATA
Change-Id: Ib12f41dec023e20dfd1182513bf11571950d7c85
Reviewed-on: https://boringssl-review.googlesource.com/28245
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>
2018-05-08 19:00:35 +00:00
David Benjamin
bf33114b51 Rename third_party/wycheproof to satisfy a bureaucrat.
Make it clear this is not a pristine full copy of all of Wycheproof as a
library.

Change-Id: I1aa5253a1d7c696e69b2e8d7897924f15303d9ac
Reviewed-on: https://boringssl-review.googlesource.com/28188
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-05-07 18:33:50 +00:00
David Benjamin
179c4e257a Update Wycheproof, add keywrap tests, and fix a bug.
The bug, courtesy of Wycheproof, is that AES key wrap requires the input
be at least two blocks, not one. This also matches the OpenSSL behavior
of those two APIs.

Update-Note: AES_wrap_key with in_len = 8 and AES_unwrap_key with
in_len = 16 will no longer work.

Change-Id: I5fc63ebc16920c2f9fd488afe8c544e0647d7507
Reviewed-on: https://boringssl-review.googlesource.com/27925
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-05-04 17:08:44 +00:00
David Benjamin
8e75ae4880 Add a Wycheproof driver for AES-CBC.
Change-Id: I782ea51e1db8d05f552832a7c6910954fa2dda5f
Reviewed-on: https://boringssl-review.googlesource.com/27924
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-05-02 19:41:48 +00:00
David Benjamin
302bb3964a Small curve25519 cleanups.
Per Brian, x25519_ge_frombytes_vartime does not match the usual
BoringSSL return value convention, and we're slightly inconsistent about
whether to mask the last byte with 63 or 127. (It then gets ANDed with
64, so it doesn't matter which.) Use 127 to align with the curve25519
RFC. Finally, when we invert the transformation, use the same constants
inverted so that they're parallel.

Bug: 243, 244
Change-Id: I0e3aca0433ead210446c58d86b2f57526bde1eac
Reviewed-on: https://boringssl-review.googlesource.com/27984
Reviewed-by: Adam Langley <agl@google.com>
2018-05-02 19:24:00 +00:00
David Benjamin
3f944674b2 Add an ECDH Wycheproof driver.
Unfortunately, this driver suffers a lot from Wycheproof's Java
heritgate, but so it goes. Their test formats bake in a lot of Java API
mistakes.

Change-Id: I3299e85efb58e99e4fa34841709c3bea6518968d
Reviewed-on: https://boringssl-review.googlesource.com/27865
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-05-01 19:38:07 +00:00
David Benjamin
7760af4bce Print tcId in converted Wycheproof files.
This is to make it easier to correlate the two.

Change-Id: I62aa381499d67ae279bbe86eebeb9a5bc9ef5266
Reviewed-on: https://boringssl-review.googlesource.com/27864
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-05-01 19:09:16 +00:00
David Benjamin
5505328633 Add AEAD Wycheproof drivers.
Change-Id: I840863c445fd9dac3fd60ac4b1c572ea7d924c9c
Reviewed-on: https://boringssl-review.googlesource.com/27826
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-05-01 18:36:00 +00:00
David Benjamin
c596415ec6 Add a DSA Wycheproof driver.
DSA is deprecated and will ultimately be removed but, in the
meantime, it still ought to be tested.

Change-Id: I75af25430b8937a43b11dced1543a98f7a6fbbd3
Reviewed-on: https://boringssl-review.googlesource.com/27825
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-04-30 16:04:31 +00:00
David Benjamin
5707274214 Add Ed25519 Wycheproof driver.
This works with basically no modifications.

Change-Id: I92f4d90f3c0ec8170d532cf7872754fadb36644d
Reviewed-on: https://boringssl-review.googlesource.com/27824
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-04-30 15:29:01 +00:00
David Benjamin
041dd68cec Clear mallocs in ec_wNAF_mul.
EC_POINT is split into the existing public EC_POINT (where the caller is
sanity-checked about group mismatches) and the low-level EC_RAW_POINT
(which, like EC_FELEM and EC_SCALAR, assume that is your problem and is
a plain old struct). Having both EC_POINT and EC_RAW_POINT is a little
silly, but we're going to want different type signatures for functions
which return void anyway (my plan is to lift a non-BIGNUM
get_affine_coordinates up through the ECDSA and ECDH code), so I think
it's fine.

This wasn't strictly necessary, but wnaf.c is a lot tidier now. Perf is
a wash; once we get up to this layer, it's only 8 entries in the table
so not particularly interesting.

Bug: 239
Change-Id: I8ace749393d359f42649a5bb0734597bb7c07a2e
Reviewed-on: https://boringssl-review.googlesource.com/27706
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-04-27 19:44:58 +00:00
David Benjamin
e14e4a7ee3 Remove ec_compute_wNAF's failure cases.
Replace them with asserts and better justify why each of the internal
cases are not reachable. Also change the loop to count up to bits+1 so
it is obvious there is no memory error. (The previous loop shape made
more sense when ec_compute_wNAF would return a variable length
schedule.)

Change-Id: I9c7df6abac4290b7a3e545e3d4aa1462108e239e
Reviewed-on: https://boringssl-review.googlesource.com/27705
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-04-27 19:24:58 +00:00
David Benjamin
40d76f4f7d Add ECDSA and RSA verify Wycheproof drivers.
Along the way, add some utility functions for getting common things
(curves, hashes, etc.) in the names Wycheproof uses.

Change-Id: I09c11ea2970cf2c8a11a8c2a861d85396efda125
Reviewed-on: https://boringssl-review.googlesource.com/27786
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-04-27 18:58:38 +00:00
David Benjamin
5509bc06d8 Add a test driver for Wycheproof's x25519_test.json.
FileTest and Wycheproof express more-or-less the same things, so I've
just written a script to mechanically convert them. Saves writing a JSON
parser.

I've also left a TODO with other files that are worth converting. Per
Thai, the webcrypto variants of the files are just a different format
and will later be consolidated, so I've ignored those. The
curve/hash-specific ECDSA files and the combined one are intended to be
the same, so I've ignored the combined one. (Just by test counts, there
are some discrepancies, but Thai says he'll fix that and we can update
when that happens.)

Change-Id: I5fcbd5cb0e1bea32964b09fb469cb43410f53c2d
Reviewed-on: https://boringssl-review.googlesource.com/27785
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-04-27 18:55:38 +00:00
David Benjamin
2d10c3688c Check in a copy of Project Wycheproof test vectors.
This is just a pristine copy of the JSON files for now. It's not hooked
up to anything yet.

Change-Id: I608b4b0368578f159cad23950d70578ff4c23da3
Reviewed-on: https://boringssl-review.googlesource.com/27784
Reviewed-by: Adam Langley <agl@google.com>
2018-04-26 23:07:29 +00:00
David Benjamin
32e0d10069 Add EC_FELEM for EC_POINTs and related temporaries.
This introduces EC_FELEM, which is analogous to EC_SCALAR. It is used
for EC_POINT's representation in the generic EC_METHOD, as well as
random operations on tuned EC_METHODs that still are implemented
genericly.

Unlike EC_SCALAR, EC_FELEM's exact representation is awkwardly specific
to the EC_METHOD, analogous to how the old values were BIGNUMs but may
or may not have been in Montgomery form. This is kind of a nuisance, but
no more than before. (If p224-64.c were easily convertable to Montgomery
form, we could say |EC_FELEM| is always in Montgomery form. If we
exposed the internal add and double implementations in each of the
curves, we could give |EC_POINT| an |EC_METHOD|-specific representation
and |EC_FELEM| is purely a |EC_GFp_mont_method| type. I'll leave this
for later.)

The generic add and doubling formulas are aligned with the formulas
proved in fiat-crypto. Those only applied to a = -3, so I've proved a
generic one in https://github.com/mit-plv/fiat-crypto/pull/356, in case
someone uses a custom curve.  The new formulas are verified,
constant-time, and swap a multiply for a square. As expressed in
fiat-crypto they do use more temporaries, but this seems to be fine with
stack-allocated EC_FELEMs. (We can try to help the compiler later,
but benchamrks below suggest this isn't necessary.)

Unlike BIGNUM, EC_FELEM can be stack-allocated. It also captures the
bounds in the type system and, in particular, that the width is correct,
which will make it easier to select a point in constant-time in the
future. (Indeed the old code did not always have the correct width. Its
point formula involved halving and implemented this in variable time and
variable width.)

Before:
Did 77274 ECDH P-256 operations in 10046087us (7692.0 ops/sec)
Did 5959 ECDH P-384 operations in 10031701us (594.0 ops/sec)
Did 10815 ECDSA P-384 signing operations in 10087892us (1072.1 ops/sec)
Did 8976 ECDSA P-384 verify operations in 10071038us (891.3 ops/sec)
Did 2600 ECDH P-521 operations in 10091688us (257.6 ops/sec)
Did 4590 ECDSA P-521 signing operations in 10055195us (456.5 ops/sec)
Did 3811 ECDSA P-521 verify operations in 10003574us (381.0 ops/sec)

After:
Did 77736 ECDH P-256 operations in 10029858us (7750.5 ops/sec) [+0.8%]
Did 7519 ECDH P-384 operations in 10068076us (746.8 ops/sec) [+25.7%]
Did 13335 ECDSA P-384 signing operations in 10029962us (1329.5 ops/sec) [+24.0%]
Did 11021 ECDSA P-384 verify operations in 10088600us (1092.4 ops/sec) [+22.6%]
Did 2912 ECDH P-521 operations in 10001325us (291.2 ops/sec) [+13.0%]
Did 5150 ECDSA P-521 signing operations in 10027462us (513.6 ops/sec) [+12.5%]
Did 4264 ECDSA P-521 verify operations in 10069694us (423.4 ops/sec) [+11.1%]

This more than pays for removing points_make_affine previously and even
speeds up ECDH P-256 slightly. (The point-on-curve check uses the
generic code.)

Next is to push the stack-allocating up to ec_wNAF_mul, followed by a
constant-time single-point multiplication.

Bug: 239
Change-Id: I44a2dff7c52522e491d0f8cffff64c4ab5cd353c
Reviewed-on: https://boringssl-review.googlesource.com/27668
Reviewed-by: Adam Langley <agl@google.com>
2018-04-25 16:39:58 +00:00
David Benjamin
364a51ec3a Abstract scalar inversion in EC_METHOD.
This introduces a hook for the OpenSSL assembly.

Change-Id: I35e0588f0ed5bed375b12f738d16c9f46ceedeea
Reviewed-on: https://boringssl-review.googlesource.com/27592
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-04-24 16:13:24 +00:00
David Benjamin
5fca613918 Fix typo in point_add.
Rather than writing the answer into the output, it wrote it into some
awkwardly-named temporaries. Thanks to Daniel Hirche for reporting this
issue!

Bug: chromium:825273
Change-Id: I5def4be045cd1925453c9873218e5449bf25e3f5
Reviewed-on: https://boringssl-review.googlesource.com/26785
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>
2018-03-23 21:12:29 +00:00
Daniel Hirche
8d4f7e5421 Remove redundant assertion in fe_mul_121666_impl.
Change-Id: Ie2368dc9f6be791b7c3ad1c610dcd603634be6e4
Reviewed-on: https://boringssl-review.googlesource.com/26244
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>
2018-02-27 23:50:02 +00:00
Martin Kreichgauer
8041d8c40e third_party: re-format METATADA files
Change-Id: Ic2e9f54f5ced053c1463d5c09a74db5b2a3ea098
Reviewed-on: https://boringssl-review.googlesource.com/26224
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>
2018-02-27 19:57:12 +00:00
David Benjamin
638a408cd2 Add a tuned variable-time P-256 multiplication function.
This reuses wnaf.c's window scheduling, but has access to the tuned
field arithemetic and pre-computed base point table. Unlike wnaf.c, we
do not make the points affine as it's not worth it for a single table.
(We already precomputed the base point table.)

Annoyingly, 32-bit x86 gets slower by a bit, but the other platforms are
faster. My guess is that that the generic code gets to use the
bn_mul_mont assembly and the compiler, faced with the increased 32-bit
register pressure and the extremely register-poor x86, is making
bad decisions on the otherwise P-256-tuned C code. The three platforms
that see much larger gains are significantly more important than 32-bit
x86 at this point, so go with this change.

armv7a (Nexus 5X) before/after [+14.4%]:
Did 2703 ECDSA P-256 verify operations in 5034539us (536.9 ops/sec)
Did 3127 ECDSA P-256 verify operations in 5091379us (614.2 ops/sec)

aarch64 (Nexus 5X) before/after [+9.2%]:
Did 6783 ECDSA P-256 verify operations in 5031324us (1348.2 ops/sec)
Did 7410 ECDSA P-256 verify operations in 5033291us (1472.2 ops/sec)

x86 before/after [-2.7%]:
Did 8961 ECDSA P-256 verify operations in 10075901us (889.3 ops/sec)
Did 8568 ECDSA P-256 verify operations in 10003001us (856.5 ops/sec)

x86_64 before/after [+8.6%]:
Did 29808 ECDSA P-256 verify operations in 10008662us (2978.2 ops/sec)
Did 32528 ECDSA P-256 verify operations in 10057137us (3234.3 ops/sec)

Change-Id: I5fa643149f5bfbbda9533e3008baadfee9979b93
Reviewed-on: https://boringssl-review.googlesource.com/25684
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-02-12 22:00:48 +00:00
Adam Langley
472ba2c2dd Require that Ed25519 |s| values be < order.
https://tools.ietf.org/html/rfc8032#section-5.1.7 adds this requirement
to prevent signature malleability.

Change-Id: Iac9a3649d97fc69e6efb4aea1ab1e002768fadc9
Reviewed-on: https://boringssl-review.googlesource.com/25564
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-02-02 20:45:08 +00:00
David Benjamin
884086e0e2 Remove x86_64 x25519 assembly.
Now that we have 64-bit C code, courtesy of fiat-crypto, the tradeoff
for carrying the assembly changes:

Assembly:
Did 16000 Curve25519 base-point multiplication operations in 1059932us (15095.3 ops/sec)
Did 16000 Curve25519 arbitrary point multiplication operations in 1060023us (15094.0 ops/sec)

fiat64:
Did 39000 Curve25519 base-point multiplication operations in 1004712us (38817.1 ops/sec)
Did 14000 Curve25519 arbitrary point multiplication operations in 1006827us (13905.1 ops/sec)

The assembly is still about 9% faster than fiat64, but fiat64 gets to
use the Ed25519 tables for the base point multiplication, so overall it
is actually faster to disable the assembly:

>>> 1/(1/15094.0 + 1/15095.3)
7547.324986004976
>>> 1/(1/38817.1 + 1/13905.1)
10237.73016319501

(At the cost of touching a 30kB table.)

The assembly implementation is no longer pulling its weight. Remove it
and use the fiat code in all build configurations.

Change-Id: Id736873177d5568bb16ea06994b9fcb1af104e33
Reviewed-on: https://boringssl-review.googlesource.com/25524
Reviewed-by: Adam Langley <agl@google.com>
2018-02-01 21:44:58 +00:00
David Benjamin
cb1ad205d0 Use 51-bit limbs from fiat-crypto in 64-bit.
Our 64-bit performance was much lower than it could have been, since we
weren't using the 64-bit multipliers. Fortunately, fiat-crypto is
awesome, so this is just a matter of synthesizing new code and
integration work.

Functions without the signature fiat-crypto curly braces were written by
hand and warrant more review. (It's just redistributing some bits.)

These use the donna variants which takes (and proves) some of the
instruction scheduling from donna as that's significantly faster.
Glancing over things, I suspect but have not confirmed the gap is due to
this:
https://github.com/mit-plv/fiat-crypto/pull/295#issuecomment-356892413

Clang without OPENSSL_SMALL (ECDH omitted since that uses assembly and
is unaffected by this CL).

Before:
Did 105149 Ed25519 key generation operations in 5025208us (20924.3 ops/sec)
Did 125000 Ed25519 signing operations in 5024003us (24880.6 ops/sec)
Did 37642 Ed25519 verify operations in 5072539us (7420.7 ops/sec)

After:
Did 206000 Ed25519 key generation operations in 5020547us (41031.4 ops/sec)
Did 227000 Ed25519 signing operations in 5005232us (45352.5 ops/sec)
Did 69840 Ed25519 verify operations in 5004769us (13954.7 ops/sec)

Clang + OPENSSL_SMALL:

Before:
Did 68598 Ed25519 key generation operations in 5024629us (13652.4 ops/sec)
Did 73000 Ed25519 signing operations in 5067837us (14404.6 ops/sec)
Did 36765 Ed25519 verify operations in 5078684us (7239.1 ops/sec)
Did 74000 Curve25519 base-point multiplication operations in 5016465us (14751.4 ops/sec)
Did 45600 Curve25519 arbitrary point multiplication operations in 5034680us (9057.2 ops/sec)

After:
Did 117315 Ed25519 key generation operations in 5021860us (23360.9 ops/sec)
Did 126000 Ed25519 signing operations in 5003521us (25182.3 ops/sec)
Did 64974 Ed25519 verify operations in 5047790us (12871.8 ops/sec)
Did 134000 Curve25519 base-point multiplication operations in 5058946us (26487.7 ops/sec)
Did 86000 Curve25519 arbitrary point multiplication operations in 5050478us (17028.1 ops/sec)

GCC without OPENSSL_SMALL (ECDH omitted since that uses assembly and
is unaffected by this CL).

Before:
Did 35552 Ed25519 key generation operations in 5030756us (7066.9 ops/sec)
Did 38286 Ed25519 signing operations in 5001648us (7654.7 ops/sec)
Did 10584 Ed25519 verify operations in 5068158us (2088.3 ops/sec)

After:
Did 92158 Ed25519 key generation operations in 5024021us (18343.5 ops/sec)
Did 99000 Ed25519 signing operations in 5011908us (19753.0 ops/sec)
Did 31122 Ed25519 verify operations in 5069878us (6138.6 ops/sec)

Change-Id: Ic0c24d50b4ee2bbc408b94965e9d63319936107d
Reviewed-on: https://boringssl-review.googlesource.com/24805
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-01-23 22:25:07 +00:00
David Benjamin
a42d7bee85 Reorganize curve25519.c slightly.
Adding 51-bit limbs will require two implementations of most of the
field operations. Group them together to make this more manageable. Also
move the representation-independent functions to the end.

Change-Id: I264e8ac64318a1d5fa72e6ad6f7ccf2f0a2c2be9
Reviewed-on: https://boringssl-review.googlesource.com/24804
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-01-23 21:42:14 +00:00
David Benjamin
0c1eafc6fe Add additional constants to make_curve25519_tables.py.
These are also constants that depend on the field representation.

Change-Id: I22333c099352ad64eb27fe15ffdc38c6ae7c07ff
Reviewed-on: https://boringssl-review.googlesource.com/24746
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-01-23 21:35:03 +00:00
David Benjamin
2d77d4084a Generate curve25519 tables with a script.
This is to make it easier to add new field element representations. The
Ed25519 logic in the script is partially adapted from RFC 8032's Python
code, but I replaced the point addition logic with the naive textbook
formula since this script only cares about being obviously correct.

Change-Id: I0b90bf470993c177070fd1010ac5865fedb46c82
Reviewed-on: https://boringssl-review.googlesource.com/24745
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-01-23 21:21:54 +00:00
David Benjamin
042b49cf3c Extract curve25519 tables into a separate header.
This is in preparation for writing a script to generate them. I'm
manually moving the existing tables over so it will be easier to confirm
the script didn't change the values.

Change-Id: Id83e95c80d981e19d1179d45bf47559b3e1fc86e
Reviewed-on: https://boringssl-review.googlesource.com/24744
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-01-23 21:08:49 +00:00
David Benjamin
186df3a655 Implement fe_sq2_tt with fe_sq_tt.
fiat-crypto only generates fe_mul and fe_sq, but the original Ed25519
implementation we had also had fe_sq2 for computing 2*f^2. Previously,
we inlined a version of fe_mul.

Instead, we could implement it with fe_sq and fe_add. Performance-wise,
this seems to not regress. If anything, it makes it faster?

Before (clang, run for 10 seconds):
Did 243000 Ed25519 key generation operations in 10025910us (24237.2 ops/sec)
Did 250000 Ed25519 signing operations in 10035580us (24911.4 ops/sec)
Did 73305 Ed25519 verify operations in 10071101us (7278.7 ops/sec)
Did 184000 Curve25519 base-point multiplication operations in 10040138us (18326.4 ops/sec)
Did 186000 Curve25519 arbitrary point multiplication operations in 10052721us (18502.5 ops/sec)

After (clang, run for 10 seconds):
Did 242424 Ed25519 key generation operations in 10013117us (24210.6 ops/sec)
Did 253000 Ed25519 signing operations in 10011744us (25270.3 ops/sec)
Did 73899 Ed25519 verify operations in 10048040us (7354.6 ops/sec)
Did 194000 Curve25519 base-point multiplication operations in 10005389us (19389.6 ops/sec)
Did 195000 Curve25519 arbitrary point multiplication operations in 10028443us (19444.7 ops/sec)

Before (clang + OPENSSL_SMALL, run for 10 seconds):
Did 144000 Ed25519 key generation operations in 10019344us (14372.2 ops/sec)
Did 146000 Ed25519 signing operations in 10011653us (14583.0 ops/sec)
Did 74052 Ed25519 verify operations in 10005789us (7400.9 ops/sec)
Did 150000 Curve25519 base-point multiplication operations in 10007468us (14988.8 ops/sec)
Did 91392 Curve25519 arbitrary point multiplication operations in 10057678us (9086.8 ops/sec)

After (clang + OPENSSL_SMALL, run for 10 seconds):
Did 144000 Ed25519 key generation operations in 10066724us (14304.6 ops/sec)
Did 148000 Ed25519 signing operations in 10062043us (14708.7 ops/sec)
Did 74820 Ed25519 verify operations in 10058557us (7438.4 ops/sec)
Did 151000 Curve25519 base-point multiplication operations in 10063492us (15004.7 ops/sec)
Did 90402 Curve25519 arbitrary point multiplication operations in 10049141us (8996.0 ops/sec)

Change-Id: I31e9f61833492c3ff2dfd78e1dee5e06f43c850f
Reviewed-on: https://boringssl-review.googlesource.com/24724
Reviewed-by: Adam Langley <agl@google.com>
2018-01-23 20:49:50 +00:00
David Benjamin
94cd196a80 Add files in third_party/fiat for Chromium to pick up.
Chromium's licenses.py is a little finicky.

Change-Id: I015a3565eb8f3cfecb357d142facc796a9c80888
Reviewed-on: https://boringssl-review.googlesource.com/24784
Reviewed-by: Adam Langley <agl@google.com>
2018-01-10 22:02:03 +00:00
David Benjamin
b6317b98ee Update googletest.
The latest MSVC 2017 complains about std::tr1::tuple, which was fixed in
upstream GTest.

Upstream have also merged all our patches, we now no longer are carrying
a diff. (Thanks, Gennadiy!)

Change-Id: I6932687b8e8c1eff8c2edf42da0a12080e7b61dd
Reviewed-on: https://boringssl-review.googlesource.com/24685
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-10 12:11:44 +00:00
David Benjamin
0c9b7b5de2 Align various point_get_affine_coordinates implementations.
The P-224 implementation was missing the optimization to avoid doing
extra work when asking for only one coordinate (ECDH and ECDSA both
involve an x-coordinate query). The P-256 implementation was missing the
optimization to do one less Montgomery reduction.

TODO - Benchmarks

Change-Id: I268d9c24737c6da9efaf1c73395b73dd97355de7
Reviewed-on: https://boringssl-review.googlesource.com/24690
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-08 20:03:42 +00:00
David Benjamin
9112631c1f Remove ftmp* comments from P-256 addition code.
These are remnants of the old code which had a bunch of ftmp variables.

Change-Id: Id14cf414cb67ff08e240970767f7a5a58e883ce4
Reviewed-on: https://boringssl-review.googlesource.com/24689
Reviewed-by: Adam Langley <agl@google.com>
2018-01-08 19:51:03 +00:00
David Benjamin
00208b443c Use fiat-crypto's freeze function for fe_tobytes.
It requires a handful of additional intrinsics for now.

Fiat's freeze function only works on the tight bounds, so fe_isnonzero
gains an extra fe_carry. But all other calls of fe_tobytes are of tight
bounds anyway.

Change-Id: I834858cee7863c7344e456d7a7dbf4f414f04ae5
Reviewed-on: https://boringssl-review.googlesource.com/24545
Reviewed-by: Adam Langley <agl@google.com>
2018-01-05 23:38:26 +00:00
David Benjamin
915c121bb5 Remove some outdated preconditions and postconditions.
These date to the old code and have been replaced by the fe and fe_loose
bounds in the header file. Also fix up a comment that the comment
converter didn't manage to convert.

Change-Id: I2e3ea867a8cea2b347d09c304a17e532b2e36545
Reviewed-on: https://boringssl-review.googlesource.com/24525
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-03 23:03:32 +00:00
David Benjamin
3144d92ab8 Add some missing array parameter length annotations.
Not that anything checks them...

Change-Id: Iae1b5dbdb3c20a9ebd841bcd32cc5c725c68eb01
Reviewed-on: https://boringssl-review.googlesource.com/24524
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-03 22:34:22 +00:00
Andres Erbsen
0a54e99848 Add links to proofs of elliptic curve formulas.
Change-Id: I166f740185f26770b51759714efd5d634fbcc173
Reviewed-on: https://boringssl-review.googlesource.com/24424
Reviewed-by: David Benjamin <davidben@google.com>
2017-12-22 19:52:44 +00:00