fs-mistrust: add various methods.

This includes:

  * a CachedDir::join method.
  * functions to read and write from provided filenames in a
    CachedDir.
  * a method to tell whether a fs-mistrust error is about bad file
    permissions, or failure to inspect file permissions or some other
    kind of IO problem.
This commit is contained in:
Nick Mathewson 2022-05-03 12:56:55 -04:00
parent 62d159e3c4
commit 14e8243bdc
2 changed files with 174 additions and 7 deletions

View File

@ -3,6 +3,7 @@
use std::{
fs::{File, OpenOptions},
io::{Read, Write},
path::{Path, PathBuf},
};
@ -103,9 +104,7 @@ impl CheckedDir {
options.custom_flags(libc::O_NOFOLLOW);
}
let file = options
.open(&path)
.map_err(|e| Error::inspecting(e, &path))?;
let file = options.open(&path).map_err(|e| Error::io(e, &path))?;
let meta = file.metadata().map_err(|e| Error::inspecting(e, &path))?;
if let Some(error) = self
@ -129,6 +128,90 @@ impl CheckedDir {
self.location.as_path()
}
/// Return a new [`PathBuf`] containing this directory's path, with `path`
/// appended to it.
///
/// Return an error if `path` has any components that could take us outside
/// of this directory.
pub fn join<P: AsRef<Path>>(&self, path: P) -> Result<PathBuf> {
let path = path.as_ref();
self.check_path(path)?;
Ok(self.location.join(path))
}
/// Read the contents of the file at `path` within this directory, as a
/// String, if possible.
///
/// Return an error if `path` is absent, if its permissions are incorrect,
/// if it has any components that could take us outside of this directory,
/// or if its contents are not UTF-8.
pub fn read_to_string<P: AsRef<Path>>(&self, path: P) -> Result<String> {
let path = path.as_ref();
let mut file = self.open(path, OpenOptions::new().read(true))?;
let mut result = String::new();
file.read_to_string(&mut result)
.map_err(|e| Error::io(e, path))?;
Ok(result)
}
/// Read the contents of the file at `path` within this directory, as a
/// vector of bytes, if possible.
///
/// Return an error if `path` is absent, if its permissions are incorrect,
/// or if it has any components that could take us outside of this
/// directory.
pub fn read<P: AsRef<Path>>(&self, path: P) -> Result<Vec<u8>> {
let path = path.as_ref();
let mut file = self.open(path, OpenOptions::new().read(true))?;
let mut result = Vec::new();
file.read_to_end(&mut result)
.map_err(|e| Error::io(e, path))?;
Ok(result)
}
/// Store `contents` into the file located at `path` within this directory.
///
/// We won't write to `path` directly: instead, we'll write to a temporary
/// file in the same directory as `path`, and then replace `path` with that
/// temporary file if we were successful. (This isn't truly atomic on all
/// file systems, but it's closer than many alternatives.)
///
/// # Limitations
///
/// This function will clobber any existing files with the same name as
/// `path` but with the extension `tmp`. (That is, if you are writing to
/// "foo.txt", it will replace "foo.tmp" in the same directory.)
///
/// This function may give incorrect behavior if multiple threads or
/// processes are writing to the same file at the same time: it is the
/// programmer's responsibility to use appropriate locking to avoid this.
pub fn write_and_replace<P: AsRef<Path>, C: AsRef<[u8]>>(
&self,
path: P,
contents: C,
) -> Result<()> {
let path = path.as_ref();
self.check_path(path)?;
let tmp_name = path.with_extension("tmp");
let mut tmp_file = self.open(
&tmp_name,
OpenOptions::new().create(true).truncate(true).write(true),
)?;
// Write the data.
tmp_file
.write_all(contents.as_ref())
.map_err(|e| Error::io(e, &tmp_name))?;
// Flush and close.
drop(tmp_file);
// Replace the old file.
std::fs::rename(self.location.join(tmp_name), self.location.join(path))
.map_err(|e| Error::io(e, path))?;
Ok(())
}
/// Helper: create a [`Verifier`] with the appropriate rules for this
/// `CheckedDir`.
fn verifier(&self) -> Verifier<'_> {
@ -143,7 +226,10 @@ impl CheckedDir {
/// guaranteed to stay within this directory.
fn check_path(&self, p: &Path) -> Result<()> {
use std::path::Component;
if p.is_absolute() {}
// This check should be redundant, but let's be certain.
if p.is_absolute() {
return Err(Error::InvalidSubdirectory);
}
for component in p.components() {
match component {
@ -240,4 +326,42 @@ mod test {
sd.make_directory("hello/world").unwrap();
}
#[test]
fn read_and_write() {
let d = Dir::new();
d.dir("a");
d.chmod("a", 0o700);
let mut m = Mistrust::new();
m.ignore_prefix(d.canonical_root()).unwrap();
let checked = m.verifier().secure_dir(d.path("a")).unwrap();
// Simple case: write and read.
checked
.write_and_replace("foo.txt", "this is incredibly silly")
.unwrap();
let s1 = checked.read_to_string("foo.txt").unwrap();
let s2 = checked.read("foo.txt").unwrap();
assert_eq!(s1, "this is incredibly silly");
assert_eq!(s1.as_bytes(), &s2[..]);
// Trickier: write when the preferred temporary already has content.
checked
.open("bar.tmp", OpenOptions::new().create(true).write(true))
.unwrap()
.write_all("be the other guy".as_bytes())
.unwrap();
assert!(checked.join("bar.tmp").unwrap().exists());
checked
.write_and_replace("bar.txt", "its hard and nobody understands")
.unwrap();
// Temp file should be gone.
assert!(!checked.join("bar.tmp").unwrap().exists());
let s4 = checked.read_to_string("bar.txt").unwrap();
assert_eq!(s4, "its hard and nobody understands");
}
}

View File

@ -83,15 +83,23 @@ pub enum Error {
#[error("Unable to list directory")]
Listing(#[source] Arc<walkdir::Error>),
/// We were unable to open a file with [`CheckedDir::open`](crate::CheckedDir::open)
/// Tried to use an invalid path with a [`CheckedDir`](crate::CheckedDir),
#[error("Path was not valid for use with CheckedDir.")]
InvalidSubdirectory,
/// We encountered an error while attempting an IO operation on a file.
#[error("IO error on {0}")]
Io(PathBuf, #[source] Arc<IoError>),
/// We could not create an unused temporary path when trying to write a
/// file.
#[error("Could not name temporary file for {0}")]
NoTempFile(PathBuf),
}
impl Error {
/// Create an error from an IoError object.
/// Create an error from an IoError encountered while inspecting permissions
/// on an object.
pub(crate) fn inspecting(err: IoError, fname: impl Into<PathBuf>) -> Self {
match err.kind() {
IoErrorKind::NotFound => Error::NotFound(fname.into()),
@ -99,6 +107,15 @@ impl Error {
}
}
/// Create an error from an IoError encountered while performing IO (open,
/// read, write) on an object.
pub(crate) fn io(err: IoError, fname: impl Into<PathBuf>) -> Self {
match err.kind() {
IoErrorKind::NotFound => Error::NotFound(fname.into()),
_ => Error::Io(fname.into(), Arc::new(err)),
}
}
/// Return the path, if any, associated with this error.
pub fn path(&self) -> Option<&Path> {
Some(
@ -108,6 +125,8 @@ impl Error {
Error::BadOwner(pb, _) => pb,
Error::BadType(pb) => pb,
Error::CouldNotInspect(pb, _) => pb,
Error::Io(pb, _) => pb,
Error::NoTempFile(pb) => pb,
Error::Multiple(_) => return None,
Error::StepsExceeded => return None,
Error::CurrentDirectory(_) => return None,
@ -120,6 +139,30 @@ impl Error {
)
}
/// Return true iff this error indicates a problem with filesystem
/// permissions.
///
/// (Other errors typically indicate an IO problem, possibly one preventing
/// us from looking at permissions in the first place)
pub fn is_bad_permission(&self) -> bool {
match self {
Error::BadPermission(_, _) | Error::BadOwner(_, _) | Error::BadType(_) => true,
Error::NotFound(_)
| Error::CouldNotInspect(_, _)
| Error::StepsExceeded
| Error::CurrentDirectory(_)
| Error::CreatingDir(_)
| Error::Listing(_)
| Error::InvalidSubdirectory
| Error::Io(_, _)
| Error::NoTempFile(_) => false,
Error::Multiple(errs) => errs.iter().any(|e| e.is_bad_permission()),
Error::Content(err) => err.is_bad_permission(),
}
}
/// Return an iterator over all of the errors contained in this Error.
///
/// If this is a singleton, the iterator returns only a single element.