Skip to content

Commit

Permalink
Fix handling of retransmitted cookie-echo
Browse files Browse the repository at this point in the history 
Relates to pion/webrtc#1270
  • Loading branch information
@enobufs
enobufs committed Jul 6, 2020
1 parent dbc92ec commit f64ec5c
Show file tree
Hide file tree
Showing 2 changed files with 160 additions and 22 deletions.
27 changes: 14 additions & 13 deletions association.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -979,29 +979,30 @@ func (a *Association) handleHeartbeat(c *chunkHeartbeat) []*packet {
func (a *Association) handleCookieEcho(c *chunkCookieEcho) []*packet {
state := a.getState()
a.log.Debugf("[%s] COOKIE-ECHO received in state '%s'", a.name, getAssociationStateString(state))
if state != closed && state != cookieWait && state != cookieEchoed {
return nil
}
if state != established {
if state != closed && state != cookieWait && state != cookieEchoed {
return nil
}
if !bytes.Equal(a.myCookie.cookie, c.cookie) {
return nil
}

if !bytes.Equal(a.myCookie.cookie, c.cookie) {
return nil
}
a.t1Init.stop()
a.storedInit = nil

a.t1Init.stop()
a.storedInit = nil
a.t1Cookie.stop()
a.storedCookieEcho = nil

a.t1Cookie.stop()
a.storedCookieEcho = nil
a.setState(established)
a.handshakeCompletedCh <- nil
}

p := &packet{
verificationTag: a.peerVerificationTag,
sourcePort: a.sourcePort,
destinationPort: a.destinationPort,
chunks: []chunk{&chunkCookieAck{}},
}

a.setState(established)
a.handshakeCompletedCh <- nil
return pack(p)
}

Expand Down
155 changes: 146 additions & 9 deletions vnet_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ type vNetEnvConfig struct {
}

type vNetEnv struct {
wan *vnet.Router
net0 *vnet.Net
net1 *vnet.Net
numToDropData int
numToDropReconfig int
wan *vnet.Router
net0 *vnet.Net
net1 *vnet.Net
numToDropData int
numToDropReconfig int
numToDropCookieEcho int
numToDropCookieAck int
}

func (venv *vNetEnv) dropNextDataChunk(numToDrop int) {
Expand All @@ -36,6 +38,14 @@ func (venv *vNetEnv) dropNextReconfigChunk(numToDrop int) {
venv.numToDropReconfig = numToDrop
}

func (venv *vNetEnv) dropNextCookieEchoChunk(numToDrop int) {
venv.numToDropCookieEcho = numToDrop
}

func (venv *vNetEnv) dropNextCookieAckChunk(numToDrop int) {
venv.numToDropCookieAck = numToDrop
}

func buildVNetEnv(cfg *vNetEnvConfig) (*vNetEnv, error) {
log := cfg.log

Expand Down Expand Up @@ -88,6 +98,20 @@ func buildVNetEnv(cfg *vNetEnvConfig) (*vNetEnv, error) {
log.Infof("Chunk filter: drop RECONFIG %s", chunk.String())
break loop
}
case *chunkCookieEcho:
if venv.numToDropCookieEcho > 0 {
toDrop = true
venv.numToDropCookieEcho--
log.Infof("Chunk filter: drop %s", chunk.String())
break loop
}
case *chunkCookieAck:
if venv.numToDropCookieAck > 0 {
toDrop = true
venv.numToDropCookieAck--
log.Infof("Chunk filter: drop %s", chunk.String())
break loop
}
}
}
return !toDrop
Expand Down Expand Up @@ -183,7 +207,7 @@ func testRwndFull(t *testing.T, unordered bool) {
}
defer assoc.Close() // nolint:errcheck

log.Info("server handlshake complete")
log.Info("server handshake complete")
close(serverHandshakeDone)

stream, err := assoc.AcceptStream()
Expand Down Expand Up @@ -257,7 +281,7 @@ func testRwndFull(t *testing.T, unordered bool) {
}
defer assoc.Close() // nolint:errcheck

log.Info("client handlshake complete")
log.Info("client handshake complete")
close(clientHandshakeDone)

stream, err := assoc.OpenStream(777, PayloadTypeWebRTCBinary)
Expand Down Expand Up @@ -408,7 +432,7 @@ func testStreamClose(t *testing.T, dropReconfig bool) {
}
defer assoc.Close() // nolint:errcheck

log.Info("server handlshake complete")
log.Info("server handshake complete")

stream, err := assoc.AcceptStream()
if !assert.NoError(t, err, "should succeed") {
Expand Down Expand Up @@ -457,7 +481,7 @@ func testStreamClose(t *testing.T, dropReconfig bool) {
}
defer assoc.Close() // nolint:errcheck

log.Info("client handlshake complete")
log.Info("client handshake complete")

stream, err := assoc.OpenStream(777, PayloadTypeWebRTCBinary)
if !assert.NoError(t, err, "should succeed") {
Expand Down Expand Up @@ -544,3 +568,116 @@ func TestStreamClose(t *testing.T) {
testStreamClose(t, true)
})
}

// this test case reproduces the issue mentioned in
// https://github.com/pion/webrtc/issues/1270#issuecomment-653953743
// and confirmes the fix.
// To reproduce the case mentioned above:
// * Use simultaneous-open (SCTP)
// * Drop both of the first COOKIE-ECHO and COOKIE-ACK
func TestCookieEchoRetransmission(t *testing.T) {
lim := test.TimeOut(time.Second * 10)
defer lim.Stop()

loggerFactory := logging.NewDefaultLoggerFactory()
log := loggerFactory.NewLogger("test")

venv, err := buildVNetEnv(&vNetEnvConfig{
minDelay: 200 * time.Millisecond,
loggerFactory: loggerFactory,
log: log,
})
if !assert.NoError(t, err, "should succeed") {
return
}
if !assert.NotNil(t, venv, "should not be nil") {
return
}
defer venv.wan.Stop() // nolint:errcheck

// To cause the cookie echo retransmission, both COOKIE-ECHO
// and COOKIE-ACK chunks need to be dropped at the same time.
venv.dropNextCookieEchoChunk(1)
venv.dropNextCookieAckChunk(1)

serverHandshakeDone := make(chan struct{})
clientHandshakeDone := make(chan struct{})
waitAllHandshakeDone := make(chan struct{})
clientShutDown := make(chan struct{})
serverShutDown := make(chan struct{})

maxReceiveBufferSize := uint32(64 * 1024)

// Go routine for Server
go func() {
defer close(serverShutDown)
// connected UDP conn for server
conn, err := venv.net0.DialUDP("udp4",
&net.UDPAddr{IP: net.ParseIP("1.1.1.1"), Port: 5000},
&net.UDPAddr{IP: net.ParseIP("2.2.2.2"), Port: 5000},
)
if !assert.NoError(t, err, "should succeed") {
return
}
defer conn.Close() // nolint:errcheck

// server association
// using Client for simultaneous open
assoc, err := Client(Config{
NetConn: conn,
MaxReceiveBufferSize: maxReceiveBufferSize,
LoggerFactory: loggerFactory,
})
if !assert.NoError(t, err, "should succeed") {
return
}
defer assoc.Close() // nolint:errcheck

log.Info("server handshake complete")
close(serverHandshakeDone)
<-waitAllHandshakeDone
}()

// Go routine for Client
go func() {
defer close(clientShutDown)
// connected UDP conn for client
conn, err := venv.net1.DialUDP("udp4",
&net.UDPAddr{IP: net.ParseIP("2.2.2.2"), Port: 5000},
&net.UDPAddr{IP: net.ParseIP("1.1.1.1"), Port: 5000},
)
if !assert.NoError(t, err, "should succeed") {
return
}

// client association
assoc, err := Client(Config{
NetConn: conn,
MaxReceiveBufferSize: maxReceiveBufferSize,
LoggerFactory: loggerFactory,
})
if !assert.NoError(t, err, "should succeed") {
return
}
defer assoc.Close() // nolint:errcheck

log.Info("client handshake complete")
close(clientHandshakeDone)
<-waitAllHandshakeDone
}()

//
// Scenario
//

// wait until both handshake complete
<-clientHandshakeDone
<-serverHandshakeDone
close(waitAllHandshakeDone)

log.Info("handshake complete")

<-clientShutDown
<-serverShutDown
log.Info("all done")
}

0 comments on commit f64ec5c

Please sign in to comment.