Windows RDP without GFX rounds the requested width down to a multiple
of 16, so a 1710-wide window gets a 1696-wide desktop. The uniform fit
in scaleDisplay() then keeps the scale at 1 (the height matches) and
the missing 14px shows as a strip of bare page down the right edge.
After the uniform fit, stretch the display element by the remaining
fraction on each axis when it is at most 2.5%, and divide the same
factors out of pointer coordinates before sendMouseState. A larger gap
is a real aspect mismatch and keeps the letterbox. Multi-monitor
pop-outs reset the fill.
Fixes#251
A browser test harness under tests/e2e: serve.mjs serves the real static
tree over HTTP and lib/api.mjs answers /api/* from a small fixture address
book, recording every request the page makes, so the shipped pages run
against data the test controls without Vault, guacd or a session. CI gains
an e2e job that installs Chromium and runs the suite.
The first spec pins #252: at 1710px the connections, recordings and reports
tables fit inside the viewport and nothing scrolls sideways.
The 1800px breakpoint set a tighter page gutter on the connections page
only, so switching to Sessions or any other tab snapped the content
sideways. Move it to rustguac.css so every page moves together.
Part of #254
The 18px tables with 20px cell padding ran past the right edge on a
1710px MacBook display, the recording link cut off. Same treatment as
the connections table: 16px cells with tighter padding, 14px headers,
actions kept on one line. Nothing is hidden.
Part of #254
The 18px table with 20px cell padding and a separate cell each for
edit, clone and delete overran the main column by ~300px on a 1710px
MacBook display, cutting "delete" off the right edge.
Put the three admin actions in one cell, and add a max-width: 1800px
breakpoint on the connections page: 20px page padding, 240-280px
sidebar, 16px cells with tighter padding, 14px headers, a 110px
Connect button and a smaller folder title. The icon-only "open in new
window" button keeps its compact size. Nothing is hidden; at 1710px the
table fits with about 250px to spare.
Fixes#252
With fullscreen-on-connect, the first mousedown after CONNECTED is the
browser's fullscreen gesture, but Guacamole.Mouse forwarded it to guacd
as well, and the transition that followed swallowed its release. guacd
was left with the button held and the terminal selected text on every
pointer move; the same with a key press as the gesture.
Stop the gesture event before the display's handlers see it, and on
entering fullscreen release everything explicitly: keyboard.reset() and
a mouse state with no buttons at the last pointer position, so a release
lost for any other reason during the transition cannot stick either.
Fixes#256
The Content-Security-Policy had no img-src, so default-src 'self'
governed images and refused the data: URIs the Guacamole client draws
every tile from on browsers without WebCodecs ImageDecoder. Safari and
Firefox rendered SSH and RDP sessions black with only rect fills and the
cursor cell showing; Chrome decodes straight from the stream and never
fetches a URL, which hid it. The same directive blocks the remote mouse
cursor (cursor: url(data:...)) and blob: image URLs.
Move the policy into a documented constant, add img-src 'self' data:
blob:, and add a unit test that fails if img-src loses either source.
Document the proxy-side header replacement for releases up to v1.10.5.
Fixes#249
VeNCrypt, RealVNC and macOS Screen Sharing need a username as well as a
password. Pass it to guacd's VNC `username` arg and expose it in the
connection editor, the connect-time credentials prompt and ad-hoc
sessions.
(cherry picked from commit fac8d0aec5)
Folder access was by OIDC group only, so giving one person one VM meant
a group per person. Folders now also take allowed_users, a list of
login emails, next to allowed_groups. A user can open a folder if a
group matches or their email is listed (ignoring case). They need not
have logged in yet: access is decided from their identity when they
use rustguac. The list lives in the folder's config in Vault, so it
applies on every instance sharing that Vault.
Every folder access decision goes through resolve_folder_access, which
now takes the user (email and groups) rather than just groups, so the
compiler enforced the change at every call site: the tree, listings,
search, credential variables and connect.
The tree now also shows a folder the user cannot open when it leads to
one they can, at the top level and in subfolder listings, as the code
already intended one level down. Without that, a grant on users/jsmith
was unreachable from the tree when users itself was admin-only (this
also applied to group grants). Such a folder is listed without entries,
and opening it says it has nothing shared with you directly.
The allowed_users list is only returned to admins (folder config);
listings that operators read leave it out, so a user does not learn who
else has access. Saving a folder without the field keeps the stored
list, so older clients cannot wipe it. Entries are trimmed, repeats
dropped, and anything with spaces or control characters refused, up to
500 per folder. The folder dialog gains an "Allowed users" box, and the
bulk move dialog's access warning compares users as well as groups.
Docs: folder access rewritten, including the correction that a folder
with no allowed groups is admin-only (the docs said it was open to
every authenticated user), and the API reference.
Requested by @3zAlb.
Every connect checks the target against its network allowlist, and the
hostname was resolved with the blocking system resolver straight from
the async request handler. With a DNS server that does not answer
("Temporary failure in name resolution"), each lookup held a tokio
worker thread for the resolver's whole retry budget. A few at once, one
user retrying or two users clicking, occupied every worker, and
rustguac stopped answering anything, health checks included, so the
proxy returned 503 for everyone until the lookups gave up.
Lookups now go through tokio::net::lookup_host, which runs the resolver
on the blocking thread pool, with a 5 second limit after which that one
connect fails with "DNS did not answer within 5 seconds".
slow_dns_does_not_stall_the_runtime reproduces it: with a resolver that
never answers, a 2-worker runtime managed 1 heartbeat in 3 seconds
before this change and runs normally after it. It needs that resolver,
so it is ignored by default; the comment has the command to run it.
Reported by Jon Sobels-Craven (@jons-c).
Re-filing more than a handful of entries meant opening each one. Admins
now get a checkbox column on the entries table (select all, shift-click
for ranges) and an action bar with Move to, Copy to and Delete. One
dialog handles all three: a filterable list of every folder, a preview
naming any entries that already exist in the target, a choice to leave
those or give them a new name, warnings when the move crosses between
shared and instance or the folders have different access groups, and a
result per entry afterwards.
POST /api/addressbook/bulk does the work, up to 1000 entries a request,
8 at a time. Each entry goes through the same stored-entry copy as the
single move: credentials included, Vault check-and-set so nothing is
overwritten, and a move removes the original only after the copy is
written. Clashes are skipped or renamed (name-2 .. name-99, within the
64-character limit); there is no overwrite option. Repeated items are
acted on once. Each entry is audited on its own, headline only. The
route alone accepts a 256KB body, since 1000 entries is about 100KB;
it is admin-only and authentication happens before the body is read.
GET /api/addressbook/folder-paths lists every folder for the picker; the
existing "Move to folder" list only knew about folders already expanded
in the tree.
Delete now checks the entry exists first, in both the bulk and single
endpoints. Vault reports success for deleting a path that is not there,
so a missing entry was answered, and audited, as deleted.
tokio-rustls 0.26.6, pageant 0.2.3 and rand 0.10.3 in the main build;
pageant 0.2.3 and russh 0.63.3 in fuzz/. Lockfile only (#228, #233,
#234, #235, #236).
The entrypoint created the docker-admin key whenever no file existed at
/opt/rustguac/data/rustguac.db. With db_path set anywhere else, for
example on a Kubernetes persistent volume, that file never exists, so
every container start tried to create the admin again and failed with
"UNIQUE constraint failed: admins.name".
add-admin gains --if-none: create the admin only when no admin account
exists, otherwise say so and exit successfully. rustguac opens the
database db_path names, so the entrypoint no longer needs to know where
it is. Any existing admin counts, so a docker-admin an operator removed
after creating their own does not come back. The save-the-key banner is
printed only when a key was actually created.
Reported by @hit-rich.
make install replaces guacd's binary and libraries while the old guacd is
still running, and the end of the install restarted rustguac alone.
Requires= starts a stopped guacd but never restarts a running one, so an
upgrade rebuilt guacd and then went on serving the previous build until
someone restarted it by hand -- which looks exactly like a guacd change
that did nothing.
guacd is now restarted, ahead of rustguac, whenever it is active. Nothing
is cut off by it: sessions go through rustguac, which is already stopped
for the upgrade or was never running. A first install still leaves both
stopped.
Moving an entry to another folder was done in the browser as "create
a copy from the edit form, then delete the original". The page never
receives an entry's secrets, so the copy had no password, private key,
jump host credentials, Proxmox token or container password, and a
$variable reference stored in the password field was lost the same way.
The original was then deleted permanently. Clone had the same flaw.
(#232)
Two admin-only endpoints now do it from the stored entry:
- POST .../entries/{entry}/move copies the full entry to the target and
only then removes the original. If that removal fails it says the
entry was copied, rather than reporting a plain failure.
- POST .../entries/{entry}/copy copies it under a new name. Clone uses
it and then applies the form's edits as an ordinary update, which
keeps stored secrets it is not given.
The target is written with Vault check-and-set (cas 0), so an existing
entry is never overwritten, even by a concurrent writer. Scope, folder
and names go through the existing Vault validation (only shared and
instance scopes, no traversal, .config reserved, so a copy cannot
replace a folder's access settings), and those failures are 400s. Both
actions are audited, headline only. A note on the shared helper records
that any future per-folder editing (#231) must require rights on both
the source and the target folder.
A busy main thread does not delay pointer movement, it discards it: moves
arriving while a handler runs are merged into one, and what is dispatched
afterwards carries only the last position. A drag feels that immediately --
the path between two points is gone, so a dragged scrollbar thumb is dropped
rather than merely lagged -- while a hover or a video degrades gracefully
under the same load.
getCoalescedEvents() returns what was merged. Replaying it in order restores
the path, though not the timing: nothing running on a blocked thread can do
that, and the positions still arrive together once the thread is free. What
changes is that the remote end sees where the pointer went rather than only
where it ended up.
Deliberately additive. The mousemove listener is untouched and still delivers
every position it did before, so touch, the compatibility mouse events it
produces and the counter that swallows them all behave as they did; the
dispatched position arrives twice and the second is dropped by the
unchanged-coordinates check already in move(). Migrating this file to pointer
events outright is the tidier shape and a far larger change -- every listener
would have to move together, or synthetic events from touch would arrive with
nothing left counting them down.
Only while a button is held, because the path matters for a drag and the
endpoint is all that matters for a hover, and replaying every merged move of
an idle pointer would multiply a session's upstream mouse traffic for movement
nobody can perceive. Capped and evenly sampled, because a pointer reporting at
1000Hz behind a thread blocked for 100ms offers a hundred positions for one
drag, and answering a rendering problem with a traffic one is no answer. An
ordinary pointer never reaches the cap.
The pop-out monitor windows get the same replay. They bypass Guacamole.Mouse
entirely -- it does not track the X axis correctly in a popup -- and map the
pointer themselves, so a drag on a second head would otherwise lose its path
exactly as the first one did. The sampler moves to a static so there is one
definition of the rule rather than two that can drift, and that path needs one
thing the main display does not: it sends unconditionally, where move() drops
an unchanged position, so it keeps what it last sent and drops a repeat --
keyed on the buttons as well as the position, since a press and a release
happen at one place and are not repeats of each other. Feature-detected on the
event rather than on PointerEvent, because a pop-out window is a realm of its
own and the constructor this page can see is not the one that made the event
it is handed.
That pointer handling lives in Guacamole.MonitorPointer rather than inline in
client.html, because these windows cannot be opened without a live
multi-monitor session: while it sat in the page the only thing a test could do
was match the source text and hope, and the two things most worth checking
there are a coordinate mapping and a suppression rule -- exactly the kind that
look right in a diff and are off by a term. It takes the canvas, a function
returning this monitor's rectangle in the combined framebuffer (a call rather
than a captured value, since the layout changes under a running window) and a
function to send a state.
Writing those tests found a real defect in the wheel, which sends a momentary
press and release: it built one state object, sent it, mutated it and sent it
again, which works only while every consumer copies synchronously. It sends
two states now.
Tested rather than tried, because this path only runs once the browser has
already thrown something away: the case it exists for is the one that cannot
be reliably produced, and a broken version feels like the behaviour it
replaced. Twenty tests cover the mapping, the button mask, the repeat
suppression, the wheel's reset of it and the replay, plus a harness driving
real pointer input through the debugger with the main thread deliberately
blocked -- because the unit tests feed events shaped the way the code expects,
and that assumption is the one worth checking against a browser.
(cherry picked from commit d8ce743fe0801bd8c776281ee5541abf8040a339)
A client with no hardware H.264 decoder lost H.264 altogether, and lost it in
the worst available way. Chrome reports a configuration carrying
hardwareAcceleration 'prefer-hardware' as unsupported outright rather than
falling back, so configure() failed asynchronously, the decoder closed, every
frame was then held waiting for a keyframe that cured nothing -- and since
guacd suppresses ordinary image operations for a layer carrying H.264, what
the user saw was a permanently black screen rather than a degraded picture.
Measured on Chrome 153 against an Intel UHD 770 with no VA-API decode
available: isConfigSupported() answers supported for avc1.4d4020 with no hint
and with 'prefer-software', and not supported with 'prefer-hardware'. The
same box then decoded the stream perfectly once the hint was dropped.
So ask for hardware once and, if that is refused, build again without asking.
Dropping the hint unconditionally is the smaller change and the wrong one: it
hands the choice to the browser on every client, including the ones this
whole path exists for, where a software decode costs far more than it saves.
It says so when it happens. A session quietly decoding H.264 in software is
worth knowing about -- it is the difference between the cost this feature was
built to avoid and the cost it was built around -- so the fallback reports
decoder_software_fallback rather than only appearing as a slow session.
Tested, because the hint reads like a preference and restoring it would look
harmless. The test drives the shape Chrome actually presents -- configure()
accepted, the refusal arriving later through the error callback -- and checks
that hardware is asked for first, asked for only once, that no frame is
stranded by the refusal, and that the fallback is audible.
Reachable on a VM, a machine whose driver is blocklisted, or a browser with
acceleration switched off, none of which are exotic.
(cherry picked from commit 59fc416e80daab20ac07bdac5dbebb6ff7eddf75)
Upstream: ported by hand, as the fork's decoder has diverged. The
fallback reports through console.warn only, since the fork's diagnostic()
channel is not carried here. tests/h264-hardware-fallback.mjs passes
unchanged (node tests/h264-hardware-fallback.mjs).
The mirror picked up from the fork (rsync --delete, or rm -rf without
rsync) would delete anything an operator put in $PREFIX/static, and
docs/configuration.md tells them to put a theme logo there. Each install
now records the files it copied in $PREFIX/.static-installed-files
(outside static/, so it is not served), and the next install removes only
files from that record that have left the repo. The stray nested
static/static copy, which predates any record, is removed by name.
Guacamole.Display puts its software cursor layer in the DOM from
construction, and client.html never handed the pointer over -- no oncursor
handler, nothing hiding the browser's arrow -- so both were always drawn.
They overlap at rest and separate during fast movement: the browser's is
composited by the OS, while the software one is moved by the page on each
mousemove and trails by however long the main thread takes to get to it.
The remote pointer image now becomes the CSS cursor and the layer is hidden,
falling back to the layer only where a CSS cursor is refused. Declared
through image-set() at the display's scale rather than via
Guacamole.Mouse.setCursor(), which assumes one image pixel per CSS pixel
and would draw a HiDPI host's pointer at twice its size under Native
Resolution. Re-applied from scaleDisplay(), since the scale changes under
a pointer that does not.
(cherry picked from commit 9cc6f77f2ee3e48a10d38055d6fe8debffd37ef8)
The encrypted drive for RDP file transfer is the only question this installer
asks, and the only way to answer it ahead of time was an environment variable
that --help did not mention. So the one thing a scripted install needs to know
was the one thing the usage text left out, next to --no-deps and --no-tls,
which are flags precisely because skipping a step is a normal thing to want.
--no-drive is an alias for RUSTGUAC_DRIVE_SETUP=no rather than a second switch
reaching the same place, so there is one variable to read when the answer comes
out wrong. Setting it during argument parsing also lets it win over an
inherited RUSTGUAC_DRIVE_SETUP=yes, which is the precedence an explicit flag
should have.
--help now lists all five drive variables and, more usefully, the three ways
the prompt already answers itself: an existing container, a missing cryptsetup,
or stdin that is not a terminal. Someone seeing the prompt on one machine and
not another is looking at one of those rather than at a bug, and someone not
seeing it where they expected it is looking at the first.
It also points at PREFIX/guacd.env, since the neighbouring trap is setting
something directly in a unit file the next install rewrites.
(cherry picked from commit 8d2f2c000a62d5125e71f2e5395c9edef08ea81a)
Three failures, all of which cost time during a day of deploys and all of
which present as something other than what they are.
The binary could not be overwritten while the service ran. cp reported "Text
file busy" and left the old binary in place, after the script had already said
it built the new one -- a deploy that changed nothing and looked like a deploy
that worked. It now stops the service first and restarts it afterwards, but
only when it was already running: a first install still leaves it down so an
admin can be created before anything is reachable, and it checks the service
actually came back rather than assuming.
The static assets were copied over the top, so nothing ever left. That is not
just clutter, because ServeDir is rooted at the install's static directory and
whatever sits there is reachable: a stray nested copy of the entire tree lived
at static/static from July until September 2026, serving two-month-old HTML
and JavaScript at /static/... to anyone who asked, and three files from an
abandoned branch outlived the branch the same way. Mirrored now, with rsync
where it exists and a replace where it does not, and copying `static/.`
instead of `static/*` -- the latter being one slip from `cp -r static dest/`,
which nests the tree rather than filling the directory, and is how that copy
got there.
And the optional drive prompt blocked for ever without a terminal. It sits
after the binary and the assets are installed, so the hang looked like a
completed deploy whose service simply had not come back. It is skipped when
stdin is not a tty, and says how to answer it from a script.
(cherry picked from commit b24d3c1b80c152a20558301a14dfeb3edd7a5a1a)
The branded HTML handlers returned `Html(...)` and nothing else: no ETag,
no Last-Modified, no Cache-Control. With no freshness information at all
Chrome caches heuristically and does not revalidate, so a restart served
the old page for hours -- incognito was the only reliable way to see a
change. The static assets had the same gap from the other side: ServeDir
already sent ETag and Last-Modified and honoured both, it was just never
asked, so a rebuilt /guac/*.js was invisible too.
The pages are branded once at startup into a map, so the tag is computed
there as well -- one sha256 over each, against a rewrite already walking
the same strings. A request is then a string compare, and an unchanged
page costs a 304 rather than 136K of client.html or 255K of
connections.html. `no-cache` is the revalidate directive, not
do-not-store: the browser keeps the body and asks. Tags are
content-derived, not boot-derived, so a restart that changes nothing
still answers 304.
Revalidating every asset is correct but not free, and this is reached
over a WAN. The same startup pass therefore rewrites the 36 script tags
and the stylesheet to carry a hash of the bytes on disk, and a URL
bearing one is served with a year and `immutable`. The round trip it
saves is a single one, since the browser-to-proxy leg is HTTP/2 and the
conditional requests go out together -- but client.html reloads on every
session launch and relaunch, so it is a round trip paid exactly when the
link is worst. Only a 2xx or 304 earns the long directive: a 404 under a
versioned URL is a deployment that has gone wrong, and pinning it in the
cache until next year would make it outlive its cause.
Query strings rather than hashed filenames because there is no build step
to rename anything and the rewrite already existed; the old caution about
proxies declining to cache them has not been true in many years.
The cost is that editing an asset now needs a restart where a reload was
enough, since the URL and the bytes it names are produced together --
hence RUSTGUAC_NO_ASSET_VERSIONING, which leaves everything revalidating.
Verified against a running instance: matching and weak If-None-Match give
304 on the HTML, a stale tag gives 200, a versioned asset is immutable on
both its 200 and its 304, a bare one and a 404 are not, the kill switch
emits bare URLs, and API responses are untouched -- the middleware is on
the ServeDir fallback only.
(cherry picked from commit 966aba765c8a8db043b5f62603abdb6a18dd0eb5)
Unrelated fixes carried by the fork: session times and date filters in local
time, no local resolution of the target when a jump host is set, no resize
request that changes no pixels, and rustfmt on three hunks left unformatted.
(cherry picked from commit d9b361db6eb216d871f3fdac41c423a586e2d559)
Upstream changes on pick: the client.html resize skip is kept but
without the native-resolution sizing (displayScale/deviceSize), which
belongs to the HiDPI commit not taken here, so sizes stay in CSS pixels.
The oidc.rs hunk was formatting only and is superseded.
The installer rewrites both unit files on every run, so anything set directly in
them -- or through "systemctl edit" -- is lost at the next install. guacd's
environment moves to $PREFIX/guacd.env, created only when absent and never
overwritten, and the unit reads it through EnvironmentFile. That is where
GUACD_LOG_LEVEL belongs, since trace is what prints per-surface-command codec
detail and debug what prints it per frame.
Systemd drop-ins are the other half of the same trap. They merge with the unit
just written and take precedence over it, but live in a directory this script
never touches, so a forgotten one silently overrides everything here --
including ExecStart, which discards the log-level wrapper entirely. Any that
exist are now listed with their contents, and the command that shows the unit
as systemd actually assembles it.
BUILD-CONTAINER.md records the container build this runs in, since guacd is
built from source against FreeRDP 3 rather than installed from a package, and
the environment a measurement was taken in is otherwise left to be guessed at.
(cherry picked from commit 30558b38be1ea807d1271fe7b3a89f7717c7abc4)
Upstream: BUILD-CONTAINER.md and the fork notes were left out.
Provider discovery is lazy with retry, so SSO recovers on its own once the
provider comes up, and /api/auth/status reports availability separately from
enablement. Includes the callback diagnostics for state-cookie mismatch.
(cherry picked from commit dffe34660601a223021819c8efbed2ad503bfc14)
Upstream changes on pick: resolved against the ID token diagnostics from
#237 (the provider key ids now travel with the cached client and are
logged on rediscovery), and the unauthenticated /api/auth/status warm-up
is single-flight via OidcState::warm_in_background, so polling it while
the provider is down cannot queue up unbounded discovery tasks.
guacd sends the magic packet to whatever broadcast address and UDP port
it is given and then waits the given time before connecting. On an
ad-hoc request from a non-admin those came from the caller, so a
poweruser could have guacd send UDP to an arbitrary address and port,
or hold a guacd process for as long as they liked. Wake-on-LAN is now
ignored on those requests (it is configured on connection entries), the
wait is capped at 10 minutes, and the SSH font size is clamped to 6-72.
Wake-on-LAN (per entry): new wol_send_packet/wol_mac_addr/wol_broadcast_addr/
wol_udp_port/wol_wait_time fields on AddressBookEntry, threaded through
CreateSessionRequest into the SSH/VNC/RDP guacd param structs and emitted as
guacd's native wol-* connection args (guacd sends the magic packet and polls the
target before connecting). New 'Wake-on-LAN' section in the entry editor;
Guacamole imports carry existing wol-* params over.
SSH terminal font: new per-entry ssh_font_size (default 12), replacing the
hardcoded font-size. SSH DPI is now pinned to 96 so terminal text no longer
renders oversized on HiDPI displays — the client auto-scales the canvas, so a
fixed baseline keeps text size consistent across normal and HiDPI screens.
(RDP/VNC keep the device-scaled DPI.)
Onboarding modal: add a corner close button that permanently dismisses the
welcome tour (re-show still available via Settings -> Welcome Tour).
(cherry picked from commit e881bbc1e8933b46f7f2a8de691c86343dbbf15b)
openidconnect reports every signature problem as "Signature
verification failed" and keeps the reason (no matching key, crypto
error, disallowed algorithm) in the error's source chain, which we
logged with Display and so dropped. Log the whole chain, plus the
token's alg and kid from its header and whether that kid is among the
provider keys fetched at startup. When it is not, say so and name the
two usual causes: a key rotation since startup, and an Entra app with
an app-specific signing key (#237).
The login `next` check allowed anything starting with a single `/`.
Browsers treat `\` as `/` and drop tabs and newlines in Location, so
`/\evil.com`, and `/%09/evil.com` once the query was decoded, both
redirected to another site straight after a successful login on the
real login page. `next` must now be visible ASCII with no backslash,
and must not start with an encoded slash or backslash. That also keeps
it safe to place in the rustguac_next cookie unescaped, where a `;`
could previously add cookie attributes. Checked both when the cookie is
set and when it is consumed.
The network allowlists are what fence powerusers creating ad-hoc
sessions, and several paths went around them:
- POST /api/ssh/probe-host-key dialled any host:port, a port scanner and
banner grabber for anything rustguac can reach (guacd, Vault, metadata
services). Non-admin probes are now checked against
ssh_allowed_networks and dialled at the checked address.
- Jump hosts were never checked, though rustguac dials the first one
itself. For non-admin ad-hoc sessions it is now fenced by
ssh_allowed_networks and pinned.
- Ad-hoc Proxmox sessions called any proxmox_url and let guacd dial
whatever SPICE proxy that API returned. For non-admin ad-hoc sessions
both are now fenced by vnc_allowed_networks, as direct SPICE is.
The jump host and Proxmox fences are deliberately limited to non-admin
ad-hoc requests: connection entries and admins name bastions and
clusters that sit outside the target lists, and on the canary every
entry-based Proxmox and jump host session would otherwise have broken.
The flag is serde(skip), so a request body cannot set it.
The allowlist passed a hostname if any resolved address was allowed and
then handed guacd the hostname to resolve again, so a name whose DNS the
caller controls could pass the check and connect elsewhere. SSH and VNC
now dial the checked address when there are no jump hosts. RDP, SPICE
and web keep the hostname (TLS names, Kerberos) and the residual risk is
documented. Docs also no longer claim every resolved address must match.
client_ip took the LEFTMOST X-Forwarded-For entry from a trusted proxy.
Behind any proxy that appends to an incoming header (nginx
$proxy_add_x_forwarded_for, as our own reverse-proxy doc recommended, or
Apache mod_proxy), that entry is whatever the client sent, so a client
could pick an address inside an IP allowlist and walk through it. The
header is now walked from the right, skipping trusted proxies, across
every header line, split on raw bytes so a non-UTF-8 left entry cannot
hide the address a proxy appended.
Rate limiters used tower_governor's SmartIpKeyExtractor, which believes
forwarded headers from any peer: a different forged X-Forwarded-For per
request meant a fresh bucket each time, including on the always-on OIDC
limiter. They now key on the same trusted-proxy resolution. When
rate_limit is enabled, the API and WebSocket limiters sit outside
authentication so requests with bad credentials are counted. The OIDC
burst goes from 5 to 20 to cover an office behind one NAT address.
Docs: nginx recipe overwrites X-Forwarded-For with $remote_addr, and the
rate-limit section now states what is actually limited and when.
The first WebSocket to a pending session takes its guacd stream, already
logged in to the target. Any operator could claim another user's pending
session that way, and session ids are visible in reports. Now only the
session's creator may make that connection, with no admin bypass: an admin
"shadowing" a pending session used to silently become its owner and lock
the real owner out, so shadow now refuses sessions that have not started
(409). handle_ws only takes the stream when the owner check passed at
upgrade time.
POST /api/sessions accepts an admin-only `owner` field so a backend using
an API key can create a session for a user who then connects with their
own login. That user is recorded as the creator.
Share links and shadow tokens gain an optional read-only mode; the default
stays read/write. Each shareable session gets a second token
(share_url_readonly), and shadow takes {"read_only": true}. guacd enforces
it per joining user via the read-only connect arg, which was previously
hardcoded to false. The Share dialog offers both links, the Sessions page
offers watch and shadow, and docs no longer claim a read-only mode that
did not exist.
They call assert_cmd's cargo_bin, which needs CARGO_BIN_EXE_rustguac.
Cargo exports that only to integration tests under tests/, never to a
#[cfg(test)] module inside src/. As unit tests they passed locally purely
because a stale target/debug/rustguac was lying around for assert_cmd's
fallback to find; on a clean checkout, and so in CI, all five failed with
"`CARGO_BIN_EXE_rustguac` is unset". They have been broken since they were
written and nothing noticed, because the suite had never run in CI.
Caught by the cargo test step added in 83a9110, on its first run.
Database assertions now go through rusqlite directly rather than the
crate's own db helpers, since rustguac is a binary crate with no lib target
for an integration test to import. That is the better boundary regardless:
it checks the state the CLI actually leaves behind. rusqlite joins
dev-dependencies, which adds nothing to the tree as it is already a runtime
dependency.
CI ran fmt, clippy, build and audit but never `cargo test`, so none of the
310 tests had ever run on a pull request. That included the regression
tests written specifically to guard past incidents: the recording rotation
that deleted in-progress files, and the credential-variable folder scan
that hid subfolder-granted entries from operators. Both could have been
reintroduced without CI noticing.
Runs the debug profile rather than --release on purpose. debug_assertions
and integer overflow checks are compiled in there and switched off in a
release build, and the wire-protocol parser is where an overflow would
otherwise pass silently.
Noted by @joshsol1 on #230.
`Config::load` exited with "[oidc] is configured but no client_secret
was provided" for every subcommand, including `add-admin`. The secret
normally lives in /opt/rustguac/env, which only systemd's
EnvironmentFile loads, so running the admin CLI from an interactive
shell on a freshly installed host failed unless the secret was also
copied into config.toml, which defeats the point of the env file.
Keep the OIDC_CLIENT_SECRET override in `Config::load`, but move the
presence check into `Config::validate_oidc_secret` and call it only on
the `serve` path. Admin subcommands touch just the SQLite database and
never contact the IdP. Add unit tests for the validation function.
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.
Jad Seifeddine