Fix deadlock or bad data read in CI connection when sending data burst

When reading from a TCP connection, a buffered reader
was created for each read.
As it turns out, it can happen that the buffer reads more than
needed and the next buffer is missing that data.
It instead reads the package size from the payload, which
results in random behavior (lock or incomplete packages).

Author: g3force

cmd/ssl-auto-ref-client/main.go

@@ -26,8 +26,9 @@ var clientIdentifier = flag.String("identifier", "test", "The identifier of the
 var privateKey *rsa.PrivateKey
 
 type Client struct {
-	conn  net.Conn
-	token string
+	conn   net.Conn
+	reader *bufio.Reader
+	token  string
 }
 
 func main() {
@@ -56,6 +57,7 @@ func main() {
 	log.Printf("Connected to game-controller at %v", *refBoxAddr)
 	c := Client{}
 	c.conn = conn
+	c.reader = bufio.NewReaderSize(conn, 1)
 
 	c.register()
 
@@ -103,7 +105,7 @@ func main() {
 
 func (c *Client) register() {
 	reply := rcon.ControllerToAutoRef{}
-	if err := sslconn.ReceiveMessage(c.conn, &reply); err != nil {
+	if err := sslconn.ReceiveMessage(c.reader, &reply); err != nil {
 		log.Fatal("Failed receiving controller reply: ", err)
 	}
 	if reply.GetControllerReply() == nil || reply.GetControllerReply().NextToken == nil {
@@ -122,7 +124,7 @@ func (c *Client) register() {
 	}
 	log.Print("Sent registration, waiting for reply")
 	reply = rcon.ControllerToAutoRef{}
-	if err := sslconn.ReceiveMessage(c.conn, &reply); err != nil {
+	if err := sslconn.ReceiveMessage(c.reader, &reply); err != nil {
 		log.Fatal("Failed receiving controller reply: ", err)
 	}
 	if reply.GetControllerReply().StatusCode == nil || *reply.GetControllerReply().StatusCode != rcon.ControllerReply_OK {
@@ -209,7 +211,7 @@ func (c *Client) sendRequest(request *rcon.AutoRefToController, doLog bool) {
 
 	logIf(doLog, "Waiting for reply...")
 	reply := rcon.ControllerToAutoRef{}
-	if err := sslconn.ReceiveMessage(c.conn, &reply); err != nil {
+	if err := sslconn.ReceiveMessage(c.reader, &reply); err != nil {
 		log.Fatal("Failed receiving controller reply: ", err)
 	}
 	logIf(doLog, "Received reply: ", reply)

cmd/ssl-ci-test-client/main.go

@@ -123,9 +123,10 @@ func send(conn net.Conn) {
 }
 
 func receive(conn net.Conn) {
+	reader := bufio.NewReaderSize(conn, 1)
 	for {
 		output := ci.CiOutput{}
-		if err := sslconn.ReceiveMessage(conn, &output); err != nil {
+		if err := sslconn.ReceiveMessage(reader, &output); err != nil {
 			log.Println("Could not receive message: ", err)
 		}
 		log.Println("RefereeMsg: ", output.RefereeMsg)

cmd/ssl-remote-control-client/main.go

@@ -27,8 +27,9 @@ var team = flag.String("team", "YELLOW", "The team to control, either YELLOW or
 var privateKey *rsa.PrivateKey
 
 type Client struct {
-	conn  net.Conn
-	token string
+	conn   net.Conn
+	reader *bufio.Reader
+	token  string
 }
 
 func main() {
@@ -57,6 +58,7 @@ func main() {
 	log.Printf("Connected to game-controller at %v", *refBoxAddr)
 	c := Client{}
 	c.conn = conn
+	c.reader = bufio.NewReaderSize(conn, 1)
 
 	c.register()
 
@@ -142,7 +144,7 @@ func main() {
 
 func (c *Client) register() {
 	reply := rcon.ControllerToRemoteControl{}
-	if err := sslconn.ReceiveMessage(c.conn, &reply); err != nil {
+	if err := sslconn.ReceiveMessage(c.reader, &reply); err != nil {
 		log.Fatal("Failed receiving controller reply: ", err)
 	}
 	if reply.GetControllerReply().NextToken == nil {
@@ -162,7 +164,7 @@ func (c *Client) register() {
 	}
 	log.Print("Sent registration, waiting for reply")
 	reply = rcon.ControllerToRemoteControl{}
-	if err := sslconn.ReceiveMessage(c.conn, &reply); err != nil {
+	if err := sslconn.ReceiveMessage(c.reader, &reply); err != nil {
 		log.Fatal("Failed receiving controller reply: ", err)
 	}
 	if reply.GetControllerReply().StatusCode == nil || *reply.GetControllerReply().StatusCode != rcon.ControllerReply_OK {
@@ -200,7 +202,7 @@ func (c *Client) sendRequest(request *rcon.RemoteControlToController) (accepted
 
 	log.Print("Waiting for reply...")
 	reply := rcon.ControllerToRemoteControl{}
-	if err := sslconn.ReceiveMessage(c.conn, &reply); err != nil {
+	if err := sslconn.ReceiveMessage(c.reader, &reply); err != nil {
 		log.Fatal("Failed receiving controller reply: ", err)
 	}
 	log.Print("Received reply: ", proto.MarshalTextString(&reply))

cmd/ssl-team-client/main.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"bufio"
 	"crypto/rsa"
 	"flag"
 	"github.com/RoboCup-SSL/ssl-game-controller/internal/app/rcon"
@@ -21,8 +22,9 @@ var teamName = flag.String("teamName", "Test Team", "The name of the team as it
 var privateKey *rsa.PrivateKey
 
 type Client struct {
-	conn  net.Conn
-	token string
+	conn   net.Conn
+	reader *bufio.Reader
+	token  string
 }
 
 func main() {
@@ -51,6 +53,7 @@ func main() {
 	log.Printf("Connected to game-controller at %v", *refBoxAddr)
 	c := Client{}
 	c.conn = conn
+	c.reader = bufio.NewReaderSize(conn, 1)
 
 	c.register()
 	for !c.sendDesiredKeeper(3) {
@@ -64,7 +67,7 @@ func main() {
 
 func (c *Client) register() {
 	reply := rcon.ControllerToTeam{}
-	if err := sslconn.ReceiveMessage(c.conn, &reply); err != nil {
+	if err := sslconn.ReceiveMessage(c.reader, &reply); err != nil {
 		log.Fatal("Failed receiving controller reply: ", err)
 	}
 	if reply.GetControllerReply().NextToken == nil {
@@ -83,7 +86,7 @@ func (c *Client) register() {
 	}
 	log.Print("Sent registration, waiting for reply")
 	reply = rcon.ControllerToTeam{}
-	if err := sslconn.ReceiveMessage(c.conn, &reply); err != nil {
+	if err := sslconn.ReceiveMessage(c.reader, &reply); err != nil {
 		log.Fatal("Failed receiving controller reply: ", err)
 	}
 	if reply.GetControllerReply().StatusCode == nil || *reply.GetControllerReply().StatusCode != rcon.ControllerReply_OK {
@@ -121,7 +124,7 @@ func (c *Client) sendRequest(request *rcon.TeamToController) (accepted bool) {
 
 	log.Print("Waiting for reply...")
 	reply := rcon.ControllerToTeam{}
-	if err := sslconn.ReceiveMessage(c.conn, &reply); err != nil {
+	if err := sslconn.ReceiveMessage(c.reader, &reply); err != nil {
 		log.Fatal("Failed receiving controller reply: ", err)
 	}
 	log.Print("Received reply: ", proto.MarshalTextString(&reply))

internal/app/ci/ci.go

@@ -1,6 +1,7 @@
 package ci
 
 import (
+	"bufio"
 	"github.com/RoboCup-SSL/ssl-game-controller/internal/app/engine"
 	"github.com/RoboCup-SSL/ssl-game-controller/internal/app/sslconn"
 	"github.com/RoboCup-SSL/ssl-game-controller/internal/app/state"
@@ -74,9 +75,10 @@ func (s *Server) serve(conn net.Conn) {
 		}
 	}()
 
+	reader := bufio.NewReaderSize(conn, 1)
 	for {
 		input := CiInput{}
-		if data, err := sslconn.Receive(conn); err != nil {
+		if data, err := sslconn.Receive(reader); err != nil {
 			log.Println("Could not receive message from CI connection: ", err)
 			return
 		} else if err := sslconn.Unmarshal(data, &input); err != nil {

internal/app/rcon/server_autoref.go

@@ -1,6 +1,7 @@
 package rcon
 
 import (
+	"bufio"
 	"github.com/RoboCup-SSL/ssl-game-controller/internal/app/engine"
 	"github.com/RoboCup-SSL/ssl-game-controller/internal/app/sslconn"
 	"github.com/RoboCup-SSL/ssl-game-controller/internal/app/state"
@@ -28,9 +29,9 @@ func NewAutoRefServer(address string, gcEngine *engine.Engine) (s *AutoRefServer
 	return
 }
 
-func (c *AutoRefClient) receiveRegistration(server *AutoRefServer) error {
+func (c *AutoRefClient) receiveRegistration(reader *bufio.Reader, server *AutoRefServer) error {
 	registration := AutoRefRegistration{}
-	if err := sslconn.ReceiveMessage(c.conn, &registration); err != nil {
+	if err := sslconn.ReceiveMessage(reader, &registration); err != nil {
 		return err
 	}
 
@@ -66,10 +67,12 @@ func (s *AutoRefServer) handleClientConnection(conn net.Conn) {
 		}
 	}()
 
+	reader := bufio.NewReaderSize(conn, 1)
+
 	client := AutoRefClient{Client: &Client{conn: conn, token: uuid.New()}}
 	client.reply(client.Ok())
 
-	err := client.receiveRegistration(s)
+	err := client.receiveRegistration(reader, s)
 	if err != nil {
 		client.reply(client.Reject(err.Error()))
 		return
@@ -102,7 +105,7 @@ func (s *AutoRefServer) handleClientConnection(conn net.Conn) {
 
 	for {
 		req := AutoRefToController{}
-		if err := sslconn.ReceiveMessage(conn, &req); err != nil {
+		if err := sslconn.ReceiveMessage(reader, &req); err != nil {
 			if err == io.EOF {
 				return
 			}

internal/app/rcon/server_remotecontrol.go

@@ -1,6 +1,7 @@
 package rcon
 
 import (
+	"bufio"
 	"github.com/RoboCup-SSL/ssl-game-controller/internal/app/engine"
 	"github.com/RoboCup-SSL/ssl-game-controller/internal/app/sslconn"
 	"github.com/RoboCup-SSL/ssl-game-controller/internal/app/state"
@@ -33,9 +34,9 @@ func NewRemoteControlServer(address string, gcEngine *engine.Engine) (s *RemoteC
 	return
 }
 
-func (c *RemoteControlClient) receiveRegistration(server *RemoteControlServer) error {
+func (c *RemoteControlClient) receiveRegistration(reader *bufio.Reader, server *RemoteControlServer) error {
 	registration := RemoteControlRegistration{}
-	if err := sslconn.ReceiveMessage(c.conn, &registration); err != nil {
+	if err := sslconn.ReceiveMessage(reader, &registration); err != nil {
 		return err
 	}
 
@@ -73,13 +74,15 @@ func (s *RemoteControlServer) handleClientConnection(conn net.Conn) {
 		}
 	}()
 
+	reader := bufio.NewReaderSize(conn, 1)
+
 	client := RemoteControlClient{
 		Client:   &Client{conn: conn, token: uuid.New()},
 		gcEngine: s.gcEngine,
 	}
 	client.reply(client.Ok())
 
-	err := client.receiveRegistration(s)
+	err := client.receiveRegistration(reader, s)
 	if err != nil {
 		client.reply(client.Reject(err.Error()))
 		return
@@ -96,7 +99,7 @@ func (s *RemoteControlServer) handleClientConnection(conn net.Conn) {
 
 	for {
 		req := RemoteControlToController{}
-		if err := sslconn.ReceiveMessage(conn, &req); err != nil {
+		if err := sslconn.ReceiveMessage(reader, &req); err != nil {
 			if err == io.EOF {
 				return
 			}

internal/app/rcon/server_team.go

@@ -1,6 +1,7 @@
 package rcon
 
 import (
+	"bufio"
 	"github.com/RoboCup-SSL/ssl-game-controller/internal/app/engine"
 	"github.com/RoboCup-SSL/ssl-game-controller/internal/app/sslconn"
 	"github.com/RoboCup-SSL/ssl-game-controller/internal/app/state"
@@ -30,9 +31,9 @@ func NewTeamServer(address string, gcEngine *engine.Engine) (s *TeamServer) {
 	return
 }
 
-func (c *TeamClient) receiveRegistration(server *TeamServer) error {
+func (c *TeamClient) receiveRegistration(reader *bufio.Reader, server *TeamServer) error {
 	registration := TeamRegistration{}
-	if err := sslconn.ReceiveMessage(c.conn, &registration); err != nil {
+	if err := sslconn.ReceiveMessage(reader, &registration); err != nil {
 		return err
 	}
 
@@ -83,10 +84,12 @@ func (s *TeamServer) handleClientConnection(conn net.Conn) {
 		}
 	}()
 
+	reader := bufio.NewReaderSize(conn, 1)
+
 	client := TeamClient{Client: &Client{conn: conn, token: uuid.New()}}
 	client.reply(client.Ok())
 
-	err := client.receiveRegistration(s)
+	err := client.receiveRegistration(reader, s)
 	if err != nil {
 		client.reply(client.Reject(err.Error()))
 		return
@@ -117,7 +120,7 @@ func (s *TeamServer) handleClientConnection(conn net.Conn) {
 
 	for {
 		req := TeamToController{}
-		if err := sslconn.ReceiveMessage(conn, &req); err != nil {
+		if err := sslconn.ReceiveMessage(reader, &req); err != nil {
 			if err == io.EOF {
 				return
 			}

internal/app/sslconn/sslconn.go

@@ -39,9 +39,8 @@ func SendMessage(conn net.Conn, message proto.Message) error {
 }
 
 // Receive reads data and the preceding size from the given connection
-func Receive(conn net.Conn) ([]byte, error) {
+func Receive(reader *bufio.Reader) ([]byte, error) {
 
-	reader := bufio.NewReaderSize(conn, 1)
 	dataLength, err := readDataLength(reader)
 	if err != nil {
 		return nil, err
@@ -65,9 +64,9 @@ func Unmarshal(data []byte, message proto.Message) error {
 }
 
 // ReceiveMessage reads a protobuf message and the preceding size from the given connection
-func ReceiveMessage(conn net.Conn, message proto.Message) error {
+func ReceiveMessage(reader *bufio.Reader, message proto.Message) error {
 
-	data, err := Receive(conn)
+	data, err := Receive(reader)
 	if err != nil {
 		return err
 	}