From 0808a665bc75ab0845906bfeca0d12fb520ae6eb Mon Sep 17 00:00:00 2001
From: Alexander Potapenko <glider@google.com>
Date: Thu, 20 Feb 2025 12:25:04 +0100
Subject: pkg/rpcserver: pkg/flatrpc: executor: add handshake stage 0

As we figured out in #5805, syz-manager treats random incoming RPC
connections as trusted, and will crash if a non-executor client sends
an invalid packet to it.

To address this issue, we introduce another stage of handshake, which
includes a cookie exchange:
 - upon connection from an executor, the manager sends a ConnectHello RPC
   message to it, which contains a random 64-bit cookie;
 - the executor calculates a hash of that cookie and includes it into
   its ConnectRequest together with the other information;
 - before checking the validity of ConnectRequest, the manager ensures
   client sanity (passed ID didn't change, hashed cookie has the expected
   value)

We deliberately pick a random cookie instead of a magic number: if the
fuzzer somehow learns to send packets to the manager, we don't want it to
crash multiple managers on the same machine.
---
 pkg/flatrpc/conn_test.go | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

(limited to 'pkg/flatrpc/conn_test.go')

diff --git a/pkg/flatrpc/conn_test.go b/pkg/flatrpc/conn_test.go
index 4b108a5a4..87d5e9a8a 100644
--- a/pkg/flatrpc/conn_test.go
+++ b/pkg/flatrpc/conn_test.go
@@ -20,7 +20,11 @@ import (
 )
 
 func TestConn(t *testing.T) {
+	connectHello := &ConnectHello{
+		Cookie: 1,
+	}
 	connectReq := &ConnectRequest{
+		Cookie:      73856093,
 		Id:          1,
 		Arch:        "arch",
 		GitRevision: "rev1",
@@ -52,6 +56,9 @@ func TestConn(t *testing.T) {
 	go func() {
 		done <- serv.Serve(context.Background(),
 			func(_ context.Context, c *Conn) error {
+				if err := Send(c, connectHello); err != nil {
+					return err
+				}
 				connectReqGot, err := Recv[*ConnectRequestRaw](c)
 				if err != nil {
 					return err
@@ -79,6 +86,12 @@ func TestConn(t *testing.T) {
 	c := dial(t, serv.Addr.String())
 	defer c.Close()
 
+	connectHelloGot, err := Recv[*ConnectHelloRaw](c)
+	if err != nil {
+		t.Fatal(err)
+	}
+	assert.Equal(t, connectHello, connectHelloGot)
+
 	if err := Send(c, connectReq); err != nil {
 		t.Fatal(err)
 	}
@@ -102,7 +115,11 @@ func TestConn(t *testing.T) {
 }
 
 func BenchmarkConn(b *testing.B) {
+	connectHello := &ConnectHello{
+		Cookie: 1,
+	}
 	connectReq := &ConnectRequest{
+		Cookie:      73856093,
 		Id:          1,
 		Arch:        "arch",
 		GitRevision: "rev1",
@@ -125,7 +142,11 @@ func BenchmarkConn(b *testing.B) {
 		done <- serv.Serve(context.Background(),
 			func(_ context.Context, c *Conn) error {
 				for i := 0; i < b.N; i++ {
-					_, err := Recv[*ConnectRequestRaw](c)
+					if err := Send(c, connectHello); err != nil {
+						return err
+					}
+
+					_, err = Recv[*ConnectRequestRaw](c)
 					if err != nil {
 						return err
 					}
@@ -143,10 +164,14 @@ func BenchmarkConn(b *testing.B) {
 	b.ReportAllocs()
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
+		_, err := Recv[*ConnectHelloRaw](c)
+		if err != nil {
+			b.Fatal(err)
+		}
 		if err := Send(c, connectReq); err != nil {
 			b.Fatal(err)
 		}
-		_, err := Recv[*ConnectReplyRaw](c)
+		_, err = Recv[*ConnectReplyRaw](c)
 		if err != nil {
 			b.Fatal(err)
 		}
-- 
cgit mrf-deployment