Fix review findings: PAX sizes, agent stdin, PEM parity, sticky writer

- absorb uses the PAX-corrected entry size (large Go/bsdtar archives were
  stored truncated) and refuses GNU sparse entries
- the agent reads the key from fd 0 straight into locked memory, and never
  unlinks a live agent's socket
- the key file PEM parser is a port of Go's pem.Decode
- the STREAM writer fails permanently after a write error; plaintext is
  cleared on reader error paths; AEAD, HMAC and hash states are zeroized
- add/migrate refuse paths inside the vault; publish reports filesystems
  without an atomic no-replace primitive clearly and treats a leftover
  temporary name as a warning; Windows rejects NTFS-special names
- the leak test runs in its own process

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Andy Dixon
2026-10-08 12:23:27 +01:00
co-authored by Claude
parent 25f50ed73b
commit 91d49aeecf
12 changed files with 358 additions and 69 deletions
Generated
+4
View File
@@ -88,6 +88,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d2f6c7dbe95a6ed67ad9f18e57daf93a2f034c524b99fd2b76d18fdfeb6660aa" checksum = "d2f6c7dbe95a6ed67ad9f18e57daf93a2f034c524b99fd2b76d18fdfeb6660aa"
dependencies = [ dependencies = [
"hybrid-array", "hybrid-array",
"zeroize",
] ]
[[package]] [[package]]
@@ -121,6 +122,7 @@ dependencies = [
"cfg-if", "cfg-if",
"cipher", "cipher",
"cpufeatures", "cpufeatures",
"zeroize",
] ]
[[package]] [[package]]
@@ -133,6 +135,7 @@ dependencies = [
"chacha20", "chacha20",
"cipher", "cipher",
"poly1305", "poly1305",
"zeroize",
] ]
[[package]] [[package]]
@@ -277,6 +280,7 @@ dependencies = [
"block-buffer", "block-buffer",
"crypto-common", "crypto-common",
"ctutils", "ctutils",
"zeroize",
] ]
[[package]] [[package]]
+4 -4
View File
@@ -11,11 +11,11 @@ publish = false
[dependencies] [dependencies]
argon2 = { version = "0.6", default-features = false, features = ["alloc", "zeroize"] } argon2 = { version = "0.6", default-features = false, features = ["alloc", "zeroize"] }
chacha20poly1305 = { version = "0.11", default-features = false, features = ["alloc"] } chacha20poly1305 = { version = "0.11", default-features = false, features = ["alloc", "zeroize"] }
hkdf = "0.13" hkdf = "0.13"
hmac = "0.13" hmac = { version = "0.13", features = ["zeroize"] }
sha2 = { version = "0.11", default-features = false } sha2 = { version = "0.11", default-features = false, features = ["zeroize"] }
sha3 = { version = "0.11", default-features = false } sha3 = { version = "0.11", default-features = false, features = ["zeroize"] }
ml-kem = { version = "0.3.2", features = ["zeroize", "hazmat"] } ml-kem = { version = "0.3.2", features = ["zeroize", "hazmat"] }
zeroize = { version = "1.9", features = ["derive", "serde"] } zeroize = { version = "1.9", features = ["derive", "serde"] }
getrandom = "0.4" getrandom = "0.4"
+15 -5
View File
@@ -63,7 +63,8 @@ None of this is erasure from physical memory.
`VAULT_PASSPHRASE` and `VAULT_DECOY_PASSPHRASE` removed from its environment, `VAULT_PASSPHRASE` and `VAULT_DECOY_PASSPHRASE` removed from its environment,
no secret in its arguments, its working directory `/` and standard error no secret in its arguments, its working directory `/` and standard error
closed. It hands over the 34-byte suite ID and seed through a pipe. Before closed. It hands over the 34-byte suite ID and seed through a pipe. Before
reading it, the agent: reading it (straight from the pipe into the locked page, with no
intermediate buffer), the agent:
1. sets the core-dump size limit to zero; 1. sets the core-dump size limit to zero;
2. on Linux, sets `PR_SET_DUMPABLE` to 0 (no core dumps, no same-user 2. on Linux, sets `PR_SET_DUMPABLE` to 0 (no core dumps, no same-user
@@ -108,16 +109,25 @@ for the passphrase.
A failure after a record is already in place never deletes its objects. A failure after a record is already in place never deletes its objects.
Windows has no directory fsync through the APIs used; NTFS journals Windows has no directory fsync through the APIs used; NTFS journals
metadata. metadata.
- `migrate` checks an original is still the file it stored (device, inode,
size, time; on Windows only size and time) and then deletes it by name; a
replacement in the instant between the check and the deletion is not
excluded. Paths inside the vault directory are refused outright.
- A filesystem with neither `RENAME_NOREPLACE` nor hard links (some FUSE
mounts, FAT) cannot publish without risking a replace, so `get` fails
there with a message saying so rather than fall back.
- The vault lock uses the same POSIX `fcntl` locks as the Go version, so - The vault lock uses the same POSIX `fcntl` locks as the Go version, so
mixed Go and Rust processes exclude each other. Only a read-only mixed Go and Rust processes exclude each other. Only a read-only
filesystem lets a reader proceed without the lock. filesystem lets a reader proceed without the lock.
## Plaintext on disk ## Plaintext on disk
- `vault add URL` and `vault mirror` stream non-HTML downloads to a private - `vault add URL` and `vault mirror` stream non-HTML downloads to a 0600
(0600) temporary file in `$TMPDIR` before storing them, because the size temporary file in `$TMPDIR` before storing them, because the size must be
must be known first; it is removed on every normal path, including known first. On Unix the file is unlinked as soon as it is created, so it
errors. HTML and CSS are rewritten in memory. `vault absorb` stores tar has no name and disappears even if vault is killed; on other systems it
is a named file removed on every normal path, including errors. HTML and
CSS are rewritten in memory. `vault absorb` stores tar
entries straight from the stream with no temporary file. entries straight from the stream with no temporary file.
- `vault get` writes plaintext where you ask; `vault migrate` deletes the - `vault get` writes plaintext where you ask; `vault migrate` deletes the
originals with `unlink`, which is not secure deletion. originals with `unlink`, which is not secure deletion.
+8
View File
@@ -23,6 +23,14 @@ Safety and correctness:
use, see `SECURITY.md`.) use, see `SECURITY.md`.)
- **Extracted files are synced** to disk before they appear under their - **Extracted files are synced** to disk before they appear under their
name, and their directory afterwards. name, and their directory afterwards.
- **`get` needs an atomic no-replace primitive.** On a filesystem with
neither `RENAME_NOREPLACE` nor hard links (some FUSE mounts, FAT) it fails
with a message saying so, where Go would rename.
- **`add`/`migrate` refuse paths inside the vault directory** (Go refused
only the directory itself; `migrate store/records` would have deleted the
vault's records).
- **`absorb` refuses GNU sparse files** (Go expanded them), and honours PAX
size records as Go does.
- **A source file that changes while being added is not stored.** If it - **A source file that changes while being added is not stored.** If it
grows, shrinks, is modified or replaced during `add`/`migrate`, the grows, shrinks, is modified or replaced during `add`/`migrate`, the
command fails and nothing is stored. Go stored whatever it read (for a command fails and nothing is stored. Go stored whatever it read (for a
+56 -4
View File
@@ -52,10 +52,14 @@ pub fn absorb(
}; };
let name = join(&[dest, &rel]); let name = join(&[dest, &rel]);
let name = VaultName::new(&name).ok_or_else(|| Error::new(format!("invalid vault path {name:?}")))?; let name = VaultName::new(&name).ok_or_else(|| Error::new(format!("invalid vault path {name:?}")))?;
let size = entry // The entry's size, with a PAX "size" override applied, as Go's
.header() // hdr.Size; the fixed header field alone would truncate files that
.size() // Go's archive/tar writes for sizes past 8 GiB.
.map_err(|e| Error::new(format!("reading archive: {}", crate::util::io_reason(&e))))?; let size = entry.size();
if is_gnu_sparse(&mut entry) {
eprintln!("skipping {shown}: sparse files are not supported");
continue;
}
let mode = entry.header().mode().unwrap_or(0) & 0o777; let mode = entry.header().mode().unwrap_or(0) & 0o777;
let mtime = mtime(&mut entry)?; let mtime = mtime(&mut entry)?;
match store.add_reader(cat, &mut entry, size, mode, mtime, &name) { match store.add_reader(cat, &mut entry, size, mode, mtime, &name) {
@@ -84,6 +88,18 @@ pub fn safe_name(name: &str) -> Option<String> {
Some(n) Some(n)
} }
/// A PAX-format GNU sparse file, whose data is an encoded map, not the
/// file's bytes.
fn is_gnu_sparse<R: Read>(entry: &mut tar::Entry<R>) -> bool {
if entry.header().entry_type() == tar::EntryType::GNUSparse {
return true;
}
match entry.pax_extensions() {
Ok(Some(exts)) => exts.flatten().any(|e| e.key_bytes().starts_with(b"GNU.sparse.")),
_ => false,
}
}
/// The entry's modification time: the PAX `mtime` (with its fraction) if /// The entry's modification time: the PAX `mtime` (with its fraction) if
/// present, else the header's whole seconds. /// present, else the header's whole seconds.
fn mtime<R: Read>(entry: &mut tar::Entry<R>) -> Result<Instant> { fn mtime<R: Read>(entry: &mut tar::Entry<R>) -> Result<Instant> {
@@ -236,6 +252,42 @@ mod tests {
assert_eq!(s.catalogue().unwrap().len(), before); assert_eq!(s.catalogue().unwrap().len(), before);
} }
/// A PAX "size" record overrides the header's size field, as Go writes
/// for files over 8 GiB; the whole body must be stored.
#[test]
fn pax_size_override() {
let (d, s) = store("absorb-pax");
let mut t = Vec::new();
let rec = |k: &str, v: &str| {
let body = format!(" {k}={v}\n");
let mut n = body.len() + 1;
while format!("{n}{body}").len() != n {
n += 1;
}
format!("{n}{body}")
};
let pax = rec("size", "11");
let mut h = header("PaxHeaders/big", tar::EntryType::XHeader, pax.len() as u64, 0o644, 0);
h.set_cksum();
t.extend_from_slice(h.as_bytes());
let mut p = pax.into_bytes();
p.resize(p.len().div_ceil(512) * 512, 0);
t.extend_from_slice(&p);
let h = header("big.bin", tar::EntryType::Regular, 0, 0o644, 0);
t.extend_from_slice(h.as_bytes());
let mut body = b"hello world".to_vec();
body.resize(512, 0);
t.extend_from_slice(&body);
t.extend_from_slice(&[0u8; 1024]);
let mut cat = s.catalogue().unwrap();
absorb(&s, &mut cat, &t[..], "", &mut |_, _| {}).unwrap();
let r = cat.current("big.bin").unwrap();
assert_eq!(r.size, 11);
let dir = crate::platform::fs::Dir::open(&d.0).unwrap();
s.extract(r, &dir, std::ffi::OsStr::new("o")).unwrap();
assert_eq!(std::fs::read(d.0.join("o")).unwrap(), b"hello world");
}
#[test] #[test]
fn names_and_times() { fn names_and_times() {
// As in Go, only names that would climb out are refused; others are // As in Go, only names that would climb out are refused; others are
+22 -3
View File
@@ -226,6 +226,19 @@ mod imp {
} }
} }
fn read_fd0(buf: &mut [u8]) -> std::io::Result<()> {
let mut done = 0;
while done < buf.len() {
match rustix::io::read(rustix::stdio::stdin(), &mut buf[done..]) {
Ok(0) => return Err(std::io::ErrorKind::UnexpectedEof.into()),
Ok(n) => done += n,
Err(rustix::io::Errno::INTR) => {}
Err(e) => return Err(e.into()),
}
}
Ok(())
}
/// The agent process. Returns when it expires or is told to quit. /// The agent process. Returns when it expires or is told to quit.
pub fn run(sock: &Path, ttl: &str, mode: &str) { pub fn run(sock: &Path, ttl: &str, mode: &str) {
let mut out = std::io::stdout(); let mut out = std::io::stdout();
@@ -249,7 +262,9 @@ mod imp {
Ok(b) => b, Ok(b) => b,
Err(e) => return report(&mut out, &e.to_string()), Err(e) => return report(&mut out, &e.to_string()),
}; };
if std::io::stdin().lock().read_exact(secret.as_mut_slice()).is_err() { // Read straight from fd 0 into the locked page: std's buffered
// stdin would leave a copy in ordinary heap memory.
if read_fd0(secret.as_mut_slice()).is_err() {
return report(&mut out, "no key received"); return report(&mut out, "no key received");
} }
{ {
@@ -258,8 +273,12 @@ mod imp {
return report(&mut out, &e); return report(&mut out, &e);
} }
} }
// A socket left by an agent that died stops bind; start has already // A socket left by an agent that died stops bind. Remove it only if
// stopped any live one. // nothing answers on it, so a concurrent unlock's live agent is not
// cut off from `vault lock`.
if UnixStream::connect(sock).is_ok() {
return report(&mut out, "another agent is already running for this key");
}
let _ = std::fs::remove_file(sock); let _ = std::fs::remove_file(sock);
let l = match UnixListener::bind(sock) { let l = match UnixListener::bind(sock) {
Ok(l) => l, Ok(l) => l,
+8 -2
View File
@@ -477,8 +477,14 @@ fn run(cmd: &str, args: &[String]) -> Result<i32> {
} }
let (s, _lock) = ctx.open(true)?; let (s, _lock) = ctx.open(true)?;
let mut cat = s.catalogue()?; let mut cat = s.catalogue()?;
// Never ingest, or delete, the vault or its key. // Never ingest, or delete, the vault, anything inside it, or its key.
let skip: Vec<FileId> = [ctx.vault_dir()?, ctx.key()?].iter().filter_map(|p| FileId::of(p, true)).collect(); let vault_dir = ctx.vault_dir()?;
if let (Ok(v), Ok(src)) = (std::fs::canonicalize(&vault_dir), std::fs::canonicalize(&pos[0])) {
if src != v && src.starts_with(&v) {
crate::bail!("{} is inside the vault", pos[0]);
}
}
let skip: Vec<FileId> = [vault_dir, ctx.key()?].iter().filter_map(|p| FileId::of(p, true)).collect();
let yes = ctx.flags.yes; let yes = ctx.flags.yes;
add_path(&s, &mut cat, Path::new(&pos[0]), &dest, migrate, &skip, &mut |names| { add_path(&s, &mut cat, Path::new(&pos[0]), &dest, migrate, &skip, &mut |names| {
yes || confirm_overwrite(names) yes || confirm_overwrite(names)
+64 -7
View File
@@ -122,6 +122,8 @@ pub struct StreamWriter<W: Write> {
dst: Option<W>, dst: Option<W>,
buf: Vec<u8>, buf: Vec<u8>,
index: u64, index: u64,
/// Set by any failure: like Go's sticky error, every later call fails.
failed: bool,
} }
impl<W: Write> StreamWriter<W> { impl<W: Write> StreamWriter<W> {
@@ -131,6 +133,7 @@ impl<W: Write> StreamWriter<W> {
dst: Some(dst), dst: Some(dst),
buf: Vec::with_capacity(ENC_CHUNK_SIZE), buf: Vec::with_capacity(ENC_CHUNK_SIZE),
index: 0, index: 0,
failed: false,
} }
} }
@@ -141,7 +144,24 @@ impl<W: Write> StreamWriter<W> {
w w
} }
fn check(&self) -> io::Result<()> {
if self.failed {
Err(io::Error::other("encrypted stream writer already failed"))
} else {
Ok(())
}
}
fn flush_chunk(&mut self, last: bool) -> io::Result<()> { fn flush_chunk(&mut self, last: bool) -> io::Result<()> {
let r = self.seal_and_write(last);
if r.is_err() {
self.failed = true;
self.buf.zeroize();
}
r
}
fn seal_and_write(&mut self, last: bool) -> io::Result<()> {
let n = self.buf.len(); let n = self.buf.len();
self.buf.resize(n + TAG_SIZE, 0); self.buf.resize(n + TAG_SIZE, 0);
self.cipher.seal(self.index, last, &mut self.buf); self.cipher.seal(self.index, last, &mut self.buf);
@@ -152,6 +172,7 @@ impl<W: Write> StreamWriter<W> {
} }
pub fn finish(mut self) -> io::Result<W> { pub fn finish(mut self) -> io::Result<W> {
self.check()?;
self.flush_chunk(true)?; self.flush_chunk(true)?;
Ok(self.dst.take().expect("writer in use")) Ok(self.dst.take().expect("writer in use"))
} }
@@ -159,6 +180,7 @@ impl<W: Write> StreamWriter<W> {
impl<W: Write> Write for StreamWriter<W> { impl<W: Write> Write for StreamWriter<W> {
fn write(&mut self, mut p: &[u8]) -> io::Result<usize> { fn write(&mut self, mut p: &[u8]) -> io::Result<usize> {
self.check()?;
let total = p.len(); let total = p.len();
while !p.is_empty() { while !p.is_empty() {
let take = p.len().min(CHUNK_SIZE - self.buf.len()); let take = p.len().min(CHUNK_SIZE - self.buf.len());
@@ -174,6 +196,7 @@ impl<W: Write> Write for StreamWriter<W> {
} }
fn flush(&mut self) -> io::Result<()> { fn flush(&mut self) -> io::Result<()> {
self.check()?;
self.dst.as_mut().expect("writer in use").flush() self.dst.as_mut().expect("writer in use").flush()
} }
} }
@@ -287,8 +310,11 @@ impl<R: Read> StreamReader<R> {
} }
if raw.len() < ENC_CHUNK_SIZE { if raw.len() < ENC_CHUNK_SIZE {
// A short first chunk is final; empty is the empty file. // A short first chunk is final; empty is the empty file.
let p = self.open(raw, true)?; let mut p = self.open(raw, true)?;
expect_eof(&mut self.src)?; if let Err(e) = expect_eof(&mut self.src) {
p.zeroize();
return Err(e);
}
self.deliver(p); self.deliver(p);
self.done = true; self.done = true;
return Ok(()); return Ok(());
@@ -307,9 +333,19 @@ impl<R: Read> StreamReader<R> {
if next.len() == TAG_SIZE { if next.len() == TAG_SIZE {
return Err(StreamError::EmptyLastChunk.into()); return Err(StreamError::EmptyLastChunk.into());
} }
let p = self.open(pending, false)?; let mut p = self.open(pending, false)?;
let q = self.open(next, true)?; let mut q = match self.open(next, true) {
expect_eof(&mut self.src)?; Ok(q) => q,
Err(e) => {
p.zeroize();
return Err(e);
}
};
if let Err(e) = expect_eof(&mut self.src) {
p.zeroize();
q.zeroize();
return Err(e);
}
self.deliver(p); self.deliver(p);
self.queued = Some(q); self.queued = Some(q);
self.done = true; self.done = true;
@@ -514,7 +550,8 @@ impl<'a, S: ReadAt + ?Sized> StreamReaderAt<'a, S> {
)); ));
} }
let mut n = 0; let mut n = 0;
let mut chunk = vec![0u8; ENC_CHUNK_SIZE]; // Cleared on every exit, early returns included.
let mut chunk = zeroize::Zeroizing::new(vec![0u8; ENC_CHUNK_SIZE]);
let mut cache = self.cache.lock().unwrap_or_else(|e| e.into_inner()); let mut cache = self.cache.lock().unwrap_or_else(|e| e.into_inner());
while n < p.len() && off < self.size { while n < p.len() && off < self.size {
let idx = off / CHUNK_SIZE as u64; let idx = off / CHUNK_SIZE as u64;
@@ -538,7 +575,7 @@ impl<'a, S: ReadAt + ?Sized> StreamReaderAt<'a, S> {
n += take; n += take;
off += take as u64; off += take as u64;
} }
chunk.zeroize();
Ok(n) Ok(n)
} }
} }
@@ -750,6 +787,26 @@ pub(crate) mod tests {
assert!(encrypted_chunk_count(ENC_CHUNK_SIZE as u64 + 16).is_err()); assert!(encrypted_chunk_count(ENC_CHUNK_SIZE as u64 + 16).is_err());
} }
/// After a failed write the writer refuses everything, as Go's does,
/// instead of growing past a chunk.
#[test]
fn writer_failure_is_sticky() {
struct Full;
impl Write for Full {
fn write(&mut self, _: &[u8]) -> io::Result<usize> {
Err(io::Error::other("disk full"))
}
fn flush(&mut self) -> io::Result<()> {
Ok(())
}
}
let mut w = StreamWriter::new(&[1u8; 32], Full);
assert!(w.write_all(&pt(CHUNK_SIZE + 1)).is_err());
assert!(w.write_all(&pt(10)).is_err());
assert!(w.flush().is_err());
assert!(w.finish().is_err());
}
#[test] #[test]
fn counter_overflow_is_refused() { fn counter_overflow_is_refused() {
let mut w = StreamWriter::starting_at(&[1u8; 32], Vec::new(), u64::MAX); let mut w = StreamWriter::starting_at(&[1u8; 32], Vec::new(), u64::MAX);
+111 -37
View File
@@ -124,49 +124,102 @@ pub fn encode_pem(body: &[u8]) -> String {
} }
/// The block body if `data` is one `VAULT KEY` PEM block, optionally with /// The block body if `data` is one `VAULT KEY` PEM block, optionally with
/// text before it (as Go's `pem.Decode` allows) and only whitespace after. /// text before it and only whitespace after: a port of Go's `pem.Decode`
/// followed by vault's checks on its result.
pub fn decode_pem(data: &[u8]) -> Option<Vec<u8>> { pub fn decode_pem(data: &[u8]) -> Option<Vec<u8>> {
let begin = format!("-----BEGIN {PEM_TYPE}-----"); let (typ, body, rest) = go_pem_decode(data)?;
let end = format!("-----END {PEM_TYPE}-----"); if typ != PEM_TYPE.as_bytes() || !go_trim_space(rest).is_empty() {
let start = if data.starts_with(b"-----BEGIN ") {
0
} else {
find(data, b"\n-----BEGIN ")? + 1
};
let mut lines = data[start..].split_inclusive(|&c| c == b'\n');
let first = lines.next()?;
if trim_right(first) != begin.as_bytes() {
return None; return None;
} }
let mut b64 = Vec::new(); Some(body)
let mut after = None;
let mut consumed = start + first.len();
for line in lines.by_ref() {
consumed += line.len();
let t = trim_right(line);
if t == end.as_bytes() {
after = Some(consumed);
break;
}
if t.contains(&b':') {
// A PEM header line; Go accepts and ignores them.
continue;
}
b64.extend(t.iter().filter(|c| !matches!(c, b' ' | b'\t')));
}
let after = after?;
if !data[after..].iter().all(|c| c.is_ascii_whitespace()) {
return None;
}
base64::engine::general_purpose::STANDARD.decode(&b64).ok()
} }
fn trim_right(l: &[u8]) -> &[u8] { /// Go's `getLine`: up to the next LF (dropping one CR before it), with
let mut e = l.len(); /// trailing spaces and tabs removed, and the rest after the LF.
while e > 0 && matches!(l[e - 1], b' ' | b'\t' | b'\r' | b'\n') { fn get_line(data: &[u8]) -> (&[u8], &[u8]) {
e -= 1; let (mut i, j) = match data.iter().position(|&c| c == b'\n') {
Some(i) => (i, i + 1),
None => (data.len(), data.len()),
};
if j > i && i > 0 && data[i - 1] == b'\r' {
i -= 1;
}
let mut line = &data[..i];
while let [rest @ .., b' ' | b'\t'] = line {
line = rest;
}
(line, &data[j..])
}
/// Go's `bytes.TrimSpace` (Unicode white space in UTF-8 text).
fn go_trim_space(b: &[u8]) -> &[u8] {
match std::str::from_utf8(b) {
Ok(s) => s.trim_matches(char::is_whitespace).as_bytes(),
Err(_) => b.trim_ascii(),
}
}
/// Go's `encoding/pem.Decode`: the first well-formed block's type, bytes
/// and the data after it. Malformed candidates are skipped, as in Go.
fn go_pem_decode(data: &[u8]) -> Option<(Vec<u8>, Vec<u8>, &[u8])> {
const START: &[u8] = b"\n-----BEGIN ";
const END: &[u8] = b"\n-----END ";
const EOL: &[u8] = b"-----";
let mut rest = data;
loop {
if rest.starts_with(&START[1..]) {
rest = &rest[START.len() - 1..];
} else if let Some(i) = find(rest, START) {
rest = &rest[i + START.len()..];
} else {
return None;
}
let (type_line, r) = get_line(rest);
rest = r;
let Some(typ) = type_line.strip_suffix(EOL) else { continue };
let mut headers = 0;
loop {
if rest.is_empty() {
return None;
}
let (line, next) = get_line(rest);
if !line.contains(&b':') {
break;
}
headers += 1;
rest = next;
}
let (end_index, trailer_index) = if headers == 0 && rest.starts_with(&END[1..]) {
(0, END.len() - 1)
} else {
match find(rest, END) {
Some(i) => (i, i + END.len()),
None => continue,
}
};
let trailer = &rest[trailer_index..];
let tlen = typ.len() + EOL.len();
if trailer.len() < tlen {
continue;
}
let (t, rest_of_end) = trailer.split_at(tlen);
if !t.starts_with(typ) || !t.ends_with(EOL) || !get_line(rest_of_end).0.is_empty() {
continue;
}
// Go's StdEncoding skips CR and LF; removeSpacesAndTabs drops the rest.
let b64: Vec<u8> = rest[..end_index]
.iter()
.copied()
.filter(|c| !matches!(c, b' ' | b'\t' | b'\r' | b'\n'))
.collect();
let lenient = base64::engine::GeneralPurpose::new(
&base64::alphabet::STANDARD,
base64::engine::GeneralPurposeConfig::new().with_decode_allow_trailing_bits(true),
);
let Ok(bytes) = lenient.decode(&b64) else { continue };
let after = get_line(&rest[end_index + END.len() - 1..]).1;
return Some((typ.to_vec(), bytes, after));
} }
&l[..e]
} }
fn find(h: &[u8], n: &[u8]) -> Option<usize> { fn find(h: &[u8], n: &[u8]) -> Option<usize> {
@@ -327,6 +380,27 @@ mod tests {
assert!(decode_pem(format!("{pem}x").as_bytes()).is_none()); assert!(decode_pem(format!("{pem}x").as_bytes()).is_none());
assert!(decode_pem(pem.replace("VAULT KEY", "VAULT KEX").as_bytes()).is_none()); assert!(decode_pem(pem.replace("VAULT KEY", "VAULT KEX").as_bytes()).is_none());
assert!(decode_pem(pem.replace("-----END", "-----FIN").as_bytes()).is_none()); assert!(decode_pem(pem.replace("-----END", "-----FIN").as_bytes()).is_none());
// Cases where an earlier, simpler parser differed from Go's pem.Decode.
// (In the second body line: a colon in the first makes it a header.)
let first = pem.find('\n').unwrap();
let mid = first + 1 + pem[first + 1..].find('\n').unwrap() + 30;
assert!(
decode_pem(format!("{}:{}", &pem[..mid], &pem[mid..]).as_bytes()).is_none(),
"colon inside the body"
);
assert!(
decode_pem(format!("{pem}\u{b}\u{a0}").as_bytes()).is_some(),
"Unicode space after the block"
);
assert!(decode_pem(pem.replacen("\n", "\r\n", 3).as_bytes()).is_some(), "CRLF lines");
assert!(
decode_pem(format!("-----BEGIN X\n{pem}").as_bytes()).is_some(),
"a bad BEGIN line before the block"
);
assert!(
decode_pem(format!("-----BEGIN VAULT KEY-----\nProc-Type: 4,x\n\n{}", &pem[26..]).as_bytes()).is_some(),
"headers"
);
let mut short = body.clone(); let mut short = body.clone();
short.pop(); short.pop();
assert_eq!(parse_body(&short).err(), Some(Error::NotAKeyFile)); assert_eq!(parse_body(&short).err(), Some(Error::NotAKeyFile));
+46 -7
View File
@@ -165,10 +165,33 @@ fn publish_inner(tmp: &Path, dst: &Path) -> io::Result<()> {
/// atomic no-clobber publish wherever hard links work. /// atomic no-clobber publish wherever hard links work.
#[cfg(not(windows))] #[cfg(not(windows))]
fn link_publish(tmp: &Path, dst: &Path) -> io::Result<()> { fn link_publish(tmp: &Path, dst: &Path) -> io::Result<()> {
std::fs::hard_link(tmp, dst)?; std::fs::hard_link(tmp, dst).map_err(no_atomic)?;
// The data is safely at dst; a failure to drop the temporary name is // The data is safely at dst; failing to drop the temporary name is only
// reported but dst stays. // worth a warning.
std::fs::remove_file(tmp) if let Err(e) = std::fs::remove_file(tmp) {
eprintln!(
"vault: could not remove temporary file {}: {}",
tmp.display(),
crate::util::io_reason(&e)
);
}
Ok(())
}
/// A filesystem with neither RENAME_NOREPLACE nor hard links cannot publish
/// without risking a replace; say so rather than fall back.
#[cfg(not(windows))]
fn no_atomic(e: io::Error) -> io::Error {
if e.kind() == io::ErrorKind::AlreadyExists {
return e;
}
io::Error::new(
e.kind(),
format!(
"this filesystem offers no atomic no-replace rename or hard link ({})",
crate::util::io_reason(&e)
),
)
} }
#[cfg(windows)] #[cfg(windows)]
@@ -325,9 +348,11 @@ impl Dir {
match linkat(&self.fd, tmp, &self.fd, name, AtFlags::empty()) { match linkat(&self.fd, tmp, &self.fd, name, AtFlags::empty()) {
Ok(()) => {} Ok(()) => {}
Err(rustix::io::Errno::EXIST) => return Err(exists_error(&dst)), Err(rustix::io::Errno::EXIST) => return Err(exists_error(&dst)),
Err(e) => return Err(e.into()), Err(e) => return Err(no_atomic(e.into())),
}
if let Err(e) = unlinkat(&self.fd, tmp, AtFlags::empty()) {
eprintln!("vault: could not remove temporary file {}: {e}", self.path.join(tmp).display());
} }
unlinkat(&self.fd, tmp, AtFlags::empty())?;
Ok(()) Ok(())
} }
#[cfg(not(unix))] #[cfg(not(unix))]
@@ -380,13 +405,27 @@ fn is_reparse(_: &std::fs::Metadata) -> bool {
false false
} }
/// On Windows, also refuse names NTFS would treat specially: alternate
/// data streams (`a:b`), reserved device names, trailing dots or spaces.
fn windows_safe(name: &OsStr) -> bool {
if !cfg!(windows) {
return true;
}
let Some(n) = name.to_str() else { return false };
let stem = n.split('.').next().unwrap_or("").trim_end().to_ascii_uppercase();
let reserved = matches!(stem.as_str(), "CON" | "PRN" | "AUX" | "NUL" | "CONIN$" | "CONOUT$")
|| ((stem.starts_with("COM") || stem.starts_with("LPT")) && stem.len() == 4 && stem.as_bytes()[3].is_ascii_digit());
!n.contains([':', '<', '>', '"', '|', '?', '*']) && !n.ends_with(['.', ' ']) && !reserved
}
/// A single, plain path component: no separators, not "." or "..". /// A single, plain path component: no separators, not "." or "..".
fn check_component(name: &OsStr) -> io::Result<()> { fn check_component(name: &OsStr) -> io::Result<()> {
let ok = !name.is_empty() let ok = !name.is_empty()
&& name != "." && name != "."
&& name != ".." && name != ".."
&& Path::new(name).components().count() == 1 && Path::new(name).components().count() == 1
&& matches!(Path::new(name).components().next(), Some(std::path::Component::Normal(_))); && matches!(Path::new(name).components().next(), Some(std::path::Component::Normal(_)))
&& windows_safe(name);
if ok { if ok {
Ok(()) Ok(())
} else { } else {
+11
View File
@@ -458,6 +458,17 @@ fn duplicate_records_recover() {
#[cfg(target_os = "linux")] #[cfg(target_os = "linux")]
#[test] #[test]
fn repeated_failures_do_not_leak() { fn repeated_failures_do_not_leak() {
// Thread counts are per process, and other tests run in parallel: run
// the real check alone in a child copy of this test binary.
if std::env::var_os("VAULT_LEAK_CHILD").is_none() {
let o = std::process::Command::new(std::env::current_exe().unwrap())
.args(["--exact", "store::tests::repeated_failures_do_not_leak", "--test-threads=1"])
.env("VAULT_LEAK_CHILD", "1")
.output()
.unwrap();
assert!(o.status.success(), "{}", String::from_utf8_lossy(&o.stdout));
return;
}
let threads = || std::fs::read_dir("/proc/self/task").unwrap().count(); let threads = || std::fs::read_dir("/proc/self/task").unwrap().count();
let (d, s) = vault("leak"); let (d, s) = vault("leak");
let mut cat = s.catalogue().unwrap(); let mut cat = s.catalogue().unwrap();
+9
View File
@@ -256,6 +256,15 @@ fn migrate_deletes_only_stored_originals() {
// Never the vault or the key. // Never the vault or the key.
let o = e.run(RUST, Some(REAL), &["add", "store"]); let o = e.run(RUST, Some(REAL), &["add", "store"]);
assert!(String::from_utf8_lossy(&o.stderr).contains("is the vault or its key")); assert!(String::from_utf8_lossy(&o.stderr).contains("is the vault or its key"));
// Nor anything inside the vault: migrating its records would delete them.
let before = e.names(RUST, REAL);
let o = e.run(RUST, Some(REAL), &["migrate", "--yes", "store/records"]);
assert!(
String::from_utf8_lossy(&o.stderr).contains("is inside the vault"),
"{}",
String::from_utf8_lossy(&o.stderr)
);
assert_eq!(e.names(RUST, REAL), before);
} }
#[cfg(unix)] #[cfg(unix)]