Validate inbound success messages

Validate all inbound binding success messages, before
peers could create valid prflx candidates without doing
a full request response

Resolves #21

agent.go

@@ -3,6 +3,7 @@
 package ice
 
 import (
+	"bytes"
 	"fmt"
 	"math/rand"
 	"net"
@@ -28,6 +29,9 @@ const (
 	// the number of bytes that can be buffered before we start to error
 	maxBufferSize = 1000 * 1000 // 1MB
 
+	// the number of outbound binding requests we cache
+	maxPendingBindingRequests = 50
+
 	stunAttrHeaderLength = 4
 )
 
@@ -94,6 +98,9 @@ type Agent struct {
 
 	buffer *packetio.Buffer
 
+	// LRU of outbound Binding request Transaction IDs
+	pendingBindingRequests [][]byte
+
 	// State for closing
 	done chan struct{}
 	err  atomicError
@@ -160,11 +167,12 @@ func NewAgent(config *AgentConfig) (*Agent, error) {
 	}
 
 	a := &Agent{
-		tieBreaker:       rand.New(rand.NewSource(time.Now().UnixNano())).Uint64(),
-		gatheringState:   GatheringStateComplete, // TODO trickle-ice
-		connectionState:  ConnectionStateNew,
-		localCandidates:  make(map[NetworkType][]*Candidate),
-		remoteCandidates: make(map[NetworkType][]*Candidate),
+		tieBreaker:             rand.New(rand.NewSource(time.Now().UnixNano())).Uint64(),
+		gatheringState:         GatheringStateComplete, // TODO trickle-ice
+		connectionState:        ConnectionStateNew,
+		localCandidates:        make(map[NetworkType][]*Candidate),
+		remoteCandidates:       make(map[NetworkType][]*Candidate),
+		pendingBindingRequests: make([][]byte, 0, maxPendingBindingRequests),
 
 		localUfrag:  randSeq(16),
 		localPwd:    randSeq(32),
@@ -268,8 +276,9 @@ func (a *Agent) pingCandidate(local, remote *Candidate) {
 	// agent MUST NOT include the USE-CANDIDATE attribute in a Binding
 	// request.
 
+	transactionID := stun.GenerateTransactionID()
 	if a.isControlling {
-		msg, err = stun.Build(stun.ClassRequest, stun.MethodBinding, stun.GenerateTransactionID(),
+		msg, err = stun.Build(stun.ClassRequest, stun.MethodBinding, transactionID,
 			&stun.Username{Username: a.remoteUfrag + ":" + a.localUfrag},
 			&stun.UseCandidate{},
 			&stun.IceControlling{TieBreaker: a.tieBreaker},
@@ -280,7 +289,7 @@ func (a *Agent) pingCandidate(local, remote *Candidate) {
 			&stun.Fingerprint{},
 		)
 	} else {
-		msg, err = stun.Build(stun.ClassRequest, stun.MethodBinding, stun.GenerateTransactionID(),
+		msg, err = stun.Build(stun.ClassRequest, stun.MethodBinding, transactionID,
 			&stun.Username{Username: a.remoteUfrag + ":" + a.localUfrag},
 			&stun.IceControlled{TieBreaker: a.tieBreaker},
 			&stun.Priority{Priority: local.Priority()},
@@ -297,6 +306,12 @@ func (a *Agent) pingCandidate(local, remote *Candidate) {
 	}
 
 	a.log.Tracef("ping STUN from %s to %s\n", local.String(), remote.String())
+	if overflow := len(a.pendingBindingRequests) - maxPendingBindingRequests; overflow > 1 {
+		a.log.Debugf("Discarded %d pending binding requests, pendingBindingRequests is full", overflow)
+		a.pendingBindingRequests = a.pendingBindingRequests[overflow:]
+	}
+
+	a.pendingBindingRequests = append(a.pendingBindingRequests, transactionID)
 	a.sendSTUN(msg, local, remote)
 }
 
@@ -575,12 +590,11 @@ func (a *Agent) handleInboundControlled(m *stun.Message, localCandidate, remoteC
 	}
 
 	successResponse := m.Method == stun.MethodBinding && m.Class == stun.ClassSuccessResponse
-	_, usepair := m.GetOneAttribute(stun.AttrUseCandidate)
-	a.log.Tracef("got controlled message (success? %t, usepair? %t)", successResponse, usepair)
-	// Remember the working pair and select it when marked with usepair
-	a.setValidPair(localCandidate, remoteCandidate, usepair, false)
-
-	if !successResponse {
+	a.log.Tracef("got controlled message (success? %t)", successResponse)
+	if successResponse {
+		// Remember the working pair and select it when marked with usepair
+		a.setValidPair(localCandidate, remoteCandidate, true, false)
+	} else {
 		// Send success response
 		a.sendBindingSuccess(m, localCandidate, remoteCandidate)
 	}
@@ -594,21 +608,29 @@ func (a *Agent) handleInboundControlling(m *stun.Message, localCandidate, remote
 		a.log.Debug("useCandidate && a.isControlling == true")
 		return
 	}
-	a.log.Tracef("got controlling message: %#v", m)
 
 	successResponse := m.Method == stun.MethodBinding && m.Class == stun.ClassSuccessResponse
-	// Remember the working pair and select it when receiving a success response
-	a.setValidPair(localCandidate, remoteCandidate, successResponse, true)
-
-	if !successResponse {
+	a.log.Tracef("got controlled message (success? %t)", successResponse)
+	if successResponse {
+		// Remember the working pair and select it when receiving a success response
+		a.setValidPair(localCandidate, remoteCandidate, true, true)
+	} else {
 		// Send success response
 		a.sendBindingSuccess(m, localCandidate, remoteCandidate)
-
-		// We received a ping from the controlled agent. We know the pair works so now we ping with use-candidate set:
-		a.pingCandidate(localCandidate, remoteCandidate)
 	}
 }
 
+// Assert that the passed TransactionID is in our pendingBindingRequests and remove if it is
+func (a *Agent) handleInboundBindingSuccess(id []byte) bool {
+	for i := range a.pendingBindingRequests {
+		if bytes.Equal(a.pendingBindingRequests[i], id) {
+			a.pendingBindingRequests = append(a.pendingBindingRequests[:i], a.pendingBindingRequests[:i+1]...)
+			return true
+		}
+	}
+	return false
+}
+
 // handleInbound processes STUN traffic from a remote candidate
 func (a *Agent) handleInbound(m *stun.Message, local *Candidate, remote net.Addr) {
 	if m == nil || local == nil {
@@ -621,6 +643,9 @@ func (a *Agent) handleInbound(m *stun.Message, local *Candidate, remote net.Addr
 		if err := assertInboundMessageIntegrity(m, []byte(a.remotePwd)); err != nil {
 			a.log.Warnf("discard message from (%s), %v", remote, err)
 			return
+		} else if !a.handleInboundBindingSuccess(m.TransactionID) {
+			a.log.Warnf("discard message from (%s), invalid TransactionID %s", remote, m.TransactionID)
+			return
 		}
 	case m.Method == stun.MethodBinding && m.Class == stun.ClassRequest:
 		if err := assertInboundUsername(m, a.localUfrag+":"+a.remoteUfrag); err != nil {

gather_test.go

@@ -0,0 +1,41 @@
+package ice
+
+import (
+	"net"
+	"testing"
+)
+
+func TestListenUDP(t *testing.T) {
+	localIPs := localInterfaces([]NetworkType{NetworkTypeUDP4})
+	if len(localIPs) == 0 {
+		t.Fatal("localInterfaces found no interfaces, unable to test")
+	}
+
+	ip := localIPs[0]
+
+	conn, err := listenUDP(0, 0, udp, &net.UDPAddr{IP: ip, Port: 0})
+	if err != nil {
+		t.Fatalf("listenUDP error with no port restriction %v", err)
+	} else if conn == nil {
+		t.Fatalf("listenUDP error with no port restriction return a nil conn")
+	}
+
+	_, err = listenUDP(500, 499, udp, &net.UDPAddr{IP: ip, Port: 0})
+	if err != ErrPort {
+		t.Fatal("listenUDP with invalid port range did not return ErrPort")
+	}
+
+	conn, err = listenUDP(5000, 5000, udp, &net.UDPAddr{IP: ip, Port: 0})
+	if err != nil {
+		t.Fatalf("listenUDP error with no port restriction %v", err)
+	} else if conn == nil {
+		t.Fatalf("listenUDP error with no port restriction return a nil conn")
+	}
+
+	_, port, err := net.SplitHostPort(conn.LocalAddr().String())
+	if err != nil {
+		t.Fatal(err)
+	} else if port != "5000" {
+		t.Fatalf("listenUDP with port restriction of 5000 listened on incorrect port (%s)", port)
+	}
+}

transport_test.go

@@ -63,7 +63,7 @@ func TestTimeout(t *testing.T) {
 		panic(err)
 	}
 
-	testTimeout(t, ca, 30*time.Second)
+	testTimeout(t, ca, defaultConnectionTimeout)
 
 	ca, cb = pipeWithTimeout(5*time.Second, 3*time.Second)
 	err = cb.Close()