Commit Graph

2504 Commits

Author SHA1 Message Date
Brian Smith
3426d10119 Convert RSA blinding to use Montgomery multiplication.
|BN_mod_mul_montgomery| has better constant-time behavior (usually)
than |BN_mod_mul| and |BN_mod_sqr| and on platforms where we have
assembly language optimizations (when |OPENSSL_BN_ASM_MONT| is set in
crypto/bn/montgomery.c) it is faster. While doing so, reorder and
rename the |BN_MONT_CTX| parameters of the blinding functions to match
the order normally used in Montgomery math functions.

As a bonus, remove a redundant copy of the RSA public modulus from the
|BN_BLINDING| structure, which reduces memory usage.

Change-Id: I70597e40246429c7964947a1dc46d0d81c7530ef
Reviewed-on: https://boringssl-review.googlesource.com/7524
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-29 22:07:22 +00:00
David Benjamin
feaa57d13d Only call thread-local destructors on DLL_THREAD_DETACH.
In VS2015's debug runtime, the C runtime has been unloaded by the time
DLL_PROCESS_DETACH is called and things crash. Instead, don't run destructors
at that point.

This means we do *not* free memory associated with any remaining thread-locals
on application shutdown, only shutdown of individual threads. This is actually
desirable since it's consistent with pthreads. If an individual thread calls
pthread_exit, destructors are run. If the entire process exits, they are not.

(It's also consistent with thread_none.c which never bothers to free
anything.)

BUG=chromium:595795

Change-Id: I3e64d46ea03158fefff583c1e3e12dfa0c0e172d
Reviewed-on: https://boringssl-review.googlesource.com/7601
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-29 18:45:32 +00:00
David Benjamin
1e4ae00ac2 Add a comment about final empty extension intolerance.
We reordered extensions some time ago to ensure a non-empty extension was last,
but the comment was since lost (or I forgot to put one in in the first place).
Add one now so we don't regress.

Change-Id: I2f6e2c3777912eb2c522a54bbbee579ee37ee58a
Reviewed-on: https://boringssl-review.googlesource.com/7570
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-29 00:46:05 +00:00
Brian Smith
44477c03b9 Fix |BN_CTX_get| error checking in |BN_from_montgomery|.
In the case |BN_CTX_get| failed, the function returned without calling
|BN_CTX_end|. Fix that.

Change-Id: Ia24cba3256e2cec106b539324e9679d690048780
Reviewed-on: https://boringssl-review.googlesource.com/7592
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-29 00:44:10 +00:00
Brian Smith
9d354693ff Small tweak to P-256-x86-64 inversion.
Change-Id: I2a55db93e6140a0adc741b4ee5ee090d524605e0
Reviewed-on: https://boringssl-review.googlesource.com/7593
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-29 00:43:01 +00:00
David Benjamin
040ff622dc Remove duplicate BN_from_montgomery_word implementation.
It looks like we started reformatting that function and adding curly braces,
etc., but forget to finish it. This is corroborated by the diff. Although git
thinks I removed the EAY-style one and tweaked the #if-0'd one, I actually
clang-formatted the EAY-style one anew and deleted the #if-0'd one after
tweaking the style to match. Only difference is the alignment stuff is
uintptr_t rather than intptr_t since the old logic was using unsigned
arithmetic.

Change-Id: Ia244e4082a6b6aed3ef587d392d171382c32db33
Reviewed-on: https://boringssl-review.googlesource.com/7574
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-29 00:27:32 +00:00
Brian Smith
95cc3bea3b Remove dead code from |ec_GFp_mont_point_get_affine_coordinates|.
This code is only used in ec_montgomery.c, so |field_encode| and
|field_decode| are never NULL.

Change-Id: I42a3ad5744d4ed6f0be1707494411e7efcf930ff
Reviewed-on: https://boringssl-review.googlesource.com/7585
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-28 17:22:29 +00:00
Brian Smith
a00f845434 Move & rename |ec_GFp_simple_point_get_affine_coordinates|.
It is only used in ec_montgomery.c, so move it there.

Change-Id: Ib189d5579d6363bdc1da89b775ad3df824129758
Reviewed-on: https://boringssl-review.googlesource.com/7584
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-28 17:21:32 +00:00
David Benjamin
b7c5e84847 Fix some malloc test failures.
These only affect the tests.

Change-Id: If22d047dc98023501c771787b485276ece92d4a2
Reviewed-on: https://boringssl-review.googlesource.com/7573
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-28 17:17:32 +00:00
David Benjamin
e29ea166a6 Use ssl3_is_version_enabled to skip offering sessions.
We do an ad-hoc upper-bound check, but if the version is too low, we also
shouldn't offer the session. This isn't fatal to the connection and doesn't
have issues (we'll check the version later regardless), but offering a session
we're never going to accept is pointless. The check should match what we do in
ServerHello.

Credit to Matt Caswell for noticing the equivalent issue in an OpenSSL pull
request.

Change-Id: I17a4efd37afa63b34fca53f4c9b7ac3ae2fa3336
Reviewed-on: https://boringssl-review.googlesource.com/7543
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-28 16:01:37 +00:00
David Benjamin
762e1d039c Import chacha-x86.pl fix.
Patch from https://mta.openssl.org/pipermail/openssl-dev/2016-March/005625.html.

Upstream has yet to make a decision on aliasing requirements for their
assembly. If they choose to go with the stricter aliasing requirement rather
than land this patch, we'll probably want to tweak EVP_AEAD's API guarantees
accordingly and then undiverge.

In the meantime, import this to avoid a regression on x86 from when we had
compiler-vectorized code on GCC platforms.  Per our assembly coverage tools and
pending multi-CPU-variant tests, we have good coverage here. Unlike Poly1305
(which is currently waiting on yet another upstream bugfix), where there is
risk of missed carries everywhere, it is much more difficult to accidentally
make a ChaCha20 implementation that fails based on the data passed into it.

This restores a sizeable speed improvement on x86.

Before:
Did 1131000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000205us (1130768.2 ops/sec): 18.1 MB/s
Did 161000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1006136us (160018.1 ops/sec): 216.0 MB/s
Did 28000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1023264us (27363.4 ops/sec): 224.2 MB/s
Did 1166000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000447us (1165479.0 ops/sec): 18.6 MB/s
Did 160000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1004818us (159232.8 ops/sec): 215.0 MB/s
Did 30000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1016977us (29499.2 ops/sec): 241.7 MB/s

After:
Did 2208000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000031us (2207931.6 ops/sec): 35.3 MB/s
Did 402000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1001717us (401310.9 ops/sec): 541.8 MB/s
Did 97000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1005394us (96479.6 ops/sec): 790.4 MB/s
Did 2444000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000089us (2443782.5 ops/sec): 39.1 MB/s
Did 459000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1000563us (458741.7 ops/sec): 619.3 MB/s
Did 97000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1007942us (96235.7 ops/sec): 788.4 MB/s

Change-Id: I976da606dae062a776e0cc01229ec03a074035d1
Reviewed-on: https://boringssl-review.googlesource.com/7561
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-28 15:58:24 +00:00
David Benjamin
17d729e61b Remove unnecessary include.
Change-Id: I24d0179ca5019e82ca1494c8773f373f8c09ce82
Reviewed-on: https://boringssl-review.googlesource.com/7566
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-28 15:57:17 +00:00
David Benjamin
2aca226412 Fix typo in comment.
Change-Id: I0effe99d244c4ccdbb0e34db6e01a59c9463cb15
Reviewed-on: https://boringssl-review.googlesource.com/7572
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-28 15:57:00 +00:00
David Benjamin
baca950e8e Remove in_handshake.
The removes the last of OpenSSL's variables that count occurrences of a
function on the stack.

Change-Id: I1722c6d47bedb47b1613c4a5da01375b5c4cc220
Reviewed-on: https://boringssl-review.googlesource.com/7450
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-26 20:24:28 +00:00
David Benjamin
c79845c2a8 Move implicit handshake driving out of read_bytes.
This removes the final use of in_handshake. Note that there is still a
rentrant call of read_bytes -> handshake_func when we see a
HelloRequest. That will need to be signaled up to ssl_read_impl
separately out of read_app_data.

Change-Id: I823de243f75e6b73eb40c6cf44157b4fc21eb8fb
Reviewed-on: https://boringssl-review.googlesource.com/7439
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-26 20:23:25 +00:00
David Benjamin
b2a7318858 Switch some 0s to NULLs.
Change-Id: Id89c982f8f524720f189b528c987c9e58ca06ddf
Reviewed-on: https://boringssl-review.googlesource.com/7438
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-26 20:19:53 +00:00
David Benjamin
d7ac143814 Lift the handshake driving in write_bytes up to SSL_write.
This removes one use of in_handshake and consolidates some DTLS and TLS
code.

Change-Id: Ibbdd38360a983dabfb7b18c7bd59cb5e316b2adb
Reviewed-on: https://boringssl-review.googlesource.com/7435
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-26 20:09:37 +00:00
David Benjamin
282511d7eb Consolidate shutdown state.
fatal_alert isn't read at all right now, and warn_alert is only checked
for close_notify. We only need three states:

 - Not shutdown.
 - Got a fatal alert (don't care which).
 - Got a warning close_notify.

Leave ssl->shutdown alone for now as it's tied up with SSL_set_shutdown
and friends. To distinguish the remaining two, we only need a boolean.

Change-Id: I5877723af82b76965c75cefd67ec1f981242281b
Reviewed-on: https://boringssl-review.googlesource.com/7434
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-26 20:04:34 +00:00
David Benjamin
a2d4c0c426 Work around Android devices without AT_HWCAP2.
Some ARMv8 Android devices don't have AT_HWCAP2. This means, when running in
32-bit mode (ARM capability APIs on Linux are different between AArch32 and
AArch64), we can't discover the various nice instructions.

On a Nexus 6P, this gives a, uh, minor performance win when running in 32-bit
mode.

Before:
Did 1085000 AES-128-GCM (16 bytes) seal operations in 1000003us (1084996.7 ops/sec): 17.4 MB/s
Did 60000 AES-128-GCM (1350 bytes) seal operations in 1013416us (59205.7 ops/sec): 79.9 MB/s
Did 11000 AES-128-GCM (8192 bytes) seal operations in 1019778us (10786.7 ops/sec): 88.4 MB/s
Did 1009000 AES-256-GCM (16 bytes) seal operations in 1000650us (1008344.6 ops/sec): 16.1 MB/s
Did 49000 AES-256-GCM (1350 bytes) seal operations in 1015698us (48242.7 ops/sec): 65.1 MB/s
Did 9394 AES-256-GCM (8192 bytes) seal operations in 1071104us (8770.4 ops/sec): 71.8 MB/s
Did 1557000 SHA-1 (16 bytes) operations in 1000317us (1556506.6 ops/sec): 24.9 MB/s
Did 762000 SHA-1 (256 bytes) operations in 1000527us (761598.6 ops/sec): 195.0 MB/s
Did 45000 SHA-1 (8192 bytes) operations in 1013773us (44388.6 ops/sec): 363.6 MB/s
Did 1459000 SHA-256 (16 bytes) operations in 1000271us (1458604.7 ops/sec): 23.3 MB/s
Did 538000 SHA-256 (256 bytes) operations in 1000990us (537467.9 ops/sec): 137.6 MB/s
Did 26000 SHA-256 (8192 bytes) operations in 1008403us (25783.3 ops/sec): 211.2 MB/s

After:
Did 1890000 AES-128-GCM (16 bytes) seal operations in 1000068us (1889871.5 ops/sec): 30.2 MB/s
Did 509000 AES-128-GCM (1350 bytes) seal operations in 1000112us (508943.0 ops/sec): 687.1 MB/s
Did 110000 AES-128-GCM (8192 bytes) seal operations in 1007966us (109130.7 ops/sec): 894.0 MB/s
Did 1960000 AES-256-GCM (16 bytes) seal operations in 1000303us (1959406.3 ops/sec): 31.4 MB/s
Did 460000 AES-256-GCM (1350 bytes) seal operations in 1001873us (459140.0 ops/sec): 619.8 MB/s
Did 97000 AES-256-GCM (8192 bytes) seal operations in 1005337us (96485.1 ops/sec): 790.4 MB/s
Did 1927000 SHA-1 (16 bytes) operations in 1000429us (1926173.7 ops/sec): 30.8 MB/s
Did 1151000 SHA-1 (256 bytes) operations in 1000425us (1150511.0 ops/sec): 294.5 MB/s
Did 87000 SHA-1 (8192 bytes) operations in 1003089us (86732.1 ops/sec): 710.5 MB/s
Did 2357390 SHA-256 (16 bytes) operations in 1000116us (2357116.6 ops/sec): 37.7 MB/s
Did 1410000 SHA-256 (256 bytes) operations in 1000176us (1409751.9 ops/sec): 360.9 MB/s
Did 101000 SHA-256 (8192 bytes) operations in 1007007us (100297.2 ops/sec): 821.6 MB/s

BUG=chromium:596156

Change-Id: Iacc1f8d8a07e991d4615f2e12c5c54923fb31aa2
Reviewed-on: https://boringssl-review.googlesource.com/7507
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-26 04:56:45 +00:00
David Benjamin
054e151b16 Rewrite ARM feature detection.
This removes the thread-unsafe SIGILL-based detection and the
multi-consumer-hostile CRYPTO_set_NEON_capable API. (Changing
OPENSSL_armcap_P after initialization is likely to cause problems.)

The right way to detect ARM features on Linux is getauxval. On aarch64,
we should be able to rely on this, so use it straight. Split this out
into its own file. The #ifdefs in the old cpu-arm.c meant it shared all
but no code with its arm counterpart anyway.

Unfortunately, various versions of Android have different missing APIs, so, on
arm, we need a series of workarounds. Previously, we used a SIGILL fallback
based on OpenSSL's logic, but this is inherently not thread-safe. (SIGILL also
does not tell us if the OS knows how to save and restore NEON state.) Instead,
base the behavior on Android NDK's cpu-features library, what Chromium
currently uses with CRYPTO_set_NEON_capable:

- Android before API level 20 does not provide getauxval. Where missing,
  we can read from /proc/self/auxv.

- On some versions of Android, /proc/self/auxv is also not readable, so
  use /proc/cpuinfo's Features line.

- Linux only advertises optional features in /proc/cpuinfo. ARMv8 makes NEON
  mandatory, so /proc/cpuinfo can't be used without additional effort.

Finally, we must blacklist a particular chip because the NEON unit is broken
(https://crbug.com/341598).

Unfortunately, this means CRYPTO_library_init now depends on /proc being
available, which will require some care with Chromium's sandbox. The
simplest solution is to just call CRYPTO_library_init before entering
the sandbox.

It's worth noting that Chromium's current EnsureOpenSSLInit function already
depends on /proc/cpuinfo to detect the broken CPU, by way of base::CPU.
android_getCpuFeatures also interally depends on it. We were already relying on
both of those being stateful and primed prior to entering the sandbox.

BUG=chromium:589200

Change-Id: Ic5d1c341aab5a614eb129d8aa5ada2809edd6af8
Reviewed-on: https://boringssl-review.googlesource.com/7506
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-26 04:54:44 +00:00
Brian Smith
dc6c1b8381 Fix build when using Visual Studio 2015 Update 1.
Many of the compatibility issues are described at
https://msdn.microsoft.com/en-us/library/mt612856.aspx. The macros
that suppressed warnings on a per-function basis no longer work in
Update 1, so replace them with #pragmas. Update 1 warns when |size_t|
arguments to |printf| are casted, so stop doing that casting.
Unfortunately, this requires an ugly hack to continue working in
MSVC 2013 as MSVC 2013 doesn't support "%zu". Finally, Update 1 has new
warnings, some of which need to be suppressed.

---

Updated by davidben to give up on suppressing warnings in crypto/x509 and
crypto/x509v3 as those directories aren't changed much from upstream. In each
of these cases, upstream opted just blindly initialize the variable, so do the
same. Also switch C4265 to level 4, per Microsoft's recommendation and work
around a bug in limits.h that happens to get fixed by Google include order
style.

(limits.h is sensitive to whether corecrt.h, pulled in by stddef.h and some
other headers, is included before it. The reason it affected just one file is
we often put the file's header first, which means base.h is pulling in
stddef.h. Relying on this is ugly, but it's no worse than what everything else
is doing and this doesn't seem worth making something as tame as limits.h so
messy to use.)

Change-Id: I02d1f935356899f424d3525d03eca401bfa3e6cd
Reviewed-on: https://boringssl-review.googlesource.com/7480
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-25 21:39:52 +00:00
David Benjamin
db50299b24 Add tests for RSA objects with only n and d.
Conscrypt, thanks to Java's RSAPrivateKeySpec API, must be able to use RSA keys
with only modulus and exponent. This is kind of silly and breaks the blinding
code so they, both in OpenSSL and BoringSSL, had to explicitly turn blinding
off.

Add a test for this as we're otherwise sure to break it on accident.

We may wish to avoid the silly rsa->flags modification, I'm not sure. For now,
keep the requirement in so other consumers do not accidentally rely on this.

(Also add a few missing ERR_clear_error calls. Functions which are expected to
fail should be followed by an ERR_clear_error so later unexpected failures
don't get confused.)

BUG=boringssl:12

Change-Id: I674349821f1f59292b8edd085f21dc37e8bcaa75
Reviewed-on: https://boringssl-review.googlesource.com/7560
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-25 20:52:17 +00:00
Brian Smith
cbf56a5683 Clarify lifecycle of |BN_BLINDING|.
In |bn_blinding_update| the condition |b->e != NULL| would never be
true (probably), but the test made reasoning about the correctness of
the code confusing. That confusion was amplified by the circuitous and
unusual way in which |BN_BLINDING|s are constructed. Clarify all this
by simplifying the construction of |BN_BLINDING|s, making it more like
the construction of other structures.

Also, make counter unsigned as it is no longer ever negative.

Change-Id: I6161dcfeae19a80c780ccc6762314079fca1088b
Reviewed-on: https://boringssl-review.googlesource.com/7530
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-25 20:08:04 +00:00
Brian Smith
24493a4ff4 Always cache Montgomery contexts in RSA.
Simplify the code by always caching Montgomery contexts in the RSA
structure, regardless of the |RSA_FLAG_CACHE_PUBLIC| and
|RSA_FLAG_CACHE_PRIVATE| flags. Deprecate those flags.

Now that we do this no more than once per key per RSA exponent, the
private key exponents better because the initialization of the
Montgomery contexts isn't perfectly side-channel protected.

Change-Id: I4fbcfec0f2f628930bfeb811285b0ae3d103ac5e
Reviewed-on: https://boringssl-review.googlesource.com/7521
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-25 20:04:24 +00:00
David Benjamin
4339552fbb Flip the arguments to ExpectBytesEqual in poly1305_test.
The function wants the expected value first.

Change-Id: I6d3e21ebfa55d6dd99a34fe8380913641b4f5ff6
Reviewed-on: https://boringssl-review.googlesource.com/7501
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-24 19:30:47 +00:00
David Benjamin
270f0a7761 Print an error if no tests match in runner.
Otherwise it's confusing if you mistype the test name.

Change-Id: Idf32081958f85f3b5aeb8993a07f6975c27644f8
Reviewed-on: https://boringssl-review.googlesource.com/7500
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-24 19:30:29 +00:00
David Benjamin
4c34026d12 Fix poly1305-x86.pl.
Imported from patch attached to
https://rt.openssl.org/Ticket/Display.html?id=4439.

But with the extra vs $extra typo fixed.

The root problem appears to be that lazy_reduction tries to use paddd instead
of paddq when they believe the sum will not overflow a u32. In the final call
to lazy_reduction, this is not true. svaldez and I attempted to work through
the bounds, but the bounds derived from the cited paper imply paddd is always
fine. Empirically in a debugger, the bounds are exceeded in the test case.

I requested more comments from upstream on the bug. When upstream lands their
final fix (hopefully with comments), I will update this code. In the meantime,
let's stop carrying known-broken stuff.

(vlazy_reduction is probably something similar, but since we don't enable that
code, we haven't bothered analyzing it.)

Also add the smaller of the two test cases that catch the bug. (The other uses
an update pattern which isn't quite what poly1305_test does.)

Change-Id: I446ed47c21f10b41a0745de96ab119a3f6fd7801
Reviewed-on: https://boringssl-review.googlesource.com/7544
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-24 15:04:23 +00:00
Piotr Sikora
fdb88ba2e9 Fix build with -Wwrite-strings.
Change-Id: If76154c8d255600e925a408acdc674fc7dad0359
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/7526
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-24 03:11:20 +00:00
Matt Mueller
897be6afe3 Add CBS_ASN1_UTF8STRING define.
Change-Id: I34384feb46c15c4f443f506d724ad500a4cf0f36
Reviewed-on: https://boringssl-review.googlesource.com/7525
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-23 19:29:49 +00:00
Steven Valdez
0a0f83d308 Fixing assembly coverage
We failed to correctly parse files that executed from the very start of
the file due to a missing '- line XXX'. We now use the 'Ir' indicator to
recognize the beginning of a file.

Change-Id: I529fae9458ac634bf7bf8af61ef18f080e808535
Reviewed-on: https://boringssl-review.googlesource.com/7542
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-23 18:23:42 +00:00
Brian Smith
afd6d9d61a Use |size_t| and |int| consistently in p{224,256}-64.c.
Use |size_t| for array indexes. Use |int| for boolean flags. Declare
the variables that had their types changed closer to where they are
used.

Previously, some `for` loops depended on `i` being signed, so their
structure had to be changed to work with the unsigned type.

Change-Id: I247e4f04468419466733b6818d81d28666da0ad3
Reviewed-on: https://boringssl-review.googlesource.com/7468
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-22 23:28:08 +00:00
David Benjamin
9539ebbf70 Update FUZZING documentation about max_len.
Maintain the max_len values in foo.options files which ClusterFuzz can process.
Also recompute the recommended client and server lengths as they've since
gotten much more extensive.

Change-Id: Ie87a80d8a4a0c41e215f0537c8ccf82b38c4de09
Reviewed-on: https://boringssl-review.googlesource.com/7509
Reviewed-by: Mike Aizatsky <aizatsky@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-22 18:46:35 +00:00
David Benjamin
78f8aabe44 ssl->ctx cannot be NULL.
Most code already dereferences it directly.

Change-Id: I227fa91ecbf25a19077f7cfba21b0abd2bc2bd1d
Reviewed-on: https://boringssl-review.googlesource.com/7422
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-22 15:24:10 +00:00
Steven Valdez
c087c332f8 Fix potential double free in EVP_DigestInit_ex
There is a potential double free in EVP_DigestInit_ex. This is believed
to be reached only as a result of programmer error - but we should fix it
anyway.

(Imported from upstream's e78dc7e279ed98e1ab9845a70d14dafdfdc88f58)

Change-Id: I1da7be7db7afcbe9f30f168df000d64ed73d7edd
Reviewed-on: https://boringssl-review.googlesource.com/7541
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-22 15:17:32 +00:00
David Benjamin
be12248829 Fix aarch64 build.
We recently gained -Werror=missing-prototypes. (See also, we really need to get
those Android bots...)

Change-Id: I3962d3050bccf5f5a057d029b5cbff1695ca1a03
Reviewed-on: https://boringssl-review.googlesource.com/7540
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-21 22:56:55 +00:00
Brian Smith
95b9769340 Fix error handling in |bn_blinding_update|.
The fields of the |bn_blinding_st| are not updated atomically.
Consequently, one field (|A| or |Ai|) might get updated while the
other field (|Ai| or |A|) doesn't get updated, if an error occurs in
the middle of updating. Deal with this by reseting the counter so that
|A| and |Ai| will both get recreated the next time the blinding is
used.

Fix a separate but related issue by resetting the counter to zero after
calling |bn_blinding_create_param| only if |bn_blinding_create_param|
succeeded. Previously, regardless of whether an error occured in
|bn_blinding_create_param|, |b->counter| would get reset to zero. The
consequence of this was that potentially-bad blinding values would get
used 32 times instead of (32 - |b->counter|) times.

Change-Id: I236cdb6120870ef06cba129ed86619f593cbcf3d
Reviewed-on: https://boringssl-review.googlesource.com/7520
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-21 20:26:21 +00:00
David Benjamin
e11988f511 Tweak FUZZING.md and minimise_corpuses.sh.
Change-Id: If312ce3783bcc39ebd2047470251334aa0897d3d
Reviewed-on: https://boringssl-review.googlesource.com/7508
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-21 20:23:15 +00:00
Brian Smith
fdc955cf14 Fix parameter type of p256-64.c's |select_point|.
Make it match how it is done in p224-64.c. Note in particular that
|size| may be 17, so presumably |pre_comp[16]| is accessed, which one
would not expect when it was declared |precomp[16][3]|.

Change-Id: I54c1555f9e20ccaacbd4cd75a7154b483b4197b7
Reviewed-on: https://boringssl-review.googlesource.com/7467
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-20 17:18:35 +00:00
Brian Smith
df1201e6ee Remove unnecessary |BN_CTX_start|/|BN_CTX_end| in |BN_mod_exp_mont_consttime|.
Since the function doesn't call |BN_CTX_get|, it doesn't need to call
|BN_CTX_start|/|BN_CTX_end|.

Change-Id: I6cb954d3fee2959bdbc81b9b97abc52bb6f7704c
Reviewed-on: https://boringssl-review.googlesource.com/7469
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-20 17:16:27 +00:00
Brian Smith
7cf6085b00 Check for |BN_CTX_new| failure in |mod_exp|.
As far as I can tell, this is the last place within libcrypto where
this type of check is missing.

Change-Id: I3d09676abab8c9f6c4e87214019a382ec2ba90ee
Reviewed-on: https://boringssl-review.googlesource.com/7519
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-20 17:09:51 +00:00
Piotr Sikora
35673b945d Build with -Wmissing-prototypes -Wmissing-declarations.
Change-Id: Ieba81f114483095f3657e87f669c7562ff75b58c
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/7516
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-20 17:05:03 +00:00
Piotr Sikora
f932894c7f Move function declarations to internal header.
Partially fixes build with -Wmissing-declarations.

Change-Id: Ia563063fb077cda79244c21f02fd1c0f550353c2
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/7515
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-20 16:56:32 +00:00
Piotr Sikora
f188f9dce8 Fix typo in function name.
Partially fixes build with -Wmissing-prototypes.

Change-Id: I828bcfb49b23c5a9ea403038bc3fb76750556ef8
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/7514
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-20 16:55:41 +00:00
David Benjamin
cd4cf9a12e Fix Windows build
Change-Id: I66ecb9f89ec13e432e888e3825d01a015b117568
Reviewed-on: https://boringssl-review.googlesource.com/7505
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-20 16:46:10 +00:00
Piotr Sikora
8b0fe8c0ac Add missing prototypes.
Partially fixes build with -Wmissing-prototypes.

Change-Id: If04d8fe7cbf068883485e95bd5ea6cdab6743e46
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/7513
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-20 16:43:50 +00:00
Piotr Sikora
c6d3029eda Add missing internal includes.
Partially fixes build with -Wmissing-prototypes -Wmissing-declarations.

Change-Id: I51209c30f532899f57cfdd9a50cff0a8ee3da5b5
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/7512
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-20 16:38:54 +00:00
Piotr Sikora
9bb8ba6ba1 Make local functions static.
Partially fixes build with -Wmissing-prototypes -Wmissing-declarations.

Change-Id: I6048f5b7ef31560399b25ed9880156bc7d8abac2
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/7511
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-20 16:37:58 +00:00
Piotr Sikora
537cfc37b8 Use UINT64_C instead of unsigned long long integer constant.
Change-Id: I44aa9be26ad9aea6771cb46a886a721b4bc28fde
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/7510
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-18 23:04:48 +00:00
David Benjamin
594e7d2b77 Add a test that declining ALPN works.
Inspired by https://mta.openssl.org/pipermail/openssl-dev/2016-March/006150.html

Change-Id: I973b3baf054ed1051002f7bb9941cb1deeb36d78
Reviewed-on: https://boringssl-review.googlesource.com/7504
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-18 19:47:46 +00:00
David Benjamin
f277add6c2 Run ripemd_test as part of all_tests.go.
Change-Id: I9c5e66c34d0f1b735c69d033daee5d312e3c2fe7
Reviewed-on: https://boringssl-review.googlesource.com/7410
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-17 21:37:14 +00:00