From 8fa64813d122d833f029d30f2be7e84dc703cbf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Thu, 28 May 2015 04:04:53 +0300 Subject: [PATCH 1/5] p2p: temporarily ban nodes deemed useless --- p2p/server.go | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/p2p/server.go b/p2p/server.go index 5890418108..53bab1dcdd 100644 --- a/p2p/server.go +++ b/p2p/server.go @@ -19,6 +19,11 @@ const ( refreshPeersInterval = 30 * time.Second staticPeerCheckInterval = 15 * time.Second + banStartingTimeout = 30 * time.Second // Amount of time a first offender is banned + banMaximumTimeout = 30 * time.Minute // Maximum time for banning somebody + banAggrevationMultiplier = 2 // Multiplier for repeat offenders + banAbsolutionMultiplier = 10 // Multiplier for absolving a node + // Maximum number of concurrently handshaking inbound connections. maxAcceptConns = 50 @@ -122,6 +127,7 @@ type Server struct { quit chan struct{} addstatic chan *discover.Node + addbanned chan *Peer posthandshake chan *conn addpeer chan *conn delpeer chan *Peer @@ -307,6 +313,7 @@ func (srv *Server) Start() (err error) { srv.delpeer = make(chan *Peer) srv.posthandshake = make(chan *conn) srv.addstatic = make(chan *discover.Node) + srv.addbanned = make(chan *Peer) srv.peerOp = make(chan peerOpFunc) srv.peerOpDone = make(chan struct{}) @@ -380,6 +387,9 @@ func (srv *Server) run(dialstate dialer) { peers = make(map[discover.NodeID]*Peer) trusted = make(map[discover.NodeID]bool, len(srv.TrustedNodes)) + banned = make(map[discover.NodeID]time.Time) + penalties = make(map[discover.NodeID]time.Duration) + tasks []task pendingTasks []task taskdone = make(chan task, maxActiveDialTasks) @@ -436,6 +446,18 @@ running: // it will keep the node connected. glog.V(logger.Detail).Infoln("<-addstatic:", n) dialstate.addStatic(n) + case p := <-srv.addbanned: + // Add a node to the banned list, doubling any previous bans + glog.V(logger.Detail).Infoln("<-addbanned:", p) + penalty := banStartingTimeout + if prev, ok := penalties[p.ID()]; ok { + penalty = prev * banAggrevationMultiplier + if penalty > banMaximumTimeout { + penalty = banMaximumTimeout + } + } + banned[p.ID()] = time.Now().Add(penalty) + glog.V(logger.Debug).Infoln("Banned", p.ID(), "until", banned[p.ID()]) case op := <-srv.peerOp: // This channel is used by Peers and PeerCount. op(peers) @@ -450,6 +472,11 @@ running: case c := <-srv.posthandshake: // A connection has passed the encryption handshake so // the remote identity is known (but hasn't been verified yet). + if exp, ok := banned[c.id]; ok && time.Now().Before(exp) { + glog.V(logger.Detail).Infoln("<-banned:", c) + c.cont <- errors.New("temporarily banned") + continue + } if trusted[c.id] { // Ensure that the trusted flag is set before checking against MaxPeers. c.flags |= trustedConn @@ -478,6 +505,15 @@ running: // A peer disconnected. glog.V(logger.Detail).Infoln("<-delpeer:", p) delete(peers, p.ID()) + + // Lift any previous bans for good behavior + if exp, ok := banned[p.ID()]; ok { + lift := exp.Add(penalties[p.ID()] * banAbsolutionMultiplier) + if time.Now().After(lift) { + delete(banned, p.ID()) + delete(penalties, p.ID()) + } + } } } @@ -494,6 +530,13 @@ running: // is closed. glog.V(logger.Detail).Infof("ignoring %d pending tasks at spindown", len(tasks)) for len(peers) > 0 { + // Drop any in flight ban requests + select { + case p := <-srv.addbanned: + glog.V(logger.Detail).Infoln("<-addbanned (spindown):", p) + default: + } + // Drop the peer itself p := <-srv.delpeer glog.V(logger.Detail).Infoln("<-delpeer (spindown):", p) delete(peers, p.ID()) @@ -640,6 +683,10 @@ func (srv *Server) runPeer(p *Peer) { srv.newPeerHook(p) } discreason := p.run() + if discreason == DiscUselessPeer { + // Temporarily disallow the peer to reconnect + srv.addbanned <- p + } // Note: run waits for existing peers to be sent on srv.delpeer // before returning, so this send should not select on srv.quit. srv.delpeer <- p From fea3d9c60e10a48d7dbd7c17e5037e5074777561 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Thu, 28 May 2015 04:26:35 +0300 Subject: [PATCH 2/5] p2p: don't silence oridinal disconnect reason --- p2p/peer.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/p2p/peer.go b/p2p/peer.go index cbe5ccc84a..b18ac32a82 100644 --- a/p2p/peer.go +++ b/p2p/peer.go @@ -123,10 +123,8 @@ func (p *Peer) run() DiscReason { p.startProtocols() // Wait for an error or disconnect. - var ( - reason DiscReason - requested bool - ) + var reason DiscReason + select { case err := <-readErr: if r, ok := err.(DiscReason); ok { @@ -140,15 +138,11 @@ func (p *Peer) run() DiscReason { case err := <-p.protoErr: reason = discReasonForError(err) case reason = <-p.disc: - requested = true } close(p.closed) p.rw.close(reason) p.wg.Wait() - if requested { - reason = DiscRequested - } glog.V(logger.Debug).Infof("%v: Disconnected: %v\n", p, reason) return reason } From e01d61bacb2439fe991e177d5c6ae374b19fcdfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Thu, 28 May 2015 04:45:44 +0300 Subject: [PATCH 3/5] p2p: fix banning error messages a bit --- p2p/server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/p2p/server.go b/p2p/server.go index 53bab1dcdd..9b0883136c 100644 --- a/p2p/server.go +++ b/p2p/server.go @@ -457,7 +457,7 @@ running: } } banned[p.ID()] = time.Now().Add(penalty) - glog.V(logger.Debug).Infoln("Banned", p.ID(), "until", banned[p.ID()]) + glog.V(logger.Debug).Infof("Banned %s until %v", p.ID().String()[:16], banned[p.ID()]) case op := <-srv.peerOp: // This channel is used by Peers and PeerCount. op(peers) @@ -474,7 +474,7 @@ running: // the remote identity is known (but hasn't been verified yet). if exp, ok := banned[c.id]; ok && time.Now().Before(exp) { glog.V(logger.Detail).Infoln("<-banned:", c) - c.cont <- errors.New("temporarily banned") + c.cont <- fmt.Errorf("banned until %v", exp) continue } if trusted[c.id] { From 1b25ffaca5da01d6201c59181db18c6cd435d374 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Thu, 28 May 2015 05:11:19 +0300 Subject: [PATCH 4/5] p2p: raise the ban limits to harsher --- p2p/server.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/p2p/server.go b/p2p/server.go index 9b0883136c..687d1bed29 100644 --- a/p2p/server.go +++ b/p2p/server.go @@ -19,10 +19,10 @@ const ( refreshPeersInterval = 30 * time.Second staticPeerCheckInterval = 15 * time.Second - banStartingTimeout = 30 * time.Second // Amount of time a first offender is banned - banMaximumTimeout = 30 * time.Minute // Maximum time for banning somebody - banAggrevationMultiplier = 2 // Multiplier for repeat offenders - banAbsolutionMultiplier = 10 // Multiplier for absolving a node + banStartingTimeout = time.Minute // Amount of time a first offender is banned + banMaximumTimeout = time.Hour // Maximum time for banning somebody + banAggrevationMultiplier = 4 // Multiplier for repeat offenders + banAbsolutionMultiplier = 24 // Multiplier for absolving a node // Maximum number of concurrently handshaking inbound connections. maxAcceptConns = 50 From 065b9bd8c802205fb9efda8a613bef55eb6c05f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Thu, 28 May 2015 05:19:58 +0300 Subject: [PATCH 5/5] p2p: actually save the updated penalty too --- p2p/server.go | 1 + 1 file changed, 1 insertion(+) diff --git a/p2p/server.go b/p2p/server.go index 687d1bed29..4cde009f1d 100644 --- a/p2p/server.go +++ b/p2p/server.go @@ -456,6 +456,7 @@ running: penalty = banMaximumTimeout } } + penalties[p.ID()] = penalty banned[p.ID()] = time.Now().Add(penalty) glog.V(logger.Debug).Infof("Banned %s until %v", p.ID().String()[:16], banned[p.ID()]) case op := <-srv.peerOp: