Merge pull request #358 from ScorexFoundation/banning-peers

Banning peers for sending adversarially constructed messages

src/main/scala/scorex/core/network/MaliciousBehaviorException.scala

@@ -0,0 +1,8 @@
+package scorex.core.network
+
+/**
+  * Custom exception to distinguish malicious behaviour of external peers from non-adversarial network issues
+  *
+  * @param message - exception message
+  */
+case class MaliciousBehaviorException(message: String) extends Exception(message)

src/main/scala/scorex/core/network/PeerConnectionHandler.scala

@@ -99,8 +99,9 @@ class PeerConnectionHandler(val settings: NetworkSettings,
 
         case Failure(t) =>
           log.info(s"Error during parsing a handshake", t)
-          selfPeer.foreach(c => peerManagerRef ! PenalizePeer(c.connectionId.remoteAddress, PenaltyType.PermanentPenalty))
-          self ! CloseConnection
+          //ban the peer for the wrong handshake message
+          //peer will be added to the blacklist and the network controller will send CloseConnection
+          selfPeer.foreach(c => networkControllerRef ! PenalizePeer(c.connectionId.remoteAddress, PenaltyType.PermanentPenalty))
       }
   }
 
@@ -192,7 +193,17 @@ class PeerConnectionHandler(val settings: NetworkSettings,
             chunksBuffer = chunksBuffer.drop(message.messageLength)
             process()
           case Success(None) =>
-          case Failure(e) => log.info(s"Corrupted data from ${connectionId.toString}: ${e.getMessage}")
+          case Failure(e) =>
+            e match {
+              //peer is doing bad things, ban it
+              case MaliciousBehaviorException(msg) =>
+                log.warn(s"Banning peer for malicious behaviour($msg): ${connectionId.toString}")
+                //peer will be added to the blacklist and the network controller will send CloseConnection
+                networkControllerRef ! PenalizePeer(connectionId.remoteAddress, PenaltyType.PermanentPenalty)
+              //non-malicious corruptions
+              case _ =>
+                log.info(s"Corrupted data from ${connectionId.toString}: ${e.getMessage}")
+            }
         }
       }
 

src/main/scala/scorex/core/network/message/Message.scala

@@ -4,7 +4,7 @@ import java.nio.ByteOrder
 
 import akka.actor.DeadLetterSuppression
 import akka.util.ByteString
-import scorex.core.network.ConnectedPeer
+import scorex.core.network.{ConnectedPeer, MaliciousBehaviorException}
 import scorex.crypto.hash.Blake2b256
 
 import scala.util.{Success, Try}
@@ -67,8 +67,16 @@ class MessageSerializer(specs: Seq[MessageSpec[_]], magicBytes: Array[Byte]) {
       val magic = it.getBytes(MagicLength)
       val msgCode = it.getByte
       val length = it.getInt
-      require(java.util.Arrays.equals(magic, magicBytes), "Wrong magic bytes" + magic.mkString)
-      require(length >= 0, "Data length is negative!")
+
+      //peer is from another network
+      if (!java.util.Arrays.equals(magic, magicBytes)) {
+        throw MaliciousBehaviorException(s"Wrong magic bytes, expected ${magicBytes.mkString}, got ${magic.mkString}")
+      }
+
+      //peer is trying to cause buffer overflow or breaking the parsing
+      if (length < 0) {
+        throw MaliciousBehaviorException("Data length is negative!")
+      }
 
       val spec = specsMap.getOrElse(msgCode, throw new Error(s"No message handler found for $msgCode"))
 
@@ -79,7 +87,11 @@ class MessageSerializer(specs: Seq[MessageSpec[_]], magicBytes: Array[Byte]) {
           val checksum = it.getBytes(ChecksumLength)
           val data = it.getBytes(length)
           val digest = Blake2b256.hash(data).take(ChecksumLength)
-          if (!java.util.Arrays.equals(checksum, digest)) throw new Error(s"Invalid data checksum length = $length")
+
+          //peer reported incorrect checksum
+          if (!java.util.Arrays.equals(checksum, digest)) {
+            throw MaliciousBehaviorException(s"Wrong checksum, expected ${checksum.mkString}, got ${checksum.mkString}")
+          }
           data
         } else {
           Array.empty[Byte]

src/main/scala/scorex/core/network/peer/PeerManager.scala

@@ -30,7 +30,10 @@ class PeerManager(settings: ScorexSettings, scorexContext: ScorexContext) extend
     }
   }
 
-  override def receive: Receive = peersManagement orElse apiInterface
+  override def receive: Receive = peersManagement orElse apiInterface orElse {
+    case a: Any =>
+      log.error(s"Wrong input for peer manager: $a")
+  }
 
   private def peersManagement: Receive = {
 
@@ -152,7 +155,8 @@ object PeerManager {
                           blacklistedPeers: Seq[InetAddress],
                           sc: ScorexContext): Option[PeerInfo] = {
         val candidates = knownPeers.values.filterNot { p =>
-          excludedPeers.exists(_.peerSpec.address == p.peerSpec.address)
+          excludedPeers.exists(_.peerSpec.address == p.peerSpec.address) &&
+            blacklistedPeers.exists(addr => p.peerSpec.address.map(_.getAddress).contains(addr))
         }.toSeq
         if (candidates.nonEmpty) Some(candidates(Random.nextInt(candidates.size)))
         else None