p2p/net/identify: clump addr observers into groups

jbenet · jbenet · commit 500bb5175950 · 2015-06-07T17:00:56.000-07:00
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.
diff --git 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
@@ -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")
 	}
