| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 6 Dec 2019 16:11:13 -0500
From: Paul Moore <paul@...l-moore.com>
To: Jiri Olsa <jolsa@...nel.org>
Cc: Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org,
bpf@...r.kernel.org, linux-audit@...hat.com,
Andrii Nakryiko <andriin@...com>, Yonghong Song <yhs@...com>,
Martin KaFai Lau <kafai@...com>,
Jakub Kicinski <jakub.kicinski@...ronome.com>,
Steve Grubb <sgrubb@...hat.com>,
David Miller <davem@...hat.com>,
Eric Paris <eparis@...hat.com>, Jiri Benc <jbenc@...hat.com>
Subject: Re: [PATCHv2] bpf: Emit audit messages upon successful prog load and unload
On Thu, Dec 5, 2019 at 5:26 AM Jiri Olsa <jolsa@...nel.org> wrote:
> From: Daniel Borkmann <daniel@...earbox.net>
>
> 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.
>
> Raw example output:
>
> # auditctl -D
> # auditctl -a always,exit -F arch=x86_64 -S bpf
> # ausearch --start recent -m 1334
> ...
> ----
> time->Wed Nov 27 16:04:13 2019
> type=PROCTITLE msg=audit(1574867053.120:84664): proctitle="./bpf"
> type=SYSCALL msg=audit(1574867053.120:84664): arch=c000003e syscall=321 \
> success=yes exit=3 a0=5 a1=7ffea484fbe0 a2=70 a3=0 items=0 ppid=7477 \
> pid=12698 auid=1001 uid=1001 gid=1001 euid=1001 suid=1001 fsuid=1001 \
> egid=1001 sgid=1001 fsgid=1001 tty=pts2 ses=4 comm="bpf" \
> exe="/home/jolsa/auditd/audit-testsuite/tests/bpf/bpf" \
> subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null)
> type=UNKNOWN[1334] msg=audit(1574867053.120:84664): prog-id=76 op=LOAD
> ----
> time->Wed Nov 27 16:04:13 2019
> type=UNKNOWN[1334] msg=audit(1574867053.120:84665): prog-id=76 op=UNLOAD
> ...
>
> Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
> Co-developed-by: Jiri Olsa <jolsa@...nel.org>
> Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> ---
> include/uapi/linux/audit.h | 1 +
> kernel/bpf/syscall.c | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 34 insertions(+)
>
> v2 changes:
> addressed Paul's comments from audit side:
> - change 'event' field to 'op'
> - change audit context passing
> - check on 'op' value is within the limit
>
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index c89c6495983d..32a5db900f47 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -116,6 +116,7 @@
> #define AUDIT_FANOTIFY 1331 /* Fanotify access decision */
> #define AUDIT_TIME_INJOFFSET 1332 /* Timekeeping offset injected */
> #define AUDIT_TIME_ADJNTPVAL 1333 /* NTP value adjustment */
> +#define AUDIT_BPF 1334 /* BPF subsystem */
>
> #define AUDIT_AVC 1400 /* SE Linux avc denial or grant */
> #define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index e3461ec59570..6536665f562c 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -23,6 +23,7 @@
> #include <linux/timekeeping.h>
> #include <linux/ctype.h>
> #include <linux/nospec.h>
> +#include <linux/audit.h>
> #include <uapi/linux/btf.h>
>
> #define IS_FD_ARRAY(map) ((map)->map_type == BPF_MAP_TYPE_PERF_EVENT_ARRAY || \
> @@ -1306,6 +1307,36 @@ static int find_prog_type(enum bpf_prog_type type, struct bpf_prog *prog)
> return 0;
> }
>
> +enum bpf_audit {
> + BPF_AUDIT_LOAD,
> + BPF_AUDIT_UNLOAD,
> + BPF_AUDIT_MAX,
> +};
> +
> +static const char * const bpf_audit_str[BPF_AUDIT_MAX] = {
> + [BPF_AUDIT_LOAD] = "LOAD",
> + [BPF_AUDIT_UNLOAD] = "UNLOAD",
> +};
> +
> +static void bpf_audit_prog(const struct bpf_prog *prog, unsigned int op)
> +{
> + struct audit_context *ctx = NULL;
> + struct audit_buffer *ab;
> +
> + if (audit_enabled == AUDIT_OFF)
> + return;
> + if (WARN_ON_ONCE(op >= BPF_AUDIT_MAX))
> + return;
I feel bad saying this given the number of revisions we are at with
this patch, but since we aren't even at -rc1 yet (although it will be
here soon), I'm going to mention it anyway ;)
... if we move the "op >= BPF_AUDIT_MAX" above the audit_enabled check
we will catch problems sooner in development, which is a very good
thing as far as I'm concerned.
Other than that, this looks good to me, and I see Steve has already
given the userspace portion a thumbs-up. Have you started on the
audit-testsuite test for this yet?
> + if (op == BPF_AUDIT_LOAD)
> + ctx = audit_context();
> + ab = audit_log_start(ctx, GFP_ATOMIC, AUDIT_BPF);
> + if (unlikely(!ab))
> + return;
> + audit_log_format(ab, "prog-id=%u op=%s",
> + prog->aux->id, bpf_audit_str[op]);
> + audit_log_end(ab);
> +}
> +
> int __bpf_prog_charge(struct user_struct *user, u32 pages)
> {
> unsigned long memlock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> @@ -1421,6 +1452,7 @@ static void __bpf_prog_put(struct bpf_prog *prog, bool do_idr_lock)
> {
> if (atomic64_dec_and_test(&prog->aux->refcnt)) {
> perf_event_bpf_event(prog, PERF_BPF_EVENT_PROG_UNLOAD, 0);
> + bpf_audit_prog(prog, BPF_AUDIT_UNLOAD);
> /* bpf_prog_free_id() must be called first */
> bpf_prog_free_id(prog, do_idr_lock);
> __bpf_prog_put_noref(prog, true);
> @@ -1830,6 +1862,7 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
> */
> bpf_prog_kallsyms_add(prog);
> perf_event_bpf_event(prog, PERF_BPF_EVENT_PROG_LOAD, 0);
> + bpf_audit_prog(prog, BPF_AUDIT_LOAD);
>
> err = bpf_prog_new_fd(prog);
> if (err < 0)
> --
> 2.21.0
--
paul moore
www.paul-moore.com
Powered by blists - more mailing lists