Implemented --resolved-symlink-deletion and added tests.

4 files changed, 310 insertions, 50 deletions

diff --git a/src/bindfs.1 b/src/bindfs.1
index 01887ac..00e434d 100644
--- a/src/bindfs.1
+++ b/src/bindfs.1
@@ -168,7 +168,7 @@ but actually does nothing.
 Makes chmod always fail with a 'permission denied' error.
 
 .TP
-.B \-\-chmod\-filter=\fIpermissions\fP,, \-o chmod\-filter=...
+.B \-\-chmod\-filter=\fIpermissions\fP, \-o chmod\-filter=...
 Changes the permission bits of a chmod request before it is applied to the
 original file. Accepts the same permission syntax as \-\-perms.
 See \fB\%PERMISSION \%SPECIFICATION\fP below for details.
@@ -249,6 +249,19 @@ not supported and will return an error. This is because a FUSE filesystem cannot
 reliably call itself recursively without deadlocking, especially in
 single-threaded mode.
 
+.TP
+.B \-\-resolved\-symlink\-deletion=\fIpolicy\fP, \-o resolved\-symlink\-deletion=\fIpolicy\fP
+If \fB\-\-resolve\-symlinks\fP is enabled, decides what happens when a resolved
+symlink is deleted.  The options are: \fBdeny\fP (resolved symlinks cannot be
+deleted), \fBsymlink-only\fP (the underlying symlink is deleted, its target is
+not), \fBsymlink-first\fP (the symlink is deleted, and if that succeeds,
+the target is deleted but no error is reported if that fails) or
+\fBtarget-first\fP (the target is deleted first, and the symlink is deleted
+only if deleting the target succeeded).  The default is \fBsymlink-only\fP.
+
+Note that deleting files inside symlinked directories is always possible with
+all settings, including \fBdeny\fP, unless something else protects those files.
+
 
 .SH MISCELLANEOUS OPTIONS
 
diff --git a/src/bindfs.c b/src/bindfs.c
index 6d08a11..22517ad 100644
--- a/src/bindfs.c
+++ b/src/bindfs.c
@@ -148,11 +148,19 @@ static struct Settings {
     gid_t *mirrored_members;
     int num_mirrored_members;
 
+    int hide_hard_links;
+    int resolve_symlinks;
+
+    enum ResolvedSymlinkDeletion {
+        RESOLVED_SYMLINK_DELETION_DENY,
+        RESOLVED_SYMLINK_DELETION_SYMLINK_ONLY,
+        RESOLVED_SYMLINK_DELETION_SYMLINK_FIRST,
+        RESOLVED_SYMLINK_DELETION_TARGET_FIRST
+    } resolved_symlink_deletion_policy;
+
     int realistic_permissions;
 
     int ctime_from_mtime;
-    int hide_hard_links;
-    int resolve_symlinks;
 
 } settings;
 
@@ -174,6 +182,9 @@ static int getattr_common(const char *path, struct stat *stbuf);
 /* Chowns a new file if necessary. */
 static void chown_new_file(const char *path, struct fuse_context *fc, int (*chown_func)(const char*, uid_t, gid_t));
 
+/* Unified implementation of unlink and rmdir. */
+static int delete_file(const char *path, int (*target_delete_func)(const char *));
+
 /* FUSE callbacks */
 static void *bindfs_init();
 static void bindfs_destroy(void *private_data);
@@ -380,6 +391,75 @@ static void chown_new_file(const char *path, struct fuse_context *fc, int (*chow
     }
 }
 
+static int delete_file(const char *path, int (*target_delete_func)(const char *)) {
+    int res;
+    char *real_path;
+    struct stat st;
+    char *also_try_delete = NULL;
+    char *unlink_first = NULL;
+    int (*main_delete_func)(const char*) = target_delete_func;
+
+    real_path = process_path(path, false);
+    if (real_path == NULL)
+        return -errno;
+
+    if (settings.resolve_symlinks) {
+        if (lstat(real_path, &st) == -1) {
+            free(real_path);
+            return -errno;
+        }
+
+        if (S_ISLNK(st.st_mode)) {
+            switch(settings.resolved_symlink_deletion_policy) {
+            case RESOLVED_SYMLINK_DELETION_DENY:
+                free(real_path);
+                return -EPERM;
+            case RESOLVED_SYMLINK_DELETION_SYMLINK_ONLY:
+                main_delete_func = &unlink;
+                break;
+            case RESOLVED_SYMLINK_DELETION_SYMLINK_FIRST:
+                main_delete_func = &unlink;
+
+                also_try_delete = realpath(real_path, NULL);
+                if (also_try_delete == NULL && errno != ENOENT) {
+                    free(real_path);
+                    return -errno;
+                }
+                break;
+            case RESOLVED_SYMLINK_DELETION_TARGET_FIRST:
+                unlink_first = realpath(real_path, NULL);
+                if (unlink_first == NULL && errno != ENOENT) {
+                    free(real_path);
+                    return -errno;
+                }
+
+                if (unlink_first != NULL) {
+                    res = unlink(unlink_first);
+                    free(unlink_first);
+                    if (res == -1) {
+                        free(real_path);
+                        return -errno;
+                    }
+                }
+                break;
+            }
+        }
+    }
+
+    res = main_delete_func(real_path);
+    free(real_path);
+    if (res == -1) {
+        free(also_try_delete);
+        return -errno;
+    }
+
+    if (also_try_delete != NULL) {
+        (void)target_delete_func(also_try_delete);
+        free(also_try_delete);
+    }
+
+    return 0;
+}
 
 
 static void *bindfs_init()
@@ -598,52 +678,12 @@ static int bindfs_mkdir(const char *path, mode_t mode)
 
 static int bindfs_unlink(const char *path)
 {
-    int res;
-    char *real_path;
-
-    real_path = process_path(path, false);
-    if (real_path == NULL)
-        return -errno;
-
-    res = unlink(real_path);
-    free(real_path);
-    if (res == -1)
-        return -errno;
-
-    return 0;
+    return delete_file(path, &unlink);
 }
 
 static int bindfs_rmdir(const char *path)
 {
-    int res;
-    char *real_path;
-    struct stat st;
-
-    real_path = process_path(path, false);
-    if (real_path == NULL)
-        return -errno;
-
-    if (settings.resolve_symlinks) {
-        if (lstat(real_path, &st) == -1) {
-            free(real_path);
-            return -errno;
-        }
-
-        if (S_ISLNK(st.st_mode)) {
-            res = unlink(real_path);
-            free(real_path);
-            if (res == -1)
-                return -errno;
-            return 0;
-        }
-    }
-
-    res = rmdir(real_path);
-    free(real_path);
-    if (res == -1)
-        return -errno;
-
-    return 0;
+    return delete_file(path, &rmdir);
 }
 
 static int bindfs_symlink(const char *from, const char *to)
@@ -1286,6 +1326,7 @@ static void print_usage(const char *progname)
            "                            from file content modification time.\n"
            "  --hide-hard-links         Always report a hard link count of 1.\n"
            "  --resolve-symlinks        Resolve symbolic links.\n"
+           "  --resolved-symlink-deletion=...  Decide how to delete resolved symlinks.\n"
            "  --multithreaded           Enable multithreaded mode. See man page\n"
            "                            for security issue with current implementation.\n"
            "\n"
@@ -1677,6 +1718,7 @@ int main(int argc, char *argv[])
         char *create_for_group;
         char *create_with_perms;
         char *chmod_filter;
+        char *resolved_symlink_deletion;
         int no_allow_other;
         int multithreaded;
     } od;
@@ -1734,10 +1776,12 @@ int main(int argc, char *argv[])
         OPT2("--xattr-ro", "xattr-ro", OPTKEY_XATTR_READ_ONLY),
         OPT2("--xattr-rw", "xattr-rw", OPTKEY_XATTR_READ_WRITE),
 
-        OPT2("--realistic-permissions", "realistic-permissions", OPTKEY_REALISTIC_PERMISSIONS),
-        OPT2("--ctime-from-mtime", "ctime-from-mtime", OPTKEY_CTIME_FROM_MTIME),
         OPT2("--hide-hard-links", "hide-hard-links", OPTKEY_HIDE_HARD_LINKS),
         OPT2("--resolve-symlinks", "resolve-symlinks", OPTKEY_RESOLVE_SYMLINKS),
+        OPT_OFFSET2("--resolved-symlink-deletion=%s", "resolved-symlink-deletion=%s", resolved_symlink_deletion, -1),
+
+        OPT2("--realistic-permissions", "realistic-permissions", OPTKEY_REALISTIC_PERMISSIONS),
+        OPT2("--ctime-from-mtime", "ctime-from-mtime", OPTKEY_CTIME_FROM_MTIME),
         OPT_OFFSET2("--multithreaded", "multithreaded", multithreaded, -1),
         FUSE_OPT_END
     };
@@ -1774,10 +1818,11 @@ int main(int argc, char *argv[])
     settings.num_mirrored_users = 0;
     settings.mirrored_members = NULL;
     settings.num_mirrored_members = 0;
-    settings.realistic_permissions = 0;
-    settings.ctime_from_mtime = 0;
     settings.hide_hard_links = 0;
     settings.resolve_symlinks = 0;
+    settings.resolved_symlink_deletion_policy = RESOLVED_SYMLINK_DELETION_SYMLINK_ONLY;
+    settings.realistic_permissions = 0;
+    settings.ctime_from_mtime = 0;
     atexit(&atexit_func);
 
     /* Parse options */
@@ -1912,6 +1957,23 @@ int main(int argc, char *argv[])
     }
 
 
+    /* Parse resolved_symlink_deletion */
+    if (od.resolved_symlink_deletion) {
+        if (strcmp(od.resolved_symlink_deletion, "deny") == 0) {
+            settings.resolved_symlink_deletion_policy = RESOLVED_SYMLINK_DELETION_DENY;
+        } else if (strcmp(od.resolved_symlink_deletion, "symlink-only") == 0) {
+            settings.resolved_symlink_deletion_policy = RESOLVED_SYMLINK_DELETION_SYMLINK_ONLY;
+        } else if (strcmp(od.resolved_symlink_deletion, "symlink-first") == 0) {
+            settings.resolved_symlink_deletion_policy = RESOLVED_SYMLINK_DELETION_SYMLINK_FIRST;
+        } else if (strcmp(od.resolved_symlink_deletion, "target-first") == 0) {
+            settings.resolved_symlink_deletion_policy = RESOLVED_SYMLINK_DELETION_TARGET_FIRST;
+        } else {
+            fprintf(stderr, "Invalid setting for --resolved-symlink-deletion: '%s'\n", od.resolved_symlink_deletion);
+            return 1;
+        }
+    }
+
+
     /* Single-threaded mode by default */
     if (!od.multithreaded) {
         fuse_opt_add_arg(&args, "-s");
diff --git a/tests/common.rb b/tests/common.rb
index 36576e5..5701123 100755
--- a/tests/common.rb
+++ b/tests/common.rb
@@ -179,6 +179,18 @@ def root_testenv(bindfs_args, options = {}, &block)
     end
 end
 
+# Like testenv but skips the test if not running as non-root.
+# TODO: make all tests runnable as root
+def nonroot_testenv(bindfs_args, options = {}, &block)
+    if Process.uid != 0
+        testenv(bindfs_args, options, &block)
+    else
+        puts "--- #{bindfs_args} ---"
+        puts "[  #{bindfs_args}  ]"
+        puts "SKIP (requires running as non-root)"
+    end
+end
+
 def umount_cmd
     if `which fusermount`.strip.empty?
     then 'umount'
diff --git a/tests/test_bindfs.rb b/tests/test_bindfs.rb
index 6c3fb0b..be9a612 100755
--- a/tests/test_bindfs.rb
+++ b/tests/test_bindfs.rb
@@ -373,7 +373,7 @@ end
 testenv("", :title => "utimens on symlinks") do
     touch('mnt/file')
     Dir.chdir "mnt" do
-      system('ln -sf file link')
+      symlink('file', 'link')
     end
 
     system("#{$tests_dir}/utimens_nofollow mnt/link 12 34 56 78")
@@ -385,6 +385,179 @@ testenv("", :title => "utimens on symlinks") do
     assert { File.lstat('mnt/file').mtime.to_i > 100 }
 end
 
+testenv("--resolve-symlinks", :title => "resolving symlinks") do
+  mkdir('src/dir')
+  File.write('src/dir/file', 'hello')
+  Dir.chdir 'src' do
+    symlink('dir', 'dirlink')
+    symlink('dir/file', 'filelink')
+    symlink('dirlink/file', 'filelink2')
+  end
+
+  assert { !File.lstat('mnt/dirlink').symlink? }
+  assert { File.lstat('mnt/dirlink').directory? }
+  assert { !File.lstat('mnt/dirlink/file').symlink? }
+  assert { File.lstat('mnt/dirlink/file').file? }
+  assert { File.lstat('mnt/filelink').file? }
+  assert { File.read('mnt/filelink') == 'hello' }
+end
+
+testenv("--resolve-symlinks", :title => "attributes of resolved symlinks") do
+  Dir.chdir 'src' do
+    touch('file')
+    symlink('file', 'link')
+    chmod(0654, 'file')
+  end
+
+  assert { File.lstat('mnt/link').mode & 0777 == 0654 }
+end
+
+testenv("--resolve-symlinks", :title => "writing through resolved symlinks") do
+  Dir.chdir 'src' do
+    File.write('file', 'initial_content')
+    symlink('file', 'link')
+  end
+
+  File.write('mnt/link', 'new_content')
+  assert { File.read('src/file') == 'new_content' }
+  assert { File.read('src/link') == 'new_content' }
+  assert { File.symlink?('src/link') }
+end
+
+testenv("--resolve-symlinks", :title => "moving over resolved symlinks") do
+  Dir.chdir 'src' do
+    File.write('file', 'initial_content')
+    File.write('newfile', 'new_content')
+    symlink('file', 'link')
+  end
+
+  Dir.chdir 'mnt' do
+    system("mv newfile link")
+  end
+  assert { File.symlink?('src/link') }
+  assert { File.read('src/file') == 'new_content' }
+  assert { !File.exist?('src/newfile') }
+end
+
+testenv("--resolve-symlinks", :title => "moving resolved symlinks") do
+  Dir.chdir 'src' do
+    touch('file')
+    symlink('file', 'link')
+  end
+
+  Dir.chdir 'mnt' do
+    system("mv link lonk")
+  end
+  assert { !File.symlink?('src/link') }
+  assert { File.lstat('src/lonk').symlink? }
+  assert { File.readlink('src/lonk') == 'file' }
+end
+
+testenv("--resolve-symlinks", :title => "--resolve-symlinks disallows new symlinks") do
+  touch('mnt/file')
+  Dir.chdir "mnt" do
+    begin
+      File.symlink("file", "link")
+    rescue Errno::EPERM => exception
+    end
+    assert { exception != nil }
+  end
+end
+
+testenv("--resolve-symlinks", :title => "deleting a resolved symlink deletes the underlying symlink only by default") do
+  Dir.chdir 'src' do
+    touch('file')
+    symlink('file', 'link')
+    symlink('broken', 'broken_link')
+  end
+
+  File.unlink('mnt/link')
+  assert { !File.symlink?('src/link') }
+  assert { File.exist?('src/file') }
+
+  File.unlink('mnt/broken_link')
+  assert { !File.symlink?('src/broken_link') }
+end
+
+testenv("--resolve-symlinks --resolved-symlink-deletion=deny") do
+  Dir.chdir 'src' do
+    touch('file')
+    symlink('file', 'link')
+  end
+
+  begin
+    File.unlink('mnt/link')
+  rescue Errno::EPERM => exception
+  end
+  assert { exception != nil }
+  assert { File.symlink?('src/link') }
+  assert { File.exist?('src/file') }
+end
+
+# TODO: make all tests runnable as root. This is nonroot because we can't
+# easily prevent a bindfs running as root from deleting a file.
+nonroot_testenv("--resolve-symlinks --resolved-symlink-deletion=symlink-first") do
+  begin
+    Dir.chdir 'src' do
+      mkdir('dir')
+      touch('deletable_file')
+      touch('dir/undeletable_file')
+      chmod(0555, 'dir')
+      symlink('deletable_file', 'link1')
+      symlink('dir/undeletable_file', 'link2')
+      symlink('broken', 'link3')
+    end
+
+    File.unlink('mnt/link1')
+    assert { !File.symlink?('src/link1') }
+    assert { !File.exist?('src/dir/deletable_file') }
+
+    File.unlink('mnt/link2')
+    assert { !File.symlink?('src/link2') }
+    assert { File.exist?('src/dir/undeletable_file') }
+
+    File.unlink('mnt/link3')
+    assert { !File.symlink?('src/link3') }
+  ensure
+    chmod(0777, 'src/dir')  # So the cleanup code can delete dir/*
+  end
+end
+
+# TODO: make all tests runnable as root. This is nonroot because we can't
+# easily prevent a bindfs running as root from deleting a file.
+nonroot_testenv("--resolve-symlinks --resolved-symlink-deletion=target-first -p a+w") do
+  begin
+    Dir.chdir 'src' do
+      mkdir('dir')
+      touch('file1')
+      touch('file2')
+      symlink('file1', 'deletable_link')
+      Dir.chdir('dir') do
+        symlink('../file2', 'undeletable_link')
+      end
+      chmod(0555, 'dir')
+      symlink('broken', 'broken_link')
+    end
+
+    File.unlink('mnt/deletable_link')
+    assert { !File.symlink?('src/deletable_link') }
+    assert { !File.exist?('src/file1') }
+
+    begin
+      File.unlink('mnt/dir/undeletable_link')
+    rescue Errno::EACCES => exception
+    end
+    assert { exception != nil }
+    assert { File.symlink?('src/dir/undeletable_link') }
+    assert { !File.exist?('src/file2') }
+
+    File.unlink('mnt/broken_link')
+    assert { !File.symlink?('src/broken_link') }
+  ensure
+    chmod(0777, 'src/dir')  # So the cleanup code can delete dir/*
+  end
+end
+
 # FIXME: this stuff around testenv is a hax, and testenv may also exit(), which defeats the 'ensure' below.
 # the test setup ought to be refactored. It might well use MiniTest or something.
 if Process.uid == 0