Repository: pion/turn (go)

Issue:
Permission should not include port
From the rfc https://tools.ietf.org/html/rfc5766#section-2.3, it should only include the IP address and lifetime.

Pull request:
Use IP address only for permission fingerprint
Please see #65 for details

NOTE:
* #41 is a duplicate

Fixes #41 and #65

Patch:
diff --git a/internal/allocation/allocation.go b/internal/allocation/allocation.go
index 0aeb6c3..772cced 100644
--- a/internal/allocation/allocation.go
+++ b/internal/allocation/allocation.go
@@ -30,6 +30,16 @@ type Allocation struct {
 	log                 logging.LeveledLogger
 }
 
+func addr2IPFingerprint(addr net.Addr) string {
+	switch a := addr.(type) {
+	case *net.UDPAddr:
+		return a.IP.String()
+	case *net.TCPAddr: // Do we really need this case?
+		return a.IP.String()
+	}
+	return "" // shoud never happen
+}
+
 // NewAllocation creates a new instance of NewAllocation.
 func NewAllocation(turnSocket net.PacketConn, fiveTuple *FiveTuple, log logging.LeveledLogger) *Allocation {
 	return &Allocation{
@@ -42,15 +52,17 @@ func NewAllocation(turnSocket net.PacketConn, fiveTuple *FiveTuple, log logging.
 }
 
 // GetPermission gets the Permission from the allocation
-func (a *Allocation) GetPermission(fingerprint string) *Permission {
+func (a *Allocation) GetPermission(addr net.Addr) *Permission {
 	a.permissionsLock.RLock()
 	defer a.permissionsLock.RUnlock()
+
+	fingerprint := addr2IPFingerprint(addr)
 	return a.permissions[fingerprint]
 }
 
 // AddPermission adds a new permission to the allocation
 func (a *Allocation) AddPermission(p *Permission) {
-	fingerprint := p.Addr.String()
+	fingerprint := addr2IPFingerprint(p.Addr)
 
 	a.permissionsLock.RLock()
 	existedPermission, ok := a.permissions[fingerprint]
@@ -69,9 +81,10 @@ func (a *Allocation) AddPermission(p *Permission) {
 }
 
 // RemovePermission removes the net.Addr's fingerprint from the allocation's permissions
-func (a *Allocation) RemovePermission(fingerprint string) {
+func (a *Allocation) RemovePermission(addr net.Addr) {
 	a.permissionsLock.Lock()
 	defer a.permissionsLock.Unlock()
+	fingerprint := addr2IPFingerprint(addr)
 	delete(a.permissions, fingerprint)
 }
 
@@ -217,6 +230,11 @@ func (a *Allocation) packetHandler(m *Manager) {
 			return
 		}
 
+		a.log.Debugf("relay socket %s received %d bytes from %s",
+			a.RelaySocket.LocalAddr().String(),
+			n,
+			srcAddr.String())
+
 		if channel := a.GetChannelByAddr(a.RelaySocket.LocalAddr()); channel != nil {
 			channelData := make([]byte, 4)
 			binary.BigEndian.PutUint16(channelData[0:], uint16(channel.ID))
@@ -226,7 +244,7 @@ func (a *Allocation) packetHandler(m *Manager) {
 			if _, err = a.TurnSocket.WriteTo(channelData, a.fiveTuple.SrcAddr); err != nil {
 				a.log.Errorf("Failed to send ChannelData from allocation %v %v", srcAddr, err)
 			}
-		} else if p := a.GetPermission(srcAddr.String()); p != nil {
+		} else if p := a.GetPermission(srcAddr); p != nil {
 			udpAddr := srcAddr.(*net.UDPAddr)
 			peerAddressAttr := turn.PeerAddress{IP: udpAddr.IP, Port: udpAddr.Port}
 			dataAttr := turn.Data(buffer[:n])
@@ -235,6 +253,7 @@ func (a *Allocation) packetHandler(m *Manager) {
 			if err != nil {
 				a.log.Errorf("Failed to send DataIndication from allocation %v %v", srcAddr, err)
 			}
+			a.log.Debugf("relaying message to client at %s", a.fiveTuple.SrcAddr.String())
 			if _, err = a.TurnSocket.WriteTo(msg.Raw, a.fiveTuple.SrcAddr); err != nil {
 				a.log.Errorf("Failed to send DataIndication from allocation %v %v", srcAddr, err)
 			}
diff --git a/internal/allocation/permission.go b/internal/allocation/permission.go
index dad8313..03538eb 100644
--- a/internal/allocation/permission.go
+++ b/internal/allocation/permission.go
@@ -29,7 +29,7 @@ func NewPermission(addr net.Addr, log logging.LeveledLogger) *Permission {
 
 func (p *Permission) start(lifetime time.Duration) {
 	p.lifetimeTimer = time.AfterFunc(lifetime, func() {
-		p.allocation.RemovePermission(p.Addr.String())
+		p.allocation.RemovePermission(p.Addr)
 	})
 }
 
diff --git a/turn.go b/turn.go
index 7694040..48593d6 100644
--- a/turn.go
+++ b/turn.go
@@ -369,7 +369,7 @@ func (s *Server) handleSendIndication(conn net.PacketConn, srcAddr net.Addr, m *
 	}
 
 	msgDst := &net.UDPAddr{IP: peerAddress.IP, Port: peerAddress.Port}
-	if perm := a.GetPermission(msgDst.String()); perm == nil {
+	if perm := a.GetPermission(msgDst); perm == nil {
 		return errors.Errorf("Unable to handle send-indication, no permission added: %v", msgDst)
 	}
 


Test patch:
diff --git a/internal/allocation/allocation_test.go b/internal/allocation/allocation_test.go
index bd86d3b..5e03bbd 100644
--- a/internal/allocation/allocation_test.go
+++ b/internal/allocation/allocation_test.go
@@ -28,7 +28,7 @@ func subTestGetPermission(t *testing.T) {
 
 	addr, _ := net.ResolveUDPAddr("udp", "127.0.0.1:3478")
 	addr2, _ := net.ResolveUDPAddr("udp", "127.0.0.1:3479")
-	addr3, _ := net.ResolveUDPAddr("udp", "127.0.0.1:3480")
+	addr3, _ := net.ResolveUDPAddr("udp", "127.0.0.2:3478")
 
 	p := &Permission{
 		Addr: addr,
@@ -36,23 +36,27 @@ func subTestGetPermission(t *testing.T) {
 	p2 := &Permission{
 		Addr: addr2,
 	}
+	p3 := &Permission{
+		Addr: addr3,
+	}
 
 	a.AddPermission(p)
 	a.AddPermission(p2)
+	a.AddPermission(p3)
 
-	foundP1 := a.GetPermission(addr.String())
+	foundP1 := a.GetPermission(addr)
 	if foundP1 != p {
-		t.Error("Got permission is not same as the the added.")
+		t.Error("Should keep the first one.")
 	}
 
-	foundP2 := a.GetPermission(addr2.String())
-	if foundP2 != p2 {
-		t.Error("Got permission is not same as the the added.")
+	foundP2 := a.GetPermission(addr2)
+	if foundP2 != p {
+		t.Error("Second one should be ignored.")
 	}
 
-	foundP3 := a.GetPermission(addr3.String())
-	if foundP3 != nil {
-		t.Error("Got permission should be nil if not added.")
+	foundP3 := a.GetPermission(addr3)
+	if foundP3 != p3 {
+		t.Error("Permission with another IP should be found")
 	}
 }
 
@@ -69,7 +73,7 @@ func subTestAddPermission(t *testing.T) {
 		t.Error("Permission's allocation should be the adder.")
 	}
 
-	foundPermission := a.GetPermission(p.Addr.String())
+	foundPermission := a.GetPermission(p.Addr)
 	if foundPermission != p {
 		t.Error("Got permission is not same as the the added.")
 	}
@@ -85,14 +89,14 @@ func subTestRemovePermission(t *testing.T) {
 
 	a.AddPermission(p)
 
-	foundPermission := a.GetPermission(p.Addr.String())
+	foundPermission := a.GetPermission(p.Addr)
 	if foundPermission != p {
 		t.Error("Got permission is not same as the the added.")
 	}
 
-	a.RemovePermission(p.Addr.String())
+	a.RemovePermission(p.Addr)
 
-	foundPermission = a.GetPermission(p.Addr.String())
+	foundPermission = a.GetPermission(p.Addr)
 	if foundPermission != nil {
 		t.Error("Got permission should be nil after removed.")
 	}