We use the standard Apache 2.0 file header, described in "APPENDIX: How
to apply the Apache License to your work."
This was primarily automated by running:
git ls-tree -r --name-only HEAD | xargs go run ./util/relicense.go
See go/boringssl-relicensing-triage for the results of triaging the
output of the tool.
As part of this, switch from taking fiat-crypto under MIT license to
Apache 2.0. (It is licensed under MIT OR Apache-2.0 OR BSD-1-Clause.)
The copyright_summary tool can also be used to confirm we didn't
accidentally drop any copyright lines:
# Run before the CL
git grep -l Copyright | xargs go run ./util/copyright_summary.go -out /tmp/old.json
# Run after the CL
git grep -l Copyright | xargs go run ./util/copyright_summary.go -compare /tmp/old.json
Bug: 364634028
Change-Id: I17c50e761e9d077a1f92e25969e50ed35e320c59
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/75852
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This CL addresses a number of issues:
1. If an ACK or retransmit hit SSL_ERROR_WANT_WRITE, we do not provide a
way for the caller to retry it.
2. If the retransmit timer fires too many times, we do not track the
fatal error. Instead, we return an error out of the
DTLSv1_handle_timeout and leave the timeout unhandled. If the caller
does not tear down the connection in response to
DTLSv1_handle_timeout failing, they may register a timeout for 0s,
wake up again and loop forever.
This happened in https://crbug.com/42224241. I filed
https://crbug.com/42290595 to track fixing this API wart.
3. When the DTLS open_app_data hook needs to retransmit or send an ACK,
we write immediately in that callback, but the eventual aim was to
provide an in-place encrypt/decrypt API, so those hooks should not
perform I/O.
DTLS 1.3 adds a lot more cases where we'll want to do this:
- We might read a message and then want to ACK on a timer
- We might read a message and then want to ACK immediately
- We might read a past retransmit and then want to retransmit
the next flight
- We might read an ACK, clear the current flight, and then want to
send a queued up KeyUpdate that was waiting on it.
To address all these, this CL rearranges things slightly. Now the DTLS
record layer keeps track of whether it owes the peer an ACK and a
flight. If so, the flush() operation will try to write those out. If it
fails, it keeps track of how far it got. If it succeeds, it clears those
flags.
Various operations can set those flags, notably the timeouts. This means
the actual "handle timeout" part of DTLSv1_handle_timeout is
infallible. Handling a timeout means cashing the timeout expiry in for
setting the flag.
From there, we drive flush() to completion where we need to. For now,
I've added it to the handshake and SSL_read(), though I'm not sure if
I've gotten exactly the right spots. (Should we also flush on
SSL_write?) We could also go further and have flush() drive
handle_timeout(), and then handle_timeout() can be deprecated
altogether. The model is just "you must retry this operation after the
timeout, in addition to the SSL_ERROR_WANT_WHATEVER you got". That's
quite attractive, but for now I haven't quite gone that far.
Another complication is that flush() currently behaves very differently
between TLS and DTLS. In DTLS, I had to add an explicit finish_flight()
to queue up the flight for sending. If you leave things in there, it's
no big deal because everything in DTLS is unordered. In TLS, everything
is ordered, so once we've encrypted records, we have to flush them
regardless. To that end, in TLS we generally like to flush things
implicitly on write, because that's when the caller is expecting I/O
anyway, and we can concatenate the, say, encrypted KeyUpdate with the
encrypted application data. Not sure what's best to do there. For now
I've just gated flush() calls on SSL_is_dtls().
I'm sure we'll find this also isn't quite right and rearrange it again
later, but hopefully this works a bit better than what we had before.
Bug: 42290594, 376718254
Fixed: 42290595
Change-Id: I4d9b5c753530b514a1bc5e4e69ddb25dfc8c1d06
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/73527
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
This does not yet implement sending ACKs in general, just the one
immediate ACK when the handshake completes. The general case will
require scheduling an ACK-send timer, but this one can be sent
immediately.
One interesting case to test is when the server would like to ACK
Finished, but cannot because the records were merged with fragments that
we had to discard.
Bug: 42290594
Change-Id: I64b3f8ecbef4ffee68d923f83ea89d2349847f8b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/72867
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Nick Harper <nharper@chromium.org>
In DTLS 1.2, we install the new read epoch as soon as we change keys,
and immediately stop accepting data from the previous epoch. This is
fine because there is only one key change, and we don't need to receive
data from the previous epoch at that point. (Even if we used previous
flights to trigger retransmits---which we probably should but don't---I
believe all DTLS 1.2 key changes are such that the previous flight will
contain *some* message from the epoch we expect and we can key on that.)
In DTLS 1.3, things are different. The peer's KeyUpdates do not apply
until we send an ACK, but that ACK might be lost. So that means, at
least for KeyUpdate, we must stage the new epoch and only apply it
later.
During the handshake, this comes up in two cases:
- A server might send ServerHello..Finished but some of ServerHello (or
the whole flight) is lost. The server is now expecting
Certificate..Finished (epoch 2), but the client cannot transition to
epoch 2 without the ServerHello. The client will then try to induce a
retransmit by either sending an ACK or resending the ClientHello. The
client must parse epoch 0 to be able to catch this. This is nice
but not critical, because the server also has a retransmit timer.
- A client might send Certificate..Finished but it was all lost. The
client is now done with the handshake (epoch 3), but the server
cannot complete. The server will then retransmit its
ServerHello..Finished flight or send an ACK. The ACK will actually
come at epoch 3, so we don't need epoch 2 in that case. But the
retransmit is purely in epochs 0 and 2, so the client must parse
epoch 2 to catch this. This is also not critical because the
client is expected to keep its retransmit timer running after the
handshake (still TODO).
To support all this, new DTLS 1.3 read epochs are staged until we get a
record.
This means that record processing must now account for receiving records
at more epochs, including some invalid cases. Specifically:
- We might receive new handshake messages at the current epoch, even
though the current epoch is being closed. This is an error.
- We might receive application data at epoch 2 even though we've
finished the handshake and expect it at epoch 3. This is an error.
I've added checks for both of these, with tests. This also resolves a
few TODOs. The app data check will also help with 0-RTT, where epochs 1
and 2 will flow concurrently. (We'll still need an inverse check that
epoch 1 never carries handshake data.) It also once again changes the
broken SSL_get_read_sequence API from one kind of broken to another kind
of broken, so I've updated the comment to explain the current state.
As part of this, stop tracking read_level and write_level outside of
QUIC. It's only used in QUIC and the semantics are a little weird when
we defer activating new read epochs.
(I'm not sure whether this behavior is really necessary for epochs
before epoch 3. The only case it accomodates is the client using epoch 0
to induce a retransmit when ServerHello is lost. The impact it has on
SSL_get_read_sequence immediately after the handshake is kind of weird,
so maybe we want to special case it a bit. I can't think of any case
where we'd want to defer installing epoch 3.)
Bug: 42290594
Change-Id: Id222de2b77ff8e68e64f301f32d8c2b95ad3e7a7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/72450
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
This wraps DTLS per-epoch state into DTLSReadEpoch and DTLSWriteEpoch
structures. For now, we only keep one DTLSReadEpoch though we will need
to keep two in DTLS 1.3. In preparation for that, I've reworked the DTLS
record parser to resolve the DTLSReadEpoch at the same time, even though
there's currently only one of them.
On the write side, this removes the special-cased initial write epoch
and just stores an array of the last few epochs.
Some things this does not yet do, but that we ideally would do as
follow-ups:
1. Move RecordNumberEncrypter out of SSLAEADContext, now that we have a
DTLS-specific struct for it.
2. Pass just a byte secret into set_read_state / set_write_state and let
the transport-aware code construct the SSLAEADContext.
3. Don't construct the SSLAEADContext at all for QUIC.
4. Don't save the read and write traffic secrets in QUIC at all.
5. KeyUpdate should be part of the SSL_PROTOCOL_METHOD interface to
accomodate DTLS driving KeyUpdate by ACK.
Update-Note: As part of rearranging the record parser, when the DTLS 1.2
implementation encounters a DTLS 1.3 record, it will now discard just
that record and continue parsing records out of the packet, rather than
discarding the whole packet. This isn't expected to make any difference.
Bug: 371998381
Change-Id: Ie2ae657d41e33152208a001df177630398798394
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/72128
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Nick Harper <nharper@chromium.org>
We spend a lot of effort implementing a big-endian sequence number
update, etc., when the sequence number is just a 64-bit counter. (Or
48-bit counter in DTLS because we currently retain the epoch
separately. We can probably tidy that a bit too, but I'll leave that
for later. Right now the DTLS record layer state is a bit entwined
with the TLS one.)
Just store it as uint64_t. This should also simplify
https://boringssl-review.googlesource.com/c/boringssl/+/54325 a little.
Change-Id: I95233f924a660bc523b21496fdc9211055b75073
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54505
Reviewed-by: Bob Beck <bbe@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
See go/handshake-hints (internal).
CL originally by Bin Wu <wub@google.com>. I just reworked the tests and
tidied it up a bit. This is the start of a replacement for the split
handshakes API. For now, only TLS 1.3 is supported. It starts with an
initial set of hints, but we can add more later. (In particular, we
should probably apply the remote handshaker's extension order to avoid
needing to capability protect such changes.)
Change-Id: I7b6a6dfaa84c6c6e3436d2a4026c3652b8a79f0f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46535
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
set_write_state flushes buffered handshake data, and we should finish
writing to a level before moving on to the next one.
I've moved the callback into set_{read,write}_state to ensure we still
update read_level and write_level after installing secrets, since that's
how we decide what level to write things and we should never write
alerts with keys we don't have. (I believe the only way this can come up
is if the QUIC callback itself fails, but it still seems prudent to
defer updating the levels.)
This does unfortunately mean a goofy secret_for_quic parameter, though
it is arguably more "correct" in that QUIC would ideally be a third
SSL_PROTOCOL_METHOD, rather than escape hatches over TLS. Probably a
cleaner abstraction would be for set_read_state and set_write_state to
take the secret and derive an SSLAEADContext internally.
Update-Note: See b/151142920#comment9
Change-Id: I4bbb76e15b5d95615ea643bccf796db87fae4989
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/40244
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
This is a little bit of internal cleanup. The original intent was so
QUIC could install secrets in set_(read|write)_state, but that was
somewhat annoying, so I've left it just before the call for now.
There is one TLS 1.3 state transition which doesn't carry an encryption
level: switching from 0-RTT keys back to unencrypted on an HRR-based
0-RTT reject. The TCP code doesn't care about write_level and the QUIC
code is currently fine because we never "install" the 0-RTT keys. But we
should get this correct.
This also opens the door for DTLS 1.3, if we ever implement it, because
DTLS 1.3 will need to know which level it is to handle 0-RTT keys funny.
(Clients sending 0-RTT will briefly have handshake and 0-RTT write keys
active simultaneously.)
QUIC has the same property, but we can fudge it because only the caller
is aware of this.
Change-Id: Ia76d787e1b96a058d9818948b6d9a051e8592207
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/40124
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The TLS handshake runs over a sequence of messages, serialized onto a
stream, which is then packetized into records, with no correlation to
message boundaries.
TLS messages may span records, so a TLS implementation will buffer up
excess data in a record for the next message. If not checked, that next
message may a round-trip or even a key change later. Carrying data
across a key change has security consequences, so we reject any excess
data across key changes (see ChangeCipherSpec synchronization tests and
(d)tls_set_read_state). However, we do not currently check it across
network round trips that do not change keys.
For instance, a TLS 1.2 client may pack part of ClientKeyExchange (the
first byte, at least, is deterministic) into the ClientHello record,
before even receiving ServerHello. Most TLS implementations will accept
this.
However, the handback logic does *not* serialize excess data in hs_buf.
There shouldn't be any, but if the peer is doing strange things as
above, that data will get silently dropped. The way TLS 1.3 0-RTT
handback logic works (the key isn't installed until after handback),
this data is even silently dropped though there is a key change.
To keep all our behavior consistent, check for unprocessed handshake
data at the end of each flight and reject it. Add a bunch of tests.
Update-Note: If the peer packs data across handshake flights, or packs
HelloRequest into the same record as Finished, this will now be an
error. (The former is pathologically odd behavior. The latter is also
rejected by Schannel and also odd.)
Change-Id: I9412bbdea09ee7fdcfeb78d3456329505a190641
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/39987
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Some of the TLS-specific functions begin with ssl3_, otherwise with
tls_. Align on tls_ since we don't implement SSL 3.0 anymore. (Plain ssl_
means common to TLS and DTLS, which is an odd backronym, but SSL_foo for
the APIs are thoroughly stuck.)
Change-Id: Ib7acffd21ee370bb9bed46789fb511d00fac24ca
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/39985
Reviewed-by: Adam Langley <agl@google.com>
Upstream did this in 7644a9aef8932ed4d1c3f25ed776c997702982be, so align
with them. Add the new OPENSSL_* names and switch all callers witihn the
library to match. Keep the old BUF_* names around for compatibility.
Note there were two functions where we already had an OPENSSL_* version:
OPENSSL_strdup and OPENSSL_strnlen. The former now gains a NULL check to
align with BUF_strdup. The latter gets deduplicated; we had two
implementations.
Change-Id: Ia1cd4527a752fcd62e142ed1e1d7768d323279ba
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/38425
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This was added to support the no_certificate warning alert in SSLv3. That has
since been removed. In the long run, I would like for ssl_send_alert to go
through a flow similar to add_alert so the BIO-free APIs work right and avoid a
host of strangeness surrounding wpend_buf. For now, remove the unused hook.
Change-Id: I1995028b8af4ffa836028794e6b33b2cd1b2435b
Reviewed-on: https://boringssl-review.googlesource.com/31984
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>
- In base.h, if BORINGSSL_PREFIX is defined, include
boringssl_prefix_symbols.h
- In all .S files, if BORINGSSL_PREFIX is defined, include
boringssl_prefix_symbols_asm.h
- In base.h, BSSL_NAMESPACE_BEGIN and BSSL_NAMESPACE_END are
defined with appropriate values depending on whether
BORINGSSL_PREFIX is defined; these macros are used in place
of 'namespace bssl {' and '}'
- Add util/make_prefix_headers.go, which takes a list of symbols
and auto-generates the header files mentioned above
- In CMakeLists.txt, if BORINGSSL_PREFIX and BORINGSSL_PREFIX_SYMBOLS
are defined, run util/make_prefix_headers.go to generate header
files
- In various CMakeLists.txt files, add "global_target" that all
targets depend on to give us a place to hook logic that must run
before all other targets (in particular, the header file generation
logic)
- Document this in BUILDING.md, including the fact that it is
the caller's responsibility to provide the symbol list and keep it
up to date
- Note that this scheme has not been tested on Windows, and likely
does not work on it; Windows support will need to be added in a
future commit
Change-Id: If66a7157f46b5b66230ef91e15826b910cf979a2
Reviewed-on: https://boringssl-review.googlesource.com/31364
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
We have a successful TLS 1.3 deployment, in spite of non-compliant
middleboxes everywhere, so now let's get this optimization in. It would
have been nice to test with this from the beginning, but sadly we forgot
about it. Ah well. This shaves 63 bytes off the server's first flight,
and then another 21 bytes off the pair of NewSessionTickets.
So we'll more easily notice in case of anything catastrophic, tie this
behavior to draft 28.
Update-Note: This slightly tweaks our draft-28 behavior.
Change-Id: I4f176a919bf7181239d6ebb31e7870f12364e0f9
Reviewed-on: https://boringssl-review.googlesource.com/28744
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
|SSL_CONFIG| is a container for bits of configuration that are
unneeded after the handshake completes. By default it is retained for
the life of the |SSL|, but it may be shed at the caller's option by
calling SSL_set_shed_handshake_config(). This is incompatible with
renegotiation, and with SSL_clear().
|SSL_CONFIG| is reachable by |ssl->config| and by |hs->config|. The
latter is always non-NULL. To avoid null checks, I've changed the
signature of a number of functions from |SSL*| arguments to
|SSL_HANDSHAKE*| arguments.
When configuration has been shed, setters that touch |SSL_CONFIG|
return an error value if that is possible. Setters that return |void|
do nothing.
Getters that request |SSL_CONFIG| values will fail with an |assert| if
the configuration has been shed. When asserts are compiled out, they
will return an error value.
The aim of this commit is to simplify analysis of split-handshakes by
making it obvious that some bits of state have no effects beyond the
handshake. It also cuts down on memory usage.
Of note: |SSL_CTX| is still reachable after the configuration has been
shed, and a couple things need to be retained only for the sake of
post-handshake hooks. Perhaps these can be fixed in time.
Change-Id: Idf09642e0518945b81a1e9fcd7331cc9cf7cc2d6
Bug: 123
Reviewed-on: https://boringssl-review.googlesource.com/27644
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
SSLv3_method, SSLv3_client_method, and SSLv3_server_method produce
SSL_CTXs which fail every handshake. They appear no longer necessary for
compatibility, so remove them.
SSLv3 is still accessible to callers who explicitly re-enable SSLv3 on a
TLS_method, but that will be removed completely later this year.
Meanwhile, clear out a weird hack we had here.
Update-Note: I believe there are no more callers of these functions. Any
that were were already non-functional as these methods haven't been
unable to handshake for a while now.
Change-Id: I622f785b428ab0ceab77b5a9db05b2b0df28145a
Reviewed-on: https://boringssl-review.googlesource.com/26004
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>
This reverts commit 75d43b5785. Chatting
with EKR, there is some reason to believe that doing this might cause
more middlebox issues. Since we're still in the middle of working
towards viable deployment in the first place, revert this.
We can experiment with this later. I should have arranged for this to be
controlled more carefully anyway.
Change-Id: I0c8bf578f9d7364e913894e1bf3c2b8123dfd770
Reviewed-on: https://boringssl-review.googlesource.com/22204
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>
This does not affect TLS 1.2 (beyond Channel ID or NPN) but, in TLS 1.3,
we send several encrypted handshake messages in a row. For the server,
this means 66 wasted bytes in TLS 1.3. Since OpenSSL has otherwise used
one record per message since the beginning and unencrypted overhead is
less interesting, leave that behavior as-is for the time being. (This
isn't the most pressing use of the breakage budget.) But TLS 1.3 is new,
so get this tight from the start.
Change-Id: I64dbd590a62469d296e1f10673c14bcd0c62919a
Reviewed-on: https://boringssl-review.googlesource.com/22068
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
While a fairly small hook, open_close_notify is pretty weird. It
processes things at the record level and not above. Notably, this will
break if it skips past a TLS 1.3 KeyUpdate.
Instead, it can share the core part of SSL_read/SSL_peek, with slight
tweaks to post-handshake processing. Note this does require some tweaks
to that code. Notably, to retain the current semantics that SSL_shutdown
does not call funny callbacks, we suppress tickets.
Change-Id: Ia0cbd0b9f4527f1b091dd2083a5d8c7efb2bac65
Reviewed-on: https://boringssl-review.googlesource.com/21885
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
This removes the last place where non-app-data hooks leave anything
uncomsumed in rrec. (There is still a place where non-app-data hooks see
a non-empty rrec an entrance. read_app_data calls into read_handshake.
That'll be fixed in a later patch in this series.)
This should not change behavior, though some error codes may change due
to some processing happening in a slightly different order.
Since we do this in a few places, this adds a BUF_MEM_append with tests.
Change-Id: I9fe1fc0103e47f90e3c9f4acfe638927aecdeff6
Reviewed-on: https://boringssl-review.googlesource.com/21345
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
These are common between TLS and DTLS so should not have the ssl3_
prefix. (TLS-only stuff should really have a tls_ prefix, but we still
have a lot of that one.)
This also fixes a stray reference to ssl3_send_client_key_exchange..
Change-Id: Ia05b360aa090ab3b5f075d5f80f133cbfe0520d4
Reviewed-on: https://boringssl-review.googlesource.com/21346
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Rather than init_msg/init_num, there is a get_message function which
either returns success or try again. This function does not advance the
current message (see the previous preparatory change). It only completes
the current one if necessary.
Being idempotent means it may be freely placed at the top of states
which otherwise have other asychronous operations. It also eases
converting the TLS 1.2 state machine. See
https://docs.google.com/a/google.com/document/d/11n7LHsT3GwE34LAJIe3EFs4165TI4UR_3CqiM9LJVpI/edit?usp=sharing
for details.
The read_message hook (later to be replaced by something which doesn't
depend on BIO) intentionally does not finish the handshake, only "makes
progress". A follow-up change will align both TLS and DTLS on consuming
one handshake record and always consuming the entire record (so init_buf
may contain trailing data). In a few places I've gone ahead and
accounted for that case because it was more natural to do so.
This change also removes a couple pointers of redundant state from every
socket.
Bug: 128
Change-Id: I89d8f3622d3b53147d69ee3ac34bb654ed044a71
Reviewed-on: https://boringssl-review.googlesource.com/18806
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>
WebRTC will need this (probably among other things) to lose crypto/x509
at some point.
Bug: chromium:706445
Change-Id: I988e7300c4d913986b6ebbd1fa4130548dde76a4
Reviewed-on: https://boringssl-review.googlesource.com/18904
Reviewed-by: David Benjamin <davidben@google.com>
With on_handshake_complete, this can be managed internally by the TLS
code. The next commit will add a ton more calls to this function.
Change-Id: I91575d3e4bfcccbbe492017ae33c74b8cc1d1340
Reviewed-on: https://boringssl-review.googlesource.com/18865
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Instead, the DTLS driver can detect these states implicitly based on
when we write flights and when the handshake completes. When we flush a
new flight, the peer has enough information to send their reply, so we
start a timer. When we begin assembling a new flight, we must have
received the final message in the peer's flight. (If there are
asynchronous events between, we may stop the timer later, but we may
freely stop the timer anytime before we next try to read something.)
The only place this fails is if we were the last to write a flight,
we'll have a stray timer. Clear it in a handshake completion hook.
Change-Id: I973c592ee5721192949a45c259b93192fa309edb
Reviewed-on: https://boringssl-review.googlesource.com/18864
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>
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>
This adds several utilities as replacements for new and delete and makes
bssl::UniquePtr work with our private types.
Later work can convert more incrementally. I did this one more
aggressively to see how it'd work. Unfortunately, in doing so, I needed
to remove the NULL SSL_AEAD_CTX "method" receiver trick to appease
clang. The null cipher is now represented by a concrete SSL_AEAD_CTX.
The long-lived references to SSL_AEAD_CTX are not yet in types with
constructors, so they still bare Delete rather than UniquePtr for now.
Though this does mean we may be able to move the sequence number into
SSLAEADContext later which is one less object for DTLS to carry around.
Bug: 132
Change-Id: I506b404addafb692055d5709b0ca6d5439a4e6be
Reviewed-on: https://boringssl-review.googlesource.com/18164
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
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>
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>
David Benjamin