diff --git a/src/api.rs b/src/api.rs index cfe236da..bb960b80 100644 --- a/src/api.rs +++ b/src/api.rs @@ -2738,6 +2738,13 @@ pub struct ConnectRequest { pub width: Option, #[serde(default)] pub height: Option, + /// The screen's size in CSS pixels. An entry that opens fullscreen on + /// connect is created at this size instead of the window's, so nothing + /// has to be resized once the fullscreen transition lands. + #[serde(default)] + pub screen_width: Option, + #[serde(default)] + pub screen_height: Option, #[serde(default)] pub dpi: Option, #[serde(default)] @@ -2829,6 +2836,62 @@ pub struct ProbeHostKeyRequest { pub port: Option, } +/// The size a new session is created at. +/// +/// A bitmap (RDP/VNC) entry that opens fullscreen on connect is created at +/// the screen's size, which is the size it will have once fullscreen lands. +/// Until the client's fullscreen gesture fires it scales that picture to fit +/// the window, so the desktop is painted once at its final size and the +/// fullscreen transition brings no resize, no decoder rebuild and no second +/// repaint (#257). Everything else keeps the window's size: a terminal +/// reflows to fullscreen instantly. +/// +/// Both sizes are CSS pixels, which is what the client's post-connect size +/// send uses today, so creation and that send agree. When #238 lands +/// (native/HiDPI resolution, `display_scale`), that send becomes the +/// device-pixel size, and the mod-16 rounding below must then be applied to +/// the final size after the devicePixelRatio multiply, not to the CSS size. +pub fn initial_session_size( + session_type: &str, + fullscreen_on_connect: bool, + window: (Option, Option), + screen: (Option, Option), +) -> (Option, Option) { + let bitmap = matches!(session_type, "rdp" | "vnc"); + let (w, h) = match (bitmap && fullscreen_on_connect, screen) { + (true, (Some(w), Some(h))) if w > 0 && h > 0 => (Some(w), Some(h)), + _ => window, + }; + // RDP's display channel rounds every requested size down to a multiple + // of 16 (patches/007, which prevents the H.264 green band), so the + // client's first post-connect size send always lands on that grid. + // Create the session there already, fullscreen or windowed, so the two + // match: guacd coalesces them and there is no resize, the H.264 decoder + // is not rebuilt and the picture never flashes on connect (#257). VNC + // has no such rounding, so it keeps the exact size. + // + // #238 (not merged at the time of writing): once the session is created + // at the device-pixel size, round that size, i.e. apply round_down_mod16 + // after multiplying by devicePixelRatio, since rounding the CSS size and + // then scaling it would leave the grid again. + if session_type == "rdp" { + (w.map(round_down_mod16), h.map(round_down_mod16)) + } else { + (w, h) + } +} + +/// Round a dimension down to a multiple of 16, with a 16px floor so a tiny +/// screen never rounds to zero. Mirrors guacd's RDP display-channel rounding. +fn round_down_mod16(v: u32) -> u32 { + let r = v - (v % 16); + if r < 16 { + 16 + } else { + r + } +} + /// POST /api/addressbook/folders/:scope/:folder/entries/:entry/connect — Create session from entry. #[allow(clippy::too_many_arguments)] pub async fn ab_connect_entry( @@ -2931,6 +2994,12 @@ pub async fn ab_connect_entry( // Build CreateSessionRequest from the Vault entry + connect request display params. // ConnectRequest credentials override address book values (for prompted credentials). let ab_entry_key = format!("{}/{}/{}", scope, folder, entry); + let (initial_width, initial_height) = initial_session_size( + &ab_entry.session_type, + ab_entry.fullscreen_on_connect.unwrap_or(false), + (req.width, req.height), + (req.screen_width, req.screen_height), + ); let create_req = CreateSessionRequest { session_type, hostname: ab_entry.hostname, @@ -2954,8 +3023,8 @@ pub async fn ab_connect_entry( jump_username: None, jump_password: None, jump_private_key: None, - width: req.width, - height: req.height, + width: initial_width, + height: initial_height, dpi: req.dpi, banner: req.banner.or(ab_entry.banner), enable_drive: ab_entry.enable_drive, @@ -2964,7 +3033,7 @@ pub async fn ab_connect_entry( remote_app_args: ab_entry.remote_app_args, enable_recording: ab_entry.enable_recording, record_typescript: ab_entry.record_typescript, - address_book_entry: Some(ab_entry_key), + address_book_entry: Some(ab_entry_key.clone()), address_book_folder: Some(folder.to_string()), entry_display_name: ab_entry.display_name.clone(), max_recordings: ab_entry.max_recordings, @@ -3025,6 +3094,28 @@ pub async fn ab_connect_entry( "Address book connect requested" ); + // Reuse a Pending session this user already opened for this entry rather + // than spawning a duplicate (e.g. a repeat Connect click). Matched on the + // fully built request above, credentials included: a retry with a + // different password, key or domain (a mistyped prompt) must not get + // the old Pending session, which still holds the old credentials. The + // fingerprint is a hash, compared only, never logged. + if let Some(existing) = manager + .find_reusable_session( + &admin_name, + &ab_entry_key, + &create_req.credential_fingerprint(), + ) + .await + { + tracing::info!( + user = %admin_name, + session_id = %existing.session_id, + "Reusing existing address book session" + ); + return (StatusCode::CREATED, Json(json!(existing))).into_response(); + } + match manager.create_session(create_req, admin_name.clone()).await { Ok(info) => { tracing::info!( @@ -5780,6 +5871,53 @@ mod tests { use super::*; use std::path::Path; + #[test] + fn fullscreen_sizing_applies_to_bitmap_sessions_only() { + let win = (Some(1710), Some(954)); + let scr = (Some(1710), Some(1112)); + // RDP opening fullscreen: created at the screen size rounded down to + // a multiple of 16, matching guacd's display-channel rounding so there + // is no post-connect resize (#257). VNC is not mod-16 rounded. + assert_eq!( + initial_session_size("rdp", true, win, scr), + (Some(1696), Some(1104)) + ); + assert_eq!(initial_session_size("vnc", true, win, scr), scr); + // A screen that is already mod-16 is created at its exact size. + assert_eq!( + initial_session_size("rdp", true, win, (Some(1920), Some(1088))), + (Some(1920), Some(1088)) + ); + // a terminal reflows instantly: window size, fullscreen or not + assert_eq!(initial_session_size("ssh", true, win, scr), win); + assert_eq!(initial_session_size("telnet", true, win, scr), win); + assert_eq!(initial_session_size("web", true, win, scr), win); + // not fullscreen, or no screen size offered: window size — still on + // the mod-16 grid for RDP, since the client's first size send is + // rounded to it by guacd either way (no resize on connect). + let win16 = (Some(1696), Some(944)); + assert_eq!(initial_session_size("rdp", false, win, scr), win16); + assert_eq!(initial_session_size("rdp", true, win, (None, None)), win16); + assert_eq!( + initial_session_size("rdp", true, win, (Some(0), Some(0))), + win16 + ); + assert_eq!(initial_session_size("vnc", false, win, scr), win); + assert_eq!( + initial_session_size("rdp", false, (None, None), scr), + (None, None) + ); + } + + #[test] + fn round_down_mod16_floors_at_sixteen() { + assert_eq!(round_down_mod16(1710), 1696); + assert_eq!(round_down_mod16(1696), 1696); + assert_eq!(round_down_mod16(16), 16); + assert_eq!(round_down_mod16(15), 16); + assert_eq!(round_down_mod16(0), 16); + } + #[test] fn report_bounds_read_a_bare_date_as_a_whole_local_day() { // Sydney in September: UTC+10, so getTimezoneOffset() is -600. diff --git a/src/session.rs b/src/session.rs index ef28dcbb..b1b57592 100644 --- a/src/session.rs +++ b/src/session.rs @@ -205,6 +205,61 @@ pub struct CreateSessionRequest { pub wol_wait_time: Option, } +impl CreateSessionRequest { + /// SHA-256 over the target and every credential this request would + /// connect with, as hex. Two requests with the same fingerprint would + /// open the same connection as the same principal, so a fresh Pending + /// session made from one can stand in for the other + /// (`SessionManager::find_reusable_session`). A retry with a different + /// password, key, domain or jump credential gets a different + /// fingerprint and therefore a new session. The hash is never logged + /// or returned to a client, and a secret cannot be recovered from it. + pub fn credential_fingerprint(&self) -> String { + use sha2::{Digest, Sha256}; + let mut h = Sha256::new(); + // Every field is tagged and length-prefixed, so `("ab", "c")` and + // `("a", "bc")` hash differently and an absent field differs from + // an empty one. + let mut field = |tag: &str, value: Option<&str>| { + h.update(tag.as_bytes()); + match value { + Some(v) => { + h.update((v.len() as u64).to_le_bytes()); + h.update(v.as_bytes()); + } + None => h.update([0xffu8; 8]), + } + }; + let port = self.port.map(|p| p.to_string()); + let jump_hosts = self + .jump_hosts + .as_ref() + .and_then(|j| serde_json::to_string(j).ok()); + let jump_port = self.jump_port.map(|p| p.to_string()); + field("type", Some(&format!("{:?}", self.session_type))); + field("host", self.hostname.as_deref()); + field("port", port.as_deref()); + field("user", self.username.as_deref()); + field("pass", self.password.as_deref()); + field("key", self.private_key.as_deref()); + field("domain", self.domain.as_deref()); + field("auth_pkg", self.auth_pkg.as_deref()); + field("kdc", self.kdc_url.as_deref()); + field("url", self.url.as_deref()); + field("jumps", jump_hosts.as_deref()); + field("jump_host", self.jump_host.as_deref()); + field("jump_port", jump_port.as_deref()); + field("jump_user", self.jump_username.as_deref()); + field("jump_pass", self.jump_password.as_deref()); + field("jump_key", self.jump_private_key.as_deref()); + field("container_user", self.container_username.as_deref()); + field("container_pass", self.container_password.as_deref()); + field("proxmox_token_id", self.proxmox_token_id.as_deref()); + field("proxmox_token_secret", self.proxmox_token_secret.as_deref()); + hex::encode(h.finalize()) + } +} + /// Session status in the lifecycle. #[derive(Debug, Clone, Serialize, PartialEq)] #[serde(rename_all = "lowercase")] @@ -325,6 +380,10 @@ pub struct Session { /// (#154). Surfaced verbatim in `SessionInfo` so client.html can /// trigger fullscreen on first user gesture after CONNECTED. pub fullscreen_on_connect: bool, + /// `CreateSessionRequest::credential_fingerprint()` of the request this + /// session was created from. Compared, never logged or served, when a + /// repeat Connect asks to reuse a Pending session. + pub credential_fingerprint: String, /// Copied from the source entry's `autohide_side_tabs` flag. /// Surfaced in `SessionInfo` so client.html can auto-hide the /// clipboard/files side tabs. @@ -789,6 +848,9 @@ impl SessionManager { req: CreateSessionRequest, created_by: String, ) -> Result { + // Taken before any field of the request is moved out below. + let credential_fingerprint = req.credential_fingerprint(); + // Enforce session limits (only count active/pending sessions) { let sessions = self.sessions.read().await; @@ -918,6 +980,8 @@ impl SessionManager { session_id = %session_id, hostname = %hostname, username = %username, + width, + height, "Creating new SSH session" ); @@ -1945,6 +2009,7 @@ impl SessionManager { shadow_tokens: Vec::new(), share_allowed, fullscreen_on_connect: req.fullscreen_on_connect.unwrap_or(false), + credential_fingerprint, autohide_side_tabs: req.autohide_side_tabs.unwrap_or(false), }; @@ -2009,6 +2074,45 @@ impl SessionManager { result } + /// How long after creation a Pending session is offered to a repeat + /// Connect instead of a fresh one. Short on purpose: an older Pending + /// one, whose guacd may since have gone away, is left to the + /// pending-timeout reaper. + pub const REUSABLE_PENDING_SECS: i64 = 15; + + /// Find a live session this user can be sent back to instead of opening a + /// fresh one for the same address-book entry. Reusing a `Pending` session + /// (guacd connected, owner never attached) stops repeat Connect clicks from + /// piling up orphans; only a Pending session is reusable, since an owner + /// connection consumes the guacd stream and cannot be re-owned. + /// + /// `credential_fingerprint` is `CreateSessionRequest::credential_fingerprint()` + /// of the fully built request, so a session is only reused by a request + /// that would connect to the same target with the same credentials. A + /// retry with a different password, key or domain (a mistyped prompt) + /// never gets the old session, which still holds the old credentials. + pub async fn find_reusable_session( + &self, + created_by: &str, + entry_key: &str, + credential_fingerprint: &str, + ) -> Option { + let sessions = self.sessions.read().await; + for session in sessions.values() { + let s = session.lock().await; + if s.status == SessionStatus::Pending + && s.guacd_stream.is_some() + && s.created_by == created_by + && s.address_book_entry.as_deref() == Some(entry_key) + && s.credential_fingerprint == credential_fingerprint + && (Utc::now() - s.created_at).num_seconds() < Self::REUSABLE_PENDING_SECS + { + return Some(s.info()); + } + } + None + } + /// Get a specific session's info. pub async fn get_session(&self, id: Uuid) -> Option { let sessions = self.sessions.read().await; @@ -2915,6 +3019,7 @@ mod tests { shadow_tokens: Vec::new(), share_allowed: true, fullscreen_on_connect: false, + credential_fingerprint: String::new(), autohide_side_tabs: false, } } @@ -3455,6 +3560,182 @@ mod tests { let cfg = DriveConfig::default(); assert_eq!(drive_cleanup_settings(&Some(cfg)), (true, 0)); } + + // ── Pending-session reuse (repeat Connect on an entry) ── + + fn entry_request(extra: serde_json::Value) -> CreateSessionRequest { + let mut body = serde_json::json!({ + "session_type": "rdp", + "hostname": "rdp.example.test", + "port": 3389, + "username": "alice", + "password": "correct horse", + "domain": "CORP", + "address_book_entry": "shared/ops/rdp-1", + }); + body.as_object_mut() + .unwrap() + .extend(extra.as_object().unwrap().clone()); + serde_json::from_value(body).unwrap() + } + + /// A Pending session as `create_session` leaves it for an entry connect: + /// guacd connected (any stream will do here), owner not yet attached. + fn pending_entry_session(req: &CreateSessionRequest, created_by: &str) -> Session { + let (a, _b) = tokio::io::duplex(64); + let mut s = seed_test_session(&format!("tok-{}", Uuid::new_v4())); + s.status = SessionStatus::Pending; + s.guacd_stream = Some(Box::new(a)); + s.created_by = created_by.into(); + s.address_book_entry = req.address_book_entry.clone(); + s.credential_fingerprint = req.credential_fingerprint(); + s + } + + #[test] + fn credential_fingerprint_tracks_target_and_credentials() { + let base = entry_request(serde_json::json!({})); + let same = entry_request(serde_json::json!({})); + assert_eq!(base.credential_fingerprint(), same.credential_fingerprint()); + assert_eq!(base.credential_fingerprint().len(), 64); + // Does not contain a secret in the clear. + assert!(!base.credential_fingerprint().contains("correct horse")); + + for (key, value) in [ + ("password", serde_json::json!("wrong horse")), + ("username", serde_json::json!("bob")), + ("domain", serde_json::json!("OTHER")), + ("private_key", serde_json::json!("-----BEGIN KEY-----")), + ("hostname", serde_json::json!("rdp2.example.test")), + ("port", serde_json::json!(3390)), + ("jump_password", serde_json::json!("hop")), + ] { + let other = entry_request(serde_json::json!({ key: value })); + assert_ne!( + base.credential_fingerprint(), + other.credential_fingerprint(), + "{key} must change the fingerprint" + ); + } + // Display params are not part of it: a second click from a resized + // window still reuses the session. + let resized = entry_request(serde_json::json!({ "width": 800, "height": 600 })); + assert_eq!( + base.credential_fingerprint(), + resized.credential_fingerprint() + ); + // Absent and empty are different, and so is a shifted boundary. + let empty_pw = entry_request(serde_json::json!({ "password": "" })); + let no_pw = { + let mut r = entry_request(serde_json::json!({})); + r.password = None; + r + }; + assert_ne!( + empty_pw.credential_fingerprint(), + no_pw.credential_fingerprint() + ); + let ab_c = entry_request(serde_json::json!({ "username": "ab", "password": "c" })); + let a_bc = entry_request(serde_json::json!({ "username": "a", "password": "bc" })); + assert_ne!(ab_c.credential_fingerprint(), a_bc.credential_fingerprint()); + } + + #[tokio::test] + async fn fresh_pending_session_reused_for_same_entry_and_credentials() { + let mgr = new_manager_for_tests(); + let req = entry_request(serde_json::json!({})); + let id = insert_session(&mgr, pending_entry_session(&req, "alice@example.com")).await; + + let again = entry_request(serde_json::json!({})); + let found = mgr + .find_reusable_session( + "alice@example.com", + "shared/ops/rdp-1", + &again.credential_fingerprint(), + ) + .await + .expect("same entry, same credentials, within the window: reused"); + assert_eq!(found.session_id, id); + } + + #[tokio::test] + async fn retry_with_different_credentials_gets_a_new_session() { + let mgr = new_manager_for_tests(); + let req = entry_request(serde_json::json!({})); + insert_session(&mgr, pending_entry_session(&req, "alice@example.com")).await; + + // A mistyped password at the prompt, retried within 15 s: the old + // Pending session holds the old password, so it must not be handed + // back. + let retry = entry_request(serde_json::json!({ "password": "wrong horse" })); + assert!(mgr + .find_reusable_session( + "alice@example.com", + "shared/ops/rdp-1", + &retry.credential_fingerprint(), + ) + .await + .is_none()); + let other_user = entry_request(serde_json::json!({ "username": "bob" })); + assert!(mgr + .find_reusable_session( + "alice@example.com", + "shared/ops/rdp-1", + &other_user.credential_fingerprint(), + ) + .await + .is_none()); + } + + #[tokio::test] + async fn reuse_is_scoped_to_user_entry_status_and_age() { + let mgr = new_manager_for_tests(); + let req = entry_request(serde_json::json!({})); + let fp = req.credential_fingerprint(); + + insert_session(&mgr, pending_entry_session(&req, "alice@example.com")).await; + // Another user's identical Pending session is not ours to take. + assert!(mgr + .find_reusable_session("mallory@example.com", "shared/ops/rdp-1", &fp) + .await + .is_none()); + // Nor is the same user's session for a different entry. + assert!(mgr + .find_reusable_session("alice@example.com", "shared/ops/rdp-2", &fp) + .await + .is_none()); + + // An owner already attached: the stream is consumed, not re-ownable. + let mgr = new_manager_for_tests(); + let mut active = pending_entry_session(&req, "alice@example.com"); + active.status = SessionStatus::Active; + insert_session(&mgr, active).await; + assert!(mgr + .find_reusable_session("alice@example.com", "shared/ops/rdp-1", &fp) + .await + .is_none()); + + // Older than the window: left to the pending-timeout reaper. + let mgr = new_manager_for_tests(); + let mut stale = pending_entry_session(&req, "alice@example.com"); + stale.created_at = + Utc::now() - chrono::Duration::seconds(SessionManager::REUSABLE_PENDING_SECS + 1); + insert_session(&mgr, stale).await; + assert!(mgr + .find_reusable_session("alice@example.com", "shared/ops/rdp-1", &fp) + .await + .is_none()); + + // No guacd stream (deferred connect): nothing to hand over yet. + let mgr = new_manager_for_tests(); + let mut deferred = pending_entry_session(&req, "alice@example.com"); + deferred.guacd_stream = None; + insert_session(&mgr, deferred).await; + assert!(mgr + .find_reusable_session("alice@example.com", "shared/ops/rdp-1", &fp) + .await + .is_none()); + } } impl std::fmt::Display for SessionError { diff --git a/static/client.html b/static/client.html index 055d1961..49fa3dea 100644 --- a/static/client.html +++ b/static/client.html @@ -1750,6 +1750,8 @@

Session Ended

var body = { width: window.innerWidth, height: window.innerHeight, + screen_width: screen.width, + screen_height: screen.height, dpi: Math.round((window.devicePixelRatio || 1) * 96) }; diff --git a/static/connections.html b/static/connections.html index 4b0f0b24..1e888bea 100644 --- a/static/connections.html +++ b/static/connections.html @@ -1601,7 +1601,7 @@

Enter Credentials

method: 'POST', headers: Object.assign({'Content-Type': 'application/json'}, apiHeaders()), credentials: 'same-origin', - body: JSON.stringify({ width: window.innerWidth, height: window.innerHeight }) + body: JSON.stringify({ width: window.innerWidth, height: window.innerHeight, screen_width: screen.width, screen_height: screen.height }) }) .then(function(res) { return res.json(); }) .then(function(data) { @@ -1895,7 +1895,7 @@

Enter Credentials

method: 'POST', headers: Object.assign({'Content-Type': 'application/json'}, apiHeaders()), credentials: 'same-origin', - body: JSON.stringify({ width: window.innerWidth, height: window.innerHeight }) + body: JSON.stringify({ width: window.innerWidth, height: window.innerHeight, screen_width: screen.width, screen_height: screen.height }) }).then(function(res) { return res.json(); }).then(function(data) { if (data.session_id) window.open('/client/' + data.session_id, '_blank'); else showError(data.error || 'Failed to reconnect'); @@ -3021,6 +3021,13 @@

Enter Credentials

body.height = bh; if (!body.dpi) body.dpi = Math.round((window.devicePixelRatio || 1) * 96); } + // And the screen's, so an entry that opens fullscreen on connect + // is created at the size it will be shown at (no resize after + // the fullscreen transition). + if (!body.screen_width && window.screen && screen.width > 0 && screen.height > 0) { + body.screen_width = screen.width; + body.screen_height = screen.height; + } fetch('/api/addressbook/folders/' + encodeURIComponent(scope) + '/' + encodeURIComponent(folder) + '/entries/' + encodeURIComponent(name) + '/connect', { method: 'POST', headers: apiHeaders({ 'Content-Type': 'application/json' }),