From: Paul Moore <[email protected]>
To: Joel Granados <[email protected]>
Cc: [email protected], [email protected], [email protected],
[email protected], [email protected],
[email protected], [email protected]
Subject: Re: [RFC v2 1/1] Use a fs callback to set security specific data
Date: Wed, 23 Nov 2022 16:02:02 -0500 [thread overview]
Message-ID: <CAHC9VhTs26x+6TSbPyQg7y0j=gLCc=_GPgmiUgA34wfxmakvZQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
On Tue, Nov 22, 2022 at 5:35 AM Joel Granados <[email protected]> wrote:
>
> Signed-off-by: Joel Granados <[email protected]>
> ---
> drivers/nvme/host/core.c | 10 ++++++++++
> include/linux/fs.h | 2 ++
> include/linux/lsm_hook_defs.h | 3 ++-
> include/linux/security.h | 16 ++++++++++++++--
> io_uring/uring_cmd.c | 3 ++-
> security/security.c | 5 +++--
> security/selinux/hooks.c | 16 +++++++++++++++-
> 7 files changed, 48 insertions(+), 7 deletions(-)
...
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index f553c370397e..9fe3a230c671 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -21,6 +21,8 @@
> * Copyright (C) 2016 Mellanox Technologies
> */
>
> +#include "linux/nvme_ioctl.h"
> +#include "linux/security.h"
> #include <linux/init.h>
> #include <linux/kd.h>
> #include <linux/kernel.h>
> @@ -6999,18 +7001,30 @@ static int selinux_uring_sqpoll(void)
> * IORING_OP_URING_CMD against the device/file specified in @ioucmd.
> *
> */
> -static int selinux_uring_cmd(struct io_uring_cmd *ioucmd)
> +static int selinux_uring_cmd(struct io_uring_cmd *ioucmd,
> + int (*uring_cmd_sec)(struct io_uring_cmd *ioucmd, struct security_uring_cmd*))
> {
As we discussed in the previous thread, and Casey mentioned already,
passing a function pointer for the LSM to call isn't a great practice.
When it was proposed we hadn't really thought of any alternatives, but
if we can't find a good scalar value to compare somewhere, I think
inspecting the file_operations::owner::name string to determine the
target is preferable to the function pointer approach described here.
Although I really would like to see us find, or create, some sort of
scalar token ID we could use instead. I fear that doing a lot of
strcmp()'s to identify the uring command target is going to be a
problem (one strcmp() for each possible target, multiplied by the
number of LSMs which implement a io_uring command hook).
> struct file *file = ioucmd->file;
> struct inode *inode = file_inode(file);
> struct inode_security_struct *isec = selinux_inode(inode);
> struct common_audit_data ad;
> + const struct cred *cred = current_cred();
> + struct security_uring_cmd sec_uring = {0};
> + int ret;
>
> ad.type = LSM_AUDIT_DATA_FILE;
> ad.u.file = file;
>
> + ret = uring_cmd_sec(ioucmd, &sec_uring);
> + if (ret)
> + return ret;
> +
> + if (sec_uring.flags & SECURITY_URING_CMD_TYPE_IOCTL)
> + return ioctl_has_perm(cred, file, FILE__IOCTL, (u16) ioucmd->cmd_op);
As mentioned previously, we'll need a SELinux policy capability here
to preserve the SECCLASS_IO_URING/IO_URING__CMD access check for
existing users/policies. I expect the logic would look something like
this (of course the details are dependent on how we identify the
target module/device/etc.):
if (polcap_foo && uring_tgt) {
switch (uring_tgt) {
case NVME:
return avc_has_perm(...);
default:
WARN();
return avc_has_perm(SECCLASS_IO_URING, IO_URING__CMD);
}
} else
return avc_has_perm(SECCLASS_IO_URING, IO_URING__CMD);
> return avc_has_perm(&selinux_state, current_sid(), isec->sid,
> SECCLASS_IO_URING, IO_URING__CMD, &ad);
> +
> }
> #endif /* CONFIG_IO_URING */
--
paul-moore.com
next prev parent reply other threads:[~2022-11-23 21:02 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20221122103536eucas1p2a0bc5ebdf063715f063e5b6254d0b058@eucas1p2.samsung.com>
2022-11-22 10:31 ` [RFC v2 0/1] RFC on how to include LSM hooks for io_uring commands Joel Granados
[not found] ` <CGME20221122103536eucas1p28f1c88f2300e49942c789721fe70c428@eucas1p2.samsung.com>
2022-11-22 10:31 ` [RFC v2 1/1] Use a fs callback to set security specific data Joel Granados
2022-11-22 15:18 ` Casey Schaufler
2022-11-28 8:19 ` Joel Granados
2022-11-28 9:06 ` Joel Granados
2022-11-23 21:02 ` Paul Moore [this message]
2022-11-28 9:27 ` Joel Granados
2022-11-29 14:24 ` Christoph Hellwig
2022-11-30 21:29 ` Joel Granados
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAHC9VhTs26x+6TSbPyQg7y0j=gLCc=_GPgmiUgA34wfxmakvZQ@mail.gmail.com' \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox