On 2021-09-16 10:02, Paul Moore wrote:

On Thu, Sep 16, 2021 at 9:33 AM Richard Guy Briggs [off-list ref] wrote:

quoted

On 2021-09-15 12:49, Paul Moore wrote:

quoted

This patch adds basic auditing to io_uring operations, regardless of
their context.  This is accomplished by allocating audit_context
structures for the io-wq worker and io_uring SQPOLL kernel threads
as well as explicitly auditing the io_uring operations in
io_issue_sqe().  Individual io_uring operations can bypass auditing
through the "audit_skip" field in the struct io_op_def definition for
the operation; although great care must be taken so that security
relevant io_uring operations do not bypass auditing; please contact
the audit mailing list (see the MAINTAINERS file) with any questions.

The io_uring operations are audited using a new AUDIT_URINGOP record,
an example is shown below:

  type=UNKNOWN[1336] msg=audit(1630523381.288:260):
    uring_op=19 success=yes exit=0 items=0 ppid=853 pid=1204
    uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0
    subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
    key=(null)
    AUID="root" UID="root" GID="root" EUID="root" SUID="root"
    FSUID="root" EGID="root" SGID="root" FSGID="root"

Thanks to Richard Guy Briggs for review and feedback.

I share Steve's concerns about the missing auid and ses.  The userspace
log interpreter conjured up AUID="root" from the absent auid=.

Some of the creds are here including ppid, pid, a herd of *id and subj.
*Something* initiated this action and then delegated it to iouring to
carry out.  That should be in there somewhere.  You had a concern about
shared queues and mis-attribution.  All of these creds including auid
and ses should be kept together to get this right.

Look, there are a lot of things about io_uring that frustrate me from
a security perspective - this is one of them - but I've run out of
ways to say it's not possible to reliably capture the audit ID or
session ID here.  With io_uring it is possible to submit an io_uring
operation, and capture the results, by simply reading and writing to a
mmap'd buffer.  Yes, it would be nice to have that information, but I
don't believe there is a practical way to capture it.  If you have any
suggestions on how to do so, please share, but please make it
concrete; hand wavy solutions aren't useful at this stage.

I was hoping to give a more concrete solution but have other
distractions at the moment.  My concern is adding it later once the
message format is committed.  We have too many field orderings already.
Recognizing this adds useless characters to the record type at this
time, I'm even thinking auid=? ses=? until a solution can be found.

So you are sure the rest of the creds are correct?

As for the userspace mysteriously creating an AUID out of thin air,
that was my mistake: I simply removed the "auid=" field from the
example and didn't remove the additional fields, e.g. AUID, that
auditd appends to the end of the record.  I've updated the commit
description with a freshly generated record and removed the auditd
bonus bits as those probably shouldn't be shown in an example of a
kernel generated audit record.  I'm not going to repost the patchset
just for this small edit to the description, but I have force-pushed
the update to the selinux/working-io_uring branch.

Understood, no problem here.

paul moore

- RGB

--
Richard Guy Briggs [off-list ref]
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635