runner: ACK flights in DTLS 1.3 by default
Tests can override this behavior if they want. This required fixing up
some logic around tracking lastRecordInFlight. We implicitly assumed
that, by the time we're ready to write, there's nothing more to read in
the current record. But BoringSSL currently sends a single record with
two NewSessionTickets in it, even though they're nominally two flights.
Instead, only wipe the state if the packet is empty. There's probably a
better way to process this, but this will do.
Bug: 42290594
Change-Id: Ib22d575777eb6866dbc02b9ba3b74e8d61a74b6c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/72847
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go
index cef3091..7f6ea64 100644
--- a/ssl/test/runner/conn.go
+++ b/ssl/test/runner/conn.go
@@ -1206,7 +1206,9 @@
// c.out.Mutex <= L.
func (c *Conn) writeRecord(typ recordType, data []byte) (n int, err error) {
c.seenHandshakePackEnd = false
- c.lastRecordInFlight = nil
+ if c.hand.Len() == 0 {
+ c.lastRecordInFlight = nil
+ }
if typ == recordTypeHandshake {
msgType := data[0]
if c.config.Bugs.SendWrongMessageType != 0 && msgType == c.config.Bugs.SendWrongMessageType {
diff --git a/ssl/test/runner/dtls.go b/ssl/test/runner/dtls.go
index f360e38..e2c4871 100644
--- a/ssl/test/runner/dtls.go
+++ b/ssl/test/runner/dtls.go
@@ -466,7 +466,9 @@
if c.config.Bugs.ACKFlightDTLS != nil {
c.config.Bugs.ACKFlightDTLS(&controller, prev, received, records)
} else {
- // TODO(crbug.com/42290594): In DTLS 1.3, send an ACK by default.
+ if c.vers >= VersionTLS13 {
+ controller.WriteACK(controller.OutEpoch(), records)
+ }
}
if err := controller.Err(); err != nil {
return err
@@ -589,7 +591,9 @@
}
func (c *Conn) dtlsFlushPacket() error {
- c.lastRecordInFlight = nil
+ if c.hand.Len() == 0 {
+ c.lastRecordInFlight = nil
+ }
if len(c.pendingPacket) == 0 {
return nil
}
@@ -791,7 +795,8 @@
// func ACKFlight(c *DTLSController, prev, received []DTLSMessage)
//
// Like WriteFlight, ACKFlight may simulate packet loss with the DTLSController.
-// It returns when it is ready to proceed.
+// It returns when it is ready to proceed. If not specified, it does nothing in
+// DTLS 1.2 and ACKs the final flight in DTLS 1.3.
//
// This test design implicitly assumes the shim will never start a
// post-handshake transaction before the previous one is complete. Otherwise the
@@ -809,9 +814,6 @@
// TODO(crbug.com/42290594): When we implement ACK-sending on the shim, add a
// way for the test to specify which ACKs are expected, unless we can derive
// that automatically?
-//
-// TODO(crbug.com/42290594): The default behavior for ACKFlight should be to
-// send an ACK.
type DTLSController struct {
conn *Conn
err error
