From: Paul Moore <paul@paul-moore.com> To: Richard Guy Briggs <rgb@redhat.com> Cc: linux-security-module@vger.kernel.org, selinux@vger.kernel.org, linux-audit@redhat.com, io-uring@vger.kernel.org, linux-fsdevel@vger.kernel.org, Jens Axboe <axboe@kernel.dk>, Pavel Begunkov <asml.silence@gmail.com>, Kumar Kartikeya Dwivedi <memxor@gmail.com> Subject: Re: [PATCH v4 2/8] audit,io_uring,io-wq: add some basic audit support to io_uring Date: Thu, 16 Sep 2021 10:47:17 -0400 [thread overview] Message-ID: <CAHC9VhTo8XPEeNPddHzXS8qCvzTvJUnLALF38VxeFEsXJRbn_Q@mail.gmail.com> (raw) In-Reply-To: <20210916141935.GQ490529@madcap2.tricolour.ca> On Thu, Sep 16, 2021 at 10:19 AM Richard Guy Briggs <rgb@redhat.com> wrote: > On 2021-09-16 10:02, Paul Moore wrote: > > On Thu, Sep 16, 2021 at 9:33 AM Richard Guy Briggs <rgb@redhat.com> wrote: > > > On 2021-09-15 12:49, Paul Moore wrote: > > > > 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. You know my feelings on audit record field orderings, so let's not follow that distraction right now. Regarding proactively inserting a placeholder for auid= and ses=, I'm reasonably convinced it is not practical, and likely not possible, to capture that information for the audit record. As a result, I see no reason to waste the space in the record. However, if you (or anyone else for that matter) can show that we can reliably capture that information then I'm in complete agreement that it should be added. What I'm not going to do is hold this patchset any longer on a vague feeling that it should be possible. On the plus side I'm only merging this into selinux/next, not selinux/stable-5.15, so worst case you have a couple more weeks before things are "set". I realize we all have competing priorities, but as the saying goes, "it's time to put up or shut up" ;) > So you are sure the rest of the creds are correct? Yes, the credentials that are logged in audit_log_uring() are all taken from the currently executing task via a call to current_cred(). Regardless of the io_uring calling context (synchronous, io-wq, sqpoll) the current_cred() call should always return the correct credentials; look at how io_uring manages credentials in io_issue_sqe(). While the audit log treats the audit ID just as if it were another credential on the system, it is definitely not like a normal credential and requires special handling; this is why there was the mixup where I mistakenly left it in the audit record, and one of the reasons why we will not always have a valid audit ID for io_uring operations. -- paul moore www.paul-moore.com
WARNING: multiple messages have this Message-ID (diff)
From: Paul Moore <paul@paul-moore.com> To: Richard Guy Briggs <rgb@redhat.com> Cc: Jens Axboe <axboe@kernel.dk>, selinux@vger.kernel.org, Pavel Begunkov <asml.silence@gmail.com>, linux-security-module@vger.kernel.org, linux-audit@redhat.com, Kumar Kartikeya Dwivedi <memxor@gmail.com>, linux-fsdevel@vger.kernel.org, io-uring@vger.kernel.org Subject: Re: [PATCH v4 2/8] audit, io_uring, io-wq: add some basic audit support to io_uring Date: Thu, 16 Sep 2021 10:47:17 -0400 [thread overview] Message-ID: <CAHC9VhTo8XPEeNPddHzXS8qCvzTvJUnLALF38VxeFEsXJRbn_Q@mail.gmail.com> (raw) In-Reply-To: <20210916141935.GQ490529@madcap2.tricolour.ca> On Thu, Sep 16, 2021 at 10:19 AM Richard Guy Briggs <rgb@redhat.com> wrote: > On 2021-09-16 10:02, Paul Moore wrote: > > On Thu, Sep 16, 2021 at 9:33 AM Richard Guy Briggs <rgb@redhat.com> wrote: > > > On 2021-09-15 12:49, Paul Moore wrote: > > > > 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. You know my feelings on audit record field orderings, so let's not follow that distraction right now. Regarding proactively inserting a placeholder for auid= and ses=, I'm reasonably convinced it is not practical, and likely not possible, to capture that information for the audit record. As a result, I see no reason to waste the space in the record. However, if you (or anyone else for that matter) can show that we can reliably capture that information then I'm in complete agreement that it should be added. What I'm not going to do is hold this patchset any longer on a vague feeling that it should be possible. On the plus side I'm only merging this into selinux/next, not selinux/stable-5.15, so worst case you have a couple more weeks before things are "set". I realize we all have competing priorities, but as the saying goes, "it's time to put up or shut up" ;) > So you are sure the rest of the creds are correct? Yes, the credentials that are logged in audit_log_uring() are all taken from the currently executing task via a call to current_cred(). Regardless of the io_uring calling context (synchronous, io-wq, sqpoll) the current_cred() call should always return the correct credentials; look at how io_uring manages credentials in io_issue_sqe(). While the audit log treats the audit ID just as if it were another credential on the system, it is definitely not like a normal credential and requires special handling; this is why there was the mixup where I mistakenly left it in the audit record, and one of the reasons why we will not always have a valid audit ID for io_uring operations. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
next prev parent reply other threads:[~2021-09-16 14:47 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-09-15 16:49 [PATCH v4 0/8] Add LSM access controls and auditing to io_uring Paul Moore 2021-09-15 16:49 ` Paul Moore 2021-09-15 16:49 ` [PATCH v4 1/8] audit: prepare audit_context for use in calling contexts beyond syscalls Paul Moore 2021-09-15 16:49 ` Paul Moore 2021-09-15 16:49 ` [PATCH v4 2/8] audit,io_uring,io-wq: add some basic audit support to io_uring Paul Moore 2021-09-15 16:49 ` [PATCH v4 2/8] audit, io_uring, io-wq: " Paul Moore 2021-09-16 13:33 ` [PATCH v4 2/8] audit,io_uring,io-wq: " Richard Guy Briggs 2021-09-16 13:33 ` Richard Guy Briggs 2021-09-16 14:02 ` Paul Moore 2021-09-16 14:02 ` [PATCH v4 2/8] audit, io_uring, io-wq: " Paul Moore 2021-09-16 14:19 ` [PATCH v4 2/8] audit,io_uring,io-wq: " Richard Guy Briggs 2021-09-16 14:19 ` Richard Guy Briggs 2021-09-16 14:47 ` Paul Moore [this message] 2021-09-16 14:47 ` [PATCH v4 2/8] audit, io_uring, io-wq: " Paul Moore 2021-09-15 16:49 ` [PATCH v4 3/8] audit: add filtering for io_uring records Paul Moore 2021-09-15 16:49 ` Paul Moore 2021-09-15 21:48 ` Richard Guy Briggs 2021-09-15 21:48 ` Richard Guy Briggs 2021-09-15 16:49 ` [PATCH v4 4/8] fs: add anon_inode_getfile_secure() similar to anon_inode_getfd_secure() Paul Moore 2021-09-15 16:49 ` Paul Moore 2021-09-15 16:49 ` [PATCH v4 5/8] io_uring: convert io_uring to the secure anon inode interface Paul Moore 2021-09-15 16:49 ` Paul Moore 2021-09-15 16:49 ` [PATCH v4 6/8] lsm,io_uring: add LSM hooks to io_uring Paul Moore 2021-09-15 16:49 ` Paul Moore 2021-09-15 16:50 ` [PATCH v4 7/8] selinux: add support for the io_uring access controls Paul Moore 2021-09-15 16:50 ` Paul Moore 2021-09-15 16:50 ` [PATCH v4 8/8] Smack: Brutalist io_uring support Paul Moore 2021-09-15 16:50 ` Paul Moore 2021-09-20 2:44 ` [PATCH v4 0/8] Add LSM access controls and auditing to io_uring Paul Moore 2021-09-20 2:44 ` Paul Moore
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAHC9VhTo8XPEeNPddHzXS8qCvzTvJUnLALF38VxeFEsXJRbn_Q@mail.gmail.com \ --to=paul@paul-moore.com \ --cc=asml.silence@gmail.com \ --cc=axboe@kernel.dk \ --cc=io-uring@vger.kernel.org \ --cc=linux-audit@redhat.com \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-security-module@vger.kernel.org \ --cc=memxor@gmail.com \ --cc=rgb@redhat.com \ --cc=selinux@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.