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

[Daniel Borkmann <daniel@iogearbox.net>]

On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> On Thu, 4 Oct 2018 10:11:43 -0700
> Alexei Starovoitov <alexei.starovoitov@gmail.com> 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?