Commit Graph

138 Commits

Author SHA1 Message Date
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
301efc8cea Fix error handling in |p256-x86_64|.
This makes similar fixes as were done in the following OpenSSL commits:

    c028254b12a8ea0d0f8a677172eda2e2d78073f3: Correctly set Z_is_one on
    the return value in the NISTZ256 implementation.

    e22d2199e2a5cc9b243f45c2b633d1e31fadecd7: Error checking and memory
    leak leak fixes in NISTZ256.

    4446044a793a9103a4bc70c0214005e6a4463767: NISTZ256: set Z_is_one to
    boolean 0/1 as is customary.

    a4d5269e6d0dba0c276c968448a3576f7604666a: NISTZ256: don't swallow
    malloc errors.

The fixes aren't exactly the same. In particular, the comments "This is
an unusual input, we don't guarantee constant-timeness" and the changes
to |ecp_nistz256_mult_precompute| (which isn't in BoringSSL) were
omitted.

Change-Id: Ia7bb982daa62fb328e8bd2d4dd49a8857e104096
Reviewed-on: https://boringssl-review.googlesource.com/6492
Reviewed-by: Adam Langley <agl@google.com>
2015-11-19 00:52:33 +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
Brian Smith
5058d79948 Remove p224-64 and p256-64 dead code for non-default generators.
This extends 9f1f04f313 to the other
implementations.

|EC_GFp_nistp224_method| and |EC_GFp_nistp256_method| are not marked
|OPENSSL_EXPORT|. |EC_GROUP_set_generator| doesn't allow the generator
to be changed for any |EC_GROUP| for built-in curves. Consequently,
there's no way (except some kind of terrible abuse) that this code
could be executed with a non-default generator.

Change-Id: I5d9b6be4e6f9d384159cb3d708390a8e3c69f23f
Reviewed-on: https://boringssl-review.googlesource.com/6489
Reviewed-by: Adam Langley <agl@google.com>
2015-11-19 00:23:14 +00:00
Adam Langley
d9e27021e1 Don't encode or decode ∞.
|EC_POINT_point2oct| would encode ∞, which is surprising, and
|EC_POINT_oct2point| would decode ∞, which is insane. This change
removes both behaviours.

Thanks to Brian Smith for pointing it out.

Change-Id: Ia89f257dc429a69b9ea7b7b15f75454ccc9c3bdd
Reviewed-on: https://boringssl-review.googlesource.com/6488
Reviewed-by: Adam Langley <agl@google.com>
2015-11-12 23:52:37 +00:00
Adam Langley
e7806fd477 Remove point-on-curve check from |ec_GFp_simple_oct2point|.
In the case of a compressed point, the decompression ensures that the
point is on the curve. In the uncompressed case,
|EC_POINT_set_affine_coordinates_GFp| checks that the point is on the
curve as of 38feb990a1.

Change-Id: Icd69809ae396838b4aef4fa89b3b354560afed55
Reviewed-on: https://boringssl-review.googlesource.com/6487
Reviewed-by: Brian Smith <brian@briansmith.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-11-12 23:51:17 +00:00
Brian Smith
7308aaa9b4 Remove EC_GFp_simple_method (dead code).
Change-Id: I1820bd5412313e00a69123370178c0fe3e12b5ef
Reviewed-on: https://boringssl-review.googlesource.com/6482
Reviewed-by: Adam Langley <agl@google.com>
2015-11-12 20:07:51 +00:00
Brian Smith
f872951880 Fix null pointer dereference when using "simple" EC.
This regressed in f0523e9f20.

Change-Id: I70c3fcb0d91ac00e5088b086312384756eda6140
Reviewed-on: https://boringssl-review.googlesource.com/6481
Reviewed-by: Adam Langley <agl@google.com>
2015-11-12 20:05:13 +00:00
Brian Smith
9f1f04f313 Remove nistz256 dead code for non-default generators.
|EC_GFp_nistz256_method| is not marked |OPENSSL_EXPORT| so only the
built-in P-256 curve uses it. |EC_GROUP_set_generator| doesn't allow
the generator to be changed for any |EC_GROUP| for a built-in curve.
Consequently, there's no way (except some kind of terrible abuse) that
the nistz code could be executed with a non-default generator.

Change-Id: Ib22f00bc74c103b7869ed1e35032b1f3d26cdad2
Reviewed-on: https://boringssl-review.googlesource.com/6446
Reviewed-by: Adam Langley <agl@google.com>
2015-11-12 19:59:16 +00:00
David Benjamin
ef14b2d86e Remove stl_compat.h.
Chromium's toolchains may now assume C++11 library support, so we may freely
use C++11 features. (Chromium's still in the process of deciding what to allow,
but we use Google's style guide directly, toolchain limitations aside.)

Change-Id: I1c7feb92b7f5f51d9091a4c686649fb574ac138d
Reviewed-on: https://boringssl-review.googlesource.com/6465
Reviewed-by: Adam Langley <agl@google.com>
2015-11-11 22:19:36 +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
8f7ecb8f0c (Hopefully) fix a warning on Windows.
MSVC unhelpfuly says: warning C4146: unary minus operator applied to
unsigned type, result still unsigned.

Change-Id: Ia1e6b9fc415908920abb1bcd98fc7f7a5670c2c7
2015-11-03 14:29:01 -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
Adam Langley
96c2a28171 Fix all sign/unsigned warnings with Clang and GCC.
Change-Id: If2a83698236f7b0dcd46701ccd257a85463d6ce5
Reviewed-on: https://boringssl-review.googlesource.com/4992
Reviewed-by: Adam Langley <agl@google.com>
2015-10-27 22:48:00 +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
David Benjamin
301afaf223 Add a run_tests target to run all tests.
It's very annoying having to remember the right incant every time I want
to switch around between my build, build-release, build-asan, etc.,
output directories.

Unfortunately, this target is pretty unfriendly without CMake 3.2+ (and
Ninja 1.5+). This combination gives a USES_TERMINAL flag to
add_custom_target which uses Ninja's "console" pool, otherwise the
output buffering gets in the way. Ubuntu LTS is still on an older CMake,
so do a version check in the meantime.

CMake also has its own test mechanism (CTest), but this doesn't use it.
It seems to prefer knowing what all the tests are and then tries to do
its own output management and parallelizing and such. We already have
our own runners. all_tests.go could actually be converted tidily, but
generate_build_files.py also needs to read it, and runner.go has very
specific needs.

Naming the target ninja -C build test would be nice, but CTest squats
that name and CMake grumps when you use a reserved name, so I've gone
with run_tests.

Change-Id: Ibd20ebd50febe1b4e91bb19921f3bbbd9fbcf66c
Reviewed-on: https://boringssl-review.googlesource.com/6270
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 20:33:44 +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
David Benjamin
15e4deb165 d2i: don't update input pointer on failure
(Imported from upstream's 728bcd59d3d41e152aead0d15acc51a8958536d3.)

Actually this one was reported by us, but the commit message doesn't
mention this.

This is slightly modified from upstream's version to fix some problems
noticed in import. Specifically one of d2i_X509_AUX's success paths is
bust and d2i_PrivateKey still updates on one error path. Resolve the
latter by changing both it and d2i_AutoPrivateKey to explicitly hit the
error path on ret == NULL. This lets us remove the NULL check in
d2i_AutoPrivateKey.

We'll want to report the problems back upstream.

Change-Id: Ifcfc965ca6d5ec0a08ac154854bd351cafbaba25
Reviewed-on: https://boringssl-review.googlesource.com/5948
Reviewed-by: Adam Langley <agl@google.com>
2015-09-28 22:15:17 +00:00
Adam Langley
73415b6aa0 Move arm_arch.h and fix up lots of include paths.
arm_arch.h is included from ARM asm files, but lives in crypto/, not
openssl/include/. Since the asm files are often built from a different
location than their position in the source tree, relative include paths
are unlikely to work so, rather than having crypto/ be a de-facto,
second global include path, this change moves arm_arch.h to
include/openssl/.

It also removes entries from many include paths because they should be
needed as relative includes are always based on the locations of the
source file.

Change-Id: I638ff43d641ca043a4fc06c0d901b11c6ff73542
Reviewed-on: https://boringssl-review.googlesource.com/5746
Reviewed-by: Adam Langley <agl@google.com>
2015-08-26 01:57:59 +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
b2d987b47c Free tmp_wNAF in all exit paths.
BUG=517495

Change-Id: I67c9c511d4ed558ab7a976d6708398e02194cd0f
Reviewed-on: https://boringssl-review.googlesource.com/5628
Reviewed-by: Adam Langley <agl@google.com>
2015-08-07 02:00:35 +00:00
Brian Smith
f4bbc2a360 Improve error checking of some |BN_CTX_get| callers.
The documentation for |BN_CTX_get| states: "Once |BN_CTX_get| has
returned NULL, all future calls will also return NULL until
|BN_CTX_end| is called." Some code takes advantage of that guarantee
by only checking the return value of the last call to |BN_CTX_get| in a
series of calls. That is correct and the most efficient way of doing
it. However, that pattern is inconsistent with most of the other uses
of |BN_CTX_get|. Also, static analysis tools like Coverity cannot
understand that pattern. This commit removes the instances of that
pattern that Coverity complained about when scanning *ring*.

Change-Id: Ie36d0223ea1caee460c7979547cf5bfd5fb16f93
Reviewed-on: https://boringssl-review.googlesource.com/5611
Reviewed-by: Adam Langley <agl@google.com>
2015-08-07 00:50:17 +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
David Benjamin
3fa65f0f05 Fix some malloc test crashs.
This isn't exhaustive. There are still failures in some tests which probably
ought to get C++'d first.

Change-Id: Iac58df9d98cdfd94603d54374a531b2559df64c3
Reviewed-on: https://boringssl-review.googlesource.com/4795
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 18:00:10 +00:00
David Benjamin
0b635c52b2 Add malloc test support to unit tests.
Currently far from passing and I haven't even tried with a leak checker yet.
Also bn_test is slow.

Change-Id: I4fe2783aa5f7897839ca846062ae7e4a367d2469
Reviewed-on: https://boringssl-review.googlesource.com/4794
Reviewed-by: Adam Langley <agl@google.com>
2015-05-21 17:59:48 +00:00
Adam Langley
0da323a8b8 Convert reference counts in crypto/
This change converts the reference counts in crypto/ to use
|CRYPTO_refcount_t|. The reference counts in |X509_PKEY| and |X509_INFO|
were never actually used and so were dropped.

Change-Id: I75d572cdac1f8c1083c482e29c9519282d7fd16c
Reviewed-on: https://boringssl-review.googlesource.com/4772
Reviewed-by: Adam Langley <agl@google.com>
2015-05-20 19:15:26 +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
Shawn Willden
785e07b23d Copy ecdsa_meth in EC_KEY_copy.
Change-Id: Ia97e76d6e5a5dc216a05741864c3d59b50d855bd
Reviewed-on: https://boringssl-review.googlesource.com/4750
Reviewed-by: Adam Langley <agl@google.com>
2015-05-14 20:37:21 +00:00
Matt Braithwaite
3bf1cca262 Don't report |ERR_R_MALLOC_FAILURE| on failure of |EC_KEY_new_by_curve_name|.
Change |EC_KEY_new_by_curve_name| to report |ERR_R_MALLOC_FAILURE|
itself, so that reporting of |EC_R_UNKNOWN_GROUP| is not confused by
the caller's addition of a spurious |ERR_R_MALLOC_FAILURE|.

Change-Id: Id3f5364f01eb8e3597bcddd6484bc03d5578befb
Reviewed-on: https://boringssl-review.googlesource.com/4690
Reviewed-by: Adam Langley <agl@google.com>
2015-05-09 00:05:30 +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
David Benjamin
4fcc2e2031 Make a few variable names saner.
Change-Id: I6790dc9651dc400992fc59a4c900210edeb2520c
Reviewed-on: https://boringssl-review.googlesource.com/4511
Reviewed-by: Adam Langley <agl@google.com>
2015-05-04 22:58:05 +00:00
David Benjamin
1022dd3d52 Don't inclue crypto/ec/internal.h in ec_test.cc.
MSVC seems to dislike the zero-array trick in C++, but not C. Turns out there
was no need for the include, so that's an easy fix.

Change-Id: I6def7b430a450c4ff7eeafa3611f0d40f5fc5945
Reviewed-on: https://boringssl-review.googlesource.com/4580
Reviewed-by: Adam Langley <agl@google.com>
2015-04-28 21:16:50 +00:00
David Benjamin
7743c026cb Ensure EC private keys retain leading zeros
RFC 5915 requires the use of the I2OSP primitive as defined in RFC 3447
for encoding ECPrivateKey. Fix this and add a test.

See also upstream's 30cd4ff294252c4b6a4b69cbef6a5b4117705d22, though it mixes
up degree and order.

Change-Id: I81ba14da3c8d69e3799422c669fab7f16956f322
Reviewed-on: https://boringssl-review.googlesource.com/4469
Reviewed-by: Adam Langley <agl@google.com>
2015-04-28 21:03:27 +00:00
David Benjamin
67be048e1a Convert ec_test to C++
Change-Id: I5e25ddbc87370b58d9b6fc410f51e259947df8dd
Reviewed-on: https://boringssl-review.googlesource.com/4468
Reviewed-by: Adam Langley <agl@google.com>
2015-04-28 21:00:34 +00:00
David Benjamin
f0df86a1f6 Fix standalone build on Mac.
CRYPTO_MUTEX was the wrong size. Fortunately, Apple was kind enough to define
pthread_rwlock_t unconditionally, so we can be spared fighting with feature
macros. Some of the stdlib.h removals were wrong and clang is pick about
multiply-defined typedefs. Apparently that's a C11 thing?

BUG=478598

Change-Id: Ibdcb8de9e5d83ca28e4c55b2979177d1ef0f9721
Reviewed-on: https://boringssl-review.googlesource.com/4404
Reviewed-by: Adam Langley <agl@google.com>
2015-04-20 16:50:18 +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
David Benjamin
9f33fc63c6 Remove hash table lookups from ex_data.
Instead, each module defines a static CRYPTO_EX_DATA_CLASS to hold the values.
This makes CRYPTO_cleanup_all_ex_data a no-op as spreading the
CRYPTO_EX_DATA_CLASSes across modules (and across crypto and ssl) makes cleanup
slightly trickier. We can make it do something if needbe, but it's probably not
worth the trouble.

Change-Id: Ib6f6fd39a51d8ba88649f0fa29c66db540610c76
Reviewed-on: https://boringssl-review.googlesource.com/4375
Reviewed-by: Adam Langley <agl@google.com>
2015-04-15 23:59:35 +00:00
David Benjamin
546f1a59ef Unexpose the generic ex_data functions.
Callers are required to use the wrappers now. They still need OPENSSL_EXPORT
since crypto and ssl get built separately in the standalone shared library
build.

Change-Id: I61186964e6099b9b589c4cd45b8314dcb2210c89
Reviewed-on: https://boringssl-review.googlesource.com/4372
Reviewed-by: Adam Langley <agl@google.com>
2015-04-15 23:27:22 +00: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
Brian Smith
0acef5ec27 Simplify ec_group_st on the assumption it is used for GF(p) only.
Change-Id: I90e8f9ce7b996471daed129794eb1b0fa80a27cc
Reviewed-on: https://boringssl-review.googlesource.com/4272
Reviewed-by: Adam Langley <agl@google.com>
2015-04-13 20:53:20 +00:00
Brian Smith
054e682675 Eliminate unnecessary includes from low-level crypto modules.
Beyond generally eliminating unnecessary includes, eliminate as many
includes of headers that declare/define particularly error-prone
functionality like strlen, malloc, and free. crypto/err/internal.h was
added to remove the dependency on openssl/thread.h from the public
openssl/err.h header. The include of <stdlib.h> in openssl/mem.h was
retained since it defines OPENSSL_malloc and friends as macros around
the stdlib.h functions. The public x509.h, x509v3.h, and ssl.h headers
were not changed in order to minimize breakage of source compatibility
with external code.

Change-Id: I0d264b73ad0a720587774430b2ab8f8275960329
Reviewed-on: https://boringssl-review.googlesource.com/4220
Reviewed-by: Adam Langley <agl@google.com>
2015-04-13 20:49:18 +00:00
Brian Smith
83a82981dc Rename BIO_print_errors_fp back to ERR_print_errors_fp & refactor it.
A previous change in BoringSSL renamed ERR_print_errors_fp to
BIO_print_errors_fp as part of refactoring the code to improve the
layering of modules within BoringSSL. Rename it back for better
compatibility with code that was using the function under the original
name. Move its definition back to crypto/err using an implementation
that avoids depending on crypto/bio.

Change-Id: Iee7703bb1eb4a3d640aff6485712bea71d7c1052
Reviewed-on: https://boringssl-review.googlesource.com/4310
Reviewed-by: Adam Langley <agl@google.com>
2015-04-13 20:23:29 +00:00
Brian Smith
d6405beb2c Avoid calling BN_CTX_end without BN_CTX_start in ec_wNAF_precompute_mult.
Prior to this change, when EC_GROUP_get0_generator fails, BN_CTX_end
would get called even though BN_CTX_start hadn't been called yet, in
the case where the caller-supplied |ctx| is not NULL.

Change-Id: I6f728e74f0167193891cdb6f122b20b0770283dc
Reviewed-on: https://boringssl-review.googlesource.com/4271
Reviewed-by: Adam Langley <agl@google.com>
2015-04-09 00:29:14 +00:00
Brian Smith
69fe82ae1e Remove unused ec_pre_comp_st.group back pointer to group.
Change-Id: If15f2f0e2b4627318c9cdfbc76d5ca56a6894e3f
Reviewed-on: https://boringssl-review.googlesource.com/4270
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-04-08 20:27:58 +00:00
David Benjamin
4b1510c71e Fix a failure to NULL a pointer freed on error.
Reported by the LibreSSL project as a follow on to CVE-2015-0209

(Imported from upstream's 5e5d53d341fd9a9b9cc0a58eb3690832ca7a511f.)

Change-Id: Ic2e5dc5c96e316c55f76bedc6ea55b416be3287a
Reviewed-on: https://boringssl-review.googlesource.com/4049
Reviewed-by: Adam Langley <agl@google.com>
2015-03-19 19:50:32 +00:00
David Benjamin
5ae21bc02b Remove pointless free, and use preferred way of calling d2i_* functions
(Imported from upstream's 535bc8faf69dc4ff39e2ee99195b268cf99b9569)

Change-Id: Ia5abf4dce9dc8cdf5a9b77a3e360a40b5f815adf
Reviewed-on: https://boringssl-review.googlesource.com/3672
Reviewed-by: Adam Langley <agl@google.com>
2015-02-27 19:47:36 +00:00
David Benjamin
29b50eab6c Align d2i_ECPrivateKey error-handling with upstream.
Upstream decided to make the caller free the scratch space rather than the
callee. May as well match. (Existing code is pretty inconsistent. This API
pattern needs to go.)

See upstream's 9e442d485008046933cdc7da65080f436a4af089.

Change-Id: I7c9fcae5778a74d6ae8e9f546e03fb2cf6e48426
Reviewed-on: https://boringssl-review.googlesource.com/3671
Reviewed-by: Adam Langley <agl@google.com>
2015-02-27 19:45:42 +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
David Benjamin
03741f61d9 Use EC_GROUP_dup and EC_POINT_dup in EC_KEY_copy.
They do the same thing. This removes all callers of EC_GROUP_copy outside
EC_GROUP_dup.

Change-Id: I65433ee36040de79e56483dfece774e01e2e2743
Reviewed-on: https://boringssl-review.googlesource.com/3630
Reviewed-by: Adam Langley <agl@google.com>
2015-02-25 21:44:24 +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
c35fb014d9 Fix more memory leaks on malloc failure.
Caught by malloc valgrind tests on Basic-Client-Sync. Also one by inspection
and verified with valgrind. Those should pass now with the exception of
CRYPTO_free_ex_data being internally implemented with malloc.

(Clearly we next should make our malloc tests assert that the containing
function fails to catch when we fail to check for some error and things
silently move one.)

Change-Id: I56c51dc8a32a7d3c7ac907d54015dc241728c761
Reviewed-on: https://boringssl-review.googlesource.com/3440
Reviewed-by: Adam Langley <agl@google.com>
2015-02-13 18:43:01 +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
29b186736c Precompute sorted array for error strings.
Previously, error strings were kept in arrays for each subdirectory and
err.c would iterate over them all and insert them at init time to a hash
table.

This means that, even if you have a shared library and lots of processes
using that, each process has ~30KB of private memory from building that
hash table.

This this change, all the error strings are built into a sorted list and
are thus static data. This means that processes can share the error
information and it actually saves binary space because of all the
pointer overhead in the old scheme. Also it saves the time taken
building the hash table at startup.

This removes support for externally-supplied error string data.

Change-Id: Ifca04f335c673a048e1a3e76ff2b69c7264635be
2015-02-09 17:35:31 -08: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
Adam Langley
4a0f0c4910 Change CMakeLists.txt to two-space indent.
find -name CMakeLists.txt -type f | xargs sed -e 's/\t/  /g' -i

Change-Id: I01636b1849c00ba918f48828252492d99b0403ac
2015-01-28 16:37:10 -08:00
Eric Roman
517073cd4b Set output EC_KEY to NULL when d2i_ECPrivateKey() fails.
BUG=crbug.com/445679

Change-Id: Ia012d806964bd7240148779797eccd326484f364
Reviewed-on: https://boringssl-review.googlesource.com/2722
Reviewed-by: Adam Langley <agl@google.com>
2015-01-06 00:55:19 +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
Håvard Molland
306e520cda Remove ec hybrid point format
According to rfc5480 and rfc4492 the hybrid format is not allowed
neither in certificates or the tls protocol.

Change-Id: I1d3fb5bef765bc7b58d29bdd60e15247fac4dc7a
Reviewed-on: https://boringssl-review.googlesource.com/2510
Reviewed-by: Adam Langley <agl@google.com>
2014-12-08 22:47:41 +00:00
David Benjamin
3087f6e594 Fix garbage free on malloc failure in ec_wNAF_mul.
PR#3595

(Imported from upstream's e04d426bf98ebb22abf0f15b6f09d333a6e8b2ad.)

Change-Id: I01a9d9bef7e911b3fb1565f8a582f5d6cc7d5537
Reviewed-on: https://boringssl-review.googlesource.com/2290
Reviewed-by: Adam Langley <agl@google.com>
2014-11-14 18:47:41 +00:00
David Benjamin
2f3ba910a2 Fix ec_test build on Windows.
No need to include unistd.h. (Though it probably should include string.h for
memcmp and strcmp.)

Change-Id: Ib09d2da4f7079c9d87338df75ec3560f4f203764
Reviewed-on: https://boringssl-review.googlesource.com/2260
Reviewed-by: Adam Langley <agl@google.com>
2014-11-12 19:39:55 +00:00
David Benjamin
f2f3cfedb7 Use BN_bn2bin_padded rather than doing math to figure out leading zeros.
Saves doing it ad-hoc all the time.

Change-Id: Ic1a1180f56eec37c19799649bb8f18237bd617f8
Reviewed-on: https://boringssl-review.googlesource.com/2241
Reviewed-by: Adam Langley <agl@google.com>
2014-11-10 23:04:52 +00:00
Adam Langley
9398f168f9 Explicitly check for empty ASN.1 strings in d2i_ECPrivateKey.
The old code implicitly relies on the ASN.1 code returning a \0-prefixed
buffer when the buffer length is 0. Change this to verify explicitly
that the ASN.1 string has positive length.

(Imported from upstream's 7f7c05ca638c3cc6d261961fae439cd91e3c1d27)

Change-Id: Icc6c44b874bdcb02374016a36d209830d6162a8a
2014-11-10 13:45:32 -08:00
Adam Langley
9f5a314d35 Automatically generate a missing EC public key.
When d2i_ECPrivateKey reads a private key with a missing (optional)
public key, generate one automatically from the group and private key.

(Imported from upstream's 2083f7c465d07867dd9867b8742bb71c03d1f203)

Change-Id: I9e5090de87cf846ab92e4be5b6bf64e6091d02e4
2014-11-10 13:45:31 -08:00
David Benjamin
9ecafa5c78 Shush some dead assignments.
Appease clang scan-build a bit. I'm not sure it's actually worth silencing all
of them because some of them look like preserving invariants between local
variables, but some are clearly pointless or can be restructured slightly.

Change-Id: I0bc81e2589bb402ff3ef0182d7a8921e31b85052
Reviewed-on: https://boringssl-review.googlesource.com/2205
Reviewed-by: Adam Langley <agl@google.com>
2014-11-06 01:34:33 +00:00
Adam Langley
404e6e64d0 Small tweak to ec_GFp_simple_points_make_affine.
(Imported from upstream's 267e6f3cc0ef78dea4e5cf93907a71556a45f008)

Change-Id: I99cfd0196b9625449c50494562c44f57f09fed17
Reviewed-on: https://boringssl-review.googlesource.com/2167
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2014-11-05 21:26:04 +00:00
David Benjamin
775b75f9de Fix Windows shared library build.
Windows is much pickier about dllimport/dllexport. Declare it on
the declaration, not the definition. Also ensure that the declaration
precedes the definition. Finally, remove a stray OPENSSL_EXPORT.

Change-Id: Id50b9de5acbe5adf1b15b22dd60b7a5c13a80cce
Reviewed-on: https://boringssl-review.googlesource.com/1862
Reviewed-by: Adam Langley <agl@google.com>
2014-10-01 02:01:53 +00:00
David Benjamin
a70c75cfc0 Add a CRYPTO_library_init and static-initializer-less build option.
Chromium does not like static initializers, and the CPU logic uses one to
initialize CPU bits. However, the crypto library lacks an explicit
initialization function, which could complicate (no compile-time errors)
porting existing code which uses crypto/, but not ssl/.

Add an explicit CRYPTO_library_init function, but make it a no-op by default.
It only does anything (and is required) if building with
BORINGSSL_NO_STATIC_INITIALIZER.

Change-Id: I6933bdc3447fb382b1f87c788e5b8142d6f3fe39
Reviewed-on: https://boringssl-review.googlesource.com/1770
Reviewed-by: Adam Langley <agl@google.com>
2014-09-12 00:10:53 +00:00
Adam Langley
b2cb0ece76 Fix minor issues found by Clang's analysis.
Thanks to Denis Denisov for running the analysis.

Change-Id: I80810261e013423e746fd8d8afefb3581cffccc0
Reviewed-on: https://boringssl-review.googlesource.com/1701
Reviewed-by: Adam Langley <agl@google.com>
2014-09-02 22:39:41 +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
David Benjamin
c44d2f4cb8 Convert all zero-argument functions to '(void)'
Otherwise, in C, it becomes a K&R function declaration which doesn't actually
type-check the number of arguments.

Change-Id: I0731a9fefca46fb1c266bfb1c33d464cf451a22e
Reviewed-on: https://boringssl-review.googlesource.com/1582
Reviewed-by: Adam Langley <agl@google.com>
2014-08-21 01:06:07 +00:00
Adam Langley
993fde5162 Simplify ec_GFp_simple_points_make_affine.
Replace the tree-like structure by a linear approach, with fewer special
cases to handle value 0.

(Imported from upstream's d5213519c0ed87c71136084e7e843a4125ecc024.)

Change-Id: Icdd4815066bdbab0d2c0020db6a8cacc49b3d82a
Reviewed-on: https://boringssl-review.googlesource.com/1400
Reviewed-by: Adam Langley <agl@google.com>
2014-08-05 21:22:38 +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
David Benjamin
ecc0ce7e67 Introduce EVP_PKEY_is_opaque to replace RSA_METHOD_FLAG_NO_CHECK.
Custom RSA and ECDSA keys may not expose the key material. Plumb and "opaque"
bit out of the *_METHOD up to EVP_PKEY. Query that in ssl_rsa.c to skip the
sanity checks for certificate and key matching.

Change-Id: I362a2d5116bfd1803560dfca1d69a91153e895fc
Reviewed-on: https://boringssl-review.googlesource.com/1255
Reviewed-by: Adam Langley <agl@google.com>
2014-07-18 23:35:04 +00:00
Adam Langley
4c921e1bbc Move public headers to include/openssl/
Previously, public headers lived next to the respective code and there
were symlinks from include/openssl to them.

This doesn't work on Windows.

This change moves the headers to live in include/openssl. In cases where
some symlinks pointed to the same header, I've added a file that just
includes the intended target. These cases are all for backwards-compat.

Change-Id: I6e285b74caf621c644b5168a4877db226b07fd92
Reviewed-on: https://boringssl-review.googlesource.com/1180
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 22:42:18 +00:00
Adam Langley
cdf96e5886 Double free in i2o_ECPublicKey
PR: 3338

(Imported from upstream's e34af3ec2b1230a8a523d383f7de505f7cbd381d)
2014-06-20 13:17:40 -07:00
Adam Langley
aeb088ac09 EC infinity fix.
Fix handling of points at infinity in ec_GFp_simple_points_make_affine.
When inverting an array of Z coordinates, the algorithm is supposed to
treat any 0 essentially like a 1 to remain in the multiplicative group;
however, for one of the cases, we incorrectly multiplied by 0 and thus
ended up with garbage.
2014-06-20 13:17:36 -07:00
Adam Langley
f71a27920a Fix EC crash.
This change saves several EC routines from crashing when an EC_KEY is
missing a public key. The public key is optional in the EC private key
format and, without this patch, running the following through `openssl
ec` causes a crash:

-----BEGIN EC PRIVATE KEY-----
MBkCAQEECAECAwQFBgcIoAoGCCqGSM49AwEH
-----END EC PRIVATE KEY-----
2014-06-20 13:17:34 -07: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