Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ICE STUN detection will fail on packet loss #336

Open
lzhou-em opened this issue Mar 10, 2021 · 0 comments
Open

ICE STUN detection will fail on packet loss #336

lzhou-em opened this issue Mar 10, 2021 · 0 comments
Labels
bug Something isn't working

Comments

@lzhou-em
Copy link

lzhou-em commented Mar 10, 2021

Environment.

  • Version: Linux Ubuntu 20.04
  • Linux running behind an IPv4 NAT router

What did you do?

This is the example code. This is a func to get all candidates, using STUN: stun.qq.com

func getCandidates() ([]*webrtc.ICECandidate, error) {

	pendingCandidates := make([]*webrtc.ICECandidate, 0)
	// Prepare the configuration
	config := webrtc.Configuration{
		ICEServers: []webrtc.ICEServer{
			{
				URLs: []string{"stun:stun.qq.com"},
			},
		},
		BundlePolicy: webrtc.BundlePolicyMaxBundle,
	}

	// Create a new RTCPeerConnection
	peerConnection, err := webrtc.NewPeerConnection(config)
	if err != nil {
		panic(err)
	}

	// When an ICE candidate is available send to the other Pion instance
	// the other Pion instance will add this candidate by calling AddICECandidate
	doneCandidates := false
	peerConnection.OnICECandidate(func(c *webrtc.ICECandidate) {
		fmt.Printf("OnICECandidate %p\n", c)
		if c == nil {
			doneCandidates = true
			return
		}
		pendingCandidates = append(pendingCandidates, c)
	})

	// Create an offer to send to the other process
	offer, err := peerConnection.CreateOffer(nil)
	if err != nil {
		panic(err)
	}

	// Sets the LocalDescription, and starts our UDP listeners
	// Note: this will start the gathering of ICE candidates
	if err = peerConnection.SetLocalDescription(offer); err != nil {
		panic(err)
	}

	// Send our offer to the HTTP server listening in the other process
	payload, err := json.Marshal(offer)
	if err != nil {
		panic(err)
	}
	fmt.Print(string(payload)) // nolint:noctx
	//peerConnection.Close()
	// Block to wait ICE detection done
	for {
		if doneCandidates {
			break
		}
		time.Sleep(time.Duration(10) * time.Millisecond)
	}
	return pendingCandidates, nil}

What did you expect?

I want to detect the local ip(Linux eth0) and external ip address(router WAN) correctly, but I found 2 issues that maybe improve:

  1. When there is packet loss in UDP WriteTo() (in file util.go, func stunRequest()), the srflx detection will fail. A simple retransmition strategy will work. I just modified the code for 4 retries, it worked perfectly.
  2. Some stun servers are still using with RFC3489, no XORMappedAddress, such as stun.qq.com. I just add some code in util.go: getXORMappedAddr() to check the missing Attribution error and fall back to RFC3489 to get MappedAddress. It works with stun.qq.com.
  3. The stun detection of pion/webrtc has a odd state, sometimes it will run srflx detection twice, for example, my demo code will do it twice. After fixed the 2nd MappedAddress, it seems been fixed. I don't know why.

This is the patched file util.go: getXORMappedAddr(), I don't submit a pull request, the patch is for your reference. It's working.

func getXORMappedAddr(conn net.PacketConn, serverAddr net.Addr, deadline time.Duration) (*stun.XORMappedAddress, error) {

	var resp *stun.Message
	var err error
	for i := 0; i < 4; i++ { // 4 retries
		if deadline > 0 {
			if err := conn.SetReadDeadline(time.Now().Add(deadline)); err != nil {
				return nil, err
			}
		}
		defer func() {
			if deadline > 0 {
				_ = conn.SetReadDeadline(time.Time{})
			}
		}()
		resp, err = stunRequest(
			func(p []byte) (int, error) {
				n, _, errr := conn.ReadFrom(p)
				return n, errr
			},
			func(b []byte) (int, error) {
				return conn.WriteTo(b, serverAddr)
			},
		)
		if err == nil {
			break
		}
	}
	if err != nil {
		return nil, err
	}
	var addr stun.XORMappedAddress
	if err = addr.GetFrom(resp); err != nil {
		var addrMap stun.MappedAddress  //fall back to MappedAddress when error
		if err = addrMap.GetFrom(resp); err != nil {
			return nil, fmt.Errorf("%w: %v", errGetXorMappedAddrResponse, err)
		}
		addr = stun.XORMappedAddress(addrMap)
	}

	return (*stun.XORMappedAddress)(&addr), nil
}
@stv0g stv0g changed the title ICE stun detection will fail on packet loss ICE STUN detection will fail on packet loss Jan 12, 2023
@stv0g stv0g added the bug Something isn't working label Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants