rustls 0.23.45, uuid 1.26.1, russh 0.63.3, tokio-rustls 0.26.5, aes 0.9.3,
rcgen 0.14.10, tower-http 0.7.1, toml 1.1.6. All are patch level within the
existing Cargo.toml constraints, so this is a lockfile-only change.
Two transitive majors ride along: rcgen takes pem 3.0.6 to 4.0.0, and rustls
takes aws-lc-rs 1.17.3 to 1.18.1 with aws-lc-sys 0.43.0 to 0.45.0. Release
build, 273 tests, clippy and cargo audit all clean.
Closes#215, #216, #217, #219, #224, #225, #226, #227.
Supersedes the cherry-picked 2449aa5 (install wrappers once), which stopped
the recursion crash but left passthrough silently dead after the RDPGFX
channel reconnect that xrdp performs at login. b3b2cce guards per callback
on what is actually installed, covering both failure directions.
Also brings 869c738, which releases the sync gate at the pipeline depth
waitForPending() gates on rather than at zero. Releasing only at zero meant
a continuously decoding session never drained fully, so every sync waited
out its 200ms timeout: choppy audio and a framerate throttled by guacd
pacing against an inflated lag figure.
# Conflicts:
# patches/004-h264-passthrough.patch
Documents the map-group subcommand in the group-to-role mappings section:
creating a mapping, the one-mapping-per-group rule and the --force re-map,
and that mappings are applied at the user's next OIDC login rather than
immediately, which matters in both directions when revoking access.
Documents the new cli_audit_log table: what is and is not recorded, that
the actor prefers SUDO_USER so sudo names the person rather than root,
that only successful actions are logged, how to read it with sqlite3, and
the 90 day retention.
Also corrects two pre-existing errors found while writing the above:
- deployment-guide.md showed an [oidc.group_role_mappings] config table.
No such config field exists, so anyone following the guide would add it
and silently get no mappings at all. Replaced with the CLI commands that
actually create them. The same block had session_ttl_secs under [oidc];
the real key is auth_session_ttl_secs and it is top level.
- roles-and-access-control.md documented an enable-user subcommand that
does not exist. Re-enabling is done from the Admin page or the API, so
the doc now says that, with a warning about disabling your own account.
Follow-up to the map-group subcommand.
oidc_group is UNIQUE, so mapping a group that already had a mapping failed
on the constraint and surfaced a raw "UNIQUE constraint failed" error, with
no way to change a mapping from the CLI at all. The command now looks the
group up first: an unchanged mapping is a no-op, a conflicting one explains
itself and points at the new --force flag, and --force re-maps through
db::update_group_mapping. This matches the API, which already answers 409
rather than 500 on a duplicate.
CLI commands hold the database directly and so bypass the API's
authenticated audit trail, leaving role grants and account deletions made
from a shell with no record anywhere. A new cli_audit_log table and
db::log_cli_event record the admin, user-role and group-mapping mutations,
attributed to SUDO_USER where present so an action taken through sudo names
the person rather than root. Entries are headline only, never API keys or
passwords, and are covered by the existing retention cleanup.
Tests: assert_cmd's Command is aliased so it no longer shadows the clap
Command enum that use super::* brings into scope, and the CLI tests run in
a per-test scratch directory removed on drop rather than writing a database
into tests/cli inside the repo. Adds coverage for role rejection, the
duplicate path, --force re-mapping, and the audit row.
This reverts 8988165, restoring the unconditional full-range BT.709
conversion. The commit it undoes identified a real inconsistency and
resolved it toward the wrong side.
Its reasoning was that the 4:2:0 path never reaches the shader, so the
combined 4:4:4 picture converted differently from the 4:2:0 one beside
it, and that adopting the decoded frame's colour space would make the two
agree. The two did then agree -- as limited range, on hosts that send
full range. Before it, 4:4:4 was correct and 4:2:0 was crushed; after it,
both were crushed. The symptom it describes, "the combined 4:4:4 picture
renders visibly flatter", was the correct picture beside a crushed one.
What every RDP host actually sends is now read out of the SPS rather than
assumed. Windows writes video_full_range_flag=1 with no colour
description. The xrdp fork writes it with a complete BT.709 description.
Stock xrdp 0.10.6 omits the video_signal_type block altogether -- it
passes x264 no VUI parameters -- while naming its own conversion
XRDP_yuv444_709fr. All three encode full-range BT.709, which is what
MS-RDPEGFX defines.
What the browser reports for those streams is a different question.
Chrome's hardware decoder ignores a bare range flag with no description
beside it and reports limited for a host that plainly declared full, so
a shader that adopts the frame's colour space expands 16-235 to 0-255 on
full-range samples: blacks crushed to zero, whites clipped, chroma
over-saturated by 255/224.
The inconsistency this reinstates is real and is not being dismissed: a
session can render 4:4:4 correctly and 4:2:0 crushed. One path right
beats both wrong, and the repair belongs upstream of the browser --
completing the host's SPS on the wire, so that neither path has to guess
and third-party clients benefit too. That follows separately; it is
server-side and does not belong in this PR.
The coefficients are exact BT.709 rather than the 8-bit approximations
the original cited from FreeRDP's prim_internal.h. Under passthrough
FreeRDP never decodes these streams, so matching its rounding was never
the point.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013Rxg4cT87G9czw4nUxKhHG
waitForPending() holds the Guacamole sync response while pendingDecodes
exceeds MAX_PIPELINE_DEPTH, but resolveIfIdle() only fired its resolvers at
pendingDecodes <= 0. So once the backlog crossed the depth it had to drain
completely before a sync could go out, and a session decoding continuously
never reaches zero. Every sync therefore waited out its full timeout,
logging "sync wait timeout" and reporting that much processing lag however
fast the client was really going.
guacd paces frames against the client's processing lag, so the session runs
slow while the client is not in fact behind. The function's own
documentation already said the callback fires "when the backlog is within
the allowed pipeline depth"; only the implementation disagreed.
It shows up first on AVC444, where a picture is two access units and the
backlog therefore runs twice as deep for the same frame rate, making it far
less likely to touch zero between frames. That presents as AVC444 being
expensive rather than as a threshold mismatch, so an A/B that varies the
server between AVC444 and AVC420 while holding the client constant cannot
tell the two apart.
The colour-space change introduced "float y" for the scaled luma, but y is
already the int pixel row in that scope. GLSL rejected the redefinition and
then every arithmetic line that mixed the two, so the shader failed to
compile and the renderer fell back to 4:2:0 for the whole session:
ERROR: 0:89: 'y' : redefinition
ERROR: 0:93: '+' : no operation exists that takes highp int and highp float
Name it luma.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The combining shader hardcoded BT.709 at full range, citing the coefficients
FreeRDP decodes these streams with. But in passthrough guacd never decodes --
the browser does, and the 4:2:0 path never reaches the shader at all: that
VideoFrame is drawn by the browser, which applies the frame's own colour
space including the limited-range expansion of 16-235 to 0-255.
So the two paths converted differently on the same session. An H.264 stream
carrying no VUI signalling is limited range, and treating it as full leaves
black at RGB 16 and white at 235, with chroma unscaled to match: the combined
4:4:4 picture renders visibly flatter than the 4:2:0 one beside it, depending
only on which codec the server chose.
Take the range and matrix from the decoded frame's colorSpace instead, read
once from the first main view, and default to limited range BT.709 when the
decoder reports nothing -- which is what the absence of signalling means.
VideoFrame.colorSpace was available in scope and never consulted.
The previous commit stopped guac_rdp_rdpgfx_channel_connected() from wrapping
its own wrapper, which fixed the recursion crash, but it guarded on
orig_surface_command being NULL:
if (rdp_client->settings->enable_h264
&& rdp_client->orig_surface_command == NULL) {
orig_surface_command lives on guac_rdp_client, so it is per-connection, while
the handler it protects is per-channel-instance. FreeRDP's
gdi_graphics_pipeline_uninit() sets SurfaceCommand to NULL and
gdi_graphics_pipeline_init() assigns gdi_SurfaceCommand back
(libfreerdp/gdi/gfx.c:1864, :1911), so a reconnect restores FreeRDP's own
handler -- and the once-only guard, seeing a non-NULL orig_surface_command
from the first install, declines to wrap it again. Passthrough is then dead
for the rest of the session.
xrdp reconnects the channel at login, for the display resize the previous
commit already identified, so this fired on essentially every xrdp session.
It was silent: the surface commands still reached FreeRDP's GDI decoder and
guacd re-encoded them as JPEG/WebP, so the desktop rendered correctly at
~100% of a core while no H.264 reached the browser. A trace on xrdp 0.10.6
showed one keyframe forwarded, the channel unloading 52ms later, and no
"H.264 passthrough enabled" line after the reconnect that followed.
Guard on what is actually installed, per callback. The two are restored on
different schedules -- gdi_graphics_pipeline_init() assigns SurfaceCommand
but never touches CapsConfirm -- so after a reconnect CapsConfirm is still
ours while SurfaceCommand is FreeRDP's again. A single guard keyed on
SurfaceCommand would pass and then point orig_caps_confirm at
guac_rdp_gfx_caps_confirm itself, reaching the same stack exhaustion by the
other callback.
With this, the same host forwards 807 AVC420 deltas and 3 keyframes over 60s
of desktop use, with guacd at 3.3% of a core.
guac_rdp_rdpgfx_channel_connected() installed the SurfaceCommand and
CapsConfirm wrappers unconditionally, every time it ran. It runs again when
the RDPGFX channel reconnects, which xrdp provokes with the display resize it
performs at login, and the second pass saved the wrapper the first pass had
installed:
rdp_client->orig_surface_command = (void*) rdpgfx->SurfaceCommand;
rdpgfx->SurfaceCommand = guac_rdp_gfx_surface_command;
leaving orig_surface_command pointing at guac_rdp_gfx_surface_command itself.
Any surface command that is not H.264 falls through to orig(context, cmd),
re-enters the wrapper, and recurses until the stack is exhausted and the guacd
child dies -- a white screen in the browser, with no "Client terminated
successfully" in the log and the segfault visible only outside a container.
H.264 commands take skip_decode and never reach that call, so a server sending
nothing but H.264 is unaffected. A stock xrdp is the opposite case: one trace
showed 7440 planar and 12 progressive surface commands against 437 AVC420, so
it dies on the first frame. Distro xrdp packages (0.10.1 on Ubuntu) are built
--enable-rfxcodec with no H.264 encoder at all, so they send planar for
everything and hit this immediately.
The wrappers were also installed for every GFX session regardless of
enable_h264, so this reproduced with RFX alone and H.264 unticked -- no
H.264-capable server required.
Both halves are fixed: install only when the connection asked for H.264, and
only when nothing is installed yet. guac_rdp_client is guac_mem_zalloc'd, so
the NULL check is sound on first entry.
guac_rdp_rdpgfx_channel_connected() installed the SurfaceCommand and
CapsConfirm wrappers unconditionally, every time it ran. It runs again when
the RDPGFX channel reconnects, which xrdp provokes with the display resize it
performs at login, and the second pass saved the wrapper the first pass had
installed:
rdp_client->orig_surface_command = (void*) rdpgfx->SurfaceCommand;
rdpgfx->SurfaceCommand = guac_rdp_gfx_surface_command;
leaving orig_surface_command pointing at guac_rdp_gfx_surface_command itself.
Any surface command that is not H.264 falls through to orig(context, cmd),
re-enters the wrapper, and recurses until the stack is exhausted and the guacd
child dies -- a white screen in the browser, with no "Client terminated
successfully" in the log and the segfault visible only outside a container.
H.264 commands take skip_decode and never reach that call, so a server sending
nothing but H.264 is unaffected. A stock xrdp is the opposite case: one trace
showed 7440 planar and 12 progressive surface commands against 437 AVC420, so
it dies on the first frame. Distro xrdp packages (0.10.1 on Ubuntu) are built
--enable-rfxcodec with no H.264 encoder at all, so they send planar for
everything and hit this immediately.
The wrappers were also installed for every GFX session regardless of
enable_h264, so this reproduced with RFX alone and H.264 unticked -- no
H.264-capable server required.
Both halves are fixed: install only when the connection asked for H.264, and
only when nothing is installed yet. guac_rdp_client is guac_mem_zalloc'd, so
the NULL check is sound on first entry.
Restores GfxAVC444=TRUE, skips the server-side H.264 decode, forwards both
AVC444 views (new 'view' field on the h264 instruction + Yuv444.js WebGL2
4:4:4 reconstruction), and fixes blank H.264 recording playback. Review
findings folded in (f16eed7). guacd CPU ~100%/90% of a core -> ~2%.
russh 0.63 changed the check_server_key callback to take
&PublicKeyOrCertificate; resolve it to a PublicKey via .public_key() in
both the tunnel and probe handlers, and widen the russh constraint to
0.63. Closes the retargeted dependabot PRs #204 (uuid) and #205 (russh).
The CI `cargo audit` step was failing on RUSTSEC-2026-0258 (h2 unbounded
empty DATA frames, a HTTP/2 DoS), pulled in transitively via reqwest/hyper.
Bump h2 0.4.15 -> 0.4.19 (patched) and chacha20 0.10.1 -> 0.10.2 (the prior
version was yanked).
Adds a per-entry RDP keyboard layout, wiring guacd's `server-layout`
parameter (previously hardcoded to empty) through the address-book entry,
the connect request, and the guacd handshake, with a dropdown in the
connections editor and a migration mapping from Guacamole's `server-layout`
on import. Empty keeps guacd's default (en-us-qwerty).
list_credential_variables walked from top-level folders using
check_folder_access + continue, short-circuiting the whole subtree when an
ancestor was not directly accessible. An operator granted access at a
subfolder level saw "No credential variables" in My Credentials even though
the connections tree showed the entry (the tree is descendant-aware). It now
descends past inaccessible ancestors, collecting variables only from folders
the user can directly access. Adds regression tests.
CI's runners updated to clippy 0.1.98, which enforces two lints that
existing code tripped under -D warnings. Rewrite the two late-init
if/else blocks in the SSH tunnel builder as tuple `let` bindings
(needless-late-init), and allow result-large-err on check_folder_access,
whose Err is an axum Response by design.
Review notes from the upstream issue thread:
- The auxiliary-view snapshot was sized to the auxiliary frame's own
dimensions. The v1 chroma layout pads that view to a multiple of 16
rows, so the snapshot could be taller than the picture the renderer
draws; with no rects the whole thing is blitted, painting a blank
strip below the image. Size it from the rendered canvas instead.
- The per-frame watchdog was cleared before combineFrame() ran, so the
4:4:4 path had no timeout despite the comment saying otherwise: a
copyTo() that never settled would hold the ordered display queue with
nothing able to release it. Clear it once the frame is done with
instead -- in the finally for the synchronous paths, and after the
copy settles for the combine.
- Drop the unreachable view != 0 branch after the combine hand-off.
- '0' no longer coerces to false in override(), so ?h264ChromaFilter=0
sets threshold zero rather than switching the filter off. Boolean
overrides are unaffected: they coerce, and 0 is falsy.
- Drop the stale stats() reference from client.html; the decoder has no
such method.
Recordings capture the raw protocol stream, so a recording of an H.264
session contains h264 instructions. recordings.html loaded neither
H264Decoder.js nor Yuv444.js, so playback of those sessions rendered
nothing.
H.264 passthrough had two independent defects. On Windows hosts it
negotiated no H.264 at all, and where it did engage it delivered about
half the saving it should.
GfxAVC444 = FALSE was set to make the client advertise AVC420-only,
which is not what the combination does. FreeRDP gates the entire RDPGFX
V10+ capability block on
!GfxH264 || GfxAVC444
(rdpgfx_main.c:254, rdpgfx_send_caps_advertise_pdu), so with GfxH264 set
and GfxAVC444 clear no V10 capability set is sent at all -- the client
advertises only V8 and V8.1. Windows will not offer H.264 at V8.1: it
falls back to CLEARCODEC and CAPROGRESSIVE, which guacd then decodes and
re-encodes as JPEG/WebP, exactly the cost the feature exists to avoid.
The display stays correct, so the regression is invisible without
measuring CPU or counting codecs on the wire. Over a minute of 1080p
video on Windows 11 Pro: 4050 CLEARCODEC + 1447 CAPROGRESSIVE + zero
H.264 frames before, all frames codec 15 after. xrdp is unaffected,
which is why this went unnoticed -- it accepts AVC420 at V8.1 and never
needed the V10 capsets.
AVC444 was disabled because forwarding it was broken, not because
AVC420 was wanted: only bitstream[0] reached the client, rendering as a
green/magenta split. Forwarding one view cannot work -- both bitstreams
decode through the same H264_CONTEXT in FreeRDP's avc444_decompress, so
they are one H.264 sequence with alternating views. Dropping either
breaks reference frames for the other, and the decoder reports no error
because what arrives is well formed. Both views are now forwarded,
tagged with a view field on the h264 instruction; the client decodes
both, draws only view 0, and combines the auxiliary view's chroma into
full 4:4:4 in a WebGL2 pass, falling back to 4:2:0 if it cannot.
The GDI decode is now skipped for captured H.264 commands. Previously
the wrapper always called through to the original handler, so only the
re-encode was saved. RemoteFX, planar and progressive still decode
normally, so a server mixing codecs within a frame stays correct.
guacd CPU with 1080p video playing, sampled per-thread over 30s:
xrdp (AVC420) ~100% of a core -> 2.0%
Windows 11 (AVC444) 90.6% of a core -> 2.1%
Configuration is unchanged: the per-connection enable-h264 argument is
the only switch, and there are no environment variables.
Maps Apache Guacamole passthrough tokens (${GUAC_USERNAME} etc.) to rustguac
credential variables during import, so imported connections arrive wired to
My Credentials. Repeatable FROM=TO; applied to the username, password,
domain, and private_key fields. Adds tests and docs.
Disk-based rotation deleted the oldest .guac unconditionally until statvfs
usage fell below max_disk_percent. Recordings for live sessions are held
open by the recording tee, so unlinking one frees no space and statvfs never
drops: the loop then deleted every recording in the directory in a single
pass while the disk stayed full (a production host lost all recordings this
way).
rotate() now excludes live-session recordings (SessionManager::
active_recording_paths) and stops if a deletion frees no space. Adds
regression tests.
The builder stage compiles the guacd SPICE plugin (--with-spice + the
-dev lib), but the runtime stage never installed the SPICE runtime
shared library. libguac-client-spice.so was present yet failed to
dlopen (libspice-client-glib-2.0.so.8 missing), so guacd reported
"Support for protocol spice is not installed". Add the runtime lib
to the runtime stage; apt pulls the rest of the chain (gstreamer,
opus). Bare-metal installs were unaffected. Broken since SPICE was
vendored (c2c5679).
build-deb.sh had lost its guacd pin during a patch rebase and was cloning
apache/guacamole-server master (--depth 1, latest) and applying patches
best-effort with git apply --check, silently skipping any that no longer
applied. The sol1 apt CI runs build-deb.sh in a fresh container, so its
.deb built guacd from whatever upstream master was that day, with patches
possibly dropped - a non-reproducible build that diverged from the pinned
release.yml and install.sh paths and shipped a broken guacd.
Restore GUACD_COMMIT=6719b20d (the commit the patch set is rebased onto,
matching release.yml and install.sh), build from a fresh pinned clone into
a throwaway dir (leaving any local ../guacamole-server checkout untouched;
GUACD_SRC_OVERRIDE reuses one for local patch iteration), and apply patches
strictly so a non-applying patch fails the build instead of silently
shipping without it.
Refresh Connections (credentials banner/pills, Credentials nav, larger
folder chevron), and the RDP/VDI/Web/SSH-tunnel editors, plus the
Sessions/Recordings/Reports/Admin pages (real data sanitised). Add a new
My Credentials screenshot (scroll/filter/collapse, masked fields). Drop
the stale version note. tokens_view and recordings_player_histogram are
left as their prior captures.
- configuration.md: [vault_shared]/[vault_local] backends, the DR rationale,
user_credentials_default_scope, and the VAULT_SHARED/LOCAL_SECRET_ID env vars.
- credential-variables.md: Credentials nav link, scroll/filter/collapsible
editor, resumable focused prompt, needed-pill + banner, and the shared/local
per-credential scope with its outage trade-off.
- migration.md: vault-migrate runbook (dry-run, copy, cutover ordering).
- CLAUDE.md: multi-Vault backend summary under the Vault section.
Dismissing the "N credentials to set up" banner now persists in
localStorage instead of a page-lifetime flag, so it stays gone across
refreshes. It records exactly which credentials were dismissed and
re-appears only if a genuinely new unset credential shows up — so a
partial save that shrinks the set won't re-nag, but a new requirement
still surfaces.
The collapsible group caret used the small-triangle glyph (U+25BE) like
the old folder chevron did, so it read faint. Switch to the full-size
U+25BC and bump its size, matching the folder-tree chevron.
The credential editor rendered a flat list inside a modal that scrolled as
a whole, so past a few variables the Save/Close buttons and context
scrolled off and it got unusable. Restructure it: the modal header (title,
description, filter box) and the Save/Close footer are pinned and only the
credential list scrolls. Add a filter box (shown once there are more than a
handful of variables) that live-filters rows by name/domain, and group each
domain into a collapsible section. Focused single-connect mode is unchanged
(few rows, no filter box).
Fetch the user's set credential variables on load and cross-reference
against each entry's credential_variables (already on EntryInfo). Entries
that reference variables the user hasn't set get a "credentials needed"
pill, and a dismissible banner reports how many credentials are still to
set up (aggregated across the search index). Both link to the editor, and
refresh after a save. Turns the old reactive "connect fails, then hunt for
the menu" flow into something visible up front.
When a connect fails for missing credential variables, open the My
Credentials editor focused on just the missing variables ("credentials
needed to connect to X") and, on save, re-run that exact connect instead
of dead-ending the user in the full editor with a toast. openMyCredentials
now takes { onlyVars, heading, onSaved }.
Also promote My Credentials to a visible "Credentials" nav link (kept in
the Settings menu too) so people can find it without hitting a failure
first.
Each per-user credential variable can live in the shared backend (fleet-
wide) or the local backend (this instance) - its location is the scope.
Reads merge both backends (local wins); writes partition by a per-key
scope sent from My Credentials, moving a key between backends when its
toggle changes and honouring "leave blank to keep". A "Shared across
sites" toggle appears per credential.
Quick-start preserved: with one Vault the shared and local cells alias the
default, creds_split() is false, the toggle is hidden, and everything is
written to the single store in one call - byte-identical to before. New
config user_credentials_default_scope (default "local") seeds new toggles.
The write partition is a pure function with unit tests covering single-
store, blank-keeps-existing, scope routing, cross-scope moves, and the
default-scope fallback.
list_all_folders now returns (folders, unavailable_scopes); a scope whose
dedicated backend is down is reported instead of silently dropped, and
GET /api/addressbook includes unavailable_scopes. The Connections tree
renders a non-interactive greyed "<scope> connections unavailable" node
so a partial outage is visible while other scopes stay usable; the 15s
/api/me poll refreshes it.
Single-Vault is unaffected: every scope aliases the one backend, so
unavailable_scopes is always empty while it is up (full outage still
returns 503 and the existing banner). Adds a test asserting a lone
[vault] block is sufficient (quick-start invariant).
Dave Kempe