From: Paul Moore <paul@paul-moore.com>
To: Casey Schaufler <casey@schaufler-ca.com>
Cc: LSM <linux-security-module@vger.kernel.org>,
James Morris <jmorris@namei.org>,
John Johansen <john.johansen@canonical.com>,
Stephen Smalley <sds@tycho.nsa.gov>,
Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
Kees Cook <keescook@chromium.org>,
LKLM <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 2/3] LSM: module hierarchy in /proc/.../attr
Date: Fri, 24 Jun 2016 15:11:54 -0400 [thread overview]
Message-ID: <CAHC9VhSdhbsgPGOtrRkN76-TrUnxgL0m1WbU+dg73ybY=ELQ3w@mail.gmail.com> (raw)
In-Reply-To: <fdbc93e7-e60d-a29e-a690-0e617f16fd3b@schaufler-ca.com>
On Thu, Jun 23, 2016 at 5:11 PM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> Subject: [PATCH v4 2/3] LSM: module hierarchy in /proc/.../attr
>
> Back in 2007 I made what turned out to be a rather serious
> mistake in the implementation of the Smack security module.
> The SELinux module used an interface in /proc to manipulate
> the security context on processes. Rather than use a similar
> interface, I used the same interface. The AppArmor team did
> likewise. Now /proc/.../attr/current will tell you the
> security "context" of the process, but it will be different
> depending on the security module you're using. That hasn't
> been a problem to date, as you can only have one module
> that supports process attributes at a time. We are coming
> up on a change to that, where multiple modules with process
> attributes can be supported. (Not included here)
>
> This patch provides a subdirectory in /proc/.../attr for
> each of the security modules that use the LSM hooks
> getprocattr() and setprocattr(). Each of the interfaces
> used by a module are presented in the subdirectory. The
> old interfaces remain and work the same as before.
> User space code can begin migrating to the subdirectory
> interfaces in anticipation of the time when what comes
> from /proc/self/attr/current might not be what a runtime
> wants.
>
> The original implementation is by Kees Cook. The code
> has been changed a bit to reflect changes in the direction
> of the multiple concurrent module work, to be independent
> of it, and to bring it up to date with the current tree.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>
> ---
> Documentation/security/LSM.txt | 26 +++++++++---
> fs/proc/base.c | 91 +++++++++++++++++++++++++++++++++++++-----
> fs/proc/internal.h | 1 +
> include/linux/security.h | 15 ++++---
> security/security.c | 31 ++++++++++++--
> 5 files changed, 140 insertions(+), 24 deletions(-)
>
> diff --git a/Documentation/security/LSM.txt b/Documentation/security/LSM.txt
> index 3db7e67..125c489 100644
> --- a/Documentation/security/LSM.txt
> +++ b/Documentation/security/LSM.txt
> @@ -16,11 +16,25 @@ MAC extensions, other extensions can be built using the LSM to provide
> specific changes to system operation when these tweaks are not available
> in the core functionality of Linux itself.
>
> -Without a specific LSM built into the kernel, the default LSM will be the
> -Linux capabilities system. Most LSMs choose to extend the capabilities
> -system, building their checks on top of the defined capability hooks.
> -For more details on capabilities, see capabilities(7) in the Linux
> -man-pages project.
> +The Linux capabilities modules will always be included. For more details
> +on capabilities, see capabilities(7) in the Linux man-pages project.
> +This may be followed by any number of "minor" modules and at most one
> +"major" module.
> +
> +A list of the active security modules can be found by reading
> +/sys/kernel/security/lsm. This is a comma separated list, and
> +will always include the capability module. The list reflects the
> +order in which checks are made. The capability module will always
> +be first, followed by any "minor" modules (e.g. Yama) and then
> +the one "major" module (e.g. SELinux) if there is one configured.
I wouldn't respin it just for this, but it seems like the paragraph
above should really be part of patch 1/3, yes?
> +Process attributes associated with "ma
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index a11eb71..182bc28 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -131,9 +131,13 @@ struct pid_entry {
> #define REG(NAME, MODE, fops) \
> NOD(NAME, (S_IFREG|(MODE)), NULL, &fops, {})
> #define ONE(NAME, MODE, show) \
> - NOD(NAME, (S_IFREG|(MODE)), \
> + NOD(NAME, (S_IFREG|(MODE)), \
> NULL, &proc_single_file_operations, \
> { .proc_show = show } )
> +#define ATTR(LSM, NAME, MODE) \
> + NOD(NAME, (S_IFREG|(MODE)), \
> + NULL, &proc_pid_attr_operations, \
> + { .lsm = LSM })
>
> /*
> * Count the number of hardlinks for the pid_entry table, excluding the .
> @@ -2433,7 +2437,7 @@ static ssize_t proc_pid_attr_read(struct file * file, char __user * buf,
> if (!task)
> return -ESRCH;
>
> - length = security_getprocattr(task,
> + length = security_getprocattr(task, PROC_I(inode)->op.lsm,
> (char*)file->f_path.dentry->d_name.name,
> &p);
> put_task_struct(task);
> @@ -2473,7 +2477,7 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
> if (length < 0)
> goto out_free;
>
> - length = security_setprocattr(task,
> + length = security_setprocattr(task, PROC_I(inode)->op.lsm,
> (char*)file->f_path.dentry->d_name.name,
> page, count);
> mutex_unlock(&task->signal->cred_guard_mutex);
> @@ -2491,13 +2495,82 @@ static const struct file_operations proc_pid_attr_operations = {
> .llseek = generic_file_llseek,
> };
>
> +#define LSM_DIR_OPS(LSM) \
> +static int proc_##LSM##_attr_dir_iterate(struct file *filp, \
> + struct dir_context *ctx) \
> +{ \
> + return proc_pident_readdir(filp, ctx, \
> + LSM##_attr_dir_stuff, \
> + ARRAY_SIZE(LSM##_attr_dir_stuff)); \
> +} \
> +\
> +static const struct file_operations proc_##LSM##_attr_dir_ops = { \
> + .read = generic_read_dir, \
> + .iterate = proc_##LSM##_attr_dir_iterate, \
> + .llseek = default_llseek, \
> +}; \
> +\
> +static struct dentry *proc_##LSM##_attr_dir_lookup(struct inode *dir, \
> + struct dentry *dentry, unsigned int flags) \
> +{ \
> + return proc_pident_lookup(dir, dentry, \
> + LSM##_attr_dir_stuff, \
> + ARRAY_SIZE(LSM##_attr_dir_stuff)); \
> +} \
> +\
> +static const struct inode_operations proc_##LSM##_attr_dir_inode_ops = { \
> + .lookup = proc_##LSM##_attr_dir_lookup, \
> + .getattr = pid_getattr, \
> + .setattr = proc_setattr, \
> +}
> +
> +#ifdef CONFIG_SECURITY_SELINUX
> +static const struct pid_entry selinux_attr_dir_stuff[] = {
> + ATTR("selinux", "current", S_IRUGO|S_IWUGO),
> + ATTR("selinux", "prev", S_IRUGO),
> + ATTR("selinux", "exec", S_IRUGO|S_IWUGO),
> + ATTR("selinux", "fscreate", S_IRUGO|S_IWUGO),
> + ATTR("selinux", "keycreate", S_IRUGO|S_IWUGO),
> + ATTR("selinux", "sockcreate", S_IRUGO|S_IWUGO),
> +};
> +LSM_DIR_OPS(selinux);
> +#endif
> +
> +#ifdef CONFIG_SECURITY_SMACK
> +static const struct pid_entry smack_attr_dir_stuff[] = {
> + ATTR("smack", "current", S_IRUGO|S_IWUGO),
> +};
> +LSM_DIR_OPS(smack);
> +#endif
> +
> +#ifdef CONFIG_SECURITY_APPARMOR
> +static const struct pid_entry apparmor_attr_dir_stuff[] = {
> + ATTR("apparmor", "current", S_IRUGO|S_IWUGO),
> + ATTR("apparmor", "prev", S_IRUGO),
> + ATTR("apparmor", "exec", S_IRUGO|S_IWUGO),
> +};
> +LSM_DIR_OPS(apparmor);
> +#endif
> +
> static const struct pid_entry attr_dir_stuff[] = {
> - REG("current", S_IRUGO|S_IWUGO, proc_pid_attr_operations),
> - REG("prev", S_IRUGO, proc_pid_attr_operations),
> - REG("exec", S_IRUGO|S_IWUGO, proc_pid_attr_operations),
> - REG("fscreate", S_IRUGO|S_IWUGO, proc_pid_attr_operations),
> - REG("keycreate", S_IRUGO|S_IWUGO, proc_pid_attr_operations),
> - REG("sockcreate", S_IRUGO|S_IWUGO, proc_pid_attr_operations),
> + ATTR(NULL, "current", S_IRUGO|S_IWUGO),
> + ATTR(NULL, "prev", S_IRUGO),
> + ATTR(NULL, "exec", S_IRUGO|S_IWUGO),
> + ATTR(NULL, "fscreate", S_IRUGO|S_IWUGO),
> + ATTR(NULL, "keycreate", S_IRUGO|S_IWUGO),
> + ATTR(NULL, "sockcreate", S_IRUGO|S_IWUGO),
> +#ifdef CONFIG_SECURITY_SELINUX
> + DIR("selinux", S_IRUGO|S_IXUGO,
> + proc_selinux_attr_dir_inode_ops, proc_selinux_attr_dir_ops),
> +#endif
> +#ifdef CONFIG_SECURITY_SMACK
> + DIR("smack", S_IRUGO|S_IXUGO,
> + proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
> +#endif
> +#ifdef CONFIG_SECURITY_APPARMOR
> + DIR("apparmor", S_IRUGO|S_IXUGO,
> + proc_apparmor_attr_dir_inode_ops, proc_apparmor_attr_dir_ops),
> +#endif
> };
With the number of LSMs set to grow, it seems like it might be a lot
cleaner, and easier to maintain, if we moved the various LSM pid_entry
definitions into the LSMs themselves. Granted, I say this without
seriously looking at how one would do that, I'm just mentioning it
here; it may prove to be more hassle than it is worth.
--
paul moore
www.paul-moore.com
next prev parent reply other threads:[~2016-06-24 19:12 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-23 21:07 [PATCH v4 0/3] LSM: module hierarchy in /proc/.../attr Casey Schaufler
2016-06-23 21:10 ` [PATCH v4 1/3] LSM: Add /sys/kernel/security/lsm Casey Schaufler
2016-06-24 18:56 ` Paul Moore
2016-06-23 21:11 ` [PATCH v4 2/3] LSM: module hierarchy in /proc/.../attr Casey Schaufler
2016-06-24 19:11 ` Paul Moore [this message]
2016-06-24 20:05 ` Casey Schaufler
2016-06-24 20:08 ` Kees Cook
2016-06-24 20:29 ` Paul Moore
2016-06-24 23:26 ` [PATCH v5 0/3] LSM: security module information improvements Casey Schaufler
2016-06-24 23:27 ` [PATCH v5 1/3] LSM: Add /sys/kernel/security/lsm Casey Schaufler
2016-06-29 17:01 ` Paul Moore
2016-07-02 17:21 ` John Johansen
2016-06-24 23:29 ` [PATCH v5 2/3] LSM: module hierarchy in /proc/.../attr Casey Schaufler
2016-06-29 17:03 ` Paul Moore
2016-07-02 17:24 ` John Johansen
2016-06-24 23:29 ` [PATCH v4 3/3] LSM: Add context interface for proc attrs Casey Schaufler
2016-06-24 23:38 ` [PATCH v5 " Casey Schaufler
2016-06-29 17:04 ` [PATCH v4 " Paul Moore
2016-07-02 17:25 ` John Johansen
2016-07-05 15:52 ` [PATCH v5 0/3] LSM: security module information improvements - Acked Casey Schaufler
2016-07-08 10:05 ` James Morris
2016-07-08 15:31 ` Casey Schaufler
2016-07-05 15:52 ` [PATCH v5 1/3] LSM: Add /sys/kernel/security/lsm " Casey Schaufler
2016-07-05 15:52 ` [PATCH v5 2/3] LSM: module hierarchy in /proc/.../attr " Casey Schaufler
2016-07-05 15:52 ` [PATCH v5 3/3] LSM: Add context interface for proc attrs " Casey Schaufler
2016-06-23 21:11 ` [PATCH v4 3/3] LSM: Add context interface for proc attrs Casey Schaufler
2016-06-23 21:49 ` Kees Cook
2016-06-23 22:10 ` Casey Schaufler
2016-06-24 16:38 ` [PATCH v4 4/3] LSM: Improve " Casey Schaufler
2016-06-24 17:48 ` Kees Cook
2016-06-24 19:15 ` [PATCH v4 3/3] LSM: Add " Paul Moore
2016-06-24 19:56 ` Casey Schaufler
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='CAHC9VhSdhbsgPGOtrRkN76-TrUnxgL0m1WbU+dg73ybY=ELQ3w@mail.gmail.com' \
--to=paul@paul-moore.com \
--cc=casey@schaufler-ca.com \
--cc=jmorris@namei.org \
--cc=john.johansen@canonical.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=penguin-kernel@i-love.sakura.ne.jp \
--cc=sds@tycho.nsa.gov \
/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;
as well as URLs for NNTP newsgroup(s).