Skip to content

[ETCM-844] Validate peer fork id during eth/64 handshake #1064

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 15, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 4 additions & 1 deletion src/main/scala/io/iohk/ethereum/forkid/ForkIdValidator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@ case object Connect extends ForkIdValidationResult
case object ErrRemoteStale extends ForkIdValidationResult
case object ErrLocalIncompatibleOrStale extends ForkIdValidationResult

import cats.effect._

object ForkIdValidator {

implicit val unsafeLogger: SelfAwareStructuredLogger[Task] = Slf4jLogger.getLogger[Task]
implicit val taskLogger: SelfAwareStructuredLogger[Task] = Slf4jLogger.getLogger[Task]
implicit val syncIoLogger: SelfAwareStructuredLogger[SyncIO] = Slf4jLogger.getLogger[SyncIO]

val maxUInt64: BigInt = (BigInt(0x7fffffffffffffffL) << 1) + 1 // scalastyle:ignore magic.number

Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
package io.iohk.ethereum.network.handshaker

import cats.effect.SyncIO

import io.iohk.ethereum.forkid.Connect
import io.iohk.ethereum.forkid.ForkId
import io.iohk.ethereum.forkid.ForkIdValidator
import io.iohk.ethereum.network.EtcPeerManagerActor.PeerInfo
import io.iohk.ethereum.network.EtcPeerManagerActor.RemoteStatus
import io.iohk.ethereum.network.p2p.Message
import io.iohk.ethereum.network.p2p.MessageSerializable
import io.iohk.ethereum.network.p2p.messages.Capability
import io.iohk.ethereum.network.p2p.messages.ETH64
import io.iohk.ethereum.network.p2p.messages.WireProtocol.Disconnect

case class EthNodeStatus64ExchangeState(
handshakerConfiguration: EtcHandshakerConfiguration
Expand All @@ -15,8 +20,17 @@ case class EthNodeStatus64ExchangeState(
import handshakerConfiguration._

def applyResponseMessage: PartialFunction[Message, HandshakerState[PeerInfo]] = { case status: ETH64.Status =>
// TODO: validate fork id of the remote peer
applyRemoteStatusMessage(RemoteStatus(status))
import ForkIdValidator.syncIoLogger
(for {
validationResult <-
ForkIdValidator.validatePeer[SyncIO](blockchainReader.genesisHeader.hash, blockchainConfig)(
blockchainReader.getBestBlockNumber(),
status.forkId
)
} yield validationResult match {
case Connect => applyRemoteStatusMessage(RemoteStatus(status))
case _ => DisconnectedState[PeerInfo](Disconnect.Reasons.UselessPeer)
}).unsafeRunSync()
}

override protected def createStatusMsg(): MessageSerializable = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,8 @@ class EtcHandshakerSpec extends AnyFlatSpec with Matchers {
}
}

it should "send status with fork id when peer supports ETH64" in new LocalPeerETH64Setup with RemotePeerETH64Setup {
it should "connect correctly after validating fork id when peer supports ETH64" in new LocalPeerETH64Setup
with RemotePeerETH64Setup {

val newChainWeight = ChainWeight.zero.increase(genesisBlock.header).increase(firstBlock.header)

Expand Down Expand Up @@ -223,6 +224,45 @@ class EtcHandshakerSpec extends AnyFlatSpec with Matchers {
}
}

it should "disconnect from a useless peer after validating fork id when peer supports ETH64" in new LocalPeerETH64Setup
with RemotePeerETH64Setup {

val newChainWeight = ChainWeight.zero.increase(genesisBlock.header).increase(firstBlock.header)

blockchainWriter.save(firstBlock, Nil, newChainWeight, saveAsBestBlock = true)

val newLocalStatusMsg =
localStatusMsg
.copy(
bestHash = firstBlock.header.hash,
totalDifficulty = newChainWeight.totalDifficulty,
forkId = ForkId(0xfc64ec04L, Some(1150000))
)

initHandshakerWithoutResolver.nextMessage.map(_.messageToSend) shouldBe Right(localHello: HelloEnc)

val newRemoteStatusMsg =
remoteStatusMsg
.copy(
forkId = ForkId(1, None) // ForkId that is incompatible with our chain
)

val handshakerAfterHelloOpt = initHandshakerWithoutResolver.applyMessage(remoteHello)
assert(handshakerAfterHelloOpt.isDefined)

handshakerAfterHelloOpt.get.nextMessage.map(_.messageToSend.underlyingMsg) shouldBe Right(newLocalStatusMsg)

val handshakerAfterStatusOpt = handshakerAfterHelloOpt.get.applyMessage(newRemoteStatusMsg)
assert(handshakerAfterStatusOpt.isDefined)

handshakerAfterStatusOpt.get.nextMessage match {
case Left(HandshakeFailure(Disconnect.Reasons.UselessPeer)) => succeed
case other =>
fail(s"Invalid handshaker state: $other")
}

}

it should "fail if a timeout happened during hello exchange" in new TestSetup {
val handshakerAfterTimeout = initHandshakerWithoutResolver.processTimeout
handshakerAfterTimeout.nextMessage.map(_.messageToSend) shouldBe Left(
Expand Down Expand Up @@ -447,7 +487,7 @@ class EtcHandshakerSpec extends AnyFlatSpec with Matchers {
totalDifficulty = 0,
bestHash = genesisBlock.header.hash,
genesisHash = genesisBlock.header.hash,
forkId = ForkId(2L, Some(3L))
forkId = ForkId(0xfc64ec04L, Some(1150000))
)

val remoteStatus: RemoteStatus = RemoteStatus(remoteStatusMsg)
Expand Down