[PATCH v1 0/2] Add LSM access controls for io_uring_setup

[PATCH v1 0/2] Add LSM access controls for io_uring_setup

[Gil Cukierman]

This patchset provides the changes required for controlling access to
the io_uring_setup system call by LSMs. It does this by adding a new
hook to io_uring. It also provides the SELinux implementation for a new
permission, io_uring { setup }, using the new hook.

This is important because existing io_uring hooks only support limiting
the sharing of credentials and access to the sensitive uring_cmd file
op. Users of LSMs may also want the ability to tightly control which
callers can retrieve an io_uring capable fd from the kernel, which is
needed for all subsequent io_uring operations.

This was tested by running the liburing test suite on a kernel
containing these patches.

Gil Cukierman (2):
  lsm,io_uring: add LSM hook for io_uring_setup
  selinux: add support for the io_uring setup permission

 include/linux/lsm_hook_defs.h       |  1 +
 include/linux/lsm_hooks.h           |  3 +++
 include/linux/security.h            |  5 +++++
 io_uring/io_uring.c                 |  5 +++++
 security/security.c                 |  4 ++++
 security/selinux/hooks.c            | 13 +++++++++++++
 security/selinux/include/classmap.h |  2 +-
 7 files changed, 32 insertions(+), 1 deletion(-)

-- 
2.38.0.135.g90850a2211-goog

[PATCH v1 1/2] lsm,io_uring: add LSM hook for io_uring_setup

[Gil Cukierman]

This patch allows LSMs to apply security policies that control
access to the io_uring_setup syscall. This is accomplished by
adding a new hook:

int security_uring_setup(void)
Check whether the current task is allowed to call io_uring_setup.

This hook, together with the existing hooks for sharing of file
descriptors and io_uring credentials, allow LSMs to expose
comprehensive controls on the usage of io_uring overall.

Signed-off-by: Gil Cukierman <cukie@google.com>
---
 include/linux/lsm_hook_defs.h | 1 +
 include/linux/lsm_hooks.h     | 3 +++
 include/linux/security.h      | 5 +++++
 io_uring/io_uring.c           | 5 +++++
 security/security.c           | 4 ++++
 5 files changed, 18 insertions(+)

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index ec119da1d89b..ffbf29b32a48 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -409,4 +409,5 @@ LSM_HOOK(int, 0, perf_event_write, struct perf_event *event)
 LSM_HOOK(int, 0, uring_override_creds, const struct cred *new)
 LSM_HOOK(int, 0, uring_sqpoll, void)
 LSM_HOOK(int, 0, uring_cmd, struct io_uring_cmd *ioucmd)
+LSM_HOOK(int, 0, uring_setup, void)
 #endif /* CONFIG_IO_URING */
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 4ec80b96c22e..bc13a8e664c9 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1589,6 +1589,9 @@
  * @uring_cmd:
  *      Check whether the file_operations uring_cmd is allowed to run.
  *
+ * @uring_setup:
+ *      Check whether the current task is allowed to call io_uring_setup.
+ *
  */
 union security_list_options {
 	#define LSM_HOOK(RET, DEFAULT, NAME, ...) RET (*NAME)(__VA_ARGS__);
diff --git a/include/linux/security.h b/include/linux/security.h
index ca1b7109c0db..0bba7dd85691 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -2069,6 +2069,7 @@ static inline int security_perf_event_write(struct perf_event *event)
 extern int security_uring_override_creds(const struct cred *new);
 extern int security_uring_sqpoll(void);
 extern int security_uring_cmd(struct io_uring_cmd *ioucmd);
+extern int security_uring_setup(void);
 #else
 static inline int security_uring_override_creds(const struct cred *new)
 {
@@ -2082,6 +2083,10 @@ static inline int security_uring_cmd(struct io_uring_cmd *ioucmd)
 {
 	return 0;
 }
+static inline int security_uring_setup(void)
+{
+	return 0;
+}
 #endif /* CONFIG_SECURITY */
 #endif /* CONFIG_IO_URING */
 
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 6cc16e39b27f..1456c85648ed 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -3574,6 +3574,11 @@ static long io_uring_setup(u32 entries, struct io_uring_params __user *params)
 {
 	struct io_uring_params p;
 	int i;
+	int ret;
+
+	ret = security_uring_setup();
+	if (ret)
+		return ret;
 
 	if (copy_from_user(&p, params, sizeof(p)))
 		return -EFAULT;
diff --git a/security/security.c b/security/security.c
index 79d82cb6e469..b1bc95df5a5d 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2671,4 +2671,8 @@ int security_uring_cmd(struct io_uring_cmd *ioucmd)
 {
 	return call_int_hook(uring_cmd, 0, ioucmd);
 }
+int security_uring_setup(void)
+{
+	return call_int_hook(uring_setup, 0);
+}
 #endif /* CONFIG_IO_URING */
-- 
2.38.0.135.g90850a2211-goog

Re: [PATCH v1 0/2] Add LSM access controls for io_uring_setup

[Paul Moore]

On Mon, Nov 7, 2022 at 3:58 PM Gil Cukierman <cukie@google.com> wrote:
>
> This patchset provides the changes required for controlling access to
> the io_uring_setup system call by LSMs. It does this by adding a new
> hook to io_uring. It also provides the SELinux implementation for a new
> permission, io_uring { setup }, using the new hook.
>
> This is important because existing io_uring hooks only support limiting
> the sharing of credentials and access to the sensitive uring_cmd file
> op. Users of LSMs may also want the ability to tightly control which
> callers can retrieve an io_uring capable fd from the kernel, which is
> needed for all subsequent io_uring operations.

It isn't immediately obvious to me why simply obtaining a io_uring fd
from io_uring_setup() would present a problem, as the security
relevant operations that are possible with that io_uring fd *should*
still be controlled by other LSM hooks.  Can you help me understand
what security issue you are trying to resolve with this control?

-- 
paul-moore.com

Re: [PATCH v1 0/2] Add LSM access controls for io_uring_setup

[Jeffrey Vander Stoep]

Hi Paul,

There are a few reasons why we want this particular hook.

1.  It aligns well with how other resources are managed by selinux
where access to the resource is the first control point (e.g. "create"
for files, sockets, or bpf_maps, "prog_load" for bpf programs, and
"open" for perf_event) and then additional functionality or
capabilities require additional permissions.
2. It aligns well with how resources are managed on Android. We often
do not grant direct access to resources (like memory buffers). For
example, a single domain on Android manages the loading of all bpf
programs and the creation of all bpf maps. Other domains can be
granted access to these only once they're created. We can enforce base
properties with MAC, while allowing the system to manage and grant
access to resources at run-time via DAC (e.g. using Android's
permission model). This allows us to do better management and
accounting of resources.
3. Attack surface management. One of the primary uses of selinux on
Android is to assess and limit attack surface (e.g.
https://twitter.com/jeffvanderstoep/status/1422771606309335043) . As
io_uring vulnerabilities have made their way through our vulnerability
management system, it's become apparent that it's complicated to
assess the impact. Is a use-after-free reachable? Creating
proof-of-concept exploits takes a lot of time, and often functionality
can be reached by multiple paths. How many of the known io_uring
vulnerabilities would be gated by the existing checks? How many future
ones will be gated by the existing checks? I don't know the answer to
either of these questions and it's not obvious. I believe some of them
currently are exploitable without any selinux permissions. But in any
case, this hook makes that initial assessment simple and effective.

On Mon, Nov 7, 2022 at 10:17 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Mon, Nov 7, 2022 at 3:58 PM Gil Cukierman <cukie@google.com> wrote:
> >
> > This patchset provides the changes required for controlling access to
> > the io_uring_setup system call by LSMs. It does this by adding a new
> > hook to io_uring. It also provides the SELinux implementation for a new
> > permission, io_uring { setup }, using the new hook.
> >
> > This is important because existing io_uring hooks only support limiting
> > the sharing of credentials and access to the sensitive uring_cmd file
> > op. Users of LSMs may also want the ability to tightly control which
> > callers can retrieve an io_uring capable fd from the kernel, which is
> > needed for all subsequent io_uring operations.
>
> It isn't immediately obvious to me why simply obtaining a io_uring fd
> from io_uring_setup() would present a problem, as the security
> relevant operations that are possible with that io_uring fd *should*
> still be controlled by other LSM hooks.  Can you help me understand
> what security issue you are trying to resolve with this control?


I think there are a few reasons why we want this particular hook.

1.  It aligns well with how other resources are managed by selinux
where access to the resource is the first control point (e.g. "create"
for files, sockets, or bpf_maps, "prog_load" for bpf programs, and
"open" for perf_event) and then additional functionality or
capabilities require additional permissions.
2. It aligns well with how resources are managed on Android. We often
do not grant direct access to resources (like memory buffers). For
example, a single domain on Android manages the loading of all bpf
programs and the creation of all bpf maps. Other domains can be
granted access to these only once they're created. We can enforce base
properties with MAC, while allowing the system to manage and grant
access to resources at run-time via DAC (e.g. using Android's
permission model). This allows us to do better management and
accounting of resources.
3. Attack surface management. One of the primary uses of selinux on
Android is to assess and limit attack surface (e.g.
https://twitter.com/jeffvanderstoep/status/1422771606309335043) . As
io_uring vulnerabilities have made their way through our vulnerability
management system, it's become apparent that it's complicated to
assess the impact. Is a use-after-free reachable? Creating
proof-of-concept exploits takes a lot of time, and often functionality
can be reached by multiple paths. How many of the known io_uring
vulnerabilities would be gated by the existing checks? How many future
ones will be gated by the existing checks? I don't know the answer to
either of these questions and it's not obvious. This hook makes that
initial assessment simple and effective.
>

>
> --
> paul-moore.com

Re: [PATCH v1 0/2] Add LSM access controls for io_uring_setup

[Paul Moore]

On Thu, Nov 10, 2022 at 12:54 PM Jeffrey Vander Stoep <jeffv@google.com> wrote:
> On Mon, Nov 7, 2022 at 10:17 PM Paul Moore <paul@paul-moore.com> wrote:
> >
> > On Mon, Nov 7, 2022 at 3:58 PM Gil Cukierman <cukie@google.com> wrote:
> > >
> > > This patchset provides the changes required for controlling access to
> > > the io_uring_setup system call by LSMs. It does this by adding a new
> > > hook to io_uring. It also provides the SELinux implementation for a new
> > > permission, io_uring { setup }, using the new hook.
> > >
> > > This is important because existing io_uring hooks only support limiting
> > > the sharing of credentials and access to the sensitive uring_cmd file
> > > op. Users of LSMs may also want the ability to tightly control which
> > > callers can retrieve an io_uring capable fd from the kernel, which is
> > > needed for all subsequent io_uring operations.
> >
> > It isn't immediately obvious to me why simply obtaining a io_uring fd
> > from io_uring_setup() would present a problem, as the security
> > relevant operations that are possible with that io_uring fd *should*
> > still be controlled by other LSM hooks.  Can you help me understand
> > what security issue you are trying to resolve with this control?
>
> I think there are a few reasons why we want this particular hook.
>
> 1.  It aligns well with how other resources are managed by selinux
> where access to the resource is the first control point (e.g. "create"
> for files, sockets, or bpf_maps, "prog_load" for bpf programs, and
> "open" for perf_event) and then additional functionality or
> capabilities require additional permissions.

[NOTE: there were two reply sections in your email, and while similar,
they were not identical; I've trimmed the other for the sake of
clarity]

The resources you mention are all objects which contain some type of
information (either user data, configuration, or program
instructions), with the resulting fd being a handle to those objects.
In the case of io_uring the fd is a handle to the io_uring
interface/rings, which by itself does not contain any information
which is not already controlled by other permissions.

I/O operations which transfer data between the io_uring buffers and
other system objects, e.g. IORING_OP_READV, are still subject to the
same file access controls as those done by the application using
syscalls.  Even the IORING_OP_OPENAT command goes through the standard
VFS code path which means it will trigger the same access control
checks as an open*() done by the application normally.

The 'interesting' scenarios are those where the io_uring operation
servicing credentials, aka personalities, differ from the task
controlling the io_uring.  However in those cases we have the new
io_uring controls to gate these delegated operations.  Passing an
io_uring fd is subject to the fd/use permission like any other fd.

Although perhaps the most relevant to your request is the fact that
the io_uring inode is created using the new(ish) secure anon inode
interface which ensures that the creating task has permission to
create an io_uring.  This io_uring inode label also comes into play
when a task attempts to mmap() the io_uring rings, a critical part of
the io_uring API.

If I'm missing something you believe to be important, please share the details.

> 2. It aligns well with how resources are managed on Android. We often
> do not grant direct access to resources (like memory buffers).

Accessing the io_uring buffers requires a task to mmap() the io_uring
fd which is controlled by the normal SELinux mmap() access controls.

> 3. Attack surface management. One of the primary uses of selinux on
> Android is to assess and limit attack surface (e.g.
> https://twitter.com/jeffvanderstoep/status/1422771606309335043) . As
> io_uring vulnerabilities have made their way through our vulnerability
> management system, it's become apparent that it's complicated to
> assess the impact. Is a use-after-free reachable? Creating
> proof-of-concept exploits takes a lot of time, and often functionality
> can be reached by multiple paths. How many of the known io_uring
> vulnerabilities would be gated by the existing checks? How many future
> ones will be gated by the existing checks? I don't know the answer to
> either of these questions and it's not obvious. This hook makes that
> initial assessment simple and effective.

It should be possible to deny access to io_uring via the anonymous
inode labels, the mmap() controls, and the fd/use permission.  If you
find a way to do meaningful work with an io_uring fd that can't be
controlled via an existing permission check please let me know.

--
paul-moore.com

Re: [PATCH v1 0/2] Add LSM access controls for io_uring_setup

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 5112 bytes --]

On Thu, Nov 10, 2022 at 04:04:46PM -0500, Paul Moore wrote:
> On Thu, Nov 10, 2022 at 12:54 PM Jeffrey Vander Stoep <jeffv@google.com> wrote:
> > On Mon, Nov 7, 2022 at 10:17 PM Paul Moore <paul@paul-moore.com> wrote:
> > >
> > > On Mon, Nov 7, 2022 at 3:58 PM Gil Cukierman <cukie@google.com> wrote:
> > > >
> > > > This patchset provides the changes required for controlling access to
> > > > the io_uring_setup system call by LSMs. It does this by adding a new
> > > > hook to io_uring. It also provides the SELinux implementation for a new
> > > > permission, io_uring { setup }, using the new hook.
> > > >
> > > > This is important because existing io_uring hooks only support limiting
> > > > the sharing of credentials and access to the sensitive uring_cmd file
> > > > op. Users of LSMs may also want the ability to tightly control which
> > > > callers can retrieve an io_uring capable fd from the kernel, which is
> > > > needed for all subsequent io_uring operations.
> > >
> > > It isn't immediately obvious to me why simply obtaining a io_uring fd
> > > from io_uring_setup() would present a problem, as the security
> > > relevant operations that are possible with that io_uring fd *should*
> > > still be controlled by other LSM hooks.  Can you help me understand
> > > what security issue you are trying to resolve with this control?
> >
> > I think there are a few reasons why we want this particular hook.
> >
> > 1.  It aligns well with how other resources are managed by selinux
> > where access to the resource is the first control point (e.g. "create"
> > for files, sockets, or bpf_maps, "prog_load" for bpf programs, and
> > "open" for perf_event) and then additional functionality or
> > capabilities require additional permissions.
> 
> [NOTE: there were two reply sections in your email, and while similar,
> they were not identical; I've trimmed the other for the sake of
> clarity]
> 
> The resources you mention are all objects which contain some type of
> information (either user data, configuration, or program
> instructions), with the resulting fd being a handle to those objects.
> In the case of io_uring the fd is a handle to the io_uring
> interface/rings, which by itself does not contain any information
> which is not already controlled by other permissions.
> 
> I/O operations which transfer data between the io_uring buffers and
> other system objects, e.g. IORING_OP_READV, are still subject to the
> same file access controls as those done by the application using
> syscalls.  Even the IORING_OP_OPENAT command goes through the standard
> VFS code path which means it will trigger the same access control
> checks as an open*() done by the application normally.
> 
> The 'interesting' scenarios are those where the io_uring operation
> servicing credentials, aka personalities, differ from the task
> controlling the io_uring.  However in those cases we have the new
> io_uring controls to gate these delegated operations.  Passing an
> io_uring fd is subject to the fd/use permission like any other fd.
> 
> Although perhaps the most relevant to your request is the fact that
> the io_uring inode is created using the new(ish) secure anon inode
> interface which ensures that the creating task has permission to
> create an io_uring.  This io_uring inode label also comes into play
> when a task attempts to mmap() the io_uring rings, a critical part of
> the io_uring API.
> 
> If I'm missing something you believe to be important, please share the details.
> 
> > 2. It aligns well with how resources are managed on Android. We often
> > do not grant direct access to resources (like memory buffers).
> 
> Accessing the io_uring buffers requires a task to mmap() the io_uring
> fd which is controlled by the normal SELinux mmap() access controls.
> 
> > 3. Attack surface management. One of the primary uses of selinux on
> > Android is to assess and limit attack surface (e.g.
> > https://twitter.com/jeffvanderstoep/status/1422771606309335043) . As
> > io_uring vulnerabilities have made their way through our vulnerability
> > management system, it's become apparent that it's complicated to
> > assess the impact. Is a use-after-free reachable? Creating
> > proof-of-concept exploits takes a lot of time, and often functionality
> > can be reached by multiple paths. How many of the known io_uring
> > vulnerabilities would be gated by the existing checks? How many future
> > ones will be gated by the existing checks? I don't know the answer to
> > either of these questions and it's not obvious. This hook makes that
> > initial assessment simple and effective.
> 
> It should be possible to deny access to io_uring via the anonymous
> inode labels, the mmap() controls, and the fd/use permission.  If you
> find a way to do meaningful work with an io_uring fd that can't be
> controlled via an existing permission check please let me know.

Also interested in a more specific case. Sending reply so I get added to
the group response.
> 
> --
> paul-moore.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v1 0/2] Add LSM access controls for io_uring_setup

[Jeffrey Vander Stoep]

Super helpful, thanks Paul! We'll look into this and get back to you
if it doesn't fit our needs.

On Mon, Nov 14, 2022 at 3:31 PM Joel Granados <j.granados@samsung.com> wrote:
>
> On Thu, Nov 10, 2022 at 04:04:46PM -0500, Paul Moore wrote:
> > On Thu, Nov 10, 2022 at 12:54 PM Jeffrey Vander Stoep <jeffv@google.com> wrote:
> > > On Mon, Nov 7, 2022 at 10:17 PM Paul Moore <paul@paul-moore.com> wrote:
> > > >
> > > > On Mon, Nov 7, 2022 at 3:58 PM Gil Cukierman <cukie@google.com> wrote:
> > > > >
> > > > > This patchset provides the changes required for controlling access to
> > > > > the io_uring_setup system call by LSMs. It does this by adding a new
> > > > > hook to io_uring. It also provides the SELinux implementation for a new
> > > > > permission, io_uring { setup }, using the new hook.
> > > > >
> > > > > This is important because existing io_uring hooks only support limiting
> > > > > the sharing of credentials and access to the sensitive uring_cmd file
> > > > > op. Users of LSMs may also want the ability to tightly control which
> > > > > callers can retrieve an io_uring capable fd from the kernel, which is
> > > > > needed for all subsequent io_uring operations.
> > > >
> > > > It isn't immediately obvious to me why simply obtaining a io_uring fd
> > > > from io_uring_setup() would present a problem, as the security
> > > > relevant operations that are possible with that io_uring fd *should*
> > > > still be controlled by other LSM hooks.  Can you help me understand
> > > > what security issue you are trying to resolve with this control?
> > >
> > > I think there are a few reasons why we want this particular hook.
> > >
> > > 1.  It aligns well with how other resources are managed by selinux
> > > where access to the resource is the first control point (e.g. "create"
> > > for files, sockets, or bpf_maps, "prog_load" for bpf programs, and
> > > "open" for perf_event) and then additional functionality or
> > > capabilities require additional permissions.
> >
> > [NOTE: there were two reply sections in your email, and while similar,
> > they were not identical; I've trimmed the other for the sake of
> > clarity]
> >
> > The resources you mention are all objects which contain some type of
> > information (either user data, configuration, or program
> > instructions), with the resulting fd being a handle to those objects.
> > In the case of io_uring the fd is a handle to the io_uring
> > interface/rings, which by itself does not contain any information
> > which is not already controlled by other permissions.
> >
> > I/O operations which transfer data between the io_uring buffers and
> > other system objects, e.g. IORING_OP_READV, are still subject to the
> > same file access controls as those done by the application using
> > syscalls.  Even the IORING_OP_OPENAT command goes through the standard
> > VFS code path which means it will trigger the same access control
> > checks as an open*() done by the application normally.
> >
> > The 'interesting' scenarios are those where the io_uring operation
> > servicing credentials, aka personalities, differ from the task
> > controlling the io_uring.  However in those cases we have the new
> > io_uring controls to gate these delegated operations.  Passing an
> > io_uring fd is subject to the fd/use permission like any other fd.
> >
> > Although perhaps the most relevant to your request is the fact that
> > the io_uring inode is created using the new(ish) secure anon inode
> > interface which ensures that the creating task has permission to
> > create an io_uring.  This io_uring inode label also comes into play
> > when a task attempts to mmap() the io_uring rings, a critical part of
> > the io_uring API.
> >
> > If I'm missing something you believe to be important, please share the details.
> >
> > > 2. It aligns well with how resources are managed on Android. We often
> > > do not grant direct access to resources (like memory buffers).
> >
> > Accessing the io_uring buffers requires a task to mmap() the io_uring
> > fd which is controlled by the normal SELinux mmap() access controls.
> >
> > > 3. Attack surface management. One of the primary uses of selinux on
> > > Android is to assess and limit attack surface (e.g.
> > > https://twitter.com/jeffvanderstoep/status/1422771606309335043) . As
> > > io_uring vulnerabilities have made their way through our vulnerability
> > > management system, it's become apparent that it's complicated to
> > > assess the impact. Is a use-after-free reachable? Creating
> > > proof-of-concept exploits takes a lot of time, and often functionality
> > > can be reached by multiple paths. How many of the known io_uring
> > > vulnerabilities would be gated by the existing checks? How many future
> > > ones will be gated by the existing checks? I don't know the answer to
> > > either of these questions and it's not obvious. This hook makes that
> > > initial assessment simple and effective.
> >
> > It should be possible to deny access to io_uring via the anonymous
> > inode labels, the mmap() controls, and the fd/use permission.  If you
> > find a way to do meaningful work with an io_uring fd that can't be
> > controlled via an existing permission check please let me know.
>
> Also interested in a more specific case. Sending reply so I get added to
> the group response.
> >
> > --
> > paul-moore.com

Re: [PATCH v1 0/2] Add LSM access controls for io_uring_setup

[Dmytro Maluka]

Hi Paul,

On 11/10/22 22:04, Paul Moore wrote:
> On Thu, Nov 10, 2022 at 12:54 PM Jeffrey Vander Stoep <jeffv@google.com> wrote:
>> On Mon, Nov 7, 2022 at 10:17 PM Paul Moore <paul@paul-moore.com> wrote:
>>>
>>> On Mon, Nov 7, 2022 at 3:58 PM Gil Cukierman <cukie@google.com> wrote:
>>>>
>>>> This patchset provides the changes required for controlling access to
>>>> the io_uring_setup system call by LSMs. It does this by adding a new
>>>> hook to io_uring. It also provides the SELinux implementation for a new
>>>> permission, io_uring { setup }, using the new hook.
>>>>
>>>> This is important because existing io_uring hooks only support limiting
>>>> the sharing of credentials and access to the sensitive uring_cmd file
>>>> op. Users of LSMs may also want the ability to tightly control which
>>>> callers can retrieve an io_uring capable fd from the kernel, which is
>>>> needed for all subsequent io_uring operations.
>>>
>>> It isn't immediately obvious to me why simply obtaining a io_uring fd
>>> from io_uring_setup() would present a problem, as the security
>>> relevant operations that are possible with that io_uring fd *should*
>>> still be controlled by other LSM hooks.  Can you help me understand
>>> what security issue you are trying to resolve with this control?
>>
>> I think there are a few reasons why we want this particular hook.
>>
>> 1.  It aligns well with how other resources are managed by selinux
>> where access to the resource is the first control point (e.g. "create"
>> for files, sockets, or bpf_maps, "prog_load" for bpf programs, and
>> "open" for perf_event) and then additional functionality or
>> capabilities require additional permissions.
> 
> [NOTE: there were two reply sections in your email, and while similar,
> they were not identical; I've trimmed the other for the sake of
> clarity]
> 
> The resources you mention are all objects which contain some type of
> information (either user data, configuration, or program
> instructions), with the resulting fd being a handle to those objects.
> In the case of io_uring the fd is a handle to the io_uring
> interface/rings, which by itself does not contain any information
> which is not already controlled by other permissions.
> 
> I/O operations which transfer data between the io_uring buffers and
> other system objects, e.g. IORING_OP_READV, are still subject to the
> same file access controls as those done by the application using
> syscalls.  Even the IORING_OP_OPENAT command goes through the standard
> VFS code path which means it will trigger the same access control
> checks as an open*() done by the application normally.
> 
> The 'interesting' scenarios are those where the io_uring operation
> servicing credentials, aka personalities, differ from the task
> controlling the io_uring.  However in those cases we have the new
> io_uring controls to gate these delegated operations.  Passing an
> io_uring fd is subject to the fd/use permission like any other fd.
> 
> Although perhaps the most relevant to your request is the fact that
> the io_uring inode is created using the new(ish) secure anon inode
> interface which ensures that the creating task has permission to
> create an io_uring.  This io_uring inode label also comes into play
> when a task attempts to mmap() the io_uring rings, a critical part of
> the io_uring API.
> 
> If I'm missing something you believe to be important, please share the details.
> 
>> 2. It aligns well with how resources are managed on Android. We often
>> do not grant direct access to resources (like memory buffers).
> 
> Accessing the io_uring buffers requires a task to mmap() the io_uring
> fd which is controlled by the normal SELinux mmap() access controls.
> 
>> 3. Attack surface management. One of the primary uses of selinux on
>> Android is to assess and limit attack surface (e.g.
>> https://twitter.com/jeffvanderstoep/status/1422771606309335043) . As
>> io_uring vulnerabilities have made their way through our vulnerability
>> management system, it's become apparent that it's complicated to
>> assess the impact. Is a use-after-free reachable? Creating
>> proof-of-concept exploits takes a lot of time, and often functionality
>> can be reached by multiple paths. How many of the known io_uring
>> vulnerabilities would be gated by the existing checks? How many future
>> ones will be gated by the existing checks? I don't know the answer to
>> either of these questions and it's not obvious. This hook makes that
>> initial assessment simple and effective.
> 
> It should be possible to deny access to io_uring via the anonymous
> inode labels, the mmap() controls, and the fd/use permission.  If you
> find a way to do meaningful work with an io_uring fd that can't be
> controlled via an existing permission check please let me know.

Thank you a lot for this explanation. However, IMHO we should not
confuse 2 somewhat different problems here:

- protecting io_uring related resources (file descriptors, memory
  buffers) against unauthorized access

- protecting the entire system against potential vulnerabilities in
  io_uring

And while I agree that the existing permission checks should be already
sufficient for the former, I'm not quite sure they are sufficient for
the latter.

(The background behind these concerns is: due to a high number of major
vulnerabilities frequently found in io_uring, we consider the io_uring
subsystem insecure as a whole; however, since io_uring brings serious
performance advantages for some I/O intensive applications, we are
willing to take the risk of enabling io_uring in kernel, as long as we
can guarantee that only a small set of trusted userspace programs are
allowed to use it, so that the rest of programs are not able to exploit
vulnerabilities in it.)

So, it seems that in order to prevent a userspace process from
exploiting vulnerabilities in io_uring, we need to prevent it from
triggering execution of any io_uring code in kernel at all, not just
from accessing io_uring resources. (Which, in particular, means that an
LSM hook for io_uring_setup() is not enough for that, we'd need to add
the LSM checks to the entry points of the other 2 io_uring syscalls as
well.)

Although, in fact, denying access to io_uring resources via existing
security checks does, as a side effect, also prevent many of io_uring
vulnerabilities (probably most of them) from being exploited. But can we
be sure that it prevents all of them? (In other words, can we be sure
that any code paths in io_uring not guarded by existing security checks
are trivial enough to assume that vulnerabilities in those code paths
are unlikely?)

Now, assuming that we cannot be sure of that and thus do need a way to
prevent triggering any io_uring kernel code by non-trusted processes, -
there are ways to do that without LSM, but those ways have some
drawbacks:

One option is to use the io_uring_disabled sysctl implemented recently
in [1]. But:

  - That would require trusted processes (i.e. those which we want to
    allow using io_uring) to have CAP_SYS_ADMIN, i.e. way too much
    privilege in the general case. Also, anyone with CAP_SYS_ADMIN, not
    just our trusted processes, would be allowed to use io_uring.

  - There was a proposal [2] to extend this sysctl to allow io_uring for
    processes in a specific group, instead of requiring CAP_SYS_ADMIN.
    But it would still only provide discretionary control: anyone who
    becomes a member of the group can use io_uring.

  - Also, [1] still adds the check to io_uring_setup() only, not to the
    other two syscalls, i.e. still assumes that denying access to
    io_uring objects is enough to prevent attacks on the kernel.

Another option is to use seccomp. For example, the init process could
implement enforcing seccomp filters blocking io_uring syscalls for all
its child processes except the trusted ones. Such a solution does not
even require new changes in kernel, but it has its downsides too:

  - It imposes limitations on the userspace environment, e.g. the init
    system needs to implement this seccomp enforcement, the trusted
    processes need to be children of the init, etc.

  - It is still not fully non-discretionary: trusted processes may
    execute other (non-trusted) programs which then can use io_uring
    too.

  - It has system-wide performance implications: seccomp overhead added
    to all syscalls from any processes that would not have any seccomp
    filter installed otherwise.

So, if we want a solution avoiding the above drawbacks, i.e. providing
more mandatory control and having less intrusive impact on the overall
userspace environment, a possible option is to use LSM: add a new LSM
hook, e.g. uring_allowed, invoked on entry points of all io_uring
syscalls.

I already have a PoC patch [3] adding such LSM hook. But before I try to
submit it for upstream, I'd like to know your opinion on the whole idea.

[1] https://lore.kernel.org/io-uring/20230630151003.3622786-1-matteorizzo@google.com/
[2] https://lore.kernel.org/io-uring/20230726174549.cg4jgx2d33fom4rb@awork3.anarazel.de/
[3] https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/4705534/3

Thanks

Re: [PATCH v1 0/2] Add LSM access controls for io_uring_setup

[Paul Moore]

On Tue, Aug 8, 2023 at 4:40 PM Dmytro Maluka <dmy@semihalf.com> wrote:
> On 11/10/22 22:04, Paul Moore wrote:
> > On Thu, Nov 10, 2022 at 12:54 PM Jeffrey Vander Stoep <jeffv@google.com> wrote:
> >> On Mon, Nov 7, 2022 at 10:17 PM Paul Moore <paul@paul-moore.com> wrote:
> >>>
> >>> On Mon, Nov 7, 2022 at 3:58 PM Gil Cukierman <cukie@google.com> wrote:
> >>>>
> >>>> This patchset provides the changes required for controlling access to
> >>>> the io_uring_setup system call by LSMs. It does this by adding a new
> >>>> hook to io_uring. It also provides the SELinux implementation for a new
> >>>> permission, io_uring { setup }, using the new hook.
> >>>>
> >>>> This is important because existing io_uring hooks only support limiting
> >>>> the sharing of credentials and access to the sensitive uring_cmd file
> >>>> op. Users of LSMs may also want the ability to tightly control which
> >>>> callers can retrieve an io_uring capable fd from the kernel, which is
> >>>> needed for all subsequent io_uring operations.
> >>>
> >>> It isn't immediately obvious to me why simply obtaining a io_uring fd
> >>> from io_uring_setup() would present a problem, as the security
> >>> relevant operations that are possible with that io_uring fd *should*
> >>> still be controlled by other LSM hooks.  Can you help me understand
> >>> what security issue you are trying to resolve with this control?
> >>
> >> I think there are a few reasons why we want this particular hook.
> >>
> >> 1.  It aligns well with how other resources are managed by selinux
> >> where access to the resource is the first control point (e.g. "create"
> >> for files, sockets, or bpf_maps, "prog_load" for bpf programs, and
> >> "open" for perf_event) and then additional functionality or
> >> capabilities require additional permissions.
> >
> > [NOTE: there were two reply sections in your email, and while similar,
> > they were not identical; I've trimmed the other for the sake of
> > clarity]
> >
> > The resources you mention are all objects which contain some type of
> > information (either user data, configuration, or program
> > instructions), with the resulting fd being a handle to those objects.
> > In the case of io_uring the fd is a handle to the io_uring
> > interface/rings, which by itself does not contain any information
> > which is not already controlled by other permissions.
> >
> > I/O operations which transfer data between the io_uring buffers and
> > other system objects, e.g. IORING_OP_READV, are still subject to the
> > same file access controls as those done by the application using
> > syscalls.  Even the IORING_OP_OPENAT command goes through the standard
> > VFS code path which means it will trigger the same access control
> > checks as an open*() done by the application normally.
> >
> > The 'interesting' scenarios are those where the io_uring operation
> > servicing credentials, aka personalities, differ from the task
> > controlling the io_uring.  However in those cases we have the new
> > io_uring controls to gate these delegated operations.  Passing an
> > io_uring fd is subject to the fd/use permission like any other fd.
> >
> > Although perhaps the most relevant to your request is the fact that
> > the io_uring inode is created using the new(ish) secure anon inode
> > interface which ensures that the creating task has permission to
> > create an io_uring.  This io_uring inode label also comes into play
> > when a task attempts to mmap() the io_uring rings, a critical part of
> > the io_uring API.
> >
> > If I'm missing something you believe to be important, please share the details.
> >
> >> 2. It aligns well with how resources are managed on Android. We often
> >> do not grant direct access to resources (like memory buffers).
> >
> > Accessing the io_uring buffers requires a task to mmap() the io_uring
> > fd which is controlled by the normal SELinux mmap() access controls.
> >
> >> 3. Attack surface management. One of the primary uses of selinux on
> >> Android is to assess and limit attack surface (e.g.
> >> https://twitter.com/jeffvanderstoep/status/1422771606309335043) . As
> >> io_uring vulnerabilities have made their way through our vulnerability
> >> management system, it's become apparent that it's complicated to
> >> assess the impact. Is a use-after-free reachable? Creating
> >> proof-of-concept exploits takes a lot of time, and often functionality
> >> can be reached by multiple paths. How many of the known io_uring
> >> vulnerabilities would be gated by the existing checks? How many future
> >> ones will be gated by the existing checks? I don't know the answer to
> >> either of these questions and it's not obvious. This hook makes that
> >> initial assessment simple and effective.
> >
> > It should be possible to deny access to io_uring via the anonymous
> > inode labels, the mmap() controls, and the fd/use permission.  If you
> > find a way to do meaningful work with an io_uring fd that can't be
> > controlled via an existing permission check please let me know.
>
> Thank you a lot for this explanation. However, IMHO we should not
> confuse 2 somewhat different problems here:
>
> - protecting io_uring related resources (file descriptors, memory
>   buffers) against unauthorized access
>
> - protecting the entire system against potential vulnerabilities in
>   io_uring
>
> And while I agree that the existing permission checks should be already
> sufficient for the former, I'm not quite sure they are sufficient for
> the latter.

...

> I already have a PoC patch [3] adding such LSM hook. But before I try to
> submit it for upstream, I'd like to know your opinion on the whole idea.

First please explain how the existing LSM/SELinux control points are
not sufficient for restricting io_uring operations.  I'm looking for a
real program flow that is able to "do meaningful work with an io_uring
fd that can't be controlled via an existing permission check".

-- 
paul-moore.com

Re: [PATCH v1 0/2] Add LSM access controls for io_uring_setup

[Dmytro Maluka]

On 8/9/23 02:31, Paul Moore wrote:
> On Tue, Aug 8, 2023 at 4:40 PM Dmytro Maluka <dmy@semihalf.com> wrote:
>> On 11/10/22 22:04, Paul Moore wrote:
>>> On Thu, Nov 10, 2022 at 12:54 PM Jeffrey Vander Stoep <jeffv@google.com> wrote:
>>>> On Mon, Nov 7, 2022 at 10:17 PM Paul Moore <paul@paul-moore.com> wrote:
>>>>>
>>>>> On Mon, Nov 7, 2022 at 3:58 PM Gil Cukierman <cukie@google.com> wrote:
>>>>>>
>>>>>> This patchset provides the changes required for controlling access to
>>>>>> the io_uring_setup system call by LSMs. It does this by adding a new
>>>>>> hook to io_uring. It also provides the SELinux implementation for a new
>>>>>> permission, io_uring { setup }, using the new hook.
>>>>>>
>>>>>> This is important because existing io_uring hooks only support limiting
>>>>>> the sharing of credentials and access to the sensitive uring_cmd file
>>>>>> op. Users of LSMs may also want the ability to tightly control which
>>>>>> callers can retrieve an io_uring capable fd from the kernel, which is
>>>>>> needed for all subsequent io_uring operations.
>>>>>
>>>>> It isn't immediately obvious to me why simply obtaining a io_uring fd
>>>>> from io_uring_setup() would present a problem, as the security
>>>>> relevant operations that are possible with that io_uring fd *should*
>>>>> still be controlled by other LSM hooks.  Can you help me understand
>>>>> what security issue you are trying to resolve with this control?
>>>>
>>>> I think there are a few reasons why we want this particular hook.
>>>>
>>>> 1.  It aligns well with how other resources are managed by selinux
>>>> where access to the resource is the first control point (e.g. "create"
>>>> for files, sockets, or bpf_maps, "prog_load" for bpf programs, and
>>>> "open" for perf_event) and then additional functionality or
>>>> capabilities require additional permissions.
>>>
>>> [NOTE: there were two reply sections in your email, and while similar,
>>> they were not identical; I've trimmed the other for the sake of
>>> clarity]
>>>
>>> The resources you mention are all objects which contain some type of
>>> information (either user data, configuration, or program
>>> instructions), with the resulting fd being a handle to those objects.
>>> In the case of io_uring the fd is a handle to the io_uring
>>> interface/rings, which by itself does not contain any information
>>> which is not already controlled by other permissions.
>>>
>>> I/O operations which transfer data between the io_uring buffers and
>>> other system objects, e.g. IORING_OP_READV, are still subject to the
>>> same file access controls as those done by the application using
>>> syscalls.  Even the IORING_OP_OPENAT command goes through the standard
>>> VFS code path which means it will trigger the same access control
>>> checks as an open*() done by the application normally.
>>>
>>> The 'interesting' scenarios are those where the io_uring operation
>>> servicing credentials, aka personalities, differ from the task
>>> controlling the io_uring.  However in those cases we have the new
>>> io_uring controls to gate these delegated operations.  Passing an
>>> io_uring fd is subject to the fd/use permission like any other fd.
>>>
>>> Although perhaps the most relevant to your request is the fact that
>>> the io_uring inode is created using the new(ish) secure anon inode
>>> interface which ensures that the creating task has permission to
>>> create an io_uring.  This io_uring inode label also comes into play
>>> when a task attempts to mmap() the io_uring rings, a critical part of
>>> the io_uring API.
>>>
>>> If I'm missing something you believe to be important, please share the details.
>>>
>>>> 2. It aligns well with how resources are managed on Android. We often
>>>> do not grant direct access to resources (like memory buffers).
>>>
>>> Accessing the io_uring buffers requires a task to mmap() the io_uring
>>> fd which is controlled by the normal SELinux mmap() access controls.
>>>
>>>> 3. Attack surface management. One of the primary uses of selinux on
>>>> Android is to assess and limit attack surface (e.g.
>>>> https://twitter.com/jeffvanderstoep/status/1422771606309335043) . As
>>>> io_uring vulnerabilities have made their way through our vulnerability
>>>> management system, it's become apparent that it's complicated to
>>>> assess the impact. Is a use-after-free reachable? Creating
>>>> proof-of-concept exploits takes a lot of time, and often functionality
>>>> can be reached by multiple paths. How many of the known io_uring
>>>> vulnerabilities would be gated by the existing checks? How many future
>>>> ones will be gated by the existing checks? I don't know the answer to
>>>> either of these questions and it's not obvious. This hook makes that
>>>> initial assessment simple and effective.
>>>
>>> It should be possible to deny access to io_uring via the anonymous
>>> inode labels, the mmap() controls, and the fd/use permission.  If you
>>> find a way to do meaningful work with an io_uring fd that can't be
>>> controlled via an existing permission check please let me know.
>>
>> Thank you a lot for this explanation. However, IMHO we should not
>> confuse 2 somewhat different problems here:
>>
>> - protecting io_uring related resources (file descriptors, memory
>>   buffers) against unauthorized access
>>
>> - protecting the entire system against potential vulnerabilities in
>>   io_uring
>>
>> And while I agree that the existing permission checks should be already
>> sufficient for the former, I'm not quite sure they are sufficient for
>> the latter.
> 
> ...
> 
>> I already have a PoC patch [3] adding such LSM hook. But before I try to
>> submit it for upstream, I'd like to know your opinion on the whole idea.
> 
> First please explain how the existing LSM/SELinux control points are
> not sufficient for restricting io_uring operations.  I'm looking for a
> real program flow that is able to "do meaningful work with an io_uring
> fd that can't be controlled via an existing permission check".

As I said at the beginning of my reply, I agree with you that the
existing LSM controls are sufficient for restricting io_uring I/O
operations. That is not my concern here. The concern is: how to (and
do we need to) restrict triggering execution of *any* io_uring code in
kernel, *in addition to* restricting the actual io_uring operations.

In other words, "a real program doing a meaningful work with io_uring"
in this case would mean "an exploit for a real vulnerability in io_uring
code (in the current or any older kernel) which does not require an
access to io_uring operations to be exploited". I don't claim that such
vulnerabilities exist or are likely to be introduced in future kernels.
But I'm neither an io_uring expert nor, more importantly, a security
expert, so I cannot tell with confidence that they are not and we have
nothing to worry about here. So I'm interested in your and others'
opinion on that.

As an example, IIUC the inode_init_security_anon LSM hook already allows
us to prevent a process from obtaining a valid io_uring fd via
io_uring_setup(). But what if the process passes an invalid (unrelated)
fd to io_uring_register() or io_uring_enter()?

It looks like all that happens is: it will quickly fail the
io_is_uring_fops() check and return an error to userspace. So I suppose
we may reasonably assume that this particular simple code path will
remain bug-free and thus we don't need to worry about potential
vulnerabilities in this case. Even if so, can we assume that any other
code paths in io_uring that are reachable without passing the existing
permission checks are similarly trivial?

Re: [PATCH v1 0/2] Add LSM access controls for io_uring_setup

[Paul Moore]

On Wed, Aug 9, 2023 at 7:22 AM Dmytro Maluka <dmy@semihalf.com> wrote:
> On 8/9/23 02:31, Paul Moore wrote:
> > On Tue, Aug 8, 2023 at 4:40 PM Dmytro Maluka <dmy@semihalf.com> wrote:
> >> On 11/10/22 22:04, Paul Moore wrote:
> >>> On Thu, Nov 10, 2022 at 12:54 PM Jeffrey Vander Stoep <jeffv@google.com> wrote:
> >>>> On Mon, Nov 7, 2022 at 10:17 PM Paul Moore <paul@paul-moore.com> wrote:
> >>>>>
> >>>>> On Mon, Nov 7, 2022 at 3:58 PM Gil Cukierman <cukie@google.com> wrote:
> >>>>>>
> >>>>>> This patchset provides the changes required for controlling access to
> >>>>>> the io_uring_setup system call by LSMs. It does this by adding a new
> >>>>>> hook to io_uring. It also provides the SELinux implementation for a new
> >>>>>> permission, io_uring { setup }, using the new hook.
> >>>>>>
> >>>>>> This is important because existing io_uring hooks only support limiting
> >>>>>> the sharing of credentials and access to the sensitive uring_cmd file
> >>>>>> op. Users of LSMs may also want the ability to tightly control which
> >>>>>> callers can retrieve an io_uring capable fd from the kernel, which is
> >>>>>> needed for all subsequent io_uring operations.
> >>>>>
> >>>>> It isn't immediately obvious to me why simply obtaining a io_uring fd
> >>>>> from io_uring_setup() would present a problem, as the security
> >>>>> relevant operations that are possible with that io_uring fd *should*
> >>>>> still be controlled by other LSM hooks.  Can you help me understand
> >>>>> what security issue you are trying to resolve with this control?
> >>>>
> >>>> I think there are a few reasons why we want this particular hook.
> >>>>
> >>>> 1.  It aligns well with how other resources are managed by selinux
> >>>> where access to the resource is the first control point (e.g. "create"
> >>>> for files, sockets, or bpf_maps, "prog_load" for bpf programs, and
> >>>> "open" for perf_event) and then additional functionality or
> >>>> capabilities require additional permissions.
> >>>
> >>> [NOTE: there were two reply sections in your email, and while similar,
> >>> they were not identical; I've trimmed the other for the sake of
> >>> clarity]
> >>>
> >>> The resources you mention are all objects which contain some type of
> >>> information (either user data, configuration, or program
> >>> instructions), with the resulting fd being a handle to those objects.
> >>> In the case of io_uring the fd is a handle to the io_uring
> >>> interface/rings, which by itself does not contain any information
> >>> which is not already controlled by other permissions.
> >>>
> >>> I/O operations which transfer data between the io_uring buffers and
> >>> other system objects, e.g. IORING_OP_READV, are still subject to the
> >>> same file access controls as those done by the application using
> >>> syscalls.  Even the IORING_OP_OPENAT command goes through the standard
> >>> VFS code path which means it will trigger the same access control
> >>> checks as an open*() done by the application normally.
> >>>
> >>> The 'interesting' scenarios are those where the io_uring operation
> >>> servicing credentials, aka personalities, differ from the task
> >>> controlling the io_uring.  However in those cases we have the new
> >>> io_uring controls to gate these delegated operations.  Passing an
> >>> io_uring fd is subject to the fd/use permission like any other fd.
> >>>
> >>> Although perhaps the most relevant to your request is the fact that
> >>> the io_uring inode is created using the new(ish) secure anon inode
> >>> interface which ensures that the creating task has permission to
> >>> create an io_uring.  This io_uring inode label also comes into play
> >>> when a task attempts to mmap() the io_uring rings, a critical part of
> >>> the io_uring API.
> >>>
> >>> If I'm missing something you believe to be important, please share the details.
> >>>
> >>>> 2. It aligns well with how resources are managed on Android. We often
> >>>> do not grant direct access to resources (like memory buffers).
> >>>
> >>> Accessing the io_uring buffers requires a task to mmap() the io_uring
> >>> fd which is controlled by the normal SELinux mmap() access controls.
> >>>
> >>>> 3. Attack surface management. One of the primary uses of selinux on
> >>>> Android is to assess and limit attack surface (e.g.
> >>>> https://twitter.com/jeffvanderstoep/status/1422771606309335043) . As
> >>>> io_uring vulnerabilities have made their way through our vulnerability
> >>>> management system, it's become apparent that it's complicated to
> >>>> assess the impact. Is a use-after-free reachable? Creating
> >>>> proof-of-concept exploits takes a lot of time, and often functionality
> >>>> can be reached by multiple paths. How many of the known io_uring
> >>>> vulnerabilities would be gated by the existing checks? How many future
> >>>> ones will be gated by the existing checks? I don't know the answer to
> >>>> either of these questions and it's not obvious. This hook makes that
> >>>> initial assessment simple and effective.
> >>>
> >>> It should be possible to deny access to io_uring via the anonymous
> >>> inode labels, the mmap() controls, and the fd/use permission.  If you
> >>> find a way to do meaningful work with an io_uring fd that can't be
> >>> controlled via an existing permission check please let me know.
> >>
> >> Thank you a lot for this explanation. However, IMHO we should not
> >> confuse 2 somewhat different problems here:
> >>
> >> - protecting io_uring related resources (file descriptors, memory
> >>   buffers) against unauthorized access
> >>
> >> - protecting the entire system against potential vulnerabilities in
> >>   io_uring
> >>
> >> And while I agree that the existing permission checks should be already
> >> sufficient for the former, I'm not quite sure they are sufficient for
> >> the latter.
> >
> > ...
> >
> >> I already have a PoC patch [3] adding such LSM hook. But before I try to
> >> submit it for upstream, I'd like to know your opinion on the whole idea.
> >
> > First please explain how the existing LSM/SELinux control points are
> > not sufficient for restricting io_uring operations.  I'm looking for a
> > real program flow that is able to "do meaningful work with an io_uring
> > fd that can't be controlled via an existing permission check".
>
> As I said at the beginning of my reply, I agree with you that the
> existing LSM controls are sufficient for restricting io_uring I/O
> operations. That is not my concern here. The concern is: how to (and
> do we need to) restrict triggering execution of *any* io_uring code in
> kernel, *in addition to* restricting the actual io_uring operations.

If your concern is preventing *any* io_uring code from being executed,
I would suggest simply not enabling io_uring at build time.  If you
need to selectively enable io_uring for some subset of processes, you
will need to make use of one of the options you discussed previously,
e.g. a LSM, seccomp, etc.

From a LSM perspective, I don't believe we want to be in the business
of blocking entire kernel subsystems from execution, rather we want to
provide control points so that admins and users can have better, or
more granular control over the security relevant operations that take
place within the different kernel subsystems.

> In other words, "a real program doing a meaningful work with io_uring"
> in this case would mean "an exploit for a real vulnerability in io_uring
> code (in the current or any older kernel) which does not require an
> access to io_uring operations to be exploited". I don't claim that such
> vulnerabilities exist or are likely to be introduced in future kernels.
> But I'm neither an io_uring expert nor, more importantly, a security
> expert, so I cannot tell with confidence that they are not and we have
> nothing to worry about here. So I'm interested in your and others'
> opinion on that.

Once again, if you have serious concerns about the security or safety
of an individual kernel subsystem, your best option is to simply build
a kernel without that subsystem enabled.

-- 
paul-moore.com

Re: [PATCH v1 0/2] Add LSM access controls for io_uring_setup

[Dmytro Maluka]

On 8/9/23 16:49, Paul Moore wrote:
> On Wed, Aug 9, 2023 at 7:22 AM Dmytro Maluka <dmy@semihalf.com> wrote:
>> On 8/9/23 02:31, Paul Moore wrote:
>>> On Tue, Aug 8, 2023 at 4:40 PM Dmytro Maluka <dmy@semihalf.com> wrote:
>>>> On 11/10/22 22:04, Paul Moore wrote:
>>>>> On Thu, Nov 10, 2022 at 12:54 PM Jeffrey Vander Stoep <jeffv@google.com> wrote:
>>>>>> On Mon, Nov 7, 2022 at 10:17 PM Paul Moore <paul@paul-moore.com> wrote:
>>>>>>>
>>>>>>> On Mon, Nov 7, 2022 at 3:58 PM Gil Cukierman <cukie@google.com> wrote:
>>>>>>>>
>>>>>>>> This patchset provides the changes required for controlling access to
>>>>>>>> the io_uring_setup system call by LSMs. It does this by adding a new
>>>>>>>> hook to io_uring. It also provides the SELinux implementation for a new
>>>>>>>> permission, io_uring { setup }, using the new hook.
>>>>>>>>
>>>>>>>> This is important because existing io_uring hooks only support limiting
>>>>>>>> the sharing of credentials and access to the sensitive uring_cmd file
>>>>>>>> op. Users of LSMs may also want the ability to tightly control which
>>>>>>>> callers can retrieve an io_uring capable fd from the kernel, which is
>>>>>>>> needed for all subsequent io_uring operations.
>>>>>>>
>>>>>>> It isn't immediately obvious to me why simply obtaining a io_uring fd
>>>>>>> from io_uring_setup() would present a problem, as the security
>>>>>>> relevant operations that are possible with that io_uring fd *should*
>>>>>>> still be controlled by other LSM hooks.  Can you help me understand
>>>>>>> what security issue you are trying to resolve with this control?
>>>>>>
>>>>>> I think there are a few reasons why we want this particular hook.
>>>>>>
>>>>>> 1.  It aligns well with how other resources are managed by selinux
>>>>>> where access to the resource is the first control point (e.g. "create"
>>>>>> for files, sockets, or bpf_maps, "prog_load" for bpf programs, and
>>>>>> "open" for perf_event) and then additional functionality or
>>>>>> capabilities require additional permissions.
>>>>>
>>>>> [NOTE: there were two reply sections in your email, and while similar,
>>>>> they were not identical; I've trimmed the other for the sake of
>>>>> clarity]
>>>>>
>>>>> The resources you mention are all objects which contain some type of
>>>>> information (either user data, configuration, or program
>>>>> instructions), with the resulting fd being a handle to those objects.
>>>>> In the case of io_uring the fd is a handle to the io_uring
>>>>> interface/rings, which by itself does not contain any information
>>>>> which is not already controlled by other permissions.
>>>>>
>>>>> I/O operations which transfer data between the io_uring buffers and
>>>>> other system objects, e.g. IORING_OP_READV, are still subject to the
>>>>> same file access controls as those done by the application using
>>>>> syscalls.  Even the IORING_OP_OPENAT command goes through the standard
>>>>> VFS code path which means it will trigger the same access control
>>>>> checks as an open*() done by the application normally.
>>>>>
>>>>> The 'interesting' scenarios are those where the io_uring operation
>>>>> servicing credentials, aka personalities, differ from the task
>>>>> controlling the io_uring.  However in those cases we have the new
>>>>> io_uring controls to gate these delegated operations.  Passing an
>>>>> io_uring fd is subject to the fd/use permission like any other fd.
>>>>>
>>>>> Although perhaps the most relevant to your request is the fact that
>>>>> the io_uring inode is created using the new(ish) secure anon inode
>>>>> interface which ensures that the creating task has permission to
>>>>> create an io_uring.  This io_uring inode label also comes into play
>>>>> when a task attempts to mmap() the io_uring rings, a critical part of
>>>>> the io_uring API.
>>>>>
>>>>> If I'm missing something you believe to be important, please share the details.
>>>>>
>>>>>> 2. It aligns well with how resources are managed on Android. We often
>>>>>> do not grant direct access to resources (like memory buffers).
>>>>>
>>>>> Accessing the io_uring buffers requires a task to mmap() the io_uring
>>>>> fd which is controlled by the normal SELinux mmap() access controls.
>>>>>
>>>>>> 3. Attack surface management. One of the primary uses of selinux on
>>>>>> Android is to assess and limit attack surface (e.g.
>>>>>> https://twitter.com/jeffvanderstoep/status/1422771606309335043) . As
>>>>>> io_uring vulnerabilities have made their way through our vulnerability
>>>>>> management system, it's become apparent that it's complicated to
>>>>>> assess the impact. Is a use-after-free reachable? Creating
>>>>>> proof-of-concept exploits takes a lot of time, and often functionality
>>>>>> can be reached by multiple paths. How many of the known io_uring
>>>>>> vulnerabilities would be gated by the existing checks? How many future
>>>>>> ones will be gated by the existing checks? I don't know the answer to
>>>>>> either of these questions and it's not obvious. This hook makes that
>>>>>> initial assessment simple and effective.
>>>>>
>>>>> It should be possible to deny access to io_uring via the anonymous
>>>>> inode labels, the mmap() controls, and the fd/use permission.  If you
>>>>> find a way to do meaningful work with an io_uring fd that can't be
>>>>> controlled via an existing permission check please let me know.
>>>>
>>>> Thank you a lot for this explanation. However, IMHO we should not
>>>> confuse 2 somewhat different problems here:
>>>>
>>>> - protecting io_uring related resources (file descriptors, memory
>>>>   buffers) against unauthorized access
>>>>
>>>> - protecting the entire system against potential vulnerabilities in
>>>>   io_uring
>>>>
>>>> And while I agree that the existing permission checks should be already
>>>> sufficient for the former, I'm not quite sure they are sufficient for
>>>> the latter.
>>>
>>> ...
>>>
>>>> I already have a PoC patch [3] adding such LSM hook. But before I try to
>>>> submit it for upstream, I'd like to know your opinion on the whole idea.
>>>
>>> First please explain how the existing LSM/SELinux control points are
>>> not sufficient for restricting io_uring operations.  I'm looking for a
>>> real program flow that is able to "do meaningful work with an io_uring
>>> fd that can't be controlled via an existing permission check".
>>
>> As I said at the beginning of my reply, I agree with you that the
>> existing LSM controls are sufficient for restricting io_uring I/O
>> operations. That is not my concern here. The concern is: how to (and
>> do we need to) restrict triggering execution of *any* io_uring code in
>> kernel, *in addition to* restricting the actual io_uring operations.
> 
> If your concern is preventing *any* io_uring code from being executed,
> I would suggest simply not enabling io_uring at build time.  If you
> need to selectively enable io_uring for some subset of processes, you
> will need to make use of one of the options you discussed previously,
> e.g. a LSM, seccomp, etc.
> 
> From a LSM perspective, I don't believe we want to be in the business
> of blocking entire kernel subsystems from execution, rather we want to
> provide control points so that admins and users can have better, or
> more granular control over the security relevant operations that take
> place within the different kernel subsystems.
> 
>> In other words, "a real program doing a meaningful work with io_uring"
>> in this case would mean "an exploit for a real vulnerability in io_uring
>> code (in the current or any older kernel) which does not require an
>> access to io_uring operations to be exploited". I don't claim that such
>> vulnerabilities exist or are likely to be introduced in future kernels.
>> But I'm neither an io_uring expert nor, more importantly, a security
>> expert, so I cannot tell with confidence that they are not and we have
>> nothing to worry about here. So I'm interested in your and others'
>> opinion on that.
> 
> Once again, if you have serious concerns about the security or safety
> of an individual kernel subsystem, your best option is to simply build
> a kernel without that subsystem enabled.

Thanks for the answer. Yeah, disabling a problematic kernel subsystem at
build time is surely the safest option (and that is what we are already
doing in ChromeOS for io_uring, for that matter), and if we still want
to enable it for a limited subset of processes, it seems the cleanest
option is to use seccomp, rather than to add new ad-hoc LSM hooks for
blocking a specific subsystem.

One of the angles I'm coming from is actually the following:

- Android currently enables io_uring but limits its use to a few
  processes. But the way Android does that is by relying on the existing
  SELinux access controls for io_uring resources [1][2], rather than by
  preventing execution of any io_uring code via seccomp or other means.

  I guess the reason why Android doesn't use seccomp for that is the
  downsides of seccomp which I mentioned previously: in short, seccomp
  is well-suited for selectively denying syscalls for specific
  processes, but not so well-suited for selectively allowing them.

  So one of the questions I'm wondering about is: if Android implemented
  preventing execution of any io_uring code by non-trusted processes
  (via seccomp or any other way), how much would it help to reduce the
  risk of attacks, compared to its current SELinux based solution?

- ChromeOS currently completely disables io_uring in kernel, but we do
  want to allow it for a limited set of processes similarly to Android,
  and we are exploring ways to do it securely. Thus the above
  considerations for Android apply to ChromeOS as well.

[1] https://android-review.git.corp.google.com/c/platform/system/sepolicy/+/2302679
[2] https://android-review.git.corp.google.com/c/platform/system/sepolicy/+/2302679/6/public/te_macros

Re: [PATCH v1 0/2] Add LSM access controls for io_uring_setup

[Dmytro Maluka]

On 8/9/23 19:28, Dmytro Maluka wrote:
>   So one of the questions I'm wondering about is: if Android implemented
>   preventing execution of any io_uring code by non-trusted processes
>   (via seccomp or any other way), how much would it help to reduce the
>   risk of attacks, compared to its current SELinux based solution?

And why exactly I'm wondering about that: AFAICT, Android folks are
concerned about the high likelihood of vulnerabilities in io_uring code
just like we (ChromeOS folks) are, and that is the main reason why
Android takes care of restricting io_uring usage in the first place.

Re: [PATCH v1 0/2] Add LSM access controls for io_uring_setup

[Stephen Smalley]

On Thu, Aug 10, 2023 at 5:08 AM Dmytro Maluka <dmy@semihalf.com> wrote:
>
> On 8/9/23 19:28, Dmytro Maluka wrote:
> >   So one of the questions I'm wondering about is: if Android implemented
> >   preventing execution of any io_uring code by non-trusted processes
> >   (via seccomp or any other way), how much would it help to reduce the
> >   risk of attacks, compared to its current SELinux based solution?
>
> And why exactly I'm wondering about that: AFAICT, Android folks are
> concerned about the high likelihood of vulnerabilities in io_uring code
> just like we (ChromeOS folks) are, and that is the main reason why
> Android takes care of restricting io_uring usage in the first place.

I think if you audit the io_uring syscalls and find a code path that
is not already mediated by a LSM hook (potentially at an earlier point
during setup / fd creation) that accesses any shared resource or
performs a privileged action, we would be open to adding a LSM hook to
cover that code path. But you'd have to do the work to identify and
propose such cases.