Skip to content

Commit af6b8e2

Browse files
committed
fix: resend original WHOAREYOU for any new ordinary packet during handshake
When a responder has already issued a WHOAREYOU challenge and receives a second ordinary packet with a different nonce, it now resends the original WHOAREYOU rather than issuing a fresh challenge. This matches the discv5 spec intent: hold one challenge per session attempt so the initiator can complete the handshake it already started. On the initiator side, WHOAREYOU validation is relaxed from 'must match the last sent nonce' to 'must match any recently sent nonce' (bounded window of 8). The nonce-to-session lookup in NodeSessionManager is updated to keep all recent nonces (not just the last), cleared together when the session is deleted, reset on handshake timeout, and cleared on AUTHENTICATED transition.
1 parent 0b2fbf4 commit af6b8e2

5 files changed

Lines changed: 64 additions & 14 deletions

File tree

src/main/java/org/ethereum/beacon/discovery/pipeline/handler/NodeSessionManager.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,9 @@ private void deleteSession(final SessionKey sessionKey) {
128128
if (removedSession != null) {
129129
// Mark inactive to prevent registering any new nonces
130130
removedSession.markInactive();
131-
// And then clean up the last recorded nonce, if any
132-
removedSession.getLastOutboundNonce().ifPresent(lastNonceToSession::remove);
131+
// Remove all nonces associated with this session (we keep multiple recent nonces per session
132+
// so a single-nonce removal is no longer sufficient).
133+
lastNonceToSession.values().removeIf(s -> s == removedSession); // identity check intentional
133134
}
134135
}
135136

@@ -147,7 +148,8 @@ public Optional<NodeSession> getNodeSessionByLastOutboundNonce(final Bytes12 non
147148

148149
public void onSessionLastNonceUpdate(
149150
final NodeSession session, final Optional<Bytes12> previousNonce, final Bytes12 newNonce) {
150-
previousNonce.ifPresent(lastNonceToSession::remove);
151+
// Keep the previous nonce in the map — a WHOAREYOU echoing it may still arrive if the remote
152+
// resends an earlier challenge. All nonces for a session are removed together on session delete.
151153
lastNonceToSession.put(newNonce, session);
152154
}
153155

src/main/java/org/ethereum/beacon/discovery/pipeline/handler/UnauthorizedMessagePacketHandler.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,14 +46,21 @@ protected void handlePacket(Envelope envelope) {
4646
Bytes12 msgNonce = unknownPacket.getHeader().getStaticHeader().getNonce();
4747

4848
// If we have already issued a WHOAREYOU for this exact ordinary packet nonce, resend that
49-
// same challenge — the initiator may have already signed against it. For any other nonce,
50-
// fall through and issue a fresh WHOAREYOU; previous in-flight challenges remain queued so a
51-
// delayed handshake signed against one of them can still be validated.
49+
// same challenge — the initiator may have already signed against it.
5250
if (session.hasPendingWhoAreYouForNonce(msgNonce)) {
5351
session.resendOutgoingWhoAreYouFor(msgNonce);
5452
return;
5553
}
5654

55+
// If we're already awaiting handshake completion (a different nonce triggered the challenge),
56+
// resend the original WHOAREYOU rather than issuing a new one. The spec requires the responder
57+
// to hold the challenge open until the handshake times out; issuing a fresh nonce would race
58+
// with the initiator signing the earlier one.
59+
if (session.getState() == SessionState.WHOAREYOU_SENT) {
60+
session.resendFirstPendingWhoAreYou();
61+
return;
62+
}
63+
5764
try {
5865
Bytes16 idNonce = Bytes16.random(Functions.getRandom());
5966

src/main/java/org/ethereum/beacon/discovery/pipeline/handler/WhoAreYouPacketHandler.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,9 @@ protected void handlePacket(final Envelope envelope) {
6565
final NodeRecord nodeRecord = session.getNodeRecord().orElseThrow();
6666

6767
Bytes12 whoAreYouNonce = whoAreYouPacket.getHeader().getStaticHeader().getNonce();
68-
boolean nonceMatches =
69-
session.getLastOutboundNonce().map(whoAreYouNonce::equals).orElse(false);
68+
// Accept WHOAREYOU for any recently sent nonce, not just the last one. The remote may echo
69+
// back an earlier nonce if it resends the original challenge rather than issuing a new one.
70+
boolean nonceMatches = session.hasRecentOutboundNonce(whoAreYouNonce);
7071
if (!nonceMatches) {
7172
LOG.trace(
7273
"Verification not passed for message [{}] from node {} in status {}",

src/main/java/org/ethereum/beacon/discovery/schema/NodeSession.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
import java.util.Collections;
1515
import java.util.HashSet;
1616
import java.util.LinkedHashMap;
17+
import java.util.LinkedHashSet;
18+
import java.util.List;
1719
import java.util.Map;
1820
import java.util.Optional;
1921
import java.util.Random;
@@ -63,6 +65,13 @@ public class NodeSession {
6365
*/
6466
@VisibleForTesting static final int MAX_PENDING_WHOAREYOU = 5;
6567

68+
/**
69+
* Maximum number of recent outbound nonces tracked per session on the initiator side. Allows
70+
* WHOAREYOU validation to succeed even when the remote echoes back the nonce of an earlier
71+
* ordinary packet rather than the most recent one.
72+
*/
73+
@VisibleForTesting static final int MAX_RECENT_OUTBOUND_NONCES = 8;
74+
6675
private final Bytes32 homeNodeId;
6776
private final LocalNodeRecordStore localNodeRecordStore;
6877
private final NodeSessionManager nodeSessionManager;
@@ -94,6 +103,10 @@ protected boolean removeEldestEntry(final Map.Entry<Bytes12, PendingWhoAreYou> e
94103
}
95104
});
96105

106+
// Bounded set of recent outbound nonces in insertion order. Used by the initiator to accept
107+
// WHOAREYOU packets that echo an earlier nonce rather than the most recent one.
108+
private final LinkedHashSet<Bytes12> recentOutboundNonces = new LinkedHashSet<>();
109+
97110
private Optional<Bytes12> lastOutboundNonce = Optional.empty();
98111
private boolean active = true;
99112
private final Function<Random, Bytes12> nonceGenerator;
@@ -196,6 +209,17 @@ public boolean hasPendingWhoAreYouForNonce(final Bytes12 nonce) {
196209
return pendingWhoAreYou.containsKey(nonce);
197210
}
198211

212+
public void resendFirstPendingWhoAreYou() {
213+
final Bytes12 firstNonce;
214+
synchronized (pendingWhoAreYou) {
215+
if (pendingWhoAreYou.isEmpty()) {
216+
return;
217+
}
218+
firstNonce = pendingWhoAreYou.keySet().iterator().next();
219+
}
220+
resendOutgoingWhoAreYouFor(firstNonce);
221+
}
222+
199223
public void resendOutgoingWhoAreYouFor(final Bytes12 nonce) {
200224
final PendingWhoAreYou pending = pendingWhoAreYou.get(nonce);
201225
if (pending == null) {
@@ -287,6 +311,7 @@ public synchronized RequestInfo createNextRequest(final Request<?> request) {
287311
private synchronized void resetHandshakeState() {
288312
if (state == SessionState.WHOAREYOU_SENT || state == SessionState.RANDOM_PACKET_SENT) {
289313
pendingWhoAreYou.clear();
314+
recentOutboundNonces.clear();
290315
setState(SessionState.INITIAL);
291316
}
292317
}
@@ -321,6 +346,10 @@ public synchronized Bytes12 generateNonce() {
321346
final Bytes12 newNonce = nonceGenerator.apply(rnd);
322347
final Optional<Bytes12> oldNonce = lastOutboundNonce;
323348
lastOutboundNonce = Optional.of(newNonce);
349+
recentOutboundNonces.add(newNonce);
350+
if (recentOutboundNonces.size() > MAX_RECENT_OUTBOUND_NONCES) {
351+
recentOutboundNonces.remove(recentOutboundNonces.iterator().next());
352+
}
324353
if (active) {
325354
// Update while synchronized to ensure only one update in flight at a time. Otherwise the
326355
// previous nonce may not have been recorded before we try to remove it leading to a memory
@@ -330,6 +359,14 @@ public synchronized Bytes12 generateNonce() {
330359
return newNonce;
331360
}
332361

362+
public synchronized boolean hasRecentOutboundNonce(final Bytes12 nonce) {
363+
return recentOutboundNonces.contains(nonce);
364+
}
365+
366+
public synchronized Collection<Bytes12> getRecentOutboundNonces() {
367+
return List.copyOf(recentOutboundNonces);
368+
}
369+
333370
public synchronized Optional<Bytes12> getLastOutboundNonce() {
334371
return lastOutboundNonce;
335372
}
@@ -456,6 +493,9 @@ public synchronized void setState(final SessionState newStatus) {
456493
LOG.trace(
457494
() -> String.format("Switching status of node %s from %s to %s", nodeId, state, newStatus));
458495
this.state = newStatus;
496+
if (newStatus == SessionState.AUTHENTICATED) {
497+
recentOutboundNonces.clear();
498+
}
459499
}
460500

461501
public Signer getSigner() {

src/test/java/org/ethereum/beacon/discovery/pipeline/handler/UnauthorizedMessagePacketHandlerTest.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,22 +61,22 @@ void shouldNotChangeStateWhenResendingForKnownNonce() {
6161
}
6262

6363
@Test
64-
void shouldSendNewWhoAreYouWhenIncomingNonceIsUnknown() {
64+
void shouldResendFirstPendingWhoAreYouForDifferentNonceDuringHandshake() {
65+
// When WHOAREYOU_SENT and the incoming nonce doesn't match any pending challenge, resend the
66+
// original challenge so the initiator can complete the handshake it already started.
6567
final NodeSession session = mock(NodeSession.class);
6668
when(session.getState()).thenReturn(SessionState.WHOAREYOU_SENT);
67-
when(session.getNodeRecord()).thenReturn(Optional.empty());
6869

6970
final OrdinaryMessagePacket packet = createOrdinaryPacket();
7071
final Bytes12 incomingNonce = packet.getHeader().getStaticHeader().getNonce();
71-
// No pending WhoAreYou for this nonce — an earlier challenge was for a different nonce.
7272
when(session.hasPendingWhoAreYouForNonce(incomingNonce)).thenReturn(false);
7373

7474
handler.handle(envelopeWith(session, packet));
7575

76+
verify(session).resendFirstPendingWhoAreYou();
7677
verify(session, never()).resendOutgoingWhoAreYouFor(any());
77-
final ArgumentCaptor<WhoAreYouPacket> captor = ArgumentCaptor.forClass(WhoAreYouPacket.class);
78-
verify(session).sendOutgoingWhoAreYou(captor.capture());
79-
assertThat(captor.getValue().getHeader().getStaticHeader().getNonce()).isEqualTo(incomingNonce);
78+
verify(session, never()).sendOutgoingWhoAreYou(any());
79+
verify(session, never()).setState(any());
8080
}
8181

8282
@Test

0 commit comments

Comments
 (0)