Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1580269 > unrolled thread

[PATCH v3 0/4] Improved seccomp logging

Started byTyler Hicks <tyhicks@canonical.com>
First post2017-02-14 04:50 +0100
Last post2017-02-22 19:50 +0100
Articles 7 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v3 0/4] Improved seccomp logging Tyler Hicks <tyhicks@canonical.com> - 2017-02-14 04:50 +0100
    Re: [PATCH v3 0/4] Improved seccomp logging Andy Lutomirski <luto@amacapital.net> - 2017-02-16 04:30 +0100
      Re: [PATCH v3 0/4] Improved seccomp logging Tyler Hicks <tyhicks@canonical.com> - 2017-02-16 20:40 +0100
      Re: [PATCH v3 0/4] Improved seccomp logging Kees Cook <keescook@chromium.org> - 2017-02-17 00:30 +0100
        Re: [PATCH v3 0/4] Improved seccomp logging Andy Lutomirski <luto@amacapital.net> - 2017-02-17 18:10 +0100
          Re: [PATCH v3 0/4] Improved seccomp logging Kees Cook <keescook@chromium.org> - 2017-02-22 19:40 +0100
        Re: [PATCH v3 0/4] Improved seccomp logging Kees Cook <keescook@chromium.org> - 2017-02-22 19:50 +0100

#1580269 — [PATCH v3 0/4] Improved seccomp logging

FromTyler Hicks <tyhicks@canonical.com>
Date2017-02-14 04:50 +0100
Subject[PATCH v3 0/4] Improved seccomp logging
Message-ID<taChP-4Iv-3@gated-at.bofh.it>
This patch set is the third revision of the following two previously
submitted patch sets:

v1: http://lkml.kernel.org/r/1483375990-14948-1-git-send-email-tyhicks@canonical.com
v1: http://lkml.kernel.org/r/1483377999-15019-2-git-send-email-tyhicks@canonical.com

v2: http://lkml.kernel.org/r/1486100262-32391-1-git-send-email-tyhicks@canonical.com

The patch set aims to address some known deficiencies in seccomp's current
logging capabilities:

  1. Inability to log all filter actions.
  2. Inability to selectively enable filtering; e.g. devs want noisy logging,
     users want relative quiet.
  3. Consistent behavior with audit enabled and disabled.
  4. Inability to easily develop a filter due to the lack of a
     permissive/complain mode.

Changes since v2 to address feedback from Kees:
- Patch 1
  + Log a warning when sysctl registration fails
  + Move comment describing SECCOMP_RET_*_NAME from PATCH 2
  + Document the actions_avail sysctl
- Patch 2
  + Inline seccomp_log()
  + Optimize logging for RET_ALLOW hot path
  + Use "{ }" for name buffer initialization
  + Make a copy of the ctl_table and only modify the copy
  + Rename max_action_to_log sysctl to log_max_action
  + Document the log_max_action sysctl
- Patch 3
  + Put some space between RET_LOG and RET_ALLOW for future actions
  + Separate the RET_ALLOW and RET_LOG cases in __seccomp_filter()
- Patch 4
  + Adjust the selftests for the updated RET_LOG value

Tyler


Tyler Hicks (4):
  seccomp: Add sysctl to display available actions
  seccomp: Add sysctl to configure actions that should be logged
  seccomp: Create an action to log before allowing
  seccomp: Add tests for SECCOMP_RET_LOG

 Documentation/prctl/seccomp_filter.txt        |  43 ++++++
 Documentation/sysctl/kernel.txt               |   1 +
 include/linux/audit.h                         |   6 +-
 include/uapi/linux/seccomp.h                  |   1 +
 kernel/seccomp.c                              | 185 +++++++++++++++++++++++++-
 tools/testing/selftests/seccomp/seccomp_bpf.c |  94 +++++++++++++
 6 files changed, 322 insertions(+), 8 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]


#1582296

FromAndy Lutomirski <luto@amacapital.net>
Date2017-02-16 04:30 +0100
Message-ID<tbkVA-qe-1@gated-at.bofh.it>
In reply to#1580269
On Mon, Feb 13, 2017 at 7:45 PM, Tyler Hicks <tyhicks@canonical.com> wrote:
> This patch set is the third revision of the following two previously
> submitted patch sets:
>
> v1: http://lkml.kernel.org/r/1483375990-14948-1-git-send-email-tyhicks@canonical.com
> v1: http://lkml.kernel.org/r/1483377999-15019-2-git-send-email-tyhicks@canonical.com
>
> v2: http://lkml.kernel.org/r/1486100262-32391-1-git-send-email-tyhicks@canonical.com
>
> The patch set aims to address some known deficiencies in seccomp's current
> logging capabilities:
>
>   1. Inability to log all filter actions.
>   2. Inability to selectively enable filtering; e.g. devs want noisy logging,
>      users want relative quiet.
>   3. Consistent behavior with audit enabled and disabled.
>   4. Inability to easily develop a filter due to the lack of a
>      permissive/complain mode.

I think I dislike this, but I think my dislikes may be fixable with
minor changes.

What I dislike is that this mixes app-specific built-in configuration
(seccomp) with global privileged stuff (audit).  The result is a
potentially difficult to use situation in which you need to modify an
app to make it loggable (using RET_LOG) and then fiddle with
privileged config (auditctl, etc) to actually see the logs.

What if, instead of logging straight to the audit log, SECCOMP_RET_LOG
[1] merely meant "tell our parent about this syscall"?  (Ideally we'd
also figure out a way to express "log this and allow", "log this and
do ERRNO", etc.)  Then we could have another mechanism that installs a
layer in the seccomp stack that, instead of catching syscalls, catches
log events and sticks them in a ring buffer (or audit).

Concretely, it might work like this.  If a filter returns
SECCOMP_RET_LOG, then we "log" and keep processing.  SECCOMP_RET_LOG
is otherwise treated literally like SECCOMP_RET_ALLOW and has no
effect on return value.  If you want log-and-kill, you install two
filters.

There's a new seccomp(2) action that returns an fd.  That fd
references a new thing in the seccomp stack that is a BPF program that
is called whenever SECCOMP_RET_LOG is returned from lower down.  The
output of this filter determines whether the log event is ignored,
stuck in the ring buffer, or passed up the stack for further
processing.  You read(2) the fd to access the ring buffer.

Using this mechanism, you could write a simple seccomptrace tool that
needs no privilege and dumps SECCOMP_RET_LOG events from the target
program to stderr.

Thoughts?

[1] If we went this route, it might want to be renamed.

P.S. We ought to be able to write a BPF verifier pass that makes sure
that filters don't return unsupported return values if we cared to do
so.

[toc] | [prev] | [next] | [standalone]


#1582895

FromTyler Hicks <tyhicks@canonical.com>
Date2017-02-16 20:40 +0100
Message-ID<tbA4i-2sg-19@gated-at.bofh.it>
In reply to#1582296

[Multipart message — attachments visible in raw view] — view raw

On 02/15/2017 09:24 PM, Andy Lutomirski wrote:
> On Mon, Feb 13, 2017 at 7:45 PM, Tyler Hicks <tyhicks@canonical.com> wrote:
>> This patch set is the third revision of the following two previously
>> submitted patch sets:
>>
>> v1: http://lkml.kernel.org/r/1483375990-14948-1-git-send-email-tyhicks@canonical.com
>> v1: http://lkml.kernel.org/r/1483377999-15019-2-git-send-email-tyhicks@canonical.com
>>
>> v2: http://lkml.kernel.org/r/1486100262-32391-1-git-send-email-tyhicks@canonical.com
>>
>> The patch set aims to address some known deficiencies in seccomp's current
>> logging capabilities:
>>
>>   1. Inability to log all filter actions.
>>   2. Inability to selectively enable filtering; e.g. devs want noisy logging,
>>      users want relative quiet.
>>   3. Consistent behavior with audit enabled and disabled.
>>   4. Inability to easily develop a filter due to the lack of a
>>      permissive/complain mode.
> 
> I think I dislike this, but I think my dislikes may be fixable with
> minor changes.
> 
> What I dislike is that this mixes app-specific built-in configuration
> (seccomp) with global privileged stuff (audit).  The result is a
> potentially difficult to use situation in which you need to modify an
> app to make it loggable (using RET_LOG) and then fiddle with
> privileged config (auditctl, etc) to actually see the logs.

There's no need to fiddle with audictl. You would only need to write
"log" to /proc/sys/kernel/seccomp/log_max_action. I wouldn't think that
would be an issue considering that this will typically be be done on a
developer's system. Additionally, I plan to make "log" the default for
log_max_action in Ubuntu so there'd be nothing to change to get RET_LOG
working.

If auditd is running, the messages will go to the audit.log. If auditd
is not running, the messages will go to the syslog.

What if we make "log" the default for log_max_action in this patch set?
Would that address your concerns?

> What if, instead of logging straight to the audit log, SECCOMP_RET_LOG
> [1] merely meant "tell our parent about this syscall"?  (Ideally we'd
> also figure out a way to express "log this and allow", "log this and
> do ERRNO", etc.)  Then we could have another mechanism that installs a
> layer in the seccomp stack that, instead of catching syscalls, catches
> log events and sticks them in a ring buffer (or audit).
> 
> Concretely, it might work like this.  If a filter returns
> SECCOMP_RET_LOG, then we "log" and keep processing.  SECCOMP_RET_LOG
> is otherwise treated literally like SECCOMP_RET_ALLOW and has no
> effect on return value.  If you want log-and-kill, you install two
> filters.
> 
> There's a new seccomp(2) action that returns an fd.  That fd
> references a new thing in the seccomp stack that is a BPF program that
> is called whenever SECCOMP_RET_LOG is returned from lower down.  The
> output of this filter determines whether the log event is ignored,
> stuck in the ring buffer, or passed up the stack for further
> processing.  You read(2) the fd to access the ring buffer.
> 
> Using this mechanism, you could write a simple seccomptrace tool that
> needs no privilege and dumps SECCOMP_RET_LOG events from the target
> program to stderr.
> 
> Thoughts?

IMHO, this sounds like yet another logging daemon. It is also more
complex than what's needed for my use case of SECCOMP_RET_LOG. I only
need a seccomp learning mode, to accompany the learning modes
implemented in the various LSMs, that allows for everything marked with
RET_LOG to hit the syslog (or audit log).

Having to wedge seccomptrace tool between my application launcher, which
handles sandboxing, and the application being launched is less than
ideal. The application launcher could reimplement what seccomptrace is
doing but it'd be nice if that was left up to auditd/rsyslogd/journald/etc.

Tyler

> 
> [1] If we went this route, it might want to be renamed.
> 
> P.S. We ought to be able to write a BPF verifier pass that makes sure
> that filters don't return unsupported return values if we cared to do
> so.
> 


[toc] | [prev] | [next] | [standalone]


#1582990

FromKees Cook <keescook@chromium.org>
Date2017-02-17 00:30 +0100
Message-ID<tbDER-4Oo-7@gated-at.bofh.it>
In reply to#1582296
On Wed, Feb 15, 2017 at 7:24 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Mon, Feb 13, 2017 at 7:45 PM, Tyler Hicks <tyhicks@canonical.com> wrote:
>> This patch set is the third revision of the following two previously
>> submitted patch sets:
>>
>> v1: http://lkml.kernel.org/r/1483375990-14948-1-git-send-email-tyhicks@canonical.com
>> v1: http://lkml.kernel.org/r/1483377999-15019-2-git-send-email-tyhicks@canonical.com
>>
>> v2: http://lkml.kernel.org/r/1486100262-32391-1-git-send-email-tyhicks@canonical.com
>>
>> The patch set aims to address some known deficiencies in seccomp's current
>> logging capabilities:
>>
>>   1. Inability to log all filter actions.
>>   2. Inability to selectively enable filtering; e.g. devs want noisy logging,
>>      users want relative quiet.
>>   3. Consistent behavior with audit enabled and disabled.
>>   4. Inability to easily develop a filter due to the lack of a
>>      permissive/complain mode.
>
> I think I dislike this, but I think my dislikes may be fixable with
> minor changes.
>
> What I dislike is that this mixes app-specific built-in configuration
> (seccomp) with global privileged stuff (audit).  The result is a
> potentially difficult to use situation in which you need to modify an
> app to make it loggable (using RET_LOG) and then fiddle with
> privileged config (auditctl, etc) to actually see the logs.

You make a good point about RET_LOG vs log_max_action. I think making
RET_LOG the default value would work for 99% of the cases.

> What if, instead of logging straight to the audit log, SECCOMP_RET_LOG
> [1] merely meant "tell our parent about this syscall"?  (Ideally we'd
> also figure out a way to express "log this and allow", "log this and
> do ERRNO", etc.)  Then we could have another mechanism that installs a
> layer in the seccomp stack that, instead of catching syscalls, catches
> log events and sticks them in a ring buffer (or audit).

So, I really don't like this because it's yet another logging system.
We already have a security event logger: audit. This continues to use
that subsystem without changing semantics very much.

> Concretely, it might work like this.  If a filter returns
> SECCOMP_RET_LOG, then we "log" and keep processing.  SECCOMP_RET_LOG
> is otherwise treated literally like SECCOMP_RET_ALLOW and has no
> effect on return value.  If you want log-and-kill, you install two
> filters.
>
> There's a new seccomp(2) action that returns an fd.  That fd
> references a new thing in the seccomp stack that is a BPF program that
> is called whenever SECCOMP_RET_LOG is returned from lower down.  The
> output of this filter determines whether the log event is ignored,
> stuck in the ring buffer, or passed up the stack for further
> processing.  You read(2) the fd to access the ring buffer.
>
> Using this mechanism, you could write a simple seccomptrace tool that
> needs no privilege and dumps SECCOMP_RET_LOG events from the target
> program to stderr.

If someone was going to do this, they could just as well set up a
tracer to use RET_TRAP. (And this is what things like minijail does
already, IIRC.) The reality of the situation is that this is way too
much overhead for the common case. We need a generalized logging
system that uses the existing logging mechanisms.

> Thoughts?
>
> [1] If we went this route, it might want to be renamed.
>
> P.S. We ought to be able to write a BPF verifier pass that makes sure
> that filters don't return unsupported return values if we cared to do
> so.

Can we? I thought the BPF_RET used the BPF registers, and validating
that might be less-than-easy?

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]


#1583602

FromAndy Lutomirski <luto@amacapital.net>
Date2017-02-17 18:10 +0100
Message-ID<tbUcG-76j-19@gated-at.bofh.it>
In reply to#1582990
On Thu, Feb 16, 2017 at 3:29 PM, Kees Cook <keescook@chromium.org> wrote:
> On Wed, Feb 15, 2017 at 7:24 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Mon, Feb 13, 2017 at 7:45 PM, Tyler Hicks <tyhicks@canonical.com> wrote:
>>> This patch set is the third revision of the following two previously
>>> submitted patch sets:
>>>
>>> v1: http://lkml.kernel.org/r/1483375990-14948-1-git-send-email-tyhicks@canonical.com
>>> v1: http://lkml.kernel.org/r/1483377999-15019-2-git-send-email-tyhicks@canonical.com
>>>
>>> v2: http://lkml.kernel.org/r/1486100262-32391-1-git-send-email-tyhicks@canonical.com
>>>
>>> The patch set aims to address some known deficiencies in seccomp's current
>>> logging capabilities:
>>>
>>>   1. Inability to log all filter actions.
>>>   2. Inability to selectively enable filtering; e.g. devs want noisy logging,
>>>      users want relative quiet.
>>>   3. Consistent behavior with audit enabled and disabled.
>>>   4. Inability to easily develop a filter due to the lack of a
>>>      permissive/complain mode.
>>
>> I think I dislike this, but I think my dislikes may be fixable with
>> minor changes.
>>
>> What I dislike is that this mixes app-specific built-in configuration
>> (seccomp) with global privileged stuff (audit).  The result is a
>> potentially difficult to use situation in which you need to modify an
>> app to make it loggable (using RET_LOG) and then fiddle with
>> privileged config (auditctl, etc) to actually see the logs.
>
> You make a good point about RET_LOG vs log_max_action. I think making
> RET_LOG the default value would work for 99% of the cases.
>
>> What if, instead of logging straight to the audit log, SECCOMP_RET_LOG
>> [1] merely meant "tell our parent about this syscall"?  (Ideally we'd
>> also figure out a way to express "log this and allow", "log this and
>> do ERRNO", etc.)  Then we could have another mechanism that installs a
>> layer in the seccomp stack that, instead of catching syscalls, catches
>> log events and sticks them in a ring buffer (or audit).
>
> So, I really don't like this because it's yet another logging system.
> We already have a security event logger: audit. This continues to use
> that subsystem without changing semantics very much.

Audit sucks for this kind of thing, though.  It's mostly useless in a
container, for example.  But let me propose a middle ground.

>
>> Concretely, it might work like this.  If a filter returns
>> SECCOMP_RET_LOG, then we "log" and keep processing.  SECCOMP_RET_LOG
>> is otherwise treated literally like SECCOMP_RET_ALLOW and has no
>> effect on return value.  If you want log-and-kill, you install two
>> filters.
>>
>> There's a new seccomp(2) action that returns an fd.  That fd
>> references a new thing in the seccomp stack that is a BPF program that
>> is called whenever SECCOMP_RET_LOG is returned from lower down.  The
>> output of this filter determines whether the log event is ignored,
>> stuck in the ring buffer, or passed up the stack for further
>> processing.  You read(2) the fd to access the ring buffer.
>>
>> Using this mechanism, you could write a simple seccomptrace tool that
>> needs no privilege and dumps SECCOMP_RET_LOG events from the target
>> program to stderr.
>
> If someone was going to do this, they could just as well set up a
> tracer to use RET_TRAP. (And this is what things like minijail does
> already, IIRC.) The reality of the situation is that this is way too
> much overhead for the common case. We need a generalized logging
> system that uses the existing logging mechanisms.

True.  And we can always add this part later if we want to.

But let me propose a different, much more minor change to the patches:

First, we currently have seccomp_run_filters running the whole stack
and keeping (more or less) the lowest value.  What if we changed it a
bit so that return values of 0xff???????? were special.  Specifically,
a return value of 0xff?????? from a filter means "take some action
right now but don't change the outcome of the filter stack".  Then we
define SECCOMP_RET_LOG as 0xff000000 and perhaps reserve a few bits to
be a number reflected in the log entry.  (e.g. SECCOMP_RET_LOG(x) =
0xff000000 | (x & 0xff)).

Now SECCOMP_RET_LOG or SECCOMP_RET_LOG(0) does approximately what it
does in the current patch series if used in isolation, but you can
install two filters, one of which logs and one of which kills, to get
"log and kill".

If we do this, we might want SECCOMP_RET_KILL to stop running filters
so that filters farther up the stack don't log the syscall.

What do you think?  This should be a very small delta on top of the
current patches.

[toc] | [prev] | [next] | [standalone]


#1586393

FromKees Cook <keescook@chromium.org>
Date2017-02-22 19:40 +0100
Message-ID<tdJZw-5Qa-13@gated-at.bofh.it>
In reply to#1583602
On Fri, Feb 17, 2017 at 9:00 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Thu, Feb 16, 2017 at 3:29 PM, Kees Cook <keescook@chromium.org> wrote:
>> On Wed, Feb 15, 2017 at 7:24 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> If someone was going to do this, they could just as well set up a
>> tracer to use RET_TRAP. (And this is what things like minijail does
>> already, IIRC.) The reality of the situation is that this is way too
>> much overhead for the common case. We need a generalized logging
>> system that uses the existing logging mechanisms.
>
> True.  And we can always add this part later if we want to.
>
> But let me propose a different, much more minor change to the patches:
>
> First, we currently have seccomp_run_filters running the whole stack
> and keeping (more or less) the lowest value.  What if we changed it a
> bit so that return values of 0xff???????? were special.  Specifically,
> a return value of 0xff?????? from a filter means "take some action
> right now but don't change the outcome of the filter stack".  Then we
> define SECCOMP_RET_LOG as 0xff000000 and perhaps reserve a few bits to
> be a number reflected in the log entry.  (e.g. SECCOMP_RET_LOG(x) =
> 0xff000000 | (x & 0xff)).

I'm not a fan of adding more logic to the filter-running loop, but
this idea is tempting.

> Now SECCOMP_RET_LOG or SECCOMP_RET_LOG(0) does approximately what it
> does in the current patch series if used in isolation, but you can
> install two filters, one of which logs and one of which kills, to get
> "log and kill".
>
> If we do this, we might want SECCOMP_RET_KILL to stop running filters
> so that filters farther up the stack don't log the syscall.

I don't want to change the semantics of filter execution for this, so
I'd prefer to avoid adding an early abort.

> What do you think?  This should be a very small delta on top of the
> current patches.

What I don't like about this is that the logging and the action taken
are now totally separate. You could even end up having something
install multiple RET_LOGs, which aren't tied to what seccomp actually
decides to do in the end.

I'm not entirely opposed to the 0xff.... idea, but I don't think it
overlaps with logging very well. I would want seccomp logging to
reflect the actual action taken. Okay, I will now begin
stream-of-consciousness dumping to email...

I'm sure gmail will mangle whitespace, but here's sort of the 0xff
idea, well, actually 0x80....

diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
index 0f238a43ff1e..f61a0b783f6d 100644
--- a/include/uapi/linux/seccomp.h
+++ b/include/uapi/linux/seccomp.h
@@ -32,6 +32,7 @@
 #define SECCOMP_RET_ALLOW      0x7fff0000U /* allow */

 /* Masks for the return value sections. */
+#define SECCOMP_RET_OOB 0x80000000U
 #define SECCOMP_RET_ACTION     0x7fff0000U
 #define SECCOMP_RET_DATA       0x0000ffffU

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index f8f88ebcb3ba..4fac64fdfdc1 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -197,6 +197,9 @@ static u32 seccomp_run_filters(const struct
seccomp_data *sd)
        for (; f; f = f->prev) {
                u32 cur_ret = BPF_PROG_RUN(f->prog, sd);

+               if (unlikely(cur_ret & SECCOMP_RET_OOB))
+                       seccomp_oob_action(cur_ret);
+
                if ((cur_ret & SECCOMP_RET_ACTION) < (ret & SECCOMP_RET_ACTION))
                        ret = cur_ret;
        }

So, if SECCOMP_RET_OOB_LOG existed, it'd need to be installed as a
stand-alone filter, since it's still a BPF return value. That seems
... unfriendly ... as it would have to duplicate all the same logic as
another filter running along side it. e.g.:

filter 1 (actual actions):
- check arch
- check syscalls, jump to resolution
- ok: ret_allow
- unexpected: ret_allow
- weird: ret_errno
- bad: ret_kill

filter 2 ("oob" logging):
- check arch
- check syscalls, jump to resolution
- ok: ret_allow
- unexpected: ret_oob_log <- only difference
- weird: ret_errno
- bad: ret_kill

I mean, filter 2 could also just be:
- check arch
- check syscalls, jump to resolution
- unexpected: ret_oob_log
- everything else: ret_allow

But this kind of split logic seems needlessly complex and error-prone
on the filter developer's end. I don't think OOB logging should be
used here. Adding RET_LOG seems like the right approach to me.

The observations that it's per-process logging vs system-wide log
reporting is, I think, still problematic, but the case where a
developer is using RET_LOG is under non-production development and it
can be argued that the general case is developer==system owner, so
this is, again, sufficient.

So... I remain convinced that Tyler's series is the correct direction
to solve the bulk of the logging needs currently expressed by
developers and system owners.

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]


#1586402

FromKees Cook <keescook@chromium.org>
Date2017-02-22 19:50 +0100
Message-ID<tdK9c-5W6-31@gated-at.bofh.it>
In reply to#1582990
On Thu, Feb 16, 2017 at 3:29 PM, Kees Cook <keescook@chromium.org> wrote:
> On Wed, Feb 15, 2017 at 7:24 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Mon, Feb 13, 2017 at 7:45 PM, Tyler Hicks <tyhicks@canonical.com> wrote:
>>> This patch set is the third revision of the following two previously
>>> submitted patch sets:
>>>
>>> v1: http://lkml.kernel.org/r/1483375990-14948-1-git-send-email-tyhicks@canonical.com
>>> v1: http://lkml.kernel.org/r/1483377999-15019-2-git-send-email-tyhicks@canonical.com
>>>
>>> v2: http://lkml.kernel.org/r/1486100262-32391-1-git-send-email-tyhicks@canonical.com
>>>
>>> The patch set aims to address some known deficiencies in seccomp's current
>>> logging capabilities:
>>>
>>>   1. Inability to log all filter actions.
>>>   2. Inability to selectively enable filtering; e.g. devs want noisy logging,
>>>      users want relative quiet.
>>>   3. Consistent behavior with audit enabled and disabled.
>>>   4. Inability to easily develop a filter due to the lack of a
>>>      permissive/complain mode.
>>
>> I think I dislike this, but I think my dislikes may be fixable with
>> minor changes.
>>
>> What I dislike is that this mixes app-specific built-in configuration
>> (seccomp) with global privileged stuff (audit).  The result is a
>> potentially difficult to use situation in which you need to modify an
>> app to make it loggable (using RET_LOG) and then fiddle with
>> privileged config (auditctl, etc) to actually see the logs.
>
> You make a good point about RET_LOG vs log_max_action. I think making
> RET_LOG the default value would work for 99% of the cases.

Actually, I take this back: making "log" the default means that
everything else gets logged too, include "expected" return values like
errno, trap, etc. I think that would be extremely noisy as a default
(for upstream or Ubuntu).

Perhaps RET_LOG should unconditionally log? Or maybe the logged
actions should be a bit field instead of a single value? Then the
default could be "RET_KILL and RET_LOG", but an admin could switch it
to just RET_KILL, or even nothing at all? Hmmm...

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web