Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload

2018-10-10 Thread Alexei Starovoitov
On Mon, Oct 08, 2018 at 01:57:40PM +0200, Jiri Olsa wrote:
> 
> I check that discussion and it's related only to bpf program load/unload,
> is there any plan to also notify about bpf program attachment?
> 
> in the step 2 you described:
> 
>   step 2 (future work)
>   single event for bpf prog_load with prog_id only.
>   Either via perf ring buffer or ftrace or tracepoints or some
>   other notification mechanism.
> 
> would you see this to be feasible also for bpf prog attachment notification?

I agree that on the first glance ring buffer notifications
for program attachment sound useful, but I have a hard time
seeing how we can build the complete solution on top of them.
progs can be attached via netlink, perf_event ioctl, bpf syscall.
Theoretically we can insert ring buffer events in all those places,
but there is no common format we can use. In networking cases
ifindex alone won't be enough, since there is xdp vs tc vs lwt vs etc.
Furthermore the program can run without being attached to anything.
Like the way folks use xdp is they load mini bpf prog that dispatches
all other progs via prog_array via tail_call mechanism.
So to execute newly loaded program users space only needs to store its FD
into prog_array.
Would you want to add notifications for all map updates then as well?
What would be the format of such notification? map_id and slot index?
but how audit daemon will now that this particular map is used
for running and under what conditions?
Single bpf dispatcher program can use multiple prog_arrays.
It seems to me that attach notifications are not a practical way
to introspect the "bpf program execution graph" in the kernel.
I suggest to take a look at bpftool. It can inspect networking,
cgroup, tracing progs already and show what programs are loaded and where
they are attached to. I think improving bpftool style of introspection
is more practical than inventing notifications for everything.



Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload

2018-10-08 Thread Jiri Olsa
On Thu, Oct 04, 2018 at 03:10:15PM -0700, Alexei Starovoitov wrote:
> On Thu, Oct 04, 2018 at 10:22:31PM +0200, Jesper Dangaard Brouer wrote:
> > On Thu, 4 Oct 2018 21:41:17 +0200 Daniel Borkmann  
> > wrote:
> > 
> > > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov 
> > > >  wrote:  
> > > >> On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote:  
> > [...]
> > > >>
> > > >> If the purpose of the patch is to give user space visibility into
> > > >> bpf prog load/unload as a notification, then I completely agree that
> > > >> some notification mechanism is necessary.  
> > > 
> > > Yeah, I did only regard it as only that, nothing more. Some means
> > > of timeline and notification that can be kept in a record in user
> > > space and later retrieved e.g. for introspection on what has been
> > > loaded.
> > > 
> > > >> I've started working on such mechanism via perf ring buffer which is
> > > >> the fastest mechanism we have in the kernel so far.
> > > >> See long discussion here: https://patchwork.ozlabs.org/patch/971970/  

I check that discussion and it's related only to bpf program load/unload,
is there any plan to also notify about bpf program attachment?

in the step 2 you described:

  step 2 (future work)
  single event for bpf prog_load with prog_id only.
  Either via perf ring buffer or ftrace or tracepoints or some
  other notification mechanism.

would you see this to be feasible also for bpf prog attachment notification?

thanks,
jirka


Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload

2018-10-07 Thread Jesper Dangaard Brouer
On Sat, 6 Oct 2018 00:05:22 +0200
Jiri Olsa  wrote:

> On Fri, Oct 05, 2018 at 11:44:35AM -0700, Alexei Starovoitov wrote:
> > On Fri, Oct 05, 2018 at 08:14:09AM +0200, Jiri Olsa wrote:  
> > > On Thu, Oct 04, 2018 at 03:10:15PM -0700, Alexei Starovoitov wrote:  
> > > > On Thu, Oct 04, 2018 at 10:22:31PM +0200, Jesper Dangaard Brouer wrote: 
> > > >  
> > > > > On Thu, 4 Oct 2018 21:41:17 +0200 Daniel Borkmann 
> > > > >  wrote:
> > > > >   
> > > > > > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:  
> > > > > > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov 
> > > > > > >  wrote:
> > > > > > >> On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote: 
> > > > > > >>
> > > > > [...]  
> > > > > > >>
> > > > > > >> If the purpose of the patch is to give user space visibility into
> > > > > > >> bpf prog load/unload as a notification, then I completely agree 
> > > > > > >> that
> > > > > > >> some notification mechanism is necessary.
> > > > > > 
> > > > > > Yeah, I did only regard it as only that, nothing more. Some means
> > > > > > of timeline and notification that can be kept in a record in user
> > > > > > space and later retrieved e.g. for introspection on what has been
> > > > > > loaded.
> > > > > >   
> > > > > > >> I've started working on such mechanism via perf ring buffer 
> > > > > > >> which is
> > > > > > >> the fastest mechanism we have in the kernel so far.
> > > > > > >> See long discussion here: 
> > > > > > >> https://patchwork.ozlabs.org/patch/971970/
> > > 
[...]
> > > > > > 
> > > > > > That one is definitely needed in any case to resolve the kallsyms
> > > > > > limitations, and it does have overlap in that in either case we
> > > > > > want to look at past BPF programs that have been unloaded in the
> > > > > > meantime, so I don't have a strong preference either way, and the
> > > > > > former is needed in any case. Though thought was that audit might
> > > > > > be an option for those not running profiling daemons 24/7, but
> > > > > > presumably bpftool could be extended to record these events as
> > > > > > well if we don't want to reuse audit infra.  
> > > > > 
> > > > > Yes, exactly, I don't want to run a profiling daemon 24/7 to record
> > > > > these events.  I do acknowledge that this perf event is relevant,
> > > > > especially for catching the kernel symbols (I need that myself), but 
> > > > > it
> > > > > does not cover my use-case.
> > > > > 
> > > > > My use-case is to 24/7 collect and keep records in userspace, and 
> > > > > have a
> > > > > timeline of these notifications, for later retrieval.  The idea is 
> > > > > that
> > > > > our support engineers can look at these records when troubleshooting
> > > > > the system.  And the plan is also to collect these records as part of
> > > > > our sosreport tool, which is part of the support case.  
> > > > 
> > > > I don't think you're implying that prog load/unload should be spamming 
> > > > dmesg
> > > > and auditd not even running...  
> > > 
> > > I think the problem Jesper implied is that in order to collect
> > > those logs you'll need perf tool running all the time.. which
> > > it's not equipped for yet  
> > 
> > I'm not proposing to run 'perf' binary all the time.
> > Setting up perf ring buffer just for these new bpf prog load/unload events
> > and epolling it is simple enough to do from any application including 
> > auditd.
> > selftests/bpf/ do it for bpf output events.  
> 
> ok, did not think about the possibility to teach auditd talk to perf,
> time to get that tool evsel/evlist/rb library ready ;-)

Interesting, I also didn't consider teaching auditd to gets its 'bpf'
events from a separate perf ring-buffer, that might work.  I do wonder
how the audit people will take this suggestion.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload

2018-10-05 Thread Jiri Olsa
On Fri, Oct 05, 2018 at 11:44:35AM -0700, Alexei Starovoitov wrote:
> On Fri, Oct 05, 2018 at 08:14:09AM +0200, Jiri Olsa wrote:
> > On Thu, Oct 04, 2018 at 03:10:15PM -0700, Alexei Starovoitov wrote:
> > > On Thu, Oct 04, 2018 at 10:22:31PM +0200, Jesper Dangaard Brouer wrote:
> > > > On Thu, 4 Oct 2018 21:41:17 +0200 Daniel Borkmann 
> > > >  wrote:
> > > > 
> > > > > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > > > > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov 
> > > > > >  wrote:  
> > > > > >> On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote:  
> > > > [...]
> > > > > >>
> > > > > >> If the purpose of the patch is to give user space visibility into
> > > > > >> bpf prog load/unload as a notification, then I completely agree 
> > > > > >> that
> > > > > >> some notification mechanism is necessary.  
> > > > > 
> > > > > Yeah, I did only regard it as only that, nothing more. Some means
> > > > > of timeline and notification that can be kept in a record in user
> > > > > space and later retrieved e.g. for introspection on what has been
> > > > > loaded.
> > > > > 
> > > > > >> I've started working on such mechanism via perf ring buffer which 
> > > > > >> is
> > > > > >> the fastest mechanism we have in the kernel so far.
> > > > > >> See long discussion here: 
> > > > > >> https://patchwork.ozlabs.org/patch/971970/  
> > 
> > cool, could you please CC me if there's another version
> > of that patchset?
> 
> will do.

thanks

> 
> > > > > 
> > > > > That one is definitely needed in any case to resolve the kallsyms
> > > > > limitations, and it does have overlap in that in either case we
> > > > > want to look at past BPF programs that have been unloaded in the
> > > > > meantime, so I don't have a strong preference either way, and the
> > > > > former is needed in any case. Though thought was that audit might
> > > > > be an option for those not running profiling daemons 24/7, but
> > > > > presumably bpftool could be extended to record these events as
> > > > > well if we don't want to reuse audit infra.
> > > > 
> > > > Yes, exactly, I don't want to run a profiling daemon 24/7 to record
> > > > these events.  I do acknowledge that this perf event is relevant,
> > > > especially for catching the kernel symbols (I need that myself), but it
> > > > does not cover my use-case.
> > > > 
> > > > My use-case is to 24/7 collect and keep records in userspace, and have a
> > > > timeline of these notifications, for later retrieval.  The idea is that
> > > > our support engineers can look at these records when troubleshooting
> > > > the system.  And the plan is also to collect these records as part of
> > > > our sosreport tool, which is part of the support case.
> > > 
> > > I don't think you're implying that prog load/unload should be spamming 
> > > dmesg
> > > and auditd not even running...
> > 
> > I think the problem Jesper implied is that in order to collect
> > those logs you'll need perf tool running all the time.. which
> > it's not equipped for yet
> 
> I'm not proposing to run 'perf' binary all the time.
> Setting up perf ring buffer just for these new bpf prog load/unload events
> and epolling it is simple enough to do from any application including auditd.
> selftests/bpf/ do it for bpf output events.

ok, did not think about the possibility to teach auditd talk to perf,
time to get that tool evsel/evlist/rb library ready ;-)

jirka


Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload

2018-10-05 Thread Alexei Starovoitov
On Fri, Oct 05, 2018 at 04:42:49PM -0300, Arnaldo Carvalho de Melo wrote:
> 
> Is there a way for us to synthesize those prog load/unload for
> preexisting loaded bpf objects?

see 'bpftool prog show'.
get_next_id + get_fd_by_id solve it race free.



Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload

2018-10-05 Thread Arnaldo Carvalho de Melo
Em Fri, Oct 05, 2018 at 11:44:35AM -0700, Alexei Starovoitov escreveu:
> On Fri, Oct 05, 2018 at 08:14:09AM +0200, Jiri Olsa wrote:
> > On Thu, Oct 04, 2018 at 03:10:15PM -0700, Alexei Starovoitov wrote:
> > > On Thu, Oct 04, 2018 at 10:22:31PM +0200, Jesper Dangaard Brouer wrote:
> > > > My use-case is to 24/7 collect and keep records in userspace, and have a
> > > > timeline of these notifications, for later retrieval.  The idea is that
> > > > our support engineers can look at these records when troubleshooting
> > > > the system.  And the plan is also to collect these records as part of
> > > > our sosreport tool, which is part of the support case.

> > > I don't think you're implying that prog load/unload should be spamming 
> > > dmesg
> > > and auditd not even running...

> > I think the problem Jesper implied is that in order to collect
> > those logs you'll need perf tool running all the time.. which
> > it's not equipped for yet

> I'm not proposing to run 'perf' binary all the time.

I think Jiri just said that one would have to run something all the time
to get all the records, see below

> Setting up perf ring buffer just for these new bpf prog load/unload events
> and epolling it is simple enough to do from any application including auditd.
> selftests/bpf/ do it for bpf output events.

I think he is talking about the preexisting loaded BPF programs. We have
the same problem with mmaps, where the perf tool will, with races,
enumerate the existing mmaps as PERF_RECORD_MMAP synthesized from
/proc/PIDS/smaps.

There was talk in the past to ask the kernel to emit PERF_RECORD_MMAP
into the ring buffer for those pre-existing entries, reducing a bit the
races, but as there doesn't seem to have a good way of doing it, we
continued with the synthesizing from procfs.

Is there a way for us to synthesize those prog load/unload for
preexisting loaded bpf objects?

- Arnaldo


Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload

2018-10-05 Thread Alexei Starovoitov
On Fri, Oct 05, 2018 at 08:14:09AM +0200, Jiri Olsa wrote:
> On Thu, Oct 04, 2018 at 03:10:15PM -0700, Alexei Starovoitov wrote:
> > On Thu, Oct 04, 2018 at 10:22:31PM +0200, Jesper Dangaard Brouer wrote:
> > > On Thu, 4 Oct 2018 21:41:17 +0200 Daniel Borkmann  
> > > wrote:
> > > 
> > > > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > > > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov 
> > > > >  wrote:  
> > > > >> On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote:  
> > > [...]
> > > > >>
> > > > >> If the purpose of the patch is to give user space visibility into
> > > > >> bpf prog load/unload as a notification, then I completely agree that
> > > > >> some notification mechanism is necessary.  
> > > > 
> > > > Yeah, I did only regard it as only that, nothing more. Some means
> > > > of timeline and notification that can be kept in a record in user
> > > > space and later retrieved e.g. for introspection on what has been
> > > > loaded.
> > > > 
> > > > >> I've started working on such mechanism via perf ring buffer which is
> > > > >> the fastest mechanism we have in the kernel so far.
> > > > >> See long discussion here: https://patchwork.ozlabs.org/patch/971970/ 
> > > > >>  
> 
> cool, could you please CC me if there's another version
> of that patchset?

will do.

> > > > 
> > > > That one is definitely needed in any case to resolve the kallsyms
> > > > limitations, and it does have overlap in that in either case we
> > > > want to look at past BPF programs that have been unloaded in the
> > > > meantime, so I don't have a strong preference either way, and the
> > > > former is needed in any case. Though thought was that audit might
> > > > be an option for those not running profiling daemons 24/7, but
> > > > presumably bpftool could be extended to record these events as
> > > > well if we don't want to reuse audit infra.
> > > 
> > > Yes, exactly, I don't want to run a profiling daemon 24/7 to record
> > > these events.  I do acknowledge that this perf event is relevant,
> > > especially for catching the kernel symbols (I need that myself), but it
> > > does not cover my use-case.
> > > 
> > > My use-case is to 24/7 collect and keep records in userspace, and have a
> > > timeline of these notifications, for later retrieval.  The idea is that
> > > our support engineers can look at these records when troubleshooting
> > > the system.  And the plan is also to collect these records as part of
> > > our sosreport tool, which is part of the support case.
> > 
> > I don't think you're implying that prog load/unload should be spamming dmesg
> > and auditd not even running...
> 
> I think the problem Jesper implied is that in order to collect
> those logs you'll need perf tool running all the time.. which
> it's not equipped for yet

I'm not proposing to run 'perf' binary all the time.
Setting up perf ring buffer just for these new bpf prog load/unload events
and epolling it is simple enough to do from any application including auditd.
selftests/bpf/ do it for bpf output events.



Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload

2018-10-05 Thread Jiri Olsa
On Thu, Oct 04, 2018 at 03:10:15PM -0700, Alexei Starovoitov wrote:
> On Thu, Oct 04, 2018 at 10:22:31PM +0200, Jesper Dangaard Brouer wrote:
> > On Thu, 4 Oct 2018 21:41:17 +0200 Daniel Borkmann  
> > wrote:
> > 
> > > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov 
> > > >  wrote:  
> > > >> On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote:  
> > [...]
> > > >>
> > > >> If the purpose of the patch is to give user space visibility into
> > > >> bpf prog load/unload as a notification, then I completely agree that
> > > >> some notification mechanism is necessary.  
> > > 
> > > Yeah, I did only regard it as only that, nothing more. Some means
> > > of timeline and notification that can be kept in a record in user
> > > space and later retrieved e.g. for introspection on what has been
> > > loaded.
> > > 
> > > >> I've started working on such mechanism via perf ring buffer which is
> > > >> the fastest mechanism we have in the kernel so far.
> > > >> See long discussion here: https://patchwork.ozlabs.org/patch/971970/  

cool, could you please CC me if there's another version
of that patchset?

> > > 
> > > That one is definitely needed in any case to resolve the kallsyms
> > > limitations, and it does have overlap in that in either case we
> > > want to look at past BPF programs that have been unloaded in the
> > > meantime, so I don't have a strong preference either way, and the
> > > former is needed in any case. Though thought was that audit might
> > > be an option for those not running profiling daemons 24/7, but
> > > presumably bpftool could be extended to record these events as
> > > well if we don't want to reuse audit infra.
> > 
> > Yes, exactly, I don't want to run a profiling daemon 24/7 to record
> > these events.  I do acknowledge that this perf event is relevant,
> > especially for catching the kernel symbols (I need that myself), but it
> > does not cover my use-case.
> > 
> > My use-case is to 24/7 collect and keep records in userspace, and have a
> > timeline of these notifications, for later retrieval.  The idea is that
> > our support engineers can look at these records when troubleshooting
> > the system.  And the plan is also to collect these records as part of
> > our sosreport tool, which is part of the support case.
> 
> I don't think you're implying that prog load/unload should be spamming dmesg
> and auditd not even running...

I think the problem Jesper implied is that in order to collect
those logs you'll need perf tool running all the time.. which
it's not equipped for yet

jirka

> Also auditd has to be changed to support retrieving prog related info (like 
> license)
> via sys_bpf system call when it sees prog_id.
> Since it has to change it can just as easily use perf ring buffer
> to receive notifications.
> So we solve notification problem once and all user space tools can use it.
> 


Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload

2018-10-04 Thread Alexei Starovoitov
On Thu, Oct 04, 2018 at 10:22:31PM +0200, Jesper Dangaard Brouer wrote:
> On Thu, 4 Oct 2018 21:41:17 +0200 Daniel Borkmann  
> wrote:
> 
> > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov 
> > >  wrote:  
> > >> On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote:  
> [...]
> > >>
> > >> If the purpose of the patch is to give user space visibility into
> > >> bpf prog load/unload as a notification, then I completely agree that
> > >> some notification mechanism is necessary.  
> > 
> > Yeah, I did only regard it as only that, nothing more. Some means
> > of timeline and notification that can be kept in a record in user
> > space and later retrieved e.g. for introspection on what has been
> > loaded.
> > 
> > >> I've started working on such mechanism via perf ring buffer which is
> > >> the fastest mechanism we have in the kernel so far.
> > >> See long discussion here: https://patchwork.ozlabs.org/patch/971970/  
> > 
> > That one is definitely needed in any case to resolve the kallsyms
> > limitations, and it does have overlap in that in either case we
> > want to look at past BPF programs that have been unloaded in the
> > meantime, so I don't have a strong preference either way, and the
> > former is needed in any case. Though thought was that audit might
> > be an option for those not running profiling daemons 24/7, but
> > presumably bpftool could be extended to record these events as
> > well if we don't want to reuse audit infra.
> 
> Yes, exactly, I don't want to run a profiling daemon 24/7 to record
> these events.  I do acknowledge that this perf event is relevant,
> especially for catching the kernel symbols (I need that myself), but it
> does not cover my use-case.
> 
> My use-case is to 24/7 collect and keep records in userspace, and have a
> timeline of these notifications, for later retrieval.  The idea is that
> our support engineers can look at these records when troubleshooting
> the system.  And the plan is also to collect these records as part of
> our sosreport tool, which is part of the support case.

I don't think you're implying that prog load/unload should be spamming dmesg
and auditd not even running...
Also auditd has to be changed to support retrieving prog related info (like 
license)
via sys_bpf system call when it sees prog_id.
Since it has to change it can just as easily use perf ring buffer
to receive notifications.
So we solve notification problem once and all user space tools can use it.



Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload

2018-10-04 Thread Jesper Dangaard Brouer
On Thu, 4 Oct 2018 21:41:17 +0200 Daniel Borkmann  wrote:

> On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov 
> >  wrote:  
> >> On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote:  
[...]
> >>
> >> If the purpose of the patch is to give user space visibility into
> >> bpf prog load/unload as a notification, then I completely agree that
> >> some notification mechanism is necessary.  
> 
> Yeah, I did only regard it as only that, nothing more. Some means
> of timeline and notification that can be kept in a record in user
> space and later retrieved e.g. for introspection on what has been
> loaded.
> 
> >> I've started working on such mechanism via perf ring buffer which is
> >> the fastest mechanism we have in the kernel so far.
> >> See long discussion here: https://patchwork.ozlabs.org/patch/971970/  
> 
> That one is definitely needed in any case to resolve the kallsyms
> limitations, and it does have overlap in that in either case we
> want to look at past BPF programs that have been unloaded in the
> meantime, so I don't have a strong preference either way, and the
> former is needed in any case. Though thought was that audit might
> be an option for those not running profiling daemons 24/7, but
> presumably bpftool could be extended to record these events as
> well if we don't want to reuse audit infra.

Yes, exactly, I don't want to run a profiling daemon 24/7 to record
these events.  I do acknowledge that this perf event is relevant,
especially for catching the kernel symbols (I need that myself), but it
does not cover my use-case.

My use-case is to 24/7 collect and keep records in userspace, and have a
timeline of these notifications, for later retrieval.  The idea is that
our support engineers can look at these records when troubleshooting
the system.  And the plan is also to collect these records as part of
our sosreport tool, which is part of the support case.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload

2018-10-04 Thread Daniel Borkmann
On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> On Thu, 4 Oct 2018 10:11:43 -0700
> Alexei Starovoitov  wrote:
>> On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote:
>>> Allow for audit messages to be emitted upon BPF program load and
>>> unload for having a timeline of events. The load itself is in
>>> syscall context, so additional info about the process initiating
>>> the BPF prog creation can be logged and later directly correlated
>>> to the unload event.
>>>
>>> The only info really needed from BPF side is the globally unique
>>> prog ID where then audit user space tooling can query / dump all
>>> info needed about the specific BPF program right upon load event
>>> and enrich the record, thus these changes needed here can be kept
>>> small and non-intrusive to the core.  
>>
>> The above description is correct, but the commit log doesn't explain
>> _why_ this audit logging is needed and _why_ for load/unload.
>> My understanding of audit subsystem that it's very heavy weight
>> and absolutely not suitable for high frequency events.
>> Audit suppose to log the events that alter security of the system.
>> I don't see how loading/unloading bpf program influences security
>> at the time of the load.
>> The actions that program may take later (like dropping a packet
>> in XDP due to firewalling reasons) can be considered security
>> related, but not at the time of prog load.
> 
> I actually consider loading (well attaching) an XDP program as a
> security event.  With XDP you can bypass any kernel firewall and you are
> invisible to tcpdump.  And without an audit event, you leave no record
> of your actions.
> 
> This is not a thought up attack angle.  The NorthSec CTF hacker event,
> had an XDP challenge that did just this.
> 
> Code: https://github.com/jdesfossez/nsec17-xdpbpf
> Blog: 
> https://suchakra.wordpress.com/2017/05/23/an-entertaining-ebpf-xdp-adventure/

Well, I don't think this argument is valid. First of all it is a
perfectly valid use case that XDP itself /is/ your firewall, and I
would presume it may even be the case or part of it in majority of
users of XDP, so saying bypassing any kernel firewall is not true.
Second, it offers all the introspection needed from iproute2 and
bpftool side to check what it is doing, and aside from that adding
to tc ingress is not a "security" event either that would need an
audit event.

>> Classic bpf for sockets and seccomp has been around for long time,
>> but seccomp audit messages don't trigger on bpf load/unload,
>> but rather on events like killing a process due to seccomp bpf return value.
>>
>> If the purpose of the patch is to give user space visibility into
>> bpf prog load/unload as a notification, then I completely agree that
>> some notification mechanism is necessary.

Yeah, I did only regard it as only that, nothing more. Some means
of timeline and notification that can be kept in a record in user
space and later retrieved e.g. for introspection on what has been
loaded.

>> I've started working on such mechanism via perf ring buffer which is
>> the fastest mechanism we have in the kernel so far.
>> See long discussion here: https://patchwork.ozlabs.org/patch/971970/

That one is definitely needed in any case to resolve the kallsyms
limitations, and it does have overlap in that in either case we
want to look at past BPF programs that have been unloaded in the
meantime, so I don't have a strong preference either way, and the
former is needed in any case. Though thought was that audit might
be an option for those not running profiling daemons 24/7, but
presumably bpftool could be extended to record these events as
well if we don't want to reuse audit infra.

>> Essentially we need perf binary to see bpf prog load/unload events with
>> single argument bpf_prog_id to be able to do its job.
>>
>> I think from bpf kernel side there should be only one mechanism for user 
>> space
>> notifications and perf ring buffer fits the best, since amount
>> of load/unload in the system can be very large.
>> Anything but ring buffer will likely choke under volume of events.
> 
> I don't really buy the volume argument. Will the BPF-verifier not be
> the bottleneck for the amount/volume of bpf_load events that can be
> generated?


Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload

2018-10-04 Thread Jesper Dangaard Brouer
On Thu, 4 Oct 2018 10:11:43 -0700
Alexei Starovoitov  wrote:

> On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote:
> > Allow for audit messages to be emitted upon BPF program load and
> > unload for having a timeline of events. The load itself is in
> > syscall context, so additional info about the process initiating
> > the BPF prog creation can be logged and later directly correlated
> > to the unload event.
> > 
> > The only info really needed from BPF side is the globally unique
> > prog ID where then audit user space tooling can query / dump all
> > info needed about the specific BPF program right upon load event
> > and enrich the record, thus these changes needed here can be kept
> > small and non-intrusive to the core.  
> 
> The above description is correct, but the commit log doesn't explain
> _why_ this audit logging is needed and _why_ for load/unload.
> My understanding of audit subsystem that it's very heavy weight
> and absolutely not suitable for high frequency events.
> Audit suppose to log the events that alter security of the system.
> I don't see how loading/unloading bpf program influences security
> at the time of the load.
> The actions that program may take later (like dropping a packet
> in XDP due to firewalling reasons) can be considered security
> related, but not at the time of prog load.

I actually consider loading (well attaching) an XDP program as a
security event.  With XDP you can bypass any kernel firewall and you are
invisible to tcpdump.  And without an audit event, you leave no record
of your actions.

This is not a thought up attack angle.  The NorthSec CTF hacker event,
had an XDP challenge that did just this.

Code: https://github.com/jdesfossez/nsec17-xdpbpf
Blog: 
https://suchakra.wordpress.com/2017/05/23/an-entertaining-ebpf-xdp-adventure/

 
> Classic bpf for sockets and seccomp has been around for long time,
> but seccomp audit messages don't trigger on bpf load/unload,
> but rather on events like killing a process due to seccomp bpf return value.
> 
> If the purpose of the patch is to give user space visibility into
> bpf prog load/unload as a notification, then I completely agree that
> some notification mechanism is necessary.
> I've started working on such mechanism via perf ring buffer which is
> the fastest mechanism we have in the kernel so far.
> See long discussion here: https://patchwork.ozlabs.org/patch/971970/
> 
> Essentially we need perf binary to see bpf prog load/unload events with
> single argument bpf_prog_id to be able to do its job.
> 
> I think from bpf kernel side there should be only one mechanism for user space
> notifications and perf ring buffer fits the best, since amount
> of load/unload in the system can be very large.
> Anything but ring buffer will likely choke under volume of events.

I don't really buy the volume argument. Will the BPF-verifier not be
the bottleneck for the amount/volume of bpf_load events that can be
generated?

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload

2018-10-04 Thread Alexei Starovoitov
On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote:
> Allow for audit messages to be emitted upon BPF program load and
> unload for having a timeline of events. The load itself is in
> syscall context, so additional info about the process initiating
> the BPF prog creation can be logged and later directly correlated
> to the unload event.
> 
> The only info really needed from BPF side is the globally unique
> prog ID where then audit user space tooling can query / dump all
> info needed about the specific BPF program right upon load event
> and enrich the record, thus these changes needed here can be kept
> small and non-intrusive to the core.

The above description is correct, but the commit log doesn't explain
_why_ this audit logging is needed and _why_ for load/unload.
My understanding of audit subsystem that it's very heavy weight
and absolutely not suitable for high frequency events.
Audit suppose to log the events that alter security of the system.
I don't see how loading/unloading bpf program influences security
at the time of the load.
The actions that program may take later (like dropping a packet
in XDP due to firewalling reasons) can be considered security
related, but not at the time of prog load.

Classic bpf for sockets and seccomp has been around for long time,
but seccomp audit messages don't trigger on bpf load/unload,
but rather on events like killing a process due to seccomp bpf return value.

If the purpose of the patch is to give user space visibility into
bpf prog load/unload as a notification, then I completely agree that
some notification mechanism is necessary.
I've started working on such mechanism via perf ring buffer which is
the fastest mechanism we have in the kernel so far.
See long discussion here: https://patchwork.ozlabs.org/patch/971970/

Essentially we need perf binary to see bpf prog load/unload events with
single argument bpf_prog_id to be able to do its job.

I think from bpf kernel side there should be only one mechanism for user space
notifications and perf ring buffer fits the best, since amount
of load/unload in the system can be very large.
Anything but ring buffer will likely choke under volume of events.
Audit is not suitable for such notifications.

If in the future we have something other than seccomp killing
task via bpf return values, such code points would be good candidates
for audit logging.