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

Groups > linux.kernel > #1238394 > unrolled thread

v5 of seccomp filter c/r patches

Back to article view | Back to linux.kernel

Contents

  v5 of seccomp filter c/r patches Tycho Andersen <tycho.andersen@canonical.com> - 2015-10-02 18:30 +0200
    Re: v5 of seccomp filter c/r patches Kees Cook <keescook@chromium.org> - 2015-10-02 23:20 +0200
      Re: v5 of seccomp filter c/r patches Andy Lutomirski <luto@amacapital.net> - 2015-10-02 23:40 +0200
        Re: v5 of seccomp filter c/r patches Kees Cook <keescook@chromium.org> - 2015-10-03 00:10 +0200
          Re: v5 of seccomp filter c/r patches Andy Lutomirski <luto@amacapital.net> - 2015-10-03 00:20 +0200
        Re: v5 of seccomp filter c/r patches Andy Lutomirski <luto@amacapital.net> - 2015-10-03 00:10 +0200
        Re: v5 of seccomp filter c/r patches Kees Cook <keescook@chromium.org> - 2015-10-03 00:10 +0200
      Re: v5 of seccomp filter c/r patches Tycho Andersen <tycho.andersen@canonical.com> - 2015-10-03 00:50 +0200
        Re: v5 of seccomp filter c/r patches Tycho Andersen <tycho.andersen@canonical.com> - 2015-10-03 01:00 +0200
        Re: v5 of seccomp filter c/r patches Daniel Borkmann <daniel@iogearbox.net> - 2015-10-03 01:00 +0200
          Re: v5 of seccomp filter c/r patches Tycho Andersen <tycho.andersen@canonical.com> - 2015-10-03 01:00 +0200
          Re: v5 of seccomp filter c/r patches Kees Cook <keescook@chromium.org> - 2015-10-03 01:10 +0200
        Re: v5 of seccomp filter c/r patches Andy Lutomirski <luto@amacapital.net> - 2015-10-03 01:00 +0200

#1238394 — v5 of seccomp filter c/r patches

Hi all,

Here's v5 of the seccomp filter c/r set. The individual patch notes have
changes, but two highlights are:

* This series is now based on http://patchwork.ozlabs.org/patch/525492/ and
  will need to be built with that patch applied. This gets rid of two incorrect
  patches in the previous series and is a nicer API.

* I couldn't figure out a nice way to have SECCOMP_GET_FILTER_FD return the
  same struct file across calls, so we still need a kcmp command. I've narrowed
  the scope of the one being added to only compare seccomp fds.

Thoughts welcome,

Tycho

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1238622

On Fri, Oct 2, 2015 at 9:27 AM, Tycho Andersen
<tycho.andersen@canonical.com> wrote:
> Hi all,
>
> Here's v5 of the seccomp filter c/r set. The individual patch notes have
> changes, but two highlights are:
>
> * This series is now based on http://patchwork.ozlabs.org/patch/525492/ and
>   will need to be built with that patch applied. This gets rid of two incorrect
>   patches in the previous series and is a nicer API.
>
> * I couldn't figure out a nice way to have SECCOMP_GET_FILTER_FD return the
>   same struct file across calls, so we still need a kcmp command. I've narrowed
>   the scope of the one being added to only compare seccomp fds.
>
> Thoughts welcome,

Hi, sorry I've been slow/busy. I'm finally reading through these threads.

Happy bit:
- avoiding eBPF and just saving the original filters makes things much easier.

Sad bit:
- inventing a new interface for seccompfds feels like massive overkill to me.

While Andy has big dreams, we're not presently doing seccompfd
monitoring, etc. There's no driving user for that kind of interface,
and accepting the maintenance burden of it only for CRIU seems unwise.

So, I'll go back to what I originally proposed at LSS (which it looks
like we're half way there now):

- save the original filter (done!)
- extract filters through a single special-purpose interface (looks
like ptrace is the way to go: root-only, stopped process, etc)
- compare filter content and issue TSYNCs to merge detected sibling
threads, since merging things that weren't merged before creates no
problems.

This means the parenting logic is heuristic, but it's entirely in
userspace, so the complexity burden doesn't live in seccomp which we,
by design, want to keep as simple as possible.

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1238628

On Fri, Oct 2, 2015 at 2:10 PM, Kees Cook <keescook@chromium.org> wrote:
> On Fri, Oct 2, 2015 at 9:27 AM, Tycho Andersen
> <tycho.andersen@canonical.com> wrote:
>> Hi all,
>>
>> Here's v5 of the seccomp filter c/r set. The individual patch notes have
>> changes, but two highlights are:
>>
>> * This series is now based on http://patchwork.ozlabs.org/patch/525492/ and
>>   will need to be built with that patch applied. This gets rid of two incorrect
>>   patches in the previous series and is a nicer API.
>>
>> * I couldn't figure out a nice way to have SECCOMP_GET_FILTER_FD return the
>>   same struct file across calls, so we still need a kcmp command. I've narrowed
>>   the scope of the one being added to only compare seccomp fds.
>>
>> Thoughts welcome,
>
> Hi, sorry I've been slow/busy. I'm finally reading through these threads.
>
> Happy bit:
> - avoiding eBPF and just saving the original filters makes things much easier.
>
> Sad bit:
> - inventing a new interface for seccompfds feels like massive overkill to me.
>
> While Andy has big dreams, we're not presently doing seccompfd
> monitoring, etc. There's no driving user for that kind of interface,
> and accepting the maintenance burden of it only for CRIU seems unwise.
>
> So, I'll go back to what I originally proposed at LSS (which it looks
> like we're half way there now):
>
> - save the original filter (done!)
> - extract filters through a single special-purpose interface (looks
> like ptrace is the way to go: root-only, stopped process, etc)
> - compare filter content and issue TSYNCs to merge detected sibling
> threads, since merging things that weren't merged before creates no
> problems.
>
> This means the parenting logic is heuristic, but it's entirely in
> userspace, so the complexity burden doesn't live in seccomp which we,
> by design, want to keep as simple as possible.

This is okay with me with a future-proofing caveat: I think that
whatever reads out the filter should be clearly documented as
returning some special error code that indicates that that filter it
tried to read wasn't in the expected form.  That would happen for
native eBPF filters, and it would also happen for seccomp monitors
even if those monitors use classic BPF.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1238648

On Fri, Oct 2, 2015 at 3:04 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Fri, Oct 2, 2015 at 3:02 PM, Kees Cook <keescook@chromium.org> wrote:
>> On Fri, Oct 2, 2015 at 2:29 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> On Fri, Oct 2, 2015 at 2:10 PM, Kees Cook <keescook@chromium.org> wrote:
>>>> On Fri, Oct 2, 2015 at 9:27 AM, Tycho Andersen
>>>> <tycho.andersen@canonical.com> wrote:
>>>>> Hi all,
>>>>>
>>>>> Here's v5 of the seccomp filter c/r set. The individual patch notes have
>>>>> changes, but two highlights are:
>>>>>
>>>>> * This series is now based on http://patchwork.ozlabs.org/patch/525492/ and
>>>>>   will need to be built with that patch applied. This gets rid of two incorrect
>>>>>   patches in the previous series and is a nicer API.
>>>>>
>>>>> * I couldn't figure out a nice way to have SECCOMP_GET_FILTER_FD return the
>>>>>   same struct file across calls, so we still need a kcmp command. I've narrowed
>>>>>   the scope of the one being added to only compare seccomp fds.
>>>>>
>>>>> Thoughts welcome,
>>>>
>>>> Hi, sorry I've been slow/busy. I'm finally reading through these threads.
>>>>
>>>> Happy bit:
>>>> - avoiding eBPF and just saving the original filters makes things much easier.
>>>>
>>>> Sad bit:
>>>> - inventing a new interface for seccompfds feels like massive overkill to me.
>>>>
>>>> While Andy has big dreams, we're not presently doing seccompfd
>>>> monitoring, etc. There's no driving user for that kind of interface,
>>>> and accepting the maintenance burden of it only for CRIU seems unwise.
>>>>
>>>> So, I'll go back to what I originally proposed at LSS (which it looks
>>>> like we're half way there now):
>>>>
>>>> - save the original filter (done!)
>>>> - extract filters through a single special-purpose interface (looks
>>>> like ptrace is the way to go: root-only, stopped process, etc)
>>>> - compare filter content and issue TSYNCs to merge detected sibling
>>>> threads, since merging things that weren't merged before creates no
>>>> problems.
>>>>
>>>> This means the parenting logic is heuristic, but it's entirely in
>>>> userspace, so the complexity burden doesn't live in seccomp which we,
>>>> by design, want to keep as simple as possible.
>>>
>>> This is okay with me with a future-proofing caveat: I think that
>>> whatever reads out the filter should be clearly documented as
>>> returning some special error code that indicates that that filter it
>>> tried to read wasn't in the expected form.  That would happen for
>>> native eBPF filters, and it would also happen for seccomp monitors
>>> even if those monitors use classic BPF.
>>
>> As in, it should have something like "give me BPF" and that'll start
>> failing when it's only eBPF in the future?
>
> Yes, but it might also start failing when if my dreams come true, it's
> still classic BPF, but it's no longer a classic seccomp bpf filter
> layer with the semantics we expect today.  (E.g. if it's classic bpf
> but has a monitor attached, then the read should fail because
> restoring it without restoring the monitor will cause all kinds of
> mess.)

Ah-ha! Understood, and yeah, that seems fine.

Speaking of dreams -- what do you think about re-running seccomp in
the face of changed syscalls due to ptrace? Closing the ptrace hole
would be really nice.

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1238655

On Fri, Oct 2, 2015 at 3:06 PM, Kees Cook <keescook@chromium.org> wrote:
> On Fri, Oct 2, 2015 at 3:04 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Fri, Oct 2, 2015 at 3:02 PM, Kees Cook <keescook@chromium.org> wrote:
>>> On Fri, Oct 2, 2015 at 2:29 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>> On Fri, Oct 2, 2015 at 2:10 PM, Kees Cook <keescook@chromium.org> wrote:
>>>>> On Fri, Oct 2, 2015 at 9:27 AM, Tycho Andersen
>>>>> <tycho.andersen@canonical.com> wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> Here's v5 of the seccomp filter c/r set. The individual patch notes have
>>>>>> changes, but two highlights are:
>>>>>>
>>>>>> * This series is now based on http://patchwork.ozlabs.org/patch/525492/ and
>>>>>>   will need to be built with that patch applied. This gets rid of two incorrect
>>>>>>   patches in the previous series and is a nicer API.
>>>>>>
>>>>>> * I couldn't figure out a nice way to have SECCOMP_GET_FILTER_FD return the
>>>>>>   same struct file across calls, so we still need a kcmp command. I've narrowed
>>>>>>   the scope of the one being added to only compare seccomp fds.
>>>>>>
>>>>>> Thoughts welcome,
>>>>>
>>>>> Hi, sorry I've been slow/busy. I'm finally reading through these threads.
>>>>>
>>>>> Happy bit:
>>>>> - avoiding eBPF and just saving the original filters makes things much easier.
>>>>>
>>>>> Sad bit:
>>>>> - inventing a new interface for seccompfds feels like massive overkill to me.
>>>>>
>>>>> While Andy has big dreams, we're not presently doing seccompfd
>>>>> monitoring, etc. There's no driving user for that kind of interface,
>>>>> and accepting the maintenance burden of it only for CRIU seems unwise.
>>>>>
>>>>> So, I'll go back to what I originally proposed at LSS (which it looks
>>>>> like we're half way there now):
>>>>>
>>>>> - save the original filter (done!)
>>>>> - extract filters through a single special-purpose interface (looks
>>>>> like ptrace is the way to go: root-only, stopped process, etc)
>>>>> - compare filter content and issue TSYNCs to merge detected sibling
>>>>> threads, since merging things that weren't merged before creates no
>>>>> problems.
>>>>>
>>>>> This means the parenting logic is heuristic, but it's entirely in
>>>>> userspace, so the complexity burden doesn't live in seccomp which we,
>>>>> by design, want to keep as simple as possible.
>>>>
>>>> This is okay with me with a future-proofing caveat: I think that
>>>> whatever reads out the filter should be clearly documented as
>>>> returning some special error code that indicates that that filter it
>>>> tried to read wasn't in the expected form.  That would happen for
>>>> native eBPF filters, and it would also happen for seccomp monitors
>>>> even if those monitors use classic BPF.
>>>
>>> As in, it should have something like "give me BPF" and that'll start
>>> failing when it's only eBPF in the future?
>>
>> Yes, but it might also start failing when if my dreams come true, it's
>> still classic BPF, but it's no longer a classic seccomp bpf filter
>> layer with the semantics we expect today.  (E.g. if it's classic bpf
>> but has a monitor attached, then the read should fail because
>> restoring it without restoring the monitor will cause all kinds of
>> mess.)
>
> Ah-ha! Understood, and yeah, that seems fine.
>
> Speaking of dreams -- what do you think about re-running seccomp in
> the face of changed syscalls due to ptrace? Closing the ptrace hole
> would be really nice.

Yes, absolutely!  We might even want to just move the seccomp check
after ptrace (except for seccomp-induced ptrace).

Unfortunately, I backed us into a corner with two-phase seccomp on
x86, and it's a big mess.  (I wrote the seccomp vs ptrace patches, and
I don't think they're acceptable.)  My big x86 low-level rewrite is an
attempt to get back out of that corner, and I'm hoping to resubmit the
bulk of it today or tomorrow.  Once that happens, I just need to fix
up the 64-bit native case (trivial, I know) and then revert two-phase
seccomp.

One nice outcome of all of this will be that the syscall tables will
contain bona fide C ABI compliant function pointers, which is
currently not the case.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1238649

On Fri, Oct 2, 2015 at 3:02 PM, Kees Cook <keescook@chromium.org> wrote:
> On Fri, Oct 2, 2015 at 2:29 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Fri, Oct 2, 2015 at 2:10 PM, Kees Cook <keescook@chromium.org> wrote:
>>> On Fri, Oct 2, 2015 at 9:27 AM, Tycho Andersen
>>> <tycho.andersen@canonical.com> wrote:
>>>> Hi all,
>>>>
>>>> Here's v5 of the seccomp filter c/r set. The individual patch notes have
>>>> changes, but two highlights are:
>>>>
>>>> * This series is now based on http://patchwork.ozlabs.org/patch/525492/ and
>>>>   will need to be built with that patch applied. This gets rid of two incorrect
>>>>   patches in the previous series and is a nicer API.
>>>>
>>>> * I couldn't figure out a nice way to have SECCOMP_GET_FILTER_FD return the
>>>>   same struct file across calls, so we still need a kcmp command. I've narrowed
>>>>   the scope of the one being added to only compare seccomp fds.
>>>>
>>>> Thoughts welcome,
>>>
>>> Hi, sorry I've been slow/busy. I'm finally reading through these threads.
>>>
>>> Happy bit:
>>> - avoiding eBPF and just saving the original filters makes things much easier.
>>>
>>> Sad bit:
>>> - inventing a new interface for seccompfds feels like massive overkill to me.
>>>
>>> While Andy has big dreams, we're not presently doing seccompfd
>>> monitoring, etc. There's no driving user for that kind of interface,
>>> and accepting the maintenance burden of it only for CRIU seems unwise.
>>>
>>> So, I'll go back to what I originally proposed at LSS (which it looks
>>> like we're half way there now):
>>>
>>> - save the original filter (done!)
>>> - extract filters through a single special-purpose interface (looks
>>> like ptrace is the way to go: root-only, stopped process, etc)
>>> - compare filter content and issue TSYNCs to merge detected sibling
>>> threads, since merging things that weren't merged before creates no
>>> problems.
>>>
>>> This means the parenting logic is heuristic, but it's entirely in
>>> userspace, so the complexity burden doesn't live in seccomp which we,
>>> by design, want to keep as simple as possible.
>>
>> This is okay with me with a future-proofing caveat: I think that
>> whatever reads out the filter should be clearly documented as
>> returning some special error code that indicates that that filter it
>> tried to read wasn't in the expected form.  That would happen for
>> native eBPF filters, and it would also happen for seccomp monitors
>> even if those monitors use classic BPF.
>
> As in, it should have something like "give me BPF" and that'll start
> failing when it's only eBPF in the future?

Yes, but it might also start failing when if my dreams come true, it's
still classic BPF, but it's no longer a classic seccomp bpf filter
layer with the semantics we expect today.  (E.g. if it's classic bpf
but has a monitor attached, then the read should fail because
restoring it without restoring the monitor will cause all kinds of
mess.)

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1238650

On Fri, Oct 2, 2015 at 2:29 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Fri, Oct 2, 2015 at 2:10 PM, Kees Cook <keescook@chromium.org> wrote:
>> On Fri, Oct 2, 2015 at 9:27 AM, Tycho Andersen
>> <tycho.andersen@canonical.com> wrote:
>>> Hi all,
>>>
>>> Here's v5 of the seccomp filter c/r set. The individual patch notes have
>>> changes, but two highlights are:
>>>
>>> * This series is now based on http://patchwork.ozlabs.org/patch/525492/ and
>>>   will need to be built with that patch applied. This gets rid of two incorrect
>>>   patches in the previous series and is a nicer API.
>>>
>>> * I couldn't figure out a nice way to have SECCOMP_GET_FILTER_FD return the
>>>   same struct file across calls, so we still need a kcmp command. I've narrowed
>>>   the scope of the one being added to only compare seccomp fds.
>>>
>>> Thoughts welcome,
>>
>> Hi, sorry I've been slow/busy. I'm finally reading through these threads.
>>
>> Happy bit:
>> - avoiding eBPF and just saving the original filters makes things much easier.
>>
>> Sad bit:
>> - inventing a new interface for seccompfds feels like massive overkill to me.
>>
>> While Andy has big dreams, we're not presently doing seccompfd
>> monitoring, etc. There's no driving user for that kind of interface,
>> and accepting the maintenance burden of it only for CRIU seems unwise.
>>
>> So, I'll go back to what I originally proposed at LSS (which it looks
>> like we're half way there now):
>>
>> - save the original filter (done!)
>> - extract filters through a single special-purpose interface (looks
>> like ptrace is the way to go: root-only, stopped process, etc)
>> - compare filter content and issue TSYNCs to merge detected sibling
>> threads, since merging things that weren't merged before creates no
>> problems.
>>
>> This means the parenting logic is heuristic, but it's entirely in
>> userspace, so the complexity burden doesn't live in seccomp which we,
>> by design, want to keep as simple as possible.
>
> This is okay with me with a future-proofing caveat: I think that
> whatever reads out the filter should be clearly documented as
> returning some special error code that indicates that that filter it
> tried to read wasn't in the expected form.  That would happen for
> native eBPF filters, and it would also happen for seccomp monitors
> even if those monitors use classic BPF.

As in, it should have something like "give me BPF" and that'll start
failing when it's only eBPF in the future?

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1238677

On Fri, Oct 02, 2015 at 02:10:24PM -0700, Kees Cook wrote:
> On Fri, Oct 2, 2015 at 9:27 AM, Tycho Andersen
> <tycho.andersen@canonical.com> wrote:
> > Hi all,
> >
> > Here's v5 of the seccomp filter c/r set. The individual patch notes have
> > changes, but two highlights are:
> >
> > * This series is now based on http://patchwork.ozlabs.org/patch/525492/ and
> >   will need to be built with that patch applied. This gets rid of two incorrect
> >   patches in the previous series and is a nicer API.
> >
> > * I couldn't figure out a nice way to have SECCOMP_GET_FILTER_FD return the
> >   same struct file across calls, so we still need a kcmp command. I've narrowed
> >   the scope of the one being added to only compare seccomp fds.
> >
> > Thoughts welcome,
> 
> Hi, sorry I've been slow/busy. I'm finally reading through these threads.
> 
> Happy bit:
> - avoiding eBPF and just saving the original filters makes things much easier.
> 
> Sad bit:
> - inventing a new interface for seccompfds feels like massive overkill to me.
> 
> While Andy has big dreams, we're not presently doing seccompfd
> monitoring, etc. There's no driving user for that kind of interface,
> and accepting the maintenance burden of it only for CRIU seems unwise.
> 
> So, I'll go back to what I originally proposed at LSS (which it looks
> like we're half way there now):
> 
> - save the original filter (done!)
> - extract filters through a single special-purpose interface (looks
> like ptrace is the way to go: root-only, stopped process, etc)
> - compare filter content and issue TSYNCs to merge detected sibling
> threads, since merging things that weren't merged before creates no
> problems.
> 
> This means the parenting logic is heuristic, but it's entirely in
> userspace, so the complexity burden doesn't live in seccomp which we,
> by design, want to keep as simple as possible.

Ok, how about,

struct sock_filter insns[BPF_MAXINSNS];
insn_cnt = ptrace(PTRACE_SECCOMP_GET_FILTER, pid, insns, i);

when asking for the ith filter? It returns either the number of
instructions, -EINVAL if something was wrong (i, pid,
CONFIG_CHECKPOINT_RESTORE isn't enabled). While it would always
succeed now, if/when the underlying filter was not created from a bpf
classic filter, we can return -EMEDIUMTYPE? (Suggestions welcome, I
picked this mostly based on what sounds nice.)

Tycho
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1238680

On Fri, Oct 02, 2015 at 03:52:03PM -0700, Andy Lutomirski wrote:
> On Fri, Oct 2, 2015 at 3:44 PM, Tycho Andersen
> <tycho.andersen@canonical.com> wrote:
> > On Fri, Oct 02, 2015 at 02:10:24PM -0700, Kees Cook wrote:
> >> On Fri, Oct 2, 2015 at 9:27 AM, Tycho Andersen
> >> <tycho.andersen@canonical.com> wrote:
> >> > Hi all,
> >> >
> >> > Here's v5 of the seccomp filter c/r set. The individual patch notes have
> >> > changes, but two highlights are:
> >> >
> >> > * This series is now based on http://patchwork.ozlabs.org/patch/525492/ and
> >> >   will need to be built with that patch applied. This gets rid of two incorrect
> >> >   patches in the previous series and is a nicer API.
> >> >
> >> > * I couldn't figure out a nice way to have SECCOMP_GET_FILTER_FD return the
> >> >   same struct file across calls, so we still need a kcmp command. I've narrowed
> >> >   the scope of the one being added to only compare seccomp fds.
> >> >
> >> > Thoughts welcome,
> >>
> >> Hi, sorry I've been slow/busy. I'm finally reading through these threads.
> >>
> >> Happy bit:
> >> - avoiding eBPF and just saving the original filters makes things much easier.
> >>
> >> Sad bit:
> >> - inventing a new interface for seccompfds feels like massive overkill to me.
> >>
> >> While Andy has big dreams, we're not presently doing seccompfd
> >> monitoring, etc. There's no driving user for that kind of interface,
> >> and accepting the maintenance burden of it only for CRIU seems unwise.
> >>
> >> So, I'll go back to what I originally proposed at LSS (which it looks
> >> like we're half way there now):
> >>
> >> - save the original filter (done!)
> >> - extract filters through a single special-purpose interface (looks
> >> like ptrace is the way to go: root-only, stopped process, etc)
> >> - compare filter content and issue TSYNCs to merge detected sibling
> >> threads, since merging things that weren't merged before creates no
> >> problems.
> >>
> >> This means the parenting logic is heuristic, but it's entirely in
> >> userspace, so the complexity burden doesn't live in seccomp which we,
> >> by design, want to keep as simple as possible.
> >
> > Ok, how about,
> >
> > struct sock_filter insns[BPF_MAXINSNS];
> > insn_cnt = ptrace(PTRACE_SECCOMP_GET_FILTER, pid, insns, i);
> >
> > when asking for the ith filter? It returns either the number of
> > instructions, -EINVAL if something was wrong (i, pid,
> > CONFIG_CHECKPOINT_RESTORE isn't enabled). While it would always
> > succeed now, if/when the underlying filter was not created from a bpf
> > classic filter, we can return -EMEDIUMTYPE? (Suggestions welcome, I
> > picked this mostly based on what sounds nice.)
> >
> 
> Are we still requiring global permissions or that the caller isn't
> seccomped at all?  I've not lost track of how we're resolving the case
> where the caller and the tracee have exactly the same seccomp state
> (or the tracee is derived from the caller's state or they're totally
> unrelated states).

At least for now, I think requiring real root and no seccomp is fine,
so I'll do that.

Tycho
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1238681

On 10/03/2015 12:44 AM, Tycho Andersen wrote:
> On Fri, Oct 02, 2015 at 02:10:24PM -0700, Kees Cook wrote:
...
> Ok, how about,
>
> struct sock_filter insns[BPF_MAXINSNS];
> insn_cnt = ptrace(PTRACE_SECCOMP_GET_FILTER, pid, insns, i);

Would also be good that when the storage buffer (insns) is NULL,
it just returns you the number of sock_filter insns (or 0 when
nothing attached).

That would be consistent with classic socket filters (see
sk_get_filter()), and user space could allocate a specific
size instead of always passing in max insns.

> when asking for the ith filter? It returns either the number of
> instructions, -EINVAL if something was wrong (i, pid,
> CONFIG_CHECKPOINT_RESTORE isn't enabled). While it would always
> succeed now, if/when the underlying filter was not created from a bpf
> classic filter, we can return -EMEDIUMTYPE? (Suggestions welcome, I
> picked this mostly based on what sounds nice.)
>
> Tycho
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1238690

On Sat, Oct 03, 2015 at 12:57:49AM +0200, Daniel Borkmann wrote:
> On 10/03/2015 12:44 AM, Tycho Andersen wrote:
> >On Fri, Oct 02, 2015 at 02:10:24PM -0700, Kees Cook wrote:
> ...
> >Ok, how about,
> >
> >struct sock_filter insns[BPF_MAXINSNS];
> >insn_cnt = ptrace(PTRACE_SECCOMP_GET_FILTER, pid, insns, i);
> 
> Would also be good that when the storage buffer (insns) is NULL,
> it just returns you the number of sock_filter insns (or 0 when
> nothing attached).
> 
> That would be consistent with classic socket filters (see
> sk_get_filter()), and user space could allocate a specific
> size instead of always passing in max insns.

Yep, the current set does this with SECCOMP_FD_DUMP and I agree that
it's nice behavior, so I'll plan on preserving it.

Thanks,

Tycho
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1238691

On Fri, Oct 2, 2015 at 3:57 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 10/03/2015 12:44 AM, Tycho Andersen wrote:
>>
>> On Fri, Oct 02, 2015 at 02:10:24PM -0700, Kees Cook wrote:
>
> ...
>>
>> Ok, how about,
>>
>> struct sock_filter insns[BPF_MAXINSNS];
>> insn_cnt = ptrace(PTRACE_SECCOMP_GET_FILTER, pid, insns, i);
>
>
> Would also be good that when the storage buffer (insns) is NULL,
> it just returns you the number of sock_filter insns (or 0 when
> nothing attached).
>
> That would be consistent with classic socket filters (see
> sk_get_filter()), and user space could allocate a specific
> size instead of always passing in max insns.

Yes please. :)

>> when asking for the ith filter? It returns either the number of
>> instructions, -EINVAL if something was wrong (i, pid,
>> CONFIG_CHECKPOINT_RESTORE isn't enabled). While it would always
>> succeed now, if/when the underlying filter was not created from a bpf
>> classic filter, we can return -EMEDIUMTYPE? (Suggestions welcome, I
>> picked this mostly based on what sounds nice.)

We can bikeshed the non-classic case when we need it, but I think
EINVAL is "not under seccomp", and ENOENT is "no such index".

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1238687

On Fri, Oct 2, 2015 at 3:44 PM, Tycho Andersen
<tycho.andersen@canonical.com> wrote:
> On Fri, Oct 02, 2015 at 02:10:24PM -0700, Kees Cook wrote:
>> On Fri, Oct 2, 2015 at 9:27 AM, Tycho Andersen
>> <tycho.andersen@canonical.com> wrote:
>> > Hi all,
>> >
>> > Here's v5 of the seccomp filter c/r set. The individual patch notes have
>> > changes, but two highlights are:
>> >
>> > * This series is now based on http://patchwork.ozlabs.org/patch/525492/ and
>> >   will need to be built with that patch applied. This gets rid of two incorrect
>> >   patches in the previous series and is a nicer API.
>> >
>> > * I couldn't figure out a nice way to have SECCOMP_GET_FILTER_FD return the
>> >   same struct file across calls, so we still need a kcmp command. I've narrowed
>> >   the scope of the one being added to only compare seccomp fds.
>> >
>> > Thoughts welcome,
>>
>> Hi, sorry I've been slow/busy. I'm finally reading through these threads.
>>
>> Happy bit:
>> - avoiding eBPF and just saving the original filters makes things much easier.
>>
>> Sad bit:
>> - inventing a new interface for seccompfds feels like massive overkill to me.
>>
>> While Andy has big dreams, we're not presently doing seccompfd
>> monitoring, etc. There's no driving user for that kind of interface,
>> and accepting the maintenance burden of it only for CRIU seems unwise.
>>
>> So, I'll go back to what I originally proposed at LSS (which it looks
>> like we're half way there now):
>>
>> - save the original filter (done!)
>> - extract filters through a single special-purpose interface (looks
>> like ptrace is the way to go: root-only, stopped process, etc)
>> - compare filter content and issue TSYNCs to merge detected sibling
>> threads, since merging things that weren't merged before creates no
>> problems.
>>
>> This means the parenting logic is heuristic, but it's entirely in
>> userspace, so the complexity burden doesn't live in seccomp which we,
>> by design, want to keep as simple as possible.
>
> Ok, how about,
>
> struct sock_filter insns[BPF_MAXINSNS];
> insn_cnt = ptrace(PTRACE_SECCOMP_GET_FILTER, pid, insns, i);
>
> when asking for the ith filter? It returns either the number of
> instructions, -EINVAL if something was wrong (i, pid,
> CONFIG_CHECKPOINT_RESTORE isn't enabled). While it would always
> succeed now, if/when the underlying filter was not created from a bpf
> classic filter, we can return -EMEDIUMTYPE? (Suggestions welcome, I
> picked this mostly based on what sounds nice.)
>

Are we still requiring global permissions or that the caller isn't
seccomped at all?  I've not lost track of how we're resolving the case
where the caller and the tracee have exactly the same seccomp state
(or the tracee is derived from the caller's state or they're totally
unrelated states).

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web