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>
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>
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>
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>
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>
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>
BoringSSL always uses uncompressed points. This function aborts if
another form is requested or does nothing if uncompressed points are
requested.
Change-Id: I80bc01444cdf9c789c9c75312b5527bf4957361b
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>
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>
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>
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
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
(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>
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>
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>
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
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>
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>
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>
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>
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>
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
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
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>
(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>
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>
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>
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>
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>
