Commit 2087dde3 authored by Nick Mathewson's avatar Nick Mathewson 🦞
Browse files

fs-mistrust: Add a dangerously_trust_everyone method. 

This helps make it possible to use `SecureDir` (name pending) even
when we want to disable permissions checks.  Otherwise, optional
permission checking would require users of this crate to maintain
separate code paths for the "check" and "don't check" cases.
parent 0c01fb97

crates/fs-mistrust/src/imp.rs

+14 −1
Original line number Original line Diff line number Diff line
@@ -42,6 +42,19 @@ impl<'a> super::Verifier<'a> { 
    // to the code.  It's not urgent, since the allocations won't cost much

    // to the code.  It's not urgent, since the allocations won't cost much

    // compared to the filesystem access.

    // compared to the filesystem access.

    pub(crate) fn check_errors(&self, path: &Path) -> impl Iterator<Item = Error> + '_ {

    pub(crate) fn check_errors(&self, path: &Path) -> impl Iterator<Item = Error> + '_ {

        if self.mistrust.disable_ownership_and_permission_checks {

            // We don't want to walk the path in this case at all: we'll just

            // look at the last element.



            let meta = match path.metadata() {

                Ok(meta) => meta,

                Err(e) => return boxed(vec![Error::inspecting(e, path)].into_iter()),

            };

            let mut errors = Vec::new();

            self.check_type(path, PathType::Final, &meta, &mut errors);

            return boxed(errors.into_iter());

        }



        let rp = match ResolvePath::new(path) {

        let rp = match ResolvePath::new(path) {

            Ok(rp) => rp,

            Ok(rp) => rp,

            Err(e) => return boxed(vec![e].into_iter()),

            Err(e) => return boxed(vec![e].into_iter()),
@@ -73,7 +86,7 @@ impl<'a> super::Verifier<'a> { 
    pub(crate) fn check_content_errors(&self, path: &Path) -> impl Iterator<Item = Error> + '_ {

    pub(crate) fn check_content_errors(&self, path: &Path) -> impl Iterator<Item = Error> + '_ {

        use std::sync::Arc;

        use std::sync::Arc;





        if !self.check_contents {

        if !self.check_contents || self.mistrust.disable_ownership_and_permission_checks {

            return boxed(std::iter::empty());

            return boxed(std::iter::empty());

        }

        }

crates/fs-mistrust/src/lib.rs

+45 −0
Original line number Original line Diff line number Diff line
@@ -283,6 +283,9 @@ pub struct Mistrust { 
    /// (This is stored in canonical form.)

    /// (This is stored in canonical form.)

    ignore_prefix: Option<PathBuf>,

    ignore_prefix: Option<PathBuf>,





    /// Are we configured to enable all permission and ownership tests?

    disable_ownership_and_permission_checks: bool,



    /// What user ID do we trust by default (if any?)

    /// What user ID do we trust by default (if any?)

    #[cfg(target_family = "unix")]

    #[cfg(target_family = "unix")]

    trust_uid: Option<u32>,

    trust_uid: Option<u32>,
@@ -296,6 +299,7 @@ impl Default for Mistrust { 
    fn default() -> Self {

    fn default() -> Self {

        Self {

        Self {

            ignore_prefix: None,

            ignore_prefix: None,

            disable_ownership_and_permission_checks: false,

            #[cfg(target_family = "unix")]

            #[cfg(target_family = "unix")]

            trust_uid: Some(unsafe { libc::getuid() }),

            trust_uid: Some(unsafe { libc::getuid() }),

            #[cfg(target_family = "unix")]

            #[cfg(target_family = "unix")]
@@ -415,6 +419,21 @@ impl Mistrust { 
        self

        self

    }

    }





    /// Configure this `Mistrust` to trust every user and every group.

    ///

    /// With this option set, every file and directory is treated as having

    /// valid permissions: even world-writeable files are allowed.  File-type

    /// checks are still performed.

    ///

    /// This option is mainly useful to handle cases where you want to make

    /// these checks optional, and still use [`SecureDir`] without having to

    /// implement separate code paths for the "checking on" and "checking off"

    /// cases.

    pub fn dangerously_trust_everyone(&mut self) -> &mut Self {

        self.disable_ownership_and_permission_checks = true;

        self

    }



    /// Create a new [`Verifier`] with this configuration, to perform a single check.

    /// Create a new [`Verifier`] with this configuration, to perform a single check.

    pub fn verifier(&self) -> Verifier<'_> {

    pub fn verifier(&self) -> Verifier<'_> {

        Verifier {

        Verifier {
@@ -881,6 +900,32 @@ mod test { 
        assert_eq!(2, e.errors().count());

        assert_eq!(2, e.errors().count());

    }

    }





    #[test]

    fn trust_everyone() {

        let d = Dir::new();

        d.dir("a/b/c");

        d.file("a/b/c/d");

        d.chmod("a", 0o777);

        d.chmod("a/b", 0o777);

        d.chmod("a/b/c", 0o777);

        d.chmod("a/b/c/d", 0o666);



        let mut m = Mistrust::new();

        m.dangerously_trust_everyone();



        // This is fine.

        m.check_directory(d.path("a/b/c")).unwrap();

        // This isn't a directory!

        let err = m.check_directory(d.path("a/b/c/d")).unwrap_err();

        assert!(matches!(err, Error::BadType(_)));



        // But it _is_ a file.

        m.verifier()

            .require_file()

            .check(d.path("a/b/c/d"))

            .unwrap();

    }



    // TODO: Write far more tests.

    // TODO: Write far more tests.

    // * Can there be a test for a failed readlink()?  I can't see an easy way

    // * Can there be a test for a failed readlink()?  I can't see an easy way

    //   to provoke that without trying to make a time-of-check/time-of-use race

    //   to provoke that without trying to make a time-of-check/time-of-use race