Project

General

Profile

Actions

Feature #19347

closed

Add Dir.fchdir

Added by jeremyevans0 (Jeremy Evans) almost 2 years ago. Updated over 1 year ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:111842]

Description

Recently, I was working on a program that passes file descriptors over UNIX sockets (using send_io/recv_io). For file/socket/device descriptors, this works fine using File#reopen. However, I found that while Ruby supports Dir#fileno to return a directory file descriptor, it cannot use that file descriptor when changing directories.

I worked around this in my program by writing the directory path over the UNIX socket, but this results in a TOCTOU vulnerability in certain cases. Passing the directory file descriptor would be simpler and avoid the vulnerability.

I've submitted a pull request to implement this method: https://github.com/ruby/ruby/pull/7135

Updated by kjtsanaktsidis (KJ Tsanaktsidis) almost 2 years ago

I have definitely wanted this before FWIW. The main use-case I've had for this feature is writing code to look at things in the /proc filesystem on Linux. If a process exits, its pid can be re-used, and so the same path in /proc can now refer to a totally different process!

To operate on /proc safely, you need to open the the /proc/#{pid} directory first, and use openat(2) to open individual files from within that directory. If the process exits, the directory suddenly becomes empty, so your operations will begin failing with ENOENT. Even if a new process is started with the same pid, the entry in /proc has a different inode, so you will never accidentally read a different process's data.

Updated by kjtsanaktsidis (KJ Tsanaktsidis) almost 2 years ago

Having actually read the title of this issue, I see it's for implementing fchdir, not openat - but the point is the same, directory file descriptors are important to avoid some race conditions and Ruby doesn't really have a way to use them.

I see a discussion on openat fizzled away many years ago - https://bugs.ruby-lang.org/issues/10181. I would be happy to take another stab at an API design for openat in Ruby if people generally agree with the proposition "we should have better support for directory file descriptors in Ruby".

Updated by ko1 (Koichi Sasada) almost 2 years ago

At the dev meeting there are 4 ideas:

  • 4 ideas
    • Dir.fchdir(int)
      • Proposed
      • Consistent to the POSIX API
      • Can be extended for Dir.fchdir(dir) and Dir.fchdir(io)
    • Dir.chdir(int)
      • Do not need to add new method
      • Dir.chdir partially implemented on Windows because fchdir is not supported by Windows. respond_to? is not usable to test the availability.
      • Can be extended for Dir.fchdir(dir) and Dir.fchdir(io)
    • dir = Dir.for_fd(int); dir.chdir
      • More object oriented?
      • 2 more methods (Dir.for_fd(int) and Dir#chdir) are needed
      • Dir.for_fd(io.fileno).chdir needs two objects (IO and Dir) references one FD. It would have a problem with GC. closedir frees DIR structure and closes FD. Thus, we cannot implement autoclose: false for Dir.for_fd.
      • Dir.for_fd(io.fileno, autoclose: false)
        • use dup(io.fileno) and close that, leave the original one alone?
    • IO#fchdir

but not concluded.

More discussions are welcome.

Updated by jeremyevans0 (Jeremy Evans) almost 2 years ago

ko1 (Koichi Sasada) wrote in #note-3:

At the dev meeting there are 4 ideas:

I considered the first three ideas when implementing this support. Let me add my comments on each.

  • 4 ideas
    • Dir.fchdir(int)
      • Proposed
      • Consistent to the POSIX API
      • Can be extended for Dir.fchdir(dir) and Dir.fchdir(io)

The reason I used this approach is that this feature is likely to be rarely used, and not all platforms support it.

I'm OK with adding support for Dir.fchdir(dir), but prefer the Dir#chdir approach mentioned below.

  • Dir.chdir(int)
    • Do not need to add new method
    • Dir.chdir partially implemented on Windows because fchdir is not supported by Windows. respond_to? is not usable to test the availability.
    • Can be extended for Dir.fchdir(dir) and Dir.fchdir(io)

I considered this option, but I think it's more likely that Ruby programmers would accidentally call Dir.chdir with an integer argument by accident than deliberately for passing a directory file descriptor, and then they would receive either a confusing error message, or in unfortunate cases, an unintended directory change.

  • dir = Dir.for_fd(int); dir.chdir
    • More object oriented?
    • 2 more methods (Dir.for_fd(int) and Dir#chdir) are needed

I considered this option, but it requires an additional and potentially unnecessary Dir allocation.

I think it would be useful to add Dir.for_fd (the equivalent of the C function fdopendir). However, I didn't have an immediate need for it, so I didn't want to include it in this feature request.

I also like the idea of Dir#chdir, which could use Dir.fchdir (if directory file descriptors are supported) or Dir.chdir (otherwise).

I recommend supporting both Dir.for_fd and Dir#chdir, but as separate features after Dir.fchdir is merged. I volunteer to implement Dir.for_fd and/or Dir#chdir if either/both are approved.

* `Dir.for_fd(io.fileno).chdir` needs two objects (IO and Dir) references one FD.  It would have a problem with GC. `closedir` frees DIR structure and closes FD. Thus, we cannot implement `autoclose: false` for `Dir.for_fd`.
* Dir.for_fd(io.fileno, autoclose: false)
    * use dup(io.fileno) and close that, leave the original one alone?
  • IO#fchdir

I'm against IO#fchdir as almost no IO objects are for directories. I don't think we should offer any direct support for IO objects representing directories.

Updated by kjtsanaktsidis (KJ Tsanaktsidis) almost 2 years ago

I like the idea of having Dir.for_fd and Dir#chdir - it opens the door to more object-oriented directory file descriptor methods on Dir (of particular interest to me - Dir#openat).

Updated by matz (Yukihiro Matsumoto) almost 2 years ago

I agree with having Dir.for_fd and Dir#chdir as well. If the assumed use-case is handling a file descriptor sent via UNIX domain socket, allowing chdir only is not sufficient. We should allow other directory-related operations via Dir class.

Matz.

Updated by jeremyevans0 (Jeremy Evans) almost 2 years ago

matz (Yukihiro Matsumoto) wrote in #note-6:

I agree with having Dir.for_fd and Dir#chdir as well. If the assumed use-case is handling a file descriptor sent via UNIX domain socket, allowing chdir only is not sufficient. We should allow other directory-related operations via Dir class.

I've updated https://github.com/ruby/ruby/pull/7135 to add Dir.for_fd and Dir#chdir. I'm not sure if it is an issue, but Dir.for_fd(fd).path returns nil, because there is no path associated with such a directory. Hopefully that is acceptable. I think it is possible to generate a path by walking upwards in the directory tree to the root of the file system, scanning each directory visited to find the name pointing to the previous directory, and using that to build a path. However, I'm not sure that is worth doing.

Updated by kjtsanaktsidis (KJ Tsanaktsidis) almost 2 years ago

I'm not sure if it is an issue, but Dir.for_fd(fd).path returns nil, because there is no path associated with such a directory. Hopefully that is acceptable. I think it is possible to generate a path by walking upwards in the directory tree to the root of the file system, scanning each directory visited to find the name pointing to the previous directory, and using that to build a path. However, I'm not sure that is worth doing.

On Linux you could implement it as File.readlink("/proc/self/fd/#{self.fileno}"), although I don't know if it's better to have #path working on only some platforms vs just not working at all anywhere, consistently.

Actions #9

Updated by jeremyevans (Jeremy Evans) over 1 year ago

  • Status changed from Open to Closed

Applied in changeset git|466ca7ae205126c7cac83735db887d69e293f816.


Add Dir.fchdir

This is useful for passing directory file descriptors over UNIX
sockets or to child processes to avoid TOCTOU vulnerabilities.

The implementation follows the Dir.chdir code.

This will raise NotImplementedError on platforms not supporting
both fchdir and dirfd.

Implements [Feature #19347]

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0