diff --git a/CHANGELOG.md b/CHANGELOG.md index 44a24234..f3b92deb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ BREAKING CHANGES: IMPROVEMENTS: - [abci, libs/common] Generated gogoproto static marshaller methods - [config] Increase default send/recv rates to 5 mB/s +- [p2p] reject addresses coming from private peers - [p2p] allow persistent peers to be private BUG FIXES: diff --git a/docs/spec/reactors/pex/pex.md b/docs/spec/reactors/pex/pex.md index 317803b8..0f13c0cb 100644 --- a/docs/spec/reactors/pex/pex.md +++ b/docs/spec/reactors/pex/pex.md @@ -12,7 +12,8 @@ them. Some peers can be marked as `private`, which means we will not put them in the address book or gossip them to others. -All peers except private peers are tracked using the address book. +All peers except private peers and peers coming from them are tracked using the +address book. ## Discovery diff --git a/p2p/pex/addrbook.go b/p2p/pex/addrbook.go index c630d14c..ef7d7eda 100644 --- a/p2p/pex/addrbook.go +++ b/p2p/pex/addrbook.go @@ -638,6 +638,7 @@ func (a *addrBook) addAddress(addr, src *p2p.NetAddress) error { if a.routabilityStrict && !addr.Routable() { return ErrAddrBookNonRoutable{addr} } + // TODO: we should track ourAddrs by ID and by IP:PORT and refuse both. if _, ok := a.ourAddrs[addr.String()]; ok { return ErrAddrBookSelf{addr} @@ -647,6 +648,10 @@ func (a *addrBook) addAddress(addr, src *p2p.NetAddress) error { return ErrAddrBookPrivate{addr} } + if _, ok := a.privateIDs[src.ID]; ok { + return ErrAddrBookPrivateSrc{src} + } + ka := a.addrLookup[addr.ID] if ka != nil { // If its already old and the addr is the same, ignore it. diff --git a/p2p/pex/addrbook_test.go b/p2p/pex/addrbook_test.go index 8b64c380..761c1187 100644 --- a/p2p/pex/addrbook_test.go +++ b/p2p/pex/addrbook_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" cmn "github.com/tendermint/tendermint/libs/common" "github.com/tendermint/tendermint/libs/log" @@ -374,10 +373,19 @@ func TestPrivatePeers(t *testing.T) { } book.AddPrivateIDs(private) + // private addrs must not be added for _, addr := range addrs { err := book.AddAddress(addr, addr) - require.Error(t, err, "AddAddress should have failed with private peer %s", addr) - _, ok := err.(ErrAddrBookPrivate) - require.True(t, ok, "Wrong error type, wanted ErrAddrBookPrivate, got error: %s", err) + if assert.Error(t, err) { + _, ok := err.(ErrAddrBookPrivate) + assert.True(t, ok) + } + } + + // addrs coming from private peers must not be added + err := book.AddAddress(randIPv4Address(t), addrs[0]) + if assert.Error(t, err) { + _, ok := err.(ErrAddrBookPrivateSrc) + assert.True(t, ok) } } diff --git a/p2p/pex/errors.go b/p2p/pex/errors.go index 34bfb5ab..7f660bdc 100644 --- a/p2p/pex/errors.go +++ b/p2p/pex/errors.go @@ -30,6 +30,14 @@ func (err ErrAddrBookPrivate) Error() string { return fmt.Sprintf("Cannot add private peer with address %v", err.Addr) } +type ErrAddrBookPrivateSrc struct { + Src *p2p.NetAddress +} + +func (err ErrAddrBookPrivateSrc) Error() string { + return fmt.Sprintf("Cannot add peer coming from private peer with address %v", err.Src) +} + type ErrAddrBookNilAddr struct { Addr *p2p.NetAddress Src *p2p.NetAddress