diff mbox series

[03/26] fuse-bpf: Update uapi for fuse-bpf

Message ID 20220926231822.994383-4-drosen@google.com (mailing list archive)
State New, archived
Headers show
Series FUSE BPF: A Stacked Filesystem Extension for FUSE | expand

Commit Message

Daniel Rosenberg Sept. 26, 2022, 11:17 p.m. UTC 
This adds the bpf prog type for fuse-bpf, and the associated structures.

Signed-off-by: Daniel Rosenberg <drosen@google.com>
Signed-off-by: Paul Lawrence <paullawrence@google.com>
---
 include/linux/bpf_fuse.h  | 50 +++++++++++++++++++++++++++++++++++++++
 include/linux/bpf_types.h |  4 ++++
 include/uapi/linux/bpf.h  | 31 ++++++++++++++++++++++++
 include/uapi/linux/fuse.h | 13 +++++++++-
 4 files changed, 97 insertions(+), 1 deletion(-)

Comments

Miklos Szeredi Sept. 27, 2022, 6:19 p.m. UTC | #1 
On Tue, 27 Sept 2022 at 01:18, Daniel Rosenberg <drosen@google.com> wrote:

> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index d6ccee961891..8c80c146e69b 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -572,6 +572,17 @@ struct fuse_entry_out {
>         struct fuse_attr attr;
>  };
>
> +#define FUSE_ACTION_KEEP       0
> +#define FUSE_ACTION_REMOVE     1
> +#define FUSE_ACTION_REPLACE    2
> +
> +struct fuse_entry_bpf_out {
> +       uint64_t        backing_action;
> +       uint64_t        backing_fd;

This is a security issue.   See this post from Jann:

https://lore.kernel.org/all/CAG48ez17uXtjCTa7xpa=JWz3iBbNDQTKO2hvn6PAZtfW3kXgcA@mail.gmail.com/

The fuse-passthrough series solved this by pre-registering the
passthrogh fd with an ioctl. Since this requires an expicit syscall on
the server side the attack is thwarted.

It would be nice if this mechanism was agreed between these projects.

BTW, does fuse-bpf provide a superset of fuse-passthrough?  I mean
could fuse-bpf work with a NULL bpf program as a simple passthrough?

Thanks,
Miklos
Paul Lawrence Sept. 30, 2022, 10:02 p.m. UTC | #2 
On Tue, Sep 27, 2022 at 11:19 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Tue, 27 Sept 2022 at 01:18, Daniel Rosenberg <drosen@google.com> wrote:
>
> > diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> > index d6ccee961891..8c80c146e69b 100644
> > --- a/include/uapi/linux/fuse.h
> > +++ b/include/uapi/linux/fuse.h
> > @@ -572,6 +572,17 @@ struct fuse_entry_out {
> >         struct fuse_attr attr;
> >  };
> >
> > +#define FUSE_ACTION_KEEP       0
> > +#define FUSE_ACTION_REMOVE     1
> > +#define FUSE_ACTION_REPLACE    2
> > +
> > +struct fuse_entry_bpf_out {
> > +       uint64_t        backing_action;
> > +       uint64_t        backing_fd;
>
> This is a security issue.   See this post from Jann:
>
> https://lore.kernel.org/all/CAG48ez17uXtjCTa7xpa=JWz3iBbNDQTKO2hvn6PAZtfW3kXgcA@mail.gmail.com/
>
> The fuse-passthrough series solved this by pre-registering the
> passthrogh fd with an ioctl. Since this requires an expicit syscall on
> the server side the attack is thwarted.
>
> It would be nice if this mechanism was agreed between these projects.
>
> BTW, does fuse-bpf provide a superset of fuse-passthrough?  I mean
> could fuse-bpf work with a NULL bpf program as a simple passthrough?
>
> Thanks,
> Miklos

To deal with the easy part. Yes, fuse-bpf can take a null bpf program, and
if you install that on files, it should behave exactly like bpf passthrough.

Our intent is that all accesses to the backing files go through the normal
vfs layer checks, so even once a backing file is installed, it can only be
accessed if the client already has sufficient rights. However, the same
statement seems to be true for the fuse passthrough code so I assume
that is not sufficient. I would be interested in further understanding the
remaining security issue (or is it defense in depth?) We understand that
the solution in fuse passthrough was to change the response to a fuse open
to be an ioctl? This would seem straightforward in fuse-bpf as well if it is
needed, though of course it would be in the lookup.

Thank you for reminding us of this,

Paul
Amir Goldstein Oct. 1, 2022, 7:47 a.m. UTC | #3 
On Sat, Oct 1, 2022 at 1:03 AM Paul Lawrence <paullawrence@google.com> wrote:
>
> On Tue, Sep 27, 2022 at 11:19 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Tue, 27 Sept 2022 at 01:18, Daniel Rosenberg <drosen@google.com> wrote:
> >
> > > diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> > > index d6ccee961891..8c80c146e69b 100644
> > > --- a/include/uapi/linux/fuse.h
> > > +++ b/include/uapi/linux/fuse.h
> > > @@ -572,6 +572,17 @@ struct fuse_entry_out {
> > >         struct fuse_attr attr;
> > >  };
> > >
> > > +#define FUSE_ACTION_KEEP       0
> > > +#define FUSE_ACTION_REMOVE     1
> > > +#define FUSE_ACTION_REPLACE    2
> > > +
> > > +struct fuse_entry_bpf_out {
> > > +       uint64_t        backing_action;
> > > +       uint64_t        backing_fd;
> >
> > This is a security issue.   See this post from Jann:
> >
> > https://lore.kernel.org/all/CAG48ez17uXtjCTa7xpa=JWz3iBbNDQTKO2hvn6PAZtfW3kXgcA@mail.gmail.com/
> >
> > The fuse-passthrough series solved this by pre-registering the
> > passthrogh fd with an ioctl. Since this requires an expicit syscall on
> > the server side the attack is thwarted.
> >
> > It would be nice if this mechanism was agreed between these projects.
> >
> > BTW, does fuse-bpf provide a superset of fuse-passthrough?  I mean
> > could fuse-bpf work with a NULL bpf program as a simple passthrough?
> >
> > Thanks,
> > Miklos
>
> To deal with the easy part. Yes, fuse-bpf can take a null bpf program, and
> if you install that on files, it should behave exactly like bpf passthrough.
>
> Our intent is that all accesses to the backing files go through the normal
> vfs layer checks, so even once a backing file is installed, it can only be
> accessed if the client already has sufficient rights. However, the same
> statement seems to be true for the fuse passthrough code so I assume
> that is not sufficient. I would be interested in further understanding the
> remaining security issue (or is it defense in depth?) We understand that
> the solution in fuse passthrough was to change the response to a fuse open
> to be an ioctl? This would seem straightforward in fuse-bpf as well if it is
> needed, though of course it would be in the lookup.

Not only in lookup.
In lookup userspace can install an O_PATH fd for backing_path,
but userspace will also need to install readable/writeable fds
as backing_file to be used by open to match the open mode.

When talking about the server-less passthrough mode, some
security model like overlayfs mounter creds model will need to be
employed, although in the private case of one-to-one passthrough
I guess using the caller creds should be good enough, as long as
the security model is spelled out and implementation is audited.

Thanks,
Amir.
diff mbox series

Patch

diff --git a/include/linux/bpf_fuse.h b/include/linux/bpf_fuse.h
index 18e2ec5bf453..9d22205c9ae0 100644
--- a/include/linux/bpf_fuse.h
+++ b/include/linux/bpf_fuse.h
@@ -6,6 +6,56 @@ 
 #ifndef _BPF_FUSE_H
 #define _BPF_FUSE_H
 
+/*
+ * Fuse BPF Args
+ *
+ * Used to communicate with bpf programs to allow checking or altering certain values.
+ * The end_offset allows the bpf verifier to check boundaries statically. This reflects
+ * the ends of the buffer. size shows the length that was actually used.
+ *
+ * In order to write to the output args, you must use the pointer returned by
+ * bpf_fuse_get_writeable.
+ *
+ */
+
+#define FUSE_MAX_ARGS_IN 3
+#define FUSE_MAX_ARGS_OUT 2
+
+struct bpf_fuse_arg {
+	void *value;		// Start of the buffer
+	void *end_offset;	// End of the buffer
+	uint32_t size;		// Used size of the buffer
+	uint32_t max_size;	// Max permitted size, if buffer is resizable. Otherwise 0
+	uint32_t flags;		// Flags indicating buffer status
+};
+
+#define FUSE_BPF_FORCE (1 << 0)
+#define FUSE_BPF_OUT_ARGVAR (1 << 6)
+
+struct bpf_fuse_args {
+	uint64_t nodeid;
+	uint32_t opcode;
+	uint32_t error_in;
+	uint32_t in_numargs;
+	uint32_t out_numargs;
+	uint32_t flags;
+	struct bpf_fuse_arg in_args[FUSE_MAX_ARGS_IN];
+	struct bpf_fuse_arg out_args[FUSE_MAX_ARGS_OUT];
+};
+
+/* These flags are used internally to track information about the fuse buffers.
+ * Fuse sets some of the flags in init. The helper functions sets others, depending on what
+ * was requested by the bpf program.
+ */
+// Flags set by FUSE
+#define BPF_FUSE_IMMUTABLE	(1 << 0) // Buffer may not be written to
+#define BPF_FUSE_VARIABLE_SIZE	(1 << 1) // Buffer length may be changed (growth requires alloc)
+#define BPF_FUSE_MUST_ALLOCATE	(1 << 2) // Buffer must be re allocated before allowing writes
+
+// Flags set by helper function
+#define BPF_FUSE_MODIFIED	(1 << 3) // The helper function allowed writes to the buffer
+#define BPF_FUSE_ALLOCATED	(1 << 4) // The helper function allocated the buffer
+
 bool bpf_helper_changes_one_pkt_data(void *func);
 
 #endif /* _BPF_FUSE_H */
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index 2b9112b80171..80c7f7d69794 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -79,6 +79,10 @@  BPF_PROG_TYPE(BPF_PROG_TYPE_LSM, lsm,
 #endif
 BPF_PROG_TYPE(BPF_PROG_TYPE_SYSCALL, bpf_syscall,
 	      void *, void *)
+#ifdef CONFIG_FUSE_BPF
+BPF_PROG_TYPE(BPF_PROG_TYPE_FUSE, fuse,
+	      struct __bpf_fuse_args, struct bpf_fuse_args)
+#endif
 
 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 59a217ca2dfd..ac81763f002b 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -952,6 +952,7 @@  enum bpf_prog_type {
 	BPF_PROG_TYPE_LSM,
 	BPF_PROG_TYPE_SK_LOOKUP,
 	BPF_PROG_TYPE_SYSCALL, /* a program that can execute syscalls */
+	BPF_PROG_TYPE_FUSE,
 };
 
 enum bpf_attach_type {
@@ -6848,4 +6849,34 @@  struct bpf_core_relo {
 	enum bpf_core_relo_kind kind;
 };
 
+struct __bpf_fuse_arg {
+	__u64 value;
+	__u64 end_offset;
+	__u32 size;
+	__u32 max_size;
+};
+
+struct __bpf_fuse_args {
+	__u64 nodeid;
+	__u32 opcode;
+	__u32 error_in;
+	__u32 in_numargs;
+	__u32 out_numargs;
+	__u32 flags;
+	struct __bpf_fuse_arg in_args[3];
+	struct __bpf_fuse_arg out_args[2];
+};
+
+/* Return Codes for Fuse BPF programs */
+#define BPF_FUSE_CONTINUE		0
+#define BPF_FUSE_USER			1
+#define BPF_FUSE_USER_PREFILTER		2
+#define BPF_FUSE_POSTFILTER		3
+#define BPF_FUSE_USER_POSTFILTER	4
+
+/* Op Code Filter values for BPF Programs */
+#define FUSE_OPCODE_FILTER	0x0ffff
+#define FUSE_PREFILTER		0x10000
+#define FUSE_POSTFILTER		0x20000
+
 #endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index d6ccee961891..8c80c146e69b 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -572,6 +572,17 @@  struct fuse_entry_out {
 	struct fuse_attr attr;
 };
 
+#define FUSE_ACTION_KEEP	0
+#define FUSE_ACTION_REMOVE	1
+#define FUSE_ACTION_REPLACE	2
+
+struct fuse_entry_bpf_out {
+	uint64_t	backing_action;
+	uint64_t	backing_fd;
+	uint64_t	bpf_action;
+	uint64_t	bpf_fd;
+};
+
 struct fuse_forget_in {
 	uint64_t	nlookup;
 };
@@ -870,7 +881,7 @@  struct fuse_in_header {
 	uint32_t	uid;
 	uint32_t	gid;
 	uint32_t	pid;
-	uint32_t	padding;
+	uint32_t	error_in;
 };
 
 struct fuse_out_header {