Minijail: add logging for seccomp filter failures.

BUG=chromium-os:33361
TEST=unit tests
TEST=security_Minijail0, security_Minijail_seccomp, platform_CrosDisksArchive

Change-Id: I16cdb8fbcf1cb13f2dee5521f97fb8d0bdbdf93b
Reviewed-on: https://gerrit.chromium.org/gerrit/29053
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Tested-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Commit-Ready: Jorge Lucangeli Obes <jorgelo@chromium.org>

12 files changed, 244 insertions, 31 deletions

diff --git a/Makefile b/Makefile
index ea7f8b1..80eae9e 100644
--- a/Makefile
+++ b/Makefile
@@ -11,11 +11,12 @@ all : minijail0 libminijail.so libminijailpreload.so
 
 tests : libminijail_unittest.wrapper syscall_filter_unittest
 
-minijail0 : libsyscalls.gen.o libminijail.o syscall_filter.o bpf.o util.o \
-		minijail0.c
+minijail0 : libsyscalls.gen.o libminijail.o syscall_filter.o \
+		signal.o bpf.o util.o minijail0.c
 	$(CC) $(CFLAGS) -o $@ $^ -lcap
 
-libminijail.so : libminijail.o syscall_filter.o bpf.o util.o libsyscalls.gen.o
+libminijail.so : libminijail.o syscall_filter.o signal.o bpf.o util.o \
+		libsyscalls.gen.o
 	$(CC) $(CFLAGS) -shared -o $@ $^ -lcap
 
 # Allow unittests to access what are normally internal symbols.
@@ -26,11 +27,11 @@ libminijail_unittest.wrapper :
 
 libminijail_unittest : CFLAGS := $(filter-out -fvisibility=%,$(CFLAGS))
 libminijail_unittest : libminijail_unittest.o libminijail.o \
-		syscall_filter.o bpf.o util.o libsyscalls.gen.o
+		syscall_filter.o signal.o bpf.o util.o libsyscalls.gen.o
 	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(filter-out $(CFLAGS_FILE),$^) -lcap
 
 libminijailpreload.so : libminijailpreload.c libminijail.o libsyscalls.gen.o \
-		syscall_filter.o bpf.o util.o
+		syscall_filter.o signal.o bpf.o util.o
 	$(CC) $(CFLAGS) -shared -o $@ $^ -ldl -lcap
 
 libminijail.o : libminijail.c libminijail.h
@@ -49,6 +50,8 @@ syscall_filter_unittest.o : syscall_filter_unittest.c test_harness.h
 
 syscall_filter.o : syscall_filter.c syscall_filter.h
 
+signal.o : signal.c signal.h
+
 bpf.o : bpf.c bpf.h
 
 util.o : util.c util.h
@@ -101,5 +104,5 @@ clean : test-clean
 	@rm -f libminijail.so
 	@rm -f libminijail_unittest
 	@rm -f libsyscalls.gen.c
-	@rm -f syscall_filter.o bpf.o util.o
+	@rm -f syscall_filter.o signal.o bpf.o util.o
 	@rm -f syscall_filter_unittest syscall_filter_unittest.o
diff --git a/bpf.h b/bpf.h
index 04ece14..5b41b55 100644
--- a/bpf.h
+++ b/bpf.h
@@ -59,7 +59,7 @@ struct seccomp_data {
 #define ARCH_NR	AUDIT_ARCH_X86_64
 #elif defined(__arm__)
 /*
- * <linux/audit.h> includes <linux/elf-em.h>, which does not include EM_ARM.
+ * <linux/audit.h> includes <linux/elf-em.h>, which does not define EM_ARM.
  * <linux/elf.h> only includes <asm/elf.h> if we're in the kernel.
  */
 # ifndef EM_ARM
@@ -147,6 +147,9 @@ inline size_t set_bpf_instr(struct sock_filter *instr,
 #define set_bpf_ret_kill(_block) \
 	set_bpf_stmt((_block), BPF_RET+BPF_K, SECCOMP_RET_KILL)
 
+#define set_bpf_ret_trap(_block) \
+	set_bpf_stmt((_block), BPF_RET+BPF_K, SECCOMP_RET_TRAP)
+
 #define set_bpf_ret_errno(_block, _errno) \
 	set_bpf_stmt((_block), BPF_RET+BPF_K, \
 		SECCOMP_RET_ERRNO | ((_errno) & SECCOMP_RET_DATA))
diff --git a/libminijail.c b/libminijail.c
index 2c3d5b3..4da1f66 100644
--- a/libminijail.c
+++ b/libminijail.c
@@ -35,6 +35,7 @@
 #include "libminijail.h"
 #include "libminijail-private.h"
 
+#include "signal.h"
 #include "syscall_filter.h"
 #include "util.h"
 
@@ -71,6 +72,7 @@ struct minijail {
 		int ptrace:1;
 		int no_new_privs:1;
 		int seccomp_filter:1;
+		int log_seccomp_filter:1;
 		int chroot:1;
 	} flags;
 	uid_t uid;
@@ -185,6 +187,11 @@ void API minijail_use_seccomp_filter(struct minijail *j)
 	j->flags.seccomp_filter = 1;
 }
 
+void API minijail_log_seccomp_filter_failures(struct minijail *j)
+{
+	j->flags.log_seccomp_filter = 1;
+}
+
 void API minijail_use_caps(struct minijail *j, uint64_t capmask)
 {
 	j->caps = capmask;
@@ -278,8 +285,9 @@ void API minijail_parse_seccomp_filters(struct minijail *j, const char *path)
 	}
 
 	struct sock_fprog *fprog = malloc(sizeof(struct sock_fprog));
-	if (compile_filter(file, fprog)) {
-		die("failed to compile seccomp filter BPF program in '%s'", path);
+	if (compile_filter(file, fprog, j->flags.log_seccomp_filter)) {
+		die("failed to compile seccomp filter BPF program in '%s'",
+		    path);
 	}
 
 	j->filter_len = fprog->len;
@@ -334,7 +342,8 @@ void minijail_marshal_helper(struct marshal_state *state,
 	for (b = j->bindings_head; b; b = b->next) {
 		marshal_append(state, b->src, strlen(b->src) + 1);
 		marshal_append(state, b->dest, strlen(b->dest) + 1);
-		marshal_append(state, (char *)&b->writeable, sizeof(b->writeable));
+		marshal_append(state, (char *)&b->writeable,
+				sizeof(b->writeable));
 	}
 }
 
@@ -640,6 +649,16 @@ void API minijail_enter(const struct minijail *j)
 	}
 
 	/*
+	 * If we're logging seccomp filter failures,
+	 * install the SIGSYS handler first.
+	 */
+	if (j->flags.seccomp_filter && j->flags.log_seccomp_filter) {
+		if (install_sigsys_handler())
+			pdie("install SIGSYS handler");
+		warn("logging seccomp filter failures");
+	}
+
+	/*
 	 * Install seccomp filter before dropping root and caps.
 	 * WARNING: this means that filter policies *must* allow
 	 * setgroups()/setresgid()/setresuid() for dropping root and
diff --git a/libminijail.h b/libminijail.h
index 13f9ab4..d83dc21 100644
--- a/libminijail.h
+++ b/libminijail.h
@@ -46,6 +46,7 @@ void minijail_use_seccomp(struct minijail *j);
 void minijail_no_new_privs(struct minijail *j);
 void minijail_use_seccomp_filter(struct minijail *j);
 void minijail_parse_seccomp_filters(struct minijail *j, const char *path);
+void minijail_log_seccomp_filter_failures(struct minijail *j);
 void minijail_use_caps(struct minijail *j, uint64_t capmask);
 void minijail_namespace_vfs(struct minijail *j);
 /* Implies namespace_vfs and remount_readonly.
diff --git a/minijail0.c b/minijail0.c
index 9f2a765..9ab5195 100644
--- a/minijail0.c
+++ b/minijail0.c
@@ -11,6 +11,8 @@
 #include "libminijail.h"
 #include "libsyscalls.h"
 
+#include "util.h"
+
 static void set_user(struct minijail *j, const char *arg)
 {
 	char *end = NULL;
@@ -69,6 +71,8 @@ static void add_binding(struct minijail *j, char *arg) {
 
 static void usage(const char *progn)
 {
+	size_t i;
+
 	printf("Usage: %s [-Ghnprsv] [-b <src>,<dest>[,<writeable>]] "
 	       "[-c <caps>] [-C <dir>] [-g <group>] [-S <file>] [-u <user>] "
 	       "<program> [args...]\n"
@@ -80,14 +84,21 @@ static void usage(const char *progn)
 	       "  -g <group>: change gid to <group>\n"
 	       "  -h:         help (this message)\n"
 	       "  -H:         seccomp filter help message\n"
+	       "  -L:         log blocked syscalls when using seccomp filter. "
+	       "Forces the following syscalls to be allowed:\n"
+	       "              ", progn);
+	for (i = 0; i < log_syscalls_len; i++)
+		printf("%s ", log_syscalls[i]);
+
+	printf("\n"
 	       "  -n:         set no_new_privs\n"
 	       "  -p:         use pid namespace (implies -vr)\n"
 	       "  -r:         remount /proc readonly (implies -v)\n"
 	       "  -s:         use seccomp\n"
-	       "  -S <file>:  set seccomp filters using <file>\n"
+	       "  -S <file>:  set seccomp filter using <file>\n"
 	       "              E.g., -S /usr/share/filters/<prog>.$(uname -m)\n"
 	       "  -u <user>:  change uid to <user>\n"
-	       "  -v:         use vfs namespace\n", progn);
+	       "  -v:         use vfs namespace\n");
 }
 
 static void seccomp_filter_usage(const char *progn)
@@ -105,7 +116,7 @@ int main(int argc, char *argv[])
 	struct minijail *j = minijail_new();
 
 	int opt;
-	while ((opt = getopt(argc, argv, "u:g:sS:c:C:b:vrGhHnp")) != -1) {
+	while ((opt = getopt(argc, argv, "u:g:sS:c:C:b:vrGhHnpL")) != -1) {
 		switch (opt) {
 		case 'u':
 			set_user(j, optarg);
@@ -123,6 +134,9 @@ int main(int argc, char *argv[])
 			minijail_parse_seccomp_filters(j, optarg);
 			minijail_use_seccomp_filter(j);
 			break;
+		case 'L':
+			minijail_log_seccomp_filter_failures(j);
+			break;
 		case 'b':
 			add_binding(j, optarg);
 			break;
diff --git a/signal.c b/signal.c
new file mode 100644
index 0000000..85103c7
--- /dev/null
+++ b/signal.c
@@ -0,0 +1,70 @@
+/* Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include <asm/siginfo.h>
+#define __have_siginfo_t 1
+#define __have_sigval_t 1
+#define __have_sigevent_t 1
+
+#include <signal.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "signal.h"
+
+#include "util.h"
+
+struct local_sigsys {
+	void		*ip;
+	int		nr;
+	unsigned int	arch;
+};
+
+void log_sigsys_handler(int nr, siginfo_t *info, void *void_context)
+{
+	struct local_sigsys sigsys;
+	const char *syscall_name;
+	memcpy(&sigsys, &info->_sifields, sizeof(sigsys));
+	syscall_name = lookup_syscall_name(sigsys.nr);
+
+	if (syscall_name)
+		warn("blocked syscall: %s", syscall_name);
+	else
+		warn("blocked syscall: %d", nr);
+
+	(void) void_context;
+
+	/*
+	 * We trapped on a syscall that should have killed the process.
+	 * This should never ever return, but we're paranoid.
+	 */
+	for (;;)
+		_exit(1);
+}
+
+int install_sigsys_handler()
+{
+	int ret = 0;
+	struct sigaction act;
+	sigset_t mask;
+
+	memset(&act, 0, sizeof(act));
+	act.sa_sigaction = &log_sigsys_handler;
+	act.sa_flags = SA_SIGINFO;
+
+	sigemptyset(&mask);
+	sigaddset(&mask, SIGSYS);
+
+	ret = sigaction(SIGSYS, &act, NULL);
+	if (ret < 0)
+		return ret;
+
+	ret = sigprocmask(SIG_UNBLOCK, &mask, NULL);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
diff --git a/signal.h b/signal.h
new file mode 100644
index 0000000..d68bbb2
--- /dev/null
+++ b/signal.h
@@ -0,0 +1,14 @@
+/* signal.h
+ * Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ *
+ * Signal handling functions.
+ */
+
+#ifndef SIGNAL_H
+#define SIGNAL_H
+
+int install_sigsys_handler();
+
+#endif /* SIGNAL_H */
diff --git a/syscall_filter.c b/syscall_filter.c
index e96ad60..b7dfb77 100644
--- a/syscall_filter.c
+++ b/syscall_filter.c
@@ -11,7 +11,9 @@
 
 #include "util.h"
 
-#define MAX_LINE_LENGTH 1024
+#define MAX_LINE_LENGTH		1024
+#define MAX_POLICY_LINE_LENGTH	1024
+
 #define ONE_INSTR	1
 #define TWO_INSTRS	2
 
@@ -85,6 +87,13 @@ void append_ret_kill(struct filter_block *head)
 	append_filter_block(head, filter, ONE_INSTR);
 }
 
+void append_ret_trap(struct filter_block *head)
+{
+	struct sock_filter *filter = new_instr_buf(ONE_INSTR);
+	set_bpf_ret_trap(filter);
+	append_filter_block(head, filter, ONE_INSTR);
+}
+
 void append_ret_errno(struct filter_block *head, int errno_val)
 {
 	struct sock_filter *filter = new_instr_buf(ONE_INSTR);
@@ -92,6 +101,22 @@ void append_ret_errno(struct filter_block *head, int errno_val)
 	append_filter_block(head, filter, ONE_INSTR);
 }
 
+void append_allow_syscall(struct filter_block *head, int nr) {
+	struct sock_filter *filter = new_instr_buf(ALLOW_SYSCALL_LEN);
+	size_t len = bpf_allow_syscall(filter, nr);
+	if (len != ALLOW_SYSCALL_LEN)
+		die("error building syscall number comparison");
+
+	append_filter_block(head, filter, len);
+}
+
+void allow_log_syscalls(struct filter_block *head)
+{
+	unsigned int i;
+	for (i = 0; i < log_syscalls_len; i++)
+		append_allow_syscall(head, lookup_syscall(log_syscalls[i]));
+}
+
 unsigned int get_label_id(struct bpf_labels *labels, const char *label_str)
 {
 	int label_id = bpf_label_id(labels, label_str);
@@ -147,11 +172,11 @@ struct filter_block *compile_section(int nr, const char *policy_line,
 	int group_idx = 0;
 
 	/* Checks for overly long policy lines. */
-	if (strlen(policy_line) >= MAX_POLICY_LINE_LEN)
+	if (strlen(policy_line) >= MAX_POLICY_LINE_LENGTH)
 		return NULL;
 
 	/* strtok() modifies its first argument, so let's make a copy. */
-	char *line = strndup(policy_line, MAX_POLICY_LINE_LEN);
+	char *line = strndup(policy_line, MAX_POLICY_LINE_LENGTH);
 	if (!line)
 		return NULL;
 
@@ -311,7 +336,8 @@ struct filter_block *compile_section(int nr, const char *policy_line,
 	return head;
 }
 
-int compile_filter(FILE *policy, struct sock_fprog *prog)
+int compile_filter(FILE *policy, struct sock_fprog *prog,
+		int log_failures)
 {
 	char line[MAX_LINE_LENGTH];
 	int line_count = 0;
@@ -334,11 +360,15 @@ int compile_filter(FILE *policy, struct sock_fprog *prog)
 	size_t len = bpf_validate_arch(valid_arch);
 	append_filter_block(head, valid_arch, len);
 
-	/* Loading syscall number. */
+	/* Load syscall number. */
 	struct sock_filter *load_nr = new_instr_buf(ONE_INSTR);
 	len = bpf_load_syscall_nr(load_nr);
 	append_filter_block(head, load_nr, len);
 
+	/* If we're logging failures, allow the necessary syscalls first. */
+	if (log_failures)
+		allow_log_syscalls(head);
+
 	/*
 	 * Loop through all the lines in the policy file.
 	 * Build a jump table for the syscall number.
@@ -374,10 +404,7 @@ int compile_filter(FILE *policy, struct sock_fprog *prog)
 		 */
 		if (strcmp(policy, "1") == 0) {
 			/* Add simple ALLOW. */
-			struct sock_filter *nr_comp =
-					new_instr_buf(ALLOW_SYSCALL_LEN);
-			bpf_allow_syscall(nr_comp, nr);
-			append_filter_block(head, nr_comp, ALLOW_SYSCALL_LEN);
+			append_allow_syscall(head, nr);
 		} else {
 			/*
 			 * Create and jump to the label that will hold
@@ -404,8 +431,14 @@ int compile_filter(FILE *policy, struct sock_fprog *prog)
 		}
 	}
 
-	/* If none of the syscalls match, fall back to KILL. */
-	append_ret_kill(head);
+	/*
+	 * If none of the syscalls match, either fall back to KILL,
+	 * or return TRAP.
+	 */
+	if (!log_failures)
+		append_ret_kill(head);
+	else
+		append_ret_trap(head);
 
 	/* Allocate the final buffer, now that we know its size. */
 	size_t final_filter_len = head->total_len +
diff --git a/syscall_filter.h b/syscall_filter.h
index 18ea415..a9d83d1 100644
--- a/syscall_filter.h
+++ b/syscall_filter.h
@@ -11,7 +11,8 @@
 
 #include "bpf.h"
 
-#define MAX_POLICY_LINE_LEN 1024
+#define NO_LOGGING  0
+#define USE_LOGGING 1
 
 struct filter_block {
 	struct sock_filter *instrs;
@@ -26,7 +27,7 @@ struct bpf_labels;
 
 struct filter_block *compile_section(int nr, const char *policy_line,
 		unsigned int label_id, struct bpf_labels *labels);
-int compile_filter(FILE *policy, struct sock_fprog *prog);
+int compile_filter(FILE *policy, struct sock_fprog *prog, int log_failures);
 
 int flatten_block_list(struct filter_block *head, struct sock_filter *filter,
 		size_t index, size_t cap);
diff --git a/syscall_filter_unittest.c b/syscall_filter_unittest.c
index c6b11cb..404919b 100644
--- a/syscall_filter_unittest.c
+++ b/syscall_filter_unittest.c
@@ -14,6 +14,8 @@
 #include "bpf.h"
 #include "syscall_filter.h"
 
+#include "util.h"
+
 /* BPF testing macros. */
 #define EXPECT_EQ_BLOCK(_block, _code, _k, _jt, _jf)	\
 do {	\
@@ -399,7 +401,7 @@ FIXTURE_TEARDOWN(filter) {}
 TEST_F(filter, seccomp_mode1) {
 	struct sock_fprog actual;
 	FILE *policy = fopen("test/seccomp.policy", "r");
-	int res = compile_filter(policy, &actual);
+	int res = compile_filter(policy, &actual, NO_LOGGING);
 
 	/*
 	 * Checks return value, filter length, and that the filter
@@ -427,7 +429,7 @@ TEST_F(filter, seccomp_mode1) {
 TEST_F(filter, seccomp_read_write) {
 	struct sock_fprog actual;
 	FILE *policy = fopen("test/stdin_stdout.policy", "r");
-	int res = compile_filter(policy, &actual);
+	int res = compile_filter(policy, &actual, NO_LOGGING);
 
 	/*
 	 * Checks return value, filter length, and that the filter
@@ -459,12 +461,12 @@ TEST_F(filter, invalid) {
 	struct sock_fprog actual;
 
 	FILE *policy = fopen("test/invalid_syscall_name.policy", "r");
-	int res = compile_filter(policy, &actual);
+	int res = compile_filter(policy, &actual, NO_LOGGING);
 	ASSERT_NE(res, 0);
 	fclose(policy);
 
 	policy = fopen("test/invalid_arg_filter.policy", "r");
-	res = compile_filter(policy, &actual);
+	res = compile_filter(policy, &actual, NO_LOGGING);
 	ASSERT_NE(res, 0);
 	fclose(policy);
 }
@@ -473,8 +475,46 @@ TEST_F(filter, nonexistent) {
 	struct sock_fprog actual;
 
 	FILE *policy = fopen("test/nonexistent-file.policy", "r");
-	int res = compile_filter(policy, &actual);
+	int res = compile_filter(policy, &actual, NO_LOGGING);
 	ASSERT_NE(res, 0);
 }
 
 TEST_HARNESS_MAIN
+
+TEST_F(filter, log) {
+	struct sock_fprog actual;
+
+	FILE *policy = fopen("test/seccomp.policy", "r");
+	int res = compile_filter(policy, &actual, USE_LOGGING);
+
+	size_t i;
+	size_t index = 0;
+	/*
+	 * Checks return value, filter length, and that the filter
+	 * validates arch, loads syscall number, only allows expected syscalls,
+	 * and returns TRAP on failure.
+	 * NOTE(jorgelo): the filter is longer since we add the syscalls needed
+	 * for logging.
+	 */
+	ASSERT_EQ(res, 0);
+	EXPECT_EQ(actual.len, 13 + 2 * log_syscalls_len);
+	EXPECT_ARCH_VALIDATION(actual.filter);
+	EXPECT_EQ_STMT(actual.filter + ARCH_VALIDATION_LEN,
+			BPF_LD+BPF_W+BPF_ABS, syscall_nr);
+
+	index = ARCH_VALIDATION_LEN + 1;
+	for (i = 0; i < log_syscalls_len; i++)
+		EXPECT_ALLOW_SYSCALL(actual.filter + (index + 2 * i),
+				     lookup_syscall(log_syscalls[i]));
+
+	index += 2 * log_syscalls_len;
+
+	EXPECT_ALLOW_SYSCALL(actual.filter + index, __NR_read);
+	EXPECT_ALLOW_SYSCALL(actual.filter + index + 2, __NR_write);
+	EXPECT_ALLOW_SYSCALL(actual.filter + index + 4, __NR_rt_sigreturn);
+	EXPECT_ALLOW_SYSCALL(actual.filter + index + 6, __NR_exit);
+	EXPECT_EQ_STMT(actual.filter + index + 8, BPF_RET+BPF_K, SECCOMP_RET_TRAP);
+
+	free(actual.filter);
+	fclose(policy);
+}
diff --git a/util.c b/util.c
index ea3c722..d16f3bb 100644
--- a/util.c
+++ b/util.c
@@ -10,6 +10,18 @@
 
 #include "libsyscalls.h"
 
+#if defined(__x86_64__)
+const char *log_syscalls[] = { "connect", "sendto" };
+#elif defined(__i386__)
+const char *log_syscalls[] = { "socketcall", "time" };
+#elif defined(__arm__)
+const char *log_syscalls[] = { "connect", "gettimeofday", "send" };
+#else
+#error "Unsupported platform"
+#endif
+
+const size_t log_syscalls_len = sizeof(log_syscalls)/sizeof(log_syscalls[0]);
+
 int lookup_syscall(const char *name)
 {
 	const struct syscall_entry *entry = syscall_table;
diff --git a/util.h b/util.h
index 8f0fa7b..09a67dc 100644
--- a/util.h
+++ b/util.h
@@ -26,6 +26,9 @@
 #define info(_msg, ...) \
         syslog(LOG_INFO, "libminijail: " _msg, ## __VA_ARGS__)
 
+extern const char *log_syscalls[];
+extern const size_t log_syscalls_len;
+
 int lookup_syscall(const char *name);
 const char *lookup_syscall_name(int nr);
 char *strip(char *s);