Commit Graph

42 Commits

Author SHA1 Message Date
David Benjamin
1b93a42b37 Don't use function wrappers for EC_METHOD.
The weird function thing is a remnant of OpenSSL and I think something
weird involving Windows and symbols exported from dlls. These aren't
exposed in the public API, so have everything point to the tables
directly.

This is in preparation for making built-in EC_GROUPs static. (The static
EC_GROUPs won't be able to call a function wrapper.)

BUG=20

Change-Id: If33888430f32e51f48936db4046769aa1894e3aa
Reviewed-on: https://boringssl-review.googlesource.com/10346
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>
2016-08-16 19:27:52 +00:00
David Benjamin
86aa5dab14 Tidy up EC_POINT_dup.
The old one was written somewhat weirdly.

Change-Id: I414185971a7d70105fded558da6d165570429d31
Reviewed-on: https://boringssl-review.googlesource.com/10345
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-08-16 18:57:49 +00:00
David Benjamin
161ef92c39 Inline ec_group_copy and simplify.
A lot of codepaths are unreachable since the EC_GROUP is known to be
blank.

Change-Id: I5829934762e503241aa73f833c982ad9680d8856
Reviewed-on: https://boringssl-review.googlesource.com/10344
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-08-16 18:52:42 +00:00
David Benjamin
4186b711f4 Don't bother storing the cofactor.
It's always one. We don't support other kinds of curves with this framework.
(Curve25519 uses a much simpler API.) This also allows us to remove the
check_pub_key_order logic.

Change-Id: Ic15e1ecd68662b838c76b1e0aa15c3a93200d744
Reviewed-on: https://boringssl-review.googlesource.com/8350
Reviewed-by: Adam Langley <agl@google.com>
2016-06-20 17:26:02 +00:00
David Benjamin
8cf79af7d1 Always use Fermat's Little Theorem in ecdsa_sign_setup.
The case where ec_group_get_mont_data is NULL is only for arbitrary groups
which we now require to be prime order. BN_mod_exp_mont is fine with a NULL
BN_MONT_CTX. It will just compute it. Saves a bit of special-casing.

Also don't mark p-2 as BN_FLG_CONSTTIME as the exponent is public anyway.

Change-Id: Ie868576d52fc9ae5f5c9f2a4039a729151bf84c7
Reviewed-on: https://boringssl-review.googlesource.com/8307
Reviewed-by: Adam Langley <agl@google.com>
2016-06-20 17:11:42 +00:00
David Benjamin
2f02854c24 Remove EC_GROUP_new_arbitrary.
The Conscrypt revert cycled in long ago.

Change-Id: If3cdb211d7347dca88bd70bdc643f80b19a7e528
Reviewed-on: https://boringssl-review.googlesource.com/8306
Reviewed-by: Adam Langley <agl@google.com>
2016-06-16 20:25:39 +00:00
David Benjamin
0d76c402b8 Decouple crypto/ec from the OID table.
Instead, embed the (very short) encoding of the OID into built_in_curve.

BUG=chromium:499653

Change-Id: I0db36f83c71fbd3321831f54fa5022f8304b30cd
Reviewed-on: https://boringssl-review.googlesource.com/7564
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-31 22:12:09 +00:00
David Benjamin
981936791e Remove some easy obj.h dependencies.
A lot of consumers of obj.h only want the NID values. Others didn't need
it at all. This also removes some OBJ_nid2sn and OBJ_nid2ln calls in EVP
error paths which isn't worth pulling a large table in for.

BUG=chromium:499653

Change-Id: Id6dff578f993012e35b740a13b8e4f9c2edc0744
Reviewed-on: https://boringssl-review.googlesource.com/7563
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-31 20:50:33 +00:00
David Benjamin
6f7374b0ed Restore EC_GROUP_new_by_curve_name and EC_GROUP_set_generator.
Having a different API for this case than upstream is more trouble than is
worth it. This is sad since the new API avoids incomplete EC_GROUPs at least,
but I don't believe supporting this pair of functions will be significantly
more complex than supporting EC_GROUP_new_arbitrary even when we have static
EC_GROUPs.

For now, keep both sets of APIs around, but we'll be able to remove the scar
tissue once Conscrypt's complex dependencies are resolved.

Make the restored EC_GROUP_set_generator somewhat simpler than before by
removing the ability to call it multiple times and with some parameters set to
NULL. Keep the test.

Change-Id: I64e3f6a742678411904cb15c0ad15d56cdae4a73
Reviewed-on: https://boringssl-review.googlesource.com/7432
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-15 18:53:12 +00:00
Adam Langley
ce9d85eedd Tweaks for node.js
node.js is, effectively, another bindings library. However, it's better
written than most and, with these changes, only a couple of tiny fixes
are needed in node.js. Some of these changes are a little depressing
however so we'll need to push node.js to use APIs where possible.

Changes:
  ∙ Support verify_recover. This is very obscure and the motivation
    appears to be https://github.com/nodejs/node/issues/477 – where it's
    not clear that anyone understands what it means :(
  ∙ Add a few, no-op #defines
  ∙ Add some members to |SSL_CTX| and |SSL| – node.js needs to not
    reach into these structs in the future.
  ∙ Add EC_get_builtin_curves.
  ∙ Add EVP_[CIPHER|MD]_do_all_sorted – these functions are limited to
    decrepit.

Change-Id: I9a3566054260d6c4db9d430beb7c46cc970a9d46
Reviewed-on: https://boringssl-review.googlesource.com/6952
Reviewed-by: Adam Langley <agl@google.com>
2016-01-26 23:23:42 +00:00
David Benjamin
232127d245 Fold EC_GROUP_new_curve_GFp and EC_GROUP_set_generator into a EC_GROUP_new_arbitrary.
This is only for Conscrypt which always calls the pair in succession. (Indeed
it wouldn't make any sense to not call it.) Remove those two APIs and replace
with a single merged API. This way incomplete EC_GROUPs never escape outside
our API boundary and EC_GROUPs may *finally* be made immutable.

Also add a test for this to make sure I didn't mess it up.

Add a temporary BORINGSSL_201512 define to ease the transition for Conscrypt.
Conscrypt requires https://android-review.googlesource.com/#/c/187801/ before
picking up this change.

Change-Id: I3706c2ceac31ed2313175ba5ee724bd5c74ef6e1
Reviewed-on: https://boringssl-review.googlesource.com/6550
Reviewed-by: Adam Langley <agl@google.com>
2016-01-21 22:35:46 +00:00
Brian Smith
a3d9de05fb Add |EC_GROUP_get0_order| to replace |EC_GROUP_get_order|.
|EC_GROUP_get0_order| doesn't require any heap allocations and never
fails, so it is much more convenient and more efficient for callers to
call.

Change-Id: Ic60f768875e7bc8e74362dacdb5cbbc6957b05a6
Reviewed-on: https://boringssl-review.googlesource.com/6532
Reviewed-by: Adam Langley <agl@google.com>
2015-12-15 18:18:13 +00:00
Brian Smith
f3376ace43 Remove |EC_POINTs_mul| & simplify p256-x86_64.
Without |EC_POINTs_mul|, there's never more than one variable point
passed to a |EC_METHOD|'s |mul| method. This allows them to be
simplified considerably. In this commit, the p256-x86_64 implementation
has been simplified to eliminate the heap allocation and looping
related that was previously necessary to deal with the possibility of
there being multiple input points. The other implementations were left
mostly as-is; they should be similarly simplified in the future.

Change-Id: I70751d1d5296be2562af0730e7ccefdba7a1acae
Reviewed-on: https://boringssl-review.googlesource.com/6493
Reviewed-by: Adam Langley <agl@google.com>
2015-11-19 01:08:46 +00:00
Brian Smith
e2136d9c28 Remove |EC_GROUP_precompute_mult| and |EC_KEY_precompute_mult|.
Change-Id: I1663ec6046b8f1f67a62e4c6483af719d6f362ad
Reviewed-on: https://boringssl-review.googlesource.com/6486
Reviewed-by: Adam Langley <agl@google.com>
2015-11-19 00:35:59 +00:00
Brian Smith
9b26297608 Make |EC_GROUP_precompute_mult|/|EC_KEY_precompute_mult| no-ops.
This moves us closer to having |EC_GROUP| and |EC_KEY| being immutable.
The functions are left as no-ops for backward compatibility.

Change-Id: Ie23921ab0364f0771c03aede37b064804c9f69e0
Reviewed-on: https://boringssl-review.googlesource.com/6485
Reviewed-by: Adam Langley <agl@google.com>
2015-11-19 00:27:39 +00:00
Adam Langley
38feb990a1 Require that EC points are on the curve.
This removes a sharp corner in the API where |ECDH_compute_key| assumed
that callers were either using ephemeral keys, or else had already
checked that the public key was on the curve.

A public key that's not on the curve can be in a small subgroup and thus
the result can leak information about the private key.

This change causes |EC_POINT_set_affine_coordinates_GFp| to require that
points are on the curve. |EC_POINT_oct2point| already does this.

Change-Id: I77d10ce117b6efd87ebb4a631be3a9630f5e6636
Reviewed-on: https://boringssl-review.googlesource.com/5861
Reviewed-by: Adam Langley <agl@google.com>
2015-11-06 19:35:42 +00:00
Adam Langley
165248c24f Fix several MSVC warnings.
MSVC reports lots of:
warning C4090: 'function' : different 'const' qualifiers

Change-Id: If8184538c44e657f6234252d0147396d1a18b36c
2015-11-03 14:31:33 -08:00
Adam Langley
466b9895ac Initialise variable before jump.
Clang finds:

crypto/ec/ec.c:420:7: error: variable 'ok' is used uninitialized
whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
  if (ctx == NULL) {
        ^~~~~~~~~~~

Change-Id: I33fc4d74ff3a3bd52ab155f8273fbcd9c6256e35
2015-11-03 14:20:45 -08:00
Adam Langley
1895493868 Add Intel's P-256
This change incorporates Intel's P-256 implementation. The record of
Intel's submission under CLA is in internal bug number 25330687.

Before:
Did 3582 ECDH P-256 operations in 1049114us (3414.3 ops/sec)
Did 8525 ECDSA P-256 signing operations in 1028778us (8286.5 ops/sec)
Did 3487 ECDSA P-256 verify operations in 1008996us (3455.9 ops/sec)
build/tool/bssl is 1434704 bytes after strip -s

After:
Did 8618 ECDH P-256 operations in 1027884us (8384.2 ops/sec)
Did 21000 ECDSA P-256 signing operations in 1049490us (20009.7 ops/sec)
Did 8268 ECDSA P-256 verify operations in 1079481us (7659.2 ops/sec)
build/tool/bssl is 1567216 bytes after strip -s

Change-Id: I147971a8e19849779c8ed7e20310d41bd4962299
Reviewed-on: https://boringssl-review.googlesource.com/6371
Reviewed-by: Adam Langley <agl@google.com>
2015-11-03 22:08:47 +00:00
Brian Smith
274341dd6e Change the type of |EC_GROUP_get_degree| and friends to |unsigned|.
These functions ultimately return the result of |BN_num_bits|, and that
function's return type is |unsigned|. Thus, these functions' return
type should also be |unsigned|.

Change-Id: I2cef63e6f75425857bac71f7c5517ef22ab2296b
Reviewed-on: https://boringssl-review.googlesource.com/6170
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-27 16:48:04 +00:00
Adam Langley
2e3c978d14 Add OPENSSL_SMALL.
Intel's P-256 code has very large tables and things like Chromium just
don't need that extra size. However, servers generally do so this change
adds an OPENSSL_SMALL define that currently just drops the 64-bit P-224
but will gate Intel's P-256 in the future too.

Change-Id: I2e55c6e06327fafabef9b96d875069d95c0eea81
Reviewed-on: https://boringssl-review.googlesource.com/6362
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-27 16:40:20 +00:00
Brian Smith
f0523e9f20 Avoid hard-coded linkage of WNAF-based multiplication.
If the application is only using the P-256 implementation in p256-64.c,
then the WNAF code would all be dead code. The change reorganizes the
code so that all modern toolchains should be able to recognize that
fact and eliminate the WNAF-based code when it is unused.

Change-Id: I9f94bd934ca7d2292de4c29bb89e17c940c7cd2a
Reviewed-on: https://boringssl-review.googlesource.com/6173
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-27 16:38:25 +00:00
Brian Smith
80c5fabc63 Simplify |EC_METHOD| by removing invariant methods.
None of these methods vary per group. Factoring these out of
|EC_METHOD| should help some toolchains to do a better job optimizing
the code for size.

Change-Id: Ibd22a52992b4d549f12a8d22bddfdb3051aaa891
Reviewed-on: https://boringssl-review.googlesource.com/6172
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-27 15:55:47 +00:00
Adam Langley
f0258fe956 Add optimised version of P-224.
This imports the Google-authored P-224 implementation by Emilia Käsper
and Bodo Möller that is also in upstream OpenSSL.

Change-Id: I16005c74a2a3e374fb136d36f3f6569dab9d8919
Reviewed-on: https://boringssl-review.googlesource.com/6145
Reviewed-by: Adam Langley <agl@google.com>
2015-10-08 20:29:22 +00:00
Adam Langley
28bc6eba28 Don't use the uint128_t P-256 code under MSAN.
MSAN appears to have a bug that causes this code to be miscompiled when
compiled with optimisations. In order to prevent that bug from holding
everything up, this change disables that code when MEMORY_SANITIZER is
defined. The generic elliptic-curve code can pick up the slack in that
case.

Change-Id: I7ce26969b3ee0bc0b0496506f06a8cf9b2523cfa
2015-08-20 15:15:30 -07:00
Brian Smith
4ae9f2746c Clean up weirdness in initializing EC_GROUP cofactor & order.
Previously, |x| was reset to the value of the cofactor for no reason,
and there was an unnecessary copy made of |order|.

Change-Id: Ib6b06f651e280838299dff534c38726ebf4ccc97
Reviewed-on: https://boringssl-review.googlesource.com/4447
Reviewed-by: Adam Langley <agl@google.com>
2015-08-07 02:04:09 +00:00
David Benjamin
3570d73bf1 Remove the func parameter to OPENSSL_PUT_ERROR.
Much of this was done automatically with
  find . -name '*.c' | xargs sed -E -i '' -e 's/(OPENSSL_PUT_ERROR\([a-zA-Z_0-9]+, )[a-zA-Z_0-9]+, ([a-zA-Z_0-9]+\);)/\1\2/'
  find . -name '*.c' | xargs sed -E -i '' -e 's/(OPENSSL_PUT_ERROR\([a-zA-Z_0-9]+, )[a-zA-Z_0-9]+,  ([a-zA-Z_0-9]+\);)/\1\2/'

BUG=468039

Change-Id: I4c75fd95dff85ab1d4a546b05e6aed1aeeb499d8
Reviewed-on: https://boringssl-review.googlesource.com/5276
Reviewed-by: Adam Langley <agl@google.com>
2015-07-16 02:02:37 +00:00
Adam Langley
d72e284271 Support arbitrary elliptic curve groups.
This change exposes the functions needed to support arbitrary elliptic
curve groups. The Java API[1] doesn't allow a provider to only provide
certain elliptic curve groups. So if BoringSSL is an ECC provider on
Android, we probably need to support arbitrary groups because someone
out there is going to be using it for Bitcoin I'm sure.

Perhaps in time we can remove this support, but not yet.

[1] https://docs.oracle.com/javase/7/docs/api/java/security/spec/ECParameterSpec.html

Change-Id: Ic1d76de96f913c9ca33c46b451cddc08c5b93d80
Reviewed-on: https://boringssl-review.googlesource.com/4740
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-05-15 00:59:37 +00:00
Adam Langley
126320c881 Add dummy EC_GROUP_set_point_conversion_form.
BoringSSL always uses uncompressed points. This function aborts if
another form is requested or does nothing if uncompressed points are
requested.

Change-Id: I80bc01444cdf9c789c9c75312b5527bf4957361b
2015-05-04 17:53:02 -07:00
David Benjamin
cfaf7ff9bf Remove unnecessary NULL checks, part 2.
Stuff in crypto/ec.

Change-Id: I3bd238c365c4766ed8abc6f835a107478b43b159
Reviewed-on: https://boringssl-review.googlesource.com/4515
Reviewed-by: Adam Langley <agl@google.com>
2015-05-04 23:08:22 +00:00
Adam Langley
ad6b28e974 Add 64-bit, P-256 implementation.
This is taken from upstream, although it originally came from us. This
will only take effect on 64-bit systems (x86-64 and aarch64).

Before:

Did 1496 ECDH P-256 operations in 1038743us (1440.2 ops/sec)
Did 2783 ECDSA P-256 signing operations in 1081006us (2574.5 ops/sec)
Did 2400 ECDSA P-256 verify operations in 1059508us (2265.2 ops/sec)

After:

Did 4147 ECDH P-256 operations in 1061723us (3905.9 ops/sec)
Did 9372 ECDSA P-256 signing operations in 1040589us (9006.4 ops/sec)
Did 4114 ECDSA P-256 verify operations in 1063478us (3868.4 ops/sec)

Change-Id: I11fabb03239cc3a7c4a97325ed4e4c97421f91a9
2015-04-16 13:53:05 -07:00
Adam Langley
c3ef76f327 Compatibility changes for wpa_supplicant and OpenSSH.
OpenSSH, especially, does some terrible things that mean that it needs
the EVP_CIPHER structure to be exposed ☹. Damian is open to a better API
to replace this, but only if OpenSSL agree too. Either way, it won't be
happening soon.

Change-Id: I393b7a6af6694d4d2fe9ebcccd40286eff4029bd
Reviewed-on: https://boringssl-review.googlesource.com/4330
Reviewed-by: Adam Langley <agl@google.com>
2015-04-14 20:18:28 +00:00
David Benjamin
114ddebbf6 Unexport EC_GROUP_copy.
EC_GROUP_copy is an rather unfriendly function; it doesn't work if the groups
have different[*] underlying EC_METHODs, but this notion is not exposed through
the API. I found no callers of EC_GROUP_copy in external code.

This leaves the precompute_mult functions as the remaining mutable API exposed
through EC_GROUP.

[*] Though, of the two EC_METHODs right now, simple.c is entirely unused.

Change-Id: Iabb52518005250fb970e12b3b0ea78b4f6eff4a0
Reviewed-on: https://boringssl-review.googlesource.com/3631
Reviewed-by: Adam Langley <agl@google.com>
2015-02-27 19:43:02 +00:00
Adam Langley
7c21925a10 EC_GROUP_cmp should return zero if the groups match.
(I got this wrong when reading the OpenSSL code.)

Change-Id: Ib289ef41d0ab5a3157ad8b9454d2de96d1f86c22
Reviewed-on: https://boringssl-review.googlesource.com/3620
Reviewed-by: Adam Langley <agl@google.com>
2015-02-25 21:13:02 +00:00
Adam Langley
93531bd70f Add the CTX parameter back to EC_GROUP_cmp.
It was a mistake to remove this in the first place.

Change-Id: Icd97b4db01e49151daa41dd892f9da573ddc2842
Reviewed-on: https://boringssl-review.googlesource.com/3541
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-02-20 23:33:55 +00:00
David Benjamin
9ab14e00d5 Add in missing curly braces part 2.
ECC code.

Change-Id: I1a960620edbb30e10dcbab0e8053a1deb9db3262
Reviewed-on: https://boringssl-review.googlesource.com/3402
Reviewed-by: Adam Langley <agl@google.com>
2015-02-11 23:14:04 +00:00
Adam Langley
2b2d66d409 Remove string.h from base.h.
Including string.h in base.h causes any file that includes a BoringSSL
header to include string.h. Generally this wouldn't be a problem,
although string.h might slow down the compile if it wasn't otherwise
needed. However, it also causes problems for ipsec-tools in Android
because OpenSSL didn't have this behaviour.

This change removes string.h from base.h and, instead, adds it to each
.c file that requires it.

Change-Id: I5968e50b0e230fd3adf9b72dd2836e6f52d6fb37
Reviewed-on: https://boringssl-review.googlesource.com/3200
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-02-02 19:14:15 +00:00
Håvard Molland
3547688ee0 Remove EC_GROUP_set_point_conversion_form
All serialization functions take point format as input, and
asn1_form is never used.

Change-Id: Ib1ede692e815ac0c929e3b589c3a5869adb0dc8b
Reviewed-on: https://boringssl-review.googlesource.com/2511
Reviewed-by: Adam Langley <agl@google.com>
2014-12-09 18:47:03 +00:00
Adam Langley
0eb1aae207 Readd EC_GROUP_get_curve_GFp.
wpa_supplicant needs this in order to get the order of the coordinate
field, apparently so that they can hash to a point.

Change-Id: I92d5df7b37b67ace5f497c25f53f16bbe134aced
Reviewed-on: https://boringssl-review.googlesource.com/1622
Reviewed-by: Adam Langley <agl@google.com>
2014-08-26 21:52:12 +00:00
Adam Langley
d82d79dffe "EC_POINT_invert" was checking "dbl" function pointer instead of "invert".
PR#2569

(Imported from upstream's 4ccc2c19e299741c8e0a8678a13f40e032709e9c)

Change-Id: Id6f5181ea99401145d7b3c4074b292cae8a1c660
2014-07-28 17:05:14 -07:00
Adam Langley
c0d9484902 ec: recognise known parameters when written in full.
Some EC ASN.1 structures are using a named curve, but include the full
parameters anyway. With this change, BoringSSL will recognise the order
of the curve.

Change-Id: Iff057178453f9fdc98c8c03bcabbccef89709887
Reviewed-on: https://boringssl-review.googlesource.com/1270
Reviewed-by: Adam Langley <agl@google.com>
2014-07-19 00:03:13 +00:00
Adam Langley
95c29f3cd1 Inital import.
Initial fork from f2d678e6e89b6508147086610e985d4e8416e867 (1.0.2 beta).

(This change contains substantial changes from the original and
effectively starts a new history.)
2014-06-20 13:17:32 -07:00