From e0db6d259e9f7690d3b84a1db56644977204d9eb Mon Sep 17 00:00:00 2001
From: Sean DuBois <sean@siobud.com>
Date: Mon, 21 Feb 2022 15:36:48 -0500
Subject: [PATCH] Don't drop Binding Requests in Controlled Agent

A controlled Agent would discard incoming Binding Requests if it didn't
cause the pair to be selected. For UDP Candidate this would be
interpreted as packet loss. For TCP Candidates not responding with a
Binding Success could be interpreted as a failure.

Firefox's ICE Agent would disconnect TCP Candidates because of this
behavior.

Resolves to pion/webrtc#2125
Resolves to pion/webrtc#1356
See https://bugzilla.mozilla.org/show_bug.cgi?id=1756460
---
 candidatepair.go | 13 +++++++------
 selection.go     | 15 +++++++++------
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/candidatepair.go b/candidatepair.go
index 79cab1a..0a53245 100644
--- a/candidatepair.go
+++ b/candidatepair.go
@@ -18,12 +18,13 @@ func newCandidatePair(local, remote Candidate, controlling bool) *CandidatePair
 // CandidatePair is a combination of a
 // local and remote candidate
 type CandidatePair struct {
-	iceRoleControlling  bool
-	Remote              Candidate
-	Local               Candidate
-	bindingRequestCount uint16
-	state               CandidatePairState
-	nominated           bool
+	iceRoleControlling       bool
+	Remote                   Candidate
+	Local                    Candidate
+	bindingRequestCount      uint16
+	state                    CandidatePairState
+	nominated                bool
+	nominateOnBindingSuccess bool
 }
 
 func (p *CandidatePair) String() string {
diff --git a/selection.go b/selection.go
index 7248414..df43f03 100644
--- a/selection.go
+++ b/selection.go
@@ -229,13 +229,17 @@ func (s *controlledSelector) HandleSuccessResponse(m *stun.Message, local, remot
 
 	p.state = CandidatePairStateSucceeded
 	s.log.Tracef("Found valid candidate pair: %s", p)
+	if p.nominateOnBindingSuccess {
+		if selectedPair := s.agent.getSelectedPair(); selectedPair == nil {
+			s.agent.setSelectedPair(p)
+		}
+	}
 }
 
 func (s *controlledSelector) HandleBindingRequest(m *stun.Message, local, remote Candidate) {
 	useCandidate := m.Contains(stun.AttrUseCandidate)
 
 	p := s.agent.findPair(local, remote)
-
 	if p == nil {
 		p = s.agent.addPair(local, remote)
 	}
@@ -251,7 +255,6 @@ func (s *controlledSelector) HandleBindingRequest(m *stun.Message, local, remote
 			if selectedPair := s.agent.getSelectedPair(); selectedPair == nil {
 				s.agent.setSelectedPair(p)
 			}
-			s.agent.sendBindingSuccess(m, local, remote)
 		} else {
 			// If the received Binding request triggered a new check to be
 			// enqueued in the triggered-check queue (Section 7.3.1.4), once the
@@ -261,12 +264,12 @@ func (s *controlledSelector) HandleBindingRequest(m *stun.Message, local, remote
 			// MUST remove the candidate pair from the valid list, set the
 			// candidate pair state to Failed, and set the checklist state to
 			// Failed.
-			s.PingCandidate(local, remote)
+			p.nominateOnBindingSuccess = true
 		}
-	} else {
-		s.agent.sendBindingSuccess(m, local, remote)
-		s.PingCandidate(local, remote)
 	}
+
+	s.agent.sendBindingSuccess(m, local, remote)
+	s.PingCandidate(local, remote)
 }
 
 type liteSelector struct {