linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Tyler Hicks <tyhicks@canonical.com>
Cc: Will Drewry <wad@chromium.org>,
	Linux API <linux-api@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Andy Lutomirski <luto@amacapital.net>,
	linux-audit@redhat.com, John Crispin <john@phrozen.org>
Subject: Re: [PATCH v3 0/4] Improved seccomp logging
Date: Mon, 10 Apr 2017 20:59:15 -0700	[thread overview]
Message-ID: <CAGXu5j+x-4E8z0TK6je9oLa6yLENzQ30jNH2dXsFkGdzTUaSsQ@mail.gmail.com> (raw)
In-Reply-To: <a914076f-efb8-d867-f480-69addca4e628@canonical.com>

On Fri, Apr 7, 2017 at 4:46 PM, Tyler Hicks <tyhicks@canonical.com> wrote:
> On 04/07/2017 05:46 PM, Kees Cook wrote:
>> Does the app-controlled bitmask apply to the filter, the process, the
>> process tree, or something else? e.g. systemd launches an app with a
>> filter, leaving the defaults alone, then later process installs a
>> filter and wants everything logged -- will things from the earlier
>> filter get logged?
>
> I think implementation preferences may decide many of these questions.
> As I see it, here are the options in order of my preference:
>
> A) Claim the MSB of the filter return value and make the app logging
>    preference per-rule
>    - If the bit is set, log the rule
>    - Provides very fine-grained logging control at the "high cost" of
>      the remaining free bit in the filter return bitmask
>    - The bit can be ignored in the case of RET_KILL
>    - Can be synced across all threads in the calling task with the
>      SECCOMP_FILTER_FLAG_TSYNC filter flag
>
> B) Claim a few bits in the filter flags and make the app logging
>    preference per-filter
>    - Something like SECCOMP_FILTER_FLAG_LOG_TRAP,
>      SECCOMP_FILTER_FLAG_LOG_ERRNO, and
>      SECCOMP_FILTER_FLAG_LOG_TRACE
>    - Logging for RET_KILL and RET_LOG can't be turned off
>    - I'd prefer not to waste a bit for RET_ALLOW in this case so it
>      simply won't be loggable
>    - Works with the SECCOMP_FILTER_FLAG_TSYNC filter flag
>    - Doesn't scale well if many new actions are added in the future
>
> C) A simplified version of 'B' where only a single mode bit is claimed
>    to enable logging for all actions except RET_ALLOW
>    - Something like SECCOMP_FILTER_FLAG_LOG_ACTIONS
>    - Filters without this flag only log RET_KILL and RET_LOG
>    - Scales much better than 'B' at the expense of less flexibility
>    - Works with the SECCOMP_FILTER_FLAG_TSYNC filter flag
>
> D) Claim a bit in the filter mode and make the app logging preference
>    per-process
>    - This new SECCOMP_MODE_ENABLE_LOGGING mode would take a bitmask of
>      actions that should be logged
>    - Incurs a small per-task increase in memory footprint in the form
>      of an additional member in 'struct seccomp'
>    - Has odd behavior you described above where launchers may set the
>      logging preference and then launched application may want
>      something different
>
> I think 'A' is the cleanest design but I don't know if highly
> configurable logging is deserving of the MSB bit in the filter return.
> I'd like to hear your thoughts there.
>
> I _barely_ prefer 'B' over 'C'. They're essential equal in my use case.
>
> To be honest, I haven't completely wrapped my head around how 'D' would
> actually work in practice so I may be writing it off prematurely.
>
> Am I missing any more clever options that you can think of? Let me know
> what you think of the possibilities.

Hmm, so, I think we can just make this a bitmask in the process
seccomp struct. It'll get inherited across forks, and any filter that
wants to make sure it never changes again can just blacklist the
seccomp syscall with that argument. I don't see anything about the
logging that should be considered private, considering the logs are
going through syslog or auditd. Since it's already out-of-band, this
won't change the behavior of ptrace monitors, etc.

So, how about seccomp(SECCOMP_SET_LOGGING, flags, user_ptr) and
...GET_LOGGING? flags likely 0, and user_ptr can point to:

struct seccomp_logging {
    u32 count;
    u32 values[];
};

Where each value entry is a filter return value to log. (That way
bitmasks are just an internal storage detail and we're allowed to add
new filter returns without breaking a bitmask UAPI.)

-Kees

-- 
Kees Cook
Pixel Security

  reply	other threads:[~2017-04-11  3:59 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-14  3:45 [PATCH v3 0/4] Improved seccomp logging Tyler Hicks
2017-02-14  3:45 ` [PATCH v3 1/4] seccomp: Add sysctl to display available actions Tyler Hicks
2017-02-16  1:00   ` Kees Cook
2017-02-16  3:14   ` Andy Lutomirski
2017-02-14  3:45 ` [PATCH v3 2/4] seccomp: Add sysctl to configure actions that should be logged Tyler Hicks
2017-02-14  3:45 ` [PATCH v3 3/4] seccomp: Create an action to log before allowing Tyler Hicks
2017-02-14  3:45 ` [PATCH v3 4/4] seccomp: Add tests for SECCOMP_RET_LOG Tyler Hicks
2017-02-16  3:24 ` [PATCH v3 0/4] Improved seccomp logging Andy Lutomirski
2017-02-16 23:29   ` Kees Cook
2017-02-17 17:00     ` Andy Lutomirski
2017-02-22 18:39       ` Kees Cook
     [not found]     ` <CAGXu5j+muyh2bwtMXDHuUHsDV9ZyEY-hMHrJjVuX2vC20MVSZw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-02-22 18:46       ` Kees Cook
     [not found]         ` <CAGXu5jLtLgYmDJDfGA2EtfB7Fqze-SP768ezq=fgWZ=X-ObW3w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-04-07 22:16           ` Tyler Hicks
     [not found]             ` <ac79529e-f6b6-690c-e597-5adeb75b0f25-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
2017-04-07 22:46               ` Kees Cook
     [not found]                 ` <CAGXu5j+qUOpnDeF4TMH2AXXgHZB_WfHHfxe3TBSShmneisR-Lg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-04-07 23:46                   ` Tyler Hicks
2017-04-11  3:59                     ` Kees Cook [this message]
2017-04-27 22:17                       ` Tyler Hicks
     [not found]                         ` <0b1a2337-7006-e7cb-f519-dec389ede041-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
2017-04-27 23:42                           ` Kees Cook
2017-05-02  2:41                             ` Tyler Hicks
2017-05-02 16:14                               ` Andy Lutomirski
2017-04-10 15:18               ` Steve Grubb
2017-04-10 15:57             ` Andy Lutomirski
     [not found]               ` <CALCETrXJKtnXmzRHs=7mEXN7FVAYjzxKb=jwrqwXQoXB0dHHPg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-04-10 19:22                 ` Tyler Hicks
2017-04-11  4:03               ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAGXu5j+x-4E8z0TK6je9oLa6yLENzQ30jNH2dXsFkGdzTUaSsQ@mail.gmail.com \
    --to=keescook@chromium.org \
    --cc=john@phrozen.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-audit@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=tyhicks@canonical.com \
    --cc=wad@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).