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/rpcserver/rpcserver_test.go | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

(limited to 'pkg/rpcserver/rpcserver_test.go')

diff --git a/pkg/rpcserver/rpcserver_test.go b/pkg/rpcserver/rpcserver_test.go
index 2da916286..429b275ac 100644
--- a/pkg/rpcserver/rpcserver_test.go
+++ b/pkg/rpcserver/rpcserver_test.go
@@ -6,6 +6,7 @@ package rpcserver
 import (
 	"context"
 	"net"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -18,6 +19,7 @@ import (
 	"github.com/google/syzkaller/pkg/vminfo"
 	"github.com/google/syzkaller/prog"
 	"github.com/google/syzkaller/sys/targets"
+	"golang.org/x/sync/errgroup"
 )
 
 func getTestDefaultCfg() mgrconfig.Config {
@@ -176,12 +178,14 @@ func TestHandleConn(t *testing.T) {
 	defaultCfg := getTestDefaultCfg()
 
 	tests := []struct {
-		name      string
-		modifyCfg func() *mgrconfig.Config
-		req       *flatrpc.ConnectRequest
+		name       string
+		wantErrMsg string
+		modifyCfg  func() *mgrconfig.Config
+		req        *flatrpc.ConnectRequest
 	}{
 		{
-			name: "error, cfg.VMLess = false - unknown VM tries to connect",
+			name:       "error, cfg.VMLess = false - unknown VM tries to connect",
+			wantErrMsg: "tries to connect",
 			modifyCfg: func() *mgrconfig.Config {
 				return &defaultCfg
 			},
@@ -214,10 +218,22 @@ func TestHandleConn(t *testing.T) {
 
 			injectExec := make(chan bool)
 			serv.CreateInstance(1, injectExec, nil)
-
-			go flatrpc.Send(clientConn, tt.req)
-			err = serv.handleConn(context.Background(), serverConn)
-			if err != nil {
+			g := errgroup.Group{}
+			g.Go(func() error {
+				hello, err := flatrpc.Recv[*flatrpc.ConnectHelloRaw](clientConn)
+				if err != nil {
+					return err
+				}
+				tt.req.Cookie = authHash(hello.Cookie)
+				flatrpc.Send(clientConn, tt.req)
+				return nil
+			})
+			if err := serv.handleConn(context.Background(), serverConn); err != nil {
+				if !strings.Contains(err.Error(), tt.wantErrMsg) {
+					t.Fatal(err)
+				}
+			}
+			if err := g.Wait(); err != nil {
 				t.Fatal(err)
 			}
 		})
-- 
cgit mrf-deployment