DHT can ban peer forever

[GoogleCodeExporter]

There is a same code snippet in 
 - dos_blocker::incoming(...) in src/kademlia/dos_blocker.cpp in trunk;
 - dht_tracker::incoming_packet(...) in src/kademlia/dht_tracker.cpp in 1.x;
 - dht_tracker::on_receive(...) in src/kademlia/dht_tracker.cpp in 0.16.x.

== Snippet start ==
node_ban_entry* match = 0;
node_ban_entry* min = m_ban_nodes;
for (node_ban_entry* i = m_ban_nodes; i < m_ban_nodes + num_ban_nodes; ++i)
{
  if (i->src == addr) {
    match = i;
    break;
  }
  if (i->count < min->count) min = i;
}

if (match) {
  ++match->count;
  if (match->count >= 20) {
    if (now < match->limit) {

#ifdef TORRENT_DHT_VERBOSE_LOGGING
      if (match->count == 20) {
        TORRENT_LOG(dht_tracker) << " BANNING PEER [ ip: "
        << addr << " time: " << total_milliseconds((now - match->limit) + seconds(5)) / 1000.f
        << " count: " << match->count << " ]";
      }
#endif

      // we've received 20 messages in less than 5 seconds from
      // this node. Ignore it until it's silent for 5 minutes
      match->limit = now + minutes(5);
      return false;
    }
    // we got 50 messages from this peer, but it was in
    // more than 5 seconds. Reset the counter and the timer
    match->count = 0;
    match->limit = now + seconds(5);
  }
}
else {
  min->count = 1;
  min->limit = now + seconds(5);
  min->src = addr;
}
return true;
== Snippet end ==

Assume we have only few peers that managed to send more than 20 messages in 5 
seconds. After that each next message from those peers will prolongue
theirs ban time for next 5 minutes (even if message rate becomes normal).
The only way to get out of ban list for peer is not to send any message at all 
for whole ban period (or get swapped out by some other peer, there is 20
items in ban list).

Another problem with this snippet is that all banning parameters (message 
count, time slice for counting, ban time) are hardcoded and aren't configurable.
There is no way to get information that peer get banned (no alerts sent), only 
debug build will produce message to log file.

To be honest I'm not sure whether it makes sense to fix this code or to remove 
it completely. Should application care about DOS-attack or it is up to 
operation system/ISP/network routers?

Easiest fix looks to be either remove this coder or allow to disable it during 
library build.

Original issue reported on code.google.com by vost...@gmail.com on 26 Aug 2014 at 8:15

[GoogleCodeExporter]

I think it is useful to at least have some basic flood protection in the DHT, 
given how vulnerable it is otherwise. Granted, you don't have to be all that 
sophisticated to spoof your source IP, but at least a little bit.

How about lifting the ban after 5 minutes unconditionally? if the message rate 
is still excessive, it would just re-ban the node. I agree that the limits 
should be configurable though.

I don't think the intention is to prolong the ban timer when the message rate 
is low, this might be a bug. I will review this code.

Original comment by arvid.no...@gmail.com on 26 Aug 2014 at 6:06

[GoogleCodeExporter]

I've altered the logic to just ban for 5 minutes and then unban it again. I 
also made it tolerate larger bursts (50 packets in 10 seconds instead of 20 
packets in 5 seconds).

making it configurable in 1.0.x is a bit tricky because the settings structs 
would have to change, breaking the ABI. I will make a note in trunk though, to 
make it configurable. trunk also makes the settings more extensible without 
altering the ABI.

Original comment by arvid.no...@gmail.com on 28 Aug 2014 at 12:30

[GoogleCodeExporter]

Original comment by arvid.no...@gmail.com on 28 Aug 2014 at 12:31

• Changed state: Fixed