Commit Graph

3666 Commits

Author SHA1 Message Date
David Benjamin
48891ad07c Simplify BoGo's TLS 1.3 key derivation.
finishedHash should keep a running secret and incorporate entropy as is
available.

Change-Id: I2d245897e7520b2317bc0051fa4d821c32eeaa10
Reviewed-on: https://boringssl-review.googlesource.com/12586
Reviewed-by: Nick Harper <nharper@chromium.org>
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-05 18:45:09 +00:00
David Benjamin
0d1faefdde Also add util/bot/golang to .gitignore.
I missed this one.

Change-Id: I642fb5878568870743727579126f63246ff179c5
Reviewed-on: https://boringssl-review.googlesource.com/12580
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-12-02 23:39:35 +00:00
Adam Langley
c629e8b688 Split CRYPTO_ghash_init from CRYPTO_gcm128_init.
CRYPTO_ghash_init exposes the (often hardware accelerated) internals for
evaluating GHASH. These can be used for evaluating POLYVAL[1] on
platforms where we don't have dedicated code for it.

[1] https://tools.ietf.org/html/draft-irtf-cfrg-gcmsiv-02#section-3

Change-Id: Ida49ce4911f8657fa384b0bca968daa2ac6b26c1
Reviewed-on: https://boringssl-review.googlesource.com/12478
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-02 21:09:11 +00:00
Adam Langley
a00cafc50c Drop H (the key) from the GCM context.
The key is only needed during initialisation because after that point it
is implicit in the table of powers. So no need to keep it around. There
was a non-specific “haunted house” comment about not changing this, but
I've successfully tested with all the assembly versions so I think that
comment is no longer true.

Change-Id: Id110156afb528904f114d9a4ff2440e03a1a69b8
Reviewed-on: https://boringssl-review.googlesource.com/12477
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-12-02 20:59:47 +00:00
Adam Langley
e8bbc6cf6c Assume little-endian in GCM code.
The GCM code has lots of cases of big-endian support left over from
OpenSSL. Since we don't support big-endian systems, drop that code.

Change-Id: I28eb95a9c235c6f705a145fbea72e7569dad2c70
Reviewed-on: https://boringssl-review.googlesource.com/12476
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>
2016-12-02 20:46:35 +00:00
David Benjamin
0ec5639092 Don't extract archives if unchanged.
This should shave 20% (40 seconds) off our Windows cycle times, going by
the graphs. It's 15% off our Linux ones, but that 15% is only 11
seconds.

Change-Id: I077c3924c722d597f66fc6dec72932ed0c81660a
Reviewed-on: https://boringssl-review.googlesource.com/12562
Reviewed-by: Adam Langley <agl@google.com>
2016-12-02 18:36:32 +00:00
David Benjamin
65241cf555 Add util/bot files to .gitignore.
bot_update does a git clean -dff before each run, so we were
redownloading all the utilities on each run. This should make the bots
only download them when the change. (Chromium's setup is similar.)

Change-Id: I7eb83217761ceabe58b5480242a7df93d9bfaa52
Reviewed-on: https://boringssl-review.googlesource.com/12561
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-12-02 18:35:32 +00:00
Adam Langley
f18ad089f1 tool: don't generate negative serial numbers.
MSVC, on 32-bit systems, defines sizeof(long)=4 which means that a
uint32_t could end up negative when passed to |ASN1_INTEGER_set| on
Windows.

Change-Id: Ib07487ab524550c832909bf10521aae61d654416
Reviewed-on: https://boringssl-review.googlesource.com/12560
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-12-01 23:32:49 +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
David Benjamin
aedf303cc2 Parse the entire PSK extension.
Although we ignore all but the first identity, keep clients honest by
parsing the whole thing. Also explicitly check that the binder and
identity counts match.

Change-Id: Ib9c4caae18398360f3b80f8db1b22d4549bd5746
Reviewed-on: https://boringssl-review.googlesource.com/12469
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-01 21:53:13 +00:00
Alessandro Ghedini
bf48364a8c Support setting per-connection default session lifetime value
Due to recent changes, changing the SSL session timeout from cert_cb is
not possible anymore since the new |SSL_SESSION| is initialized *after*
cert_cb is run. The alternative would be using |SSL_CTX_set_timeout| but
the specific |SSL_CTX| could be shared by multiple |SSL|s.

Setting a value on a per-connection basis is useful in case timeouts
need to be calculated dynamically based on specific certificate/domain
information that would be retrieved from inside cert_cb (or other
callbacks).

It would also be possible to set the value to 0 to prevent session
resumption, which is not otherwise doable in the handshake callbacks.

Change-Id: I730a528c647f83f7f77f59b5b21d7e060e4c9843
Reviewed-on: https://boringssl-review.googlesource.com/12440
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-01 21:01:30 +00:00
Robert Sloan
15073af5b7 Adding a fuzzer for Sessions
Change-Id: I69cbb0679e1dbb6292a8f4737851736e58c17508
Reviewed-on: https://boringssl-review.googlesource.com/12481
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-01 20:22:56 +00:00
Steven Valdez
a4ee74dadf Skipping early data on 0RTT rejection.
BUG=101

Change-Id: Ia1edbccee535b0bc3a0e18465286d5bcca240035
Reviewed-on: https://boringssl-review.googlesource.com/12470
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-01 20:16:08 +00:00
David Benjamin
8f820b4e43 Clean up resumption secret "derivation" step.
There is no more derivation step. We just use the resumption secret
directly. This saves us an unnecessary memcpy.

Change-Id: I203bdcc0463780c47cce655046aa1be560bb5b18
Reviewed-on: https://boringssl-review.googlesource.com/12472
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-12-01 19:26:14 +00:00
Martin Kreichgauer
ce1f62c3be Make bssl server generate a self-signed cert if none is provided.
Change-Id: I99e63341110151fc5584aa2ed874bc0441984eea
Reviewed-on: https://boringssl-review.googlesource.com/12471
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-01 17:54:47 +00:00
Adam Langley
509889d3d0 Sync with upstream's version of sha256-armv4.pl.
This change imports sha256-armv4.pl from upstream at rev 8d1ebff4. This
includes changes to remove the use of adrl, which is not supported by
Clang.

Change-Id: I429e7051d63b59acad21601e40883fc3bd8dd2f5
Reviewed-on: https://boringssl-review.googlesource.com/12480
Commit-Queue: Adam Langley <alangley@gmail.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>
2016-11-30 17:37:24 +00:00
David Benjamin
f086df9f5f signed char => int8_t.
This code wants something which can represent -128..127 or so, not
something about characters.

Change-Id: Icdbfec370317a5e03803939a3b8d1555f8efff1d
Reviewed-on: https://boringssl-review.googlesource.com/12468
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-11-29 01:03:52 +00:00
David Benjamin
e4a9dbcf02 Minor formatting fixups.
clang-format mangled this a little.

Change-Id: Ic4d8de0e1f6e926efbe8d14e390fe874b4a7cdcb
Reviewed-on: https://boringssl-review.googlesource.com/12467
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-11-29 01:03:28 +00:00
David Benjamin
bfe5f08170 Rewrite EC_window_bits_for_scalar_size into a function.
The compiler should be plenty smart enough to decide whether to inline a
static function called only once. We don't need to resort to so
unreadable a ternary chain.

Change-Id: Iacc8e0c4147fc69008806a0cc36d9e632169601a
Reviewed-on: https://boringssl-review.googlesource.com/12466
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-11-29 01:01:28 +00:00
David Benjamin
c6d09e83c7 Check for BN_lshift failure in BN_sqrt.
Change-Id: I3350ff0e4ffe7495a83211b89c675a0125fb2f06
Reviewed-on: https://boringssl-review.googlesource.com/12465
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-11-29 00:55:56 +00:00
David Benjamin
1c68effac9 Fix error code for unreduced x.
EC_R_INVALID_COMPRESSED_POINT makes more sense than
EC_R_INVALID_COMPRESSION_BIT here.

Change-Id: I0dbdc91bab59843d5c04f2d0e97600fe7644753e
Reviewed-on: https://boringssl-review.googlesource.com/12464
Reviewed-by: Adam Langley <agl@google.com>
2016-11-29 00:55:04 +00:00
David Benjamin
14ebb4ff27 Don't compute the Kronecker symbol in ec_GFp_simple_set_compressed_coordinates.
If y is zero, there is no point with odd y, so the odd bit may not be
set, hence EC_R_INVALID_COMPRESSION_BIT. This code instead computed the
Kronecker symbol of x and changed the error code to
EC_R_INVALID_COMPRESSED_POINT if not a square.

As the comment says, this was (intended to be) unreachable. But it
seems x was a typo for tmp1. It dates to before upstream's
6fb60a84dd1ec81953917e0444dab50186617432, when BN_mod_sqrt gave
garbage if its input was not square. Now it emits BN_R_NOT_A_SQUARE.
Upstream's 48fe4d6233ac2d60745742a27f820dd88bc6689d then mapped
BN_R_NOT_A_SQUARE to EC_R_INVALID_COMPRESSED_POINT.

Change-Id: Id9e02fa1c154b61cc0c3a768c9cfe6bd9674c378
Reviewed-on: https://boringssl-review.googlesource.com/12463
Reviewed-by: Adam Langley <agl@google.com>
2016-11-29 00:36:04 +00:00
David Benjamin
bd691756f2 Test that BN_mod_sqrt detects quadratic non-residues.
Change-Id: I99abb8f99c060f0830c1df9cdbde159c85ccfe92
Reviewed-on: https://boringssl-review.googlesource.com/12462
Reviewed-by: Adam Langley <agl@google.com>
2016-11-29 00:24:43 +00:00
David Benjamin
55a1eccc0c Don't allow BN_mod_sqrt(0) to return P in tests.
Zero only has one allowed square root, not two.

Change-Id: I1dbd2137a7011d2f327b271b267099771e5499c3
Reviewed-on: https://boringssl-review.googlesource.com/12461
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-11-29 00:23:07 +00:00
David Benjamin
3d622e554e Add missing bounds check in tls13_derive_resumption_secret.
This is fine because TLS PRFs only go up to SHA-384, but since
SSL_SESSION::master_key is sized to 48, not EVP_MAX_MD_SIZE, this should
explicitly check the bounds.

Change-Id: I2b1bcaab5cdfc3ce4d7a8b8ed5cc4c6d15d10270
Reviewed-on: https://boringssl-review.googlesource.com/12460
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-11-28 20:36:32 +00:00
David Benjamin
68f37b7a3f Run TestOneSidedShutdown at all versions.
Change-Id: I3a5d949eec9241ea43da40ce23e0e7f2a25e30e5
Reviewed-on: https://boringssl-review.googlesource.com/12381
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2016-11-21 18:56:48 +00:00
David Benjamin
0fef3056eb Add a ForEachVersion function to ssl_test.
This aligns with ec_test which has a ForEachCurve helper and avoids
writing these loops all the time. As a bonus, these tests start working
in DTLS now.

Change-Id: I613fc08b641ddc12a819d8a1268a1e6a29043663
Reviewed-on: https://boringssl-review.googlesource.com/12380
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-11-21 18:56:34 +00:00
Adam Langley
9b885c5d0f Don't allow invalid SCT lists to be set.
This change causes SSL_CTX_set_signed_cert_timestamp_list to check the
SCT list for shallow validity before allowing it to be set.

Change-Id: Ib8a1fe185224ff02ed4ce53a0109e60d934e96b3
Reviewed-on: https://boringssl-review.googlesource.com/12401
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2016-11-19 00:24:18 +00:00
Adam Langley
6f5f49f33d Flush TLS 1.3 certificate extensions.
(Otherwise we end up touching potentially unwound stack.)

I looked into why our builders didn't catch this and it appears that, at
least with Clang 3.7, ASAN doesn't notice this. Perhaps Clang at that
version is being lazy about destructing the scoped CBB and so doesn't
actually go wrong.

Change-Id: Ia0f73e7eb662676439f024805fc8287a4e991ce0
Reviewed-on: https://boringssl-review.googlesource.com/12400
Reviewed-by: Adam Langley <agl@google.com>
2016-11-18 22:01:38 +00:00
Adam Langley
cfa08c3b77 Enforce basic sanity of SCT lists.
According to the RFC[1], SCT lists may not be empty and nor may any SCT
itself be empty.

[1] https://tools.ietf.org/html/rfc6962#section-3.3

Change-Id: Ia1f855907588b36a4fea60872f87e25dc20782b4
Reviewed-on: https://boringssl-review.googlesource.com/12362
Reviewed-by: Adam Langley <agl@google.com>
2016-11-18 19:19:48 +00:00
David Benjamin
b5172a722c Make tls1_setup_key_block static.
It is not called outside of t1_enc.c.

Change-Id: Ifd9d109eeb432e931361ebdf456243c490b93ecf
Reviewed-on: https://boringssl-review.googlesource.com/12340
Reviewed-by: Steven Valdez <svaldez@chromium.org>
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-11-18 03:58:26 +00:00
Doug Kwan
7da8ea72a6 Add forward declaration to avoid a compiler warning
This prevents a compiler warning from breaking ppc64le build.

Change-Id: I6752109bd02c6d078e656f89327093f8fb13a125
Reviewed-on: https://boringssl-review.googlesource.com/12363
Reviewed-by: Adam Langley <agl@google.com>
2016-11-18 00:25:50 +00:00
Adam Langley
fbbef12918 Don't put a colon in the extra error message.
Since the printed format for errors uses colons to separate different
parts of the error message, this was confusing.

Change-Id: I4742becec2bcb56ad8dc2fdb9a3bb23e4452d1b2
Reviewed-on: https://boringssl-review.googlesource.com/12361
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Commit-Queue: Adam Langley <alangley@gmail.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-11-17 21:46:34 +00:00
Adam Langley
d7b9002aa5 Define BORINGSSL_SHARED_LIBRARY when building tests with Bazel.
Bazel builds tests as shared libraries and the new p256-x86_64_test
depends on accessing unexported symbols. Thus we need to define
BORINGSSL_SHARED_LIBRARY when building tests.

Change-Id: I1270c69ac9d1bcf6baa05ef6666078bd368d80cf
Reviewed-on: https://boringssl-review.googlesource.com/12360
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-11-17 20:49:27 +00:00
Doug Kwan
5f04b6bc3a Add ppc64le vector implementation of SHA-1.
This change contains a C implementation of SHA-1 for POWER using
AltiVec. It is almost as fast as the scalar-only assembly implementation
for POWER/POWERPC family in OpenSSL but it is easier to maintain and it
allows error checking with tools like ASAN.

This is tested only for ppc64le. It may nor may not work for other
platforms in the POWER/POWERPC familiy.

Before:

SHA-1 @ 16 bytes: ~30 MB/s
SHA-1 @ 8K: ~140 MB/s

After:

SHA-1 @ 16 bytes: ~70 MB/s
SHA-1 @ 8K: ~480 MB/s

Change-Id: I790352e86d9c0cc4e1e57d11c5a0aa5b0780ca6b
Reviewed-on: https://boringssl-review.googlesource.com/12203
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-11-17 18:38:14 +00:00
David Benjamin
35598ae8dd Remove ext_alpn_init.
We do not change ALPN on renego, so the value should carry over and not
be cleared.

Change-Id: Id54a083945542b4457d9c2787f0fe7c30239b76f
Reviewed-on: https://boringssl-review.googlesource.com/12306
Reviewed-by: David Benjamin <davidben@google.com>
2016-11-17 06:46:48 +00:00
David Benjamin
e7f60a2852 Fix alert on tls1_process_alert failure.
If the function fails, it's an internal_error.

Change-Id: I4b7cf7a6ca2527f04b708303ab1bc71df762b55b
Reviewed-on: https://boringssl-review.googlesource.com/12312
Reviewed-by: David Benjamin <davidben@google.com>
2016-11-17 06:45:38 +00:00
David Benjamin
12d6bafed8 Make ssl_ext_pre_shared_key_add_clienthello static.
It doesn't need to be exported out of t1_lib.c.

Change-Id: I000493e1e330457051da1719ca9f8152a4ff845a
Reviewed-on: https://boringssl-review.googlesource.com/12316
Reviewed-by: Steven Valdez <svaldez@chromium.org>
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-11-17 06:33:30 +00:00
David Benjamin
2aaaa16251 Depend all_tests on p256-x86_64_test.
Otherwise the run_tests target sometimes gets confused.

Change-Id: If49e945bf5137c68db4927ab0f9845d25be63bac
Reviewed-on: https://boringssl-review.googlesource.com/12315
Reviewed-by: Steven Valdez <svaldez@chromium.org>
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-11-17 02:56:51 +00:00
David Benjamin
bbaf367969 Add |SSL_set_retain_only_sha256_of_client_certs|.
Previously the option to retain only the SHA-256 hash of client
certificates could only be set at the |SSL_CTX| level. This change makes
|SSL| objects inherit the setting from the |SSL_CTX|, but allows it to
be overridden on a per-|SSL| basis.

Change-Id: Id435934af3d425d5f008d2f3b9751d1d0884ee55
Reviewed-on: https://boringssl-review.googlesource.com/12182
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-11-17 02:49:19 +00:00
David Benjamin
a933c38f1a Test setting session ID context in early or SNI callback.
The former has always worked. The latter is new to the revised
processing order.

Change-Id: I993d29ccaca091725524847695df4d1944b609cf
Reviewed-on: https://boringssl-review.googlesource.com/11848
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-11-17 01:22:05 +00:00
David Benjamin
f01f42a2ce Negotiate ciphers before resumption.
This changes our resumption strategy. Before, we would negotiate ciphers
only on fresh handshakes. On resumption, we would blindly use whatever
was in the session.

Instead, evaluate cipher suite preferences on every handshake.
Resumption requires that the saved cipher suite match the one that would
have been negotiated anyway. If client or server preferences changed
sufficiently, we decline the session.

This is much easier to reason about (we always pick the best cipher
suite), simpler, and avoids getting stuck under old preferences if
tickets are continuously renewed. Notably, although TLS 1.2 ticket
renewal does not work in practice, TLS 1.3 will renew tickets like
there's no tomorrow.

It also means we don't need dedicated code to avoid resuming a cipher
which has since been disabled. (That dedicated code was a little odd
anyway since the mask_k, etc., checks didn't occur. When cert_cb was
skipped on resumption, one could resume without ever configuring a
certificate! So we couldn't know whether to mask off RSA or ECDSA cipher
suites.)

Add tests which assert on this new arrangement.

BUG=116

Change-Id: Id40d851ccd87e06c46c6ec272527fd8ece8abfc6
Reviewed-on: https://boringssl-review.googlesource.com/11847
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-11-17 01:02:42 +00:00
David Benjamin
34202b93b6 Call cert_cb before resolving resumption.
This is in preparation for determining the cipher suite (which, in TLS
1.2, requires the certificate be known) before resumption.

Note this has caller-visible effects:

- cert_cb is now called whether resumption occurs or not. Our only
  consumer which uses this as a server is Node which will require a
  patch to fix up their mucking about with SSL_get_session. (But the
  patch should be quite upstreamable. More 1.1.0-compatible and
  generally saner.)

- cert_cb is now called before new_session_cb and dos_protection_cb.

BUG=116

Change-Id: I6cc745757f63281fad714d4548f23880570204b0
Reviewed-on: https://boringssl-review.googlesource.com/11846
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-11-17 00:29:46 +00:00
David Benjamin
4eb95ccfd6 Parse ClientHello extensions before deciding on resumption.
This simplifies a little code around EMS and PSK KE modes, but requires
tweaking the SNI code.

The extensions that are more tightly integrated with the handshake are
still processed inline for now. It does, however, require an extra state
in 1.2 so the asynchronous session callback does not cause extensions to
be processed twice. Tweak a test enforce this.

This and a follow-up to move cert_cb before resumption are done in
preparation for resolving the cipher suite before resumption and only
resuming on match.

Note this has caller-visible effects:

- The legacy SNI callback happens before resumption.

- The ALPN callback happens before resumption.

- Custom extension ClientHello parsing callbacks also cannot depend on
  resumption state.

- The DoS protection callback now runs after all the extension callbacks
  as it is documented to be called after the resumption decision.

BUG=116

Change-Id: I1281a3b61789b95c370314aaed4f04c1babbc65f
Reviewed-on: https://boringssl-review.googlesource.com/11845
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-11-16 23:58:02 +00:00
Alessandro Ghedini
8d3f130c55 tool: print client's SNI value, if any.
Change-Id: I4fbce046090f0b0e78c9de923643268cfe6f142f
Reviewed-on: https://boringssl-review.googlesource.com/12241
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-11-16 19:32:16 +00:00
David Benjamin
4008c7a80d Fix some more negative zeros and add tests for each case.
See https://github.com/openssl/openssl/pull/1672.

Change-Id: I4c93a568b9b7ce582b03e955d3aa9cb6b0e89794
Reviewed-on: https://boringssl-review.googlesource.com/12314
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-11-16 19:20:27 +00:00
David Benjamin
ca0b603153 Remove unnecessary BN_is_negative check in p256-x86_64_test.
A BN_ULONG[P256_LIMBS] can't represent a negative number and
bn_set_words won't produce one. We only need to compare against P.

Change-Id: I7bd1c9e8c162751637459f23f3cfc56884d85864
Reviewed-on: https://boringssl-review.googlesource.com/12304
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-11-16 19:19:38 +00:00
David Benjamin
dc16f38685 ec/ecp_nistz256: harmonize is_infinity with ec_GFp_simple_is_at_infinity.
RT#4625

(Imported from upstream's e3057a57caf4274ea1fb074518e4714059dfcabf.)

Add a test in ec_test to cover the ecp_nistz256_points_mul change. Also
revise the low-level infinity tests to cover the changes in
ecp_nistz256_point_add. Upstream's 'infty' logic was also cleaned up to
be simpler and take advantage of the only cases where |p| is infinity.

Change-Id: Ie22de834bf79ecb6191e824ad9fc1bd6f9681b8b
Reviewed-on: https://boringssl-review.googlesource.com/12225
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-11-16 18:16:54 +00:00