diff --git a/crates/fs-mistrust/src/dir.rs b/crates/fs-mistrust/src/dir.rs index dd3c50d72..28f9e0efa 100644 --- a/crates/fs-mistrust/src/dir.rs +++ b/crates/fs-mistrust/src/dir.rs @@ -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>(&self, path: P) -> Result { + 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>(&self, path: P) -> Result { + 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>(&self, path: P) -> Result> { + 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, 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"); + } } diff --git a/crates/fs-mistrust/src/err.rs b/crates/fs-mistrust/src/err.rs index cf699d09c..79e3846f6 100644 --- a/crates/fs-mistrust/src/err.rs +++ b/crates/fs-mistrust/src/err.rs @@ -83,15 +83,23 @@ pub enum Error { #[error("Unable to list directory")] Listing(#[source] Arc), - /// 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), + + /// 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) -> 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) -> 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.