DTLS fragments may not be split across two records.
See also upstream's 9dcab127e14467733523ff7626da8906e67eedd6. The root problem
is dtls1_read_bytes is wrong, but we can get the right behavior now and add a
regression test for it before cleaning it up.
Change-Id: I4e5c39ab254a872d9f64242c9b77b020bdded6e6
Reviewed-on: https://boringssl-review.googlesource.com/5123
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 207de0c..928c2b2 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -668,17 +668,10 @@
// handshake fragments in DTLS to have the wrong message length.
FragmentMessageLengthMismatch bool
- // SplitFragmentHeader, if true, causes the handshake fragments in DTLS
- // to be split across two records.
- SplitFragmentHeader bool
-
- // SplitFragmentBody, if true, causes the handshake bodies in DTLS to be
- // split across two records.
- //
- // TODO(davidben): There's one final split to test: when the header and
- // body are split across two records. But those are (incorrectly)
- // accepted right now.
- SplitFragmentBody bool
+ // SplitFragments, if non-zero, causes the handshake fragments in DTLS
+ // to be split across two records. The value of |SplitFragments| is the
+ // number of bytes in the first fragment.
+ SplitFragments int
// SendEmptyFragments, if true, causes handshakes to include empty
// fragments in DTLS.
diff --git a/ssl/test/runner/dtls.go b/ssl/test/runner/dtls.go
index 50f7786..538bf51 100644
--- a/ssl/test/runner/dtls.go
+++ b/ssl/test/runner/dtls.go
@@ -216,13 +216,10 @@
// Pack handshake fragments into records.
var records [][]byte
for _, fragment := range fragments {
- if c.config.Bugs.SplitFragmentHeader {
- records = append(records, fragment[:2])
- records = append(records, fragment[2:])
- } else if c.config.Bugs.SplitFragmentBody {
- if len(fragment) > 12 {
- records = append(records, fragment[:13])
- records = append(records, fragment[13:])
+ if n := c.config.Bugs.SplitFragments; n > 0 {
+ if len(fragment) > n {
+ records = append(records, fragment[:n])
+ records = append(records, fragment[n:])
} else {
records = append(records, fragment)
}
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 524b3d0..f02a221 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -913,10 +913,10 @@
},
{
protocol: dtls,
- name: "SplitFragmentHeader-DTLS",
+ name: "SplitFragments-Header-DTLS",
config: Config{
Bugs: ProtocolBugs{
- SplitFragmentHeader: true,
+ SplitFragments: 2,
},
},
shouldFail: true,
@@ -924,14 +924,25 @@
},
{
protocol: dtls,
- name: "SplitFragmentBody-DTLS",
+ name: "SplitFragments-Boundary-DTLS",
config: Config{
Bugs: ProtocolBugs{
- SplitFragmentBody: true,
+ SplitFragments: dtlsRecordHeaderLen,
},
},
shouldFail: true,
- expectedError: ":UNEXPECTED_MESSAGE:",
+ expectedError: ":EXCESSIVE_MESSAGE_SIZE:",
+ },
+ {
+ protocol: dtls,
+ name: "SplitFragments-Body-DTLS",
+ config: Config{
+ Bugs: ProtocolBugs{
+ SplitFragments: dtlsRecordHeaderLen + 1,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":EXCESSIVE_MESSAGE_SIZE:",
},
{
protocol: dtls,
