Re: [PATCH 2/2] keys, trusted: seal with a policy

2015-12-07 Thread Jarkko Sakkinen 
On Fri, Nov 20, 2015 at 01:34:35PM +1100, James Morris wrote:
> On Wed, 18 Nov 2015, Jarkko Sakkinen wrote:
> 
> > On Wed, Nov 18, 2015 at 11:21:01AM +1100, James Morris wrote:
> > > On Tue, 17 Nov 2015, Jarkko Sakkinen wrote:
> > > 
> > > > }
> > > > break;
> > > > +   case Opt_policydigest:
> > > > +   if (!tpm2 ||
> > > > +   strlen(args[0].from) != (2 * 
> > > > opt->digest_len))
> > > > +   return -EINVAL;
> > > > +   kfree(opt->policydigest);
> > > > +   opt->policydigest = kzalloc(opt->digest_len,
> > > > +   GFP_KERNEL);
> > > 
> > > Is it correct to kfree opt->policydigest here before allocating it?
> > 
> > I think so. The same option might be encountered multiple times.
> 
> This would surely signify an error?

I'm following the semantics of other options. That's why I implemented
it that way for example:

keyctl add trusted kmk "new 32 keyhandle=0x8000 keyhandle=0x8000"

is perfectly OK. I just thought that it'd be more odd if this option
behaved in a different way...

> -- 
> James Morris
> 

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ptrace: use fsuid, fsgid, effective creds for fs access checks

2015-12-07 Thread Kees Cook 
On Sat, Dec 5, 2015 at 6:04 PM, Jann Horn  wrote:
> By checking the effective credentials instead of the real UID /
> permitted capabilities, ensure that the calling process actually
> intended to use its credentials.
>
> To ensure that all ptrace checks use the correct caller
> credentials (e.g. in case out-of-tree code or newly added code
> omits the PTRACE_MODE_*CREDS flag), use two new flags and
> require one of them to be set.
>
> The problem was that when a privileged task had temporarily dropped
> its privileges, e.g. by calling setreuid(0, user_uid), with the
> intent to perform following syscalls with the credentials of
> a user, it still passed ptrace access checks that the user would
> not be able to pass.
>
> While an attacker should not be able to convince the privileged
> task to perform a ptrace() syscall, this is a problem because the
> ptrace access check is reused for things in procfs.
>
> In particular, the following somewhat interesting procfs entries
> only rely on ptrace access checks:
>
>  /proc/$pid/stat - uses the check for determining whether pointers
>  should be visible, useful for bypassing ASLR
>  /proc/$pid/maps - also useful for bypassing ASLR
>  /proc/$pid/cwd - useful for gaining access to restricted
>  directories that contain files with lax permissions, e.g. in
>  this scenario:
>  lrwxrwxrwx root root /proc/13020/cwd -> /root/foobar
>  drwx-- root root /root
>  drwxr-xr-x root root /root/foobar
>  -rw-r--r-- root root /root/foobar/secret
>
> Therefore, on a system where a root-owned mode 6755 binary
> changes its effective credentials as described and then dumps a
> user-specified file, this could be used by an attacker to reveal
> the memory layout of root's processes or reveal the contents of
> files he is not allowed to access (through /proc/$pid/cwd).
>
> Signed-off-by: Jann Horn 
> ---
>  fs/proc/array.c|  2 +-
>  fs/proc/base.c | 21 +++--
>  fs/proc/namespaces.c   |  4 ++--
>  include/linux/ptrace.h | 24 +++-
>  kernel/events/core.c   |  2 +-
>  kernel/futex.c |  2 +-
>  kernel/futex_compat.c  |  2 +-
>  kernel/kcmp.c  |  4 ++--
>  kernel/ptrace.c| 36 +---
>  mm/process_vm_access.c |  2 +-
>  security/commoncap.c   |  7 ++-
>  11 files changed, 78 insertions(+), 28 deletions(-)
>
> diff --git a/fs/proc/array.c b/fs/proc/array.c
> index d73291f..b6c00ce 100644
> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -395,7 +395,7 @@ static int do_task_stat(struct seq_file *m, struct 
> pid_namespace *ns,
>
> state = *get_task_state(task);
> vsize = eip = esp = 0;
> -   permitted = ptrace_may_access(task, PTRACE_MODE_READ | 
> PTRACE_MODE_NOAUDIT);
> +   permitted = ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS | 
> PTRACE_MODE_NOAUDIT);
> mm = get_task_mm(task);
> if (mm) {
> vsize = task_vsize(mm);
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index bd3e9e6..c0a2f29 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -403,7 +403,7 @@ static const struct file_operations proc_pid_cmdline_ops 
> = {
>  static int proc_pid_auxv(struct seq_file *m, struct pid_namespace *ns,
>  struct pid *pid, struct task_struct *task)
>  {
> -   struct mm_struct *mm = mm_access(task, PTRACE_MODE_READ);
> +   struct mm_struct *mm = mm_access(task, PTRACE_MODE_READ_FSCREDS);
> if (mm && !IS_ERR(mm)) {
> unsigned int nwords = 0;
> do {
> @@ -430,7 +430,8 @@ static int proc_pid_wchan(struct seq_file *m, struct 
> pid_namespace *ns,
>
> wchan = get_wchan(task);
>
> -   if (wchan && ptrace_may_access(task, PTRACE_MODE_READ) && 
> !lookup_symbol_name(wchan, symname))
> +   if (wchan && ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS)
> +   && !lookup_symbol_name(wchan, symname))
> seq_printf(m, "%s", symname);
> else
> seq_putc(m, '0');
> @@ -444,7 +445,7 @@ static int lock_trace(struct task_struct *task)
> int err = mutex_lock_killable(>signal->cred_guard_mutex);
> if (err)
> return err;
> -   if (!ptrace_may_access(task, PTRACE_MODE_ATTACH)) {
> +   if (!ptrace_may_access(task, PTRACE_MODE_ATTACH_FSCREDS)) {
> mutex_unlock(>signal->cred_guard_mutex);
> return -EPERM;
> }
> @@ -697,7 +698,7 @@ static int proc_fd_access_allowed(struct inode *inode)
>  */
> task = get_proc_task(inode);
> if (task) {
> -   allowed = ptrace_may_access(task, PTRACE_MODE_READ);
> +   allowed = ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
> put_task_struct(task);
> }
> return allowed;
> @@ -732,7 +733,7 @@ static bool has_pid_permissions(struct pid_namespace *pid,
>

Re: [PATCH] ptrace: use fsuid, fsgid, effective creds for fs access checks

2015-12-07 Thread Jann Horn 
On Mon, Dec 07, 2015 at 12:32:06PM -0800, Kees Cook wrote:
> On Sat, Dec 5, 2015 at 6:04 PM, Jann Horn  wrote:
[...]
> > -   if (ptrace_may_access(task, PTRACE_MODE_READ)) {
> > +   if (ptrace_may_access(task, PTRACE_MODE_READ | 
> > PTRACE_MODE_FSCREDS)) {
> 
> This should maybe use the PTRACE_MODE_READ_FSCREDS macro?

Oh, yes. I don't know how I missed that. :/


> > error = ns_get_path(_path, task, ns_ops);
> > if (!error)
> > nd_jump_link(_path);
> > @@ -63,7 +63,7 @@ static int proc_ns_readlink(struct dentry *dentry, char 
> > __user *buffer, int bufl
> > if (!task)
> > return res;
> >
> > -   if (ptrace_may_access(task, PTRACE_MODE_READ)) {
> > +   if (ptrace_may_access(task, PTRACE_MODE_READ | 
> > PTRACE_MODE_FSCREDS)) {
> 
> same here?

Yes.

signature.asc
Description: Digital signature

[PATCH v2 13/18] fs: Allow superblock owner to access do_remount_sb()

2015-12-07 Thread Seth Forshee 
Superblock level remounts are currently restricted to global
CAP_SYS_ADMIN, as is the path for changing the root mount to
read only on umount. Loosen both of these permission checks to
also allow CAP_SYS_ADMIN in any namespace which is privileged
towards the userns which originally mounted the filesystem.

Signed-off-by: Seth Forshee 
Acked-by: "Eric W. Biederman" 
Acked-by: Serge Hallyn 
---
 fs/namespace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 18fc58760aec..b00a765895e7 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1510,7 +1510,7 @@ static int do_umount(struct mount *mnt, int flags)
 * Special case for "unmounting" root ...
 * we just try to remount it readonly.
 */
-   if (!capable(CAP_SYS_ADMIN))
+   if (!ns_capable(sb->s_user_ns, CAP_SYS_ADMIN))
return -EPERM;
down_write(>s_umount);
if (!(sb->s_flags & MS_RDONLY))
@@ -2199,7 +2199,7 @@ static int do_remount(struct path *path, int flags, int 
mnt_flags,
down_write(>s_umount);
if (flags & MS_BIND)
err = change_mount_flags(path->mnt, flags);
-   else if (!capable(CAP_SYS_ADMIN))
+   else if (!ns_capable(sb->s_user_ns, CAP_SYS_ADMIN))
err = -EPERM;
else
err = do_remount_sb(sb, flags, data, 0);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ptrace: use fsuid, fsgid, effective creds for fs access checks

2015-12-07 Thread Jann Horn 
Whoops. After Kees pointed out my last mistake, I decided to grep around a bit 
to make sure
I didn't miss anything else and noticed that apparently, Yama and Smack aren't 
completely
aware that the ptrace access mode can have flags ORed in? Until now, it was 
just the
NOAUDIT flag for /proc/$pid/stat, but with my patch, that would have been 
broken completely
as far as I can tell. I don't use either of those LSMs and didn't test with 
them.

Can the LSM maintainers have a look at this and say whether this looks okay now?
--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 16/18] fuse: Support fuse filesystems outside of init_user_ns

2015-12-07 Thread Seth Forshee 
In order to support mounts from namespaces other than
init_user_ns, fuse must translate uids and gids to/from the
userns of the process servicing requests on /dev/fuse. This
patch does that, with a couple of restrictions on the namespace:

 - The userns for the fuse connection is fixed to the namespace
   from which /dev/fuse is opened.

 - The namespace must be the same as s_user_ns.

These restrictions simplify the implementation by avoiding the
need to pass around userns references and by allowing fuse to
rely on the checks in inode_change_ok for ownership changes.
Either restriction could be relaxed in the future if needed.

For cuse the namespace used for the connection is also simply
current_user_ns() at the time /dev/cuse is opened.

Signed-off-by: Seth Forshee 
---
 fs/fuse/cuse.c   |  3 ++-
 fs/fuse/dev.c| 13 -
 fs/fuse/dir.c| 14 +++---
 fs/fuse/fuse_i.h |  6 +-
 fs/fuse/inode.c  | 35 +++
 5 files changed, 45 insertions(+), 26 deletions(-)

diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c
index eae2c11268bc..a10aca57bfe4 100644
--- a/fs/fuse/cuse.c
+++ b/fs/fuse/cuse.c
@@ -48,6 +48,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "fuse_i.h"
 
@@ -498,7 +499,7 @@ static int cuse_channel_open(struct inode *inode, struct 
file *file)
if (!cc)
return -ENOMEM;
 
-   fuse_conn_init(>fc);
+   fuse_conn_init(>fc, current_user_ns());
 
fud = fuse_dev_alloc(>fc);
if (!fud) {
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index a4f6f30d6d86..11b4cb0a0e2f 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -127,8 +127,8 @@ static void __fuse_put_request(struct fuse_req *req)
 
 static void fuse_req_init_context(struct fuse_conn *fc, struct fuse_req *req)
 {
-   req->in.h.uid = from_kuid_munged(_user_ns, current_fsuid());
-   req->in.h.gid = from_kgid_munged(_user_ns, current_fsgid());
+   req->in.h.uid = from_kuid(fc->user_ns, current_fsuid());
+   req->in.h.gid = from_kgid(fc->user_ns, current_fsgid());
req->in.h.pid = pid_nr_ns(task_pid(current), fc->pid_ns);
 }
 
@@ -186,7 +186,8 @@ static struct fuse_req *__fuse_get_req(struct fuse_conn 
*fc, unsigned npages,
__set_bit(FR_WAITING, >flags);
if (for_background)
__set_bit(FR_BACKGROUND, >flags);
-   if (req->in.h.pid == 0) {
+   if (req->in.h.pid == 0 || req->in.h.uid == (uid_t)-1 ||
+   req->in.h.gid == (gid_t)-1) {
fuse_put_request(fc, req);
return ERR_PTR(-EOVERFLOW);
}
@@ -1248,7 +1249,8 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, 
struct file *file,
struct fuse_in *in;
unsigned reqsize;
 
-   if (task_active_pid_ns(current) != fc->pid_ns)
+   if (task_active_pid_ns(current) != fc->pid_ns ||
+   current_user_ns() != fc->user_ns)
return -EIO;
 
  restart:
@@ -1880,7 +1882,8 @@ static ssize_t fuse_dev_do_write(struct fuse_dev *fud,
struct fuse_req *req;
struct fuse_out_header oh;
 
-   if (task_active_pid_ns(current) != fc->pid_ns)
+   if (task_active_pid_ns(current) != fc->pid_ns ||
+   current_user_ns() != fc->user_ns)
return -EIO;
 
if (nbytes < sizeof(struct fuse_out_header))
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 5e2e08712d3b..8fd9fe4dcd43 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -841,8 +841,8 @@ static void fuse_fillattr(struct inode *inode, struct 
fuse_attr *attr,
stat->ino = attr->ino;
stat->mode = (inode->i_mode & S_IFMT) | (attr->mode & 0);
stat->nlink = attr->nlink;
-   stat->uid = make_kuid(_user_ns, attr->uid);
-   stat->gid = make_kgid(_user_ns, attr->gid);
+   stat->uid = inode->i_uid;
+   stat->gid = inode->i_gid;
stat->rdev = inode->i_rdev;
stat->atime.tv_sec = attr->atime;
stat->atime.tv_nsec = attr->atimensec;
@@ -1455,17 +1455,17 @@ static bool update_mtime(unsigned ivalid, bool 
trust_local_mtime)
return true;
 }
 
-static void iattr_to_fattr(struct iattr *iattr, struct fuse_setattr_in *arg,
-  bool trust_local_cmtime)
+static void iattr_to_fattr(struct fuse_conn *fc, struct iattr *iattr,
+  struct fuse_setattr_in *arg, bool trust_local_cmtime)
 {
unsigned ivalid = iattr->ia_valid;
 
if (ivalid & ATTR_MODE)
arg->valid |= FATTR_MODE,   arg->mode = iattr->ia_mode;
if (ivalid & ATTR_UID)
-   arg->valid |= FATTR_UID,arg->uid = from_kuid(_user_ns, 
iattr->ia_uid);
+   arg->valid |= FATTR_UID,arg->uid = from_kuid(fc->user_ns, 
iattr->ia_uid);
if (ivalid & ATTR_GID)
-   arg->valid |= FATTR_GID,arg->gid = from_kgid(_user_ns, 
iattr->ia_gid);
+   arg->valid |= FATTR_GID,arg->gid = from_kgid(fc->user_ns, 
iattr->ia_gid);

[PATCH 1/2] security: let security modules use PTRACE_MODE_* with bitmasks

2015-12-07 Thread Jann Horn 
It looks like smack and yama weren't aware that the ptrace mode
can have flags ORed into it - PTRACE_MODE_NOAUDIT until now, but
only for /proc/$pid/stat, and with the PTRACE_MODE_*CREDS patch,
all modes have flags ORed into them.

Signed-off-by: Jann Horn 
---
 security/smack/smack_lsm.c | 8 +++-
 security/yama/yama_lsm.c   | 4 ++--
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index ff81026..7c57c7f 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -398,12 +398,10 @@ static int smk_copy_relabel(struct list_head *nhead, 
struct list_head *ohead,
  */
 static inline unsigned int smk_ptrace_mode(unsigned int mode)
 {
-   switch (mode) {
-   case PTRACE_MODE_READ:
-   return MAY_READ;
-   case PTRACE_MODE_ATTACH:
+   if (mode & PTRACE_MODE_ATTACH)
return MAY_READWRITE;
-   }
+   if (mode & PTRACE_MODE_READ)
+   return MAY_READ;
 
return 0;
 }
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index d3c19c9..cb6ed10 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -281,7 +281,7 @@ static int yama_ptrace_access_check(struct task_struct 
*child,
int rc = 0;
 
/* require ptrace target be a child of ptracer on attach */
-   if (mode == PTRACE_MODE_ATTACH) {
+   if (mode & PTRACE_MODE_ATTACH) {
switch (ptrace_scope) {
case YAMA_SCOPE_DISABLED:
/* No additional restrictions. */
@@ -307,7 +307,7 @@ static int yama_ptrace_access_check(struct task_struct 
*child,
}
}
 
-   if (rc) {
+   if (rc && (mode & PTRACE_MODE_NOAUDIT) == 0) {
printk_ratelimited(KERN_NOTICE
"ptrace of pid %d was attempted by: %s (pid %d)\n",
child->pid, current->comm, current->pid);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] ptrace: use fsuid, fsgid, effective creds for fs access checks

2015-12-07 Thread Jann Horn 
By checking the effective credentials instead of the real UID /
permitted capabilities, ensure that the calling process actually
intended to use its credentials.

To ensure that all ptrace checks use the correct caller
credentials (e.g. in case out-of-tree code or newly added code
omits the PTRACE_MODE_*CREDS flag), use two new flags and
require one of them to be set.

The problem was that when a privileged task had temporarily dropped
its privileges, e.g. by calling setreuid(0, user_uid), with the
intent to perform following syscalls with the credentials of
a user, it still passed ptrace access checks that the user would
not be able to pass.

While an attacker should not be able to convince the privileged
task to perform a ptrace() syscall, this is a problem because the
ptrace access check is reused for things in procfs.

In particular, the following somewhat interesting procfs entries
only rely on ptrace access checks:

 /proc/$pid/stat - uses the check for determining whether pointers
 should be visible, useful for bypassing ASLR
 /proc/$pid/maps - also useful for bypassing ASLR
 /proc/$pid/cwd - useful for gaining access to restricted
 directories that contain files with lax permissions, e.g. in
 this scenario:
 lrwxrwxrwx root root /proc/13020/cwd -> /root/foobar
 drwx-- root root /root
 drwxr-xr-x root root /root/foobar
 -rw-r--r-- root root /root/foobar/secret

Therefore, on a system where a root-owned mode 6755 binary
changes its effective credentials as described and then dumps a
user-specified file, this could be used by an attacker to reveal
the memory layout of root's processes or reveal the contents of
files he is not allowed to access (through /proc/$pid/cwd).

Signed-off-by: Jann Horn 
---
 fs/proc/array.c|  2 +-
 fs/proc/base.c | 21 +++--
 fs/proc/namespaces.c   |  4 ++--
 include/linux/ptrace.h | 24 +++-
 kernel/events/core.c   |  2 +-
 kernel/futex.c |  2 +-
 kernel/futex_compat.c  |  2 +-
 kernel/kcmp.c  |  4 ++--
 kernel/ptrace.c| 36 +---
 mm/process_vm_access.c |  2 +-
 security/commoncap.c   |  7 ++-
 11 files changed, 78 insertions(+), 28 deletions(-)

diff --git a/fs/proc/array.c b/fs/proc/array.c
index d73291f..b6c00ce 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -395,7 +395,7 @@ static int do_task_stat(struct seq_file *m, struct 
pid_namespace *ns,
 
state = *get_task_state(task);
vsize = eip = esp = 0;
-   permitted = ptrace_may_access(task, PTRACE_MODE_READ | 
PTRACE_MODE_NOAUDIT);
+   permitted = ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS | 
PTRACE_MODE_NOAUDIT);
mm = get_task_mm(task);
if (mm) {
vsize = task_vsize(mm);
diff --git a/fs/proc/base.c b/fs/proc/base.c
index bd3e9e6..c0a2f29 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -403,7 +403,7 @@ static const struct file_operations proc_pid_cmdline_ops = {
 static int proc_pid_auxv(struct seq_file *m, struct pid_namespace *ns,
 struct pid *pid, struct task_struct *task)
 {
-   struct mm_struct *mm = mm_access(task, PTRACE_MODE_READ);
+   struct mm_struct *mm = mm_access(task, PTRACE_MODE_READ_FSCREDS);
if (mm && !IS_ERR(mm)) {
unsigned int nwords = 0;
do {
@@ -430,7 +430,8 @@ static int proc_pid_wchan(struct seq_file *m, struct 
pid_namespace *ns,
 
wchan = get_wchan(task);
 
-   if (wchan && ptrace_may_access(task, PTRACE_MODE_READ) && 
!lookup_symbol_name(wchan, symname))
+   if (wchan && ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS)
+   && !lookup_symbol_name(wchan, symname))
seq_printf(m, "%s", symname);
else
seq_putc(m, '0');
@@ -444,7 +445,7 @@ static int lock_trace(struct task_struct *task)
int err = mutex_lock_killable(>signal->cred_guard_mutex);
if (err)
return err;
-   if (!ptrace_may_access(task, PTRACE_MODE_ATTACH)) {
+   if (!ptrace_may_access(task, PTRACE_MODE_ATTACH_FSCREDS)) {
mutex_unlock(>signal->cred_guard_mutex);
return -EPERM;
}
@@ -697,7 +698,7 @@ static int proc_fd_access_allowed(struct inode *inode)
 */
task = get_proc_task(inode);
if (task) {
-   allowed = ptrace_may_access(task, PTRACE_MODE_READ);
+   allowed = ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
put_task_struct(task);
}
return allowed;
@@ -732,7 +733,7 @@ static bool has_pid_permissions(struct pid_namespace *pid,
return true;
if (in_group_p(pid->pid_gid))
return true;
-   return ptrace_may_access(task, PTRACE_MODE_READ);
+   return ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
 }
 
 
@@ -809,7 +810,7 @@ struct mm_struct *proc_mem_open(struct

[PATCH v2 18/18] fuse: Allow user namespace mounts

2015-12-07 Thread Seth Forshee 
Signed-off-by: Seth Forshee 
---
 fs/fuse/inode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index b7bdfdac3521..2fd338c199ce 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1201,7 +1201,7 @@ static void fuse_kill_sb_anon(struct super_block *sb)
 static struct file_system_type fuse_fs_type = {
.owner  = THIS_MODULE,
.name   = "fuse",
-   .fs_flags   = FS_HAS_SUBTYPE,
+   .fs_flags   = FS_HAS_SUBTYPE | FS_USERNS_MOUNT,
.mount  = fuse_mount,
.kill_sb= fuse_kill_sb_anon,
 };
@@ -1233,7 +1233,7 @@ static struct file_system_type fuseblk_fs_type = {
.name   = "fuseblk",
.mount  = fuse_mount_blk,
.kill_sb= fuse_kill_sb_blk,
-   .fs_flags   = FS_REQUIRES_DEV | FS_HAS_SUBTYPE,
+   .fs_flags   = FS_REQUIRES_DEV | FS_HAS_SUBTYPE | FS_USERNS_MOUNT,
 };
 MODULE_ALIAS_FS("fuseblk");
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 14/18] capabilities: Allow privileged user in s_user_ns to set security.* xattrs

2015-12-07 Thread Seth Forshee 
A privileged user in s_user_ns will generally have the ability to
manipulate the backing store and insert security.* xattrs into
the filesystem directly. Therefore the kernel must be prepared to
handle these xattrs from unprivileged mounts, and it makes little
sense for commoncap to prevent writing these xattrs to the
filesystem. The capability and LSM code have already been updated
to appropriately handle xattrs from unprivileged mounts, so it
is safe to loosen this restriction on setting xattrs.

The exception to this logic is that writing xattrs to a mounted
filesystem may also cause the LSM inode_post_setxattr or
inode_setsecurity callbacks to be invoked. SELinux will deny the
xattr update by virtue of applying mountpoint labeling to
unprivileged userns mounts, and Smack will deny the writes for
any user without global CAP_MAC_ADMIN, so loosening the
capability check in commoncap is safe in this respect as well.

Signed-off-by: Seth Forshee 
Acked-by: Serge Hallyn 
---
 security/commoncap.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/security/commoncap.c b/security/commoncap.c
index 2119421613f6..d6c80c19c449 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -653,15 +653,17 @@ int cap_bprm_secureexec(struct linux_binprm *bprm)
 int cap_inode_setxattr(struct dentry *dentry, const char *name,
   const void *value, size_t size, int flags)
 {
+   struct user_namespace *user_ns = dentry->d_sb->s_user_ns;
+
if (!strcmp(name, XATTR_NAME_CAPS)) {
-   if (!capable(CAP_SETFCAP))
+   if (!ns_capable(user_ns, CAP_SETFCAP))
return -EPERM;
return 0;
}
 
if (!strncmp(name, XATTR_SECURITY_PREFIX,
 sizeof(XATTR_SECURITY_PREFIX) - 1) &&
-   !capable(CAP_SYS_ADMIN))
+   !ns_capable(user_ns, CAP_SYS_ADMIN))
return -EPERM;
return 0;
 }
@@ -679,15 +681,17 @@ int cap_inode_setxattr(struct dentry *dentry, const char 
*name,
  */
 int cap_inode_removexattr(struct dentry *dentry, const char *name)
 {
+   struct user_namespace *user_ns = dentry->d_sb->s_user_ns;
+
if (!strcmp(name, XATTR_NAME_CAPS)) {
-   if (!capable(CAP_SETFCAP))
+   if (!ns_capable(user_ns, CAP_SETFCAP))
return -EPERM;
return 0;
}
 
if (!strncmp(name, XATTR_SECURITY_PREFIX,
 sizeof(XATTR_SECURITY_PREFIX) - 1) &&
-   !capable(CAP_SYS_ADMIN))
+   !ns_capable(user_ns, CAP_SYS_ADMIN))
return -EPERM;
return 0;
 }
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] ptrace: use fsuid, fsgid, effective creds for fs access checks

2015-12-07 Thread Kees Cook 
On Mon, Dec 7, 2015 at 1:25 PM, Jann Horn  wrote:
> By checking the effective credentials instead of the real UID /
> permitted capabilities, ensure that the calling process actually
> intended to use its credentials.
>
> To ensure that all ptrace checks use the correct caller
> credentials (e.g. in case out-of-tree code or newly added code
> omits the PTRACE_MODE_*CREDS flag), use two new flags and
> require one of them to be set.
>
> The problem was that when a privileged task had temporarily dropped
> its privileges, e.g. by calling setreuid(0, user_uid), with the
> intent to perform following syscalls with the credentials of
> a user, it still passed ptrace access checks that the user would
> not be able to pass.
>
> While an attacker should not be able to convince the privileged
> task to perform a ptrace() syscall, this is a problem because the
> ptrace access check is reused for things in procfs.
>
> In particular, the following somewhat interesting procfs entries
> only rely on ptrace access checks:
>
>  /proc/$pid/stat - uses the check for determining whether pointers
>  should be visible, useful for bypassing ASLR
>  /proc/$pid/maps - also useful for bypassing ASLR
>  /proc/$pid/cwd - useful for gaining access to restricted
>  directories that contain files with lax permissions, e.g. in
>  this scenario:
>  lrwxrwxrwx root root /proc/13020/cwd -> /root/foobar
>  drwx-- root root /root
>  drwxr-xr-x root root /root/foobar
>  -rw-r--r-- root root /root/foobar/secret
>
> Therefore, on a system where a root-owned mode 6755 binary
> changes its effective credentials as described and then dumps a
> user-specified file, this could be used by an attacker to reveal
> the memory layout of root's processes or reveal the contents of
> files he is not allowed to access (through /proc/$pid/cwd).
>
> Signed-off-by: Jann Horn 

Acked-by: Kees Cook 

-Kees

> ---
>  fs/proc/array.c|  2 +-
>  fs/proc/base.c | 21 +++--
>  fs/proc/namespaces.c   |  4 ++--
>  include/linux/ptrace.h | 24 +++-
>  kernel/events/core.c   |  2 +-
>  kernel/futex.c |  2 +-
>  kernel/futex_compat.c  |  2 +-
>  kernel/kcmp.c  |  4 ++--
>  kernel/ptrace.c| 36 +---
>  mm/process_vm_access.c |  2 +-
>  security/commoncap.c   |  7 ++-
>  11 files changed, 78 insertions(+), 28 deletions(-)
>
> diff --git a/fs/proc/array.c b/fs/proc/array.c
> index d73291f..b6c00ce 100644
> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -395,7 +395,7 @@ static int do_task_stat(struct seq_file *m, struct 
> pid_namespace *ns,
>
> state = *get_task_state(task);
> vsize = eip = esp = 0;
> -   permitted = ptrace_may_access(task, PTRACE_MODE_READ | 
> PTRACE_MODE_NOAUDIT);
> +   permitted = ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS | 
> PTRACE_MODE_NOAUDIT);
> mm = get_task_mm(task);
> if (mm) {
> vsize = task_vsize(mm);
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index bd3e9e6..c0a2f29 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -403,7 +403,7 @@ static const struct file_operations proc_pid_cmdline_ops 
> = {
>  static int proc_pid_auxv(struct seq_file *m, struct pid_namespace *ns,
>  struct pid *pid, struct task_struct *task)
>  {
> -   struct mm_struct *mm = mm_access(task, PTRACE_MODE_READ);
> +   struct mm_struct *mm = mm_access(task, PTRACE_MODE_READ_FSCREDS);
> if (mm && !IS_ERR(mm)) {
> unsigned int nwords = 0;
> do {
> @@ -430,7 +430,8 @@ static int proc_pid_wchan(struct seq_file *m, struct 
> pid_namespace *ns,
>
> wchan = get_wchan(task);
>
> -   if (wchan && ptrace_may_access(task, PTRACE_MODE_READ) && 
> !lookup_symbol_name(wchan, symname))
> +   if (wchan && ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS)
> +   && !lookup_symbol_name(wchan, symname))
> seq_printf(m, "%s", symname);
> else
> seq_putc(m, '0');
> @@ -444,7 +445,7 @@ static int lock_trace(struct task_struct *task)
> int err = mutex_lock_killable(>signal->cred_guard_mutex);
> if (err)
> return err;
> -   if (!ptrace_may_access(task, PTRACE_MODE_ATTACH)) {
> +   if (!ptrace_may_access(task, PTRACE_MODE_ATTACH_FSCREDS)) {
> mutex_unlock(>signal->cred_guard_mutex);
> return -EPERM;
> }
> @@ -697,7 +698,7 @@ static int proc_fd_access_allowed(struct inode *inode)
>  */
> task = get_proc_task(inode);
> if (task) {
> -   allowed = ptrace_may_access(task, PTRACE_MODE_READ);
> +   allowed = ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
> put_task_struct(task);
> }
> return allowed;
> @@ -732,7 +733,7 @@ static

[PATCH v2 12/18] fs: Don't remove suid for CAP_FSETID in s_user_ns

2015-12-07 Thread Seth Forshee 
Expand the check in should_remove_suid() to keep privileges for
CAP_FSETID in s_user_ns rather than init_user_ns.

Signed-off-by: Seth Forshee 
Acked-by: Serge Hallyn 
---
 fs/inode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/inode.c b/fs/inode.c
index 01c036fe1950..3e7c74da9304 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1684,7 +1684,8 @@ int should_remove_suid(struct dentry *dentry)
if (unlikely((mode & S_ISGID) && (mode & S_IXGRP)))
kill |= ATTR_KILL_SGID;
 
-   if (unlikely(kill && !capable(CAP_FSETID) && S_ISREG(mode)))
+   if (unlikely(kill && !ns_capable(dentry->d_sb->s_user_ns, CAP_FSETID) &&
+S_ISREG(mode)))
return kill;
 
return 0;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] security: let security modules use PTRACE_MODE_* with bitmasks

2015-12-07 Thread Kees Cook 
On Mon, Dec 7, 2015 at 1:25 PM, Jann Horn  wrote:
> It looks like smack and yama weren't aware that the ptrace mode
> can have flags ORed into it - PTRACE_MODE_NOAUDIT until now, but
> only for /proc/$pid/stat, and with the PTRACE_MODE_*CREDS patch,
> all modes have flags ORed into them.
>
> Signed-off-by: Jann Horn 

Acked-by: Kees Cook 

-Kees

> ---
>  security/smack/smack_lsm.c | 8 +++-
>  security/yama/yama_lsm.c   | 4 ++--
>  2 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index ff81026..7c57c7f 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -398,12 +398,10 @@ static int smk_copy_relabel(struct list_head *nhead, 
> struct list_head *ohead,
>   */
>  static inline unsigned int smk_ptrace_mode(unsigned int mode)
>  {
> -   switch (mode) {
> -   case PTRACE_MODE_READ:
> -   return MAY_READ;
> -   case PTRACE_MODE_ATTACH:
> +   if (mode & PTRACE_MODE_ATTACH)
> return MAY_READWRITE;
> -   }
> +   if (mode & PTRACE_MODE_READ)
> +   return MAY_READ;
>
> return 0;
>  }
> diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
> index d3c19c9..cb6ed10 100644
> --- a/security/yama/yama_lsm.c
> +++ b/security/yama/yama_lsm.c
> @@ -281,7 +281,7 @@ static int yama_ptrace_access_check(struct task_struct 
> *child,
> int rc = 0;
>
> /* require ptrace target be a child of ptracer on attach */
> -   if (mode == PTRACE_MODE_ATTACH) {
> +   if (mode & PTRACE_MODE_ATTACH) {
> switch (ptrace_scope) {
> case YAMA_SCOPE_DISABLED:
> /* No additional restrictions. */
> @@ -307,7 +307,7 @@ static int yama_ptrace_access_check(struct task_struct 
> *child,
> }
> }
>
> -   if (rc) {
> +   if (rc && (mode & PTRACE_MODE_NOAUDIT) == 0) {
> printk_ratelimited(KERN_NOTICE
> "ptrace of pid %d was attempted by: %s (pid %d)\n",
> child->pid, current->comm, current->pid);
> --
> 2.1.4
>



-- 
Kees Cook
Chrome OS & Brillo Security
--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 09/18] fs: Refuse uid/gid changes which don't map into s_user_ns

2015-12-07 Thread Seth Forshee 
Add checks to inode_change_ok to verify that uid and gid changes
will map into the superblock's user namespace. If they do not
fail with -EOVERFLOW. This cannot be overriden with ATTR_FORCE.

Signed-off-by: Seth Forshee 
Acked-by: Serge Hallyn 
---
 fs/attr.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/fs/attr.c b/fs/attr.c
index 6530ced19697..55b46e3aa888 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -42,6 +42,17 @@ int inode_change_ok(const struct inode *inode, struct iattr 
*attr)
return error;
}
 
+   /*
+* Verify that uid/gid changes are valid in the target namespace
+* of the superblock. This cannot be overriden using ATTR_FORCE.
+*/
+   if (ia_valid & ATTR_UID &&
+   from_kuid(inode->i_sb->s_user_ns, attr->ia_uid) == (uid_t)-1)
+   return -EOVERFLOW;
+   if (ia_valid & ATTR_GID &&
+   from_kgid(inode->i_sb->s_user_ns, attr->ia_gid) == (gid_t)-1)
+   return -EOVERFLOW;
+
/* If force is set do it anyway. */
if (ia_valid & ATTR_FORCE)
return 0;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 10/18] fs: Update posix_acl support to handle user namespace mounts

2015-12-07 Thread Seth Forshee 
ids in on-disk ACLs should be converted to s_user_ns instead of
init_user_ns as is done now. This introduces the possibility for
id mappings to fail, and when this happens syscalls will return
EOVERFLOW.

Signed-off-by: Seth Forshee 
Acked-by: Serge Hallyn 
---
 fs/posix_acl.c  | 67 ++---
 fs/xattr.c  | 19 +---
 include/linux/posix_acl_xattr.h | 17 ---
 3 files changed, 70 insertions(+), 33 deletions(-)

diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index 4adde1e2cbec..a29442eb4af8 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -595,59 +595,77 @@ EXPORT_SYMBOL_GPL(posix_acl_create);
 /*
  * Fix up the uids and gids in posix acl extended attributes in place.
  */
-static void posix_acl_fix_xattr_userns(
+static int posix_acl_fix_xattr_userns(
struct user_namespace *to, struct user_namespace *from,
void *value, size_t size)
 {
posix_acl_xattr_header *header = (posix_acl_xattr_header *)value;
posix_acl_xattr_entry *entry = (posix_acl_xattr_entry *)(header+1), 
*end;
int count;
-   kuid_t uid;
-   kgid_t gid;
+   kuid_t kuid;
+   uid_t uid;
+   kgid_t kgid;
+   gid_t gid;
 
if (!value)
-   return;
+   return 0;
if (size < sizeof(posix_acl_xattr_header))
-   return;
+   return 0;
if (header->a_version != cpu_to_le32(POSIX_ACL_XATTR_VERSION))
-   return;
+   return 0;
 
count = posix_acl_xattr_count(size);
if (count < 0)
-   return;
+   return 0;
if (count == 0)
-   return;
+   return 0;
 
for (end = entry + count; entry != end; entry++) {
switch(le16_to_cpu(entry->e_tag)) {
case ACL_USER:
-   uid = make_kuid(from, le32_to_cpu(entry->e_id));
-   entry->e_id = cpu_to_le32(from_kuid(to, uid));
+   kuid = make_kuid(from, le32_to_cpu(entry->e_id));
+   if (!uid_valid(kuid))
+   return -EOVERFLOW;
+   uid = from_kuid(to, kuid);
+   if (uid == (uid_t)-1)
+   return -EOVERFLOW;
+   entry->e_id = cpu_to_le32(uid);
break;
case ACL_GROUP:
-   gid = make_kgid(from, le32_to_cpu(entry->e_id));
-   entry->e_id = cpu_to_le32(from_kgid(to, gid));
+   kgid = make_kgid(from, le32_to_cpu(entry->e_id));
+   if (!gid_valid(kgid))
+   return -EOVERFLOW;
+   gid = from_kgid(to, kgid);
+   if (gid == (gid_t)-1)
+   return -EOVERFLOW;
+   entry->e_id = cpu_to_le32(gid);
break;
default:
break;
}
}
+
+   return 0;
 }
 
-void posix_acl_fix_xattr_from_user(void *value, size_t size)
+int
+posix_acl_fix_xattr_from_user(struct user_namespace *target_ns, void *value,
+ size_t size)
 {
-   struct user_namespace *user_ns = current_user_ns();
-   if (user_ns == _user_ns)
-   return;
-   posix_acl_fix_xattr_userns(_user_ns, user_ns, value, size);
+   struct user_namespace *source_ns = current_user_ns();
+   if (source_ns == target_ns)
+   return 0;
+   return posix_acl_fix_xattr_userns(target_ns, source_ns, value, size);
 }
 
-void posix_acl_fix_xattr_to_user(void *value, size_t size)
+int
+posix_acl_fix_xattr_to_user(struct user_namespace *source_ns, void *value,
+   size_t size)
 {
-   struct user_namespace *user_ns = current_user_ns();
-   if (user_ns == _user_ns)
-   return;
-   posix_acl_fix_xattr_userns(user_ns, _user_ns, value, size);
+   struct user_namespace *target_ns = current_user_ns();
+   if (target_ns == source_ns)
+   return 0;
+   return posix_acl_fix_xattr_userns(target_ns, source_ns, value, size);
 }
 
 /*
@@ -782,7 +800,7 @@ posix_acl_xattr_get(const struct xattr_handler *handler,
if (acl == NULL)
return -ENODATA;
 
-   error = posix_acl_to_xattr(_user_ns, acl, value, size);
+   error = posix_acl_to_xattr(dentry->d_sb->s_user_ns, acl, value, size);
posix_acl_release(acl);
 
return error;
@@ -810,7 +828,8 @@ posix_acl_xattr_set(const struct xattr_handler *handler,
return -EPERM;
 
if (value) {
-   acl = posix_acl_from_xattr(_user_ns, value, size);
+   acl = posix_acl_from_xattr(dentry->d_sb->s_user_ns, value,
+  size);

[PATCH v2 07/18] fs: Check for invalid i_uid in may_follow_link()

2015-12-07 Thread Seth Forshee 
Filesystem uids which don't map into a user namespace may result
in inode->i_uid being INVALID_UID. A symlink and its parent
could have different owners in the filesystem can both get
mapped to INVALID_UID, which may result in following a symlink
when this would not have otherwise been permitted when protected
symlinks are enabled.

Add a new helper function, uid_valid_eq(), and use this to
validate that the ids in may_follow_link() are both equal and
valid. Also add an equivalent helper for gids, which is
currently unused.

Signed-off-by: Seth Forshee 
Acked-by: Serge Hallyn 
---
 fs/namei.c |  2 +-
 include/linux/uidgid.h | 10 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/namei.c b/fs/namei.c
index 288e8a74bf88..4ccafd391697 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -902,7 +902,7 @@ static inline int may_follow_link(struct nameidata *nd)
return 0;
 
/* Allowed if parent directory and link owner match. */
-   if (uid_eq(parent->i_uid, inode->i_uid))
+   if (uid_valid_eq(parent->i_uid, inode->i_uid))
return 0;
 
if (nd->flags & LOOKUP_RCU)
diff --git a/include/linux/uidgid.h b/include/linux/uidgid.h
index 03835522dfcb..e09529fe2668 100644
--- a/include/linux/uidgid.h
+++ b/include/linux/uidgid.h
@@ -117,6 +117,16 @@ static inline bool gid_valid(kgid_t gid)
return __kgid_val(gid) != (gid_t) -1;
 }
 
+static inline bool uid_valid_eq(kuid_t left, kuid_t right)
+{
+   return uid_eq(left, right) && uid_valid(left);
+}
+
+static inline bool gid_valid_eq(kgid_t left, kgid_t right)
+{
+   return gid_eq(left, right) && gid_valid(left);
+}
+
 #ifdef CONFIG_USER_NS
 
 extern kuid_t make_kuid(struct user_namespace *from, uid_t uid);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 08/18] cred: Reject inodes with invalid ids in set_create_file_as()

2015-12-07 Thread Seth Forshee 
Using INVALID_[UG]ID for the LSM file creation context doesn't
make sense, so return an error if the inode passed to
set_create_file_as() has an invalid id.

Signed-off-by: Seth Forshee 
Acked-by: Serge Hallyn 
---
 kernel/cred.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/cred.c b/kernel/cred.c
index 71179a09c1d6..ff8606f77d90 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -689,6 +689,8 @@ EXPORT_SYMBOL(set_security_override_from_ctx);
  */
 int set_create_files_as(struct cred *new, struct inode *inode)
 {
+   if (!uid_valid(inode->i_uid) || !gid_valid(inode->i_gid))
+   return -EINVAL;
new->fsuid = inode->i_uid;
new->fsgid = inode->i_gid;
return security_kernel_create_files_as(new, inode);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 05/18] userns: Replace in_userns with current_in_userns

2015-12-07 Thread Seth Forshee 
All current callers of in_userns pass current_user_ns as the
first argument. Simplify by replacing in_userns with
current_in_userns which checks whether current_user_ns is in the
namespace supplied as an argument.

Signed-off-by: Seth Forshee 
Acked-by: James Morris 
Acked-by: Serge Hallyn 
---
 fs/namespace.c | 2 +-
 include/linux/user_namespace.h | 6 ++
 kernel/user_namespace.c| 6 +++---
 security/commoncap.c   | 2 +-
 4 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 2101ce7b96ab..18fc58760aec 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3286,7 +3286,7 @@ bool mnt_may_suid(struct vfsmount *mnt)
 * in other namespaces.
 */
return !(mnt->mnt_flags & MNT_NOSUID) && check_mnt(real_mount(mnt)) &&
-  in_userns(current_user_ns(), mnt->mnt_sb->s_user_ns);
+  current_in_userns(mnt->mnt_sb->s_user_ns);
 }
 
 static struct ns_common *mntns_get(struct task_struct *task)
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index a43faa727124..9217169c64cb 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -72,8 +72,7 @@ extern ssize_t proc_projid_map_write(struct file *, const 
char __user *, size_t,
 extern ssize_t proc_setgroups_write(struct file *, const char __user *, 
size_t, loff_t *);
 extern int proc_setgroups_show(struct seq_file *m, void *v);
 extern bool userns_may_setgroups(const struct user_namespace *ns);
-extern bool in_userns(const struct user_namespace *ns,
- const struct user_namespace *target_ns);
+extern bool current_in_userns(const struct user_namespace *target_ns);
 #else
 
 static inline struct user_namespace *get_user_ns(struct user_namespace *ns)
@@ -103,8 +102,7 @@ static inline bool userns_may_setgroups(const struct 
user_namespace *ns)
return true;
 }
 
-static inline bool in_userns(const struct user_namespace *ns,
-const struct user_namespace *target_ns)
+static inline bool current_in_userns(const struct user_namespace *target_ns)
 {
return true;
 }
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 69fbc377357b..5960edc7e644 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -949,10 +949,10 @@ bool userns_may_setgroups(const struct user_namespace *ns)
  * Returns true if @ns is the same namespace as or a descendant of
  * @target_ns.
  */
-bool in_userns(const struct user_namespace *ns,
-  const struct user_namespace *target_ns)
+bool current_in_userns(const struct user_namespace *target_ns)
 {
-   for (; ns; ns = ns->parent) {
+   struct user_namespace *ns;
+   for (ns = current_user_ns(); ns; ns = ns->parent) {
if (ns == target_ns)
return true;
}
diff --git a/security/commoncap.c b/security/commoncap.c
index 6243aef5860e..2119421613f6 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -450,7 +450,7 @@ static int get_file_caps(struct linux_binprm *bprm, bool 
*effective, bool *has_c
 
if (!mnt_may_suid(bprm->file->f_path.mnt))
return 0;
-   if (!in_userns(current_user_ns(), 
bprm->file->f_path.mnt->mnt_sb->s_user_ns))
+   if (!current_in_userns(bprm->file->f_path.mnt->mnt_sb->s_user_ns))
return 0;
 
rc = get_vfs_caps_from_disk(bprm->file->f_path.dentry, );
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 06/18] Smack: Handle labels consistently in untrusted mounts

2015-12-07 Thread Seth Forshee 
The SMACK64, SMACK64EXEC, and SMACK64MMAP labels are all handled
differently in untrusted mounts. This is confusing and
potentically problematic. Change this to handle them all the same
way that SMACK64 is currently handled; that is, read the label
from disk and check it at use time. For SMACK64 and SMACK64MMAP
access is denied if the label does not match smk_root. To be
consistent with suid, a SMACK64EXEC label which does not match
smk_root will still allow execution of the file but will not run
with the label supplied in the xattr.

Signed-off-by: Seth Forshee 
Acked-by: Casey Schaufler 
---
 security/smack/smack_lsm.c | 29 +++--
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 16cac04214e2..0e555f64ded0 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -921,6 +921,7 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm)
struct inode *inode = file_inode(bprm->file);
struct task_smack *bsp = bprm->cred->security;
struct inode_smack *isp;
+   struct superblock_smack *sbsp;
int rc;
 
if (bprm->cred_prepared)
@@ -930,6 +931,11 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm)
if (isp->smk_task == NULL || isp->smk_task == bsp->smk_task)
return 0;
 
+   sbsp = inode->i_sb->s_security;
+   if ((sbsp->smk_flags & SMK_SB_UNTRUSTED) &&
+   isp->smk_task != sbsp->smk_root)
+   return 0;
+
if (bprm->unsafe & (LSM_UNSAFE_PTRACE | LSM_UNSAFE_PTRACE_CAP)) {
struct task_struct *tracer;
rc = 0;
@@ -1733,6 +1739,7 @@ static int smack_mmap_file(struct file *file,
struct task_smack *tsp;
struct smack_known *okp;
struct inode_smack *isp;
+   struct superblock_smack *sbsp;
int may;
int mmay;
int tmay;
@@ -1744,6 +1751,10 @@ static int smack_mmap_file(struct file *file,
isp = file_inode(file)->i_security;
if (isp->smk_mmap == NULL)
return 0;
+   sbsp = file_inode(file)->i_sb->s_security;
+   if (sbsp->smk_flags & SMK_SB_UNTRUSTED &&
+   isp->smk_mmap != sbsp->smk_root)
+   return -EACCES;
mkp = isp->smk_mmap;
 
tsp = current_security();
@@ -3532,16 +3543,14 @@ static void smack_d_instantiate(struct dentry 
*opt_dentry, struct inode *inode)
if (rc >= 0)
transflag = SMK_INODE_TRANSMUTE;
}
-   if (!(sbsp->smk_flags & SMK_SB_UNTRUSTED)) {
-   /*
-* Don't let the exec or mmap label be "*" or "@".
-*/
-   skp = smk_fetch(XATTR_NAME_SMACKEXEC, inode, dp);
-   if (IS_ERR(skp) || skp == _known_star ||
-   skp == _known_web)
-   skp = NULL;
-   isp->smk_task = skp;
-   }
+   /*
+* Don't let the exec or mmap label be "*" or "@".
+*/
+   skp = smk_fetch(XATTR_NAME_SMACKEXEC, inode, dp);
+   if (IS_ERR(skp) || skp == _known_star ||
+   skp == _known_web)
+   skp = NULL;
+   isp->smk_task = skp;
 
skp = smk_fetch(XATTR_NAME_SMACKMMAP, inode, dp);
if (IS_ERR(skp) || skp == _known_star ||
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 03/18] fs: Treat foreign mounts as nosuid

2015-12-07 Thread Seth Forshee 
From: Andy Lutomirski 

If a process gets access to a mount from a different user
namespace, that process should not be able to take advantage of
setuid files or selinux entrypoints from that filesystem.  Prevent
this by treating mounts from other mount namespaces and those not
owned by current_user_ns() or an ancestor as nosuid.

This will make it safer to allow more complex filesystems to be
mounted in non-root user namespaces.

This does not remove the need for MNT_LOCK_NOSUID.  The setuid,
setgid, and file capability bits can no longer be abused if code in
a user namespace were to clear nosuid on an untrusted filesystem,
but this patch, by itself, is insufficient to protect the system
from abuse of files that, when execed, would increase MAC privilege.

As a more concrete explanation, any task that can manipulate a
vfsmount associated with a given user namespace already has
capabilities in that namespace and all of its descendents.  If they
can cause a malicious setuid, setgid, or file-caps executable to
appear in that mount, then that executable will only allow them to
elevate privileges in exactly the set of namespaces in which they
are already privileges.

On the other hand, if they can cause a malicious executable to
appear with a dangerous MAC label, running it could change the
caller's security context in a way that should not have been
possible, even inside the namespace in which the task is confined.

As a hardening measure, this would have made CVE-2014-5207 much
more difficult to exploit.

Signed-off-by: Andy Lutomirski 
Signed-off-by: Seth Forshee 
Acked-by: James Morris 
Acked-by: Serge Hallyn 
---
 fs/exec.c|  2 +-
 fs/namespace.c   | 13 +
 include/linux/mount.h|  1 +
 security/commoncap.c |  2 +-
 security/selinux/hooks.c |  2 +-
 5 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index b06623a9347f..ea7311d72cc3 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1295,7 +1295,7 @@ static void bprm_fill_uid(struct linux_binprm *bprm)
bprm->cred->euid = current_euid();
bprm->cred->egid = current_egid();
 
-   if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)
+   if (!mnt_may_suid(bprm->file->f_path.mnt))
return;
 
if (task_no_new_privs(current))
diff --git a/fs/namespace.c b/fs/namespace.c
index da70f7c4ece1..2101ce7b96ab 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3276,6 +3276,19 @@ found:
return visible;
 }
 
+bool mnt_may_suid(struct vfsmount *mnt)
+{
+   /*
+* Foreign mounts (accessed via fchdir or through /proc
+* symlinks) are always treated as if they are nosuid.  This
+* prevents namespaces from trusting potentially unsafe
+* suid/sgid bits, file caps, or security labels that originate
+* in other namespaces.
+*/
+   return !(mnt->mnt_flags & MNT_NOSUID) && check_mnt(real_mount(mnt)) &&
+  in_userns(current_user_ns(), mnt->mnt_sb->s_user_ns);
+}
+
 static struct ns_common *mntns_get(struct task_struct *task)
 {
struct ns_common *ns = NULL;
diff --git a/include/linux/mount.h b/include/linux/mount.h
index f822c3c11377..54a594d49733 100644
--- a/include/linux/mount.h
+++ b/include/linux/mount.h
@@ -81,6 +81,7 @@ extern void mntput(struct vfsmount *mnt);
 extern struct vfsmount *mntget(struct vfsmount *mnt);
 extern struct vfsmount *mnt_clone_internal(struct path *path);
 extern int __mnt_is_readonly(struct vfsmount *mnt);
+extern bool mnt_may_suid(struct vfsmount *mnt);
 
 struct path;
 extern struct vfsmount *clone_private_mount(struct path *path);
diff --git a/security/commoncap.c b/security/commoncap.c
index 400aa224b491..6243aef5860e 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -448,7 +448,7 @@ static int get_file_caps(struct linux_binprm *bprm, bool 
*effective, bool *has_c
if (!file_caps_enabled)
return 0;
 
-   if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)
+   if (!mnt_may_suid(bprm->file->f_path.mnt))
return 0;
if (!in_userns(current_user_ns(), 
bprm->file->f_path.mnt->mnt_sb->s_user_ns))
return 0;
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index d0cfaa9f19d0..a5b93df6553f 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2171,7 +2171,7 @@ static int check_nnp_nosuid(const struct linux_binprm 
*bprm,
const struct task_security_struct *new_tsec)
 {
int nnp = (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS);
-   int nosuid = (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID);
+   int nosuid = !mnt_may_suid(bprm->file->f_path.mnt);
int rc;
 
if (!nnp && !nosuid)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in

[PATCH v2 04/18] selinux: Add support for unprivileged mounts from user namespaces

2015-12-07 Thread Seth Forshee 
Security labels from unprivileged mounts in user namespaces must
be ignored. Force superblocks from user namespaces whose labeling
behavior is to use xattrs to use mountpoint labeling instead.
For the mountpoint label, default to converting the current task
context into a form suitable for file objects, but also allow the
policy writer to specify a different label through policy
transition rules.

Pieced together from code snippets provided by Stephen Smalley.

Signed-off-by: Seth Forshee 
Acked-by: Stephen Smalley 
Acked-by: James Morris 
---
 security/selinux/hooks.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index a5b93df6553f..5fedc36dd6b2 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -756,6 +756,28 @@ static int selinux_set_mnt_opts(struct super_block *sb,
goto out;
}
}
+
+   /*
+* If this is a user namespace mount, no contexts are allowed
+* on the command line and security labels must be ignored.
+*/
+   if (sb->s_user_ns != _user_ns) {
+   if (context_sid || fscontext_sid || rootcontext_sid ||
+   defcontext_sid) {
+   rc = -EACCES;
+   goto out;
+   }
+   if (sbsec->behavior == SECURITY_FS_USE_XATTR) {
+   sbsec->behavior = SECURITY_FS_USE_MNTPOINT;
+   rc = security_transition_sid(current_sid(), 
current_sid(),
+SECCLASS_FILE, NULL,
+>mntpoint_sid);
+   if (rc)
+   goto out;
+   }
+   goto out_set_opts;
+   }
+
/* sets the context of the superblock for the fs being mounted. */
if (fscontext_sid) {
rc = may_context_mount_sb_relabel(fscontext_sid, sbsec, cred);
@@ -824,6 +846,7 @@ static int selinux_set_mnt_opts(struct super_block *sb,
sbsec->def_sid = defcontext_sid;
}
 
+out_set_opts:
rc = sb_finish_set_opts(sb);
 out:
mutex_unlock(>lock);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 01/18] block_dev: Support checking inode permissions in lookup_bdev()

2015-12-07 Thread Seth Forshee 
When looking up a block device by path no permission check is
done to verify that the user has access to the block device inode
at the specified path. In some cases it may be necessary to
check permissions towards the inode, such as allowing
unprivileged users to mount block devices in user namespaces.

Add an argument to lookup_bdev() to optionally perform this
permission check. A value of 0 skips the permission check and
behaves the same as before. A non-zero value specifies the mask
of access rights required towards the inode at the specified
path. The check is always skipped if the user has CAP_SYS_ADMIN.

All callers of lookup_bdev() currently pass a mask of 0, so this
patch results in no functional change. Subsequent patches will
add permission checks where appropriate.

Signed-off-by: Seth Forshee 
Acked-by: Serge Hallyn 
---
 drivers/md/bcache/super.c |  2 +-
 drivers/md/dm-table.c |  2 +-
 drivers/mtd/mtdsuper.c|  2 +-
 fs/block_dev.c| 13 ++---
 fs/quota/quota.c  |  2 +-
 include/linux/fs.h|  2 +-
 6 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 679a093a3bf6..e8287b0d1dac 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -1926,7 +1926,7 @@ static ssize_t register_bcache(struct kobject *k, struct 
kobj_attribute *attr,
  sb);
if (IS_ERR(bdev)) {
if (bdev == ERR_PTR(-EBUSY)) {
-   bdev = lookup_bdev(strim(path));
+   bdev = lookup_bdev(strim(path), 0);
mutex_lock(_register_lock);
if (!IS_ERR(bdev) && bch_is_open(bdev))
err = "device already registered";
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 061152a43730..81c60b2495ed 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -380,7 +380,7 @@ int dm_get_device(struct dm_target *ti, const char *path, 
fmode_t mode,
BUG_ON(!t);
 
/* convert the path to a device */
-   bdev = lookup_bdev(path);
+   bdev = lookup_bdev(path, 0);
if (IS_ERR(bdev)) {
dev = name_to_dev_t(path);
if (!dev)
diff --git a/drivers/mtd/mtdsuper.c b/drivers/mtd/mtdsuper.c
index 20c02a3b7417..b5b60e1af31c 100644
--- a/drivers/mtd/mtdsuper.c
+++ b/drivers/mtd/mtdsuper.c
@@ -176,7 +176,7 @@ struct dentry *mount_mtd(struct file_system_type *fs_type, 
int flags,
/* try the old way - the hack where we allowed users to mount
 * /dev/mtdblock$(n) but didn't actually _use_ the blockdev
 */
-   bdev = lookup_bdev(dev_name);
+   bdev = lookup_bdev(dev_name, 0);
if (IS_ERR(bdev)) {
ret = PTR_ERR(bdev);
pr_debug("MTDSB: lookup_bdev() returned %d\n", ret);
diff --git a/fs/block_dev.c b/fs/block_dev.c
index f90d91efa1b4..3ebbde85d898 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1426,7 +1426,7 @@ struct block_device *blkdev_get_by_path(const char *path, 
fmode_t mode,
struct block_device *bdev;
int err;
 
-   bdev = lookup_bdev(path);
+   bdev = lookup_bdev(path, 0);
if (IS_ERR(bdev))
return bdev;
 
@@ -1736,12 +1736,14 @@ EXPORT_SYMBOL(ioctl_by_bdev);
 /**
  * lookup_bdev  - lookup a struct block_device by name
  * @pathname:  special file representing the block device
+ * @mask:  rights to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC)
  *
  * Get a reference to the blockdevice at @pathname in the current
  * namespace if possible and return it.  Return ERR_PTR(error)
- * otherwise.
+ * otherwise.  If @mask is non-zero, check for access rights to the
+ * inode at @pathname.
  */
-struct block_device *lookup_bdev(const char *pathname)
+struct block_device *lookup_bdev(const char *pathname, int mask)
 {
struct block_device *bdev;
struct inode *inode;
@@ -1756,6 +1758,11 @@ struct block_device *lookup_bdev(const char *pathname)
return ERR_PTR(error);
 
inode = d_backing_inode(path.dentry);
+   if (mask != 0 && !capable(CAP_SYS_ADMIN)) {
+   error = __inode_permission(inode, mask);
+   if (error)
+   goto fail;
+   }
error = -ENOTBLK;
if (!S_ISBLK(inode->i_mode))
goto fail;
diff --git a/fs/quota/quota.c b/fs/quota/quota.c
index 3746367098fd..a40eaecbd5cc 100644
--- a/fs/quota/quota.c
+++ b/fs/quota/quota.c
@@ -733,7 +733,7 @@ static struct super_block *quotactl_block(const char __user 
*special, int cmd)
 
if (IS_ERR(tmp))
return ERR_CAST(tmp);
-   bdev = lookup_bdev(tmp->name);
+   bdev = lookup_bdev(tmp->name, 0);
putname(tmp);
if (IS_ERR(bdev))
return ERR_CAST(bdev);
diff --git a/include/linux/fs.h b/include/linux/fs.h

[PATCH v2 00/19] Support fuse mounts in user namespaces

2015-12-07 Thread Seth Forshee 
These patches implement support for mounting filesystems in user
namespaces using fuse. They are based on the patches in the for-testing
branch of
git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git,
but I've rebased them onto 4.4-rc3. I've pushed all of this to:

 git://git.kernel.org/pub/scm/linux/kernel/git/sforshee/linux.git fuse-userns

The patches are organized into three high-level groups.

Patches 1-6 are related to security, adding restrictions for
unprivileged mounts and updating the LSMs as needed. Patches 1-2
(checking inode permissions for block device mounts) may not be strictly
necessary for fuseblk mounts since fuse doesn't do any IO on the block
device in the kernel, but it still seems like a good idea to fail the
mount if the user doesn't have the required permissions for the inode
(though this is a bit misleading with fuse since the mounts are done via
a suid-root helper).

Patches 7-14 update most of the vfs to translate ids correctly and deal
with inodes which may have invalid user/group ids. I've omitted patches
for anything not used by fuse - quota, fs freezing, some helper
functions, etc. - but if these are wanted for the sake of completeness I
can include them.

Patches 15-18 update fuse to deal with mounts from non-init pid and user
namespaces and enable mounting from user namespaces.

Changes since v1:
 - Drop patch for FIBMAP.
 - Use current_in_userns in fuse_allow_current_process.
 - Remove checks for uid/gid validity in fuse. Intead, ids from the
   backing store which do not map into s_user_ns will result in invalid
   ids in the vfs inode. Checks in the vfs will prevent unmappable ids
   from being passed in from above.
 - Update a couple of commit messages to provide more detail about
   changes.

Thanks,
Seth

Andy Lutomirski (1):
  fs: Treat foreign mounts as nosuid

Seth Forshee (17):
  block_dev: Support checking inode permissions in lookup_bdev()
  block_dev: Check permissions towards block device inode when mounting
  selinux: Add support for unprivileged mounts from user namespaces
  userns: Replace in_userns with current_in_userns
  Smack: Handle labels consistently in untrusted mounts
  fs: Check for invalid i_uid in may_follow_link()
  cred: Reject inodes with invalid ids in set_create_file_as()
  fs: Refuse uid/gid changes which don't map into s_user_ns
  fs: Update posix_acl support to handle user namespace mounts
  fs: Ensure the mounter of a filesystem is privileged towards its
inodes
  fs: Don't remove suid for CAP_FSETID in s_user_ns
  fs: Allow superblock owner to access do_remount_sb()
  capabilities: Allow privileged user in s_user_ns to set security.*
xattrs
  fuse: Add support for pid namespaces
  fuse: Support fuse filesystems outside of init_user_ns
  fuse: Restrict allow_other to the superblock's namespace or a
descendant
  fuse: Allow user namespace mounts

 drivers/md/bcache/super.c   |  2 +-
 drivers/md/dm-table.c   |  2 +-
 drivers/mtd/mtdsuper.c  |  2 +-
 fs/attr.c   | 11 +++
 fs/block_dev.c  | 18 +--
 fs/exec.c   |  2 +-
 fs/fuse/cuse.c  |  3 +-
 fs/fuse/dev.c   | 26 
 fs/fuse/dir.c   | 16 +-
 fs/fuse/file.c  | 22 +++---
 fs/fuse/fuse_i.h| 10 +-
 fs/fuse/inode.c | 42 +-
 fs/inode.c  |  6 +++-
 fs/namei.c  |  2 +-
 fs/namespace.c  | 17 +--
 fs/posix_acl.c  | 67 ++---
 fs/quota/quota.c|  2 +-
 fs/xattr.c  | 19 +---
 include/linux/fs.h  |  2 +-
 include/linux/mount.h   |  1 +
 include/linux/posix_acl_xattr.h | 17 ---
 include/linux/uidgid.h  | 10 ++
 include/linux/user_namespace.h  |  6 ++--
 kernel/capability.c | 13 +---
 kernel/cred.c   |  2 ++
 kernel/user_namespace.c |  6 ++--
 security/commoncap.c| 16 ++
 security/selinux/hooks.c| 25 ++-
 security/smack/smack_lsm.c  | 29 --
 29 files changed, 287 insertions(+), 109 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 02/18] block_dev: Check permissions towards block device inode when mounting

2015-12-07 Thread Seth Forshee 
Unprivileged users should not be able to mount block devices when
they lack sufficient privileges towards the block device inode.
Update blkdev_get_by_path() to validate that the user has the
required access to the inode at the specified path. The check
will be skipped for CAP_SYS_ADMIN, so privileged mounts will
continue working as before.

Signed-off-by: Seth Forshee 
Acked-by: Serge Hallyn 
---
 fs/block_dev.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 3ebbde85d898..4fdb6ab59816 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1424,9 +1424,14 @@ struct block_device *blkdev_get_by_path(const char 
*path, fmode_t mode,
void *holder)
 {
struct block_device *bdev;
+   int perm = 0;
int err;
 
-   bdev = lookup_bdev(path, 0);
+   if (mode & FMODE_READ)
+   perm |= MAY_READ;
+   if (mode & FMODE_WRITE)
+   perm |= MAY_WRITE;
+   bdev = lookup_bdev(path, perm);
if (IS_ERR(bdev))
return bdev;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Smack: File receive for sockets

2015-12-07 Thread Casey Schaufler 

Subject: [PATCH] Smack: File receive for sockets

The existing file receive hook checks for access on
the file inode even for UDS. This is not right, as
the inode is not used by Smack to make access checks
for sockets. This change checks for an appropriate
access relationship between the receiving (current)
process and the socket. If the process can't write
to the socket's send label or the socket's receive
label can't write to the process fail.

This will allow the legitimate cases, where the
socket sender and socket receiver can freely communicate.
Only strangly set socket labels should cause a problem.

Signed-off-by: Casey Schaufler 
---
 security/smack/smack_lsm.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index ff81026..b20ef06 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1860,12 +1860,34 @@ static int smack_file_receive(struct file *file)
int may = 0;
struct smk_audit_info ad;
struct inode *inode = file_inode(file);
+   struct socket *sock;
+   struct task_smack *tsp;
+   struct socket_smack *ssp;
 
 	if (unlikely(IS_PRIVATE(inode)))

return 0;
 
 	smk_ad_init(, __func__, LSM_AUDIT_DATA_PATH);

smk_ad_setfield_u_fs_path(, file->f_path);
+
+   if (S_ISSOCK(inode->i_mode)) {
+   sock = SOCKET_I(inode);
+   ssp = sock->sk->sk_security;
+   tsp = current_security();
+   /*
+* If the receiving process can't write to the
+* passed socket or if the passed socket can't
+* write to the receiving process don't accept
+* the passed socket.
+*/
+   rc = smk_access(tsp->smk_task, ssp->smk_out, MAY_WRITE, );
+   rc = smk_bu_file(file, may, rc);
+   if (rc < 0)
+   return rc;
+   rc = smk_access(ssp->smk_in, tsp->smk_task, MAY_WRITE, );
+   rc = smk_bu_file(file, may, rc);
+   return rc;
+   }
/*
 * This code relies on bitmasks.
 */

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] keys, trusted: seal with a policy

2015-12-07 Thread James Morris 
On Mon, 7 Dec 2015, Jarkko Sakkinen wrote:

> On Fri, Nov 20, 2015 at 01:34:35PM +1100, James Morris wrote:
> > On Wed, 18 Nov 2015, Jarkko Sakkinen wrote:
> > 
> > > On Wed, Nov 18, 2015 at 11:21:01AM +1100, James Morris wrote:
> > > > On Tue, 17 Nov 2015, Jarkko Sakkinen wrote:
> > > > 
> > > > >   }
> > > > >   break;
> > > > > + case Opt_policydigest:
> > > > > + if (!tpm2 ||
> > > > > + strlen(args[0].from) != (2 * 
> > > > > opt->digest_len))
> > > > > + return -EINVAL;
> > > > > + kfree(opt->policydigest);
> > > > > + opt->policydigest = kzalloc(opt->digest_len,
> > > > > + GFP_KERNEL);
> > > > 
> > > > Is it correct to kfree opt->policydigest here before allocating it?
> > > 
> > > I think so. The same option might be encountered multiple times.
> > 
> > This would surely signify an error?
> 
> I'm following the semantics of other options. That's why I implemented
> it that way for example:
> 
> keyctl add trusted kmk "new 32 keyhandle=0x8000 keyhandle=0x8000"
> 
> is perfectly OK. I just thought that it'd be more odd if this option
> behaved in a different way...

It seems broken to me -- if you're messing up keyctl commands you might 
want to know about it, but we should remain consistent.


-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html