From 3e4fbce034b384c99afeead6cf0f72be0a2b8f13 Mon Sep 17 00:00:00 2001 From: thinkAfCod Date: Wed, 2 Apr 2025 19:47:44 +0800 Subject: [PATCH 1/2] p2p/discover: repeat exact encoding when resending WHOAREYOU packet (#31543) When resending the WHOAREYOU packet, a new nonce and random IV should not be generated. The sent packet needs to match the previously-sent one exactly in order to make the handshake retry work. --------- Co-authored-by: Felix Lange --- p2p/discover/v5_udp_test.go | 40 +++++++++++++++++++++++---------- p2p/discover/v5wire/encoding.go | 16 +++++++++++-- p2p/discover/v5wire/msg.go | 3 +++ 3 files changed, 45 insertions(+), 14 deletions(-) diff --git a/p2p/discover/v5_udp_test.go b/p2p/discover/v5_udp_test.go index 3026dff538..606b35c4f2 100644 --- a/p2p/discover/v5_udp_test.go +++ b/p2p/discover/v5_udp_test.go @@ -181,29 +181,35 @@ func TestUDPv5_handshakeRepeatChallenge(t *testing.T) { nonce1 := v5wire.Nonce{1} nonce2 := v5wire.Nonce{2} nonce3 := v5wire.Nonce{3} - check := func(p *v5wire.Whoareyou, wantNonce v5wire.Nonce) { + var firstAuthTag *v5wire.Nonce + check := func(p *v5wire.Whoareyou, authTag, wantNonce v5wire.Nonce) { t.Helper() if p.Nonce != wantNonce { - t.Error("wrong nonce in WHOAREYOU:", p.Nonce, wantNonce) + t.Error("wrong nonce in WHOAREYOU:", p.Nonce, "want:", wantNonce) + } + if firstAuthTag == nil { + firstAuthTag = &authTag + } else if authTag != *firstAuthTag { + t.Error("wrong auth tag in WHOAREYOU header:", authTag, "want:", *firstAuthTag) } } // Unknown packet from unknown node. test.packetIn(&v5wire.Unknown{Nonce: nonce1}) - test.waitPacketOut(func(p *v5wire.Whoareyou, addr netip.AddrPort, _ v5wire.Nonce) { - check(p, nonce1) + test.waitPacketOut(func(p *v5wire.Whoareyou, addr netip.AddrPort, authTag v5wire.Nonce) { + check(p, authTag, nonce1) }) // Second unknown packet. Here we expect the response to reference the // first unknown packet. test.packetIn(&v5wire.Unknown{Nonce: nonce2}) - test.waitPacketOut(func(p *v5wire.Whoareyou, addr netip.AddrPort, _ v5wire.Nonce) { - check(p, nonce1) + test.waitPacketOut(func(p *v5wire.Whoareyou, addr netip.AddrPort, authTag v5wire.Nonce) { + check(p, authTag, nonce1) }) // Third unknown packet. This should still return the first nonce. test.packetIn(&v5wire.Unknown{Nonce: nonce3}) - test.waitPacketOut(func(p *v5wire.Whoareyou, addr netip.AddrPort, _ v5wire.Nonce) { - check(p, nonce1) + test.waitPacketOut(func(p *v5wire.Whoareyou, addr netip.AddrPort, authTag v5wire.Nonce) { + check(p, authTag, nonce1) }) } @@ -766,20 +772,30 @@ type testCodecFrame struct { } func (c *testCodec) Encode(toID enode.ID, addr string, p v5wire.Packet, _ *v5wire.Whoareyou) ([]byte, v5wire.Nonce, error) { + // To match the behavior of v5wire.Codec, we return the cached encoding of + // WHOAREYOU challenges. + if wp, ok := p.(*v5wire.Whoareyou); ok && len(wp.Encoded) > 0 { + return wp.Encoded, wp.Nonce, nil + } + c.ctr++ var authTag v5wire.Nonce binary.BigEndian.PutUint64(authTag[:], c.ctr) + penc, _ := rlp.EncodeToBytes(p) + frame, err := rlp.EncodeToBytes(testCodecFrame{c.id, authTag, p.Kind(), penc}) + if err != nil { + return frame, authTag, err + } + // Store recently sent challenges. if w, ok := p.(*v5wire.Whoareyou); ok { - // Store recently sent Whoareyou challenges. + w.Nonce = authTag + w.Encoded = frame if c.sentChallenges == nil { c.sentChallenges = make(map[enode.ID]*v5wire.Whoareyou) } c.sentChallenges[toID] = w } - - penc, _ := rlp.EncodeToBytes(p) - frame, err := rlp.EncodeToBytes(testCodecFrame{c.id, authTag, p.Kind(), penc}) return frame, authTag, err } diff --git a/p2p/discover/v5wire/encoding.go b/p2p/discover/v5wire/encoding.go index e50b7cd16d..b16d14eda5 100644 --- a/p2p/discover/v5wire/encoding.go +++ b/p2p/discover/v5wire/encoding.go @@ -189,6 +189,11 @@ func (c *Codec) Encode(id enode.ID, addr string, packet Packet, challenge *Whoar ) switch { case packet.Kind() == WhoareyouPacket: + // just send the WHOAREYOU packet raw again, rather than the re-encoded challenge data + w := packet.(*Whoareyou) + if len(w.Encoded) > 0 { + return w.Encoded, w.Nonce, nil + } head, err = c.encodeWhoareyou(id, packet.(*Whoareyou)) case challenge != nil: // We have an unanswered challenge, send handshake. @@ -218,15 +223,22 @@ func (c *Codec) Encode(id enode.ID, addr string, packet Packet, challenge *Whoar // Store sent WHOAREYOU challenges. if challenge, ok := packet.(*Whoareyou); ok { challenge.ChallengeData = bytesCopy(&c.buf) + enc, err := c.EncodeRaw(id, head, msgData) + if err != nil { + return nil, Nonce{}, err + } + challenge.Encoded = bytes.Clone(enc) c.sc.storeSentHandshake(id, addr, challenge) - } else if msgData == nil { + return enc, head.Nonce, err + } + + if msgData == nil { headerData := c.buf.Bytes() msgData, err = c.encryptMessage(session, packet, &head, headerData) if err != nil { return nil, Nonce{}, err } } - enc, err := c.EncodeRaw(id, head, msgData) return enc, head.Nonce, err } diff --git a/p2p/discover/v5wire/msg.go b/p2p/discover/v5wire/msg.go index 401db2f6c5..089fd4ebdc 100644 --- a/p2p/discover/v5wire/msg.go +++ b/p2p/discover/v5wire/msg.go @@ -73,6 +73,9 @@ type ( Node *enode.Node sent mclock.AbsTime // for handshake GC. + + // Encoded is packet raw data for sending out, but should not be include in the RLP encoding. + Encoded []byte `rlp:"-"` } // PING is sent during liveness checks. From d2176f463b873567ff8982f362b843a898c429d1 Mon Sep 17 00:00:00 2001 From: thinkAfCod Date: Wed, 2 Apr 2025 20:56:21 +0800 Subject: [PATCH 2/2] p2p/discover: pass node instead of node ID to TALKREQ handler (#31075) This is for the implementation of Portal Network in the Shisui client. Their handler needs access to the node object in order to send further calls to the requesting node. This is a breaking API change but it should be fine, since there are basically no known users of TALKREQ outside of Portal network. --------- Signed-off-by: thinkAfCod Co-authored-by: Felix Lange --- p2p/discover/v5_talk.go | 14 ++++++++++---- p2p/discover/v5_udp.go | 3 +++ p2p/discover/v5_udp_test.go | 6 +++++- p2p/discover/v5wire/encoding.go | 8 ++++++-- p2p/discover/v5wire/encoding_test.go | 14 +++++++------- p2p/discover/v5wire/session.go | 16 ++++++++++++++-- 6 files changed, 45 insertions(+), 16 deletions(-) diff --git a/p2p/discover/v5_talk.go b/p2p/discover/v5_talk.go index 2246b47141..dca09870d8 100644 --- a/p2p/discover/v5_talk.go +++ b/p2p/discover/v5_talk.go @@ -39,7 +39,7 @@ const talkHandlerLaunchTimeout = 400 * time.Millisecond // Note that talk handlers are expected to come up with a response very quickly, within at // most 200ms or so. If the handler takes longer than that, the remote end may time out // and wont receive the response. -type TalkRequestHandler func(enode.ID, *net.UDPAddr, []byte) []byte +type TalkRequestHandler func(*enode.Node, *net.UDPAddr, []byte) []byte type talkSystem struct { transport *UDPv5 @@ -72,13 +72,19 @@ func (t *talkSystem) register(protocol string, handler TalkRequestHandler) { // handleRequest handles a talk request. func (t *talkSystem) handleRequest(id enode.ID, addr netip.AddrPort, req *v5wire.TalkRequest) { + n := t.transport.codec.SessionNode(id, addr.String()) + if n == nil { + // The node must be contained in the session here, since we wouldn't have + // received the request otherwise. + panic("missing node in session") + } t.mutex.Lock() handler, ok := t.handlers[req.Protocol] t.mutex.Unlock() if !ok { resp := &v5wire.TalkResponse{ReqID: req.ReqID} - t.transport.sendResponse(id, addr, resp) + t.transport.sendResponse(n.ID(), addr, resp) return } @@ -90,9 +96,9 @@ func (t *talkSystem) handleRequest(id enode.ID, addr netip.AddrPort, req *v5wire go func() { defer func() { t.slots <- struct{}{} }() udpAddr := &net.UDPAddr{IP: addr.Addr().AsSlice(), Port: int(addr.Port())} - respMessage := handler(id, udpAddr, req.Message) + respMessage := handler(n, udpAddr, req.Message) resp := &v5wire.TalkResponse{ReqID: req.ReqID, Message: respMessage} - t.transport.sendFromAnotherThread(id, addr, resp) + t.transport.sendFromAnotherThread(n.ID(), addr, resp) }() case <-timeout.C: // Couldn't get it in time, drop the request. diff --git a/p2p/discover/v5_udp.go b/p2p/discover/v5_udp.go index 6f7c797152..9679f5c61a 100644 --- a/p2p/discover/v5_udp.go +++ b/p2p/discover/v5_udp.go @@ -64,6 +64,9 @@ type codecV5 interface { // CurrentChallenge returns the most recent WHOAREYOU challenge that was encoded to given node. // This will return a non-nil value if there is an active handshake attempt with the node, and nil otherwise. CurrentChallenge(id enode.ID, addr string) *v5wire.Whoareyou + + // SessionNode returns a node that has completed the handshake. + SessionNode(id enode.ID, addr string) *enode.Node } // UDPv5 is the implementation of protocol version 5. diff --git a/p2p/discover/v5_udp_test.go b/p2p/discover/v5_udp_test.go index 606b35c4f2..3a384aab12 100644 --- a/p2p/discover/v5_udp_test.go +++ b/p2p/discover/v5_udp_test.go @@ -492,7 +492,7 @@ func TestUDPv5_talkHandling(t *testing.T) { defer test.close() var recvMessage []byte - test.udp.RegisterTalkHandler("test", func(id enode.ID, addr *net.UDPAddr, message []byte) []byte { + test.udp.RegisterTalkHandler("test", func(n *enode.Node, addr *net.UDPAddr, message []byte) []byte { recvMessage = message return []byte("test response") }) @@ -811,6 +811,10 @@ func (c *testCodec) Decode(input []byte, addr string) (enode.ID, *enode.Node, v5 return frame.NodeID, nil, p, nil } +func (c *testCodec) SessionNode(id enode.ID, addr string) *enode.Node { + return c.test.nodesByID[id].Node() +} + func (c *testCodec) decodeFrame(input []byte) (frame testCodecFrame, p v5wire.Packet, err error) { if err = rlp.DecodeBytes(input, &frame); err != nil { return frame, nil, fmt.Errorf("invalid frame: %v", err) diff --git a/p2p/discover/v5wire/encoding.go b/p2p/discover/v5wire/encoding.go index b16d14eda5..ec5ef8a261 100644 --- a/p2p/discover/v5wire/encoding.go +++ b/p2p/discover/v5wire/encoding.go @@ -359,7 +359,7 @@ func (c *Codec) encodeHandshakeHeader(toID enode.ID, addr string, challenge *Who } // TODO: this should happen when the first authenticated message is received - c.sc.storeNewSession(toID, addr, session) + c.sc.storeNewSession(toID, addr, session, challenge.Node) // Encode the auth header. var ( @@ -534,7 +534,7 @@ func (c *Codec) decodeHandshakeMessage(fromAddr string, head *Header, headerData } // Handshake OK, drop the challenge and store the new session keys. - c.sc.storeNewSession(auth.h.SrcID, fromAddr, session) + c.sc.storeNewSession(auth.h.SrcID, fromAddr, session, node) c.sc.deleteHandshake(auth.h.SrcID, fromAddr) return node, msg, nil } @@ -656,6 +656,10 @@ func (c *Codec) decryptMessage(input, nonce, headerData, readKey []byte) (Packet return DecodeMessage(msgdata[0], msgdata[1:]) } +func (c *Codec) SessionNode(id enode.ID, addr string) *enode.Node { + return c.sc.readNode(id, addr) +} + // checkValid performs some basic validity checks on the header. // The packetLen here is the length remaining after the static header. func (h *StaticHeader) checkValid(packetLen int, protocolID [6]byte) error { diff --git a/p2p/discover/v5wire/encoding_test.go b/p2p/discover/v5wire/encoding_test.go index df97e40e89..2304d0f132 100644 --- a/p2p/discover/v5wire/encoding_test.go +++ b/p2p/discover/v5wire/encoding_test.go @@ -166,7 +166,7 @@ func TestHandshake_rekey(t *testing.T) { readKey: []byte("BBBBBBBBBBBBBBBB"), writeKey: []byte("AAAAAAAAAAAAAAAA"), } - net.nodeA.c.sc.storeNewSession(net.nodeB.id(), net.nodeB.addr(), session) + net.nodeA.c.sc.storeNewSession(net.nodeB.id(), net.nodeB.addr(), session, net.nodeB.n()) // A -> B FINDNODE (encrypted with zero keys) findnode, authTag := net.nodeA.encode(t, net.nodeB, &Findnode{}) @@ -209,8 +209,8 @@ func TestHandshake_rekey2(t *testing.T) { readKey: []byte("CCCCCCCCCCCCCCCC"), writeKey: []byte("DDDDDDDDDDDDDDDD"), } - net.nodeA.c.sc.storeNewSession(net.nodeB.id(), net.nodeB.addr(), initKeysA) - net.nodeB.c.sc.storeNewSession(net.nodeA.id(), net.nodeA.addr(), initKeysB) + net.nodeA.c.sc.storeNewSession(net.nodeB.id(), net.nodeB.addr(), initKeysA, net.nodeB.n()) + net.nodeB.c.sc.storeNewSession(net.nodeA.id(), net.nodeA.addr(), initKeysB, net.nodeA.n()) // A -> B FINDNODE encrypted with initKeysA findnode, authTag := net.nodeA.encode(t, net.nodeB, &Findnode{Distances: []uint{3}}) @@ -362,8 +362,8 @@ func TestTestVectorsV5(t *testing.T) { ENRSeq: 2, }, prep: func(net *handshakeTest) { - net.nodeA.c.sc.storeNewSession(idB, addr, session) - net.nodeB.c.sc.storeNewSession(idA, addr, session.keysFlipped()) + net.nodeA.c.sc.storeNewSession(idB, addr, session, net.nodeB.n()) + net.nodeB.c.sc.storeNewSession(idA, addr, session.keysFlipped(), net.nodeA.n()) }, }, { @@ -499,8 +499,8 @@ func BenchmarkV5_DecodePing(b *testing.B) { readKey: []byte{233, 203, 93, 195, 86, 47, 177, 186, 227, 43, 2, 141, 244, 230, 120, 17}, writeKey: []byte{79, 145, 252, 171, 167, 216, 252, 161, 208, 190, 176, 106, 214, 39, 178, 134}, } - net.nodeA.c.sc.storeNewSession(net.nodeB.id(), net.nodeB.addr(), session) - net.nodeB.c.sc.storeNewSession(net.nodeA.id(), net.nodeA.addr(), session.keysFlipped()) + net.nodeA.c.sc.storeNewSession(net.nodeB.id(), net.nodeB.addr(), session, net.nodeB.n()) + net.nodeB.c.sc.storeNewSession(net.nodeA.id(), net.nodeA.addr(), session.keysFlipped(), net.nodeA.n()) addrB := net.nodeA.addr() ping := &Ping{ReqID: []byte("reqid"), ENRSeq: 5} enc, _, err := net.nodeA.c.Encode(net.nodeB.id(), addrB, ping, nil) diff --git a/p2p/discover/v5wire/session.go b/p2p/discover/v5wire/session.go index 862c21fcee..5a2166b143 100644 --- a/p2p/discover/v5wire/session.go +++ b/p2p/discover/v5wire/session.go @@ -54,11 +54,12 @@ type session struct { writeKey []byte readKey []byte nonceCounter uint32 + node *enode.Node } // keysFlipped returns a copy of s with the read and write keys flipped. func (s *session) keysFlipped() *session { - return &session{s.readKey, s.writeKey, s.nonceCounter} + return &session{s.readKey, s.writeKey, s.nonceCounter, s.node} } func NewSessionCache(maxItems int, clock mclock.Clock) *SessionCache { @@ -103,8 +104,19 @@ func (sc *SessionCache) readKey(id enode.ID, addr string) []byte { return nil } +func (sc *SessionCache) readNode(id enode.ID, addr string) *enode.Node { + if s := sc.session(id, addr); s != nil { + return s.node + } + return nil +} + // storeNewSession stores new encryption keys in the cache. -func (sc *SessionCache) storeNewSession(id enode.ID, addr string, s *session) { +func (sc *SessionCache) storeNewSession(id enode.ID, addr string, s *session, n *enode.Node) { + if n == nil { + panic("nil node in storeNewSession") + } + s.node = n sc.sessions.Add(sessionID{id, addr}, s) }