seccomp: Filter flag to log all actions except SECCOMP_RET_ALLOW

BugLink: https://launchpad.net/bugs/1721676
Add a new filter flag, SECCOMP_FILTER_FLAG_LOG, that enables logging for
all actions except for SECCOMP_RET_ALLOW for the given filter.

SECCOMP_RET_KILL actions are always logged, when "kill" is in the
actions_logged sysctl, and SECCOMP_RET_ALLOW actions are never logged,
regardless of this flag.

This flag can be used to create noisy filters that result in all
non-allowed actions to be logged. A process may have one noisy filter,
which is loaded with this flag, as well as a quiet filter that's not
loaded with this flag. This allows for the actions in a set of filters
to be selectively conveyed to the admin.

Since a system could have a large number of allocated seccomp_filter
structs, struct packing was taken in consideration. On 64 bit x86, the
new log member takes up one byte of an existing four byte hole in the
struct. On 32 bit x86, the new log member creates a new four byte hole
(unavoidable) and consumes one of those bytes.

Unfortunately, the tests added for SECCOMP_FILTER_FLAG_LOG are not
capable of inspecting the audit log to verify that the actions taken in
the filter were logged.

With this patch, the logic for deciding if an action will be logged is:

if action == RET_ALLOW:
  do not log
else if action == RET_KILL && RET_KILL in actions_logged:
  log
else if filter-requests-logging && action in actions_logged:
  log
else if audit_enabled && process-is-being-audited:
  log
else:
  do not log

Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
(backported from commit e66a39977985b1e69e17c4042cb290768eca9b02)
[tyhicks: disabled ability to log SECCOMP_RET_TRACE due to code changes]
Acked-by: Stefan Bader <stefan.bader@canonical.com>
Acked-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>

diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
index 2296e6b2f690760e5b1c2ab33091349be8d9ef58..32ec7488537a8a21c8da0684bb2b07e8dbd24dc1 100644 (file)
--- a/include/linux/seccomp.h
+++ b/include/linux/seccomp.h
@@ -3,7 +3,8 @@
 
 #include <uapi/linux/seccomp.h>
 
-#define SECCOMP_FILTER_FLAG_MASK       (SECCOMP_FILTER_FLAG_TSYNC)
+#define SECCOMP_FILTER_FLAG_MASK       (SECCOMP_FILTER_FLAG_TSYNC | \
+                                        SECCOMP_FILTER_FLAG_LOG)
 
 #ifdef CONFIG_SECCOMP
 

diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
index aaad61cc46bc614bbd96c6930fe1b3546bbd8296..19a611d0712e04ef7f7cd90c5c21a251adcd0910 100644 (file)
--- a/include/uapi/linux/seccomp.h
+++ b/include/uapi/linux/seccomp.h
@@ -17,6 +17,7 @@
 
 /* Valid flags for SECCOMP_SET_MODE_FILTER */
 #define SECCOMP_FILTER_FLAG_TSYNC      1
+#define SECCOMP_FILTER_FLAG_LOG                2
 
 /*
  * All BPF programs must return a 32-bit value.

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 29cfc92411cabb51163c8c2627bd4dbd3142cc72..6a4434f398e796f81ca997a48b84ead8ba6770cf 100644 (file)
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -42,6 +42,7 @@
  *         get/put helpers should be used when accessing an instance
  *         outside of a lifetime-guarded section.  In general, this
  *         is only needed for handling filters shared across tasks.
+ * @log: true if all actions except for SECCOMP_RET_ALLOW should be logged
  * @prev: points to a previously installed, or inherited, filter
  * @len: the number of instructions in the program
  * @insnsi: the BPF program instructions to evaluate
@@ -58,6 +59,7 @@
  */
 struct seccomp_filter {
        atomic_t usage;
+       bool log;
        struct seccomp_filter *prev;
        struct bpf_prog *prog;
 };
@@ -451,6 +453,10 @@ static long seccomp_attach_filter(unsigned int flags,
                        return ret;
        }
 
+       /* Set log flag, if present. */
+       if (flags & SECCOMP_FILTER_FLAG_LOG)
+               filter->log = true;
+
        /*
         * If there is an existing filter, make it the prev and don't drop its
         * task reference.
@@ -535,15 +541,22 @@ static void seccomp_send_sigsys(int syscall, int reason)
 static u32 seccomp_actions_logged = SECCOMP_LOG_KILL  | SECCOMP_LOG_TRAP  |
                                    SECCOMP_LOG_ERRNO | SECCOMP_LOG_TRACE;
 
-static inline void seccomp_log(unsigned long syscall, long signr, u32 action)
+static inline void seccomp_log(unsigned long syscall, long signr, u32 action,
+                              bool requested)
 {
        bool log = false;
 
        switch (action) {
        case SECCOMP_RET_ALLOW:
+               break;
        case SECCOMP_RET_TRAP:
+               log = requested && seccomp_actions_logged & SECCOMP_LOG_TRAP;
+               break;
        case SECCOMP_RET_ERRNO:
+               log = requested && seccomp_actions_logged & SECCOMP_LOG_ERRNO;
+               break;
        case SECCOMP_RET_TRACE:
+               log = requested && seccomp_actions_logged & SECCOMP_LOG_TRACE;
                break;
        case SECCOMP_RET_KILL:
        default:
@@ -551,8 +564,9 @@ static inline void seccomp_log(unsigned long syscall, long signr, u32 action)
        }
 
        /*
-        * Force an audit message to be emitted when the action is RET_KILL and
-        * the action is allowed to be logged by the admin.
+        * Force an audit message to be emitted when the action is RET_KILL or
+        * the FILTER_FLAG_LOG bit was set and the action is allowed to be
+        * logged by the admin.
         */
        if (log)
                return __audit_seccomp(syscall, signr, action);
@@ -596,7 +610,7 @@ static void __secure_computing_strict(int this_syscall)
 #ifdef SECCOMP_DEBUG
        dump_stack();
 #endif
-       seccomp_log(this_syscall, SIGKILL, SECCOMP_RET_KILL);
+       seccomp_log(this_syscall, SIGKILL, SECCOMP_RET_KILL, true);
        do_exit(SIGKILL);
 }
 
@@ -675,14 +689,14 @@ static u32 __seccomp_phase1_filter(int this_syscall, struct seccomp_data *sd)
 
        case SECCOMP_RET_KILL:
        default:
-               seccomp_log(this_syscall, SIGSYS, action);
+               seccomp_log(this_syscall, SIGSYS, action, true);
                do_exit(SIGSYS);
        }
 
        unreachable();
 
 skip:
-       seccomp_log(this_syscall, 0, action);
+       seccomp_log(this_syscall, 0, action, match ? match->log : false);
        return SECCOMP_PHASE1_SKIP;
 }
 #endif
@@ -749,7 +763,13 @@ int seccomp_phase2(u32 phase1_result)
 
        BUG_ON(action != SECCOMP_RET_TRACE);
 
-       seccomp_log(syscall_get_nr(current, regs), 0, action);
+       /* We don't have access to the filter that was matched in the phase1
+        * stage in order to know if logging was requested when the filter was
+        * loaded. Logging for SECCOMP_RET_TRACE isn't particularly useful so
+        * hard-coding the _requested_ parameter of seccomp_log() to 'false'
+        * will suffice for this backport.
+        */
+       seccomp_log(syscall_get_nr(current, regs), 0, action, false);
 
        /* Skip these calls if there is no tracer. */
        if (!ptrace_event_enabled(current, PTRACE_EVENT_SECCOMP)) {

diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index 973a4a82c3e7fed2045769279cc28c9eba7fe04c..5dfbd0a154a800448850f7d6ada4dc6dd44371f7 100644 (file)
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -1484,6 +1484,10 @@ TEST_F(TRACE_syscall, syscall_dropped)
 #define SECCOMP_FLAG_FILTER_TSYNC 1
 #endif
 
+#ifndef SECCOMP_FILTER_FLAG_LOG
+#define SECCOMP_FILTER_FLAG_LOG 2
+#endif
+
 #ifndef seccomp
 int seccomp(unsigned int op, unsigned int flags, struct sock_fprog *filter)
 {
@@ -1589,7 +1593,8 @@ TEST(seccomp_syscall_mode_lock)
  */
 TEST(detect_seccomp_filter_flags)
 {
-       unsigned int flags[] = { SECCOMP_FILTER_FLAG_TSYNC };
+       unsigned int flags[] = { SECCOMP_FILTER_FLAG_TSYNC,
+                                SECCOMP_FILTER_FLAG_LOG };
        unsigned int flag, all_flags;
        int i;
        long ret;
@@ -2261,6 +2266,67 @@ TEST(syscall_restart)
                _metadata->passed = 0;
 }
 
+TEST_SIGNAL(filter_flag_log, SIGSYS)
+{
+       struct sock_filter allow_filter[] = {
+               BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
+       };
+       struct sock_filter kill_filter[] = {
+               BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
+                       offsetof(struct seccomp_data, nr)),
+               BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_getpid, 0, 1),
+               BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL),
+               BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
+       };
+       struct sock_fprog allow_prog = {
+               .len = (unsigned short)ARRAY_SIZE(allow_filter),
+               .filter = allow_filter,
+       };
+       struct sock_fprog kill_prog = {
+               .len = (unsigned short)ARRAY_SIZE(kill_filter),
+               .filter = kill_filter,
+       };
+       long ret;
+       pid_t parent = getppid();
+
+       ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
+       ASSERT_EQ(0, ret);
+
+       /* Verify that the FILTER_FLAG_LOG flag isn't accepted in strict mode */
+       ret = seccomp(SECCOMP_SET_MODE_STRICT, SECCOMP_FILTER_FLAG_LOG,
+                     &allow_prog);
+       ASSERT_NE(ENOSYS, errno) {
+               TH_LOG("Kernel does not support seccomp syscall!");
+       }
+       EXPECT_NE(0, ret) {
+               TH_LOG("Kernel accepted FILTER_FLAG_LOG flag in strict mode!");
+       }
+       EXPECT_EQ(EINVAL, errno) {
+               TH_LOG("Kernel returned unexpected errno for FILTER_FLAG_LOG flag in strict mode!");
+       }
+
+       /* Verify that a simple, permissive filter can be added with no flags */
+       ret = seccomp(SECCOMP_SET_MODE_FILTER, 0, &allow_prog);
+       EXPECT_EQ(0, ret);
+
+       /* See if the same filter can be added with the FILTER_FLAG_LOG flag */
+       ret = seccomp(SECCOMP_SET_MODE_FILTER, SECCOMP_FILTER_FLAG_LOG,
+                     &allow_prog);
+       ASSERT_NE(EINVAL, errno) {
+               TH_LOG("Kernel does not support the FILTER_FLAG_LOG flag!");
+       }
+       EXPECT_EQ(0, ret);
+
+       /* Ensure that the kill filter works with the FILTER_FLAG_LOG flag */
+       ret = seccomp(SECCOMP_SET_MODE_FILTER, SECCOMP_FILTER_FLAG_LOG,
+                     &kill_prog);
+       EXPECT_EQ(0, ret);
+
+       EXPECT_EQ(parent, syscall(__NR_getppid));
+       /* getpid() should never return. */
+       EXPECT_EQ(0, syscall(__NR_getpid));
+}
+
 TEST(get_action_avail)
 {
        __u32 actions[] = { SECCOMP_RET_KILL,  SECCOMP_RET_TRAP,
@@ -2301,6 +2367,7 @@ TEST(get_action_avail)
  * - endianness checking when appropriate
  * - 64-bit arg prodding
  * - arch value testing (x86 modes especially)
+ * - verify that FILTER_FLAG_LOG filters generate log messages
  * - ...
  */