Use new qssb_append_*_syscall functions, remove old fields

The purpose of these new functions is to make it simpler for users
to add new syscalls to the whitelist and blacklist.

The current approach uses a user-supplied pointer which however
was difficult to manage with "no_fs", which may add systemcalls
to the blacklist. Then we must resize arrays, and suddenly
it's our job to free them.

As a bonus, implementing them here allows easier data structure
changes and decreases the chances tgat users of this API
do something wrong, like forgetting -1 at then end, etc.

qssb.h

@@ -29,6 +29,8 @@
 #include <sys/mount.h>
 #include <sys/prctl.h>
 #include <sys/random.h>
+#include <sys/time.h>
+#include <sys/resource.h>
 #include <stdarg.h>
 #include <stdlib.h>
 #include <string.h>
@@ -155,7 +157,28 @@ static int default_blacklisted_syscals[] = {
 	QSSB_SYS(init_module),
 	QSSB_SYS(finit_module),
 	QSSB_SYS(delete_module),
-	-1
+};
+
+/* TODO: Check for completion
+ * Known blacklisting problem (catch up game, etc.)
+ *
+ * However, we use it to enhance "no_fs" policy, which does not solely rely
+ * on seccomp anyway */
+static int fs_access_syscalls[] = {
+	QSSB_SYS(chdir),
+	QSSB_SYS(truncate),
+	QSSB_SYS(stat),
+	QSSB_SYS(flock),
+	QSSB_SYS(chmod),
+	QSSB_SYS(chown),
+	QSSB_SYS(setxattr),
+	QSSB_SYS(utime),
+	QSSB_SYS(ioctl),
+	QSSB_SYS(fcntl),
+	QSSB_SYS(access),
+	QSSB_SYS(open),
+	QSSB_SYS(openat),
+	QSSB_SYS(unlink),
 };
 
 struct qssb_path_policy
@@ -165,6 +188,16 @@ struct qssb_path_policy
 	struct qssb_path_policy *next;
 };
 
+struct qssb_allocated_entry
+{
+	void *data; /* the actual data */
+	size_t size; /* number of bytes allocated for size */
+	size_t used; /* number of bytes in use */
+};
+
+/* Number of bytes to grow the buffer in qssb_allocated_entry  with */
+#define QSSB_ENTRY_ALLOC_SIZE 32
+
 
 /* Policy tells qssb what to do */
 struct qssb_policy
@@ -173,36 +206,106 @@ struct qssb_policy
 	int preserve_cwd;
 	int not_dumpable;
 	int no_new_privs;
+	int no_fs;
+	int no_new_fds;
 	int namespace_options;
 	/* Bind mounts all paths in path_policies into the chroot and applies
 	 non-landlock policies */
 	int mount_path_policies_to_chroot;
-	int *blacklisted_syscalls;
-	int *whitelisted_syscalls;
 	char chroot_target_path[PATH_MAX];
 	const char *chdir_path;
 
 	/* Do not manually add policies here, use qssb_append_path_polic*() */
 	struct qssb_path_policy *path_policies;
 	struct qssb_path_policy **path_policies_tail;
+
+	/* Do not manually add entries here, use qssb_append_denied_syscall() etc. */
+	struct qssb_allocated_entry denied_syscalls;
+	struct qssb_allocated_entry allowed_syscalls;
+
 };
 
+static int qssb_entry_append(struct qssb_allocated_entry *entry, void *data, size_t bytes)
+{
+	size_t remaining = entry->size - entry->used;
+	if(remaining < bytes)
+	{
+		size_t expandval = QSSB_ENTRY_ALLOC_SIZE > bytes ? QSSB_ENTRY_ALLOC_SIZE : bytes;
+		size_t sizenew = entry->size + expandval;
+		int *datanew = (int *) realloc(entry->data, sizenew);
+		if(datanew == NULL)
+		{
+			QSSB_LOG_ERROR("failed to resize array: %s\n", strerror(errno));
+			return -1;
+		}
+		entry->size = sizenew;
+		entry->data = datanew;
+	}
+	uint8_t *target = (uint8_t *) entry->data;
+	memcpy(target + entry->used, data, bytes);
+	entry->used = entry->used + bytes;
+	return 0;
+}
+
+static int qssb_append_syscall(struct qssb_allocated_entry *entry, int *syscalls, size_t n)
+{
+	return qssb_entry_append(entry, syscalls, n * sizeof(int));
+}
+
+
+int qssb_append_denied_syscall(struct qssb_policy *qssb_policy, int syscall)
+{
+	return qssb_append_syscall(&qssb_policy->denied_syscalls, &syscall, 1);
+}
+
+int qssb_append_allowed_syscall(struct qssb_policy *qssb_policy, int syscall)
+{
+	return qssb_append_syscall(&qssb_policy->allowed_syscalls, &syscall, 1);
+}
+
+int qssb_append_allowed_syscalls(struct qssb_policy *qssb_policy, int *syscalls, size_t n)
+{
+
+	return qssb_append_syscall(&qssb_policy->allowed_syscalls, syscalls, n);
+}
+
+int qssb_append_denied_syscalls(struct qssb_policy *qssb_policy, int *syscalls, size_t n)
+{
+
+	return qssb_append_syscall(&qssb_policy->denied_syscalls, syscalls, n);
+}
+
 /* Creates the default policy
  * Must be freed using qssb_free_policy
  * @returns: default policy */
 struct qssb_policy *qssb_init_policy()
 {
 	struct qssb_policy *result = (struct qssb_policy *) calloc(1, sizeof(struct qssb_policy));
-	result->blacklisted_syscalls = default_blacklisted_syscals;
 	result->drop_caps = 1;
 	result->not_dumpable = 1;
 	result->no_new_privs = 1;
+	result->no_fs = 0;
+	result->no_new_fds = 0;
 	result->namespace_options = QSSB_UNSHARE_MOUNT | QSSB_UNSHARE_USER;
 	result->chdir_path = NULL;
 	result->mount_path_policies_to_chroot = 0;
 	result->chroot_target_path[0] = '\0';
 	result->path_policies = NULL;
 	result->path_policies_tail = &(result->path_policies);
+	result->allowed_syscalls.data = NULL;
+	result->allowed_syscalls.size = 0;
+	result->allowed_syscalls.used = 0;
+	result->denied_syscalls.data = NULL;
+	result->denied_syscalls.size = 0;
+	result->denied_syscalls.used = 0;
+
+	size_t blacklisted_syscalls_count = sizeof(default_blacklisted_syscals)/sizeof(default_blacklisted_syscals[0]);
+
+	int appendresult = qssb_append_denied_syscalls(result, default_blacklisted_syscals, blacklisted_syscalls_count);
+	if(appendresult != 0)
+	{
+		return NULL;
+	}
 	return result;
 }
 
@@ -526,13 +629,13 @@ static int drop_caps()
 /*
  * Enables the per_syscall seccomp action for system calls
  *
- * syscalls: array of system calls numbers. -1 must be the last entry.
+ * syscalls: array of system calls numbers.
  * per_syscall: action to apply for each system call
  * default_action: the default action at the end
  *
  * @returns: 0 on success, -1 on error
  */
-static int seccomp_enable(int *syscalls, unsigned int per_syscall, unsigned int default_action)
+static int seccomp_enable(int *syscalls, size_t n, unsigned int per_syscall, unsigned int default_action)
 {
 	struct sock_filter filter[1024] =
 	{
@@ -540,15 +643,13 @@ static int seccomp_enable(int *syscalls, unsigned int per_syscall, unsigned int
 	};
 
 	unsigned short int current_filter_index = 1;
-	while(*syscalls >= 0)
+	for(size_t i = 0; i < n; i++)
 	{
-		unsigned int sysc = (unsigned int) *syscalls;
+		unsigned int sysc = (unsigned int) syscalls[i];
 		struct sock_filter syscall = BPF_JUMP(BPF_JMP+BPF_JEQ+BPF_K, sysc, 0, 1);
 		struct sock_filter action = BPF_STMT(BPF_RET+BPF_K, per_syscall);
 		filter[current_filter_index++] = syscall;
 		filter[current_filter_index++] = action;
-
-		++syscalls;
 	}
 
 	struct sock_filter da = BPF_STMT(BPF_RET+BPF_K, default_action);
@@ -572,21 +673,21 @@ static int seccomp_enable(int *syscalls, unsigned int per_syscall, unsigned int
 /*
  * Blacklists the specified systemcalls.
  *
- * syscalls: array of system calls numbers. -1 must be the last entry.
+ * syscalls: array of system calls numbers.
  */
-static int seccomp_enable_blacklist(int *syscalls)
+static int seccomp_enable_blacklist(int *syscalls, size_t n)
 {
-	return seccomp_enable(syscalls, SECCOMP_RET_KILL, SECCOMP_RET_ALLOW);
+	return seccomp_enable(syscalls, n, SECCOMP_RET_KILL, SECCOMP_RET_ALLOW);
 }
 
 /*
  * Whitelists the specified systemcalls.
  *
- * syscalls: array of system calls numbers. -1 must be the last entry.
+ * syscalls: array of system calls numbers.
  */
-static int seccomp_enable_whitelist(int *syscalls)
+static int seccomp_enable_whitelist(int *syscalls, size_t n)
 {
-	return seccomp_enable(syscalls, SECCOMP_RET_ALLOW, SECCOMP_RET_KILL);
+	return seccomp_enable(syscalls, n, SECCOMP_RET_ALLOW, SECCOMP_RET_KILL);
 }
 
 #if HAVE_LANDLOCK == 1
@@ -708,9 +809,9 @@ static int landlock_prepare_ruleset(struct qssb_path_policy *policies)
 /* Checks for illogical or dangerous combinations */
 static int check_policy_sanity(struct qssb_policy *policy)
 {
-	if(policy->blacklisted_syscalls != NULL && policy->whitelisted_syscalls != NULL)
+	if(policy->denied_syscalls.used > 0 && policy->allowed_syscalls.used > 0)
 	{
-		QSSB_LOG_ERROR("Error: Cannot mix blacklisted and whitelisted systemcalls\n");
+		QSSB_LOG_ERROR("Error: Cannot mix allowed and denied systemcalls in policy\n");
 		return -EINVAL;
 	}
 
@@ -730,23 +831,86 @@ static int check_policy_sanity(struct qssb_policy *policy)
 
 	if(policy->no_new_privs != 1)
 	{
-		if(policy->blacklisted_syscalls != NULL || policy->whitelisted_syscalls != NULL)
+		if(policy->allowed_syscalls.used > 0 || policy->denied_syscalls.used > 0)
 		{
 			QSSB_LOG_ERROR("no_new_privs = 1 is required for seccomp filtering!\n");
 			return -1;
 		}
 	}
 
-	if(policy->path_policies != NULL && policy->mount_path_policies_to_chroot != 1)
+	if(policy->path_policies != NULL)
 	{
-		#if HAVE_LANDLOCK != 1
-			QSSB_LOG_ERROR("Path policies cannot be enforced! System needs landlock support or set mount_path_policies_to_chroot = 1\n");
+
+		if(policy->mount_path_policies_to_chroot != 1)
+		{
+			#if HAVE_LANDLOCK != 1
+				QSSB_LOG_ERROR("Path policies cannot be enforced! System needs landlock support or set mount_path_policies_to_chroot = 1\n");
+				return -1;
+			#endif
+		}
+		if(policy->no_fs == 1)
+		{
+			QSSB_LOG_ERROR("If path_policies are specified, no_fs cannot be set to 1");
 			return -1;
-		#endif
+		}
 	}
 	return 0;
 }
 
+static void close_file_fds()
+{
+	long max_files = sysconf(_SC_OPEN_MAX);
+	for(long i = 3; i <= max_files; i++)
+	{
+		close((int)i);
+	}
+}
+
+/* Takes away file system access from the process
+ *
+ * We use this when "no_fs" is given in the policy.
+ *
+ * This is useful for restricted subprocesses that do some computational work
+ * and do not require filesystem access
+ *
+ * @returns: 0 on success, < 0 on error
+ */
+static int enable_no_fs(struct qssb_policy *policy)
+{
+		close_file_fds();
+
+		if(chdir("/proc/self/fdinfo") != 0)
+		{
+			QSSB_LOG_ERROR("Failed to change to safe directory: %s\n", strerror(errno));
+			return -1;
+		}
+
+		if(chroot(".") != 0)
+		{
+			QSSB_LOG_ERROR("Failed to chroot into safe directory: %s\n", strerror(errno));
+			return -1;
+		}
+
+		if(chdir("/") != 0)
+		{
+			QSSB_LOG_ERROR("Failed to chdir into safe directory inside chroot: %s\n", strerror(errno));
+			return -1;
+		}
+
+		if(policy->allowed_syscalls.used == 0)
+		{
+			size_t fs_access_syscalls_count = sizeof(fs_access_syscalls)/sizeof(fs_access_syscalls[0]);
+
+			int ret = qssb_append_denied_syscalls(policy, fs_access_syscalls, fs_access_syscalls_count);
+			if(ret != 0)
+			{
+				QSSB_LOG_ERROR("Failed to add system calls to blacklist\n");
+				return -1;
+			}
+		}
+		return 0;
+}
+
 /* Enables the specified qssb_policy.
  *
  * This function is not atomic (and can't be). This means some
@@ -827,7 +991,6 @@ int qssb_enable_policy(struct qssb_policy *policy)
 		}
 	}
 #endif
-
 	if(policy->chdir_path == NULL)
 	{
 		policy->chdir_path = "/";
@@ -839,6 +1002,25 @@ int qssb_enable_policy(struct qssb_policy *policy)
 		return -1;
 	}
 
+	if(policy->no_fs)
+	{
+		if(enable_no_fs(policy) != 0)
+		{
+			QSSB_LOG_ERROR("Failed to take away filesystem access of process\n");
+			return -1;
+		}
+	}
+
+	if(policy->no_new_fds)
+	{
+		const struct rlimit nofile = {0, 0};
+		if (setrlimit(RLIMIT_NOFILE, &nofile) == -1)
+		{
+			QSSB_LOG_ERROR("setrlimit: Failed to set rlimit: %s\n", strerror(errno));
+			return -1;
+		}
+	}
+
 	if(policy->drop_caps)
 	{
 		if(drop_caps() < 0)
@@ -876,18 +1058,22 @@ int qssb_enable_policy(struct qssb_policy *policy)
 	close(landlock_ruleset_fd);
 #endif
 
-	if(policy->whitelisted_syscalls != NULL)
+	if(policy->allowed_syscalls.used > 0)
 	{
-		if(seccomp_enable_whitelist(policy->whitelisted_syscalls) <0)
+		int *syscalls = (int *)policy->allowed_syscalls.data;
+		size_t n = policy->allowed_syscalls.used / sizeof(int);
+		if(seccomp_enable_whitelist(syscalls, n) < 0)
 		{
 			QSSB_LOG_ERROR("seccomp_enable_whitelist failed\n");
 			return -1;
 		}
 	}
 
-	if(policy->blacklisted_syscalls != NULL)
+	if(policy->denied_syscalls.used > 0)
 	{
-		if(seccomp_enable_blacklist(policy->blacklisted_syscalls) <0)
+		int *syscalls = (int *)policy->denied_syscalls.data;
+		size_t n = policy->denied_syscalls.used / sizeof(int);
+		if(seccomp_enable_blacklist(syscalls, n) < 0)
 		{
 			QSSB_LOG_ERROR("seccomp_enable_blacklist failed\n");
 			return -1;

test.c

@@ -1,5 +1,10 @@
 #include "qssb.h"
 #include <stdbool.h>
+#include <sys/types.h>
+#include <dirent.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+
 int test_default_main(int argc, char *argv[])
 {
 	struct qssb_policy *policy = qssb_init_policy();
@@ -10,9 +15,11 @@ int test_default_main(int argc, char *argv[])
 int test_both_syscalls(int argc, char *argv[])
 {
 	struct qssb_policy *policy = qssb_init_policy();
-	int bla[] = { 1,2,3};
-	policy->blacklisted_syscalls = &bla;
-	policy->whitelisted_syscalls = &bla;
+	int syscalls[] = {1,2,3};
+
+	qssb_append_denied_syscalls(policy, syscalls, 3);
+	qssb_append_allowed_syscalls(policy, syscalls, 3);
+
 	int ret = qssb_enable_policy(policy);
 	if(ret != 0)
 	{
@@ -24,8 +31,9 @@ int test_both_syscalls(int argc, char *argv[])
 int test_seccomp_blacklisted(int argc, char *argv[])
 {
 	struct qssb_policy *policy = qssb_init_policy();
-	int blacklisted[] = { QSSB_SYS(getuid) };
-	policy->blacklisted_syscalls = blacklisted;
+
+	qssb_append_denied_syscall(policy, QSSB_SYS(getuid));
+
 	int ret = qssb_enable_policy(policy);
 	uid_t pid = geteuid();
 	pid = getuid();
@@ -35,8 +43,9 @@ int test_seccomp_blacklisted(int argc, char *argv[])
 int test_seccomp_blacklisted_call_permitted(int argc, char *argv[])
 {
 	struct qssb_policy *policy = qssb_init_policy();
-	int blacklisted[] = { QSSB_SYS(getuid) };
-	policy->blacklisted_syscalls = blacklisted;
+
+	qssb_append_denied_syscall(policy, QSSB_SYS(getuid));
+
 	int ret = qssb_enable_policy(policy);
 	//geteuid is not blacklisted, so must succeed
 	uid_t pid = geteuid();
@@ -69,6 +78,65 @@ int test_landlock_deny_write(int argc, char *argv[])
 	return 1;
 }
 
+int test_nofs(int argc, char *argv[])
+{
+	struct qssb_policy *policy = qssb_init_policy();
+	policy->no_fs = 1;
+
+	int ret = qssb_enable_policy(policy);
+	if(ret != 0)
+	{
+		fprintf(stderr, "Failed to activate nofs sandbox\n");
+		return -1;
+	}
+
+	int s = socket(AF_INET,SOCK_STREAM,0);
+	if(s == -1)
+	{
+		fprintf(stderr, "Failed to open socket but this was not requested by policy\n");
+		return 0;
+	}
+
+	/* Expect seccomp to take care of this */
+	if(open("/test", O_CREAT | O_WRONLY) >= 0)
+	{
+		fprintf(stderr, "Failed: Do not expect write access\n");
+		return -1;
+	}
+
+	return 0;
+}
+
+
+int test_no_new_fds(int argc, char *argv[])
+{
+	struct qssb_policy *policy = qssb_init_policy();
+	policy->no_new_fds = 1;
+
+	int ret = qssb_enable_policy(policy);
+	if(ret != 0)
+	{
+		fprintf(stderr, "Failed to activate no_new_fd sandbox\n");
+		return -1;
+	}
+
+	if(open("/tmp/test", O_CREAT | O_WRONLY) >= 0)
+	{
+		fprintf(stderr, "Failed: Could open new file descriptor\n");
+		return -1;
+	}
+
+	int s = socket(AF_INET,SOCK_STREAM,0);
+	if(s >= 0)
+	{
+		fprintf(stderr, "Failed: socket got opened but policy denied\n");
+		return -1;
+	}
+
+	return 0;
+
+}
+
 struct dispatcher
 {
 	char *name;
@@ -81,7 +149,9 @@ struct dispatcher dispatchers[] = {
 	{ "seccomp-blacklisted", &test_seccomp_blacklisted, false },
 	{ "seccomp-blacklisted-permitted", &test_seccomp_blacklisted_call_permitted, true },
 	{ "landlock", &test_landlock, true },
-	{ "landlock-deny-write", &test_landlock_deny_write, true }
+	{ "landlock-deny-write", &test_landlock_deny_write, true },
+	{ "no_fs", &test_nofs, false},
+	{ "no_new_fds", &test_no_new_fds, true}
 };
 
 int main(int argc, char *argv[])