On 02/22/2017 12:46 PM, Kees Cook wrote:
> On Thu, Feb 16, 2017 at 3:29 PM, Kees Cook <[email protected]> wrote:
>> On Wed, Feb 15, 2017 at 7:24 PM, Andy Lutomirski <[email protected]> wrote:
>>> On Mon, Feb 13, 2017 at 7:45 PM, Tyler Hicks <[email protected]> wrote:
>>>> This patch set is the third revision of the following two previously
>>>> submitted patch sets:
>>>>
>>>> v1: http://lkml.kernel.org/r/[email protected]
>>>> v1: http://lkml.kernel.org/r/[email protected]
>>>>
>>>> v2: http://lkml.kernel.org/r/[email protected]
>>>>
>>>> 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...
Hi Kees - my apologies for going quiet on this topic after we spoke
about it more in IRC. I needed to tend to other work but now I'm able to
return to this seccomp logging patch set.
To summarize what we discussed in IRC, the Chrome browser makes
extensive use of RET_ERRNO, RET_TRACE, etc., to sandbox code that may
not ever be adjusted to keep from bump into the sandbox walls.
Therefore, it is unreasonable to enable logging of something like
RET_ERRNO on a system-wide level where Chrome browser is in use.
In contrast, snapd wants to set up "noisier" sandboxes for applications
to make it clear to the developers and the users that the sandboxed
application is bumping into the sandbox walls. Developers will know why
their code may not be working as intended and users will know that the
application is doing things that the platform doesn't agree with. These
sandboxes will end up using RET_ERRNO in the majority of cases.
This means that with the current design of this patch set, Chrome
browser will either be unintentionally spamming the logs or snapd's
sandboxes will be helplessly silent when both Chrome and snapd is
installed at the same time, depending on the admin's preferences.
To bring it back up a level, two applications may have a very different
outlook on how acceptable a given seccomp action is and they may
disagree on whether or not the user/administrator should be informed.
I've been giving thought to the idea of providing a way for the
application setting up the filter to opt into logging of certain
actions. Here's a high level breakdown:
- The administrator will have control of system-wide seccomp logging
and the application will have control of how its seccomp actions are
logged
- Both the administrator's and application's logging preferences are
bitmasks of actions that should be logged
- The default administrator bitmask is set to log all actions except
RET_ALLOW
+ Configurable via a sysctl
+ Very similar to the log_max_action syscall that was proposed in
this patch set but a bitmask instead of a max action
- The default application bitmask is set to log only RET_KILL and
RET_LOG
+ Configurable via the seccomp(2) syscall (details TBD)
- Actions are only logged when the application has requested the
action to be logged and the administrator has allowed the action to
be logged
+ By default, this is only RET_KILL and RET_LOG actions
Let me know what you think about this and I can turn out another patch
set next week if it sounds reasonable.
Tyler
On Fri, Apr 7, 2017 at 3:16 PM, Tyler Hicks <[email protected]> wrote:
> On 02/22/2017 12:46 PM, Kees Cook wrote:
>> On Thu, Feb 16, 2017 at 3:29 PM, Kees Cook <[email protected]> wrote:
>>> On Wed, Feb 15, 2017 at 7:24 PM, Andy Lutomirski <[email protected]> wrote:
>>>> On Mon, Feb 13, 2017 at 7:45 PM, Tyler Hicks <[email protected]> wrote:
>>>>> This patch set is the third revision of the following two previously
>>>>> submitted patch sets:
>>>>>
>>>>> v1: http://lkml.kernel.org/r/[email protected]
>>>>> v1: http://lkml.kernel.org/r/[email protected]
>>>>>
>>>>> v2: http://lkml.kernel.org/r/[email protected]
>>>>>
>>>>> 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...
>
> Hi Kees - my apologies for going quiet on this topic after we spoke
> about it more in IRC. I needed to tend to other work but now I'm able to
> return to this seccomp logging patch set.
>
> To summarize what we discussed in IRC, the Chrome browser makes
> extensive use of RET_ERRNO, RET_TRACE, etc., to sandbox code that may
> not ever be adjusted to keep from bump into the sandbox walls.
> Therefore, it is unreasonable to enable logging of something like
> RET_ERRNO on a system-wide level where Chrome browser is in use.
>
> In contrast, snapd wants to set up "noisier" sandboxes for applications
> to make it clear to the developers and the users that the sandboxed
> application is bumping into the sandbox walls. Developers will know why
> their code may not be working as intended and users will know that the
> application is doing things that the platform doesn't agree with. These
> sandboxes will end up using RET_ERRNO in the majority of cases.
>
> This means that with the current design of this patch set, Chrome
> browser will either be unintentionally spamming the logs or snapd's
> sandboxes will be helplessly silent when both Chrome and snapd is
> installed at the same time, depending on the admin's preferences.
>
> To bring it back up a level, two applications may have a very different
> outlook on how acceptable a given seccomp action is and they may
> disagree on whether or not the user/administrator should be informed.
>
> I've been giving thought to the idea of providing a way for the
> application setting up the filter to opt into logging of certain
> actions. Here's a high level breakdown:
>
> - The administrator will have control of system-wide seccomp logging
> and the application will have control of how its seccomp actions are
> logged
> - Both the administrator's and application's logging preferences are
> bitmasks of actions that should be logged
> - The default administrator bitmask is set to log all actions except
> RET_ALLOW
> + Configurable via a sysctl
> + Very similar to the log_max_action syscall that was proposed in
> this patch set but a bitmask instead of a max action
> - The default application bitmask is set to log only RET_KILL and
> RET_LOG
> + Configurable via the seccomp(2) syscall (details TBD)
> - Actions are only logged when the application has requested the
> action to be logged and the administrator has allowed the action to
> be logged
> + By default, this is only RET_KILL and RET_LOG actions
>
> Let me know what you think about this and I can turn out another patch
> set next week if it sounds reasonable.
This seems good to me. With a bitmask we don't have to play crazy
games with levels, and with the app able to declare what it wants
logged, we get the flexibility needed by app developers.
One change I think I'd like to make is that an app can't block
RET_KILL from being logged (the sysadmin can, though). What do think
of that minor tweak?
Is RET_ALLOW allowed to be logged by an application? I guess with it
default-off for an admin, it should be okay.
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?
-Kees
--
Kees Cook
Pixel Security
On 04/07/2017 05:46 PM, Kees Cook wrote:
> On Fri, Apr 7, 2017 at 3:16 PM, Tyler Hicks <[email protected]> wrote:
>> On 02/22/2017 12:46 PM, Kees Cook wrote:
>>> On Thu, Feb 16, 2017 at 3:29 PM, Kees Cook <[email protected]> wrote:
>>>> On Wed, Feb 15, 2017 at 7:24 PM, Andy Lutomirski <[email protected]> wrote:
>>>>> On Mon, Feb 13, 2017 at 7:45 PM, Tyler Hicks <[email protected]> wrote:
>>>>>> This patch set is the third revision of the following two previously
>>>>>> submitted patch sets:
>>>>>>
>>>>>> v1: http://lkml.kernel.org/r/[email protected]
>>>>>> v1: http://lkml.kernel.org/r/[email protected]
>>>>>>
>>>>>> v2: http://lkml.kernel.org/r/[email protected]
>>>>>>
>>>>>> 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...
>>
>> Hi Kees - my apologies for going quiet on this topic after we spoke
>> about it more in IRC. I needed to tend to other work but now I'm able to
>> return to this seccomp logging patch set.
>>
>> To summarize what we discussed in IRC, the Chrome browser makes
>> extensive use of RET_ERRNO, RET_TRACE, etc., to sandbox code that may
>> not ever be adjusted to keep from bump into the sandbox walls.
>> Therefore, it is unreasonable to enable logging of something like
>> RET_ERRNO on a system-wide level where Chrome browser is in use.
>>
>> In contrast, snapd wants to set up "noisier" sandboxes for applications
>> to make it clear to the developers and the users that the sandboxed
>> application is bumping into the sandbox walls. Developers will know why
>> their code may not be working as intended and users will know that the
>> application is doing things that the platform doesn't agree with. These
>> sandboxes will end up using RET_ERRNO in the majority of cases.
>>
>> This means that with the current design of this patch set, Chrome
>> browser will either be unintentionally spamming the logs or snapd's
>> sandboxes will be helplessly silent when both Chrome and snapd is
>> installed at the same time, depending on the admin's preferences.
>>
>> To bring it back up a level, two applications may have a very different
>> outlook on how acceptable a given seccomp action is and they may
>> disagree on whether or not the user/administrator should be informed.
>>
>> I've been giving thought to the idea of providing a way for the
>> application setting up the filter to opt into logging of certain
>> actions. Here's a high level breakdown:
>>
>> - The administrator will have control of system-wide seccomp logging
>> and the application will have control of how its seccomp actions are
>> logged
>> - Both the administrator's and application's logging preferences are
>> bitmasks of actions that should be logged
>> - The default administrator bitmask is set to log all actions except
>> RET_ALLOW
>> + Configurable via a sysctl
>> + Very similar to the log_max_action syscall that was proposed in
>> this patch set but a bitmask instead of a max action
>> - The default application bitmask is set to log only RET_KILL and
>> RET_LOG
>> + Configurable via the seccomp(2) syscall (details TBD)
>> - Actions are only logged when the application has requested the
>> action to be logged and the administrator has allowed the action to
>> be logged
>> + By default, this is only RET_KILL and RET_LOG actions
>>
>> Let me know what you think about this and I can turn out another patch
>> set next week if it sounds reasonable.
>
> This seems good to me. With a bitmask we don't have to play crazy
> games with levels, and with the app able to declare what it wants
> logged, we get the flexibility needed by app developers.
>
> One change I think I'd like to make is that an app can't block
> RET_KILL from being logged (the sysadmin can, though). What do think
> of that minor tweak?
That's fine from my point of view.
>
> Is RET_ALLOW allowed to be logged by an application? I guess with it
> default-off for an admin, it should be okay.
As you say, it should be fine to allow but I don't know if there's any
real use in it. I can go either way here.
>
> 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.
Tyler
On Friday, April 7, 2017 6:16:08 PM EDT Tyler Hicks wrote:
> On 02/22/2017 12:46 PM, Kees Cook wrote:
> > On Thu, Feb 16, 2017 at 3:29 PM, Kees Cook <[email protected]> wrote:
> >> On Wed, Feb 15, 2017 at 7:24 PM, Andy Lutomirski <[email protected]>
wrote:
> >>> On Mon, Feb 13, 2017 at 7:45 PM, Tyler Hicks <[email protected]>
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@can
> >>>> onical.com v1:
> >>>> http://lkml.kernel.org/r/1483377999-15019-2-git-send-email-tyhicks@can
> >>>> onical.com
> >>>>
> >>>> v2:
> >>>> http://lkml.kernel.org/r/1486100262-32391-1-git-send-email-tyhicks@can
> >>>> onical.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...
>
> Hi Kees - my apologies for going quiet on this topic after we spoke
> about it more in IRC. I needed to tend to other work but now I'm able to
> return to this seccomp logging patch set.
>
> To summarize what we discussed in IRC, the Chrome browser makes
> extensive use of RET_ERRNO, RET_TRACE, etc., to sandbox code that may
> not ever be adjusted to keep from bump into the sandbox walls.
> Therefore, it is unreasonable to enable logging of something like
> RET_ERRNO on a system-wide level where Chrome browser is in use.
Just to illustrate what can happen, /usr/lib64/qt5/libexec/QtWebEngineProcess
is causing hundreds of errno SECCOMP audit events.
See bz https://bugzilla.redhat.com/show_bug.cgi?id=1438973
The developers should not have enabled this for distribution's runtime. It
should be a debug option.
-Steve
> In contrast, snapd wants to set up "noisier" sandboxes for applications
> to make it clear to the developers and the users that the sandboxed
> application is bumping into the sandbox walls. Developers will know why
> their code may not be working as intended and users will know that the
> application is doing things that the platform doesn't agree with. These
> sandboxes will end up using RET_ERRNO in the majority of cases.
>
> This means that with the current design of this patch set, Chrome
> browser will either be unintentionally spamming the logs or snapd's
> sandboxes will be helplessly silent when both Chrome and snapd is
> installed at the same time, depending on the admin's preferences.
>
> To bring it back up a level, two applications may have a very different
> outlook on how acceptable a given seccomp action is and they may
> disagree on whether or not the user/administrator should be informed.
>
> I've been giving thought to the idea of providing a way for the
> application setting up the filter to opt into logging of certain
> actions. Here's a high level breakdown:
>
> - The administrator will have control of system-wide seccomp logging
> and the application will have control of how its seccomp actions are
> logged
> - Both the administrator's and application's logging preferences are
> bitmasks of actions that should be logged
> - The default administrator bitmask is set to log all actions except
> RET_ALLOW
> + Configurable via a sysctl
> + Very similar to the log_max_action syscall that was proposed in
> this patch set but a bitmask instead of a max action
> - The default application bitmask is set to log only RET_KILL and
> RET_LOG
> + Configurable via the seccomp(2) syscall (details TBD)
> - Actions are only logged when the application has requested the
> action to be logged and the administrator has allowed the action to
> be logged
> + By default, this is only RET_KILL and RET_LOG actions
>
> Let me know what you think about this and I can turn out another patch
> set next week if it sounds reasonable.
>
> Tyler
On Fri, Apr 7, 2017 at 3:16 PM, Tyler Hicks <[email protected]> wrote:
> On 02/22/2017 12:46 PM, Kees Cook wrote:
>> On Thu, Feb 16, 2017 at 3:29 PM, Kees Cook <[email protected]> wrote:
>>> On Wed, Feb 15, 2017 at 7:24 PM, Andy Lutomirski <[email protected]> wrote:
>>>> On Mon, Feb 13, 2017 at 7:45 PM, Tyler Hicks <[email protected]> wrote:
>>>>> This patch set is the third revision of the following two previously
>>>>> submitted patch sets:
>>>>>
>>>>> v1: http://lkml.kernel.org/r/[email protected]
>>>>> v1: http://lkml.kernel.org/r/[email protected]
>>>>>
>>>>> v2: http://lkml.kernel.org/r/[email protected]
>>>>>
>>>>> 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...
>
> Hi Kees - my apologies for going quiet on this topic after we spoke
> about it more in IRC. I needed to tend to other work but now I'm able to
> return to this seccomp logging patch set.
>
> To summarize what we discussed in IRC, the Chrome browser makes
> extensive use of RET_ERRNO, RET_TRACE, etc., to sandbox code that may
> not ever be adjusted to keep from bump into the sandbox walls.
> Therefore, it is unreasonable to enable logging of something like
> RET_ERRNO on a system-wide level where Chrome browser is in use.
>
> In contrast, snapd wants to set up "noisier" sandboxes for applications
> to make it clear to the developers and the users that the sandboxed
> application is bumping into the sandbox walls. Developers will know why
> their code may not be working as intended and users will know that the
> application is doing things that the platform doesn't agree with. These
> sandboxes will end up using RET_ERRNO in the majority of cases.
>
> This means that with the current design of this patch set, Chrome
> browser will either be unintentionally spamming the logs or snapd's
> sandboxes will be helplessly silent when both Chrome and snapd is
> installed at the same time, depending on the admin's preferences.
>
> To bring it back up a level, two applications may have a very different
> outlook on how acceptable a given seccomp action is and they may
> disagree on whether or not the user/administrator should be informed.
>
> I've been giving thought to the idea of providing a way for the
> application setting up the filter to opt into logging of certain
> actions. Here's a high level breakdown:
At the risk of overcomplicating things, I think this issue may be a
decent argument for doing something more like what I suggested
earlier: let seccomp users emit loggable things and let their parents
(optionally) catch and handle those things. Then we get real scoping
rather than fiddling with bitmasks and hoping we get what we want to
see without generating massive log spam.
--Andy
On 04/10/2017 10:57 AM, Andy Lutomirski wrote:
> On Fri, Apr 7, 2017 at 3:16 PM, Tyler Hicks <[email protected]> wrote:
>> On 02/22/2017 12:46 PM, Kees Cook wrote:
>>> On Thu, Feb 16, 2017 at 3:29 PM, Kees Cook <[email protected]> wrote:
>>>> On Wed, Feb 15, 2017 at 7:24 PM, Andy Lutomirski <[email protected]> wrote:
>>>>> On Mon, Feb 13, 2017 at 7:45 PM, Tyler Hicks <[email protected]> wrote:
>>>>>> This patch set is the third revision of the following two previously
>>>>>> submitted patch sets:
>>>>>>
>>>>>> v1: http://lkml.kernel.org/r/[email protected]
>>>>>> v1: http://lkml.kernel.org/r/[email protected]
>>>>>>
>>>>>> v2: http://lkml.kernel.org/r/[email protected]
>>>>>>
>>>>>> 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...
>>
>> Hi Kees - my apologies for going quiet on this topic after we spoke
>> about it more in IRC. I needed to tend to other work but now I'm able to
>> return to this seccomp logging patch set.
>>
>> To summarize what we discussed in IRC, the Chrome browser makes
>> extensive use of RET_ERRNO, RET_TRACE, etc., to sandbox code that may
>> not ever be adjusted to keep from bump into the sandbox walls.
>> Therefore, it is unreasonable to enable logging of something like
>> RET_ERRNO on a system-wide level where Chrome browser is in use.
>>
>> In contrast, snapd wants to set up "noisier" sandboxes for applications
>> to make it clear to the developers and the users that the sandboxed
>> application is bumping into the sandbox walls. Developers will know why
>> their code may not be working as intended and users will know that the
>> application is doing things that the platform doesn't agree with. These
>> sandboxes will end up using RET_ERRNO in the majority of cases.
>>
>> This means that with the current design of this patch set, Chrome
>> browser will either be unintentionally spamming the logs or snapd's
>> sandboxes will be helplessly silent when both Chrome and snapd is
>> installed at the same time, depending on the admin's preferences.
>>
>> To bring it back up a level, two applications may have a very different
>> outlook on how acceptable a given seccomp action is and they may
>> disagree on whether or not the user/administrator should be informed.
>>
>> I've been giving thought to the idea of providing a way for the
>> application setting up the filter to opt into logging of certain
>> actions. Here's a high level breakdown:
>
> At the risk of overcomplicating things, I think this issue may be a
> decent argument for doing something more like what I suggested
> earlier: let seccomp users emit loggable things and let their parents
> (optionally) catch and handle those things. Then we get real scoping
> rather than fiddling with bitmasks and hoping we get what we want to
> see without generating massive log spam.
Hi Andy - I remembered your proposal and considered it when I was
writing up mine. I still feel like it is easier to get the logging
bitmask right, if you even need to adjust the default bitmask, than to
force parent processes to act as a relay for log messages.
The logging bitmasks can easily be adjusted in debug builds or when a
user specifies a runtime option to enable seccomp logging in a program
that sets up filters.
Another potential issue with the parent process handling the logging is
that it isn't always possible when audit is in use.
audit_log_user_message() requires CAP_AUDIT_WRITE (seems like
CAP_AUDIT_CONTROL might also be required but I can't remember why) so
unprivileged processes would have to divert their log messages elsewhere.
Tyler
On Fri, Apr 7, 2017 at 4:46 PM, Tyler Hicks <[email protected]> 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
On Mon, Apr 10, 2017 at 8:57 AM, Andy Lutomirski <[email protected]> wrote:
> On Fri, Apr 7, 2017 at 3:16 PM, Tyler Hicks <[email protected]> wrote:
>> On 02/22/2017 12:46 PM, Kees Cook wrote:
>>> On Thu, Feb 16, 2017 at 3:29 PM, Kees Cook <[email protected]> wrote:
>>>> On Wed, Feb 15, 2017 at 7:24 PM, Andy Lutomirski <[email protected]> wrote:
>>>>> On Mon, Feb 13, 2017 at 7:45 PM, Tyler Hicks <[email protected]> wrote:
>>>>>> This patch set is the third revision of the following two previously
>>>>>> submitted patch sets:
>>>>>>
>>>>>> v1: http://lkml.kernel.org/r/[email protected]
>>>>>> v1: http://lkml.kernel.org/r/[email protected]
>>>>>>
>>>>>> v2: http://lkml.kernel.org/r/[email protected]
>>>>>>
>>>>>> 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...
>>
>> Hi Kees - my apologies for going quiet on this topic after we spoke
>> about it more in IRC. I needed to tend to other work but now I'm able to
>> return to this seccomp logging patch set.
>>
>> To summarize what we discussed in IRC, the Chrome browser makes
>> extensive use of RET_ERRNO, RET_TRACE, etc., to sandbox code that may
>> not ever be adjusted to keep from bump into the sandbox walls.
>> Therefore, it is unreasonable to enable logging of something like
>> RET_ERRNO on a system-wide level where Chrome browser is in use.
>>
>> In contrast, snapd wants to set up "noisier" sandboxes for applications
>> to make it clear to the developers and the users that the sandboxed
>> application is bumping into the sandbox walls. Developers will know why
>> their code may not be working as intended and users will know that the
>> application is doing things that the platform doesn't agree with. These
>> sandboxes will end up using RET_ERRNO in the majority of cases.
>>
>> This means that with the current design of this patch set, Chrome
>> browser will either be unintentionally spamming the logs or snapd's
>> sandboxes will be helplessly silent when both Chrome and snapd is
>> installed at the same time, depending on the admin's preferences.
>>
>> To bring it back up a level, two applications may have a very different
>> outlook on how acceptable a given seccomp action is and they may
>> disagree on whether or not the user/administrator should be informed.
>>
>> I've been giving thought to the idea of providing a way for the
>> application setting up the filter to opt into logging of certain
>> actions. Here's a high level breakdown:
>
> At the risk of overcomplicating things, I think this issue may be a
> decent argument for doing something more like what I suggested
> earlier: let seccomp users emit loggable things and let their parents
> (optionally) catch and handle those things. Then we get real scoping
> rather than fiddling with bitmasks and hoping we get what we want to
> see without generating massive log spam.
I still think this is overkill. We already have an out-of-band logging
system, and having something that is tied to the parent duplicates
much of ptrace monitor capabilities without really adding much. I'd
like to continue with the syslog/auditd approach until we have an
actual use-case like what you've described. I think Tyler (and others,
e.g. openWRT) have demonstrated a need that would be addressed via the
syslog path.
-Kees
--
Kees Cook
Pixel Security
On 04/10/2017 10:59 PM, Kees Cook wrote:
> On Fri, Apr 7, 2017 at 4:46 PM, Tyler Hicks <[email protected]> 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
On Thu, Apr 27, 2017 at 3:17 PM, Tyler Hicks <[email protected]> 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