diff --git a/core/core.go b/core/core.go index be8f8e643..0a008c83c 100644 --- a/core/core.go +++ b/core/core.go @@ -359,7 +359,7 @@ func constructPeerHost(ctx context.Context, ctxg ctxgroup.ContextGroup, cfg *con // make sure we error out if our config does not have addresses we can use log.Debugf("Config.Addresses.Swarm:%s", listenAddrs) - filteredAddrs := addrutil.FilterAddrs(listenAddrs) + filteredAddrs := addrutil.FilterUsableAddrs(listenAddrs) log.Debugf("Config.Addresses.Swarm:%s (filtered)", listenAddrs) if len(filteredAddrs) < 1 { return nil, debugerror.Errorf("addresses in config not usable: %s", listenAddrs) diff --git a/p2p/net/swarm/addr/addr.go b/p2p/net/swarm/addr/addr.go index 91e8febf4..bae6e8605 100644 --- a/p2p/net/swarm/addr/addr.go +++ b/p2p/net/swarm/addr/addr.go @@ -41,19 +41,27 @@ func init() { SupportedTransportProtocols = transports } -// FilterAddrs is a filter that removes certain addresses -// from a list. the addresses removed are those known NOT -// to work with our network. Namely, addresses with UTP. -func FilterAddrs(a []ma.Multiaddr) []ma.Multiaddr { +// FilterAddrs is a filter that removes certain addresses, according to filter. +// if filter returns true, the address is kept. +func FilterAddrs(a []ma.Multiaddr, filter func(ma.Multiaddr) bool) []ma.Multiaddr { b := make([]ma.Multiaddr, 0, len(a)) for _, addr := range a { - if AddrUsable(addr, false) { + if filter(addr) { b = append(b, addr) } } return b } +// FilterUsableAddrs removes certain addresses +// from a list. the addresses removed are those known NOT +// to work with our network. Namely, addresses with UTP. +func FilterUsableAddrs(a []ma.Multiaddr) []ma.Multiaddr { + return FilterAddrs(a, func(m ma.Multiaddr) bool { + return AddrUsable(m, false) + }) +} + // AddrOverNonLocalIP returns whether the addr uses a non-local ip link func AddrOverNonLocalIP(a ma.Multiaddr) bool { split := ma.Split(a) @@ -228,13 +236,19 @@ func AddrIsShareableOnWAN(addr ma.Multiaddr) bool { // WANShareableAddrs filters addresses based on whether they're shareable on WAN func WANShareableAddrs(inp []ma.Multiaddr) []ma.Multiaddr { - out := make([]ma.Multiaddr, 0, len(inp)) - for _, a := range inp { - if AddrIsShareableOnWAN(a) { - out = append(out, a) + return FilterAddrs(inp, AddrIsShareableOnWAN) +} + +// Subtract filters out all addrs in b from a +func Subtract(a, b []ma.Multiaddr) []ma.Multiaddr { + return FilterAddrs(a, func(m ma.Multiaddr) bool { + for _, bb := range b { + if m.Equal(bb) { + return false + } } - } - return out + return true + }) } // CheckNATWarning checks if our observed addresses differ. if so, diff --git a/p2p/net/swarm/addr/addr_test.go b/p2p/net/swarm/addr/addr_test.go index 7f355ef5e..13e2028cf 100644 --- a/p2p/net/swarm/addr/addr_test.go +++ b/p2p/net/swarm/addr/addr_test.go @@ -53,9 +53,9 @@ func TestFilterAddrs(t *testing.T) { } } - subtestAddrsEqual(t, FilterAddrs(bad), []ma.Multiaddr{}) - subtestAddrsEqual(t, FilterAddrs(good), good) - subtestAddrsEqual(t, FilterAddrs(goodAndBad), good) + subtestAddrsEqual(t, FilterUsableAddrs(bad), []ma.Multiaddr{}) + subtestAddrsEqual(t, FilterUsableAddrs(good), good) + subtestAddrsEqual(t, FilterUsableAddrs(goodAndBad), good) } func subtestAddrsEqual(t *testing.T, a, b []ma.Multiaddr) { @@ -196,3 +196,37 @@ func TestWANShareable(t *testing.T) { t.Error("should be zero") } } + +func TestSubtract(t *testing.T) { + + a := []ma.Multiaddr{ + newMultiaddr(t, "/ip4/127.0.0.1/tcp/1234"), + newMultiaddr(t, "/ip4/0.0.0.0/tcp/1234"), + newMultiaddr(t, "/ip6/::1/tcp/1234"), + newMultiaddr(t, "/ip6/::/tcp/1234"), + newMultiaddr(t, "/ip6/fe80::1/tcp/1234"), + newMultiaddr(t, "/ip6/fe80::/tcp/1234"), + } + + b := []ma.Multiaddr{ + newMultiaddr(t, "/ip4/127.0.0.1/tcp/1234"), + newMultiaddr(t, "/ip6/::1/tcp/1234"), + newMultiaddr(t, "/ip6/fe80::1/tcp/1234"), + } + + c1 := []ma.Multiaddr{ + newMultiaddr(t, "/ip4/0.0.0.0/tcp/1234"), + newMultiaddr(t, "/ip6/::/tcp/1234"), + newMultiaddr(t, "/ip6/fe80::/tcp/1234"), + } + + c2 := Subtract(a, b) + if len(c1) != len(c2) { + t.Error("should be the same") + } + for i, ca := range c1 { + if !c2[i].Equal(ca) { + t.Error("should be the same", ca, c2[i]) + } + } +} diff --git a/p2p/net/swarm/swarm.go b/p2p/net/swarm/swarm.go index 54d53f8d9..8080fcde9 100644 --- a/p2p/net/swarm/swarm.go +++ b/p2p/net/swarm/swarm.go @@ -41,7 +41,7 @@ func NewSwarm(ctx context.Context, listenAddrs []ma.Multiaddr, local peer.ID, peers peer.Peerstore) (*Swarm, error) { if len(listenAddrs) > 0 { - filtered := addrutil.FilterAddrs(listenAddrs) + filtered := addrutil.FilterUsableAddrs(listenAddrs) if len(filtered) < 1 { return nil, fmt.Errorf("swarm cannot use any addr in: %s", listenAddrs) } diff --git a/p2p/net/swarm/swarm_addr_test.go b/p2p/net/swarm/swarm_addr_test.go index 65e196efa..6b0988d81 100644 --- a/p2p/net/swarm/swarm_addr_test.go +++ b/p2p/net/swarm/swarm_addr_test.go @@ -51,9 +51,9 @@ func TestFilterAddrs(t *testing.T) { } } - subtestAddrsEqual(t, addrutil.FilterAddrs(bad), []ma.Multiaddr{}) - subtestAddrsEqual(t, addrutil.FilterAddrs(good), good) - subtestAddrsEqual(t, addrutil.FilterAddrs(goodAndBad), good) + subtestAddrsEqual(t, addrutil.FilterUsableAddrs(bad), []ma.Multiaddr{}) + subtestAddrsEqual(t, addrutil.FilterUsableAddrs(good), good) + subtestAddrsEqual(t, addrutil.FilterUsableAddrs(goodAndBad), good) // now test it with swarm diff --git a/p2p/net/swarm/swarm_dial.go b/p2p/net/swarm/swarm_dial.go index d17d31c8d..96581009e 100644 --- a/p2p/net/swarm/swarm_dial.go +++ b/p2p/net/swarm/swarm_dial.go @@ -10,6 +10,7 @@ import ( lgbl "github.com/jbenet/go-ipfs/util/eventlog/loggables" context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" + ma "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-multiaddr" ) // Dial connects to a peer. @@ -38,17 +39,23 @@ func (s *Swarm) Dial(ctx context.Context, p peer.ID) (*Conn, error) { log.Warning("Dial not given PrivateKey, so WILL NOT SECURE conn.") } - remoteAddrs := s.peers.Addresses(p) - // make sure we can use the addresses. - remoteAddrs = addrutil.FilterAddrs(remoteAddrs) - if len(remoteAddrs) == 0 { - return nil, errors.New("peer has no addresses") - } + // get our own addrs localAddrs := s.peers.Addresses(s.local) if len(localAddrs) == 0 { log.Debug("Dialing out with no local addresses.") } + // get remote peer addrs + remoteAddrs := s.peers.Addresses(p) + // make sure we can use the addresses. + remoteAddrs = addrutil.FilterUsableAddrs(remoteAddrs) + // drop out any addrs that would just dial ourselves. use ListenAddresses + // as that is a more authoritative view than localAddrs. + remoteAddrs = addrutil.Subtract(remoteAddrs, s.ListenAddresses()) + if len(remoteAddrs) == 0 { + return nil, errors.New("peer has no addresses") + } + // open connection to peer d := &conn.Dialer{ LocalPeer: s.local, @@ -56,23 +63,11 @@ func (s *Swarm) Dial(ctx context.Context, p peer.ID) (*Conn, error) { PrivateKey: sk, } - // try to connect to one of the peer's known addresses. - // for simplicity, we do this sequentially. - // A future commit will do this asynchronously. - var connC conn.Conn - var err error - for _, addr := range remoteAddrs { - connC, err = d.Dial(ctx, addr, p) - if err == nil { - break - } - } + // try to get a connection to any addr + connC, err := s.dialAddrs(ctx, d, p, remoteAddrs) if err != nil { return nil, err } - if connC == nil { - err = fmt.Errorf("failed to dial %s", p) - } // ok try to setup the new connection. swarmC, err := dialConnSetup(ctx, s, connC) @@ -87,6 +82,39 @@ func (s *Swarm) Dial(ctx context.Context, p peer.ID) (*Conn, error) { return swarmC, nil } +func (s *Swarm) dialAddrs(ctx context.Context, d *conn.Dialer, p peer.ID, remoteAddrs []ma.Multiaddr) (conn.Conn, error) { + + // try to connect to one of the peer's known addresses. + // for simplicity, we do this sequentially. + // A future commit will do this asynchronously. + for _, addr := range remoteAddrs { + connC, err := d.Dial(ctx, addr, p) + if err != nil { + continue + } + + // if the connection is not to whom we thought it would be... + if connC.RemotePeer() != p { + log.Infof("misdial to %s through %s (got %s)", p, addr, connC.RemoteMultiaddr()) + connC.Close() + continue + } + + // if the connection is to ourselves... + // this can happen TONS when Loopback addrs are advertized. + // (this should be caught by two checks above, but let's just make sure.) + if connC.RemotePeer() == s.local { + log.Infof("misdial to %s through %s", p, addr) + connC.Close() + continue + } + + // success! we got one! + return connC, nil + } + return nil, fmt.Errorf("failed to dial %s", p) +} + // dialConnSetup is the setup logic for a connection from the dial side. it // needs to add the Conn to the StreamSwarm, then run newConnSetup func dialConnSetup(ctx context.Context, s *Swarm, connC conn.Conn) (*Conn, error) {