diff --git a/client/internal/debug/debug_test.go b/client/internal/debug/debug_test.go index 5830583a3..2ed010354 100644 --- a/client/internal/debug/debug_test.go +++ b/client/internal/debug/debug_test.go @@ -863,6 +863,7 @@ func TestAddConfig_AllFieldsCovered(t *testing.T) { RosenpassPermissive: true, ServerSSHAllowed: &bTrue, ServerVNCAllowed: &bTrue, + DisableVNCApproval: &bTrue, EnableSSHRoot: &bTrue, EnableSSHSFTP: &bTrue, EnableSSHLocalPortForwarding: &bTrue, diff --git a/client/internal/profilemanager/config.go b/client/internal/profilemanager/config.go index a255a92c3..06ed7afa8 100644 --- a/client/internal/profilemanager/config.go +++ b/client/internal/profilemanager/config.go @@ -433,7 +433,7 @@ func (config *Config) apply(input ConfigInput) (updated bool, err error) { updated = true } } else if config.ServerVNCAllowed == nil { - config.ServerVNCAllowed = util.True() + config.ServerVNCAllowed = util.False() updated = true } diff --git a/client/server/capture.go b/client/server/capture.go index 8975fdc75..0b1eaa8e8 100644 --- a/client/server/capture.go +++ b/client/server/capture.go @@ -111,7 +111,7 @@ func (s *Server) StartCapture(req *proto.StartCaptureRequest, stream proto.Daemo return status.Errorf(codes.Internal, "create capture session: %v", err) } - engine, err := s.claimCapture(sess) + engine, err := s.claimCapture(sess, func() { pw.Close() }) if err != nil { sess.Stop() pw.Close() @@ -305,7 +305,7 @@ func (s *Server) cleanupBundleCapture() { // capture is already running it is evicted: a previous streaming session // whose gRPC client died and never freed the slot stays stuck otherwise, // and a bundle capture is just informational state. -func (s *Server) claimCapture(sess *capture.Session) (*internal.Engine, error) { +func (s *Server) claimCapture(sess *capture.Session, cancel func()) (*internal.Engine, error) { s.mutex.Lock() defer s.mutex.Unlock() @@ -315,6 +315,7 @@ func (s *Server) claimCapture(sess *capture.Session) (*internal.Engine, error) { return nil, err } s.activeCapture = sess + s.activeCaptureCancel = cancel return engine, nil } @@ -331,13 +332,18 @@ func (s *Server) evictActiveCaptureLocked() { } log.Infof("evicting previous streaming capture to start a new one") prev := s.activeCapture + cancel := s.activeCaptureCancel if engine, err := s.getCaptureEngineLocked(); err == nil { if err := engine.SetCapture(nil); err != nil { log.Debugf("clear previous capture: %v", err) } } s.activeCapture = nil + s.activeCaptureCancel = nil prev.Stop() + if cancel != nil { + cancel() + } } // releaseCapture clears the active-capture owner if it still matches sess. @@ -346,6 +352,7 @@ func (s *Server) releaseCapture(sess *capture.Session) { defer s.mutex.Unlock() if s.activeCapture == sess { s.activeCapture = nil + s.activeCaptureCancel = nil } } @@ -360,6 +367,7 @@ func (s *Server) clearCaptureIfOwner(sess *capture.Session, engine *internal.Eng log.Debugf("clear capture: %v", err) } s.activeCapture = nil + s.activeCaptureCancel = nil } func (s *Server) getCaptureEngineLocked() (*internal.Engine, error) { diff --git a/client/server/server.go b/client/server/server.go index 1784c91a8..143bc8177 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -93,8 +93,12 @@ type Server struct { captureEnabled bool bundleCapture *bundleCapture // activeCapture is the session currently installed on the engine; guarded by s.mutex. - activeCapture *capture.Session - networksDisabled bool + activeCapture *capture.Session + // activeCaptureCancel tears down the streaming pipe/cancel for the + // active streaming capture so eviction unblocks the StartCapture RPC + // handler. Nil for bundle captures (they own their own context). + activeCaptureCancel func() + networksDisabled bool sleepHandler *sleephandler.SleepHandler diff --git a/client/ui/approval.go b/client/ui/approval.go index 70c8963d5..95e16882e 100644 --- a/client/ui/approval.go +++ b/client/ui/approval.go @@ -113,15 +113,17 @@ func (s *serviceClient) showApprovalUI(req approvalRequest) { decide(outcome{}) return } - updateCountdown() + fyne.Do(updateCountdown) } }() go func() { o := <-decided s.sendApprovalResponse(req.requestID, o.accept, o.viewOnly) - w.Close() - s.app.Quit() + fyne.Do(func() { + w.Close() + s.app.Quit() + }) }() w.Show() diff --git a/client/vnc/server/agent_windows.go b/client/vnc/server/agent_windows.go index 735ab274d..32c803834 100644 --- a/client/vnc/server/agent_windows.go +++ b/client/vnc/server/agent_windows.go @@ -47,8 +47,6 @@ var ( procWTSEnumerateSessionsW = wtsapi32.NewProc("WTSEnumerateSessionsW") procWTSFreeMemory = wtsapi32.NewProc("WTSFreeMemory") procWTSQuerySessionInformation = wtsapi32.NewProc("WTSQuerySessionInformationW") - - iphlpapi = windows.NewLazySystemDLL("iphlpapi.dll") ) // GetCurrentSessionID returns the session ID of the current process. @@ -514,6 +512,8 @@ func (m *sessionManager) reapExitedAgent() { log.Debugf("close agent handle: %v", err) } m.agentProc = 0 + m.authToken = "" + m.socketPath = "" } // scheduleNextSpawn applies an exponential backoff on fast crashes (<5s) and @@ -586,6 +586,8 @@ func (m *sessionManager) killAgent() { _ = windows.TerminateProcess(m.agentProc, 0) _ = windows.CloseHandle(m.agentProc) m.agentProc = 0 + m.authToken = "" + m.socketPath = "" log.Info("killed old agent") } diff --git a/client/vnc/server/console_user_darwin.go b/client/vnc/server/console_user_darwin.go index 18091850a..97df2b8a1 100644 --- a/client/vnc/server/console_user_darwin.go +++ b/client/vnc/server/console_user_darwin.go @@ -1,19 +1,10 @@ package server -import "errors" - -// consoleHasInteractiveUser returns true when a user is logged into the -// console (i.e. an Aqua session is active). At the loginwindow there is -// nobody to display an approval prompt to, so callers can decline -// without waiting on the broker. -func consoleHasInteractiveUser() bool { - if _, err := consoleUserID(); err != nil { - if errors.Is(err, errNoConsoleUser) { - return false - } - // Unknown error: fail closed so a probe-time glitch does not - // silently let an unattended console accept VNC sessions. - return false - } - return true +// interactiveUserError returns nil when a user is logged into the console +// (i.e. an Aqua session is active). At the loginwindow there is nobody to +// display an approval prompt to, so callers can decline without waiting on +// the broker. Any error (including errNoConsoleUser) is treated as decline. +func interactiveUserError() error { + _, err := consoleUserID() + return err } diff --git a/client/vnc/server/console_user_other.go b/client/vnc/server/console_user_other.go index ddc67bfd4..9824fd385 100644 --- a/client/vnc/server/console_user_other.go +++ b/client/vnc/server/console_user_other.go @@ -2,6 +2,6 @@ package server -// consoleHasInteractiveUser is unused outside service mode (darwin/windows) -// but the symbol must exist so gateApproval compiles on all platforms. -func consoleHasInteractiveUser() bool { return true } +// interactiveUserError is unused outside service mode (darwin/windows) but +// the symbol must exist so gateApproval compiles on all platforms. +func interactiveUserError() error { return nil } diff --git a/client/vnc/server/console_user_windows.go b/client/vnc/server/console_user_windows.go index 70197d8cc..1356fb3ce 100644 --- a/client/vnc/server/console_user_windows.go +++ b/client/vnc/server/console_user_windows.go @@ -1,13 +1,15 @@ package server -// consoleHasInteractiveUser returns true when there is a logged-in user -// session on the box. At the lock/login screen WTSQueryUserName is empty, -// which means there is nobody to display an approval prompt to. Callers -// should decline without waiting on the broker in that case. -func consoleHasInteractiveUser() bool { +// interactiveUserError returns nil when there is a logged-in user session +// on the box. At the lock/login screen WTSQueryUserName is empty, which +// means there is nobody to display an approval prompt to. +func interactiveUserError() error { sid := getActiveSessionID() if sid == 0 { - return false + return errNoConsoleUser } - return wtsSessionHasUser(sid) + if !wtsSessionHasUser(sid) { + return errNoConsoleUser + } + return nil } diff --git a/client/vnc/server/server.go b/client/vnc/server/server.go index eb9751018..daa274f66 100644 --- a/client/vnc/server/server.go +++ b/client/vnc/server/server.go @@ -431,10 +431,12 @@ func (s *Server) gateApproval(conn net.Conn, header *connectionHeader, connLog * connLog.Warn("VNC connection rejected: approval required but no approver") return false, ApprovalDecision{} } - if s.serviceMode && !consoleHasInteractiveUser() { - rejectConnection(conn, codeMessage(RejectCodeNoConsoleUser, "no interactive user session")) - connLog.Info("VNC connection rejected: no interactive user session to approve") - return false, ApprovalDecision{} + if s.serviceMode { + if err := interactiveUserError(); err != nil { + rejectConnection(conn, codeMessage(RejectCodeNoConsoleUser, "no interactive user session")) + connLog.Infof("VNC connection rejected: no interactive user session to approve: %v", err) + return false, ApprovalDecision{} + } } info := ApprovalInfo{ SourceIP: sourceIPString(conn.RemoteAddr()), @@ -449,7 +451,7 @@ func (s *Server) gateApproval(conn net.Conn, header *connectionHeader, connLog * } decision, err := s.approver.Request(s.ctx, info) if err != nil { - rejectConnection(conn, codeMessage(RejectCodeApprovalDenied, err.Error())) + rejectConnection(conn, codeMessage(RejectCodeApprovalDenied, "approval denied")) connLog.Infof("VNC connection rejected: approval %v", err) return false, ApprovalDecision{} } @@ -737,9 +739,13 @@ func (s *Server) validateCapturer(capturer ScreenCapturer) error { func (s *Server) isAllowedSource(addr net.Addr) bool { // Unix-socket remotes (the agent path) are local IPC, gated by the // token, not by overlay membership. + if _, ok := addr.(*net.UnixAddr); ok { + return true + } tcpAddr, ok := addr.(*net.TCPAddr) if !ok { - return true + s.log.Warnf("connection rejected: unsupported remote address type %T", addr) + return false } remoteIP, ok := netip.AddrFromSlice(tcpAddr.IP) diff --git a/client/vnc/server/server_windows.go b/client/vnc/server/server_windows.go index a8c58bd7f..7ad88eef7 100644 --- a/client/vnc/server/server_windows.go +++ b/client/vnc/server/server_windows.go @@ -269,4 +269,3 @@ func (s *Server) serviceAcceptLoop() { }(conn) } } - diff --git a/client/vnc/server/session.go b/client/vnc/server/session.go index 210e4379f..8a5d9bd31 100644 --- a/client/vnc/server/session.go +++ b/client/vnc/server/session.go @@ -545,6 +545,9 @@ func (s *session) handleFBUpdateRequest() error { // in sync with the active session (e.g. username changes after login on // a virtual session). func (s *session) SendDesktopName(name string) error { + if s.viewOnly { + name = ViewOnlyDesktopNamePrefix + name + } s.encMu.RLock() supported := s.clientSupportsDesktopName s.encMu.RUnlock() @@ -629,13 +632,13 @@ func (s *session) handlePointerEvent() error { mask = (mask & 0x7f) | uint16(hi[0])<<7 } + if s.viewOnly { + return nil + } s.pointerMu.Lock() s.lastPointerX = x s.lastPointerY = y s.pointerMu.Unlock() - if s.viewOnly { - return nil - } s.injector.InjectPointer(mask, x, y, s.serverW, s.serverH) return nil } @@ -661,6 +664,9 @@ var stickyModifierKeysyms = [...]uint32{ // when the client disconnects mid-press. Mouse coordinates are reused // from the last PointerEvent so we don't warp the cursor. func (s *session) releaseStickyInput() { + if s.viewOnly { + return + } for _, ks := range stickyModifierKeysyms { s.injector.InjectKey(ks, false) } diff --git a/management/server/http/handlers/peers/peers_handler.go b/management/server/http/handlers/peers/peers_handler.go index 8e7385f7a..8d410de77 100644 --- a/management/server/http/handlers/peers/peers_handler.go +++ b/management/server/http/handlers/peers/peers_handler.go @@ -475,6 +475,37 @@ func (h *Handler) CreateTemporaryAccess(w http.ResponseWriter, r *http.Request) newPeer := &nbpeer.Peer{} newPeer.FromAPITemporaryAccessRequest(&req) + parsedRules := make([]struct { + raw string + protocol types.PolicyRuleProtocolType + portRange types.RulePortRange + }, 0, len(req.Rules)) + needsVNCKey := false + for _, rule := range req.Rules { + protocol, portRange, err := types.ParseRuleString(rule) + if err != nil { + util.WriteError(r.Context(), err, w) + return + } + if protocol == types.PolicyRuleProtocolNetbirdVNC { + needsVNCKey = true + } + parsedRules = append(parsedRules, struct { + raw string + protocol types.PolicyRuleProtocolType + portRange types.RulePortRange + }{rule, protocol, portRange}) + } + + var vncSessionPubKey string + if needsVNCKey { + vncSessionPubKey, err = validateVNCSessionPubKey(req.SessionPubKey) + if err != nil { + util.WriteError(r.Context(), err, w) + return + } + } + targetPeer, err := h.accountManager.GetPeer(r.Context(), userAuth.AccountId, peerID, userAuth.UserId) if err != nil { util.WriteError(r.Context(), err, w) @@ -487,12 +518,7 @@ func (h *Handler) CreateTemporaryAccess(w http.ResponseWriter, r *http.Request) return } - for _, rule := range req.Rules { - protocol, portRange, err := types.ParseRuleString(rule) - if err != nil { - util.WriteError(r.Context(), err, w) - return - } + for _, pr := range parsedRules { policy := &types.Policy{ AccountID: userAuth.AccountId, Description: "Temporary access policy for peer " + peer.Name, @@ -512,34 +538,28 @@ func (h *Handler) CreateTemporaryAccess(w http.ResponseWriter, r *http.Request) ID: targetPeer.ID, }, Bidirectional: false, - Protocol: protocol, - PortRanges: []types.RulePortRange{portRange}, + Protocol: pr.protocol, + PortRanges: []types.RulePortRange{pr.portRange}, }}, } - if protocol == types.PolicyRuleProtocolNetbirdSSH || protocol == types.PolicyRuleProtocolNetbirdVNC { + if pr.protocol == types.PolicyRuleProtocolNetbirdSSH || pr.protocol == types.PolicyRuleProtocolNetbirdVNC { policy.Rules[0].AuthorizedUser = userAuth.UserId } - if protocol == types.PolicyRuleProtocolNetbirdVNC { - pubKey, err := validateVNCSessionPubKey(req.SessionPubKey) - if err != nil { - util.WriteError(r.Context(), err, w) - return - } - policy.Rules[0].SessionPubKey = pubKey + if pr.protocol == types.PolicyRuleProtocolNetbirdVNC { + policy.Rules[0].SessionPubKey = vncSessionPubKey policy.Rules[0].SessionDisplayName = h.displayNameForUser(r.Context(), userAuth) } - _, err = h.accountManager.SavePolicy(r.Context(), userAuth.AccountId, userAuth.UserId, policy, true) - if err != nil { + if _, err = h.accountManager.SavePolicy(r.Context(), userAuth.AccountId, userAuth.UserId, policy, true); err != nil { util.WriteError(r.Context(), err, w) return } } resp := &api.PeerTemporaryAccessResponse{ - Id: peer.ID, - Name: peer.Name, - Rules: req.Rules, + Id: peer.ID, + Name: peer.Name, + Rules: req.Rules, TargetPubKey: targetPeer.Key, }