Commit Graph

1787 Commits

Author SHA1 Message Date
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
8df6766d01 Support setting per-connection SCT list
Right now the only way to set an SCT list is the per-context function
SSL_CTX_set_signed_cert_timestamp_list. However this assumes that all the
SSLs generated from a SSL_CTX share the same SCT list, which is wrong.

In order to avoid memory duplication in case SSL_CTX has its own list, a
CRYPTO_BUFFER is used for both SSL_CTX and SSL.

Change-Id: Id20e6f128c33cf3e5bff1be390645441be6518c6
Reviewed-on: https://boringssl-review.googlesource.com/13642
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:50:13 +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
023d419eae Test that we tolerate server name acknowledgements.
The SNI extension may be ACKed by the server. This is kind of pointless,
but make sure we cover these codepaths.

Change-Id: I14b25ab865dd6e35a30f11ebc9027a1518bbeed9
Reviewed-on: https://boringssl-review.googlesource.com/13633
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-06 23:18:47 +00:00
Nick Harper
ab20cec1c1 Read 0-RTT data in Bogo.
Change-Id: I878dfb9f5d3736c3ec0d5fa39052cca58932dbb7
Reviewed-on: https://boringssl-review.googlesource.com/12981
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-06 22:36:53 +00:00
Nick Harper
f2511f19b9 Send 0-RTT data in bogo.
Change-Id: I38cd04fa40edde4e4dd31fdc16bbf92985430198
Reviewed-on: https://boringssl-review.googlesource.com/12702
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-06 22:35:45 +00:00
David Benjamin
3f2611a98f Hide SSL struct.
BUG=6

Change-Id: I5383ad230f1fdc54f9536c9922bfbf991401a00c
Reviewed-on: https://boringssl-review.googlesource.com/13632
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-02-06 18:45:50 +00:00
Steven Valdez
2f82a0e51b Don't stash tlsext_hostname in ssl_get_new_session.
ssl_get_new_session would stash a copy of the configured hostname
into the SSL_SESSION on the server. Servers have no reason to
configuring that anyway, but, if one did, we'd leak when filling in
the client-supplied SNI later.

Remove this code and guard against this by remembering to OPENSSL_free
when overwriting that field (although it should always be NULL).

Reported-By: Robert Swiecki <swiecki@google.com>
Change-Id: Ib901b5f82e5cf818060ef47a9585363e05dd9932
Reviewed-on: https://boringssl-review.googlesource.com/13631
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-06 18:42:53 +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
Adam Langley
bdcfd1366f Move the SSL BIO into ssl/ from decrepit/.
This is purely to support curl, which now has HTTPS proxy support that,
sadly, uses the BIO SSL. Don't use the BIO SSL for anything else.

Change-Id: I9ef6c9773ec87a11e0b5a93968386ac4b351986d
Reviewed-on: https://boringssl-review.googlesource.com/13600
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-02-03 21:08:10 +00:00
David Benjamin
daa0539276 Remove an unnecessary TLS 1.3 ClientHello state.
The TLS 1.2 and 1.3 state machines do the exact same thing at the
beginning. Let them process the ClientHello extensions, etc., and
finalize the certificate in common code. Once we start picking
parameters, we begin to diverge. Everything before this point is
arguably part of setting up the configuration, which is
version-agnostic.

BUG=128

Change-Id: I293ea3087ecbc3267bd8cdaa011c98d26a699789
Reviewed-on: https://boringssl-review.googlesource.com/13562
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-02-03 20:03:37 +00:00
David Benjamin
42bfeb3623 Remove an unnecessary TLS 1.2 ClientHello state.
The version negotiation logic was a little bizarrely wedged in the
middle of the state machine. (We don't support server renegotiation, so
have_version is always false here.)

BUG=128

Change-Id: I9448dce374004b92e8bd5172c36a4e0eea51619c
Reviewed-on: https://boringssl-review.googlesource.com/13561
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-02-03 20:01:31 +00:00
David Benjamin
8671c47bd8 Fold ssl3_write_bytes into ssl3_write_app_data.
It has no other callers, now that the handshake is written elsewhere.

Change-Id: Ib04bbdc4a54fc7d01405d9b3f765fa9f186244de
Reviewed-on: https://boringssl-review.googlesource.com/13540
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-02-02 22:23:46 +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
David Benjamin
0b1bb12ce8 Push the SSL_CTX session_timeout zero logic up.
This special-case is almost unexposed (the timeout is initialized to the
default) except if the caller calls SSL_CTX_set_timeout(0). Preserve
that behavior by mapping 0 to SSL_DEFAULT_SESSION_TIMEOUT in
SSL_CTX_set_timeout but simplify the internal state.

Change-Id: Ice03a519c25284b925f1e0cf485f2d8c54dc5038
Reviewed-on: https://boringssl-review.googlesource.com/13502
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-02-02 17:52:07 +00:00
David Benjamin
0efa7592e3 dispatch_alert is not an incidental write.
It is impossible to have to call dispatch_alert when writing application
data. Now that we don't send warning alerts through ssl3_send_alert, all
alerts are closure alerts, which means attempts to write will fail.

This prunes a lot of dead code, avoiding the re-entrancy in the write
path. With that gone, tracking alert_dispatch is much more
straightforward.

BUG=146

Change-Id: Ie5fe677daee71e463d79562f3d2cea822a92581d
Reviewed-on: https://boringssl-review.googlesource.com/13500
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>
2017-02-02 17:49:44 +00:00
David Benjamin
e79fe70be9 Bit-pack SSL_AEAD_CTX's various toggles.
Change-Id: Ibb479a0a739a44d0568e37cdfdb30b30e5410c02
Reviewed-on: https://boringssl-review.googlesource.com/13520
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-02-01 23:37:34 +00:00
David Benjamin
b5c58db9ff TLS 1.3 sessions should not be added to the server session cache.
Fix this and add a test. Otherwise enabling TLS 1.3 will cause a server
to blow through its session cache.

Change-Id: I67edbc468faedfd94a6c30cf842af085a6543b50
Reviewed-on: https://boringssl-review.googlesource.com/13501
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-02-01 23:07:55 +00:00
Adam Langley
c26692cfdd Push the use of X509 upwards, out of |ssl_set_cert|.
This change moves the interface between |X509| and |CRYPTO_BUFFER| a
little further out, towards the API.

Change-Id: I1c014d20f12ad83427575843ca0b3bb22de1a694
Reviewed-on: https://boringssl-review.googlesource.com/13365
Reviewed-by: Adam Langley <agl@google.com>
2017-02-01 20:00:10 +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
Nick Harper
7cd0a978cc Bogo: Send and receive 0.5-RTT data.
Change-Id: I44202457841f06a899e140f78ae8afa7ac720283
Reviewed-on: https://boringssl-review.googlesource.com/12600
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-01 00:04:04 +00:00
David Benjamin
f71036e4e3 Remove ssl_hash_message_t from ssl_get_message.
Move to explicit hashing everywhere, matching TLS 1.2 with TLS 1.3. The
ssl_get_message calls between all the handshake states are now all
uniform so, when we're ready, we can rewire the TLS 1.2 state machine to
look like the TLS 1.3 one. (ssl_get_message calls become an
ssl_hs_read_message transition, reuse_message becomes an ssl_hs_ok
transition.)

This avoids some nuisance in processing the ServerHello at the 1.2 / 1.3
transition.

The downside of explicit hashing is we may forget to hash something, but
this will fail to interop with our tests and anyone else, so we should
be able to catch it.

BUG=128

Change-Id: I01393943b14dfaa98eec2a78f62c3a41c29b3a0e
Reviewed-on: https://boringssl-review.googlesource.com/13266
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-27 23:23:57 +00:00
David Benjamin
1a444daca6 Detach V2ClientHello hashing hack from ssl_hash_message_t.
This is kind of annoying (even new state is needed to keep the layering
right). As part of aligning the read paths of the TLS 1.2 and TLS 1.3
state machine, we'll want to move to states calling
ssl_hash_current_message when the process the message, rather than when
the message is read. Right now the TLS 1.2 optional message story
(reuse_message) depends on all messages preceded by an optional message
using ssl_hash_message. For instance, if TLS 1.2 decided to place
CertificateStatus before ServerKeyExchange, we would not be able to
handle it.

However, V2ClientHello, by being handled in the message layer, relies on
ssl_get_message-driven hashing to replace the usual ClientHello hash
with a hash of something custom. This switches things so rather than
ClientHellos being always pre-hashed by the message layer, simulated
ClientHellos no-op ssl_hash_current_message.

This just replaces one hack with another (V2ClientHello is inherently
nasty), but this hack should be more compatible with future plans.

BUG=128

Change-Id: If807ea749d91e306a37bb2362ecc69b84bf224c9
Reviewed-on: https://boringssl-review.googlesource.com/13265
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-27 23:22:14 +00:00
David Benjamin
276b7e8127 Move optional message type checks out of ssl_get_message.
This aligns the TLS 1.2 state machine closer with the TLS 1.3 state
machine. This is more work for the handshake, but ultimately the
plan is to take the ssl_get_message call out of the handshake (so it is
just the state machine rather than calling into BIO), so the parameters
need to be folded out as in TLS 1.3.

The WrongMessageType-* family of tests should make sure we don't miss
one of these.

BUG=128

Change-Id: I17a1e6177c52a7540b2bc6b0b3f926ab386c4950
Reviewed-on: https://boringssl-review.googlesource.com/13264
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-27 23:15:52 +00:00
Adam Langley
6f07d726c9 Don't up_ref a NULL |CRYPTO_BUFFER|.
If an existing chain had a NULL placeholder for a leaf we could end up
trying to increment its reference count. That results in a crash at
configuration time. Found via the SSL_CTX API fuzzer.

BUG=oss-fuzz:480

Change-Id: I0ddc2cbde2e625015768f1bdc8da625e8a4f05fd
Reviewed-on: https://boringssl-review.googlesource.com/13383
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-27 22:09:49 +00:00
David Benjamin
42e3e191e4 Restore mapping BIO_flush errors to -1.
This was originally changed so that flush_flight could forward BIO_write
errors as-is, but we can and probably should still map BIO_flush errors.
This is unlikely to matter (every relevant BIO likely just has a no-op
flush which returns one), but, e.g., our file BIO returns 0, not -1, on
error.

We possibly should also be mapping BIO_write errors, but I'll leave that
alone for now. It's primarily BIO_read where the BIO return value must
be preserved due to error vs. EOF.

(We probably can just remove the BIO_flush calls altogether, but since
the buffer BIO forwarded the flush signal it would be a user-visible
behavior change to confirm.)

Change-Id: Ib495cc5d043867cf964f99b7ee4535114f7b2230
Reviewed-on: https://boringssl-review.googlesource.com/13367
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-01-27 16:24:19 +00:00
Adam Langley
830f7009eb Rename some single-letter argument names.
(I split this change off to minimise the noise in future diffs that
actually do something meaningful.)

Change-Id: I7a054dcfc90a44ab5bb89b8f46704e5e3410e524
Reviewed-on: https://boringssl-review.googlesource.com/13364
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-01-27 16:21:23 +00:00
Adam Langley
3a2b47ab5b Don't use |X509| objects in |CERT|, by default.
This change converts the |CERT| struct to holding certificates as binary
blobs, rather than in parsed form. The members for holding the parsed
form are still there, however, but are only used as a cache for the
event that someone asks us for a non-owning pointer to the parsed leaf
or chain.

Next steps:
  * Move more functions in to ssl_x509.c
  * Create an X509_OPS struct of function pointers that will hang off
    the |SSL_METHOD| to abstract out the current calls to crypto/x509
    operations.

BUG=chromium:671420

Change-Id: Ifa05d88c49a987fd561b349705c9c48f106ec868
Reviewed-on: https://boringssl-review.googlesource.com/13280
Reviewed-by: Adam Langley <agl@google.com>
2017-01-27 16:21:05 +00:00
David Benjamin
2fe6e227fb Remove mask_a and mask_k from CERT.
This resolves a TODO, trims per-connection memory, and makes more sense.
These masks have nothing to do with certificate configuration.

Change-Id: I783e6158e51f58cce88e3e68dfa0ed965bdc894c
Reviewed-on: https://boringssl-review.googlesource.com/13368
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-01-27 15:44:17 +00:00
David Benjamin
9b16066654 Ignore 0-RTT-capable tickets unless enabled.
Until we've gotten it fully working, we should not mint any of these
SSL_SESSIONs, to avoid constraining future versions of our client code.

Notably, if any of our TLS 1.3 clients today serialized sessions, we
would need to rev the serialization format. Without opting into 0-RTT, a
TLS 1.3 client will create SSL_SESSIONs tagged as 0-RTT-capable but
missing important fields (ALPN, etc.). When that serialized session
makes its way to a future version of our client code, it would disagree
with the server about the ALPN value stored in the ticket and cause
interop failures.

I believe the only client code enabling TLS 1.3 right now is Chrome, and
the window is small, so it should be fine. But fix this now before it
becomes a problem.

Change-Id: Ie2b109f8d158017a6f3b4cb6169050d38a66b31c
Reviewed-on: https://boringssl-review.googlesource.com/13342
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>
2017-01-26 21:29:32 +00:00
Steven Valdez
258508fce1 Adding V2ClientHello counter.
Change-Id: I324743e7d1864fbbb9653209ff93e4da872c8d31
Reviewed-on: https://boringssl-review.googlesource.com/13340
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-01-26 20:32:00 +00:00
Nick Harper
47383aadff Skip over early data in bogo.
Change-Id: Idc93fdca2f1c5c23e4ba48c4efed2edbad1e857b
Reviewed-on: https://boringssl-review.googlesource.com/12521
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-01-26 02:38:56 +00:00
David Benjamin
16315f7cc7 Remove the rest of write_message.
The TLS 1.2 state machine now looks actually much closer to the TLS 1.3
one on the write side. Although the write states still have a BIO-style
return, they don't actually send anything anymore. Only the BIO flush
state does. Reads are still integrated into the states themselves
though, so I haven't made it match TLS 1.3 yet.

BUG=72

Change-Id: I7708162efca13cd335723efa5080718a5f2808ab
Reviewed-on: https://boringssl-review.googlesource.com/13228
Reviewed-by: Adam Langley <agl@google.com>
2017-01-25 23:39:23 +00:00
David Benjamin
0f24bedb05 Rename tls13_prepare_* to tls13_add_*.
The SSL code suffers from needing too many verbs for variations on
writing things without actually writing them. We used to have queuing
the message up to be written to the buffer BIO, writing to the buffer
BIO, and flushing the buffer BIO. (Reading, conversely, has a similar
mess of verbs.)

Now we just have adding to the pending flight and flushing the pending
flight, match the SSL_PROTOCOL_METHOD naming.

BUG=72

Change-Id: I332966928bf13f03dfb8eddd519c2fefdd7f24d4
Reviewed-on: https://boringssl-review.googlesource.com/13227
Reviewed-by: Adam Langley <agl@google.com>
2017-01-25 23:37:30 +00:00
David Benjamin
81b7bc3979 Trim unnecessary TLS 1.3 states.
Large chunks of contiguous messages can now be sent in a row. Notably,
the ServerHello flight involves a number of optional messages which can
now be collapsed into straight-line code.

BUG=72

Change-Id: I1429d22a12401aa0f811a04e495bd5d754c084a4
Reviewed-on: https://boringssl-review.googlesource.com/13226
Reviewed-by: Adam Langley <agl@google.com>
2017-01-25 23:36:19 +00:00
David Benjamin
25ac251a1d Remove write_message from TLS 1.3 handshakes.
BUG=72

Change-Id: I4aad718762925191d85f0a468eeec4aa5d85d1e8
Reviewed-on: https://boringssl-review.googlesource.com/13225
Reviewed-by: Adam Langley <agl@google.com>
2017-01-25 23:36:02 +00:00
David Benjamin
daf207a52a Don't use the buffer BIO in TLS.
On the TLS side, we introduce a running buffer of ciphertext. Queuing up
pending data consists of encrypting the record into the buffer. This
effectively reimplements what the buffer BIO was doing previously, but
this resizes to fit the whole flight.

As part of this, rename all the functions to add to the pending flight
to be more uniform. This CL proposes "add_foo" to add to the pending
flight and "flush_flight" to drain it.

We add an add_alert hook for alerts but, for now, only the SSL 3.0
warning alert (sent mid-handshake) uses this mechanism.  Later work will
push this down to the rest of the write path so closure alerts use it
too, as in DTLS. The intended end state is that all the ssl_buffer.c and
wpend_ret logic will only be used for application data and eventually
optionally replaced by the in-place API, while all "incidental" data
will be handled internally.

For now, the two buffers are mutually exclusive. Moving closure alerts
to "incidentals" will change this, but flushing application data early
is tricky due to wpend_ret. (If we call ssl_write_buffer_flush,
do_ssl3_write doesn't realize it still has a wpend_ret to replay.) That
too is all left alone in this change.

To keep the diff down, write_message is retained for now and will be
removed from the state machines in a follow-up change.

BUG=72

Change-Id: Ibce882f5f7196880648f25d5005322ca4055c71d
Reviewed-on: https://boringssl-review.googlesource.com/13224
Reviewed-by: Adam Langley <agl@google.com>
2017-01-25 23:35:47 +00:00
David Benjamin
1a999cf54d Don't use the buffer BIO in DTLS.
Instead, "writing" a message merely adds it to the outgoing_messages
structure. The code to write the flight then loops over it all and now
shares code with retransmission. The verbs here are all a little odd,
but they'll be fixed in later commits.

In doing so, this fixes a slight miscalculation of the record-layer
overhead when retransmitting a flight that spans two epochs. (We'd use
the encrypted epoch's overhead for the unencrypted epoch.)

BUG=72

Change-Id: I8ac897c955cc74799f8b5ca6923906e97d6dad17
Reviewed-on: https://boringssl-review.googlesource.com/13223
Reviewed-by: Adam Langley <agl@google.com>
2017-01-25 23:35:32 +00:00
David Benjamin
6d50f475e3 Remove support for RSA premaster logging.
This was replaced by the more general CLIENT_RANDOM scheme that records
the master secret. Support was added in Wireshark 1.8.0, released in
June 2012. At this point, ECDHE is sufficiently widely deployed that
anyone that cares about this feature must have upgraded their Wireshark
by now.

Change-Id: I9b708f245ec8728c1999daf91aca663be7d25661
Reviewed-on: https://boringssl-review.googlesource.com/13263
Reviewed-by: David Benjamin <davidben@google.com>
2017-01-25 16:48:35 +00:00
David Benjamin
a772b16f9f Allow dtls_seal_record to work in-place.
This will let us avoid a scratch buffer when assembling DTLS handshake
packets in the write_message-less flow.

BUG=72

Change-Id: I15e78efe3a9e3933c307e599f0043427330f4a9e
Reviewed-on: https://boringssl-review.googlesource.com/13262
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-01-25 16:27:32 +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
5db7c9b8c2 Get OPENSSL_COMPILE_ASSERT working in function bodies.
Change-Id: Ifc28887cbf91c7a80bdaf56e3bf80b2f8cfa7d53
Reviewed-on: https://boringssl-review.googlesource.com/13260
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2017-01-24 21:30:33 +00:00
Adam Langley
d1515a3b0a Move a number of X.509 functions from ssl_lib.c to ssl_x509.c
Eventually, all uses of crypto/x509 will be from ssl_x509.c, but this is
just a start.

Change-Id: I2f38cdcbf18b1f26add0aac10a70af10a79dee0e
Reviewed-on: https://boringssl-review.googlesource.com/13242
Reviewed-by: Adam Langley <agl@google.com>
2017-01-24 17:53:07 +00:00
Adam Langley
03b96d70f9 Remove unused |ssl_parse_x509|.
Change-Id: Id81297add5dcba8b861ca107a57a322df4c41c3d
Reviewed-on: https://boringssl-review.googlesource.com/13241
Reviewed-by: Adam Langley <agl@google.com>
2017-01-24 17:51:04 +00:00
David Benjamin
8d5f9da2e3 Abstract away BIO_flush calls in the handshake.
This is the first part to removing the buffer BIO. The eventual end
state is the SSL_PROTOCOL_METHOD is responsible for maintaining one
flight's worth of messages. In TLS, it will just be a buffer containing
the flight's ciphertext. In DTLS, it's the existing structure for
retransmit purposes. There will be hooks:

- add_message (synchronous)
- add_change_cipher_spec (synchronous)
- add_warning_alert (synchronous; needed until we lose SSLv3 client auth
  and TLS 1.3 draft 18; draft 19 will switch end_of_early_data to a
  handshake message)
- write_flight (BIO; flush_flight will be renamed to this)

This also preserves the exact return value of BIO_flush. Eventually all
the BIO_write calls will be hidden behind BIO_flush to, to be consistent
with other BIO-based calls, preserve the return value.

BUG=72

Change-Id: I74cd23759a17356aab3bb475a8ea42bd2cd115c9
Reviewed-on: https://boringssl-review.googlesource.com/13222
Reviewed-by: Adam Langley <agl@google.com>
2017-01-24 16:16:02 +00:00
Nick Harper
44c1a65760 Run go fmt on bogo code.
Change-Id: I15363a9c9ebb4e08bd9cf45ba2c95368766bb19b
Reviewed-on: https://boringssl-review.googlesource.com/13240
Reviewed-by: David Benjamin <davidben@google.com>
2017-01-24 00:29:38 +00:00
David Benjamin
5b410b6bec Remove unnecessary CBS_get_asn1_element.
EVP_parse_public_key already acts like CBS_get_* in that it peels one
element off and leaves a remainder.

Change-Id: Ic90952785005ed81664a6f46503b13ecd293176c
Reviewed-on: https://boringssl-review.googlesource.com/13045
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2017-01-21 00:50:13 +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
Alessandro Ghedini
958346a5e7 Run select_certificate_cb multiple times
It's not completely clear to me why select_cetificate_cb behaves the way it
does, however not only is it confusing, but it makes assumptions about the
application using BoringSSL (it's not always possible to implement custom
logic outside of the callbacks provided by libssl), that make this callback
somewhat useless.

Case in point, the callback can be used for changing min/max protocol versions
based on per-site policies, and select_certificate_cb is the only place where
SSL_set_min/max_proto_version() can be used (e.g. you can't call them in
cert_cb because it's too late), but the decision on the specific versions to
use might depend on configuration that needs retrieving asynchronously from
over the network, which requires re-running the callback multiple times.

Change-Id: Ia8e151b163628545373e7fd1f327e9af207478a6
Reviewed-on: https://boringssl-review.googlesource.com/13000
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2017-01-20 23:55:50 +00:00