Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify Filter Whitelisting #92

Merged
merged 1 commit into from
Jun 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 21 additions & 79 deletions peerdiversity/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,18 @@ package peerdiversity
import (
"errors"
"fmt"
asnutil "github.com/libp2p/go-libp2p-asn-util"
"github.com/libp2p/go-libp2p-core/peer"
"net"
"sort"
"sync"

"github.com/libp2p/go-libp2p-core/peer"

"github.com/libp2p/go-cidranger"
asnutil "github.com/libp2p/go-libp2p-asn-util"

logging "github.com/ipfs/go-log"
ma "github.com/multiformats/go-multiaddr"
manet "github.com/multiformats/go-multiaddr-net"

"github.com/libp2p/go-cidranger"
)

var dfLog = logging.Logger("diversityFilter")
Expand All @@ -36,10 +37,9 @@ var legacyClassA = []string{"12.0.0.0/8", "17.0.0.0/8", "19.0.0.0/8", "38.0.0.0/

// PeerGroupInfo represents the grouping info for a Peer.
type PeerGroupInfo struct {
Id peer.ID
Cpl int
IPGroupKey PeerIPGroupKey
isWhiteListed bool
Id peer.ID
Cpl int
IPGroupKey PeerIPGroupKey
}

// PeerIPGroupFilter is the interface that must be implemented by callers who want to
Expand All @@ -49,10 +49,7 @@ type PeerIPGroupFilter interface {
// Allow is called by the Filter to test if a peer with the given
// grouping info should be allowed/rejected by the Filter. This will be called ONLY
// AFTER the peer has successfully passed all of the Filter's internal checks.
// Note: If the peer is deemed accepted because of a whitelisting criteria configured on the Filter,
// the peer will be allowed by the Filter without calling this function.
// Similarly, if the peer is deemed rejected because of a blacklisting criteria
// configured on the Filter, the peer will be rejected without calling this function.
// Note: If the peer is whitelisted on the Filter, the peer will be allowed by the Filter without calling this function.
Allow(PeerGroupInfo) (allow bool)

// Increment is called by the Filter when a peer with the given Grouping Info.
Expand All @@ -70,21 +67,18 @@ type PeerIPGroupFilter interface {
PeerAddresses(peer.ID) []ma.Multiaddr
}

// Filter is a peer diversity filter that accepts or rejects peers based on the blacklisting/whitelisting
// rules configured AND the diversity policies defined by the implementation of the PeerIPGroupFilter interface
// Filter is a peer diversity filter that accepts or rejects peers based on the whitelisting rules configured
// AND the diversity policies defined by the implementation of the PeerIPGroupFilter interface
// passed to it.
type Filter struct {
mu sync.Mutex
// An implementation of the `PeerIPGroupFilter` interface defined above.
pgm PeerIPGroupFilter
peerGroups map[peer.ID][]PeerGroupInfo

// whitelist peers
// whitelisted peers
wlpeers map[peer.ID]struct{}
// whitelisted Networks
wls cidranger.Ranger
// blacklisted Networks.
bls cidranger.Ranger

// legacy IPv4 Class A networks.
legacyCidrs cidranger.Ranger

Expand Down Expand Up @@ -119,8 +113,6 @@ func NewFilter(pgm PeerIPGroupFilter, logKey string, cplFnc func(peer.ID) int) (
pgm: pgm,
peerGroups: make(map[peer.ID][]PeerGroupInfo),
wlpeers: make(map[peer.ID]struct{}),
wls: cidranger.NewPCTrieRanger(),
bls: cidranger.NewPCTrieRanger(),
legacyCidrs: legacyCidrs,
logKey: logKey,
cplFnc: cplFnc,
Expand All @@ -136,9 +128,6 @@ func (f *Filter) Remove(p peer.ID) {
cpl := f.cplFnc(p)

for _, info := range f.peerGroups[p] {
if info.isWhiteListed {
continue
}
f.pgm.Decrement(info)
}
f.peerGroups[p] = nil
Expand All @@ -155,6 +144,10 @@ func (f *Filter) TryAdd(p peer.ID) bool {
f.mu.Lock()
defer f.mu.Unlock()

if _, ok := f.wlpeers[p]; ok {
return true
}

cpl := f.cplFnc(p)

// don't allow peers for which we can't determine addresses.
Expand All @@ -165,11 +158,7 @@ func (f *Filter) TryAdd(p peer.ID) bool {
}

peerGroups := make([]PeerGroupInfo, 0, len(addrs))
isWhiteListed := false
for _, a := range addrs {
// if the IP belongs to a whitelisted network, allow it straight away.
// if the IP belongs to a blacklisted network, reject it.
// Otherwise, call the `PeerIPGroupFilter.Allow` hook to determine if we should allow/reject the peer.
ip, err := manet.ToIP(a)
if err != nil {
dfLog.Errorw("failed to parse IP from multiaddr", "appKey", f.logKey,
Expand All @@ -185,29 +174,11 @@ func (f *Filter) TryAdd(p peer.ID) bool {
return false
}
if len(key) == 0 {
dfLog.Errorw("group key is empty", "appKey", f.logKey, "ip", ip.String(), "peer", p)
dfLog.Debugw("group key is empty", "appKey", f.logKey, "ip", ip.String(), "peer", p)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aschmahmann This is intentional as it causes needless noise in the logs.

return false
}
group := PeerGroupInfo{Id: p, Cpl: cpl, IPGroupKey: key}

// is it a whitelisted peer
if _, ok := f.wlpeers[p]; ok {
isWhiteListed = true
peerGroups = append(peerGroups, group)
continue
}

// is it on a whitelisted network
if rs, _ := f.wls.ContainingNetworks(ip); len(rs) != 0 {
isWhiteListed = true
peerGroups = append(peerGroups, group)
continue
}

if rs, _ := f.bls.ContainingNetworks(ip); len(rs) != 0 {
return false
}

if !f.pgm.Allow(group) {
return false
}
Expand All @@ -220,45 +191,16 @@ func (f *Filter) TryAdd(p peer.ID) bool {
}

for _, g := range peerGroups {
g.isWhiteListed = isWhiteListed
if !g.isWhiteListed {
f.pgm.Increment(g)
}
f.pgm.Increment(g)

f.peerGroups[p] = append(f.peerGroups[p], g)
f.cplPeerGroups[cpl][p] = append(f.cplPeerGroups[cpl][p], g.IPGroupKey)
}

return true
}

// BlacklistIPv4Network will blacklist the IPv4/6 network with the given IP CIDR.
func (f *Filter) BlacklistIPNetwork(cidr string) error {
f.mu.Lock()
defer f.mu.Unlock()

_, nn, err := net.ParseCIDR(cidr)
if err != nil {
return err
}
return f.bls.Insert(cidranger.NewBasicRangerEntry(*nn))
}

// WhitelistIPNetwork will always allow IP addresses from networks with the given CIDR.
// This will always override the blacklist.
func (f *Filter) WhitelistIPNetwork(cidr string) error {
f.mu.Lock()
defer f.mu.Unlock()

_, nn, err := net.ParseCIDR(cidr)
if err != nil {
return err
}

return f.wls.Insert(cidranger.NewBasicRangerEntry(*nn))
}

// WhiteListPeerIds will always allow the peers given here.
// This will always override the blacklist.
// WhitelistPeers will always allow the given peers.
func (f *Filter) WhitelistPeers(peers ...peer.ID) {
f.mu.Lock()
defer f.mu.Unlock()
Expand Down
49 changes: 6 additions & 43 deletions peerdiversity/filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,19 +148,16 @@ func TestDiversityFilter(t *testing.T) {
},
isWhitelisted: true,
},

"whitelisted network": {
"whitelist peers works even if peer has no addresses": {
peersForTest: func() []peer.ID {
return []peer.ID{"p1", "p2"}
},
mFnc: func(m *mockPeerGroupFilter) {
m.peerAddressFunc = func(id peer.ID) []ma.Multiaddr {
if id == "p1" {
return []ma.Multiaddr{ma.StringCast("/ip4/127.0.0.1/tcp/0"),
ma.StringCast("/ip4/127.0.0.1/tcp/0")}
return []ma.Multiaddr{ma.StringCast("/ip4/127.0.0.1/tcp/0")}
} else {
return []ma.Multiaddr{ma.StringCast("/ip4/127.0.0.1/tcp/0"),
ma.StringCast("/ip4/192.168.1.1/tcp/0")}
return nil
}
}

Expand All @@ -169,49 +166,15 @@ func TestDiversityFilter(t *testing.T) {
}
},
allowed: map[peer.ID]bool{
"p1": true,
"p2": false,
"p1": false,
"p2": true,
},
fFnc: func(f *Filter) {
err := f.WhitelistIPNetwork("127.0.0.1/16")
if err != nil {
t.Fatal(err)
}
f.WhitelistPeers(peer.ID("p2"))
},
isWhitelisted: true,
},

"blacklisting": {
peersForTest: func() []peer.ID {
return []peer.ID{"p1", "p2"}
},
mFnc: func(m *mockPeerGroupFilter) {
m.peerAddressFunc = func(id peer.ID) []ma.Multiaddr {
if id == "p1" {
return []ma.Multiaddr{ma.StringCast("/ip4/127.0.0.1/tcp/0"),
ma.StringCast("/ip4/127.0.0.1/tcp/0")}
} else {
return []ma.Multiaddr{ma.StringCast("/ip4/127.0.0.1/tcp/0"),
ma.StringCast("/ip4/192.168.1.1/tcp/0")}
}
}

m.allowFnc = func(g PeerGroupInfo) bool {
return true
}
},
allowed: map[peer.ID]bool{
"p1": true,
"p2": false,
},
fFnc: func(f *Filter) {
err := f.BlacklistIPNetwork("192.168.1.1/16")
if err != nil {
t.Fatal(err)
}
},
},

"peer has no addresses": {
peersForTest: func() []peer.ID {
return []peer.ID{"p1"}
Expand Down