Reject candidates from old generation

Return an error if a candidate with a username fragment that does
not match the username fragment in the remote description is added.
This usually indicates that the candidate was generated before the
renegotiation.

Author: JoeTurki

peerconnection.go

@@ -1962,33 +1962,65 @@ func (pc *PeerConnection) RemoteDescription() *SessionDescription {
 // AddICECandidate accepts an ICE candidate string and adds it
 // to the existing set of candidates.
 func (pc *PeerConnection) AddICECandidate(candidate ICECandidateInit) error {
-	if pc.RemoteDescription() == nil {
+	remoteDesc := pc.RemoteDescription()
+	if remoteDesc == nil {
 		return &rtcerr.InvalidStateError{Err: ErrNoRemoteDescription}
 	}
 
 	candidateValue := strings.TrimPrefix(candidate.Candidate, "candidate:")
 
-	var iceCandidate *ICECandidate
-	if candidateValue != "" {
-		candidate, err := ice.UnmarshalCandidate(candidateValue)
-		if err != nil {
-			if errors.Is(err, ice.ErrUnknownCandidateTyp) || errors.Is(err, ice.ErrDetermineNetworkType) {
-				pc.log.Warnf("Discarding remote candidate: %s", err)
+	if candidateValue == "" {
+		return pc.iceTransport.AddRemoteCandidate(nil)
+	}
 
-				return nil
-			}
+	cand, err := ice.UnmarshalCandidate(candidateValue)
+	if err != nil {
+		if errors.Is(err, ice.ErrUnknownCandidateTyp) || errors.Is(err, ice.ErrDetermineNetworkType) {
+			pc.log.Warnf("Discarding remote candidate: %s", err)
 
-			return err
+			return nil
 		}
 
-		c, err := newICECandidateFromICE(candidate, "", 0)
-		if err != nil {
-			return err
+		return err
+	}
+
+	// Reject candidates from old generations.
+	// If candidate.usernameFragment is not null,
+	// and is not equal to any username fragment present in the corresponding media
+	//  description of an applied remote description,
+	// return a promise rejected with a newly created OperationError.
+	// https://w3c.github.io/webrtc-pc/#dom-peerconnection-addicecandidate
+	if ufrag, ok := cand.GetExtension("ufrag"); ok {
+		if !pc.descriptionContainsUfrag(remoteDesc.parsed, ufrag.Value) {
+			pc.log.Errorf("dropping candidate with ufrag %s because it doesn't match the current ufrags", ufrag.Value)
+
+			return nil
 		}
-		iceCandidate = &c
 	}
 
-	return pc.iceTransport.AddRemoteCandidate(iceCandidate)
+	c, err := newICECandidateFromICE(cand, "", 0)
+	if err != nil {
+		return err
+	}
+
+	return pc.iceTransport.AddRemoteCandidate(&c)
+}
+
+// Return true if the sdp contains a specific ufrag.
+func (pc *PeerConnection) descriptionContainsUfrag(sdp *sdp.SessionDescription, matchUfrag string) bool {
+	ufrag, ok := sdp.Attribute("ice-ufrag")
+	if ok && ufrag == matchUfrag {
+		return true
+	}
+
+	for _, media := range sdp.MediaDescriptions {
+		ufrag, ok := media.Attribute("ice-ufrag")
+		if ok && ufrag == matchUfrag {
+			return true
+		}
+	}
+
+	return false
 }
 
 // ICEConnectionState returns the ICE connection state of the

peerconnection_go_test.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/pion/dtls/v3"
 	"github.com/pion/ice/v4"
+	"github.com/pion/logging"
 	"github.com/pion/rtcp"
 	"github.com/pion/rtp"
 	"github.com/pion/transport/v3/test"
@@ -1916,3 +1917,91 @@ func Test_IPv6(t *testing.T) { //nolint: cyclop
 
 	closePairNow(t, offerPC, answerPC)
 }
+
+type testICELogger struct {
+	lastErrorMessage string
+}
+
+func (t *testICELogger) Trace(string)                  {}
+func (t *testICELogger) Tracef(string, ...interface{}) {}
+func (t *testICELogger) Debug(string)                  {}
+func (t *testICELogger) Debugf(string, ...interface{}) {}
+func (t *testICELogger) Info(string)                   {}
+func (t *testICELogger) Infof(string, ...interface{})  {}
+func (t *testICELogger) Warn(string)                   {}
+func (t *testICELogger) Warnf(string, ...interface{})  {}
+func (t *testICELogger) Error(msg string)              { t.lastErrorMessage = msg }
+func (t *testICELogger) Errorf(format string, args ...interface{}) {
+	t.lastErrorMessage = fmt.Sprintf(format, args...)
+}
+
+type testICELoggerFactory struct {
+	logger *testICELogger
+}
+
+func (t *testICELoggerFactory) NewLogger(string) logging.LeveledLogger {
+	return t.logger
+}
+
+func TestAddICECandidate__DroppingOldGenerationCandidates(t *testing.T) {
+	lim := test.TimeOut(time.Second * 30)
+	defer lim.Stop()
+
+	report := test.CheckRoutines(t)
+	defer report()
+
+	testLogger := &testICELogger{}
+	loggerFactory := &testICELoggerFactory{logger: testLogger}
+
+	// Create a new API with the custom logger
+	api := NewAPI(WithSettingEngine(SettingEngine{
+		LoggerFactory: loggerFactory,
+	}))
+
+	pc, err := api.NewPeerConnection(Configuration{})
+	assert.NoError(t, err)
+
+	_, err = pc.CreateDataChannel("test", nil)
+	assert.NoError(t, err)
+
+	offer, err := pc.CreateOffer(nil)
+	assert.NoError(t, err)
+
+	offerGatheringComplete := GatheringCompletePromise(pc)
+	assert.NoError(t, pc.SetLocalDescription(offer))
+	<-offerGatheringComplete
+
+	remotePC, err := api.NewPeerConnection(Configuration{})
+	assert.NoError(t, err)
+
+	assert.NoError(t, remotePC.SetRemoteDescription(offer))
+
+	remoteDesc := remotePC.RemoteDescription()
+	assert.NotNil(t, remoteDesc)
+
+	ufrag, hasUfrag := remoteDesc.parsed.MediaDescriptions[0].Attribute("ice-ufrag")
+	assert.True(t, hasUfrag)
+
+	emptyUfragCandidate := ICECandidateInit{
+		Candidate: "candidate:1 1 UDP 2122252543 192.168.1.1 12345 typ host",
+	}
+	err = remotePC.AddICECandidate(emptyUfragCandidate)
+	assert.NoError(t, err)
+	assert.Empty(t, testLogger.lastErrorMessage)
+
+	validCandidate := ICECandidateInit{
+		Candidate: fmt.Sprintf("candidate:1 1 UDP 2122252543 192.168.1.1 12345 typ host ufrag %s", ufrag),
+	}
+	err = remotePC.AddICECandidate(validCandidate)
+	assert.NoError(t, err)
+	assert.Empty(t, testLogger.lastErrorMessage)
+
+	invalidCandidate := ICECandidateInit{
+		Candidate: "candidate:1 1 UDP 2122252543 192.168.1.1 12345 typ host ufrag invalid",
+	}
+	err = remotePC.AddICECandidate(invalidCandidate)
+	assert.NoError(t, err)
+	assert.Contains(t, testLogger.lastErrorMessage, "dropping candidate with ufrag")
+
+	closePairNow(t, pc, remotePC)
+}