Commit Graph

1670 Commits

Author SHA1 Message Date
David Benjamin
e025f30507 Guard the _GNU_SOURCE #define.
It is hard to control what flags consumers may try to build us with.
Account for someone adding _GNU_SOURCE to the build line.

Change-Id: I4c931da70a9dccc89382ce9100c228c29d28d4bf
Reviewed-on: https://boringssl-review.googlesource.com/13621
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-02-03 22:15:09 +00:00
Adam Langley
5fa2cdf1ed Test SSE4.1 before using ChaCha20-Poly1305 asm.
This change guards the ChaCha20-Poly1305 asm on having SSE4.1. The
pinsrb instruction that it uses requires this, which I didn't notice,
and so this would fail on Core 2 and older chips.

BUG=chromium:688384

Change-Id: I177e3492782a1a9974b6df29d26fc4809009ad48
Reviewed-on: https://boringssl-review.googlesource.com/13586
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-02-03 16:46:26 +00:00
Adam Langley
772a5bed7d Reorder the X25519 ladderstep stack frame on x86-64.
The current X25519 assembly has a 352-byte stack frame and saves the
regsiters at the bottom. This means that the CFI information cannot be
represented in the “compact” form that MacOS seems to want to use (see
linked bug).

The stack frame looked like:

 360 CFA
 352 return address
 ⋮
 56  (296 bytes of scratch space)
 48  saved RBP
 40  saved RBX
 32  saved R15
 24  saved R14
 16  saved R13
 8   saved R12
 0   (hole left from 3f38d80b dropping the superfluous saving of R11)

Now it looks like:

 352 CFA
 344 return address
 336 saved RBP
 328 saved RBX
 320 saved R15
 312 saved R14
 304 saved R13
 296 saved R12
 ⋮
 0   (296 bytes of scratch space)

The bulk of the changes involve subtracting 56 from all the offsets to
RSP when working in the scratch space. This was done in Vim with:
  '<,'>s/\([1-9][0-9]*\)(%rsp)/\=submatch(1)-56."(%rsp)"/

BUG=176

Change-Id: I022830e8f896fe2d877015fa3ecfa1d073207679
Reviewed-on: https://boringssl-review.googlesource.com/13580
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-02-02 22:47:05 +00:00
David Benjamin
6342111c2e Remove BIO puts hooks.
These are unused. BIO_puts is implemented genericly.

Change-Id: Iecf1b6736291de8c48ce1adbb7401963a120d122
Reviewed-on: https://boringssl-review.googlesource.com/13366
Reviewed-by: Steven Valdez <svaldez@google.com>
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-02 22:22:08 +00:00
David Benjamin
5c9d411e14 Fix some compact unwind errors.
The Mac ld gets unhappy about "weird" unwind directives:

In chacha20_poly1305_x86_64.pl, $keyp is being pushed on the stack
(according to the comment) because it gets clobbered in the computation
somewhere. $keyp is %r9 which is not callee-saved (it's an argument
register), so we don't need to tag it with .cfi_offset.

In x25519-asm-x86_64.S, x25519_x86_64_mul saves %rdi on the stack.
However it too is not callee-saved (it's an argument register) and
should not have a .cfi_offset. %rdi also does not appear to be written
to anywhere in the function, so there's no need to save it at all.

(This does not resolve the "r15 is saved too far from return address"
errors. Just the non-standard register ones.)

BUG=176

Change-Id: I53f3f7db3d1745384fb47cb52cd6536aabb5065e
Reviewed-on: https://boringssl-review.googlesource.com/13560
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-02-02 22:05:06 +00:00
Adam Langley
3f38d80b2f Add CFI information to the x86-64 X25519 asm.
This change serves to check that all our consumers can process assembly
with CFI directives in it.

For the first change I picked a file that's not perlasm to keep things
slightly simplier, but that might have been a mistake:

DJB's tooling always aligns the stack to 32 bytes and it's not possible
to express this in DWARF format (without using a register to store the
old stack pointer).

Since none of the functions here appear to care about that alignment, I
removed it from each of them. I also trimmed the set of saved registers
where possible and used the redzone for functions that didn't need much
stack.

Overall, this appears to have slightly improved the performance (by
about 0.7%):

Before:

Did 46000 Curve25519 base-point multiplication operations in 3023288us (15215.2 ops/sec)
Did 46000 Curve25519 arbitrary point multiplication operations in 3017315us (15245.3 ops/sec)
Did 46000 Curve25519 base-point multiplication operations in 3015346us (15255.3 ops/sec)
Did 46000 Curve25519 arbitrary point multiplication operations in 3018609us (15238.8 ops/sec)
Did 46000 Curve25519 base-point multiplication operations in 3019004us (15236.8 ops/sec)
Did 46000 Curve25519 arbitrary point multiplication operations in 3013135us (15266.5 ops/sec)

After:

Did 46000 Curve25519 base-point multiplication operations in 3007659us (15294.3 ops/sec)
Did 47000 Curve25519 arbitrary point multiplication operations in 3054202us (15388.6 ops/sec)
Did 46000 Curve25519 base-point multiplication operations in 3008714us (15288.9 ops/sec)
Did 46000 Curve25519 arbitrary point multiplication operations in 3004740us (15309.1 ops/sec)
Did 46000 Curve25519 base-point multiplication operations in 3009140us (15286.8 ops/sec)
Did 47000 Curve25519 arbitrary point multiplication operations in 3057518us (15371.9 ops/sec)

Change-Id: I31df11c45b2ea0bf44dde861d52c27f848331691
Reviewed-on: https://boringssl-review.googlesource.com/13200
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-31 17:55:19 +00:00
Adam Langley
8c2480f740 Push to error queue in |EVP_PKEY_CTX_ctrl| for wrong keytype.
Change-Id: I81a94be94103d3c763cd6b2c1b8196300808c6fe
Reviewed-on: https://boringssl-review.googlesource.com/13386
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-30 19:16:05 +00:00
Brian Smith
360a4c2616 chacha20_poly1305_x86_64.pl: Use NASM-compatible syntax for |ldea|.
Cargo-cult the way other Perlasm scripts do it.

Change-Id: I86aaf725e41b601f24595518a8a6bc481fa0c7fc
Reviewed-on: https://boringssl-review.googlesource.com/13382
Reviewed-by: Adam Langley <agl@google.com>
2017-01-27 23:17:13 +00:00
Brian Smith
357a9f23fe chacha20_poly1305_x86_64.pl: Use |imulq| instead of |imul|.
Perlasm requires the size suffix when targeting NASM and Yasm; without
it, the resulting .asm file has |imu| instead of |imul|.

Change-Id: Icb95b8c0b68cf4f93becdc1930dc217398f56bec
Reviewed-on: https://boringssl-review.googlesource.com/13381
Reviewed-by: Adam Langley <agl@google.com>
2017-01-27 23:16:52 +00:00
Brian Smith
3416d28a57 chacha20_poly1305_x86_64.pl: Escape command line args like other PerlAsm scripts.
Use the same quoting used in other files so that this file can be built
the same way as other files on platforms that require the other kind of
quoting.

Change-Id: I808769bf014fbfe526fedcdc1e1f617b3490d03b
Reviewed-on: https://boringssl-review.googlesource.com/13380
Reviewed-by: Adam Langley <agl@google.com>
2017-01-27 23:16:27 +00:00
Adam Langley
3b3b62f39c X509_parse_from_buffer: reject massive certificates.
Otherwise we could pass a negative value into |d2i_X509|.

Change-Id: I52a35dd9648269094110b69eddd7667a56ec8253
Reviewed-on: https://boringssl-review.googlesource.com/13363
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-01-27 16:21:16 +00:00
David Benjamin
41a26e819f Remove buffer BIOs.
These are completely unused, but for BIO_set_write_buffer_size which is
in some (unreachable) nginx codepath. Keep that around so nginx
continues to build, but otherwise delete it.

Change-Id: I1a50a4f7b23e5fdbc7f132900ecacd74e8775a7f
Reviewed-on: https://boringssl-review.googlesource.com/13362
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-26 23:09:10 +00:00
Adam Langley
71e4aff654 Use |extern| when referencing assembly functions from C.
I don't think that this makes a difference, but it's a little more
consistent with what we've done previously. I made this change when
trying to get the DFSAN build working, although that issue turned out to
be unrelated.

Change-Id: I21041689c5df70ca2bddf33065d687763af8c3c7
Reviewed-on: https://boringssl-review.googlesource.com/13361
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-26 22:12:09 +00:00
David Benjamin
d103616db1 bn/asm/x86_64-mont5.pl: fix carry bug in bn_sqr8x_internal.
CVE-2017-3732

(Imported from upstream's 3f4bcf5bb664b47ed369a70b99fac4e0ad141bb3 and
3e7a496307ab1174c1f8f64eed4454c1c9cde1a8.)

Change-Id: I40255fdf4184e3b919758a72c3d3a7486d91ff65
Reviewed-on: https://boringssl-review.googlesource.com/13360
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-26 18:29:44 +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
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
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
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
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
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
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
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
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
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
David Benjamin
ac83bea85d Trim dead code from PKCS#5 PBE2 bits.
Many of these parameters are constants.

Change-Id: I148dbea0063e478a132253f4e9dc71d5d20320c2
Reviewed-on: https://boringssl-review.googlesource.com/13064
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:13:59 +00:00
David Benjamin
9ba19b8e88 Test we can round-trip PKCS8_{encrypt,decrypt}.
This is a very basic test, but it's something.

Change-Id: Ic044297e97ce5719673869113ce581de4621ebbd
Reviewed-on: https://boringssl-review.googlesource.com/13061
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-10 23:49:37 +00:00
David Benjamin
a5eee1c7f3 Decouple EVP_get_digestbyobj from the giant OID table.
libcrypto can now be split in two, with everything that depends on
crypto/asn1 in a separate library. That said, Chromium still needs
crypto/pkcs8 to be implemented with CBS/CBB first. (Also libssl and
anything which uses X509* directly.)

BUG=54

Change-Id: Iec976ae637209882408457e94a1eb2465bce8d56
Reviewed-on: https://boringssl-review.googlesource.com/13059
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-10 23:45:00 +00:00
David Benjamin
8f3f6be0d5 Const-correct the PKCS8 salt parameter.
Change-Id: Iad9b0898b3a602fc2e554c4fd59a599c61cd8ef7
Reviewed-on: https://boringssl-review.googlesource.com/13063
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-10 23:42:10 +00:00
David Benjamin
35349e9fac Unexport PKCS5 functions.
They're not called externally. Unexporting these will make it easier to
rewrite the PKCS{5,8,12} code to use CBS/CBB rather than X509_ALGOR.
Getting rid of those callers in Chromium probably won't happen for a
while since it's in our on-disk formats. (And a unit test for some NSS
client cert glue uses it.)

BUG=54

Change-Id: Id4148a2ad567484782a6e0322b68dde0619159fc
Reviewed-on: https://boringssl-review.googlesource.com/13062
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-10 23:41:44 +00:00
David Benjamin
20dbc1ff20 Import some PKCS8_decrypt test vectors from Chromium.
This includes examples with both the NULL and empty passwords, thanks to
PKCS#12's password ambiguity.

Change-Id: Iae31840c1d31929fa9ac231509acaa80ef5b74bb
Reviewed-on: https://boringssl-review.googlesource.com/13060
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
2017-01-10 23:40:54 +00:00
Matthew Braithwaite
e2c083dfd6 ASN1_UTCTIME_print: fix, comment, test.
The motiviation is that M2Crypto passes an ASN1_GENERALIZEDTIME to
this function.  This is not distinct from ASN1_UTCTIME (both are
asn1_string_st), but ASN1_GENERALIZEDTIME uses a 4-digit year in its
string representation, whereas ASN1_UTCTIME uses a 2-digit year.

ASN1_UTCTIME_print previously did not return an error on such inputs.
So, stricten (?) the function, ensuring that it checks for trailing
data, and rejects values that are invalid for their place.  Along the
way, clean it up and add tests.

Change-Id: Ia8298bed573f2acfdab96638ea69c78b5bba4e4b
Reviewed-on: https://boringssl-review.googlesource.com/13082
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-10 23:32:12 +00:00
Rob Sloan
45573cc04d Add Little-endian BIGNUM conversions
Towards an eventual goal of opaquifying BoringSSL structs, we want
our consumers -- in this case, Android's libcore -- to not directly
manipulate BigNums; and it would be convenient for them if we would
perform the appropriate gymnastics to interpret little-endian byte
streams.

It also seems a priori a bit strange to have only big-endian varieties
of BN byte-conversions.

This CL provides little-endian equivalents of BN_bn2bin_padded
and BN_bin2bn.

BUG=97
Change-Id: I0e92483286def86d9bd71a46d6a967a3be50f80b
Reviewed-on: https://boringssl-review.googlesource.com/12641
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-06 18:20:09 +00:00