netbird/ice
0
0
Fork 0
mirror of https://github.com/netbirdio/ice.git synced 2026-05-22 17:10:58 -07:00
Code Issues Packages Projects Releases Wiki Activity

Assert bindingSuccessResponse is non-symmetric

Browse Source 
We previously didn't correctly follow [0] This updates
the agent to assert that a bindingSuccessResponse is
for the address it was sent too.

https://tools.ietf.org/html/rfc8445#section-7.2.5.2.1
This commit is contained in:
Sean DuBois
2019-04-28 17:53:21 -07:00
parent bb69dcb592
commit 5ca0eeb721
4 changed files with 159 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files
agent.go
+71 -39
View File
@@ -47,6 +47,11 @@ func (bp byPairPriority) Less(i, j int) bool {
return bp.candidatePairs[i].Priority() > bp.candidatePairs[j].Priority()
}
type bindingRequest struct {
transactionID []byte
destination net.Addr
}
// Agent represents the ICE agent
type Agent struct {
onConnectionStateChangeHdlr func(ConnectionState)
@@ -99,7 +104,7 @@ type Agent struct {
buffer *packetio.Buffer
// LRU of outbound Binding request Transaction IDs
pendingBindingRequests [][]byte
pendingBindingRequests []bindingRequest
// State for closing
done chan struct{}
@@ -172,7 +177,7 @@ func NewAgent(config *AgentConfig) (*Agent, error) {
connectionState: ConnectionStateNew,
localCandidates: make(map[NetworkType][]*Candidate),
remoteCandidates: make(map[NetworkType][]*Candidate),
pendingBindingRequests: make([][]byte, 0, maxPendingBindingRequests),
pendingBindingRequests: make([]bindingRequest, 0, maxPendingBindingRequests),
localUfrag: randSeq(16),
localPwd: randSeq(32),
@@ -313,7 +318,10 @@ func (a *Agent) pingCandidate(local, remote *Candidate) {
a.pendingBindingRequests = a.pendingBindingRequests[overflow:]
}
a.pendingBindingRequests = append(a.pendingBindingRequests, transactionID)
a.pendingBindingRequests = append(a.pendingBindingRequests, bindingRequest{
transactionID: transactionID,
destination: remote.addr(),
})
a.sendSTUN(msg, local, remote)
}
@@ -622,58 +630,82 @@ func (a *Agent) handleInboundControlling(m *stun.Message, localCandidate, remote
}
}
// Assert that the passed TransactionID is in our pendingBindingRequests and remove if it is
func (a *Agent) handleInboundBindingSuccess(id []byte) bool {
// Assert that the passed TransactionID is in our pendingBindingRequests and returns the destination
// If the bindingRequest was valid remove it from our pending cache
func (a *Agent) handleInboundBindingSuccess(id []byte) (bool, net.Addr) {
for i := range a.pendingBindingRequests {
if bytes.Equal(a.pendingBindingRequests[i], id) {
a.pendingBindingRequests = append(a.pendingBindingRequests[:i], a.pendingBindingRequests[i+1:]...)
return true
if bytes.Equal(a.pendingBindingRequests[i].transactionID, id) {
defer func() {
a.pendingBindingRequests = append(a.pendingBindingRequests[:i], a.pendingBindingRequests[i+1:]...)
}()
return true, a.pendingBindingRequests[i].destination
}
}
return false
return false, nil
}
// handleInbound processes STUN traffic from a remote candidate
func (a *Agent) handleInbound(m *stun.Message, local *Candidate, remote net.Addr) {
var err error
if m == nil || local == nil {
return
}
a.log.Tracef("inbound STUN from %s to %s", remote.String(), local.String())
switch {
case m.Method == stun.MethodBinding && m.Class == stun.ClassSuccessResponse:
if err := assertInboundMessageIntegrity(m, []byte(a.remotePwd)); err != nil {
a.log.Warnf("discard message from (%s), %v", remote, err)
return
} else if !a.handleInboundBindingSuccess(m.TransactionID) {
a.log.Warnf("discard message from (%s), invalid TransactionID %s", remote, m.TransactionID)
return
}
case m.Method == stun.MethodBinding && m.Class == stun.ClassRequest:
if err := assertInboundUsername(m, a.localUfrag+":"+a.remoteUfrag); err != nil {
a.log.Warnf("discard message from (%s), %v", remote, err)
return
} else if err := assertInboundMessageIntegrity(m, []byte(a.localPwd)); err != nil {
a.log.Warnf("discard message from (%s), %v", remote, err)
return
}
default:
} else if m.Method != stun.MethodBinding || !(m.Class == stun.ClassSuccessResponse || m.Class == stun.ClassRequest) {
a.log.Tracef("unhandled STUN from %s to %s class(%s) method(%s)", remote.String(), local.String(), m.Method.String(), m.Class.String())
return
}
remoteCandidate := a.findRemoteCandidate(local.NetworkType, remote)
if remoteCandidate == nil {
a.log.Debugf("detected a new peer-reflexive candiate: %s ", remote)
pflxCandidate, err := handleNewPeerReflexiveCandidate(local, remote)
if err != nil {
a.log.Warn(err.Error())
if m.Class == stun.ClassSuccessResponse {
if err = assertInboundMessageIntegrity(m, []byte(a.remotePwd)); err != nil {
a.log.Warnf("discard message from (%s), %v", remote, err)
return
}
a.addRemoteCandidate(pflxCandidate)
return
}
remoteCandidate.seen(false)
ok, transactionAddr := a.handleInboundBindingSuccess(m.TransactionID)
if !ok {
a.log.Errorf("discard message from (%s), invalid TransactionID %s", remote, m.TransactionID)
return
}
// Assert that NAT is not symmetric
// https://tools.ietf.org/html/rfc8445#section-7.2.5.2.1
if !addrEqual(transactionAddr, remote) {
a.log.Debugf("discard message: transaction source and destination does not match expected(%s), actual(%s)", transactionAddr, remote)
return
} else if remoteCandidate == nil { // Should fail previous check, better to be safe though
a.log.Warnf("discard success message from (%s), no such remote", remote)
return
}
} else {
if err = assertInboundUsername(m, a.localUfrag+":"+a.remoteUfrag); err != nil {
a.log.Warnf("discard message from (%s), %v", remote, err)
return
} else if err = assertInboundMessageIntegrity(m, []byte(a.localPwd)); err != nil {
a.log.Warnf("discard message from (%s), %v", remote, err)
return
}
if remoteCandidate == nil {
ip, port, networkType, ok := parseAddr(remote)
if !ok {
a.log.Errorf("Failed to create parse remote net.Addr when creating remote prflx candidate")
return
}
prflxCandidate, err := NewCandidatePeerReflexive(networkType.String(), ip, port, local.Component, "", 0)
if err != nil {
a.log.Errorf("Failed to create new remote prflx candidate (%s)", err)
return
}
remoteCandidate = prflxCandidate
a.log.Debugf("adding a new peer-reflexive candiate: %s ", remote)
a.addRemoteCandidate(remoteCandidate)
}
}
a.log.Tracef("inbound STUN from %s to %s", remote.String(), local.String())
remoteCandidate.seen(false)
if a.isControlling {
a.handleInboundControlling(m, local, remoteCandidate)
} else {
agent_test.go
+64 -41
View File
@@ -10,6 +10,17 @@ import (
"github.com/pion/transport/test"
)
type mockPacketConn struct {
}
func (m *mockPacketConn) ReadFrom(p []byte) (n int, addr net.Addr, err error) { return 0, nil, nil }
func (m *mockPacketConn) WriteTo(p []byte, addr net.Addr) (n int, err error) { return 0, nil }
func (m *mockPacketConn) Close() error { return nil }
func (m *mockPacketConn) LocalAddr() net.Addr { return nil }
func (m *mockPacketConn) SetDeadline(t time.Time) error { return nil }
func (m *mockPacketConn) SetReadDeadline(t time.Time) error { return nil }
func (m *mockPacketConn) SetWriteDeadline(t time.Time) error { return nil }
func TestPairSearch(t *testing.T) {
// Limit runtime in case of deadlocks
lim := test.TimeOut(time.Second * 10)
@@ -185,7 +196,7 @@ func TestHandlePeerReflexive(t *testing.T) {
lim := test.TimeOut(time.Second * 2)
defer lim.Stop()
t.Run("UDP pflx candidate from handleInboud()", func(t *testing.T) {
t.Run("UDP pflx candidate from handleInbound()", func(t *testing.T) {
var config AgentConfig
a, err := NewAgent(&config)
@@ -195,6 +206,7 @@ func TestHandlePeerReflexive(t *testing.T) {
ip := net.ParseIP("192.168.0.2")
local, err := NewCandidateHost("udp", ip, 777, 1)
local.conn = &mockPacketConn{}
if err != nil {
t.Fatalf("failed to create a new candidate: %v", err)
}
@@ -276,56 +288,36 @@ func TestHandlePeerReflexive(t *testing.T) {
}
})
t.Run("TCP prflx with handleNewPeerReflexiveCandidate()", func(t *testing.T) {
var config AgentConfig
a, err := NewAgent(&config)
t.Run("Success from unknown remote, prflx candidate MUST only be created via Binding Request", func(t *testing.T) {
a, err := NewAgent(&AgentConfig{})
if err != nil {
t.Fatal("Error constructing ice.Agent")
t.Fatalf("Error constructing ice.Agent")
}
ip := net.ParseIP("192.168.0.2")
local, err := NewCandidateHost("tcp", ip, 777, 1)
a.pendingBindingRequests = []bindingRequest{
{[]byte("ABC"), &net.UDPAddr{}},
}
local, err := NewCandidateHost("udp", net.ParseIP("192.168.0.2"), 777, 1)
local.conn = &mockPacketConn{}
if err != nil {
t.Fatalf("failed to create a new candidate: %v", err)
}
remote := &net.TCPAddr{IP: net.ParseIP("172.17.0.3"), Port: 999}
candidate, err := handleNewPeerReflexiveCandidate(local, remote)
remote := &net.UDPAddr{IP: net.ParseIP("172.17.0.3"), Port: 999}
msg, err := stun.Build(stun.ClassSuccessResponse, stun.MethodBinding, []byte("ABC"),
&stun.MessageIntegrity{
Key: []byte(a.remotePwd),
},
&stun.Fingerprint{},
)
if err != nil {
t.Fatalf("handleNewPeerReflexiveCandidate() should not fail: %v", err)
}
a.addRemoteCandidate(candidate)
// length of remote candidate list must be one now
if len(a.remoteCandidates) != 1 {
t.Fatal("failed to add a network type to the remote candidate list")
t.Fatal(err)
}
// length of remote candidate list for a network type must be 1
set := a.remoteCandidates[local.NetworkType]
if len(set) != 1 {
t.Fatal("failed to add prflx candidate to remote candidate list")
}
c := set[0]
if c.Type != CandidateTypePeerReflexive {
t.Fatal("candidate type must be prflx")
}
if !c.IP.Equal(net.ParseIP("172.17.0.3")) {
t.Fatal("IP address mismatch")
}
if c.Port != 999 {
t.Fatal("Port number mismatch")
}
err = a.Close()
if err != nil {
t.Fatalf("Close agent emits error %v", err)
a.handleInbound(msg, local, remote)
if len(a.remoteCandidates) != 0 {
t.Fatal("unknown remote was able to create a candidate")
}
})
}
@@ -387,6 +379,7 @@ func TestInboundValidity(t *testing.T) {
remote := &net.UDPAddr{IP: net.ParseIP("172.17.0.3"), Port: 999}
local, err := NewCandidateHost("udp", net.ParseIP("192.168.0.2"), 777, 1)
local.conn = &mockPacketConn{}
if err != nil {
t.Fatalf("failed to create a new candidate: %v", err)
}
@@ -465,6 +458,36 @@ func TestInboundValidity(t *testing.T) {
t.Fatal("Binding with valid values (but no fingerprint) was unable to create prflx candidate")
}
})
t.Run("Success with invalid TransactionID", func(t *testing.T) {
a, err := NewAgent(&AgentConfig{})
if err != nil {
t.Fatalf("Error constructing ice.Agent")
}
local, err := NewCandidateHost("udp", net.ParseIP("192.168.0.2"), 777, 1)
local.conn = &mockPacketConn{}
if err != nil {
t.Fatalf("failed to create a new candidate: %v", err)
}
remote := &net.UDPAddr{IP: net.ParseIP("172.17.0.3"), Port: 999}
msg, err := stun.Build(stun.ClassSuccessResponse, stun.MethodBinding, []byte("ABC"),
&stun.MessageIntegrity{
Key: []byte(a.remotePwd),
},
&stun.Fingerprint{},
)
if err != nil {
t.Fatal(err)
}
a.handleInbound(msg, local, remote)
if len(a.remoteCandidates) != 0 {
t.Fatal("unknown remote was able to create a candidate")
}
})
}
func TestInvalidAgentStarts(t *testing.T) {
candidate.go
-33
View File
@@ -117,39 +117,6 @@ func NewCandidateRelay(network string, ip net.IP, port int, component uint16, re
}, nil
}
// handleNewPeerReflexiveCandidate creates a ReflexiveCandidate from a remote transport address
func handleNewPeerReflexiveCandidate(local *Candidate, remote net.Addr) (*Candidate, error) {
var ip net.IP
var port int
switch addr := remote.(type) {
case *net.UDPAddr:
ip = addr.IP
port = addr.Port
case *net.TCPAddr:
ip = addr.IP
port = addr.Port
default:
return nil, fmt.Errorf("unsupported address type %T", addr)
}
pflxCandidate, err := NewCandidatePeerReflexive(
local.NetworkType.String(), // assume, same as that of local
ip,
port,
local.Component,
"", // unknown at this moment. TODO: need a review
0, // unknown at this moment. TODO: need a review
)
if err != nil {
return nil, flattenErrs([]error{fmt.Errorf("failed to create peer-reflexive candidate: %v", remote), err})
}
// Add pflxCandidate to the remote candidate list
return pflxCandidate, nil
}
// start runs the candidate using the provided connection
func (c *Candidate) start(a *Agent, conn net.PacketConn) {
c.agent = a
util.go
+24
View File
@@ -68,3 +68,27 @@ func flattenErrs(errs []error) error {
return fmt.Errorf(strings.Join(errstrings, "\n"))
}
func parseAddr(in net.Addr) (net.IP, int, NetworkType, bool) {
switch addr := in.(type) {
case *net.UDPAddr:
return addr.IP, addr.Port, NetworkTypeUDP4, true
case *net.TCPAddr:
return addr.IP, addr.Port, NetworkTypeTCP4, true
}
return nil, 0, 0, false
}
func addrEqual(a, b net.Addr) bool {
aIP, aPort, aType, aOk := parseAddr(a)
if !aOk {
return false
}
bIP, bPort, bType, bOk := parseAddr(b)
if !bOk {
return false
}
return aType == bType && aIP.Equal(bIP) && aPort == bPort
}