[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Mon, 8 Oct 2018 02:56:15 +0200
From: Jann Horn <jannh@...gle.com>
To: Alexei Starovoitov <ast@...nel.org>
Cc: "David S. Miller" <davem@...emloft.net>, Daniel Borkmann <daniel@...earbox.net>,
Andy Lutomirski <luto@...capital.net>, Al Viro <viro@...iv.linux.org.uk>,
Network Development <netdev@...r.kernel.org>, kernel list <linux-kernel@...r.kernel.org>,
kernel-team@...com, Kernel Hardening <kernel-hardening@...ts.openwall.com>,
Mickaël Salaün <mic@...ikod.net>
Subject: Re: [PATCH bpf-next 1/6] bpf: introduce BPF_PROG_TYPE_FILE_FILTER
+cc kernel-hardening because this is related to sandboxing
+cc Mickaël Salaün because this looks related to his Landlock proposal
On Mon, Oct 8, 2018 at 2:30 AM Alexei Starovoitov <ast@...nel.org> wrote:
> Similar to networking sandboxing programs and cgroup-v2 based hooks
> (BPF_CGROUP_INET_[INGRESS|EGRESS,] BPF_CGROUP_INET[4|6]_[BIND|CONNECT], etc)
> introduce basic per-container sandboxing for file access via
> new BPF_PROG_TYPE_FILE_FILTER program type that attaches after
> security_file_open() LSM hook and works as additional file_open filter.
> The new cgroup bpf hook is called BPF_CGROUP_FILE_OPEN.
Why do_dentry_open() specifically, and nothing else? If you want to
filter open, wouldn't you also want to filter a bunch of other
filesystem operations with LSM hooks, like rename, unlink, truncate
and so on? Landlock benefits there from re-using the existing security
hooks.
> Just like other cgroup-bpf programs new BPF_PROG_TYPE_FILE_FILTER type
> is only available to root.
>
> This program type has access to single argument 'struct bpf_file_info'
> that contains standard sys_stat fields:
> struct bpf_file_info {
> __u64 inode;
> __u32 dev_major;
> __u32 dev_minor;
> __u32 fs_magic;
> __u32 mnt_id;
> __u32 nlink;
> __u32 mode; /* file mode S_ISDIR, S_ISLNK, 0755, etc */
> __u32 flags; /* open flags O_RDWR, O_CREAT, etc */
> };
> Other file attributes can be added in the future to the end of this struct
> without breaking bpf programs.
>
> For debugging introduce bpf_get_file_path() helper that returns
> NUL-terminated full path of the file. It should never be used for sandboxing.
>
> Use cases:
> - disallow certain FS types within containers (fs_magic == CGROUP2_SUPER_MAGIC)
> - restrict permissions in particular mount (mnt_id == X && (flags & O_RDWR))
> - disallow access to hard linked sensitive files (nlink > 1 && mode == 0700)
> - disallow access to world writeable files (mode == 0..7)
> - disallow access to given set of files (dev_major == X && dev_minor == Y && inode == Z)
That last one sounds weird. It doesn't work if you want to ban access
to a whole directory at once. And in general, highly specific
blocklists make me nervous, because if you add anything new and forget
to put it on the list, you have a problem.
> Signed-off-by: Alexei Starovoitov <ast@...nel.org>
> ---
> include/linux/bpf-cgroup.h | 10 +++
> include/linux/bpf_types.h | 1 +
> include/uapi/linux/bpf.h | 28 +++++-
> kernel/bpf/cgroup.c | 171 +++++++++++++++++++++++++++++++++++++
> kernel/bpf/syscall.c | 7 ++
> 5 files changed, 216 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
> index 588dd5f0bd85..766f0223c222 100644
> --- a/include/linux/bpf-cgroup.h
> +++ b/include/linux/bpf-cgroup.h
> @@ -109,6 +109,8 @@ int __cgroup_bpf_run_filter_sock_ops(struct sock *sk,
> int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor,
> short access, enum bpf_attach_type type);
>
> +int __cgroup_bpf_file_filter(struct file *file, enum bpf_attach_type type);
> +
> static inline enum bpf_cgroup_storage_type cgroup_storage_type(
> struct bpf_map *map)
> {
> @@ -253,6 +255,13 @@ int bpf_percpu_cgroup_storage_update(struct bpf_map *map, void *key,
> \
> __ret; \
> })
> +#define BPF_CGROUP_RUN_PROG_FILE_FILTER(file) \
> +({ \
> + int __ret = 0; \
> + if (cgroup_bpf_enabled) \
> + __ret = __cgroup_bpf_file_filter(file, BPF_CGROUP_FILE_OPEN); \
> + __ret; \
> +})
> int cgroup_bpf_prog_attach(const union bpf_attr *attr,
> enum bpf_prog_type ptype, struct bpf_prog *prog);
> int cgroup_bpf_prog_detach(const union bpf_attr *attr,
> @@ -321,6 +330,7 @@ static inline int bpf_percpu_cgroup_storage_update(struct bpf_map *map,
> #define BPF_CGROUP_RUN_PROG_UDP6_SENDMSG_LOCK(sk, uaddr, t_ctx) ({ 0; })
> #define BPF_CGROUP_RUN_PROG_SOCK_OPS(sock_ops) ({ 0; })
> #define BPF_CGROUP_RUN_PROG_DEVICE_CGROUP(type,major,minor,access) ({ 0; })
> +#define BPF_CGROUP_RUN_PROG_FILE_FILTER(file) ({ 0; })
>
> #define for_each_cgroup_storage_type(stype) for (; false; )
>
> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> index 5432f4c9f50e..f182b2e37b94 100644
> --- a/include/linux/bpf_types.h
> +++ b/include/linux/bpf_types.h
> @@ -33,6 +33,7 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_LIRC_MODE2, lirc_mode2)
> #ifdef CONFIG_INET
> BPF_PROG_TYPE(BPF_PROG_TYPE_SK_REUSEPORT, sk_reuseport)
> #endif
> +BPF_PROG_TYPE(BPF_PROG_TYPE_FILE_FILTER, file_filter)
>
> BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY, array_map_ops)
> BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_ARRAY, percpu_array_map_ops)
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index f9187b41dff6..c0df8dd99edc 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -154,6 +154,7 @@ enum bpf_prog_type {
> BPF_PROG_TYPE_LIRC_MODE2,
> BPF_PROG_TYPE_SK_REUSEPORT,
> BPF_PROG_TYPE_FLOW_DISSECTOR,
> + BPF_PROG_TYPE_FILE_FILTER,
> };
>
> enum bpf_attach_type {
> @@ -175,6 +176,7 @@ enum bpf_attach_type {
> BPF_CGROUP_UDP6_SENDMSG,
> BPF_LIRC_MODE2,
> BPF_FLOW_DISSECTOR,
> + BPF_CGROUP_FILE_OPEN,
> __MAX_BPF_ATTACH_TYPE
> };
>
> @@ -2215,6 +2217,18 @@ union bpf_attr {
> * pointer that was returned from bpf_sk_lookup_xxx\ ().
> * Return
> * 0 on success, or a negative error in case of failure.
> + *
> + * int bpf_get_file_path(struct bpf_file_info *file, char *buf, u32 size_of_buf)
> + * Description
> + * Reconstruct the full path of *file* and store it into *buf* of
> + * *size_of_buf*. The *size_of_buf* must be strictly positive.
> + * On success, the helper makes sure that the *buf* is NUL-terminated.
> + * On failure, it is filled with string "(error)".
> + * This helper should only be used for debugging.
> + * 'char *path' should never be used for permission checks.
> + * Return
> + * 0 on success, or a negative error in case of failure.
> + *
> */
> #define __BPF_FUNC_MAPPER(FN) \
> FN(unspec), \
> @@ -2303,7 +2317,8 @@ union bpf_attr {
> FN(skb_ancestor_cgroup_id), \
> FN(sk_lookup_tcp), \
> FN(sk_lookup_udp), \
> - FN(sk_release),
> + FN(sk_release), \
> + FN(get_file_path),
>
> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> * function eBPF program intends to call
> @@ -2896,4 +2911,15 @@ struct bpf_flow_keys {
> };
> };
>
> +struct bpf_file_info {
> + __u64 inode;
> + __u32 dev_major;
> + __u32 dev_minor;
> + __u32 fs_magic;
> + __u32 mnt_id;
> + __u32 nlink;
> + __u32 mode; /* file mode S_ISDIR, S_ISLNK, 0755, etc */
> + __u32 flags; /* open flags O_RDWR, O_CREAT, etc */
> +};
[...]
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
