Fix login start packet decoding for 1.18.2 up to 1.20.2 (#421)

Signed-off-by: Fred Heinecke <fred.heinecke@yahoo.com>

Dockerfile

@@ -1,4 +1,4 @@
-FROM golang:1.23 AS builder
+FROM golang:1.24 AS builder
 
 WORKDIR /build
 

mcproto/decode.go

@@ -21,10 +21,11 @@ func DecodeHandshake(data interface{}) (*Handshake, error) {
 	buffer := bytes.NewBuffer(dataBytes)
 	var err error
 
-	handshake.ProtocolVersion, err = ReadVarInt(buffer)
+	protocolVersion, err := ReadVarInt(buffer)
 	if err != nil {
 		return nil, err
 	}
+	handshake.ProtocolVersion = ProtocolVersion(protocolVersion)
 
 	handshake.ServerAddress, err = ReadString(buffer)
 	if err != nil {
@@ -48,13 +49,13 @@ func DecodeHandshake(data interface{}) (*Handshake, error) {
 }
 
 // DecodeLoginStart takes the Packet.Data bytes and decodes a LoginStart message from it
-func DecodeLoginStart(data interface{}) (*LoginStart, error) {
+func DecodeLoginStart(protocolVersion ProtocolVersion, data interface{}) (*LoginStart, error) {
 	dataBytes, ok := data.([]byte)
 	if !ok {
 		return nil, errors.New(invalidPacketDataBytesMsg)
 	}
 
-	loginStart := &LoginStart{}
+	loginStart := NewLoginStart()
 	buffer := bytes.NewBuffer(dataBytes)
 	var err error
 
@@ -63,9 +64,69 @@ func DecodeLoginStart(data interface{}) (*LoginStart, error) {
 		return loginStart, errors.Wrap(err, "failed to read username")
 	}
 
-	loginStart.PlayerUuid, err = ReadUuid(buffer)
+	// These versions can send player keypair data. Ignore it.
-	if err != nil {
+	// References:
-		return loginStart, errors.Wrap(err, "failed to read player uuid")
+	// * https://github.com/MCCTeam/Minecraft-Console-Client/blob/f785f509f228bf787c237ac139e6f666a960819a/MinecraftClient/Protocol/Handlers/Protocol18.cs#L2808-L2828
+	// * https://minecraft.wiki/w/Minecraft_Wiki:Projects/wiki.vg_merge/Protocol?oldid=2772902#Login_Start
+	if protocolVersion >= ProtocolVersion1_19 && protocolVersion <= ProtocolVersion1_19_2 {
+		hasSignatureData, err := ReadBoolean(buffer)
+		if err != nil {
+			return loginStart, errors.Wrap(err, "failed to read has signature data flag")
+		}
+
+		if hasSignatureData {
+			// Read and discard the data
+			_, err = ReadLong(buffer) // Expiration time
+			if err != nil {
+				return loginStart, errors.Wrap(err, "failed to read expiration time")
+			}
+
+			pubKeyLength, err := ReadVarInt(buffer) // Length of the public key
+			if err != nil {
+				return loginStart, errors.Wrap(err, "failed to read public key length")
+			}
+
+			_, err = ReadByteArray(buffer, pubKeyLength) // Public key data
+			if err != nil {
+				return loginStart, errors.Wrap(err, "failed to read public key")
+			}
+
+			signatureLength, err := ReadVarInt(buffer) // Length of the signature
+			if err != nil {
+				return loginStart, errors.Wrap(err, "failed to read signature length")
+			}
+
+			_, err = ReadByteArray(buffer, signatureLength) // Signature data
+			if err != nil {
+				return loginStart, errors.Wrap(err, "failed to read signature")
+			}
+		}
+	}
+
+	// References:
+	// * https://github.com/MCCTeam/Minecraft-Console-Client/blob/f785f509f228bf787c237ac139e6f666a960819a/MinecraftClient/Protocol/Handlers/Protocol18.cs#L2831-L2853
+	// * https://minecraft.wiki/w/Minecraft_Wiki:Projects/wiki.vg_merge/Protocol?oldid=2772944#Login_Start
+	switch {
+	case protocolVersion >= ProtocolVersion1_19_2 && protocolVersion < ProtocolVersion1_20_2:
+		// Check to see if a UUID was provided at all
+		hasUUID, err := ReadBoolean(buffer)
+		if err != nil {
+			return loginStart, errors.Wrap(err, "failed to read has uuid flag")
+		}
+
+		if !hasUUID {
+			break
+		}
+		fallthrough
+	case protocolVersion >= ProtocolVersion1_20_2:
+		// For 1.20.2 and later, the UUID is always present
+		playerUuid, err := ReadUuid(buffer)
+		if err != nil {
+			return loginStart, errors.Wrap(err, "failed to read player uuid")
+		}
+		loginStart.PlayerUuid = playerUuid
+	default:
+		// For versions before 1.19.2, the UUID is not present
 	}
 
 	return loginStart, nil

mcproto/handshake-login-start-1.18.2.hex

@@ -0,0 +1,2 @@
+1000f605096c6f63616c686f737463dd02
+06000469747a67

mcproto/handshake-login-start-1.19.2-all-info.hex

@@ -0,0 +1,2 @@
+1000f805096c6f63616c686f737463dd02
+27000469747a670100fefdfc0102030402fbfa03f9f8f7015cddfd26fc864981b52ec42bb10bfdef

mcproto/handshake-login-start-1.19.2-min-info.hex

@@ -0,0 +1,2 @@
+1000f805096c6f63616c686f737463dd02
+08000469747a670000

mcproto/read.go

@@ -6,12 +6,13 @@ import (
 	"bufio"
 	"bytes"
 	"encoding/binary"
-	"github.com/google/uuid"
 	"io"
 	"net"
 	"strings"
 	"time"
 
+	"github.com/google/uuid"
+
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/text/encoding/unicode"
@@ -104,11 +105,17 @@ func ReadLegacyServerListPing(reader *bufio.Reader, addr net.Addr) (*Packet, err
 	}
 
 	messageName, err := ReadUTF16BEString(reader, messageNameShortLen)
+	if err != nil {
+		return nil, err
+	}
 	if messageName != "MC|PingHost" {
 		return nil, errors.Errorf("expected messageName=MC|PingHost, got %s", messageName)
 	}
 
 	remainingLen, err := ReadUnsignedShort(reader)
+	if err != nil {
+		return nil, err
+	}
 	remainingReader := io.LimitReader(reader, int64(remainingLen))
 
 	protocolVersion, err := ReadByte(remainingReader)
@@ -238,6 +245,21 @@ func ReadVarInt(reader io.Reader) (int, error) {
 	return 0, errors.New("VarInt is too big")
 }
 
+func ReadBoolean(reader io.Reader) (bool, error) {
+	byteVal, err := ReadByte(reader)
+	if err != nil {
+		return false, err
+	}
+	switch byteVal {
+	case 0x00:
+		return false, nil
+	case 0x01:
+		return true, nil
+	default:
+		return false, errors.Errorf("expected 0x00 or 0x01 for boolean, got 0x%02X", byteVal)
+	}
+}
+
 func ReadString(reader io.Reader) (string, error) {
 	length, err := ReadVarInt(reader)
 	if err != nil {
@@ -270,6 +292,19 @@ func ReadByte(reader io.Reader) (byte, error) {
 	}
 }
 
+func ReadByteArray(reader io.Reader, length int) ([]byte, error) {
+	if length < 0 {
+		return nil, errors.New("length cannot be negative")
+	}
+
+	data := make([]byte, length)
+	_, err := io.ReadFull(reader, data)
+	if err != nil {
+		return nil, err
+	}
+	return data, nil
+}
+
 func ReadUnsignedShort(reader io.Reader) (uint16, error) {
 	var value uint16
 	err := binary.Read(reader, binary.BigEndian, &value)
@@ -288,6 +323,15 @@ func ReadUnsignedInt(reader io.Reader) (uint32, error) {
 	return value, nil
 }
 
+func ReadLong(reader io.Reader) (int64, error) {
+	var value int64
+	err := binary.Read(reader, binary.BigEndian, &value)
+	if err != nil {
+		return 0, err
+	}
+	return value, nil
+}
+
 func ReadUuid(reader io.Reader) (uuid.UUID, error) {
 	uuidBytes := make([]byte, 16)
 	_, err := io.ReadFull(reader, uuidBytes)

mcproto/read_test.go

@@ -5,12 +5,13 @@ import (
 	"bytes"
 	"encoding/hex"
 	"fmt"
-	"github.com/google/uuid"
 	"os"
 	"strings"
 	"testing"
 	"unicode"
 
+	"github.com/google/uuid"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -57,35 +58,76 @@ func TestHandshakeThenStatus(t *testing.T) {
 
 	assert.Equal(t, "localhost", handshake.ServerAddress)
 	assert.Equal(t, uint16(25565), handshake.ServerPort)
-	assert.Equal(t, 770 /*for 1.21.5*/, handshake.ProtocolVersion)
+	assert.Equal(t, ProtocolVersion1_21_5, handshake.ProtocolVersion)
 	assert.Equal(t, StateStatus, handshake.NextState)
 }
 
-func TestHandshakeThenLoginStart(t *testing.T) {
+func TestHandshakeThenLoginStartVersion(t *testing.T) {
-	content, err := ReadHexDumpFile("handshake-login-start.hex")
+	playerUuid := uuid.MustParse("5cddfd26-fc86-4981-b52e-c42bb10bfdef")
-	require.NoError(t, err)
 
-	reader := bufio.NewReader(bytes.NewReader(content))
+	tests := []struct {
+		Name                    string
+		Filename                string
+		ExpectedProtocolVersion ProtocolVersion
+		ExpectedPlayerUuid      uuid.UUID
+	}{
+		{
+			Name:                    "1.20.2",
+			Filename:                "handshake-login-start-1.21.5.hex",
+			ExpectedProtocolVersion: ProtocolVersion1_21_5,
+			ExpectedPlayerUuid:      playerUuid,
+		},
+		// This version only conditionally provides a UUID, and may provide other information
+		// as well
+		{
+			Name:                    "1.19.2-all-info",
+			Filename:                "handshake-login-start-1.19.2-all-info.hex",
+			ExpectedProtocolVersion: ProtocolVersion1_19_2,
+			ExpectedPlayerUuid:      playerUuid,
+		},
+		{
+			Name:                    "1.19.2-min-info",
+			Filename:                "handshake-login-start-1.19.2-min-info.hex",
+			ExpectedProtocolVersion: ProtocolVersion1_19_2,
+			ExpectedPlayerUuid:      uuid.Nil, // No UUID provided in this case
+		},
+		// This is the last version that does not provide a UUID
+		{
+			Name:                    "1.18.2",
+			Filename:                "handshake-login-start-1.18.2.hex",
+			ExpectedProtocolVersion: ProtocolVersion1_18_2,
+			ExpectedPlayerUuid:      uuid.Nil, // No UUID provided by this version
+		},
+	}
 
-	handshakePacket, err := ReadPacket(reader, nil, StateHandshaking)
+	for _, tt := range tests {
-	require.NoError(t, err)
+		t.Run(tt.Name, func(t *testing.T) {
+			content, err := ReadHexDumpFile(tt.Filename)
+			require.NoError(t, err)
 
-	handshake, err := DecodeHandshake(handshakePacket.Data)
+			reader := bufio.NewReader(bytes.NewReader(content))
-	require.NoError(t, err)
 
-	assert.Equal(t, "localhost", handshake.ServerAddress)
+			handshakePacket, err := ReadPacket(reader, nil, StateHandshaking)
-	assert.Equal(t, uint16(25565), handshake.ServerPort)
+			require.NoError(t, err)
-	assert.Equal(t, 770 /*for 1.21.5*/, handshake.ProtocolVersion)
-	assert.Equal(t, StateLogin, handshake.NextState)
 
-	loginStartPacket, err := ReadPacket(reader, nil, StateLogin)
+			handshake, err := DecodeHandshake(handshakePacket.Data)
-	require.NoError(t, err)
+			require.NoError(t, err)
 
-	loginStart, err := DecodeLoginStart(loginStartPacket.Data)
+			assert.Equal(t, "localhost", handshake.ServerAddress)
-	require.NoError(t, err)
+			assert.Equal(t, uint16(25565), handshake.ServerPort)
+			assert.Equal(t, tt.ExpectedProtocolVersion, handshake.ProtocolVersion)
+			assert.Equal(t, StateLogin, handshake.NextState)
 
-	assert.Equal(t, "itzg", loginStart.Name)
+			loginStartPacket, err := ReadPacket(reader, nil, StateLogin)
-	assert.Equal(t, uuid.MustParse("5cddfd26-fc86-4981-b52e-c42bb10bfdef"), loginStart.PlayerUuid)
+			require.NoError(t, err)
+
+			loginStart, err := DecodeLoginStart(handshake.ProtocolVersion, loginStartPacket.Data)
+			require.NoError(t, err)
+
+			assert.Equal(t, "itzg", loginStart.Name)
+			assert.Equal(t, tt.ExpectedPlayerUuid, loginStart.PlayerUuid)
+		})
+	}
 }
 
 func ReadHexDumpFile(filename string) ([]byte, error) {

mcproto/types.go

@@ -2,6 +2,7 @@ package mcproto
 
 import (
 	"fmt"
+
 	"github.com/google/uuid"
 )
 
@@ -53,6 +54,27 @@ func (p *Packet) String() string {
 	}
 }
 
+type ProtocolVersion int
+
+// Source: https://minecraft.wiki/w/Minecraft_Wiki:Projects/wiki.vg_merge/Protocol_History
+const (
+	// ProtocolVersion1_18_2 is the protocol version for Minecraft 1.18.2
+	// Docs: https://minecraft.wiki/w/Java_Edition_protocol/Packets?oldid=2772791
+	ProtocolVersion1_18_2 ProtocolVersion = 758
+	// ProtocolVersion1_19 is the protocol version for Minecraft 1.19
+	// Docs: https://minecraft.wiki/w/Java_Edition_protocol/Packets?oldid=2772904
+	ProtocolVersion1_19 ProtocolVersion = 759
+	// ProtocolVersion1_19_2 is the protocol version for Minecraft 1.19.2
+	// Docs: https://minecraft.wiki/w/Java_Edition_protocol/Packets?oldid=2772944
+	ProtocolVersion1_19_2 ProtocolVersion = 760
+	// ProtocolVersion1_19_2 is the protocol version for Minecraft 1.19.3
+	ProtocolVersion1_19_3 ProtocolVersion = 761
+	// ProtocolVersion1_20_2 is the protocol version for Minecraft 1.20.2
+	ProtocolVersion1_20_2 ProtocolVersion = 764
+	// ProtocolVersion1_21_5 is the protocol version for Minecraft 1.21.5
+	ProtocolVersion1_21_5 ProtocolVersion = 770
+)
+
 const (
 	PacketIdHandshake            = 0x00
 	PacketIdLogin                = 0x00 // during StateLogin
@@ -60,7 +82,7 @@ const (
 )
 
 type Handshake struct {
-	ProtocolVersion int
+	ProtocolVersion ProtocolVersion
 	ServerAddress   string
 	ServerPort      uint16
 	NextState       State
@@ -71,6 +93,13 @@ type LoginStart struct {
 	PlayerUuid uuid.UUID
 }
 
+func NewLoginStart() *LoginStart {
+	return &LoginStart{
+		// Note: This is indistinguishable between no UUID provided, and a provided UUID of all 0s
+		PlayerUuid: uuid.Nil,
+	}
+}
+
 type LegacyServerListPing struct {
 	ProtocolVersion int
 	ServerAddress   string

server/connector.go

@@ -6,13 +6,14 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"github.com/google/uuid"
 	"io"
 	"net"
 	"sync"
 	"sync/atomic"
 	"time"
 
+	"github.com/google/uuid"
+
 	"golang.ngrok.com/ngrok"
 	"golang.ngrok.com/ngrok/config"
 
@@ -65,6 +66,7 @@ func (p *PlayerInfo) String() string {
 	if p == nil {
 		return ""
 	}
+
 	return fmt.Sprintf("%s/%s", p.Name, p.Uuid)
 }
 
@@ -318,7 +320,7 @@ func (c *Connector) HandleConnection(ctx context.Context, frontendConn net.Conn)
 
 		var playerInfo *PlayerInfo = nil
 		if handshake.NextState == mcproto.StateLogin {
-			playerInfo, err = c.readPlayerInfo(bufferedReader, clientAddr, handshake.NextState)
+			playerInfo, err = c.readPlayerInfo(handshake.ProtocolVersion, bufferedReader, clientAddr, handshake.NextState)
 			if err != nil {
 				if errors.Is(err, io.EOF) {
 					logrus.
@@ -372,14 +374,14 @@ func (c *Connector) HandleConnection(ctx context.Context, frontendConn net.Conn)
 	}
 }
 
-func (c *Connector) readPlayerInfo(bufferedReader *bufio.Reader, clientAddr net.Addr, state mcproto.State) (*PlayerInfo, error) {
+func (c *Connector) readPlayerInfo(protocolVersion mcproto.ProtocolVersion, bufferedReader *bufio.Reader, clientAddr net.Addr, state mcproto.State) (*PlayerInfo, error) {
 	loginPacket, err := mcproto.ReadPacket(bufferedReader, clientAddr, state)
 	if err != nil {
 		return nil, fmt.Errorf("failed to read login packet: %w", err)
 	}
 
 	if loginPacket.PacketID == mcproto.PacketIdLogin {
-		loginStart, err := mcproto.DecodeLoginStart(loginPacket.Data)
+		loginStart, err := mcproto.DecodeLoginStart(protocolVersion, loginPacket.Data)
 		if err != nil {
 			return nil, fmt.Errorf("failed to decode login start: %w", err)
 		}