From: Paul Moore <paul@paul-moore.com> Date: 2021-08-11 20:48:15
Draft #2 of the patchset which brings auditing and proper LSM access
controls to the io_uring subsystem. The original patchset was posted
in late May and can be found via lore using the link below:
https://lore.kernel.org/linux-security-module/162163367115.8379.8459012634106035341.stgit@sifl/
This draft should incorporate all of the feedback from the original
posting as well as a few smaller things I noticed while playing
further with the code. The big change is of course the selective
auditing in the io_uring op servicing, but that has already been
discussed quite a bit in the original thread so I won't go into
detail here; the important part is that we found a way to move
forward and this draft captures that. For those of you looking to
play with these patches, they are based on Linus' v5.14-rc5 tag and
on my test system they boot and appear to function without problem;
they pass the selinux-testsuite and audit-testsuite and I have not
noticed any regressions in the normal use of the system. If you want
to get a copy of these patches straight from git you can use the
"working-io_uring" branch in the repo below:
git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
Beyond the existing test suite tests mentioned above, I've cobbled
together some very basic, very crude tests to exercise some of the
things I care about from a LSM/audit perspective. These tests are
pretty awful (I'm not kidding), but they might be helpful for the
other LSM/audit developers who want to test things:
https://drop.paul-moore.com/90.kUgq
There are currently two tests: 'iouring.2' and 'iouring.3';
'iouring.1' was lost in a misguided and overzealous 'rm' command.
The first test is standalone and basically tests the SQPOLL
functionality while the second tests sharing io_urings across process
boundaries and the credential/personality sharing mechanism. The
console output of both tests isn't particularly useful, the more
interesting bits are in the audit and LSM specific logs. The
'iouring.2' command requires no special arguments to run but the
'iouring.3' test is split into a "server" and "client"; the server
should be run without argument:
% ./iouring.3s
>>> server started, pid = 11678
>>> memfd created, fd = 3
>>> io_uring created; fd = 5, creds = 1
... while the client should be run with two arguments: the first is
the PID of the server process, the second is the "memfd" fd number:
% ./iouring.3c 11678 3
>>> client started, server_pid = 11678 server_memfd = 3
>>> io_urings = 5 (server) / 5 (client)
>>> io_uring ops using creds = 1
>>> async op result: 36
>>> async op result: 36
>>> async op result: 36
>>> async op result: 36
>>> START file contents
What is this life if, full of care,
we have no time to stand and stare.
>>> END file contents
The tests were hacked together from various sources online,
attribution and links to additional info can be found in the test
sources, but I expect these tests to die a fiery death in the not
to distant future as I work to add some proper tests to the SELinux
and audit test suites.
As I believe these patches should spend a full -rcX cycle in
linux-next, my current plan is to continue to solicit feedback on
these patches while they undergo additional testing (next up is
verification of the audit filter code for io_uring). Assuming no
critical issues are found on the mailing lists or during testing, I
will post a proper patchset later with the idea of merging it into
selinux/next after the upcoming merge window closes.
Any comments, feedback, etc. are welcome.
---
Casey Schaufler (1):
Smack: Brutalist io_uring support with debug
Paul Moore (8):
audit: prepare audit_context for use in calling contexts beyond
syscalls
audit,io_uring,io-wq: add some basic audit support to io_uring
audit: dev/test patch to force io_uring auditing
audit: add filtering for io_uring records
fs: add anon_inode_getfile_secure() similar to
anon_inode_getfd_secure()
io_uring: convert io_uring to the secure anon inode interface
lsm,io_uring: add LSM hooks to io_uring
selinux: add support for the io_uring access controls
fs/anon_inodes.c | 29 ++
fs/io-wq.c | 4 +
fs/io_uring.c | 69 +++-
include/linux/anon_inodes.h | 4 +
include/linux/audit.h | 26 ++
include/linux/lsm_hook_defs.h | 5 +
include/linux/lsm_hooks.h | 13 +
include/linux/security.h | 16 +
include/uapi/linux/audit.h | 4 +-
kernel/audit.h | 7 +-
kernel/audit_tree.c | 3 +-
kernel/audit_watch.c | 3 +-
kernel/auditfilter.c | 15 +-
kernel/auditsc.c | 483 +++++++++++++++++++-----
security/security.c | 12 +
security/selinux/hooks.c | 34 ++
security/selinux/include/classmap.h | 2 +
security/smack/smack_lsm.c | 64 ++++
18 files changed, 678 insertions(+), 115 deletions(-)
From: Paul Moore <paul@paul-moore.com> Date: 2021-08-11 20:48:26
WARNING - This is a work in progress and should not be merged
anywhere important. It is almost surely not complete, and while it
probably compiles it likely hasn't been booted and will do terrible
things. You have been warned.
This patch cleans up some of our audit_context handling by
abstracting out the reset and return code fixup handling to dedicated
functions. Not only does this help make things easier to read and
inspect, it allows for easier reuse be future patches. We also
convert the simple audit_context->in_syscall flag into an enum which
can be used to by future patches to indicate a calling context other
than the syscall context.
Thanks to Richard Guy Briggs for review and feedback.
Acked-by: Richard Guy Briggs <redacted>
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
v2:
- no change
v1:
- initial draft
---
kernel/audit.h | 5 +
kernel/auditsc.c | 256 ++++++++++++++++++++++++++++++++++--------------------
2 files changed, 167 insertions(+), 94 deletions(-)
@@ -97,7 +97,10 @@ struct audit_proctitle {/* The per-task audit context. */structaudit_context{intdummy;/* must be the first element */-intin_syscall;/* 1 if task is in a syscall */+enum{+AUDIT_CTX_UNUSED,/* audit_context is currently unused */+AUDIT_CTX_SYSCALL,/* in use by syscall */+}context;enumaudit_statestate,current_state;unsignedintserial;/* serial number for record */intmajor;/* syscall number */
@@ -953,7 +1024,7 @@ int audit_alloc(struct task_struct *tsk)char*key=NULL;if(likely(!audit_ever_enabled))-return0;/* Return if not auditing. */+return0;state=audit_filter_task(tsk,&key);if(state==AUDIT_STATE_DISABLED){
@@ -975,14 +1046,10 @@ int audit_alloc(struct task_struct *tsk)staticinlinevoidaudit_free_context(structaudit_context*context){-audit_free_module(context);-audit_free_names(context);-unroll_tree_refs(context,NULL,0);+/* resetting is extra work, but it is likely just noise */+audit_reset_context(context);free_tree_refs(context);-audit_free_aux(context);kfree(context->filterkey);-kfree(context->sockaddr);-audit_proctitle_free(context);kfree(context);}
@@ -1489,29 +1556,35 @@ static void audit_log_exit(void)context->personality=current->personality;-ab=audit_log_start(context,GFP_KERNEL,AUDIT_SYSCALL);-if(!ab)-return;/* audit_panic has been called */-audit_log_format(ab,"arch=%x syscall=%d",-context->arch,context->major);-if(context->personality!=PER_LINUX)-audit_log_format(ab," per=%lx",context->personality);-if(context->return_valid!=AUDITSC_INVALID)-audit_log_format(ab," success=%s exit=%ld",-(context->return_valid==AUDITSC_SUCCESS)?"yes":"no",-context->return_code);--audit_log_format(ab,-" a0=%lx a1=%lx a2=%lx a3=%lx items=%d",-context->argv[0],-context->argv[1],-context->argv[2],-context->argv[3],-context->name_count);--audit_log_task_info(ab);-audit_log_key(ab,context->filterkey);-audit_log_end(ab);+switch(context->context){+caseAUDIT_CTX_SYSCALL:+ab=audit_log_start(context,GFP_KERNEL,AUDIT_SYSCALL);+if(!ab)+return;+audit_log_format(ab,"arch=%x syscall=%d",+context->arch,context->major);+if(context->personality!=PER_LINUX)+audit_log_format(ab," per=%lx",context->personality);+if(context->return_valid!=AUDITSC_INVALID)+audit_log_format(ab," success=%s exit=%ld",+(context->return_valid==AUDITSC_SUCCESS?+"yes":"no"),+context->return_code);+audit_log_format(ab,+" a0=%lx a1=%lx a2=%lx a3=%lx items=%d",+context->argv[0],+context->argv[1],+context->argv[2],+context->argv[3],+context->name_count);+audit_log_task_info(ab);+audit_log_key(ab,context->filterkey);+audit_log_end(ab);+break;+default:+BUG();+break;+}for(aux=context->aux;aux;aux=aux->next){
@@ -1602,14 +1675,15 @@ static void audit_log_exit(void)audit_log_name(context,n,NULL,i++,&call_panic);}-audit_log_proctitle();+if(context->context==AUDIT_CTX_SYSCALL)+audit_log_proctitle();/* Send end of event record to help user space know we are finished */ab=audit_log_start(context,GFP_KERNEL,AUDIT_EOE);if(ab)audit_log_end(ab);if(call_panic)-audit_panic("error converting sid to string");+audit_panic("error in audit_log_exit()");}/**
@@ -1625,6 +1699,7 @@ void __audit_free(struct task_struct *tsk)if(!context)return;+/* this may generate CONFIG_CHANGE records */if(!list_empty(&context->killed_trees))audit_kill_trees(context);
@@ -1672,7 +1776,12 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,if(!audit_enabled||!context)return;-BUG_ON(context->in_syscall||context->name_count);+WARN_ON(context->context!=AUDIT_CTX_UNUSED);+WARN_ON(context->name_count);+if(context->context!=AUDIT_CTX_UNUSED||context->name_count){+audit_panic("unrecoverable error in audit_syscall_entry()");+return;+}state=context->state;if(state==AUDIT_STATE_DISABLED)
@@ -1691,10 +1800,8 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,context->argv[1]=a2;context->argv[2]=a3;context->argv[3]=a4;-context->serial=0;-context->in_syscall=1;+context->context=AUDIT_CTX_SYSCALL;context->current_state=state;-context->ppid=0;ktime_get_coarse_real_ts64(&context->ctime);}
@@ -1711,63 +1818,27 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,*/void__audit_syscall_exit(intsuccess,longreturn_code){-structaudit_context*context;+structaudit_context*context=audit_context();-context=audit_context();-if(!context)-return;+if(!context||context->dummy||+context->context!=AUDIT_CTX_SYSCALL)+gotoout;+/* this may generate CONFIG_CHANGE records */if(!list_empty(&context->killed_trees))audit_kill_trees(context);-if(!context->dummy&&context->in_syscall){-if(success)-context->return_valid=AUDITSC_SUCCESS;-else-context->return_valid=AUDITSC_FAILURE;--/*-*weneedtofixupthereturncodeintheauditlogsifthe-*actualreturncodesarelatergoingtobefixedupbythe-*archspecificsignalhandlers-*-*Thisisactuallyatestfor:-*(rc==ERESTARTSYS)||(rc==ERESTARTNOINTR)||-*(rc==ERESTARTNOHAND)||(rc==ERESTART_RESTARTBLOCK)-*-*butisfasterthanabunchof||-*/-if(unlikely(return_code<=-ERESTARTSYS)&&-(return_code>=-ERESTART_RESTARTBLOCK)&&-(return_code!=-ENOIOCTLCMD))-context->return_code=-EINTR;-else-context->return_code=return_code;--audit_filter_syscall(current,context);-audit_filter_inodes(current,context);-if(context->current_state==AUDIT_STATE_RECORD)-audit_log_exit();-}+/* run through both filters to ensure we set the filterkey properly */+audit_filter_syscall(current,context);+audit_filter_inodes(current,context);+if(context->current_state<AUDIT_STATE_RECORD)+gotoout;-context->in_syscall=0;-context->prio=context->state==AUDIT_STATE_RECORD?~0ULL:0;+audit_return_fixup(context,success,return_code);+audit_log_exit();-audit_free_module(context);-audit_free_names(context);-unroll_tree_refs(context,NULL,0);-audit_free_aux(context);-context->aux=NULL;-context->aux_pids=NULL;-context->target_pid=0;-context->target_sid=0;-context->sockaddr_len=0;-context->type=0;-context->fds[0]=-1;-if(context->state!=AUDIT_STATE_RECORD){-kfree(context->filterkey);-context->filterkey=NULL;-}+out:+audit_reset_context(context);}staticinlinevoidhandle_one(conststructinode*inode)
From: Paul Moore <paul@paul-moore.com> Date: 2021-08-11 20:48:47
WARNING - This patch is intended only to aid in the initial dev/test
of the audit/io_uring support, it is not intended to be merged.
With this patch, you can emit io_uring operation audit records with
the following commands (the first clears any blocking rules):
% auditctl -D
% auditctl -a exit,always -S io_uring_enter
Signed-off-by: DO NOT COMMIT
---
kernel/auditsc.c | 4 ++++
1 file changed, 4 insertions(+)
@@ -1910,6 +1910,10 @@ void __audit_uring_exit(int success, long code)audit_log_uring(ctx);return;}+#if 1+/* XXX - temporary hack to force record generation */+ctx->current_state=AUDIT_STATE_RECORD;+#endif/* this may generate CONFIG_CHANGE records */if(!list_empty(&ctx->killed_trees))
From: Paul Moore <paul@paul-moore.com> Date: 2021-08-11 20:48:48
WARNING - This is a work in progress and should not be merged
anywhere important. It is almost surely not complete, and while it
probably compiles it likely hasn't been booted and will do terrible
things. You have been warned.
This patch adds basic audit io_uring filtering, using as much of the
existing audit filtering infrastructure as possible. In order to do
this we reuse the audit filter rule's syscall mask for the io_uring
operation and we create a new filter for io_uring operations as
AUDIT_FILTER_URING_EXIT/audit_filter_list[7].
<TODO - provide some additional guidance for the userspace tools>
Thanks to Richard Guy Briggs for his review and feedback.
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
v2:
- incorporate feedback from Richard
v1:
- initial draft
---
include/uapi/linux/audit.h | 3 +-
kernel/audit_tree.c | 3 +-
kernel/audit_watch.c | 3 +-
kernel/auditfilter.c | 15 ++++++++--
kernel/auditsc.c | 65 ++++++++++++++++++++++++++++++++++----------
5 files changed, 68 insertions(+), 21 deletions(-)
@@ -805,6 +805,35 @@ static int audit_in_mask(const struct audit_krule *rule, unsigned long val)returnrule->mask[word]&bit;}+/**+*audit_filter_uring-applyfilterstoanio_uringoperation+*@tsk:associatedtask+*@ctx:auditcontext+*/+staticvoidaudit_filter_uring(structtask_struct*tsk,+structaudit_context*ctx)+{+structaudit_entry*e;+enumaudit_statestate;++if(auditd_test_task(tsk))+return;++rcu_read_lock();+list_for_each_entry_rcu(e,&audit_filter_list[AUDIT_FILTER_URING_EXIT],+list){+if(audit_in_mask(&e->rule,ctx->uring_op)&&+audit_filter_rules(tsk,&e->rule,ctx,NULL,&state,+false)){+rcu_read_unlock();+ctx->current_state=state;+return;+}+}+rcu_read_unlock();+return;+}+/* At syscall exit time, this filter is called if the audit_state is*notlowenoughthatauditingcannottakeplace,butisalsonot*highenoughthatwealreadyknowwehavetowriteanauditrecord
@@ -1783,15 +1812,21 @@ void __audit_free(struct task_struct *tsk)*randomtask_structthatdoesn'tdoesn'thaveanymeaningfuldatawe*needtologviaaudit_log_exit().*/-if(tsk==current&&!context->dummy&&-context->context==AUDIT_CTX_SYSCALL){+if(tsk==current&&!context->dummy){context->return_valid=AUDITSC_INVALID;context->return_code=0;--audit_filter_syscall(tsk,context);-audit_filter_inodes(tsk,context);-if(context->current_state==AUDIT_STATE_RECORD)-audit_log_exit();+if(context->context==AUDIT_CTX_SYSCALL){+audit_filter_syscall(tsk,context);+audit_filter_inodes(tsk,context);+if(context->current_state==AUDIT_STATE_RECORD)+audit_log_exit();+}elseif(context->context==AUDIT_CTX_URING){+/* TODO: verify this case is real and valid */+audit_filter_uring(tsk,context);+audit_filter_inodes(tsk,context);+if(context->current_state==AUDIT_STATE_RECORD)+audit_log_uring(context);+}}audit_set_context(tsk,NULL);
@@ -1875,12 +1910,6 @@ void __audit_uring_exit(int success, long code){structaudit_context*ctx=audit_context();-/*-*TODO:Atsomepointwewilllikelywanttofilteronio_uringops-*andotherthingssimilartowhatwedoforsyscalls,butthat-*issomethingforanotherday;justrecordwhatwecanhere.-*/-if(ctx->context==AUDIT_CTX_SYSCALL){/**NOTE:Seethenotein__audit_uring_entry()aboutthecase
@@ -1903,6 +1932,8 @@ void __audit_uring_exit(int success, long code)*thebehaviorhere.*/audit_filter_syscall(current,ctx);+if(ctx->current_state!=AUDIT_STATE_RECORD)+audit_filter_uring(current,ctx);audit_filter_inodes(current,ctx);if(ctx->current_state!=AUDIT_STATE_RECORD)return;
@@ -1911,7 +1942,9 @@ void __audit_uring_exit(int success, long code)return;}#if 1-/* XXX - temporary hack to force record generation */+/* XXX - temporary hack to force record generation, we are leaving this+*enabled,butifyouwanttoactuallytestthefilteringyou+*needtodisablethis#if/#endifblock*/ctx->current_state=AUDIT_STATE_RECORD;#endif
@@ -1919,6 +1952,8 @@ void __audit_uring_exit(int success, long code)if(!list_empty(&ctx->killed_trees))audit_kill_trees(ctx);+/* run through both filters to ensure we set the filterkey properly */+audit_filter_uring(current,ctx);audit_filter_inodes(current,ctx);if(ctx->current_state!=AUDIT_STATE_RECORD)gotoout;
From: Paul Moore <paul@paul-moore.com> Date: 2021-08-11 20:48:50
Extending the secure anonymous inode support to other subsystems
requires that we have a secure anon_inode_getfile() variant in
addition to the existing secure anon_inode_getfd() variant.
Thankfully we can reuse the existing __anon_inode_getfile() function
and just wrap it with the proper arguments.
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
v2:
- no change
v1:
- initial draft
---
fs/anon_inodes.c | 29 +++++++++++++++++++++++++++++
include/linux/anon_inodes.h | 4 ++++
2 files changed, 33 insertions(+)
Extending the secure anonymous inode support to other subsystems
requires that we have a secure anon_inode_getfile() variant in
addition to the existing secure anon_inode_getfd() variant.
Thankfully we can reuse the existing __anon_inode_getfile() function
and just wrap it with the proper arguments.
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
v2:
- no change
v1:
- initial draft
---
fs/anon_inodes.c | 29 +++++++++++++++++++++++++++++
include/linux/anon_inodes.h | 4 ++++
2 files changed, 33 insertions(+)
This is not directly related to this patch but why using the "secure"
boolean in __anon_inode_getfile() and __anon_inode_getfd() instead of
checking that context_inode is not NULL? This would simplify the code,
remove this anon_inode_getfile_secure() wrapper and avoid potential
inconsistencies.
quoted hunk
+}
+
static int __anon_inode_getfd(const char *name,
const struct file_operations *fops,
void *priv, int flags,
From: Paul Moore <paul@paul-moore.com> Date: 2021-08-12 14:33:04
On Thu, Aug 12, 2021 at 5:32 AM Mickaël Salaün [off-list ref] wrote:
On 11/08/2021 22:48, Paul Moore wrote:
quoted
Extending the secure anonymous inode support to other subsystems
requires that we have a secure anon_inode_getfile() variant in
addition to the existing secure anon_inode_getfd() variant.
Thankfully we can reuse the existing __anon_inode_getfile() function
and just wrap it with the proper arguments.
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
v2:
- no change
v1:
- initial draft
---
fs/anon_inodes.c | 29 +++++++++++++++++++++++++++++
include/linux/anon_inodes.h | 4 ++++
2 files changed, 33 insertions(+)
This is not directly related to this patch but why using the "secure"
boolean in __anon_inode_getfile() and __anon_inode_getfd() instead of
checking that context_inode is not NULL? This would simplify the code,
remove this anon_inode_getfile_secure() wrapper and avoid potential
inconsistencies.
The issue is that it is acceptable for the context_inode to be either
valid or NULL for callers who request the "secure" code path.
Look at the SELinux implementation of the anonymous inode hook in
selinux_inode_init_security_anon() and you will see that in cases
where the context_inode is valid we simply inherit the label from the
given inode, whereas if context_inode is NULL we do a type transition
using the requesting task and the anonymous inode's "name".
--
paul moore
www.paul-moore.com
On Thu, Aug 12, 2021 at 5:32 AM Mickaël Salaün [off-list ref] wrote:
quoted
On 11/08/2021 22:48, Paul Moore wrote:
quoted
Extending the secure anonymous inode support to other subsystems
requires that we have a secure anon_inode_getfile() variant in
addition to the existing secure anon_inode_getfd() variant.
Thankfully we can reuse the existing __anon_inode_getfile() function
and just wrap it with the proper arguments.
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
v2:
- no change
v1:
- initial draft
---
fs/anon_inodes.c | 29 +++++++++++++++++++++++++++++
include/linux/anon_inodes.h | 4 ++++
2 files changed, 33 insertions(+)
This is not directly related to this patch but why using the "secure"
boolean in __anon_inode_getfile() and __anon_inode_getfd() instead of
checking that context_inode is not NULL? This would simplify the code,
remove this anon_inode_getfile_secure() wrapper and avoid potential
inconsistencies.
The issue is that it is acceptable for the context_inode to be either
valid or NULL for callers who request the "secure" code path.
Look at the SELinux implementation of the anonymous inode hook in
selinux_inode_init_security_anon() and you will see that in cases
where the context_inode is valid we simply inherit the label from the
given inode, whereas if context_inode is NULL we do a type transition
using the requesting task and the anonymous inode's "name".
From: Paul Moore <paul@paul-moore.com> Date: 2021-08-11 20:49:17
Converting io_uring's anonymous inode to the secure anon inode API
enables LSMs to enforce policy on the io_uring anonymous inodes if
they chose to do so. This is an important first step towards
providing the necessary mechanisms so that LSMs can apply security
policy to io_uring operations.
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
v2:
- no change
v1:
- initial draft
---
fs/io_uring.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Paul Moore <paul@paul-moore.com> Date: 2021-08-11 20:49:27
WARNING - This is a work in progress, this patch, including the
description, may be incomplete or even incorrect. You have been
warned.
A full expalantion of io_uring is beyond the scope of this commit
description, but in summary it is an asynchronous I/O mechanism
which allows for I/O requests and the resulting data to be queued
in memory mapped "rings" which are shared between the kernel and
userspace. Optionally, io_uring offers the ability for applications
to spawn kernel threads to dequeue I/O requests from the ring and
submit the requests in the kernel, helping to minimize the syscall
overhead. Rings are accessed in userspace by memory mapping a file
descriptor provided by the io_uring_setup(2), and can be shared
between applications as one might do with any open file descriptor.
Finally, process credentials can be registered with a given ring
and any process with access to that ring can submit I/O requests
using any of the registered credentials.
While the io_uring functionality is widely recognized as offering a
vastly improved, and high performing asynchronous I/O mechanism, its
ability to allow processes to submit I/O requests with credentials
other than its own presents a challenge to LSMs. When a process
creates a new io_uring ring the ring's credentials are inhertied
from the calling process; if this ring is shared with another
process operating with different credentials there is the potential
to bypass the LSMs security policy. Similarly, registering
credentials with a given ring allows any process with access to that
ring to submit I/O requests with those credentials.
In an effort to allow LSMs to apply security policy to io_uring I/O
operations, this patch adds two new LSM hooks. These hooks, in
conjunction with the LSM anonymous inode support previously
submitted, allow an LSM to apply access control policy to the
sharing of io_uring rings as well as any io_uring credential changes
requested by a process.
The new LSM hooks are described below:
* int security_uring_override_creds(cred)
Controls if the current task, executing an io_uring operation,
is allowed to override it's credentials with @cred. In cases
where the current task is a user application, the current
credentials will be those of the user application. In cases
where the current task is a kernel thread servicing io_uring
requests the current credentials will be those of the io_uring
ring (inherited from the process that created the ring).
* int security_uring_sqpoll(void)
Controls if the current task is allowed to create an io_uring
polling thread (IORING_SETUP_SQPOLL). Without a SQPOLL thread
in the kernel processes must submit I/O requests via
io_uring_enter(2) which allows us to compare any requested
credential changes against the application making the request.
With a SQPOLL thread, we can no longer compare requested
credential changes against the application making the request,
the comparison is made against the ring's credentials.
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
v2:
- no change
v1:
- initial draft
---
fs/io_uring.c | 10 ++++++++++
include/linux/lsm_hook_defs.h | 5 +++++
include/linux/lsm_hooks.h | 13 +++++++++++++
include/linux/security.h | 16 ++++++++++++++++
security/security.c | 12 ++++++++++++
5 files changed, 56 insertions(+)
From: Paul Moore <paul@paul-moore.com> Date: 2021-08-11 20:49:33
WARNING - This is a work in progress, this patch, including the
description, may be incomplete or even incorrect. You have been
warned.
This patch implements two new io_uring access controls, specifically
support for controlling the io_uring "personalities" and
IORING_SETUP_SQPOLL. Controlling the sharing of io_urings themselves
is handled via the normal file/inode labeling and sharing mechanisms.
The io_uring { override_creds } permission restricts which domains
the subject domain can use to override it's own credentials.
Granting a domain the io_uring { override_creds } permission allows
it to impersonate another domain in io_uring operations.
The io_uring { sqpoll } permission restricts which domains can create
asynchronous io_uring polling threads. This is important from a
security perspective as operations queued by this asynchronous thread
inherit the credentials of the thread creator by default; if an
io_uring is shared across process/domain boundaries this could result
in one domain impersonating another. Controlling the creation of
sqpoll threads, and the sharing of io_urings across processes, allow
policy authors to restrict the ability of one domain to impersonate
another via io_uring.
As a quick summary, this patch adds a new object class with two
permissions:
io_uring { override_creds sqpoll }
These permissions can be seen in the two simple policy statements
below:
allow domA_t domB_t : io_uring { override_creds };
allow domA_t self : io_uring { sqpoll };
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
v2:
- made the selinux_uring_* funcs static
- removed the debugging code
v1:
- initial draft
---
security/selinux/hooks.c | 34 ++++++++++++++++++++++++++++++++++
security/selinux/include/classmap.h | 2 ++
2 files changed, 36 insertions(+)
From: Paul Moore <paul@paul-moore.com> Date: 2021-08-11 20:49:34
From: Casey Schaufler <casey@schaufler-ca.com>
Add Smack privilege checks for io_uring. Use CAP_MAC_OVERRIDE
for the override_creds case and CAP_MAC_ADMIN for creating a
polling thread. These choices are based on conjecture regarding
the intent of the surrounding code.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
[PM: make the smack_uring_* funcs static]
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
v2:
- made the smack_uring_* funcs static
v1:
- initial draft
---
security/smack/smack_lsm.c | 64 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
@@ -4691,6 +4691,66 @@ static int smack_dentry_create_files_as(struct dentry *dentry, int mode,return0;}+#ifdef CONFIG_IO_URING+/**+*smack_uring_override_creds-Isio_uringcredoverrideallowed?+*@new:thetargetcreds+*+*Checktoseeifthecurrenttaskisallowedtooverrideit'scredentials+*toserviceanio_uringoperation.+*/+staticintsmack_uring_override_creds(conststructcred*new)+{+structtask_smack*tsp=smack_cred(current_cred());+structtask_smack*nsp=smack_cred(new);++#if 1+if(tsp->smk_task==nsp->smk_task)+pr_info("%s: Smack matches %s\n",__func__,+tsp->smk_task->smk_known);+else+pr_info("%s: Smack override check %s to %s\n",__func__,+tsp->smk_task->smk_known,nsp->smk_task->smk_known);+#endif+/*+*AllowthedegeneratecasewherethenewSmackvalueis+*thesameasthecurrentSmackvalue.+*/+if(tsp->smk_task==nsp->smk_task)+return0;++#if 1+pr_info("%s: Smack sqpoll %s\n",__func__,+smack_privileged_cred(CAP_MAC_OVERRIDE,current_cred())?+"ok by Smack":"disallowed (No CAP_MAC_OVERRIDE)");+#endif+if(smack_privileged_cred(CAP_MAC_OVERRIDE,current_cred()))+return0;++return-EPERM;+}++/**+*smack_uring_sqpoll-checkifaio_uringpollingthreadcanbecreated+*+*Checktoseeifthecurrenttaskisallowedtocreateanewio_uring+*kernelpollingthread.+*/+staticintsmack_uring_sqpoll(void)+{+#if 1+pr_info("%s: Smack new ring %s\n",__func__,+smack_privileged_cred(CAP_MAC_ADMIN,current_cred())?+"ok by Smack":"disallowed (No CAP_MAC_ADMIN)");+#endif+if(smack_privileged_cred(CAP_MAC_ADMIN,current_cred()))+return0;++return-EPERM;+}++#endif /* CONFIG_IO_URING */+structlsm_blob_sizessmack_blob_sizes__lsm_ro_after_init={.lbs_cred=sizeof(structtask_smack),.lbs_file=sizeof(structsmack_known*),
From: Paul Moore <paul@paul-moore.com> Date: 2021-08-31 14:45:18
On Wed, Aug 11, 2021 at 4:49 PM Paul Moore [off-list ref] wrote:
quoted hunk
From: Casey Schaufler <casey@schaufler-ca.com>
Add Smack privilege checks for io_uring. Use CAP_MAC_OVERRIDE
for the override_creds case and CAP_MAC_ADMIN for creating a
polling thread. These choices are based on conjecture regarding
the intent of the surrounding code.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
[PM: make the smack_uring_* funcs static]
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
v2:
- made the smack_uring_* funcs static
v1:
- initial draft
---
security/smack/smack_lsm.c | 64 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
@@ -4691,6 +4691,66 @@ static int smack_dentry_create_files_as(struct dentry *dentry, int mode,return0;}+#ifdef CONFIG_IO_URING+/**+*smack_uring_override_creds-Isio_uringcredoverrideallowed?+*@new:thetargetcreds+*+*Checktoseeifthecurrenttaskisallowedtooverrideit'scredentials+*toserviceanio_uringoperation.+*/+staticintsmack_uring_override_creds(conststructcred*new)+{+structtask_smack*tsp=smack_cred(current_cred());+structtask_smack*nsp=smack_cred(new);++#if 1+if(tsp->smk_task==nsp->smk_task)+pr_info("%s: Smack matches %s\n",__func__,+tsp->smk_task->smk_known);+else+pr_info("%s: Smack override check %s to %s\n",__func__,+tsp->smk_task->smk_known,nsp->smk_task->smk_known);+#endif
Casey, with the idea of posting a v3 towards the end of the merge
window next week, without the RFC tag and with the intention of
merging it into -next during the first/second week of the -rcX phase,
do you have any objections to me removing the debug code (#if 1 ...
#endif) from your patch? Did you have any other changes?
--
paul moore
www.paul-moore.com
On Wed, Aug 11, 2021 at 4:49 PM Paul Moore [off-list ref] wrote:
quoted
From: Casey Schaufler <casey@schaufler-ca.com>
Add Smack privilege checks for io_uring. Use CAP_MAC_OVERRIDE
for the override_creds case and CAP_MAC_ADMIN for creating a
polling thread. These choices are based on conjecture regarding
the intent of the surrounding code.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
[PM: make the smack_uring_* funcs static]
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
v2:
- made the smack_uring_* funcs static
v1:
- initial draft
---
security/smack/smack_lsm.c | 64 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
@@ -4691,6 +4691,66 @@ static int smack_dentry_create_files_as(struct dentry *dentry, int mode,return0;}+#ifdef CONFIG_IO_URING+/**+*smack_uring_override_creds-Isio_uringcredoverrideallowed?+*@new:thetargetcreds+*+*Checktoseeifthecurrenttaskisallowedtooverrideit'scredentials+*toserviceanio_uringoperation.+*/+staticintsmack_uring_override_creds(conststructcred*new)+{+structtask_smack*tsp=smack_cred(current_cred());+structtask_smack*nsp=smack_cred(new);++#if 1+if(tsp->smk_task==nsp->smk_task)+pr_info("%s: Smack matches %s\n",__func__,+tsp->smk_task->smk_known);+else+pr_info("%s: Smack override check %s to %s\n",__func__,+tsp->smk_task->smk_known,nsp->smk_task->smk_known);+#endif
Casey, with the idea of posting a v3 towards the end of the merge
window next week, without the RFC tag and with the intention of
merging it into -next during the first/second week of the -rcX phase,
do you have any objections to me removing the debug code (#if 1 ...
#endif) from your patch? Did you have any other changes?
I have no other changes. And yes, the debug code should be stripped.
Thank you.
From: Paul Moore <paul@paul-moore.com> Date: 2021-08-31 16:43:49
On Tue, Aug 31, 2021 at 11:03 AM Casey Schaufler [off-list ref] wrote:
On 8/31/2021 7:44 AM, Paul Moore wrote:
quoted
Casey, with the idea of posting a v3 towards the end of the merge
window next week, without the RFC tag and with the intention of
merging it into -next during the first/second week of the -rcX phase,
do you have any objections to me removing the debug code (#if 1 ...
#endif) from your patch? Did you have any other changes?
I have no other changes. And yes, the debug code should be stripped.
Thank you.
Great, I'll remove that code for the v3 dump.
--
paul moore
www.paul-moore.com
From: Paul Moore <paul@paul-moore.com> Date: 2021-08-11 20:49:49
WARNING - This is a work in progress and should not be merged
anywhere important. It is almost surely not complete, and while it
probably compiles it likely hasn't been booted and will do terrible
things. You have been warned.
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(). The io_uring operations are audited using a new
AUDIT_URINGOP record, an example is shown below:
% <TODO - insert AUDIT_URINGOP record example>
Thanks to Richard Guy Briggs for review and feedback.
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
v2:
- added dummy funcs for audit_uring_{entry,exit}()
- replaced opcode checks in io_issue_sqe() with audit_skip checks
- moved fastpath checks into audit_uring_{entry,exit}()
- audit_log_uring() uses GFP_ATOMIC
- don't record the arch in __audit_uring_entry()
v1:
- initial draft
---
fs/io-wq.c | 4 +
fs/io_uring.c | 55 ++++++++++++--
include/linux/audit.h | 26 +++++++
include/uapi/linux/audit.h | 1
kernel/audit.h | 2 +
kernel/auditsc.c | 174 ++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 256 insertions(+), 6 deletions(-)
@@ -896,6 +897,8 @@ struct io_op_def {unsignedneeds_async_setup:1;/* should block plug */unsignedplug:1;+/* skip auditing */+unsignedaudit_skip:1;/* size of async data needed, if any */unsignedshortasync_size;};
@@ -286,7 +286,10 @@ static inline int audit_signal_info(int sig, struct task_struct *t)/* These are defined in auditsc.c *//* Public API */externintaudit_alloc(structtask_struct*task);+externintaudit_alloc_kernel(structtask_struct*task);externvoid__audit_free(structtask_struct*task);+externvoid__audit_uring_entry(u8op);+externvoid__audit_uring_exit(intsuccess,longcode);externvoid__audit_syscall_entry(intmajor,unsignedlonga0,unsignedlonga1,unsignedlonga2,unsignedlonga3);externvoid__audit_syscall_exit(intret_success,longret_value);
@@ -100,10 +100,12 @@ struct audit_context {enum{AUDIT_CTX_UNUSED,/* audit_context is currently unused */AUDIT_CTX_SYSCALL,/* in use by syscall */+AUDIT_CTX_URING,/* in use by io_uring */}context;enumaudit_statestate,current_state;unsignedintserial;/* serial number for record */intmajor;/* syscall number */+inturing_op;/* uring operation */structtimespec64ctime;/* time of syscall entry */unsignedlongargv[4];/* syscall arguments */longreturn_code;/* syscall return code */
@@ -1044,6 +1045,31 @@ int audit_alloc(struct task_struct *tsk)return0;}+/**+*audit_alloc_kernel-allocateanaudit_contextforakerneltask+*@tsk:thekerneltask+*+*Similartotheaudit_alloc()function,butintendedforkernelprivate+*threads.Returnszeroonsuccess,negativevaluesonfailure.+*/+intaudit_alloc_kernel(structtask_struct*tsk)+{+/*+*Atthemomentwearejustgoingtocallintoaudit_alloc()to+*simplifythecode,buttheretwothingstokeepinmindwiththis+*approach:+*+*1.Filteringinternalkerneltasksisabitlaughableinalmostall+*cases,butthereisatleastonecasewherethereisabenefit:+*the'-atask,never'caseallowstheadmintoeffectivelydisable+*taskauditingatruntime.+*+*2.The{set,clear}_task_syscall_work()opslikelyhavezeroeffect+*ontheseinternalkerneltasks,buttheyprobablydon'thurteither.+*/+returnaudit_alloc(tsk);+}+staticinlinevoidaudit_free_context(structaudit_context*context){/* resetting is extra work, but it is likely just noise */
@@ -1751,6 +1826,105 @@ static void audit_return_fixup(struct audit_context *ctx,ctx->return_valid=(success?AUDITSC_SUCCESS:AUDITSC_FAILURE);}+/**+*__audit_uring_entry-preparethekerneltask'sauditcontextforio_uring+*@op:theio_uringopcode+*+*Thisissimilartoaudit_syscall_entry()butisintendedforusebyio_uring+*operations.Thisfunctionshouldonlyeverbecalledfrom+*audit_uring_entry()aswerelyontheauditcontextcheckingpresentinthat+*function.+*/+void__audit_uring_entry(u8op)+{+structaudit_context*ctx=audit_context();++if(ctx->state==AUDIT_STATE_DISABLED)+return;++/*+*NOTE:It'spossiblethatwecanbecalledfromtheprocess'context+*beforeitreturnstouserspace,andbeforeaudit_syscall_exit()+*iscalled.Inthiscasethereisnotmuchtodo,justrecord+*theio_uringdetailsandreturn.+*/+ctx->uring_op=op;+if(ctx->context==AUDIT_CTX_SYSCALL)+return;++ctx->dummy=!audit_n_rules;+if(!ctx->dummy&&ctx->state==AUDIT_STATE_BUILD)+ctx->prio=0;++ctx->context=AUDIT_CTX_URING;+ctx->current_state=ctx->state;+ktime_get_coarse_real_ts64(&ctx->ctime);+}++/**+*__audit_uring_exit-wrapupthekerneltask'sauditcontextafterio_uring+*@success:true/falsevaluetoindicateiftheoperationsucceededornot+*@code:operationreturncode+*+*Thisissimilartoaudit_syscall_exit()butisintendedforusebyio_uring+*operations.Thisfunctionshouldonlyeverbecalledfrom+*audit_uring_exit()aswerelyontheauditcontextcheckingpresentinthat+*function.+*/+void__audit_uring_exit(intsuccess,longcode)+{+structaudit_context*ctx=audit_context();++/*+*TODO:Atsomepointwewilllikelywanttofilteronio_uringops+*andotherthingssimilartowhatwedoforsyscalls,butthat+*issomethingforanotherday;justrecordwhatwecanhere.+*/++if(ctx->context==AUDIT_CTX_SYSCALL){+/*+*NOTE:Seethenotein__audit_uring_entry()aboutthecase+*wherewemaybecalledfromprocesscontextbeforewe+*returntouserspaceviaaudit_syscall_exit().Inthis+*casewesimplyemitaURINGOPrecordandbail,the+*normalsyscallexithandlingwilltakecareof+*everythingelse.+*Itisalsoworthmentioningthatwhenwearecalled,+*thecurrentprocesscredsmaydifferfromthecreds+*usedduringthenormalsyscallprocessing;keepthat+*inmindif/whenwemovetherecordgenerationcode.+*/++/*+*Weneedtofilteronthesyscallinfoheretodecideifwe+*shouldemitaURINGOPrecord.Iknowitseemsoddbutthis+*solvestheproblemwhereusershaveafiltertoblock*all*+*syscallrecordsinthe"exit"filter;wewanttopreserve+*thebehaviorhere.+*/+audit_filter_syscall(current,ctx);+audit_filter_inodes(current,ctx);+if(ctx->current_state!=AUDIT_STATE_RECORD)+return;++audit_log_uring(ctx);+return;+}++/* this may generate CONFIG_CHANGE records */+if(!list_empty(&ctx->killed_trees))+audit_kill_trees(ctx);++audit_filter_inodes(current,ctx);+if(ctx->current_state!=AUDIT_STATE_RECORD)+gotoout;+audit_return_fixup(ctx,success,code);+audit_log_exit();++out:+audit_reset_context(ctx);+}+/***__audit_syscall_entry-fillinanauditrecordatsyscallentry*@major:majorsyscalltype(function)
From: Richard Guy Briggs <hidden> Date: 2021-08-24 20:57:42
On 2021-08-11 16:48, Paul Moore wrote:
Draft #2 of the patchset which brings auditing and proper LSM access
controls to the io_uring subsystem. The original patchset was posted
in late May and can be found via lore using the link below:
https://lore.kernel.org/linux-security-module/162163367115.8379.8459012634106035341.stgit@sifl/
This draft should incorporate all of the feedback from the original
posting as well as a few smaller things I noticed while playing
further with the code. The big change is of course the selective
auditing in the io_uring op servicing, but that has already been
discussed quite a bit in the original thread so I won't go into
detail here; the important part is that we found a way to move
forward and this draft captures that. For those of you looking to
play with these patches, they are based on Linus' v5.14-rc5 tag and
on my test system they boot and appear to function without problem;
they pass the selinux-testsuite and audit-testsuite and I have not
noticed any regressions in the normal use of the system. If you want
to get a copy of these patches straight from git you can use the
"working-io_uring" branch in the repo below:
git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
Beyond the existing test suite tests mentioned above, I've cobbled
together some very basic, very crude tests to exercise some of the
things I care about from a LSM/audit perspective. These tests are
pretty awful (I'm not kidding), but they might be helpful for the
other LSM/audit developers who want to test things:
https://drop.paul-moore.com/90.kUgq
There are currently two tests: 'iouring.2' and 'iouring.3';
'iouring.1' was lost in a misguided and overzealous 'rm' command.
The first test is standalone and basically tests the SQPOLL
functionality while the second tests sharing io_urings across process
boundaries and the credential/personality sharing mechanism. The
console output of both tests isn't particularly useful, the more
interesting bits are in the audit and LSM specific logs. The
'iouring.2' command requires no special arguments to run but the
'iouring.3' test is split into a "server" and "client"; the server
should be run without argument:
% ./iouring.3s
>>> server started, pid = 11678
>>> memfd created, fd = 3
>>> io_uring created; fd = 5, creds = 1
... while the client should be run with two arguments: the first is
the PID of the server process, the second is the "memfd" fd number:
% ./iouring.3c 11678 3
>>> client started, server_pid = 11678 server_memfd = 3
>>> io_urings = 5 (server) / 5 (client)
>>> io_uring ops using creds = 1
>>> async op result: 36
>>> async op result: 36
>>> async op result: 36
>>> async op result: 36
>>> START file contents
What is this life if, full of care,
we have no time to stand and stare.
>>> END file contents
The tests were hacked together from various sources online,
attribution and links to additional info can be found in the test
sources, but I expect these tests to die a fiery death in the not
to distant future as I work to add some proper tests to the SELinux
and audit test suites.
As I believe these patches should spend a full -rcX cycle in
linux-next, my current plan is to continue to solicit feedback on
these patches while they undergo additional testing (next up is
verification of the audit filter code for io_uring). Assuming no
critical issues are found on the mailing lists or during testing, I
will post a proper patchset later with the idea of merging it into
selinux/next after the upcoming merge window closes.
Any comments, feedback, etc. are welcome.
Thanks for the tests. I have a bunch of userspace patches to add to the
last set I posted and these tests will help exercise them. I also have
one more kernel patch to post... I'll dive back into that now. I had
wanted to post them before now but got distracted with AUDIT_TRIM
breakage.
---
Casey Schaufler (1):
Smack: Brutalist io_uring support with debug
Paul Moore (8):
audit: prepare audit_context for use in calling contexts beyond
syscalls
audit,io_uring,io-wq: add some basic audit support to io_uring
audit: dev/test patch to force io_uring auditing
audit: add filtering for io_uring records
fs: add anon_inode_getfile_secure() similar to
anon_inode_getfd_secure()
io_uring: convert io_uring to the secure anon inode interface
lsm,io_uring: add LSM hooks to io_uring
selinux: add support for the io_uring access controls
fs/anon_inodes.c | 29 ++
fs/io-wq.c | 4 +
fs/io_uring.c | 69 +++-
include/linux/anon_inodes.h | 4 +
include/linux/audit.h | 26 ++
include/linux/lsm_hook_defs.h | 5 +
include/linux/lsm_hooks.h | 13 +
include/linux/security.h | 16 +
include/uapi/linux/audit.h | 4 +-
kernel/audit.h | 7 +-
kernel/audit_tree.c | 3 +-
kernel/audit_watch.c | 3 +-
kernel/auditfilter.c | 15 +-
kernel/auditsc.c | 483 +++++++++++++++++++-----
security/security.c | 12 +
security/selinux/hooks.c | 34 ++
security/selinux/include/classmap.h | 2 +
security/smack/smack_lsm.c | 64 ++++
18 files changed, 678 insertions(+), 115 deletions(-)
- 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
From: Paul Moore <paul@paul-moore.com> Date: 2021-08-24 22:28:08
On Tue, Aug 24, 2021 at 4:57 PM Richard Guy Briggs [off-list ref] wrote:
Thanks for the tests. I have a bunch of userspace patches to add to the
last set I posted and these tests will help exercise them. I also have
one more kernel patch to post... I'll dive back into that now. I had
wanted to post them before now but got distracted with AUDIT_TRIM
breakage.
If it helps, last week I started working on a little test tool for the
audit-testsuite and selinux-testsuite (see attached). It may not be
final, but I don't expect too many changes to it before I post the
test suite patches; it is definitely usable now. It's inspired by the
previous tests, but it uses a much more test suite friendly fork/exec
model for testing the sharing of io_urings across process boundaries.
Would you mind sharing your latest userspace patches, if not publicly
I would be okay with privately off-list; I'm putting together the test
suite patches this week and it would be good to make sure I'm using
your latest take on the userspace changes.
Also, what is the kernel patch? Did you find a bug or is this some
new functionality you think might be useful? Both can be important,
but the bug is *really* important; even if you don't have a fix for
that, just a description of the problem would be good.
--
paul moore
www.paul-moore.com
From: Richard Guy Briggs <hidden> Date: 2021-08-25 01:36:42
On 2021-08-24 18:27, Paul Moore wrote:
On Tue, Aug 24, 2021 at 4:57 PM Richard Guy Briggs [off-list ref] wrote:
quoted
Thanks for the tests. I have a bunch of userspace patches to add to the
last set I posted and these tests will help exercise them. I also have
one more kernel patch to post... I'll dive back into that now. I had
wanted to post them before now but got distracted with AUDIT_TRIM
breakage.
If it helps, last week I started working on a little test tool for the
audit-testsuite and selinux-testsuite (see attached). It may not be
final, but I don't expect too many changes to it before I post the
test suite patches; it is definitely usable now. It's inspired by the
previous tests, but it uses a much more test suite friendly fork/exec
model for testing the sharing of io_urings across process boundaries.
Would you mind sharing your latest userspace patches, if not publicly
I would be okay with privately off-list; I'm putting together the test
suite patches this week and it would be good to make sure I'm using
your latest take on the userspace changes.
I intend to publish them but they need squashing and some documentation
first. And a run through with io_uring specific tests would be good to
catch anything obvious...
Also, what is the kernel patch? Did you find a bug or is this some
new functionality you think might be useful? Both can be important,
but the bug is *really* important; even if you don't have a fix for
that, just a description of the problem would be good.
It was a very small patch that I realize I had already talked about and
you justified not including sessionid along with auid. That was
addressed in a reply tacked on to your v1 patchset just now.
paul moore
/*
* io_uring test tool to exercise LSM/SELinux and audit kernel code paths
* Author: Paul Moore [off-list ref]
*
* Copyright 2021 Microsoft Corporation
*
* At the time this code was written the best, and most current, source of info
* on io_uring seemed to be the liburing sources themselves (link below). The
* code below is based on the lessons learned from looking at the liburing
* code.
*
* -> https://github.com/axboe/liburing
*
* The liburing LICENSE file contains the following:
*
* Copyright 2020 Jens Axboe
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to
* deal in the Software without restriction, including without limitation the
* rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
* sell copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* DEALINGS IN THE SOFTWARE.
*
*/
/*
* BUILDING:
*
* gcc -o <binary> -g -O0 -luring -lrt <source>
*
* RUNNING:
*
* The program can be run using the following command lines:
*
* % prog sqpoll
* ... this invocation runs the io_uring SQPOLL test.
*
* % prog t1
* ... this invocation runs the parent/child io_uring sharing test.
*
* % prog t1 <domain>
* ... this invocation runs the parent/child io_uring sharing test with the
* child process run in the specified SELinux domain.
*
*/
#include <stdlib.h>
#include <stdio.h>
#include <errno.h>
#include <string.h>
#include <fcntl.h>
#include <unistd.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <sys/wait.h>
#include <liburing.h>
struct urt_config {
struct io_uring ring;
struct io_uring_params ring_params;
int ring_creds;
};
#define URING_ENTRIES 8
#define URING_SHM_NAME "/iouring_test_4"
int selinux_state = -1;
#define SELINUX_CTX_MAX 512
char selinux_ctx[SELINUX_CTX_MAX] = "\0";
/**
* Display an error message and exit
* @param msg the error message
*
* Output @msg to stderr and exit with errno as the exit value.
*/
void fatal(const char *msg)
{
const char *str = (msg ? msg : "unknown");
if (!errno) {
errno = 1;
fprintf(stderr, "%s: unknown error\n", msg);
} else
perror(str);
if (errno < 0)
exit(-errno);
exit(errno);
}
/**
* Determine if SELinux is enabled and set the internal state
*
* Attempt to read from /proc/self/attr/current and determine if SELinux is
* enabled, store the current context/domain in @selinux_ctx if SELinux is
* enabled. We avoid using the libselinux API in order to increase portability
* and make it easier for other LSMs to adopt this test.
*/
int selinux_enabled(void)
{
int fd = -1;
ssize_t ctx_len;
char ctx[SELINUX_CTX_MAX];
if (selinux_state >= 0)
return selinux_state;
/* attempt to get the current context */
fd = open("/proc/self/attr/current", O_RDONLY);
if (fd < 0)
goto err;
ctx_len = read(fd, ctx, SELINUX_CTX_MAX - 1);
if (ctx_len <= 0)
goto err;
close(fd);
/* save the current context */
ctx[ctx_len] = '\0';
strcpy(selinux_ctx, ctx);
selinux_state = 1;
return selinux_state;
err:
if (fd >= 0)
close(fd);
selinux_state = 0;
return selinux_state;
}
/**
* Return the current SELinux domain or "DISABLED" if SELinux is not enabled
*
* The returned string should not be free()'d.
*/
const char *selinux_current(void)
{
int rc;
rc = selinux_enabled();
if (!rc)
return "DISABLED";
return selinux_ctx;
}
/**
* Set the SELinux domain for the next exec()'d process
* @param ctx the SELinux domain
*
* This is similar to the setexeccon() libselinux API but we do it manually to
* help increase portability and make it easier for other LSMs to adopt this
* test.
*/
int selinux_exec(const char *ctx)
{
int fd = -1;
ssize_t len;
if (!ctx)
return -EINVAL;
fd = open("/proc/self/attr/exec", O_WRONLY);
if (fd < 0)
return -errno;
len = write(fd, ctx, strlen(ctx) + 1);
close(fd);
return len;
}
/**
* Setup the io_uring
* @param ring the io_uring pointer
* @param params the io_uring parameters
* @param creds pointer to the current process' registered io_uring personality
*
* Create a new io_uring using @params and return it in @ring with the
* registered personality returned in @creds. Returns 0 on success, negative
* values on failure.
*/
int uring_setup(struct io_uring *ring,
struct io_uring_params *params, int *creds)
{
int rc;
/* call into liburing to do the setup heavy lifting */
rc = io_uring_queue_init_params(URING_ENTRIES, ring, params);
if (rc < 0)
fatal("io_uring_queue_init_params");
/* register our creds/personality */
rc = io_uring_register_personality(ring);
if (rc < 0)
fatal("io_uring_register_personality()");
*creds = rc;
rc = 0;
printf(">>> io_uring created; fd = %d, personality = %d\n",
ring->ring_fd, *creds);
return rc;
}
/**
* Import an existing io_uring based on the given file descriptor
* @param fd the io_uring's file descriptor
* @param ring the io_uring pointer
* @param params the io_uring parameters
*
* This function takes an io_uring file descriptor in @fd as well as the
* io_uring parameters in @params and creates a valid io_uring in @ring.
* Returns 0 on success, negative values on failure.
*/
int uring_import(int fd, struct io_uring *ring, struct io_uring_params *params)
{
int rc;
memset(ring, 0, sizeof(*ring));
ring->flags = params->flags;
ring->features = params->features;
ring->ring_fd = fd;
ring->sq.ring_sz = params->sq_off.array +
params->sq_entries * sizeof(unsigned);
ring->cq.ring_sz = params->cq_off.cqes +
params->cq_entries * sizeof(struct io_uring_cqe);
ring->sq.ring_ptr = mmap(NULL, ring->sq.ring_sz, PROT_READ | PROT_WRITE,
MAP_SHARED | MAP_POPULATE, fd,
IORING_OFF_SQ_RING);
if (ring->sq.ring_ptr == MAP_FAILED)
fatal("import mmap(ring)");
ring->cq.ring_ptr = mmap(0, ring->cq.ring_sz, PROT_READ | PROT_WRITE,
MAP_SHARED | MAP_POPULATE,
fd, IORING_OFF_CQ_RING);
if (ring->cq.ring_ptr == MAP_FAILED) {
ring->cq.ring_ptr = NULL;
goto err;
}
ring->sq.khead = ring->sq.ring_ptr + params->sq_off.head;
ring->sq.ktail = ring->sq.ring_ptr + params->sq_off.tail;
ring->sq.kring_mask = ring->sq.ring_ptr + params->sq_off.ring_mask;
ring->sq.kring_entries = ring->sq.ring_ptr +
params->sq_off.ring_entries;
ring->sq.kflags = ring->sq.ring_ptr + params->sq_off.flags;
ring->sq.kdropped = ring->sq.ring_ptr + params->sq_off.dropped;
ring->sq.array = ring->sq.ring_ptr + params->sq_off.array;
ring->sq.sqes = mmap(NULL,
params->sq_entries * sizeof(struct io_uring_sqe),
PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE,
fd, IORING_OFF_SQES);
if (ring->sq.sqes == MAP_FAILED)
goto err;
ring->cq.khead = ring->cq.ring_ptr + params->cq_off.head;
ring->cq.ktail = ring->cq.ring_ptr + params->cq_off.tail;
ring->cq.kring_mask = ring->cq.ring_ptr + params->cq_off.ring_mask;
ring->cq.kring_entries = ring->cq.ring_ptr +
params->cq_off.ring_entries;
ring->cq.koverflow = ring->cq.ring_ptr + params->cq_off.overflow;
ring->cq.cqes = ring->cq.ring_ptr + params->cq_off.cqes;
if (params->cq_off.flags)
ring->cq.kflags = ring->cq.ring_ptr + params->cq_off.flags;
return 0;
err:
if (ring->sq.ring_ptr)
munmap(ring->sq.ring_ptr, ring->sq.ring_sz);
if (ring->cq.ring_ptr);
munmap(ring->cq.ring_ptr, ring->cq.ring_sz);
fatal("import mmap");
}
void uring_shutdown(struct io_uring *ring)
{
if (!ring)
return;
io_uring_queue_exit(ring);
}
/**
* An io_uring test
* @param ring the io_uring pointer
* @param personality the registered personality to use or 0
* @param path the file path to use for the test
*
* This function executes an io_uring test, see the function body for more
* details. Returns 0 on success, negative values on failure.
*/
int uring_op_a(struct io_uring *ring, int personality, const char *path)
{
#define __OP_A_BSIZE 512
#define __OP_A_STR "Lorem ipsum dolor sit amet.\n"
int rc;
int fds[1];
char buf1[__OP_A_BSIZE];
char buf2[__OP_A_BSIZE];
struct io_uring_sqe *sqe;
struct io_uring_cqe *cqe;
int str_sz = strlen(__OP_A_STR);
memset(buf1, 0, __OP_A_BSIZE);
memset(buf2, 0, __OP_A_BSIZE);
strncpy(buf1, __OP_A_STR, str_sz);
if (personality > 0)
printf(">>> io_uring ops using personality = %d\n",
personality);
/*
* open
*/
sqe = io_uring_get_sqe(ring);
if (!sqe)
fatal("io_uring_get_sqe(open)");
io_uring_prep_openat(sqe, AT_FDCWD, path,
O_RDWR | O_TRUNC | O_CREAT, 0644);
if (personality > 0)
sqe->personality = personality;
rc = io_uring_submit(ring);
if (rc < 0)
fatal("io_uring_submit(open)");
rc = io_uring_wait_cqe(ring, &cqe);
fds[0] = cqe->res;
if (rc < 0)
fatal("io_uring_wait_cqe(open)");
if (fds[0] < 0)
fatal("uring_open");
io_uring_cqe_seen(ring, cqe);
rc = io_uring_register_files(ring, fds, 1);
if(rc)
fatal("io_uring_register_files");
printf(">>> io_uring open(): OK\n");
/*
* write
*/
sqe = io_uring_get_sqe(ring);
if (!sqe)
fatal("io_uring_get_sqe(write1)");
io_uring_prep_write(sqe, 0, buf1, str_sz, 0);
io_uring_sqe_set_flags(sqe, IOSQE_FIXED_FILE);
if (personality > 0)
sqe->personality = personality;
rc = io_uring_submit(ring);
if (rc < 0)
fatal("io_uring_submit(write)");
rc = io_uring_wait_cqe(ring, &cqe);
if (rc < 0)
fatal("io_uring_wait_cqe(write)");
if (cqe->res < 0)
fatal("uring_write");
if (cqe->res != str_sz)
fatal("uring_write(length)");
io_uring_cqe_seen(ring, cqe);
printf(">>> io_uring write(): OK\n");
/*
* read
*/
sqe = io_uring_get_sqe(ring);
if (!sqe)
fatal("io_uring_get_sqe(read1)");
io_uring_prep_read(sqe, 0, buf2,__OP_A_BSIZE, 0);
io_uring_sqe_set_flags(sqe, IOSQE_FIXED_FILE);
if (personality > 0)
sqe->personality = personality;
rc = io_uring_submit(ring);
if (rc < 0)
fatal("io_uring_submit(read)");
rc = io_uring_wait_cqe(ring, &cqe);
if (rc < 0)
fatal("io_uring_wait_cqe(read)");
if (cqe->res < 0)
fatal("uring_read");
if (cqe->res != str_sz)
fatal("uring_read(length)");
io_uring_cqe_seen(ring, cqe);
if (strncmp(buf1, buf2, str_sz))
fatal("strncmp(buf1,buf2)");
printf(">>> io_uring read(): OK\n");
/*
* close
*/
sqe = io_uring_get_sqe(ring);
if (!sqe)
fatal("io_uring_get_sqe(close)");
io_uring_prep_close(sqe, 0);
if (personality > 0)
sqe->personality = personality;
rc = io_uring_submit(ring);
if (rc < 0)
fatal("io_uring_submit(close)");
rc = io_uring_wait_cqe(ring, &cqe);
if (rc < 0)
fatal("io_uring_wait_cqe(close)");
if (cqe->res < 0)
fatal("uring_close");
io_uring_cqe_seen(ring, cqe);
rc = io_uring_unregister_files(ring);
if (rc < 0)
fatal("io_uring_unregister_files");
printf(">>> io_uring close(): OK\n");
return 0;
}
/**
* The main entrypoint to the test program
* @param argc number of command line options
* @param argv the command line options array
*/
int main(int argc, char *argv[])
{
int rc = 1;
int ring_shm_fd;
struct io_uring ring_storage, *ring;
struct urt_config *cfg_p;
enum { TST_UNKNOWN,
TST_SQPOLL,
TST_T1_PARENT, TST_T1_CHILD } tst_method;
/* parse the command line and do some sanity checks */
tst_method = TST_UNKNOWN;
if (argc >= 2) {
if (!strcmp(argv[1], "sqpoll"))
tst_method = TST_SQPOLL;
else if (!strcmp(argv[1], "t1") ||
!strcmp(argv[1], "t1_parent"))
tst_method = TST_T1_PARENT;
else if (!strcmp(argv[1], "t1_child"))
tst_method = TST_T1_CHILD;
}
if (tst_method == TST_UNKNOWN) {
fprintf(stderr, "usage: %s <method> ... \n", argv[0]);
exit(EINVAL);
}
/* simple header */
printf(">>> running as PID = %d\n", getpid());
printf(">>> LSM/SELinux = %s\n", selinux_current());
/*
* test setup (if necessary)
*/
if (tst_method == TST_SQPOLL || tst_method == TST_T1_PARENT) {
/* create an io_uring and prepare it for optional sharing */
int flags;
/* create a shm segment to hold the io_uring info */
ring_shm_fd = shm_open(URING_SHM_NAME, O_CREAT | O_RDWR,
S_IRUSR | S_IWUSR);
if (ring_shm_fd < 0)
fatal("shm_open(create)");
rc = ftruncate(ring_shm_fd, sizeof(struct urt_config));
if (rc < 0)
fatal("ftruncate(shm)");
cfg_p = mmap(NULL, sizeof(*cfg_p), PROT_READ | PROT_WRITE,
MAP_SHARED, ring_shm_fd, 0);
if (!cfg_p)
fatal("mmap(shm)");
/* create the io_uring */
memset(&cfg_p->ring, 0, sizeof(cfg_p->ring));
memset(&cfg_p->ring_params, 0, sizeof(cfg_p->ring_params));
if (tst_method == TST_SQPOLL)
cfg_p->ring_params.flags |= IORING_SETUP_SQPOLL;
rc = uring_setup(&cfg_p->ring, &cfg_p->ring_params,
&cfg_p->ring_creds);
if (rc)
fatal("uring_setup");
ring = &cfg_p->ring;
/* explicitly clear FD_CLOEXEC on the io_uring */
flags = fcntl(cfg_p->ring.ring_fd, F_GETFD, 0);
if (flags < 0)
fatal("fcntl(ring_shm_fd,getfd)");
flags &= ~FD_CLOEXEC;
rc = fcntl(cfg_p->ring.ring_fd, F_SETFD, flags);
if (rc)
fatal("fcntl(ring_shm_fd,setfd)");
} else if (tst_method = TST_T1_CHILD) {
/* import a previously created and shared io_uring */
/* open the existing shm segment with the io_uring info */
ring_shm_fd = shm_open(URING_SHM_NAME, O_RDWR, 0);
if (ring_shm_fd < 0)
fatal("shm_open(existing)");
cfg_p = mmap(NULL, sizeof(*cfg_p), PROT_READ | PROT_WRITE,
MAP_SHARED, ring_shm_fd, 0);
if (!cfg_p)
fatal("mmap(shm)");
/* import the io_uring */
ring = &ring_storage;
rc = uring_import(cfg_p->ring.ring_fd,
ring, &cfg_p->ring_params);
if (rc < 0)
fatal("uring_import");
}
/*
* fork/exec a child process (if necessary)
*/
if (tst_method == TST_T1_PARENT) {
pid_t pid;
/* set the ctx for the next exec */
if (argc >= 3) {
printf(">>> set LSM/SELinux exec: %s\n",
(selinux_exec(argv[2]) > 0 ? "OK" : "FAILED"));
}
/* fork/exec */
pid = fork();
if (!pid) {
/* start the child */
rc = execl(argv[0], argv[0], "t1_child", (char *)NULL);
if (rc < 0)
fatal("exec");
} else {
/* wait for the child to exit */
int status;
waitpid(pid, &status, 0);
if (WIFEXITED(status))
rc = WEXITSTATUS(status);
}
}
/*
* run test(s)
*/
if (tst_method == TST_SQPOLL || tst_method == TST_T1_CHILD) {
rc = uring_op_a(ring, cfg_p->ring_creds, "/tmp/iouring.4.txt");
if (rc < 0)
fatal("uring_op_a(\"/tmp/iouring.4.txt\")");
}
/*
* cleanup
*/
if (tst_method == TST_SQPOLL || tst_method == TST_T1_PARENT) {
printf(">>> shutdown\n");
uring_shutdown(&cfg_p->ring);
shm_unlink(URING_SHM_NAME);
} else if (tst_method == TST_T1_CHILD) {
shm_unlink(URING_SHM_NAME);
}
return rc;
}
- 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
From: Richard Guy Briggs <hidden> Date: 2021-08-26 01:16:59
On 2021-08-24 16:57, Richard Guy Briggs wrote:
On 2021-08-11 16:48, Paul Moore wrote:
quoted
Draft #2 of the patchset which brings auditing and proper LSM access
controls to the io_uring subsystem. The original patchset was posted
in late May and can be found via lore using the link below:
https://lore.kernel.org/linux-security-module/162163367115.8379.8459012634106035341.stgit@sifl/
This draft should incorporate all of the feedback from the original
posting as well as a few smaller things I noticed while playing
further with the code. The big change is of course the selective
auditing in the io_uring op servicing, but that has already been
discussed quite a bit in the original thread so I won't go into
detail here; the important part is that we found a way to move
forward and this draft captures that. For those of you looking to
play with these patches, they are based on Linus' v5.14-rc5 tag and
on my test system they boot and appear to function without problem;
they pass the selinux-testsuite and audit-testsuite and I have not
noticed any regressions in the normal use of the system. If you want
to get a copy of these patches straight from git you can use the
"working-io_uring" branch in the repo below:
git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
Beyond the existing test suite tests mentioned above, I've cobbled
together some very basic, very crude tests to exercise some of the
things I care about from a LSM/audit perspective. These tests are
pretty awful (I'm not kidding), but they might be helpful for the
other LSM/audit developers who want to test things:
https://drop.paul-moore.com/90.kUgq
There are currently two tests: 'iouring.2' and 'iouring.3';
'iouring.1' was lost in a misguided and overzealous 'rm' command.
The first test is standalone and basically tests the SQPOLL
functionality while the second tests sharing io_urings across process
boundaries and the credential/personality sharing mechanism. The
console output of both tests isn't particularly useful, the more
interesting bits are in the audit and LSM specific logs. The
'iouring.2' command requires no special arguments to run but the
'iouring.3' test is split into a "server" and "client"; the server
should be run without argument:
% ./iouring.3s
>>> server started, pid = 11678
>>> memfd created, fd = 3
>>> io_uring created; fd = 5, creds = 1
... while the client should be run with two arguments: the first is
the PID of the server process, the second is the "memfd" fd number:
% ./iouring.3c 11678 3
>>> client started, server_pid = 11678 server_memfd = 3
>>> io_urings = 5 (server) / 5 (client)
>>> io_uring ops using creds = 1
>>> async op result: 36
>>> async op result: 36
>>> async op result: 36
>>> async op result: 36
>>> START file contents
What is this life if, full of care,
we have no time to stand and stare.
>>> END file contents
The tests were hacked together from various sources online,
attribution and links to additional info can be found in the test
sources, but I expect these tests to die a fiery death in the not
to distant future as I work to add some proper tests to the SELinux
and audit test suites.
As I believe these patches should spend a full -rcX cycle in
linux-next, my current plan is to continue to solicit feedback on
these patches while they undergo additional testing (next up is
verification of the audit filter code for io_uring). Assuming no
critical issues are found on the mailing lists or during testing, I
will post a proper patchset later with the idea of merging it into
selinux/next after the upcoming merge window closes.
Any comments, feedback, etc. are welcome.
Thanks for the tests. I have a bunch of userspace patches to add to the
last set I posted and these tests will help exercise them. I also have
one more kernel patch to post... I'll dive back into that now. I had
wanted to post them before now but got distracted with AUDIT_TRIM
breakage.
Please tell me about liburing.h that is needed for these. There is one
in tools/io_uring/liburing.h but I don't think that one is right.
The next obvious one would be include/uapi/linux/io_uring.h
I must be missing something obvious here...
quoted
---
Casey Schaufler (1):
Smack: Brutalist io_uring support with debug
Paul Moore (8):
audit: prepare audit_context for use in calling contexts beyond
syscalls
audit,io_uring,io-wq: add some basic audit support to io_uring
audit: dev/test patch to force io_uring auditing
audit: add filtering for io_uring records
fs: add anon_inode_getfile_secure() similar to
anon_inode_getfd_secure()
io_uring: convert io_uring to the secure anon inode interface
lsm,io_uring: add LSM hooks to io_uring
selinux: add support for the io_uring access controls
fs/anon_inodes.c | 29 ++
fs/io-wq.c | 4 +
fs/io_uring.c | 69 +++-
include/linux/anon_inodes.h | 4 +
include/linux/audit.h | 26 ++
include/linux/lsm_hook_defs.h | 5 +
include/linux/lsm_hooks.h | 13 +
include/linux/security.h | 16 +
include/uapi/linux/audit.h | 4 +-
kernel/audit.h | 7 +-
kernel/audit_tree.c | 3 +-
kernel/audit_watch.c | 3 +-
kernel/auditfilter.c | 15 +-
kernel/auditsc.c | 483 +++++++++++++++++++-----
security/security.c | 12 +
security/selinux/hooks.c | 34 ++
security/selinux/include/classmap.h | 2 +
security/smack/smack_lsm.c | 64 ++++
18 files changed, 678 insertions(+), 115 deletions(-)
- 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
- 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
From: Paul Moore <paul@paul-moore.com> Date: 2021-08-26 01:34:50
On Wed, Aug 25, 2021 at 9:16 PM Richard Guy Briggs [off-list ref] wrote:
On 2021-08-24 16:57, Richard Guy Briggs wrote:
quoted
On 2021-08-11 16:48, Paul Moore wrote:
quoted
Draft #2 of the patchset which brings auditing and proper LSM access
controls to the io_uring subsystem. The original patchset was posted
in late May and can be found via lore using the link below:
https://lore.kernel.org/linux-security-module/162163367115.8379.8459012634106035341.stgit@sifl/
This draft should incorporate all of the feedback from the original
posting as well as a few smaller things I noticed while playing
further with the code. The big change is of course the selective
auditing in the io_uring op servicing, but that has already been
discussed quite a bit in the original thread so I won't go into
detail here; the important part is that we found a way to move
forward and this draft captures that. For those of you looking to
play with these patches, they are based on Linus' v5.14-rc5 tag and
on my test system they boot and appear to function without problem;
they pass the selinux-testsuite and audit-testsuite and I have not
noticed any regressions in the normal use of the system. If you want
to get a copy of these patches straight from git you can use the
"working-io_uring" branch in the repo below:
git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
Beyond the existing test suite tests mentioned above, I've cobbled
together some very basic, very crude tests to exercise some of the
things I care about from a LSM/audit perspective. These tests are
pretty awful (I'm not kidding), but they might be helpful for the
other LSM/audit developers who want to test things:
https://drop.paul-moore.com/90.kUgq
There are currently two tests: 'iouring.2' and 'iouring.3';
'iouring.1' was lost in a misguided and overzealous 'rm' command.
The first test is standalone and basically tests the SQPOLL
functionality while the second tests sharing io_urings across process
boundaries and the credential/personality sharing mechanism. The
console output of both tests isn't particularly useful, the more
interesting bits are in the audit and LSM specific logs. The
'iouring.2' command requires no special arguments to run but the
'iouring.3' test is split into a "server" and "client"; the server
should be run without argument:
% ./iouring.3s
>>> server started, pid = 11678
>>> memfd created, fd = 3
>>> io_uring created; fd = 5, creds = 1
... while the client should be run with two arguments: the first is
the PID of the server process, the second is the "memfd" fd number:
% ./iouring.3c 11678 3
>>> client started, server_pid = 11678 server_memfd = 3
>>> io_urings = 5 (server) / 5 (client)
>>> io_uring ops using creds = 1
>>> async op result: 36
>>> async op result: 36
>>> async op result: 36
>>> async op result: 36
>>> START file contents
What is this life if, full of care,
we have no time to stand and stare.
>>> END file contents
The tests were hacked together from various sources online,
attribution and links to additional info can be found in the test
sources, but I expect these tests to die a fiery death in the not
to distant future as I work to add some proper tests to the SELinux
and audit test suites.
As I believe these patches should spend a full -rcX cycle in
linux-next, my current plan is to continue to solicit feedback on
these patches while they undergo additional testing (next up is
verification of the audit filter code for io_uring). Assuming no
critical issues are found on the mailing lists or during testing, I
will post a proper patchset later with the idea of merging it into
selinux/next after the upcoming merge window closes.
Any comments, feedback, etc. are welcome.
Thanks for the tests. I have a bunch of userspace patches to add to the
last set I posted and these tests will help exercise them. I also have
one more kernel patch to post... I'll dive back into that now. I had
wanted to post them before now but got distracted with AUDIT_TRIM
breakage.
Please tell me about liburing.h that is needed for these. There is one
in tools/io_uring/liburing.h but I don't think that one is right.
The next obvious one would be include/uapi/linux/io_uring.h
I must be missing something obvious here...
You are looking for the liburing header files, the upstream is here:
-> https://github.com/axboe/liburing
If you are on a RH/IBM based distro it is likely called liburing[-devel]:
% dnf whatprovides */liburing.h
Last metadata expiration check: 0:38:37 ago on Wed 25 Aug 2021 08:54:22 PM EDT.
liburing-devel-2.0-2.fc35.i686 : Development files for Linux-native io_uring I/O
: access library
Repo : rawhide
Matched from:
Filename : /usr/include/liburing.h
liburing-devel-2.0-2.fc35.x86_64 : Development files for Linux-native io_uring
: I/O access library
Repo : @System
Matched from:
Filename : /usr/include/liburing.h
liburing-devel-2.0-2.fc35.x86_64 : Development files for Linux-native io_uring
: I/O access library
Repo : rawhide
Matched from:
Filename : /usr/include/liburing.h
--
paul moore
www.paul-moore.com
From: Richard Guy Briggs <hidden> Date: 2021-08-26 16:32:54
On 2021-08-25 21:34, Paul Moore wrote:
On Wed, Aug 25, 2021 at 9:16 PM Richard Guy Briggs [off-list ref] wrote:
quoted
On 2021-08-24 16:57, Richard Guy Briggs wrote:
quoted
On 2021-08-11 16:48, Paul Moore wrote:
quoted
Draft #2 of the patchset which brings auditing and proper LSM access
controls to the io_uring subsystem. The original patchset was posted
in late May and can be found via lore using the link below:
https://lore.kernel.org/linux-security-module/162163367115.8379.8459012634106035341.stgit@sifl/
This draft should incorporate all of the feedback from the original
posting as well as a few smaller things I noticed while playing
further with the code. The big change is of course the selective
auditing in the io_uring op servicing, but that has already been
discussed quite a bit in the original thread so I won't go into
detail here; the important part is that we found a way to move
forward and this draft captures that. For those of you looking to
play with these patches, they are based on Linus' v5.14-rc5 tag and
on my test system they boot and appear to function without problem;
they pass the selinux-testsuite and audit-testsuite and I have not
noticed any regressions in the normal use of the system. If you want
to get a copy of these patches straight from git you can use the
"working-io_uring" branch in the repo below:
git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
Beyond the existing test suite tests mentioned above, I've cobbled
together some very basic, very crude tests to exercise some of the
things I care about from a LSM/audit perspective. These tests are
pretty awful (I'm not kidding), but they might be helpful for the
other LSM/audit developers who want to test things:
https://drop.paul-moore.com/90.kUgq
There are currently two tests: 'iouring.2' and 'iouring.3';
'iouring.1' was lost in a misguided and overzealous 'rm' command.
The first test is standalone and basically tests the SQPOLL
functionality while the second tests sharing io_urings across process
boundaries and the credential/personality sharing mechanism. The
console output of both tests isn't particularly useful, the more
interesting bits are in the audit and LSM specific logs. The
'iouring.2' command requires no special arguments to run but the
'iouring.3' test is split into a "server" and "client"; the server
should be run without argument:
% ./iouring.3s
>>> server started, pid = 11678
>>> memfd created, fd = 3
>>> io_uring created; fd = 5, creds = 1
... while the client should be run with two arguments: the first is
the PID of the server process, the second is the "memfd" fd number:
% ./iouring.3c 11678 3
>>> client started, server_pid = 11678 server_memfd = 3
>>> io_urings = 5 (server) / 5 (client)
>>> io_uring ops using creds = 1
>>> async op result: 36
>>> async op result: 36
>>> async op result: 36
>>> async op result: 36
>>> START file contents
What is this life if, full of care,
we have no time to stand and stare.
>>> END file contents
The tests were hacked together from various sources online,
attribution and links to additional info can be found in the test
sources, but I expect these tests to die a fiery death in the not
to distant future as I work to add some proper tests to the SELinux
and audit test suites.
As I believe these patches should spend a full -rcX cycle in
linux-next, my current plan is to continue to solicit feedback on
these patches while they undergo additional testing (next up is
verification of the audit filter code for io_uring). Assuming no
critical issues are found on the mailing lists or during testing, I
will post a proper patchset later with the idea of merging it into
selinux/next after the upcoming merge window closes.
Any comments, feedback, etc. are welcome.
Thanks for the tests. I have a bunch of userspace patches to add to the
last set I posted and these tests will help exercise them. I also have
one more kernel patch to post... I'll dive back into that now. I had
wanted to post them before now but got distracted with AUDIT_TRIM
breakage.
Please tell me about liburing.h that is needed for these. There is one
in tools/io_uring/liburing.h but I don't think that one is right.
The next obvious one would be include/uapi/linux/io_uring.h
I must be missing something obvious here...
You are looking for the liburing header files, the upstream is here:
-> https://github.com/axboe/liburing
If you are on a RH/IBM based distro it is likely called liburing[-devel]:
Found it but struct io_uring missing "features" in everything except
rawhide. Forced upgrade of my test VMs. :-)
audit-testsuite still passes.
I'm getting:
# ./iouring.2
Kernel thread io_uring-sq is not running.
Unable to setup io_uring: Permission denied
# ./iouring.3s
>>> server started, pid = 2082
>>> memfd created, fd = 3
io_uring_queue_init: Permission denied
I have CONFIG_IO_URING=y set, what else is needed?
% dnf whatprovides */liburing.h
Last metadata expiration check: 0:38:37 ago on Wed 25 Aug 2021 08:54:22 PM EDT.
liburing-devel-2.0-2.fc35.i686 : Development files for Linux-native io_uring I/O
: access library
Repo : rawhide
Matched from:
Filename : /usr/include/liburing.h
liburing-devel-2.0-2.fc35.x86_64 : Development files for Linux-native io_uring
: I/O access library
Repo : @System
Matched from:
Filename : /usr/include/liburing.h
liburing-devel-2.0-2.fc35.x86_64 : Development files for Linux-native io_uring
: I/O access library
Repo : rawhide
Matched from:
Filename : /usr/include/liburing.h
--
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
From: Richard Guy Briggs <hidden> Date: 2021-08-27 13:36:14
On 2021-08-26 15:14, Paul Moore wrote:
On Thu, Aug 26, 2021 at 12:32 PM Richard Guy Briggs [off-list ref] wrote:
quoted
I'm getting:
# ./iouring.2
Kernel thread io_uring-sq is not running.
Unable to setup io_uring: Permission denied
# ./iouring.3s
>>> server started, pid = 2082
>>> memfd created, fd = 3
io_uring_queue_init: Permission denied
I have CONFIG_IO_URING=y set, what else is needed?
I'm not sure how you tried to run those tests, but try running as root
and with SELinux in permissive mode.
Ok, they ran, including iouring.4. iouring.2 claimed twice: "Kernel
thread io_uring-sq is not running." and I didn't get any URING records
with ausearch. I don't know if any of this is expected.
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
From: Paul Moore <paul@paul-moore.com> Date: 2021-08-27 19:49:38
On Fri, Aug 27, 2021 at 9:36 AM Richard Guy Briggs [off-list ref] wrote:
On 2021-08-26 15:14, Paul Moore wrote:
quoted
On Thu, Aug 26, 2021 at 12:32 PM Richard Guy Briggs [off-list ref] wrote:
quoted
I'm getting:
# ./iouring.2
Kernel thread io_uring-sq is not running.
Unable to setup io_uring: Permission denied
# ./iouring.3s
>>> server started, pid = 2082
>>> memfd created, fd = 3
io_uring_queue_init: Permission denied
I have CONFIG_IO_URING=y set, what else is needed?
I'm not sure how you tried to run those tests, but try running as root
and with SELinux in permissive mode.
Ok, they ran, including iouring.4. iouring.2 claimed twice: "Kernel
thread io_uring-sq is not running." and I didn't get any URING records
with ausearch. I don't know if any of this is expected.
Now that I've written iouring.4, I would skip the others; while
helpful at the time, they are pretty crap.
I have no idea what kernel you are running, but I'm going to assume
you've applied the v2 patches (if not, you obviously need to do that
<g>). Beyond that you may need to set a filter for the
io_uring_enter() syscall to force the issue; theoretically your audit
userspace patches should allow a uring op specifically to be filtered
but I haven't had a chance to try that yet so either the kernel or
userspace portion could be broken.
At this point if you are running into problems you'll probably need to
spend some time debugging them, as I think you're the only person who
has tested your audit userspace patches at this point (and the only
one who has access to your latest bits).
--
paul moore
www.paul-moore.com
From: Richard Guy Briggs <hidden> Date: 2021-08-28 15:04:15
On 2021-08-27 15:49, Paul Moore wrote:
On Fri, Aug 27, 2021 at 9:36 AM Richard Guy Briggs [off-list ref] wrote:
quoted
On 2021-08-26 15:14, Paul Moore wrote:
quoted
On Thu, Aug 26, 2021 at 12:32 PM Richard Guy Briggs [off-list ref] wrote:
quoted
I'm getting:
# ./iouring.2
Kernel thread io_uring-sq is not running.
Unable to setup io_uring: Permission denied
# ./iouring.3s
>>> server started, pid = 2082
>>> memfd created, fd = 3
io_uring_queue_init: Permission denied
I have CONFIG_IO_URING=y set, what else is needed?
I'm not sure how you tried to run those tests, but try running as root
and with SELinux in permissive mode.
Ok, they ran, including iouring.4. iouring.2 claimed twice: "Kernel
thread io_uring-sq is not running." and I didn't get any URING records
with ausearch. I don't know if any of this is expected.
Now that I've written iouring.4, I would skip the others; while
helpful at the time, they are pretty crap.
Ok.
I have no idea what kernel you are running, but I'm going to assume
you've applied the v2 patches (if not, you obviously need to do that
<g>). Beyond that you may need to set a filter for the
io_uring_enter() syscall to force the issue; theoretically your audit
userspace patches should allow a uring op specifically to be filtered
but I haven't had a chance to try that yet so either the kernel or
userspace portion could be broken.
I'm running audit/next (on 5.14-rc1) with your v2 patches.
I did set a syscall filter for
-a exit,always -F arch=b64 -S io_uring_enter,io_uring_setup,io_uring_register -F key=iouringsyscall
and that yielded some records with a couple of orphans that surprised me
a bit. I've attached that log. I was a bit surprised there were no
records for ./iouring.3*.
I'm now testing the new "-a uring,always -U ..." to get that userspace
code working as expected...
At this point if you are running into problems you'll probably need to
spend some time debugging them, as I think you're the only person who
has tested your audit userspace patches at this point (and the only
one who has access to your latest bits).
Yes, I'll do some basic debugging and then publish to avoid wasting
people's time on silly bugs, but to get help on the more serious ones.
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
From: Paul Moore <paul@paul-moore.com> Date: 2021-08-29 15:18:46
On Sat, Aug 28, 2021 at 11:04 AM Richard Guy Briggs [off-list ref] wrote:
I did set a syscall filter for
-a exit,always -F arch=b64 -S io_uring_enter,io_uring_setup,io_uring_register -F key=iouringsyscall
and that yielded some records with a couple of orphans that surprised me
a bit.
Without looking too closely at the log you sent, you can expect URING
records without an associated SYSCALL record when the uring op is
being processed in the io-wq or sqpoll context. In the io-wq case the
processing is happening after the thread finished the syscall but
before the execution context returns to userspace and in the case of
sqpoll the processing is handled by a separate kernel thread with no
association to a process thread.
--
paul moore
www.paul-moore.com
From: Paul Moore <paul@paul-moore.com> Date: 2021-09-01 19:21:21
On Sun, Aug 29, 2021 at 11:18 AM Paul Moore [off-list ref] wrote:
On Sat, Aug 28, 2021 at 11:04 AM Richard Guy Briggs [off-list ref] wrote:
quoted
I did set a syscall filter for
-a exit,always -F arch=b64 -S io_uring_enter,io_uring_setup,io_uring_register -F key=iouringsyscall
and that yielded some records with a couple of orphans that surprised me
a bit.
Without looking too closely at the log you sent, you can expect URING
records without an associated SYSCALL record when the uring op is
being processed in the io-wq or sqpoll context. In the io-wq case the
processing is happening after the thread finished the syscall but
before the execution context returns to userspace and in the case of
sqpoll the processing is handled by a separate kernel thread with no
association to a process thread.
I spent some time this morning/afternoon playing with the io_uring
audit filtering capability and with your audit userspace
ghau-iouring-filtering.v1.0 branch it appears to work correctly. Yes,
the userspace tooling isn't quite 100% yet (e.g. `auditctl -l` doesn't
map the io_uring ops correctly), but I know you mentioned you have a
number of fixes/improvements still as a work-in-progress there so I'm
not too concerned. The important part is that the kernel pieces look
to be working correctly.
As usual, if you notice anything awry while playing with the userspace
changes please let me know.
--
paul moore
www.paul-moore.com
From: Richard Guy Briggs <hidden> Date: 2021-09-10 01:01:10
On 2021-09-01 15:21, Paul Moore wrote:
On Sun, Aug 29, 2021 at 11:18 AM Paul Moore [off-list ref] wrote:
quoted
On Sat, Aug 28, 2021 at 11:04 AM Richard Guy Briggs [off-list ref] wrote:
quoted
I did set a syscall filter for
-a exit,always -F arch=b64 -S io_uring_enter,io_uring_setup,io_uring_register -F key=iouringsyscall
and that yielded some records with a couple of orphans that surprised me
a bit.
Without looking too closely at the log you sent, you can expect URING
records without an associated SYSCALL record when the uring op is
being processed in the io-wq or sqpoll context. In the io-wq case the
processing is happening after the thread finished the syscall but
before the execution context returns to userspace and in the case of
sqpoll the processing is handled by a separate kernel thread with no
association to a process thread.
I spent some time this morning/afternoon playing with the io_uring
audit filtering capability and with your audit userspace
ghau-iouring-filtering.v1.0 branch it appears to work correctly. Yes,
the userspace tooling isn't quite 100% yet (e.g. `auditctl -l` doesn't
map the io_uring ops correctly), but I know you mentioned you have a
number of fixes/improvements still as a work-in-progress there so I'm
not too concerned. The important part is that the kernel pieces look
to be working correctly.
As usual, if you notice anything awry while playing with the userspace
changes please let me know.
Same for userspace... I think I already see one mapping uring op names
in ausearch...
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
From: Paul Moore <paul@paul-moore.com> Date: 2021-09-13 19:23:56
On Thu, Sep 9, 2021 at 8:59 PM Richard Guy Briggs [off-list ref] wrote:
On 2021-09-01 15:21, Paul Moore wrote:
quoted
On Sun, Aug 29, 2021 at 11:18 AM Paul Moore [off-list ref] wrote:
quoted
On Sat, Aug 28, 2021 at 11:04 AM Richard Guy Briggs [off-list ref] wrote:
quoted
I did set a syscall filter for
-a exit,always -F arch=b64 -S io_uring_enter,io_uring_setup,io_uring_register -F key=iouringsyscall
and that yielded some records with a couple of orphans that surprised me
a bit.
Without looking too closely at the log you sent, you can expect URING
records without an associated SYSCALL record when the uring op is
being processed in the io-wq or sqpoll context. In the io-wq case the
processing is happening after the thread finished the syscall but
before the execution context returns to userspace and in the case of
sqpoll the processing is handled by a separate kernel thread with no
association to a process thread.
I spent some time this morning/afternoon playing with the io_uring
audit filtering capability and with your audit userspace
ghau-iouring-filtering.v1.0 branch it appears to work correctly. Yes,
the userspace tooling isn't quite 100% yet (e.g. `auditctl -l` doesn't
map the io_uring ops correctly), but I know you mentioned you have a
number of fixes/improvements still as a work-in-progress there so I'm
not too concerned. The important part is that the kernel pieces look
to be working correctly.
Thanks Richard.
FYI, I rebased the io_uring/LSM/audit patchset on top of v5.15-rc1
today and tested both with your v1.0 and with your v2.1 branch and the
various combinations seemed to work just fine (of course the v2.1
userspace branch was more polished, less warts, etc.). I'm going to
go over the patch set one more time to make sure everything is still
looking good, write up an updated cover letter, and post a v3 revision
later tonight with the hope of merging it into -next later this week.
--
paul moore
www.paul-moore.com
From: Paul Moore <paul@paul-moore.com> Date: 2021-09-14 01:50:32
On Mon, Sep 13, 2021 at 3:23 PM Paul Moore [off-list ref] wrote:
On Thu, Sep 9, 2021 at 8:59 PM Richard Guy Briggs [off-list ref] wrote:
quoted
On 2021-09-01 15:21, Paul Moore wrote:
quoted
On Sun, Aug 29, 2021 at 11:18 AM Paul Moore [off-list ref] wrote:
quoted
On Sat, Aug 28, 2021 at 11:04 AM Richard Guy Briggs [off-list ref] wrote:
quoted
I did set a syscall filter for
-a exit,always -F arch=b64 -S io_uring_enter,io_uring_setup,io_uring_register -F key=iouringsyscall
and that yielded some records with a couple of orphans that surprised me
a bit.
Without looking too closely at the log you sent, you can expect URING
records without an associated SYSCALL record when the uring op is
being processed in the io-wq or sqpoll context. In the io-wq case the
processing is happening after the thread finished the syscall but
before the execution context returns to userspace and in the case of
sqpoll the processing is handled by a separate kernel thread with no
association to a process thread.
I spent some time this morning/afternoon playing with the io_uring
audit filtering capability and with your audit userspace
ghau-iouring-filtering.v1.0 branch it appears to work correctly. Yes,
the userspace tooling isn't quite 100% yet (e.g. `auditctl -l` doesn't
map the io_uring ops correctly), but I know you mentioned you have a
number of fixes/improvements still as a work-in-progress there so I'm
not too concerned. The important part is that the kernel pieces look
to be working correctly.
Thanks Richard.
FYI, I rebased the io_uring/LSM/audit patchset on top of v5.15-rc1
today and tested both with your v1.0 and with your v2.1 branch and the
various combinations seemed to work just fine (of course the v2.1
userspace branch was more polished, less warts, etc.). I'm going to
go over the patch set one more time to make sure everything is still
looking good, write up an updated cover letter, and post a v3 revision
later tonight with the hope of merging it into -next later this week.
Best laid plans of mice and men ...
It turns out the LSM hook macros are full of warnings-now-errors that
should likely be resolved before sending anything LSM related to
Linus. I'll post v3 once I fix this, which may not be until tomorrow.
(To be clear, the warnings/errors aren't new to this patchset, I'm
likely just the first person to notice them.)
--
paul moore
www.paul-moore.com
From: Paul Moore <paul@paul-moore.com> Date: 2021-09-14 02:49:55
On Mon, Sep 13, 2021 at 9:50 PM Paul Moore [off-list ref] wrote:
On Mon, Sep 13, 2021 at 3:23 PM Paul Moore [off-list ref] wrote:
quoted
On Thu, Sep 9, 2021 at 8:59 PM Richard Guy Briggs [off-list ref] wrote:
quoted
On 2021-09-01 15:21, Paul Moore wrote:
quoted
On Sun, Aug 29, 2021 at 11:18 AM Paul Moore [off-list ref] wrote:
quoted
On Sat, Aug 28, 2021 at 11:04 AM Richard Guy Briggs [off-list ref] wrote:
quoted
I did set a syscall filter for
-a exit,always -F arch=b64 -S io_uring_enter,io_uring_setup,io_uring_register -F key=iouringsyscall
and that yielded some records with a couple of orphans that surprised me
a bit.
Without looking too closely at the log you sent, you can expect URING
records without an associated SYSCALL record when the uring op is
being processed in the io-wq or sqpoll context. In the io-wq case the
processing is happening after the thread finished the syscall but
before the execution context returns to userspace and in the case of
sqpoll the processing is handled by a separate kernel thread with no
association to a process thread.
I spent some time this morning/afternoon playing with the io_uring
audit filtering capability and with your audit userspace
ghau-iouring-filtering.v1.0 branch it appears to work correctly. Yes,
the userspace tooling isn't quite 100% yet (e.g. `auditctl -l` doesn't
map the io_uring ops correctly), but I know you mentioned you have a
number of fixes/improvements still as a work-in-progress there so I'm
not too concerned. The important part is that the kernel pieces look
to be working correctly.
Thanks Richard.
FYI, I rebased the io_uring/LSM/audit patchset on top of v5.15-rc1
today and tested both with your v1.0 and with your v2.1 branch and the
various combinations seemed to work just fine (of course the v2.1
userspace branch was more polished, less warts, etc.). I'm going to
go over the patch set one more time to make sure everything is still
looking good, write up an updated cover letter, and post a v3 revision
later tonight with the hope of merging it into -next later this week.
Best laid plans of mice and men ...
It turns out the LSM hook macros are full of warnings-now-errors that
should likely be resolved before sending anything LSM related to
Linus. I'll post v3 once I fix this, which may not be until tomorrow.
(To be clear, the warnings/errors aren't new to this patchset, I'm
likely just the first person to notice them.)
Actually, scratch that ... I'm thinking that might just be an oddity
of the Intel 0day test robot building for the xtensa arch. I'll post
the v3 patchset tonight.
--
paul moore
www.paul-moore.com
From: Richard Guy Briggs <hidden> Date: 2021-09-15 12:29:26
On 2021-09-13 22:49, Paul Moore wrote:
On Mon, Sep 13, 2021 at 9:50 PM Paul Moore [off-list ref] wrote:
quoted
On Mon, Sep 13, 2021 at 3:23 PM Paul Moore [off-list ref] wrote:
quoted
On Thu, Sep 9, 2021 at 8:59 PM Richard Guy Briggs [off-list ref] wrote:
quoted
On 2021-09-01 15:21, Paul Moore wrote:
quoted
On Sun, Aug 29, 2021 at 11:18 AM Paul Moore [off-list ref] wrote:
quoted
On Sat, Aug 28, 2021 at 11:04 AM Richard Guy Briggs [off-list ref] wrote:
quoted
I did set a syscall filter for
-a exit,always -F arch=b64 -S io_uring_enter,io_uring_setup,io_uring_register -F key=iouringsyscall
and that yielded some records with a couple of orphans that surprised me
a bit.
Without looking too closely at the log you sent, you can expect URING
records without an associated SYSCALL record when the uring op is
being processed in the io-wq or sqpoll context. In the io-wq case the
processing is happening after the thread finished the syscall but
before the execution context returns to userspace and in the case of
sqpoll the processing is handled by a separate kernel thread with no
association to a process thread.
I spent some time this morning/afternoon playing with the io_uring
audit filtering capability and with your audit userspace
ghau-iouring-filtering.v1.0 branch it appears to work correctly. Yes,
the userspace tooling isn't quite 100% yet (e.g. `auditctl -l` doesn't
map the io_uring ops correctly), but I know you mentioned you have a
number of fixes/improvements still as a work-in-progress there so I'm
not too concerned. The important part is that the kernel pieces look
to be working correctly.
Thanks Richard.
FYI, I rebased the io_uring/LSM/audit patchset on top of v5.15-rc1
today and tested both with your v1.0 and with your v2.1 branch and the
various combinations seemed to work just fine (of course the v2.1
userspace branch was more polished, less warts, etc.). I'm going to
go over the patch set one more time to make sure everything is still
looking good, write up an updated cover letter, and post a v3 revision
later tonight with the hope of merging it into -next later this week.
Best laid plans of mice and men ...
It turns out the LSM hook macros are full of warnings-now-errors that
should likely be resolved before sending anything LSM related to
Linus. I'll post v3 once I fix this, which may not be until tomorrow.
(To be clear, the warnings/errors aren't new to this patchset, I'm
likely just the first person to notice them.)
Actually, scratch that ... I'm thinking that might just be an oddity
of the Intel 0day test robot building for the xtensa arch. I'll post
the v3 patchset tonight.
I was in the middle of reviewing the v2 patchset to add my acks when I
forgot to add the comment that you still haven't convinced me that ses=
isn't needed or relevant if we are including auid=.
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
From: Steve Grubb <hidden> Date: 2021-09-15 13:02:37
On Wednesday, September 15, 2021 8:29:08 AM EDT Richard Guy Briggs wrote:
I was in the middle of reviewing the v2 patchset to add my acks when I
forgot to add the comment that you still haven't convinced me that ses=
isn't needed or relevant if we are including auid=.
The session id is needed to disambiguate which login the event belongs to. It
is necessary sometimes to trace an event back to the login because it was a
remote login from an unexpected IP address.
-Steve
From: Paul Moore <paul@paul-moore.com> Date: 2021-09-15 14:12:34
On Wed, Sep 15, 2021 at 8:29 AM Richard Guy Briggs [off-list ref] wrote:
I was in the middle of reviewing the v2 patchset to add my acks when I
forgot to add the comment that you still haven't convinced me that ses=
isn't needed or relevant if we are including auid=.
[Side note: v3 was posted on Monday, it would be more helpful to see
the Reviewed-by tags on the v3 patchset.]
Ah, okay, it wasn't clear to me from your earlier comments that this
was your concern. It sounded as if you were arguing that both session
ID and audit ID needed to be logged for every io_uring op, which
doesn't make sense (as previously discussed). However, I see your
point, and in fact pulling the audit ID from @current in the
audit_log_uring() function is just plain wrong ... likely a vestige of
the original copy-n-paste or format matching, I'll drop that now.
Thanks.
While a small code change, it is somewhat significant so I'll post an
updated v4 patchset later today once it passes through a round of
testing.
--
paul moore
www.paul-moore.com