aboutgitcodelistschat:MatrixIRC
diff options
context:
space:
mode:
authorAlice Frosi <afrosi@redhat.com>2023-03-31 11:48:40 +0200
committerAlice Frosi <afrosi@redhat.com>2023-04-03 14:43:21 +0200
commit79aa938d899c451fed517005c22d00cb03f4bad2 (patch)
tree646d8bcbe37024bfdb4a832ef9f21eb7dab6e50b
parentb7350faf8e466184ac665730306c99f6612eb5fd (diff)
downloadseitan-79aa938d899c451fed517005c22d00cb03f4bad2.tar
seitan-79aa938d899c451fed517005c22d00cb03f4bad2.tar.gz
seitan-79aa938d899c451fed517005c22d00cb03f4bad2.tar.bz2
seitan-79aa938d899c451fed517005c22d00cb03f4bad2.tar.lz
seitan-79aa938d899c451fed517005c22d00cb03f4bad2.tar.xz
seitan-79aa938d899c451fed517005c22d00cb03f4bad2.tar.zst
seitan-79aa938d899c451fed517005c22d00cb03f4bad2.zip
filter: fix filter
An additional notification is need either when we jump from an instruction without arguments then at the end of the argument checks.
-rw-r--r--cooker/filter.c99
-rw-r--r--tests/unit/test_filter.c74
-rw-r--r--tests/unit/testutil.h1
-rw-r--r--tests/unit/util.c15
4 files changed, 133 insertions, 56 deletions
diff --git a/cooker/filter.c b/cooker/filter.c
index 717e525..350d6bc 100644
--- a/cooker/filter.c
+++ b/cooker/filter.c
@@ -172,43 +172,63 @@ static unsigned get_n_args_syscall(const struct syscall_entry *table)
return n;
}
-static unsigned int get_total_args_instr(const struct syscall_entry table[],
- unsigned int n_syscall)
+static unsigned int get_n_args_syscall_instr(const struct syscall_entry *table)
{
- unsigned i, k, j, n = 0, total_instr = 0;
- const struct syscall_entry *t = &table[0];
const struct bpf_call *entry;
-
- for (j = 0; j < n_syscall; j++) {
- t = &table[j];
- for (i = 0; i < t->count; i++) {
- entry = t->entry + i;
- n = 0;
- /* For every argument there are 2 instructions, one to
- * load the value and the second to evaluate the
- * argument
- */
- for (k = 0; k < 6; k++) {
- if (entry->check_arg[k])
- n += 2;
+ bool has_arg = false;
+ unsigned n = 0, total_instr = 0;
+
+ for (unsigned int i = 0; i < table->count; i++) {
+ entry = table->entry + i;
+ n = 0;
+
+ /* For every argument there are 2 instructions, one to
+ * load the value and the second to evaluate the
+ * argument
+ */
+ for (unsigned int k = 0; k < 6; k++) {
+ if (entry->check_arg[k]) {
+ n += 2;
}
- total_instr += n;
- /* If there is at least an arguments then there is an additional
- * instruction for the notification
- */
- if (n > 0)
- total_instr++;
+ }
+ total_instr += n;
+ if (n > 0) {
+ has_arg = true;
+ total_instr++;
}
}
+ if (has_arg)
+ total_instr++;
+
return total_instr;
}
+static unsigned int get_total_args_instr(const struct syscall_entry table[],
+ unsigned int n_syscall)
+{
+ unsigned i, n = 0;
+
+ for (i = 0; i < n_syscall; i++) {
+ n += get_n_args_syscall_instr(&table[i]);
+ }
+ return n;
+}
+
static bool check_args_syscall_entry(const struct bpf_call *entry){
return entry->check_arg[0] || entry->check_arg[1] ||
entry->check_arg[2] || entry->check_arg[3] ||
entry->check_arg[4] || entry->check_arg[5];
}
+static bool check_args_syscall(const struct syscall_entry *table)
+{
+ for (unsigned int i = 0; i < table->count; i++) {
+ if (check_args_syscall_entry(table->entry + i))
+ return true;
+ }
+ return false;
+}
+
unsigned int create_bpf_program_log(struct sock_filter filter[])
{
filter[0] = (struct sock_filter)BPF_STMT(
@@ -232,20 +252,22 @@ unsigned int create_bfp_program(struct syscall_entry table[],
unsigned int n_args, n_nodes;
unsigned int notify, accept;
unsigned int i, j, k, size;
- unsigned int next_offset;
+ unsigned int next_offset, offset;
unsigned int next_args_off;
int nodes[MAX_JUMPS];
create_lookup_nodes(nodes, n_syscall);
+ /* First 3 checks */
size = 3;
/* No nodes if there is a single syscall */
n_nodes = (1 << count_shift_right(n_syscall - 1)) - 1;
n_args = get_total_args_instr(table, n_syscall);
- accept = 2 + n_nodes + 2 * n_syscall + n_args + 1;
- notify = 2 + n_nodes + 2 * n_syscall + n_args + 2;
+ /* pre-check instruction + load syscall number (4 instructions) */
+ accept = 3 + n_nodes + n_syscall + n_args;
+ notify = accept + 1;
/* Pre */
/* cppcheck-suppress badBitmaskCheck */
@@ -274,8 +296,15 @@ unsigned int create_bfp_program(struct syscall_entry table[],
next_offset = n_syscall - 1;
/* Insert leaves */
for (i = 0; i < n_syscall; i++) {
- filter[size++] = (struct sock_filter)EQ(
- table[i].nr, next_offset, accept - size);
+ /* If the syscall doesn't have any arguments, jump directly to
+ * the notification
+ */
+ if (check_args_syscall(&table[i]))
+ offset = next_offset;
+ else
+ offset = notify - size - 1;
+ filter[size++] = (struct sock_filter)EQ(table[i].nr, offset,
+ accept - size);
next_offset += get_n_args_syscall(&table[i]);
}
@@ -285,11 +314,12 @@ unsigned int create_bfp_program(struct syscall_entry table[],
* check the following entry of the syscall and its arguments.
*/
for (i = 0; i < n_syscall; i++) {
+ bool has_arg = false;
for (j = 0; j < (table[i]).count; j++) {
unsigned n_checks = 0;
entry = table[i].entry + j;
next_args_off = get_n_args_syscall_entry(entry);
- for (k = 0; k < 6; k++)
+ for (k = 0; k < 6; k++) {
if (entry->check_arg[k]) {
filter[size++] = (struct sock_filter)
LOAD((offsetof(
@@ -299,18 +329,19 @@ unsigned int create_bfp_program(struct syscall_entry table[],
(table[i].entry + j)->args[k],
0, next_args_off - n_checks);
n_checks++;
+ has_arg = true;
}
- /* If there is at least an argument, it needs to notify
- * if all the arguments checks have passed.
- */
- if (check_args_syscall_entry(entry))
+ }
+ if (check_args_syscall_entry(table[i].entry))
filter[size++] = (struct sock_filter)JUMPA(
notify - size);
}
/* At this point none of the checks was positive, it jumps to
* the default behavior
*/
- filter[size++] = (struct sock_filter)JUMPA(accept - size);
+ if (has_arg)
+ filter[size++] =
+ (struct sock_filter)JUMPA(accept - size);
}
/* Seccomp accept and notify instruction */
diff --git a/tests/unit/test_filter.c b/tests/unit/test_filter.c
index 5ff9d65..9583b56 100644
--- a/tests/unit/test_filter.c
+++ b/tests/unit/test_filter.c
@@ -6,21 +6,9 @@
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
-#include <string.h>
-#include <sched.h>
-#include <unistd.h>
-#include <limits.h>
-#include <fcntl.h>
-#include <sys/prctl.h>
#include <sys/syscall.h>
-#include <sys/ioctl.h>
-#include <sys/wait.h>
-#include <linux/audit.h>
-#include <linux/filter.h>
-#include <linux/seccomp.h>
#include <sys/mman.h>
-#include <sys/un.h>
-#include <sys/socket.h>
+#include <sys/resource.h>
#include <check.h>
@@ -28,52 +16,94 @@
#include "common.h"
#include "testutil.h"
#include "filter.h"
+#include "disasm.h"
static int generate_install_filter(struct args_target *at)
{
- struct bpf_call calls[] = { {} };
+ unsigned int i;
+ struct bpf_call calls[1];
struct syscall_entry table[] = {
{ .count = 1, .nr = at->nr, .entry = &calls[0] }
};
struct sock_filter filter[30];
unsigned int size;
+ for (i = 0; i < 6; i++) {
+ if (at->args[i] != NULL) {
+ calls[0].args[i] = (int)at->args[i];
+ calls[0].check_arg[i] = true;
+ } else {
+ calls[0].check_arg[i] = false;
+ }
+ }
size = create_bfp_program(table, filter, 1);
+ //bpf_disasm_all(filter, size);
return install_filter(filter, size);
}
-void setup_build_filter()
+START_TEST(no_args)
{
at = mmap(NULL, sizeof(struct args_target), PROT_READ | PROT_WRITE,
MAP_SHARED | MAP_ANONYMOUS, -1, 0);
at->check_fd = false;
at->nr = __NR_getpid;
- at->args[0] = NULL;
at->install_filter = generate_install_filter;
setup();
+ mock_syscall_target();
}
+END_TEST
-START_TEST(filter)
+START_TEST(with_getsid)
{
- continue_target();
+ int id = 12345;
+ at = mmap(NULL, sizeof(struct args_target), PROT_READ | PROT_WRITE,
+ MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+ at->check_fd = false;
+ at->nr = __NR_getsid;
+ at->args[0] = &id;
+ at->install_filter = generate_install_filter;
+ setup();
+ mock_syscall_target();
}
END_TEST
+START_TEST(with_getpriority)
+{
+ int which = 12345;
+ id_t who = PRIO_PROCESS;
+ at = mmap(NULL, sizeof(struct args_target), PROT_READ | PROT_WRITE,
+ MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+ at->check_fd = false;
+ at->nr = __NR_getpriority;
+ at->args[0] = &which;
+ at->args[1] = &who;
+ at->install_filter = generate_install_filter;
+ setup();
+ mock_syscall_target();
+}
+END_TEST
Suite *op_call_suite(void)
{
Suite *s;
int timeout = 30;
- TCase *simple;
+ TCase *simple, *args32;
s = suite_create("Test filter with target");
- simple = tcase_create("simple");
- tcase_add_checked_fixture(simple, setup_build_filter, teardown);
+ simple = tcase_create("no args");
+ tcase_add_checked_fixture(simple, NULL, teardown);
tcase_set_timeout(simple, timeout);
- tcase_add_test(simple, filter);
+ tcase_add_test(simple, no_args);
suite_add_tcase(s, simple);
+ args32 = tcase_create("with args 32 bit");
+ tcase_add_checked_fixture(args32, NULL, teardown);
+ tcase_set_timeout(args32, timeout);
+ tcase_add_test(args32, with_getsid);
+ tcase_add_test(args32, with_getpriority);
+ suite_add_tcase(s, args32);
+
return s;
}
diff --git a/tests/unit/testutil.h b/tests/unit/testutil.h
index d4f83af..dd4f1e9 100644
--- a/tests/unit/testutil.h
+++ b/tests/unit/testutil.h
@@ -42,5 +42,6 @@ void setup();
void teardown();
int install_notification_filter(struct args_target *at);
void continue_target();
+void mock_syscall_target();
#endif /* TESTUTIL_H */
diff --git a/tests/unit/util.c b/tests/unit/util.c
index c6fc3fb..5a1c5aa 100644
--- a/tests/unit/util.c
+++ b/tests/unit/util.c
@@ -170,6 +170,21 @@ void continue_target()
ck_assert_msg(ret == 0, strerror(errno));
}
+void mock_syscall_target()
+{
+ struct seccomp_notif_resp resp;
+ int ret;
+
+ ret = ioctl(notifyfd, SECCOMP_IOCTL_NOTIF_ID_VALID, &req.id);
+ ck_assert_msg(ret == 0, strerror(errno));
+ resp.id = req.id;
+ resp.flags = 0;
+ resp.error = 0;
+ resp.val = 0;
+ ret = ioctl(notifyfd, SECCOMP_IOCTL_NOTIF_SEND, &resp);
+ ck_assert_msg(ret == 0, strerror(errno));
+}
+
void setup()
{
int ret;