Commit Graph

191 Commits

Author SHA1 Message Date
David Benjamin
e664a534af Return null from SSL_get0_peer_certificates if unauthenticated.
SSL_get0_peer_certificates is documented to return NULL if the peer was
anonymous, but it actually returns a non-NULL empty list (except in SSL
3.0 where the Certificate message and thus ssl_parse_cert_chain is
skipped).

Make the implementation match the documentation.

Change-Id: Ib3e25d2155f316cc5e9eb3ab7f74b78e08b8a86b
Reviewed-on: https://boringssl-review.googlesource.com/18226
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-07-25 18:53:48 +00:00
Martin Kreichgauer
17c3057f26 Add bssl::SealRecord and bssl::OpenRecord.
This is a C++ interface for encrypting and decrypting TLS application
data records in-place, wrapping the existing C API in tls_record.cc.

Also add bssl::Span, a non-owning reference to a contiguous array of
elements which can be used as a common interface over contiguous
container types (like std::vector), pointer-length-pairs, arrays, etc.

Change-Id: Iaa2ca4957cde511cb734b997db38f54e103b0d92
Reviewed-on: https://boringssl-review.googlesource.com/18104
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-07-24 20:14:08 +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
71dfad4d10 Add new functions for configuring the client CA list.
This is needed to switch Chromium's SSLServerSocket and parts of
Conscrypt to CRYPTO_BUFFER.

Bug: 54
Change-Id: Iacd417970607bc1a162057676b576956a3bdfa3f
Reviewed-on: https://boringssl-review.googlesource.com/17965
Reviewed-by: Adam Langley <agl@google.com>
2017-07-17 22:34:04 +00:00
David Benjamin
3a1dd46e4e Add async certificate verification callback.
This also serves as a certificate verification callback for
CRYPTO_BUFFER-based consumers. Remove the silly
SSL_CTX_i_promise_to_verify_certs_after_the_handshake placeholder.

Bug: 54, chromium:347402
Change-Id: I4c6b445cb9cd7204218acb2e5d1625e6f37aff6f
Reviewed-on: https://boringssl-review.googlesource.com/17964
Reviewed-by: David Benjamin <davidben@google.com>
2017-07-17 20:55:23 +00:00
David Benjamin
bf5f192310 Add some addition tests for the cipher parsing code and tidy.
The in_group check is redundant and test an extremely absurd corner of
the syntax.

Change-Id: Ia54bcd7cda7ba05415d3a250ee93e1acedcc43d6
Reviewed-on: https://boringssl-review.googlesource.com/17542
Reviewed-by: David Benjamin <davidben@google.com>
2017-07-05 23:52:05 +00:00
David Benjamin
353577cdc7 Fix SSL_set_{min,max}_proto_version APIs in invalid versions.
SSL_set_max_proto_version(TLS1_3_DRAFT_VERSION) worked unintentionally.
Fix that. Also add an error when it fails.

Change-Id: I1048fede7b163e1c170e17bf4370b468221a7077
Reviewed-on: https://boringssl-review.googlesource.com/17525
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-07-05 19:43:26 +00:00
David Benjamin
5df5be1a4b Fix record header callback on writes.
These broke at some point. Add a test for them.

Change-Id: Ie45869e07d9615ae33aae4613f6d9b996af39528
Reviewed-on: https://boringssl-review.googlesource.com/17330
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-06-23 23:01:36 +00:00
David Benjamin
6fff386492 Support standard RFC cipher suite names alongside OpenSSL ones.
Both Conscrypt and Netty have a lot of logic to map between the two
kinds of names. WebRTC needed an SSL_CIPHER_get_rfc_name for something.
Just have both in the library. Also deprecate SSL_CIPHER_get_rfc_name
in favor of SSL_CIPHER_standard_name, which matches upstream if built
with enable-ssl-trace. And, unlike SSL_CIPHER_get_rfc_name, this does
not require dealing with the malloc.

(Strangely this decreases bssl's binary size, even though we're carrying
more strings around. It seems the old SSL_CIPHER_get_rfc_name was
somewhat large in comparison. Regardless, a consumer that disliked 30
short strings probably also disliked the OpenSSL names. That would be
better solved by opaquifying SSL_CIPHER and adding a less stringy API
for configuring cipher lists. That's something we can explore later if
needed.)

I also made the command-line tool print out the standard names since
they're more standard. May as well push folks towards those going
forward.

Change-Id: Ieeb3d63e67ef4da87458e68d130166a4c1090596
Reviewed-on: https://boringssl-review.googlesource.com/17324
Reviewed-by: Robert Sloan <varomodt@google.com>
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-06-22 02:45:37 +00:00
David Benjamin
fc08dfc4cd Rename {ssl,ctx}->{min,max}_version.
These are not the true version filters due to SSL_OP_NO_* filters.

Change-Id: I4c2db967d885f7c1875a3e052c5b02ea8d612fe1
Reviewed-on: https://boringssl-review.googlesource.com/17266
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-06-20 19:37:43 +00:00
David Benjamin
ca7435822f Test SSL_select_next_proto and SSL_get_fd.
Free code coverage. Also rename things in SSL_select_next_proto so it
works for NPN and ALPN. (I found some code which uses it for ALPN.)

Change-Id: I8d06b768f9484dc3eda1a20506ec84ec3ddbc883
Reviewed-on: https://boringssl-review.googlesource.com/17206
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-06-16 13:47:04 +00:00
Steven Valdez
e831a81518 Adding support for sending early data on the client.
BUG=76

Change-Id: If58a73da38e46549fd55f84a9104e2dfebfda43f
Reviewed-on: https://boringssl-review.googlesource.com/14164
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: 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-06-15 19:34:59 +00:00
David Benjamin
321fcdc458 Convert default version tests in ssl_test.
BUG=129

Change-Id: Ie6950ebf8eac82f876cb185a025418d43ea752f6
Reviewed-on: https://boringssl-review.googlesource.com/15467
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-04-24 20:51:05 +00:00
David Benjamin
e11726a9a4 Properly convert more of ssl_test.
BUG=129

Change-Id: Ic7133d269be8a069734153efc36e9cfeb10c698e
Reviewed-on: https://boringssl-review.googlesource.com/15466
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-04-24 20:47:55 +00:00
Matthew Braithwaite
7e06de5d2d Really remove DHE ciphersuites from TLS.
This follows up on cedc6f18 by removing support for the
-DBORINGSSL_ENABLE_DHE_TLS compile flag, and the code needed to
support it.

Change-Id: I53b6aa7a0eddd23ace8b770edb2a31b18ba2ce26
Reviewed-on: https://boringssl-review.googlesource.com/14886
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-04-11 23:41:31 +00:00
David Benjamin
3cfeb9522b Disable SSLv3 by default.
As a precursor to removing the code entirely later, disable the protocol
by default. Callers must use SSL_CTX_set_min_version to enable it.

This change also makes SSLv3_method *not* enable SSL 3.0. Normally
version-specific methods set the minimum and maximum version to their
version. SSLv3_method leaves the minimum at the default, so we will
treat it as all versions disabled. To help debugging, the error code is
switched from WRONG_SSL_VERSION to a new NO_SUPPORTED_VERSIONS_ENABLED.

This also defines OPENSSL_NO_SSL3 and OPENSSL_NO_SSL3_METHOD to kick in
any no-ssl3 build paths in consumers which should provide a convenient
hook for any upstreaming changes that may be needed. (OPENSSL_NO_SSL3
existed in older versions of OpenSSL, so in principle one may encounter
an OpenSSL with the same settings.)

Change-Id: I96a8f2f568eb77b2537b3a774b2f7108bd67dd0c
Reviewed-on: https://boringssl-review.googlesource.com/14031
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-04-11 16:38:16 +00:00
Alessandro Ghedini
57e81e666a Name |select_certificate_cb| return values
The |select_certificate_cb| return values are somewhat confusing due
to the fact that they don't match the |cert_cb| ones, despite the
similarities between the two callbacks (they both have "certificate" in
the name! well, sort of).

This also documents the error return value (-1) which was previously
undocumented, and it expands the |SSL_CTX_set_select_certificate_cb|
documentation regarding retrial (by shamelessly copying from
|SSL_CTX_set_ticket_aead_method|).

Also updates other scattered documentation that was missed by previous
changes.

Change-Id: Ib962b31d08e6475e09954cbc3c939988b0ba13f7
Reviewed-on: https://boringssl-review.googlesource.com/14245
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-03-22 01:27:00 +00:00
Matthew Braithwaite
cedc6f1824 Remove DHE ciphersuites from TLS.
They can be restored by compiling with -DBORINGSSL_ENABLE_DHE_TLS.

This is similar to 9c8c4188 for RC4 ciphers.

Change-Id: I7cd3421b108a024f1ee11f13a6df881c2d0de3c3
Reviewed-on: https://boringssl-review.googlesource.com/14284
Commit-Queue: Matt Braithwaite <mab@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-03-21 23:55:10 +00:00
David Benjamin
707af294a8 Support asynchronous ticket decryption with TLS 1.3.
This shuffles a bit of the code around session resumption in TLS 1.3 to
make the async point cleaner to inject. It also fills in cipher and
tlsext_hostname more uniformly.

Filling in the cipher on resumption is a no-op as SSL_SESSION_dup
already copies it, but avoids confusion should we ever implement TLS
1.3's laxer cipher matching on the server. Not filling in
tlsext_hostname on resumption was an oversight; the relevant check isn't
whether we are resuming but whether we have a fresh SSL_SESSION to fill
things into.

Change-Id: Ic02eb079ff228ce4a4d3e0de7445e18cd367e8b2
Reviewed-on: https://boringssl-review.googlesource.com/14205
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-03-11 00:05:29 +00:00
Adam Langley
4c341d0299 Support asynchronous ticket decryption with TLS 1.0–1.2.
This change adds support for setting an |SSL_TICKET_AEAD_METHOD| which
allows a caller to control ticket encryption and decryption to a greater
extent than previously possible and also permits asynchronous ticket
decryption.

This change only includes partial support: TLS 1.3 work remains to be
done.

Change-Id: Ia2e10ebb3257e1a119630c463b6bf389cf20ef18
Reviewed-on: https://boringssl-review.googlesource.com/14144
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-03-11 00:04:18 +00:00
David Benjamin
91222b8d38 Fix configuring the empty cipher list.
Although it returns failure, the cipher list should still be updated.
Conscrypt relies on this behavior to support a Java API edge case.

Change-Id: If58efafc6a4a81e85a0e2ee2c38873a7a4938123
Reviewed-on: https://boringssl-review.googlesource.com/14165
Reviewed-by: Kenny Root <kroot@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-03-10 01:20:25 +00:00
Adam Langley
d04ca95356 Add |SSL[_CTX]_set_chain_and_key|.
This allows a caller to configure a serving chain without dealing with
crypto/x509.

Change-Id: Ib42bb2ab9227d32071cf13ab07f92d029643a9a6
Reviewed-on: https://boringssl-review.googlesource.com/14126
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>
2017-03-08 19:11:57 +00:00
Adam Langley
34b4c829fd Hold CA names as |CRYPTO_BUFFER|s.
This change converts the CA names that are parsed from a server's
CertificateRequest, as well as the CA names that are configured for
sending to clients in the same, to use |CRYPTO_BUFFER|.

The |X509_NAME|-based interfaces are turned into compatibility wrappers.

Change-Id: I95304ecc988ee39320499739a0866c7f8ff5ed98
Reviewed-on: https://boringssl-review.googlesource.com/13585
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-02-28 22:24:31 +00:00
Matthew Braithwaite
a57dcfb69c Add new cipherlist-setting APIs that reject nonsense.
The new APIs are SSL_CTX_set_strict_cipher_list() and
SSL_set_strict_cipher_list().  They have two motivations:

First, typos in cipher lists can go undetected for a long time, and
can have surprising consequences when silently ignored.

Second, there is a tendency to use superstition in the construction of
cipher lists, for example by "turning off" things that do not actually
exist.  This leads to the corrosive belief that DEFAULT and ALL ought
not to be trusted.  This belief is false.

Change-Id: I42909b69186e0b4cf45457e5c0bc968f6bbf231a
Reviewed-on: https://boringssl-review.googlesource.com/13925
Commit-Queue: Matt Braithwaite <mab@google.com>
Reviewed-by: Matt Braithwaite <mab@google.com>
2017-02-22 00:09:27 +00:00
David Benjamin
83a321231b Move SCT lists and OCSP responses to CERT.
Recent changes added SSL-level setters to these APIs. Unfortunately,
this has the side effect of breaking SSL_set_SSL_CTX, which is how SNI
is typically handled. SSL_set_SSL_CTX is kind of a weird function in
that it's very sensitive to which of the hodge-podge of config styles is
in use. I previously listed out all the config styles here, but it was
long and unhelpful. (I counted up to 7.)

Of the various SSL_set_SSL_CTX-visible config styles, the sanest seems
to be to move it to CERT. In this case, it's actually quite reasonable
since they're very certificate-related.

Later we may wish to think about whether we can cut down all 7 kinds of
config styles because this is kinda nuts. I'm wondering we should do
CERT => SSL_CONFIG, move everything there, and make that be the same
structure that is dropped post-handshake (supposing the caller has
disavowed SSL_clear and renego). Fruit for later thought. (Note though
that comes with a behavior change for all the existing config.)

Change-Id: I9aa47d8bd37bf2847869e0b577739d4d579ee4ae
Reviewed-on: https://boringssl-review.googlesource.com/13864
Reviewed-by: Martin Kreichgauer <martinkr@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-15 00:42:38 +00:00
David Benjamin
7d7554b6b3 Add a helper for comparing byte strings.
We compare pointer/length pairs constantly. To avoid needing to type it
everywhere and get GTest's output, add a StringPiece-alike for byte
slices which supports ==, !=, and std::ostream.

BUG=129

Change-Id: I108342cbd2c6a58fec0b9cb87ebdf50364bda099
Reviewed-on: https://boringssl-review.googlesource.com/13625
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-11 00:42:00 +00:00
Adam Langley
46db7af2c9 Remove |X509| things from SSL_SESSION.
|SSL_SESSION_from_bytes| now takes an |SSL_CTX*|, from which it uses the
|X509_METHOD| and buffer pool. This is our API so we can do this.

This also requires adding an |SSL_CTX*| argument to |SSL_SESSION_new|
for the same reason. However, |SSL_SESSION_new| already has very few
callers (and none in third-party code that I can see) so I think we can
get away with this.

Change-Id: I1337cd2bd8cff03d4b9405ea3146b3b59584aa72
Reviewed-on: https://boringssl-review.googlesource.com/13584
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-10 19:12:04 +00:00
David Benjamin
6b0edfb9e6 Add a common TestEventListener for the error queue.
Replicate the logic in the AllTests targets to dump the error queue on
failure. GTest seems to print to stdout, so we do here too.

BUG=129

Change-Id: I623b695fb9a474945834c3653728f54e5b122187
Reviewed-on: https://boringssl-review.googlesource.com/13623
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-07 21:33:22 +00:00
David Benjamin
f0d8e22078 Convert some of ssl_test to GTest more thoroughly.
The more complex ones will want a TEST_P, but here are a few easy ones
to start with.

BUG=129

Change-Id: I2e341d04910c0b05a5bc7afec961c4541ca7db41
Reviewed-on: https://boringssl-review.googlesource.com/13622
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-07 21:33:15 +00:00
Alessandro Ghedini
33fe4a0d14 Remove support for setting per-connection default session timeout
As previously discussed, it turns out we don't actually need this, so
there's no point in keeping it.

Change-Id: If549c917b6bd818cd36948e37cb7839c8d122b1a
Reviewed-on: https://boringssl-review.googlesource.com/13641
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-07 17:46:25 +00:00
David Benjamin
b2ff2623a8 Add a basic SSL_get_certificate test.
With the CRYPTO_BUFFER stuff, this API is now slightly more complex. Add
some tests as a sanity-check.

Change-Id: I9da20e3eb6391fc86ed215c5fabec71aa32ef56f
Reviewed-on: https://boringssl-review.googlesource.com/13620
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-03 22:19:51 +00:00
David Benjamin
17b3083373 Use a separate timeout scheme for TLS 1.3.
In TLS 1.2, resumption's benefits are more-or-less subsumed by False
Start. TLS 1.2 resumption lifetime is bounded by how much traffic we are
willing to encrypt without fresh key material, so the lifetime is short.
Renewal uses the same key, so we do not allow it to increase lifetimes.

In TLS 1.3, resumption unlocks 0-RTT. We do not implement psk_ke, so
resumption incorporates fresh key material into both encrypted traffic
(except for early data) and renewed tickets. Thus we are both more
willing to and more interested in longer lifetimes for tickets. Renewal
is also not useless. Thus in TLS 1.3, lifetime is bound separately by
the lifetime of a given secret as a psk_dhe_ke authenticator and the
lifetime of the online signature which authenticated the initial
handshake.

This change maintains two lifetimes on an SSL_SESSION: timeout which is
the renewable lifetime of this ticket, and auth_timeout which is the
non-renewable cliff. It also separates the TLS 1.2 and TLS 1.3 timeouts.
The old session timeout defaults and configuration apply to TLS 1.3, and
we define new ones for TLS 1.3.

Finally, this makes us honor the NewSessionTicket timeout in TLS 1.3.
It's no longer a "hint" in 1.3 and there's probably value in avoiding
known-useless 0-RTT offers.

BUG=120

Change-Id: Iac46d56e5a6a377d8b88b8fa31f492d534cb1b85
Reviewed-on: https://boringssl-review.googlesource.com/13503
Reviewed-by: Adam Langley <agl@google.com>
2017-02-02 19:51:49 +00:00
Adam Langley
e1e78130f5 Keep a reference to |X509|s appended to a chain.
The recent CRYPTO_BUFFER changes meant that |X509| objects passed to
SSL_CTX_add_extra_chain_cert would be |free|ed immediately. However,
some third-party code (at least serf and curl) continue to use the
|X509| even after handing over ownership.

In order to unblock things, keep the past |X509| around for a while to
paper over the issues with those libraries while we try and upstream
changes.

Change-Id: I832b458af9b265749fed964658c5c34c84d518df
Reviewed-on: https://boringssl-review.googlesource.com/13480
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-02-01 00:24:24 +00:00
David Benjamin
3b584332ee Fix ssl_test with BORINGSSL_ANDROID_SYSTEM.
We need to suppress a few tests on the system Android build until
RSA-PSS is shipped there.

Change-Id: I5843997aae9fa499ec08d76f44fdf3b523599e1c
Reviewed-on: https://boringssl-review.googlesource.com/13267
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-01-25 16:23:10 +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
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
48063c2aea Add tests around SSL_write's retry behavior.
SSL_write is remarkably complicated.

Change-Id: I1cb8d00af1b4c5e2d90187d5f87951f25e27f224
Reviewed-on: https://boringssl-review.googlesource.com/13050
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-04 04:54:04 +00:00
David Benjamin
650aa1c80a Clean up certificate auto-chaining.
Rather than doing it right before outputing, treat this as a part of the
pipeline to finalize the certificate chain, and run it right after
cert_cb to modify the certificate configuration itself. This means
nothing else in the stack needs to worry about this case existing.

It also makes it easy to support in both TLS 1.2 and TLS 1.3.

Change-Id: I6a088297a54449f1f5f5bb8b5385caa4e8665eb6
Reviewed-on: https://boringssl-review.googlesource.com/12966
Reviewed-by: Adam Langley <agl@google.com>
2017-01-04 01:36:26 +00:00
David Benjamin
1444c3ace0 Add tests for auto-chaining.
Alas, wpa_supplicant relies on the auto-chaining feature, so we can't
easily remove it. Write tests for it to ensure it stays working.

These test currently do not work for TLS 1.3 because the feature is
broken in 1.3. A follow-up change will fix this.

BUG=70

Change-Id: I2c04f55b712d66f5af1556254a5b017ebc3244f7
Reviewed-on: https://boringssl-review.googlesource.com/12965
Reviewed-by: Adam Langley <agl@google.com>
2016-12-21 23:10:07 +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
458334a159 Test name-based version APIs are reported as expected.
Notably, Conscrypt uses SSL_SESSION_get_version, so we should have tests
for it.

Change-Id: I670f1b1b9951f840f27cb62dd36ef4f05042c974
Reviewed-on: https://boringssl-review.googlesource.com/12881
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-19 21:50:06 +00:00
Adam Langley
364f7a6d21 Push the difference in chain semantics to the edge.
OpenSSL includes a leaf certificate in a certificate chain when it's a
client, but doesn't when it's a server. This is also reflected in the
serialisation of sessions.

This change makes the internal semantics consistent: the leaf is always
included in the chain in memory, and never duplicated when serialised.
To maintain the same API, SSL_get_peer_cert_chain will construct a copy
of the chain without the leaf if needed.

Since the serialised format of a client session has changed, an
|is_server| boolean is added to the ASN.1 that defaults to true. Thus
any old client sessions will be parsed as server sessions and (silently)
discarded by a client.

Change-Id: Ibcf72bc8a130cedb423bc0fd3417868e0af3ca3e
Reviewed-on: https://boringssl-review.googlesource.com/12704
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-13 17:18:53 +00:00
Adam Langley
c0fc7a1385 Revert "Add |SSL_CTX_set0_buffer_pool|." and "Hold certificates in an SSL_SESSION as CRYPTO_BUFFERSs as well."
This reverts commits 5a6e616961 and
e8509090cf. I'm going to unify how the
chains are kept in memory between client and server first otherwise the
mess just keeps growing.

Change-Id: I76df0d94c9053b2454821d22a3c97951b6419831
Reviewed-on: https://boringssl-review.googlesource.com/12701
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-09 23:31:12 +00:00
Matthew Braithwaite
651aaefb44 Remove CECPQ1 (experimental post-quantum key agreement).
Change-Id: Ie947ab176d10feb709c6e135d5241c6cf605b8e8
Reviewed-on: https://boringssl-review.googlesource.com/12700
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-09 19:16:56 +00:00
Adam Langley
e8509090cf Hold certificates in an SSL_SESSION as CRYPTO_BUFFERSs as well.
This change adds a STACK_OF(CRYPTO_BUFFER) to an SSL_SESSION which
contains the raw form of the received certificates. The X509-based
members still exist, but their |enc| buffer will alias the
CRYPTO_BUFFERs.

The serialisation format of SSL_SESSIONs is also changed, in a backwards
compatible way. Previously, some sessions would duplicate the leaf
certificate in the certificate chain. These sessions can still be read,
but will be written in a way incompatible with older versions of the
code. This should be fine because the situation where multiple versions
exchange serialised sessions is at the server, and the server doesn't
duplicate the leaf certifiate in the chain anyway.

Change-Id: Id3b75d24f1745795315cb7f8089a4ee4263fa938
Reviewed-on: https://boringssl-review.googlesource.com/12163
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-12-09 18:12:40 +00:00
David Benjamin
b79cc84635 Fix SSL_clear's interaction with session resumption.
Prior to 87eab4902d, due to some
confusions between configuration and connection state, SSL_clear had the
side effect of offering the previously established session on the new
connection.

wpa_supplicant relies on this behavior, so restore it for TLS 1.2 and
below and add a test. (This behavior is largely incompatible with TLS
1.3's post-handshake tickets, so it won't work in 1.3. It'll act as if
we configured an unresumable session instead.)

Change-Id: Iaee8c0afc1cb65c0ab7397435602732b901b1c2d
Reviewed-on: https://boringssl-review.googlesource.com/12632
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-08 16:57:57 +00:00
David Benjamin
731058ec8e Typedef ssl_early_callback_ctx to SSL_CLIENT_HELLO.
It's our ClientHello representation. May as well name it accordingly.
Also switch away from calling the variable name ctx as that conflicts
with SSL_CTX.

Change-Id: Iec0e597af37137270339e9754c6e08116198899e
Reviewed-on: https://boringssl-review.googlesource.com/12581
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 19:52:11 +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
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
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
9b63f2964d Fix run_tests on fuzzer-mode builds.
Change-Id: I0767cd4801924170ce13b8143a9586485b8f78af
Reviewed-on: https://boringssl-review.googlesource.com/12280
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-15 07:01:24 +00:00
Steven Valdez
a833c357ed Update to TLS 1.3 draft 18.
This is the squash of the following CLs:
https://boringssl-review.googlesource.com/c/12021/9
https://boringssl-review.googlesource.com/c/12022/9
https://boringssl-review.googlesource.com/c/12107/19
https://boringssl-review.googlesource.com/c/12141/22
https://boringssl-review.googlesource.com/c/12181/33

The Go portions were written by Nick Harper

BUG=112

Change-Id: I375a1fcead493ec3e0282e231ccc8d7c4dde5063
Reviewed-on: https://boringssl-review.googlesource.com/12300
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-11-15 06:57:21 +00:00
David Benjamin
123db57009 Measure session->timeout from ticket issuance.
The distinction for full handshakes is not meaningful (the timestamp is
currently the start of the handshake), but for renewed sessions, we
currently retain the timestamp of the original issuance.

Instead, when minting or receiving tickets, adjust session->time and
session->timeout so that session->time is the ticket issuance time.

This is still not our final TLS 1.3 behavior (which will need a both
renewable and non-renewable times to honor the server ticket lifetime),
but it gets us closer and unblocks handling ticket_age_add from TLS 1.3
draft 18 and sends the correct NewSessionTicket lifetime.

This fixes the ticket lifetime hint which we emit on the server to
mirror the true ticket lifetime. It also fixes the TLS 1.3 server code
to not set the ticket lifetime hint. There is no need to waste ticket
size with it, it is no longer a "hint" in TLS 1.3, and even in the TLS
1.3 code we didn't fill it in on the server.

Change-Id: I140541f1005a24e53e1b1eaa90996d6dada1c3a1
Reviewed-on: https://boringssl-review.googlesource.com/12105
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-08 23:51:10 +00:00
David Benjamin
8e816eb7b6 Treat sessions as expired on the boundary second.
TLS 1.3 clarifies that a ticket lifetime of zero means the session is
unusable. We don't currently pay attention to that field (to be fixed in
later changes) but, in preparation for this, switch the >= to a >.

Change-Id: I0e67a0d97bc8def04914f121e84d3e7a2d640d2c
Reviewed-on: https://boringssl-review.googlesource.com/12102
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-11-08 23:00:04 +00:00
David Benjamin
3c51d9b1b9 Test that session renewals interact with lifetimes correctly.
A renewed session does not refresh the timeout. Add tests for this in
preparation for future changes which will revise this logic.

Specifically, TLS 1.3 draft 18's ticket_age_add logic will require some
tweaks in lifetime tracking to record when the ticket was minted. We'll
also likely wish to tweak the parameters for 1.3 to account for (a)
ECDHE-PSK means we're only worried about expiring a short-circuited
authentication rather than forward secrecy and (b) two hours is too
short for a QUIC 0-RTT replacement.

Change-Id: I0f1edd09151e7fcb5aee2742ef8600fbd7080df6
Reviewed-on: https://boringssl-review.googlesource.com/12002
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-03 21:42:00 +00:00
David Benjamin
abbbee10ad Detach TLS 1.3 cipher configuration from the cipher language.
TLS 1.3 ciphers are now always enabled and come with a hard-coded
preference order.

BUG=110

Change-Id: Idd9cb0d75fb6bf2676ecdee27d88893ff974c4a3
Reviewed-on: https://boringssl-review.googlesource.com/12025
Reviewed-by: Adam Langley <agl@google.com>
2016-11-02 20:47:55 +00:00
Adam Langley
fb73e97292 Test that version is available in the ALPN callback.
HTTP/2 requires TLS 1.2 so the negotiated version should be available
during the ALPN callback.

Change-Id: Iea332808b531a6e5c917de5b8c8917c0aa7428a1
Reviewed-on: https://boringssl-review.googlesource.com/12060
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-02 20:35:08 +00:00
David Benjamin
9ef31f01af Negotiate the cipher suite before ALPN.
HTTP/2 places requirements on the cipher suite. So that servers can
decline HTTP/2 when these requirements aren't met, defer ALPN
negotiation.

See also b/32553041.

Change-Id: Idbcf049f9c8bda06a8be52a0154fe76e84607268
Reviewed-on: https://boringssl-review.googlesource.com/11982
Reviewed-by: Adam Langley <agl@google.com>
2016-11-02 18:06:23 +00:00
David Benjamin
b2e2e32c35 Test that client and server enforce session timeouts.
We were only testing one side.

Change-Id: Ieb755e27b235aaf1317bd2c8e5fb374cb0ecfdb3
Reviewed-on: https://boringssl-review.googlesource.com/12001
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-02 13:53:40 +00:00
David Benjamin
1b22f85a56 Reject tickets from the future.
This shouldn't happen, but it is good to check to avoid the potential
underflow in ssl_session_is_time_valid.

This required tweaking the mock clock in bssl_shim to stop going back in
time.

Change-Id: Id3ab8755139e989190d0b53d4bf90fe1ac203022
Reviewed-on: https://boringssl-review.googlesource.com/11841
Reviewed-by: David Benjamin <davidben@google.com>
2016-10-27 22:32:19 +00:00
David Benjamin
3a322f5e48 Revise signing preferences.
We currently preferentially sign the largest hash available and
advertise such a preference for signatures we accept. We're just as
happy with SHA-256 and, all else equal, a smaller hash would be epsilon
more performant. We also currently claim, in TLS 1.3, we prefer P-384
over P-256 which is off.

Instead order SHA-256 first, next the larger SHA-2 hashes, and leave
SHA-1 at the bottom. Within a hash, order ECDSA > RSA-PSS > RSA-PKCS1.

This has the added consequence that we will preferentially pair P-256
with SHA-256 in signatures we generate instead of larger hashes that get
truncated anyway.

Change-Id: If4aee068ba6829e8c0ef7948f56e67a5213e4c50
Reviewed-on: https://boringssl-review.googlesource.com/11821
Reviewed-by: Adam Langley <agl@google.com>
2016-10-26 17:20:19 +00:00
David Benjamin
3ef7697ed3 Don't accept {sha1, ecdsa} and {sha512, ecdsa}.
{sha1, ecdsa} is virtually nonexistent. {sha512, ecdsa} is pointless
when we only accept P-256 and P-384. See Chromium Intent thread here:

https://groups.google.com/a/chromium.org/d/msg/blink-dev/kWwLfeIQIBM/9chGZ40TCQAJ

This tweaks the signature algorithm logic slightly so that sign and
verify preferences are separate.

BUG=chromium:655318

Change-Id: I1097332600dcaa38e62e4dffa0194fb734c6df3f
Reviewed-on: https://boringssl-review.googlesource.com/11621
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-10-18 19:07:36 +00:00
Steven Valdez
2c62fe9c58 Run TestSequenceNumber at all versions.
There were some logic errors that were revealed by testing at TLS 1.3.
Also explicitly test GetClientHelloLen at TLS 1.2 (rather than relying
on the default) since the TLS 1.3 ClientHello is too large.

Change-Id: I907cb6ac04b40f845e99593bad06739132ca56b2
Reviewed-on: https://boringssl-review.googlesource.com/11605
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-10-18 00:53:14 +00:00
Steven Valdez
3cbdc34619 Add GENERIC selector for TLS 1.3 AEAD-only cipher suites.
Change-Id: Ib499b3393962a4d41cf9694e055ed3eb869d91a2
Reviewed-on: https://boringssl-review.googlesource.com/11504
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-10-06 19:37:40 +00:00
Steven Valdez
803c77a681 Update crypto negotation to draft 15.
BUG=77

Change-Id: If568412655aae240b072c29d763a5b17bb5ca3f7
Reviewed-on: https://boringssl-review.googlesource.com/10840
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
2016-10-06 14:37:09 +00:00
Adam Langley
f99f2448bd Return immediately if a cipher command is invalid.
Breaking from inside the inner loop doesn't do what the code wants.
Instead the outer loop will continue running and it's possible for it to
read off the end of the buffer. (Found with libFuzzer.)

Next change will update the other abort points in this code to match.

Change-Id: I006dca0cd4c31db1c4b5e84b996fe24b2f1e6c13
Reviewed-on: https://boringssl-review.googlesource.com/11421
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-10-03 18:03:08 +00:00
Alessandro Ghedini
5fd1807d95 Implement SSL_CTX_set1_curves_list()
This function is used by NGINX to enable specific curves for ECDH from a
configuration file. However when building with BoringSSL, since it's not
implmeneted, it falls back to using EC_KEY_new_by_curve_name() wich doesn't
support X25519.

Change-Id: I533df4ef302592c1a9f9fc8880bd85f796ce0ef3
Reviewed-on: https://boringssl-review.googlesource.com/11382
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-30 00:45:19 +00:00
David Benjamin
cb18ac2bc3 Add a test for SSL_version.
We were never really testing this.

Change-Id: Ia953870053d16d3994ae48172017d384c7bc3601
Reviewed-on: https://boringssl-review.googlesource.com/11341
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-09-27 18:16:26 +00:00
David Benjamin
e34bcc91c0 Support default versions with set_{min,max}_proto_version.
Upstream makes 0 mean "min/max supported version". Match that behavior,
although call it "default" instead. It shouldn't get you TLS 1.3 until
we're ready to turn it on everywhere.

BUG=90

Change-Id: I9f122fceb701b7d4de2ff70afbc1ffdf370cb97e
Reviewed-on: https://boringssl-review.googlesource.com/11181
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-21 21:41:49 +00:00
David Benjamin
af56fbd62a Renumber TLS 1.3 signature algorithms.
The old numbers violate a MUST-level requirement in TLS 1.2 to not
advertise anonymous (0x0700 ends in 0x00). The spec has been updated
with new allocations which avoid these.

BUG=webrtc:6342

Change-Id: Ia5663ada98fa1ebf0f8a7f50fe74a0e9206c4194
Reviewed-on: https://boringssl-review.googlesource.com/11131
Reviewed-by: Adam Langley <agl@google.com>
2016-09-21 20:54:15 +00:00
David Benjamin
e470690633 Align SSL_set_{min,max}_version with upstream.
Upstream added these functions after we did but decided to change the
names slightly. I'm not sure why they wanted to add the "proto" in
there, but align with them nonetheless so the ecosystem only has one set
of these functions.

BUG=90

Change-Id: Ia9863c58c9734374092051f02952b112806040cc
Reviewed-on: https://boringssl-review.googlesource.com/11123
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-21 20:06:18 +00:00
David Benjamin
2dc0204603 Don't return invalid versions in version_from_wire.
This is in preparation for using the supported_versions extension to
experiment with draft TLS 1.3 versions, since we don't wish to restore
the fallback. With versions begin opaque values, we will want
version_from_wire to reject unknown values, not attempt to preserve
order in some way.

This means ClientHello.version processing needs to be separate code.
That's just written out fully in negotiate_version now. It also means
SSL_set_{min,max}_version will notice invalid inputs which aligns us
better with upstream's versions of those APIs.

This CL doesn't replace ssl->version with an internal-representation
version, though follow work should do it once a couple of changes land
in consumers.

BUG=90

Change-Id: Id2f5e1fa72847c823ee7f082e9e69f55e51ce9da
Reviewed-on: https://boringssl-review.googlesource.com/11122
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-21 19:51:45 +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
Steven Valdez
84b5c003cc Serialize unresumable sessions to a placeholder value.
Change-Id: I676d7fb00d63d74946b96c22ae2705072033c5f7
Reviewed-on: https://boringssl-review.googlesource.com/10620
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-12 19:25:02 +00:00
Matt Braithwaite
d17d74d73f Replace Scoped* heap types with bssl::UniquePtr.
Unlike the Scoped* types, bssl::UniquePtr is available to C++ users, and
offered for a large variety of types.  The 'extern "C++"' trick is used
to make the C++ bits digestible to C callers that wrap header files in
'extern "C"'.

Change-Id: Ifbca4c2997d6628e33028c7d7620c72aff0f862e
Reviewed-on: https://boringssl-review.googlesource.com/10521
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-01 22:22:54 +00:00
David Benjamin
57e929f3c8 Enable RSA-PSS in TLS 1.2 by default.
Add a test that RSA-PSS is available in TLS 1.2 by default, both for
signing and verifying. Note that if a custom SSL_PRIVATE_KEY_METHOD is
used and it sets signing preferences, it won't use RSA-PSS if it doesn't
know about it. (See *-Sign-Negotiate-* tests.)

Change-Id: I3776a0c95480188a135795f7ebf31f2b0e0626cc
Reviewed-on: https://boringssl-review.googlesource.com/10723
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-30 22:50:05 +00:00
David Benjamin
9962057197 Test that the early callback can affect max_version.
The early callback needs to run before even version negotiation has been
resolved.

Change-Id: Ibb449ccec07dedef19b7827400ef318fa2f422c0
Reviewed-on: https://boringssl-review.googlesource.com/10722
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-30 15:56:37 +00:00
Matt Braithwaite
9c8c418853 Remove RC4 ciphersuites from TLS.
For now, they can be restored by compiling with -DBORINGSSL_RC4_TLS.

Of note, this means that `MEDIUM' is now empty.

Change-Id: Ic77308e7bd4849bdb2b4882c6b34af85089fe3cc
Reviewed-on: https://boringssl-review.googlesource.com/10580
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Matt Braithwaite <mab@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-08-26 19:32:44 +00:00
David Benjamin
0fc37ef082 Fix a number of sigalg scope issues.
peer_sigalgs should live on SSL_HANDSHAKE. This both releases a little
bit of memory after the handshake is over and also avoids the bug where
the sigalgs get dropped if SSL_set_SSL_CTX is called at a bad time. See
also upstream's 14e14bf6964965d02ce89805d9de867f000095aa.

This only affects consumers using the old SNI callback and not
select_certificate_cb.

Add a test that the SNI callback works as expected. In doing so, add an
SSL_CTX version of the signing preferences API. This is a property of
the cert/key pair (really just the key) and should be tied to that. This
makes it a bit easier to have the regression test work with TLS 1.2 too.

I thought we'd fixed this already, but apparently not... :-/

BUG=95

Change-Id: I75b02fad4059e6aa46c3b05183a07d72880711b3
Reviewed-on: https://boringssl-review.googlesource.com/10445
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-24 00:24:34 +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
Steven Valdez
4aa154e08f Adding code to send session as PSK Identity.
BUG=75

Change-Id: Ied864cfccbc0e68d71c55c5ab563da27b7253463
Reviewed-on: https://boringssl-review.googlesource.com/9043
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-16 20:35:32 +00:00
David Benjamin
96a16cd10e Finish aligning up_ref functions with OpenSSL 1.1.0.
All external callers should be resolved now.

BUG=89

Change-Id: I6055450e8202c59cca49e4a824be3ec11c32a15a
Reviewed-on: https://boringssl-review.googlesource.com/10285
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 16:51:52 +00:00
David Benjamin
721e8b79a9 Test that servers enforce session timeouts.
Extend the DTLS mock clock to apply to sessions too and test that
resumption behaves as expected.

Change-Id: Ib8fdec91b36e11cfa032872b63cf589f93b3da13
Reviewed-on: https://boringssl-review.googlesource.com/9110
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-03 21:27:07 +00:00
David Benjamin
a20e535fb1 Add a test for session ID context logic.
We almost forgot to handle this in TLS 1.3, so add a test for it.

Change-Id: I28600325d8fb6c09365e909db607cbace12ecac7
Reviewed-on: https://boringssl-review.googlesource.com/9093
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-03 21:20:33 +00:00
David Benjamin
afc64dec74 Add tests to ensure our ClientHello does not change.
We'll need to update it on occasion, but we should not update our
default ClientHello without noticing.

Change-Id: I19ca52fdbe10e3ac14413fecd16be2e58af5a1f6
Reviewed-on: https://boringssl-review.googlesource.com/9075
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-08-02 19:34:18 +00:00
David Benjamin
4501bd5118 Align with OpenSSL on SSL_set_bio behavior.
SSL_set_bio is a nightmare.

In f715c42322, we noticed that, among
other problems, SSL_set_bio's actual behavior did not match how
SSL_set_rfd was calling it due to an asymmetry in the rbio/wbio
handling. This resulted in SSL_set_fd/SSL_set_rfd calls to crash.  We
decided that SSL_set_rfd's believed semantics were definitive and
changed SSL_set_bio.

Upstream, in 65e2d672548e7c4bcb28f1c5c835362830b1745b, decided that
SSL_set_bio's behavior, asymmetry and all, was definitive and that the
SSL_set_rfd crash was a bug in SSL_set_rfd. Accordingly, they switched
the fd callers to use the side-specific setters, new in 1.1.0.

Align with upstream's behavior and add tests for all of SSL_set_bio's
insanity. Also export the new side-specific setters in anticipation of
wanting to be mostly compatible with OpenSSL 1.1.0.

Change-Id: Iceac9508711f79750a3cc2ded081b2bb2cbf54d8
Reviewed-on: https://boringssl-review.googlesource.com/9064
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-02 17:50:39 +00:00
Steven Valdez
87eab4902d Splitting SSL session state.
To prevent configuration/established session confusion, the handshake
session state is separated into the configured session (ssl->session)
and the newly created session (ssl->s3->new_session). Upon conclusion of
the handshake, the finalized session is stored
in (ssl->s3->established_session). During the handshake, any requests
for the session (SSL_get_session) return a non-resumable session, to
prevent resumption of a partially filled session. Sessions should only
be cached upon the completion of the full handshake, using the resulting
established_session. The semantics of accessors on the session are
maintained mid-renego.

Change-Id: I4358aecb71fce4fe14a6746c5af1416a69935078
Reviewed-on: https://boringssl-review.googlesource.com/8612
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-07-29 21:22:46 +00:00
David Benjamin
14a077c353 Revert "Add tests to ensure our ClientHello does not change."
This reverts commit 69f40dff83. I'm not
sure why the CQ didn't catch it while the bots didn't, but I'll look
into it after the QUIC BoF.

Change-Id: Ia187787c86aab082b9cffe0c86c828805dfc212d
Reviewed-on: https://boringssl-review.googlesource.com/8870
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-20 09:18:11 +00:00
David Benjamin
69f40dff83 Add tests to ensure our ClientHello does not change.
We'll need to update it on occasion, but we should not update our
default ClientHello without noticing.

Change-Id: Id9c4734f8e3f8c66b757a82ca123ce949bbcd02e
Reviewed-on: https://boringssl-review.googlesource.com/8845
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-07-20 09:10:37 +00:00
David Benjamin
add5e52d74 Test SSL_get_peer_certificate and SSL_get_peer_cert_chain behavior.
Between client and server, the second API behaves very very differently.

Change-Id: I2a6c3cab717466a2d67ae102810a5ecd99362d9e
Reviewed-on: https://boringssl-review.googlesource.com/8781
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-14 15:52:21 +00:00
David Benjamin
25490f2d08 Add a unit test for SSL_CTX_set_retain_only_sha256_of_client_certs.
Change-Id: I5addfb1e8ec97fc426ae8ca39769120856470451
Reviewed-on: https://boringssl-review.googlesource.com/8780
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-14 15:52:00 +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