* [PATCH] vfs: Move kernel_read_file() to fs/read_write.c
From: David Howells @ 2019-03-04 15:11 UTC (permalink / raw)
To: viro
Cc: Mimi Zohar, dhowells, linux-fsdevel, linux-security-module,
linux-integrity, linux-kernel
Move kernel_read_file() to fs/read_write.c and out of fs/exec.c as it's not
actually used by anything in the execve subsystem.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
fs/exec.c | 106 -------------------------------------------------------
fs/read_write.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 106 insertions(+), 106 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index fb72d36f7823..cbb1a9cd25ca 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -61,7 +61,6 @@
#include <linux/pipe_fs_i.h>
#include <linux/oom.h>
#include <linux/compat.h>
-#include <linux/vmalloc.h>
#include <linux/uaccess.h>
#include <asm/mmu_context.h>
@@ -892,111 +891,6 @@ struct file *open_exec(const char *name)
}
EXPORT_SYMBOL(open_exec);
-int kernel_read_file(struct file *file, void **buf, loff_t *size,
- loff_t max_size, enum kernel_read_file_id id)
-{
- loff_t i_size, pos;
- ssize_t bytes = 0;
- int ret;
-
- if (!S_ISREG(file_inode(file)->i_mode) || max_size < 0)
- return -EINVAL;
-
- ret = deny_write_access(file);
- if (ret)
- return ret;
-
- ret = security_kernel_read_file(file, id);
- if (ret)
- goto out;
-
- i_size = i_size_read(file_inode(file));
- if (i_size <= 0) {
- ret = -EINVAL;
- goto out;
- }
- if (i_size > SIZE_MAX || (max_size > 0 && i_size > max_size)) {
- ret = -EFBIG;
- goto out;
- }
-
- if (id != READING_FIRMWARE_PREALLOC_BUFFER)
- *buf = vmalloc(i_size);
- if (!*buf) {
- ret = -ENOMEM;
- goto out;
- }
-
- pos = 0;
- while (pos < i_size) {
- bytes = kernel_read(file, *buf + pos, i_size - pos, &pos);
- if (bytes < 0) {
- ret = bytes;
- goto out;
- }
-
- if (bytes == 0)
- break;
- }
-
- if (pos != i_size) {
- ret = -EIO;
- goto out_free;
- }
-
- ret = security_kernel_post_read_file(file, *buf, i_size, id);
- if (!ret)
- *size = pos;
-
-out_free:
- if (ret < 0) {
- if (id != READING_FIRMWARE_PREALLOC_BUFFER) {
- vfree(*buf);
- *buf = NULL;
- }
- }
-
-out:
- allow_write_access(file);
- return ret;
-}
-EXPORT_SYMBOL_GPL(kernel_read_file);
-
-int kernel_read_file_from_path(const char *path, void **buf, loff_t *size,
- loff_t max_size, enum kernel_read_file_id id)
-{
- struct file *file;
- int ret;
-
- if (!path || !*path)
- return -EINVAL;
-
- file = filp_open(path, O_RDONLY, 0);
- if (IS_ERR(file))
- return PTR_ERR(file);
-
- ret = kernel_read_file(file, buf, size, max_size, id);
- fput(file);
- return ret;
-}
-EXPORT_SYMBOL_GPL(kernel_read_file_from_path);
-
-int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
- enum kernel_read_file_id id)
-{
- struct fd f = fdget(fd);
- int ret = -EBADF;
-
- if (!f.file)
- goto out;
-
- ret = kernel_read_file(f.file, buf, size, max_size, id);
-out:
- fdput(f);
- return ret;
-}
-EXPORT_SYMBOL_GPL(kernel_read_file_from_fd);
-
ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
{
ssize_t res = vfs_read(file, (void __user *)addr, len, &pos);
diff --git a/fs/read_write.c b/fs/read_write.c
index ff3c5e6f87cf..555dcaec00ac 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -20,6 +20,7 @@
#include <linux/compat.h>
#include <linux/mount.h>
#include <linux/fs.h>
+#include <linux/vmalloc.h>
#include "internal.h"
#include <linux/uaccess.h>
@@ -1362,6 +1363,111 @@ COMPAT_SYSCALL_DEFINE6(pwritev2, compat_ulong_t, fd,
#endif
+int kernel_read_file(struct file *file, void **buf, loff_t *size,
+ loff_t max_size, enum kernel_read_file_id id)
+{
+ loff_t i_size, pos;
+ ssize_t bytes = 0;
+ int ret;
+
+ if (!S_ISREG(file_inode(file)->i_mode) || max_size < 0)
+ return -EINVAL;
+
+ ret = deny_write_access(file);
+ if (ret)
+ return ret;
+
+ ret = security_kernel_read_file(file, id);
+ if (ret)
+ goto out;
+
+ i_size = i_size_read(file_inode(file));
+ if (i_size <= 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+ if (i_size > SIZE_MAX || (max_size > 0 && i_size > max_size)) {
+ ret = -EFBIG;
+ goto out;
+ }
+
+ if (id != READING_FIRMWARE_PREALLOC_BUFFER)
+ *buf = vmalloc(i_size);
+ if (!*buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ pos = 0;
+ while (pos < i_size) {
+ bytes = kernel_read(file, *buf + pos, i_size - pos, &pos);
+ if (bytes < 0) {
+ ret = bytes;
+ goto out;
+ }
+
+ if (bytes == 0)
+ break;
+ }
+
+ if (pos != i_size) {
+ ret = -EIO;
+ goto out_free;
+ }
+
+ ret = security_kernel_post_read_file(file, *buf, i_size, id);
+ if (!ret)
+ *size = pos;
+
+out_free:
+ if (ret < 0) {
+ if (id != READING_FIRMWARE_PREALLOC_BUFFER) {
+ vfree(*buf);
+ *buf = NULL;
+ }
+ }
+
+out:
+ allow_write_access(file);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(kernel_read_file);
+
+int kernel_read_file_from_path(const char *path, void **buf, loff_t *size,
+ loff_t max_size, enum kernel_read_file_id id)
+{
+ struct file *file;
+ int ret;
+
+ if (!path || !*path)
+ return -EINVAL;
+
+ file = filp_open(path, O_RDONLY, 0);
+ if (IS_ERR(file))
+ return PTR_ERR(file);
+
+ ret = kernel_read_file(file, buf, size, max_size, id);
+ fput(file);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(kernel_read_file_from_path);
+
+int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
+ enum kernel_read_file_id id)
+{
+ struct fd f = fdget(fd);
+ int ret = -EBADF;
+
+ if (!f.file)
+ goto out;
+
+ ret = kernel_read_file(f.file, buf, size, max_size, id);
+out:
+ fdput(f);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(kernel_read_file_from_fd);
+
static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos,
size_t count, loff_t max)
{
^ permalink raw reply related
* Re: [PATCH] vfs: Move kernel_read_file() to fs/read_write.c
From: Mimi Zohar @ 2019-03-04 16:12 UTC (permalink / raw)
To: David Howells, viro
Cc: Mimi Zohar, linux-fsdevel, linux-security-module, linux-integrity,
linux-kernel
In-Reply-To: <155171231301.4764.5429281379303710262.stgit@warthog.procyon.org.uk>
On Mon, 2019-03-04 at 15:11 +0000, David Howells wrote:
> Move kernel_read_file() to fs/read_write.c and out of fs/exec.c as it's not
> actually used by anything in the execve subsystem.
All files being opened by the kernel should be calling one of these
helper routines. Has that changed?
Mimi
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
>
> fs/exec.c | 106 -------------------------------------------------------
> fs/read_write.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 106 insertions(+), 106 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index fb72d36f7823..cbb1a9cd25ca 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -61,7 +61,6 @@
> #include <linux/pipe_fs_i.h>
> #include <linux/oom.h>
> #include <linux/compat.h>
> -#include <linux/vmalloc.h>
>
> #include <linux/uaccess.h>
> #include <asm/mmu_context.h>
> @@ -892,111 +891,6 @@ struct file *open_exec(const char *name)
> }
> EXPORT_SYMBOL(open_exec);
>
> -int kernel_read_file(struct file *file, void **buf, loff_t *size,
> - loff_t max_size, enum kernel_read_file_id id)
> -{
> - loff_t i_size, pos;
> - ssize_t bytes = 0;
> - int ret;
> -
> - if (!S_ISREG(file_inode(file)->i_mode) || max_size < 0)
> - return -EINVAL;
> -
> - ret = deny_write_access(file);
> - if (ret)
> - return ret;
> -
> - ret = security_kernel_read_file(file, id);
> - if (ret)
> - goto out;
> -
> - i_size = i_size_read(file_inode(file));
> - if (i_size <= 0) {
> - ret = -EINVAL;
> - goto out;
> - }
> - if (i_size > SIZE_MAX || (max_size > 0 && i_size > max_size)) {
> - ret = -EFBIG;
> - goto out;
> - }
> -
> - if (id != READING_FIRMWARE_PREALLOC_BUFFER)
> - *buf = vmalloc(i_size);
> - if (!*buf) {
> - ret = -ENOMEM;
> - goto out;
> - }
> -
> - pos = 0;
> - while (pos < i_size) {
> - bytes = kernel_read(file, *buf + pos, i_size - pos, &pos);
> - if (bytes < 0) {
> - ret = bytes;
> - goto out;
> - }
> -
> - if (bytes == 0)
> - break;
> - }
> -
> - if (pos != i_size) {
> - ret = -EIO;
> - goto out_free;
> - }
> -
> - ret = security_kernel_post_read_file(file, *buf, i_size, id);
> - if (!ret)
> - *size = pos;
> -
> -out_free:
> - if (ret < 0) {
> - if (id != READING_FIRMWARE_PREALLOC_BUFFER) {
> - vfree(*buf);
> - *buf = NULL;
> - }
> - }
> -
> -out:
> - allow_write_access(file);
> - return ret;
> -}
> -EXPORT_SYMBOL_GPL(kernel_read_file);
> -
> -int kernel_read_file_from_path(const char *path, void **buf, loff_t *size,
> - loff_t max_size, enum kernel_read_file_id id)
> -{
> - struct file *file;
> - int ret;
> -
> - if (!path || !*path)
> - return -EINVAL;
> -
> - file = filp_open(path, O_RDONLY, 0);
> - if (IS_ERR(file))
> - return PTR_ERR(file);
> -
> - ret = kernel_read_file(file, buf, size, max_size, id);
> - fput(file);
> - return ret;
> -}
> -EXPORT_SYMBOL_GPL(kernel_read_file_from_path);
> -
> -int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
> - enum kernel_read_file_id id)
> -{
> - struct fd f = fdget(fd);
> - int ret = -EBADF;
> -
> - if (!f.file)
> - goto out;
> -
> - ret = kernel_read_file(f.file, buf, size, max_size, id);
> -out:
> - fdput(f);
> - return ret;
> -}
> -EXPORT_SYMBOL_GPL(kernel_read_file_from_fd);
> -
> ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
> {
> ssize_t res = vfs_read(file, (void __user *)addr, len, &pos);
> diff --git a/fs/read_write.c b/fs/read_write.c
> index ff3c5e6f87cf..555dcaec00ac 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -20,6 +20,7 @@
> #include <linux/compat.h>
> #include <linux/mount.h>
> #include <linux/fs.h>
> +#include <linux/vmalloc.h>
> #include "internal.h"
>
> #include <linux/uaccess.h>
> @@ -1362,6 +1363,111 @@ COMPAT_SYSCALL_DEFINE6(pwritev2, compat_ulong_t, fd,
>
> #endif
>
> +int kernel_read_file(struct file *file, void **buf, loff_t *size,
> + loff_t max_size, enum kernel_read_file_id id)
> +{
> + loff_t i_size, pos;
> + ssize_t bytes = 0;
> + int ret;
> +
> + if (!S_ISREG(file_inode(file)->i_mode) || max_size < 0)
> + return -EINVAL;
> +
> + ret = deny_write_access(file);
> + if (ret)
> + return ret;
> +
> + ret = security_kernel_read_file(file, id);
> + if (ret)
> + goto out;
> +
> + i_size = i_size_read(file_inode(file));
> + if (i_size <= 0) {
> + ret = -EINVAL;
> + goto out;
> + }
> + if (i_size > SIZE_MAX || (max_size > 0 && i_size > max_size)) {
> + ret = -EFBIG;
> + goto out;
> + }
> +
> + if (id != READING_FIRMWARE_PREALLOC_BUFFER)
> + *buf = vmalloc(i_size);
> + if (!*buf) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + pos = 0;
> + while (pos < i_size) {
> + bytes = kernel_read(file, *buf + pos, i_size - pos, &pos);
> + if (bytes < 0) {
> + ret = bytes;
> + goto out;
> + }
> +
> + if (bytes == 0)
> + break;
> + }
> +
> + if (pos != i_size) {
> + ret = -EIO;
> + goto out_free;
> + }
> +
> + ret = security_kernel_post_read_file(file, *buf, i_size, id);
> + if (!ret)
> + *size = pos;
> +
> +out_free:
> + if (ret < 0) {
> + if (id != READING_FIRMWARE_PREALLOC_BUFFER) {
> + vfree(*buf);
> + *buf = NULL;
> + }
> + }
> +
> +out:
> + allow_write_access(file);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(kernel_read_file);
> +
> +int kernel_read_file_from_path(const char *path, void **buf, loff_t *size,
> + loff_t max_size, enum kernel_read_file_id id)
> +{
> + struct file *file;
> + int ret;
> +
> + if (!path || !*path)
> + return -EINVAL;
> +
> + file = filp_open(path, O_RDONLY, 0);
> + if (IS_ERR(file))
> + return PTR_ERR(file);
> +
> + ret = kernel_read_file(file, buf, size, max_size, id);
> + fput(file);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(kernel_read_file_from_path);
> +
> +int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
> + enum kernel_read_file_id id)
> +{
> + struct fd f = fdget(fd);
> + int ret = -EBADF;
> +
> + if (!f.file)
> + goto out;
> +
> + ret = kernel_read_file(f.file, buf, size, max_size, id);
> +out:
> + fdput(f);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(kernel_read_file_from_fd);
> +
> static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos,
> size_t count, loff_t max)
> {
>
^ permalink raw reply
* Re: [PATCH] vfs: Move kernel_read_file() to fs/read_write.c
From: David Howells @ 2019-03-04 16:22 UTC (permalink / raw)
To: Mimi Zohar
Cc: dhowells, viro, Mimi Zohar, linux-fsdevel, linux-security-module,
linux-integrity, linux-kernel
In-Reply-To: <1551715930.10911.468.camel@linux.ibm.com>
Mimi Zohar <zohar@linux.ibm.com> wrote:
> > Move kernel_read_file() to fs/read_write.c and out of fs/exec.c as it's not
> > actually used by anything in the execve subsystem.
>
> All files being opened by the kernel should be calling one of these
> helper routines. Has that changed?
prepare_binprm() uses kernel_read() and has done since at least 2014. The
binfmt drivers also use kernel_read().
Since kernel_read_file() is used by a bunch of things that aren't exec, even
if we switch exec to it, it should probably still go in fs/read_write.c since
it seems generic.
David
^ permalink raw reply
* Re: [PATCH] vfs: Move kernel_read_file() to fs/read_write.c
From: Mimi Zohar @ 2019-03-04 16:49 UTC (permalink / raw)
To: David Howells
Cc: viro, linux-fsdevel, linux-security-module, linux-integrity,
linux-kernel
In-Reply-To: <12096.1551716540@warthog.procyon.org.uk>
On Mon, 2019-03-04 at 16:22 +0000, David Howells wrote:
> Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> > > Move kernel_read_file() to fs/read_write.c and out of fs/exec.c as it's not
> > > actually used by anything in the execve subsystem.
> >
> > All files being opened by the kernel should be calling one of these
> > helper routines. Has that changed?
>
> prepare_binprm() uses kernel_read() and has done since at least 2014. The
> binfmt drivers also use kernel_read().
>
> Since kernel_read_file() is used by a bunch of things that aren't exec, even
> if we switch exec to it, it should probably still go in fs/read_write.c since
> it seems generic.
Oh, commit bdd1d2d3d251 ("fs: fix kernel_read prototype") moved
kernel_read() to fs/read_write.c without moving the helpers.
Definitely makes sense to move the helpers. Please include a
reference to the commit in this patch.
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
^ permalink raw reply
* Re: overlayfs access checks on underlying layers
From: Casey Schaufler @ 2019-03-04 17:56 UTC (permalink / raw)
To: Mark Salyzyn, Vivek Goyal, Miklos Szeredi
Cc: Stephen Smalley, Ondrej Mosnacek, J. Bruce Fields, Paul Moore,
linux-kernel, overlayfs, linux-fsdevel, selinux, Daniel J Walsh,
Linux Security Module list
In-Reply-To: <fe78aaed-5fb1-3c51-1330-39de46ae39c5@android.com>
On 3/4/2019 9:01 AM, Mark Salyzyn wrote:
Adding linux-security-module to the CC. Please keep the general
LSM community in to loop.
> On 11/29/2018 05:49 AM, Vivek Goyal wrote:
>> So will override_creds=off solve the NFS issue also where all access
>> will
>> happen with the creds of task now? Though it will stil require more
>> priviliges in task for other operations in overlay to succeed.
>
> NFS problems seems to have ended the discussion, too many
> stakeholders? too many outstanding questions?
>
> Do we accept the limitations of the override_creds patch as is, and
> then have the folks more familiar with the NFS scenario(s) build on it?
>
> [TL;DR]
>
> After looking at all this discussion, it feels like a larger audited
> rewrite of the security model is in order and override_creds=off may
> be a disservice (although expediently deals with Android's needs) to a
> correct general solution. I admit I have little idea where to go from
> here for a general solution.
>
> As far as I see it, the model of creator && caller credentials is a
> problem for any non-overlapping (MAC) privilege models. This patch
> allows one to drop any creator privilege escalation, re-introducing
> the "caller" to the lower layers.
>
> As such I would expect a better model is to _always_ check the caller
> credentials again in the lower layers, and only check the creator
> credentials, some without caller credentials, for some special cases?
> Change an && to an || for some of the checks? What are those special
> cases? I must admit _none_ of those special cases need attention in
> the Android usage models though making it difficult for me to do the
> fight thing for the associated stakeholders.
>
> The lower privileged application access to the directory cache
> inherited by other callers troubles me (not for Android, but in
> general) and feels troublesome (flush out the directory cache? how to
> tag the privileges associated with the current instance of the
> directory cache?). Some operations (eg: delete a file for incoming,
> create mknod in upperdir) are special cases requiring the checking of
> caller credentaisl to function (not a problem for Android as the
> caller that deletes a file just so happens to have the necessary
> privileges).
>
> Also, mount namespaces (in upper, lower, etc), how will they affect
> this all, is there a need for more attention to this as well?
>
> -- Mark
>
^ permalink raw reply
* Re: [PATCH v4 1/2] LSM: SafeSetID: gate setgid transitions
From: Micah Morton @ 2019-03-04 18:10 UTC (permalink / raw)
To: James Morris, Serge E. Hallyn, Kees Cook, Casey Schaufler,
Stephen Smalley, linux-security-module
In-Reply-To: <20190228235534.248869-1-mortonm@chromium.org>
On Thu, Feb 28, 2019 at 3:55 PM <mortonm@chromium.org> wrote:
>
> From: Micah Morton <mortonm@chromium.org>
>
> This patch generalizes the 'task_fix_setuid' LSM hook to enable hooking
> setgid transitions as well as setuid transitions. The hook is renamed to
> 'task_fix_setid'. The patch introduces calls to this hook from the
> setgid functions in kernel/sys.c. This will allow the SafeSetID LSM to
> govern setgid transitions in addition to setuid transitions. This patch
> also makes sure the setgid functions in kernel/sys.c call
> security_capable_setid rather than the ordinary security_capable
> function, so that the security_capable hook in the SafeSetID LSM knows
> it is being invoked from a setid function.
>
> Signed-off-by: Micah Morton <mortonm@chromium.org>
> ---
> Changes since the last patch: Add break statements for the
> setgid-related case statements in cap_task_fix_setid in
> security/commoncap.c. We don't want those cases to fall through to the
> default statement and return -EINVAL. Are the setreuid and setuid cases
> for this function always returning -EINVAL or am I missing something
> really obvious?.. Seems strange if that is the case.
Just realized that the 'break' statement in the 3rd case
(LSM_SETUID_RES) is what keeps the first 2 cases (LSM_SETUID_RE and
LSM_SETUID_ID) from dropping down to the 'default' case, which
explains the behavior I was seeing. Learned something new today about
switch statements.
>
> Documentation/admin-guide/LSM/SafeSetID.rst | 2 +-
> include/linux/lsm_hooks.h | 8 ++---
> include/linux/security.h | 36 ++++++++++++++-------
> kernel/sys.c | 35 ++++++++++++++------
> security/commoncap.c | 25 +++++++++-----
> security/safesetid/lsm.c | 12 +++----
> security/security.c | 4 +--
> 7 files changed, 79 insertions(+), 43 deletions(-)
>
> diff --git a/Documentation/admin-guide/LSM/SafeSetID.rst b/Documentation/admin-guide/LSM/SafeSetID.rst
> index 212434ef65ad..670a6544fd39 100644
> --- a/Documentation/admin-guide/LSM/SafeSetID.rst
> +++ b/Documentation/admin-guide/LSM/SafeSetID.rst
> @@ -88,7 +88,7 @@ other system interactions, including use of pid namespaces and device creation.
> Use an existing LSM
> -------------------
> None of the other in-tree LSMs have the capability to gate setid transitions, or
> -even employ the security_task_fix_setuid hook at all. SELinux says of that hook:
> +even employ the security_task_fix_setid hook at all. SELinux says of that hook:
> "Since setuid only affects the current process, and since the SELinux controls
> are not based on the Linux identity attributes, SELinux does not need to control
> this operation."
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 22fc786d723a..47fd04410fde 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -594,14 +594,14 @@
> * @size length of the file contents.
> * @id kernel read file identifier
> * Return 0 if permission is granted.
> - * @task_fix_setuid:
> + * @task_fix_setid:
> * Update the module's state after setting one or more of the user
> * identity attributes of the current process. The @flags parameter
> * indicates which of the set*uid system calls invoked this hook. If
> * @new is the set of credentials that will be installed. Modifications
> * should be made to this rather than to @current->cred.
> * @old is the set of credentials that are being replaces
> - * @flags contains one of the LSM_SETID_* values.
> + * @flags contains one of the LSM_SET*ID_* values.
> * Return 0 on success.
> * @task_setpgid:
> * Check permission before setting the process group identifier of the
> @@ -1594,7 +1594,7 @@ union security_list_options {
> int (*kernel_read_file)(struct file *file, enum kernel_read_file_id id);
> int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
> enum kernel_read_file_id id);
> - int (*task_fix_setuid)(struct cred *new, const struct cred *old,
> + int (*task_fix_setid)(struct cred *new, const struct cred *old,
> int flags);
> int (*task_setpgid)(struct task_struct *p, pid_t pgid);
> int (*task_getpgid)(struct task_struct *p);
> @@ -1886,7 +1886,7 @@ struct security_hook_heads {
> struct hlist_head kernel_read_file;
> struct hlist_head kernel_post_read_file;
> struct hlist_head kernel_module_request;
> - struct hlist_head task_fix_setuid;
> + struct hlist_head task_fix_setid;
> struct hlist_head task_setpgid;
> struct hlist_head task_getpgid;
> struct hlist_head task_getsid;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 13537a49ae97..76df3e22fed1 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -95,7 +95,7 @@ extern int cap_inode_getsecurity(struct inode *inode, const char *name,
> extern int cap_mmap_addr(unsigned long addr);
> extern int cap_mmap_file(struct file *file, unsigned long reqprot,
> unsigned long prot, unsigned long flags);
> -extern int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags);
> +extern int cap_task_fix_setid(struct cred *new, const struct cred *old, int flags);
> extern int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> unsigned long arg4, unsigned long arg5);
> extern int cap_task_setscheduler(struct task_struct *p);
> @@ -128,17 +128,29 @@ extern unsigned long dac_mmap_min_addr;
> /*
> * Values used in the task_security_ops calls
> */
> -/* setuid or setgid, id0 == uid or gid */
> -#define LSM_SETID_ID 1
> +/* setuid, id0 == uid */
> +#define LSM_SETUID_ID 1
>
> -/* setreuid or setregid, id0 == real, id1 == eff */
> -#define LSM_SETID_RE 2
> +/* setreuid, id0 == real, id1 == eff */
> +#define LSM_SETUID_RE 2
>
> -/* setresuid or setresgid, id0 == real, id1 == eff, uid2 == saved */
> -#define LSM_SETID_RES 4
> +/* setresuid, id0 == real, id1 == eff, uid2 == saved */
> +#define LSM_SETUID_RES 4
>
> -/* setfsuid or setfsgid, id0 == fsuid or fsgid */
> -#define LSM_SETID_FS 8
> +/* setfsuid, id0 == fsgid */
> +#define LSM_SETUID_FS 8
> +
> +/* setgid, id0 == gid */
> +#define LSM_SETGID_ID 16
> +
> +/* setregid, id0 == real, id1 == eff */
> +#define LSM_SETGID_RE 32
> +
> +/* setresgid, id0 == real, id1 == eff, uid2 == saved */
> +#define LSM_SETGID_RES 64
> +
> +/* setfsgid, id0 == fsgid */
> +#define LSM_SETGID_FS 128
>
> /* Flags for security_task_prlimit(). */
> #define LSM_PRLIMIT_READ 1
> @@ -324,7 +336,7 @@ int security_kernel_load_data(enum kernel_load_data_id id);
> int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
> int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
> enum kernel_read_file_id id);
> -int security_task_fix_setuid(struct cred *new, const struct cred *old,
> +int security_task_fix_setid(struct cred *new, const struct cred *old,
> int flags);
> int security_task_setpgid(struct task_struct *p, pid_t pgid);
> int security_task_getpgid(struct task_struct *p);
> @@ -923,11 +935,11 @@ static inline int security_kernel_post_read_file(struct file *file,
> return 0;
> }
>
> -static inline int security_task_fix_setuid(struct cred *new,
> +static inline int security_task_fix_setid(struct cred *new,
> const struct cred *old,
> int flags)
> {
> - return cap_task_fix_setuid(new, old, flags);
> + return cap_task_fix_setid(new, old, flags);
> }
>
> static inline int security_task_setpgid(struct task_struct *p, pid_t pgid)
> diff --git a/kernel/sys.c b/kernel/sys.c
> index c5f875048aef..615b44939238 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -372,7 +372,7 @@ long __sys_setregid(gid_t rgid, gid_t egid)
> if (rgid != (gid_t) -1) {
> if (gid_eq(old->gid, krgid) ||
> gid_eq(old->egid, krgid) ||
> - ns_capable(old->user_ns, CAP_SETGID))
> + ns_capable_setid(old->user_ns, CAP_SETGID))
> new->gid = krgid;
> else
> goto error;
> @@ -381,7 +381,7 @@ long __sys_setregid(gid_t rgid, gid_t egid)
> if (gid_eq(old->gid, kegid) ||
> gid_eq(old->egid, kegid) ||
> gid_eq(old->sgid, kegid) ||
> - ns_capable(old->user_ns, CAP_SETGID))
> + ns_capable_setid(old->user_ns, CAP_SETGID))
> new->egid = kegid;
> else
> goto error;
> @@ -392,6 +392,10 @@ long __sys_setregid(gid_t rgid, gid_t egid)
> new->sgid = new->egid;
> new->fsgid = new->egid;
>
> + retval = security_task_fix_setid(new, old, LSM_SETGID_RE);
> + if (retval < 0)
> + goto error;
> +
> return commit_creds(new);
>
> error:
> @@ -427,13 +431,17 @@ long __sys_setgid(gid_t gid)
> old = current_cred();
>
> retval = -EPERM;
> - if (ns_capable(old->user_ns, CAP_SETGID))
> + if (ns_capable_setid(old->user_ns, CAP_SETGID))
> new->gid = new->egid = new->sgid = new->fsgid = kgid;
> else if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->sgid))
> new->egid = new->fsgid = kgid;
> else
> goto error;
>
> + retval = security_task_fix_setid(new, old, LSM_SETGID_ID);
> + if (retval < 0)
> + goto error;
> +
> return commit_creds(new);
>
> error:
> @@ -539,7 +547,7 @@ long __sys_setreuid(uid_t ruid, uid_t euid)
> new->suid = new->euid;
> new->fsuid = new->euid;
>
> - retval = security_task_fix_setuid(new, old, LSM_SETID_RE);
> + retval = security_task_fix_setid(new, old, LSM_SETUID_RE);
> if (retval < 0)
> goto error;
>
> @@ -597,7 +605,7 @@ long __sys_setuid(uid_t uid)
>
> new->fsuid = new->euid = kuid;
>
> - retval = security_task_fix_setuid(new, old, LSM_SETID_ID);
> + retval = security_task_fix_setid(new, old, LSM_SETUID_ID);
> if (retval < 0)
> goto error;
>
> @@ -672,7 +680,7 @@ long __sys_setresuid(uid_t ruid, uid_t euid, uid_t suid)
> new->suid = ksuid;
> new->fsuid = new->euid;
>
> - retval = security_task_fix_setuid(new, old, LSM_SETID_RES);
> + retval = security_task_fix_setid(new, old, LSM_SETUID_RES);
> if (retval < 0)
> goto error;
>
> @@ -735,7 +743,7 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid)
> old = current_cred();
>
> retval = -EPERM;
> - if (!ns_capable(old->user_ns, CAP_SETGID)) {
> + if (!ns_capable_setid(old->user_ns, CAP_SETGID)) {
> if (rgid != (gid_t) -1 && !gid_eq(krgid, old->gid) &&
> !gid_eq(krgid, old->egid) && !gid_eq(krgid, old->sgid))
> goto error;
> @@ -755,6 +763,10 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid)
> new->sgid = ksgid;
> new->fsgid = new->egid;
>
> + retval = security_task_fix_setid(new, old, LSM_SETGID_RES);
> + if (retval < 0)
> + goto error;
> +
> return commit_creds(new);
>
> error:
> @@ -817,7 +829,7 @@ long __sys_setfsuid(uid_t uid)
> ns_capable_setid(old->user_ns, CAP_SETUID)) {
> if (!uid_eq(kuid, old->fsuid)) {
> new->fsuid = kuid;
> - if (security_task_fix_setuid(new, old, LSM_SETID_FS) == 0)
> + if (security_task_fix_setid(new, old, LSM_SETUID_FS) == 0)
> goto change_okay;
> }
> }
> @@ -858,10 +870,13 @@ long __sys_setfsgid(gid_t gid)
>
> if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->egid) ||
> gid_eq(kgid, old->sgid) || gid_eq(kgid, old->fsgid) ||
> - ns_capable(old->user_ns, CAP_SETGID)) {
> + ns_capable_setid(old->user_ns, CAP_SETGID)) {
> if (!gid_eq(kgid, old->fsgid)) {
> new->fsgid = kgid;
> - goto change_okay;
> + if (security_task_fix_setid(new,
> + old,
> + LSM_SETGID_FS) == 0)
> + goto change_okay;
> }
> }
>
> diff --git a/security/commoncap.c b/security/commoncap.c
> index f1d117c3d8ae..6f514d91d010 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -1026,27 +1026,27 @@ static inline void cap_emulate_setxuid(struct cred *new, const struct cred *old)
> }
>
> /**
> - * cap_task_fix_setuid - Fix up the results of setuid() call
> + * cap_task_fix_setid - Fix up the results of setid() call
> * @new: The proposed credentials
> * @old: The current task's current credentials
> * @flags: Indications of what has changed
> *
> - * Fix up the results of setuid() call before the credential changes are
> + * Fix up the results of setid() call before the credential changes are
> * actually applied, returning 0 to grant the changes, -ve to deny them.
> */
> -int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags)
> +int cap_task_fix_setid(struct cred *new, const struct cred *old, int flags)
> {
> switch (flags) {
> - case LSM_SETID_RE:
> - case LSM_SETID_ID:
> - case LSM_SETID_RES:
> + case LSM_SETUID_RE:
> + case LSM_SETUID_ID:
> + case LSM_SETUID_RES:
> /* juggle the capabilities to follow [RES]UID changes unless
> * otherwise suppressed */
> if (!issecure(SECURE_NO_SETUID_FIXUP))
> cap_emulate_setxuid(new, old);
> break;
>
> - case LSM_SETID_FS:
> + case LSM_SETUID_FS:
> /* juggle the capabilties to follow FSUID changes, unless
> * otherwise suppressed
> *
> @@ -1066,6 +1066,15 @@ int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags)
> }
> break;
>
> + case LSM_SETGID_RE:
> + break;
> + case LSM_SETGID_ID:
> + break;
> + case LSM_SETGID_RES:
> + break;
> + case LSM_SETGID_FS:
> + break;
> +
> default:
> return -EINVAL;
> }
> @@ -1355,7 +1364,7 @@ struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(inode_getsecurity, cap_inode_getsecurity),
> LSM_HOOK_INIT(mmap_addr, cap_mmap_addr),
> LSM_HOOK_INIT(mmap_file, cap_mmap_file),
> - LSM_HOOK_INIT(task_fix_setuid, cap_task_fix_setuid),
> + LSM_HOOK_INIT(task_fix_setid, cap_task_fix_setid),
> LSM_HOOK_INIT(task_prctl, cap_task_prctl),
> LSM_HOOK_INIT(task_setscheduler, cap_task_setscheduler),
> LSM_HOOK_INIT(task_setioprio, cap_task_setioprio),
> diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
> index cecd38e2ac80..5deffa92f25f 100644
> --- a/security/safesetid/lsm.c
> +++ b/security/safesetid/lsm.c
> @@ -120,7 +120,7 @@ static int check_uid_transition(kuid_t parent, kuid_t child)
> * set*uid to user under new cred struct, or the UID transition is allowed (by
> * Linux set*uid rules) even without CAP_SETUID.
> */
> -static int safesetid_task_fix_setuid(struct cred *new,
> +static int safesetid_task_fix_setid(struct cred *new,
> const struct cred *old,
> int flags)
> {
> @@ -130,7 +130,7 @@ static int safesetid_task_fix_setuid(struct cred *new,
> return 0;
>
> switch (flags) {
> - case LSM_SETID_RE:
> + case LSM_SETUID_RE:
> /*
> * Users for which setuid restrictions exist can only set the
> * real UID to the real UID or the effective UID, unless an
> @@ -152,7 +152,7 @@ static int safesetid_task_fix_setuid(struct cred *new,
> return check_uid_transition(old->euid, new->euid);
> }
> break;
> - case LSM_SETID_ID:
> + case LSM_SETUID_ID:
> /*
> * Users for which setuid restrictions exist cannot change the
> * real UID or saved set-UID unless an explicit whitelist
> @@ -163,7 +163,7 @@ static int safesetid_task_fix_setuid(struct cred *new,
> if (!uid_eq(old->suid, new->suid))
> return check_uid_transition(old->suid, new->suid);
> break;
> - case LSM_SETID_RES:
> + case LSM_SETUID_RES:
> /*
> * Users for which setuid restrictions exist cannot change the
> * real UID, effective UID, or saved set-UID to anything but
> @@ -187,7 +187,7 @@ static int safesetid_task_fix_setuid(struct cred *new,
> return check_uid_transition(old->suid, new->suid);
> }
> break;
> - case LSM_SETID_FS:
> + case LSM_SETUID_FS:
> /*
> * Users for which setuid restrictions exist cannot change the
> * filesystem UID to anything but one of: the current real UID,
> @@ -256,7 +256,7 @@ void flush_safesetid_whitelist_entries(void)
> }
>
> static struct security_hook_list safesetid_security_hooks[] = {
> - LSM_HOOK_INIT(task_fix_setuid, safesetid_task_fix_setuid),
> + LSM_HOOK_INIT(task_fix_setid, safesetid_task_fix_setid),
> LSM_HOOK_INIT(capable, safesetid_security_capable)
> };
>
> diff --git a/security/security.c b/security/security.c
> index ed9b8cbf21cf..450784fd1d2b 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1568,10 +1568,10 @@ int security_kernel_load_data(enum kernel_load_data_id id)
> }
> EXPORT_SYMBOL_GPL(security_kernel_load_data);
>
> -int security_task_fix_setuid(struct cred *new, const struct cred *old,
> +int security_task_fix_setid(struct cred *new, const struct cred *old,
> int flags)
> {
> - return call_int_hook(task_fix_setuid, 0, new, old, flags);
> + return call_int_hook(task_fix_setid, 0, new, old, flags);
> }
>
> int security_task_setpgid(struct task_struct *p, pid_t pgid)
> --
> 2.21.0.352.gf09ad66450-goog
>
^ permalink raw reply
* [PATCH v5 1/2] LSM: SafeSetID: gate setgid transitions
From: mortonm @ 2019-03-04 18:27 UTC (permalink / raw)
To: jmorris, serge, keescook, casey, sds, linux-security-module; +Cc: Micah Morton
In-Reply-To: <CAJ-EccMu97tZCWXTakcTwqFwzXdzVDedgc3jWaRvaJu-duH6tQ@mail.gmail.com>
From: Micah Morton <mortonm@chromium.org>
This patch generalizes the 'task_fix_setuid' LSM hook to enable hooking
setgid transitions as well as setuid transitions. The hook is renamed to
'task_fix_setid'. The patch introduces calls to this hook from the
setgid functions in kernel/sys.c. This will allow the SafeSetID LSM to
govern setgid transitions in addition to setuid transitions. This patch
also makes sure the setgid functions in kernel/sys.c call
security_capable_setid rather than the ordinary security_capable
function, so that the security_capable hook in the SafeSetID LSM knows
it is being invoked from a setid function.
Signed-off-by: Micah Morton <mortonm@chromium.org>
---
Changes since the last patch: Only one break is needed for the four
LSM_SETGID_* cases in security/commoncap.c.
Documentation/admin-guide/LSM/SafeSetID.rst | 2 +-
include/linux/lsm_hooks.h | 8 ++---
include/linux/security.h | 36 ++++++++++++++-------
kernel/sys.c | 35 ++++++++++++++------
security/commoncap.c | 22 ++++++++-----
security/safesetid/lsm.c | 12 +++----
security/security.c | 4 +--
7 files changed, 76 insertions(+), 43 deletions(-)
diff --git a/Documentation/admin-guide/LSM/SafeSetID.rst b/Documentation/admin-guide/LSM/SafeSetID.rst
index 212434ef65ad..670a6544fd39 100644
--- a/Documentation/admin-guide/LSM/SafeSetID.rst
+++ b/Documentation/admin-guide/LSM/SafeSetID.rst
@@ -88,7 +88,7 @@ other system interactions, including use of pid namespaces and device creation.
Use an existing LSM
-------------------
None of the other in-tree LSMs have the capability to gate setid transitions, or
-even employ the security_task_fix_setuid hook at all. SELinux says of that hook:
+even employ the security_task_fix_setid hook at all. SELinux says of that hook:
"Since setuid only affects the current process, and since the SELinux controls
are not based on the Linux identity attributes, SELinux does not need to control
this operation."
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 22fc786d723a..47fd04410fde 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -594,14 +594,14 @@
* @size length of the file contents.
* @id kernel read file identifier
* Return 0 if permission is granted.
- * @task_fix_setuid:
+ * @task_fix_setid:
* Update the module's state after setting one or more of the user
* identity attributes of the current process. The @flags parameter
* indicates which of the set*uid system calls invoked this hook. If
* @new is the set of credentials that will be installed. Modifications
* should be made to this rather than to @current->cred.
* @old is the set of credentials that are being replaces
- * @flags contains one of the LSM_SETID_* values.
+ * @flags contains one of the LSM_SET*ID_* values.
* Return 0 on success.
* @task_setpgid:
* Check permission before setting the process group identifier of the
@@ -1594,7 +1594,7 @@ union security_list_options {
int (*kernel_read_file)(struct file *file, enum kernel_read_file_id id);
int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
enum kernel_read_file_id id);
- int (*task_fix_setuid)(struct cred *new, const struct cred *old,
+ int (*task_fix_setid)(struct cred *new, const struct cred *old,
int flags);
int (*task_setpgid)(struct task_struct *p, pid_t pgid);
int (*task_getpgid)(struct task_struct *p);
@@ -1886,7 +1886,7 @@ struct security_hook_heads {
struct hlist_head kernel_read_file;
struct hlist_head kernel_post_read_file;
struct hlist_head kernel_module_request;
- struct hlist_head task_fix_setuid;
+ struct hlist_head task_fix_setid;
struct hlist_head task_setpgid;
struct hlist_head task_getpgid;
struct hlist_head task_getsid;
diff --git a/include/linux/security.h b/include/linux/security.h
index 13537a49ae97..76df3e22fed1 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -95,7 +95,7 @@ extern int cap_inode_getsecurity(struct inode *inode, const char *name,
extern int cap_mmap_addr(unsigned long addr);
extern int cap_mmap_file(struct file *file, unsigned long reqprot,
unsigned long prot, unsigned long flags);
-extern int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags);
+extern int cap_task_fix_setid(struct cred *new, const struct cred *old, int flags);
extern int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
unsigned long arg4, unsigned long arg5);
extern int cap_task_setscheduler(struct task_struct *p);
@@ -128,17 +128,29 @@ extern unsigned long dac_mmap_min_addr;
/*
* Values used in the task_security_ops calls
*/
-/* setuid or setgid, id0 == uid or gid */
-#define LSM_SETID_ID 1
+/* setuid, id0 == uid */
+#define LSM_SETUID_ID 1
-/* setreuid or setregid, id0 == real, id1 == eff */
-#define LSM_SETID_RE 2
+/* setreuid, id0 == real, id1 == eff */
+#define LSM_SETUID_RE 2
-/* setresuid or setresgid, id0 == real, id1 == eff, uid2 == saved */
-#define LSM_SETID_RES 4
+/* setresuid, id0 == real, id1 == eff, uid2 == saved */
+#define LSM_SETUID_RES 4
-/* setfsuid or setfsgid, id0 == fsuid or fsgid */
-#define LSM_SETID_FS 8
+/* setfsuid, id0 == fsgid */
+#define LSM_SETUID_FS 8
+
+/* setgid, id0 == gid */
+#define LSM_SETGID_ID 16
+
+/* setregid, id0 == real, id1 == eff */
+#define LSM_SETGID_RE 32
+
+/* setresgid, id0 == real, id1 == eff, uid2 == saved */
+#define LSM_SETGID_RES 64
+
+/* setfsgid, id0 == fsgid */
+#define LSM_SETGID_FS 128
/* Flags for security_task_prlimit(). */
#define LSM_PRLIMIT_READ 1
@@ -324,7 +336,7 @@ int security_kernel_load_data(enum kernel_load_data_id id);
int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
enum kernel_read_file_id id);
-int security_task_fix_setuid(struct cred *new, const struct cred *old,
+int security_task_fix_setid(struct cred *new, const struct cred *old,
int flags);
int security_task_setpgid(struct task_struct *p, pid_t pgid);
int security_task_getpgid(struct task_struct *p);
@@ -923,11 +935,11 @@ static inline int security_kernel_post_read_file(struct file *file,
return 0;
}
-static inline int security_task_fix_setuid(struct cred *new,
+static inline int security_task_fix_setid(struct cred *new,
const struct cred *old,
int flags)
{
- return cap_task_fix_setuid(new, old, flags);
+ return cap_task_fix_setid(new, old, flags);
}
static inline int security_task_setpgid(struct task_struct *p, pid_t pgid)
diff --git a/kernel/sys.c b/kernel/sys.c
index c5f875048aef..615b44939238 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -372,7 +372,7 @@ long __sys_setregid(gid_t rgid, gid_t egid)
if (rgid != (gid_t) -1) {
if (gid_eq(old->gid, krgid) ||
gid_eq(old->egid, krgid) ||
- ns_capable(old->user_ns, CAP_SETGID))
+ ns_capable_setid(old->user_ns, CAP_SETGID))
new->gid = krgid;
else
goto error;
@@ -381,7 +381,7 @@ long __sys_setregid(gid_t rgid, gid_t egid)
if (gid_eq(old->gid, kegid) ||
gid_eq(old->egid, kegid) ||
gid_eq(old->sgid, kegid) ||
- ns_capable(old->user_ns, CAP_SETGID))
+ ns_capable_setid(old->user_ns, CAP_SETGID))
new->egid = kegid;
else
goto error;
@@ -392,6 +392,10 @@ long __sys_setregid(gid_t rgid, gid_t egid)
new->sgid = new->egid;
new->fsgid = new->egid;
+ retval = security_task_fix_setid(new, old, LSM_SETGID_RE);
+ if (retval < 0)
+ goto error;
+
return commit_creds(new);
error:
@@ -427,13 +431,17 @@ long __sys_setgid(gid_t gid)
old = current_cred();
retval = -EPERM;
- if (ns_capable(old->user_ns, CAP_SETGID))
+ if (ns_capable_setid(old->user_ns, CAP_SETGID))
new->gid = new->egid = new->sgid = new->fsgid = kgid;
else if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->sgid))
new->egid = new->fsgid = kgid;
else
goto error;
+ retval = security_task_fix_setid(new, old, LSM_SETGID_ID);
+ if (retval < 0)
+ goto error;
+
return commit_creds(new);
error:
@@ -539,7 +547,7 @@ long __sys_setreuid(uid_t ruid, uid_t euid)
new->suid = new->euid;
new->fsuid = new->euid;
- retval = security_task_fix_setuid(new, old, LSM_SETID_RE);
+ retval = security_task_fix_setid(new, old, LSM_SETUID_RE);
if (retval < 0)
goto error;
@@ -597,7 +605,7 @@ long __sys_setuid(uid_t uid)
new->fsuid = new->euid = kuid;
- retval = security_task_fix_setuid(new, old, LSM_SETID_ID);
+ retval = security_task_fix_setid(new, old, LSM_SETUID_ID);
if (retval < 0)
goto error;
@@ -672,7 +680,7 @@ long __sys_setresuid(uid_t ruid, uid_t euid, uid_t suid)
new->suid = ksuid;
new->fsuid = new->euid;
- retval = security_task_fix_setuid(new, old, LSM_SETID_RES);
+ retval = security_task_fix_setid(new, old, LSM_SETUID_RES);
if (retval < 0)
goto error;
@@ -735,7 +743,7 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid)
old = current_cred();
retval = -EPERM;
- if (!ns_capable(old->user_ns, CAP_SETGID)) {
+ if (!ns_capable_setid(old->user_ns, CAP_SETGID)) {
if (rgid != (gid_t) -1 && !gid_eq(krgid, old->gid) &&
!gid_eq(krgid, old->egid) && !gid_eq(krgid, old->sgid))
goto error;
@@ -755,6 +763,10 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid)
new->sgid = ksgid;
new->fsgid = new->egid;
+ retval = security_task_fix_setid(new, old, LSM_SETGID_RES);
+ if (retval < 0)
+ goto error;
+
return commit_creds(new);
error:
@@ -817,7 +829,7 @@ long __sys_setfsuid(uid_t uid)
ns_capable_setid(old->user_ns, CAP_SETUID)) {
if (!uid_eq(kuid, old->fsuid)) {
new->fsuid = kuid;
- if (security_task_fix_setuid(new, old, LSM_SETID_FS) == 0)
+ if (security_task_fix_setid(new, old, LSM_SETUID_FS) == 0)
goto change_okay;
}
}
@@ -858,10 +870,13 @@ long __sys_setfsgid(gid_t gid)
if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->egid) ||
gid_eq(kgid, old->sgid) || gid_eq(kgid, old->fsgid) ||
- ns_capable(old->user_ns, CAP_SETGID)) {
+ ns_capable_setid(old->user_ns, CAP_SETGID)) {
if (!gid_eq(kgid, old->fsgid)) {
new->fsgid = kgid;
- goto change_okay;
+ if (security_task_fix_setid(new,
+ old,
+ LSM_SETGID_FS) == 0)
+ goto change_okay;
}
}
diff --git a/security/commoncap.c b/security/commoncap.c
index f1d117c3d8ae..4a038d3c7196 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1026,27 +1026,27 @@ static inline void cap_emulate_setxuid(struct cred *new, const struct cred *old)
}
/**
- * cap_task_fix_setuid - Fix up the results of setuid() call
+ * cap_task_fix_setid - Fix up the results of setid() call
* @new: The proposed credentials
* @old: The current task's current credentials
* @flags: Indications of what has changed
*
- * Fix up the results of setuid() call before the credential changes are
+ * Fix up the results of setid() call before the credential changes are
* actually applied, returning 0 to grant the changes, -ve to deny them.
*/
-int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags)
+int cap_task_fix_setid(struct cred *new, const struct cred *old, int flags)
{
switch (flags) {
- case LSM_SETID_RE:
- case LSM_SETID_ID:
- case LSM_SETID_RES:
+ case LSM_SETUID_RE:
+ case LSM_SETUID_ID:
+ case LSM_SETUID_RES:
/* juggle the capabilities to follow [RES]UID changes unless
* otherwise suppressed */
if (!issecure(SECURE_NO_SETUID_FIXUP))
cap_emulate_setxuid(new, old);
break;
- case LSM_SETID_FS:
+ case LSM_SETUID_FS:
/* juggle the capabilties to follow FSUID changes, unless
* otherwise suppressed
*
@@ -1066,6 +1066,12 @@ int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags)
}
break;
+ case LSM_SETGID_RE:
+ case LSM_SETGID_ID:
+ case LSM_SETGID_RES:
+ case LSM_SETGID_FS:
+ break;
+
default:
return -EINVAL;
}
@@ -1355,7 +1361,7 @@ struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(inode_getsecurity, cap_inode_getsecurity),
LSM_HOOK_INIT(mmap_addr, cap_mmap_addr),
LSM_HOOK_INIT(mmap_file, cap_mmap_file),
- LSM_HOOK_INIT(task_fix_setuid, cap_task_fix_setuid),
+ LSM_HOOK_INIT(task_fix_setid, cap_task_fix_setid),
LSM_HOOK_INIT(task_prctl, cap_task_prctl),
LSM_HOOK_INIT(task_setscheduler, cap_task_setscheduler),
LSM_HOOK_INIT(task_setioprio, cap_task_setioprio),
diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
index cecd38e2ac80..5deffa92f25f 100644
--- a/security/safesetid/lsm.c
+++ b/security/safesetid/lsm.c
@@ -120,7 +120,7 @@ static int check_uid_transition(kuid_t parent, kuid_t child)
* set*uid to user under new cred struct, or the UID transition is allowed (by
* Linux set*uid rules) even without CAP_SETUID.
*/
-static int safesetid_task_fix_setuid(struct cred *new,
+static int safesetid_task_fix_setid(struct cred *new,
const struct cred *old,
int flags)
{
@@ -130,7 +130,7 @@ static int safesetid_task_fix_setuid(struct cred *new,
return 0;
switch (flags) {
- case LSM_SETID_RE:
+ case LSM_SETUID_RE:
/*
* Users for which setuid restrictions exist can only set the
* real UID to the real UID or the effective UID, unless an
@@ -152,7 +152,7 @@ static int safesetid_task_fix_setuid(struct cred *new,
return check_uid_transition(old->euid, new->euid);
}
break;
- case LSM_SETID_ID:
+ case LSM_SETUID_ID:
/*
* Users for which setuid restrictions exist cannot change the
* real UID or saved set-UID unless an explicit whitelist
@@ -163,7 +163,7 @@ static int safesetid_task_fix_setuid(struct cred *new,
if (!uid_eq(old->suid, new->suid))
return check_uid_transition(old->suid, new->suid);
break;
- case LSM_SETID_RES:
+ case LSM_SETUID_RES:
/*
* Users for which setuid restrictions exist cannot change the
* real UID, effective UID, or saved set-UID to anything but
@@ -187,7 +187,7 @@ static int safesetid_task_fix_setuid(struct cred *new,
return check_uid_transition(old->suid, new->suid);
}
break;
- case LSM_SETID_FS:
+ case LSM_SETUID_FS:
/*
* Users for which setuid restrictions exist cannot change the
* filesystem UID to anything but one of: the current real UID,
@@ -256,7 +256,7 @@ void flush_safesetid_whitelist_entries(void)
}
static struct security_hook_list safesetid_security_hooks[] = {
- LSM_HOOK_INIT(task_fix_setuid, safesetid_task_fix_setuid),
+ LSM_HOOK_INIT(task_fix_setid, safesetid_task_fix_setid),
LSM_HOOK_INIT(capable, safesetid_security_capable)
};
diff --git a/security/security.c b/security/security.c
index ed9b8cbf21cf..450784fd1d2b 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1568,10 +1568,10 @@ int security_kernel_load_data(enum kernel_load_data_id id)
}
EXPORT_SYMBOL_GPL(security_kernel_load_data);
-int security_task_fix_setuid(struct cred *new, const struct cred *old,
+int security_task_fix_setid(struct cred *new, const struct cred *old,
int flags)
{
- return call_int_hook(task_fix_setuid, 0, new, old, flags);
+ return call_int_hook(task_fix_setid, 0, new, old, flags);
}
int security_task_setpgid(struct task_struct *p, pid_t pgid)
--
2.21.0.352.gf09ad66450-goog
^ permalink raw reply related
* Re: overlayfs access checks on underlying layers
From: Stephen Smalley @ 2019-03-04 18:44 UTC (permalink / raw)
To: Mark Salyzyn, Vivek Goyal, Miklos Szeredi
Cc: Ondrej Mosnacek, J. Bruce Fields, Paul Moore, linux-kernel,
overlayfs, linux-fsdevel, selinux, Daniel J Walsh, LSM
In-Reply-To: <fe78aaed-5fb1-3c51-1330-39de46ae39c5@android.com>
On 3/4/19 12:01 PM, Mark Salyzyn wrote:
> On 11/29/2018 05:49 AM, Vivek Goyal wrote:
>> So will override_creds=off solve the NFS issue also where all access will
>> happen with the creds of task now? Though it will stil require more
>> priviliges in task for other operations in overlay to succeed.
>
> NFS problems seems to have ended the discussion, too many stakeholders?
> too many outstanding questions?
>
> Do we accept the limitations of the override_creds patch as is, and then
> have the folks more familiar with the NFS scenario(s) build on it?
>
> [TL;DR]
>
> After looking at all this discussion, it feels like a larger audited
> rewrite of the security model is in order and override_creds=off may be
> a disservice (although expediently deals with Android's needs) to a
> correct general solution. I admit I have little idea where to go from
> here for a general solution.
>
> As far as I see it, the model of creator && caller credentials is a
> problem for any non-overlapping (MAC) privilege models. This patch
> allows one to drop any creator privilege escalation, re-introducing the
> "caller" to the lower layers.
>
> As such I would expect a better model is to _always_ check the caller
> credentials again in the lower layers, and only check the creator
> credentials, some without caller credentials, for some special cases?
> Change an && to an || for some of the checks? What are those special
> cases? I must admit _none_ of those special cases need attention in the
> Android usage models though making it difficult for me to do the fight
> thing for the associated stakeholders.
As I recall, there were multiple problems with using current process'
creds for the operations on the lower/upper/work directories:
- Some overlayfs operations on the lower/upper/work directories required
privilege (capabilities) that the current process might lack, e.g. to
set ownership and the like on upper or work files, to set special xattrs
used internally by overlayfs for whiteouts or similar purposes, to act
on files within the work dir which was inaccessible to the current
process to prevent accessing files in an incomplete state, etc.
Originally that was handled by temporarily elevating the effective
capabilities around the privileged operation in the overlayfs code but
that didn't help with the SELinux or other LSM capability checking that
was triggered upon the capable calls. Without some change there you'd
have to allow all client process domains all of the relevant
capabilities in policy, greatly increasing their privileges.
- The original logic was checking access to the lower dir/files in the
context of the current process when performing some operation that
modifies the file content or metadata, thereby triggering a SELinux/LSM
write or similar check, even though the actual data or metadata
modification occurs to the copied-up file instead and does not affect
the lower dir/files. That was preventing making the lower dir/file
labels read-only to the client processes in the policy, which was
desired for the container use case.
You'd need to solve those problems in some way.
> The lower privileged application access to the directory cache inherited
> by other callers troubles me (not for Android, but in general) and feels
> troublesome (flush out the directory cache? how to tag the privileges
> associated with the current instance of the directory cache?). Some
> operations (eg: delete a file for incoming, create mknod in upperdir)
> are special cases requiring the checking of caller credentaisl to
> function (not a problem for Android as the caller that deletes a file
> just so happens to have the necessary privileges).
>
> Also, mount namespaces (in upper, lower, etc), how will they affect this
> all, is there a need for more attention to this as well?
>
> -- Mark
^ permalink raw reply
* Re: overlayfs access checks on underlying layers
From: Amir Goldstein @ 2019-03-04 19:21 UTC (permalink / raw)
To: Mark Salyzyn
Cc: Vivek Goyal, Miklos Szeredi, Ondrej Mosnacek, J. Bruce Fields,
Paul Moore, linux-kernel, overlayfs, linux-fsdevel, selinux,
Daniel J Walsh, LSM, Stephen Smalley
In-Reply-To: <ae2682f2-4a15-39e6-eb49-b9c0e72f10e4@tycho.nsa.gov>
On Mon, Mar 4, 2019 at 8:53 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>
> On 3/4/19 12:01 PM, Mark Salyzyn wrote:
> > On 11/29/2018 05:49 AM, Vivek Goyal wrote:
> >> So will override_creds=off solve the NFS issue also where all access will
> >> happen with the creds of task now? Though it will stil require more
> >> priviliges in task for other operations in overlay to succeed.
> >
> > NFS problems seems to have ended the discussion, too many stakeholders?
> > too many outstanding questions?
> >
> > Do we accept the limitations of the override_creds patch as is, and then
> > have the folks more familiar with the NFS scenario(s) build on it?
> >
> > [TL;DR]
> >
> > After looking at all this discussion, it feels like a larger audited
> > rewrite of the security model is in order and override_creds=off may be
> > a disservice (although expediently deals with Android's needs) to a
> > correct general solution. I admit I have little idea where to go from
> > here for a general solution.
> >
> > As far as I see it, the model of creator && caller credentials is a
> > problem for any non-overlapping (MAC) privilege models. This patch
> > allows one to drop any creator privilege escalation, re-introducing the
> > "caller" to the lower layers.
> >
> > As such I would expect a better model is to _always_ check the caller
> > credentials again in the lower layers, and only check the creator
> > credentials, some without caller credentials, for some special cases?
> > Change an && to an || for some of the checks? What are those special
> > cases? I must admit _none_ of those special cases need attention in the
> > Android usage models though making it difficult for me to do the fight
> > thing for the associated stakeholders.
>
> As I recall, there were multiple problems with using current process'
> creds for the operations on the lower/upper/work directories:
>
> - Some overlayfs operations on the lower/upper/work directories required
> privilege (capabilities) that the current process might lack, e.g. to
> set ownership and the like on upper or work files, to set special xattrs
> used internally by overlayfs for whiteouts or similar purposes, to act
> on files within the work dir which was inaccessible to the current
> process to prevent accessing files in an incomplete state, etc.
> Originally that was handled by temporarily elevating the effective
> capabilities around the privileged operation in the overlayfs code but
> that didn't help with the SELinux or other LSM capability checking that
> was triggered upon the capable calls. Without some change there you'd
> have to allow all client process domains all of the relevant
> capabilities in policy, greatly increasing their privileges.
>
> - The original logic was checking access to the lower dir/files in the
> context of the current process when performing some operation that
> modifies the file content or metadata, thereby triggering a SELinux/LSM
> write or similar check, even though the actual data or metadata
> modification occurs to the copied-up file instead and does not affect
> the lower dir/files. That was preventing making the lower dir/file
> labels read-only to the client processes in the policy, which was
> desired for the container use case.
>
> You'd need to solve those problems in some way.
>
This is entire discussion in avoiding the elephant in the room -
Tampering with underlying layers.
We only talk about functionality and enforcement of legit overlayfs
operation, but what about an adversary that writes/changes files on
underlying layers? For example, permission to write new files can
be elevated to effectively modifying or deleting files.
To me the logic in "mounter credentials" is not only functional.
Whoever has access to underlying layer can really cause a lot of
damage to users accessing the overlay mount, so in a way, this
power needs to be translated to some sort of elevated credentials.
The the current model seems solid in that respect - access to
underlying layer can be restricted to the "mounter" who effectively
crafts an entirely new set of files.
Another way to think about this - consider a user space overlayfs
implementation, for example:
https://github.com/containers/fuse-overlayfs
In what way should the requirements to overlayfs security model
differ from the model already imposed on a user space implementation?
I do understand the Android use case for override_creds=off, but
to me, this is a functional configuration that says "turn off overlay
specific security considerations... I know what I am doing".
I am not saying that the Android adb debug use case is insecure.
I am just not seeing how a && or || model for mounter and caller
credential add up to a coherent story (yet...).
Thanks,
Amir.
^ permalink raw reply
* Re: [PULL REQUEST] Lock down patches
From: Matthew Garrett @ 2019-03-04 22:10 UTC (permalink / raw)
To: jmorris; +Cc: LSM List, Linux Kernel Mailing List, David Howells
In-Reply-To: <CACdnJuvW47m3JvEcuEX1bsr+L2Ht9LDn_iCuPbHLOoaohOFW4Q@mail.gmail.com>
Hi James,
Based on feedback, I'm going to make a couple of small changes to this
patchset and then resend.
^ permalink raw reply
* Re: [PATCH] tomoyo: Add a kernel config option for fuzzing testing.
From: Tetsuo Handa @ 2019-03-04 23:59 UTC (permalink / raw)
To: Stephen Smalley; +Cc: James Morris, linux-security-module
In-Reply-To: <05b05166-d24a-3c50-6556-472f50a239b9@tycho.nsa.gov>
Stephen Smalley wrote:
> On 3/4/19 8:35 AM, Tetsuo Handa wrote:
> > James, please include this patch for 5.1-rc1, for failing to include
> > this patch will prevent various trees (SELinux/Smack/AppArmor) from
> > proper testing due to this problem because syzbot is enabling both
> > TOMOYO and one of SELinux/Smack/AppArmor via lsm= boot parameter.
> >
> > By including this patch and building kernels with this config option
> > enabled, syzbot will be able to continue proper testing.
>
> Could you clarify the status of upstream TOMOYO? Is its MAINTAINERS
> entry still accurate? Is it still actively maintained?
Mainly bugfixes and Q&A phase like
https://osdn.net/projects/tomoyo/lists/archive/users-en/2017-July/000685.html .
Now that TOMOYO can coexist with one of SELinux/Smack/AppArmor, TOMOYO users
can borrow ready-made rules from them and utilize TOMOYO's ability to generate
custom-made rules for things like
https://tomoyo.osdn.jp/1.8/ssh-protection-using-environment.html .
> Its existing
> documentation (in-tree and the tomoyo.osdn.jp site) seem to suggest that
> using the pre-LSM version and/or AKARI are preferred to using the
> upstream version. Is that still true, and do you envision it changing?
I guess that majority of TOMOYO users are now using the upstream version. But
pre-LSM version and/or AKARI will remain there until LKM-based LSMs becomes
officially supported, for e.g. Fedora/RHEL users will need to use AKARI because
TOMOYO is not available ( https://bugzilla.redhat.com/show_bug.cgi?id=542986 ).
^ permalink raw reply
* Re: [PATCH] keys: fix missing __user in KEYCTL_PKEY_QUERY
From: James Morris @ 2019-03-05 3:28 UTC (permalink / raw)
To: Ben Dooks; +Cc: keyrings, linux-security-module, dhowells, serge, linux-kernel
In-Reply-To: <20190301113026.17721-1-ben.dooks@codethink.co.uk>
On Fri, 1 Mar 2019, Ben Dooks wrote:
> The arg5 of KEYCTL_PKEY_QUERY should have a __user pointer tag on
> it as it is a user pointer. This clears the following sparse warning
> for this:
>
> security/keys/keyctl.c:1755:43: warning: incorrect type in argument 3 (different address spaces)
> security/keys/keyctl.c:1755:43: expected struct keyctl_pkey_query [noderef] <asn:1>*<noident>
> security/keys/keyctl.c:1755:43: got struct keyctl_pkey_query *<noident>
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-general
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v4 1/2] LSM: SafeSetID: gate setgid transitions
From: James Morris @ 2019-03-05 3:30 UTC (permalink / raw)
To: Micah Morton
Cc: Serge E. Hallyn, Kees Cook, Casey Schaufler, Stephen Smalley,
linux-security-module
In-Reply-To: <CAJ-EccMu97tZCWXTakcTwqFwzXdzVDedgc3jWaRvaJu-duH6tQ@mail.gmail.com>
On Mon, 4 Mar 2019, Micah Morton wrote:
> > + case LSM_SETGID_RE:
> > + break;
> > + case LSM_SETGID_ID:
> > + break;
> > + case LSM_SETGID_RES:
> > + break;
> > + case LSM_SETGID_FS:
> > + break;
> > +
These should be collapsed into multiple case statements with one break.
Also, please start a new thread when you post a new version of the
patchset, and include all of the series in each version.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH] tomoyo: Add a kernel config option for fuzzing testing.
From: James Morris @ 2019-03-05 3:32 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: Stephen Smalley, linux-security-module
In-Reply-To: <201903042359.x24NxJwx065719@www262.sakura.ne.jp>
On Tue, 5 Mar 2019, Tetsuo Handa wrote:
> I guess that majority of TOMOYO users are now using the upstream version. But
> pre-LSM version and/or AKARI will remain there until LKM-based LSMs becomes
> officially supported
You mean dynamically loadable LSMs?
There are no plans to support this.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH] keys: fix missing __user in KEYCTL_PKEY_QUERY
From: David Howells @ 2019-03-05 7:41 UTC (permalink / raw)
To: Ben Dooks
Cc: dhowells, keyrings, linux-security-module, jmorris, serge,
linux-kernel
In-Reply-To: <20190301113026.17721-1-ben.dooks@codethink.co.uk>
Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> The arg5 of KEYCTL_PKEY_QUERY should have a __user pointer tag on
> it as it is a user pointer. This clears the following sparse warning
> for this:
>
> security/keys/keyctl.c:1755:43: warning: incorrect type in argument 3 (different address spaces)
> security/keys/keyctl.c:1755:43: expected struct keyctl_pkey_query [noderef] <asn:1>*<noident>
> security/keys/keyctl.c:1755:43: got struct keyctl_pkey_query *<noident>
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Acked-by: David Howells <dhowells@redhat.com>
^ permalink raw reply
* Re: [PATCH v4 1/2] LSM: SafeSetID: gate setgid transitions
From: Micah Morton @ 2019-03-05 15:46 UTC (permalink / raw)
To: James Morris
Cc: Serge E. Hallyn, Kees Cook, Casey Schaufler, Stephen Smalley,
linux-security-module
In-Reply-To: <alpine.LRH.2.21.1903051429300.20560@namei.org>
Yeah, sorry I actually fixed that in the last patch, but probably was
hard to follow since I haven't been starting new threads like you
said. Will do that from now on.
On Mon, Mar 4, 2019 at 7:30 PM James Morris <jmorris@namei.org> wrote:
>
> On Mon, 4 Mar 2019, Micah Morton wrote:
>
> > > + case LSM_SETGID_RE:
> > > + break;
> > > + case LSM_SETGID_ID:
> > > + break;
> > > + case LSM_SETGID_RES:
> > > + break;
> > > + case LSM_SETGID_FS:
> > > + break;
> > > +
>
> These should be collapsed into multiple case statements with one break.
>
> Also, please start a new thread when you post a new version of the
> patchset, and include all of the series in each version.
>
>
> --
> James Morris
> <jmorris@namei.org>
>
^ permalink raw reply
* [PATCH v5 1/2] LSM: SafeSetID: gate setgid transitions
From: mortonm @ 2019-03-05 15:49 UTC (permalink / raw)
To: jmorris, serge, keescook, casey, sds, linux-security-module; +Cc: Micah Morton
From: Micah Morton <mortonm@chromium.org>
This patch generalizes the 'task_fix_setuid' LSM hook to enable hooking
setgid transitions as well as setuid transitions. The hook is renamed to
'task_fix_setid'. The patch introduces calls to this hook from the
setgid functions in kernel/sys.c. This will allow the SafeSetID LSM to
govern setgid transitions in addition to setuid transitions. This patch
also makes sure the setgid functions in kernel/sys.c call
security_capable_setid rather than the ordinary security_capable
function, so that the security_capable hook in the SafeSetID LSM knows
it is being invoked from a setid function.
Signed-off-by: Micah Morton <mortonm@chromium.org>
---
Changes since the last patch: Only one break is needed for the four
LSM_SETGID_* cases in security/commoncap.c.
Documentation/admin-guide/LSM/SafeSetID.rst | 2 +-
include/linux/lsm_hooks.h | 8 ++---
include/linux/security.h | 36 ++++++++++++++-------
kernel/sys.c | 35 ++++++++++++++------
security/commoncap.c | 22 ++++++++-----
security/safesetid/lsm.c | 12 +++----
security/security.c | 4 +--
7 files changed, 76 insertions(+), 43 deletions(-)
diff --git a/Documentation/admin-guide/LSM/SafeSetID.rst b/Documentation/admin-guide/LSM/SafeSetID.rst
index 212434ef65ad..670a6544fd39 100644
--- a/Documentation/admin-guide/LSM/SafeSetID.rst
+++ b/Documentation/admin-guide/LSM/SafeSetID.rst
@@ -88,7 +88,7 @@ other system interactions, including use of pid namespaces and device creation.
Use an existing LSM
-------------------
None of the other in-tree LSMs have the capability to gate setid transitions, or
-even employ the security_task_fix_setuid hook at all. SELinux says of that hook:
+even employ the security_task_fix_setid hook at all. SELinux says of that hook:
"Since setuid only affects the current process, and since the SELinux controls
are not based on the Linux identity attributes, SELinux does not need to control
this operation."
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 22fc786d723a..47fd04410fde 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -594,14 +594,14 @@
* @size length of the file contents.
* @id kernel read file identifier
* Return 0 if permission is granted.
- * @task_fix_setuid:
+ * @task_fix_setid:
* Update the module's state after setting one or more of the user
* identity attributes of the current process. The @flags parameter
* indicates which of the set*uid system calls invoked this hook. If
* @new is the set of credentials that will be installed. Modifications
* should be made to this rather than to @current->cred.
* @old is the set of credentials that are being replaces
- * @flags contains one of the LSM_SETID_* values.
+ * @flags contains one of the LSM_SET*ID_* values.
* Return 0 on success.
* @task_setpgid:
* Check permission before setting the process group identifier of the
@@ -1594,7 +1594,7 @@ union security_list_options {
int (*kernel_read_file)(struct file *file, enum kernel_read_file_id id);
int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
enum kernel_read_file_id id);
- int (*task_fix_setuid)(struct cred *new, const struct cred *old,
+ int (*task_fix_setid)(struct cred *new, const struct cred *old,
int flags);
int (*task_setpgid)(struct task_struct *p, pid_t pgid);
int (*task_getpgid)(struct task_struct *p);
@@ -1886,7 +1886,7 @@ struct security_hook_heads {
struct hlist_head kernel_read_file;
struct hlist_head kernel_post_read_file;
struct hlist_head kernel_module_request;
- struct hlist_head task_fix_setuid;
+ struct hlist_head task_fix_setid;
struct hlist_head task_setpgid;
struct hlist_head task_getpgid;
struct hlist_head task_getsid;
diff --git a/include/linux/security.h b/include/linux/security.h
index 13537a49ae97..76df3e22fed1 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -95,7 +95,7 @@ extern int cap_inode_getsecurity(struct inode *inode, const char *name,
extern int cap_mmap_addr(unsigned long addr);
extern int cap_mmap_file(struct file *file, unsigned long reqprot,
unsigned long prot, unsigned long flags);
-extern int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags);
+extern int cap_task_fix_setid(struct cred *new, const struct cred *old, int flags);
extern int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
unsigned long arg4, unsigned long arg5);
extern int cap_task_setscheduler(struct task_struct *p);
@@ -128,17 +128,29 @@ extern unsigned long dac_mmap_min_addr;
/*
* Values used in the task_security_ops calls
*/
-/* setuid or setgid, id0 == uid or gid */
-#define LSM_SETID_ID 1
+/* setuid, id0 == uid */
+#define LSM_SETUID_ID 1
-/* setreuid or setregid, id0 == real, id1 == eff */
-#define LSM_SETID_RE 2
+/* setreuid, id0 == real, id1 == eff */
+#define LSM_SETUID_RE 2
-/* setresuid or setresgid, id0 == real, id1 == eff, uid2 == saved */
-#define LSM_SETID_RES 4
+/* setresuid, id0 == real, id1 == eff, uid2 == saved */
+#define LSM_SETUID_RES 4
-/* setfsuid or setfsgid, id0 == fsuid or fsgid */
-#define LSM_SETID_FS 8
+/* setfsuid, id0 == fsgid */
+#define LSM_SETUID_FS 8
+
+/* setgid, id0 == gid */
+#define LSM_SETGID_ID 16
+
+/* setregid, id0 == real, id1 == eff */
+#define LSM_SETGID_RE 32
+
+/* setresgid, id0 == real, id1 == eff, uid2 == saved */
+#define LSM_SETGID_RES 64
+
+/* setfsgid, id0 == fsgid */
+#define LSM_SETGID_FS 128
/* Flags for security_task_prlimit(). */
#define LSM_PRLIMIT_READ 1
@@ -324,7 +336,7 @@ int security_kernel_load_data(enum kernel_load_data_id id);
int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
enum kernel_read_file_id id);
-int security_task_fix_setuid(struct cred *new, const struct cred *old,
+int security_task_fix_setid(struct cred *new, const struct cred *old,
int flags);
int security_task_setpgid(struct task_struct *p, pid_t pgid);
int security_task_getpgid(struct task_struct *p);
@@ -923,11 +935,11 @@ static inline int security_kernel_post_read_file(struct file *file,
return 0;
}
-static inline int security_task_fix_setuid(struct cred *new,
+static inline int security_task_fix_setid(struct cred *new,
const struct cred *old,
int flags)
{
- return cap_task_fix_setuid(new, old, flags);
+ return cap_task_fix_setid(new, old, flags);
}
static inline int security_task_setpgid(struct task_struct *p, pid_t pgid)
diff --git a/kernel/sys.c b/kernel/sys.c
index c5f875048aef..615b44939238 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -372,7 +372,7 @@ long __sys_setregid(gid_t rgid, gid_t egid)
if (rgid != (gid_t) -1) {
if (gid_eq(old->gid, krgid) ||
gid_eq(old->egid, krgid) ||
- ns_capable(old->user_ns, CAP_SETGID))
+ ns_capable_setid(old->user_ns, CAP_SETGID))
new->gid = krgid;
else
goto error;
@@ -381,7 +381,7 @@ long __sys_setregid(gid_t rgid, gid_t egid)
if (gid_eq(old->gid, kegid) ||
gid_eq(old->egid, kegid) ||
gid_eq(old->sgid, kegid) ||
- ns_capable(old->user_ns, CAP_SETGID))
+ ns_capable_setid(old->user_ns, CAP_SETGID))
new->egid = kegid;
else
goto error;
@@ -392,6 +392,10 @@ long __sys_setregid(gid_t rgid, gid_t egid)
new->sgid = new->egid;
new->fsgid = new->egid;
+ retval = security_task_fix_setid(new, old, LSM_SETGID_RE);
+ if (retval < 0)
+ goto error;
+
return commit_creds(new);
error:
@@ -427,13 +431,17 @@ long __sys_setgid(gid_t gid)
old = current_cred();
retval = -EPERM;
- if (ns_capable(old->user_ns, CAP_SETGID))
+ if (ns_capable_setid(old->user_ns, CAP_SETGID))
new->gid = new->egid = new->sgid = new->fsgid = kgid;
else if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->sgid))
new->egid = new->fsgid = kgid;
else
goto error;
+ retval = security_task_fix_setid(new, old, LSM_SETGID_ID);
+ if (retval < 0)
+ goto error;
+
return commit_creds(new);
error:
@@ -539,7 +547,7 @@ long __sys_setreuid(uid_t ruid, uid_t euid)
new->suid = new->euid;
new->fsuid = new->euid;
- retval = security_task_fix_setuid(new, old, LSM_SETID_RE);
+ retval = security_task_fix_setid(new, old, LSM_SETUID_RE);
if (retval < 0)
goto error;
@@ -597,7 +605,7 @@ long __sys_setuid(uid_t uid)
new->fsuid = new->euid = kuid;
- retval = security_task_fix_setuid(new, old, LSM_SETID_ID);
+ retval = security_task_fix_setid(new, old, LSM_SETUID_ID);
if (retval < 0)
goto error;
@@ -672,7 +680,7 @@ long __sys_setresuid(uid_t ruid, uid_t euid, uid_t suid)
new->suid = ksuid;
new->fsuid = new->euid;
- retval = security_task_fix_setuid(new, old, LSM_SETID_RES);
+ retval = security_task_fix_setid(new, old, LSM_SETUID_RES);
if (retval < 0)
goto error;
@@ -735,7 +743,7 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid)
old = current_cred();
retval = -EPERM;
- if (!ns_capable(old->user_ns, CAP_SETGID)) {
+ if (!ns_capable_setid(old->user_ns, CAP_SETGID)) {
if (rgid != (gid_t) -1 && !gid_eq(krgid, old->gid) &&
!gid_eq(krgid, old->egid) && !gid_eq(krgid, old->sgid))
goto error;
@@ -755,6 +763,10 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid)
new->sgid = ksgid;
new->fsgid = new->egid;
+ retval = security_task_fix_setid(new, old, LSM_SETGID_RES);
+ if (retval < 0)
+ goto error;
+
return commit_creds(new);
error:
@@ -817,7 +829,7 @@ long __sys_setfsuid(uid_t uid)
ns_capable_setid(old->user_ns, CAP_SETUID)) {
if (!uid_eq(kuid, old->fsuid)) {
new->fsuid = kuid;
- if (security_task_fix_setuid(new, old, LSM_SETID_FS) == 0)
+ if (security_task_fix_setid(new, old, LSM_SETUID_FS) == 0)
goto change_okay;
}
}
@@ -858,10 +870,13 @@ long __sys_setfsgid(gid_t gid)
if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->egid) ||
gid_eq(kgid, old->sgid) || gid_eq(kgid, old->fsgid) ||
- ns_capable(old->user_ns, CAP_SETGID)) {
+ ns_capable_setid(old->user_ns, CAP_SETGID)) {
if (!gid_eq(kgid, old->fsgid)) {
new->fsgid = kgid;
- goto change_okay;
+ if (security_task_fix_setid(new,
+ old,
+ LSM_SETGID_FS) == 0)
+ goto change_okay;
}
}
diff --git a/security/commoncap.c b/security/commoncap.c
index f1d117c3d8ae..4a038d3c7196 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1026,27 +1026,27 @@ static inline void cap_emulate_setxuid(struct cred *new, const struct cred *old)
}
/**
- * cap_task_fix_setuid - Fix up the results of setuid() call
+ * cap_task_fix_setid - Fix up the results of setid() call
* @new: The proposed credentials
* @old: The current task's current credentials
* @flags: Indications of what has changed
*
- * Fix up the results of setuid() call before the credential changes are
+ * Fix up the results of setid() call before the credential changes are
* actually applied, returning 0 to grant the changes, -ve to deny them.
*/
-int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags)
+int cap_task_fix_setid(struct cred *new, const struct cred *old, int flags)
{
switch (flags) {
- case LSM_SETID_RE:
- case LSM_SETID_ID:
- case LSM_SETID_RES:
+ case LSM_SETUID_RE:
+ case LSM_SETUID_ID:
+ case LSM_SETUID_RES:
/* juggle the capabilities to follow [RES]UID changes unless
* otherwise suppressed */
if (!issecure(SECURE_NO_SETUID_FIXUP))
cap_emulate_setxuid(new, old);
break;
- case LSM_SETID_FS:
+ case LSM_SETUID_FS:
/* juggle the capabilties to follow FSUID changes, unless
* otherwise suppressed
*
@@ -1066,6 +1066,12 @@ int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags)
}
break;
+ case LSM_SETGID_RE:
+ case LSM_SETGID_ID:
+ case LSM_SETGID_RES:
+ case LSM_SETGID_FS:
+ break;
+
default:
return -EINVAL;
}
@@ -1355,7 +1361,7 @@ struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(inode_getsecurity, cap_inode_getsecurity),
LSM_HOOK_INIT(mmap_addr, cap_mmap_addr),
LSM_HOOK_INIT(mmap_file, cap_mmap_file),
- LSM_HOOK_INIT(task_fix_setuid, cap_task_fix_setuid),
+ LSM_HOOK_INIT(task_fix_setid, cap_task_fix_setid),
LSM_HOOK_INIT(task_prctl, cap_task_prctl),
LSM_HOOK_INIT(task_setscheduler, cap_task_setscheduler),
LSM_HOOK_INIT(task_setioprio, cap_task_setioprio),
diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
index cecd38e2ac80..5deffa92f25f 100644
--- a/security/safesetid/lsm.c
+++ b/security/safesetid/lsm.c
@@ -120,7 +120,7 @@ static int check_uid_transition(kuid_t parent, kuid_t child)
* set*uid to user under new cred struct, or the UID transition is allowed (by
* Linux set*uid rules) even without CAP_SETUID.
*/
-static int safesetid_task_fix_setuid(struct cred *new,
+static int safesetid_task_fix_setid(struct cred *new,
const struct cred *old,
int flags)
{
@@ -130,7 +130,7 @@ static int safesetid_task_fix_setuid(struct cred *new,
return 0;
switch (flags) {
- case LSM_SETID_RE:
+ case LSM_SETUID_RE:
/*
* Users for which setuid restrictions exist can only set the
* real UID to the real UID or the effective UID, unless an
@@ -152,7 +152,7 @@ static int safesetid_task_fix_setuid(struct cred *new,
return check_uid_transition(old->euid, new->euid);
}
break;
- case LSM_SETID_ID:
+ case LSM_SETUID_ID:
/*
* Users for which setuid restrictions exist cannot change the
* real UID or saved set-UID unless an explicit whitelist
@@ -163,7 +163,7 @@ static int safesetid_task_fix_setuid(struct cred *new,
if (!uid_eq(old->suid, new->suid))
return check_uid_transition(old->suid, new->suid);
break;
- case LSM_SETID_RES:
+ case LSM_SETUID_RES:
/*
* Users for which setuid restrictions exist cannot change the
* real UID, effective UID, or saved set-UID to anything but
@@ -187,7 +187,7 @@ static int safesetid_task_fix_setuid(struct cred *new,
return check_uid_transition(old->suid, new->suid);
}
break;
- case LSM_SETID_FS:
+ case LSM_SETUID_FS:
/*
* Users for which setuid restrictions exist cannot change the
* filesystem UID to anything but one of: the current real UID,
@@ -256,7 +256,7 @@ void flush_safesetid_whitelist_entries(void)
}
static struct security_hook_list safesetid_security_hooks[] = {
- LSM_HOOK_INIT(task_fix_setuid, safesetid_task_fix_setuid),
+ LSM_HOOK_INIT(task_fix_setid, safesetid_task_fix_setid),
LSM_HOOK_INIT(capable, safesetid_security_capable)
};
diff --git a/security/security.c b/security/security.c
index ed9b8cbf21cf..450784fd1d2b 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1568,10 +1568,10 @@ int security_kernel_load_data(enum kernel_load_data_id id)
}
EXPORT_SYMBOL_GPL(security_kernel_load_data);
-int security_task_fix_setuid(struct cred *new, const struct cred *old,
+int security_task_fix_setid(struct cred *new, const struct cred *old,
int flags)
{
- return call_int_hook(task_fix_setuid, 0, new, old, flags);
+ return call_int_hook(task_fix_setid, 0, new, old, flags);
}
int security_task_setpgid(struct task_struct *p, pid_t pgid)
--
2.21.0.352.gf09ad66450-goog
^ permalink raw reply related
* [PATCH v4 2/2] LSM: SafeSetID: gate setgid transitions
From: mortonm @ 2019-03-05 15:52 UTC (permalink / raw)
To: jmorris, serge, keescook, casey, sds, linux-security-module; +Cc: Micah Morton
In-Reply-To: <20190305154922.61040-1-mortonm@chromium.org>
From: Micah Morton <mortonm@chromium.org>
The SafeSetID LSM already gates setuid transitions for UIDs on the
system whose use of CAP_SETUID has been 'restricted'. This patch
implements the analogous functionality for setgid transitions, in order
to restrict the use of CAP_SETGID for certain UIDs on the system. One
notable consequence of this addition is that a process running under a
restricted UID (i.e. one that is only allowed to setgid to certain
approved GIDs) will not be allowed to call the setgroups() syscall to
set its supplementary group IDs. For now, we leave such support for
restricted setgroups() to future work, as it would require hooking the
logic in setgroups() and verifying that the array of GIDs passed in from
userspace only consists of approved GIDs.
Signed-off-by: Micah Morton <mortonm@chromium.org>
---
Changes since the last patch: Minor updates since patch 1/2 combined the
'task_fix_setuid' and 'task_fix_setgid' hooks into one hook called
'task_fix_setid'.
Documentation/admin-guide/LSM/SafeSetID.rst | 9 +-
security/safesetid/lsm.c | 272 +++++++++++++++++---
security/safesetid/lsm.h | 11 +-
security/safesetid/securityfs.c | 105 +++++---
4 files changed, 320 insertions(+), 77 deletions(-)
diff --git a/Documentation/admin-guide/LSM/SafeSetID.rst b/Documentation/admin-guide/LSM/SafeSetID.rst
index 670a6544fd39..33307a8e9555 100644
--- a/Documentation/admin-guide/LSM/SafeSetID.rst
+++ b/Documentation/admin-guide/LSM/SafeSetID.rst
@@ -98,10 +98,13 @@ Directions for use
==================
This LSM hooks the setid syscalls to make sure transitions are allowed if an
applicable restriction policy is in place. Policies are configured through
-securityfs by writing to the safesetid/add_whitelist_policy and
+securityfs by writing to the safesetid/add_whitelist_{uid/gid}_policy* and
safesetid/flush_whitelist_policies files at the location where securityfs is
-mounted. The format for adding a policy is '<UID>:<UID>', using literal
+mounted. The format for adding a policy is '<ID>:<ID>', using literal
numbers, such as '123:456'. To flush the policies, any write to the file is
sufficient. Again, configuring a policy for a UID will prevent that UID from
obtaining auxiliary setid privileges, such as allowing a user to set up user
-namespace UID mappings.
+namespace ID mappings.
+
+*safesetid/add_whitelist_policy can also be used for UID policies, since it is
+an alias for safesetid/add_whitelist_uid_policy
diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
index 5deffa92f25f..95c6f35b29f3 100644
--- a/security/safesetid/lsm.c
+++ b/security/safesetid/lsm.c
@@ -26,27 +26,30 @@ int safesetid_initialized;
#define NUM_BITS 8 /* 128 buckets in hash table */
-static DEFINE_HASHTABLE(safesetid_whitelist_hashtable, NUM_BITS);
+static DEFINE_HASHTABLE(safesetid_whitelist_uid_hashtable, NUM_BITS);
+static DEFINE_HASHTABLE(safesetid_whitelist_gid_hashtable, NUM_BITS);
+
+static DEFINE_SPINLOCK(safesetid_whitelist_uid_hashtable_spinlock);
+static DEFINE_SPINLOCK(safesetid_whitelist_gid_hashtable_spinlock);
/*
* Hash table entry to store safesetid policy signifying that 'parent' user
- * can setid to 'child' user.
+ * can setid to 'child' user. This struct is used in both the uid and gid
+ * hashtables.
*/
-struct entry {
+struct id_entry {
struct hlist_node next;
struct hlist_node dlist; /* for deletion cleanup */
uint64_t parent_kuid;
- uint64_t child_kuid;
+ uint64_t child_kid; /* Represents either a UID or a GID */
};
-static DEFINE_SPINLOCK(safesetid_whitelist_hashtable_spinlock);
-
static bool check_setuid_policy_hashtable_key(kuid_t parent)
{
- struct entry *entry;
+ struct id_entry *entry;
rcu_read_lock();
- hash_for_each_possible_rcu(safesetid_whitelist_hashtable,
+ hash_for_each_possible_rcu(safesetid_whitelist_uid_hashtable,
entry, next, __kuid_val(parent)) {
if (entry->parent_kuid == __kuid_val(parent)) {
rcu_read_unlock();
@@ -61,13 +64,49 @@ static bool check_setuid_policy_hashtable_key(kuid_t parent)
static bool check_setuid_policy_hashtable_key_value(kuid_t parent,
kuid_t child)
{
- struct entry *entry;
+ struct id_entry *entry;
+
+ rcu_read_lock();
+ hash_for_each_possible_rcu(safesetid_whitelist_uid_hashtable,
+ entry, next, __kuid_val(parent)) {
+ if (entry->parent_kuid == __kuid_val(parent) &&
+ entry->child_kid == __kuid_val(child)) {
+ rcu_read_unlock();
+ return true;
+ }
+ }
+ rcu_read_unlock();
+
+ return false;
+}
+
+static bool check_setgid_policy_hashtable_key(kuid_t parent)
+{
+ struct id_entry *entry;
+
+ rcu_read_lock();
+ hash_for_each_possible_rcu(safesetid_whitelist_gid_hashtable,
+ entry, next, __kuid_val(parent)) {
+ if (entry->parent_kuid == __kuid_val(parent)) {
+ rcu_read_unlock();
+ return true;
+ }
+ }
+ rcu_read_unlock();
+
+ return false;
+}
+
+static bool check_setgid_policy_hashtable_key_value(kuid_t parent,
+ kgid_t child)
+{
+ struct id_entry *entry;
rcu_read_lock();
- hash_for_each_possible_rcu(safesetid_whitelist_hashtable,
+ hash_for_each_possible_rcu(safesetid_whitelist_gid_hashtable,
entry, next, __kuid_val(parent)) {
if (entry->parent_kuid == __kuid_val(parent) &&
- entry->child_kuid == __kuid_val(child)) {
+ entry->child_kid == __kgid_val(child)) {
rcu_read_unlock();
return true;
}
@@ -77,6 +116,12 @@ static bool check_setuid_policy_hashtable_key_value(kuid_t parent,
return false;
}
+/*
+ * This hook causes the security_capable check to fail when there are
+ * restriction policies for a UID and the process is trying to do something
+ * (other than a setid transition) that is gated by CAP_SETUID/CAP_SETGID
+ * (e.g. allowing user to set up userns UID/GID mappings).
+ */
static int safesetid_security_capable(const struct cred *cred,
struct user_namespace *ns,
int cap,
@@ -85,17 +130,19 @@ static int safesetid_security_capable(const struct cred *cred,
if (cap == CAP_SETUID &&
check_setuid_policy_hashtable_key(cred->uid)) {
if (!(opts & CAP_OPT_INSETID)) {
- /*
- * Deny if we're not in a set*uid() syscall to avoid
- * giving powers gated by CAP_SETUID that are related
- * to functionality other than calling set*uid() (e.g.
- * allowing user to set up userns uid mappings).
- */
pr_warn("Operation requires CAP_SETUID, which is not available to UID %u for operations besides approved set*uid transitions",
__kuid_val(cred->uid));
return -1;
}
}
+ if (cap == CAP_SETGID &&
+ check_setgid_policy_hashtable_key(cred->uid)) {
+ if (!(opts & CAP_OPT_INSETID)) {
+ pr_warn("Operation requires CAP_SETGID, which is not available to UID %u for operations besides approved set*gid transitions",
+ __kuid_val(cred->uid));
+ return -1;
+ }
+ }
return 0;
}
@@ -115,19 +162,48 @@ static int check_uid_transition(kuid_t parent, kuid_t child)
return -EACCES;
}
+static int check_gid_transition(kuid_t parent, kgid_t child)
+{
+ if (check_setgid_policy_hashtable_key_value(parent, child))
+ return 0;
+ pr_warn("Denied UID %d setting GID to %d",
+ __kuid_val(parent),
+ __kgid_val(child));
+ /*
+ * Kill this process to avoid potential security vulnerabilities
+ * that could arise from a missing whitelist entry preventing a
+ * privileged process from dropping to a lesser-privileged one.
+ */
+ force_sig(SIGKILL, current);
+ return -EACCES;
+}
+
/*
* Check whether there is either an exception for user under old cred struct to
- * set*uid to user under new cred struct, or the UID transition is allowed (by
- * Linux set*uid rules) even without CAP_SETUID.
+ * set*id to user under new cred struct, or the ID transition is allowed (by
+ * Linux set*id rules) even without CAP_SET*ID.
*/
static int safesetid_task_fix_setid(struct cred *new,
const struct cred *old,
int flags)
{
+ if (flags == LSM_SETUID_RE ||
+ flags == LSM_SETUID_ID ||
+ flags == LSM_SETUID_RES ||
+ flags == LSM_SETUID_FS) {
+ /* Do nothing if there are no setuid restrictions for this UID. */
+ if (!check_setuid_policy_hashtable_key(old->uid))
+ return 0;
+ }
- /* Do nothing if there are no setuid restrictions for this UID. */
- if (!check_setuid_policy_hashtable_key(old->uid))
+ if (flags == LSM_SETGID_RE ||
+ flags == LSM_SETGID_ID ||
+ flags == LSM_SETGID_RES ||
+ flags == LSM_SETGID_FS) {
+ /* Do nothing if there are no setgid restrictions for this UID. */
+ if (!check_setgid_policy_hashtable_key(old->uid))
return 0;
+ }
switch (flags) {
case LSM_SETUID_RE:
@@ -201,6 +277,77 @@ static int safesetid_task_fix_setid(struct cred *new,
return check_uid_transition(old->fsuid, new->fsuid);
}
break;
+ case LSM_SETGID_RE:
+ /*
+ * Users for which setgid restrictions exist can only set the
+ * real GID to the real GID or the effective GID, unless an
+ * explicit whitelist policy allows the transition.
+ */
+ if (!gid_eq(old->gid, new->gid) &&
+ !gid_eq(old->egid, new->gid)) {
+ return check_gid_transition(old->uid, new->gid);
+ }
+ /*
+ * Users for which setgid restrictions exist can only set the
+ * effective GID to the real GID, the effective GID, or the
+ * saved set-GID, unless an explicit whitelist policy allows
+ * the transition.
+ */
+ if (!gid_eq(old->gid, new->egid) &&
+ !gid_eq(old->egid, new->egid) &&
+ !gid_eq(old->sgid, new->egid)) {
+ return check_gid_transition(old->euid, new->egid);
+ }
+ break;
+ case LSM_SETGID_ID:
+ /*
+ * Users for which setgid restrictions exist cannot change the
+ * real GID or saved set-GID unless an explicit whitelist
+ * policy allows the transition.
+ */
+ if (!gid_eq(old->gid, new->gid))
+ return check_gid_transition(old->uid, new->gid);
+ if (!gid_eq(old->sgid, new->sgid))
+ return check_gid_transition(old->suid, new->sgid);
+ break;
+ case LSM_SETGID_RES:
+ /*
+ * Users for which setgid restrictions exist cannot change the
+ * real GID, effective GID, or saved set-GID to anything but
+ * one of: the current real GID, the current effective GID or
+ * the current saved set-user-ID unless an explicit whitelist
+ * policy allows the transition.
+ */
+ if (!gid_eq(new->gid, old->gid) &&
+ !gid_eq(new->gid, old->egid) &&
+ !gid_eq(new->gid, old->sgid)) {
+ return check_gid_transition(old->uid, new->gid);
+ }
+ if (!gid_eq(new->egid, old->gid) &&
+ !gid_eq(new->egid, old->egid) &&
+ !gid_eq(new->egid, old->sgid)) {
+ return check_gid_transition(old->euid, new->egid);
+ }
+ if (!gid_eq(new->sgid, old->gid) &&
+ !gid_eq(new->sgid, old->egid) &&
+ !gid_eq(new->sgid, old->sgid)) {
+ return check_gid_transition(old->suid, new->sgid);
+ }
+ break;
+ case LSM_SETGID_FS:
+ /*
+ * Users for which setgid restrictions exist cannot change the
+ * filesystem GID to anything but one of: the current real GID,
+ * the current effective GID or the current saved set-GID
+ * unless an explicit whitelist policy allows the transition.
+ */
+ if (!gid_eq(new->fsgid, old->gid) &&
+ !gid_eq(new->fsgid, old->egid) &&
+ !gid_eq(new->fsgid, old->sgid) &&
+ !gid_eq(new->fsgid, old->fsgid)) {
+ return check_gid_transition(old->fsuid, new->fsgid);
+ }
+ break;
default:
pr_warn("Unknown setid state %d\n", flags);
force_sig(SIGKILL, current);
@@ -209,49 +356,96 @@ static int safesetid_task_fix_setid(struct cred *new,
return 0;
}
-int add_safesetid_whitelist_entry(kuid_t parent, kuid_t child)
+int add_safesetid_whitelist_uid_entry(kuid_t parent, kuid_t child)
{
- struct entry *new;
+ struct id_entry *new;
/* Return if entry already exists */
if (check_setuid_policy_hashtable_key_value(parent, child))
return 0;
- new = kzalloc(sizeof(struct entry), GFP_KERNEL);
+ new = kzalloc(sizeof(struct id_entry), GFP_KERNEL);
+ if (!new)
+ return -ENOMEM;
+ new->parent_kuid = __kuid_val(parent);
+ new->child_kid = __kuid_val(child);
+ spin_lock(&safesetid_whitelist_uid_hashtable_spinlock);
+ /* Return if the entry got added since we checked above */
+ if (check_setuid_policy_hashtable_key_value(parent, child)) {
+ spin_unlock(&safesetid_whitelist_uid_hashtable_spinlock);
+ kfree(new);
+ return 0;
+ }
+ hash_add_rcu(safesetid_whitelist_uid_hashtable,
+ &new->next,
+ __kuid_val(parent));
+ spin_unlock(&safesetid_whitelist_uid_hashtable_spinlock);
+ return 0;
+}
+
+int add_safesetid_whitelist_gid_entry(kuid_t parent, kgid_t child)
+{
+ struct id_entry *new;
+
+ /* Return if entry already exists */
+ if (check_setgid_policy_hashtable_key_value(parent, child))
+ return 0;
+
+ new = kzalloc(sizeof(struct id_entry), GFP_KERNEL);
if (!new)
return -ENOMEM;
new->parent_kuid = __kuid_val(parent);
- new->child_kuid = __kuid_val(child);
- spin_lock(&safesetid_whitelist_hashtable_spinlock);
- hash_add_rcu(safesetid_whitelist_hashtable,
+ new->child_kid = __kgid_val(child);
+ spin_lock(&safesetid_whitelist_gid_hashtable_spinlock);
+ /* Return if the entry got added since we checked above */
+ if (check_setgid_policy_hashtable_key_value(parent, child)) {
+ spin_unlock(&safesetid_whitelist_gid_hashtable_spinlock);
+ kfree(new);
+ return 0;
+ }
+ hash_add_rcu(safesetid_whitelist_gid_hashtable,
&new->next,
__kuid_val(parent));
- spin_unlock(&safesetid_whitelist_hashtable_spinlock);
+ spin_unlock(&safesetid_whitelist_gid_hashtable_spinlock);
return 0;
}
void flush_safesetid_whitelist_entries(void)
{
- struct entry *entry;
+ struct id_entry *id_entry;
struct hlist_node *hlist_node;
unsigned int bkt_loop_cursor;
- HLIST_HEAD(free_list);
+ HLIST_HEAD(uid_free_list);
+ HLIST_HEAD(gid_free_list);
/*
* Could probably use hash_for_each_rcu here instead, but this should
* be fine as well.
*/
- spin_lock(&safesetid_whitelist_hashtable_spinlock);
- hash_for_each_safe(safesetid_whitelist_hashtable, bkt_loop_cursor,
- hlist_node, entry, next) {
- hash_del_rcu(&entry->next);
- hlist_add_head(&entry->dlist, &free_list);
+ spin_lock(&safesetid_whitelist_uid_hashtable_spinlock);
+ hash_for_each_safe(safesetid_whitelist_uid_hashtable, bkt_loop_cursor,
+ hlist_node, id_entry, next) {
+ hash_del_rcu(&id_entry->next);
+ hlist_add_head(&id_entry->dlist, &uid_free_list);
+ }
+ spin_unlock(&safesetid_whitelist_uid_hashtable_spinlock);
+ synchronize_rcu();
+ hlist_for_each_entry_safe(id_entry, hlist_node, &uid_free_list, dlist) {
+ hlist_del(&id_entry->dlist);
+ kfree(id_entry);
+ }
+
+ spin_lock(&safesetid_whitelist_gid_hashtable_spinlock);
+ hash_for_each_safe(safesetid_whitelist_gid_hashtable, bkt_loop_cursor,
+ hlist_node, id_entry, next) {
+ hash_del_rcu(&id_entry->next);
+ hlist_add_head(&id_entry->dlist, &gid_free_list);
}
- spin_unlock(&safesetid_whitelist_hashtable_spinlock);
+ spin_unlock(&safesetid_whitelist_gid_hashtable_spinlock);
synchronize_rcu();
- hlist_for_each_entry_safe(entry, hlist_node, &free_list, dlist) {
- hlist_del(&entry->dlist);
- kfree(entry);
+ hlist_for_each_entry_safe(id_entry, hlist_node, &gid_free_list, dlist) {
+ hlist_del(&id_entry->dlist);
+ kfree(id_entry);
}
}
diff --git a/security/safesetid/lsm.h b/security/safesetid/lsm.h
index c1ea3c265fcf..e9ae192caff2 100644
--- a/security/safesetid/lsm.h
+++ b/security/safesetid/lsm.h
@@ -21,13 +21,16 @@ extern int safesetid_initialized;
/* Function type. */
enum safesetid_whitelist_file_write_type {
- SAFESETID_WHITELIST_ADD, /* Add whitelist policy. */
+ SAFESETID_WHITELIST_ADD_UID, /* Add UID whitelist policy. */
+ SAFESETID_WHITELIST_ADD_GID, /* Add GID whitelist policy. */
SAFESETID_WHITELIST_FLUSH, /* Flush whitelist policies. */
};
-/* Add entry to safesetid whitelist to allow 'parent' to setid to 'child'. */
-int add_safesetid_whitelist_entry(kuid_t parent, kuid_t child);
-
+/* Add entry to safesetid whitelist to allow 'parent' to setuid to 'child'. */
+int add_safesetid_whitelist_uid_entry(kuid_t parent, kuid_t child);
+/* Add entry to safesetid whitelist to allow 'parent' to setgid to 'child'. */
+int add_safesetid_whitelist_gid_entry(kgid_t parent, kgid_t child);
+/* Flush all UID/GID whitelist policies. */
void flush_safesetid_whitelist_entries(void);
#endif /* _SAFESETID_H */
diff --git a/security/safesetid/securityfs.c b/security/safesetid/securityfs.c
index 2c6c829be044..c4c25ba7275f 100644
--- a/security/safesetid/securityfs.c
+++ b/security/safesetid/securityfs.c
@@ -26,20 +26,19 @@ struct safesetid_file_entry {
static struct safesetid_file_entry safesetid_files[] = {
{.name = "add_whitelist_policy",
- .type = SAFESETID_WHITELIST_ADD},
+ .type = SAFESETID_WHITELIST_ADD_UID},
+ {.name = "add_whitelist_uid_policy",
+ .type = SAFESETID_WHITELIST_ADD_UID},
+ {.name = "add_whitelist_gid_policy",
+ .type = SAFESETID_WHITELIST_ADD_GID},
{.name = "flush_whitelist_policies",
.type = SAFESETID_WHITELIST_FLUSH},
};
-/*
- * In the case the input buffer contains one or more invalid UIDs, the kuid_t
- * variables pointed to by 'parent' and 'child' will get updated but this
- * function will return an error.
- */
-static int parse_safesetid_whitelist_policy(const char __user *buf,
+static int parse_userbuf_to_longs(const char __user *buf,
size_t len,
- kuid_t *parent,
- kuid_t *child)
+ long *parent,
+ long *child)
{
char *kern_buf;
char *parent_buf;
@@ -47,8 +46,6 @@ static int parse_safesetid_whitelist_policy(const char __user *buf,
const char separator[] = ":";
int ret;
size_t first_substring_length;
- long parsed_parent;
- long parsed_child;
/* Duplicate string from user memory and NULL-terminate */
kern_buf = memdup_user_nul(buf, len);
@@ -71,27 +68,15 @@ static int parse_safesetid_whitelist_policy(const char __user *buf,
goto free_kern;
}
- ret = kstrtol(parent_buf, 0, &parsed_parent);
+ ret = kstrtol(parent_buf, 0, parent);
if (ret)
goto free_both;
child_buf = kern_buf + first_substring_length + 1;
- ret = kstrtol(child_buf, 0, &parsed_child);
+ ret = kstrtol(child_buf, 0, child);
if (ret)
goto free_both;
- *parent = make_kuid(current_user_ns(), parsed_parent);
- if (!uid_valid(*parent)) {
- ret = -EINVAL;
- goto free_both;
- }
-
- *child = make_kuid(current_user_ns(), parsed_child);
- if (!uid_valid(*child)) {
- ret = -EINVAL;
- goto free_both;
- }
-
free_both:
kfree(parent_buf);
free_kern:
@@ -99,6 +84,52 @@ static int parse_safesetid_whitelist_policy(const char __user *buf,
return ret;
}
+static int parse_safesetid_whitelist_uid_policy(const char __user *buf,
+ size_t len,
+ kuid_t *parent_uid,
+ kuid_t *child_uid)
+{
+ int ret;
+ long parent, child;
+
+ ret = parse_userbuf_to_longs(buf, len, &parent, &child);
+ if (ret)
+ return ret;
+
+ *parent_uid = make_kuid(current_user_ns(), parent);
+ if (!uid_valid(*parent_uid))
+ return -EINVAL;
+
+ *child_uid = make_kuid(current_user_ns(), child);
+ if (!uid_valid(*child_uid))
+ return -EINVAL;
+
+ return 0;
+}
+
+static int parse_safesetid_whitelist_gid_policy(const char __user *buf,
+ size_t len,
+ kgid_t *parent_gid,
+ kgid_t *child_gid)
+{
+ int ret;
+ long parent, child;
+
+ ret = parse_userbuf_to_longs(buf, len, &parent, &child);
+ if (ret)
+ return ret;
+
+ *parent_gid = make_kgid(current_user_ns(), parent);
+ if (!gid_valid(*parent_gid))
+ return -EINVAL;
+
+ *child_gid = make_kgid(current_user_ns(), child);
+ if (!gid_valid(*child_gid))
+ return -EINVAL;
+
+ return 0;
+}
+
static ssize_t safesetid_file_write(struct file *file,
const char __user *buf,
size_t len,
@@ -106,8 +137,10 @@ static ssize_t safesetid_file_write(struct file *file,
{
struct safesetid_file_entry *file_entry =
file->f_inode->i_private;
- kuid_t parent;
- kuid_t child;
+ kuid_t uid_parent;
+ kuid_t uid_child;
+ kgid_t gid_parent;
+ kgid_t gid_child;
int ret;
if (!ns_capable(current_user_ns(), CAP_MAC_ADMIN))
@@ -120,13 +153,23 @@ static ssize_t safesetid_file_write(struct file *file,
case SAFESETID_WHITELIST_FLUSH:
flush_safesetid_whitelist_entries();
break;
- case SAFESETID_WHITELIST_ADD:
- ret = parse_safesetid_whitelist_policy(buf, len, &parent,
- &child);
+ case SAFESETID_WHITELIST_ADD_UID:
+ ret = parse_safesetid_whitelist_uid_policy(buf, len, &uid_parent,
+ &uid_child);
+ if (ret)
+ return ret;
+
+ ret = add_safesetid_whitelist_uid_entry(uid_parent, uid_child);
+ if (ret)
+ return ret;
+ break;
+ case SAFESETID_WHITELIST_ADD_GID:
+ ret = parse_safesetid_whitelist_gid_policy(buf, len, &gid_parent,
+ &gid_child);
if (ret)
return ret;
- ret = add_safesetid_whitelist_entry(parent, child);
+ ret = add_safesetid_whitelist_gid_entry(gid_parent, gid_child);
if (ret)
return ret;
break;
--
2.21.0.352.gf09ad66450-goog
^ permalink raw reply related
* [GIT PULL] security subsystem changes for v5.1
From: James Morris @ 2019-03-05 18:57 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-security-module
Please pull these changes for the security subsystem.
Summary:
- Extend LSM stacking to allow sharing of cred, file, ipc, inode, and task
blobs. This paves the way for more full-featured LSMs to be merged, and is
specifically aimed at LandLock and SARA LSMs. This work is from Casey and
Kees.
- There's a new LSM from Micah Morton: "SafeSetID gates the setid family
of syscalls to restrict UID/GID transitions from a given UID/GID to only
those approved by a system-wide whitelist." This feature is currently
shipping in ChromeOS.
---
The following changes since commit 49a57857aeea06ca831043acbb0fa5e0f50602fd:
Linux 5.0-rc3 (2019-01-21 13:14:44 +1300)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-general
for you to fetch changes up to 468e91cecb3218afd684b8c422490dfebe0691bb:
keys: fix missing __user in KEYCTL_PKEY_QUERY (2019-03-04 15:48:37 -0800)
----------------------------------------------------------------
Ben Dooks (1):
keys: fix missing __user in KEYCTL_PKEY_QUERY
Casey Schaufler (19):
LSM: Add all exclusive LSMs to ordered initialization
procfs: add smack subdir to attrs
Smack: Abstract use of cred security blob
SELinux: Abstract use of cred security blob
SELinux: Remove cred security blob poisoning
SELinux: Remove unused selinux_is_enabled
AppArmor: Abstract use of cred security blob
TOMOYO: Abstract use of cred security blob
Infrastructure management of the cred security blob
SELinux: Abstract use of file security blob
Smack: Abstract use of file security blob
LSM: Infrastructure management of the file security
SELinux: Abstract use of inode security blob
Smack: Abstract use of inode security blob
LSM: Infrastructure management of the inode security
LSM: Infrastructure management of the task security
SELinux: Abstract use of ipc security blobs
Smack: Abstract use of ipc security blobs
LSM: Infrastructure management of the ipc security blob
Gustavo A. R. Silva (1):
security: mark expected switch fall-throughs and add a missing break
James Morris (3):
Merge tag 'v5.0-rc1' into next-general
Merge tag 'blob-stacking-security-next' of https://git.kernel.org/.../kees/linux into next-general
Merge tag 'v5.0-rc3' into next-general
Kees Cook (20):
LSM: Introduce LSM_FLAG_LEGACY_MAJOR
LSM: Provide separate ordered initialization
LSM: Plumb visibility into optional "enabled" state
LSM: Lift LSM selection out of individual LSMs
LSM: Build ordered list of LSMs to initialize
LSM: Introduce CONFIG_LSM
LSM: Introduce "lsm=" for boottime LSM selection
LSM: Tie enabling logic to presence in ordered list
LSM: Prepare for reorganizing "security=" logic
LSM: Refactor "security=" in terms of enable/disable
LSM: Separate idea of "major" LSM from "exclusive" LSM
apparmor: Remove SECURITY_APPARMOR_BOOTPARAM_VALUE
selinux: Remove SECURITY_SELINUX_BOOTPARAM_VALUE
LSM: Split LSM preparation from initialization
LoadPin: Initialize as ordered LSM
Yama: Initialize as ordered LSM
LSM: Introduce enum lsm_order
capability: Initialize as LSM_ORDER_FIRST
TOMOYO: Update LSM flags to no longer be exclusive
LSM: Ignore "security=" when "lsm=" is specified
Mathieu Malaterre (4):
capabilities:: annotate implicit fall through
security: keys: annotate implicit fall through
security: keys: annotate implicit fall throughs
security: keys: annotate implicit fall throughs
Micah Morton (8):
LSM: generalize flag passing to security_capable
LSM: add SafeSetID module that gates setid calls
LSM: add SafeSetID module that gates setid calls
LSM: Add 'name' field for SafeSetID in DEFINE_LSM
LSM: SafeSetID: 'depend' on CONFIG_SECURITY
LSM: SafeSetID: remove unused include
LSM: SafeSetID: add selftest
LSM: Update function documentation for cap_capable
Petr Vorel (1):
LSM: Update list of SECURITYFS users in Kconfig
Tetsuo Handa (6):
LSM: Make lsm_early_cred() and lsm_early_task() local functions.
apparmor: Adjust offset when accessing task blob.
tomoyo: Swicth from cred->security to task_struct->security.
tomoyo: Coding style fix.
tomoyo: Allow multiple use_group lines.
tomoyo: Bump version.
Wei Yongjun (2):
LSM: Make some functions static
LSM: fix return value check in safesetid_init_securityfs()
Documentation/admin-guide/LSM/SafeSetID.rst | 107 ++++
Documentation/admin-guide/LSM/index.rst | 14 +-
Documentation/admin-guide/kernel-parameters.txt | 12 +-
MAINTAINERS | 11 +-
fs/proc/base.c | 64 +-
fs/proc/internal.h | 1 +
include/linux/capability.h | 5 +
include/linux/cred.h | 1 -
include/linux/lsm_hooks.h | 45 +-
include/linux/security.h | 43 +-
include/linux/selinux.h | 35 --
kernel/capability.c | 45 +-
kernel/cred.c | 13 -
kernel/seccomp.c | 4 +-
kernel/sys.c | 10 +-
security/Kconfig | 45 +-
security/Makefile | 2 +
security/apparmor/Kconfig | 16 -
security/apparmor/capability.c | 14 +-
security/apparmor/domain.c | 4 +-
security/apparmor/include/capability.h | 2 +-
security/apparmor/include/cred.h | 16 +-
security/apparmor/include/file.h | 5 +-
security/apparmor/include/lib.h | 4 +
security/apparmor/include/task.h | 18 +-
security/apparmor/ipc.c | 3 +-
security/apparmor/lsm.c | 67 +--
security/apparmor/resource.c | 2 +-
security/apparmor/task.c | 6 +-
security/commoncap.c | 28 +-
security/integrity/ima/ima_appraise.c | 1 +
security/integrity/ima/ima_policy.c | 4 +
security/integrity/ima/ima_template_lib.c | 1 +
security/keys/keyctl.c | 2 +-
security/keys/keyring.c | 1 +
security/keys/process_keys.c | 3 +
security/keys/request_key.c | 4 +
security/loadpin/loadpin.c | 8 +-
security/safesetid/Kconfig | 14 +
security/safesetid/Makefile | 7 +
security/safesetid/lsm.c | 277 +++++++++
security/safesetid/lsm.h | 33 ++
security/safesetid/securityfs.c | 193 ++++++
security/security.c | 648 ++++++++++++++++++---
security/selinux/Kconfig | 15 -
security/selinux/Makefile | 2 +-
security/selinux/exports.c | 23 -
security/selinux/hooks.c | 362 +++---------
security/selinux/include/audit.h | 3 -
security/selinux/include/objsec.h | 38 +-
security/selinux/selinuxfs.c | 4 +-
security/selinux/ss/services.c | 1 -
security/selinux/xfrm.c | 4 +-
security/smack/smack.h | 44 +-
security/smack/smack_access.c | 6 +-
security/smack/smack_lsm.c | 317 ++++------
security/smack/smackfs.c | 18 +-
security/tomoyo/audit.c | 31 +-
security/tomoyo/common.c | 199 +++++--
security/tomoyo/common.h | 51 +-
security/tomoyo/condition.c | 59 +-
security/tomoyo/domain.c | 76 ++-
security/tomoyo/file.c | 20 +
security/tomoyo/gc.c | 19 +
security/tomoyo/group.c | 5 +
security/tomoyo/load_policy.c | 8 +-
security/tomoyo/memory.c | 9 +-
security/tomoyo/mount.c | 2 +
security/tomoyo/realpath.c | 18 +-
security/tomoyo/securityfs_if.c | 30 +-
security/tomoyo/tomoyo.c | 160 +++--
security/tomoyo/util.c | 23 +-
security/yama/yama_lsm.c | 8 +-
tools/testing/selftests/safesetid/.gitignore | 1 +
tools/testing/selftests/safesetid/Makefile | 8 +
tools/testing/selftests/safesetid/config | 2 +
tools/testing/selftests/safesetid/safesetid-test.c | 334 +++++++++++
.../testing/selftests/safesetid/safesetid-test.sh | 26 +
78 files changed, 2674 insertions(+), 1090 deletions(-)
create mode 100644 Documentation/admin-guide/LSM/SafeSetID.rst
delete mode 100644 include/linux/selinux.h
create mode 100644 security/safesetid/Kconfig
create mode 100644 security/safesetid/Makefile
create mode 100644 security/safesetid/lsm.c
create mode 100644 security/safesetid/lsm.h
create mode 100644 security/safesetid/securityfs.c
delete mode 100644 security/selinux/exports.c
create mode 100644 tools/testing/selftests/safesetid/.gitignore
create mode 100644 tools/testing/selftests/safesetid/Makefile
create mode 100644 tools/testing/selftests/safesetid/config
create mode 100644 tools/testing/selftests/safesetid/safesetid-test.c
create mode 100755 tools/testing/selftests/safesetid/safesetid-test.sh
^ permalink raw reply
* [GIT PULL] SELinux patches for v5.1
From: Paul Moore @ 2019-03-05 22:17 UTC (permalink / raw)
To: Linus Torvalds; +Cc: selinux, linux-security-module, linux-kernel
Hi Linus,
Nine SELinux patches for v5.1, all bug fixes. As far as I'm
concerned, nothing really jumps out as risky or special to me, but
each commit has a decent description so you can judge for yourself.
As usual, everything passes the selinux-testsuite; please merge for
v5.1.
Thanks,
-Paul
--
The following changes since commit bfeffd155283772bbe78c6a05dec7c0128ee500c:
Linux 5.0-rc1 (2019-01-06 17:08:20 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
tags/selinux-pr-20190305
for you to fetch changes up to 45189a1998e00f6375ebd49d1e18161acddd73de:
selinux: fix avc audit messages (2019-02-05 12:34:33 -0500)
----------------------------------------------------------------
selinux/stable-5.1 PR 20190305
----------------------------------------------------------------
Ondrej Mosnacek (6):
selinux: never allow relabeling on context mounts
selinux: do not override context on context mounts
selinux: inline some AVC functions used only once
selinux: replace some BUG_ON()s with a WARN_ON()
selinux: log invalid contexts in AVCs
selinux: replace BUG_ONs with WARN_ONs in avc.c
Stephen Smalley (3):
selinux: avoid silent denials in permissive mode under RCU walk
selinux: stop passing MAY_NOT_BLOCK to the AVC upon follow_link
selinux: fix avc audit messages
security/selinux/avc.c | 199 +++++++++++++++++-------------------
security/selinux/hooks.c | 58 ++++++++---
security/selinux/include/avc.h | 6 +-
security/selinux/include/security.h | 3 +
security/selinux/ss/services.c | 37 ++++++-
5 files changed, 176 insertions(+), 127 deletions(-)
--
paul moore
www.paul-moore.com
^ permalink raw reply
* [PATCH] vfs: Move kernel_read_file() to fs/read_write.c
From: David Howells @ 2019-03-05 23:18 UTC (permalink / raw)
To: viro
Cc: Mimi Zohar, dhowells, linux-fsdevel, linux-security-module,
linux-integrity, linux-kernel
Commit bdd1d2d3d251 ("fs: fix kernel_read prototype") moved kernel_read()
to fs/read_write.c without moving the helpers.
Move kernel_read_file() to fs/read_write.c and out of fs/exec.c as it's not
actually used by anything in the execve subsystem.
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
fs/exec.c | 106 -------------------------------------------------------
fs/read_write.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 106 insertions(+), 106 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index fb72d36f7823..cbb1a9cd25ca 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -61,7 +61,6 @@
#include <linux/pipe_fs_i.h>
#include <linux/oom.h>
#include <linux/compat.h>
-#include <linux/vmalloc.h>
#include <linux/uaccess.h>
#include <asm/mmu_context.h>
@@ -892,111 +891,6 @@ struct file *open_exec(const char *name)
}
EXPORT_SYMBOL(open_exec);
-int kernel_read_file(struct file *file, void **buf, loff_t *size,
- loff_t max_size, enum kernel_read_file_id id)
-{
- loff_t i_size, pos;
- ssize_t bytes = 0;
- int ret;
-
- if (!S_ISREG(file_inode(file)->i_mode) || max_size < 0)
- return -EINVAL;
-
- ret = deny_write_access(file);
- if (ret)
- return ret;
-
- ret = security_kernel_read_file(file, id);
- if (ret)
- goto out;
-
- i_size = i_size_read(file_inode(file));
- if (i_size <= 0) {
- ret = -EINVAL;
- goto out;
- }
- if (i_size > SIZE_MAX || (max_size > 0 && i_size > max_size)) {
- ret = -EFBIG;
- goto out;
- }
-
- if (id != READING_FIRMWARE_PREALLOC_BUFFER)
- *buf = vmalloc(i_size);
- if (!*buf) {
- ret = -ENOMEM;
- goto out;
- }
-
- pos = 0;
- while (pos < i_size) {
- bytes = kernel_read(file, *buf + pos, i_size - pos, &pos);
- if (bytes < 0) {
- ret = bytes;
- goto out;
- }
-
- if (bytes == 0)
- break;
- }
-
- if (pos != i_size) {
- ret = -EIO;
- goto out_free;
- }
-
- ret = security_kernel_post_read_file(file, *buf, i_size, id);
- if (!ret)
- *size = pos;
-
-out_free:
- if (ret < 0) {
- if (id != READING_FIRMWARE_PREALLOC_BUFFER) {
- vfree(*buf);
- *buf = NULL;
- }
- }
-
-out:
- allow_write_access(file);
- return ret;
-}
-EXPORT_SYMBOL_GPL(kernel_read_file);
-
-int kernel_read_file_from_path(const char *path, void **buf, loff_t *size,
- loff_t max_size, enum kernel_read_file_id id)
-{
- struct file *file;
- int ret;
-
- if (!path || !*path)
- return -EINVAL;
-
- file = filp_open(path, O_RDONLY, 0);
- if (IS_ERR(file))
- return PTR_ERR(file);
-
- ret = kernel_read_file(file, buf, size, max_size, id);
- fput(file);
- return ret;
-}
-EXPORT_SYMBOL_GPL(kernel_read_file_from_path);
-
-int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
- enum kernel_read_file_id id)
-{
- struct fd f = fdget(fd);
- int ret = -EBADF;
-
- if (!f.file)
- goto out;
-
- ret = kernel_read_file(f.file, buf, size, max_size, id);
-out:
- fdput(f);
- return ret;
-}
-EXPORT_SYMBOL_GPL(kernel_read_file_from_fd);
-
ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
{
ssize_t res = vfs_read(file, (void __user *)addr, len, &pos);
diff --git a/fs/read_write.c b/fs/read_write.c
index ff3c5e6f87cf..555dcaec00ac 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -20,6 +20,7 @@
#include <linux/compat.h>
#include <linux/mount.h>
#include <linux/fs.h>
+#include <linux/vmalloc.h>
#include "internal.h"
#include <linux/uaccess.h>
@@ -1362,6 +1363,111 @@ COMPAT_SYSCALL_DEFINE6(pwritev2, compat_ulong_t, fd,
#endif
+int kernel_read_file(struct file *file, void **buf, loff_t *size,
+ loff_t max_size, enum kernel_read_file_id id)
+{
+ loff_t i_size, pos;
+ ssize_t bytes = 0;
+ int ret;
+
+ if (!S_ISREG(file_inode(file)->i_mode) || max_size < 0)
+ return -EINVAL;
+
+ ret = deny_write_access(file);
+ if (ret)
+ return ret;
+
+ ret = security_kernel_read_file(file, id);
+ if (ret)
+ goto out;
+
+ i_size = i_size_read(file_inode(file));
+ if (i_size <= 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+ if (i_size > SIZE_MAX || (max_size > 0 && i_size > max_size)) {
+ ret = -EFBIG;
+ goto out;
+ }
+
+ if (id != READING_FIRMWARE_PREALLOC_BUFFER)
+ *buf = vmalloc(i_size);
+ if (!*buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ pos = 0;
+ while (pos < i_size) {
+ bytes = kernel_read(file, *buf + pos, i_size - pos, &pos);
+ if (bytes < 0) {
+ ret = bytes;
+ goto out;
+ }
+
+ if (bytes == 0)
+ break;
+ }
+
+ if (pos != i_size) {
+ ret = -EIO;
+ goto out_free;
+ }
+
+ ret = security_kernel_post_read_file(file, *buf, i_size, id);
+ if (!ret)
+ *size = pos;
+
+out_free:
+ if (ret < 0) {
+ if (id != READING_FIRMWARE_PREALLOC_BUFFER) {
+ vfree(*buf);
+ *buf = NULL;
+ }
+ }
+
+out:
+ allow_write_access(file);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(kernel_read_file);
+
+int kernel_read_file_from_path(const char *path, void **buf, loff_t *size,
+ loff_t max_size, enum kernel_read_file_id id)
+{
+ struct file *file;
+ int ret;
+
+ if (!path || !*path)
+ return -EINVAL;
+
+ file = filp_open(path, O_RDONLY, 0);
+ if (IS_ERR(file))
+ return PTR_ERR(file);
+
+ ret = kernel_read_file(file, buf, size, max_size, id);
+ fput(file);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(kernel_read_file_from_path);
+
+int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
+ enum kernel_read_file_id id)
+{
+ struct fd f = fdget(fd);
+ int ret = -EBADF;
+
+ if (!f.file)
+ goto out;
+
+ ret = kernel_read_file(f.file, buf, size, max_size, id);
+out:
+ fdput(f);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(kernel_read_file_from_fd);
+
static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos,
size_t count, loff_t max)
{
^ permalink raw reply related
* Re: [PATCH v7 0/7] Allow initializing the kernfs node's secctx based on its parent
From: Ondrej Mosnacek @ 2019-03-06 15:54 UTC (permalink / raw)
To: selinux, Paul Moore
Cc: Stephen Smalley, Linux Security Module list, Tejun Heo,
Casey Schaufler, Serge E . Hallyn, Greg Kroah-Hartman,
James Morris, linux-fsdevel, cgroups
In-Reply-To: <20190222145718.5740-1-omosnace@redhat.com>
On Fri, Feb 22, 2019 at 3:57 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> TL;DR:
> This series adds a new security hook that allows to initialize the security
> context of kernfs properly, taking into account the parent context (and
> possibly other attributes). Kernfs nodes require special handling here, since
> they are not bound to specific inodes/superblocks, but instead represent the
> backing tree structure that is used to build the VFS tree when the kernfs
> tree is mounted.
>
> Changes in v7:
> - simplify the new security hook's interface
> - rather than trying to extract kernfs data into common structures, just
> pass the kernfs nodes themselves and add helper functions to
> <linux/kernfs.h> for accessing their security xattrs
> - in case other LSMs need more kernfs node attributes than the file mode
> (uid/gid/...), they can simply add new helpers to <linux/kernfs.h> as
> needed
> - refactor "kernfs: use simple_xattrs for security attributes" to keep using
> a single common simple_xattrs structure
> - turns out having two separate simple_xattrs wouldn't work right (see
> the definition of simple_xattr_list() in fs/xattr.c)
> - drop unnecessary initializations from inode_doinit_use_xattr()
> - move the IOP_XATTR check out of inode_doinit_use_xattr()
> - add two kernfs cleanup patches
> - these could be applied independently, but the rest of the patches depend on
> them, so I'd rather they stay bundled with the rest to avoid cross-tree
> conflicts
>
> v6: https://lore.kernel.org/selinux/20190214095015.16032-1-omosnace@redhat.com/T/
> Changes in v6:
> - remove copy-pasted duplicate macro definition
>
> v5: https://lore.kernel.org/selinux/20190205110638.30782-1-omosnace@redhat.com/T/
> Changes in v5:
> - fix misplaced semicolon detected by 0day robot
>
> v4: https://lore.kernel.org/selinux/20190205085915.5183-1-omosnace@redhat.com/T/
> Changes in v4:
> - reorder and rename hook arguments
> - avoid allocating kernfs_iattrs unless needed
>
> v3: https://lore.kernel.org/selinux/20190130114150.27807-1-omosnace@redhat.com/T/
> Changes in v3:
> - rename the hook to "kernfs_init_security"
> - change the hook interface to simply pass pointers to struct iattr and
> struct simple_xattrs of both the new node and its parent
> - add full security xattr support to kernfs (and fixup SELinux behavior
> to handle it properly)
>
> v2: https://lore.kernel.org/selinux/20190109162830.8309-1-omosnace@redhat.com/T/
> Changes in v2:
> - add docstring for the new hook in union security_list_options
> - initialize *ctx to NULL and *ctxlen to 0 in case the hook is not
> implemented
>
> v1: https://lore.kernel.org/selinux/20190109091028.24485-1-omosnace@redhat.com/T/
>
> The kernfs nodes initially do not store any security context and rely on
> the LSM to assign some default context to inodes created over them. Kernfs
> inodes, however, allow setting an explicit context via the *setxattr(2)
> syscalls, in which case the context is stored inside the kernfs node's
> internal structure.
>
> SELinux (and possibly other LSMs) initialize the context of newly created
> FS objects based on the parent object's context (usually the child inherits
> the parent's context, unless the policy dictates otherwise). This is done
> by hooking the creation of the new inode corresponding to the newly created
> file/directory via security_inode_init_security() (most filesystems always
> create a fresh inode when a new FS object is created). However, kernfs nodes
> can be created "behind the scenes" while the filesystem is not mounted
> anywhere and thus no inodes can exist for them yet.
>
> Therefore, to allow maintaining similar behavior for kernfs nodes, a new
> LSM hook is needed, which will allow initializing the kernfs node's
> security context based on its own attributes and those of the parent's
> node.
>
> The main motivation for this change is that the userspace users of cgroupfs
> (which is built on kernfs) expect the usual security context inheritance
> to work under SELinux (see [1] and [2]). This functionality is required for
> better confinement of containers under SELinux.
>
> Patch 1/7 simplifies the kernfs_iattrs structure and patch 2/7 optimizes
> kernfs to not allocate kernfs_iattrs when getting the value of an xattr.
>
> Patch 3/7 changes SELinux to fetch security context from extended
> attributes on kernfs filesystems, falling back to genfs-defined context
> if that fails. Without this patch the 4/7 would be a regression for
> SELinux (due to the removal of ...notifysecctx() call.
>
> Patch 4/7 implements full security xattr support in kernfs using
> simple_xattrs; patch 5/7 adds the new LSM hook; patch 6/7 implements the
> new hook in SELinux; and patch 7/7 modifies kernfs to call the new hook
> on new node creation.
>
> Testing:
> - passed the reproducer from the commit message of the last patch
> - passed SELinux testsuite on Fedora Rawhide (x86_64) when applied on top
> of current Rawhide kernel (5.0.0-0.rc7.git2.1) [3]
> - including the new proposed selinux-testsuite subtest [4] (adapted
> from the reproducer)
>
> [1] https://github.com/SELinuxProject/selinux-kernel/issues/39
> [2] https://bugzilla.redhat.com/show_bug.cgi?id=1553803
> [3] https://koji.fedoraproject.org/koji/taskinfo?taskID=32963825
> [4] https://github.com/SELinuxProject/selinux-testsuite/pull/48
>
> Ondrej Mosnacek (7):
> kernfs: clean up struct kernfs_iattrs
> kernfs: do not alloc iattrs in kernfs_xattr_get
> selinux: try security xattr after genfs for kernfs filesystems
> kernfs: use simple_xattrs for security attributes
> LSM: add new hook for kernfs node initialization
> selinux: implement the kernfs_init_security hook
> kernfs: initialize security of newly created nodes
>
> fs/kernfs/dir.c | 28 ++--
> fs/kernfs/inode.c | 166 +++++++++------------
> fs/kernfs/kernfs-internal.h | 8 +-
> fs/kernfs/symlink.c | 4 +-
> include/linux/kernfs.h | 15 ++
> include/linux/lsm_hooks.h | 13 ++
> include/linux/security.h | 9 ++
> security/security.c | 6 +
> security/selinux/hooks.c | 223 +++++++++++++++++++---------
> security/selinux/include/security.h | 1 +
> 10 files changed, 290 insertions(+), 183 deletions(-)
>
> --
> 2.20.1
Ping about this series... Casey, are you OK with this new version?
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
^ permalink raw reply
* Re: [PATCH v7 0/7] Allow initializing the kernfs node's secctx based on its parent
From: Paul Moore @ 2019-03-06 16:20 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: selinux, Stephen Smalley, Linux Security Module list, Tejun Heo,
Casey Schaufler, Serge E . Hallyn, Greg Kroah-Hartman,
James Morris, linux-fsdevel, cgroups
In-Reply-To: <CAFqZXNs28QAxHA3=vECuG17TQ-be0ywS=r9S48q_KNBYpzgJLg@mail.gmail.com>
On Wed, Mar 6, 2019 at 10:54 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> On Fri, Feb 22, 2019 at 3:57 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > TL;DR:
> > This series adds a new security hook that allows to initialize the security
> > context of kernfs properly, taking into account the parent context (and
> > possibly other attributes). Kernfs nodes require special handling here, since
> > they are not bound to specific inodes/superblocks, but instead represent the
> > backing tree structure that is used to build the VFS tree when the kernfs
> > tree is mounted.
> >
> > Changes in v7:
> > - simplify the new security hook's interface
> > - rather than trying to extract kernfs data into common structures, just
> > pass the kernfs nodes themselves and add helper functions to
> > <linux/kernfs.h> for accessing their security xattrs
> > - in case other LSMs need more kernfs node attributes than the file mode
> > (uid/gid/...), they can simply add new helpers to <linux/kernfs.h> as
> > needed
> > - refactor "kernfs: use simple_xattrs for security attributes" to keep using
> > a single common simple_xattrs structure
> > - turns out having two separate simple_xattrs wouldn't work right (see
> > the definition of simple_xattr_list() in fs/xattr.c)
> > - drop unnecessary initializations from inode_doinit_use_xattr()
> > - move the IOP_XATTR check out of inode_doinit_use_xattr()
> > - add two kernfs cleanup patches
> > - these could be applied independently, but the rest of the patches depend on
> > them, so I'd rather they stay bundled with the rest to avoid cross-tree
> > conflicts
> >
> > v6: https://lore.kernel.org/selinux/20190214095015.16032-1-omosnace@redhat.com/T/
> > Changes in v6:
> > - remove copy-pasted duplicate macro definition
> >
> > v5: https://lore.kernel.org/selinux/20190205110638.30782-1-omosnace@redhat.com/T/
> > Changes in v5:
> > - fix misplaced semicolon detected by 0day robot
> >
> > v4: https://lore.kernel.org/selinux/20190205085915.5183-1-omosnace@redhat.com/T/
> > Changes in v4:
> > - reorder and rename hook arguments
> > - avoid allocating kernfs_iattrs unless needed
> >
> > v3: https://lore.kernel.org/selinux/20190130114150.27807-1-omosnace@redhat.com/T/
> > Changes in v3:
> > - rename the hook to "kernfs_init_security"
> > - change the hook interface to simply pass pointers to struct iattr and
> > struct simple_xattrs of both the new node and its parent
> > - add full security xattr support to kernfs (and fixup SELinux behavior
> > to handle it properly)
> >
> > v2: https://lore.kernel.org/selinux/20190109162830.8309-1-omosnace@redhat.com/T/
> > Changes in v2:
> > - add docstring for the new hook in union security_list_options
> > - initialize *ctx to NULL and *ctxlen to 0 in case the hook is not
> > implemented
> >
> > v1: https://lore.kernel.org/selinux/20190109091028.24485-1-omosnace@redhat.com/T/
> >
> > The kernfs nodes initially do not store any security context and rely on
> > the LSM to assign some default context to inodes created over them. Kernfs
> > inodes, however, allow setting an explicit context via the *setxattr(2)
> > syscalls, in which case the context is stored inside the kernfs node's
> > internal structure.
> >
> > SELinux (and possibly other LSMs) initialize the context of newly created
> > FS objects based on the parent object's context (usually the child inherits
> > the parent's context, unless the policy dictates otherwise). This is done
> > by hooking the creation of the new inode corresponding to the newly created
> > file/directory via security_inode_init_security() (most filesystems always
> > create a fresh inode when a new FS object is created). However, kernfs nodes
> > can be created "behind the scenes" while the filesystem is not mounted
> > anywhere and thus no inodes can exist for them yet.
> >
> > Therefore, to allow maintaining similar behavior for kernfs nodes, a new
> > LSM hook is needed, which will allow initializing the kernfs node's
> > security context based on its own attributes and those of the parent's
> > node.
> >
> > The main motivation for this change is that the userspace users of cgroupfs
> > (which is built on kernfs) expect the usual security context inheritance
> > to work under SELinux (see [1] and [2]). This functionality is required for
> > better confinement of containers under SELinux.
> >
> > Patch 1/7 simplifies the kernfs_iattrs structure and patch 2/7 optimizes
> > kernfs to not allocate kernfs_iattrs when getting the value of an xattr.
> >
> > Patch 3/7 changes SELinux to fetch security context from extended
> > attributes on kernfs filesystems, falling back to genfs-defined context
> > if that fails. Without this patch the 4/7 would be a regression for
> > SELinux (due to the removal of ...notifysecctx() call.
> >
> > Patch 4/7 implements full security xattr support in kernfs using
> > simple_xattrs; patch 5/7 adds the new LSM hook; patch 6/7 implements the
> > new hook in SELinux; and patch 7/7 modifies kernfs to call the new hook
> > on new node creation.
> >
> > Testing:
> > - passed the reproducer from the commit message of the last patch
> > - passed SELinux testsuite on Fedora Rawhide (x86_64) when applied on top
> > of current Rawhide kernel (5.0.0-0.rc7.git2.1) [3]
> > - including the new proposed selinux-testsuite subtest [4] (adapted
> > from the reproducer)
> >
> > [1] https://github.com/SELinuxProject/selinux-kernel/issues/39
> > [2] https://bugzilla.redhat.com/show_bug.cgi?id=1553803
> > [3] https://koji.fedoraproject.org/koji/taskinfo?taskID=32963825
> > [4] https://github.com/SELinuxProject/selinux-testsuite/pull/48
> >
> > Ondrej Mosnacek (7):
> > kernfs: clean up struct kernfs_iattrs
> > kernfs: do not alloc iattrs in kernfs_xattr_get
> > selinux: try security xattr after genfs for kernfs filesystems
> > kernfs: use simple_xattrs for security attributes
> > LSM: add new hook for kernfs node initialization
> > selinux: implement the kernfs_init_security hook
> > kernfs: initialize security of newly created nodes
> >
> > fs/kernfs/dir.c | 28 ++--
> > fs/kernfs/inode.c | 166 +++++++++------------
> > fs/kernfs/kernfs-internal.h | 8 +-
> > fs/kernfs/symlink.c | 4 +-
> > include/linux/kernfs.h | 15 ++
> > include/linux/lsm_hooks.h | 13 ++
> > include/linux/security.h | 9 ++
> > security/security.c | 6 +
> > security/selinux/hooks.c | 223 +++++++++++++++++++---------
> > security/selinux/include/security.h | 1 +
> > 10 files changed, 290 insertions(+), 183 deletions(-)
> >
> > --
> > 2.20.1
>
> Ping about this series... Casey, are you OK with this new version?
We've got two weeks, well one and a half weeks, to decide as the
earliest I would merge this would be after the merge window closes :)
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH 0/2] Create CAAM HW key in linux keyring and use in dmcrypt
From: Jan Lübbe @ 2019-03-06 16:47 UTC (permalink / raw)
To: Franck LENORMAND, linux-kernel, linux-security-module, keyrings
Cc: horia.geanta, silvano.dininno, agk, snitzer, dm-devel, dhowells,
jmorris, serge, David Gstir
In-Reply-To: <1551456599-10603-1-git-send-email-franck.lenormand@nxp.com>
Hi Franck,
thanks for working on this!
On Fri, 2019-03-01 at 17:09 +0100, Franck LENORMAND wrote:
> The creation of such structures and its use was not exposed to userspace so
> it was complicated to use and required custom development. We would like to
> ease this using interface which are known and used:
> - Linux key retention service : Allow to generate or load keys in a
> keyring which can be used by applications.
> - dm-crypt : device mapper allowing to encrypt data.
>
> The capacity to generate or load keys already available in the Linux key
> retention service does not allows to exploit CAAM capabilities hence we
> need to create a new key_type. The new key type "caam_tk" allows to:
> - Create a black key from random
> - Create a black key from a red key
> - Load a black blob to retrieve the black key
On 2018-07-23, Udit Agarwal <udit.agarwal@nxp.com> sent a series which
seems related to this:
[PATCH v2 1/2] security/keys/secure_key: Adds the secure key support
based on CAAM.
[PATCH v2 2/2] encrypted_keys: Adds support for secure key-type as
master key.
Is this series intended to continue that work and cover the same uses-
cases?
If I remember correctly, the CAAM also supports marking blobs to allow
or disallow exporting the encapsulated key from the hardware. Or is
this unneeded and we could encrypt/decrypt other (less critical) key
material against the tk(cbc(aes)) CAAM key via the keyring mechanisms?
Best regards,
Jan
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [RFC PATCH 0/2] Create CAAM HW key in linux keyring and use in dmcrypt
From: David Howells @ 2019-03-06 17:29 UTC (permalink / raw)
To: Franck LENORMAND
Cc: dhowells, linux-kernel, linux-security-module, keyrings,
horia.geanta, silvano.dininno, agk, snitzer, dm-devel, jmorris,
serge
In-Reply-To: <1551456599-10603-1-git-send-email-franck.lenormand@nxp.com>
Franck LENORMAND <franck.lenormand@nxp.com> wrote:
> The capacity to generate or load keys already available in the Linux key
> retention service does not allows to exploit CAAM capabilities hence we
> need to create a new key_type. The new key type "caam_tk" allows to:
> - Create a black key from random
> - Create a black key from a red key
> - Load a black blob to retrieve the black key
Is it possible that this could be done through an existing key type, such as
the asymmetric, trusted or encrypted key typed?
David
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox