From 4a8611b23ef65d0309f96ed5c552d62e3e35a76d Mon Sep 17 00:00:00 2001 From: Matt Joiner Date: Wed, 12 Jan 2022 14:23:30 +1100 Subject: [PATCH] Do smart banning on existing badPeerIPs --- client.go | 38 ++++++++++++++++---------------------- peerconn.go | 12 ++++++------ smartban.go | 10 ++++++---- torrent.go | 38 +++++++++++++++++++++++++++++++++----- 4 files changed, 61 insertions(+), 37 deletions(-) diff --git a/client.go b/client.go index 2a208fc9..773159e3 100644 --- a/client.go +++ b/client.go @@ -13,6 +13,7 @@ import ( "math" "net" "net/http" + "net/netip" "sort" "strconv" "strings" @@ -28,6 +29,7 @@ import ( "github.com/anacrolix/missinggo/v2/bitmap" "github.com/anacrolix/missinggo/v2/pproffd" "github.com/anacrolix/sync" + "github.com/anacrolix/torrent/generics" "github.com/anacrolix/torrent/option" request_strategy "github.com/anacrolix/torrent/request-strategy" "github.com/davecgh/go-spew/spew" @@ -75,8 +77,7 @@ type Client struct { // include ourselves if we end up trying to connect to our own address // through legitimate channels. dopplegangerAddrs map[string]struct{} - badPeerIPs map[string]struct{} - bannedPrefixes map[string]struct{} + badPeerIPs map[netip.Addr]struct{} torrents map[InfoHash]*Torrent pieceRequestOrder map[interface{}]*request_strategy.PieceRequestOrder @@ -103,7 +104,7 @@ func (cl *Client) badPeerIPsLocked() (ips []string) { ips = make([]string, len(cl.badPeerIPs)) i := 0 for k := range cl.badPeerIPs { - ips[i] = k + ips[i] = k.String() i += 1 } return @@ -201,7 +202,7 @@ func (cl *Client) announceKey() int32 { // Initializes a bare minimum Client. *Client and *ClientConfig must not be nil. func (cl *Client) init(cfg *ClientConfig) { cl.config = cfg - cl.dopplegangerAddrs = make(map[string]struct{}) + generics.MakeMap(&cl.dopplegangerAddrs) cl.torrents = make(map[metainfo.Hash]*Torrent) cl.dialRateLimiter = rate.NewLimiter(10, 10) cl.activeAnnounceLimiter.SlotsPerKey = 2 @@ -213,7 +214,6 @@ func (cl *Client) init(cfg *ClientConfig) { MaxConnsPerHost: 10, }, } - cl.bannedPrefixes = make(map[banPrefix]struct{}) } func NewClient(cfg *ClientConfig) (cl *Client, err error) { @@ -1130,12 +1130,6 @@ func (cl *Client) badPeerAddr(addr PeerRemoteAddr) bool { if ipa, ok := tryIpPortFromNetAddr(addr); ok { return cl.badPeerIPPort(ipa.IP, ipa.Port) } - addrStr := addr.String() - for prefix := range cl.bannedPrefixes { - if strings.HasPrefix(addrStr, prefix) { - return true - } - } return false } @@ -1149,7 +1143,11 @@ func (cl *Client) badPeerIPPort(ip net.IP, port int) bool { if _, ok := cl.ipBlockRange(ip); ok { return true } - if _, ok := cl.badPeerIPs[ip.String()]; ok { + ipAddr, ok := netip.AddrFromSlice(ip) + if !ok { + panic(ip) + } + if _, ok := cl.badPeerIPs[ipAddr]; ok { return true } return false @@ -1495,11 +1493,7 @@ func (cl *Client) AddDhtNodes(nodes []string) { } func (cl *Client) banPeerIP(ip net.IP) { - cl.logger.Printf("banning ip %v", ip) - if cl.badPeerIPs == nil { - cl.badPeerIPs = make(map[string]struct{}) - } - cl.badPeerIPs[ip.String()] = struct{}{} + generics.MakeMapIfNilAndSet(&cl.badPeerIPs, netip.MustParseAddr(ip.String()), struct{}{}) } func (cl *Client) newConnection(nc net.Conn, outgoing bool, remoteAddr PeerRemoteAddr, network, connString string) (c *PeerConn) { @@ -1520,8 +1514,12 @@ func (cl *Client) newConnection(nc net.Conn, outgoing bool, remoteAddr PeerRemot connString: connString, conn: nc, } + // TODO: Need to be much more explicit about this, including allowing non-IP bannable addresses. if remoteAddr != nil { - c.banPrefix = option.Some(remoteAddr.String()) + netipAddrPort, err := netip.ParseAddrPort(remoteAddr.String()) + if err == nil { + c.bannableAddr = option.Some(netipAddrPort.Addr()) + } } c.peerImpl = c c.logger = cl.logger.WithDefaultLevel(log.Warning).WithContextValue(c) @@ -1691,7 +1689,3 @@ func (cl *Client) String() string { func (cl *Client) ConnStats() ConnStats { return cl.stats.Copy() } - -func (cl *Client) banPrefix(prefix banPrefix) { - cl.bannedPrefixes[prefix] = struct{}{} -} diff --git a/peerconn.go b/peerconn.go index 566553f9..2dd2ced4 100644 --- a/peerconn.go +++ b/peerconn.go @@ -65,10 +65,10 @@ type Peer struct { peerImpl callbacks *Callbacks - outgoing bool - Network string - RemoteAddr PeerRemoteAddr - banPrefix option.T[string] + outgoing bool + Network string + RemoteAddr PeerRemoteAddr + bannableAddr option.T[bannableAddr] // True if the connection is operating over MSE obfuscation. headerEncrypted bool cryptoMethod mse.CryptoMethod @@ -1390,8 +1390,8 @@ func (c *Peer) receiveChunk(msg *pp.Message) error { req := c.t.requestIndexFromRequest(ppReq) t := c.t - if c.banPrefix.Ok() { - t.smartBanCache.RecordBlock(c.banPrefix.Value(), req, msg.Piece) + if c.bannableAddr.Ok() { + t.smartBanCache.RecordBlock(c.bannableAddr.Value(), req, msg.Piece) } if c.peerChoking { diff --git a/smartban.go b/smartban.go index 9f43104f..74f645e2 100644 --- a/smartban.go +++ b/smartban.go @@ -3,19 +3,21 @@ package torrent import ( "bytes" "crypto/sha1" + "net/netip" + "github.com/anacrolix/torrent/generics" "github.com/anacrolix/torrent/smartban" ) -type banPrefix = string +type bannableAddr = netip.Addr -type smartBanCache = smartban.Cache[banPrefix, RequestIndex, [sha1.Size]byte] +type smartBanCache = smartban.Cache[bannableAddr, RequestIndex, [sha1.Size]byte] type blockCheckingWriter struct { cache *smartBanCache requestIndex RequestIndex // Peers that didn't match blocks written now. - badPeers map[banPrefix]struct{} + badPeers map[bannableAddr]struct{} blockBuffer bytes.Buffer chunkSize int } @@ -23,7 +25,7 @@ type blockCheckingWriter struct { func (me *blockCheckingWriter) checkBlock() { b := me.blockBuffer.Next(me.chunkSize) for _, peer := range me.cache.CheckBlock(me.requestIndex, b) { - me.badPeers[peer] = struct{}{} + generics.MakeMapIfNilAndSet(&me.badPeers, peer, struct{}{}) } me.requestIndex++ } diff --git a/torrent.go b/torrent.go index 02e7c399..a70bbd7d 100644 --- a/torrent.go +++ b/torrent.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "io" + "net/netip" "net/url" "sort" "strings" @@ -27,6 +28,7 @@ import ( "github.com/anacrolix/missinggo/v2/bitmap" "github.com/anacrolix/multiless" "github.com/anacrolix/sync" + "github.com/anacrolix/torrent/option" request_strategy "github.com/anacrolix/torrent/request-strategy" "github.com/davecgh/go-spew/spew" "github.com/pion/datachannel" @@ -952,7 +954,7 @@ func (t *Torrent) smartBanBlockCheckingWriter(piece pieceIndex) *blockCheckingWr func (t *Torrent) hashPiece(piece pieceIndex) ( ret metainfo.Hash, // These are peers that sent us blocks that differ from what we hash here. - differingPeers map[banPrefix]struct{}, + differingPeers map[bannableAddr]struct{}, err error, ) { p := t.piece(piece) @@ -2035,8 +2037,11 @@ func (t *Torrent) pieceHashed(piece pieceIndex, passed bool, hashIoErr error) { if len(bannableTouchers) >= 1 { c := bannableTouchers[0] - t.cl.banPeerIP(c.remoteIp()) - c.drop() + log.Printf("would have banned %v for touching piece %v after failed piece check", c.remoteIp(), piece) + if false { + t.cl.banPeerIP(c.remoteIp()) + c.drop() + } } } t.onIncompletePiece(piece) @@ -2124,15 +2129,38 @@ func (t *Torrent) getPieceToHash() (ret pieceIndex, ok bool) { return } +func (t *Torrent) dropBannedPeers() { + t.iterPeers(func(p *Peer) { + remoteIp := p.remoteIp() + if remoteIp == nil { + if p.bannableAddr.Ok() { + log.Printf("can't get remote ip for peer %v", p) + } + return + } + netipAddr := netip.MustParseAddr(remoteIp.String()) + if option.Some(netipAddr) != p.bannableAddr { + log.Printf( + "peer remote ip does not match its bannable addr [peer=%v, remote ip=%v, bannable addr=%v]", + p, remoteIp, p.bannableAddr) + } + if _, ok := t.cl.badPeerIPs[netipAddr]; ok { + p.drop() + log.Printf("dropped %v for banned remote IP %v", p, netipAddr) + } + }) +} + func (t *Torrent) pieceHasher(index pieceIndex) { p := t.piece(index) sum, failedPeers, copyErr := t.hashPiece(index) correct := sum == *p.hash if correct { for peer := range failedPeers { - log.Printf("would smart ban %q for %v here", peer, p) - t.cl.banPrefix(peer) + t.cl.banPeerIP(peer.AsSlice()) + log.Printf("smart banned %v for piece %v", peer, index) } + t.dropBannedPeers() } switch copyErr { case nil, io.EOF: -- 2.44.0