Commit f35b4881 authored by Nick Mathewson's avatar Nick Mathewson 🦞
Browse files

fs-mistrust: write a lot about TOCTOU issues.

parent 2f467245
Loading
Loading
Loading
Loading
+13 −0
Original line number Original line Diff line number Diff line
@@ -17,6 +17,19 @@ use std::os::unix::fs::OpenOptionsExt;
/// The accessor functions will enforce that whatever security properties we
/// The accessor functions will enforce that whatever security properties we
/// checked on the the directory also apply to all of the members that we access
/// checked on the the directory also apply to all of the members that we access
/// within the directory.
/// within the directory.
///
/// ## Limitations
///
/// Having a `CheckedDir` means only that, at the time it was created, we were
/// confident that no _untrusted_ user could access it inappropriately.  It is
/// still possible, after the `CheckedDir` is created, that a _trusted_ user can
/// alter its permissions, make its path point somewhere else, or so forth.
///
/// If this kind of time-of-use/time-of-check issue is unacceptable, you may
/// wish to look at other solutions, possibly involving `openat()` or related
/// APIs.
///
/// See also the crate-level [Limitations](crate#limitations) section.
pub struct CheckedDir {
pub struct CheckedDir {
    /// The `Mistrust` object whose rules we apply to members of this directory.
    /// The `Mistrust` object whose rules we apply to members of this directory.
    mistrust: Mistrust,
    mistrust: Mistrust,
+6 −5
Original line number Original line Diff line number Diff line
@@ -71,6 +71,8 @@ impl<'a> super::Verifier<'a> {
        boxed(
        boxed(
            rp.filter(should_retain)
            rp.filter(should_retain)
                // Finally, check the path for errors.
                // Finally, check the path for errors.
                //
                // See `check_one` below for a note on TOCTOU issues.
                .flat_map(move |r| match r {
                .flat_map(move |r| match r {
                    Ok((path, path_type, metadata)) => {
                    Ok((path, path_type, metadata)) => {
                        self.check_one(path.as_path(), path_type, &metadata)
                        self.check_one(path.as_path(), path_type, &metadata)
@@ -115,12 +117,11 @@ impl<'a> super::Verifier<'a> {
        std::iter::empty()
        std::iter::empty()
    }
    }


    /// Check a single `path` for conformance with this `Concrete` mistrust.
    /// Check a single `path` for conformance with this `Verifier`.
    ///
    ///
    /// `position` is the position of the path within the ancestors of the
    /// Note that this result is only meaningful if all of the _ancestors_ of
    /// target path.  If the `position` is 0, then it's the position of the
    /// this path have been checked.  Otherwise, a non-trusted user could change
    /// target path itself. If `position` is 1, it's the target's parent, and so
    /// where this path points after it has been checked.
    /// on.
    #[must_use]
    #[must_use]
    pub(crate) fn check_one(
    pub(crate) fn check_one(
        &self,
        &self,
+35 −9
Original line number Original line Diff line number Diff line
//! # `fs-mistrust`: make sure that files are really private.
//! # `fs-mistrust`: check whether file permissions are private.
//!
//!
//! This crates provides a set of functionality to check the permissions on
//! This crate provides a set of functionality to check the permissions on files
//! files and directories to ensure that they are effectively private—that is,
//! and directories to ensure that they are effectively private—that is, that
//! that they are only readable or writable by trusted[^1] users.
//! they are only readable or writable by trusted[^1] users.
//!
//!
//! That's trickier than it sounds:
//! This kind of check can protect your users' data against misconfigurations,
//! such as cases where they've accidentally made their home directory
//! world-writable, or where they're using a symlink stored in a directory owned
//! by another user.
//!
//! The checks in this crate try to guarantee that, after a path has been shown
//! to be private, no action by a _non-trusted user_ can make that path private.
//! It's still possible for a _trusted user_ to change a path after it has been
//! checked.  Because of that, you may want to use other mechanisms if you are
//! concerned about time-of-check/time-of-use issues caused by _trusted_ users
//! altering the filesystem.
//!
//! Also see the [Limitations](#limitations) section below.
//!
//! [^1]: we define "trust" here in the computer-security sense of the word: a
//!      user is "trusted" if they have the opportunity to break our security
//!      guarantees.  For example, `root` on a Unix environment is "trusted",
//!      whether you actually trust them or not.
//!
//! ## What's so hard about checking permissions?
//!
//! Suppose that we want to know whether a given path can be read or modified by
//! an untrusted user. That's trickier than it sounds:
//!
//!
//! * Even if the permissions on the file itself are correct, we also need to
//! * Even if the permissions on the file itself are correct, we also need to
//!   check the permissions on the directory holding it, since they might allow
//!   check the permissions on the directory holding it, since they might allow
@@ -38,10 +60,6 @@
//! for file privacy checking and enforcement, along with clear justifications
//! for file privacy checking and enforcement, along with clear justifications
//! in its source for why it behaves that way.
//! in its source for why it behaves that way.
//!
//!
//! [^1]: we define "trust" here in the computer-security sense of the word: a
//!      user is "trusted" if they have the opportunity to break our security
//!      guarantees.  For example, `root` on a Unix environment is "trusted",
//!      whether you actually trust them or not.
//!
//!
//! ## What we actually do
//! ## What we actually do
//!
//!
@@ -170,12 +188,20 @@
//!
//!
//! ## Limitations
//! ## Limitations
//!
//!
//! As noted above, this crate only checks whether a path can be changed by
//! _non-trusted_ users.  After the path has been checked, a _trusted_ user can
//! still change its permissions.  (For example, the user could make their home
//! directory world-writable.)  This crate does not try to defend against _that
//! kind_ of time-of-check/time-of-use issue.
//!
//! We currently assume a fairly vanilla Unix environment: we'll tolerate other
//! We currently assume a fairly vanilla Unix environment: we'll tolerate other
//! systems, but we don't actually look at the details of any of these:
//! systems, but we don't actually look at the details of any of these:
//!    * Windows security (ACLs, SecurityDescriptors, etc)
//!    * Windows security (ACLs, SecurityDescriptors, etc)
//!    * SELinux capabilities
//!    * SELinux capabilities
//!    * POSIX (and other) ACLs.
//!    * POSIX (and other) ACLs.
//!
//!
//! On Windows, we accept all file permissions and owners.
//!
//! We don't check for mount-points and the privacy of filesystem devices
//! We don't check for mount-points and the privacy of filesystem devices
//! themselves.  (For example, we don't distinguish between our local
//! themselves.  (For example, we don't distinguish between our local
//! administrator and the administrator of a remote filesystem. We also don't
//! administrator and the administrator of a remote filesystem. We also don't
+16 −8
Original line number Original line Diff line number Diff line
@@ -54,22 +54,30 @@ pub(crate) enum PathType {
///
///
/// # Implementation notes
/// # Implementation notes
///
///
/// Abstractly, at any given point, the directory that we're resolving looks like
/// Abstractly, at any given point, the directory that we're resolving looks
/// `"resolved"/"remaining"`, where `resolved` is the part that we've already
/// like `"resolved"/"remaining"`, where `resolved` is the part that we've
/// looked at (in canonical form, with all symlinks resolved) and `remaining` is
/// already looked at (in canonical form, with all symlinks resolved) and
/// the part that we're still trying to resolve.
/// `remaining` is the part that we're still trying to resolve.
///
///
/// We represent `resolved` as a nice plain PathBuf, and  `remaining` as a stack
/// We represent `resolved` as a nice plain PathBuf, and  `remaining` as a stack
/// of strings that we want to push on to the end of the path.  We initialize the
/// of strings that we want to push on to the end of the path.  We initialize
/// algorithm with `resolved` empty and `remaining` seeded with the path we want
/// the algorithm with `resolved` empty and `remaining` seeded with the path we
/// to resolve.  Once there are no more parts to push, the path resolution is
/// want to resolve.  Once there are no more parts to push, the path resolution
/// done.
/// is done.
///
///
/// The following invariants apply whenever we are outside of the `next`
/// The following invariants apply whenever we are outside of the `next`
/// function:
/// function:
///    * `resolved.join(remaining)` is an alias for our target path.
///    * `resolved.join(remaining)` is an alias for our target path.
///    * `resolved` is in canonical form.
///    * `resolved` is in canonical form.
///    * Every ancestor of `resolved` is a key of `already_inspected`.
///    * Every ancestor of `resolved` is a key of `already_inspected`.
///
/// # Limitations
///
/// Because we're using `Path::metadata` rather than something that would use
/// `openat()` and `fstat()` under the hood, the permissions returned here are
/// potentially susceptible to TOCTOU issues.  In this crate we address these
/// issues by checking each yielded path immediately to make sure that only
/// _trusted_ users can change it after it is checked.
//
//
// TODO: This code is potentially of use outside this crate.  Maybe it should be
// TODO: This code is potentially of use outside this crate.  Maybe it should be
// public?
// public?