Skip to content
This repository has been archived by the owner on Aug 19, 2022. It is now read-only.

feat: Allowlist #47

Merged
merged 24 commits into from
Jul 1, 2022
Merged
Show file tree
Hide file tree
Changes from 7 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
190 changes: 190 additions & 0 deletions allowlist.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
package rcmgr

import (
"bytes"
"errors"
"net"

"github.com/libp2p/go-libp2p-core/peer"
"github.com/multiformats/go-multiaddr"
manet "github.com/multiformats/go-multiaddr/net"
)

type allowlist struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we have an interface type assertion for this, right after the def?
Or is there no public interface?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to make a public interface for it in extapi, and provide an accessor so that users can modify after initialization.
The RWMutex is a must in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't follow the reason for a public interface? I thought generally interfaces belonged to the caller.

Interfaces declare the behaviour the caller requires not the behaviour the type will provide. Let callers define an interface that describes the behaviour they expect. The interface belongs to them, the consumer, not you.

From: https://dave.cheney.net/practical-go/presentations/gophercon-israel.html#_let_callers_define_the_interface_they_require

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah is it because we return an interface and we can't have an interface function return this unexported type?

What's the downside of exporting this type then?

// a simple structure of lists of networks. There is probably a faster way
// to check if an IP address is in this network than iterating over this
// list, but this is good enough for small numbers of networks (<1_000).
// Analyze the benchmark before trying to optimize this.

// Any peer with these IPs are allowed
allowedNetworks []*net.IPNet

// Only the specified peers can use these IPs
allowedPeerByNetwork map[peer.ID][]*net.IPNet
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you considered unifying these into a single map? map[net.IPNet]struct{ allowsAll bool; map[peer.ID]struct{} }
allowsAll would be set if a multiaddr without /p2p is added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can I use net.IPNet as a key? The struct holds two byteslices, so I don't think I can.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just checked, you can't. That's annoying. You could use net.IPNet.String(), but not sure if that's nicer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don’t think so because:

  1. allow checking is harder. you have to convert between the string and bytes.
  2. You need to allocate the string on insertion.
  3. you need to allocate at least once when doing an allowlist check.
  4. (Depending on check implementation) you lose the quick lookup by peerid. Which may be a pretty common use case.

}

func newAllowList() allowlist {
return allowlist{
allowedPeerByNetwork: make(map[peer.ID][]*net.IPNet),
}
}

func toIPNet(ma multiaddr.Multiaddr) (*net.IPNet, string, error) {
MarcoPolo marked this conversation as resolved.
Show resolved Hide resolved
var ipString string
var mask string
var allowedPeer string
var isIPV4 bool

multiaddr.ForEach(ma, func(c multiaddr.Component) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use ma.MultiaddrToIPNet here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably not, if we want to extract the p2p component at the same time, right? We could get that one without iterating again by splitting off the last component though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also need the isIPV4 to save some time below

if c.Protocol().Code == multiaddr.P_IP4 || c.Protocol().Code == multiaddr.P_IP6 {
isIPV4 = c.Protocol().Code == multiaddr.P_IP4
ipString = c.Value()
}
if c.Protocol().Code == multiaddr.P_IPCIDR {
mask = c.Value()
}
if c.Protocol().Code == multiaddr.P_P2P {
allowedPeer = c.Value()
}
return ipString == "" || mask == "" || allowedPeer == ""
})

if ipString == "" {
return nil, allowedPeer, errors.New("missing ip address")
}

if mask == "" {
ip := net.ParseIP(ipString)
if ip == nil {
return nil, allowedPeer, errors.New("invalid ip address")
}
var mask net.IPMask
if isIPV4 {
mask = net.CIDRMask(32, 32)
} else {
mask = net.CIDRMask(128, 128)
}

net := &net.IPNet{IP: ip, Mask: mask}
return net, allowedPeer, nil
}

_, ipnet, err := net.ParseCIDR(ipString + "/" + mask)
return ipnet, allowedPeer, err

}

func (al *allowlist) Add(ma multiaddr.Multiaddr) error {
MarcoPolo marked this conversation as resolved.
Show resolved Hide resolved
MarcoPolo marked this conversation as resolved.
Show resolved Hide resolved
ipnet, allowedPeerStr, err := toIPNet(ma)
if err != nil {
return err
}

if allowedPeerStr != "" {
// We have a peerID constraint
allowedPeer, err := peer.Decode(allowedPeerStr)
if err != nil {
return err
}
if ipnet != nil {
al.allowedPeerByNetwork[allowedPeer] = append(al.allowedPeerByNetwork[allowedPeer], ipnet)
}
} else {
if ipnet != nil {
MarcoPolo marked this conversation as resolved.
Show resolved Hide resolved
al.allowedNetworks = append(al.allowedNetworks, ipnet)
}
}
return nil
}

func (al *allowlist) Remove(ma multiaddr.Multiaddr) error {
ipnet, allowedPeerStr, err := toIPNet(ma)
if err != nil {
return err
}
ipNetList := al.allowedNetworks

var allowedPeer peer.ID
if allowedPeerStr != "" {
// We have a peerID constraint
allowedPeer, err = peer.Decode(allowedPeerStr)
if err != nil {
return err
}
ipNetList = al.allowedPeerByNetwork[allowedPeer]
}

if ipnet != nil {
i := len(ipNetList)
for i > 0 {
i--
if ipNetList[i].IP.Equal(ipnet.IP) && bytes.Equal(ipNetList[i].Mask, ipnet.Mask) {
if i == len(ipNetList)-1 {
// Trim this element from the end
ipNetList = ipNetList[:i]
} else {
// swap remove
ipNetList[i] = ipNetList[len(ipNetList)-1]
ipNetList = ipNetList[:len(ipNetList)-1]
}
}
}
}

if allowedPeer != "" {
al.allowedPeerByNetwork[allowedPeer] = ipNetList
} else {
al.allowedNetworks = ipNetList
}

return nil
}

func (al *allowlist) Allowed(ma multiaddr.Multiaddr) bool {
ip, err := manet.ToIP(ma)
MarcoPolo marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return false
}

_ = ip
MarcoPolo marked this conversation as resolved.
Show resolved Hide resolved
for _, network := range al.allowedNetworks {
_ = network
MarcoPolo marked this conversation as resolved.
Show resolved Hide resolved
if network.Contains(ip) {
return true
}
}

for _, allowedNetworks := range al.allowedPeerByNetwork {
for _, network := range allowedNetworks {
if network.Contains(ip) {
return true
}
}
}

return false
}

func (al *allowlist) AllowedPeerAndMultiaddr(peerID peer.ID, ma multiaddr.Multiaddr) bool {
ip, err := manet.ToIP(ma)
if err != nil {
return false
}

for _, network := range al.allowedNetworks {
if network.Contains(ip) {
// We found a match that isn't constrained by a peerID
return true
}
}

if expectedNetworks, ok := al.allowedPeerByNetwork[peerID]; ok {
for _, expectedNetwork := range expectedNetworks {
if expectedNetwork.Contains(ip) {
return true
}
}
}

return false
}
Loading