Commit Graph

4264 Commits

Author SHA1 Message Date
David Benjamin
8d5f9da2e3 Abstract away BIO_flush calls in the handshake.
This is the first part to removing the buffer BIO. The eventual end
state is the SSL_PROTOCOL_METHOD is responsible for maintaining one
flight's worth of messages. In TLS, it will just be a buffer containing
the flight's ciphertext. In DTLS, it's the existing structure for
retransmit purposes. There will be hooks:

- add_message (synchronous)
- add_change_cipher_spec (synchronous)
- add_warning_alert (synchronous; needed until we lose SSLv3 client auth
  and TLS 1.3 draft 18; draft 19 will switch end_of_early_data to a
  handshake message)
- write_flight (BIO; flush_flight will be renamed to this)

This also preserves the exact return value of BIO_flush. Eventually all
the BIO_write calls will be hidden behind BIO_flush to, to be consistent
with other BIO-based calls, preserve the return value.

BUG=72

Change-Id: I74cd23759a17356aab3bb475a8ea42bd2cd115c9
Reviewed-on: https://boringssl-review.googlesource.com/13222
Reviewed-by: Adam Langley <agl@google.com>
2017-01-24 16:16:02 +00:00
Nick Harper
44c1a65760 Run go fmt on bogo code.
Change-Id: I15363a9c9ebb4e08bd9cf45ba2c95368766bb19b
Reviewed-on: https://boringssl-review.googlesource.com/13240
Reviewed-by: David Benjamin <davidben@google.com>
2017-01-24 00:29:38 +00:00
Adam Langley
1da9c67a99 Use a Perlasm variable rather than an #if to exclude the ChaCha20-Poly1305 asm on Windows.
The Windows assembler doesn't appear to do preprocessor macros but nor
can it cope with this style of label.

Change-Id: I0b8ca7372bb9ea0f20101ed138681d379944658e
Reviewed-on: https://boringssl-review.googlesource.com/13207
Reviewed-by: David Benjamin <davidben@google.com>
2017-01-23 22:05:06 +00:00
Adam Langley
feca9e562c Emit ssl_[c|cc]_sources for Bazel.
Bazel doesn't allow one to give different flags for C and C++ files, so
trying to set -std=c11 for all ssl/ sources (which now include C++)
blows up.

This change splits the lists for Bazel so that they can be put in
different cc_library targets and thus have different flags.

Change-Id: I1e3dee01b6558de59246bc470527d44c9c86b188
Reviewed-on: https://boringssl-review.googlesource.com/13206
Reviewed-by: Adam Langley <agl@google.com>
2017-01-23 21:36:37 +00:00
vkrasnov
8d56558031 Optimized Seal/Open routines for ChaCha20-Poly1305 for x86-64
This is basically the same implementation I wrote for Go

The Go implementation:
https://github.com/golang/crypto/blob/master/chacha20poly1305/chacha20poly1305_amd64.s
The Cloudflare patch for OpenSSL:
https://github.com/cloudflare/sslconfig/blob/master/patches/openssl__chacha20_poly1305_draft_and_rfc_ossl102j.patch

The Seal/Open is only available for the new version, the old one uses
the bundled Poly1305, and the existing ChaCha20 implementations

The benefits of this code, compared to the optimized code currently
disabled in BoringSSL:

* Passes test vectors
* Faster performance: The AVX2 code (on Haswell), is 55% faster for 16B,
  15% for 1350 and 6% for 8192 byte buffers
* Even faster on pre-AVX2 CPUs

Feel free to put whatever license, etc. is appropriate, under the
existing CLA.

Benchmarks are for 16/1350/8192 chunk sizes and given in MB/s:

Before (Ivy Bridge): 34.2   589.5  739.4
After:               68.4   692.1  799.4
Before (Skylake):    50    1233   1649
After:              119.4  1736   2196
After (Andy's):      63.6  1608   2261

Change-Id: I9186f721812655011fc17698b67ddbe8a1c7203b
Reviewed-on: https://boringssl-review.googlesource.com/13142
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-01-23 21:12:44 +00:00
David Benjamin
358baeb9a4 Add missing src/ prefix to GTest sources.
Change-Id: I2ceb88f745db6fd16b30fe6f3f8fd9c29f0d3b8d
Reviewed-on: https://boringssl-review.googlesource.com/13234
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-01-23 20:19:40 +00:00
David Benjamin
5b410b6bec Remove unnecessary CBS_get_asn1_element.
EVP_parse_public_key already acts like CBS_get_* in that it peels one
element off and leaves a remainder.

Change-Id: Ic90952785005ed81664a6f46503b13ecd293176c
Reviewed-on: https://boringssl-review.googlesource.com/13045
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2017-01-21 00:50:13 +00:00
Adam Langley
1aa4a5bdbd Delete unused Poly1305 assembly.
(These files weren't being built anyway.)

Change-Id: Id6c8d211b9ef867bdb7d83153458f9ad4e29e525
Reviewed-on: https://boringssl-review.googlesource.com/13205
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2017-01-21 00:17:30 +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
d1263b05a9 Stop emitting tests for gyp.
Chromium hasn't used gyp for a while. Get this out of the way for the
googletest transition.

BUG=129

Change-Id: Ic8808391d9f7de3e95cfc68654acf825389f6829
Reviewed-on: https://boringssl-review.googlesource.com/13231
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-01-21 00:12:22 +00:00
David Benjamin
9fb326d47e Fix MSVC C4826 issues in googletest.
This applies https://github.com/google/googletest/pull/991.

BUG=129

Change-Id: I3df7e265652f2a337721634b5ba8adf76ff7d828
Reviewed-on: https://boringssl-review.googlesource.com/13233
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-01-21 00:12:17 +00:00
David Benjamin
c10c29861d Fix ColorPrintf issues in googletest.
This applies https://github.com/google/googletest/pull/965.

BUG=129

Change-Id: Id5fda923b0d3c26e6e004dc292c8d2cbd3729b45
Reviewed-on: https://boringssl-review.googlesource.com/13230
Reviewed-by: David Benjamin <davidben@google.com>
2017-01-21 00:12:10 +00:00
David Benjamin
9b5028523f Check in a pristine copy of googletest.
Snapshotted from 5e7fd50e17b6edf1cadff973d0ec68966cf3265e in the
upstream repository:
https://github.com/google/googletest

Since standalone builds and bots will need this, checking in a copy
rather than require everyone use gclient, repo, git submodules or scary
CMake scripts is probably simplest.

Consumers with their own copies of googletest will likely wish to ignore
or even exclude this directory.

BUG=129

Change-Id: If9f4cec5ae0d7a3976dcfffd1ead6950ef7b7c4e
Reviewed-on: https://boringssl-review.googlesource.com/13229
Reviewed-by: David Benjamin <davidben@google.com>
2017-01-21 00:10:13 +00:00
Alessandro Ghedini
958346a5e7 Run select_certificate_cb multiple times
It's not completely clear to me why select_cetificate_cb behaves the way it
does, however not only is it confusing, but it makes assumptions about the
application using BoringSSL (it's not always possible to implement custom
logic outside of the callbacks provided by libssl), that make this callback
somewhat useless.

Case in point, the callback can be used for changing min/max protocol versions
based on per-site policies, and select_certificate_cb is the only place where
SSL_set_min/max_proto_version() can be used (e.g. you can't call them in
cert_cb because it's too late), but the decision on the specific versions to
use might depend on configuration that needs retrieving asynchronously from
over the network, which requires re-running the callback multiple times.

Change-Id: Ia8e151b163628545373e7fd1f327e9af207478a6
Reviewed-on: https://boringssl-review.googlesource.com/13000
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2017-01-20 23:55:50 +00:00
Adam Langley
5c7a4b8c2f Add test for truncated AEAD tags.
Several of our AEADs support truncated tags, but I don't believe that we
had a test for them previously.

Change-Id: I63fdd194c47c17b3d816b912a568534c393df9d8
Reviewed-on: https://boringssl-review.googlesource.com/13204
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-01-20 21:45:47 +00:00
David Benjamin
07820b5cee Add a getter for SSL_set_session_id_context.
We have a test somewhere which tries to read off of it. Align the getter
roughly with upstream's SSL_SESSION_get0_id_context (which we don't
currently expose).

BUG=6

Change-Id: Iab240868838ba56c1f08d112888d9536574347b4
Reviewed-on: https://boringssl-review.googlesource.com/12636
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-01-20 04:39:42 +00:00
Adam Langley
2e839244b0 Remove old ChaCha20-Poly1305 AEAD.
Before RFC 7539 we had a ChaCha20-Poly1305 cipher suite that had a 64/64
nonce/counter split (as DJB's original ChaCha20 did). RFC 7539 changed
that to 96/32 and we've supported both for some time.

This change removes the old version and the TLS cipher suites that used
it.

BUG=chromium:682816

Change-Id: I2345d6db83441691fe0c1ab6d7c6da4d24777849
Reviewed-on: https://boringssl-review.googlesource.com/13203
Reviewed-by: Adam Langley <agl@google.com>
2017-01-19 23:27:54 +00:00
Adam Langley
5322010405 Revert "Remove old ChaCha20-Poly1305 AEAD."
This reverts commit def9b46801.

(I should have uploaded a new version before sending to the commit queue.)

Change-Id: Iaead89c8d7fc1f56e6294d869db9238b467f520a
Reviewed-on: https://boringssl-review.googlesource.com/13202
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-01-19 23:07:06 +00:00
David Benjamin
6752efdeaf Never send SNI warning alerts.
TLS 1.3 forbids warning alerts, and sending these is a bad idea. Per RFC
6066:

   If the server understood the ClientHello extension but
   does not recognize the server name, the server SHOULD take one of two
   actions: either abort the handshake by sending a fatal-level
   unrecognized_name(112) alert or continue the handshake.  It is NOT
   RECOMMENDED to send a warning-level unrecognized_name(112) alert,
   because the client's behavior in response to warning-level alerts is
   unpredictable.

The motivation is to cut down on the number of places where we send
non-closing alerts. We can't remove them yet (SSL 3.0 and TLS 1.3 draft
18 need to go), but eventually this can be a simplifying assumption.
Already this means DTLS never sends warning alerts, which is good
because DTLS can't retransmit them.

Change-Id: I577a1eb9c23e66d28235c0fbe913f00965e19486
Reviewed-on: https://boringssl-review.googlesource.com/13221
Reviewed-by: Adam Langley <agl@google.com>
2017-01-19 23:03:11 +00:00
David Benjamin
a8c8b387f1 Don't call the SNI callback as a client.
This doesn't do anything useful. Every caller either never sets the
callback as a client or goes out of their way to filter out clients in
the callback.

Change-Id: I6f07d000a727f9ccba080f812e6b8e7a38e04350
Reviewed-on: https://boringssl-review.googlesource.com/13220
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-01-19 22:57:46 +00:00
Adam Langley
def9b46801 Remove old ChaCha20-Poly1305 AEAD.
Before RFC 7539 we had a ChaCha20-Poly1305 cipher suite that had a 64/64
nonce/counter split (as DJB's original ChaCha20 did). RFC 7539 changed
that to 96/32 and we've supported both for some time.

This change removes the old version and the TLS cipher suites that used
it.

Change-Id: Icd9c2117c657f3aa6df55990c618d562194ef0e8
Reviewed-on: https://boringssl-review.googlesource.com/13201
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
2017-01-19 22:54:50 +00:00
David Benjamin
1252f8758a Convert one libssl function to C++11.
This is to make sure all of libssl's consumers' have sufficiently
reasonable toolchains. Once this bakes, we can go about moving
libssl to C++.

This is just starting with libssl for now because libcrypto has more
consumers and libssl would benefit more from C++ than libcrypto (though
libcrypto also has code that would benefit).

BUG=132

Change-Id: Ie02f7b0a8a95defd289cc7e62451d4b16408ca2a
Reviewed-on: https://boringssl-review.googlesource.com/13161
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-01-17 21:51:06 +00:00
Alessandro Ghedini
0726fb76eb Add SSL_CIPHER_is_AEAD.
Change-Id: Ia6598ee4b2d4623abfc140d6a5c0eca4bcb30427
Reviewed-on: https://boringssl-review.googlesource.com/13180
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-01-17 16:41:49 +00:00
Brian Smith
a26d4c3f43 Enable stitched x86-64 AES-NI AES-GCM implementation.
Measured on a SkyLake processor:

Before:

Did 11373750 AES-128-GCM (16 bytes) seal operations in 1016000us (11194635.8 ops/sec): 179.1 MB/s
Did 2253000 AES-128-GCM (1350 bytes) seal operations in 1016000us (2217519.7 ops/sec): 2993.7 MB/s
Did 453750 AES-128-GCM (8192 bytes) seal operations in 1015000us (447044.3 ops/sec): 3662.2 MB/s
Did 10753500 AES-256-GCM (16 bytes) seal operations in 1016000us (10584153.5 ops/sec): 169.3 MB/s
Did 1898750 AES-256-GCM (1350 bytes) seal operations in 1015000us (1870689.7 ops/sec): 2525.4 MB/s
Did 374000 AES-256-GCM (8192 bytes) seal operations in 1016000us (368110.2 ops/sec): 3015.6 MB/s

After:

Did 11074000 AES-128-GCM (16 bytes) seal operations in 1015000us (10910344.8 ops/sec): 174.6 MB/s
Did 3178250 AES-128-GCM (1350 bytes) seal operations in 1016000us (3128198.8 ops/sec): 4223.1 MB/s
Did 734500 AES-128-GCM (8192 bytes) seal operations in 1016000us (722933.1 ops/sec): 5922.3 MB/s
Did 10394750 AES-256-GCM (16 bytes) seal operations in 1015000us (10241133.0 ops/sec): 163.9 MB/s
Did 2502250 AES-256-GCM (1350 bytes) seal operations in 1016000us (2462844.5 ops/sec): 3324.8 MB/s
Did 544500 AES-256-GCM (8192 bytes) seal operations in 1015000us (536453.2 ops/sec): 4394.6 MB/s

Change-Id: If058935796441ed4e577b9a72d3aa43422edba58
Reviewed-on: https://boringssl-review.googlesource.com/7273
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-16 16:54:13 +00:00
Adam Langley
abb32cc00d Restore H (the key) in the GHASH context.
This was removed in a00cafc50c because
none of the assembly actually appeared to need it. However, we found the
assembly the uses it: the MOVBE-based, x86-64 code.

Needing H seems silly since Htable is there, but rather than mess with
the assembly, it's easier to put H back in the structure—now with a
better comment.

Change-Id: Ie038cc4482387264d5e0821664fb41f575826d6f
Reviewed-on: https://boringssl-review.googlesource.com/13122
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-16 16:53:32 +00:00
Aaron Green
67ccf59161 Fix crypto/rand/urandom header guards for Fuchsia.
Fuchsia uses crypto/rand/fuchsia.c for CRYPTO_sysrand, and so must be
excluded from the Linux/Apple/POSIX variant.

Change-Id: Ide9f0aa2547d52ce0579cd0a1882b2cdcc7b95c6
Reviewed-on: https://boringssl-review.googlesource.com/13141
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-01-14 01:03:01 +00:00
David Benjamin
c253864993 Remove some node.js hacks.
These are no longer needed.

Change-Id: I909f7d690f57dafcdad6254948b5683757da69f4
Reviewed-on: https://boringssl-review.googlesource.com/13160
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-01-13 21:50:39 +00:00
Aaron Green
c80e416353 Add support for Fuchsia in crypto/rand.
This change adds the OS-specific routines to get random bytes when using
BoringSSL on Fuchsia.  Fuchsia uses the Magenta kernel, which provides
random bytes via a syscall rather than via a file or library function.

Change-Id: I32f858246425309d643d142214c7b8de0c62250a
Reviewed-on: https://boringssl-review.googlesource.com/13140
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-01-13 21:47:11 +00:00
Brian Smith
b4cc925c30 Remove specialized assembly language |ecp_nistz256_from_mont|.
This function is only called twice per ECDH or ECDSA operation, and
it only saves a few scalar multiplications and additions compared to
the alternative, so it doesn't need to be specialized.

As the TODO comment above the callers notes, the two calls can be
reduced to one. Implementing |ecp_nistz256_from_mont| in terms of
|ecp_nistz256_mul_mont| helps show that that change is safe.

This also saves a small amount of code size and improves testing and
verification efficiency.

Note that this is already how the function is implemented for targets
other than x86-64 in OpenSSL.

Change-Id: If1404951f1a787d2618c853afd1f0e99a019e012
Reviewed-on: https://boringssl-review.googlesource.com/13021
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-13 17:27:13 +00:00
Brian Smith
a2bdbb60ec Remove unused cp_nistz256_mul_by_2.
Change-Id: I7fbe3effec27a18c5c42e6140df9ebd6229e06df
Reviewed-on: https://boringssl-review.googlesource.com/13020
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-01-13 01:28:33 +00:00
Brian Smith
cb42354ac3 Clarify x86 GCM asm implementation dispatching.
There is no AVX implementation for x86. Previously on x86 the code
checked to see if AVX and MOVBE are available, and if so, then it
uses the CLMUL implementation. Otherwise it fell back to the same
CLMUL implementation. Thus, there is no reason to check if AVX + MOVBE
are enabled on x86.

Change-Id: Id4983d5d38d6b3269a40e288bca6cc51d2d13966
Reviewed-on: https://boringssl-review.googlesource.com/13024
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-13 01:22:26 +00:00
Brian Smith
18a37a4211 Remove unused "pure" MMX x86 GCM implementation.
BoringSSL will always use the SSE version so this is all dead code.

Change-Id: I0f3b51ee29144b5c83d2553c92bebae901b6366f
Reviewed-on: https://boringssl-review.googlesource.com/13023
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-13 01:19:01 +00:00
Brian Smith
ac153bded3 Remove unused non-MMX/SSE GCM assembly code.
BoringSSL can assume that MMX, SSE, and SSE2 is always supported so
there is no need for a runtime check and there's no need for this
fallback code. Removing the code improves coverage analysis and shrinks
code size.

Change-Id: I782a1bae228f700895ada0bc56687e53cd02b5df
Reviewed-on: https://boringssl-review.googlesource.com/13022
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
2017-01-13 01:11:11 +00:00
David Benjamin
6a0888dd52 Save one call to |ecp_nistz256_from_mont| in |ecp_nistz256_get_affine|.
This re-applies 3f3358ac15 which was
reverted in c7fe3b9ac5 because the field
operations did not fully-reduce operands. This was fixed in
2f1482706fadf51610a529be216fde0721709e66.

Change-Id: I3913af4b282238dbc21044454324123f961a58af
Reviewed-on: https://boringssl-review.googlesource.com/12227
Reviewed-by: Adam Langley <agl@google.com>
2017-01-12 21:10:17 +00:00
David Benjamin
745745df03 Add SSL_CIPHER_is_static_RSA.
Change-Id: Id0013a2441da206b051a05a39aa13e4eca937e03
Reviewed-on: https://boringssl-review.googlesource.com/13109
Reviewed-by: Adam Langley <agl@google.com>
2017-01-12 18:37:19 +00:00
David Benjamin
5fc99c6603 There are no more MD5 ciphers.
The last one was an RC4 cipher and those are gone.

Change-Id: I3473937ff6f0634296fc75a346627513c5970ddb
Reviewed-on: https://boringssl-review.googlesource.com/13108
Reviewed-by: Adam Langley <agl@google.com>
2017-01-12 18:36:54 +00:00
Adam Langley
dcecdfd620 Fix a couple of missing spaces in comments.
Change-Id: If8b5dea31d7f37b3b33ea41e7a6a33240cb5ee5b
Reviewed-on: https://boringssl-review.googlesource.com/13121
Reviewed-by: Adam Langley <agl@google.com>
2017-01-12 18:35:25 +00:00
David Benjamin
1d6eeb3b85 Spellcheck our public headers.
Also fix some formatting.

Change-Id: I8fb1a95d4a55e40127433f0114fd08a82a4c3d41
Reviewed-on: https://boringssl-review.googlesource.com/13103
Reviewed-by: Adam Langley <agl@google.com>
2017-01-12 18:24:27 +00:00
David Benjamin
e3fbb36005 Test SSL_set_max_send_fragment.
This gives coverage over needing to fragment something over multiple
records.

Change-Id: I2373613608ef669358d48f4e12f68577fa5a40dc
Reviewed-on: https://boringssl-review.googlesource.com/13101
Reviewed-by: Adam Langley <agl@google.com>
2017-01-12 18:22:08 +00:00
David Benjamin
8b8d22c961 Parse PKCS#12 files more accurately.
Mercifully, PKCS#12 does not actually make ContentInfo and SafeBag
mutually recursive. The top-level object in a PKCS#12 is a SEQUENCE of
data or encrypted data ContentInfos. Their payloads are a SEQUENCE of
SafeBags (aka SafeContents).

SafeBag is a similar structure to ContentInfo but not identical (it has
attributes in it which we ignore) and actually carries the objects.
There is only recursion if the SafeContents bag type is used, which we
do not process.

This means we don't need to manage recursion depth. This also no longer
allows trailing data after the SEQUENCE and removes the comment about
NSS. The test file still passes, so I'm guessing something else was
going on?

Change-Id: I68e2f8a5cc4b339597429d15dc3588bd39267e0a
Reviewed-on: https://boringssl-review.googlesource.com/13071
Reviewed-by: Adam Langley <agl@google.com>
2017-01-12 16:56:05 +00:00
David Benjamin
2df010e4f4 Remove 'pivot element' from wNAF code.
Resolving the TODO here will be messier than the other implementations
but, to start with, remove this 'pivot element' thing. All that is just
to free some array contents without having to memset the whole thing to
zero.

Change-Id: Ifd6ee0b3815006d4f1f19c9db085cb842671c6dc
Reviewed-on: https://boringssl-review.googlesource.com/13057
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-01-12 16:45:23 +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
d261004048 Report TLS 1.3 as supporting secure renegotiation.
TLS 1.3 doesn't support renegotiation in the first place, but so callers
don't report TLS 1.3 servers as missing it, always report it as
(vacuously) protected against this bug.

BUG=chromium:680281

Change-Id: Ibfec03102b2aec7eaa773c331d6844292e7bb685
Reviewed-on: https://boringssl-review.googlesource.com/13046
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-01-11 22:19:17 +00:00
David Benjamin
7f539fa008 Handle overflow in ascii_to_ucs2.
Change-Id: Ie9a0039931a1a8d48a82c11ef5c58d6ee084ca4c
Reviewed-on: https://boringssl-review.googlesource.com/13070
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-11 01:27:18 +00:00
David Benjamin
9d0e7fb6e7 Rework PKCS{5,8,12} code.
Avoid the X509_ALGOR dependency entirely. The public API is still using
the legacy ASN.1 structures for now, but the conversions are lifted to
the API boundary. Once we resolve that and the OID table dependency,
this module will no longer block unshipping crypto/asn1 and friends from
Chromium.

This changes the calling convention around the two kinds of PBE suites
we support. Each PBE suite provides a free-form encrypt_init function to
setup an EVP_CIPHER_CTX and write the AlgorithmIdentifer to a CBB. It
then provides a common decrypt_init function which sets up an
EVP_CIPHER_CTX given a CBS of the parameter. The common encrypt code
determines how to call which encrypt_init function. The common decrypt
code parses the OID out of the AlgorithmIdentifer and then dispatches to
decrypt_init.

Note this means the encryption codepath no longer involves parsing back
out a AlgorithmIdentifier it just serialized. We don't have a good story
to access an already serialized piece of a CBB in progress (reallocs can
invalidate the pointer in a CBS), so it's easier to cut this step out
entirely.

Also note this renames the "PBES1" schemes from PKCS#5 to PKCS#12. This
makes it easier to get at the PKCS#12 key derivation hooks. Although
PKCS#12 claims these are variants of PKCS#5's PBES1, they're not very
related. PKCS#12 swaps out the key derivation and even defines its own
AlgorithmIdentifier parameter structure (identical to the PKCS#5 PBES1
one). The only thing of PBES1 that survives is the CBC mode padding
scheme, which is deep in EVP_CIPHER for us. (Of course, all this musing
on layering is moot because we don't implement non-PKCS#12 PBES1 schemes
anyway.)

This also moves some of the random API features (default iteration
count, default salt generation) out of the PBE suites and into the
common code.

BUG=54

Change-Id: Ie96924c73a229be2915be98eab680cadd17326db
Reviewed-on: https://boringssl-review.googlesource.com/13069
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-11 01:25:14 +00:00
David Benjamin
314d81420c Reimplement pkcs12_pbe_keyivgen with CBS.
BUG=54

Change-Id: Ie003a9635b33ad6f7e430684f0eb6975c613ebf3
Reviewed-on: https://boringssl-review.googlesource.com/13068
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-11 00:54:08 +00:00
David Benjamin
d1afc41869 Reimplement PKCS5_pbe_set with CBB.
BUG=54

Change-Id: I41bd43948140037c8e5c1b6502e1c882293befec
Reviewed-on: https://boringssl-review.googlesource.com/13067
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-11 00:51:52 +00:00
Adam Langley
2a25aae0f5 Ensure that CBB is |CBB_zero|ed before possibly calling |CBB_cleanup|.
Change-Id: Ic1f58f87c67104c8a51af59086a1bb1e5ccb0e5b
Reviewed-on: https://boringssl-review.googlesource.com/13084
Reviewed-by: Adam Langley <alangley@gmail.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-01-11 00:49:37 +00:00
David Benjamin
4fae069c00 Reimplement PKCS5_v2_PBE_keyivgen.
This gets us closer to decoupling from crypto/asn1.

BUG=54

Change-Id: I06ec04ed3cb47c2f56a94c6defa97398bfd0e013
Reviewed-on: https://boringssl-review.googlesource.com/13066
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-11 00:37:56 +00:00
David Benjamin
e464e81f89 Reimplement PKCS5_pbe2_set with CBB.
This is not quite an end state (it still outputs an X509_ALGOR, the way
the generated salt is fed into key derivation is odd, and it uses the
giant OID table), but replaces a large chunk of it.

BUG=54

Change-Id: I0a0cca13e44e6a09dfaf6aed3b357cb077dc46d1
Reviewed-on: https://boringssl-review.googlesource.com/13065
Reviewed-by: Adam Langley <alangley@gmail.com>
Commit-Queue: Adam Langley <alangley@gmail.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-01-11 00:34:25 +00:00