Commit Graph

66 Commits

Author SHA1 Message Date
David Benjamin
392cedd0a2 Fx DH_set0_pqg.
Typo.

Change-Id: Iab3e04339bb868fd6d247c6696f33f5b7150408d
Reviewed-on: https://boringssl-review.googlesource.com/21184
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>
2017-10-05 18:50:48 +00:00
David Benjamin
81f030b106 Switch OPENSSL_VERSION_NUMBER to 1.1.0.
Although we are derived from 1.0.2, we mimic 1.1.0 in some ways around
our FOO_up_ref functions and opaque libssl types. This causes some
difficulties when porting third-party code as any OPENSSL_VERSION_NUMBER
checks for 1.1.0 APIs we have will be wrong.

Moreover, adding accessors without changing OPENSSL_VERSION_NUMBER can
break external projects. It is common to implement a compatibility
version of an accessor under #ifdef as a static function. This then
conflicts with our headers if we, unlike OpenSSL 1.0.2, have this
function.

This change switches OPENSSL_VERSION_NUMBER to 1.1.0 and atomically adds
enough accessors for software with 1.1.0 support already. The hope is
this will unblock hiding SSL_CTX and SSL_SESSION, which will be
especially useful with C++-ficiation. The cost is we will hit some
growing pains as more 1.1.0 consumers enter the ecosystem and we
converge on the right set of APIs to import from upstream.

It does not remove any 1.0.2 APIs, so we will not require that all
projects support 1.1.0. The exception is APIs which changed in 1.1.0 but
did not change the function signature. Those are breaking changes.
Specifically:

- SSL_CTX_sess_set_get_cb is now const-correct.

- X509_get0_signature is now const-correct.

For C++ consumers only, this change temporarily includes an overload
hack for SSL_CTX_sess_set_get_cb that keeps the old callback working.
This is a workaround for Node not yet supporting OpenSSL 1.1.0.

The version number is set at (the as yet unreleased) 1.1.0g to denote
that this change includes https://github.com/openssl/openssl/pull/4384.

Bug: 91
Change-Id: I5eeb27448a6db4c25c244afac37f9604d9608a76
Reviewed-on: https://boringssl-review.googlesource.com/10340
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>
2017-09-29 04:51:27 +00:00
David Benjamin
808f832917 Run the comment converter on libcrypto.
crypto/{asn1,x509,x509v3,pem} were skipped as they are still OpenSSL
style.

Change-Id: I3cd9a60e1cb483a981aca325041f3fbce294247c
Reviewed-on: https://boringssl-review.googlesource.com/19504
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-08-18 21:49:04 +00:00
David Benjamin
68a0b1b112 Remove RFC 5114 groups.
These groups are terrible, we got the function wrong (unused ENGINE
parameter does not match upstream), and the functions are unused. Unwind
them. This change doesn't unwind the X9.42 Diffie-Hellman machinery, so
the checks are still present and tested.

(We can probably get rid of the X9.42 machinery too, but it is reachable
from DSA_dup_DH.  That's only used by wpa_supplicant and, if that code
ever ran, it'd be ignored because we don't support DHE in TLS. I've left
it alone for the time being.)

Bug: 2
Change-Id: I8d9396983c8d40ed46a03ba6947720da7e9b689a
Reviewed-on: https://boringssl-review.googlesource.com/19384
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>
2017-08-18 16:48:11 +00:00
David Benjamin
27e377ec65 Fix miscellaneous clang-tidy warnings.
There are still a ton of them, almost exclusively complaints that
function declaration and definitions have different parameter names. I
just fixed a few randomly.

Change-Id: I1072f3dba8f63372cda92425aa94f4aa9e3911fa
Reviewed-on: https://boringssl-review.googlesource.com/18706
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-08-01 20:39:46 +00:00
David Benjamin
d94682dce5 Remove ex_data's dup hook.
The only place it is used is EC_KEY_{dup,copy} and no one calls that
function on an EC_KEY with ex_data. This aligns with functions like
RSAPublicKey_dup which do not copy ex_data. The logic is also somewhat
subtle in the face of malloc errors (upstream's PR 3323).

In fact, we'd even changed the function pointer signature from upstream,
so BoringSSL-only code is needed to pass this pointer in anyway. (I
haven't switched it to CRYPTO_EX_unused because there are some callers
which pass in an implementation anyway.)

Note, in upstream, the dup hook is also used for SSL_SESSIONs when those
are duplicated (for TLS 1.2 ticket renewal or TLS 1.3 resumption). Our
interpretation is that callers should treat those SSL_SESSIONs
equivalently to newly-established ones. This avoids every consumer
providing a dup hook and simplifies the interface.

(I've gone ahead and removed the TODO(fork). I don't think we'll be able
to change this API. Maybe introduce a new one, but it may not be worth
it? Then again, this API is atrocious... I've never seen anyone use argl
and argp even.)

BUG=21

Change-Id: I6c9e9d5a02347cb229d4c084c1e85125bd741d2b
Reviewed-on: https://boringssl-review.googlesource.com/16344
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>
2017-05-23 22:43:59 +00:00
Adam Langley
5c38c05b26 Move bn/ into crypto/fipsmodule/
Change-Id: I68aa4a740ee1c7f2a308a6536f408929f15b694c
Reviewed-on: https://boringssl-review.googlesource.com/15647
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>
2017-05-01 22:51:25 +00:00
David Benjamin
591f251bf3 Remove direct calls to BN_mod_exp.
Within the library, we never need to exponentiate modulo an even number.
In fact, all the remaining BN_mod_exp calls are modulo an odd prime.
This extends 617804adc5 to the rest of the
library.

Change-Id: I4273439faa6a516c99673b28f8ae38ddfff7e42d
Reviewed-on: https://boringssl-review.googlesource.com/14024
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-02-28 18:00:02 +00:00
David Benjamin
6b0edfb9e6 Add a common TestEventListener for the error queue.
Replicate the logic in the AllTests targets to dump the error queue on
failure. GTest seems to print to stdout, so we do here too.

BUG=129

Change-Id: I623b695fb9a474945834c3653728f54e5b122187
Reviewed-on: https://boringssl-review.googlesource.com/13623
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>
2017-02-07 21:33:22 +00:00
David Benjamin
966284337d Do a cursory conversion of a few tests to GTest.
For now, this is the laziest conversion possible. The intent is to just
get the build setup ready so that we can get everything working in our
consumers. The intended end state is:

- The standalone build produces three test targets, one per library:
  {crypto,ssl,decrepit}_tests.

- Each FOO_test is made up of:
    FOO/**/*_test.cc
    crypto/test/gtest_main.cc
    test_support

- generate_build_files.py emits variables crypto_test_sources and
  ssl_test_sources. These variables are populated with FindCFiles,
  looking for *_test.cc.

- The consuming file assembles those variables into the two test targets
  (plus decrepit) from there. This avoids having generate_build_files.py
  emit actual build rules.

- Our standalone builders, Chromium, and Android just run the top-level
  test targets using whatever GTest-based reporting story they have.

In transition, we start by converting one of two tests in each library
to populate the three test targets. Those are added to all_tests.json
and all_tests.go hacked to handle them transparently. This keeps our
standalone builder working.

generate_build_files.py, to start with, populates the new source lists
manually and subtracts them out of the old machinery. We emit both for
the time being. When this change rolls in, we'll write all the build
glue needed to build the GTest-based tests and add it to consumers'
continuous builders.

Next, we'll subsume a file-based test and get the consumers working with
that. (I.e. make sure the GTest targets can depend on a data file.)

Once that's all done, we'll be sure all this will work. At that point,
we start subsuming the remaining tests into the GTest targets and,
asynchronously, rewriting tests to use GTest properly rather than
cursory conversion here.

When all non-GTest tests are gone, the old generate_build_files.py hooks
will be removed, consumers updated to not depend on them, and standalone
builders converted to not rely on all_tests.go, which can then be
removed. (Unless bits end up being needed as a malloc test driver. I'm
thinking we'll want to do something with --gtest_filter.)

As part of this CL, I've bumped the CMake requirements (for
target_include_directories) and added a few suppressions for warnings
that GTest doesn't pass.

BUG=129

Change-Id: I881b26b07a8739cc0b52dbb51a30956908e1b71a
Reviewed-on: https://boringssl-review.googlesource.com/13232
Reviewed-by: Adam Langley <agl@google.com>
2017-01-21 00:17:05 +00:00
David Benjamin
0a211dfe91 Remove BN_FLG_CONSTTIME.
BN_FLG_CONSTTIME is a ridiculous API and easy to mess up
(CVE-2016-2178). Instead, code that needs a particular algorithm which
preserves secrecy of some arguemnt should call into that algorithm
directly.

This is never set outside the library and is finally unused within the
library! Credit for all this goes almost entirely to Brian Smith. I just
took care of the last bits.

Note there was one BN_FLG_CONSTTIME check that was still reachable, the
BN_mod_inverse in RSA key generation. However, it used the same code in
both cases for even moduli and φ(n) is even if n is not a power of two.
Traditionally, RSA keys are not powers of two, even though it would make
the modular reductions a lot easier.

When reviewing, check that I didn't remove a BN_FLG_CONSTTIME that led
to a BN_mod_exp(_mont) or BN_mod_inverse call (with the exception of the
RSA one mentioned above). They should all go to functions for the
algorithms themselves like BN_mod_exp_mont_consttime.

This CL shows the checks are a no-op for all our tests:
https://boringssl-review.googlesource.com/c/12927/

BUG=125

Change-Id: I19cbb375cc75aac202bd76b51ca098841d84f337
Reviewed-on: https://boringssl-review.googlesource.com/12926
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-12 02:00:44 +00:00
David Benjamin
17cf2cb1d2 Work around language and compiler bug in memcpy, etc.
Most C standard library functions are undefined if passed NULL, even
when the corresponding length is zero. This gives them (and, in turn,
all functions which call them) surprising behavior on empty arrays.
Some compilers will miscompile code due to this rule. See also
https://www.imperialviolet.org/2016/06/26/nonnull.html

Add OPENSSL_memcpy, etc., wrappers which avoid this problem.

BUG=23

Change-Id: I95f42b23e92945af0e681264fffaf578e7f8465e
Reviewed-on: https://boringssl-review.googlesource.com/12928
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-12-21 20:34:47 +00:00
David Benjamin
aac1e2dd73 Remove the remaining bssl::Main wrappers.
We've taken to writing bssl::UniquePtr in full, so it's not buying
us much.

Change-Id: Ia2689366cbb17282c8063608dddcc675518ec0ca
Reviewed-on: https://boringssl-review.googlesource.com/12628
Reviewed-by: David Benjamin <davidben@google.com>
2016-12-08 00:54:17 +00:00
David Benjamin
f0e935d7ce Fold stack-allocated types into headers.
Now that we have the extern "C++" trick, we can just embed them in the
normal headers. Move the EVP_CIPHER_CTX deleter to cipher.h and, in
doing so, take away a little bit of boilerplate in defining deleters.

Change-Id: I4a4b8d0db5274a3607914d94e76a38996bd611ec
Reviewed-on: https://boringssl-review.googlesource.com/10804
Reviewed-by: Matt Braithwaite <mab@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-09-07 21:50:05 +00:00
Matt Braithwaite
d17d74d73f Replace Scoped* heap types with bssl::UniquePtr.
Unlike the Scoped* types, bssl::UniquePtr is available to C++ users, and
offered for a large variety of types.  The 'extern "C++"' trick is used
to make the C++ bits digestible to C callers that wrap header files in
'extern "C"'.

Change-Id: Ifbca4c2997d6628e33028c7d7620c72aff0f862e
Reviewed-on: https://boringssl-review.googlesource.com/10521
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>
2016-09-01 22:22:54 +00:00
David Benjamin
d224d52aba Add constants for BN_rand and use them.
See upstream's f67cbb74437842a0f88f84f43a0faa968ca77b35 and
2301d91dd58d9827865e360d616291f2549ec5bf.

Change-Id: I3b79323847a7610143a9dfb9b5b45bf7a33d8690
Reviewed-on: https://boringssl-review.googlesource.com/10369
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-18 18:18:31 +00:00
Adam Langley
aa24851515 Don't crash when generating a key for a zero DH prime.
I didn't look into whether this was reachable, but I assume not. Still,
better to be robust here becasue DH groups are commonly under some
amount of attacker control.

Change-Id: I1e0c33ccf314c73a9d34dd48312f6f7580049ba7
Reviewed-on: https://boringssl-review.googlesource.com/10261
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-11 16:52:56 +00:00
Adam Langley
37b486aade Remove optimisation for known DH groups.
Since we are eliminating DHE support in TLS, this is just a waste of
bytes.

Change-Id: I3a23ece564e43f7e8874d1ec797def132ba59504
Reviewed-on: https://boringssl-review.googlesource.com/10260
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-11 16:41:50 +00:00
Martin Kreichgauer
19d5cf86de Move remaining ScopedContext types out of scoped_types.h
Change-Id: I7d1fa964f0d9817db885cd43057a23ec46f21702
Reviewed-on: https://boringssl-review.googlesource.com/10240
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-11 01:15:45 +00:00
David Benjamin
96e1a25943 Add BN_get_rfc3526_prime_1536.
In OpenSSL 1.1.0, this API has been renamed to gain a BN prefix. Now
that it's no longer squatting on a namespace, provide the function so
wpa_supplicant needn't carry a BoringSSL #ifdef here.

BUG=91

Change-Id: Iac8e90238c816caae6acf0e359893c14a7a970f1
Reviewed-on: https://boringssl-review.googlesource.com/10223
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-10 19:28:15 +00:00
David Benjamin
5a91503826 Add various 1.1.0 accessors.
This gets cURL building against both BoringSSL as it is and BoringSSL
with OPENSSL_VERSION_NUMBER set to 1.1.0.

BUG=91

Change-Id: I5be73b84df701fe76f3055b1239ae4704a931082
Reviewed-on: https://boringssl-review.googlesource.com/10180
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-10 16:52:15 +00:00
Brian Smith
4edca0b308 Add BN_rand_range_ex and use internally.
There are many cases where we need |BN_rand_range| but with a minimum
value other than 0. |BN_rand_range_ex| provides that.

Change-Id: I564326c9206bf4e20a37414bdbce16a951c148ce
Reviewed-on: https://boringssl-review.googlesource.com/8921
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>
2016-07-29 16:09:26 +00:00
Adam Langley
10f97f3bfc Revert "Move C++ helpers into |bssl| namespace."
This reverts commit 09feb0f3d9.

(In order to make WebRTC happy this also needs to be reverted.)
2016-07-12 08:09:33 -07:00
Adam Langley
d2b5af56cf Revert scoped_types.h change.
This reverts commits:
8d79ed6740
19fdcb5234
8d79ed6740

Because WebRTC (at least) includes our headers in an extern "C" block,
which precludes having any C++ in them.

Change-Id: Ia849f43795a40034cbd45b22ea680b51aab28b2d
2016-07-12 08:05:38 -07:00
Adam Langley
8c3c3135a2 Remove scoped_types.h.
This change scatters the contents of the two scoped_types.h files into
the headers for each of the areas of the code. The types are now in the
|bssl| namespace.

Change-Id: I802b8de68fba4786b6a0ac1bacd11d81d5842423
Reviewed-on: https://boringssl-review.googlesource.com/8731
Reviewed-by: Adam Langley <agl@google.com>
2016-07-11 23:08:27 +00:00
Adam Langley
09feb0f3d9 Move C++ helpers into |bssl| namespace.
We currently have the situation where the |tool| and |bssl_shim| code
includes scoped_types.h from crypto/test and ssl/test. That's weird and
shouldn't happen. Also, our C++ consumers might quite like to have
access to the scoped types.

Thus this change moves some of the template code to base.h and puts it
all in a |bssl| namespace to prepare for scattering these types into
their respective headers. In order that all the existing test code be
able to access these types, it's all moved into the same namespace.

Change-Id: I3207e29474dc5fcc344ace43119df26dae04eabb
Reviewed-on: https://boringssl-review.googlesource.com/8730
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-11 23:04:52 +00:00
David Benjamin
44bedc348d Handle BN_mod_word failures.
As of 67cb49d045 and the corresponding upstream
change, BN_mod_word may fail, like BN_div_word. Handle this properly and
document in bn.h. Thanks to Brian Smith for pointing this out.

Change-Id: I6d4f32dc37bcabf70847c9a8b417d55d31b3a380
Reviewed-on: https://boringssl-review.googlesource.com/8491
Reviewed-by: Adam Langley <agl@google.com>
2016-06-23 21:25:18 +00:00
David Benjamin
53409ee3d7 Fix BN_is_prime* calls.
This function returns a tri-state -1 on error. We should check this.

Change-Id: I6fe130c11d10690923aac5ac7a6dfe3e3ff3f5e9
Reviewed-on: https://boringssl-review.googlesource.com/8490
Reviewed-by: Adam Langley <agl@google.com>
2016-06-23 21:22:33 +00:00
Brian Smith
0e01eb534c Call |BN_mod_exp_mont_consttime| in crypto/dh.
|BN_mod_exp_mont| will forward to |BN_mod_exp_mont_consttime|, so this
is a no-op semantically. However, this allows the linker to drop the
implementation of |BN_mod_exp_mont| even when the DH code is in use.

Change-Id: I0cb8b260224ed661ede74923bd134acb164459c1
Reviewed-on: https://boringssl-review.googlesource.com/7730
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-11 22:34:19 +00:00
David Benjamin
3473315415 Reimplement PKCS #3 DH parameter parsing with crypto/bytestring.
Also add a test.

This is the last of the openssl/asn1.h includes from the directories that are
to be kept in the core libcrypto library. (What remains is to finish sorting
out the crypto/obj stuff. We'll also want to retain a decoupled version of the
PKCS#12 stuff.)

Functions that need to be audited for reuse:
i2d_DHparams

BUG=54

Change-Id: Ibef030a98d3a93ae26e8e56869f14858ec75601b
Reviewed-on: https://boringssl-review.googlesource.com/7900
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-09 19:36:41 +00:00
Brian Smith
d035730ac7 Make return value of |BN_MONT_CTX_set_locked| int.
This reduces the chance of double-frees.

BUG=10

Change-Id: I11a240e2ea5572effeddc05acb94db08c54a2e0b
Reviewed-on: https://boringssl-review.googlesource.com/7583
Reviewed-by: David Benjamin <davidben@google.com>
2016-04-18 23:19:08 +00:00
Brian Smith
5ba06897be Don't cast |OPENSSL_malloc|/|OPENSSL_realloc| result.
C has implicit conversion of |void *| to other pointer types so these
casts are unnecessary. Clean them up to make the code easier to read
and to make it easier to find dangerous casts.

Change-Id: I26988a672e8ed4d69c75cfbb284413999b475464
Reviewed-on: https://boringssl-review.googlesource.com/7102
Reviewed-by: David Benjamin <davidben@google.com>
2016-02-11 22:07:56 +00:00
David Benjamin
47ebec1210 Validate DH public keys for RFC 5114 groups.
This is CVE-2016-0701 for OpenSSL, reported by Antonio Sanso. It is a no-op for
us as we'd long removed SSL_OP_DH_SINGLE_USE and static DH cipher suites. (We
also do not parse or generate X9.42 DH parameters.)

However, we do still have the APIs which return RFC 5114 groups, so we should
perform the necessary checks in case later consumers reuse keys.

Unlike groups we generate, RFC 5114 groups do not use "safe primes" and have
many small subgroups. In those cases, the subprime q is available. Before using
a public key, ensure its order is q by checking y^q = 1 (mod p). (q is assumed
to be prime and the existing range checks ensure y is not 1.)

(Imported from upstream's 878e2c5b13010329c203f309ed0c8f2113f85648 and
75374adf8a6ff69d6718952121875a491ed2cd29, but with some bugs fixed. See
RT4278.)

Change-Id: Ib18c3e84819002fa36a127ac12ca00ee33ea018a
Reviewed-on: https://boringssl-review.googlesource.com/7001
Reviewed-by: Adam Langley <agl@google.com>
2016-02-02 16:44:38 +00:00
David Benjamin
8a58933db0 Remove the CRYPTO_EX_new callback.
This callback is never used. The one caller I've ever seen is in Android
code which isn't built with BoringSSL and it was a no-op.

It also doesn't actually make much sense. A callback cannot reasonably
assume that it sees every, say, SSL_CTX created because the index may be
registered after the first SSL_CTX is created. Nor is there any point in
an EX_DATA consumer in one file knowing about an SSL_CTX created in
completely unrelated code.

Replace all the pointers with a typedef to int*. This will ensure code
which passes NULL or 0 continues to compile while breaking code which
passes an actual function.

This simplifies some object creation functions which now needn't worry
about CRYPTO_new_ex_data failing. (Also avoids bouncing on the lock, but
it's taking a read lock, so this doesn't really matter.)

BUG=391192

Change-Id: I02893883c6fa8693682075b7b130aa538a0a1437
Reviewed-on: https://boringssl-review.googlesource.com/6625
Reviewed-by: Adam Langley <agl@google.com>
2015-12-15 21:29:46 +00:00
Brian Smith
7af36e1e38 Share common definitions of |TOBN| and |BIGNUM_STATIC|.
Previously, both crypto/dh and crypto/ec defined |TOBN| macros that did
the same thing, but which took their arguments in the opposite order.
This change makes the code consistently use the same macro. It also
makes |STATIC_BIGNUM| available for internal use outside of crypto/bn.

Change-Id: Ide57f6a5b74ea95b3585724c7e1a630c82a864d9
Reviewed-on: https://boringssl-review.googlesource.com/6528
Reviewed-by: Adam Langley <agl@google.com>
2015-11-19 01:38:52 +00:00
David Benjamin
20c373118c Become partially -Wmissing-variable-declarations-clean.
There's a few things that will be kind of a nuisance and possibly not worth it
(crypto/asn1 dumps a lot of undeclared things, etc.). But it caught some
mistakes. Even without the warning, making sure to include the externs before
defining a function helps catch type mismatches.

Change-Id: I3dab282aaba6023e7cebc94ed7a767a5d7446b08
Reviewed-on: https://boringssl-review.googlesource.com/6484
Reviewed-by: Adam Langley <agl@google.com>
2015-11-12 20:09:20 +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
David Benjamin
cd24a39f1b Limit DHE groups to 4096-bit.
dh.c had a 10k-bit limit but it wasn't quite correctly enforced. However,
that's still 1.12s of jank on the IO thread, which is too long. Since the SSL
code consumes DHE groups from the network, it should be responsible for
enforcing what sanity it needs on them.

Costs of various bit lengths on 2013 Macbook Air:
1024 - 1.4ms
2048 - 14ms
3072 - 24ms
4096 - 55ms
5000 - 160ms
10000 - 1.12s

UMA says that DHE groups are 0.2% 4096-bit and otherwise are 5.5% 2048-bit and
94% 1024-bit and some noise. Set the limit to 4096-bit to be conservative,
although that's already quite a lot of jank.

BUG=554295

Change-Id: I8e167748a67e4e1adfb62d73dfff094abfa7d215
Reviewed-on: https://boringssl-review.googlesource.com/6464
Reviewed-by: Adam Langley <agl@google.com>
2015-11-11 22:18:39 +00:00
David Benjamin
e8f783ac0d Unwind DH_METHOD and DSA_METHOD.
This will allow a static linker (with -ffunction-sections since things aren't
split into files) to drop unused parts of DH and DSA. Notably, the parameter
generation bits pull in primality-checking code.

Change-Id: I25087e4cb91bc9d0ab43bcb267c2e2c164e56b59
Reviewed-on: https://boringssl-review.googlesource.com/6388
Reviewed-by: Adam Langley <agl@google.com>
2015-11-03 22:54:36 +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
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
David Benjamin
911cfb7e6e Unnecessary NULL checks.
Missed a few the last time around.

Change-Id: I42fd57566d64fa1c41cba14573742d42468cc07d
Reviewed-on: https://boringssl-review.googlesource.com/6349
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 19:45:25 +00:00
David Benjamin
79680ffaed Fix various malloc failure codepaths.
CRYPTO_MUTEX_init needs a CRYPTO_MUTEX_cleanup. Also a pile of problems
with x509_lu.c I noticed trying to import some upstream change.

Change-Id: I029a65cd2d30aa31f4832e8fbfe5b2ea0dbc66fe
Reviewed-on: https://boringssl-review.googlesource.com/6346
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 19:41:01 +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
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
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
a7997f12be Set minimum DH group size to 1024 bits.
DH groups less than 1024 bits are clearly not very safe. Ideally servers
would switch to ECDHE because 1024 isn't great either, but this will
serve for the short term.

BUG=490240

Change-Id: Ic9aac714cdcdcbfae319b5eb1410675d3b903a69
Reviewed-on: https://boringssl-review.googlesource.com/4813
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-05-20 18:35:31 +00:00
David Benjamin
6610118d65 Convert dh_test to C++.
Change-Id: I68fb6b152b587442ce085806ed1f11280ab8adfb
Reviewed-on: https://boringssl-review.googlesource.com/4689
Reviewed-by: Adam Langley <agl@google.com>
2015-05-11 18:47:30 +00:00
David Benjamin
22ccc2d8f1 Remove unnecessary NULL checks, part 1.
First batch of the alphabet.

Change-Id: If4e60f4fbb69e04eb4b70aa1b2240e329251bfa5
Reviewed-on: https://boringssl-review.googlesource.com/4514
Reviewed-by: Adam Langley <agl@google.com>
2015-05-04 23:05:17 +00:00