Add openat/unlinkat/etc. abstractions to ReadDir/DirEntry/OpenOptions

[the8472]

Proposal

Problem statement

Current std::fs APIs lead to code that's vulnerable to TOCTOU issues because performing any operation relative to a directory currently has to be done by composing the directory's path and the relative path. This happens because the directory structure can change under the user's nose between enumerating the directory entries and then trying to open/create/delete/... a file in that directory.

cap_std::fs::Dir by @sunfishcode already solves this.

Motivating examples or use cases

CVE-2022-21658, CVE-2018-15664, CVE-2021-20316 and similar symlink-TOCTOUs in many applications.

Solution sketch

This is more or less adopting fs::{Dir, DirEntry} APIs or a subset thereof into std. It may be possible to add the Dir methods directly to ReadDir instead.

impl Dir/ReadDir {
     pub fn open<P: AsRef<Path>>(&self, path: P) -> Result<File>
     /// This could be put on OpenOptions instead
     pub fn open_with<P: AsRef<Path>>(&self, path: P, options: &OpenOptions) -> Result<File>
     pub fn create_dir<P: AsRef<Path>>(&self, path: P) -> Result<()>
     pub fn rename<P: AsRef<Path>, Q: AsRef<Path>>(&self, from: P, to_dir: &Self, to: Q) -> Result<()>
     pub fn remove_file<P: AsRef<Path>>(&self, path: P) -> Result<()>
     pub fn remove_dir<P: AsRef<Path>>(&self, path: P) -> Result<()>
     pub fn symlink<P: AsRef<Path>, Q: AsRef<Path>>(&self, original: P, link: Q)

     /// ... more convenience methods
}

impl DirEntry {
    pub fn open(&self) -> Result<File>
    /// This could be put on OpenOptions instead
    pub fn open_with(&self, options: &OpenOptions) -> Result<File>
    pub fn remove_file(&self) -> Result<()>
    pub fn remove_dir(&self) -> Result<()>
}

The implementation work can be done piecemeal:

• make the dir file descriptor internally available
• add basic operations corresponding to openat/statat/unlinkat/renameat
• add abstractions like copy, create_dir_all etc.

Open Questions

How do we deal with windows? NtCreateFile is considered internal. We're already using it to make remove_dir_all robust against races but in principle we could revert to simpler implementation. Adding public APIs that require it would be a foward-compatibility hazard.

How do we handle platforms that lack some of the necessary APIs? Emulate them via path manipulation (which reintroduces the TOCTOU) or return Unsupported errors?

Alternatives

Status Quo

Crates handling this already exist, we can tell users to use them in security-sensitive contexts.

IO-safety APIs for ReadDir

This would simplify using std::ReadDir as a directory handle and then passing it to crates like cap_std or openat

That would adding From<OwnedFd>, Into<OwnedFd>, AsFd (and windows equivalents) to ReadDir.

Issues:

• Posix says sharing the file descriptor of a *DIR can lead to undefined behavior. I assume in practice this mostly leads to unspecified results, not memory unsafety, but who knows. To be on the safe side we'd have to reimplement what glibc does (getdents and such on each platform) to do this safely, which is a significant amount of work because those implementations aren't portable.
• Not all unix-likes support fdopendir. Either return ErrorKind::Unsupported on those platforms or figure out some workarounds

Links and related work

• https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/ReadDir's.20fd/near/279851193
• safe_path
• cap_std
• openat
• pathrs
• java's SecureDirectoryStream

[ChrisDenton]

How do we deal with windows? NtCreateFile is considered internal. We're already using it to make remove_dir_all robust against races but in principle we could revert to simpler implementation. Adding public APIs that require it would be a foward-compatibility hazard.

I've been reassured that there is not such a hazard to using NtCreateFile. The Calling Internal APIs is mostly outdated (e.g. ntdll.lib is shipped with the normal Windows SDK, so there is "an associated import library"). The hazard is only that with CreateFileW you may get new features without recompiling, whereas with NtCreateFile they may have to be implemented manually (e.g. by setting a new flag, etc).

[BurntSushi]

SGTM.

I'm also interested in this from the perspective of avoiding lots of tiny little allocations when crawling a large directory tree. (It's as hand wavy as it sounds.)

[ChrisDenton]

This has been accepted. Should this issue be closed now? Further discussion can happen on the tracking issue.

[thomcc]

I think without https://github.com/rust-lang/rust/pull/104385 most of these won't be implementable on older macOS, FYI. (Mostly providing this as a link for further motivation of that change)

[sunfishcode]

Where is the tracking issue?

[thomcc]

The tracking issue for what?

[sunfishcode]

@ChrisDenton mentioned that further discussion should happen on the tracking issue.

[thomcc]

Oh, yeah. Good question, I missed that too.

[ChrisDenton]

I mean a tracking issue should be created. Not that there is one already. The process is ACP -> Tracking issues -> (implementation, refinement, etc).

[the8472]

tracking issue: https://github.com/rust-lang/rust/issues/120426