From 5c0921dbfc3af7eb7f609274c2e2522bdaa2ac80 Mon Sep 17 00:00:00 2001 From: Adam Kiss Date: Fri, 10 May 2019 09:07:09 +0200 Subject: [PATCH] Respect port range for srv reflexive candidates Modified gatherCandidatesReflective to respect port min and max constraints when allocating connections for server reflexive candidates. --- .golangci.yml | 1 + gather.go | 82 +++++++++++++++++++++++++++++++-------------------- go.mod | 4 +-- go.sum | 8 ++--- stun.go | 36 ---------------------- util.go | 19 ------------ 6 files changed, 57 insertions(+), 93 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index afb7ff5..d7bf1be 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -10,6 +10,7 @@ linters: - lll - maligned - gochecknoglobals + - interfacer issues: exclude-use-default: false diff --git a/gather.go b/gather.go index fed2056..1196549 100644 --- a/gather.go +++ b/gather.go @@ -1,6 +1,12 @@ package ice -import "net" +import ( + "fmt" + "net" + "time" + + "github.com/pion/stun" +) func localInterfaces(networkTypes []NetworkType) (ips []net.IP) { ifaces, err := net.Interfaces() @@ -112,42 +118,54 @@ func gatherCandidatesLocal(a *Agent, networkTypes []NetworkType) { } func gatherCandidatesReflective(a *Agent, urls []*URL, networkTypes []NetworkType) { + localIPs := localInterfaces(networkTypes) for _, networkType := range networkTypes { network := networkType.String() for _, url := range urls { - switch url.Scheme { - case SchemeTypeSTUN: - laddr, xoraddr, err := allocateUDP(network, url) - if err != nil { - a.log.Warnf("could not allocate %s %s: %v\n", network, url, err) - continue - } - conn, err := net.ListenUDP(network, laddr) - if err != nil { - a.log.Warnf("could not listen %s %s: %v\n", network, laddr, err) - } - - ip := xoraddr.IP - port := xoraddr.Port - relIP := laddr.IP.String() - relPort := laddr.Port - c, err := NewCandidateServerReflexive(network, ip, port, ComponentRTP, relIP, relPort) - if err != nil { - a.log.Warnf("Failed to create server reflexive candidate: %s %s %d: %v\n", network, ip, port, err) - continue - } - - networkType := c.NetworkType - set := a.localCandidates[networkType] - set = append(set, c) - a.localCandidates[networkType] = set - - c.start(a, conn) - - default: - a.log.Warnf("scheme %s is not implemented\n", url.Scheme) + hostPort := fmt.Sprintf("%s:%d", url.Host, url.Port) + serverAddr, err := net.ResolveUDPAddr(network, hostPort) + if err != nil { + a.log.Warnf("failed to resolve stun host: %s: %v", hostPort, err) continue } + for _, ip := range localIPs { + switch url.Scheme { + case SchemeTypeSTUN: + conn, err := listenUDP(int(a.portmax), int(a.portmin), network, &net.UDPAddr{IP: ip, Port: 0}) + if err != nil { + a.log.Warnf("could not listen %s %s\n", network, ip) + continue + } + + xoraddr, err := stun.GetMappedAddressUDP(conn, serverAddr, time.Second*5) + if err != nil { + a.log.Warnf("could not get server reflexive address %s %s: %v\n", network, url, err) + continue + } + + laddr := conn.LocalAddr().(*net.UDPAddr) + ip := xoraddr.IP + port := xoraddr.Port + relIP := laddr.IP.String() + relPort := laddr.Port + c, err := NewCandidateServerReflexive(network, ip, port, ComponentRTP, relIP, relPort) + if err != nil { + a.log.Warnf("Failed to create server reflexive candidate: %s %s %d: %v\n", network, ip, port, err) + continue + } + + networkType := c.NetworkType + set := a.localCandidates[networkType] + set = append(set, c) + a.localCandidates[networkType] = set + + c.start(a, conn) + + default: + a.log.Warnf("scheme %s is not implemented\n", url.Scheme) + continue + } + } } } } diff --git a/go.mod b/go.mod index 22dd5af..3a693e4 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.12 require ( github.com/pion/logging v0.2.1 - github.com/pion/stun v0.2.1 - github.com/pion/transport v0.6.0 + github.com/pion/stun v0.2.2 + github.com/pion/transport v0.7.0 github.com/stretchr/testify v1.3.0 ) diff --git a/go.sum b/go.sum index b080857..ae77cce 100644 --- a/go.sum +++ b/go.sum @@ -2,10 +2,10 @@ github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/pion/logging v0.2.1 h1:LwASkBKZ+2ysGJ+jLv1E/9H1ge0k1nTfi1X+5zirkDk= github.com/pion/logging v0.2.1/go.mod h1:k0/tDVsRCX2Mb2ZEmTqNa7CWsQPc+YYCB7Q+5pahoms= -github.com/pion/stun v0.2.1 h1:rSKJ0ynYkRalRD8BifmkaGLeepCFuGTwG6FxPsrPK8o= -github.com/pion/stun v0.2.1/go.mod h1:TChCNKgwnFiFG/c9K+zqEdd6pO6tlODb9yN1W/zVfsE= -github.com/pion/transport v0.6.0 h1:WAoyJg/6OI8dhCVFl/0JHTMd1iu2iHgGUXevptMtJ3U= -github.com/pion/transport v0.6.0/go.mod h1:iWZ07doqOosSLMhZ+FXUTq+TamDoXSllxpbGcfkCmbE= +github.com/pion/stun v0.2.2 h1:0IJCwJFOdEmHzz4oxl9SBGLlJbnNbF+0h6XSOmuE034= +github.com/pion/stun v0.2.2/go.mod h1:TChCNKgwnFiFG/c9K+zqEdd6pO6tlODb9yN1W/zVfsE= +github.com/pion/transport v0.7.0 h1:EsXN8TglHMlKZMo4ZGqwK6QgXBu0WYg7wfGMWIXsS+w= +github.com/pion/transport v0.7.0/go.mod h1:iWZ07doqOosSLMhZ+FXUTq+TamDoXSllxpbGcfkCmbE= github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= diff --git a/stun.go b/stun.go index b7627cf..c5910cc 100644 --- a/stun.go +++ b/stun.go @@ -3,10 +3,7 @@ package ice import ( "bytes" "encoding/binary" - "errors" "fmt" - "net" - "time" "github.com/pion/stun" ) @@ -66,36 +63,3 @@ func assertInboundMessageIntegrity(m *stun.Message, key []byte) error { return nil } - -func allocateUDP(network string, url *URL) (*net.UDPAddr, *stun.XorAddress, error) { - // TODO Do we want the timeout to be configurable? - client, err := stun.NewClient(network, fmt.Sprintf("%s:%d", url.Host, url.Port), time.Second*5) - if err != nil { - return nil, nil, flattenErrs([]error{errors.New("failed to create STUN client"), err}) - } - localAddr, ok := client.LocalAddr().(*net.UDPAddr) - if !ok { - return nil, nil, fmt.Errorf("failed to cast STUN client to UDPAddr") - } - - resp, err := client.Request() - if err != nil { - return nil, nil, flattenErrs([]error{errors.New("failed to make STUN request"), err}) - } - - if err = client.Close(); err != nil { - return nil, nil, flattenErrs([]error{errors.New("failed to close STUN client"), err}) - } - - attr, ok := resp.GetOneAttribute(stun.AttrXORMappedAddress) - if !ok { - return nil, nil, fmt.Errorf("got response from STUN server that did not contain XORAddress") - } - - var addr stun.XorAddress - if err = addr.Unpack(resp, attr); err != nil { - return nil, nil, flattenErrs([]error{errors.New("failed to unpack STUN XorAddress response"), err}) - } - - return localAddr, &addr, nil -} diff --git a/util.go b/util.go index 554c500..7691e99 100644 --- a/util.go +++ b/util.go @@ -1,10 +1,8 @@ package ice import ( - "fmt" "math/rand" "net" - "strings" "sync/atomic" "time" ) @@ -52,23 +50,6 @@ func randSeq(n int) string { return string(b) } -// flattenErrs flattens multiple errors into one -func flattenErrs(errs []error) error { - var errstrings []string - - for _, err := range errs { - if err != nil { - errstrings = append(errstrings, err.Error()) - } - } - - if len(errstrings) == 0 { - return nil - } - - return fmt.Errorf(strings.Join(errstrings, "\n")) -} - func parseAddr(in net.Addr) (net.IP, int, NetworkType, bool) { switch addr := in.(type) { case *net.UDPAddr: