From d2d2e749113526b80eef3c130e7750a3be44430d Mon Sep 17 00:00:00 2001 From: pk910 Date: Wed, 29 Jul 2026 13:54:59 +0000 Subject: [PATCH 1/6] discv4: fix ENRRESPONSE serializing an empty ENR record The ENRResponse packet embedded an enr.Record whose EncodeRLP has signature ([]byte, error) and so does not satisfy rlp.Encoder; with no exported fields the record was serialized as an empty list (c0). go-ethereum rejects the reply with 'record contains less than two list elements', so discv4 ENR requests against the bootnode time out and clients cannot resolve its EIP-2124 eth fork id. Implement rlp.Encoder on ENRResponse to serialize the record via its own RLP encoding. Add a regression test that decodes the packet with go-ethereum's v4wire type, plus discv5 official wire test vectors. --- discv4/protocol/enrresponse_test.go | 69 ++++++ discv4/protocol/messages.go | 32 +++ discv5/protocol/wire_vectors_test.go | 311 +++++++++++++++++++++++++++ 3 files changed, 412 insertions(+) create mode 100644 discv4/protocol/enrresponse_test.go create mode 100644 discv5/protocol/wire_vectors_test.go diff --git a/discv4/protocol/enrresponse_test.go b/discv4/protocol/enrresponse_test.go new file mode 100644 index 0000000..9a29da8 --- /dev/null +++ b/discv4/protocol/enrresponse_test.go @@ -0,0 +1,69 @@ +package protocol + +import ( + "bytes" + "net" + "testing" + + ethcrypto "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/p2p/discover/v4wire" + gethenode "github.com/ethereum/go-ethereum/p2p/enode" + "github.com/ethereum/go-ethereum/rlp" + "github.com/ethpandaops/bootnodoor/enr" +) + +// TestENRResponseEncodesRecord verifies that a discv4 ENRRESPONSE serializes the +// embedded ENR record correctly (regression for the empty-record bug where +// enr.Record was encoded as c0 because it does not satisfy rlp.Encoder). +func TestENRResponseEncodesRecord(t *testing.T) { + key, _ := ethcrypto.GenerateKey() + rec := enr.New() + _ = rec.Set("id", "v4") + _ = rec.Set("ip", net.IPv4(127, 0, 0, 1).To4()) + _ = rec.Set("udp", uint16(30303)) + if err := rec.Sign(key); err != nil { + t.Fatalf("sign: %v", err) + } + + resp := &ENRResponse{ReplyTok: []byte{1, 2, 3}, Record: rec} + + packet, _, err := Encode(key, resp) + if err != nil { + t.Fatalf("encode: %v", err) + } + + // Decode as geth would: parse packet, then decode the ENR via geth's enode. + decoded, err := DecodePacket(packet) + if err != nil { + t.Fatalf("self-decode: %v", err) + } + got, ok := decoded.(*ENRResponse) + if !ok { + t.Fatalf("wrong type: %T", decoded) + } + + // The embedded record must match the original record's canonical encoding. + want, _ := rec.EncodeRLP() + gotRLP, _ := got.Record.EncodeRLP() + if !bytes.Equal(want, gotRLP) { + t.Fatalf("record mismatch:\n got %x\nwant %x", gotRLP, want) + } + + // And geth's own v4wire decoder must accept it (this is what fails on the wire: + // "record contains less than two list elements"). + sigdata := packet[headSize:] // type || rlp + var gethResp v4wire.ENRResponse + if err := rlp.DecodeBytes(sigdata[1:], &gethResp); err != nil { + t.Fatalf("geth v4wire decode rejected the response: %v", err) + } + gethNode, err := gethenode.New(gethenode.ValidSchemes, &gethResp.Record) + if err != nil { + t.Fatalf("geth could not build node from record: %v", err) + } + if gethNode.Seq() != rec.Seq() { + t.Fatalf("geth decoded seq %d, want %d", gethNode.Seq(), rec.Seq()) + } + if !gethNode.IP().Equal(net.IPv4(127, 0, 0, 1)) { + t.Fatalf("geth decoded ip %s, want 127.0.0.1", gethNode.IP()) + } +} diff --git a/discv4/protocol/messages.go b/discv4/protocol/messages.go index 9828783..5aa174a 100644 --- a/discv4/protocol/messages.go +++ b/discv4/protocol/messages.go @@ -13,6 +13,7 @@ import ( "crypto/ecdsa" "crypto/elliptic" "fmt" + "io" "net" "time" @@ -192,6 +193,37 @@ func (e *ENRResponse) Name() string { return "ENRRESPONSE/v4" } // Kind returns the packet type byte. func (e *ENRResponse) Kind() byte { return ENRResponsePacket } +// EncodeRLP implements rlp.Encoder so the embedded ENR record is serialized via +// enr.Record's own RLP encoding. +// +// Without this, geth's rlp package does not recognize enr.Record as an encoder +// (its EncodeRLP has signature ([]byte, error), not the io.Writer form the +// interface requires) and, because enr.Record has no exported fields, would +// serialize the record as an empty RLP list. Real clients (go-ethereum) then +// reject the response with "record contains less than two list elements". +func (e *ENRResponse) EncodeRLP(w io.Writer) error { + var recordRLP []byte + if e.Record != nil { + encoded, err := e.Record.EncodeRLP() + if err != nil { + return err + } + recordRLP = encoded + } + + type enrResponseRLP struct { + ReplyTok []byte + Record rlp.RawValue + Rest []rlp.RawValue `rlp:"tail"` + } + + return rlp.Encode(w, &enrResponseRLP{ + ReplyTok: e.ReplyTok, + Record: recordRLP, + Rest: e.Rest, + }) +} + // Supporting Types // Pubkey represents an encoded 64-byte secp256k1 public key. diff --git a/discv5/protocol/wire_vectors_test.go b/discv5/protocol/wire_vectors_test.go new file mode 100644 index 0000000..dc4dfe1 --- /dev/null +++ b/discv5/protocol/wire_vectors_test.go @@ -0,0 +1,311 @@ +package protocol + +// Official discv5 wire test vectors from +// https://github.com/ethereum/devp2p/blob/master/discv5/discv5-wire-test-vectors.md +// +// These tests verify bootnodoor's wire-level decoding and handshake crypto against +// the specification test vectors. + +import ( + "bytes" + "crypto/ecdsa" + "encoding/hex" + "strings" + "testing" + + ethcrypto "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/rlp" + "github.com/ethpandaops/bootnodoor/discv5/node" + "github.com/ethpandaops/bootnodoor/discv5/session" +) + +func mustHex(t *testing.T, s string) []byte { + t.Helper() + s = strings.ReplaceAll(strings.TrimPrefix(s, "0x"), "\n", "") + s = strings.ReplaceAll(s, " ", "") + b, err := hex.DecodeString(s) + if err != nil { + t.Fatalf("bad hex: %v", err) + } + return b +} + +func nodeID(t *testing.T, s string) node.ID { + t.Helper() + var id node.ID + copy(id[:], mustHex(t, s)) + return id +} + +func privKey(t *testing.T, s string) *ecdsa.PrivateKey { + t.Helper() + k, err := ethcrypto.ToECDSA(mustHex(t, s)) + if err != nil { + t.Fatalf("bad key: %v", err) + } + return k +} + +const ( + srcNodeIDHex = "0xaaaa8419e9f49d0083561b48287df592939a8d19947d8c0ef88f2a4856a69fbb" + destNodeIDHex = "0xbbbb9d047f0488c0b5a93c1c3f2d8bafc7c8ff337024a55434a0d0555de64db9" + nodeAKeyHex = "0xeef77acb6c6a6eebc5b363a475ac583ec7eccdb42b6481424c60f59aa326547f" + nodeBKeyHex = "0x66fb62bfbd66b9177a138c1e5cddbe4f7c30c343e94e68df8769459cb1cde628" +) + +// --- Packet decoding vectors --- + +func TestVectorPingMessagePacket(t *testing.T) { + packetBytes := mustHex(t, ` +00000000000000000000000000000000088b3d4342774649325f313964a39e55 +ea96c005ad52be8c7560413a7008f16c9e6d2f43bbea8814a546b7409ce783d3 +4c4f53245d08dab84102ed931f66d1492acb308fa1c6715b9d139b81acbdcc`) + + destID := nodeID(t, destNodeIDHex) + srcID := nodeID(t, srcNodeIDHex) + + p, err := DecodePacket(packetBytes, destID) + if err != nil { + t.Fatalf("DecodePacket failed: %v", err) + } + if p.PacketType != OrdinaryPacket { + t.Fatalf("wrong packet type: %d", p.PacketType) + } + if !bytes.Equal(p.SrcID, srcID[:]) { + t.Fatalf("wrong src id: %x", p.SrcID) + } + wantNonce := mustHex(t, "0xffffffffffffffffffffffff") + if !bytes.Equal(p.Header.Nonce, wantNonce) { + t.Fatalf("wrong nonce: %x", p.Header.Nonce) + } + + // Decrypt with the vector read-key + readKey := mustHex(t, "0x00000000000000000000000000000000") + pt, err := session.DecryptMessage(readKey, p.Header.Nonce, p.HeaderData, p.Message) + if err != nil { + t.Fatalf("decrypt failed (GCM AAD construction mismatch?): %v", err) + } + // plaintext = message-type (0x01 ping) || rlp([req-id, enr-seq]) + if pt[0] != PingMsg { + t.Fatalf("wrong message type: %x", pt[0]) + } + // Verify the decoded RLP fields (req-id, enr-seq) rather than exact bytes. + msg := &Ping{} + if err := decodeRLP(pt[1:], msg); err != nil { + t.Fatalf("rlp decode: %v", err) + } + if !bytes.Equal(msg.RequestID, mustHex(t, "0x00000001")) { + t.Fatalf("wrong req-id: %x", msg.RequestID) + } + if msg.ENRSeq != 2 { + t.Fatalf("wrong enr-seq: %d", msg.ENRSeq) + } +} + +func TestVectorWhoareyouPacket(t *testing.T) { + packetBytes := mustHex(t, ` +00000000000000000000000000000000088b3d434277464933a1ccc59f5967ad +1d6035f15e528627dde75cd68292f9e6c27d6b66c8100a873fcbaed4e16b8d`) + + destID := nodeID(t, destNodeIDHex) + + p, err := DecodePacket(packetBytes, destID) + if err != nil { + t.Fatalf("DecodePacket failed: %v", err) + } + if p.PacketType != WHOAREYOUPacket { + t.Fatalf("wrong packet type: %d", p.PacketType) + } + if !bytes.Equal(p.Header.Nonce, mustHex(t, "0x0102030405060708090a0b0c")) { + t.Fatalf("wrong request-nonce: %x", p.Header.Nonce) + } + if !bytes.Equal(p.Challenge.IDNonce, mustHex(t, "0x0102030405060708090a0b0c0d0e0f10")) { + t.Fatalf("wrong id-nonce: %x", p.Challenge.IDNonce) + } + if p.Challenge.ENRSeq != 0 { + t.Fatalf("wrong enr-seq: %d", p.Challenge.ENRSeq) + } + // The challenge-data (HeaderData) must match the spec value exactly — + // this is the id-signature input and HKDF salt. + wantChallengeData := mustHex(t, "0x000000000000000000000000000000006469736376350001010102030405060708090a0b0c00180102030405060708090a0b0c0d0e0f100000000000000000") + if !bytes.Equal(p.HeaderData, wantChallengeData) { + t.Fatalf("challenge-data mismatch:\n got %x\nwant %x", p.HeaderData, wantChallengeData) + } +} + +func TestVectorWhoareyouChallengeDataBuilder(t *testing.T) { + // BuildWHOAREYOUChallengeData must reproduce the same challenge-data the remote + // peer extracts from the raw packet. + maskingIV := mustHex(t, "0x00000000000000000000000000000000") + nonce := mustHex(t, "0x0102030405060708090a0b0c") + challenge := &WHOAREYOUChallenge{ + IDNonce: mustHex(t, "0x0102030405060708090a0b0c0d0e0f10"), + ENRSeq: 0, + } + got := BuildWHOAREYOUChallengeData(maskingIV, nonce, challenge) + want := mustHex(t, "0x000000000000000000000000000000006469736376350001010102030405060708090a0b0c00180102030405060708090a0b0c0d0e0f100000000000000000") + if !bytes.Equal(got, want) { + t.Fatalf("challenge-data mismatch:\n got %x\nwant %x", got, want) + } +} + +func TestVectorHandshakePacket(t *testing.T) { + packetBytes := mustHex(t, ` +00000000000000000000000000000000088b3d4342774649305f313964a39e55 +ea96c005ad521d8c7560413a7008f16c9e6d2f43bbea8814a546b7409ce783d3 +4c4f53245d08da4bb252012b2cba3f4f374a90a75cff91f142fa9be3e0a5f3ef +268ccb9065aeecfd67a999e7fdc137e062b2ec4a0eb92947f0d9a74bfbf44dfb +a776b21301f8b65efd5796706adff216ab862a9186875f9494150c4ae06fa4d1 +f0396c93f215fa4ef524f1eadf5f0f4126b79336671cbcf7a885b1f8bd2a5d83 +9cf8`) + + destID := nodeID(t, destNodeIDHex) + srcID := nodeID(t, srcNodeIDHex) + + p, err := DecodePacket(packetBytes, destID) + if err != nil { + t.Fatalf("DecodePacket failed: %v", err) + } + if p.PacketType != HandshakePacket { + t.Fatalf("wrong packet type: %d", p.PacketType) + } + if !bytes.Equal(p.Handshake.SourceNodeID, srcID[:]) { + t.Fatalf("wrong src id: %x", p.Handshake.SourceNodeID) + } + if len(p.Handshake.Signature) != 64 { + t.Fatalf("wrong sig size: %d", len(p.Handshake.Signature)) + } + wantEph := mustHex(t, "0x039a003ba6517b473fa0cd74aefe99dadfdb34627f90fec6362df85803908f53a5") + if !bytes.Equal(p.Handshake.EphemeralPubKey, wantEph) { + t.Fatalf("wrong ephemeral pubkey: %x", p.Handshake.EphemeralPubKey) + } + if len(p.Handshake.ENR) != 0 { + t.Fatalf("unexpected ENR in handshake: %x", p.Handshake.ENR) + } + + // Verify the id-signature against node A's static key using the vector's + // challenge-data (from the WHOAREYOU with enr-seq=1). + challengeData := mustHex(t, "0x000000000000000000000000000000006469736376350001010102030405060708090a0b0c00180102030405060708090a0b0c0d0e0f100000000000000001") + nodeAKey := privKey(t, nodeAKeyHex) + if !verifyIDSignature(&nodeAKey.PublicKey, p.Handshake.Signature, challengeData, p.Handshake.EphemeralPubKey, destID) { + t.Fatal("id-signature verification failed") + } + + // Decrypt the message with the vector read-key. + readKey := mustHex(t, "0x4f9fac6de7567d1e3b1241dffe90f662") + pt, err := session.DecryptMessage(readKey, p.Header.Nonce, p.HeaderData, p.Message) + if err != nil { + t.Fatalf("decrypt failed: %v", err) + } + if pt[0] != PingMsg { + t.Fatalf("wrong message type: %x", pt[0]) + } +} + +func TestVectorHandshakePacketWithENR(t *testing.T) { + packetBytes := mustHex(t, ` +00000000000000000000000000000000088b3d4342774649305f313964a39e55 +ea96c005ad539c8c7560413a7008f16c9e6d2f43bbea8814a546b7409ce783d3 +4c4f53245d08da4bb23698868350aaad22e3ab8dd034f548a1c43cd246be9856 +2fafa0a1fa86d8e7a3b95ae78cc2b988ded6a5b59eb83ad58097252188b902b2 +1481e30e5e285f19735796706adff216ab862a9186875f9494150c4ae06fa4d1 +f0396c93f215fa4ef524e0ed04c3c21e39b1868e1ca8105e585ec17315e755e6 +cfc4dd6cb7fd8e1a1f55e49b4b5eb024221482105346f3c82b15fdaae36a3bb1 +2a494683b4a3c7f2ae41306252fed84785e2bbff3b022812d0882f06978df84a +80d443972213342d04b9048fc3b1d5fcb1df0f822152eced6da4d3f6df27e70e +4539717307a0208cd208d65093ccab5aa596a34d7511401987662d8cf62b1394 +71`) + + destID := nodeID(t, destNodeIDHex) + + p, err := DecodePacket(packetBytes, destID) + if err != nil { + t.Fatalf("DecodePacket failed: %v", err) + } + if p.PacketType != HandshakePacket { + t.Fatalf("wrong packet type: %d", p.PacketType) + } + if len(p.Handshake.ENR) == 0 { + t.Fatal("expected ENR in handshake packet") + } + + readKey := mustHex(t, "0x53b1c075f41876423154e157470c2f48") + pt, err := session.DecryptMessage(readKey, p.Header.Nonce, p.HeaderData, p.Message) + if err != nil { + t.Fatalf("decrypt failed: %v", err) + } + if pt[0] != PingMsg { + t.Fatalf("wrong message type: %x", pt[0]) + } +} + +// --- Crypto primitive vectors --- + +func TestVectorECDH(t *testing.T) { + pub := mustHex(t, "0x039961e4c2356d61bedb83052c115d311acb3a96f5777296dcf297351130266231") + secKey := privKey(t, "0xfb757dc581730490a1d7a00deea65e9b1936924caaea8f44d476014856b68736") + + pubKey, err := ethcrypto.DecompressPubkey(pub) + if err != nil { + t.Fatalf("bad pubkey: %v", err) + } + + shared := ecdh(secKey, pubKey) + want := mustHex(t, "0x033b11a2a1f214567e1537ce5e509ffd9b21373247f2a3ff6841f4976f53165e7e") + if !bytes.Equal(shared, want) { + t.Fatalf("ECDH mismatch:\n got %x\nwant %x", shared, want) + } +} + +func TestVectorKeyDerivation(t *testing.T) { + ephKey := privKey(t, "0xfb757dc581730490a1d7a00deea65e9b1936924caaea8f44d476014856b68736") + destPub := mustHex(t, "0x0317931e6e0840220642f230037d285d122bc59063221ef3226b1f403ddc69ca91") + nodeIDA := nodeID(t, srcNodeIDHex) + nodeIDB := nodeID(t, destNodeIDHex) + challengeData := mustHex(t, "0x000000000000000000000000000000006469736376350001010102030405060708090a0b0c00180102030405060708090a0b0c0d0e0f100000000000000000") + + destPubKey, err := ethcrypto.DecompressPubkey(destPub) + if err != nil { + t.Fatalf("bad pubkey: %v", err) + } + + keys, err := deriveKeys(ephKey, destPubKey, nodeIDA, nodeIDB, challengeData) + if err != nil { + t.Fatalf("deriveKeys failed: %v", err) + } + + wantInitiator := mustHex(t, "0xdccc82d81bd610f4f76d3ebe97a40571") + wantRecipient := mustHex(t, "0xac74bb8773749920b0d3a8881c173ec5") + if !bytes.Equal(keys.InitiatorKey, wantInitiator) { + t.Fatalf("initiator-key mismatch:\n got %x\nwant %x", keys.InitiatorKey, wantInitiator) + } + if !bytes.Equal(keys.RecipientKey, wantRecipient) { + t.Fatalf("recipient-key mismatch:\n got %x\nwant %x", keys.RecipientKey, wantRecipient) + } +} + +func TestVectorIDNonceSigning(t *testing.T) { + staticKey := privKey(t, "0xfb757dc581730490a1d7a00deea65e9b1936924caaea8f44d476014856b68736") + challengeData := mustHex(t, "0x000000000000000000000000000000006469736376350001010102030405060708090a0b0c00180102030405060708090a0b0c0d0e0f100000000000000000") + ephPub := mustHex(t, "0x039961e4c2356d61bedb83052c115d311acb3a96f5777296dcf297351130266231") + nodeIDB := nodeID(t, destNodeIDHex) + + sig, err := makeIDSignature(staticKey, challengeData, ephPub, nodeIDB) + if err != nil { + t.Fatalf("makeIDSignature failed: %v", err) + } + want := mustHex(t, "0x94852a1e2318c4e5e9d422c98eaf19d1d90d876b29cd06ca7cb7546d0fff7b484fe86c09a064fe72bdbef73ba8e9c34df0cd2b53e9d65528c2c7f336d5dfc6e6") + if !bytes.Equal(sig, want) { + t.Fatalf("id-signature mismatch:\n got %x\nwant %x", sig, want) + } + + // And verify direction + if !verifyIDSignature(&staticKey.PublicKey, want, challengeData, ephPub, nodeIDB) { + t.Fatal("verifyIDSignature rejected the spec vector signature") + } +} + +// decodeRLP is a tiny helper wrapping geth's rlp for the tests. +func decodeRLP(data []byte, val interface{}) error { + return rlp.DecodeBytes(data, val) +} From 01ad1c96ff927aae5c8c74bcc1dd78908b82be2b Mon Sep 17 00:00:00 2001 From: pk910 Date: Wed, 29 Jul 2026 14:05:54 +0000 Subject: [PATCH 2/6] discv5: cap NODES response to <=15 nodes / <=5 packets go-ethereum honours only the first NODES packet's total and reads at most 5 packets; sigp/discv5 caps at 16 nodes. Cap the served set at 15 nodes / <=5 packets so no served node is silently dropped by a requester. --- discv5/protocol/handler.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/discv5/protocol/handler.go b/discv5/protocol/handler.go index 476553c..1b29221 100644 --- a/discv5/protocol/handler.go +++ b/discv5/protocol/handler.go @@ -1040,10 +1040,19 @@ func (h *Handler) handleFindNode(msg *FindNode, remoteID node.ID, from *net.UDPA }).Debug("handler: FINDNODE lookup completed via callback") } - // Split nodes into multiple packets if needed to stay under max packet size - // Each ENR is typically 200-400 bytes, so we limit to 3 nodes per packet to be safe + // Split nodes into multiple packets if needed to stay under max packet size. + // Each ENR is typically 200-400 bytes, so we limit to 3 nodes per packet to be safe. const maxNodesPerPacket = 3 + // Cap the total response so it never exceeds what real clients consume. go-ethereum + // honours only the first packet's `total` and reads at most 5 NODES packets + // (totalNodesResponseLimit); anything beyond that is dropped as unsolicited. sigp/discv5 + // caps at 16 nodes. Keep to <=5 packets / <=15 nodes so no served node is silently lost. + const maxNodesPerResponse = 15 + if len(nodes) > maxNodesPerResponse { + nodes = nodes[:maxNodesPerResponse] + } + // Calculate total number of packets needed totalPackets := (len(nodes) + maxNodesPerPacket - 1) / maxNodesPerPacket if totalPackets == 0 { From 6b5c5c9c97e824cec75721fc6b116a943aa742fc Mon Sep 17 00:00:00 2001 From: pk910 Date: Wed, 29 Jul 2026 14:05:54 +0000 Subject: [PATCH 3/6] enr: enforce the 300-byte record size limit on decode/ingest Matches go-ethereum and sigp/discv5. Without it an oversized but validly-signed ENR received in NODES/handshake could be stored and re-served, and a single oversized record makes real clients discard the whole NODES message. --- enr/encoding.go | 8 ++++++++ enr/record_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/enr/encoding.go b/enr/encoding.go index de89888..2fac796 100644 --- a/enr/encoding.go +++ b/enr/encoding.go @@ -57,6 +57,14 @@ func (r *Record) DecodeRLPBytes(data []byte) error { r.mu.Lock() defer r.mu.Unlock() + // Enforce the EIP-778 300-byte size limit on ingest, matching go-ethereum and + // sigp/discv5. Without this, an oversized (but validly signed) ENR received in a + // NODES/handshake message could be stored and later re-served, and a single + // oversized record makes real clients discard the entire NODES message. + if len(data) > MaxRecordSize { + return ErrRecordTooLarge + } + // Decode the RLP list var items []interface{} if err := rlp.DecodeBytes(data, &items); err != nil { diff --git a/enr/record_test.go b/enr/record_test.go index 541a297..314aef3 100644 --- a/enr/record_test.go +++ b/enr/record_test.go @@ -272,3 +272,34 @@ func BenchmarkRecordEncoding(b *testing.B) { record.EncodeRLP() } } + +// TestDecodeRejectsOversizedRecord verifies the 300-byte ingest limit (G9): an +// oversized but structurally-valid record must be rejected on decode so it can't be +// stored and re-served. +func TestDecodeRejectsOversizedRecord(t *testing.T) { + // build > 300 bytes of RLP by padding a raw record body + big := make([]byte, MaxRecordSize+50) + for i := range big { + big[i] = 0x80 // RLP empty-string items; content irrelevant, size is the point + } + rec := New() + err := rec.DecodeRLPBytes(big) + if err != ErrRecordTooLarge { + t.Fatalf("expected ErrRecordTooLarge for %d-byte input, got %v", len(big), err) + } + // a normal signed record must still decode fine + key, _ := crypto.GenerateKey() + ok := New() + _ = ok.Set("id", "v4") + if err := ok.Sign(key); err != nil { + t.Fatalf("sign: %v", err) + } + enc, _ := ok.EncodeRLP() + if len(enc) > MaxRecordSize { + t.Skip("signed record unexpectedly large") + } + rt := New() + if err := rt.DecodeRLPBytes(enc); err != nil { + t.Fatalf("valid record rejected: %v", err) + } +} From 585c617cfb08b52454ae1347251b874042c25f60 Mon Sep 17 00:00:00 2001 From: pk910 Date: Wed, 29 Jul 2026 14:06:32 +0000 Subject: [PATCH 4/6] discv4: always send NEIGHBORS and advertise the real TCP port An empty routing table previously produced a silent (zero-packet) response, so go-ethereum's querier waited out its full request timeout instead of returning early on the first reply. Always send >=1 NEIGHBORS packet. Also advertise the node's real TCP port from its ENR instead of copying the UDP port. --- discv4/protocol/handler.go | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/discv4/protocol/handler.go b/discv4/protocol/handler.go index fc1c72b..a56ae71 100644 --- a/discv4/protocol/handler.go +++ b/discv4/protocol/handler.go @@ -782,8 +782,11 @@ func (h *Handler) sendPong(to *node.Node, addr *net.UDPAddr, localAddr *net.UDPA // sendNeighbors sends NEIGHBORS response(s). func (h *Handler) sendNeighbors(to *node.Node, addr *net.UDPAddr, localAddr *net.UDPAddr, nodes []*node.Node) error { - // Split nodes into packets of MaxNeighbors - for i := 0; i < len(nodes); i += MaxNeighbors { + // Split nodes into packets of MaxNeighbors. Always send at least one packet, + // even when we have no nodes to offer: go-ethereum's querier waits for a + // NEIGHBORS reply and only stops early once at least one arrives, so a silent + // (zero-packet) response makes it wait out the full request timeout. + for i := 0; i == 0 || i < len(nodes); i += MaxNeighbors { end := i + MaxNeighbors if end > len(nodes) { end = len(nodes) @@ -793,10 +796,18 @@ func (h *Handler) sendNeighbors(to *node.Node, addr *net.UDPAddr, localAddr *net nodeRecords := make([]NodeRecord, len(batch)) for j, n := range batch { + // Advertise the node's real TCP port from its ENR when known; + // only fall back to the UDP port if no ENR tcp entry is available. + tcpPort := uint16(n.Addr().Port) + if rec := n.ENR(); rec != nil { + if t := rec.TCP(); t != 0 { + tcpPort = t + } + } nodeRecords[j] = NodeRecord{ IP: n.Addr().IP, UDP: uint16(n.Addr().Port), - TCP: uint16(n.Addr().Port), + TCP: tcpPort, ID: EncodePubkey(n.PublicKey()), } } From e44ea45cd1f6cbc34f5f1acc2cf9930df1c3616c Mon Sep 17 00:00:00 2001 From: pk910 Date: Wed, 29 Jul 2026 14:07:13 +0000 Subject: [PATCH 5/6] discv4: bond only after the peer answers our ping (endpoint proof) Bonding on a received PING alone let a spoofed-source PING coax a large NEIGHBORS reply to a victim (amplification), and granted FINDNODE/ENRRequest access without endpoint proof. Establish the bond only when the peer answers our ping-back with a PONG, matching go-ethereum. The ping-back is unchanged, so real peers still bond. --- discv4/protocol/handler.go | 16 +++++++++------- discv4/protocol/handler_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/discv4/protocol/handler.go b/discv4/protocol/handler.go index a56ae71..13b0d82 100644 --- a/discv4/protocol/handler.go +++ b/discv4/protocol/handler.go @@ -330,13 +330,15 @@ func (h *Handler) handlePing(fromNode *node.Node, from *net.UDPAddr, localAddr * return err } - // Mark node as bonded: they pinged us, we ponged them. - // This allows THEM to query US with FINDNODE immediately. - fromNode.MarkPongReceived(h.config.BondExpiration) - - // IMPORTANT: For bidirectional bonding (required by strict clients like reth for ENRRequest), - // we also need to establish that WE can reach THEM, not just that they can reach us. - // Send a PING back to them to establish bidirectional bond. + // Do NOT mark the node bonded here. A received PING only proves the sender + // claims this endpoint; the UDP source can be spoofed, so bonding on it would + // let a spoofed PING coax a large NEIGHBORS reply to a victim (amplification). + // The bond is established only once the peer answers OUR ping-back with a PONG + // (handlePong -> MarkPongReceived), matching go-ethereum's endpoint proof. + + // Send a PING back so the peer can prove its endpoint and become bonded. This + // also satisfies strict clients (e.g. reth) that require bidirectional bonding + // before answering ENRRequest. // // RATE LIMITING: Only send PING back if we haven't pinged them in the last 100ms. // This prevents ping-pong loops (max 10 pings/sec per node). diff --git a/discv4/protocol/handler_test.go b/discv4/protocol/handler_test.go index d21528f..6401916 100644 --- a/discv4/protocol/handler_test.go +++ b/discv4/protocol/handler_test.go @@ -97,3 +97,31 @@ func TestCleanupReclaimsFloodedNodes(t *testing.T) { t.Fatalf("flooded nodes not reclaimed: %d still tracked", got) } } + +// TestPingDoesNotBondBeforePong verifies that receiving a PING must not by itself +// bond the sender. The bond is only established once the peer answers our +// ping-back with a PONG (endpoint proof), matching go-ethereum. +func TestPingDoesNotBondBeforePong(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + key, _ := crypto.GenerateKey() + h := NewHandler(ctx, HandlerConfig{PrivateKey: key, BondExpiration: time.Hour, LocalAddr: testAddr(), RequestTimeout: 20 * time.Millisecond}, stubTransport{}) + + pub, id := makeNodeID(t) + n := h.getOrCreateNode(id, pub, testAddr()) + + // Handling an inbound PING sends a PONG + a ping-back but must not bond. + if err := h.handlePing(n, testAddr(), nil, &Ping{Version: 4, Expiration: MakeExpiration(time.Minute)}, []byte{0x01}); err != nil { + t.Fatalf("handlePing: %v", err) + } + if n.IsBonded() { + t.Fatal("node became bonded from an inbound PING alone") + } + + // Once the peer answers our ping-back with a PONG, the bond is established. + n.MarkPongReceived(time.Hour) + if !n.IsBonded() { + t.Fatal("node should be bonded after receiving a PONG to our ping") + } +} From 14315765a796279785d0010a177b5b890fb6d98f Mon Sep 17 00:00:00 2001 From: pk910 Date: Wed, 29 Jul 2026 14:07:48 +0000 Subject: [PATCH 6/6] discv4: evict a stale node instead of dropping new peers when the map is full The node-map cap previously returned a non-retained node once full, so under a flood of distinct signed IDs a genuinely new peer's inbound PING marked bond state on a discarded object and could never bond (memory-growth DoS turned into a bonding-lockout DoS). When full, evict one unbonded entry to admit the new node; bonded, endpoint-proven peers are never evicted this way. --- discv4/protocol/handler.go | 24 +++++++++++++++++----- discv4/protocol/handler_test.go | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/discv4/protocol/handler.go b/discv4/protocol/handler.go index 13b0d82..4a51140 100644 --- a/discv4/protocol/handler.go +++ b/discv4/protocol/handler.go @@ -884,12 +884,26 @@ func (h *Handler) getOrCreateNode(id node.ID, pubkey *ecdsa.PublicKey, addr *net n = node.New(pubkey, addr) n.UpdateLastSeen() - // Bound the map so an unauthenticated flood of distinct node IDs (for - // example fabricated NEIGHBORS records) cannot grow it without limit. Stale - // unbonded entries are reclaimed by cleanup; until a slot frees up we still - // return the node so the packet is handled, but we do not retain it. + // Bound the map so an unauthenticated flood of distinct node IDs (for example + // fabricated NEIGHBORS records, or signed PINGs from generated keys) cannot grow + // it without limit. When full, evict one unbonded entry to make room rather than + // dropping the new node: otherwise a flood that pins the map at MaxNodes would + // lock out genuine new peers (their node is never retained, so their inbound PING + // can never lead to a bond). Bonded entries are real, endpoint-proven peers and + // are never evicted here; if every entry is bonded (genuine load, not a flood) we + // leave the map as-is and return the node without retaining it. if len(h.nodes) >= h.config.MaxNodes { - return n + evicted := false + for eid, en := range h.nodes { + if !en.IsBonded() { + delete(h.nodes, eid) + evicted = true + break + } + } + if !evicted { + return n + } } h.nodes[id] = n diff --git a/discv4/protocol/handler_test.go b/discv4/protocol/handler_test.go index 6401916..85d0851 100644 --- a/discv4/protocol/handler_test.go +++ b/discv4/protocol/handler_test.go @@ -125,3 +125,38 @@ func TestPingDoesNotBondBeforePong(t *testing.T) { t.Fatal("node should be bonded after receiving a PONG to our ping") } } + +// TestFloodDoesNotEvictBondedPeers verifies that when the node map is full, +// inserts evict a stale unbonded entry (so genuine new peers are never locked +// out) while bonded, endpoint-proven peers are retained. +func TestFloodDoesNotEvictBondedPeers(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + const maxNodes = 10 + h := NewHandler(ctx, HandlerConfig{MaxNodes: maxNodes, NodeTTL: time.Hour}, nil) + + // A genuine, bonded peer. + pub, bondedID := makeNodeID(t) + bonded := h.getOrCreateNode(bondedID, pub, testAddr()) + bonded.MarkPongReceived(time.Hour) + + // Fill the rest with unbonded nodes, then flood well past the cap. + for i := 0; i < maxNodes*20; i++ { + p, id := makeNodeID(t) + h.getOrCreateNode(id, p, testAddr()) + } + + if got := len(h.AllNodes()); got != maxNodes { + t.Fatalf("map not bounded under flood: got %d want %d", got, maxNodes) + } + if h.GetNode(bondedID) == nil { + t.Fatal("bonded peer was evicted by an unbonded-ID flood") + } + // A brand-new node still gets retained (evicting an unbonded entry). + p, freshID := makeNodeID(t) + h.getOrCreateNode(freshID, p, testAddr()) + if h.GetNode(freshID) == nil { + t.Fatal("new peer not retained when map full") + } +}