Rework DTLS handshake message reassembly logic.

Notably, drop all special cases around receiving a message in order and
receiving a full message. It makes things more complicated and was the source
of bugs (the MixCompleteMessageWithFragments tests added in this CL did not
pass before). Instead, every message goes through an hm_fragment, and
dtls1_get_message always checks buffered_messages to see if the next is
complete.

The downside is that we pay one more copy of the message data in the common
case. This is only during connection setup, so I think it's worth the
simplicity. (If we want to optimize later, we could either tighten
ssl3_get_message's interface to allow the handshake data being in the
hm_fragment's backing store rather than s->init_buf or swap out s->init_buf
with the hm_fragment's backing store when a mesasge completes.

This CL does not address ssl_read_bytes being an inappropriate API for DTLS.
Future work will revise the handshake/transport boundary to align better with
DTLS's needs. Also other problems that I've left as TODOs.

Change-Id: Ib4570d45634b5181ecf192894d735e8699b1c86b
Reviewed-on: https://boringssl-review.googlesource.com/3764
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index ff8c2d7..4aead4e 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -623,6 +623,11 @@
 	// Finished and will trigger a spurious retransmit.)
 	ReorderHandshakeFragments bool
 
+	// MixCompleteMessageWithFragments, if true, causes handshake
+	// messages in DTLS to redundantly both fragment the message
+	// and include a copy of the full one.
+	MixCompleteMessageWithFragments bool
+
 	// SendInvalidRecordType, if true, causes a record with an invalid
 	// content type to be sent immediately following the handshake.
 	SendInvalidRecordType bool
@@ -630,6 +635,30 @@
 	// WrongCertificateMessageType, if true, causes Certificate message to
 	// be sent with the wrong message type.
 	WrongCertificateMessageType bool
+
+	// FragmentMessageTypeMismatch, if true, causes all non-initial
+	// handshake fragments in DTLS to have the wrong message type.
+	FragmentMessageTypeMismatch bool
+
+	// FragmentMessageLengthMismatch, if true, causes all non-initial
+	// 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
+
+	// SendEmptyFragments, if true, causes handshakes to include empty
+	// fragments in DTLS.
+	SendEmptyFragments bool
 }
 
 func (c *Config) serverInit() {
diff --git a/ssl/test/runner/dtls.go b/ssl/test/runner/dtls.go
index 4ad84e9..f9e1148 100644
--- a/ssl/test/runner/dtls.go
+++ b/ssl/test/runner/dtls.go
@@ -106,6 +106,16 @@
 	return typ, b, nil
 }
 
+func (c *Conn) makeFragment(header, data []byte, fragOffset, fragLen int) []byte {
+	fragment := make([]byte, 0, 12+fragLen)
+	fragment = append(fragment, header...)
+	fragment = append(fragment, byte(c.sendHandshakeSeq>>8), byte(c.sendHandshakeSeq))
+	fragment = append(fragment, byte(fragOffset>>16), byte(fragOffset>>8), byte(fragOffset))
+	fragment = append(fragment, byte(fragLen>>16), byte(fragLen>>8), byte(fragLen))
+	fragment = append(fragment, data[fragOffset:fragOffset+fragLen]...)
+	return fragment
+}
+
 func (c *Conn) dtlsWriteRecord(typ recordType, data []byte) (n int, err error) {
 	if typ != recordTypeHandshake {
 		// Only handshake messages are fragmented.
@@ -131,24 +141,27 @@
 
 	isFinished := header[0] == typeFinished
 
+	if c.config.Bugs.SendEmptyFragments {
+		fragment := c.makeFragment(header, data, 0, 0)
+		c.pendingFragments = append(c.pendingFragments, fragment)
+	}
+
 	firstRun := true
-	for firstRun || len(data) > 0 {
+	fragOffset := 0
+	for firstRun || fragOffset < len(data) {
 		firstRun = false
-		m := len(data)
-		if m > maxLen {
-			m = maxLen
+		fragLen := len(data) - fragOffset
+		if fragLen > maxLen {
+			fragLen = maxLen
 		}
 
-		// Standard TLS handshake header.
-		fragment := make([]byte, 0, 12+m)
-		fragment = append(fragment, header...)
-		// message_seq
-		fragment = append(fragment, byte(c.sendHandshakeSeq>>8), byte(c.sendHandshakeSeq))
-		// fragment_offset
-		fragment = append(fragment, byte(n>>16), byte(n>>8), byte(n))
-		// fragment_length
-		fragment = append(fragment, byte(m>>16), byte(m>>8), byte(m))
-		fragment = append(fragment, data[:m]...)
+		fragment := c.makeFragment(header, data, fragOffset, fragLen)
+		if c.config.Bugs.FragmentMessageTypeMismatch && fragOffset > 0 {
+			fragment[0]++
+		}
+		if c.config.Bugs.FragmentMessageLengthMismatch && fragOffset > 0 {
+			fragment[3]++
+		}
 
 		// Buffer the fragment for later. They will be sent (and
 		// reordered) on flush.
@@ -160,13 +173,17 @@
 				c.pendingFragments = append(c.pendingFragments, fragment)
 			}
 
-			if m > (maxLen+1)/2 {
+			if fragLen > (maxLen+1)/2 {
 				// Overlap each fragment by half.
-				m = (maxLen + 1) / 2
+				fragLen = (maxLen + 1) / 2
 			}
 		}
-		n += m
-		data = data[m:]
+		fragOffset += fragLen
+		n += fragLen
+	}
+	if !isFinished && c.config.Bugs.MixCompleteMessageWithFragments {
+		fragment := c.makeFragment(header, data, 0, len(data))
+		c.pendingFragments = append(c.pendingFragments, fragment)
 	}
 
 	// Increment the handshake sequence number for the next
@@ -194,6 +211,18 @@
 
 	// Send them all.
 	for _, fragment := range fragments {
+		if c.config.Bugs.SplitFragmentHeader {
+			if _, err := c.dtlsWriteRawRecord(recordTypeHandshake, fragment[:2]); err != nil {
+				return err
+			}
+			fragment = fragment[2:]
+		} else if c.config.Bugs.SplitFragmentBody && len(fragment) > 12 {
+			if _, err := c.dtlsWriteRawRecord(recordTypeHandshake, fragment[:13]); err != nil {
+				return err
+			}
+			fragment = fragment[13:]
+		}
+
 		// TODO(davidben): A real DTLS implementation needs to
 		// retransmit handshake messages. For testing purposes, we don't
 		// actually care.
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 17badb0..dc27513 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -710,17 +710,22 @@
 				ReorderHandshakeFragments: true,
 				// Large enough that no handshake message is
 				// fragmented.
-				//
-				// TODO(davidben): Also test interaction of
-				// complete handshake messages with
-				// fragments. The current logic is full of bugs
-				// here, so the reassembly logic needs a rewrite
-				// before those tests will pass.
 				MaxHandshakeRecordLength: 2048,
 			},
 		},
 	},
 	{
+		protocol: dtls,
+		name:     "MixCompleteMessageWithFragments-DTLS",
+		config: Config{
+			Bugs: ProtocolBugs{
+				ReorderHandshakeFragments:       true,
+				MixCompleteMessageWithFragments: true,
+				MaxHandshakeRecordLength:        2,
+			},
+		},
+	},
+	{
 		name: "SendInvalidRecordType",
 		config: Config{
 			Bugs: ProtocolBugs{
@@ -811,6 +816,61 @@
 		expectedError:      ":UNEXPECTED_MESSAGE:",
 		expectedLocalError: "remote error: unexpected message",
 	},
+	{
+		protocol: dtls,
+		name:     "FragmentMessageTypeMismatch-DTLS",
+		config: Config{
+			Bugs: ProtocolBugs{
+				MaxHandshakeRecordLength:    2,
+				FragmentMessageTypeMismatch: true,
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":FRAGMENT_MISMATCH:",
+	},
+	{
+		protocol: dtls,
+		name:     "FragmentMessageLengthMismatch-DTLS",
+		config: Config{
+			Bugs: ProtocolBugs{
+				MaxHandshakeRecordLength:      2,
+				FragmentMessageLengthMismatch: true,
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":FRAGMENT_MISMATCH:",
+	},
+	{
+		protocol: dtls,
+		name:     "SplitFragmentHeader-DTLS",
+		config: Config{
+			Bugs: ProtocolBugs{
+				SplitFragmentHeader: true,
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":UNEXPECTED_MESSAGE:",
+	},
+	{
+		protocol: dtls,
+		name:     "SplitFragmentBody-DTLS",
+		config: Config{
+			Bugs: ProtocolBugs{
+				SplitFragmentBody: true,
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":UNEXPECTED_MESSAGE:",
+	},
+	{
+		protocol: dtls,
+		name:     "SendEmptyFragments-DTLS",
+		config: Config{
+			Bugs: ProtocolBugs{
+				SendEmptyFragments: true,
+			},
+		},
+	},
 }
 
 func doExchange(test *testCase, config *Config, conn net.Conn, messageLen int, isResume bool) error {