gsadmin/rustguac
1
0
Fork 0
mirror of https://github.com/sol1/rustguac.git synced 2026-10-25 13:09:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

spice: blank TLS port means the entry's port

Browse Source 
The connection editor labels a blank SPICE TLS port "(same as Port)",
and the jump-host path already tunnels tls_port.unwrap_or(port). The
direct path disagreed: with TLS on it sent an empty plain port (so
spice-gtk does not try plaintext) and an empty tls-port when none was
set, and guacd's SPICE plugin then defaulted the TLS port to 5900. A
TLS-only server on any other port was unreachable unless TLS port was
filled in by hand.

Add SpiceParams::dial_port(), which returns the TLS port with TLS on,
falling back to the entry's port, and use it for both the tls-port
connect arg and the jump-host tunnel target so the two paths agree.
The SPICE arg mapping moves into spice_arg_value() so it can be unit
tested, and a stale comment claiming the password is delivered over
argv is dropped.

Proxmox consoles always set tls_port from the spiceproxy response and
are unaffected.

Fixes #309

(cherry picked from commit f5c893ab25)
This commit is contained in:
Jad Seifeddine
2026-10-19 14:35:09 +07:00
committed by Dave Kempe
parent 1450164eca
commit 30913f586d
3 changed files with 117 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/api.md
+1 -1
View File
@@ -255,7 +255,7 @@ Requests from admins, an admin API key included, are used as sent.
| `port` | integer | SPICE port (default 5900) |
| `password` | string | SPICE password / ticket |
| `spice_tls` | boolean | Connect using TLS |
| `spice_tls_port` | integer | TLS port, when different from `port` |
| `spice_tls_port` | integer | TLS port, when different from `port` (defaults to `port`) |
| `spice_ca_cert` | string | PEM CA certificate for TLS verification |
| `spice_cert_subject` | string | Expected TLS certificate subject |
| `spice_proxy` | string | SPICE proxy URL, e.g. `http://host:3128` |
src/guacd.rs
+114 -39
View File
@@ -147,6 +147,71 @@ pub struct SpiceParams {
pub secondary_monitors: u32,
}
impl SpiceParams {
/// The port guacd dials: the TLS port with TLS on (falling back to `port`
/// when no separate TLS port is set), otherwise `port`.
pub fn dial_port(&self) -> u16 {
if self.tls {
self.tls_port.unwrap_or(self.port)
} else {
self.port
}
}
}
/// Resolve a guacd SPICE connect arg name to its value.
///
/// SPICE has no width/height/dpi connect args: it sizes via the `size`
/// instruction.
fn spice_arg_value(name: &str, p: &SpiceParams) -> String {
match name {
"hostname" => p.hostname.clone(),
// TLS SPICE (e.g. Proxmox) is TLS-only: send an empty plain
// port so guacd/spice-gtk connects via tls-port with TLS rather
// than plaintext against a TLS endpoint.
"port" => {
if p.tls {
String::new()
} else {
p.port.to_string()
}
}
// Credentials go in the connect args (not a post-connect argv
// stream): guacd's SPICE client sets settings->password on the
// session and authenticates during connect without awaiting
// argv, so argv delivery races the auth. The connect arg is set
// before the server connection is opened.
"username" => p.username.clone().unwrap_or_default(),
"password" => p.password.clone().unwrap_or_default(),
"tls" => if p.tls { "true" } else { "false" }.into(),
// With TLS on, always name the port to dial: a blank TLS port means
// "same as Port" (as in the editor and the jump-host tunnel), and an
// empty value would make guacd fall back to 5900.
"tls-port" => {
if p.tls {
p.dial_port().to_string()
} else {
p.tls_port.map(|x| x.to_string()).unwrap_or_default()
}
}
"ca-cert" => p.ca_cert.clone().unwrap_or_default(),
"cert-subject" => p.cert_subject.clone().unwrap_or_default(),
"ignore-cert" => if p.ignore_cert { "true" } else { "false" }.into(),
"proxy" => p.proxy.clone().unwrap_or_default(),
"color-depth" => p.color_depth.map_or("24".into(), |d| d.to_string()),
"read-only" => "false".into(),
"swap-red-blue" => "false".into(),
"enable-audio" => if p.enable_audio { "true" } else { "false" }.into(),
"disable-copy" => if p.disable_copy { "true" } else { "false" }.into(),
"disable-paste" => if p.disable_paste { "true" } else { "false" }.into(),
"secondary-monitors" => p.secondary_monitors.to_string(),
_ => {
tracing::debug!("Unknown guacd SPICE parameter '{}', sending empty", name);
String::new()
}
}
}
/// RDP connection parameters to pass to guacd.
pub struct RdpParams {
pub hostname: String,
@@ -408,45 +473,7 @@ pub async fn connect_and_handshake(
String::new()
}),
},
// Note: SPICE has no width/height/dpi connect args (it sizes via the
// `size` instruction) and no `password` arg (delivered via argv below).
ConnectionParams::Spice(p) => match name.as_str() {
"hostname" => p.hostname.clone(),
// TLS SPICE (e.g. Proxmox) is TLS-only: send an empty plain
// port so guacd/spice-gtk connects via tls-port with TLS rather
// than plaintext against a TLS endpoint.
"port" => {
if p.tls {
String::new()
} else {
p.port.to_string()
}
}
// Credentials go in the connect args (not a post-connect argv
// stream): guacd's SPICE client sets settings->password on the
// session and authenticates during connect without awaiting
// argv, so argv delivery races the auth. The connect arg is set
// before the server connection is opened.
"username" => p.username.clone().unwrap_or_default(),
"password" => p.password.clone().unwrap_or_default(),
"tls" => if p.tls { "true" } else { "false" }.into(),
"tls-port" => p.tls_port.map(|x| x.to_string()).unwrap_or_default(),
"ca-cert" => p.ca_cert.clone().unwrap_or_default(),
"cert-subject" => p.cert_subject.clone().unwrap_or_default(),
"ignore-cert" => if p.ignore_cert { "true" } else { "false" }.into(),
"proxy" => p.proxy.clone().unwrap_or_default(),
"color-depth" => p.color_depth.map_or("24".into(), |d| d.to_string()),
"read-only" => "false".into(),
"swap-red-blue" => "false".into(),
"enable-audio" => if p.enable_audio { "true" } else { "false" }.into(),
"disable-copy" => if p.disable_copy { "true" } else { "false" }.into(),
"disable-paste" => if p.disable_paste { "true" } else { "false" }.into(),
"secondary-monitors" => p.secondary_monitors.to_string(),
_ => {
tracing::debug!("Unknown guacd SPICE parameter '{}', sending empty", name);
String::new()
}
},
ConnectionParams::Spice(p) => spice_arg_value(name, p),
})
.collect();
@@ -718,4 +745,52 @@ mod tests {
let got = join_arg_values(&names(&["hostname", "read-only", "port"]), true);
assert_eq!(got, vec!["", "true", ""]);
}
fn spice(tls: bool, port: u16, tls_port: Option<u16>) -> SpiceParams {
SpiceParams {
hostname: "spice.example.com".into(),
port,
password: None,
username: None,
tls,
tls_port,
ca_cert: None,
cert_subject: None,
ignore_cert: false,
proxy: None,
color_depth: None,
width: 1024,
height: 768,
dpi: 96,
disable_copy: false,
disable_paste: false,
enable_audio: false,
secondary_monitors: 0,
}
}
#[test]
fn spice_tls_blank_tls_port_uses_port() {
let p = spice(true, 61000, None);
assert_eq!(p.dial_port(), 61000);
assert_eq!(spice_arg_value("port", &p), "");
assert_eq!(spice_arg_value("tls-port", &p), "61000");
}
#[test]
fn spice_tls_explicit_tls_port_wins() {
let p = spice(true, 5900, Some(5901));
assert_eq!(p.dial_port(), 5901);
assert_eq!(spice_arg_value("port", &p), "");
assert_eq!(spice_arg_value("tls-port", &p), "5901");
}
#[test]
fn spice_plain_sends_port_only() {
let p = spice(false, 5930, None);
assert_eq!(p.dial_port(), 5930);
assert_eq!(spice_arg_value("port", &p), "5930");
assert_eq!(spice_arg_value("tls-port", &p), "");
assert_eq!(spice_arg_value("tls", &p), "false");
}
}
src/session.rs
+2 -8
View File
@@ -1852,14 +1852,8 @@ impl SessionManager {
guacd::ConnectionParams::Ssh(p) => (p.hostname.clone(), p.port),
guacd::ConnectionParams::Rdp(p) => (p.hostname.clone(), p.port),
guacd::ConnectionParams::Vnc(p) => (p.hostname.clone(), p.port),
// TLS SPICE connects on tls_port, so tunnel that port.
guacd::ConnectionParams::Spice(p) => {
if p.tls {
(p.hostname.clone(), p.tls_port.unwrap_or(p.port))
} else {
(p.hostname.clone(), p.port)
}
}
// Tunnel the port guacd would dial (tls_port for TLS SPICE).
guacd::ConnectionParams::Spice(p) => (p.hostname.clone(), p.dial_port()),
}
};