Fixed a testing bug

The script was correctly opening the `truth` file by piping `echo` to
it, but then it tried to deny another operation on it. But since pipes
are opened by the script process, the permission was given to the
script. And since the permissions are preserved for the entire runtime
of a process, and child processes inherit permissions of their parents,
any command executed later would also have the necessary permissions to
open `truth` (which was the case for the second operation). Now the
second operation is performed on a different file.

Makefile

@@ -43,7 +43,7 @@ endif
 
 # set up targets 
 
-TARGETS := icfs
+TARGETS := $(BUILD_DIR)/icfs
 
 ifeq ($(TEST), 1)
 	TARGETS += icfs_test
@@ -56,12 +56,11 @@ default: $(TARGETS)
 
 .PHONY: clean
 
-icfs: $(BUILD_DIR)/main.o $(BUILD_DIR)/fuse_operations.o $(BUILD_DIR)/sourcefs.o $(BUILD_DIR)/ui-socket.o $(BUILD_DIR)/temp_permissions_table.o
+$(BUILD_DIR)/icfs: $(BUILD_DIR)/main.o $(BUILD_DIR)/fuse_operations.o $(BUILD_DIR)/sourcefs.o $(BUILD_DIR)/ui-socket.o $(BUILD_DIR)/temp_permissions_table.o
 	$(CC) $(CFLAGS) $^ $(LDFLAGS) -o $(BUILD_DIR)/icfs
 
-icfs_test: $(BUILD_DIR)/main.o $(BUILD_DIR)/fuse_operations.o $(BUILD_DIR)/sourcefs.o $(BUILD_DIR)/ui-socket.o
-	$(CC) $(CFLAGS) $^ $(LDFLAGS) -o $(BUILD_DIR)/icfs_test
-	# $(BUILD_DIR)/icfs_test # TODO: implement testing
+icfs_test: $(BUILD_DIR)/icfs
+	cd ./test && ./test.bash
 
 $(BUILD_DIR)/test_access_control.o: $(TESTS_DIR)/test_access_control.c
 	$(CC) $(CFLAGS) -c $< $(LDFLAGS) -o $@

src/fuse_operations.c

@@ -108,7 +108,29 @@ static int xmp_getattr(const char *path, struct stat *stbuf,
 static int xmp_access(const char *path, int mask) {
   int res;
 
-  res = access(path, mask);
+  // if mask is F_OK, then we don't need to check the permissions
+  // (is that possible?)
+
+  if (mask != F_OK) {
+    struct process_info pi;
+    struct fuse_context *fc = fuse_get_context();
+
+    pi.PID = fc->pid;
+    pi.UID = fc->uid;
+    pi.name = get_process_name_by_pid(pi.PID);
+
+    // fprintf(stderr, "%s, %d\n", path, ask_access(path, pi));
+
+    if (!interactive_access(real_filename(path), pi)) {
+      free(pi.name);
+      return -EACCES;
+    }
+
+    free(pi.name);
+  }
+
+  res = source_access(path, mask);
+
   if (res == -1)
     return -errno;
 
@@ -304,6 +326,30 @@ static int xmp_rename(const char *from, const char *to, unsigned int flags) {
   if (flags)
     return -EINVAL;
 
+  struct process_info pi;
+  struct fuse_context *fc = fuse_get_context();
+
+  pi.PID = fc->pid;
+  pi.UID = fc->uid;
+  pi.name = get_process_name_by_pid(pi.PID);
+
+  // fprintf(stderr, "%s, %d\n", path, ask_access(path, pi));
+
+  if (!interactive_access(real_filename(from), pi)) {
+    free(pi.name);
+    return -EACCES;
+  }
+
+  // the "to" file may exist and the process needs to get persmission to modify
+  // it
+  if (source_access(to, F_OK) == 0 &&
+      !interactive_access(real_filename(to), pi)) {
+    free(pi.name);
+    return -EACCES;
+  }
+
+  free(pi.name);
+
   res = source_rename(from, to);
   if (res == -1)
     return -errno;
@@ -313,6 +359,22 @@ static int xmp_rename(const char *from, const char *to, unsigned int flags) {
 
 static int xmp_link(const char *from, const char *to) {
   int res;
+  struct process_info pi;
+  struct fuse_context *fc = fuse_get_context();
+
+  pi.PID = fc->pid;
+  pi.UID = fc->uid;
+  pi.name = get_process_name_by_pid(pi.PID);
+
+  // fprintf(stderr, "%s, %d\n", path, ask_access(path, pi));
+  if (!interactive_access(real_filename(from), pi)) {
+    free(pi.name);
+    return -EACCES;
+  }
+
+  // no need to check the access to the "to" file, see link(2)
+
+  free(pi.name);
 
   res = source_link(from, to);
   if (res == -1)
@@ -323,6 +385,20 @@ static int xmp_link(const char *from, const char *to) {
 
 static int xmp_chmod(const char *path, mode_t mode, struct fuse_file_info *fi) {
   int res;
+  struct process_info pi;
+  struct fuse_context *fc = fuse_get_context();
+
+  pi.PID = fc->pid;
+  pi.UID = fc->uid;
+  pi.name = get_process_name_by_pid(pi.PID);
+
+  // fprintf(stderr, "%s, %d\n", path, ask_access(path, pi));
+  if (!interactive_access(real_filename(path), pi)) {
+    free(pi.name);
+    return -EACCES;
+  }
+
+  free(pi.name);
 
   if (fi)
     res = fchmod(fi->fh, mode);
@@ -334,9 +410,27 @@ static int xmp_chmod(const char *path, mode_t mode, struct fuse_file_info *fi) {
   return 0;
 }
 
+/**
+ * This filesystem is not designed for multiuser operation (e.g. with
+ * allow_other) so there is little point in having chown implemnted
+ */
 static int xmp_chown(const char *path, uid_t uid, gid_t gid,
                      struct fuse_file_info *fi) {
   int res;
+  struct process_info pi;
+  struct fuse_context *fc = fuse_get_context();
+
+  pi.PID = fc->pid;
+  pi.UID = fc->uid;
+  pi.name = get_process_name_by_pid(pi.PID);
+
+  // fprintf(stderr, "%s, %d\n", path, ask_access(path, pi));
+  if (!interactive_access(real_filename(path), pi)) {
+    free(pi.name);
+    return -EACCES;
+  }
+
+  free(pi.name);
 
   if (fi)
     res = fchown(fi->fh, uid, gid);
@@ -660,7 +754,7 @@ static off_t xmp_lseek(const char *path, off_t off, int whence,
 static const struct fuse_operations xmp_oper = {
     .init = xmp_init,
     .getattr = xmp_getattr,
-    // .access = xmp_access,
+    .access = xmp_access,
     .readlink = xmp_readlink,
     .opendir = xmp_opendir,
     .readdir = xmp_readdir,
@@ -676,7 +770,7 @@ static const struct fuse_operations xmp_oper = {
     .chown = xmp_chown,
     .truncate = xmp_truncate,
 #ifdef HAVE_UTIMENSAT
-// .utimens = xmp_utimens,
+//  .utimens = xmp_utimens,
 #endif
     .create = xmp_create,
     .open = xmp_open,

src/sourcefs.c

@@ -66,6 +66,11 @@ int source_symlink(const char *target, const char *linkpath) {
   return symlinkat(target, handle.root_fd, relative_linkpath);
 }
 
+int source_access(const char *filename, int mode) {
+  const char *relative_filename = source_filename_translate(filename);
+  return faccessat(handle.root_fd, relative_filename, mode, 0);
+}
+
 DIR *source_opendir(const char *filename) {
   const char *relative_filename = source_filename_translate(filename);
   int fd = openat(handle.root_fd, relative_filename, 0);

src/sourcefs.h

@@ -47,6 +47,8 @@ int source_chown(const char *filename, uid_t owner, gid_t group);
 
 int source_truncate(const char *filename, off_t length);
 
+int source_access(const char *filename, int mode);
+
 /* `open` and `create` are designed to correspond to fuse operations, not the
  * libc's `open(2)`. Both of them actually call `openat`. */
 

test/test.bash

@@ -4,7 +4,7 @@
 
 rm -rf ./protected
 mkdir protected
-touch ./protected/do-not-remove ./protected/should-be-removed ./protected/truth ./protected/perm000 ./protected/perm777 ./protected/this-name-is-wrong
+touch ./protected/do-not-remove ./protected/should-be-removed ./protected/truth ./protected/perm000 ./protected/perm777 ./protected/should-be-renamed ./protected/do-not-rename
 chmod 777 ./protected/perm777 ./protected/perm000
 echo "Free code, free world." >./protected/motto
 
@@ -70,11 +70,11 @@ rm ./protected/should-be-removed >/dev/null 2>/dev/null &&
 # rename files
 
 zenity --set-fake-response no
-mv ./protected/truth ./protected/lie 2>/dev/null &&
+mv ./protected/do-not-rename ./protected/terrible-name 2>/dev/null &&
   echo "[ICFS-TEST]: mv can rename protected/truth despite access being denied!" ||
   echo "[ICFS-TEST]: OK" # EACCESS
 zenity --set-fake-response yes_tmp
-mv ./protected/this-name-is-wrong ./protected/this-name-is-correct 2>/dev/null &&
+mv ./protected/should-be-renamed ./protected/great-name 2>/dev/null &&
   echo "[ICFS-TEST]: OK" ||
   echo "[ICFS-TEST]: mv cannot rename should-be-removed to renamed-file despite access being permitted!" # OK