[io] restrict rights of files open with file_open_at to the rights of the directory handle

src/platform/platform_io.h

@@ -17,20 +17,29 @@
 
 typedef struct { u64 h; } file_handle;
 
-typedef u32 file_open_flags;
+typedef u16 file_open_flags;
-enum {
+enum _file_open_flags
-	FILE_OPEN_READ     = 1<<0,
+{
-	FILE_OPEN_WRITE    = 1<<1,
+	FILE_OPEN_NONE      = 0,
-	FILE_OPEN_APPEND   = 1<<2,
+	FILE_OPEN_APPEND    = 1<<1,
-	FILE_OPEN_TRUNCATE = 1<<3,
+	FILE_OPEN_TRUNCATE  = 1<<2,
-	FILE_OPEN_CREATE   = 1<<4,
+	FILE_OPEN_CREATE    = 1<<3,
 
-	FILE_OPEN_SYMLINK   = 1<<5,
+	FILE_OPEN_SYMLINK   = 1<<4,
-	FILE_OPEN_NO_FOLLOW = 1<<6,
+	FILE_OPEN_NO_FOLLOW = 1<<5,
-	FILE_OPEN_RESTRICT  = 1<<7,
+	FILE_OPEN_RESTRICT  = 1<<6,
 	//...
 };
 
+typedef u16 file_access_rights;
+enum _file_access_rights
+{
+	FILE_ACCESS_NONE  = 0,
+	FILE_ACCESS_READ  = 1<<1,
+	FILE_ACCESS_WRITE = 1<<2,
+};
+
+
 typedef enum { FILE_SEEK_SET, FILE_SEEK_END, FILE_SEEK_CURRENT } file_whence;
 
 typedef u64 io_req_id;
@@ -67,7 +76,12 @@ typedef struct io_req
 
 	union
 	{
-		file_open_flags openFlags;
+		struct
+		{
+			file_access_rights rights;
+			file_open_flags flags;
+		} open;
+
 		file_whence whence;
 	};
 
@@ -126,9 +140,8 @@ MP_API io_cmp io_wait_single_req(io_req* req);
 // File IO wrapper API
 //----------------------------------------------------------------
 
-MP_API file_handle file_open(str8 path, file_open_flags flags);
+MP_API file_handle file_open(str8 path, file_access_rights rights, file_open_flags flags);
-MP_API file_handle file_open_at(file_handle dir, str8 path, file_open_flags flags);
+MP_API file_handle file_open_at(file_handle dir, str8 path, file_access_rights rights, file_open_flags flags);
-MP_API file_handle file_open_relative(file_handle base, str8 path, file_open_flags flags);
 MP_API void file_close(file_handle file);
 
 MP_API i64 file_pos(file_handle file);

src/platform/platform_io_common.c

@@ -11,12 +11,13 @@
 // File stream read/write API
 //------------------------------------------------------------------------------
 
-file_handle file_open(str8 path, file_open_flags flags)
+file_handle file_open(str8 path, file_access_rights rights, file_open_flags flags)
 {
 	io_req req = {.op = IO_OP_OPEN_AT,
 	              .size = path.len,
 	              .buffer = path.ptr,
-	              .openFlags = flags};
+	              .open.rights = rights,
+	              .open.flags = flags };
 
 	io_cmp cmp = io_wait_single_req(&req);
 
@@ -25,13 +26,14 @@ file_handle file_open(str8 path, file_open_flags flags)
 	return(cmp.handle);
 }
 
-file_handle file_open_at(file_handle dir, str8 path, file_open_flags flags)
+file_handle file_open_at(file_handle dir, str8 path, file_access_rights rights, file_open_flags flags)
 {
 	io_req req = {.op = IO_OP_OPEN_AT,
 	              .handle = dir,
 	              .size = path.len,
 	              .buffer = path.ptr,
-	              .openFlags = flags};
+	              .open.rights = rights,
+	              .open.flags = flags,};
 
 	io_cmp cmp = io_wait_single_req(&req);
 	return(cmp.handle);

src/platform/platform_io_internal.c

@@ -20,6 +20,11 @@ file_slot* file_slot_alloc(file_table* table)
 		slot->fd = -1;
 		table->nextSlot++;
 	}
+
+	u32 tmpGeneration = slot->generation;
+	memset(slot, 0, sizeof(file_slot));
+	slot->generation = tmpGeneration;
+
 	return(slot);
 }
 

src/platform/platform_io_internal.h

@@ -26,6 +26,7 @@ typedef struct file_slot
 	bool fatal;
 	list_elt freeListElt;
 
+	file_access_rights rights;
 	PLATFORM_IO_NATIVE_MEMBER;
 
 } file_slot;

src/platform/posix_io.c

@@ -108,13 +108,13 @@ io_error io_convert_errno(int e)
 	return(error);
 }
 
-int io_convert_open_flags(file_open_flags flags)
+int io_convert_access_rights(file_access_rights rights)
 {
 	int oflags = 0;
 
-	if(flags & FILE_OPEN_READ)
+	if(rights & FILE_ACCESS_READ)
 	{
-		if(flags & FILE_OPEN_WRITE)
+		if(rights & FILE_ACCESS_WRITE)
 		{
 			oflags = O_RDWR;
 		}
@@ -123,10 +123,16 @@ int io_convert_open_flags(file_open_flags flags)
 			oflags = O_RDONLY;
 		}
 	}
-	else if(flags & FILE_OPEN_WRITE)
+	else if(rights & FILE_ACCESS_WRITE)
 	{
 		oflags = O_WRONLY;
 	}
+	return(oflags);
+}
+
+int io_convert_open_flags(file_open_flags flags)
+{
+	int oflags = 0;
 
 	if(flags & FILE_OPEN_TRUNCATE)
 	{
@@ -151,6 +157,27 @@ int io_convert_open_flags(file_open_flags flags)
 	return(oflags);
 }
 
+int io_update_dir_flags_at(int dirFd, char* path, int flags)
+{
+	struct stat s;
+	if(!fstatat(dirFd, path, &s, AT_SYMLINK_NOFOLLOW))
+	{
+		if((s.st_mode & S_IFMT) == S_IFDIR)
+		{
+			if(flags & O_WRONLY)
+			{
+				flags &= ~O_WRONLY;
+			}
+			else if(flags & O_RDWR)
+			{
+				flags &= ~O_RDWR;
+				flags |= O_RDONLY;
+			}
+		}
+	}
+	return(flags);
+}
+
 typedef struct io_open_restrict_result
 {
 	io_error error;
@@ -355,74 +382,89 @@ io_cmp io_open_at(file_slot* atSlot, io_req* req, file_table* table)
 	}
 	else
 	{
+		slot->fd = -1;
+
 		cmp.handle = file_handle_from_slot(table, slot);
 
-		int flags = io_convert_open_flags(req->openFlags);
+		slot->rights = req->open.rights;
-
-		//TODO: allow specifying access
-		mode_t mode = S_IRUSR
-		            | S_IWUSR
-		            | S_IRGRP
-		            | S_IWGRP
-		            | S_IROTH
-		            | S_IWOTH;
-
-		//NOTE: open
-		mem_arena_scope scratch = mem_scratch_begin();
-
-		str8 path = str8_from_buffer(req->size, req->buffer);
-
-
-		//TODO: if FILE_OPEN_RESTRICT, do the file traversal to check that path is in the
-		//      subtree rooted at atSlot->fd
-		slot->fd = -1;
 		if(atSlot)
 		{
-			if(path.len && path.ptr[0] == '/')
+			slot->rights &= atSlot->rights;
-			{
+		}
-				//NOTE: if path is absolute, change for a relative one, otherwise openat ignores fd.
-				str8_list list = {0};
-				str8_list_push(scratch.arena, &list, STR8("."));
-				str8_list_push(scratch.arena, &list, path);
-				path = str8_list_join(scratch.arena, list);
-			}
-			char* pathCStr = str8_to_cstring(scratch.arena, path);
 
-			if(req->openFlags & FILE_OPEN_RESTRICT)
+		if(slot->rights != req->open.rights)
+		{
+			slot->error = IO_ERR_PERM;
+			slot->fatal = true;
+		}
+		else
+		{
+			int flags = io_convert_access_rights(slot->rights);
+			flags |= io_convert_open_flags(req->open.flags);
+
+			mode_t mode = S_IRUSR
+		            	| S_IWUSR
+		            	| S_IRGRP
+		            	| S_IWGRP
+		            	| S_IROTH
+		            	| S_IWOTH;
+
+			mem_arena_scope scratch = mem_scratch_begin();
+			str8 path = str8_from_buffer(req->size, req->buffer);
+
+			slot->fd = -1;
+			if(atSlot)
 			{
-				io_open_restrict_result res = io_open_restrict(atSlot->fd, path, flags, mode);
+				if(path.len && path.ptr[0] == '/')
-				if(res.error == IO_OK)
 				{
-					slot->fd = res.fd;
+					//NOTE: if path is absolute, change for a relative one, otherwise openat ignores fd.
+					str8_list list = {0};
+					str8_list_push(scratch.arena, &list, STR8("."));
+					str8_list_push(scratch.arena, &list, path);
+					path = str8_list_join(scratch.arena, list);
+				}
+				char* pathCStr = str8_to_cstring(scratch.arena, path);
+
+				if(req->open.flags & FILE_OPEN_RESTRICT)
+				{
+					io_open_restrict_result res = io_open_restrict(atSlot->fd, path, flags, mode);
+					if(res.error == IO_OK)
+					{
+						slot->fd = res.fd;
+					}
+					else
+					{
+						slot->fatal = true;
+						slot->error = res.error;
+					}
 				}
 				else
 				{
-					slot->fatal = true;
+					flags = io_update_dir_flags_at(atSlot->fd, pathCStr, flags);
-					slot->error = res.error;
+
+					slot->fd = openat(atSlot->fd, pathCStr, flags, mode);
+					if(slot->fd < 0)
+					{
+						slot->fatal = true;
+						slot->error = io_convert_errno(errno);
+					}
 				}
 			}
 			else
 			{
-				slot->fd = openat(atSlot->fd, pathCStr, flags, mode);
+				char* pathCStr = str8_to_cstring(scratch.arena, path);
+
+				flags = io_update_dir_flags_at(AT_FDCWD, pathCStr, flags);
+
+				slot->fd = open(pathCStr, flags, mode);
 				if(slot->fd < 0)
 				{
 					slot->fatal = true;
 					slot->error = io_convert_errno(errno);
 				}
 			}
+			mem_scratch_end(scratch);
 		}
-		else
-		{
-			char* pathCStr = str8_to_cstring(scratch.arena, path);
-			slot->fd = open(pathCStr, flags, mode);
-			if(slot->fd < 0)
-			{
-				slot->fatal = true;
-				slot->error = io_convert_errno(errno);
-			}
-		}
-
-		mem_scratch_end(scratch);
 		cmp.error = slot->error;
 	}
 
@@ -552,6 +594,7 @@ io_cmp io_read(file_slot* slot, io_req* req)
 	if(cmp.result < 0)
 	{
 		slot->error = io_convert_errno(errno);
+		cmp.result = 0;
 		cmp.error = slot->error;
 	}
 
@@ -567,6 +610,7 @@ io_cmp io_write(file_slot* slot, io_req* req)
 	if(cmp.result < 0)
 	{
 		slot->error = io_convert_errno(errno);
+		cmp.result = 0;
 		cmp.error = slot->error;
 	}
 

test/files/main.c

@@ -13,7 +13,7 @@ int test_write(mem_arena* arena, str8 path, str8 test_string)
 {
 	log_info("writing\n");
 
-	file_handle f = file_open(path, FILE_OPEN_CREATE|FILE_OPEN_WRITE|FILE_OPEN_TRUNCATE);
+	file_handle f = file_open(path, FILE_ACCESS_WRITE, FILE_OPEN_CREATE|FILE_OPEN_TRUNCATE);
 	if(file_last_error(f))
 	{
 		log_error("Can't create/open file %.*s for writing\n", (int)path.len, path.ptr);
@@ -52,7 +52,7 @@ int test_read(mem_arena* arena, str8 path, str8 test_string)
 {
 	log_info("reading\n");
 
-	file_handle f = file_open(path, FILE_OPEN_READ);
+	file_handle f = file_open(path, FILE_ACCESS_READ, 0);
 	if(file_last_error(f))
 	{
 		log_error("Can't open file %.*s for reading\n", (int)path.len, path.ptr);
@@ -81,7 +81,7 @@ int test_stat_size(str8 path, u64 size)
 {
 	log_info("stat size\n");
 
-	file_handle f = file_open(path, 0);
+	file_handle f = file_open(path, 0, 0);
 	if(file_last_error(f))
 	{
 		log_error("Can't open file\n");
@@ -119,7 +119,7 @@ int test_stat_type(mem_arena* arena, str8 dataDir)
 
 	log_info("stat type, regular\n");
 
-	file_handle f = file_open(regular, 0);
+	file_handle f = file_open(regular, 0, 0);
 	if(file_last_error(f))
 	{
 		log_error("Can't open file\n");
@@ -141,7 +141,7 @@ int test_stat_type(mem_arena* arena, str8 dataDir)
 
 	log_info("stat type, directory\n");
 
-	f = file_open(dir, 0);
+	f = file_open(dir, 0, 0);
 	if(file_last_error(f))
 	{
 		log_error("Can't open file\n");
@@ -163,7 +163,7 @@ int test_stat_type(mem_arena* arena, str8 dataDir)
 
 	log_info("stat type, symlink\n");
 
-	f = file_open(link, FILE_OPEN_SYMLINK);
+	f = file_open(link, FILE_ACCESS_NONE, FILE_OPEN_SYMLINK);
 	if(file_last_error(f))
 	{
 		log_error("Can't open file\n");
@@ -190,7 +190,7 @@ int test_jail()
 {
 	log_info("test jail\n");
 
-	file_handle jail = file_open(STR8("./data/jail"), FILE_OPEN_READ);
+	file_handle jail = file_open(STR8("./data/jail"), FILE_ACCESS_READ, 0);
 	if(file_last_error(jail))
 	{
 		log_error("Can't open jail directory\n");
@@ -198,7 +198,7 @@ int test_jail()
 	}
 
 	// Check legitimates open
-	file_handle f = file_open_at(jail, STR8("/test.txt"), FILE_OPEN_READ | FILE_OPEN_RESTRICT);
+	file_handle f = file_open_at(jail, STR8("/test.txt"), FILE_ACCESS_READ, FILE_OPEN_RESTRICT);
 	if(file_last_error(f) != IO_OK)
 	{
 		log_error("Can't open jail/test.txt\n");
@@ -206,7 +206,7 @@ int test_jail()
 	}
 	file_close(f);
 
-	f = file_open_at(jail, STR8("/dir1/../test.txt"), FILE_OPEN_READ | FILE_OPEN_RESTRICT);
+	f = file_open_at(jail, STR8("/dir1/../test.txt"), FILE_ACCESS_READ, FILE_OPEN_RESTRICT);
 	if(file_last_error(f) != IO_OK)
 	{
 		log_error("Can't open jail/dir1/../test.txt\n");
@@ -215,7 +215,7 @@ int test_jail()
 	file_close(f);
 
 	// Check escapes
-	f = file_open_at(jail, STR8(".."), FILE_OPEN_READ | FILE_OPEN_RESTRICT);
+	f = file_open_at(jail, STR8(".."), FILE_ACCESS_READ, FILE_OPEN_RESTRICT);
 	if(file_last_error(f) != IO_ERR_WALKOUT)
 	{
 		log_error("Escaped jail with relative path ..\n");
@@ -223,7 +223,7 @@ int test_jail()
 	}
 	file_close(f);
 
-	f = file_open_at(jail, STR8(".."), FILE_OPEN_READ | FILE_OPEN_RESTRICT);
+	f = file_open_at(jail, STR8(".."), FILE_ACCESS_READ, FILE_OPEN_RESTRICT);
 	if(file_last_error(f) != IO_ERR_WALKOUT)
 	{
 		log_error("Escaped jail with relative path dir1/../..\n");
@@ -231,7 +231,7 @@ int test_jail()
 	}
 	file_close(f);
 
-	f = file_open_at(jail, STR8("/escape"), FILE_OPEN_READ | FILE_OPEN_RESTRICT);
+	f = file_open_at(jail, STR8("/escape"), FILE_ACCESS_READ, FILE_OPEN_RESTRICT);
 	if(file_last_error(f) != IO_ERR_WALKOUT)
 	{
 		log_error("Escaped jail with symlink\n");
@@ -242,6 +242,152 @@ int test_jail()
 	return(0);
 }
 
+int test_rights(mem_arena* arena, str8 dirPath)
+{
+	log_info("test rights\n");
+
+	//--------------------------------------------------------------------------------------
+	// base dir with no access
+	//--------------------------------------------------------------------------------------
+	{
+		file_handle dir = file_open(dirPath, FILE_ACCESS_NONE, 0);
+		if(file_last_error(dir))
+		{
+			log_error("Couldn't open ./dir1 with no access rights\n");
+			return(-1);
+		}
+
+		file_handle f = file_open_at(dir, STR8("./regular.txt"), FILE_ACCESS_READ, 0);
+		if(file_last_error(f) != IO_ERR_PERM)
+		{
+			log_error("Incorrect check when opening file with read access in dir with no access\n");
+			return(-1);
+		}
+		file_close(f);
+		file_close(dir);
+	}
+	//--------------------------------------------------------------------------------------
+	// base dir with read access
+	//--------------------------------------------------------------------------------------
+	{
+		file_handle dir = file_open(dirPath, FILE_ACCESS_READ, 0);
+		if(file_last_error(dir))
+		{
+			log_error("Couldn't open ./dir1 with read rights\n");
+			return(-1);
+		}
+
+		// check that we _can't_ open a file with write access
+		file_handle f = file_open_at(dir, STR8("./regular.txt"), FILE_ACCESS_WRITE, 0);
+		if(file_last_error(f) != IO_ERR_PERM)
+		{
+			log_error("Incorrect check when opening file with write access in dir with read access\n");
+			return(-1);
+		}
+		file_close(f);
+
+		// check that we _can_ open a file with read access
+		f = file_open_at(dir, STR8("./regular.txt"), FILE_ACCESS_READ, 0);
+		if(file_last_error(f))
+		{
+			log_error("Couldn't open file with read access in dir with read access\n");
+			return(-1);
+		}
+
+		// check that we _can't_ write to that file
+		str8 testWrite = STR8("Hello, world!\n");
+		if(file_write(f, testWrite.len, testWrite.ptr) != 0)
+		{
+			log_error("Incorrectly wrote to read-only file\n");
+			return(-1);
+		}
+		if(file_last_error(f) != IO_ERR_PERM)
+		{
+			log_error("Incorrect error returned from writing to read-only file\n");
+			return(-1);
+		}
+
+		file_close(f);
+		file_close(dir);
+	}
+	//--------------------------------------------------------------------------------------
+	// base dir with write access
+	//--------------------------------------------------------------------------------------
+	{
+		file_handle dir = file_open(dirPath, FILE_ACCESS_WRITE, 0);
+		if(file_last_error(dir))
+		{
+			log_error("Couldn't open ./dir1 with write rights\n");
+			return(-1);
+		}
+
+		// check that we _can't_ open a file with read access
+		file_handle f = file_open_at(dir, STR8("./regular.txt"), FILE_ACCESS_READ, 0);
+		if(file_last_error(f) != IO_ERR_PERM)
+		{
+			log_error("Incorrect check when opening file with read access in dir with write access\n");
+			return(-1);
+		}
+		file_close(f);
+
+		// check that we _can_ open a file with write access
+		f = file_open_at(dir, STR8("./regular.txt"), FILE_ACCESS_WRITE, 0);
+		if(file_last_error(f))
+		{
+			log_error("Couldn't open file with write access in dir with write access\n");
+			return(-1);
+		}
+
+		// check that we _can't_ read that file
+		char testRead[512];
+		if(file_read(f, 512, testRead) != 0)
+		{
+			log_error("Incorrectly read write-only file\n");
+			return(-1);
+		}
+		if(file_last_error(f) != IO_ERR_PERM)
+		{
+			log_error("Incorrect error returned from reading write-only file\n");
+			return(-1);
+		}
+
+		file_close(f);
+		file_close(dir);
+	}
+	//--------------------------------------------------------------------------------------
+	// base dir with read/write access
+	//--------------------------------------------------------------------------------------
+	{
+		file_handle dir = file_open(dirPath, FILE_ACCESS_READ|FILE_ACCESS_WRITE, 0);
+		if(file_last_error(dir))
+		{
+			log_error("Couldn't open ./dir1 with read rights\n");
+			return(-1);
+		}
+
+		// check that we can open file with read access
+		file_handle f = file_open_at(dir, STR8("./regular.txt"), FILE_ACCESS_READ, 0);
+		if(file_last_error(f))
+		{
+			log_error("Incorrect check when opening file with read access in dir with read/write access\n");
+			return(-1);
+		}
+		file_close(f);
+
+		// check that we can open file with write access
+		f = file_open_at(dir, STR8("./regular.txt"), FILE_ACCESS_WRITE, 0);
+		if(file_last_error(f))
+		{
+			log_error("Couldn't open file with write access in dir with read/write access\n");
+			return(-1);
+		}
+
+		file_close(f);
+		file_close(dir);
+	}
+	return(0);
+}
+
 int main(int argc, char** argv)
 {
 	mem_arena* arena = mem_scratch();
@@ -255,6 +401,7 @@ int main(int argc, char** argv)
 	if(test_read(arena, path, test_string)) { return(-1); }
 	if(test_stat_size(path, test_string.len)) { return(-1); }
 	if(test_stat_type(arena, dataDir)) { return(-1); }
+	if(test_rights(arena, dataDir)) { return(-1); }
 	if(test_jail()) { return(-1); }
 
 	remove("./test.txt");