The existing tests only went monotonic. Allow an arbitrary mapping
function. Also test by sending more app data. The handshake is fairly
resilient to replayed packets, whereas our test code intentionally
isn't.
Change-Id: I0fb74bbacc260c65ec5f6a1ca8f3cb23b4192855
Reviewed-on: https://boringssl-review.googlesource.com/5556
Reviewed-by: Adam Langley <agl@google.com>
We shouldn't have protocol constraints that are sensitive to whether
data is returned synchronously or not.
Per https://boringssl-review.googlesource.com/#/c/4112/, the original
limitation was to avoid OpenSSL ABI changes. This is no longer a
concern.
Add tests for the sync and async case. Send the empty records in two
batches to ensure the count is reset correctly.
Change-Id: I3fee839438527e71adb83d437879bb0d49ca5c07
Reviewed-on: https://boringssl-review.googlesource.com/5040
Reviewed-by: Adam Langley <agl@google.com>
Now that WebRTC honors packet boundaries (https://crbug.com/447431), we
can start enforcing them correctly. Configuring read-ahead now does
nothing. Instead DTLS will always set "read-ahead" and also correctly
enforce packet boundaries when reading records. Add tests to ensure that
badly fragmented packets are ignored. Because such packets don't fail
the handshake, the tests work by injecting an alert in the front of the
handshake stream and ensuring the DTLS implementation ignores them.
ssl3_read_n can be be considerably unraveled now, but leave that for
future cleanup. For now, make it correct.
BUG=468889
Change-Id: I800cfabe06615af31c2ccece436ca52aed9fe899
Reviewed-on: https://boringssl-review.googlesource.com/4820
Reviewed-by: Adam Langley <agl@google.com>
This drops in a copy of a subset of golang.org/x/crypto/poly1305 to implement
Poly1305. Hopefully this will keep them from regression as we rework the record
layer.
Change-Id: Ic1e0d941a0a9e5ec260151ced8acdf9215c4b887
Reviewed-on: https://boringssl-review.googlesource.com/4257
Reviewed-by: Adam Langley <agl@google.com>
Partly inspired by the new state exposed in
dc3da938992d209a3b36acbd9695cfcab1fdf041, stress this codepath by spamming our
poor shim with warning alerts.
Change-Id: I876c6e52911b6eb57493cf3e1782b37ea96d01f8
Reviewed-on: https://boringssl-review.googlesource.com/4112
Reviewed-by: Adam Langley <agl@google.com>
We have test coverage for invalid alerts, but not for normal ones on the DTLS
side.
Change-Id: I359dce8d4dc80dfa99b5d8bacd73f48a8e4ac310
Reviewed-on: https://boringssl-review.googlesource.com/3291
Reviewed-by: Adam Langley <agl@google.com>
The check on the DTLS side was broken anyway. On the TLS side, the spec does
say to ignore them, but there should be no need for this in future-proofing and
NSS doesn't appear to be lenient here. See also
https://boringssl-review.googlesource.com/#/c/3233/
Change-Id: I0846222936c5e08acdcfd9d6f854a99df767e468
Reviewed-on: https://boringssl-review.googlesource.com/3290
Reviewed-by: Adam Langley <agl@google.com>
For now, only test reorderings when we always or never fragment messages.
There's a third untested case: when full messages and fragments are mixed. That
will be tested later after making it actually work.
Change-Id: Ic4efb3f5e87b1319baf2d4af31eafa40f6a50fa6
Reviewed-on: https://boringssl-review.googlesource.com/3216
Reviewed-by: Adam Langley <agl@google.com>
This extends the packet adaptor protocol to send three commands:
type command =
| Packet of []byte
| Timeout of time.Duration
| TimeoutAck
When the shim processes a Timeout in BIO_read, it sends TimeoutAck, fails the
BIO_read, returns out of the SSL stack, advances the clock, calls
DTLSv1_handle_timeout, and continues.
If the Go side sends Timeout right between sending handshake flight N and
reading flight N+1, the shim won't read the Timeout until it has sent flight
N+1 (it only processes packet commands in BIO_read), so the TimeoutAck comes
after N+1. Go then drops all packets before the TimeoutAck, thus dropping one
transmit of flight N+1 without having to actually process the packets to
determine the end of the flight. The shim then sees the updated clock, calls
DTLSv1_handle_timeout, and re-sends flight N+1 for Go to process for real.
When dropping packets, Go checks the epoch and increments sequence numbers so
that we can continue to be strict here. This requires tracking the initial
sequence number of the next epoch.
The final Finished message takes an additional special-case to test. DTLS
triggers retransmits on either a timeout or seeing a stale flight. OpenSSL only
implements the former which should be sufficient (and is necessary) EXCEPT for
the final Finished message. If the peer's final Finished message is lost, it
won't be waiting for a message from us, so it won't time out anything. That
retransmit must be triggered on stale message, so we retransmit the Finished
message in Go.
Change-Id: I3ffbdb1de525beb2ee831b304670a3387877634c
Reviewed-on: https://boringssl-review.googlesource.com/3212
Reviewed-by: Adam Langley <agl@google.com>
This reverts commit c67a3ae6ba. With a
deterministic clock, we can now go back to being strict about retransmits. Our
tests will now require that the shim only retransmit when we expect it to.
Change-Id: Iab1deb9665dcd294790c8253d920089e83a9140c
Reviewed-on: https://boringssl-review.googlesource.com/3211
Reviewed-by: Adam Langley <agl@google.com>
BoringSSL currently retransmits non-deterministically on an internal timer
(rather than one supplied externally), so the tests currently fail flakily
depending on timing. Valgrind is a common source for this. We still assume an
in-order and reliable channel, but drop retransmits silently:
- Handshake messages may arrive with old sequence numbers.
- Retransmitted CCS records arrive from the previous epoch.
- We may receive a retransmitted Finished after we believe the handshake has
completed. (Aside: even in a real implementation, only Finished is possible
here. Even with out-of-order delivery, retransmitted or reordered messages
earlier in the handshake come in under a different epoch.)
Note that because DTLS renego and a Finished retransmit are ambiguous at the
record layer[*], this precludes us writing tests for DTLS renego. But DTLS
renego should get removed anyway. As BoringSSL currently implements renego,
this ambiguity is also a source of complexity in the real implementation. (See
the SSL3_MT_FINISHED check in dtls1_read_bytes.)
[*] As a further fun aside, it's also complex if dispatching renego vs Finished
after handshake message reassembly. The spec doesn't directly say the sequence
number is reset across renegos, but it says "The first message each side
transmits in /each/ handshake always has message_seq = 0". This means that such
an implementation needs the handshake message reassembly logic be aware that a
Finished fragment with high sequence number is NOT an out-of-order fragment for
the next handshake.
Change-Id: I35d13560f82bcb5eeda62f4de1571d28c818cc36
Reviewed-on: https://boringssl-review.googlesource.com/2770
Reviewed-by: Adam Langley <agl@google.com>
The record-layer version of the ServerHello should match the final version. The
record-layer version of the ClientHello should be the advertised version, but
clamped at TLS 1.0. This is to ensure future rewrites do not regress this.
Change-Id: I96f1f0674944997ff38b562453a322ce61652635
Reviewed-on: https://boringssl-review.googlesource.com/2540
Reviewed-by: Adam Langley <agl@google.com>
Just the negotiation portion as everything else is external. This feature is
used in WebRTC.
Change-Id: Iccc3983ea99e7d054b59010182f9a56a8099e116
Reviewed-on: https://boringssl-review.googlesource.com/2310
Reviewed-by: Adam Langley <agl@google.com>
Prior to this change, BoringSSL maintained a 2-byte buffer for alerts,
and would support reassembly of fragmented alerts.
NSS does not support fragmented alerts, nor would any reasonable
implementation produce them. Remove fragmented alert handling and
produce an error if a fragmented alert has ever been encountered.
Change-Id: I31530ac372e8a90b47cf89404630c1c207cfb048
Reviewed-on: https://boringssl-review.googlesource.com/2125
Reviewed-by: Adam Langley <agl@google.com>
At the record layer, DTLS maintains a window of seen sequence numbers to detect
replays. Add tests to cover that case. Test both repeated sequence numbers
within the window and sequence numbers past the window's left edge. Also test
receiving sequence numbers far past the window's right edge.
Change-Id: If6a7a24869db37fdd8fb3c4b3521b730e31f8f86
Reviewed-on: https://boringssl-review.googlesource.com/2221
Reviewed-by: Adam Langley <agl@google.com>
This change adds support to the Go code for renegotiation as a client,
meaning that we can test BoringSSL's renegotiation as a server.
Change-Id: Iaa9fb1a6022c51023bce36c47d4ef7abee74344b
Reviewed-on: https://boringssl-review.googlesource.com/2082
Reviewed-by: Adam Langley <agl@google.com>
Run against openssl s_client and openssl s_server. This seems to work for a
start, although it may need to become cleverer to stress more of BoringSSL's
implementation for test purposes.
In particular, it assumes a reliable, in-order channel. And it requires that
the peer send handshake fragments in order. Retransmit and whatnot are not
implemented. The peer under test will be expected to handle a lossy channel,
but all loss in the channel will be controlled. MAC errors, etc., are fatal.
Change-Id: I329233cfb0994938fd012667ddf7c6a791ac7164
Reviewed-on: https://boringssl-review.googlesource.com/1390
Reviewed-by: Adam Langley <agl@google.com>
Also change MaxHandshakeRecordLength to 1 in the handshake coverage tests to
better stress the state machine.
Change-Id: I27fce2c000b3d4818fd2e9a47fb09d3f646dd1bd
Reviewed-on: https://boringssl-review.googlesource.com/1452
Reviewed-by: Adam Langley <agl@google.com>
Add a framework for testing the asynchronous codepath. Move some handshake
state machine coverage tests to cover a range of record-layer and
handshake-layer asynchronicity.
This adds tests for the previous two async bugs fixed.
Change-Id: I422ef33ba3eeb0ad04766871ed8bc59b677b169e
Reviewed-on: https://boringssl-review.googlesource.com/1410
Reviewed-by: Adam Langley <agl@google.com>
Should have test coverage there as long as we care about supporting it.
Change-Id: Ic67539228b550f2ebd0b543d5a58640913b0474b
Reviewed-on: https://boringssl-review.googlesource.com/1371
Reviewed-by: Adam Langley <agl@google.com>
David Benjamin
Adam Langley
Alex Chernyakhovsky
Adam Langley
Adam Langley