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

Groups > linux.kernel > #1632412 > unrolled thread

Re: [PATCH v3 0/4] Improved seccomp logging

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v3 0/4] Improved seccomp logging Tyler Hicks <tyhicks@canonical.com> - 2017-04-28 00:20 +0200
    Re: [PATCH v3 0/4] Improved seccomp logging Kees Cook <keescook@chromium.org> - 2017-04-28 01:50 +0200
      Re: [PATCH v3 0/4] Improved seccomp logging Tyler Hicks <tyhicks@canonical.com> - 2017-05-02 04:50 +0200
        Re: [PATCH v3 0/4] Improved seccomp logging Andy Lutomirski <luto@kernel.org> - 2017-05-02 18:20 +0200

#1632412 — Re: [PATCH v3 0/4] Improved seccomp logging

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

On 04/10/2017 10:59 PM, Kees Cook wrote:
> 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.)

Quick update... I finished the move from the high-water mark
log_max_action sysctl to the bitmask based actions_logged sysctl.

Unfortunately, I've just realized that SECCOMP_SET_LOGGING, or any
process-wide logging configuration mechanism, will not work. It is fine
for the situation where two unrelated processes set up seccomp filters
that should be logged differently. However, it fails when two closely
related processes, such as parent and child, need to set up seccomp
filters that should be logged differently. Imagine a launcher that sets
up an application sandbox (including a seccomp filter) and then launches
an electron app which will have its own seccomp filter for sandboxing
untrusted code that it runs. Unless the launcher and app completely
agree on actions that should be logged, the logging won't work as
intended for both processes.

I think this needs to be configured at the filter level.

Tyler

[toc] | [next] | [standalone]

#1632445

On Thu, Apr 27, 2017 at 3:17 PM, Tyler Hicks <tyhicks@canonical.com> wrote:
> Quick update... I finished the move from the high-water mark
> log_max_action sysctl to the bitmask based actions_logged sysctl.

Awesome!

> Unfortunately, I've just realized that SECCOMP_SET_LOGGING, or any
> process-wide logging configuration mechanism, will not work. It is fine
> for the situation where two unrelated processes set up seccomp filters
> that should be logged differently. However, it fails when two closely
> related processes, such as parent and child, need to set up seccomp
> filters that should be logged differently. Imagine a launcher that sets
> up an application sandbox (including a seccomp filter) and then launches
> an electron app which will have its own seccomp filter for sandboxing
> untrusted code that it runs. Unless the launcher and app completely
> agree on actions that should be logged, the logging won't work as
> intended for both processes.

Oh, you mean the forked process sets up the logging it wants for the
filters it just installed, then after exec a process sets up new
logging requirements?

> I think this needs to be configured at the filter level.

I'm not sure that's even the right way to compose the logging desires.

So, my initial thought was "whatever ran SECCOMP_SET_LOGGING knows
what it's doing" and it should be the actual value.

If the launcher wants logs of everything the application does with its
filters, then a purely-tied-to-filter approach won't work either.

Perhaps log bits can only be enabled? I.e. SECCOMP_SET_LOGGING
performs an OR instead of an assignment?

-Kees

-- 
Kees Cook
Pixel Security

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

#1634089

On 04/27/2017 07:42 PM, Kees Cook wrote:
> On Thu, Apr 27, 2017 at 3:17 PM, Tyler Hicks <tyhicks@canonical.com> wrote:
>> Quick update... I finished the move from the high-water mark
>> log_max_action sysctl to the bitmask based actions_logged sysctl.
> 
> Awesome!
> 
>> Unfortunately, I've just realized that SECCOMP_SET_LOGGING, or any
>> process-wide logging configuration mechanism, will not work. It is fine
>> for the situation where two unrelated processes set up seccomp filters
>> that should be logged differently. However, it fails when two closely
>> related processes, such as parent and child, need to set up seccomp
>> filters that should be logged differently. Imagine a launcher that sets
>> up an application sandbox (including a seccomp filter) and then launches
>> an electron app which will have its own seccomp filter for sandboxing
>> untrusted code that it runs. Unless the launcher and app completely
>> agree on actions that should be logged, the logging won't work as
>> intended for both processes.
> 
> Oh, you mean the forked process sets up the logging it wants for the
> filters it just installed, then after exec a process sets up new
> logging requirements?

Yes - see below.

> 
>> I think this needs to be configured at the filter level.
> 
> I'm not sure that's even the right way to compose the logging desires.
> 
> So, my initial thought was "whatever ran SECCOMP_SET_LOGGING knows
> what it's doing" and it should be the actual value.
> 
> If the launcher wants logs of everything the application does with its
> filters, then a purely-tied-to-filter approach won't work either.
> 
> Perhaps log bits can only be enabled? I.e. SECCOMP_SET_LOGGING
> performs an OR instead of an assignment?

The problem that I'm envisioning with this design is this:

1. Launcher is told to launch Chrome and forks off a process.

2. Launcher sets up a filter using RET_ERRNO for all unacceptable
syscalls and enables auditing of RET_ERRNO.

3. Launcher execs Chrome.

4. Chrome then sets up its own, more restrictive filter that uses
RET_ERRNO, among other actions, but does not want auditing of RET_ERRNO.

If we use process-wide auditing controls, the logs will be filled with
RET_ERRNO messages that were unintended and unrelated to the RET_ERRNO
actions set up in the launcher's filter.

Unfortunately, the OR'ing idea doesn't solve the problem.

Tyler

> 
> -Kees
> 

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

#1634499

On Mon, May 1, 2017 at 7:41 PM, Tyler Hicks <tyhicks@canonical.com> wrote:
> On 04/27/2017 07:42 PM, Kees Cook wrote:
>> On Thu, Apr 27, 2017 at 3:17 PM, Tyler Hicks <tyhicks@canonical.com> wrote:
>>> Quick update... I finished the move from the high-water mark
>>> log_max_action sysctl to the bitmask based actions_logged sysctl.
>>
>> Awesome!
>>
>>> Unfortunately, I've just realized that SECCOMP_SET_LOGGING, or any
>>> process-wide logging configuration mechanism, will not work. It is fine
>>> for the situation where two unrelated processes set up seccomp filters
>>> that should be logged differently. However, it fails when two closely
>>> related processes, such as parent and child, need to set up seccomp
>>> filters that should be logged differently. Imagine a launcher that sets
>>> up an application sandbox (including a seccomp filter) and then launches
>>> an electron app which will have its own seccomp filter for sandboxing
>>> untrusted code that it runs. Unless the launcher and app completely
>>> agree on actions that should be logged, the logging won't work as
>>> intended for both processes.
>>
>> Oh, you mean the forked process sets up the logging it wants for the
>> filters it just installed, then after exec a process sets up new
>> logging requirements?
>
> Yes - see below.
>
>>
>>> I think this needs to be configured at the filter level.
>>
>> I'm not sure that's even the right way to compose the logging desires.
>>
>> So, my initial thought was "whatever ran SECCOMP_SET_LOGGING knows
>> what it's doing" and it should be the actual value.
>>
>> If the launcher wants logs of everything the application does with its
>> filters, then a purely-tied-to-filter approach won't work either.
>>
>> Perhaps log bits can only be enabled? I.e. SECCOMP_SET_LOGGING
>> performs an OR instead of an assignment?
>
> The problem that I'm envisioning with this design is this:
>
> 1. Launcher is told to launch Chrome and forks off a process.
>
> 2. Launcher sets up a filter using RET_ERRNO for all unacceptable
> syscalls and enables auditing of RET_ERRNO.
>
> 3. Launcher execs Chrome.
>
> 4. Chrome then sets up its own, more restrictive filter that uses
> RET_ERRNO, among other actions, but does not want auditing of RET_ERRNO.
>
> If we use process-wide auditing controls, the logs will be filled with
> RET_ERRNO messages that were unintended and unrelated to the RET_ERRNO
> actions set up in the launcher's filter.
>
> Unfortunately, the OR'ing idea doesn't solve the problem.

Things like my more complicated solution solve this completely, I
think.  The launcher would, by whatever means, say "RET_ERRNO and log
this".  The more restrictive sandbox would say "RET_ERROR and don't
log this" and we'd just make sure that the composition rules mean the
inner rule wins.

--Andy

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web