ICE connectivity checks are made for deselected network types

[adriancable]

Example - I set things up like this:

s.SetNetworkTypes([]webrtc.NetworkType{webrtc.NetworkTypeUDP4, webrtc.NetworkTypeTCP4})

But then see things like this:

ice TRACE: 20:07:56.026763 agent.go:895: Ping STUN from tcp6 host [2601:646:c200:7e:4705:3093:5b99:bb99]:41033 (resolved: [2601:646:c200:7e:4705:3093:5b99:bb99]:41033) to tcp6 host [2607:f8b0:4004:1001::7f]:19305 (resolved: [2607:f8b0:4004:1001::7f]:19305)
...
ice TRACE: 20:07:56.097495 selection.go:138: Inbound STUN (SuccessResponse) from tcp6 host [2607:f8b0:4004:1001::7f]:19305 (resolved: [2607:f8b0:4004:1001::7f]:19305) to tcp6 host [2601:646:c200:7e:4705:3093:5b99:bb99]:41033 (resolved: [2601:646:c200:7e:4705:3093:5b99:bb99]:41033)

As expected, the TCP6 candidate pairs never get nominated. But why are we even doing connectivity checks on them when we are excluding TCP6 via SetNetworkTypes?

Edit to add: I think there is a logic error in addRemotePassiveTCPCandidate in ice/agent.go. Right now it has:

func (a *Agent) addRemotePassiveTCPCandidate(remoteCandidate Candidate) {
	_, localIPs, err := localInterfaces(a.net, a.interfaceFilter, a.ipFilter, []NetworkType{remoteCandidate.NetworkType()}, a.includeLoopback)

But this doesn't check that remoteCandidate.NetworkType() is in a.networkTypes. So we probably want to add something like this at the beginning of addRemotePassiveTCPCandidate:

	found := false
	for _, v := range a.networkTypes {
		if v == remoteCandidate.NetworkType() {
			found = true
			break
		}
	}

	if !found {
		a.log.Warnf("Remote candidate ignored as network type excluded in local agent configuration: %s", remoteCandidate)
		return
	}

[Sean-Der]

I am not sure how this happened! We have a check that matches your exact logic here

[adriancable]

@Sean-Der - the issue, I think, is that the logic doesn't differentiate between TCP4 and TCP6 candidates. So for example, if only TCP4 is enabled in the network types, we still end up generating TCP6 local candidates and doing connectivity checks on them if the remote answers with TCP6 candidates, even though we (correctly) never nominate them. Does this make sense?

[Sean-Der]

@adriancable oof I missed that detail. I was able to reproduce with a test, merging now!

[adriancable]

@Sean-Der - great! Your change looks good to me.