CHROMIUM: seccomp_filter: make inherited filters composable
authorWill Drewry <wad@chromium.org>
Wed, 24 Aug 2011 21:45:01 +0000 (16:45 -0500)
committerLeann Ogasawara <leann.ogasawara@canonical.com>
Mon, 2 Apr 2012 20:13:27 +0000 (13:13 -0700)
Right now, inheritance just copies the parent ruleset.  This means
that on inheritance, if a child wants to reduce privilege, it must
walk its inherited set to drop those it doesn't want.

With this change, inherited sets are always left intact and any process
local rules are created in a child seccomp_filter object.  On evaluation,
the child rules are checked first, then all ancestors.  If the system call
is not allowed by any of them, the process will be terminated.

The change makes PR_SECCOMP_GET_FILTER less intuitive since it won't
coalesce all inherited values, but it assures that a child process
can create its ruleset normally, then call prctl(PR_SET_SECCOMP, 13) to
enable it on top of anything that is inherited.

This is similar to namespacing behavior except that the copy-on-update
methodology precludes the parent process from modifying a filter that
has been inherited.

TEST=minijail0 -S support and the seccomp_tests.c in the seccomp filters bugs
BUG=chromium-os:19459

Change-Id: Icd192b75d887fbee09d99fe86cd1a815a8ae673d
Signed-off-by: Will Drewry <wad@chromium.org>
Reviewed-on: http://gerrit.chromium.org/gerrit/7109
Reviewed-by: Elly Jones <ellyjones@chromium.org>
Signed-off-by: Leann Ogasawara <leann.ogasawara@canonical.com>

Documentation/prctl/seccomp_filter.txt
include/linux/seccomp.h
kernel/seccomp.c
kernel/seccomp_filter.c

index 88e1d3f..e21ead7 100644 (file)
@@ -53,7 +53,8 @@ and one existing one.
 
 PR_SET_SECCOMP:
        A pre-existing option for enabling strict seccomp mode (1) or
-       filtering seccomp (13).
+       filtering seccomp (13).  Performing this call (13) will enable
+       the current set of defined filters.
 
        Usage:
                prctl(PR_SET_SECCOMP, 1);  /* strict */
@@ -154,33 +155,32 @@ Inheritance
 -----------
 
 Changing the availability of the kernel ABI at runtime runs the risk of
-providing access to normally unreachable code paths in normal
-applications.  To avoid the pitfalls that accompany this risk, seccomp
-filters inheritance is restricted.
-
-In general, filters can be inherited across fork/clone, but only when
-they are active (e.g., PR_SET_SECCOMP has been set to 13) and not prior
-to use.  Inheriting only active filters stops a parent process from
-adding filters that may undermine the child process security or create
-unexpected behavior after an execve.
-
-For example, a parent process may add a rule to exposes a system call
-that was not normally part of the child process' filter set.  When the
-child process configures its filters, it would have to check
-/proc/self/seccomp_filter to ensure nothing unexpected has been added.
-The standard inheritance behavior ensures this suboptimal situation is
-avoided.
-
-Inheritance across execve follows a subset of this behavior.  In
-particular, execve can only be added to the allowed filter set by a
-process with CAP_SYS_ADMIN privileges.  The result is that an
-unprivileged process can never create a seccomp filter set that can be
-inherited across execve.  To further guarantee this behavior, any
-unprivileged modifications to a seccomp filter set will forcibly
-clear execve.  The end result is that a privileged parent may install
-a set of seccomp filters and, at any point in the hierarchy, a child may
-make a private version of the inherited filter set with their own
-changes applied but execve blocked.
+providing access to unreachable code paths in normal applications.  To
+avoid the pitfalls that accompany this risk, seccomp filter inheritance
+is restricted.
+
+Filters may be inherited across a fork/clone if they have been activated
+by a call to prctl(PR_SET_SECCOMP, 13).  If the process had the
+CAP_SYS_ADMIN capability when configuring the filters, they may also be
+inherited across an execve call.
+
+Inherited filters may not be modified by the child process.  If child
+would like to further restrict the available system calls, it may
+perform the same calls as discussed earlier: set, clear, get, and
+finally prctl(PR_SET_SECCOMP, 13).  Until the child calls
+PR_SET_SECCOMP, their filters will be ignored and only the inherited
+filter will be evaluated.  After a successful PR_SET_SECCOMP call, all
+system calls performed by the child will be checked against the filters
+that were specified by itself and then against the filters supplied by
+any ancestors.  Any system call used must be allowed by all of the
+filters tested.  This composition of the ancestral seccomp filters and
+process-local filters guarantees that only the minimal set of system calls
+will be permitted at any point and without child processes needing to be
+aware of any prior system call filtering.
+
+(If, for instance, the child process merely copied the parent filters
+and then extended them, the child would be required to enumerate all
+existing filters to determine which needed to be dropped.)
 
 
 Caveats
index 57235ad..d8ace69 100644 (file)
@@ -77,18 +77,8 @@ static inline int seccomp_mode(seccomp_t *s)
        (_tsk)->seccomp.filters = NULL; \
 } while (0)
 
-/* Do nothing unless seccomp filtering is active. If not, the execve boundary
- * can not be cleanly enforced and preset filters may leak across execve calls.
- */
-#define seccomp_filter_fork(_tsk, _orig) do { \
-       if ((_tsk)->seccomp.mode) { \
-               (_tsk)->seccomp.mode = (_orig)->seccomp.mode; \
-               mutex_lock(&(_orig)->seccomp.filters_guard); \
-               (_tsk)->seccomp.filters = \
-                       get_seccomp_filters((_orig)->seccomp.filters); \
-               mutex_unlock(&(_orig)->seccomp.filters_guard); \
-       } \
-} while (0)
+extern void seccomp_filter_fork(struct task_struct *child,
+                               struct task_struct *parent);
 
 /* No locking is needed here because the task_struct will
  * have no parallel consumers.
@@ -103,6 +93,8 @@ extern long seccomp_set_filter(int, char *);
 extern long seccomp_clear_filter(int);
 extern long seccomp_get_filter(int, char *, unsigned long);
 
+extern long seccomp_enable_filters(void);
+
 extern long prctl_set_seccomp_filter(unsigned long, unsigned long,
                                     char __user *);
 extern long prctl_get_seccomp_filter(unsigned long, unsigned long,
@@ -119,7 +111,8 @@ extern void seccomp_filter_log_failure(int);
 
 struct seccomp_filters { };
 #define seccomp_filter_init_task(_tsk) do { } while (0)
-#define seccomp_filter_fork(_tsk, _orig) do { } while (0)
+static inline void seccomp_filter_fork(struct task_struct *child,
+                                      struct task_struct *parent) { }
 #define seccomp_filter_free_task(_tsk) do { } while (0)
 
 static inline int seccomp_show_filters(struct seccomp_filters *filters,
index e0df4ad..0f0105e 100644 (file)
@@ -75,29 +75,27 @@ long prctl_get_seccomp(void)
 
 long prctl_set_seccomp(unsigned long seccomp_mode)
 {
-       long ret;
+       long ret = 0;
 
-       /* can set it only once to be even more secure */
-       ret = -EPERM;
-       if (unlikely(current->seccomp.mode))
-               goto out;
-
-       ret = 0;
        switch (seccomp_mode) {
        case 1:
 #ifdef TIF_NOTSC
                disable_TSC();
 #endif
+               break;
 #ifdef CONFIG_SECCOMP_FILTER
        case 13:
-#endif
-               current->seccomp.mode = seccomp_mode;
-               set_thread_flag(TIF_SECCOMP);
+               ret = seccomp_enable_filters();
                break;
+#endif
        default:
                ret = -EINVAL;
        }
 
-out:
+       if (!ret && !current->seccomp.mode) {
+               current->seccomp.mode = seccomp_mode;
+               set_thread_flag(TIF_SECCOMP);
+       }
+
        return ret;
 }
index 8b1b8a6..ac682cf 100644 (file)
@@ -25,6 +25,7 @@
 #include <linux/kallsyms.h>
 #include <linux/kref.h>
 #include <linux/perf_event.h>
+#include <linux/pid.h>
 #include <linux/prctl.h>
 #include <linux/seccomp.h>
 #include <linux/security.h>
  *         get/put helpers should be used when accessing an instance
  *         outside of a lifetime-guarded section.  In general, this
  *         is only needed for handling shared filters across tasks.
+ * @creator: pointer to the pid that created this filter
+ * @parent: pointer to the ancestor which this filter will be composed with.
  * @count: size of @event_filters
  * @filter: tree of pointers to seccomp filters
  *
  * seccomp_filters objects should never be modified after being attached
- * to a task_struct.
+ * to a task_struct (other than @usage).
  */
 struct seccomp_filters {
        struct kref usage;
+       struct pid *creator;
+       struct seccomp_filters *parent;
        struct {
                uint32_t compat:1,
-                        __reserved:31;
+                        enabled:1,
+                        __reserved:30;
        } flags;
        uint16_t count;
        struct btree_head32 tree;
@@ -76,7 +82,6 @@ struct seccomp_filters {
 /*
  * Make ftrace support optional
  */
-
 #if defined(CONFIG_FTRACE_SYSCALLS) && defined(CONFIG_PERF_EVENTS)
 #include <asm/syscall.h>
 
@@ -320,6 +325,7 @@ static struct seccomp_filters *seccomp_filters_alloc(void)
        kref_init(&f->usage);
        if (btree_init32(&f->tree))
                return ERR_PTR(-ENOMEM);
+       f->creator = get_task_pid(current, PIDTYPE_PID);
 
        return f;
 }
@@ -339,6 +345,8 @@ static void seccomp_filters_free(struct seccomp_filters *filters)
                free_event_filter(ef);
        }
        btree_destroy32(&filters->tree);
+       put_seccomp_filters(filters->parent);
+       put_pid(filters->creator);
        kfree(filters);
 }
 
@@ -427,6 +435,10 @@ static int seccomp_filters_copy(struct seccomp_filters *dst,
                }
                dst->count++;
        }
+       dst->parent = get_seccomp_filters(src->parent);
+       if (dst->creator)
+               put_pid(dst->creator);
+       dst->creator = get_pid(src->creator);
 
 done:
        return ret;
@@ -648,6 +660,19 @@ struct seccomp_filters *get_seccomp_filters(struct seccomp_filters *orig)
        return orig;
 }
 
+static int filters_created_by_parent(struct seccomp_filters *filters)
+{
+       struct pid *pid;
+       int ret = 0;
+       if (!filters)
+               return ret;
+       pid = get_task_pid(current, PIDTYPE_PID);
+       if (pid != filters->creator)
+               ret = 1;
+       put_pid(pid);
+       return ret;
+}
+
 /**
  * seccomp_test_filters - tests 'current' against the given syscall
  * @syscall: number of the system call to test
@@ -662,28 +687,37 @@ int seccomp_test_filters(int syscall)
 
        mutex_lock(&current->seccomp.filters_guard);
        /* No reference counting is done. filters_guard should protect the
-        * lifetime of any existing pointer below.
+        * lifetime of any existing pointer below using the task reference.
+        * Parents will be protected by the held references.
         */
        filters = current->seccomp.filters;
+
+       /* Inherited filters will always be enabled so skip ahead here. */
+       if (filters && !filters->flags.enabled)
+               filters = filters->parent;
+
+       /* Without any enabled filters, no system calls will be allowed. */
        if (!filters)
                goto out;
 
-       if (filters_compat_mismatch(filters)) {
-               pr_info("%s[%d]: seccomp_filter compat() mismatch.\n",
-                       current->comm, task_pid_nr(current));
-               goto out;
-       }
+       /* Only allow a system call if it is allowed in all ancestors. */
+       for ( ; filters != NULL; filters = filters->parent) {
+               if (filters_compat_mismatch(filters)) {
+                       pr_info("%s[%d]: seccomp_filter compat() mismatch.\n",
+                               current->comm, task_pid_nr(current));
+                       goto out;
+               }
 
-       filter = btree_lookup32(&filters->tree, syscall);
-       if (!filter)
-               goto out;
+               filter = btree_lookup32(&filters->tree, syscall);
+               if (!filter)
+                       goto out;
 
-       ret = 0;
-       if (IS_ALLOW_FILTER(filter))
+               if (IS_ALLOW_FILTER(filter) || filter_match_current(filter))
+                       continue;
                goto out;
-
-       if (!filter_match_current(filter))
-               ret = -EACCES;
+       }
+       /* If the loop terminates normally, the syscall is approved. */
+       ret = 0;
 out:
        mutex_unlock(&current->seccomp.filters_guard);
        return ret;
@@ -700,15 +734,22 @@ int seccomp_show_filters(struct seccomp_filters *filters, struct seq_file *m)
 {
        int nr;
        struct event_filter *ef;
+
        if (!filters)
                goto out;
 
-       btree_for_each_safe32(&filters->tree, nr, ef) {
-               const char *filter_string = SECCOMP_FILTER_ALLOW;
-               seq_printf(m, "%d (%s): ", nr, syscall_nr_to_name(nr));
-               if (!IS_ALLOW_FILTER(ef))
-                       filter_string = get_filter_string(ef);
-               seq_printf(m, "%s\n", filter_string);
+       for ( ; filters; filters = filters->parent) {
+               seq_printf(m, "Enabled: %d\n", filters->flags.enabled);
+               seq_printf(m, "Inherited: %d\n",
+                          filters_created_by_parent(filters));
+               btree_for_each_safe32(&filters->tree, nr, ef) {
+                       const char *filter_string = SECCOMP_FILTER_ALLOW;
+                       seq_printf(m, "%d (%s): ", nr, syscall_nr_to_name(nr));
+                       if (!IS_ALLOW_FILTER(ef))
+                               filter_string = get_filter_string(ef);
+                       seq_printf(m, "%s\n", filter_string);
+               }
+               seq_printf(m, "--\n");
        }
 out:
        return 0;
@@ -741,7 +782,7 @@ long seccomp_get_filter(int syscall_nr, char *buf, unsigned long bufsize)
        mutex_lock(&current->seccomp.filters_guard);
        filters = current->seccomp.filters;
 
-       if (!filters)
+       if (!filters || filters_created_by_parent(filters))
                goto out;
 
        ret = -ENOENT;
@@ -785,7 +826,7 @@ long seccomp_clear_filter(int syscall_nr)
        mutex_lock(&current->seccomp.filters_guard);
        orig_filters = current->seccomp.filters;
 
-       if (!orig_filters)
+       if (!orig_filters || filters_created_by_parent(orig_filters))
                goto out;
 
        if (filters_compat_mismatch(orig_filters))
@@ -836,7 +877,8 @@ EXPORT_SYMBOL_GPL(seccomp_clear_filter);
  */
 long seccomp_set_filter(int syscall_nr, char *filter)
 {
-       struct seccomp_filters *filters = NULL, *orig_filters = NULL;
+       struct seccomp_filters *filters = NULL, *orig_filters = NULL,
+                              *parent_filters = NULL;
        struct event_filter *ef = NULL;
        long ret = -EPERM;
 
@@ -856,6 +898,11 @@ long seccomp_set_filter(int syscall_nr, char *filter)
 
        orig_filters = current->seccomp.filters;
 
+       if (filters_created_by_parent(orig_filters)) {
+               parent_filters = orig_filters;
+               orig_filters = NULL;
+       }
+
        /* After the first call, compatibility mode is selected permanently. */
        ret = -EACCES;
        if (filters_compat_mismatch(orig_filters))
@@ -865,9 +912,10 @@ long seccomp_set_filter(int syscall_nr, char *filter)
                ef = btree_lookup32(&orig_filters->tree, syscall_nr);
 
        if (!ef) {
-               /* Don't allow DENYs to be changed when in a seccomp mode */
+               /* A new filter cannot be added to an active filters set. */
                ret = -EACCES;
-               if (current->seccomp.mode)
+               if (current->seccomp.mode && orig_filters &&
+                   orig_filters->flags.enabled)
                        goto out;
        }
 
@@ -895,6 +943,8 @@ long seccomp_set_filter(int syscall_nr, char *filter)
        get_seccomp_filters(filters);  /* simplify the error paths */
 
        current->seccomp.filters = filters;
+       if (parent_filters)
+               filters->parent = parent_filters;  /* already have a ref */
        put_seccomp_filters(orig_filters);  /* for the task */
 out:
        put_seccomp_filters(filters);  /* for get or task, on err */
@@ -903,6 +953,46 @@ out:
 }
 EXPORT_SYMBOL_GPL(seccomp_set_filter);
 
+long seccomp_enable_filters(void)
+{
+       struct seccomp_filters *filters = NULL, *orig_filters = NULL;
+       long ret = 0;
+
+       mutex_lock(&current->seccomp.filters_guard);
+       /* Rely on the task reference */
+       orig_filters = current->seccomp.filters;
+       if (!orig_filters)
+               goto out;
+
+       /* Cannot re-enable inherited filters */
+       ret = -EINVAL;
+       if (filters_created_by_parent(orig_filters))
+               goto out;
+
+       filters = seccomp_filters_alloc();
+       if (IS_ERR(filters)) {
+               ret = PTR_ERR(filters);
+               goto out;
+       }
+
+       ret = seccomp_filters_copy(filters, orig_filters);
+       if (ret)
+               goto out;
+
+       /* Do the real work */
+       filters->flags.enabled = 1;
+
+       get_seccomp_filters(filters);  /* simplify the error paths */
+
+       current->seccomp.filters = filters;
+       put_seccomp_filters(orig_filters);  /* for the task */
+out:
+       put_seccomp_filters(filters);  /* for get or task, on err */
+        mutex_unlock(&current->seccomp.filters_guard);
+       return ret;
+}
+EXPORT_SYMBOL_GPL(seccomp_enable_filters);
+
 long prctl_set_seccomp_filter(unsigned long id_type,
                              unsigned long id,
                              char __user *user_filter)
@@ -1024,3 +1114,30 @@ out:
        kfree(buf);
        return ret;
 }
+
+/* seccomp_filter_fork: manages inheritance on fork
+ * @child: forkee
+ * @parent: forker
+ * Ensures that @child inherit a seccomp_filters iff seccomp is enabled
+ * and the set of filters is marked as 'enabled'.
+ */
+void seccomp_filter_fork(struct task_struct *child,
+                        struct task_struct *parent)
+{
+       if (!parent->seccomp.mode)
+               return;
+       child->seccomp.mode = parent->seccomp.mode;
+       mutex_lock(&parent->seccomp.filters_guard);
+       child->seccomp.filters = get_seccomp_filters(parent->seccomp.filters);
+       mutex_unlock(&parent->seccomp.filters_guard);
+       /* If @parent's filters are not active, then inherit the ancestor
+        * if there is one.  It's possible that it will be NULL if the seccomp
+        * mode does not use seccomp_filters.
+        */
+       if (child->seccomp.filters && !child->seccomp.filters->flags.enabled) {
+               struct seccomp_filters *enabled =
+                   get_seccomp_filters(child->seccomp.filters->parent);
+               put_seccomp_filters(child->seccomp.filters);
+               child->seccomp.filters = enabled;
+       }
+}