vdi: refuse container usernames that are not plain names

The username is a directory under home_base that is bind-mounted into
the container. An override could carry ../ and the guard compared paths
as text, so it passed. container_username must now be 1-32 letters,
digits, '.', '_' or '-', not starting with '.' or '-'; the Docker driver
also requires exactly one path component and compares real paths, so a
symlink in home_base cannot redirect the mount either.
This commit is contained in:
Dave Kempe
2026-10-18 10:38:53 +11:00
parent 2fa9877d85
commit fbe5dbc714
2 changed files with 75 additions and 5 deletions
+52
View File
@@ -1591,6 +1591,18 @@ impl SessionManager {
// are scoped per-operator. When the override is set the same
// container is shared by everyone connecting with that entry,
// which is the desired behaviour for shared baked-in accounts.
// The username names the container and the home directory
// under `home_base`, so an override must be a plain name: a
// `../` here would bind-mount any host directory.
if let Some(name) = req.container_username.as_deref().filter(|s| !s.is_empty()) {
if !valid_vdi_username(name) {
return Err(SessionError::ValidationError(
"container_username must be 1-32 letters, digits, '.', '_' or '-', \
not starting with '.' or '-'"
.into(),
));
}
}
let vdi_username = req
.container_username
.as_ref()
@@ -2704,6 +2716,20 @@ fn drive_cleanup_settings(drive: &Option<DriveConfig>) -> (bool, u64) {
}
}
/// A VDI container username: it becomes the container name suffix and a
/// directory under `home_base`, so only a plain name is accepted. Letters,
/// digits, '.', '_' and '-', at most 32 characters (the Linux limit), not
/// starting with '.' or '-' (which rules out `.` and `..`).
pub(crate) fn valid_vdi_username(name: &str) -> bool {
!name.is_empty()
&& name.len() <= 32
&& !name.starts_with('.')
&& !name.starts_with('-')
&& name
.chars()
.all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-'))
}
#[derive(Debug)]
pub enum SessionError {
GuacdConnection(String),
@@ -2767,6 +2793,32 @@ pub(crate) fn check_share_token_match(
mod tests {
use super::*;
#[test]
fn vdi_username_must_be_a_plain_name() {
// It becomes a directory under home_base that is bind-mounted into the
// container, so anything that could leave home_base is refused.
for ok in ["alice", "svc.backup", "j_smith-2", "A1", &"a".repeat(32)] {
assert!(valid_vdi_username(ok), "{ok} should be accepted");
}
for bad in [
"",
"..",
".",
"../../etc",
"../root",
"a/b",
"/etc",
".hidden",
"-rf",
"a b",
"a\\b",
"é",
&"a".repeat(33),
] {
assert!(!valid_vdi_username(bad), "{bad:?} should be refused");
}
}
// ── Typescript filename templating (#159) ──
fn ts_when() -> DateTime<Utc> {
+23 -5
View File
@@ -560,15 +560,33 @@ impl DockerDriver {
// Persistent home directory bind mount
let binds = if let Some(ref base) = spec.home_base {
let base_path = std::path::Path::new(base);
let host_path = base_path.join(&spec.username);
// Verify no path traversal (username is sanitized but belt-and-suspenders)
if !host_path.starts_with(base_path) {
return Err(VdiError::Docker("path traversal in home_base".into()));
// The username is validated where the session is made;
// this is the second line of defence. A plain name has
// exactly one normal path component, so `..`, `/` and an
// absolute path never get this far.
let mut parts = std::path::Path::new(&spec.username).components();
if !matches!(
(parts.next(), parts.next()),
(Some(std::path::Component::Normal(_)), None)
) {
return Err(VdiError::Docker(
"invalid VDI username for home_base".into(),
));
}
// Ensure host directory exists
let host_path = base_path.join(&spec.username);
if let Err(e) = std::fs::create_dir_all(&host_path) {
tracing::warn!(path = ?host_path, "Failed to create VDI home dir: {}", e);
}
// And compare real paths, so a symlink inside home_base
// cannot point the mount somewhere else either.
match (base_path.canonicalize(), host_path.canonicalize()) {
(Ok(b), Ok(h)) if h.starts_with(&b) && h != b => {}
_ => {
return Err(VdiError::Docker(
"VDI home directory is outside home_base".into(),
))
}
}
let mount = format!(
"{}:/home/{}:nosuid,nodev",
host_path.display(),