richness for breach of protocol reasons (#6925)

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
pull/6931/head
Sally MacFarlane 7 months ago committed by GitHub
parent dc60b1d586
commit 2a74ea1f1b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 10
      ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java
  2. 3
      ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/RequestManager.java
  3. 9
      ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/SnapProtocolManager.java
  4. 3
      ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractGetHeadersFromPeerTask.java
  5. 2
      ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractPeerRequestTask.java
  6. 4
      ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/BlockPropagationManager.java
  7. 2
      ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/PipelineChainDownloader.java
  8. 4
      ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTask.java
  9. 2
      ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/NewPooledTransactionHashesMessageProcessor.java
  10. 2
      ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionsMessageProcessor.java
  11. 2
      ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/GetHeadersFromPeerByHashTaskTest.java
  12. 2
      ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/task/GetHeadersFromPeerByNumberTaskTest.java
  13. 3
      ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/PipelineChainDownloaderTest.java
  14. 2
      ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullSyncChainDownloaderForkTest.java
  15. 5
      ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/NetworkRunner.java
  16. 4
      ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/PeerDenylistManager.java
  17. 9
      ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/DeFramer.java
  18. 14
      ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java
  19. 10
      ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/network/PeerDenylistManagerTest.java
  20. 6
      ethereum/p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/netty/DeFramerTest.java

@ -304,7 +304,7 @@ public class EthProtocolManager implements ProtocolManager, MinedBlockObserver {
.addArgument(code)
.addArgument(ethPeer::toString)
.log();
ethPeer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
ethPeer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL_RECEIVED_OTHER_MESSAGE_BEFORE_STATUS);
return;
}
@ -324,9 +324,10 @@ public class EthProtocolManager implements ProtocolManager, MinedBlockObserver {
if (!ethPeer.validateReceivedMessage(ethMessage, getSupportedProtocol())) {
LOG.debug(
"Unsolicited message received (BREACH_OF_PROTOCOL), disconnecting from EthPeer: {}",
"Unsolicited message received {} (BREACH_OF_PROTOCOL), disconnecting from EthPeer: {}",
ethMessage.getData().getCode(),
ethPeer);
ethPeer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
ethPeer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL_UNSOLICITED_MESSAGE_RECEIVED);
return;
}
@ -354,7 +355,8 @@ public class EthProtocolManager implements ProtocolManager, MinedBlockObserver {
.addArgument(e::toString)
.log();
ethPeer.disconnect(DisconnectMessage.DisconnectReason.BREACH_OF_PROTOCOL);
ethPeer.disconnect(
DisconnectMessage.DisconnectReason.BREACH_OF_PROTOCOL_MALFORMED_MESSAGE_RECEIVED);
}
maybeResponseData.ifPresent(
responseData -> {

@ -95,7 +95,8 @@ public class RequestManager {
peer,
e);
peer.disconnect(DisconnectMessage.DisconnectReason.BREACH_OF_PROTOCOL);
peer.disconnect(
DisconnectMessage.DisconnectReason.BREACH_OF_PROTOCOL_MALFORMED_MESSAGE_RECEIVED);
}
if (count == 0) {

@ -103,8 +103,11 @@ public class SnapProtocolManager implements ProtocolManager {
}
final EthMessage ethMessage = new EthMessage(ethPeer, messageData);
if (!ethPeer.validateReceivedMessage(ethMessage, getSupportedProtocol())) {
LOG.debug("Unsolicited message received from, disconnecting: {}", ethPeer);
ethPeer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
LOG.debug(
"Unsolicited message {} received from, disconnecting: {}",
ethMessage.getData().getCode(),
ethPeer);
ethPeer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL_UNSOLICITED_MESSAGE_RECEIVED);
return;
}
@ -123,7 +126,7 @@ public class SnapProtocolManager implements ProtocolManager {
} catch (final RLPException e) {
LOG.debug(
"Received malformed message {} , disconnecting: {}", messageData.getData(), ethPeer, e);
ethPeer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
ethPeer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL_MALFORMED_MESSAGE_RECEIVED);
}
maybeResponseData.ifPresent(
responseData -> {

@ -111,7 +111,8 @@ public abstract class AbstractGetHeadersFromPeerTask
LOG.debug(
"Sequential headers must form a chain through hashes (BREACH_OF_PROTOCOL), disconnecting peer: {}",
peer.getLoggableId());
peer.disconnect(DisconnectMessage.DisconnectReason.BREACH_OF_PROTOCOL);
peer.disconnect(
DisconnectMessage.DisconnectReason.BREACH_OF_PROTOCOL_NON_SEQUENTIAL_HEADERS);
return Optional.empty();
}
}

@ -113,7 +113,7 @@ public abstract class AbstractPeerRequestTask<R> extends AbstractPeerTask<R> {
peer.getLoggableId(),
e);
LOG.trace("Peer {} Malformed message data: {}", peer, message.getData());
peer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
peer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL_MALFORMED_MESSAGE_RECEIVED);
promise.completeExceptionally(new PeerBreachedProtocolException());
}
}

@ -339,7 +339,7 @@ public class BlockPropagationManager implements UnverifiedForkchoiceListener {
"Malformed NEW_BLOCK message received from peer (BREACH_OF_PROTOCOL), disconnecting: {}",
message.getPeer(),
e);
message.getPeer().disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
message.getPeer().disconnect(DisconnectReason.BREACH_OF_PROTOCOL_MALFORMED_MESSAGE_RECEIVED);
}
}
@ -402,7 +402,7 @@ public class BlockPropagationManager implements UnverifiedForkchoiceListener {
"Malformed NEW_BLOCK_HASHES message received from peer (BREACH_OF_PROTOCOL), disconnecting: {}",
message.getPeer(),
e);
message.getPeer().disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
message.getPeer().disconnect(DisconnectReason.BREACH_OF_PROTOCOL_MALFORMED_MESSAGE_RECEIVED);
}
}

@ -120,7 +120,7 @@ public class PipelineChainDownloader implements ChainDownloader {
LOG.warn(
"Invalid block detected (BREACH_OF_PROTOCOL). Disconnecting from sync target. {}",
ExceptionUtils.rootCause(error).getMessage());
syncState.disconnectSyncTarget(DisconnectReason.BREACH_OF_PROTOCOL);
syncState.disconnectSyncTarget(DisconnectReason.BREACH_OF_PROTOCOL_INVALID_BLOCK);
}
if (!cancelled.get() && syncTargetManager.shouldContinueDownloading()) {

@ -217,7 +217,9 @@ public class DownloadHeaderSequenceTask extends AbstractRetryingPeerTask<List<Bl
LOG.debug(
"Received invalid headers from peer (BREACH_OF_PROTOCOL), disconnecting from: {}",
headersResult.getPeer());
headersResult.getPeer().disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
headersResult
.getPeer()
.disconnect(DisconnectReason.BREACH_OF_PROTOCOL_INVALID_HEADERS);
final InvalidBlockException exception;
if (invalidHeader == null) {
final String msg =

@ -129,7 +129,7 @@ public class NewPooledTransactionHashesMessageProcessor {
peer,
ex);
LOG.trace("Message data: {}", transactionsMessage.getData());
peer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
peer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL_MALFORMED_MESSAGE_RECEIVED);
}
}
}

@ -92,7 +92,7 @@ class TransactionsMessageProcessor {
"Malformed transaction message received (BREACH_OF_PROTOCOL), disconnecting: {}",
peer,
ex);
peer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
peer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL_MALFORMED_MESSAGE_RECEIVED);
}
}
}

@ -198,6 +198,6 @@ public class GetHeadersFromPeerByHashTaskTest extends PeerMessageTaskTest<List<B
assertThat(optionalBlockHeaders).isEmpty();
assertThat(peer.isDisconnected()).isTrue();
assertThat(((MockPeerConnection) peer.getConnection()).getDisconnectReason().get())
.isEqualTo(DisconnectMessage.DisconnectReason.BREACH_OF_PROTOCOL);
.isEqualTo(DisconnectMessage.DisconnectReason.BREACH_OF_PROTOCOL_NON_SEQUENTIAL_HEADERS);
}
}

@ -169,6 +169,6 @@ public class GetHeadersFromPeerByNumberTaskTest extends PeerMessageTaskTest<List
assertThat(optionalBlockHeaders).isEmpty();
assertThat(peer.isDisconnected()).isTrue();
assertThat(((MockPeerConnection) peer.getConnection()).getDisconnectReason().get())
.isEqualTo(DisconnectMessage.DisconnectReason.BREACH_OF_PROTOCOL);
.isEqualTo(DisconnectMessage.DisconnectReason.BREACH_OF_PROTOCOL_NON_SEQUENTIAL_HEADERS);
}
}

@ -320,7 +320,8 @@ public class PipelineChainDownloaderTest {
}
selectTargetFuture.completeExceptionally(InvalidBlockException.create("Failed"));
verify(syncState, times(1)).disconnectSyncTarget(DisconnectReason.BREACH_OF_PROTOCOL);
verify(syncState, times(1))
.disconnectSyncTarget(DisconnectReason.BREACH_OF_PROTOCOL_INVALID_BLOCK);
}
private CompletableFuture<Void> expectPipelineStarted(final SyncTarget syncTarget) {

@ -127,7 +127,7 @@ public class FullSyncChainDownloaderForkTest {
// We should have disconnected from our peer on the invalid chain
assertThat(peer.getEthPeer().isDisconnected()).isTrue();
assertThat(peer.getPeerConnection().getDisconnectReason())
.contains(DisconnectReason.BREACH_OF_PROTOCOL);
.contains(DisconnectReason.BREACH_OF_PROTOCOL_INVALID_BLOCK);
assertThat(syncState.syncTarget()).isEmpty();
}
}

@ -130,7 +130,10 @@ public class NetworkRunner implements AutoCloseable {
cap.getVersion(),
code,
message.getConnection().getPeerInfo().getNodeId());
message.getConnection().disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
message
.getConnection()
.disconnect(
DisconnectReason.BREACH_OF_PROTOCOL_INVALID_MESSAGE_CODE_FOR_PROTOCOL);
return;
}
inboundMessageCounter

@ -50,7 +50,9 @@ public class PeerDenylistManager implements DisconnectCallback {
final PeerConnection connection,
final DisconnectReason reason,
final boolean initiatedByPeer) {
if (shouldBlock(reason, initiatedByPeer)) {
// we have a number of reasons that use the same code, but with different message strings
// so here we use the code of the reason param to ensure we get the no-message version
if (shouldBlock(DisconnectReason.forCode(reason.getValue().get(0)), initiatedByPeer)) {
if (maintainedPeers.contains(connection.getPeer())) {
LOG.debug(
"Skip adding maintained peer {} to peer denylist for reason {}",

@ -223,7 +223,8 @@ final class DeFramer extends ByteToMessageDecoder {
new OutboundMessage(
null,
DisconnectMessage.create(
DisconnectMessage.DisconnectReason.BREACH_OF_PROTOCOL)))
DisconnectMessage.DisconnectReason
.BREACH_OF_PROTOCOL_MESSAGE_RECEIVED_BEFORE_HELLO_EXCHANGE)))
.addListener((f) -> ctx.close());
connectFuture.completeExceptionally(
new BreachOfProtocolException("Message received before HELLO's exchanged"));
@ -260,7 +261,11 @@ final class DeFramer extends ByteToMessageDecoder {
|| cause instanceof IllegalArgumentException) {
LOG.debug("Invalid incoming message (BREACH_OF_PROTOCOL)", throwable);
if (connectFuture.isDone() && !connectFuture.isCompletedExceptionally()) {
connectFuture.get().disconnect(DisconnectMessage.DisconnectReason.BREACH_OF_PROTOCOL);
connectFuture
.get()
.disconnect(
DisconnectMessage.DisconnectReason
.BREACH_OF_PROTOCOL_INVALID_MESSAGE_RECEIVED_CAUGHT_EXCEPTION);
return;
}
} else if (cause instanceof IOException) {

@ -108,7 +108,21 @@ public final class DisconnectMessage extends AbstractMessageData {
UNKNOWN(null),
REQUESTED((byte) 0x00),
TCP_SUBSYSTEM_ERROR((byte) 0x01),
BREACH_OF_PROTOCOL((byte) 0x02),
BREACH_OF_PROTOCOL_RECEIVED_OTHER_MESSAGE_BEFORE_STATUS(
(byte) 0x02, "Message other than status received first"),
BREACH_OF_PROTOCOL_UNSOLICITED_MESSAGE_RECEIVED((byte) 0x02, "Unsolicited message received"),
BREACH_OF_PROTOCOL_MALFORMED_MESSAGE_RECEIVED((byte) 0x02, "Malformed message received"),
BREACH_OF_PROTOCOL_NON_SEQUENTIAL_HEADERS((byte) 0x02, "Non-sequential headers received"),
BREACH_OF_PROTOCOL_INVALID_BLOCK((byte) 0x02, "Invalid block detected"),
BREACH_OF_PROTOCOL_INVALID_HEADERS((byte) 0x02, "Invalid headers detected"),
BREACH_OF_PROTOCOL_INVALID_MESSAGE_CODE_FOR_PROTOCOL(
(byte) 0x02, "Invalid message code for specified protocol"),
BREACH_OF_PROTOCOL_MESSAGE_RECEIVED_BEFORE_HELLO_EXCHANGE(
(byte) 0x02, "A message was received before hello's exchanged"),
BREACH_OF_PROTOCOL_INVALID_MESSAGE_RECEIVED_CAUGHT_EXCEPTION(
(byte) 0x02, "An exception was caught decoding message"),
USELESS_PEER((byte) 0x03),
USELESS_PEER_USELESS_RESPONSES((byte) 0x03, "Useless responses: exceeded threshold"),
USELESS_PEER_TRAILING_PEER((byte) 0x03, "Trailing peer requirement"),

@ -62,6 +62,16 @@ public class PeerDenylistManagerTest {
checkPermissions(denylist, peer.getPeer(), false);
}
@Test
public void denylistPeerForBadBehaviorWithDifferentMessage() {
final PeerConnection peer = generatePeerConnection();
checkPermissions(denylist, peer.getPeer(), true);
peerDenylistManager.onDisconnect(
peer, DisconnectReason.BREACH_OF_PROTOCOL_INVALID_MESSAGE_CODE_FOR_PROTOCOL, false);
checkPermissions(denylist, peer.getPeer(), false);
}
@Test
public void doesNotDenylistPeerForOurBadBehavior() {
final PeerConnection peer = generatePeerConnection();

@ -138,7 +138,8 @@ public class DeFramerTest {
deFramer.exceptionCaught(ctx, new DecoderException(new FramingException("Test")));
verify(peerConnection).disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
verify(peerConnection)
.disconnect(DisconnectReason.BREACH_OF_PROTOCOL_INVALID_MESSAGE_RECEIVED_CAUGHT_EXCEPTION);
}
@Test
@ -148,7 +149,8 @@ public class DeFramerTest {
deFramer.exceptionCaught(ctx, new DecoderException(new RLPException("Test")));
verify(peerConnection).disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
verify(peerConnection)
.disconnect(DisconnectReason.BREACH_OF_PROTOCOL_INVALID_MESSAGE_RECEIVED_CAUGHT_EXCEPTION);
}
@Test

Loading…
Cancel
Save