Address CodeRabbit VNC review feedback

client/ssh/auth/auth.go

@@ -117,12 +117,22 @@ func (a *Authorizer) Update(config *Config) {
 	a.machineUsers = machineUsers
 
 	sessionPubKeys := make(map[[sessionPubKeyLen]byte]sshuserhash.UserIDHash, len(config.SessionPubKeys))
+	conflicted := make(map[[sessionPubKeyLen]byte]struct{})
 	for _, e := range config.SessionPubKeys {
 		if len(e.PubKey) != sessionPubKeyLen {
 			continue
 		}
 		var key [sessionPubKeyLen]byte
 		copy(key[:], e.PubKey)
+		if _, bad := conflicted[key]; bad {
+			continue
+		}
+		if existing, ok := sessionPubKeys[key]; ok && existing != e.UserIDHash {
+			log.Warnf("SSH auth: session pubkey bound to conflicting user hashes; dropping binding")
+			delete(sessionPubKeys, key)
+			conflicted[key] = struct{}{}
+			continue
+		}
 		sessionPubKeys[key] = e.UserIDHash
 	}
 	a.sessionPubKeys = sessionPubKeys

client/status/status.go

@@ -1075,5 +1075,7 @@ func anonymizeServerSessions(a *anonymize.Anonymizer, overview *OutputOverview)
 	}
 	for i, sess := range overview.VNCServerState.Sessions {
 		overview.VNCServerState.Sessions[i].RemoteAddress = anonymizeRemoteAddress(a, sess.RemoteAddress)
+		overview.VNCServerState.Sessions[i].Username = a.AnonymizeString(sess.Username)
+		overview.VNCServerState.Sessions[i].UserID = a.AnonymizeString(sess.UserID)
 	}
 }

client/vnc/server/input_uinput_freebsd.go

@@ -0,0 +1,17 @@
+//go:build freebsd
+
+package server
+
+import "fmt"
+
+// UInputInjector is a freebsd placeholder; the linux uinput implementation
+// uses Linux-only ioctls (UI_DEV_CREATE etc.) and is not portable.
+type UInputInjector struct {
+	StubInputInjector
+}
+
+// NewUInputInjector always returns an error on freebsd so callers fall back
+// to a stub or platform-appropriate injector.
+func NewUInputInjector(_, _ int) (*UInputInjector, error) {
+	return nil, fmt.Errorf("uinput not implemented on freebsd")
+}

client/vnc/server/input_windows.go

@@ -169,10 +169,6 @@ func (w *WindowsInputInjector) Close() {
 func (w *WindowsInputInjector) tryEnqueue(cmd inputCmd) {
 	select {
 	case <-w.closed:
-		return
-	default:
-	}
-	select {
 	case w.ch <- cmd:
 	default:
 	}

client/vnc/server/noise_auth_test.go

@@ -268,19 +268,20 @@ func TestNoise_TruncatedMsg1_ClosesConnection(t *testing.T) {
 
 	conn, err := net.Dial("tcp", addr.String())
 	require.NoError(t, err)
+	defer conn.Close()
 
 	writeHeaderPrefix(t, conn, ModeAttach)
 	_, err = conn.Write([]byte("NBV3"))
 	require.NoError(t, err)
 	_, err = conn.Write(make([]byte, 8))
 	require.NoError(t, err)
-	require.NoError(t, conn.Close())
+	require.NoError(t, conn.(*net.TCPConn).CloseWrite())
 
-	// Re-dial just to confirm the listener is alive (the previous
-	// connection terminated server-side without affecting the listener).
-	probe, err := net.Dial("tcp", addr.String())
-	require.NoError(t, err)
-	require.NoError(t, probe.Close())
+	require.NoError(t, conn.SetReadDeadline(time.Now().Add(2*time.Second)))
+	buf := make([]byte, 64)
+	n, err := conn.Read(buf)
+	require.Equal(t, 0, n, "server must not emit RFB bytes after a truncated handshake")
+	require.ErrorIs(t, err, io.EOF, "server must close the connection on truncated msg1")
 }
 
 // TestNoise_AuthEnabled_NoHandshake_Rejected proves that with auth on,

client/wasm/internal/vnc/proxy.go

@@ -58,22 +58,19 @@ func NewSessionKey() (string, []byte, error) {
 	return id, kp.Public, nil
 }
 
-// lookupSessionKey returns the keypair for id, or false if unknown.
-func lookupSessionKey(id string) (noise.DHKey, bool) {
+// consumeSessionKey atomically retrieves and removes the keypair for id.
+// A session handle is single-use; combining lookup and delete under one
+// critical section prevents concurrent callers from observing the same key.
+func consumeSessionKey(id string) (noise.DHKey, bool) {
 	sessionKeyStore.mu.Lock()
 	defer sessionKeyStore.mu.Unlock()
 	kp, ok := sessionKeyStore.keys[id]
+	if ok {
+		delete(sessionKeyStore.keys, id)
+	}
 	return kp, ok
 }
 
-// dropSessionKey removes the keypair for id. Called after the VNC
-// connection closes (or after a connect attempt fails terminally).
-func dropSessionKey(id string) {
-	sessionKeyStore.mu.Lock()
-	delete(sessionKeyStore.keys, id)
-	sessionKeyStore.mu.Unlock()
-}
-
 const (
 	vncProxyHost   = "vnc.proxy.local"
 	vncProxyScheme = "ws"
@@ -190,13 +187,10 @@ func (p *VNCProxy) CreateProxy(req ProxyRequest) js.Value {
 		height:    height,
 	}
 	if req.KeySessionID != "" {
-		kp, ok := lookupSessionKey(req.KeySessionID)
+		kp, ok := consumeSessionKey(req.KeySessionID)
 		if !ok {
 			return rejectedPromise("unknown VNC session id")
 		}
-		// A session handle is single-use; drop it before the destination
-		// holds the private bytes so a leaked handle can't be replayed.
-		dropSessionKey(req.KeySessionID)
 		dest.sessionPriv = kp.Private
 		dest.sessionPub = kp.Public
 		pub, err := decodePeerPubKey(req.PeerPublicKey)

management/server/http/handlers/peers/peers_handler.go

@@ -2,6 +2,7 @@ package peers
 
 import (
 	"context"
+	"encoding/base64"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -518,6 +519,15 @@ func (h *Handler) CreateTemporaryAccess(w http.ResponseWriter, r *http.Request)
 			policy.Rules[0].AuthorizedUser = userAuth.UserId
 		}
 		if protocol == types.PolicyRuleProtocolNetbirdVNC && req.SessionPubKey != nil {
+			pub, err := base64.StdEncoding.DecodeString(*req.SessionPubKey)
+			if err != nil {
+				util.WriteError(r.Context(), status.Errorf(status.InvalidArgument, "session_pub_key is not valid base64: %v", err), w)
+				return
+			}
+			if len(pub) != 32 {
+				util.WriteError(r.Context(), status.Errorf(status.InvalidArgument, "session_pub_key must decode to 32 bytes, got %d", len(pub)), w)
+				return
+			}
 			policy.Rules[0].SessionPubKey = *req.SessionPubKey
 		}
 

management/server/types/policyrule.go

@@ -89,10 +89,10 @@ type PolicyRule struct {
 	// AuthorizedUser is a list of userIDs that are authorized to access local resources via ssh
 	AuthorizedUser string
 
-	// SessionPubKey is the base64 Ed25519 public key the AuthorizedUser
-	// will sign session-binding challenges with. Set together with
-	// AuthorizedUser when the rule was created via temporary-access for
-	// a VNC scope; empty otherwise.
+	// SessionPubKey is the base64 X25519 public key used with Noise_IK to
+	// bind a VNC session to the AuthorizedUser. Set together with
+	// AuthorizedUser when the rule was created via temporary-access for a
+	// VNC scope; empty otherwise.
 	SessionPubKey string
 }
 

shared/management/http/api/openapi.yml

@@ -1008,7 +1008,7 @@ components:
             type: string
             example: "tcp/80"
         session_pub_key:
-          description: Ephemeral Ed25519 public key the requester will sign session-binding challenges with. Required for VNC rules; ignored for SSH and L4.
+          description: Ephemeral base64-encoded X25519 public key used with Noise_IK to bind the VNC session. Required for VNC rules; ignored for SSH and L4.
           type: string
           example: "n0r3pL4c3h0ld3rK3y=="
       required: