d7f9d4966be064685d7a1ec3c1f3e9dcbf3457fe
10
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  ksmithandClaude Sonnet 5
|
e80fe7d634 |
Fix Apple DH auth wire-order bug, add type 33 fallback, fix missing VNC password prompt
Live testing against a real macOS Screen Sharing server surfaced two real bugs, independent of each other: 1. SessionTab::requestConnectOptions() never prompted for a password on VNC profiles (only SSH/RDP) -- every VNC connection went out with an empty password regardless of what the server needed. VNC now gets its own prompt; an empty password is allowed through (unlike RDP's hard requirement) since no-auth VNC servers exist and there's no way to know client-side before the security-type negotiation happens. 2. VncSessionBackend's Apple DH (type 30) response sent the client's public key before the encrypted credentials. Cross-checking against neatvnc's rfb-proto.h (an independent, authoritative reference: both the wire struct definitions and the full server-side verification code, matched field-by-field against this implementation) showed the correct order is credentials first, then public key -- exactly backwards from what was implemented. Fixed, with a new regression test that decrypts the credentials back out using the trailing public-key bytes to derive the shared secret, which would fail if the fields were swapped again. Also adds security type 33 (RSA + AES, src/vnc_apple_rsa_auth.h) as a fallback Apple auth scheme, sourced from the `asyncvnc` PyPI package. Preference when multiple are offered: None > AppleDH(30) > AppleRSA(33) > VNCAuth(2). Neither scheme has been gotten working live yet against the specific macOS Tahoe (26.6.2) server available for testing -- type 30's wire format is now verified correct byte-for-byte against the independent reference above, but the server still rejects it with a generic "Authentication or authorization failure"; type 33 is rejected even earlier, right after the initial host-key request. macOS Tahoe was released after this assistant's knowledge cutoff, so there may be a protocol or permission-model change specific to it that isn't reflected in either reference. Documented as an open issue in docs/PROGRESS.md rather than claimed as working. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
ksmithandClaude Sonnet 5
|
1f026dde70 |
Add Apple Screen Sharing authentication for VNC (security type 30)
Apple's macOS Screen Sharing server doesn't speak standard VNC Authentication -- it uses a Diffie-Hellman key exchange followed by AES-128-ECB-encrypted credentials, security type 30. Apple never published this scheme (it's not part of RFC 6143); this implements the well-established reverse-engineered wire format: the server sends a generator, prime, and its own DH public key; the client generates an ephemeral keypair, derives the shared secret, MD5-hashes it into an AES key, and sends back its public key plus a 128-byte encrypted username+password buffer. The DH/AES math lives in new src/vnc_apple_dh_auth.h/.cpp as a pure, socket-free helper (mirroring vncAuthResponse()'s shape for standard VNC Auth), built entirely on modern EVP_PKEY-based OpenSSL 3.0 APIs -- no deprecated low-level DH_* calls, unlike VNC Authentication's necessary use of classic DES. Reuses Profile::username (already a shared field) since Apple's scheme needs an actual macOS account name, unlike password-only VNC Authentication. Security-type preference when multiple are offered is now None > AppleDH > VNCAuth, since DH+AES is strictly stronger than static- challenge DES. Adds a DH round-trip test (generates a real 512-bit group at test time, computes the response, then independently re-derives the shared secret as the server would and decrypts the credentials back out -- proving self-consistency without needing a hand-computed expected value), a fake-server integration test for the full RFB 3.8 handshake sequencing, and a preference-order test. Not yet live-verified against a real macOS Screen Sharing server -- that's next. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
ksmithandClaude Sonnet 5
|
4fca8fce41 |
Add VNC Tight decoding
Implements RFC 6143's Tight encoding: a compression-control byte (low 4 bits reset one of 4 persistent zlib streams; high nibble selects Fill/JPEG/Basic mode) followed by Fill's 3-byte solid color, JPEG's compact-length-prefixed baseline JPEG covering the whole rectangle (decoded via libjpeg-turbo directly, not QImage's plugin, to avoid a packaging-dependent runtime failure mode), or Basic mode's compact-length-prefixed zlib payload plus a filter (Copy, Palette, or Gradient) applied after decompression. Unlike Hextile/ZRLE, Tight has no internal tiling -- one rectangle is one filtered/compressed unit. The three filters live in vnc_pixel_codecs.h/.cpp alongside the Hextile/ZRLE decoders. Adds find_package(JPEG REQUIRED) + JPEG::JPEG as a new build dependency (confirmed available via libjpeg-turbo on this dev machine). 5 new tests cover Fill, Basic+Copy, Basic+Palette, JPEG (round-tripped through a real libjpeg-turbo-encoded fixture, compared with tolerance since JPEG is lossy), and the stream-reset flag correctly tearing down and reinitializing a targeted stream rather than erroring on stale state. Known, documented gap: this decoder always treats Basic-mode payloads as zlib-compressed; the real protocol allows very small payloads to skip compression, which couldn't be verified with confidence against the RFC text alone and is narrow enough in practice (tiny solid areas are virtually always sent as Fill instead) to leave unhandled for now -- it fails that one rectangle's decode cleanly rather than misinterpreting it silently. The Gradient filter is implemented from the spec description but is the least exercised of the three in this pass. Live-verified against the TightVNC test server that nothing regressed; that server still consistently chose Raw for actual framebuffer content regardless of announced encodings, so Tight's live decode path isn't independently confirmed against a real server here either -- the unit tests are the primary evidence. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
ksmithandClaude Sonnet 5
|
bb022edcf2 |
Add VNC ZRLE decoding
Implements RFC 6143 SS7.7.6: a ZRLE rectangle is a 4-byte compressed length followed by that many zlib-compressed bytes, decompressing to 64x64 tiles each using one of five subencodings (Raw, Solid, packed palette, Plain RLE, Palette RLE). The zlib stream persists for the whole connection rather than being reset per-rectangle or per-update, so VncSessionBackend now owns a lazily-initialized, persistent z_stream torn down only in resetProtocolState() on a fresh connect/reconnect. Since the entire rectangle's compressed data decompresses into memory in one shot, tile parsing is a plain synchronous loop rather than needing its own RfbState values -- only the compressed-length and compressed-data reads are actual protocol states. Tile decoding (the five subencodings, including the continuation-byte run-length encoding shared by two of them) lives in vnc_pixel_codecs.h/.cpp alongside the Hextile decoder, unit-tested with 6 new tests covering each subencoding plus a persistence test that splits one continuous deflate stream across two separate FramebufferUpdate messages -- it only decodes correctly if the connection's inflate stream is retained between them. Adds a top-level find_package(ZLIB REQUIRED) + ZLIB::ZLIB link (previously only pulled in transitively via vendored FreeRDP's own smartcard-emulation feature, which happened to have it enabled but shouldn't be relied on for that). Live-verified against the TightVNC test server that nothing regressed (connect, cursor, clipboard); that server consistently sends Raw for actual framebuffer content regardless of announced encodings, so Hextile/ZRLE's live decode path isn't independently confirmed against a real server -- the unit tests are the primary evidence here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
ksmithandClaude Sonnet 5
|
e49fa0cf26 |
Add VNC Hextile decoding
Implements RFC 6143 SS7.7.4: rectangles announced as Hextile (type 5) tile the update into 16x16 blocks, each either raw pixels or a background fill plus an optional list of foreground/individually- colored subrects, with background/foreground persisting across tiles within one rectangle when not re-specified. The pure byte-decode logic (tile metadata, subrect list) lives in new src/vnc_pixel_codecs.h/.cpp, kept separate from VncSessionBackend's wire-sequencing state machine so it's unit-testable without a socket -- the pattern the plan calls for continuing into the ZRLE/Tight work still ahead. Adds 5 fake-server tests covering a raw tile, a background-only solid fill, uncoloured and individually-colored subrects, and a 4-tile rectangle proving background persistence and correct tile-cursor wraparound. Verified against the live TightVNC test server (connect, frame, cursor, clipboard all still work); that particular server always chose Raw for the actual framebuffer content during this session, so Hextile's real-world path isn't independently confirmed live -- the unit tests are the primary correctness evidence for this phase. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
ksmithandClaude Sonnet 5
|
6da9dc6ca5 |
Add VNC support (Milestone 6, issue #3)
Implements RFB (RFC 6143) directly against QTcpSocket. No permissively licensed VNC client library exists to vendor the way FreeRDP was for RDP: LibVNCClient is GPLv2, gtk-vnc is LGPL but GTK-tied, and vendoring either would force a licensing decision on the whole (MIT) project. This is from-scratch protocol code instead, threaded like SshSessionBackend (a QObject on its own QThread driven by Qt's own async socket signals) rather than RdpSessionBackend's manual worker-thread/blocking-loop pattern, since QTcpSocket is already async. Scope, matching SessionTab's existing SSH/RDP dispatch pattern (session_backend_factory.cpp, session_tab.cpp's widget construction and signal wiring) and VncDisplayWidget mirroring RdpDisplayWidget's scale-to-fit rendering: - Protocol handshake: RFB 3.3/3.7/3.8 negotiated explicitly (the SecurityResult message only exists in 3.8; pre-3.8 servers signal auth failure by closing the socket, which the disconnect handler accounts for) - VNC Authentication (DES challenge-response, via OpenSSL's classic DES API) and no-auth security types - Raw + CopyRect framebuffer decoding into a persistent QImage, requesting a fixed 32bpp format whose byte layout matches QImage::Format_RGB32 directly (same zero-conversion trick RdpSessionBackend uses for FreeRDP's GDI buffer) - Keyboard (Qt key -> X11 keysym, including the Unicode-beyond-Latin-1 keysym convention) and mouse/wheel input forwarding Explicit non-goals for this pass (see docs/PROGRESS.md for the full list): Apple's Screen Sharing auth (so this can't yet reach macOS's built-in VNC server), compression encodings beyond Raw/CopyRect, dynamic resize, remote cursor shape sync, clipboard sync. 19 unit tests (tests/test_vnc_session_backend.cpp): pure-function coverage (DES key prep verified against an independently documented test vector for password "COW", X11 keysym mapping, socket-error mapping) plus state-machine coverage against a scripted in-process fake RFB server covering all three protocol-version handshake shapes, auth success/ failure, unsupported security types, and pixel-accurate Raw decoding. That harness caught a real re-entrancy bug: QAbstractSocket::abort() synchronously re-emits disconnected() before returning, so failConnection() calling it was silently letting a second, generic disconnected-socket handler overwrite an already-correct, specific error message. Also verified live against a real, independently implemented VNC server (TightVNC on Windows): connect with VNC Authentication, correct framebuffer dimensions and pixel data, clean disconnect, reconnect. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
ksmithandClaude Sonnet 5
|
dbcc20d155 |
Add File -> Import from mRemoteNG...
Parses mRemoteNG's confCons.xml export format directly -- schema verified against mRemoteNG's own XmlConnectionsDeserializer.cs source and a real exported sample, not guessed at. Maps nested <Node Type="Container"> folders and <Node Type="Connection"> entries onto OrbitHub profiles: RDP stays RDP, SSH1/SSH2 collapse to OrbitHub's single SSH protocol, anything else (VNC, Telnet, HTTP, PowerShell, ...) is skipped and listed in the import summary rather than silently dropped. Passwords are never read, not even for the common case where they're technically readable without a master password (mRemoteNG only encrypts the Password attribute itself, everything else -- Hostname, Username, Domain, Protocol -- is plaintext). A FullFileEncryption="true" export encrypts the whole node tree instead and genuinely can't be read without the user's master password; that case is detected and refused with a clear message rather than failing confusingly. The parser (src/mremoteng_importer.h/.cpp) is a pure function decoupled from any file/UI I/O, matching this session's established pattern of keeping business logic separately testable from the Qt Widgets shell that calls it (ProfilesWindow::importFromMRemoteNG() is the thin wrapper: QFileDialog, call the parser, write results via ProfileRepository, show a summary). 11 test cases against realistic sample XML. Hit a real moc gotcha along the way: a literal "//" inside a raw string literal (the xmlns URL) makes moc's lexer think a line comment started there, silently desyncing its parse so it never finds the QObject-derived test class at all (no error, just a missing vtable at link time). Fixed by moving the XML fixtures into a plain non-QObject header moc never scans, split across two adjacent literals as a second safeguard. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
ksmithandClaude Sonnet 5
|
04ce6f7904 |
Add RdpSessionBackend test coverage (#1)
RdpSessionBackend can't reasonably get the same fixture-driven state-machine tests SshSessionBackend got: it's driven by FreeRDP's own event loop and a raw worker thread against a real freerdp_connect(), not a QProcess we can point at a stand-in binary. What it does have is a large amount of pure, regression-prone logic -- exactly the kind that already caused a real historical bug here (the X11-keycode/PC-AT-scancode mixup fixed in Milestone 7) -- so that's what gets covered instead. Twelve functions promoted from free functions / private members to public statics purely so tests can call them without a live connection: security-mode/performance-profile normalization, the HiDPI scale-value mapping, desktop-size clamping, both scancode-mapping functions, and the five FreeRDP error-code interpretation functions. UINT32 is surfaced as quint32 in the public signatures to keep FreeRDP/WinPR types out of the header, matching how rdp_freerdp* is already only forward-declared there. 27 test cases, including a couple of direct regression guards: verifying scancodeFromNativeScanCode() is a faithful passthrough to FreeRDP's X11 table (not a reimplementation), and that it does NOT reproduce the old "X11 keycode treated as PC/AT scancode" bug for a documented example key. This closes out #1's originally scoped work (CTest wiring, ProfileRepository, SshSessionBackend, RdpSessionBackend coverage). Deeper state-machine coverage for the two session backends remains future work if ever needed, but isn't blocking here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
ksmithandClaude Sonnet 5
|
9842a44de0 |
Add SshSessionBackend test coverage (#1)
Adds two kinds of coverage, continuing #1's remaining scope: 1. Pure-function tests for mapSshError() and escapeForShellSingleQuotes(), promoted from private members to public statics purely so tests can call them without spinning up a process. escapeForShellSingleQuotes() is the actual security boundary for password auth (it's what stops a password containing a single quote from breaking out of the askpass script's quoting), so it gets a real adversarial test, not just a happy-path one. 2. State-machine tests (connect -> Connected, auth failure -> Failed with the right mapped message, connection refused -> Failed, input round-tripping, reconnect) driven against tests/fixtures/fake_ssh.sh, a small controllable stand-in for the real ssh binary, instead of a real network/SSH server. This needed one small testability seam: a new constructor overload that overrides the launched program ("ssh" in production, the fixture script in tests). POSIX-only for now: the fixture is a shell script, so the state-machine tests QSKIP on Windows until an equivalent fixture exists there; the pure-function tests run everywhere. RdpSessionBackend coverage is still open -- it's a bigger lift again (FreeRDP's own event loop, not just a QProcess), left for a follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
ksmithandClaude Sonnet 5
|
3fab2f9de3 |
Wire up CTest and add unit test coverage for ProfileRepository
Adds a Qt6::Test-based unit test target (tests/test_profile_repository.cpp, 21 cases), gated behind an ORBITHUB_BUILD_TESTS option that no-ops gracefully if Qt6::Test isn't available, so it can't break app-only builds. Covers profile CRUD, validation rules, search/sort, tag normalization, and folder handling, each against an isolated temporary SQLite file (new ProfileRepository(databasePathOverride) constructor overload added for exactly this). Caught and fixed a real bug along the way: normalizedTags()'s result was bound directly without the nonNullTrimmed() null-guard every other field already uses, so creating a profile with no tags at all hit the `tags NOT NULL` constraint and silently failed -- including via the Import Profiles feature for any export where a profile has no tags key. Partial progress on #1 (RdpSessionBackend/SshSessionBackend state-machine coverage still open -- much larger lift, needs a testability pass on those backends first). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |