From 500bb51759503cf5481d7150c8a4a1d791ba9078 Mon Sep 17 00:00:00 2001 From: Juan Batiz-Benet Date: Sun, 7 Jun 2015 17:00:56 -0700 Subject: [PATCH] p2p/net/identify: clump addr observers into groups Different mutliaddrs is not enough. Nodes may share transports. NAT port mappings will likely only work on the base IP/TCP port pair. We go one step further, and require different root (IP) addrs. Just in case some NATs group by IP. In practice, this is what we want: use addresses only if hosts that are on different parts of the network have seen this address. --- p2p/protocol/identify/obsaddr.go | 18 ++++++++++++++++-- p2p/protocol/identify/obsaddr_test.go | 14 +++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/p2p/protocol/identify/obsaddr.go b/p2p/protocol/identify/obsaddr.go index c55757df3..f4033949b 100644 --- a/p2p/protocol/identify/obsaddr.go +++ b/p2p/protocol/identify/obsaddr.go @@ -85,11 +85,25 @@ func (oas *ObservedAddrSet) Add(addr ma.Multiaddr, observer ma.Multiaddr) { oas.addrs[s] = oa } - // Add current observer. - oa.SeenBy[observer.String()] = struct{}{} + // mark the observer + oa.SeenBy[observerGroup(observer)] = struct{}{} oa.LastSeen = time.Now() } +// observerGroup is a function that determines what part of +// a multiaddr counts as a different observer. for example, +// two ipfs nodes at the same IP/TCP transport would get +// the exact same NAT mapping; they would count as the +// same observer. This may protect against NATs who assign +// different ports to addresses at different IP hosts, but +// not TCP ports. +// +// Here, we use the root multiaddr address. This is mostly +// IP addresses. In practice, this is what we want. +func observerGroup(m ma.Multiaddr) string { + return ma.Split(m)[0].String() +} + func (oas *ObservedAddrSet) SetTTL(ttl time.Duration) { oas.Lock() defer oas.Unlock() diff --git a/p2p/protocol/identify/obsaddr_test.go b/p2p/protocol/identify/obsaddr_test.go index 416e8f681..dba769594 100644 --- a/p2p/protocol/identify/obsaddr_test.go +++ b/p2p/protocol/identify/obsaddr_test.go @@ -38,7 +38,8 @@ func TestObsAddrSet(t *testing.T) { a3 := m("/ip4/1.2.3.4/tcp/1233") a4 := m("/ip4/1.2.3.4/tcp/1234") a5 := m("/ip4/1.2.3.4/tcp/1235") - a6 := m("/ip4/1.2.3.4/tcp/1236") + a6 := m("/ip4/1.2.3.6/tcp/1236") + a7 := m("/ip4/1.2.3.7/tcp/1237") oas := ObservedAddrSet{} @@ -63,7 +64,15 @@ func TestObsAddrSet(t *testing.T) { t.Error("addrs should _still_ be empty (same obs)") } + // different observer, but same observer group. oas.Add(a1, a5) + oas.Add(a2, a5) + oas.Add(a3, a5) + if !addrsMarch(oas.Addrs(), nil) { + t.Error("addrs should _still_ be empty (same obs group)") + } + + oas.Add(a1, a6) if !addrsMarch(oas.Addrs(), []ma.Multiaddr{a1}) { t.Error("addrs should only have a1") } @@ -74,6 +83,9 @@ func TestObsAddrSet(t *testing.T) { oas.Add(a2, a6) oas.Add(a1, a6) oas.Add(a1, a6) + oas.Add(a2, a7) + oas.Add(a1, a7) + oas.Add(a1, a7) if !addrsMarch(oas.Addrs(), []ma.Multiaddr{a1, a2}) { t.Error("addrs should only have a1, a2") }