Skip to content

Commit

Permalink
[Issue 3867] Make eth subprotocol message size limit configurable (hy…
Browse files Browse the repository at this point in the history 
…perledger#4002)

Signed-off-by: Meredith Baxter <meredith.baxter@palm.io>
  • Loading branch information
@mbaxter
mbaxter authored Jun 23, 2022
1 parent 87eb4d5 commit 645fdd0
Show file tree
Hide file tree
Showing 8 changed files with 169 additions and 34 deletions.
14 changes: 14 additions & 0 deletions besu/src/main/java/org/hyperledger/besu/cli/options/unstable/EthProtocolOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import picocli.CommandLine;

public class EthProtocolOptions implements CLIOptions<EthProtocolConfiguration> {
private static final String MAX_MESSAGE_SIZE_FLAG = "--Xeth-max-message-size";
private static final String MAX_GET_HEADERS_FLAG = "--Xewp-max-get-headers";
private static final String MAX_GET_BODIES_FLAG = "--Xewp-max-get-bodies";
private static final String MAX_GET_RECEIPTS_FLAG = "--Xewp-max-get-receipts";
Expand All @@ -33,6 +34,15 @@ public class EthProtocolOptions implements CLIOptions<EthProtocolConfiguration>
private static final String LEGACY_ETH_64_FORK_ID_ENABLED =
"--compatibility-eth64-forkid-enabled";

@CommandLine.Option(
hidden = true,
names = {MAX_MESSAGE_SIZE_FLAG},
paramLabel = "<INTEGER>",
description =
"Maximum message size (in bytes) for Ethereum Wire Protocol messages. (default: ${DEFAULT-VALUE})")
private PositiveNumber maxMessageSize =
PositiveNumber.fromInt(EthProtocolConfiguration.DEFAULT_MAX_MESSAGE_SIZE);

@CommandLine.Option(
hidden = true,
names = {MAX_GET_HEADERS_FLAG},
Expand Down Expand Up @@ -93,6 +103,7 @@ public static EthProtocolOptions create() {

public static EthProtocolOptions fromConfig(final EthProtocolConfiguration config) {
final EthProtocolOptions options = create();
options.maxMessageSize = PositiveNumber.fromInt(config.getMaxMessageSize());
options.maxGetBlockHeaders = PositiveNumber.fromInt(config.getMaxGetBlockHeaders());
options.maxGetBlockBodies = PositiveNumber.fromInt(config.getMaxGetBlockBodies());
options.maxGetReceipts = PositiveNumber.fromInt(config.getMaxGetReceipts());
Expand All @@ -105,6 +116,7 @@ public static EthProtocolOptions fromConfig(final EthProtocolConfiguration confi
@Override
public EthProtocolConfiguration toDomainObject() {
return EthProtocolConfiguration.builder()
.maxMessageSize(maxMessageSize)
.maxGetBlockHeaders(maxGetBlockHeaders)
.maxGetBlockBodies(maxGetBlockBodies)
.maxGetReceipts(maxGetReceipts)
Expand All @@ -117,6 +129,8 @@ public EthProtocolConfiguration toDomainObject() {
@Override
public List<String> getCLIOptions() {
return Arrays.asList(
MAX_MESSAGE_SIZE_FLAG,
OptionParser.format(maxMessageSize.getValue()),
MAX_GET_HEADERS_FLAG,
OptionParser.format(maxGetBlockHeaders.getValue()),
MAX_GET_BODIES_FLAG,
Expand Down
40 changes: 28 additions & 12 deletions besu/src/test/java/org/hyperledger/besu/cli/options/EthProtocolOptionsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

import org.hyperledger.besu.cli.options.unstable.EthProtocolOptions;
import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration;
import org.hyperledger.besu.util.number.PositiveNumber;

import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -30,6 +29,28 @@
public class EthProtocolOptionsTest
extends AbstractCLIOptionsTest<EthProtocolConfiguration, EthProtocolOptions> {

@Test
public void parsesValidMaxMessageSizeOptions() {

final TestBesuCommand cmd = parseCommand("--Xeth-max-message-size", "4");

final EthProtocolOptions options = getOptionsFromBesuCommand(cmd);
final EthProtocolConfiguration config = options.toDomainObject();
assertThat(config.getMaxMessageSize()).isEqualTo(4);
assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
}

@Test
public void parsesInvalidMaxMessageSizeOptionsShouldFail() {
parseCommand("--Xeth-max-message-size", "-4");
verifyNoInteractions(mockRunnerBuilder);
assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8))
.contains(
"Invalid value for option '--Xeth-max-message-size': cannot convert '-4' to PositiveNumber");
}

@Test
public void parsesValidEwpMaxGetHeadersOptions() {

Expand Down Expand Up @@ -127,17 +148,12 @@ EthProtocolConfiguration createDefaultDomainObject() {
@Override
EthProtocolConfiguration createCustomizedDomainObject() {
return EthProtocolConfiguration.builder()
.maxGetBlockHeaders(
PositiveNumber.fromInt(EthProtocolConfiguration.DEFAULT_MAX_GET_BLOCK_HEADERS + 2))
.maxGetBlockBodies(
PositiveNumber.fromInt(EthProtocolConfiguration.DEFAULT_MAX_GET_BLOCK_BODIES + 2))
.maxGetReceipts(
PositiveNumber.fromInt(EthProtocolConfiguration.DEFAULT_MAX_GET_RECEIPTS + 2))
.maxGetNodeData(
PositiveNumber.fromInt(EthProtocolConfiguration.DEFAULT_MAX_GET_NODE_DATA + 2))
.maxGetPooledTransactions(
PositiveNumber.fromInt(
EthProtocolConfiguration.DEFAULT_MAX_GET_POOLED_TRANSACTIONS + 2))
.maxMessageSize(EthProtocolConfiguration.DEFAULT_MAX_MESSAGE_SIZE * 2)
.maxGetBlockHeaders(EthProtocolConfiguration.DEFAULT_MAX_GET_BLOCK_HEADERS + 2)
.maxGetBlockBodies(EthProtocolConfiguration.DEFAULT_MAX_GET_BLOCK_BODIES + 2)
.maxGetReceipts(EthProtocolConfiguration.DEFAULT_MAX_GET_RECEIPTS + 2)
.maxGetNodeData(EthProtocolConfiguration.DEFAULT_MAX_GET_NODE_DATA + 2)
.maxGetPooledTransactions(EthProtocolConfiguration.DEFAULT_MAX_GET_POOLED_TRANSACTIONS + 2)
.build();
}

Expand Down
62 changes: 54 additions & 8 deletions ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/EthProtocolConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*/
package org.hyperledger.besu.ethereum.eth;

import org.hyperledger.besu.util.number.ByteUnits;
import org.hyperledger.besu.util.number.PositiveNumber;

import java.util.Objects;
Expand All @@ -22,27 +23,35 @@

public class EthProtocolConfiguration {

public static final int DEFAULT_MAX_MESSAGE_SIZE = 10 * ByteUnits.MEGABYTE;
public static final int DEFAULT_MAX_GET_BLOCK_HEADERS = 192;
public static final int DEFAULT_MAX_GET_BLOCK_BODIES = 128;
public static final int DEFAULT_MAX_GET_RECEIPTS = 256;
public static final int DEFAULT_MAX_GET_NODE_DATA = 384;
public static final int DEFAULT_MAX_GET_POOLED_TRANSACTIONS = 256;
public static final boolean DEFAULT_LEGACY_ETH_64_FORK_ID_ENABLED = false;

// Limit the size of p2p messages (in bytes)
private final int maxMessageSize;

// These options impose limits on the max number of elements returned when responding to
// peers' p2p RPC requests
private final int maxGetBlockHeaders;
private final int maxGetBlockBodies;
private final int maxGetReceipts;
private final int maxGetNodeData;
private final int maxGetPooledTransactions;
private final boolean legacyEth64ForkIdEnabled;

public EthProtocolConfiguration(
private EthProtocolConfiguration(
final int maxMessageSize,
final int maxGetBlockHeaders,
final int maxGetBlockBodies,
final int maxGetReceipts,
final int maxGetNodeData,
final int maxGetPooledTransactions,
final boolean legacyEth64ForkIdEnabled) {
this.maxMessageSize = maxMessageSize;
this.maxGetBlockHeaders = maxGetBlockHeaders;
this.maxGetBlockBodies = maxGetBlockBodies;
this.maxGetReceipts = maxGetReceipts;
Expand All @@ -52,19 +61,17 @@ public EthProtocolConfiguration(
}

public static EthProtocolConfiguration defaultConfig() {
return new EthProtocolConfiguration(
DEFAULT_MAX_GET_BLOCK_HEADERS,
DEFAULT_MAX_GET_BLOCK_BODIES,
DEFAULT_MAX_GET_RECEIPTS,
DEFAULT_MAX_GET_NODE_DATA,
DEFAULT_MAX_GET_POOLED_TRANSACTIONS,
DEFAULT_LEGACY_ETH_64_FORK_ID_ENABLED);
return builder().build();
}

public static Builder builder() {
return new Builder();
}

public int getMaxMessageSize() {
return maxMessageSize;
}

public int getMaxGetBlockHeaders() {
return maxGetBlockHeaders;
}
Expand Down Expand Up @@ -122,6 +129,9 @@ public String toString() {
}

public static class Builder {
private PositiveNumber maxMessageSize =
PositiveNumber.fromInt(EthProtocolConfiguration.DEFAULT_MAX_MESSAGE_SIZE);

private PositiveNumber maxGetBlockHeaders =
PositiveNumber.fromInt(EthProtocolConfiguration.DEFAULT_MAX_GET_BLOCK_HEADERS);

Expand All @@ -140,38 +150,74 @@ public static class Builder {
private boolean legacyEth64ForkIdEnabled =
EthProtocolConfiguration.DEFAULT_LEGACY_ETH_64_FORK_ID_ENABLED;

public Builder maxMessageSize(final PositiveNumber maxMessageSize) {
this.maxMessageSize = maxMessageSize;
return this;
}

public Builder maxMessageSize(final int maxMessageSize) {
this.maxMessageSize = PositiveNumber.fromInt(maxMessageSize);
return this;
}

public Builder maxGetBlockHeaders(final PositiveNumber maxGetBlockHeaders) {
this.maxGetBlockHeaders = maxGetBlockHeaders;
return this;
}

public Builder maxGetBlockHeaders(final int maxGetBlockHeaders) {
this.maxGetBlockHeaders = PositiveNumber.fromInt(maxGetBlockHeaders);
return this;
}

public Builder maxGetBlockBodies(final PositiveNumber maxGetBlockBodies) {
this.maxGetBlockBodies = maxGetBlockBodies;
return this;
}

public Builder maxGetBlockBodies(final int maxGetBlockBodies) {
this.maxGetBlockBodies = PositiveNumber.fromInt(maxGetBlockBodies);
return this;
}

public Builder maxGetReceipts(final PositiveNumber maxGetReceipts) {
this.maxGetReceipts = maxGetReceipts;
return this;
}

public Builder maxGetReceipts(final int maxGetReceipts) {
this.maxGetReceipts = PositiveNumber.fromInt(maxGetReceipts);
return this;
}

public Builder maxGetNodeData(final PositiveNumber maxGetNodeData) {
this.maxGetNodeData = maxGetNodeData;
return this;
}

public Builder maxGetNodeData(final int maxGetNodeData) {
this.maxGetNodeData = PositiveNumber.fromInt(maxGetNodeData);
return this;
}

public Builder maxGetPooledTransactions(final PositiveNumber maxGetPooledTransactions) {
this.maxGetPooledTransactions = maxGetPooledTransactions;
return this;
}

public Builder maxGetPooledTransactions(final int maxGetPooledTransactions) {
this.maxGetPooledTransactions = PositiveNumber.fromInt(maxGetPooledTransactions);
return this;
}

public Builder legacyEth64ForkIdEnabled(final boolean legacyEth64ForkIdEnabled) {
this.legacyEth64ForkIdEnabled = legacyEth64ForkIdEnabled;
return this;
}

public EthProtocolConfiguration build() {
return new EthProtocolConfiguration(
maxMessageSize.getValue(),
maxGetBlockHeaders.getValue(),
maxGetBlockBodies.getValue(),
maxGetReceipts.getValue(),
Expand Down
13 changes: 10 additions & 3 deletions ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ public class EthProtocolManager implements ProtocolManager, MinedBlockObserver {
private final Blockchain blockchain;
private final BlockBroadcaster blockBroadcaster;
private final List<PeerValidator> peerValidators;
// The max size of messages (in bytes)
private final int maxMessageSize;

public EthProtocolManager(
final Blockchain blockchain,
Expand All @@ -89,6 +91,7 @@ public EthProtocolManager(
this.peerValidators = peerValidators;
this.scheduler = scheduler;
this.blockchain = blockchain;
this.maxMessageSize = ethereumWireProtocolConfiguration.getMaxMessageSize();

this.shutdown = new CountDownLatch(1);
genesisHash = blockchain.getBlockHashByNumber(0L).get();
Expand Down Expand Up @@ -241,9 +244,13 @@ public void processMessage(final Capability cap, final Message message) {
return;
}

if (messageData.getSize() > 10 * 1_000_000 /*10MB*/) {
LOG.debug("Received message over 10MB. Disconnecting from {}", ethPeer);
ethPeer.disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
if (messageData.getSize() > maxMessageSize) {
LOG.warn(
"Received message exceeding size limit of {} bytes: {} bytes. Disconnecting from {}",
maxMessageSize,
messageData.getSize(),
ethPeer);
ethPeer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED);
return;
}

Expand Down
33 changes: 29 additions & 4 deletions ...m/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ public void disconnectOnVeryLargeMessage() {
transactionPool,
EthProtocolConfiguration.defaultConfig())) {
final MessageData messageData = mock(MessageData.class);
when(messageData.getSize()).thenReturn(10 * 1_000_000 + 1 /* just over 10MB*/);
when(messageData.getSize()).thenReturn(EthProtocolConfiguration.DEFAULT_MAX_MESSAGE_SIZE + 1);
when(messageData.getCode()).thenReturn(EthPV62.TRANSACTIONS);
final MockPeerConnection peer = setupPeer(ethManager, (cap, msg, conn) -> {});

Expand All @@ -220,6 +220,25 @@ public void disconnectOnVeryLargeMessage() {
}
}

@Test
public void doNotDisconnectOnLargeMessageWithinLimits() {
try (final EthProtocolManager ethManager =
EthProtocolManagerTestUtil.create(
blockchain,
() -> false,
protocolContext.getWorldStateArchive(),
transactionPool,
EthProtocolConfiguration.defaultConfig())) {
final MessageData messageData = mock(MessageData.class);
when(messageData.getSize()).thenReturn(EthProtocolConfiguration.DEFAULT_MAX_MESSAGE_SIZE);
when(messageData.getCode()).thenReturn(EthPV62.TRANSACTIONS);
final MockPeerConnection peer = setupPeer(ethManager, (cap, msg, conn) -> {});

ethManager.processMessage(EthProtocol.ETH63, new DefaultMessage(peer, messageData));
assertThat(peer.isDisconnected()).isFalse();
}
}

@Test
public void disconnectOnWrongGenesisHash() {
try (final EthProtocolManager ethManager =
Expand Down Expand Up @@ -309,13 +328,15 @@ public void respondToGetHeaders() throws ExecutionException, InterruptedExceptio
public void respondToGetHeadersWithinLimits() throws ExecutionException, InterruptedException {
final CompletableFuture<Void> done = new CompletableFuture<>();
final int limit = 5;
final EthProtocolConfiguration config =
EthProtocolConfiguration.builder().maxGetBlockHeaders(limit).build();
try (final EthProtocolManager ethManager =
EthProtocolManagerTestUtil.create(
blockchain,
() -> false,
protocolContext.getWorldStateArchive(),
transactionPool,
new EthProtocolConfiguration(limit, limit, limit, limit, limit, false))) {
config)) {
final long startBlock = 5L;
final int blockCount = 10;
final MessageData messageData =
Expand Down Expand Up @@ -601,13 +622,15 @@ public void respondToGetBodies() throws ExecutionException, InterruptedException
public void respondToGetBodiesWithinLimits() throws ExecutionException, InterruptedException {
final CompletableFuture<Void> done = new CompletableFuture<>();
final int limit = 5;
final EthProtocolConfiguration config =
EthProtocolConfiguration.builder().maxGetBlockBodies(limit).build();
try (final EthProtocolManager ethManager =
EthProtocolManagerTestUtil.create(
blockchain,
() -> false,
protocolContext.getWorldStateArchive(),
transactionPool,
new EthProtocolConfiguration(limit, limit, limit, limit, limit, false))) {
config)) {
// Setup blocks query
final int blockCount = 10;
final long startBlock = blockchain.getChainHeadBlockNumber() - blockCount;
Expand Down Expand Up @@ -739,13 +762,15 @@ public void respondToGetReceipts() throws ExecutionException, InterruptedExcepti
public void respondToGetReceiptsWithinLimits() throws ExecutionException, InterruptedException {
final CompletableFuture<Void> done = new CompletableFuture<>();
final int limit = 5;
final EthProtocolConfiguration config =
EthProtocolConfiguration.builder().maxGetReceipts(limit).build();
try (final EthProtocolManager ethManager =
EthProtocolManagerTestUtil.create(
blockchain,
() -> false,
protocolContext.getWorldStateArchive(),
transactionPool,
new EthProtocolConfiguration(limit, limit, limit, limit, limit, false))) {
config)) {
// Setup blocks query
final int blockCount = 10;
final long startBlock = blockchain.getChainHeadBlockNumber() - blockCount;
Expand Down
Loading

0 comments on commit 645fdd0

Please sign in to comment.