Don't pack fragments as efficiently for plaintext records
This partially reverts
https://boringssl-review.googlesource.com/c/boringssl/+/72275
The more efficiently packing broke a project that talks to a server that
does not handle this correctly. See b/378742138. For now, just match
what we do in TLS and only pack encrypted records.
Since DTLS 1.2 only has multi-message flights in the plaintext epoch and
DTLS 1.3 only has them in encrypted epochs, this effectively limits the
optimization for DTLS 1.3. Later we'll want to add a toggle and roll
this out across the board, but for now just partially revert the
behavior.
Bug: 374991962
Change-Id: I4c11efcd06dc22a7866e096c5b7a5e379da281c4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/73067
Commit-Queue: Nick Harper <nharper@chromium.org>
Reviewed-by: Nick Harper <nharper@chromium.org>
Auto-Submit: David Benjamin <davidben@google.com>
diff --git a/ssl/d1_both.cc b/ssl/d1_both.cc
index 841edfe..fd5e6be 100644
--- a/ssl/d1_both.cc
+++ b/ssl/d1_both.cc
@@ -778,6 +778,11 @@
return seal_continue;
}
+ // TODO(crbug.com/374991962): For now, only send one message per record in
+ // epoch 0. Sending multiple is allowed and more efficient, but breaks
+ // b/378742138.
+ const bool allow_multiple_messages = first_msg.epoch != 0;
+
// Pack as many handshake fragments into one record as we can. We stage the
// fragments in the output buffer, to be sealed in-place.
bool should_continue = false;
@@ -860,6 +865,11 @@
goto packet_full;
}
}
+
+ if (!allow_multiple_messages) {
+ should_continue = true;
+ break;
+ }
}
packet_full:
diff --git a/ssl/test/runner/dtls.go b/ssl/test/runner/dtls.go
index e2c4871..e9e5953 100644
--- a/ssl/test/runner/dtls.go
+++ b/ssl/test/runner/dtls.go
@@ -301,7 +301,9 @@
if typ == recordTypeHandshake && c.lastRecordInFlight.typ == recordTypeHandshake && epoch.epoch == c.lastRecordInFlight.epoch {
// The previous record was compatible with this one. The shim
// should have fit more in this record before making a new one.
- if c.lastRecordInFlight.bytesAvailable > handshakeBytesNeeded {
+ // TODO(crbug.com/374991962): Enforce this for plaintext records
+ // too.
+ if c.lastRecordInFlight.bytesAvailable > handshakeBytesNeeded && epoch.epoch > 0 {
return 0, nil, c.in.setErrorLocked(fmt.Errorf("dtls: previous handshake record had %d bytes available, but shim did not fit another fragment in it", c.lastRecordInFlight.bytesAvailable))
}
} else if newPacket {
