cmd/k8s-operator,k8s-operator/sessionrecording: support recording kubectl exec sessions over WebSockets (#12947)

cmd/k8s-operator,k8s-operator/sessionrecording: support recording WebSocket sessions

Kubernetes currently supports two streaming protocols, SPDY and WebSockets.
WebSockets are replacing SPDY, see
https://github.com/kubernetes/enhancements/issues/4006.
We were currently only supporting SPDY, erroring out if session
was not SPDY and relying on the kube's built-in SPDY fallback.

This PR:

- adds support for parsing contents of 'kubectl exec' sessions streamed
over WebSockets

- adds logic to distinguish 'kubectl exec' requests for a SPDY/WebSockets
sessions and call the relevant handler

Updates tailscale/corp#19821

Signed-off-by: Irbe Krumina <irbe@tailscale.com>
Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>

k8s-operator/sessionrecording/spdy/conn.go

@@ -19,12 +19,18 @@ import (
 
 	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
-	srconn "tailscale.com/k8s-operator/sessionrecording/conn"
 	"tailscale.com/k8s-operator/sessionrecording/tsrecorder"
 	"tailscale.com/sessionrecording"
 )
 
-func New(nc net.Conn, rec *tsrecorder.Client, ch sessionrecording.CastHeader, log *zap.SugaredLogger) srconn.Conn {
+// New wraps the provided network connection and returns a connection whose reads and writes will get triggered as data is received on the hijacked connection.
+// The connection must be a hijacked connection for a 'kubectl exec' session using SPDY.
+// The hijacked connection is used to transmit SPDY streams between Kubernetes client ('kubectl') and the destination container.
+// Data read from the underlying network connection is data sent via one of the SPDY streams from the client to the container.
+// Data written to the underlying connection is data sent from the container to the client.
+// We parse the data and send everything for the STDOUT/STDERR streams to the configured tsrecorder as an asciinema recording with the provided header.
+// https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/4006-transition-spdy-to-websockets#background-remotecommand-subprotocol
+func New(nc net.Conn, rec *tsrecorder.Client, ch sessionrecording.CastHeader, log *zap.SugaredLogger) net.Conn {
 	return &conn{
 		Conn: nc,
 		rec:  rec,
@@ -49,7 +55,6 @@ type conn struct {
 
 	wmu    sync.Mutex // sequences writes
 	closed bool
-	failed bool
 
 	rmu                 sync.Mutex // sequences reads
 	writeCastHeaderOnce sync.Once
@@ -172,9 +177,6 @@ func (c *conn) Close() error {
 	if c.closed {
 		return nil
 	}
-	if !c.failed && c.writeBuf.Len() > 0 {
-		c.Conn.Write(c.writeBuf.Bytes())
-	}
 	c.writeBuf.Reset()
 	c.closed = true
 	err := c.Conn.Close()
@@ -182,14 +184,8 @@ func (c *conn) Close() error {
 	return err
 }
 
-func (s *conn) Fail() {
-	s.wmu.Lock()
-	s.failed = true
-	s.wmu.Unlock()
-}
-
 // storeStreamID parses SYN_STREAM SPDY control frame and updates
-// spdyRemoteConnRecorder to store the newly created stream's ID if it is one of
+// conn to store the newly created stream's ID if it is one of
 // the stream types we care about. Storing stream_id:stream_type mapping allows
 // us to parse received data frames (that have stream IDs) differently depening
 // on which stream they belong to (i.e send data frame payload for stdout stream

k8s-operator/sessionrecording/spdy/conn_test.go

@@ -7,6 +7,7 @@ package spdy
 
 import (
 	"encoding/json"
+	"fmt"
 	"reflect"
 	"testing"
 
@@ -234,6 +235,57 @@ func Test_Reads(t *testing.T) {
 	}
 }
 
+// Test_conn_ReadRand tests reading arbitrarily generated byte slices from conn to
+// test that we don't panic when parsing input from a broken or malicious
+// client.
+func Test_conn_ReadRand(t *testing.T) {
+	zl, err := zap.NewDevelopment()
+	if err != nil {
+		t.Fatalf("error creating a test logger: %v", err)
+	}
+	for i := range 1000 {
+		tc := &fakes.TestConn{}
+		tc.ResetReadBuf()
+		c := &conn{
+			Conn: tc,
+			log:  zl.Sugar(),
+		}
+		bb := fakes.RandomBytes(t)
+		for j, input := range bb {
+			if err := tc.WriteReadBufBytes(input); err != nil {
+				t.Fatalf("[%d] writing bytes to test conn: %v", i, err)
+			}
+			f := func() {
+				c.Read(make([]byte, len(input)))
+			}
+			testPanic(t, f, fmt.Sprintf("[%d %d] Read panic parsing input of length %d", i, j, len(input)))
+		}
+	}
+}
+
+// Test_conn_WriteRand calls conn.Write with an arbitrary input to validate that
+// it does not panic.
+func Test_conn_WriteRand(t *testing.T) {
+	zl, err := zap.NewDevelopment()
+	if err != nil {
+		t.Fatalf("error creating a test logger: %v", err)
+	}
+	for i := range 100 {
+		tc := &fakes.TestConn{}
+		c := &conn{
+			Conn: tc,
+			log:  zl.Sugar(),
+		}
+		bb := fakes.RandomBytes(t)
+		for j, input := range bb {
+			f := func() {
+				c.Write(input)
+			}
+			testPanic(t, f, fmt.Sprintf("[%d %d] Write: panic parsing input of length %d", i, j, len(input)))
+		}
+	}
+}
+
 func resizeMsgBytes(t *testing.T, width, height int) []byte {
 	t.Helper()
 	bs, err := json.Marshal(spdyResizeMsg{Width: width, Height: height})

k8s-operator/sessionrecording/spdy/frame_test.go

@@ -9,11 +9,15 @@ import (
 	"bytes"
 	"compress/zlib"
 	"encoding/binary"
+	"fmt"
 	"io"
 	"net/http"
 	"reflect"
 	"strings"
 	"testing"
+	"time"
+
+	"math/rand"
 
 	"github.com/google/go-cmp/cmp"
 	"go.uber.org/zap"
@@ -200,6 +204,29 @@ func Test_spdyFrame_parseHeaders(t *testing.T) {
 	}
 }
 
+// Test_spdyFrame_ParseRand calls spdyFrame.Parse with randomly generated bytes
+// to test that it doesn't panic.
+func Test_spdyFrame_ParseRand(t *testing.T) {
+	zl, err := zap.NewDevelopment()
+	if err != nil {
+		t.Fatal(err)
+	}
+	r := rand.New(rand.NewSource(time.Now().UnixNano()))
+	for i := range 100 {
+		n := r.Intn(4096)
+		b := make([]byte, n)
+		_, err := r.Read(b)
+		if err != nil {
+			t.Fatalf("error generating random byte slice: %v", err)
+		}
+		sf := &spdyFrame{}
+		f := func() {
+			sf.Parse(b, zl.Sugar())
+		}
+		testPanic(t, f, fmt.Sprintf("[%d] Parse panicked running with byte slice of length %d: %v", i, n, r))
+	}
+}
+
 // payload takes a control frame type and a map with 0 or more header keys and
 // values and returns a SPDY control frame payload with the header as SPDY zlib
 // compressed header name/value block. The payload is padded with arbitrary
@@ -291,3 +318,13 @@ func header(hs map[string]string) http.Header {
 	}
 	return h
 }
+
+func testPanic(t *testing.T, f func(), msg string) {
+	t.Helper()
+	defer func() {
+		if r := recover(); r != nil {
+			t.Fatal(msg, r)
+		}
+	}()
+	f()
+}