From a1bc0a999573524fd90d8ef270335a9bbf3b24cb Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Tue, 20 Oct 2026 11:43:39 +0200 Subject: [PATCH] vault, sftp: a key reads sealed files only when allowed to Since e4e3e2b every registered SSH key could read sealed files over the mount while the vault was open in the browser. A key is a standing credential on another machine, so it now needs vault access, given and taken per key with :sshkey vault on|off (PATCH /api/ssh-keys/{id}, migration 0016). Existing keys start without it. The access is asked on every read of a sealed file, so taking it away cuts off a read under way, as locking does. That includes a write handle opened over a sealed file: its .part holds the decrypted bytes and could be read back through it. A mount no longer holds the vault open: it takes the key through the new Vault::peek_key, which does not count as use. Opening sealed files over and over from an indexer kept the idle lock from ever firing. The rest of the audit's findings on this side: - An unfinished upload's .part is removed when it is dropped, so a sealed file's plaintext does not sit in uploads/ after a client goes away. - A deleted key can no longer write, setstat or close-and-save through a session that was already open. - A session holds at most 256 handles and 64 MiB in memory, the server at most 32 connections; keepalives drop a peer that stopped answering. - A sealed download in the browser ends when the vault locks, the way a read over the mount already did. - Certificate inspection refuses a key or keystore that asks for more than ten million rounds, or scrypt beyond 2^20 and r = 8, before running the derivation: the file chooses those numbers, and the work could not be stopped once begun. --- README.md | 21 ++- migrations/0016_ssh_key_vault.sql | 4 + src/routes/inspect.rs | 135 ++++++++++++++ src/routes/media.rs | 49 +++-- src/routes/mod.rs | 5 +- src/routes/ssh_keys.rs | 38 +++- src/sftp/fs.rs | 293 ++++++++++++++++++++++++++---- src/sftp/mod.rs | 45 ++++- src/vault.rs | 28 +++ web/src/lib/api.ts | 5 + web/src/lib/commands.ts | 22 ++- 11 files changed, 578 insertions(+), 67 deletions(-) create mode 100644 migrations/0016_ssh_key_vault.sql diff --git a/README.md b/README.md index f591c1d..8ad2bf6 100644 --- a/README.md +++ b/README.md @@ -138,6 +138,7 @@ keys get in, and only the ones registered from inside the workspace: :sshkey add laptop ssh-ed25519 AAAA… you@laptop :sshkey lists them, with their fingerprints :sshkey rm laptop and this one is out, mounted or not +:sshkey vault laptop on this key may read sealed files while the vault is open ``` ```sh @@ -268,13 +269,19 @@ limit at least that large too. The SFTP mount shows files beside the documents, under their own names — a later file that shares one is told apart by its id, `slides (42).pdf` — and anything copied in that is not a `.md` becomes a file in the tree, streamed to -disk under the same size limit. While the vault is open in the browser a -sealed file opens there too, decrypted as it is read, and is sealed again when -it is saved, wherever it sits; a file copied into a sealed folder is sealed on -the way in. While the vault is shut both are refused, and locking it cuts off -a read already under way — though a client that cached what it read keeps -that copy until it lets go of it. Reading over the -mount does not hold the vault open; only the browser and opening a file do. +disk under the same size limit. A file copied into a sealed folder is sealed +on the way in while the vault is open in the browser. Reading one back takes +more: a key is a standing credential kept on another machine, so it reads +sealed files only once given vault access with `:sshkey vault on`, and +none has it to start with. Such a key, while the vault is open, opens a sealed +file decrypted as it is read, and it is sealed again when it is saved, +wherever it sits. While the vault is shut, or once the access is taken away, +both are refused, and either cuts off a read already under way — though a +client that cached what it read keeps that copy until it lets go of it. +Nothing done over the mount holds the vault open; only the browser does. A +session holds at most 256 handles and 64 MB in memory, the server at most 32 +connections, and one whose client has stopped answering is dropped within a +few minutes. Editor droppings — swap files, `~` backups, dotted temp files — stay in memory for the session, and a dotted temp file renamed onto a real name is kept as that file, which is how `rsync` and most editors save. diff --git a/migrations/0016_ssh_key_vault.sql b/migrations/0016_ssh_key_vault.sql new file mode 100644 index 0000000..24c8a22 --- /dev/null +++ b/migrations/0016_ssh_key_vault.sql @@ -0,0 +1,4 @@ +-- Whether a key may read sealed files over the mount while the vault is open. +-- A key is a standing credential kept on some other machine, so it starts +-- without: the owner gives it in the browser, key by key. +ALTER TABLE ssh_keys ADD COLUMN vault INTEGER NOT NULL DEFAULT 0; diff --git a/src/routes/inspect.rs b/src/routes/inspect.rs index e2a9313..760c5c7 100644 --- a/src/routes/inspect.rs +++ b/src/routes/inspect.rs @@ -100,6 +100,8 @@ pub struct Inspection { pub enum Failure { Unreadable, WrongPassword, + /// Asks its key derivation for more work than any real export does. + Unreasonable, } pub async fn inspect( @@ -159,6 +161,9 @@ pub async fn inspect( Err(Failure::Unreadable) => Err(AppError::BadRequest( "not a certificate, key or keystore this can read".into(), )), + Err(Failure::Unreasonable) => Err(AppError::BadRequest( + "this keystore asks for unreasonable key-stretching, and is not opened".into(), + )), } } @@ -457,6 +462,7 @@ impl Found { ); return Ok(()); }; + reasonable_stretching(der)?; let plain = info.decrypt(password).map_err(|_| Failure::WrongPassword)?; if !self.private_key(plain.as_bytes(), true, None) { return Err(Failure::WrongPassword); @@ -467,6 +473,7 @@ impl Found { /// A keystore is tried with no password first, as plenty are exported /// with an empty one. fn pkcs12(&mut self, der: &[u8], password: Option<&str>) -> Result<(), Failure> { + reasonable_stretching(der)?; let archive = match Pkcs12Archive::from_pkcs12(der, password.unwrap_or("")) { Ok(archive) => archive, Err(e) if password_problem(&e) => { @@ -492,6 +499,84 @@ impl Found { } } +/// The most rounds a key derivation may ask for. Exports use from two +/// thousand to a few hundred thousand; ten million already takes seconds. +const MAX_ROUNDS: u64 = 10_000_000; +/// scrypt's cost and block size, at most: a gigabyte of memory between them. +const MAX_SCRYPT_COST: u64 = 1 << 20; +const MAX_SCRYPT_BLOCK: u64 = 8; +const SCRYPT: &str = "1.3.6.1.4.1.11591.4.11"; + +/// Refuse a key or keystore whose key derivation asks for more than any real +/// one does, before running it. The parameters are the file's to choose, and +/// a derivation runs to its end on a blocking thread whatever deadline the +/// request had — a crafted file opened in the browser would pin that thread +/// for hours, or ask scrypt for more memory than the machine has. +/// +/// Every place a derivation takes its parameters from has the salt +/// followed by the count: PBKDF2's parameters, the PKCS#12 PBEs', the MAC's +/// `{ digest, salt, iterations }`, and scrypt's `{ salt, N, r, p }`. So any +/// sequence with an INTEGER after an OCTET STRING is held to the ceiling, +/// throughout the file, inside the octet strings the content sits in too. +fn reasonable_stretching(der: &[u8]) -> Result<(), Failure> { + use x509_parser::der_parser::der::parse_der; + match parse_der(der) { + Ok((_, object)) => stretching_in(&object), + Err(_) => Ok(()), + } +} + +fn stretching_in(object: &x509_parser::der_parser::ber::BerObject) -> Result<(), Failure> { + use x509_parser::der_parser::{ber::BerObjectContent as Content, der::parse_der}; + + let count = |object: &x509_parser::der_parser::ber::BerObject| { + object.as_u64().map_err(|_| Failure::Unreasonable) + }; + // What a constructed or wrapping value holds, when it is DER itself. + let inner = |mut bytes: &[u8]| -> Result<(), Failure> { + while let Ok((rest, object)) = parse_der(bytes) { + stretching_in(&object)?; + if rest.len() == bytes.len() { + break; + } + bytes = rest; + } + Ok(()) + }; + match &object.content { + Content::Sequence(items) | Content::Set(items) => { + if let [algorithm, parameters] = items.as_slice() { + let scrypt = algorithm + .as_oid_val() + .is_ok_and(|oid| oid.to_id_string() == SCRYPT); + if scrypt { + let fields = parameters + .as_sequence() + .map_err(|_| Failure::Unreasonable)?; + let cost = fields.get(1).map(count).transpose()?.unwrap_or(0); + let block = fields.get(2).map(count).transpose()?.unwrap_or(0); + if cost > MAX_SCRYPT_COST || block > MAX_SCRYPT_BLOCK { + return Err(Failure::Unreasonable); + } + } + } + for pair in items.windows(2) { + if matches!(pair[0].content, Content::OctetString(_)) + && matches!(pair[1].content, Content::Integer(_)) + && count(&pair[1])? > MAX_ROUNDS + { + return Err(Failure::Unreasonable); + } + } + items.iter().try_for_each(stretching_in) + } + Content::OctetString(bytes) => inner(bytes), + Content::Tagged(_, _, object) => stretching_in(object), + Content::Unknown(any) => inner(any.data), + _ => Ok(()), + } +} + /// Whatever goes wrong once the structure has been recognised: a bad MAC, or /// bags that decrypt to garbage. fn password_problem(e: &P12Error) -> bool { @@ -757,6 +842,56 @@ mod tests { assert!(json.len() < 2048, "{json}"); } + /// A count past the ceiling is refused wherever it sits, before any + /// derivation runs; the counts real exports use pass. + #[test] + fn refuses_unreasonable_key_stretching() { + let params = |count: &[u8]| { + let mut der = vec![0x04, 0x02, 0xaa, 0xbb, 0x02, count.len() as u8]; + der.extend_from_slice(count); + let mut seq = vec![0x30, der.len() as u8]; + seq.extend(der); + seq + }; + assert!(reasonable_stretching(¶ms(&[0x08, 0x00])).is_ok()); + assert!(matches!( + reasonable_stretching(¶ms(&[0x7f, 0xff, 0xff, 0xff])), + Err(Failure::Unreasonable) + )); + + // Inside an octet string inside a sequence, as a keystore's bags are. + let deep = params(&[0x7f, 0xff, 0xff, 0xff]); + let mut wrapped = vec![0x04, deep.len() as u8]; + wrapped.extend(deep); + let mut outer = vec![0x30, wrapped.len() as u8]; + outer.extend(wrapped); + assert!(matches!( + reasonable_stretching(&outer), + Err(Failure::Unreasonable) + )); + + // scrypt { salt, N = 2^22, r = 8, p = 1 }: four gigabytes. + let oid = [ + 0x06, 0x09, 0x2b, 0x06, 0x01, 0x04, 0x01, 0xda, 0x47, 0x04, 0x0b, + ]; + let fields = [ + 0x30, 0x0f, 0x04, 0x02, 0xaa, 0xbb, 0x02, 0x03, 0x40, 0x00, 0x00, 0x02, 0x01, 0x08, + 0x02, 0x01, 0x01, + ]; + let mut scrypt = vec![0x30, (oid.len() + fields.len()) as u8]; + scrypt.extend(oid); + scrypt.extend(fields); + assert!(matches!( + reasonable_stretching(&scrypt), + Err(Failure::Unreasonable) + )); + + // The fixtures are ordinary exports. + for fixture in [MODERN, LEGACY] { + assert!(reasonable_stretching(fixture).is_ok()); + } + } + #[test] fn refuses_what_it_cannot_read() { assert_eq!( diff --git a/src/routes/media.rs b/src/routes/media.rs index dabad5b..f7e89d6 100644 --- a/src/routes/media.rs +++ b/src/routes/media.rs @@ -1,4 +1,7 @@ -use std::{collections::BTreeSet, sync::LazyLock}; +use std::{ + collections::BTreeSet, + sync::{Arc, LazyLock}, +}; use axum::{ body::Body, @@ -541,7 +544,9 @@ pub async fn stream_media( file.seek(std::io::SeekFrom::Start(start)).await?; Body::from_stream(ReaderStream::new(file.take(length))) } - Some(key) => Body::from_stream(open_stream(file, &key, id, start, length).await?), + Some(key) => Body::from_stream( + open_stream(file, &key, state.vault.clone(), id, start, length).await?, + ), }; let mut response = ( @@ -589,10 +594,12 @@ pub async fn stream_media( /// its first byte, so seeking into a sealed video costs one record, not the /// whole file before it. The header is checked before the first byte goes /// out, so a file that will not open is an error response rather than a -/// download that breaks off. +/// download that breaks off — unless the vault locks while it runs: then it +/// does break off, as a read over the mount does. async fn open_stream( mut file: tokio::fs::File, key: &vault::Key, + vault: Arc, id: &str, start: u64, length: u64, @@ -615,21 +622,29 @@ async fn open_stream( Ok(futures_util::stream::try_unfold( (file, opener, skip, length), - |(mut file, mut opener, skip, left)| async move { - let len = opener.next_len(); - if len == 0 || left == 0 { - return Ok(None); + move |(mut file, mut opener, skip, left)| { + let vault = vault.clone(); + async move { + let len = opener.next_len(); + if len == 0 || left == 0 { + return Ok(None); + } + if !vault.is_open() { + return Err(std::io::Error::other( + "the vault locked during the download", + )); + } + let mut record = vec![0u8; len]; + file.read_exact(&mut record).await?; + let plain = opener.open(&record).map_err(std::io::Error::other)?; + let piece = &plain[skip.min(plain.len())..]; + let piece = &piece[..piece.len().min(usize::try_from(left).unwrap_or(usize::MAX))]; + let left = left - piece.len() as u64; + Ok(Some(( + axum::body::Bytes::copy_from_slice(piece), + (file, opener, 0, left), + ))) } - let mut record = vec![0u8; len]; - file.read_exact(&mut record).await?; - let plain = opener.open(&record).map_err(std::io::Error::other)?; - let piece = &plain[skip.min(plain.len())..]; - let piece = &piece[..piece.len().min(usize::try_from(left).unwrap_or(usize::MAX))]; - let left = left - piece.len() as u64; - Ok(Some(( - axum::body::Bytes::copy_from_slice(piece), - (file, opener, 0, left), - ))) }, )) } diff --git a/src/routes/mod.rs b/src/routes/mod.rs index 4de1c15..b9e0d53 100644 --- a/src/routes/mod.rs +++ b/src/routes/mod.rs @@ -89,7 +89,10 @@ pub fn api_router(max_upload: usize) -> Router { .route("/share/{token}/{key}", get(share::read)) .route("/share/{token}/{key}/media/{id}", get(share::serve_media)) .route("/ssh-keys", get(ssh_keys::list).post(ssh_keys::add)) - .route("/ssh-keys/{id}", delete(ssh_keys::remove)) + .route( + "/ssh-keys/{id}", + delete(ssh_keys::remove).patch(ssh_keys::update), + ) .layer(DefaultBodyLimit::max(JSON_LIMIT)) } diff --git a/src/routes/ssh_keys.rs b/src/routes/ssh_keys.rs index 460916a..3b5acaa 100644 --- a/src/routes/ssh_keys.rs +++ b/src/routes/ssh_keys.rs @@ -19,6 +19,8 @@ pub struct SshKeySummary { pub fingerprint: String, pub created_at: i64, pub last_used_at: Option, + /// May read sealed files over the mount while the vault is open. + pub vault: bool, } #[derive(Deserialize)] @@ -35,7 +37,8 @@ pub async fn list( State(state): State, ) -> AppResult>> { let rows = sqlx::query( - "SELECT id, name, fingerprint, created_at, last_used_at FROM ssh_keys ORDER BY created_at", + "SELECT id, name, fingerprint, created_at, last_used_at, vault FROM ssh_keys + ORDER BY created_at", ) .fetch_all(&state.db) .await?; @@ -48,6 +51,7 @@ pub async fn list( fingerprint: row.get("fingerprint"), created_at: row.get("created_at"), last_used_at: row.get("last_used_at"), + vault: row.get("vault"), }) .collect(), )) @@ -93,9 +97,41 @@ pub async fn add( fingerprint, created_at: now, last_used_at: None, + vault: false, })) } +#[derive(Deserialize)] +pub struct UpdateInput { + pub vault: bool, +} + +/// Give a key vault access, or take it away. A live session asks on every +/// read of a sealed file, so taking it away cuts off a read under way. +pub async fn update( + _: Session, + State(state): State, + Path(id): Path, + Json(input): Json, +) -> AppResult> { + let affected = sqlx::query("UPDATE ssh_keys SET vault = ?1 WHERE id = ?2") + .bind(input.vault) + .bind(id) + .execute(&state.db) + .await? + .rows_affected(); + if affected == 0 { + return Err(AppError::NotFound); + } + tracing::info!( + key = id, + vault = input.vault, + "ssh key vault access changed" + ); + state.backup.signal(); + Ok(Json(serde_json::json!({ "ok": true }))) +} + pub async fn remove( _: Session, State(state): State, diff --git a/src/sftp/fs.rs b/src/sftp/fs.rs index 42bb5f4..9325b44 100644 --- a/src/sftp/fs.rs +++ b/src/sftp/fs.rs @@ -9,11 +9,12 @@ //! export writes, and a leading heading written back becomes the title again. //! A file is streamed from `uploads/` and written back to it through a //! `.part`, never held in memory. A sealed file opens here while the vault is -//! open in the browser, decrypted a record at a time as it is read, and is -//! sealed again when it is written over; while the vault is shut it is listed -//! but refused, and locking the vault cuts off a read already under way. A -//! file written into a sealed folder, or moved into one, is sealed while the -//! vault is open and refused while it is shut. +//! open in the browser and the session's key has been given vault access, +//! decrypted a record at a time as it is read, and is sealed again when it is +//! written over; otherwise it is listed but refused, and locking the vault +//! cuts off a read already under way. A file written into a sealed folder, or +//! moved into one, is sealed while the vault is open and refused while it is +//! shut. Nothing a mount does keeps the vault open: only the browser does. //! //! What an editor writes next to a document — vim's `.swp` and `4913` probe, //! a `foo.md~` backup, a dotted temp file it renames over the original — lives @@ -45,6 +46,13 @@ type SftpResult = Result; /// buffered in memory until it is closed, so there has to be a ceiling. const MAX_FILE: usize = 16 * 1024 * 1024; +/// What one session may hold in memory at once — documents and scratch files +/// open for writing, and the scratch files kept — and how many handles it may +/// have open. Far more than an editor or a sync tool needs; a runaway client +/// is stopped before it takes the server's memory with it. +const MAX_HELD: usize = 64 * 1024 * 1024; +const MAX_HANDLES: usize = 256; + /// The byte size of a document as [`render_doc`] lays it out, computed in SQL /// so a directory listing does not have to fetch every body. const DOC_SIZE: &str = "length(CAST(body AS BLOB)) @@ -296,11 +304,32 @@ struct Upload { /// sealed file. sealed: bool, media_id: String, - partial: std::path::PathBuf, + partial: Partial, file: tokio::fs::File, size: u64, } +/// The `.part` an upload is written to. Removed when the upload is dropped +/// unfinished — a session that ends, a client that goes away mid-write — so +/// the plaintext of a sealed file opened for writing does not sit in +/// `uploads/` until the next sweep. After a finished upload it is gone +/// already, renamed or sealed into place. +struct Partial(std::path::PathBuf); + +impl std::ops::Deref for Partial { + type Target = std::path::Path; + + fn deref(&self) -> &std::path::Path { + &self.0 + } +} + +impl Drop for Partial { + fn drop(&mut self) { + let _ = std::fs::remove_file(&self.0); + } +} + pub struct SpaceFs { db: SqlitePool, /// The key this session logged in with; `None` only in tests. @@ -371,14 +400,73 @@ impl SpaceFs { self.vault_key().map(Some) } - /// The vault's key, while it is open; a refusal while it is shut. + /// The vault's key, while it is open; a refusal while it is shut. Taken + /// without counting as a use, so a mount never holds the vault open. fn vault_key(&self) -> SftpResult> { self.state .as_ref() - .and_then(|state| state.vault.key()) + .and_then(|state| state.vault.peek_key()) .ok_or(StatusCode::PermissionDenied) } + /// Refuse unless the session's key may read sealed files. A key is a + /// standing credential on some other machine; while the vault is open, + /// only the keys given vault access in the browser read plaintext. + /// Asked afresh each time, so taking the access away takes effect at once. + async fn vault_access(&self) -> SftpResult<()> { + let Some(key) = self.key_id else { + return Ok(()); + }; + let allowed: Option = sqlx::query_scalar("SELECT vault FROM ssh_keys WHERE id = ?1") + .bind(key) + .fetch_optional(&self.db) + .await + .map_err(|_| StatusCode::Failure)?; + match allowed { + Some(true) => Ok(()), + Some(false) => { + tracing::info!( + key, + "sftp: this key has no vault access, a sealed file stays shut" + ); + Err(StatusCode::PermissionDenied) + } + None => { + tracing::warn!(key, "sftp: the session's key has been removed, refusing"); + Err(StatusCode::PermissionDenied) + } + } + } + + /// Plaintext of a sealed file leaves only while the vault is open and the + /// key has vault access — checked on every read, not only at the open. + async fn may_read_sealed(&self) -> SftpResult<()> { + if !self.vault_open() { + return Err(StatusCode::PermissionDenied); + } + self.vault_access().await + } + + /// The bytes this session holds in memory: scratch files, and what is + /// open for writing or was read into a buffer. + fn held(&self) -> usize { + let open: usize = self + .handles + .values() + .map(|handle| match handle { + OpenHandle::Read(data) + | OpenHandle::Write { buf: data, .. } + | OpenHandle::Scratch { buf: data, .. } => data.len(), + _ => 0, + }) + .sum(); + open + self + .scratch + .values() + .map(|file| file.data.len()) + .sum::() + } + /// Whether the vault is open, asked without holding it open: a mount's /// listings and reads alone do not keep the key in memory. fn vault_open(&self) -> bool { @@ -408,8 +496,9 @@ impl SpaceFs { } /// Whether the key this session logged in with is still registered. - /// Asked whenever a directory or a file is opened, so a key deleted in - /// the browser locks out a mount that is already up, not only the next. + /// Asked whenever a directory or a file is opened, written or closed, so a + /// key deleted in the browser locks out a mount that is already up, not + /// only the next. async fn authorized(&self) -> SftpResult<()> { let Some(key) = self.key_id else { return Ok(()); @@ -493,11 +582,15 @@ impl SpaceFs { .map(|(_, file)| file)) } - fn allocate(&mut self, handle: OpenHandle) -> String { + fn allocate(&mut self, handle: OpenHandle) -> SftpResult { + if self.handles.len() >= MAX_HANDLES { + tracing::warn!("sftp: {MAX_HANDLES} handles open in one session, refusing another"); + return Err(StatusCode::Failure); + } self.next_handle += 1; let key = self.next_handle.to_string(); self.handles.insert(key.clone(), handle); - key + Ok(key) } /// A space or folder directly under `parent`, by slug — or by the slug its @@ -740,8 +833,13 @@ impl SpaceFs { // Refused at the open, not at the close, when the vault is shut. let over_sealed = replaces.is_some_and(|old| old.encrypted); let key = self.seal_key(parent, over_sealed).await?; + // Written into rather than over, a sealed file is decrypted into the + // `.part`, and what the handle holds can be read back through it. + if over_sealed && !truncate { + self.vault_access().await?; + } let media_id = uuid::Uuid::new_v4().to_string(); - let partial = self.uploads.join(format!("{media_id}.part")); + let partial = Partial(self.uploads.join(format!("{media_id}.part"))); let mut size = 0; if let (Some(old), false) = (replaces, truncate) { let source = self.uploads.join(&old.media_id); @@ -749,7 +847,8 @@ impl SpaceFs { // Written into, not over: the plaintext goes into the // `.part`, which is sealed again when it is closed. (Some(key), true) => { - let (key, id, part) = (key.clone(), old.media_id.clone(), partial.clone()); + let (key, id, part) = + (key.clone(), old.media_id.clone(), partial.to_path_buf()); tokio::task::spawn_blocking(move || -> anyhow::Result { let mut plain = vault::Reader::open(&key, &id, &source)?; Ok(std::io::copy( @@ -764,7 +863,7 @@ impl SpaceFs { StatusCode::Failure })? } - _ => tokio::fs::copy(source, &partial) + _ => tokio::fs::copy(source, &*partial) .await .map_err(|_| StatusCode::Failure)?, }; @@ -774,7 +873,7 @@ impl SpaceFs { .write(true) .create(true) .truncate(truncate || replaces.is_none()) - .open(&partial) + .open(&*partial) .await .map_err(|_| StatusCode::Failure)?; Ok(Upload { @@ -804,13 +903,8 @@ impl SpaceFs { size, } = upload; // Asked again: the vault may have locked while the bytes came in. - let key = match self.seal_key(parent, sealed).await { - Ok(key) => key, - Err(e) => { - let _ = tokio::fs::remove_file(&partial).await; - return Err(e); - } - }; + // Refused, the `.part` goes with `partial`. + let key = self.seal_key(parent, sealed).await?; let landed = async { file.sync_all().await?; drop(file); @@ -820,7 +914,7 @@ impl SpaceFs { // names and the mirror never reads. Some(key) => { let (key, id, plain, sealed) = - (key.clone(), media_id.clone(), partial.clone(), target); + (key.clone(), media_id.clone(), partial.to_path_buf(), target); tokio::task::spawn_blocking(move || -> anyhow::Result<()> { vault::seal_file(&key, &id, &mut std::fs::File::open(&plain)?, &sealed)?; std::fs::remove_file(&plain)?; @@ -829,7 +923,7 @@ impl SpaceFs { .await .map_err(anyhow::Error::from)??; } - None => tokio::fs::rename(&partial, target).await?, + None => tokio::fs::rename(&*partial, target).await?, } let mime = media::mime_of(&name, None); let sealed = key.is_some(); @@ -848,7 +942,7 @@ impl SpaceFs { .await; if let Err(e) = landed { tracing::warn!("sftp: could not store {name}: {e:?}"); - let _ = tokio::fs::remove_file(&partial).await; + drop(partial); let _ = tokio::fs::remove_file(self.uploads.join(&media_id)).await; return Err(StatusCode::Failure); } @@ -1083,6 +1177,7 @@ impl russh_sftp::server::Handler for SpaceFs { path: String, attrs: FileAttributes, ) -> SftpResult { + self.authorized().await?; let target = self.target(&path).await?; if let Some(size) = attrs.size { let size = size as usize; @@ -1096,7 +1191,11 @@ impl russh_sftp::server::Handler for SpaceFs { self.save_doc(doc.id, bytes).await?; } Target::Scratch(key) => { + let held = self.held(); if let Some(file) = self.scratch.get_mut(&key) { + if held.saturating_add(size.saturating_sub(file.data.len())) > MAX_HELD { + return Err(StatusCode::Failure); + } file.data.resize(size, 0); file.mtime = crate::now(); } @@ -1118,10 +1217,13 @@ impl russh_sftp::server::Handler for SpaceFs { handle: String, attrs: FileAttributes, ) -> SftpResult { + self.authorized().await?; + let held = self.held(); match self.handles.get_mut(&handle) { Some(OpenHandle::Write { buf, .. } | OpenHandle::Scratch { buf, .. }) => { if let Some(size) = attrs.size { - if size as usize > MAX_FILE { + let grows = (size as usize).saturating_sub(buf.len()); + if size as usize > MAX_FILE || held.saturating_add(grows) > MAX_HELD { return Err(StatusCode::Failure); } buf.resize(size as usize, 0); @@ -1150,7 +1252,7 @@ impl russh_sftp::server::Handler for SpaceFs { async fn opendir(&mut self, id: u32, path: String) -> SftpResult { self.authorized().await?; let files = self.listing(&path).await?; - let handle = self.allocate(OpenHandle::Dir(files)); + let handle = self.allocate(OpenHandle::Dir(files))?; Ok(Handle { id, handle }) } @@ -1261,6 +1363,7 @@ impl russh_sftp::server::Handler for SpaceFs { } else { let file = existing.ok_or(StatusCode::NoSuchFile)?; let key = if file.encrypted { + self.vault_access().await?; Some(self.vault_key()?) } else { None @@ -1301,7 +1404,7 @@ impl russh_sftp::server::Handler for SpaceFs { } }; - let handle = self.allocate(handle); + let handle = self.allocate(handle)?; Ok(Handle { id, handle }) } @@ -1312,12 +1415,18 @@ impl russh_sftp::server::Handler for SpaceFs { offset: u64, len: u32, ) -> SftpResult { - // Locking the vault shuts what is open as well as what opens next. - let vault_open = self.vault_open(); + // Locking the vault, or taking the key's access away, shuts what is + // open as well as what opens next — a sealed file written into + // included, whose handle reads back its plaintext. + let sealed = match self.handles.get(&handle) { + Some(OpenHandle::ReadSealed(_)) => true, + Some(OpenHandle::Upload(upload)) => upload.sealed, + _ => false, + }; + if sealed { + self.may_read_sealed().await?; + } if let Some(OpenHandle::ReadSealed(sealed)) = self.handles.get_mut(&handle) { - if !vault_open { - return Err(StatusCode::PermissionDenied); - } let data = sealed.read(offset, len).await?; return Ok(russh_sftp::protocol::Data { id, data }); } @@ -1372,8 +1481,16 @@ impl russh_sftp::server::Handler for SpaceFs { offset: u64, data: Vec, ) -> SftpResult { + self.authorized().await?; + let held = self.held(); match self.handles.get_mut(&handle) { Some(OpenHandle::Write { buf, .. } | OpenHandle::Scratch { buf, .. }) => { + let grows = (offset as usize) + .saturating_add(data.len()) + .saturating_sub(buf.len()); + if held.saturating_add(grows) > MAX_HELD { + return Err(StatusCode::Failure); + } write_at(buf, offset, &data)?; Ok(ok_status(id)) } @@ -1400,7 +1517,10 @@ impl russh_sftp::server::Handler for SpaceFs { } async fn close(&mut self, id: u32, handle: String) -> SftpResult { - match self.handles.remove(&handle) { + let handle = self.handles.remove(&handle); + // A removed key's writes are dropped rather than saved. + self.authorized().await?; + match handle { Some(OpenHandle::Write { id: node, buf, @@ -2076,6 +2196,103 @@ mod tests { assert_eq!(get(&mut fs, "/proj/open/data.bin").await, "over"); } + /// A key reads sealed files only once given vault access, and taking it + /// away cuts off a read under way. Nothing it does holds the vault open. + #[tokio::test] + async fn only_a_key_with_vault_access_reads_sealed_files() { + let dir = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(dir.path().join("uploads")).unwrap(); + let state = crate::routes::tests::state_in(dir.path().into()).await; + state.vault.open_with(zeroize::Zeroizing::new([9u8; 32])); + let key: i64 = sqlx::query_scalar( + "INSERT INTO ssh_keys (name, public_key, fingerprint, created_at) + VALUES ('laptop', 'ssh-ed25519 AAAA', 'SHA256:x', 1) RETURNING id", + ) + .fetch_one(&state.db) + .await + .unwrap(); + let set_access = |on: bool| { + sqlx::query("UPDATE ssh_keys SET vault = ?1 WHERE id = ?2") + .bind(on) + .bind(key) + .execute(&state.db) + }; + let mut fs = SpaceFs::new(state.clone(), Some(key)); + fs.max_upload = 1024; + for path in ["/proj", "/proj/vault"] { + fs.mkdir(0, path.into(), FileAttributes::empty()) + .await + .expect("mkdir"); + } + sqlx::query("UPDATE nodes SET sealed = 1 WHERE slug = 'vault'") + .execute(&state.db) + .await + .unwrap(); + const NOTE: &str = "/proj/vault/note.txt"; + + // Writing in seals, and needs no access: nothing comes back out. + put(&mut fs, NOTE, "secret").await; + + for flags in [OpenFlags::READ, OpenFlags::READ | OpenFlags::WRITE] { + assert!( + matches!( + open_at(&mut fs, NOTE, flags).await, + Err(StatusCode::PermissionDenied) + ), + "{flags:?} without access" + ); + } + // No `.part` of the plaintext stays behind a refused or dropped open. + let parts = || { + std::fs::read_dir(dir.path().join("uploads")) + .unwrap() + .filter(|e| { + e.as_ref() + .unwrap() + .path() + .extension() + .is_some_and(|x| x == "part") + }) + .count() + }; + assert_eq!(parts(), 0); + + set_access(true).await.unwrap(); + assert_eq!(get(&mut fs, NOTE).await, "secret"); + let handle = open_at(&mut fs, NOTE, OpenFlags::READ).await.unwrap(); + set_access(false).await.unwrap(); + assert!(matches!( + read_at(&mut fs, handle.clone(), 0, 6).await, + Err(StatusCode::PermissionDenied) + )); + fs.close(0, handle).await.unwrap(); + + // Opened for writing over it, the decrypted copy goes when the + // session does. + set_access(true).await.unwrap(); + let handle = open_at(&mut fs, NOTE, OpenFlags::READ | OpenFlags::WRITE) + .await + .unwrap(); + assert_eq!(read_at(&mut fs, handle, 0, 6).await.unwrap(), b"secret"); + assert_eq!(parts(), 1); + drop(fs); + assert_eq!(parts(), 0); + } + + /// One session cannot open handles without end. + #[tokio::test] + async fn handles_run_out_before_memory_does() { + let dir = tempfile::tempdir().unwrap(); + let mut fs = fs_in(dir.path().to_path_buf()).await; + fs.mkdir(0, "/proj".into(), FileAttributes::empty()) + .await + .unwrap(); + for _ in 0..super::MAX_HANDLES { + fs.opendir(0, "/proj".into()).await.unwrap(); + } + assert!(fs.opendir(0, "/proj".into()).await.is_err()); + } + const CREATE: OpenFlags = OpenFlags::WRITE .union(OpenFlags::CREATE) .union(OpenFlags::TRUNCATE); @@ -2092,6 +2309,12 @@ mod tests { fs.close(0, handle).await.expect("close"); } + async fn open_at(fs: &mut SpaceFs, path: &str, flags: OpenFlags) -> Result { + fs.open(0, path.into(), flags, FileAttributes::empty()) + .await + .map(|h| h.handle) + } + async fn read_at( fs: &mut SpaceFs, handle: String, diff --git a/src/sftp/mod.rs b/src/sftp/mod.rs index f80f3b9..7419038 100644 --- a/src/sftp/mod.rs +++ b/src/sftp/mod.rs @@ -5,7 +5,14 @@ mod fs; -use std::{net::SocketAddr, sync::Arc}; +use std::{ + net::SocketAddr, + sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, + }, + time::Duration, +}; use anyhow::{Context, Result}; use russh::{ @@ -58,32 +65,61 @@ pub async fn serve(state: AppState, bind: SocketAddr) -> Result<()> { let config = Arc::new(ServerConfig { keys: vec![host_key], + // Each refusal a second late — russh's ten tries stay, as a client + // offers every key its agent holds before the one registered here — + // and a peer that has gone away is found out within minutes rather + // than held on to: three keepalives unanswered, or half an hour of + // nothing at all from a client that does not answer them. + auth_rejection_time: Duration::from_secs(1), + keepalive_interval: Some(Duration::from_secs(60)), + keepalive_max: 3, + inactivity_timeout: Some(Duration::from_secs(30 * 60)), ..Default::default() }); - let mut server = SftpServer { state }; + let mut server = SftpServer { + state, + live: Arc::new(AtomicUsize::new(0)), + }; tracing::info!("sftp mount listening on {bind}"); server.run_on_address(config, bind).await?; Ok(()) } +/// More connections than one owner's machines would ever keep open; past it, +/// a new one is turned away at login rather than given a filesystem. +const MAX_CONNECTIONS: usize = 32; + #[derive(Clone)] struct SftpServer { state: AppState, + /// Connections open now, counted up here and down by `Live`. + live: Arc, } impl russh::server::Server for SftpServer { type Handler = SshSession; fn new_client(&mut self, _addr: Option) -> Self::Handler { + self.live.fetch_add(1, Ordering::SeqCst); SshSession { state: self.state.clone(), channel: None, key_id: None, + live: Live(self.live.clone()), } } } +/// One connection's place in the count, given back when it closes. +struct Live(Arc); + +impl Drop for Live { + fn drop(&mut self) { + self.0.fetch_sub(1, Ordering::SeqCst); + } +} + struct SshSession { state: AppState, /// The one channel a mount opens for its sftp subsystem. A second @@ -93,6 +129,7 @@ struct SshSession { /// The registered key this session logged in with, re-checked by the /// filesystem so that deleting a key ends what it mounted. key_id: Option, + live: Live, } impl ServerHandler for SshSession { @@ -103,6 +140,10 @@ impl ServerHandler for SshSession { _user: &str, public_key: &PublicKey, ) -> Result { + if self.live.0.load(Ordering::SeqCst) > MAX_CONNECTIONS { + tracing::warn!("sftp: {MAX_CONNECTIONS} connections open, refusing another"); + return Ok(Auth::reject()); + } let fingerprint = public_key .fingerprint(russh::keys::HashAlg::Sha256) .to_string(); diff --git a/src/vault.rs b/src/vault.rs index fc96b13..5f039a9 100644 --- a/src/vault.rs +++ b/src/vault.rs @@ -454,6 +454,16 @@ impl Vault { .is_some_and(|open| open.touched.elapsed() < self.idle) } + /// The key while the vault is open, without counting as a use: for what + /// is not the owner at the browser — a mount — which may use the vault + /// while it is open but must not keep it so. + pub fn peek_key(&self) -> Option> { + self.slot() + .as_ref() + .filter(|open| open.touched.elapsed() < self.idle) + .map(|open| open.key.clone()) + } + pub fn lock(&self) { *self.slot() = None; } @@ -683,6 +693,24 @@ mod tests { assert!(vault.slot().is_none()); } + /// A peek finds the key while it is there and leaves the idle clock + /// alone; a `key()` winds it back. + #[tokio::test] + async fn a_peek_does_not_hold_the_vault_open() { + let db = sqlx::SqlitePool::connect("sqlite::memory:").await.unwrap(); + let vault = super::Vault::new(db, std::time::Duration::from_millis(40)); + let started = std::time::Instant::now(); + *vault.slot() = Some(super::Unlocked { + key: std::sync::Arc::new(key(1)), + touched: started, + }); + assert!(vault.peek_key().is_some()); + assert_eq!(vault.slot().as_ref().unwrap().touched, started); + std::thread::sleep(std::time::Duration::from_millis(50)); + assert!(vault.peek_key().is_none(), "idle, whatever peeked"); + assert!(vault.key().is_none()); + } + /// Reading from record `k` on gives exactly the plaintext from /// `k * CHUNK` on, and the last record still has to be the last. #[test] diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index bc054c3..9f7eb3d 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -64,6 +64,8 @@ export interface SshKey { fingerprint: string created_at: number last_used_at: number | null + /** May read sealed files over the mount while the vault is open. */ + vault: boolean } export interface SharedDocument { @@ -464,4 +466,7 @@ export const api = { request('/ssh-keys', { method: 'POST', body: body({ name, public_key }) }), removeSshKey: (id: number) => request<{ ok: true }>(`/ssh-keys/${id}`, { method: 'DELETE' }), + + setSshKeyVault: (id: number, vault: boolean) => + request<{ ok: true }>(`/ssh-keys/${id}`, { method: 'PATCH', body: body({ vault }) }), } diff --git a/web/src/lib/commands.ts b/web/src/lib/commands.ts index ad9eb9d..e6ee8df 100644 --- a/web/src/lib/commands.ts +++ b/web/src/lib/commands.ts @@ -784,16 +784,16 @@ export const commands: CommandSpec[] = [ }, { name: 'sshkey', - args: '[add | rm ]', + args: '[add | rm | vault on|off]', group: 'view', - help: 'list, add or remove keys allowed to mount the workspace over sftp', + help: 'list, add or remove keys allowed to mount the workspace over sftp, and say which may read sealed files', run: (arg) => workspace.guard(async () => { const trimmed = arg.trim() if (trimmed === '' || trimmed === 'ls' || trimmed === 'list') { const keys = await api.listSshKeys() if (keys.length === 0) return workspace.say('no keys registered — :sshkey add ') - return workspace.say(keys.map((k) => `${k.name} ${k.fingerprint}`).join(' · ')) + return workspace.say(keys.map((k) => `${k.name}${k.vault ? ' [vault]' : ''} ${k.fingerprint}`).join(' · ')) } const addMatch = /^add\s+(\S+)\s+(.+)$/.exec(trimmed) if (addMatch) { @@ -809,7 +809,21 @@ export const commands: CommandSpec[] = [ await api.removeSshKey(target.id) return workspace.say(`removed ${target.name}`) } - workspace.say('usage: :sshkey [add | rm ]', 'error') + // A key reads sealed files over the mount only once given vault access. + const vaultMatch = /^vault\s+(\S+)\s+(on|off)$/.exec(trimmed) + if (vaultMatch) { + const keys = await api.listSshKeys() + const target = keys.find((k) => k.name === vaultMatch[1]) + if (!target) return workspace.say(`no key named ${vaultMatch[1]}`, 'error') + const on = vaultMatch[2] === 'on' + await api.setSshKeyVault(target.id, on) + return workspace.say( + on + ? `${target.name} reads sealed files over the mount while the vault is open` + : `${target.name} no longer reads sealed files`, + ) + } + workspace.say('usage: :sshkey [add | rm | vault on|off]', 'error') }), }, {