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

[Jens Axboe <axboe@kernel.dk>]

On 1/23/24 2:57 PM, Paul Moore wrote:
> 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.

You bet, I'm going to augment the existing test case with one that
passes in creds as well just to verify that part fails as it should as
well.

But looking at the patch, this will surely work.

-- 
Jens Axboe