Merge pull request #489 from uselagoon/golang-ssh-cve

Address potential misuse of external state in SSH callbacks

cmd/ssh-portal/serve.go

@@ -9,7 +9,7 @@ import (
 	"syscall"
 	"time"
 
-	"github.com/nats-io/nats.go"
+	"github.com/uselagoon/ssh-portal/internal/bus"
 	"github.com/uselagoon/ssh-portal/internal/k8s"
 	"github.com/uselagoon/ssh-portal/internal/metrics"
 	"github.com/uselagoon/ssh-portal/internal/sshserver"
@@ -36,24 +36,12 @@ type ServeCmd struct {
 // Run the serve command to handle SSH connection requests.
 func (cmd *ServeCmd) Run(log *slog.Logger) error {
 	// get main process context, which cancels on SIGTERM
-	ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGTERM)
-	defer stop()
-	// get nats server connection
-	nc, err := nats.Connect(cmd.NATSServer,
-		nats.Name("ssh-portal"),
-		// exit on connection close
-		nats.ClosedHandler(func(_ *nats.Conn) {
-			log.Error("nats connection closed")
-			stop()
-		}),
-		nats.DisconnectErrHandler(func(_ *nats.Conn, err error) {
-			log.Warn("nats disconnected", slog.Any("error", err))
-		}),
-		nats.ReconnectHandler(func(nc *nats.Conn) {
-			log.Info("nats reconnected", slog.String("url", nc.ConnectedUrl()))
-		}))
+	ctx, cancel := signal.NotifyContext(context.Background(), syscall.SIGTERM)
+	defer cancel()
+	// get nats client
+	nc, err := bus.NewNATSClient(cmd.NATSServer, log, cancel)
 	if err != nil {
-		return fmt.Errorf("couldn't connect to NATS server: %v", err)
+		return fmt.Errorf("couldn't get nats client: %v", err)
 	}
 	defer nc.Close()
 	// start listening on TCP port

go.mod

@@ -9,7 +9,7 @@ require (
 	github.com/alecthomas/kong v1.5.0
 	github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
-	github.com/gliderlabs/ssh v0.3.7
+	github.com/gliderlabs/ssh v0.3.8
 	github.com/go-sql-driver/mysql v1.8.1
 	github.com/golang-jwt/jwt/v5 v5.2.1
 	github.com/google/uuid v1.6.1-0.20240806143717-0e97ed3b5379
@@ -20,10 +20,10 @@ require (
 	github.com/zitadel/oidc/v3 v3.33.1
 	go.opentelemetry.io/otel v1.32.0
 	go.uber.org/mock v0.5.0
-	golang.org/x/crypto v0.29.0
+	golang.org/x/crypto v0.31.0
 	golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63
 	golang.org/x/oauth2 v0.24.0
-	golang.org/x/sync v0.9.0
+	golang.org/x/sync v0.10.0
 	golang.org/x/time v0.8.0
 	k8s.io/api v0.31.3
 	k8s.io/apimachinery v0.31.3
@@ -74,9 +74,9 @@ require (
 	go.opentelemetry.io/otel/metric v1.32.0 // indirect
 	go.opentelemetry.io/otel/trace v1.32.0 // indirect
 	golang.org/x/net v0.26.0 // indirect
-	golang.org/x/sys v0.27.0 // indirect
-	golang.org/x/term v0.26.0 // indirect
-	golang.org/x/text v0.20.0 // indirect
+	golang.org/x/sys v0.28.0 // indirect
+	golang.org/x/term v0.27.0 // indirect
+	golang.org/x/text v0.21.0 // indirect
 	google.golang.org/protobuf v1.34.2 // indirect
 	gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect

go.sum

@@ -31,6 +31,8 @@ github.com/fxamacker/cbor/v2 v2.7.0 h1:iM5WgngdRBanHcxugY4JySA0nk1wZorNOpTgCMedv
 github.com/fxamacker/cbor/v2 v2.7.0/go.mod h1:pxXPTn3joSm21Gbwsv0w9OSA2y1HFR9qXEeXQVeNoDQ=
 github.com/gliderlabs/ssh v0.3.7 h1:iV3Bqi942d9huXnzEF2Mt+CY9gLu8DNM4Obd+8bODRE=
 github.com/gliderlabs/ssh v0.3.7/go.mod h1:zpHEXBstFnQYtGnB8k8kQLol82umzn/2/snG7alWVD8=
+github.com/gliderlabs/ssh v0.3.8 h1:a4YXD1V7xMF9g5nTkdfnja3Sxy1PVDCj1Zg4Wb8vY6c=
+github.com/gliderlabs/ssh v0.3.8/go.mod h1:xYoytBv1sV0aL3CavoDuJIQNURXkkfPA/wxQ1pL1fAU=
 github.com/go-chi/chi/v5 v5.1.0 h1:acVI1TYaD+hhedDJ3r54HyA6sExp3HfXq7QWEEY/xMw=
 github.com/go-chi/chi/v5 v5.1.0/go.mod h1:DslCQbL2OYiznFReuXYUmQ2hGd1aDpCnlMNITLSKoi8=
 github.com/go-jose/go-jose/v4 v4.0.4 h1:VsjPI33J0SB9vQM6PLmNjoHqMQNGPiZ0rHL7Ni7Q6/E=
@@ -182,6 +184,8 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/crypto v0.29.0 h1:L5SG1JTTXupVV3n6sUqMTeWbjAyfPwoda2DLX8J8FrQ=
 golang.org/x/crypto v0.29.0/go.mod h1:+F4F4N5hv6v38hfeYwTdx20oUvLLc+QfrE9Ax9HtgRg=
+golang.org/x/crypto v0.31.0 h1:ihbySMvVjLAeSH1IbfcRTkD/iNscyz8rGzjF/E5hV6U=
+golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk=
 golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63 h1:m64FZMko/V45gv0bNmrNYoDEq8U5YUhetc9cBWKS1TQ=
 golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63/go.mod h1:0v4NqG35kSWCMzLaMeX+IQrlSnVE/bqGSyC2cz/9Le8=
 golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
@@ -199,18 +203,26 @@ golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJ
 golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.9.0 h1:fEo0HyrW1GIgZdpbhCRO0PkJajUS5H9IFUztCgEo2jQ=
 golang.org/x/sync v0.9.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
+golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ=
+golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.27.0 h1:wBqf8DvsY9Y/2P8gAfPDEYNuS30J4lPHJxXSb/nJZ+s=
 golang.org/x/sys v0.27.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA=
+golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/term v0.26.0 h1:WEQa6V3Gja/BhNxg540hBip/kkaYtRg3cxg4oXSw4AU=
 golang.org/x/term v0.26.0/go.mod h1:Si5m1o57C5nBNQo5z1iq+XDijt21BDBDp2bK0QI8e3E=
+golang.org/x/term v0.27.0 h1:WP60Sv1nlK1T6SupCHbXzSaN0b9wUmsPoRS9b61A23Q=
+golang.org/x/term v0.27.0/go.mod h1:iMsnZpn0cago0GOrHO2+Y7u7JPn5AylBrcoWkElMTSM=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.20.0 h1:gK/Kv2otX8gz+wn7Rmb3vT96ZwuoxnQlY+HlJVj7Qug=
 golang.org/x/text v0.20.0/go.mod h1:D4IsuqiFMhST5bX19pQ9ikHC2GsaKyk/oF+pn3ducp4=
+golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo=
+golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ=
 golang.org/x/time v0.8.0 h1:9i3RxcPv3PZnitoVGMPDKZSq1xW1gK1Xy3ArNOGZfEg=
 golang.org/x/time v0.8.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=

internal/bus/ssh.go

@@ -1,20 +1,30 @@
 // Package bus contains the definitions of the messages passed across NATS.
 package bus
 
-import "log/slog"
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"log/slog"
+	"time"
+
+	"github.com/nats-io/nats.go"
+)
 
 const (
 	// SubjectSSHAccessQuery defines the NATS subject for SSH access queries.
 	SubjectSSHAccessQuery = "lagoon.sshportal.api"
+	// NATS request timeout.
+	natsTimeout = 8 * time.Second
 )
 
 // SSHAccessQuery defines the structure of an SSH access query.
 type SSHAccessQuery struct {
+	SessionID      string
 	SSHFingerprint string
 	NamespaceName  string
 	ProjectID      int
 	EnvironmentID  int
-	SessionID      string
 }
 
 // LogValue implements the slog.LogValuer interface.
@@ -27,3 +37,83 @@ func (q SSHAccessQuery) LogValue() slog.Value {
 		slog.String("sessionID", q.SessionID),
 	)
 }
+
+// NATSClient is a NATS client.
+type NATSClient struct {
+	conn *nats.Conn
+}
+
+// NewNATSClient constructs a new NATS client which connects to the given
+// srvAddr. It logs to the given log, and calls the given context.CancelFunc
+// when the NATS connection closes.
+//
+// The idea is that when the connection closes on the other end, this function
+// must be called again to construct a new client.
+func NewNATSClient(
+	srvAddr string,
+	log *slog.Logger,
+	cancel context.CancelFunc,
+) (*NATSClient, error) {
+	// get nats server connection
+	conn, err := nats.Connect(
+		srvAddr,
+		nats.Name("ssh-portal"),
+		// cancel upstream context on connection close
+		nats.ClosedHandler(func(_ *nats.Conn) {
+			log.Error("nats connection closed")
+			cancel()
+		}),
+		nats.DisconnectErrHandler(func(_ *nats.Conn, err error) {
+			log.Warn("nats disconnected", slog.Any("error", err))
+		}),
+		nats.ReconnectHandler(func(nc *nats.Conn) {
+			log.Info("nats reconnected", slog.String("url", nc.ConnectedUrl()))
+		}))
+	if err != nil {
+		return nil, fmt.Errorf("couldn't connect to NATS server: %v", err)
+	}
+	return &NATSClient{
+		conn: conn,
+	}, nil
+}
+
+// Close calls Close() on the underlying NATS connection.
+func (c *NATSClient) Close() {
+	c.conn.Close()
+}
+
+// KeyCanAccessEnvironment returns true if the given key can access the given
+// environment, or false otherwise.
+func (c *NATSClient) KeyCanAccessEnvironment(
+	sessionID,
+	sshFingerprint,
+	namespaceName string,
+	projectID,
+	environmentID int,
+) (bool, error) {
+	// construct ssh access query
+	queryData, err := json.Marshal(SSHAccessQuery{
+		SessionID:      sessionID,
+		SSHFingerprint: sshFingerprint,
+		NamespaceName:  namespaceName,
+		ProjectID:      projectID,
+		EnvironmentID:  environmentID,
+	})
+	if err != nil {
+		return false, fmt.Errorf("couldn't marshal NATS request: %v", err)
+	}
+	// send query
+	msg, err := c.conn.Request(
+		SubjectSSHAccessQuery,
+		queryData,
+		natsTimeout)
+	if err != nil {
+		return false, fmt.Errorf("couldn't make NATS request: %v", err)
+	}
+	// handle response
+	var ok bool
+	if err := json.Unmarshal(msg.Data, &ok); err != nil {
+		return false, fmt.Errorf("couldn't unmarshal response: %v", err)
+	}
+	return ok, nil
+}

internal/k8s/namespacedetails.go

@@ -28,19 +28,23 @@ func intFromLabel(labels map[string]string, label string) (int, error) {
 // the labels on a Lagoon environment namespace for a Lagoon namespace. If one
 // of the expected labels is missing or cannot be parsed, it will return an
 // error.
-func (c *Client) NamespaceDetails(ctx context.Context, name string) (
-	int, int, string, string, error) {
+func (c *Client) NamespaceDetails(
+	ctx context.Context,
+	name string,
+) (int, int, string, string, error) {
 	var eid, pid int
 	var ename, pname string
 	var ok bool
 	ctx, cancel := context.WithTimeout(ctx, timeout)
 	defer cancel()
-	ns, err := c.clientset.CoreV1().Namespaces().Get(ctx, name, metav1.GetOptions{})
+	ns, err :=
+		c.clientset.CoreV1().Namespaces().Get(ctx, name, metav1.GetOptions{})
 	if err != nil {
 		return 0, 0, "", "", fmt.Errorf("couldn't get namespace: %v", err)
 	}
 	if eid, err = intFromLabel(ns.Labels, environmentIDLabel); err != nil {
-		return 0, 0, "", "", fmt.Errorf("couldn't get environment ID from label: %v", err)
+		return 0, 0, "", "",
+			fmt.Errorf("couldn't get environment ID from label: %v", err)
 	}
 	if pid, err = intFromLabel(ns.Labels, projectIDLabel); err != nil {
 		return 0, 0, "", "", fmt.Errorf("couldn't get project ID from label: %v", err)
@@ -50,7 +54,8 @@ func (c *Client) NamespaceDetails(ctx context.Context, name string) (
 			environmentNameLabel)
 	}
 	if pname, ok = ns.Labels[projectNameLabel]; !ok {
-		return 0, 0, "", "", fmt.Errorf("missing project name label %v", projectNameLabel)
+		return 0, 0, "", "",
+			fmt.Errorf("missing project name label %v", projectNameLabel)
 	}
 	return eid, pid, ename, pname, nil
 }