p2p/discover: use Whoareyou.ChallengeData instead of storing encoded packet

This changes the challenge resend logic again to use the existing ChallengeData field of
v5wire.Whoareyou instead of storing a second copy of the packet in Whoareyou.Encoded. It's
more correct this way since ChallengeData is supposed to be the data that is used by the
ID verification procedure.

p2p/discover/v5_udp_test.go

@@ -772,10 +772,10 @@ 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
+	if wp, ok := p.(*v5wire.Whoareyou); ok && len(wp.ChallengeData) > 0 {
+		// To match the behavior of v5wire.Codec, we return the cached encoding of
+		// WHOAREYOU challenges.
+		return wp.ChallengeData, wp.Nonce, nil
 	}
 
 	c.ctr++
@@ -790,7 +790,7 @@ func (c *testCodec) Encode(toID enode.ID, addr string, p v5wire.Packet, _ *v5wir
 	// Store recently sent challenges.
 	if w, ok := p.(*v5wire.Whoareyou); ok {
 		w.Nonce = authTag
-		w.Encoded = frame
+		w.ChallengeData = frame
 		if c.sentChallenges == nil {
 			c.sentChallenges = make(map[enode.ID]*v5wire.Whoareyou)
 		}
@@ -827,6 +827,7 @@ func (c *testCodec) decodeFrame(input []byte) (frame testCodecFrame, p v5wire.Pa
 	case v5wire.WhoareyouPacket:
 		dec := new(v5wire.Whoareyou)
 		err = rlp.DecodeBytes(frame.Packet, &dec)
+		dec.ChallengeData = bytes.Clone(input)
 		p = dec
 	default:
 		p, err = v5wire.DecodeMessage(frame.Ptype, frame.Packet)

p2p/discover/v5wire/encoding.go

@@ -189,10 +189,17 @@ 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
+		if len(w.ChallengeData) > 0 {
+			// This WHOAREYOU packet was encoded before, so it's a resend.
+			// The unmasked packet content is stored in w.ChallengeData.
+			// Just apply the masking again here to get back the complete encoding.
+			c.buf.Reset()
+			c.buf.Write(w.ChallengeData)
+			var iv [16]byte
+			copy(iv[:], w.ChallengeData)
+			enc := applyMasking(id, head.IV, c.buf.Bytes())
+			return enc, w.Nonce, nil
 		}
 		head, err = c.encodeWhoareyou(id, packet.(*Whoareyou))
 	case challenge != nil:
@@ -227,7 +234,6 @@ func (c *Codec) Encode(id enode.ID, addr string, packet Packet, challenge *Whoar
 		if err != nil {
 			return nil, Nonce{}, err
 		}
-		challenge.Encoded = bytes.Clone(enc)
 		c.sc.storeSentHandshake(id, addr, challenge)
 		return enc, head.Nonce, err
 	}
@@ -245,14 +251,10 @@ func (c *Codec) Encode(id enode.ID, addr string, packet Packet, challenge *Whoar
 
 // EncodeRaw encodes a packet with the given header.
 func (c *Codec) EncodeRaw(id enode.ID, head Header, msgdata []byte) ([]byte, error) {
+	// header
 	c.writeHeaders(&head)
-
-	// Apply masking.
-	masked := c.buf.Bytes()[sizeofMaskingIV:]
-	mask := head.mask(id)
-	mask.XORKeyStream(masked[:], masked[:])
-
-	// Write message data.
+	applyMasking(id, head.IV, c.buf.Bytes())
+	// message data
 	c.buf.Write(msgdata)
 	return c.buf.Bytes(), nil
 }
@@ -463,7 +465,7 @@ func (c *Codec) Decode(inputData []byte, addr string) (src enode.ID, n *enode.No
 	// Unmask the static header.
 	var head Header
 	copy(head.IV[:], input[:sizeofMaskingIV])
-	mask := head.mask(c.localnode.ID())
+	mask := createMask(c.localnode.ID(), head.IV)
 	staticHeader := input[sizeofMaskingIV:sizeofStaticPacketData]
 	mask.XORKeyStream(staticHeader, staticHeader)
 
@@ -679,12 +681,19 @@ func (h *StaticHeader) checkValid(packetLen int, protocolID [6]byte) error {
 }
 
 // mask returns a cipher for 'masking' / 'unmasking' packet headers.
-func (h *Header) mask(destID enode.ID) cipher.Stream {
+func createMask(destID enode.ID, iv [16]byte) cipher.Stream {
 	block, err := aes.NewCipher(destID[:16])
 	if err != nil {
 		panic("can't create cipher")
 	}
-	return cipher.NewCTR(block, h.IV[:])
+	return cipher.NewCTR(block, iv[:])
+}
+
+func applyMasking(destID enode.ID, iv [16]byte, packet []byte) []byte {
+	masked := packet[sizeofMaskingIV:]
+	mask := createMask(destID, iv)
+	mask.XORKeyStream(masked[:], masked[:])
+	return packet
 }
 
 func bytesCopy(r *bytes.Buffer) []byte {

p2p/discover/v5wire/encoding_test.go

@@ -269,6 +269,33 @@ func TestHandshake_BadHandshakeAttack(t *testing.T) {
 	net.nodeB.expectDecodeErr(t, errUnexpectedHandshake, findnode)
 }
 
+func TestEncodeWhoareyouResend(t *testing.T) {
+	t.Parallel()
+	net := newHandshakeTest()
+	defer net.close()
+
+	// A -> B   WHOAREYOU
+	challenge := &Whoareyou{
+		Nonce:     Nonce{1, 2, 3, 4},
+		IDNonce:   testIDnonce,
+		RecordSeq: 0,
+	}
+	whoareyou, _ := net.nodeA.encode(t, net.nodeB, challenge)
+	net.nodeB.expectDecode(t, WhoareyouPacket, whoareyou)
+
+	if len(challenge.ChallengeData) == 0 {
+		t.Fatal("ChallengeData not assigned by encode")
+	}
+
+	// A -> B   WHOAREYOU
+	// Send the same challenge again. This should produce exactly
+	// the same bytes as the first send.
+	whoareyou2, _ := net.nodeA.encode(t, net.nodeB, challenge)
+	if !bytes.Equal(whoareyou2, whoareyou) {
+		t.Fatal("re-encoded challenge not equal to first")
+	}
+}
+
 // This test checks some malformed packets.
 func TestDecodeErrorsV5(t *testing.T) {
 	t.Parallel()

p2p/discover/v5wire/msg.go

@@ -63,19 +63,20 @@ type (
 
 	// WHOAREYOU contains the handshake challenge.
 	Whoareyou struct {
-		ChallengeData []byte   // Encoded challenge
-		Nonce         Nonce    // Nonce of request packet
-		IDNonce       [16]byte // Identity proof data
-		RecordSeq     uint64   // ENR sequence number of recipient
+		Nonce     Nonce    // Nonce of request packet
+		IDNonce   [16]byte // Identity proof data
+		RecordSeq uint64   // ENR sequence number of recipient
 
 		// Node is the locally known node record of recipient.
 		// This must be set by the caller of Encode.
-		Node *enode.Node
+		Node *enode.Node `rlp:"-"`
+
+		// ChallengeData stores the unmasked encoding of the whole packet. This is the
+		// input data for verification. It is assigned by both Encode and Decode
+		// operations.
+		ChallengeData []byte `rlp:"-"`
 
 		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.