diff --git a/client/cmd/vnc_agent.go b/client/cmd/vnc_agent.go index 5fe6c97a0..a6a583ac7 100644 --- a/client/cmd/vnc_agent.go +++ b/client/cmd/vnc_agent.go @@ -44,7 +44,7 @@ var vncAgentCmd = &cobra.Command{ capturer := vncserver.NewDesktopCapturer() injector := vncserver.NewWindowsInputInjector() - srv := vncserver.New(capturer, injector, "") + srv := vncserver.New(capturer, injector) srv.SetDisableAuth(true) srv.SetAgentToken(token) diff --git a/client/internal/engine_vnc.go b/client/internal/engine_vnc.go index 38de3feba..a7d9b1012 100644 --- a/client/internal/engine_vnc.go +++ b/client/internal/engine_vnc.go @@ -99,7 +99,7 @@ func (e *Engine) startVNCServer(sshConf *mgmProto.SSHConfig) error { netbirdIP := e.wgInterface.Address().IP - srv := vncserver.New(capturer, injector, "") + srv := vncserver.New(capturer, injector) if vncNeedsServiceMode() { log.Info("VNC: running in Session 0, enabling service mode (agent proxy)") srv.SetServiceMode(true) diff --git a/client/vnc/server/rfb.go b/client/vnc/server/rfb.go index 6dcb57a96..e4e87063b 100644 --- a/client/vnc/server/rfb.go +++ b/client/vnc/server/rfb.go @@ -3,9 +3,7 @@ package server import ( "bytes" "compress/zlib" - "crypto/des" //nolint:gosec // RFB protocol-defined DES challenge/response; not used for confidentiality "encoding/binary" - "fmt" "image" "image/jpeg" "unsafe" @@ -21,8 +19,7 @@ type rect struct { const ( rfbProtocolVersion = "RFB 003.008\n" - secNone = 1 - secVNCAuth = 2 + secNone = 1 // Client message types. clientSetPixelFormat = 0 @@ -297,38 +294,6 @@ func emitPixelBytes(dst []byte, pixel uint32, bytesPerPixel int, bigEndian bool) } } -// vncAuthEncrypt encrypts a 16-byte challenge using the VNC DES scheme. -func vncAuthEncrypt(challenge []byte, password string) ([]byte, error) { - key := make([]byte, 8) - pw := []byte(password) - n := len(pw) - if n > 8 { - n = 8 - } - for i := 0; i < n; i++ { - key[i] = reverseBits(pw[i]) - } - block, err := des.NewCipher(key) //nolint:gosec // RFB protocol-defined DES challenge/response; not a confidentiality cipher - if err != nil { - return nil, fmt.Errorf("des.NewCipher: %w", err) - } - if len(challenge) < 16 { //nolint:gosec // explicit length check disarms G602 - return nil, fmt.Errorf("vnc auth challenge too short: %d", len(challenge)) - } - out := make([]byte, 16) - block.Encrypt(out[:8], challenge[:8]) - block.Encrypt(out[8:], challenge[8:]) - return out, nil -} - -func reverseBits(b byte) byte { - var r byte - for range 8 { - r = (r << 1) | (b & 1) - b >>= 1 - } - return r -} // diffTiles compares two RGBA images and returns a tile-ordered list of diff --git a/client/vnc/server/server.go b/client/vnc/server/server.go index 27149a225..237a45aa6 100644 --- a/client/vnc/server/server.go +++ b/client/vnc/server/server.go @@ -129,7 +129,6 @@ type connectionHeader struct { type Server struct { capturer ScreenCapturer injector InputInjector - password string serviceMode bool disableAuth bool localAddr netip.Addr // NetBird WireGuard IP this server is bound to @@ -179,11 +178,12 @@ type virtualSessionManager interface { } // New creates a VNC server with the given screen capturer and input injector. -func New(capturer ScreenCapturer, injector InputInjector, password string) *Server { +// Authentication is handled by the dashboard JWT exchange after the RFB +// handshake; the protocol-level VNC password scheme is not supported. +func New(capturer ScreenCapturer, injector InputInjector) *Server { return &Server{ capturer: capturer, injector: injector, - password: password, authorizer: sshauth.NewAuthorizer(), log: log.WithField("component", "vnc-server"), sessions: make(map[uint64]ActiveSessionInfo), @@ -478,7 +478,6 @@ func (s *Server) handleConnection(conn net.Conn) { injector: injector, serverW: capturer.Width(), serverH: capturer.Height(), - password: s.password, log: connLog, } sess.serve() diff --git a/client/vnc/server/server_test.go b/client/vnc/server/server_test.go index 6467aacc8..9776667af 100644 --- a/client/vnc/server/server_test.go +++ b/client/vnc/server/server_test.go @@ -27,7 +27,7 @@ func (t *testCapturer) Capture() (*image.RGBA, error) { func startTestServer(t *testing.T, disableAuth bool, jwtConfig *JWTConfig) (net.Addr, *Server) { t.Helper() - srv := New(&testCapturer{}, &StubInputInjector{}, "") + srv := New(&testCapturer{}, &StubInputInjector{}) srv.SetDisableAuth(disableAuth) if jwtConfig != nil { srv.SetJWTConfig(jwtConfig) @@ -175,7 +175,7 @@ func TestAuthEnabled_InvalidJWT_RejectedBeforeRFB(t *testing.T) { // server must close immediately and the client must see EOF before any RFB // version greeting is written. func TestAuth_NoUnauthBytesPastHeader(t *testing.T) { - srv := New(&testCapturer{}, &StubInputInjector{}, "") + srv := New(&testCapturer{}, &StubInputInjector{}) srv.SetDisableAuth(true) addr := netip.MustParseAddrPort("127.0.0.1:0") // Tight overlay that excludes 127.0.0.0/8 and a non-loopback local IP, so @@ -287,7 +287,7 @@ func TestIsAllowedSource(t *testing.T) { } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - srv := New(&testCapturer{}, &StubInputInjector{}, "") + srv := New(&testCapturer{}, &StubInputInjector{}) srv.localAddr = tc.localAddr srv.network = tc.network assert.Equal(t, tc.want, srv.isAllowedSource(tc.remote)) @@ -296,7 +296,7 @@ func TestIsAllowedSource(t *testing.T) { } func TestStart_InvalidNetworkRejected(t *testing.T) { - srv := New(&testCapturer{}, &StubInputInjector{}, "") + srv := New(&testCapturer{}, &StubInputInjector{}) addr := netip.MustParseAddrPort("127.0.0.1:0") err := srv.Start(t.Context(), addr, netip.Prefix{}) require.Error(t, err, "Start must refuse an invalid overlay prefix") @@ -304,7 +304,7 @@ func TestStart_InvalidNetworkRejected(t *testing.T) { } func TestAgentToken_MismatchClosesConnection(t *testing.T) { - srv := New(&testCapturer{}, &StubInputInjector{}, "") + srv := New(&testCapturer{}, &StubInputInjector{}) srv.SetDisableAuth(true) srv.SetAgentToken("deadbeefcafebabe") @@ -332,7 +332,7 @@ func TestAgentToken_MismatchClosesConnection(t *testing.T) { } func TestAgentToken_MatchAllowsHandshake(t *testing.T) { - srv := New(&testCapturer{}, &StubInputInjector{}, "") + srv := New(&testCapturer{}, &StubInputInjector{}) srv.SetDisableAuth(true) const tokenHex = "deadbeefcafebabe" srv.SetAgentToken(tokenHex) @@ -369,7 +369,7 @@ func TestAgentToken_MatchAllowsHandshake(t *testing.T) { func TestSessionMode_RejectedWhenNoVMGR(t *testing.T) { // Default platformSessionManager() on non-Linux returns nil, so ModeSession // must be rejected with the UNSUPPORTED reason rather than crashing. - srv := New(&testCapturer{}, &StubInputInjector{}, "") + srv := New(&testCapturer{}, &StubInputInjector{}) srv.SetDisableAuth(true) addr := netip.MustParseAddrPort("127.0.0.1:0") diff --git a/client/vnc/server/session.go b/client/vnc/server/session.go index 7433731c7..d2dd88797 100644 --- a/client/vnc/server/session.go +++ b/client/vnc/server/session.go @@ -1,8 +1,6 @@ package server import ( - "bytes" - "crypto/rand" "encoding/binary" "errors" "fmt" @@ -40,7 +38,6 @@ type session struct { serverW int serverH int desktopName string - password string log *log.Entry writeMu sync.Mutex @@ -179,61 +176,19 @@ func (s *session) handshake() error { return s.sendServerInit() } +// sendSecurityTypes advertises only secNone. Authentication and access +// control are layered on top by the dashboard JWT exchange after the RFB +// handshake completes, not by the protocol-level password scheme. func (s *session) sendSecurityTypes() error { - if s.password == "" { - _, err := s.conn.Write([]byte{1, secNone}) - return err - } - _, err := s.conn.Write([]byte{1, secVNCAuth}) + _, err := s.conn.Write([]byte{1, secNone}) return err } func (s *session) handleSecurity(secType byte) error { - switch secType { - case secVNCAuth: - return s.doVNCAuth() - case secNone: - return binary.Write(s.conn, binary.BigEndian, uint32(0)) - default: + if secType != secNone { return fmt.Errorf("unsupported security type: %d", secType) } -} - -func (s *session) doVNCAuth() error { - challenge := make([]byte, 16) - if _, err := rand.Read(challenge); err != nil { - return fmt.Errorf("generate challenge: %w", err) - } - if _, err := s.conn.Write(challenge); err != nil { - return fmt.Errorf("send challenge: %w", err) - } - - response := make([]byte, 16) - if _, err := io.ReadFull(s.conn, response); err != nil { - return fmt.Errorf("read auth response: %w", err) - } - - var result uint32 - if s.password != "" { - expected, err := vncAuthEncrypt(challenge, s.password) - if err != nil { - return fmt.Errorf("vnc auth encrypt: %w", err) - } - if !bytes.Equal(expected, response) { - result = 1 - } - } - - if err := binary.Write(s.conn, binary.BigEndian, result); err != nil { - return fmt.Errorf("send auth result: %w", err) - } - if result != 0 { - msg := "authentication failed" - _ = binary.Write(s.conn, binary.BigEndian, uint32(len(msg))) - _, _ = s.conn.Write([]byte(msg)) - return fmt.Errorf("authentication failed from %s", s.addr()) - } - return nil + return binary.Write(s.conn, binary.BigEndian, uint32(0)) } func (s *session) sendServerInit() error {