swarmctl: write users.yml through an O_EXCL|O_NOFOLLOW temp file, mode/owner on the fd
This commit is contained in:
parent
49015eb553
commit
99879f850a
1 changed files with 157 additions and 17 deletions
|
|
@ -35,8 +35,8 @@ mod schedule;
|
|||
mod users;
|
||||
|
||||
use std::fs::{self, File, Permissions};
|
||||
use std::io::Write as _;
|
||||
use std::os::unix::fs::{MetadataExt as _, PermissionsExt as _};
|
||||
use std::io::{self, Write as _};
|
||||
use std::os::unix::fs::{MetadataExt as _, OpenOptionsExt as _, PermissionsExt as _};
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::process::Command;
|
||||
|
||||
|
|
@ -1003,7 +1003,6 @@ fn write_atomic(path: &Path, contents: &str) -> Result<()> {
|
|||
let name = path
|
||||
.file_name()
|
||||
.with_context(|| format!("{} has no file name", path.display()))?;
|
||||
let tmp = dir.join(format!(".{}.swarmctl.tmp", name.to_string_lossy()));
|
||||
|
||||
let existing = fs::metadata(path).ok();
|
||||
// 0600 only when the file does not exist yet: this content is
|
||||
|
|
@ -1013,21 +1012,25 @@ fn write_atomic(path: &Path, contents: &str) -> Result<()> {
|
|||
.as_ref()
|
||||
.map_or(0o600, |meta| meta.permissions().mode() & 0o7777);
|
||||
|
||||
let mut file = File::create(&tmp).with_context(|| format!("creating {}", tmp.display()))?;
|
||||
file.write_all(contents.as_bytes())
|
||||
.with_context(|| format!("writing {}", tmp.display()))?;
|
||||
file.sync_all()
|
||||
.with_context(|| format!("flushing {}", tmp.display()))?;
|
||||
drop(file);
|
||||
|
||||
fs::set_permissions(&tmp, Permissions::from_mode(mode))
|
||||
.with_context(|| format!("setting mode on {}", tmp.display()))?;
|
||||
if let Some(meta) = existing.as_ref() {
|
||||
std::os::unix::fs::chown(&tmp, Some(meta.uid()), Some(meta.gid()))
|
||||
.with_context(|| format!("setting owner on {}", tmp.display()))?;
|
||||
let (mut file, tmp) = create_temp(dir, &name.to_string_lossy())?;
|
||||
let written = (|| -> Result<()> {
|
||||
file.write_all(contents.as_bytes())
|
||||
.with_context(|| format!("writing {}", tmp.display()))?;
|
||||
// Owner before mode: chown(2) clears setuid/setgid bits.
|
||||
if let Some(meta) = existing.as_ref() {
|
||||
std::os::unix::fs::fchown(&file, Some(meta.uid()), Some(meta.gid()))
|
||||
.with_context(|| format!("setting owner on {}", tmp.display()))?;
|
||||
}
|
||||
file.set_permissions(Permissions::from_mode(mode))
|
||||
.with_context(|| format!("setting mode on {}", tmp.display()))?;
|
||||
file.sync_all()
|
||||
.with_context(|| format!("flushing {}", tmp.display()))?;
|
||||
fs::rename(&tmp, path).with_context(|| format!("renaming {} into place", tmp.display()))
|
||||
})();
|
||||
if written.is_err() {
|
||||
let _ = fs::remove_file(&tmp);
|
||||
}
|
||||
|
||||
fs::rename(&tmp, path).with_context(|| format!("renaming {} into place", tmp.display()))?;
|
||||
written?;
|
||||
|
||||
// The rename itself is metadata: without this the file can survive a
|
||||
// crash while the directory entry pointing at it does not.
|
||||
|
|
@ -1037,6 +1040,41 @@ fn write_atomic(path: &Path, contents: &str) -> Result<()> {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
/// Creates `.<name>.<random>.swarmctl.tmp` in `dir`, mode 0600.
|
||||
///
|
||||
/// `dir` can be writable by a less-privileged uid (authelia's state
|
||||
/// directory), so the name must be unguessable and the open must not
|
||||
/// follow a link planted there: see [`open_new_nofollow`].
|
||||
fn create_temp(dir: &Path, name: &str) -> Result<(File, PathBuf)> {
|
||||
for _ in 0..8 {
|
||||
let mut raw = [0u8; 8];
|
||||
getrandom::fill(&mut raw)
|
||||
.map_err(|e| anyhow::anyhow!("drawing randomness from the kernel: {e}"))?;
|
||||
let suffix = format!("{:016x}", u64::from_le_bytes(raw));
|
||||
let tmp = dir.join(format!(".{name}.{suffix}.swarmctl.tmp"));
|
||||
match open_new_nofollow(&tmp) {
|
||||
Ok(file) => return Ok((file, tmp)),
|
||||
Err(e) if e.kind() == io::ErrorKind::AlreadyExists => {}
|
||||
Err(e) => return Err(e).with_context(|| format!("creating {}", tmp.display())),
|
||||
}
|
||||
}
|
||||
bail!("no unused temp name for {name} in {}", dir.display())
|
||||
}
|
||||
|
||||
/// Opens `tmp` for writing, mode 0600, failing if anything exists there.
|
||||
///
|
||||
/// `O_EXCL` fails the open on any existing entry, a symlink included, and
|
||||
/// `O_NOFOLLOW` fails it on a symlink in the final component regardless;
|
||||
/// neither truncates or touches the link's target.
|
||||
fn open_new_nofollow(tmp: &Path) -> io::Result<File> {
|
||||
fs::OpenOptions::new()
|
||||
.write(true)
|
||||
.create_new(true)
|
||||
.mode(0o600)
|
||||
.custom_flags(rustix::fs::OFlags::NOFOLLOW.bits().cast_signed())
|
||||
.open(tmp)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
|
@ -1406,6 +1444,108 @@ mod tests {
|
|||
fs::remove_dir_all(&dir).ok();
|
||||
}
|
||||
|
||||
fn tmp_entries(dir: &Path) -> Vec<String> {
|
||||
let mut names: Vec<String> = fs::read_dir(dir)
|
||||
.expect("read dir")
|
||||
.map(|e| e.expect("entry").file_name().to_string_lossy().into_owned())
|
||||
.filter(|n| n.ends_with(".swarmctl.tmp"))
|
||||
.collect();
|
||||
names.sort();
|
||||
names
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_atomic_ignores_a_symlink_planted_at_the_old_fixed_temp_name() {
|
||||
let dir = std::env::temp_dir().join(format!("swarmctl-plant-{}", std::process::id()));
|
||||
let _ = fs::remove_dir_all(&dir);
|
||||
fs::create_dir_all(&dir).expect("temp dir");
|
||||
let path = dir.join("users.yml");
|
||||
fs::write(&path, "users: {}\n").expect("seed");
|
||||
fs::set_permissions(&path, Permissions::from_mode(0o640)).expect("chmod");
|
||||
|
||||
let sentinel = dir.join("sentinel");
|
||||
fs::write(&sentinel, "keep me\n").expect("sentinel");
|
||||
fs::set_permissions(&sentinel, Permissions::from_mode(0o644)).expect("chmod");
|
||||
let planted = dir.join(".users.yml.swarmctl.tmp");
|
||||
std::os::unix::fs::symlink(&sentinel, &planted).expect("plant");
|
||||
|
||||
write_atomic(&path, "users:\n mara:\n").expect("rewrite");
|
||||
|
||||
assert_eq!(fs::read_to_string(&sentinel).expect("read"), "keep me\n");
|
||||
let mode = fs::metadata(&sentinel).expect("stat").permissions().mode() & 0o7777;
|
||||
assert_eq!(mode, 0o644, "the link's target must keep its mode");
|
||||
assert!(
|
||||
fs::symlink_metadata(&planted)
|
||||
.expect("stat link")
|
||||
.is_symlink(),
|
||||
"the planted link is not ours to remove or rename"
|
||||
);
|
||||
assert_eq!(
|
||||
fs::read_to_string(&path).expect("read"),
|
||||
"users:\n mara:\n"
|
||||
);
|
||||
let mode = fs::metadata(&path).expect("stat").permissions().mode() & 0o7777;
|
||||
assert_eq!(mode, 0o640);
|
||||
|
||||
fs::remove_dir_all(&dir).ok();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_temp_open_refuses_a_planted_link_and_leaves_its_target_alone() {
|
||||
let dir = std::env::temp_dir().join(format!("swarmctl-excl-{}", std::process::id()));
|
||||
let _ = fs::remove_dir_all(&dir);
|
||||
fs::create_dir_all(&dir).expect("temp dir");
|
||||
|
||||
let sentinel = dir.join("sentinel");
|
||||
fs::write(&sentinel, "keep me\n").expect("sentinel");
|
||||
let to_file = dir.join("to-file");
|
||||
std::os::unix::fs::symlink(&sentinel, &to_file).expect("plant");
|
||||
let dangling = dir.join("dangling");
|
||||
std::os::unix::fs::symlink(dir.join("absent"), &dangling).expect("plant");
|
||||
|
||||
for planted in [&to_file, &dangling] {
|
||||
let err = open_new_nofollow(planted).expect_err("must refuse a link");
|
||||
assert_eq!(err.kind(), io::ErrorKind::AlreadyExists);
|
||||
}
|
||||
assert_eq!(fs::read_to_string(&sentinel).expect("read"), "keep me\n");
|
||||
assert!(
|
||||
!dir.join("absent").exists(),
|
||||
"a dangling link is not created through"
|
||||
);
|
||||
|
||||
// Control: the same call on a free name succeeds, so the refusals
|
||||
// above are the link's doing and not a broken open.
|
||||
let mut fresh = open_new_nofollow(&dir.join("fresh")).expect("free name opens");
|
||||
fresh.write_all(b"ok").expect("write");
|
||||
let mode = fs::metadata(dir.join("fresh"))
|
||||
.expect("stat")
|
||||
.permissions()
|
||||
.mode()
|
||||
& 0o7777;
|
||||
assert_eq!(mode, 0o600);
|
||||
|
||||
fs::remove_dir_all(&dir).ok();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_atomic_removes_its_temp_file_when_the_rename_fails() {
|
||||
let dir = std::env::temp_dir().join(format!("swarmctl-cleanup-{}", std::process::id()));
|
||||
let _ = fs::remove_dir_all(&dir);
|
||||
fs::create_dir_all(&dir).expect("temp dir");
|
||||
// A non-empty directory at the target makes rename(2) fail.
|
||||
let path = dir.join("users.yml");
|
||||
fs::create_dir_all(path.join("inner")).expect("blocker");
|
||||
|
||||
write_atomic(&path, "x\n").expect_err("rename over a directory fails");
|
||||
assert_eq!(tmp_entries(&dir), Vec::<String>::new());
|
||||
|
||||
// Control: the helper does see a stray temp file.
|
||||
fs::write(dir.join(".stray.swarmctl.tmp"), "").expect("stray");
|
||||
assert_eq!(tmp_entries(&dir), [".stray.swarmctl.tmp"]);
|
||||
|
||||
fs::remove_dir_all(&dir).ok();
|
||||
}
|
||||
|
||||
/// Replaces `load_store_refuses_to_take_over_a_populated_users_file`,
|
||||
/// which pinned the guard this change removes.
|
||||
///
|
||||
|
|
|
|||
Loading…
Reference in a new issue