Fix empty fragment handling in DTLS message reassembly.

Found with libFuzzer.

Bug: chromium:763097
Change-Id: I806bcfc714c0629ff7f725e37f4c0045d4ec7ac6
Reviewed-on: https://boringssl-review.googlesource.com/20105
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

ssl/d1_both.cc

@@ -228,6 +228,10 @@ static void dtls1_hm_fragment_mark(hm_fragment *frag, size_t start,
   // A zero-length message will never have a pending reassembly.
   assert(msg_len > 0);
 
+  if (start == end) {
+    return;
+  }
+
   if ((start >> 3) == (end >> 3)) {
     frag->reassembly[start >> 3] |= bit_range(start & 7, end & 7);
   } else {

ssl/test/runner/common.go

@@ -119,6 +119,7 @@ const (
 	extensionUseSRTP                    uint16 = 14
 	extensionALPN                       uint16 = 16
 	extensionSignedCertificateTimestamp uint16 = 18
+	extensionPadding                    uint16 = 21
 	extensionExtendedMasterSecret       uint16 = 23
 	extensionSessionTicket              uint16 = 35
 	extensionKeyShare                   uint16 = 40    // draft-ietf-tls-tls13-16
@@ -1434,6 +1435,10 @@ type ProtocolBugs struct {
 	// ExpectRecordSplitting, if true, causes application records to only be
 	// accepted if they follow a 1/n-1 record split.
 	ExpectRecordSplitting bool
+
+	// PadClientHello, if non-zero, pads the ClientHello to a multiple of
+	// that many bytes.
+	PadClientHello int
 }
 
 func (c *Config) serverInit() {

ssl/test/runner/dtls.go

@@ -209,8 +209,8 @@ func (c *Conn) dtlsWriteRecord(typ recordType, data []byte) (n int, err error) {
 	isFinished := header[0] == typeFinished
 
 	if c.config.Bugs.SendEmptyFragments {
-		fragment := c.makeFragment(header, data, 0, 0)
-		c.pendingFragments = append(c.pendingFragments, fragment)
+		c.pendingFragments = append(c.pendingFragments, c.makeFragment(header, data, 0, 0))
+		c.pendingFragments = append(c.pendingFragments, c.makeFragment(header, data, len(data), 0))
 	}
 
 	firstRun := true

ssl/test/runner/handshake_messages.go

@@ -175,6 +175,7 @@ type clientHelloMsg struct {
 	pskBinderFirst          bool
 	omitExtensions          bool
 	emptyExtensions         bool
+	pad                     int
 }
 
 func (m *clientHelloMsg) equal(i interface{}) bool {
@@ -222,7 +223,8 @@ func (m *clientHelloMsg) equal(i interface{}) bool {
 		m.hasGREASEExtension == m1.hasGREASEExtension &&
 		m.pskBinderFirst == m1.pskBinderFirst &&
 		m.omitExtensions == m1.omitExtensions &&
-		m.emptyExtensions == m1.emptyExtensions
+		m.emptyExtensions == m1.emptyExtensions &&
+		m.pad == m1.pad
 }
 
 func (m *clientHelloMsg) marshal() []byte {
@@ -454,6 +456,16 @@ func (m *clientHelloMsg) marshal() []byte {
 		}
 	}
 
+	if m.pad != 0 && hello.len()%m.pad != 0 {
+		extensions.addU16(extensionPadding)
+		padding := extensions.addU16LengthPrefixed()
+		// Note hello.len() has changed at this point from the length
+		// prefix.
+		if l := hello.len() % m.pad; l != 0 {
+			padding.addBytes(make([]byte, m.pad-l))
+		}
+	}
+
 	if m.omitExtensions || m.emptyExtensions {
 		// Silently erase any extensions which were sent.
 		hello.discardChild()
@@ -463,6 +475,10 @@ func (m *clientHelloMsg) marshal() []byte {
 	}
 
 	m.raw = handshakeMsg.finish()
+	// Sanity-check padding.
+	if m.pad != 0 && (len(m.raw)-4)%m.pad != 0 {
+		panic(fmt.Sprintf("%d is not a multiple of %d", len(m.raw)-4, m.pad))
+	}
 	return m.raw
 }
 

ssl/test/runner/runner.go

@@ -2122,6 +2122,19 @@ read alert 1 0
 				},
 			},
 		},
+		{
+			testType: serverTest,
+			protocol: dtls,
+			name:     "SendEmptyFragments-Padded-DTLS",
+			config: Config{
+				Bugs: ProtocolBugs{
+					// Test empty fragments for a message with a
+					// nice power-of-two length.
+					PadClientHello:     64,
+					SendEmptyFragments: true,
+				},
+			},
+		},
 		{
 			name: "BadFinished-Client",
 			config: Config{