From a4e6d48749f009c36ebeed194e6c5227d9fc21a2 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 2 Mar 2015 19:10:53 -0500 Subject: [PATCH] runner: Move Finished special-case into dtlsWriteRecord. We actually don't really care about this special-case since we only test client full handshakes where the runner sends the second Finished not the shim (otherwise the overlap logic and retransmitting on every fragment would probably break us), but it should probably live next to the fragmentation logic. Change-Id: I54097d84ad8294bc6c42a84d6f22f496e63eb2a8 Reviewed-on: https://boringssl-review.googlesource.com/3763 Reviewed-by: Adam Langley --- ssl/test/runner/dtls.go | 20 +++++++++++++------- ssl/test/runner/handshake_client.go | 12 ++++++------ ssl/test/runner/handshake_server.go | 10 +++++----- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/ssl/test/runner/dtls.go b/ssl/test/runner/dtls.go index aa8b1893..4ad84e9b 100644 --- a/ssl/test/runner/dtls.go +++ b/ssl/test/runner/dtls.go @@ -129,6 +129,8 @@ func (c *Conn) dtlsWriteRecord(typ recordType, data []byte) (n int, err error) { header := data[:4] data = data[4:] + isFinished := header[0] == typeFinished + firstRun := true for firstRun || len(data) > 0 { firstRun = false @@ -151,10 +153,17 @@ func (c *Conn) dtlsWriteRecord(typ recordType, data []byte) (n int, err error) { // Buffer the fragment for later. They will be sent (and // reordered) on flush. c.pendingFragments = append(c.pendingFragments, fragment) + if c.config.Bugs.ReorderHandshakeFragments { + // Don't duplicate Finished to avoid the peer + // interpreting it as a retransmit request. + if !isFinished { + c.pendingFragments = append(c.pendingFragments, fragment) + } - if c.config.Bugs.ReorderHandshakeFragments && m > (maxLen+1)/2 { - // Overlap each fragment by half. - m = (maxLen + 1) / 2 + if m > (maxLen+1)/2 { + // Overlap each fragment by half. + m = (maxLen + 1) / 2 + } } n += m data = data[m:] @@ -166,7 +175,7 @@ func (c *Conn) dtlsWriteRecord(typ recordType, data []byte) (n int, err error) { return } -func (c *Conn) dtlsFlushHandshake(duplicate bool) error { +func (c *Conn) dtlsFlushHandshake() error { if !c.isDTLS { return nil } @@ -175,9 +184,6 @@ func (c *Conn) dtlsFlushHandshake(duplicate bool) error { 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 { diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index 49fb6721..17cfc01f 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go @@ -214,7 +214,7 @@ NextCipherSuite: helloBytes = hello.marshal() c.writeRecord(recordTypeHandshake, helloBytes) } - c.dtlsFlushHandshake(true) + c.dtlsFlushHandshake() if err := c.simulatePacketLoss(nil); err != nil { return err @@ -238,7 +238,7 @@ NextCipherSuite: hello.cookie = helloVerifyRequest.cookie helloBytes = hello.marshal() c.writeRecord(recordTypeHandshake, helloBytes) - c.dtlsFlushHandshake(true) + c.dtlsFlushHandshake() if err := c.simulatePacketLoss(nil); err != nil { return err @@ -331,7 +331,7 @@ NextCipherSuite: // Finished. if err := c.simulatePacketLoss(func() { c.writeRecord(recordTypeHandshake, hs.finishedBytes) - c.dtlsFlushHandshake(false) + c.dtlsFlushHandshake() }); err != nil { return err } @@ -617,7 +617,7 @@ func (hs *clientHandshakeState) doFullHandshake() error { hs.writeClientHash(certVerify.marshal()) c.writeRecord(recordTypeHandshake, certVerify.marshal()) } - c.dtlsFlushHandshake(true) + c.dtlsFlushHandshake() hs.finishedHash.discardHandshakeBuffer() @@ -853,7 +853,7 @@ func (hs *clientHandshakeState) sendFinished(isResume bool) error { c.writeRecord(recordTypeHandshake, postCCSBytes[:5]) postCCSBytes = postCCSBytes[5:] } - c.dtlsFlushHandshake(true) + c.dtlsFlushHandshake() if !c.config.Bugs.SkipChangeCipherSpec && c.config.Bugs.EarlyChangeCipherSpec == 0 { @@ -866,7 +866,7 @@ func (hs *clientHandshakeState) sendFinished(isResume bool) error { if !c.config.Bugs.SkipFinished { c.writeRecord(recordTypeHandshake, postCCSBytes) - c.dtlsFlushHandshake(false) + c.dtlsFlushHandshake() } return nil } diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index 50cc20bb..e1d49e5f 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go @@ -77,7 +77,7 @@ func (c *Conn) serverHandshake() error { // Finished. if err := c.simulatePacketLoss(func() { c.writeRecord(recordTypeHandshake, hs.finishedBytes) - c.dtlsFlushHandshake(false) + c.dtlsFlushHandshake() }); err != nil { return err } @@ -149,7 +149,7 @@ func (hs *serverHandshakeState) readClientHello() (isResume bool, err error) { return false, errors.New("dtls: short read from Rand: " + err.Error()) } c.writeRecord(recordTypeHandshake, helloVerifyRequest.marshal()) - c.dtlsFlushHandshake(true) + c.dtlsFlushHandshake() if err := c.simulatePacketLoss(nil); err != nil { return false, err @@ -551,7 +551,7 @@ func (hs *serverHandshakeState) doFullHandshake() error { helloDone := new(serverHelloDoneMsg) hs.writeServerHash(helloDone.marshal()) c.writeRecord(recordTypeHandshake, helloDone.marshal()) - c.dtlsFlushHandshake(true) + c.dtlsFlushHandshake() var pub crypto.PublicKey // public key for client auth, if any @@ -845,7 +845,7 @@ func (hs *serverHandshakeState) sendFinished() error { c.writeRecord(recordTypeHandshake, postCCSBytes[:5]) postCCSBytes = postCCSBytes[5:] } - c.dtlsFlushHandshake(true) + c.dtlsFlushHandshake() if !c.config.Bugs.SkipChangeCipherSpec { c.writeRecord(recordTypeChangeCipherSpec, []byte{1}) @@ -857,7 +857,7 @@ func (hs *serverHandshakeState) sendFinished() error { if !c.config.Bugs.SkipFinished { c.writeRecord(recordTypeHandshake, postCCSBytes) - c.dtlsFlushHandshake(false) + c.dtlsFlushHandshake() } c.cipherSuite = hs.suite.id