cmd/devp2p: fix disconnect decoding in rlpx ping (#34781)
Some checks are pending
/ Linux Build (arm) (push) Waiting to run
/ Keeper Build (push) Waiting to run
/ Windows Build (push) Waiting to run
/ Linux Build (push) Waiting to run
/ Docker Image (push) Waiting to run

The rlpx ping command mishandled disconnect responses on two counts:
the error return from rlp.DecodeBytes was ignored, so decode failures
silently produced an "invalid disconnect message" error with no context;
and the decoder assumed the spec-compliant list form exclusively, while
older geth and some other implementations send the reason as a bare
byte.
                                                                  
Accept both wire forms (matching the legacy-tolerant behavior already
  in p2p.decodeDisconnectMessage), and on decode failure include the raw
payload so operators can see exactly what the peer sent. Add a unit
  test for the decoder covering both forms plus the empty-payload error
  path.
This commit is contained in:
Matus Kysel 2026-04-22 16:48:38 +02:00 committed by GitHub
parent b0ead5e17b
commit 8e2107dc39
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 109 additions and 4 deletions

View file

@ -17,6 +17,7 @@
package main
import (
"bytes"
"errors"
"fmt"
"net"
@ -30,6 +31,31 @@ import (
"github.com/urfave/cli/v2"
)
// decodeRLPxDisconnect parses a disconnect message payload. Per the RLPx spec
// the payload is a list containing a single reason, but some implementations
// (including older geth) sent the reason as a bare byte. Accept both forms.
func decodeRLPxDisconnect(data []byte) (p2p.DiscReason, error) {
s := rlp.NewStream(bytes.NewReader(data), uint64(len(data)))
k, _, err := s.Kind()
if err != nil {
return 0, err
}
var reason p2p.DiscReason
if k == rlp.List {
if _, err := s.List(); err != nil {
return 0, err
}
if err := s.Decode(&reason); err != nil {
return 0, err
}
return reason, nil
}
if err := s.Decode(&reason); err != nil {
return 0, err
}
return reason, nil
}
var (
rlpxCommand = &cli.Command{
Name: "rlpx",
@ -103,11 +129,15 @@ func rlpxPing(ctx *cli.Context) error {
}
fmt.Printf("%+v\n", h)
case 1:
var msg []p2p.DiscReason
if rlp.DecodeBytes(data, &msg); len(msg) == 0 {
return errors.New("invalid disconnect message")
// The disconnect message is specified as a list containing the reason,
// but some implementations (including older geth) send the reason as a
// single byte. Handle both forms, and on failure include the raw payload
// so the operator can see what was actually sent.
reason, decErr := decodeRLPxDisconnect(data)
if decErr != nil {
return fmt.Errorf("invalid disconnect message: %v (raw=0x%x)", decErr, data)
}
return fmt.Errorf("received disconnect message: %v", msg[0])
return fmt.Errorf("received disconnect message: %v", reason)
default:
return fmt.Errorf("invalid message code %d, expected handshake (code zero) or disconnect (code one)", code)
}

View file

@ -0,0 +1,75 @@
// Copyright 2026 The go-ethereum Authors
// This file is part of go-ethereum.
//
// go-ethereum is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// go-ethereum is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with go-ethereum. If not, see <http://www.gnu.org/licenses/>.
package main
import (
"testing"
"github.com/ethereum/go-ethereum/p2p"
)
func TestDecodeRLPxDisconnect(t *testing.T) {
tests := []struct {
name string
payload []byte
want p2p.DiscReason
wantErr bool
}{
{
name: "list form (spec-compliant)",
payload: []byte{0xc1, 0x04}, // [4] = TooManyPeers
want: p2p.DiscTooManyPeers,
},
{
name: "list form with reason zero",
payload: []byte{0xc1, 0x80}, // [0] = Requested
want: p2p.DiscRequested,
},
{
name: "bare byte form (legacy geth)",
payload: []byte{0x04}, // 4 = TooManyPeers
want: p2p.DiscTooManyPeers,
},
{
name: "bare byte form zero",
payload: []byte{0x80}, // 0 = Requested
want: p2p.DiscRequested,
},
{
name: "empty payload",
payload: []byte{},
wantErr: true,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got, err := decodeRLPxDisconnect(tc.payload)
if tc.wantErr {
if err == nil {
t.Fatalf("expected error, got reason=%v", got)
}
return
}
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if got != tc.want {
t.Fatalf("got reason %v, want %v", got, tc.want)
}
})
}
}