Commit Graph

173 Commits

Author SHA1 Message Date
David Benjamin
053a8f728e Bound EVP_tls_cbc_digest_record at runtime.
This is redundant because these "AEAD"s are not meant to be used outside
of TLS, but since we've moved them into their own layer, they should
check internally.

Change-Id: Ieb3541b2e494902527c2bb56a816cef620cb237b
Reviewed-on: https://boringssl-review.googlesource.com/14310
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-03-30 16:20:55 +00:00
David Benjamin
8d979e5430 Push variable declarations in EVP_tls_cbc_digest_record.
This makes it a bit easier to see what is what.

Change-Id: I0f73f6ffa84bd30de3efcbf2bd34e1d3a889d1ee
Reviewed-on: https://boringssl-review.googlesource.com/14309
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-03-30 16:20:36 +00:00
David Benjamin
d1c0de6fe0 Add size_t variants of constant-time functions.
These will be used in follow-up commits. The _s names are taken from
upstream, to ease importing code. I've also promoted the CONSTTIME_*
macros from the test. None of them are really necessary except
~0u cannot substitute for CONSTTIME_TRUE_S on 64-bit platforms, so
having the macros seems safer.

Once everything is converted, I expect the unsigned versions can be
removed, so I've made the _8 and _int functions act on size_t rather
than unsigned. The users of these functions basically only believe that
array indices and bytes exist.

BUG=22

Change-Id: I987bfb0c708dc726a6f2afcb05b6619bbd600564
Reviewed-on: https://boringssl-review.googlesource.com/14306
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-03-30 15:38:43 +00:00
Adam Langley
be2ee342d3 Update AES-GCM-SIV to match revision four of the draft.
This change updates AES-GCM-SIV to match revision four of the draft[1].
See [2] for the reasons behind the changes.

[1] https://tools.ietf.org/html/draft-irtf-cfrg-gcmsiv-04
[2] https://www.ietf.org/mail-archive/web/cfrg/current/msg08895.html

Change-Id: Icacfefbd2f470186051551ea227c9d6c6dd6e786
Reviewed-on: https://boringssl-review.googlesource.com/13973
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-26 18:37:53 +00:00
David Benjamin
ec1d9637e1 Fix crashes in EVP_CIPHER if cipher_data was not allocated.
(Imported from upstream's 1222d273d36277f56c3603a757240c386d55f318.)

We'd fixed half of these, but the other half are probably unreachable
from code that ran under malloc tests, so we never noticed. It's
puzzling why upstream did both this and
166e365ed84dfabec3274baf8a9ef8aa4e677891. It seems you only need one of
them.

Change-Id: I08074358134180c6661600b66958ba861e7726fb
Reviewed-on: https://boringssl-review.googlesource.com/13832
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-17 01:11:14 +00:00
Brian Smith
644539191b chacha20_poly1305_x86_64.pl: Suppress Yasm non-local label warnings.
Before, attempting to build the code using Yasm as the assembler would
result in warnings like this:

    warning : no non-local label before `.chacha20_consts'

Precede the local labels with a non-local label to suppress these
warnings.

It isn't clear why these labels are defined as local labels instead of
regular labels.  Making them non-local may be a better idea.

For reference, Yasm's interpretation of local labels is described
succinctly at
https://www.tortall.net/projects/yasm/manual/html/nasm-local-label.html.

Change-Id: Ifc92de7fd7379859fe33f1137ab20b6ec282cd0b
Reviewed-on: https://boringssl-review.googlesource.com/13384
Reviewed-by: Adam Langley <agl@google.com>
2017-02-09 18:05:41 +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
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
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
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
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
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
David Benjamin
14e18ca257 Fix AES-GCM-SIV on large inputs.
This was noticed by observing we had one line of missing test coverage
in polyval.c. CRYPTO_POLYVAL_update_blocks acts 32 blocks at a time and
all existing test vectors are smaller than that.

Test vector obtained by just picking random values and seeing what our
existing implementation did if I modified CRYPTO_POLYVAL_update_blocks
to consume many more blocks at a time. Then I fixed the bug and ensured
the answer was still the same.

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

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

BUG=23

Change-Id: I95f42b23e92945af0e681264fffaf578e7f8465e
Reviewed-on: https://boringssl-review.googlesource.com/12928
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-12-21 20:34:47 +00:00
David Benjamin
703aa16003 Import a test vector from upstream.
The original bug only affected their big-endian code which we don't
have, but import the test vector anyway. Imported from upstream's
b47f116b1e02d20b1f8a7488be5a04f7cf5bc712.

Change-Id: I349e41d87006533da0e18c948f9cc7dd15f42a44
Reviewed-on: https://boringssl-review.googlesource.com/12820
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>
2016-12-14 17:48:36 +00:00
David Benjamin
aac1e2dd73 Remove the remaining bssl::Main wrappers.
We've taken to writing bssl::UniquePtr in full, so it's not buying
us much.

Change-Id: Ia2689366cbb17282c8063608dddcc675518ec0ca
Reviewed-on: https://boringssl-review.googlesource.com/12628
Reviewed-by: David Benjamin <davidben@google.com>
2016-12-08 00:54:17 +00:00
Adam Langley
df447ba3a9 Add generic AES-GCM-SIV support.
AES-GCM-SIV is an AEAD with nonce-misuse resistance. It can reuse
hardware support for AES-GCM and thus encrypt at ~66% the speed, and
decrypt at 100% the speed, of AES-GCM.

See https://tools.ietf.org/html/draft-irtf-cfrg-gcmsiv-02

This implementation is generic, not optimised, and reuses existing AES
and GHASH support as much as possible. It is guarded by !OPENSSL_SMALL,
at least for now.

Change-Id: Ia9f77b256ef5dfb8588bb9ecfe6ee0e827626f57
Reviewed-on: https://boringssl-review.googlesource.com/12541
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-12-07 00:13:50 +00:00
David Benjamin
d8a268261d Simplify rotate_offset computation in EVP_tls_cbc_copy_mac.
Rather than Barrett reduction, we can just sample rotate_offset at the
point where we save the first byte of the MAC. Thanks to Andy Polyakov
for the idea in
https://github.com/openssl/openssl/pull/1027#issuecomment-263218179

Change-Id: If3a7c2d176406fc332ac512648e6f5ef4dc8b7e5
Reviewed-on: https://boringssl-review.googlesource.com/12475
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-12-01 22:02:03 +00:00
David Benjamin
a4ddb6e212 Remove unnecessary constant-time operation.
j and md_size are public values, so this can just be done directly. (If
they weren't, we'd have worse problems.) This makes the loop look the
same as the rotation loop below.

Change-Id: Ic75550ad4e40b2015668cb12c26ca2d20bd285b6
Reviewed-on: https://boringssl-review.googlesource.com/12474
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-12-01 21:54:45 +00:00
David Benjamin
029cce5cfd Tidy up EVP_tls_cbc_copy_mac a little.
Some declarations can be moved closer to use, etc.

Change-Id: Ifa9a51ad77639b94020b15478af234c82466390f
Reviewed-on: https://boringssl-review.googlesource.com/12473
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-12-01 21:53:29 +00:00
Brian Smith
0d211bdc4b Clarify the scope & initialization of |data_len| in |aead_tls_open|.
Neither branch of the |if| statement is expected to touch |data_len|.
Clarify this by moving |data_len| after the |if| statement.

Change-Id: Ibbc81e5b0c006882672df18442a6e7987064ca6d
Reviewed-on: https://boringssl-review.googlesource.com/11880
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-10-28 16:44:39 +00:00
David Benjamin
b1133e9565 Fix up macros.
Macros need a healthy dose of parentheses to avoid expression-level
misparses. Most of this comes from the clang-tidy CL here:
https://android-review.googlesource.com/c/235696/

Also switch most of the macros to use do { ... } while (0) to avoid all
the excessive comma operators and statement-level misparses.

Change-Id: I4c2ee51e347d2aa8c74a2d82de63838b03bbb0f9
Reviewed-on: https://boringssl-review.googlesource.com/11660
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-10-18 18:28:23 +00:00
David Benjamin
97227dc52d Replace keywrap AEADs with upstream's APIs.
This finally removes the last Android hack. Both Chromium and Android
end up needing this thing (Chromium needs it for WebCrypto but currently
uses the EVP_AEAD version and Android needs it by way of
wpa_supplicant).

On the Android side, the alternative is we finish upstream's
NEED_INTERNAL_AES_WRAP patch, but then it just uses its own key-wrap
implementation. This seems a little silly, considering we have a version
of key-wrap under a different API anyway.

It also doesn't make much sense to leave the EVP_AEAD API around if we
don't want people to use it and Chromium's the only consumer. Remove it
and I'll switch Chromium to the new---er, old--- APIs next roll.

Change-Id: I23a89cda25bddb6ac1033e4cd408165f393d1e6c
Reviewed-on: https://boringssl-review.googlesource.com/11410
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-10-04 01:37:31 +00:00
Adam Langley
4467e59bc8 Add PPC64LE assembly for AES-GCM.
This change adds AES and GHASH assembly from upstream, with the aim of
speeding up AES-GCM.

The PPC64LE assembly matches the interface of the ARMv8 assembly so I've
changed the prefix of both sets of asm functions to be the same
("aes_hw_").

Otherwise, the new assmebly files and Perlasm match exactly those from
upstream's c536b6be1a (from their master branch).

Before:
Did 1879000 AES-128-GCM (16 bytes) seal operations in 1000428us (1878196.1 ops/sec): 30.1 MB/s
Did 61000 AES-128-GCM (1350 bytes) seal operations in 1006660us (60596.4 ops/sec): 81.8 MB/s
Did 11000 AES-128-GCM (8192 bytes) seal operations in 1072649us (10255.0 ops/sec): 84.0 MB/s
Did 1665000 AES-256-GCM (16 bytes) seal operations in 1000591us (1664016.6 ops/sec): 26.6 MB/s
Did 52000 AES-256-GCM (1350 bytes) seal operations in 1006971us (51640.0 ops/sec): 69.7 MB/s
Did 8840 AES-256-GCM (8192 bytes) seal operations in 1013294us (8724.0 ops/sec): 71.5 MB/s

After:
Did 4994000 AES-128-GCM (16 bytes) seal operations in 1000017us (4993915.1 ops/sec): 79.9 MB/s
Did 1389000 AES-128-GCM (1350 bytes) seal operations in 1000073us (1388898.6 ops/sec): 1875.0 MB/s
Did 319000 AES-128-GCM (8192 bytes) seal operations in 1000101us (318967.8 ops/sec): 2613.0 MB/s
Did 4668000 AES-256-GCM (16 bytes) seal operations in 1000149us (4667304.6 ops/sec): 74.7 MB/s
Did 1202000 AES-256-GCM (1350 bytes) seal operations in 1000646us (1201224.0 ops/sec): 1621.7 MB/s
Did 269000 AES-256-GCM (8192 bytes) seal operations in 1002804us (268247.8 ops/sec): 2197.5 MB/s

Change-Id: Id848562bd4e1aa79a4683012501dfa5e6c08cfcc
Reviewed-on: https://boringssl-review.googlesource.com/11262
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-09-27 18:43:20 +00:00
David Benjamin
c446ce5294 Add EVP_AEAD_CTX_aead.
Code acting generically on an EVP_AEAD_CTX may wish to get at the
underlying EVP_AEAD.

Change-Id: I9cc905522ba76402bda4c255aa1488158323b02c
Reviewed-on: https://boringssl-review.googlesource.com/11085
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Matt Braithwaite <mab@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-09-17 01:06:40 +00:00
Matthew Braithwaite
8aaa9e12c2 Remove RC4 from TLS for real.
This withdraws support for -DBORINGSSL_ENABLE_RC4_TLS, and removes the
RC4 AEADs.

Change-Id: I1321b76bfe047d180743fa46d1b81c5d70c64e81
Reviewed-on: https://boringssl-review.googlesource.com/10940
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-09-16 03:06:36 +00:00
David Benjamin
54091230cd Use C99 for size_t loops.
This was done just by grepping for 'size_t i;' and 'size_t j;'. I left
everything in crypto/x509 and friends alone.

There's some instances in gcm.c that are non-trivial and pulled into a
separate CL for ease of review.

Change-Id: I6515804e3097f7e90855f1e7610868ee87117223
Reviewed-on: https://boringssl-review.googlesource.com/10801
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-09-12 19:44:24 +00:00
David Benjamin
c763a40101 Replace CBC_MAC_ROTATE_IN_PLACE with an N lg N rotation.
Really the only thing we should be doing with these ciphers is hastening
their demise, but it was the weekend and this seemed like fun.

EVP_tls_cbc_copy_mac needs to rotate a buffer by a secret amount. (It
extracts the MAC, but rotated.) We have two codepaths for this. If
CBC_MAC_ROTATE_IN_PLACE is defined (always on), we make some assumptions
abuot cache lines, play games with volatile, and hope that doesn't leak
anything. Otherwise, we do O(N^2) work to constant-time select the
rotation incidences.

But we can do O(N lg N). Rotate by powers of two and constant-time
select by the offset's bit positions. (Handwaivy lower-bound: an array
position has N possible values, so, armed with only a constant-time
select, we need O(lg N) work to resolve it. There's N array positions,
so O(N lg N).)

A microbenchmark of EVP_tls_cbc_copy_mac shows this is 27% faster than
the old one, but still 32% slower than the in-place version.

in-place:
Did 15724000 CopyFromMAC operations in 20000744us (786170.8 ops/sec)
N^2:
Did 8443000 CopyFromMAC operations in 20001582us (422116.6 ops/sec)
N lg N:
Did 10718000 CopyFromMAC operations in 20000763us (535879.6 ops/sec)

This results in the following the CBC ciphers. I measured
AES-128-CBC-SHA1 and AES-256-CBC-SHA384 which are, respectively, the
cipher where the other bits are the fastest and the cipher where N is
largest.

in-place:
Did 2634000 AES-128-CBC-SHA1 (16 bytes) open operations in 10000739us (263380.5 ops/sec): 4.2 MB/s
Did 1424000 AES-128-CBC-SHA1 (1350 bytes) open operations in 10002782us (142360.4 ops/sec): 192.2 MB/s
Did 531000 AES-128-CBC-SHA1 (8192 bytes) open operations in 10002460us (53086.9 ops/sec): 434.9 MB/s
N^2:
Did 2529000 AES-128-CBC-SHA1 (16 bytes) open operations in 10001474us (252862.7 ops/sec): 4.0 MB/s
Did 1392000 AES-128-CBC-SHA1 (1350 bytes) open operations in 10006659us (139107.4 ops/sec): 187.8 MB/s
Did 528000 AES-128-CBC-SHA1 (8192 bytes) open operations in 10001276us (52793.3 ops/sec): 432.5 MB/s
N lg N:
Did 2531000 AES-128-CBC-SHA1 (16 bytes) open operations in 10003057us (253022.7 ops/sec): 4.0 MB/s
Did 1390000 AES-128-CBC-SHA1 (1350 bytes) open operations in 10003287us (138954.3 ops/sec): 187.6 MB/s
Did 531000 AES-128-CBC-SHA1 (8192 bytes) open operations in 10002448us (53087.0 ops/sec): 434.9 MB/s

in-place:
Did 1249000 AES-256-CBC-SHA384 (16 bytes) open operations in 10001767us (124877.9 ops/sec): 2.0 MB/s
Did 879000 AES-256-CBC-SHA384 (1350 bytes) open operations in 10009244us (87818.8 ops/sec): 118.6 MB/s
Did 344000 AES-256-CBC-SHA384 (8192 bytes) open operations in 10025897us (34311.1 ops/sec): 281.1 MB/s
N^2:
Did 1072000 AES-256-CBC-SHA384 (16 bytes) open operations in 10008090us (107113.3 ops/sec): 1.7 MB/s
Did 780000 AES-256-CBC-SHA384 (1350 bytes) open operations in 10007787us (77939.3 ops/sec): 105.2 MB/s
Did 333000 AES-256-CBC-SHA384 (8192 bytes) open operations in 10016332us (33245.7 ops/sec): 272.3 MB/s
N lg N:
Did 1168000 AES-256-CBC-SHA384 (16 bytes) open operations in 10007671us (116710.5 ops/sec): 1.9 MB/s
Did 836000 AES-256-CBC-SHA384 (1350 bytes) open operations in 10001536us (83587.2 ops/sec): 112.8 MB/s
Did 339000 AES-256-CBC-SHA384 (8192 bytes) open operations in 10018522us (33837.3 ops/sec): 277.2 MB/s

TLS CBC performance isn't as important as it was before, and the costs
aren't that high, so avoid making assumptions about cache lines. (If we
care much about CBC open performance, we probably should get the malloc
out of EVP_tls_cbc_digest_record at the end.)

Change-Id: Ib8d8271be4b09e5635062cd3b039e1e96f0d9d3d
Reviewed-on: https://boringssl-review.googlesource.com/11003
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-09-12 19:27:25 +00:00
David Benjamin
f0e935d7ce Fold stack-allocated types into headers.
Now that we have the extern "C++" trick, we can just embed them in the
normal headers. Move the EVP_CIPHER_CTX deleter to cipher.h and, in
doing so, take away a little bit of boilerplate in defining deleters.

Change-Id: I4a4b8d0db5274a3607914d94e76a38996bd611ec
Reviewed-on: https://boringssl-review.googlesource.com/10804
Reviewed-by: Matt Braithwaite <mab@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-09-07 21:50:05 +00:00
Steven Valdez
cb96654404 Adding ARRAY_SIZE macro for getting the size of constant arrays.
Change-Id: Ie60744761f5aa434a71a998f5ca98a8f8b1c25d5
Reviewed-on: https://boringssl-review.googlesource.com/10447
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-08-19 19:30:39 +00:00
Martin Kreichgauer
19d5cf86de Move remaining ScopedContext types out of scoped_types.h
Change-Id: I7d1fa964f0d9817db885cd43057a23ec46f21702
Reviewed-on: https://boringssl-review.googlesource.com/10240
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-08-11 01:15:45 +00:00
David Benjamin
3f26a49eb6 Fix up EVP_tls_cbc_remove_padding's calling convention.
The old one was rather confusing. Switch to returning 1/0 for whether
the padding is publicly invalid and then add an output argument which
returns a constant_time_eq-style boolean.

Change-Id: Ieba89d352faf80e9bcea993b716f4b2df5439d4b
Reviewed-on: https://boringssl-review.googlesource.com/10222
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>
2016-08-10 03:52:48 +00:00
David Benjamin
65d74e4d76 Add better TLS CBC mode tests.
Add the following cases:

- Maximal padding

- Maximal padding with each possible byte position wrong.

- When the input is not publicly too short to find a MAC, but the
  unpadded value is too short. (This tests that
  EVP_tls_cbc_remove_padding and EVP_tls_cbc_copy_mac coordinate
  correctly. EVP_tls_cbc_remove_padding promises to also consider it
  invalid padding if there is no room for a MAC.)

Change-Id: I8fe18121afb915e579a8236d0e3ef354f1f835bc
Reviewed-on: https://boringssl-review.googlesource.com/10182
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>
2016-08-10 01:06:24 +00:00
Martin Kreichgauer
14343935b5 Start removing scoped_types.h
Initial stab at moving contents of scoped_types.h into
include/openssl/c++ and into the |bssl| namespace.

Started with one file. Will do the remaining ones once this looks good.

Change-Id: I51e2f7c1acbe52d508f1faee7740645f91f56386
Reviewed-on: https://boringssl-review.googlesource.com/9175
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-08-09 17:21:12 +00:00
David Benjamin
22edd87755 Resolve a small handful of size_t truncation warnings.
This is very far from all of it, but I did some easy ones before I got
bored. Snapshot the progress until someone else wants to continue this.

BUG=22

Change-Id: I2609e9766d883a273e53e01a75a4b1d4700e2436
Reviewed-on: https://boringssl-review.googlesource.com/9132
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-08-05 19:12:31 +00:00
Adam Langley
10f97f3bfc Revert "Move C++ helpers into |bssl| namespace."
This reverts commit 09feb0f3d9.

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

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

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

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

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

Change-Id: I3207e29474dc5fcc344ace43119df26dae04eabb
Reviewed-on: https://boringssl-review.googlesource.com/8730
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-11 23:04:52 +00:00
David Benjamin
a353cdb671 Wrap MSVC-only warning pragmas in a macro.
There's a __pragma expression which allows this. Android builds us Windows with
MinGW for some reason, so we actually do have to tolerate non-MSVC-compatible
Windows compilers. (Clang for Windows is much more sensible than MinGW and
intentionally mimicks MSVC.)

MinGW doesn't understand MSVC's pragmas and warns a lot. #pragma warning is
safe to suppress, so wrap those to shush them. This also lets us do away with a
few ifdefs.

Change-Id: I1f5a8bec4940d4b2d947c4c1cc9341bc15ec4972
Reviewed-on: https://boringssl-review.googlesource.com/8236
Reviewed-by: Adam Langley <agl@google.com>
2016-06-09 21:29:36 +00:00
David Benjamin
2446db0f52 Require in == out for in-place encryption.
While most of OpenSSL's assembly allows out < in too, some of it doesn't.
Upstream seems to not consider this a problem (or, at least, they're failing to
make a decision on whether it is a problem, so we should assume they'll stay
their course). Accordingly, require aliased buffers to exactly align so we
don't have to keep chasing this down.

Change-Id: I00eb3df3e195b249116c68f7272442918d7077eb
Reviewed-on: https://boringssl-review.googlesource.com/8231
Reviewed-by: Adam Langley <agl@google.com>
2016-06-09 19:49:03 +00:00
Adam Langley
adf27430ef Be consistent about 𝑥_tests.txt
Some files were named 𝑥_test.txt and some 𝑥_tests.txt. This change
unifies around the latter.

Change-Id: Id6f29bad8b998f3c3466655097ef593f7f18f82f
Reviewed-on: https://boringssl-review.googlesource.com/8150
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-06 15:57:46 +00:00
Adam Langley
8107e92a1a Add a comment with an SMT verification of the Barrett reductions.
Change-Id: I32dc13b16733fc09e53e3891ca68f51df6c1624c
Reviewed-on: https://boringssl-review.googlesource.com/7850
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-04 17:51:10 +00:00
David Benjamin
204dea8dae Fix encrypt overflow
An overflow can occur in the EVP_EncryptUpdate function. If an attacker is
able to supply very large amounts of input data after a previous call to
EVP_EncryptUpdate with a partial block then a length check can overflow
resulting in a heap corruption.

Following an analysis of all OpenSSL internal usage of the
EVP_EncryptUpdate function all usage is one of two forms.

The first form is like this:
EVP_EncryptInit()
EVP_EncryptUpdate()

i.e. where the EVP_EncryptUpdate() call is known to be the first called
function after an EVP_EncryptInit(), and therefore that specific call
must be safe.

The second form is where the length passed to EVP_EncryptUpdate() can be seen
from the code to be some small value and therefore there is no possibility of
an overflow. [BoringSSL: We also have code that calls EVP_CIPHER functions by
way of the TLS/SSL3 "AEADs". However, there we know the inputs are bounded by
2^16.]

Since all instances are one of these two forms, I believe that there can
be no overflows in internal code due to this problem.

It should be noted that EVP_DecryptUpdate() can call EVP_EncryptUpdate()
in certain code paths. Also EVP_CipherUpdate() is a synonym for
EVP_EncryptUpdate(). Therefore I have checked all instances of these
calls too, and came to the same conclusion, i.e. there are no instances
in internal usage where an overflow could occur.

This could still represent a security issue for end user code that calls
this function directly.

CVE-2016-2106

Issue reported by Guido Vranken.

(Imported from upstream's 3ab937bc440371fbbe74318ce494ba95021f850a.)

Change-Id: Iabde896555c39899c7f0f6baf7a163a7b3c2f3d6
Reviewed-on: https://boringssl-review.googlesource.com/7845
Reviewed-by: Adam Langley <agl@google.com>
2016-05-03 16:43:12 +00:00