Skip to content

Commit 0caba07

Browse files
committed
feat: harden login plugin-message and keep-alive handling
- version: the Minecraft 26.1 entry now lists every protocol-775 release name (26.1, 26.1.1, 26.1.2). - plugin message: reject serverbound payloads larger than the vanilla 32767-byte limit (clientbound unchanged) to prevent client abuse. - compression: cap a serverbound packet's claimed decompressed size at 2 MiB (clientbound keeps the 8 MiB vanilla cap). Serverbound data is untrusted, so this limits how much memory a client can force the proxy to allocate. - client play: bound the pre-join loginPluginMessages queue by message count (1024) and total bytes (4 MiB), disconnecting on overflow. A client that never completes its login/FML phase could otherwise grow the queue without limit. - keep-alive: only forward a keep-alive to the backend when it is open and in the same protocol state as the client (CONFIG/PLAY), and always consume the pending ping. Prevents mis-encoding a PLAY keep-alive to a backend still in CONFIG during a server switch. The serverbound bounds and keep-alive state guard adopt the approach Velocity uses for the same paths. Each behavioral change has a focused unit test.
1 parent fd083f6 commit 0caba07

9 files changed

Lines changed: 375 additions & 25 deletions

File tree

pkg/edition/java/proto/codec/decoder.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,15 @@ func (d *Decoder) decompress(claimedUncompressedSize int, rd io.Reader) (decompr
190190
return nil, errs.NewSilentErr("uncompressed size %d is less than set threshold %d",
191191
claimedUncompressedSize, d.compressionThreshold)
192192
}
193-
if claimedUncompressedSize > UncompressedCap {
193+
// Serverbound (client->proxy) data is untrusted, so cap it tighter than
194+
// clientbound (backend->proxy) data.
195+
maxSize := UncompressedCap
196+
if d.direction == proto.ServerBound {
197+
maxSize = ServerboundUncompressedCap
198+
}
199+
if claimedUncompressedSize > maxSize {
194200
return nil, errs.NewSilentErr("uncompressed size %d exceeds hard threshold of %d",
195-
claimedUncompressedSize, UncompressedCap)
201+
claimedUncompressedSize, maxSize)
196202
}
197203

198204
if d.zrd == nil {
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package codec
2+
3+
import (
4+
"bytes"
5+
"testing"
6+
7+
"github.com/go-logr/logr"
8+
"github.com/stretchr/testify/require"
9+
10+
"go.minekube.com/gate/pkg/gate/proto"
11+
)
12+
13+
// A serverbound packet must not be allowed to claim a decompressed size larger
14+
// than the serverbound cap, even when it is still within the larger clientbound
15+
// cap. This limits how much memory a client can force the proxy to allocate,
16+
// while trusted clientbound (backend->proxy) data keeps the higher limit.
17+
func TestDecompressServerboundCapTighterThanClientbound(t *testing.T) {
18+
claimed := ServerboundUncompressedCap + 1 // over serverbound, under clientbound
19+
require.Less(t, claimed, UncompressedCap, "test assumes serverbound cap < clientbound cap")
20+
21+
sb := NewDecoder(bytes.NewReader(nil), proto.ServerBound, logr.Discard())
22+
sb.SetCompressionThreshold(256)
23+
_, err := sb.decompress(claimed, bytes.NewReader(nil))
24+
require.Error(t, err, "serverbound decompress should reject a claim over the serverbound cap")
25+
require.Contains(t, err.Error(), "exceeds")
26+
27+
cb := NewDecoder(bytes.NewReader(nil), proto.ClientBound, logr.Discard())
28+
cb.SetCompressionThreshold(256)
29+
_, err = cb.decompress(claimed, bytes.NewReader(nil))
30+
// The claim is under the clientbound cap, so it passes the size check (and
31+
// then fails on the empty zlib stream — a different error, not the cap).
32+
if err != nil {
33+
require.NotContains(t, err.Error(), "exceeds")
34+
}
35+
}

pkg/edition/java/proto/codec/encoder.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ const (
2121
VanillaMaximumUncompressedSize = 8 * 1024 * 1024 // 8MiB
2222
HardMaximumUncompressedSize = 128 * 1024 * 1024 // 128MiB
2323
UncompressedCap = VanillaMaximumUncompressedSize
24+
// ServerboundUncompressedCap is the maximum claimed decompressed size for a
25+
// serverbound packet. It is tighter than UncompressedCap because serverbound
26+
// data comes from the (untrusted) client, limiting how much memory a client
27+
// can force the proxy to allocate. Clientbound data keeps UncompressedCap.
28+
ServerboundUncompressedCap = 2 * 1024 * 1024 // 2MiB
2429
)
2530

2631
// Encoder is a synchronized packet encoder.

pkg/edition/java/proto/packet/plugin/message.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
11
package plugin
22

33
import (
4+
"fmt"
45
"io"
56

67
"go.minekube.com/gate/pkg/edition/java/proto/util"
78
"go.minekube.com/gate/pkg/edition/java/proto/version"
89
"go.minekube.com/gate/pkg/gate/proto"
910
)
1011

12+
// MaxServerboundPayloadSize is the maximum size (in bytes) of a serverbound
13+
// plugin message payload. This matches the vanilla client limit; rejecting
14+
// larger serverbound payloads prevents a client from abusing plugin messages.
15+
// Clientbound payloads (proxy<-backend) are not subject to this limit.
16+
const MaxServerboundPayloadSize = 32767
17+
1118
// Message is a Minecraft plugin message packet.
1219
type Message struct {
1320
Channel string
@@ -40,7 +47,17 @@ func (p *Message) Decode(c *proto.PacketContext, r io.Reader) (err error) {
4047
p.Channel = TransformLegacyToModernChannel(p.Channel)
4148
}
4249
if c.Protocol.GreaterEqual(version.Minecraft_1_8) {
43-
p.Data, err = io.ReadAll(r)
50+
if c.Direction == proto.ServerBound {
51+
// Reject serverbound payloads larger than the vanilla limit. Read one
52+
// byte past the limit so we can detect (rather than silently truncate).
53+
p.Data, err = io.ReadAll(io.LimitReader(r, MaxServerboundPayloadSize+1))
54+
if err == nil && len(p.Data) > MaxServerboundPayloadSize {
55+
return fmt.Errorf("serverbound plugin message payload too large: %d > %d bytes",
56+
len(p.Data), MaxServerboundPayloadSize)
57+
}
58+
} else {
59+
p.Data, err = io.ReadAll(r)
60+
}
4461
} else {
4562
p.Data, err = util.ReadBytes17(r)
4663
}
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package plugin
2+
3+
import (
4+
"bytes"
5+
"testing"
6+
7+
"go.minekube.com/gate/pkg/edition/java/proto/util"
8+
"go.minekube.com/gate/pkg/edition/java/proto/version"
9+
"go.minekube.com/gate/pkg/gate/proto"
10+
)
11+
12+
// buildModernMessage encodes a 1.8+ plugin message frame (channel + raw data).
13+
func buildModernMessage(t *testing.T, channel string, dataLen int) *bytes.Buffer {
14+
t.Helper()
15+
buf := new(bytes.Buffer)
16+
if err := util.WriteString(buf, channel); err != nil {
17+
t.Fatal(err)
18+
}
19+
buf.Write(bytes.Repeat([]byte{0x7f}, dataLen))
20+
return buf
21+
}
22+
23+
func decodeMessage(t *testing.T, dir proto.Direction, data *bytes.Buffer) error {
24+
t.Helper()
25+
var m Message
26+
ctx := &proto.PacketContext{Direction: dir, Protocol: version.Minecraft_1_21.Protocol}
27+
return m.Decode(ctx, data)
28+
}
29+
30+
// A serverbound plugin message larger than the vanilla 32767-byte limit must be
31+
// rejected so a client cannot abuse it.
32+
func TestDecodeServerboundPluginMessageRejectsOversized(t *testing.T) {
33+
buf := buildModernMessage(t, "minecraft:brand", 32768)
34+
if err := decodeMessage(t, proto.ServerBound, buf); err == nil {
35+
t.Fatal("expected error decoding oversized serverbound plugin message, got nil")
36+
}
37+
}
38+
39+
// A serverbound plugin message at or below the limit must decode fine.
40+
func TestDecodeServerboundPluginMessageAllowsWithinLimit(t *testing.T) {
41+
buf := buildModernMessage(t, "minecraft:brand", 32767)
42+
if err := decodeMessage(t, proto.ServerBound, buf); err != nil {
43+
t.Fatalf("unexpected error decoding within-limit serverbound plugin message: %v", err)
44+
}
45+
}
46+
47+
// Clientbound messages (proxy<-backend) may legitimately be large and must not
48+
// be subject to the serverbound limit.
49+
func TestDecodeClientboundPluginMessageAllowsLarge(t *testing.T) {
50+
buf := buildModernMessage(t, "minecraft:brand", 100000)
51+
if err := decodeMessage(t, proto.ClientBound, buf); err != nil {
52+
t.Fatalf("unexpected error decoding large clientbound plugin message: %v", err)
53+
}
54+
}

pkg/edition/java/proto/version/version.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ var (
5252
Minecraft_1_21_7 = v(772, "1.21.7", "1.21.8")
5353
Minecraft_1_21_9 = v(773, "1.21.9", "1.21.10")
5454
Minecraft_1_21_11 = v(774, "1.21.11")
55-
Minecraft_26_1 = v(775, "26.1")
55+
Minecraft_26_1 = v(775, "26.1", "26.1.1", "26.1.2")
5656

5757
// Versions ordered from lowest to highest
5858
Versions = []*proto.Version{

pkg/edition/java/proxy/session_client_play.go

Lines changed: 91 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,70 @@ type clientPlaySessionHandler struct {
5454

5555
mu struct {
5656
sync.RWMutex
57-
loginPluginMessages deque.Deque[*plugin.Message]
58-
serverBossBars map[uuid.UUID]struct{}
57+
loginPluginMessages deque.Deque[*plugin.Message]
58+
loginPluginMessagesBytes int
59+
loginPluginMessagesOverflowed bool
60+
serverBossBars map[uuid.UUID]struct{}
5961
}
6062
}
6163

64+
// Caps on the pre-join plugin-message queue (loginPluginMessages). A client that
65+
// never completes its login/FML handshake phase could otherwise queue plugin
66+
// messages without bound. The count/byte limits mirror Velocity's.
67+
const (
68+
maxQueuedLoginPluginMessages = 1024
69+
maxQueuedLoginPluginMessageBytes = 4 * 1024 * 1024 // 4 MiB
70+
)
71+
72+
// enqueueLoginPluginMessage queues a plugin message received before the player
73+
// has joined a server, enforcing per-connection count and byte caps. It returns
74+
// true if the message was queued, or false if a cap was exceeded — in which case
75+
// the queue is dropped, the player disconnected, and the overflow latched so
76+
// subsequent messages are rejected without further work.
77+
func (c *clientPlaySessionHandler) enqueueLoginPluginMessage(msg *plugin.Message) bool {
78+
c.mu.Lock()
79+
if c.mu.loginPluginMessagesOverflowed {
80+
c.mu.Unlock()
81+
return false
82+
}
83+
newBytes := c.mu.loginPluginMessagesBytes + len(msg.Data)
84+
newCount := c.mu.loginPluginMessages.Len() + 1
85+
if newBytes > maxQueuedLoginPluginMessageBytes || newCount > maxQueuedLoginPluginMessages {
86+
c.mu.loginPluginMessagesOverflowed = true
87+
c.mu.loginPluginMessages.Clear()
88+
c.mu.loginPluginMessagesBytes = 0
89+
c.mu.Unlock()
90+
c.log.Info("disconnecting player: pre-join plugin message queue exceeded its limits",
91+
"messages", newCount, "bytes", newBytes)
92+
c.player.Disconnect(&component.Text{
93+
Content: "Too many plugin messages were sent before joining a server",
94+
})
95+
return false
96+
}
97+
c.mu.loginPluginMessages.PushBack(msg)
98+
c.mu.loginPluginMessagesBytes = newBytes
99+
c.mu.Unlock()
100+
return true
101+
}
102+
103+
// drainQueuedLoginPluginMessages removes and returns all queued pre-join plugin
104+
// messages, resetting the byte counter so the queue caps start fresh. Returns
105+
// nil if the queue is empty.
106+
func (c *clientPlaySessionHandler) drainQueuedLoginPluginMessages() []*plugin.Message {
107+
c.mu.Lock()
108+
defer c.mu.Unlock()
109+
c.mu.loginPluginMessagesBytes = 0
110+
n := c.mu.loginPluginMessages.Len()
111+
if n == 0 {
112+
return nil
113+
}
114+
msgs := make([]*plugin.Message, 0, n)
115+
for c.mu.loginPluginMessages.Len() != 0 {
116+
msgs = append(msgs, c.mu.loginPluginMessages.PopFront())
117+
}
118+
return msgs
119+
}
120+
62121
func newClientPlaySessionHandler(player *connectedPlayer) *clientPlaySessionHandler {
63122
log := player.log.WithName("clientPlaySession")
64123
h := &clientPlaySessionHandler{
@@ -128,6 +187,8 @@ func (c *clientPlaySessionHandler) Deactivated() {
128187
c.mu.Lock()
129188
c.player.discardChatQueue()
130189
c.mu.loginPluginMessages.Clear()
190+
c.mu.loginPluginMessagesBytes = 0
191+
c.mu.loginPluginMessagesOverflowed = false
131192
c.mu.Unlock()
132193
}
133194

@@ -177,10 +238,7 @@ func (c *clientPlaySessionHandler) FlushQueuedPluginMessages() {
177238
if !ok {
178239
return
179240
}
180-
c.mu.Lock()
181-
defer c.mu.Unlock()
182-
for c.mu.loginPluginMessages.Len() != 0 {
183-
pm := c.mu.loginPluginMessages.PopFront()
241+
for _, pm := range c.drainQueuedLoginPluginMessages() {
184242
_ = serverMc.BufferPacket(pm)
185243
}
186244
_ = serverMc.Flush()
@@ -232,16 +290,30 @@ func forwardKeepAlive(p *packet.KeepAlive, player *connectedPlayer) {
232290
}
233291

234292
func sendKeepAliveToBackend(serverConn *serverConnection, player *connectedPlayer, p *packet.KeepAlive) bool {
235-
if serverConn != nil {
236-
if sentTime, ok := serverConn.pendingPings.Get(p.RandomID); ok {
237-
serverConn.pendingPings.Delete(p.RandomID)
238-
if serverMc := serverConn.conn(); serverMc != nil {
239-
player.ping.Store(time.Since(sentTime))
240-
return serverMc.WritePacket(p) == nil
241-
}
242-
}
293+
if serverConn == nil {
294+
return false
243295
}
244-
return false
296+
sentTime, ok := serverConn.pendingPings.Get(p.RandomID)
297+
if !ok {
298+
return false
299+
}
300+
// We removed this pending ping, so it is ours: consume it regardless of
301+
// whether it can be forwarded, so it is not re-dispatched to another
302+
// connection.
303+
serverConn.pendingPings.Delete(p.RandomID)
304+
305+
// Only forward to the backend when it is open and in the same protocol state
306+
// as the client. During a server switch the backend may be in CONFIG while
307+
// the client is still in PLAY; forwarding then would mis-encode the packet.
308+
serverMc := serverConn.conn()
309+
clientState := player.State()
310+
if serverMc != nil && !netmc.Closed(serverMc) &&
311+
clientState == serverMc.State() &&
312+
(clientState == state.Config || clientState == state.Play) {
313+
player.ping.Store(time.Since(sentTime))
314+
_ = serverMc.WritePacket(p)
315+
}
316+
return true
245317
}
246318

247319
func (c *clientPlaySessionHandler) handlePluginMessage(packet *plugin.Message) {
@@ -334,10 +406,9 @@ func (c *clientPlaySessionHandler) handlePluginMessage(packet *plugin.Message) {
334406
// JoinGame packet has been received by the proxy, whichever comes first.
335407
//
336408
// We also need to make sure to retain these packets, so they can be flushed
337-
// appropriately.
338-
c.mu.Lock()
339-
c.mu.loginPluginMessages.PushBack(packet)
340-
c.mu.Unlock()
409+
// appropriately. The queue is bounded to prevent a client that never
410+
// completes its handshake phase from growing it without limit.
411+
c.enqueueLoginPluginMessage(packet)
341412
}
342413
}
343414

@@ -483,8 +554,7 @@ func (c *clientPlaySessionHandler) handleBackendJoinGame(pc *proto.PacketContext
483554
}
484555

485556
// If we had plugin messages queued during login/FML handshake, send them now.
486-
for c.mu.loginPluginMessages.Len() != 0 {
487-
pm := c.mu.loginPluginMessages.PopFront()
557+
for _, pm := range c.drainQueuedLoginPluginMessages() {
488558
if err = serverMc.BufferPacket(pm); err != nil {
489559
return fmt.Errorf("error buffering %T for backend: %w", pm, err)
490560
}

0 commit comments

Comments
 (0)