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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
The last one was an RC4 cipher and those are gone.
Change-Id: I3473937ff6f0634296fc75a346627513c5970ddb
Reviewed-on: https://boringssl-review.googlesource.com/13108
Reviewed-by: Adam Langley <agl@google.com>
This adds support for setting 0-RTT mode on tickets minted by
BoringSSL, allowing for testing of the initial handshake knowledge.
BUG=76
Change-Id: Ic199842c03b5401ef122a537fdb7ed9e9a5c635a
Reviewed-on: https://boringssl-review.googlesource.com/12740
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>
We repeat this in a bunch of places.
Change-Id: Iee2c95a13e1645453f101d8be4be9ac78d520387
Reviewed-on: https://boringssl-review.googlesource.com/13051
Reviewed-by: David Benjamin <davidben@google.com>
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>
This extension will be used to test whether
https://github.com/tlswg/tls13-spec/pull/762 is deployable against
middleboxes. For simplicity, it is mutually exclusive with 0-RTT. If
client and server agree on the extension, TLS 1.3 records will use the
format in the PR rather than what is in draft 18.
BUG=119
Change-Id: I1372ddf7b328ddf73d496df54ac03a95ede961e1
Reviewed-on: https://boringssl-review.googlesource.com/12684
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>
This is in preparation for implementing 0-RTT where, like
with client_traffic_secret_0, client_handshake_secret must
be derived slightly earlier than it is used. (The secret is
derived at ServerHello, but used at server Finished.)
Change-Id: I6a186b84829800704a62fda412992ac730422110
Reviewed-on: https://boringssl-review.googlesource.com/12920
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>
This removes another dependency on the crypto/x509 code.
Change-Id: Ia72da4d47192954c2b9a32cf4bcfd7498213c0c7
Reviewed-on: https://boringssl-review.googlesource.com/12709
Reviewed-by: Adam Langley <agl@google.com>
So we can report it cleanly out of DevTools, it should behave like
SSL_get_curve_id and be reported on resumption too.
BUG=chromium:658905
Change-Id: I0402e540a1e722e09eaebadf7fb4785d8880c389
Reviewed-on: https://boringssl-review.googlesource.com/12694
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>
This change removes the use of |X509_get_pubkey| from the TLS <= 1.2
code. That function is replaced with a shallow parse of the certificate
to extract the public key instead.
Change-Id: I8938c6c5a01b32038c6b6fa58eb065e5b44ca6d2
Reviewed-on: https://boringssl-review.googlesource.com/12707
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>
This currently only works for certificates parsed from the network, but
if making several connections that share certificates, some KB of memory
might be saved.
BUG=chromium:671420
Change-Id: I1c7a71d84e1976138641f71830aafff87f795f9d
Reviewed-on: https://boringssl-review.googlesource.com/12706
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>
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.
(This is a second attempt at
https://boringssl-review.googlesource.com/#/c/12163/.)
BUG=chromium:671420
Change-Id: I508a8a46cab89a5a3fcc0c1224185d63e3d59cb8
Reviewed-on: https://boringssl-review.googlesource.com/12705
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>
state is now initialized to SSL_ST_INIT in SSL_HANDSHAKE. If there is no
handshake present, we report SSL_ST_OK. This saves 8 bytes of
per-connection post-handshake memory.
Change-Id: Idb3f7031045caed005bd7712bc8c4b42c81a1d04
Reviewed-on: https://boringssl-review.googlesource.com/12697
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>
This avoids needing a extra state around client certificates to avoid
calling the callbacks twice. This does, however, come with a behavior
change: configuring both callbacks won't work. No consumer does this.
(Except bssl_shim which needed slight tweaks.)
Change-Id: Ia5426ed2620e40eecdcf352216c4a46764e31a9a
Reviewed-on: https://boringssl-review.googlesource.com/12690
Reviewed-by: Adam Langley <agl@google.com>
This is to free up the hs->state name for the upper-level handshake
state.
Change-Id: I1183a329f698c56911f3879a91809edad5b5e94e
Reviewed-on: https://boringssl-review.googlesource.com/12695
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>
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>
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>
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>
There is no need to retain it beyond this point.
Change-Id: Ib5722ab30fc013380198b1582d1240f0fe0aa770
Reviewed-on: https://boringssl-review.googlesource.com/12620
Reviewed-by: Adam Langley <agl@google.com>
These too have no reason to be called across files.
Change-Id: Iee477e71f956c2fa0d8817bf2777cb3a81e1c853
Reviewed-on: https://boringssl-review.googlesource.com/12585
Reviewed-by: Adam Langley <agl@google.com>
s3_lib.c is nearly gone. ssl_get_cipher_preferences will fall away once
we remove the version-specific cipher lists. ssl_get_algorithm_prf and
the PRF stuff in general needs some revising (it was the motivation for
all the SSL_HANDSHAKE business). I've left ssl3_new / ssl3_free alone
for now because we don't have a good separation between common TLS/DTLS
connection state and state internal to the TLS SSL_PROTOCOL_METHOD.
Leaving that alone for now as there's lower-hanging fruit.
Change-Id: Idf7989123a387938aa89b6a052161c9fff4cbfb3
Reviewed-on: https://boringssl-review.googlesource.com/12584
Reviewed-by: Adam Langley <agl@google.com>
Each of these functions is called only once, but they're interspersed
between s3_lib.c and ssl_lib.c.
Change-Id: Ic496e364b091fc8e01fc0653fe73c83c47f690d9
Reviewed-on: https://boringssl-review.googlesource.com/12583
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>
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>
The various key schedule cleanups have removed the need for this enum.
Change-Id: I3269aa19b834815926ad56b2d919e21b5e2603fe
Reviewed-on: https://boringssl-review.googlesource.com/12582
Reviewed-by: Adam Langley <agl@google.com>
The remaining direct accesses are in functions which expect to be called
in and out of the handshake. Accordingly, they are NULL-checked.
Change-Id: I07a7de6bdca7b6f8d09e22da11b8863ebf41389a
Reviewed-on: https://boringssl-review.googlesource.com/12343
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>
Change-Id: I84a8ff1d717f3291403f6fc49668c84f89b910da
Reviewed-on: https://boringssl-review.googlesource.com/12342
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>
Change-Id: I5ef0fe5cc3ae0d5029ae41db36e66d22d76f6158
Reviewed-on: https://boringssl-review.googlesource.com/12341
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>
Change-Id: Id8543a88929091eb004a5205a30b483253cdaa25
Reviewed-on: https://boringssl-review.googlesource.com/12319
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>
This removes all explicit ssl->s3->hs access in those files.
Change-Id: I801ca1c894936aecef21e56ec7e7acb9d1b99688
Reviewed-on: https://boringssl-review.googlesource.com/12318
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>
This takes care of many of the explicit ssl->s3->hs accesses.
Change-Id: I380fae959f3a7021d6de9d19a4ca451b9a0aefe5
Reviewed-on: https://boringssl-review.googlesource.com/12317
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>
