Commit Graph

22 Commits

Author SHA1 Message Date
David Benjamin
e9c7b1c8ae Add SSL_SESSION_is_single_use.
Querying versions is a bit of a mess between DTLS and TLS and variants
and friends. Add SSL_SESSION_is_single_use which informs the caller
whether the session should be single-use.

Bug: chromium:631988
Change-Id: I745d8a5dd5dc52008fe99930d81fed7651b92e4e
Reviewed-on: https://boringssl-review.googlesource.com/20844
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-09-28 18:38:17 +00:00
David Benjamin
21fa684236 Have fun with lock scopers.
Change-Id: I2697349024769545c2c37173e6ed68640b7d3b78
Reviewed-on: https://boringssl-review.googlesource.com/20805
Reviewed-by: 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>
2017-09-28 17:49:37 +00:00
David Benjamin
9eaa3bd55d Remove SSL_CTX_sessions and properly lock SSL_CTX_sess_number.
SSL_CTX_sessions is the only think making us expose LHASH as public API
and nothing uses it. Nothing can use it anyway as it's not thread-safe.
I haven't actually removed it yet since SSL_CTX is public, but once the
types are opaque, we could trim the number of symbols ssl.h pulls in
with some work.

Relatedly, fix thread safety of SSL_CTX_sess_number.

Change-Id: I75a6c93509d462cd5ed3ce76c587f0d1e7cd0797
Reviewed-on: https://boringssl-review.googlesource.com/20804
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-09-27 21:58:05 +00:00
David Benjamin
15868b3bba Revert "Work around a Java client bug when rotating certificates."
This reverts commit aba057a4e0 and
5a79ff5efd.

Change-Id: Ia53a3908491ec99ab25ea1d1bdedf322c2fbe5c4
Reviewed-on: https://boringssl-review.googlesource.com/20744
Reviewed-by: 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>
2017-09-26 22:27:47 +00:00
Martin Kreichgauer
6dc892fcdf Remove redundant calls to |OPENSSL_cleanse| and |OPENSSL_realloc_clean|.
Change-Id: I5c85c4d072ec157b37ed95b284a26ab32c0c42d9
Reviewed-on: https://boringssl-review.googlesource.com/19824
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-09-18 19:16:51 +00:00
David Benjamin
9a4876e193 Remove hmac.h include from ssl.h.
base.h pulls in all the forward declarations, so this isn't needed.  We
should also remove bio.h and buf.h, but cURL seems to depend on those.
Code search suggests this one is okay though.

  case:yes content:\bHMAC content:openssl/ssl.h -content:openssl/hmac.h

Change-Id: Id91686bd134649245855025940bc17f82823c734
Reviewed-on: https://boringssl-review.googlesource.com/20364
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-09-15 22:48:43 +00:00
David Benjamin
5a79ff5efd Clarify some comments.
Further testing suggests the behavior is slightly different than I
originally thought.

Change-Id: I3df6b3425dbb551e374159566ca969347d72a306
Reviewed-on: https://boringssl-review.googlesource.com/20284
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-09-14 21:15:23 +00:00
David Benjamin
aba057a4e0 Work around a Java client bug when rotating certificates.
The Java client implementation of the 3SHAKE mitigation incorrectly
rejects initial handshakes when all of the following are true:

1. The ClientHello offered a session.
2. The session was successfully resumed previously.
3. The server declines the session.
4. The server sends a certificate with a different SAN list than in the
   previous session.

(Note the 3SHAKE mitigation is to reject certificates changes on
renegotiation, while Java's logic applies to initial handshakes as
well.)

The end result is long-lived Java clients break on some certificate
rotations. Fingerprint Java clients and decline all offered sessions.
This avoids (2) while still introducing new sessions to clear any
existing problematic sessions.

See also b/65323005.

Change-Id: Ib2b84c69b5ecba285ffb8c4d03de5626838d794e
Reviewed-on: https://boringssl-review.googlesource.com/20184
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-09-12 15:56:59 +00:00
David Benjamin
a861460c89 Make SNI per-connection, not per-session.
Right now we report the per-connection value during the handshake and
the per-session value after the handshake. This also trims our tickets
slightly by removing a largely unused field from SSL_SESSION.

Putting it on SSL_HANDSHAKE would be better, but sadly a number of
bindings-type APIs expose it after the handshake.

Change-Id: I6a1383f95da9b1b141b9d6adadc05ee1e458a326
Reviewed-on: https://boringssl-review.googlesource.com/20064
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-09-06 20:25:26 +00:00
David Benjamin
fd45ee7da8 Replace bits in SSL_HANDSHAKE with bool.
Change-Id: I23f1449d8652a4aa3a9006e04c86c9430127800e
Reviewed-on: https://boringssl-review.googlesource.com/19924
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-09-01 15:05:52 +00:00
David Benjamin
c11ea942b7 Convert comments in ssl.
That's the last of it!

Change-Id: I93d1f5ab7e95b2ad105c34b24297a0bf77625263
Reviewed-on: https://boringssl-review.googlesource.com/19784
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-08-29 21:33:32 +00:00
Steven Valdez
398085ba04 Simplify states with hs_wait_t returns.
Change-Id: Ie0014bf73625144503b649e84b43ca4b03a4df1f
Reviewed-on: https://boringssl-review.googlesource.com/19704
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-08-29 19:53:42 +00:00
Steven Valdez
4d71a9a2ca Migrate TLS 1.2 and below state machines to the new style.
Bug: 128
Change-Id: Ief3779b1c43dd34a154a0f1d2f94d0da756bc07a
Reviewed-on: https://boringssl-review.googlesource.com/19144
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-08-29 19:23:22 +00:00
David Benjamin
8fc2dc07d8 Put SCTs and OCSP responses in CRYPTO_BUFFERs.
They both can be moderately large. This should hopefully relieve a little
memory pressure from both connections to hosts which serve SCTs and
TLS 1.3's single-use tickets.

Change-Id: I034bbf057fe5a064015a0f554b3ae9ea7797cd4e
Reviewed-on: https://boringssl-review.googlesource.com/19584
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-08-23 15:58:52 +00:00
Martin Kreichgauer
72912d2500 Rotate the default ticket encryption key.
The ticket encryption key is rotated automatically once every 24 hours,
unless a key has been configured manually (i.e. using
|SSL_CTX_set_tlsext_ticket_keys|) or one of the custom ticket encryption
methods is used.

Change-Id: I0dfff28b33e58e96b3bbf7f94dcd6d2642f37aec
Reviewed-on: https://boringssl-review.googlesource.com/18924
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-08-16 18:18:54 +00:00
David Benjamin
ca9e8f52f1 Tidy up handshake digest logic.
Use SSL_SESSION_get_digest instead of the lower level function where
applicable. Also, remove the failure case (Ivan Maidanski points out in
https://android-review.googlesource.com/c/337852/1/src/ssl/t1_enc.c that
this unreachable codepath is a memory leak) by passing in an SSL_CIPHER
to make it more locally obvious that other values are impossible.

Change-Id: Ie624049d47ab0d24f32b405390d6251c7343d7d6
Reviewed-on: https://boringssl-review.googlesource.com/19024
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-08-09 19:13:15 +00:00
David Benjamin
27e377ec65 Fix miscellaneous clang-tidy warnings.
There are still a ton of them, almost exclusively complaints that
function declaration and definitions have different parameter names. I
just fixed a few randomly.

Change-Id: I1072f3dba8f63372cda92425aa94f4aa9e3911fa
Reviewed-on: https://boringssl-review.googlesource.com/18706
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-08-01 20:39:46 +00:00
David Benjamin
37af90f721 Convert a few more scopers.
Bug: 132
Change-Id: I75d6ce5a2256a4b464ca6a9378ac6b63a9bd47e2
Reviewed-on: https://boringssl-review.googlesource.com/18644
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-08-01 19:45:15 +00:00
David Benjamin
31b0c9be30 Add a bunch of scopers.
I started by switching a couple fields to SSL_HANDSHAKE and then kept
following transitive bits.

Bug: 132
Change-Id: I640dadd3558615fa38c7e8498d4efe7449b0658f
Reviewed-on: https://boringssl-review.googlesource.com/18245
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2017-07-20 20:38:55 +00:00
David Benjamin
e39ac8fb59 Switch BORINGSSL_INTERNAL_CXX_TYPES in favor of subclassing games.
The previous attempt around the 'struct ssl_st' compatibility mess
offended OSS-Fuzz and UBSan because one compilation unit passed a
function pointer with ssl_st* and another called it with
bssl::SSLConnection*.

Linkers don't retain such types, of course, but to silence this alert,
instead make C-visible types be separate from the implementation and
subclass the public type. This does mean we risk polluting the symbol
namespace, but hopefully the compiler is smart enough to inline the
visible struct's constructor and destructor.

Bug: 132
Change-Id: Ia75a89b3a22a202883ad671a630b72d0aeef680e
Reviewed-on: https://boringssl-review.googlesource.com/18224
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-07-20 17:24:12 +00:00
David Benjamin
86e95b852e Move libssl's internals into the bssl namespace.
This is horrible, but everything else I tried was worse. The goal with
this CL is to take the extern "C" out of ssl/internal.h and move most
symbols to namespace bssl, so we can start using C++ helpers and
destructors without worry.

Complications:

- Public API functions must be extern "C" and match their declaration in
  ssl.h, which is unnamespaced. C++ really does not want you to
  interleave namespaced and unnamespaced things. One can actually write
  a namespaced extern "C" function, but this means, from C++'s
  perspective, the function is namespaced. Trying to namespace the
  public header would worked but ended up too deep a rabbithole.

- Our STACK_OF macros do not work right in namespaces.

- The typedefs for our exposed but opaque types are visible in the
  header files and copied into consuming projects as forward
  declarations. We ultimately want to give SSL a destructor, but
  clobbering an unnamespaced ssl_st::~ssl_st seems bad manners.

- MSVC complains about ambiguous names if one typedefs SSL to bssl::SSL.

This CL opts for:

- ssl/*.cc must begin with #define BORINGSSL_INTERNAL_CXX_TYPES. This
  informs the public headers to create forward declarations which are
  compatible with our namespaces.

- For now, C++-defined type FOO ends up at bssl::FOO with a typedef
  outside. Later I imagine we'll rename many of them.

- Internal functions get namespace bssl, so we stop worrying about
  stomping the tls1_prf symbol. Exported C functions are stuck as they
  are. Rather than try anything weird, bite the bullet and reorder files
  which have a mix of public and private functions. I expect that over
  time, the public functions will become fairly small as we move logic
  to more idiomatic C++.

  Files without any public C functions can just be written normally.

- To avoid MSVC troubles, some bssl types are renamed to CPlusPlusStyle
  in advance of them being made idiomatic C++.

Bug: 132
Change-Id: Ic931895e117c38b14ff8d6e5a273e868796c7581
Reviewed-on: https://boringssl-review.googlesource.com/18124
Reviewed-by: David Benjamin <davidben@google.com>
2017-07-19 19:10:59 +00:00
David Benjamin
0238d8f4ff Switch more files to C++.
Bug: 132
Change-Id: I2b0c87262a5a529ea264ea8ce2d11c2dba0ec1c8
Reviewed-on: https://boringssl-review.googlesource.com/17766
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2017-07-13 16:08:28 +00:00