Add initial handshake reassembly tests.
For now, only test reorderings when we always or never fragment messages.
There's a third untested case: when full messages and fragments are mixed. That
will be tested later after making it actually work.
Change-Id: Ic4efb3f5e87b1319baf2d4af31eafa40f6a50fa6
Reviewed-on: https://boringssl-review.googlesource.com/3216
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 99b0c04..3e37f1d 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -610,6 +610,12 @@
// PacketAdaptor is the packetAdaptor to use to simulate timeouts.
PacketAdaptor *packetAdaptor
+
+ // ReorderHandshakeFragments, if true, causes handshake fragments in
+ // DTLS to overlap and be sent in the wrong order. It also causes
+ // pre-CCS flights to be sent twice. (Post-CCS flights consist of
+ // Finished and will trigger a spurious retransmit.)
+ ReorderHandshakeFragments bool
}
func (c *Config) serverInit() {
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go
index 307f61d..86a7b7d 100644
--- a/ssl/test/runner/conn.go
+++ b/ssl/test/runner/conn.go
@@ -69,8 +69,9 @@
// DTLS state
sendHandshakeSeq uint16
recvHandshakeSeq uint16
- handMsg []byte // pending assembled handshake message
- handMsgLen int // handshake message length, not including the header
+ handMsg []byte // pending assembled handshake message
+ handMsgLen int // handshake message length, not including the header
+ pendingFragments [][]byte // pending outgoing handshake fragments.
tmp [16]byte
}
diff --git a/ssl/test/runner/dtls.go b/ssl/test/runner/dtls.go
index 8e4b8a6..f84530f 100644
--- a/ssl/test/runner/dtls.go
+++ b/ssl/test/runner/dtls.go
@@ -20,6 +20,7 @@
"errors"
"fmt"
"io"
+ "math/rand"
"net"
)
@@ -125,10 +126,6 @@
// FragmentAcrossChangeCipherSpec. (Which is unfortunate
// because OpenSSL's DTLS implementation will probably accept
// such fragmentation and could do with a fix + tests.)
- if len(data) < 4 {
- // This should not happen.
- panic(data)
- }
header := data[:4]
data = data[4:]
@@ -151,12 +148,13 @@
fragment = append(fragment, byte(m>>16), byte(m>>8), byte(m))
fragment = append(fragment, data[:m]...)
- // TODO(davidben): A real DTLS implementation needs to
- // retransmit handshake messages. For testing purposes, we don't
- // actually care.
- _, err = c.dtlsWriteRawRecord(recordTypeHandshake, fragment)
- if err != nil {
- break
+ // Buffer the fragment for later. They will be sent (and
+ // reordered) on flush.
+ c.pendingFragments = append(c.pendingFragments, fragment)
+
+ if c.config.Bugs.ReorderHandshakeFragments && m > (maxLen+1)/2 {
+ // Overlap each fragment by half.
+ m = (maxLen + 1) / 2
}
n += m
data = data[m:]
@@ -168,6 +166,38 @@
return
}
+func (c *Conn) dtlsFlushHandshake(duplicate bool) error {
+ if !c.isDTLS {
+ return nil
+ }
+
+ var fragments []byte
+ fragments, c.pendingFragments = c.pendingFragments, fragments
+
+ if c.config.Bugs.ReorderHandshakeFragments {
+ if duplicate {
+ fragments = append(fragments, fragments...)
+ }
+ perm := rand.New(rand.NewSource(0)).Perm(len(fragments))
+ tmp := make([][]byte, len(fragments))
+ for i := range tmp {
+ tmp[i] = fragments[perm[i]]
+ }
+ fragments = tmp
+ }
+
+ // Send them all.
+ for _, fragment := range fragments {
+ // TODO(davidben): A real DTLS implementation needs to
+ // retransmit handshake messages. For testing purposes, we don't
+ // actually care.
+ if _, err := c.dtlsWriteRawRecord(recordTypeHandshake, fragment); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
func (c *Conn) dtlsWriteRawRecord(typ recordType, data []byte) (n int, err error) {
recordHeaderLen := dtlsRecordHeaderLen
maxLen := c.config.Bugs.MaxHandshakeRecordLength
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 72a82f8..5ad3602 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -214,6 +214,9 @@
helloBytes = hello.marshal()
c.writeRecord(recordTypeHandshake, helloBytes)
}
+ if err := c.dtlsFlushHandshake(true); err != nil {
+ return err
+ }
if err := c.simulatePacketLoss(nil); err != nil {
return err
@@ -237,6 +240,9 @@
hello.cookie = helloVerifyRequest.cookie
helloBytes = hello.marshal()
c.writeRecord(recordTypeHandshake, helloBytes)
+ if err := c.dtlsFlushHandshake(true); err != nil {
+ return err
+ }
if err := c.simulatePacketLoss(nil); err != nil {
return err
@@ -327,7 +333,10 @@
// Most retransmits are triggered by a timeout, but the final
// leg of the handshake is retransmited upon re-receiving a
// Finished.
- if err := c.simulatePacketLoss(func() { c.writeRecord(recordTypeHandshake, hs.finishedBytes) }); err != nil {
+ if err := c.simulatePacketLoss(func() {
+ c.writeRecord(recordTypeHandshake, hs.finishedBytes)
+ c.dtlsFlushHandshake(false)
+ }); err != nil {
return err
}
if err := hs.readSessionTicket(); err != nil {
@@ -612,6 +621,9 @@
hs.writeClientHash(certVerify.marshal())
c.writeRecord(recordTypeHandshake, certVerify.marshal())
}
+ if err := c.dtlsFlushHandshake(true); err != nil {
+ return err
+ }
hs.finishedHash.discardHandshakeBuffer()
@@ -847,6 +859,9 @@
c.writeRecord(recordTypeHandshake, postCCSBytes[:5])
postCCSBytes = postCCSBytes[5:]
}
+ if err := c.dtlsFlushHandshake(true); err != nil {
+ return err
+ }
if !c.config.Bugs.SkipChangeCipherSpec &&
c.config.Bugs.EarlyChangeCipherSpec == 0 {
@@ -858,6 +873,9 @@
}
c.writeRecord(recordTypeHandshake, postCCSBytes)
+ if err := c.dtlsFlushHandshake(false); err != nil {
+ return err
+ }
return nil
}
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index af521fc..c13de9b 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -75,7 +75,10 @@
// Most retransmits are triggered by a timeout, but the final
// leg of the handshake is retransmited upon re-receiving a
// Finished.
- if err := c.simulatePacketLoss(func() { c.writeRecord(recordTypeHandshake, hs.finishedBytes) }); err != nil {
+ if err := c.simulatePacketLoss(func() {
+ c.writeRecord(recordTypeHandshake, hs.finishedBytes)
+ c.dtlsFlushHandshake(false)
+ }); err != nil {
return err
}
if err := hs.readFinished(isResume); err != nil {
@@ -146,6 +149,9 @@
return false, errors.New("dtls: short read from Rand: " + err.Error())
}
c.writeRecord(recordTypeHandshake, helloVerifyRequest.marshal())
+ if err := c.dtlsFlushHandshake(true); err != nil {
+ return false, err
+ }
if err := c.simulatePacketLoss(nil); err != nil {
return false, err
@@ -543,6 +549,9 @@
helloDone := new(serverHelloDoneMsg)
hs.writeServerHash(helloDone.marshal())
c.writeRecord(recordTypeHandshake, helloDone.marshal())
+ if err := c.dtlsFlushHandshake(true); err != nil {
+ return err
+ }
var pub crypto.PublicKey // public key for client auth, if any
@@ -836,6 +845,9 @@
c.writeRecord(recordTypeHandshake, postCCSBytes[:5])
postCCSBytes = postCCSBytes[5:]
}
+ if err := c.dtlsFlushHandshake(true); err != nil {
+ return err
+ }
if !c.config.Bugs.SkipChangeCipherSpec {
c.writeRecord(recordTypeChangeCipherSpec, []byte{1})
@@ -846,6 +858,9 @@
}
c.writeRecord(recordTypeHandshake, postCCSBytes)
+ if err := c.dtlsFlushHandshake(false); err != nil {
+ return err
+ }
c.cipherSuite = hs.suite.id
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 7fda082..b97bc42 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -652,6 +652,36 @@
},
},
},
+ {
+ protocol: dtls,
+ name: "ReorderHandshakeFragments-Small-DTLS",
+ config: Config{
+ Bugs: ProtocolBugs{
+ ReorderHandshakeFragments: true,
+ // Small enough that every handshake message is
+ // fragmented.
+ MaxHandshakeRecordLength: 2,
+ },
+ },
+ },
+ {
+ protocol: dtls,
+ name: "ReorderHandshakeFragments-Large-DTLS",
+ config: Config{
+ Bugs: ProtocolBugs{
+ 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,
+ },
+ },
+ },
}
func doExchange(test *testCase, config *Config, conn net.Conn, messageLen int, isResume bool) error {