Allow read_link to read absolute paths

Previously it would return an error if the link's destination were
absolute.  But there were three problems with this:

* It wouldn't return an error if the link's destination were a relative
  path that uses ../ to point outside of the sandbox. That's
  inconsistent.
* Sometimes a symlink with an absolute path doesn't escape the sandbox.
  For example /sandbox/link -> /sandbox/target.  But read_link wouldn't
  allow that.
* More importantly, sometimes it's important to read a link that points
  outside of the sandbox. For example, a backup application could copy
  all of the files from the root directory of one computer to a
  subdirectory of another. Symlinks would be broken, but would work
  again after a restore operation. The application that performs the
  restore would need to be able to read link targets, even if they point
  outside of the root. Or, a jail file system could contain absolute
  symlinks that resolve correctly for a jailed process, but not for an
  unjailed one. But it would still sometimes be useful for an unjailed
  process to read the links.

Fixes #353

Author: asomers

cap-primitives/src/fs/read_link.rs

@@ -1,7 +1,7 @@
 //! This defines `read_link`, the primary entrypoint to sandboxed symbolic link
 //! dereferencing.
 
-use crate::fs::{errors, read_link_impl};
+use crate::fs::read_link_impl;
 #[cfg(racy_asserts)]
 use crate::fs::{map_result, read_link_unchecked, stat, FollowSymlinks};
 use std::path::{Path, PathBuf};
@@ -24,27 +24,12 @@ pub fn read_link_contents(start: &fs::File, path: &Path) -> io::Result<PathBuf>
     result
 }
 
-/// Perform a `readlinkat`-like operation, ensuring that the resolution of the
-/// path never escapes the directory tree rooted at `start`, and also verifies
-/// that the link target is not absolute.
+/// Perform a `readlinkat`-like operation.
 #[cfg_attr(not(racy_asserts), allow(clippy::let_and_return))]
 #[inline]
 pub fn read_link(start: &fs::File, path: &Path) -> io::Result<PathBuf> {
     // Call the underlying implementation.
-    let result = read_link_contents(start, path);
-
-    // Don't allow reading symlinks to absolute paths. This isn't strictly
-    // necessary to preserve the sandbox, since `open` will refuse to follow
-    // absolute paths in any case. However, it is useful to enforce this
-    // restriction to avoid leaking information about the host filesystem
-    // outside the sandbox.
-    if let Ok(path) = &result {
-        if path.has_root() {
-            return Err(errors::escape_attempt());
-        }
-    }
-
-    result
+    read_link_contents(start, path)
 }
 
 #[cfg(racy_asserts)]

tests/fs.rs

@@ -935,24 +935,6 @@ fn symlink_noexist() {
 
 #[test]
 fn read_link() {
-    if cfg!(windows) {
-        // directory symlink
-        let root = Dir::open_ambient_dir(r"C:\", ambient_authority()).unwrap();
-        error_contains!(
-            root.read_link(r"Users\All Users"),
-            "a path led outside of the filesystem"
-        );
-        // junction
-        error_contains!(
-            root.read_link(r"Users\Default User"),
-            "a path led outside of the filesystem"
-        );
-        // junction with special permissions
-        error_contains!(
-            root.read_link(r"Documents and Settings\"),
-            "a path led outside of the filesystem"
-        );
-    }
     let tmpdir = tmpdir();
     let link = "link";
     if !got_symlink_permission(&tmpdir) {

tests/symlinks.rs

@@ -4,6 +4,7 @@ mod sys_common;
 use cap_fs_ext::DirExt;
 use cap_std::ambient_authority;
 use cap_std::fs::Dir;
+use std::path::Path;
 use sys_common::io::tmpdir;
 use sys_common::symlink_supported;
 
@@ -116,19 +117,19 @@ fn readlink_absolute() {
     let tmpdir = check!(Dir::open_ambient_dir(dir.path(), ambient_authority()));
 
     #[cfg(not(windows))]
-    error_contains!(
-        tmpdir.read_link("thing_symlink"),
-        "a path led outside of the filesystem"
+    assert_eq!(
+        tmpdir.read_link("thing_symlink").unwrap(),
+        Path::new("/thing")
     );
     #[cfg(windows)]
-    error_contains!(
-        tmpdir.read_link("file_symlink_file"),
-        "a path led outside of the filesystem"
+    assert_eq!(
+        tmpdir.read_link("file_symlink_file").unwrap(),
+        Path::new("/file")
     );
     #[cfg(windows)]
-    error_contains!(
-        tmpdir.read_link("dir_symlink_dir"),
-        "a path led outside of the filesystem"
+    assert_eq!(
+        tmpdir.read_link("dir_symlink_dir").unwrap(),
+        Path::new("/dir")
     );
 }