Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ protected final void channelRead0(final ChannelHandlerContext ctx, final ByteBuf
metricsSystem,
inboundInitiated,
peerLookup,
maxMessageSize);
maxMessageSize,
nodeId);

ctx.channel()
.pipeline()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import java.io.IOException;
import java.net.InetSocketAddress;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.atomic.AtomicBoolean;
Expand Down Expand Up @@ -74,6 +73,7 @@ final class DeFramer extends ByteToMessageDecoder {
private final List<SubProtocol> subProtocols;
private final boolean inboundInitiated;
private final PeerLookup peerLookup;
private final Bytes authenticatedNodeId;
private boolean hellosExchanged;
private final LabelledMetric<Counter> outboundMessagesCounter;
private final int maxMessageSize;
Expand All @@ -89,7 +89,8 @@ final class DeFramer extends ByteToMessageDecoder {
final MetricsSystem metricsSystem,
final boolean inboundInitiated,
final PeerLookup peerLookup,
final int maxMessageSize) {
final int maxMessageSize,
final Bytes authenticatedNodeId) {
this.framer = framer;
this.subProtocols = subProtocols;
this.localNode = localNode;
Expand All @@ -99,6 +100,7 @@ final class DeFramer extends ByteToMessageDecoder {
this.inboundInitiated = inboundInitiated;
this.peerLookup = peerLookup;
this.maxMessageSize = maxMessageSize;
this.authenticatedNodeId = authenticatedNodeId;
this.outboundMessagesCounter =
metricsSystem.createLabelledCounter(
BesuMetricCategory.NETWORK,
Expand Down Expand Up @@ -158,6 +160,17 @@ protected void decode(final ChannelHandlerContext ctx, final ByteBuf in, final L
return;
}
LOG.trace("Received HELLO message: {}", peerInfo);
if (!peerInfo.getNodeId().equals(authenticatedNodeId)) {
LOG.debug(
"Peer Hello nodeId {} does not match authenticated nodeId from handshake {}. Disconnecting.",
peerInfo.getNodeId(),
authenticatedNodeId);
connectFuture.completeExceptionally(
new UnexpectedPeerConnectionException(
"Hello nodeId does not match handshake identity"));
ctx.close();
return;
}
Comment on lines +163 to +173
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On HELLO/handshake identity mismatch, the code closes the channel without sending a devp2p Disconnect reason (previous behavior used UNEXPECTED_ID). This can make the disconnect harder to diagnose for the remote side and may deviate from expected protocol behavior. Consider emitting a DisconnectMessage with an appropriate reason before closing (or moving this check to a point where a RlpxConnection exists so connection.disconnect(...) can be used), then closing after flush.

Copilot uses AI. Check for mistakes.
if (peerInfo.getVersion() >= 5) {
LOG.trace("Enable compression for p2pVersion: {}", peerInfo.getVersion());
framer.enableCompression();
Expand Down Expand Up @@ -200,17 +213,6 @@ protected void decode(final ChannelHandlerContext ctx, final ByteBuf in, final L
outboundBytesCounter,
inboundInitiated);

// Check peer is who we expected
if (expectedPeer.isPresent()
&& !Objects.equals(expectedPeer.get().getId(), peerInfo.getNodeId())) {
final String unexpectedMsg =
String.format(
"Expected id %s, but got %s", expectedPeer.get().getId(), peerInfo.getNodeId());
connectFuture.completeExceptionally(new UnexpectedPeerConnectionException(unexpectedMsg));
LOG.debug("{}. Disconnecting.", unexpectedMsg);
connection.disconnect(DisconnectMessage.DisconnectReason.UNEXPECTED_ID);
}

// Check that we have shared caps
if (capabilityMultiplexer.getAgreedCapabilities().isEmpty()) {
LOG.debug("Disconnecting because no capabilities are shared: {}", peerInfo);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,7 @@ public class ECIESHandshaker implements Handshaker {
private static final Logger LOG = LoggerFactory.getLogger(ECIESHandshaker.class);
private static final SecureRandom RANDOM = SecureRandomProvider.publicSecureRandom();

static final int SIGNATURE_LENGTH = 65;
static final int HASH_EPH_PUBKEY_LENGTH = 32;
static final int PUBKEY_LENGTH = 64;
static final int NONCE_LENGTH = 32;
static final int TOKEN_FLAG_LENGTH = 1;

// Keypairs under our control.
private NodeKey nodeKey;
Expand Down Expand Up @@ -86,8 +82,6 @@ public class ECIESHandshaker implements Handshaker {
new AtomicReference<>(Handshaker.HandshakeStatus.UNINITIALIZED);
private HandshakeSecrets secrets;

private boolean version4 = true;

private final SignatureAlgorithm signatureAlgorithm = SignatureAlgorithmFactory.getInstance();

@Override
Expand Down Expand Up @@ -130,21 +124,11 @@ public ByteBuf firstMessage() throws HandshakeException {
"illegal invocation of firstMessage, handshake had already started");

final Bytes32 staticSharedSecret = nodeKey.calculateECDHKeyAgreement(partyPubKey);
if (version4) {
initiatorMsg =
InitiatorHandshakeMessageV4.create(
nodeKey.getPublicKey(), ephKeyPair, staticSharedSecret, initiatorNonce);
} else {
initiatorMsg =
InitiatorHandshakeMessageV1.create(
nodeKey.getPublicKey(), ephKeyPair, staticSharedSecret, initiatorNonce, false);
}
initiatorMsg =
InitiatorHandshakeMessageV4.create(
nodeKey.getPublicKey(), ephKeyPair, staticSharedSecret, initiatorNonce);
try {
if (version4) {
initiatorMsgEnc = EncryptedMessage.encryptMsgEip8(initiatorMsg.encode(), partyPubKey);
} else {
initiatorMsgEnc = EncryptedMessage.encryptMsg(initiatorMsg.encode(), partyPubKey);
}
initiatorMsgEnc = EncryptedMessage.encryptMsgEip8(initiatorMsg.encode(), partyPubKey);
} catch (final InvalidCipherTextException e) {
status.set(Handshaker.HandshakeStatus.FAILED);
throw new HandshakeException("Encrypting the first handshake message failed", e);
Expand All @@ -161,51 +145,26 @@ public Optional<ByteBuf> handleMessage(final ByteBuf buf) throws HandshakeExcept
status.get() == Handshaker.HandshakeStatus.IN_PROGRESS,
"illegal invocation of onMessage on handshake that is not in progress");

// Take as many bytes as expected in the next message.
int expectedLength = ECIESEncryptionEngine.ENCRYPTION_OVERHEAD;
expectedLength +=
initiator
? ResponderHandshakeMessageV1.MESSAGE_LENGTH
: InitiatorHandshakeMessageV1.MESSAGE_LENGTH;

if (buf.readableBytes() < expectedLength) {
buf.markReaderIndex();
final int size = buf.readUnsignedShort();
if (size > buf.readableBytes() + 2) {
buf.resetReaderIndex();
return Optional.empty();
}
expectedLength = size;
// Read the EIP-8 size prefix to determine the full message length.
buf.markReaderIndex();
if (buf.readableBytes() < 2) {
return Optional.empty();
}
final int size = buf.readUnsignedShort();
if (size > buf.readableBytes()) {
buf.resetReaderIndex();
return Optional.empty();
}

buf.markReaderIndex();
final ByteBuf bufferedBytes = buf.readSlice(expectedLength);
final byte[] encryptedBytes = new byte[bufferedBytes.readableBytes()];
bufferedBytes.getBytes(0, encryptedBytes);
Bytes bytes = Bytes.wrap(encryptedBytes);
// Read the full EIP-8 message (size prefix + payload).
buf.resetReaderIndex();
final byte[] fullMessage = new byte[size + 2];
buf.readBytes(fullMessage);
final Bytes encryptedMsg = Bytes.wrap(fullMessage);
Comment on lines +148 to +163
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EIP-8 size prefix is used directly to allocate new byte[size + 2] without any upper-bound validation. A peer can advertise a large size (up to 65535) to force oversized allocations and increase memory pressure/GC (or potentially OOM if repeated). Add a hard cap for handshake packets (e.g., a small constant like a few KB, aligned with expected auth/ack sizes plus EIP-8 padding) and fail the handshake early when size exceeds that limit.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot suggest changes based on this feedback


Bytes encryptedMsg = bytes;
final Bytes bytes;
try {
// Decrypt the message with our private key.
try {
// Assume new format
final int size = bufferedBytes.readUnsignedShort();
if (buf.writerIndex() >= size) {
bufferedBytes.readerIndex(0);
final byte[] fullMessage = new byte[size + 2];
bufferedBytes.readBytes(fullMessage, 0, expectedLength);
buf.readBytes(fullMessage, expectedLength, size - expectedLength + 2);
encryptedMsg = Bytes.wrap(fullMessage);
bytes = EncryptedMessage.decryptMsgEIP8(encryptedMsg, nodeKey);
version4 = true;
} else {
throw new HandshakeException("Failed to decrypt handshake message");
}
} catch (final Exception ex) {
bytes = EncryptedMessage.decryptMsg(bytes, nodeKey);
version4 = false;
}
bytes = EncryptedMessage.decryptMsgEIP8(encryptedMsg, nodeKey);
} catch (final InvalidCipherTextException e) {
status.set(Handshaker.HandshakeStatus.FAILED);
throw new HandshakeException("Decrypting an incoming handshake message failed", e);
Expand All @@ -226,11 +185,7 @@ public Optional<ByteBuf> handleMessage(final ByteBuf buf) throws HandshakeExcept

// Store the message, as we need it to generating our ingress and egress MACs.
responderMsgEnc = encryptedMsg;
if (version4) {
responderMsg = ResponderHandshakeMessageV4.decode(bytes);
} else {
responderMsg = ResponderHandshakeMessageV1.decode(bytes);
}
responderMsg = ResponderHandshakeMessageV4.decode(bytes);

// Extract the responder's nonce and ephemeral pubkey, which will be used to generate the
// shared secrets.
Expand All @@ -253,11 +208,7 @@ public Optional<ByteBuf> handleMessage(final ByteBuf buf) throws HandshakeExcept
// Store the message, as we need it to generating our ingress and egress MACs.
initiatorMsgEnc = encryptedMsg;
try {
if (version4) {
initiatorMsg = InitiatorHandshakeMessageV4.decode(bytes, nodeKey);
} else {
initiatorMsg = InitiatorHandshakeMessageV1.decode(bytes, nodeKey);
}
initiatorMsg = InitiatorHandshakeMessageV4.decode(bytes, nodeKey);
} catch (final SecurityModuleException e) {
status.set(Handshaker.HandshakeStatus.FAILED);
throw new HandshakeException(
Expand All @@ -279,25 +230,15 @@ public Optional<ByteBuf> handleMessage(final ByteBuf buf) throws HandshakeExcept
"keccak hash of recovered ephemeral pubkey does not match announced hash");

// Build the response message.
if (version4) {
responderMsg =
ResponderHandshakeMessageV4.create(ephKeyPair.getPublicKey(), responderNonce);
} else {
responderMsg =
ResponderHandshakeMessageV1.create(ephKeyPair.getPublicKey(), responderNonce, false);
}
responderMsg = ResponderHandshakeMessageV4.create(ephKeyPair.getPublicKey(), responderNonce);

LOG.trace(
"Generated responder's ECIES handshake message against peer {}...: {}",
partyPubKey.getEncodedBytes().slice(0, 16),
responderMsg);

try {
if (version4) {
responderMsgEnc = EncryptedMessage.encryptMsgEip8(responderMsg.encode(), partyPubKey);
} else {
responderMsgEnc = EncryptedMessage.encryptMsg(responderMsg.encode(), partyPubKey);
}
responderMsgEnc = EncryptedMessage.encryptMsgEip8(responderMsg.encode(), partyPubKey);
} catch (final InvalidCipherTextException e) {
status.set(Handshaker.HandshakeStatus.FAILED);
throw new HandshakeException("Encrypting the next handshake message failed", e);
Expand Down Expand Up @@ -435,14 +376,4 @@ Bytes32 getResponderNonce() {
void setResponderNonce(final Bytes32 responderNonce) {
this.responderNonce = responderNonce;
}

@VisibleForTesting
void setInitiatorMsgEnc(final Bytes initiatorMsgEnc) {
this.initiatorMsgEnc = initiatorMsgEnc;
}

@VisibleForTesting
void setResponderMsgEnc(final Bytes responderMsgEnc) {
this.responderMsgEnc = responderMsgEnc;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,34 +40,7 @@ private EncryptedMessage() {
}

/**
* Decrypts the ciphertext using our private key.
*
* @param msgBytes The ciphertext.
* @param nodeKey Abstraction of this nodes private key & associated cryptographic operations
* @return The plaintext.
* @throws InvalidCipherTextException Thrown if decryption failed.
*/
public static Bytes decryptMsg(final Bytes msgBytes, final NodeKey nodeKey)
throws InvalidCipherTextException {

// Extract the ephemeral public key, stripping off the first byte (0x04), which designates it's
// an uncompressed key.
final SECPPublicKey ephPubKey = SIGNATURE_ALGORITHM.createPublicKey(msgBytes.slice(1, 64));

// Strip off the IV to use.
final Bytes iv = msgBytes.slice(65, IV_SIZE);

// Extract the encrypted payload.
final Bytes encrypted = msgBytes.slice(65 + IV_SIZE);

// Perform the decryption.
final ECIESEncryptionEngine decryptor =
ECIESEncryptionEngine.forDecryption(nodeKey, ephPubKey, iv);
return decryptor.decrypt(encrypted);
}

/**
* Decrypts the ciphertext using our private key.
* Decrypts an EIP-8 formatted ciphertext using our private key.
*
* @param msgBytes The ciphertext.
* @param nodeKey Abstraction of this nodes private key & associated cryptographic operations
Expand All @@ -91,39 +64,7 @@ public static Bytes decryptMsgEIP8(final Bytes msgBytes, final NodeKey nodeKey)
}

/**
* Encrypts a message for the specified peer using ECIES.
*
* @param bytes The plaintext.
* @param remoteKey The peer's remote key.
* @return The ciphertext.
* @throws InvalidCipherTextException Thrown if encryption failed.
*/
public static Bytes encryptMsg(final Bytes bytes, final SECPPublicKey remoteKey)
throws InvalidCipherTextException {
// TODO: check size.
final ECIESEncryptionEngine engine = ECIESEncryptionEngine.forEncryption(remoteKey);

// Do the encryption.
final Bytes encrypted = engine.encrypt(bytes);
final Bytes iv = engine.getIv();
final SECPPublicKey ephPubKey = engine.getEphPubKey();

// Create the output message by concatenating the ephemeral public key (prefixed with
// 0x04 to designate uncompressed), IV, and encrypted bytes.
final MutableBytes answer =
MutableBytes.create(1 + ECIESHandshaker.PUBKEY_LENGTH + IV_SIZE + encrypted.size());

int offset = 0;
// Set the first byte as 0x04 to specify it's an uncompressed key.
answer.set(offset, (byte) 0x04);
ephPubKey.getEncodedBytes().copyTo(answer, offset += 1);
iv.copyTo(answer, offset += ECIESHandshaker.PUBKEY_LENGTH);
encrypted.copyTo(answer, offset + iv.size());
return answer;
}

/**
* Encrypts a message for the specified peer using ECIES.
* Encrypts a message for the specified peer using EIP-8 ECIES.
*
* @param message The plaintext.
* @param remoteKey The peer's remote key.
Expand Down
Loading
Loading