Re: [PATCH] io_uring: enable audit and restrict cred override for IORING_OP_FIXED_FD_INSTALL

[Paul Moore <paul@paul-moore.com>]

On Tue, Jan 23, 2024 at 4:55 PM Paul Moore <paul@paul-moore.com> wrote:
>
> We need to correct some aspects of the IORING_OP_FIXED_FD_INSTALL
> command to take into account the security implications of making an
> io_uring-private file descriptor generally accessible to a userspace
> task.
>
> The first change in this patch is to enable auditing of the FD_INSTALL
> operation as installing a file descriptor into a task's file descriptor
> table is a security relevant operation and something that admins/users
> may want to audit.
>
> The second change is to disable the io_uring credential override
> functionality, also known as io_uring "personalities", in the
> FD_INSTALL command.  The credential override in FD_INSTALL is
> particularly problematic as it affects the credentials used in the
> security_file_receive() LSM hook.  If a task were to request a
> credential override via REQ_F_CREDS on a FD_INSTALL operation, the LSM
> would incorrectly check to see if the overridden credentials of the
> io_uring were able to "receive" the file as opposed to the task's
> credentials.  After discussions upstream, it's difficult to imagine a
> use case where we would want to allow a credential override on a
> FD_INSTALL operation so we are simply going to block REQ_F_CREDS on
> IORING_OP_FIXED_FD_INSTALL operations.
>
> Fixes: dc18b89ab113 ("io_uring/openclose: add support for IORING_OP_FIXED_FD_INSTALL")
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
>  io_uring/opdef.c     | 1 -
>  io_uring/openclose.c | 4 ++++
>  2 files changed, 4 insertions(+), 1 deletion(-)

Not having an IORING_OP_FIXED_FD_INSTALL test handy I only did some
basic sanity tests before posting, I would appreciate it if the
io_uring folks could run this through whatever FD_INSTALL tests you
have.

> diff --git a/io_uring/opdef.c b/io_uring/opdef.c
> index 6705634e5f52..b1ee3a9c3807 100644
> --- a/io_uring/opdef.c
> +++ b/io_uring/opdef.c
> @@ -471,7 +471,6 @@ const struct io_issue_def io_issue_defs[] = {
>         },
>         [IORING_OP_FIXED_FD_INSTALL] = {
>                 .needs_file             = 1,
> -               .audit_skip             = 1,
>                 .prep                   = io_install_fixed_fd_prep,
>                 .issue                  = io_install_fixed_fd,
>         },
> diff --git a/io_uring/openclose.c b/io_uring/openclose.c
> index 0fe0dd305546..e3357dfa14ca 100644
> --- a/io_uring/openclose.c
> +++ b/io_uring/openclose.c
> @@ -277,6 +277,10 @@ int io_install_fixed_fd_prep(struct io_kiocb *req, const struct io_uring_sqe *sq
>         if (flags & ~IORING_FIXED_FD_NO_CLOEXEC)
>                 return -EINVAL;
>
> +       /* ensure the task's creds are used when installing/receiving fds */
> +       if (req->flags & REQ_F_CREDS)
> +               return -EPERM;
> +
>         /* default to O_CLOEXEC, disable if IORING_FIXED_FD_NO_CLOEXEC is set */
>         ifi = io_kiocb_to_cmd(req, struct io_fixed_install);
>         ifi->o_flags = O_CLOEXEC;
> --
> 2.43.0

-- 
paul-moore.com