diff --git a/p2p/discover/v5_talk.go b/p2p/discover/v5_talk.go index 10c4983d5a..dca09870d8 100644 --- a/p2p/discover/v5_talk.go +++ b/p2p/discover/v5_talk.go @@ -72,10 +72,11 @@ 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) { - var n *enode.Node - if n = t.transport.codec.SessionNode(id, addr.String()); n == nil { - log.Error("Got a TALKREQ from a node that has not completed the handshake", "id", id, "addr", addr) - return + 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] diff --git a/p2p/discover/v5_udp.go b/p2p/discover/v5_udp.go index 893d00d470..677143fd60 100644 --- a/p2p/discover/v5_udp.go +++ b/p2p/discover/v5_udp.go @@ -67,7 +67,7 @@ type codecV5 interface { CurrentChallenge(id enode.ID, addr string) *v5wire.Whoareyou // SessionNode returns a node that has completed the handshake. - SessionNode(enode.ID, string) *enode.Node + SessionNode(id enode.ID, addr string) *enode.Node } // UDPv5 is the implementation of protocol version 5. @@ -941,7 +941,7 @@ func (t *UDPv5) handleWhoareyou(p *v5wire.Whoareyou, fromID enode.ID, fromAddr n c.err <- errors.New("remote wants handshake, but call has no ENR") return } - + // Resend the call that was answered by WHOAREYOU. t.log.Trace("<< "+p.Name(), "id", c.node.ID(), "addr", fromAddr) if _, ok := t.noRespCallByAuth[p.Nonce]; !ok { // Resend the call that was answered by WHOAREYOU. diff --git a/p2p/discover/v5_udp_test.go b/p2p/discover/v5_udp_test.go index 34450d803d..3a384aab12 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 773d2addd9..ec5ef8a261 100644 --- a/p2p/discover/v5wire/encoding.go +++ b/p2p/discover/v5wire/encoding.go @@ -189,8 +189,10 @@ func (c *Codec) Encode(id enode.ID, addr string, packet Packet, challenge *Whoar ) switch { case packet.Kind() == WhoareyouPacket: - if sentWhoareyou := c.sc.getHandshake(id, addr); sentWhoareyou != nil { - return sentWhoareyou.encoded, sentWhoareyou.Nonce, nil + // 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: @@ -218,25 +220,26 @@ func (c *Codec) Encode(id enode.ID, addr string, packet Packet, challenge *Whoar // Encode header data. c.writeHeaders(&head) - var enc []byte // Store sent WHOAREYOU challenges. if challenge, ok := packet.(*Whoareyou); ok { challenge.ChallengeData = bytesCopy(&c.buf) - enc, err = c.EncodeRaw(id, head, msgData) + enc, err := c.EncodeRaw(id, head, msgData) if err != nil { return nil, Nonce{}, err } - challenge.encoded = enc + challenge.Encoded = bytes.Clone(enc) c.sc.storeSentHandshake(id, addr, challenge) return enc, head.Nonce, err - } else if msgData == nil { + } + + 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) + enc, err := c.EncodeRaw(id, head, msgData) return enc, head.Nonce, err } @@ -260,12 +263,6 @@ func (c *Codec) CurrentChallenge(id enode.ID, addr string) *Whoareyou { return c.sc.getHandshake(id, addr) } -// SessionNode returns the node associated. -// This will return nil while a handshake is in progress. -func (c *Codec) SessionNode(id enode.ID, addr string) *enode.Node { - return c.sc.readNode(id, addr) -} - func (c *Codec) writeHeaders(head *Header) { c.buf.Reset() c.buf.Write(head.IV[:]) @@ -659,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/msg.go b/p2p/discover/v5wire/msg.go index ca8ddb77ba..089fd4ebdc 100644 --- a/p2p/discover/v5wire/msg.go +++ b/p2p/discover/v5wire/msg.go @@ -72,8 +72,10 @@ type ( // This must be set by the caller of Encode. Node *enode.Node - sent mclock.AbsTime // for handshake GC. - encoded []byte // Encoded packet raw data for sending out + 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. diff --git a/p2p/discover/v5wire/session.go b/p2p/discover/v5wire/session.go index 53a6947942..5a2166b143 100644 --- a/p2p/discover/v5wire/session.go +++ b/p2p/discover/v5wire/session.go @@ -114,7 +114,7 @@ func (sc *SessionCache) readNode(id enode.ID, addr string) *enode.Node { // storeNewSession stores new encryption keys in the cache. func (sc *SessionCache) storeNewSession(id enode.ID, addr string, s *session, n *enode.Node) { if n == nil { - panic("session must caches a non-nil node") + panic("nil node in storeNewSession") } s.node = n sc.sessions.Add(sessionID{id, addr}, s)