Re: seccomp feature development

On Tue, May 19, 2020 at 09:18:47AM -0700, Alexei Starovoitov wrote:

On Mon, May 18, 2020 at 7:53 PM Aleksa Sarai [off-list ref] wrote:

quoted

On 2020-05-19, Jann Horn [off-list ref] wrote:

quoted

On Mon, May 18, 2020 at 11:05 PM Kees Cook [off-list ref] wrote:

quoted

## deep argument inspection

The argument caching bit is, I think, rather mechanical in nature since
it's all "just" internal to the kernel: seccomp can likely adjust how it
allocates seccomp_data (maybe going so far as to have it split across two
pages with the syscall argument struct always starting on the 2nd page
boundary), and copying the EA struct into that page, which will be both
used by the filter and by the syscall.

We could also do the same kind of thing the eBPF verifier does in
convert_ctx_accesses(), and rewrite the context accesses to actually
go through two different pointers depending on the (constant) offset
into seccomp_data.

My main worry with this is that we'll need to figure out what kind of
offset mathematics are necessary to deal with pointers inside the
extensible struct. As a very ugly proposal, you could make it so that
you multiply the offset by PAGE_SIZE each time you want to dereference
the pointer at that offset (unless we want to add new opcodes to cBPF to
allow us to represent this).

Please don't. cbpf is frozen.

https://www.youtube.com/watch?v=L0MK7qz13bU

If the only workable design paths for deep arg inspection end up needing
BPF helpers, I would agree that it's time for seccomp to grow eBPF
language support. I'm still hoping there's a clean solution that doesn't
require a seccomp language extension.

quoted

quoted

We don't need to actually zero-fill memory for this beyond what the
kernel supports - AFAIK the existing APIs already say that passing a
short length is equivalent to passing zeroes, so we can just replace
all out-of-bounds loads with zeroing registers in the filter.
The tricky case is what should happen if the userspace program passes
in fields that the filter doesn't know about. The filter can see the
length field passed in by userspace, and then just reject anything
where the length field is bigger than the structure size the filter
knows about. But maybe it'd be slightly nicer if there was an
operation for "tell me whether everything starting at offset X is
zeroes", so that if someone compiles with newer kernel headers where
the struct is bigger, and leaves the new fields zeroed, the syscalls
still go through an outdated filter properly.

I think the best way of handling this (without breaking programs
senselessly) is to have filters essentially emulate
copy_struct_from_user() semantics -- which is along the lines of what
you've suggested.

and cpbf load instruction will become copy_from_user() underneath?

No, this was meaning internal checking about struct sizes needs to exist
(not the user copy parts).

I don't see how that can work.
Have you considered implications to jits, register usage, etc ?

ebpf will become sleepable soon. It will be able to do copy_from_user()
and examine any level of user pointer dereference.
toctou is still going to be a concern though,
but such ebpf+copy_from_user analysis and syscall sandboxing
will not need to change kernel code base around syscalls at all.
No need to invent E-syscalls and all the rest I've seen in this thread.

To avoid the ToCToU, the seccomp infrastructure must do the
copy_from_user(), so there's not need for the sleepable stuff in seccomp
that I can see. The question is mainly one of flattening.

-- 
Kees Cook