[PATCH RESEND v2 07/18] fs: Check for invalid i_uid in may_follow_link()
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 <seth.fors...@canonical.com>
Acked-by: Serge Hallyn <serge.hal...@canonical.com>
---
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 RESEND v2 05/18] userns: Replace in_userns with current_in_userns
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 <seth.fors...@canonical.com>
Acked-by: James Morris <james.l.mor...@oracle.com>
Acked-by: Serge Hallyn <serge.hal...@canonical.com>
---
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 RESEND v2 00/19] Support fuse mounts in user namespaces
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 RESEND v2 18/18] fuse: Allow user namespace mounts
Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
---
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 RESEND v2 03/18] fs: Treat foreign mounts as nosuid
From: Andy Lutomirski <l...@amacapital.net>
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 <l...@amacapital.net>
Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
Acked-by: James Morris <james.l.mor...@oracle.com>
Acked-by: Serge Hallyn <serge.hal...@canonical.com>
---
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.
[PATCH RESEND v2 17/18] fuse: Restrict allow_other to the superblock's namespace or a descendant
Unprivileged users are normally restricted from mounting with the allow_other option by system policy, but this could be bypassed for a mount done with user namespace root permissions. In such cases allow_other should not allow users outside the userns to access the mount as doing so would give the unprivileged user the ability to manipulate processes it would otherwise be unable to manipulate. Restrict allow_other to apply to users in the same userns used at mount or a descendant of that namespace. Signed-off-by: Seth Forshee <seth.fors...@canonical.com> Acked-by: Serge Hallyn <serge.hal...@canonical.com> --- fs/fuse/dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index 8fd9fe4dcd43..24e4cdb554f1 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -1015,7 +1015,7 @@ int fuse_allow_current_process(struct fuse_conn *fc) const struct cred *cred; if (fc->flags & FUSE_ALLOW_OTHER) - return 1; + return current_in_userns(fc->user_ns); cred = current_cred(); if (uid_eq(cred->euid, fc->user_id) && -- 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 RESEND v2 08/18] cred: Reject inodes with invalid ids in set_create_file_as()
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 <seth.fors...@canonical.com>
Acked-by: Serge Hallyn <serge.hal...@canonical.com>
---
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 RESEND v2 09/18] fs: Refuse uid/gid changes which don't map into s_user_ns
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 <seth.fors...@canonical.com> Acked-by: Serge Hallyn <serge.hal...@canonical.com> --- 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 RESEND v2 16/18] fuse: Support fuse filesystems outside of init_user_ns
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 <seth.fors...@canonical.com>
---
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 (iva
[PATCH RESEND v2 14/18] capabilities: Allow privileged user in s_user_ns to set security.* xattrs
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 <seth.fors...@canonical.com>
Acked-by: Serge Hallyn <serge.hal...@canonical.com>
---
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
[PATCH RESEND v2 12/18] fs: Don't remove suid for CAP_FSETID in s_user_ns
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 <seth.fors...@canonical.com> Acked-by: Serge Hallyn <serge.hal...@canonical.com> --- 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
[PATCH RESEND v2 04/18] selinux: Add support for unprivileged mounts from user namespaces
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 <seth.fors...@canonical.com>
Acked-by: Stephen Smalley <s...@tycho.nsa.gov>
Acked-by: James Morris <james.l.mor...@oracle.com>
---
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 RESEND v2 15/18] fuse: Add support for pid namespaces
If the userspace process servicing fuse requests is running in
a pid namespace then pids passed via the fuse fd need to be
translated relative to that namespace. Capture the pid namespace
in use when the filesystem is mounted and use this for pid
translation.
Since no use case currently exists for changing namespaces all
translations are done relative to the pid namespace in use when
/dev/fuse is opened. Mounting or /dev/fuse IO from another
namespace will return errors.
Requests from processes whose pid cannot be translated into the
target namespace are not permitted, except for requests
allocated via fuse_get_req_nofail_nopages. For no-fail requests
in.h.pid will be 0 if the pid translation fails.
File locking changes based on previous work done by Eric
Biederman.
Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
Signed-off-by: Miklos Szeredi <mszer...@suse.cz>
---
fs/fuse/dev.c| 19 +++
fs/fuse/file.c | 22 +-
fs/fuse/fuse_i.h | 4
fs/fuse/inode.c | 3 +++
4 files changed, 39 insertions(+), 9 deletions(-)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index ebb5e37455a0..a4f6f30d6d86 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
MODULE_ALIAS_MISCDEV(FUSE_MINOR);
MODULE_ALIAS("devname:fuse");
@@ -124,11 +125,11 @@ static void __fuse_put_request(struct fuse_req *req)
atomic_dec(>count);
}
-static void fuse_req_init_context(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.pid = current->pid;
+ req->in.h.pid = pid_nr_ns(task_pid(current), fc->pid_ns);
}
void fuse_set_initialized(struct fuse_conn *fc)
@@ -181,10 +182,14 @@ static struct fuse_req *__fuse_get_req(struct fuse_conn
*fc, unsigned npages,
goto out;
}
- fuse_req_init_context(req);
+ fuse_req_init_context(fc, req);
__set_bit(FR_WAITING, >flags);
if (for_background)
__set_bit(FR_BACKGROUND, >flags);
+ if (req->in.h.pid == 0) {
+ fuse_put_request(fc, req);
+ return ERR_PTR(-EOVERFLOW);
+ }
return req;
@@ -274,7 +279,7 @@ struct fuse_req *fuse_get_req_nofail_nopages(struct
fuse_conn *fc,
if (!req)
req = get_reserved_req(fc, file);
- fuse_req_init_context(req);
+ fuse_req_init_context(fc, req);
__set_bit(FR_WAITING, >flags);
__clear_bit(FR_BACKGROUND, >flags);
return req;
@@ -1243,6 +1248,9 @@ 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)
+ return -EIO;
+
restart:
spin_lock(>waitq.lock);
err = -EAGAIN;
@@ -1872,6 +1880,9 @@ 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)
+ return -EIO;
+
if (nbytes < sizeof(struct fuse_out_header))
return -EINVAL;
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index e0faf8f2c868..a6c7484c94ee 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2061,7 +2061,8 @@ static int fuse_direct_mmap(struct file *file, struct
vm_area_struct *vma)
return generic_file_mmap(file, vma);
}
-static int convert_fuse_file_lock(const struct fuse_file_lock *ffl,
+static int convert_fuse_file_lock(struct fuse_conn *fc,
+ const struct fuse_file_lock *ffl,
struct file_lock *fl)
{
switch (ffl->type) {
@@ -2076,7 +2077,14 @@ static int convert_fuse_file_lock(const struct
fuse_file_lock *ffl,
fl->fl_start = ffl->start;
fl->fl_end = ffl->end;
- fl->fl_pid = ffl->pid;
+
+ /*
+* Convert pid into the caller's pid namespace. If the pid
+* does not map into the namespace fl_pid will get set to 0.
+*/
+ rcu_read_lock();
+ fl->fl_pid = pid_vnr(find_pid_ns(ffl->pid, fc->pid_ns));
+ rcu_read_unlock();
break;
default:
@@ -2125,7 +2133,7 @@ static int fuse_getlk(struct file *file, struct file_lock
*fl)
args.out.args[0].value =
err = fuse_simple_request(fc, );
if (!err)
- err = convert_fuse_file_lock(, fl);
+ err = convert_fuse_file_lock(fc, , fl);
return err;
}
@@ -2137,7 +2145,8 @@ static int fuse_setlk(struct file *file, struct file_lock
*fl, int flock)
[PATCH RESEND v2 13/18] fs: Allow superblock owner to access do_remount_sb()
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 <seth.fors...@canonical.com> Acked-by: "Eric W. Biederman" <ebied...@xmission.com> Acked-by: Serge Hallyn <serge.hal...@canonical.com> --- 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
[PATCH RESEND v2 02/18] block_dev: Check permissions towards block device inode when mounting
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 <seth.fors...@canonical.com>
Acked-by: Serge Hallyn <serge.hal...@canonical.com>
---
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 RESEND v2 11/18] fs: Ensure the mounter of a filesystem is privileged towards its inodes
The mounter of a filesystem should be privileged towards the
inodes of that filesystem. Extend the checks in
inode_owner_or_capable() and capable_wrt_inode_uidgid() to
permit access by users priviliged in the user namespace of the
inode's superblock.
Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
Acked-by: Serge Hallyn <serge.hal...@canonical.com>
---
fs/inode.c | 3 +++
kernel/capability.c | 13 +
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index 1be5f9003eb3..01c036fe1950 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1962,6 +1962,9 @@ bool inode_owner_or_capable(const struct inode *inode)
ns = current_user_ns();
if (ns_capable(ns, CAP_FOWNER) && kuid_has_mapping(ns, inode->i_uid))
return true;
+
+ if (ns_capable(inode->i_sb->s_user_ns, CAP_FOWNER))
+ return true;
return false;
}
EXPORT_SYMBOL(inode_owner_or_capable);
diff --git a/kernel/capability.c b/kernel/capability.c
index 45432b54d5c6..5137a38a5670 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -437,13 +437,18 @@ EXPORT_SYMBOL(file_ns_capable);
*
* Return true if the current task has the given capability targeted at
* its own user namespace and that the given inode's uid and gid are
- * mapped into the current user namespace.
+ * mapped into the current user namespace, or if the current task has
+ * the capability towards the user namespace of the inode's superblock.
*/
bool capable_wrt_inode_uidgid(const struct inode *inode, int cap)
{
- struct user_namespace *ns = current_user_ns();
+ struct user_namespace *ns;
- return ns_capable(ns, cap) && kuid_has_mapping(ns, inode->i_uid) &&
- kgid_has_mapping(ns, inode->i_gid);
+ ns = current_user_ns();
+ if (ns_capable(ns, cap) && kuid_has_mapping(ns, inode->i_uid) &&
+ kgid_has_mapping(ns, inode->i_gid))
+ return true;
+
+ return ns_capable(inode->i_sb->s_user_ns, cap);
}
EXPORT_SYMBOL(capable_wrt_inode_uidgid);
--
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 13/18] fs: Allow superblock owner to access do_remount_sb()
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 <seth.fors...@canonical.com> Acked-by: "Eric W. Biederman" <ebied...@xmission.com> Acked-by: Serge Hallyn <serge.hal...@canonical.com> --- 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
[PATCH v2 16/18] fuse: Support fuse filesystems outside of init_user_ns
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 <seth.fors...@canonical.com>
---
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 (iva
[PATCH v2 18/18] fuse: Allow user namespace mounts
Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
---
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
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 <seth.fors...@canonical.com>
Acked-by: Serge Hallyn <serge.hal...@canonical.com>
---
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
[PATCH v2 12/18] fs: Don't remove suid for CAP_FSETID in s_user_ns
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 <seth.fors...@canonical.com> Acked-by: Serge Hallyn <serge.hal...@canonical.com> --- 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
[PATCH v2 09/18] fs: Refuse uid/gid changes which don't map into s_user_ns
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 <seth.fors...@canonical.com> Acked-by: Serge Hallyn <serge.hal...@canonical.com> --- 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
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 <seth.fors...@canonical.com>
Acked-by: Serge Hallyn <serge.hal...@canonical.com>
---
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_use
[PATCH v2 07/18] fs: Check for invalid i_uid in may_follow_link()
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 <seth.fors...@canonical.com>
Acked-by: Serge Hallyn <serge.hal...@canonical.com>
---
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()
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 <seth.fors...@canonical.com>
Acked-by: Serge Hallyn <serge.hal...@canonical.com>
---
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
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 <seth.fors...@canonical.com>
Acked-by: James Morris <james.l.mor...@oracle.com>
Acked-by: Serge Hallyn <serge.hal...@canonical.com>
---
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
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 <seth.fors...@canonical.com>
Acked-by: Casey Schaufler <ca...@schaufler-ca.com>
---
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
From: Andy Lutomirski <l...@amacapital.net>
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 <l...@amacapital.net>
Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
Acked-by: James Morris <james.l.mor...@oracle.com>
Acked-by: Serge Hallyn <serge.hal...@canonical.com>
---
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.
[PATCH v2 04/18] selinux: Add support for unprivileged mounts from user namespaces
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 <seth.fors...@canonical.com>
Acked-by: Stephen Smalley <s...@tycho.nsa.gov>
Acked-by: James Morris <james.l.mor...@oracle.com>
---
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()
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 <seth.fors...@canonical.com>
Acked-by: Serge Hallyn <serge.hal...@canonical.com>
---
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(bd
[PATCH v2 00/19] Support fuse mounts in user namespaces
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
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 <seth.fors...@canonical.com>
Acked-by: Serge Hallyn <serge.hal...@canonical.com>
---
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
Re: [PATCH 17/19] fuse: Support fuse filesystems outside of init_user_ns
On Wed, Dec 02, 2015 at 09:40:17AM -0600, Seth Forshee wrote:
> @@ -155,11 +155,22 @@ static ino_t fuse_squash_ino(u64 ino64)
> return ino;
> }
>
> -void fuse_change_attributes_common(struct inode *inode, struct fuse_attr
> *attr,
> -u64 attr_valid)
> +int fuse_change_attributes_common(struct inode *inode, struct fuse_attr
> *attr,
> + u64 attr_valid)
> {
> struct fuse_conn *fc = get_fuse_conn(inode);
> struct fuse_inode *fi = get_fuse_inode(inode);
> + kuid_t uid;
> + kgid_t gid;
> +
> + uid = make_kuid(fc->user_ns, attr->uid);
> + gid = make_kgid(fc->user_ns, attr->gid);
> + if (!uid_valid(uid) || !gid_valid(gid)) {
> + make_bad_inode(inode);
> + return -EIO;
> + }
Eric - I had kind of forgotten about this part until just now, but
previously with these patches we had discussed how to handle ids from
the filesystem that aren't valid in s_user_ns. My intention is to set
the kuids in the inode to invalid, and in these patches I've updated the
vfs so that it should be safe to do that. But at some point I think you
had suggested marking the inodes bad, and I must have added this as a
result. I guess we need to decide which way to go. I favor using invalid
ids so that a user privileged in s_user_ns can still access the inode,
change ownership, etc., but I'm interested to hear your opinion.
Thanks,
Seth
--
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 09/19] fs: Refuse uid/gid changes which don't map into s_user_ns
On Fri, Dec 04, 2015 at 11:27:38AM -0600, Serge E. Hallyn wrote: > On Wed, Dec 02, 2015 at 09:40:09AM -0600, Seth Forshee wrote: > > 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 <seth.fors...@canonical.com> > > Acked-by: Serge Hallyn <serge.hal...@canonical.com> > > ... although i could see root on the host being upset that it can't > assign a uid not valid in the mounter's ns. But it does seem safer. That change wouldn't be representable in the backing store though, and that could lead to unexpected behaviour. It's better to tell root that we can't make the requested change, in my opinion. -- 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 17/19] fuse: Support fuse filesystems outside of init_user_ns
On Fri, Dec 04, 2015 at 02:03:55PM -0600, Serge E. Hallyn wrote:
> Quoting Seth Forshee (seth.fors...@canonical.com):
> > Update fuse to translate uids and gids to/from the user namspace
> > of the process servicing requests on /dev/fuse. Any ids which do
> > not map into the namespace will result in errors. inodes will
> > also be marked bad when unmappable ids are received from the
> > userspace fuse process.
> >
> > Currently no use cases are known for letting the userspace fuse
> > daemon switch namespaces after opening /dev/fuse. Given this
> > fact, and in order to keep the implementation as simple as
> > possible and ease security auditing, the user namespace from
> > which /dev/fuse is opened is used for all id translations. This
> > is required to be the same namespace as s_user_ns to maintain
> > behavior consistent with other filesystems which can be mounted
> > in user namespaces.
> >
> > 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 <seth.fors...@canonical.com>
> > ---
> > fs/fuse/cuse.c | 3 +-
> > fs/fuse/dev.c| 13 +
> > fs/fuse/dir.c| 81 ++---
> > fs/fuse/fuse_i.h | 14 ++
> > fs/fuse/inode.c | 85
> > ++--
> > 5 files changed, 136 insertions(+), 60 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)
>
> Do you think this should be using current_in_user_ns(fc->user_ns) ?
>
> Opening a file, forking (maybe unsharing) then acting on the file is
> pretty standard fare which this would break.
>
> (same for pidns, i guess, as well as for write below)
I'd rather leave it as is. It might be okay, but even if current_user_ns
is a child of fc->user_ns it could end up seeing ids that aren't valid
for it's namespaces, and that might cause issues for filesystems which
aren't expecting it.
But if you have a use case for a process in a child namespace servicing
requests for a mount, I'd be willing to reconsider.
--
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 18/19] fuse: Restrict allow_other to the superblock's namespace or a descendant
On Fri, Dec 04, 2015 at 02:05:41PM -0600, Serge E. Hallyn wrote:
> Quoting Seth Forshee (seth.fors...@canonical.com):
> > Unprivileged users are normally restricted from mounting with the
> > allow_other option by system policy, but this could be bypassed
> > for a mount done with user namespace root permissions. In such
> > cases allow_other should not allow users outside the userns
> > to access the mount as doing so would give the unprivileged user
> > the ability to manipulate processes it would otherwise be unable
> > to manipulate. Restrict allow_other to apply to users in the same
> > userns used at mount or a descendant of that namespace.
> >
> > Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
> > ---
> > fs/fuse/dir.c | 10 --
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> > index f67f4dd86b36..5b8edb1203b8 100644
> > --- a/fs/fuse/dir.c
> > +++ b/fs/fuse/dir.c
> > @@ -1018,8 +1018,14 @@ int fuse_allow_current_process(struct fuse_conn *fc)
> > {
> > const struct cred *cred;
> >
> > - if (fc->flags & FUSE_ALLOW_OTHER)
> > - return 1;
> > + if (fc->flags & FUSE_ALLOW_OTHER) {
> > + struct user_namespace *ns;
> > + for (ns = current_user_ns(); ns; ns = ns->parent) {
> > + if (ns == fc->user_ns)
> > + return 1;
> > + }
>
> use current_in_userns() ?
Yes, it should. I wrote this before I wrote the patch which adds that
function and never thought to go back to change it here.
--
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 15/19] capabilities: Allow privileged user in s_user_ns to set file caps
On Fri, Dec 04, 2015 at 01:42:06PM -0600, Serge E. Hallyn wrote:
> Quoting Seth Forshee (seth.fors...@canonical.com):
> > A privileged user in a super block's s_user_ns is privileged
> > towards that file system and thus should be allowed to set file
> > capabilities. The file capabilities will not be trusted outside
> > of s_user_ns, so an unprivileged user cannot use this to gain
> > privileges in a user namespace where they are not already
> > privileged.
> >
> > Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
> > ---
> > 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;
>
> This, for file capabilities, is fine,
>
> > return 0;
> > }
> >
> > if (!strncmp(name, XATTR_SECURITY_PREFIX,
> > sizeof(XATTR_SECURITY_PREFIX) - 1) &&
> > - !capable(CAP_SYS_ADMIN))
> > + !ns_capable(user_ns, CAP_SYS_ADMIN))
>
> but this is for all other security.*.
>
> It's probably still ok, but let's think about it a sec. MAC like
> selinux or smack should be orthogonal to DAC. Capabilities are the
> same in essence, but the reason they can be treated differently here
> is because capabilties are in fact targated at a user namespace.
> Apparmor namespaces, for instance, are completely orthogonal to user
> namespaces, as are contexts in selinux.
>
> Now, if smack or selinux xattrs are being set then those modules
> should be gating these writes. Booting a kernel without those
> modules should be a challenge for an untrusted user. But such a
> situation could be exploited opportunistically if it were to happen.
>
> The problem with simply not changing this here is that if selinux
> or smack authorizes the xattr write, then commoncap shouldn't be
> denying it.
This is partly the logic behind the change, the other part being that
the user could already insert the xattrs directly into the backing store
so the LSMs must be prepared not to trust them in any case. But the
commit message doesn't explain that, my mistake. And it's a question
worth revisiting.
> I get the feeling we need cooperation among the modules (i.e. "if
> the write is to 'security.$lsm' and $lsm is not loaded, then require
> capable(CAP_SYS_ADMIN), else just allow) But that's not how things are
> structured right now.
>
> Maybe security.ko could grow central logic to 'assign' security.*
> capabilities to specific lsms and gate writes to those if $lsm is not
> loaded.
I don't see any meaningful difference between this case and the case of
inserting them into the backing store before mounting. We can't do
anything to prevent the latter, so LSMs just have to be aware of
unprivileged mounts and handle them with care. Previous patches do this
for SELinux and Smack by adopting a policy that doesn't respect security
labels on disk for these mounts. So I don't think that refusing to set
security.* xattrs for an LSM that isn't loaded really accomplishes
anything.
Then there's the case of setting xattrs for an LSM that is currently
loaded. I think that SELinux and Smack are both going to refuse these
writes, Smack rather directly by seeing that the user lacks global
CAP_MAC_ADMIN and SELinux by virtue of the fact that the previous patch
in this series applies mountpoint labeling to these mounts. As far as I
can tell the other LSMs don't take security policy from xattrs.
So, as far as I can tell, removing the check doesn't create any
vulnerabilities.
But that's not to say it's the right thing to do. After reconsidering
it, I'm inclined to be more conservative and to keep requiring
capable(CAP_SYS_ADMIN) until such time as there's a use case for
allowing a user privileged only in s_user_ns to write these xattrs.
> Does anything break if the second hunk in each fn in this patch is
> not applied?
Not that I'm aware of, no.
Seth
--
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 14/19] fs: Permit FIBMAP for users with CAP_SYS_RAWIO in s_user_ns
On Fri, Dec 04, 2015 at 02:07:36PM -0600, Serge E. Hallyn wrote: > Heh, I was looking over > http://www.gossamer-threads.com/lists/linux/kernel/103611 > a little while ago :) The same question was asked 16 years ago. Apparently > the answer then was that it was easier than fixing the code. So it seems then that either it still isn't safe and so unprivileged users shouldn't be allowed to do it at all, or else it's safe and we should drop the requirement completely. I can't say which is right, unfortunately. > > Quoting Theodore Ts'o (ty...@mit.edu): > > The fact that we need CAP_SYS_RAIO for FIBMAP is pretty silly, given > > that FIEMAP does not require privileges --- and in fact the preferred > > interface. Why not just simply drop the requirement for privileges > > for FIBMAP? > > > > (Seth, Serge, this isn't a real objection to your patch; but the fact > > that FIBMAP requires root has always been a bit silly, and this would > > be a great opportunity to simplify things a bit.) > > > > - Ted > > > > > > On Fri, Dec 04, 2015 at 01:11:43PM -0600, Serge E. Hallyn wrote: > > > Quoting Seth Forshee (seth.fors...@canonical.com): > > > > Signed-off-by: Seth Forshee <seth.fors...@canonical.com> > > > > > > Acked-by: Serge Hallyn <serge.hal...@canonical.com> > > > > > > > --- > > > > fs/ioctl.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/fs/ioctl.c b/fs/ioctl.c > > > > index 5d01d2638ca5..45c371bed7ee 100644 > > > > --- a/fs/ioctl.c > > > > +++ b/fs/ioctl.c > > > > @@ -55,7 +55,7 @@ static int ioctl_fibmap(struct file *filp, int __user > > > > *p) > > > > /* do we support this mess? */ > > > > if (!mapping->a_ops->bmap) > > > > return -EINVAL; > > > > - if (!capable(CAP_SYS_RAWIO)) > > > > + if (!ns_capable(filp->f_inode->i_sb->s_user_ns, CAP_SYS_RAWIO)) > > > > return -EPERM; > > > > res = get_user(block, p); > > > > if (res) > > > > -- > > > > 1.9.1 > > > > > > > > -- > > > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" > > > > in > > > > the body of a message to majord...@vger.kernel.org > > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > Please read the FAQ at http://www.tux.org/lkml/ > > > -- > > > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" > > > in > > > the body of a message to majord...@vger.kernel.org > > > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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 14/19] fs: Permit FIBMAP for users with CAP_SYS_RAWIO in s_user_ns
On Fri, Dec 04, 2015 at 05:43:49PM -0600, Serge E. Hallyn wrote: > On Fri, Dec 04, 2015 at 06:11:52PM -0500, Theodore Ts'o wrote: > > On Fri, Dec 04, 2015 at 02:45:32PM -0600, Seth Forshee wrote: > > > On Fri, Dec 04, 2015 at 02:07:36PM -0600, Serge E. Hallyn wrote: > > > > Heh, I was looking over > > > > http://www.gossamer-threads.com/lists/linux/kernel/103611 > > > > a little while ago :) The same question was asked 16 years ago. > > > > Apparently > > > > the answer then was that it was easier than fixing the code. > > > > > > So it seems then that either it still isn't safe and so unprivileged > > > users shouldn't be allowed to do it at all, or else it's safe and we > > > should drop the requirement completely. I can't say which is right, > > > unfortunately. > > > > It may not have been safe 16 years agoo, but giving invalid arguments > > to FIBMAP is safe for ext4 and ext2. This is the sort of thing that > > tools like trinity should and does test for, so I think it should be > > fine to remove the root check for FIBMAP. > > Seth, can I tempt you into sending a standalone patch to remove that? :) Patch sent. I'll drop this patch in v2. -- 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 06/19] Smack: Handle labels consistently in untrusted mounts
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 <seth.fors...@canonical.com>
Acked-by: Casey Schaufler <ca...@schaufler-ca.com>
---
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 12/19] fs: Don't remove suid for CAP_FSETID in s_user_ns
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 <seth.fors...@canonical.com> --- 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
[PATCH 10/19] fs: Update posix_acl support to handle user namespace mounts
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 <seth.fors...@canonical.com>
---
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,
+
[PATCH 08/19] cred: Reject inodes with invalid ids in set_create_file_as()
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 <seth.fors...@canonical.com>
---
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 09/19] fs: Refuse uid/gid changes which don't map into s_user_ns
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 <seth.fors...@canonical.com> --- 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 13/19] fs: Allow superblock owner to access do_remount_sb()
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 <seth.fors...@canonical.com> Acked-by: "Eric W. Biederman" <ebied...@xmission.com> --- 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
[PATCH 17/19] fuse: Support fuse filesystems outside of init_user_ns
Update fuse to translate uids and gids to/from the user namspace
of the process servicing requests on /dev/fuse. Any ids which do
not map into the namespace will result in errors. inodes will
also be marked bad when unmappable ids are received from the
userspace fuse process.
Currently no use cases are known for letting the userspace fuse
daemon switch namespaces after opening /dev/fuse. Given this
fact, and in order to keep the implementation as simple as
possible and ease security auditing, the user namespace from
which /dev/fuse is opened is used for all id translations. This
is required to be the same namespace as s_user_ns to maintain
behavior consistent with other filesystems which can be mounted
in user namespaces.
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 <seth.fors...@canonical.com>
---
fs/fuse/cuse.c | 3 +-
fs/fuse/dev.c| 13 +
fs/fuse/dir.c| 81 ++---
fs/fuse/fuse_i.h | 14 ++
fs/fuse/inode.c | 85 ++--
5 files changed, 136 insertions(+), 60 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..f67f4dd86b36 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -243,9 +243,12 @@ static int fuse_dentry_revalidate(struct dentry *entry,
unsigned int flags)
if (ret || (outarg.attr.mode ^ inode->i_mode) & S_IFMT)
goto invalid;
- fuse_change_attributes(inode, ,
- entry_attr_timeout(),
- attr_version);
+ ret = fuse_change_attributes(inode, ,
+entry_attr_timeout(),
+attr_version);
+ if (ret)
+ goto invalid;
+
fuse_change_entry_timeout(entry, );
} else if (inode) {
fi = get_fuse_inode(inode);
@@ -319,8 +322,9 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid,
struct qstr *name,
*inode = fuse_iget(sb, outarg->nodeid, outarg->generation,
>attr, entry_attr_timeout(outarg),
attr_version);
- err = -ENOMEM;
- if (!*inode) {
+ if (IS_ERR(*inode)) {
+ err = PTR_ERR(*inode);
+ *inode = NULL;
fuse_queue_forget(fc, forget, outarg->nodeid, 1);
goto out;
[PATCH 18/19] fuse: Restrict allow_other to the superblock's namespace or a descendant
Unprivileged users are normally restricted from mounting with the
allow_other option by system policy, but this could be bypassed
for a mount done with user namespace root permissions. In such
cases allow_other should not allow users outside the userns
to access the mount as doing so would give the unprivileged user
the ability to manipulate processes it would otherwise be unable
to manipulate. Restrict allow_other to apply to users in the same
userns used at mount or a descendant of that namespace.
Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
---
fs/fuse/dir.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index f67f4dd86b36..5b8edb1203b8 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1018,8 +1018,14 @@ int fuse_allow_current_process(struct fuse_conn *fc)
{
const struct cred *cred;
- if (fc->flags & FUSE_ALLOW_OTHER)
- return 1;
+ if (fc->flags & FUSE_ALLOW_OTHER) {
+ struct user_namespace *ns;
+ for (ns = current_user_ns(); ns; ns = ns->parent) {
+ if (ns == fc->user_ns)
+ return 1;
+ }
+ return 0;
+ }
cred = current_cred();
if (uid_eq(cred->euid, fc->user_id) &&
--
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 04/19] selinux: Add support for unprivileged mounts from user namespaces
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 <seth.fors...@canonical.com>
Acked-by: Stephen Smalley <s...@tycho.nsa.gov>
Acked-by: James Morris <james.l.mor...@oracle.com>
---
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 01/19] block_dev: Support checking inode permissions in lookup_bdev()
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 <seth.fors...@canonical.com>
---
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
[PATCH 02/19] block_dev: Check permissions towards block device inode when mounting
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 <seth.fors...@canonical.com>
---
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 05/19] userns: Replace in_userns with current_in_userns
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 <seth.fors...@canonical.com>
Acked-by: James Morris <james.l.mor...@oracle.com>
---
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 11/19] fs: Ensure the mounter of a filesystem is privileged towards its inodes
The mounter of a filesystem should be privileged towards the
inodes of that filesystem. Extend the checks in
inode_owner_or_capable() and capable_wrt_inode_uidgid() to
permit access by users priviliged in the user namespace of the
inode's superblock.
Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
---
fs/inode.c | 3 +++
kernel/capability.c | 13 +
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index 1be5f9003eb3..01c036fe1950 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1962,6 +1962,9 @@ bool inode_owner_or_capable(const struct inode *inode)
ns = current_user_ns();
if (ns_capable(ns, CAP_FOWNER) && kuid_has_mapping(ns, inode->i_uid))
return true;
+
+ if (ns_capable(inode->i_sb->s_user_ns, CAP_FOWNER))
+ return true;
return false;
}
EXPORT_SYMBOL(inode_owner_or_capable);
diff --git a/kernel/capability.c b/kernel/capability.c
index 45432b54d5c6..5137a38a5670 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -437,13 +437,18 @@ EXPORT_SYMBOL(file_ns_capable);
*
* Return true if the current task has the given capability targeted at
* its own user namespace and that the given inode's uid and gid are
- * mapped into the current user namespace.
+ * mapped into the current user namespace, or if the current task has
+ * the capability towards the user namespace of the inode's superblock.
*/
bool capable_wrt_inode_uidgid(const struct inode *inode, int cap)
{
- struct user_namespace *ns = current_user_ns();
+ struct user_namespace *ns;
- return ns_capable(ns, cap) && kuid_has_mapping(ns, inode->i_uid) &&
- kgid_has_mapping(ns, inode->i_gid);
+ ns = current_user_ns();
+ if (ns_capable(ns, cap) && kuid_has_mapping(ns, inode->i_uid) &&
+ kgid_has_mapping(ns, inode->i_gid))
+ return true;
+
+ return ns_capable(inode->i_sb->s_user_ns, cap);
}
EXPORT_SYMBOL(capable_wrt_inode_uidgid);
--
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 v3 0/7] User namespace mount updates
On Wed, Nov 18, 2015 at 02:58:18PM +, Al Viro wrote: > On Wed, Nov 18, 2015 at 08:22:38AM -0600, Seth Forshee wrote: > > > But it still requires the admin set it up that way, no? And aren't > > privileges required to set up those devices in the first place? > > > > I'm not saying that it wouldn't be a good idea to lock down the backing > > stores for those types of devices too, just that it isn't something that > > a regular user could exploit without an admin doing something to > > facilitate it. > > Sigh... If it boils down to "all admins within all containers must be > trusted not to try and break out" (along with "roothole in any container > escalates to kernel-mode code execution on host"), then what the fuck > is the *point* of bothering with containers, userns, etc. in the first > place? If your model is basically "you want isolation, just use kvm", > fine, but where's the place for userns in all that? > > And if you are talking about the _host_ admin, then WTF not have him just > mount what's needed as part of setup and to hell with mounting those > inside the container? Yes, the host admin. I'm not talking about trusting the admin inside the container at all. >From my perspective the idea is essentially to allow mounting with fuse or with ext4 using "mount -o loop ..." within a container. -- 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 v3 0/7] User namespace mount updates
On Wed, Nov 18, 2015 at 02:10:45PM -0500, Theodore Ts'o wrote: > On Tue, Nov 17, 2015 at 12:34:44PM -0600, Seth Forshee wrote: > > On Tue, Nov 17, 2015 at 05:55:06PM +, Al Viro wrote: > > > On Tue, Nov 17, 2015 at 11:25:51AM -0600, Seth Forshee wrote: > > > > > > > Shortly after that I plan to follow with support for ext4. I've been > > > > fuzzing ext4 for a while now and it has held up well, and I'm currently > > > > working on hand-crafted attacks. Ted has commented privately (to others, > > > > not to me personally) that he will fix bugs for such attacks, though I > > > > haven't seen any public comments to that effect. > > > > > > _Static_ attacks, or change-image-under-mounted-fs attacks? > > > > Right now only static attacks, change-image-under-mounted-fs attacks > > will be next. > > I will fix bugs about static attacks. That is, it's interesting to me > that a buggy file system (no matter how it is created), not cause the > kernel to crash --- and privilege escalation attacks tend to be > strongly related to those bugs where we're not doing strong enough > checking. > > Protecting against a malicious user which changes the image under the > file system is a whole other kettle of fish. I am not at all user you > can do this without completely sacrificing performance or making the > code impossible to maintain. So my comments do *not* extend to > protecting against a malicious user who is changing the block device > underneath the kernel. > > If you want to submit patches to make the kernel more robust against > these attacks, I'm certainly willing to look at the patches. But I'm > certainly not guaranteeing that they will go in, and I'm certainly not > promising to fix all vulnerabilities that you might find that are > caused by a malicious block device. Sorry, that's too much buying a > pig in a poke Thanks Ted. My plan right now is to explore the possibility of blocking writes to the backing store from userspace while it's mounted. -- 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 v3 0/7] User namespace mount updates
On Wed, Nov 18, 2015 at 07:23:48AM -0500, Austin S Hemmelgarn wrote: > On 2015-11-17 16:32, Seth Forshee wrote: > >On Tue, Nov 17, 2015 at 03:54:50PM -0500, Austin S Hemmelgarn wrote: > >>On 2015-11-17 14:16, Seth Forshee wrote: > >>>On Tue, Nov 17, 2015 at 02:02:09PM -0500, Austin S Hemmelgarn wrote: > >>>>On 2015-11-17 12:55, Al Viro wrote: > >>>>>On Tue, Nov 17, 2015 at 11:25:51AM -0600, Seth Forshee wrote: > >>>>> > >>>>>>Shortly after that I plan to follow with support for ext4. I've been > >>>>>>fuzzing ext4 for a while now and it has held up well, and I'm currently > >>>>>>working on hand-crafted attacks. Ted has commented privately (to others, > >>>>>>not to me personally) that he will fix bugs for such attacks, though I > >>>>>>haven't seen any public comments to that effect. > >>>>> > >>>>>_Static_ attacks, or change-image-under-mounted-fs attacks? > >>>>To properly protect against attacks on mounted filesystems, we'd > >>>>need some new concept of a userspace immutable file (that is, one > >>>>where nobody can write to it except the kernel, and only the kernel > >>>>can change it between regular access and this new state), and then > >>>>have the kernel set an image (or block device) to this state when a > >>>>filesystem is mounted from it (this introduces all kinds of other > >>>>issues too however, for example stuff that allows an online fsck on > >>>>the device will stop working, as will many un-deletion tools). > >>> > >>>Yeah, Serge and I were just tossing that idea around on irc. If we can > >>>make that work then it's probably the best solution. > >>> > >>> From a naive perspective it seems like all we really have to do is make > >>>the block device inode immutable to userspace when it is mounted. And > >>>the parent block device if it's a partition, which might be a bit > >>>troublesome. We'd have to ensure that writes couldn't happen via any fds > >>>already open when the device was mounted too. > >>> > >>>We'd need some cooperation from the loop driver too I suppose, to make > >>>sure the file backing the loop device is also immutable. > >>> > >> From a completeness perspective, you'd also need to hook into DM, > >>MD, and bcache to handle their backing devices. There's not much we > >>could do about iSCSI/ATAoE/NBD devices, and I think being able to > > > >But really no one would be able to mount any of those without > >intervention from a privileged user anyway. The same is true today of > >loop devices, but I have some patches to change that. > Um, no, depending on how your system is configured, it's fully > possible to mount those as a regular user with no administrative > interaction required. All that needs done is some udev rules (or > something equivalent) to add ACL's to the device nodes allowing > regular users to access them (and there are systems I've seen that > are naive enough to do this). And I can almost assure you that > there will be someone out there who decides to directly expose such > block devices to a user namespace. But it still requires the admin set it up that way, no? And aren't privileges required to set up those devices in the first place? I'm not saying that it wouldn't be a good idea to lock down the backing stores for those types of devices too, just that it isn't something that a regular user could exploit without an admin doing something to facilitate it. -- 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 v3 0/7] User namespace mount updates
On Wed, Nov 18, 2015 at 07:46:53AM -0500, Austin S Hemmelgarn wrote: > On 2015-11-17 17:01, Seth Forshee wrote: > >On Tue, Nov 17, 2015 at 09:05:42PM +, Al Viro wrote: > >>On Tue, Nov 17, 2015 at 03:39:16PM -0500, Austin S Hemmelgarn wrote: > >> > >>>>This is absolutely insane, no matter how much LSM snake oil you slatter on > >>>>the whole thing. All of a sudden you are exposing a huge attack surface > >>>>in the place where it would hurt most and as the consolation we are > >>>>offered > >>>>basically "Ted is willing to fix holes when they are found". > > > >None of the LSM changes are intended to protect against attacks from > >these sorts of attacks at all, so that's irrelevant. > > > >As I said before, I'm also working to find holes up front. That plus a > >commitment from the maintainer seems like a good start at least. What > >bar would you set for a given filesystem to be considered "safe enough"? > > > >>>For the context of static image attacks, anything that's foun > >>>_needs_ to be fixed regardless, and unless you can find some way to > >>>actually prevent attacks on mounted filesystems that doesn't involve > >>>a complete re-write of the filesystem drivers, then there's not much > >>>we can do about it. Yes, unprivileged mounts expose an attack > >>>surface, but so does userspace access to the network stack, and so > >>>do a lot of other features that are considered essential in a modern > >>>general purpose operating system. > >> > >>"X is exposes an attack surface. Y exposes a diferent attack surface. > >>Y is considered important. Therefore X is important enough to implement it" > >> > >>Right... > > > >That isn't the argument he made. I would summarize the argument as, > >"Saying that X exposes an attack surface isn't by itself enough to > >reject X, otherwise we wouldn't expose anything (such as example Y)." > It's good to see someone understood my meaning... > > > >You believe that the attack surface is too large, and that's > >understandable. Is it your opinion that this is a fundamental problem > >for an in-kernel filesystem driver, i.e. that we can never be confident > >enough in an in-kernel filesystem parser to allow untrusted data? If > >not, what would it take to establish a level of confidence that you > >would be comfortable with? > While I can't speak for Al's opinion on this, I would like to point > out my earlier comment: > > It's unfeasible from a practical standpoint to expect filesystems > to > assume that stuff they write might change under them due to > malicious > intent of a third party. So maybe the first requirement is that the user cannot modify the backing store directly while the device is mounted. > We can't protect against everything, not without making the system > completely unusable for general purpose computing. There is always > some degree of trust involved in usage of a computer, the OS has to > trust that the hardware works correctly, the administrator has to > trust the OS to behave correctly, and the users have to trust the > administrator. The administrator also needs to have at least some > trust in the users, otherwise he shouldn't be allowing them to use > the system. > > Perhaps we should have an option that can only be enabled on > creation of the userns that would allow it to use regular kernel > mounts, and without that option we default to only allowing FUSE and > a couple of virtual filesystems (like /proc and devtmpfs). I've considered the idea of something more global like a sysctl, or a per-filesystem knob in sysfs. I guess a per-container knob is another option, I'm not sure what interface we use to expose it though. -- 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 v3 1/7] block_dev: Support checking inode permissions in lookup_bdev()
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 <seth.fors...@canonical.com>
---
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 e76ed003769e..35bb3ea4cbe2 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 26cee058dc02..f1f0aa7214a3 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1396,7 +1396,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;
@@ -1706,12 +1706,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;
@@ -1726,6 +1728,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
[PATCH v3 7/7] Smack: Handle labels consistently in untrusted mounts
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 <seth.fors...@canonical.com>
---
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 621200f86b56..9b7ff781df9a 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -891,6 +891,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)
@@ -900,6 +901,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;
@@ -1703,6 +1709,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;
@@ -1714,6 +1721,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();
@@ -3492,16 +3503,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 v3 5/7] selinux: Add support for unprivileged mounts from user namespaces
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 <seth.fors...@canonical.com>
Acked-by: Stephen Smalley <s...@tycho.nsa.gov>
---
security/selinux/hooks.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index de05207eb665..09be1dc21e58 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 v3 3/7] mtd: Check permissions towards mtd block device inode when mounting
Unprivileged users should not be able to mount mtd block devices
when they lack sufficient privileges towards the block device
inode. Update mount_mtd() 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 <seth.fors...@canonical.com>
---
drivers/mtd/mtdsuper.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/mtdsuper.c b/drivers/mtd/mtdsuper.c
index b5b60e1af31c..5d7e7705fed8 100644
--- a/drivers/mtd/mtdsuper.c
+++ b/drivers/mtd/mtdsuper.c
@@ -125,6 +125,7 @@ struct dentry *mount_mtd(struct file_system_type *fs_type,
int flags,
#ifdef CONFIG_BLOCK
struct block_device *bdev;
int ret, major;
+ int perm;
#endif
int mtdnr;
@@ -176,7 +177,10 @@ 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, 0);
+ perm = MAY_READ;
+ if (!(flags & MS_RDONLY))
+ perm |= MAY_WRITE;
+ bdev = lookup_bdev(dev_name, perm);
if (IS_ERR(bdev)) {
ret = PTR_ERR(bdev);
pr_debug("MTDSB: lookup_bdev() returned %d\n", ret);
--
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 v3 0/7] User namespace mount updates
Hi Eric, Here's another update to my patches for user namespace mounts, based on your for-testing branch. These patches add safeguards necessary to allow unprivileged mounts and update SELinux and Smack to safely handle device-backed mounts from unprivileged users. The v2 posting received very little in the way of feedback, so changes are minimal. I've made a trivial style change to the Smack changes at Casey's request, and I've added Stephen's ack for the SELinux changes. Thanks, Seth Andy Lutomirski (1): fs: Treat foreign mounts as nosuid Seth Forshee (6): block_dev: Support checking inode permissions in lookup_bdev() block_dev: Check permissions towards block device inode when mounting mtd: Check permissions towards mtd 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 drivers/md/bcache/super.c | 2 +- drivers/md/dm-table.c | 2 +- drivers/mtd/mtdsuper.c | 6 +- fs/block_dev.c | 18 +++--- fs/exec.c | 2 +- fs/namespace.c | 13 + fs/quota/quota.c | 2 +- include/linux/fs.h | 2 +- include/linux/mount.h | 1 + include/linux/user_namespace.h | 6 ++ kernel/user_namespace.c| 6 +++--- security/commoncap.c | 4 ++-- security/selinux/hooks.c | 25 - security/smack/smack_lsm.c | 29 +++-- 14 files changed, 89 insertions(+), 29 deletions(-) -- 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 v3 4/7] fs: Treat foreign mounts as nosuid
From: Andy Lutomirski <l...@amacapital.net>
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 <l...@amacapital.net>
Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
---
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 e4369d86e588..de05207eb665 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
[PATCH v3 6/7] userns: Replace in_userns with current_in_userns
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 <seth.fors...@canonical.com>
---
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
Re: [PATCH v3 0/7] User namespace mount updates
On Tue, Nov 17, 2015 at 05:55:06PM +, Al Viro wrote: > On Tue, Nov 17, 2015 at 11:25:51AM -0600, Seth Forshee wrote: > > > Shortly after that I plan to follow with support for ext4. I've been > > fuzzing ext4 for a while now and it has held up well, and I'm currently > > working on hand-crafted attacks. Ted has commented privately (to others, > > not to me personally) that he will fix bugs for such attacks, though I > > haven't seen any public comments to that effect. > > _Static_ attacks, or change-image-under-mounted-fs attacks? Right now only static attacks, change-image-under-mounted-fs attacks will be next. -- 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 v3 0/7] User namespace mount updates
On Tue, Nov 17, 2015 at 02:02:09PM -0500, Austin S Hemmelgarn wrote: > On 2015-11-17 12:55, Al Viro wrote: > >On Tue, Nov 17, 2015 at 11:25:51AM -0600, Seth Forshee wrote: > > > >>Shortly after that I plan to follow with support for ext4. I've been > >>fuzzing ext4 for a while now and it has held up well, and I'm currently > >>working on hand-crafted attacks. Ted has commented privately (to others, > >>not to me personally) that he will fix bugs for such attacks, though I > >>haven't seen any public comments to that effect. > > > >_Static_ attacks, or change-image-under-mounted-fs attacks? > To properly protect against attacks on mounted filesystems, we'd > need some new concept of a userspace immutable file (that is, one > where nobody can write to it except the kernel, and only the kernel > can change it between regular access and this new state), and then > have the kernel set an image (or block device) to this state when a > filesystem is mounted from it (this introduces all kinds of other > issues too however, for example stuff that allows an online fsck on > the device will stop working, as will many un-deletion tools). Yeah, Serge and I were just tossing that idea around on irc. If we can make that work then it's probably the best solution. >From a naive perspective it seems like all we really have to do is make the block device inode immutable to userspace when it is mounted. And the parent block device if it's a partition, which might be a bit troublesome. We'd have to ensure that writes couldn't happen via any fds already open when the device was mounted too. We'd need some cooperation from the loop driver too I suppose, to make sure the file backing the loop device is also immutable. -- 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 v3 7/7] Smack: Handle labels consistently in untrusted mounts
On Wed, Nov 18, 2015 at 11:12:51AM +1100, James Morris wrote: > On Tue, 17 Nov 2015, Seth Forshee wrote: > > > + sbsp = inode->i_sb->s_security; > > + if ((sbsp->smk_flags & SMK_SB_UNTRUSTED) && > > Where is SMK_SB_UNTRUSTED defined? > > I can't see it in this patch series, mainline or security next. Eric's already applied a few of my patches to a branch in his tree, including the one that defines this. See the for-testing branch of git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git, specifically https://git.kernel.org/cgit/linux/kernel/git/ebiederm/user-namespace.git/commit/?id=36979551f2345d4e20714c1fc53d6a15c87f8b64. Thanks, Seth -- 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 v3 0/7] User namespace mount updates
On Tue, Nov 17, 2015 at 09:05:42PM +, Al Viro wrote: > On Tue, Nov 17, 2015 at 03:39:16PM -0500, Austin S Hemmelgarn wrote: > > > >This is absolutely insane, no matter how much LSM snake oil you slatter on > > >the whole thing. All of a sudden you are exposing a huge attack surface > > >in the place where it would hurt most and as the consolation we are offered > > >basically "Ted is willing to fix holes when they are found". None of the LSM changes are intended to protect against attacks from these sorts of attacks at all, so that's irrelevant. As I said before, I'm also working to find holes up front. That plus a commitment from the maintainer seems like a good start at least. What bar would you set for a given filesystem to be considered "safe enough"? > > For the context of static image attacks, anything that's found > > _needs_ to be fixed regardless, and unless you can find some way to > > actually prevent attacks on mounted filesystems that doesn't involve > > a complete re-write of the filesystem drivers, then there's not much > > we can do about it. Yes, unprivileged mounts expose an attack > > surface, but so does userspace access to the network stack, and so > > do a lot of other features that are considered essential in a modern > > general purpose operating system. > > "X is exposes an attack surface. Y exposes a diferent attack surface. > Y is considered important. Therefore X is important enough to implement it" > > Right... That isn't the argument he made. I would summarize the argument as, "Saying that X exposes an attack surface isn't by itself enough to reject X, otherwise we wouldn't expose anything (such as example Y)." You believe that the attack surface is too large, and that's understandable. Is it your opinion that this is a fundamental problem for an in-kernel filesystem driver, i.e. that we can never be confident enough in an in-kernel filesystem parser to allow untrusted data? If not, what would it take to establish a level of confidence that you would be comfortable with? -- 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 v3 0/7] User namespace mount updates
On Tue, Nov 17, 2015 at 03:54:50PM -0500, Austin S Hemmelgarn wrote: > On 2015-11-17 14:16, Seth Forshee wrote: > >On Tue, Nov 17, 2015 at 02:02:09PM -0500, Austin S Hemmelgarn wrote: > >>On 2015-11-17 12:55, Al Viro wrote: > >>>On Tue, Nov 17, 2015 at 11:25:51AM -0600, Seth Forshee wrote: > >>> > >>>>Shortly after that I plan to follow with support for ext4. I've been > >>>>fuzzing ext4 for a while now and it has held up well, and I'm currently > >>>>working on hand-crafted attacks. Ted has commented privately (to others, > >>>>not to me personally) that he will fix bugs for such attacks, though I > >>>>haven't seen any public comments to that effect. > >>> > >>>_Static_ attacks, or change-image-under-mounted-fs attacks? > >>To properly protect against attacks on mounted filesystems, we'd > >>need some new concept of a userspace immutable file (that is, one > >>where nobody can write to it except the kernel, and only the kernel > >>can change it between regular access and this new state), and then > >>have the kernel set an image (or block device) to this state when a > >>filesystem is mounted from it (this introduces all kinds of other > >>issues too however, for example stuff that allows an online fsck on > >>the device will stop working, as will many un-deletion tools). > > > >Yeah, Serge and I were just tossing that idea around on irc. If we can > >make that work then it's probably the best solution. > > > > From a naive perspective it seems like all we really have to do is make > >the block device inode immutable to userspace when it is mounted. And > >the parent block device if it's a partition, which might be a bit > >troublesome. We'd have to ensure that writes couldn't happen via any fds > >already open when the device was mounted too. > > > >We'd need some cooperation from the loop driver too I suppose, to make > >sure the file backing the loop device is also immutable. > > > From a completeness perspective, you'd also need to hook into DM, > MD, and bcache to handle their backing devices. There's not much we > could do about iSCSI/ATAoE/NBD devices, and I think being able to But really no one would be able to mount any of those without intervention from a privileged user anyway. The same is true today of loop devices, but I have some patches to change that. > restrict stuff calling mount from a userns to only be able to use > FUSE would still be useful (FWIW, GRUB2 has a tool to use FUSE for > testing it's own filesystem drivers, which I use regularly when I > just need a read-only mount). Agreed, fuse alone is very useful, though there is a performance penalty. -- 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 v2 7/7] Smack: Handle labels consistently in untrusted mounts
On Wed, Oct 14, 2015 at 10:46:47PM -0700, Casey Schaufler wrote: > On 10/13/2015 10:04 AM, Seth Forshee wrote: > > 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 <seth.fors...@canonical.com> > > Aside from the one comment below (which I can be talked out of) > this looks fine. > > > --- > > security/smack/smack_lsm.c | 28 ++-- > > 1 file changed, 18 insertions(+), 10 deletions(-) > > > > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > > index 621200f86b56..bee0b2652bf4 100644 > > --- a/security/smack/smack_lsm.c > > +++ b/security/smack/smack_lsm.c > > @@ -891,6 +891,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) > > @@ -900,6 +901,10 @@ 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) > > Call me old fashioned, but how about > > if ((sbsp->smk_flags & SMK_SB_UNTRUSTED) && isp->smk_task != > sbsp->smk_root) > > naked '&'s give me the willies. That's fine by me. Seth -- 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 4/7] fs: Treat foreign mounts as nosuid
From: Andy Lutomirski <l...@amacapital.net>
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 <l...@amacapital.net>
Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
---
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 e4369d86e588..de05207eb665 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
[PATCH v2 3/7] mtd: Check permissions towards mtd block device inode when mounting
Unprivileged users should not be able to mount mtd block devices
when they lack sufficient privileges towards the block device
inode. Update mount_mtd() 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 <seth.fors...@canonical.com>
---
drivers/mtd/mtdsuper.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/mtdsuper.c b/drivers/mtd/mtdsuper.c
index b5b60e1af31c..5d7e7705fed8 100644
--- a/drivers/mtd/mtdsuper.c
+++ b/drivers/mtd/mtdsuper.c
@@ -125,6 +125,7 @@ struct dentry *mount_mtd(struct file_system_type *fs_type,
int flags,
#ifdef CONFIG_BLOCK
struct block_device *bdev;
int ret, major;
+ int perm;
#endif
int mtdnr;
@@ -176,7 +177,10 @@ 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, 0);
+ perm = MAY_READ;
+ if (!(flags & MS_RDONLY))
+ perm |= MAY_WRITE;
+ bdev = lookup_bdev(dev_name, perm);
if (IS_ERR(bdev)) {
ret = PTR_ERR(bdev);
pr_debug("MTDSB: lookup_bdev() returned %d\n", ret);
--
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 5/7] selinux: Add support for unprivileged mounts from user namespaces
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 <seth.fors...@canonical.com>
---
security/selinux/hooks.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index de05207eb665..09be1dc21e58 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 6/7] userns: Replace in_userns with current_in_userns
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 <seth.fors...@canonical.com>
---
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
Re: [PATCH 1/5] fs: Verify access of user towards block device file when mounting
On Thu, Oct 01, 2015 at 09:41:37AM -0500, Seth Forshee wrote:
> On Thu, Oct 01, 2015 at 09:40:52AM -0400, Mike Snitzer wrote:
> > On Thu, Oct 01 2015 at 8:55am -0400,
> > Seth Forshee <seth.fors...@canonical.com> wrote:
> >
> > > On Wed, Sep 30, 2015 at 07:42:15PM -0400, Mike Snitzer wrote:
> > > > On Wed, Sep 30 2015 at 4:15pm -0400,
> > > > Seth Forshee <seth.fors...@canonical.com> wrote:
> > > >
> > > > > When mounting a filesystem on a block device there is currently
> > > > > no verification that the user has appropriate access to the
> > > > > device file passed to mount. This has not been an issue so far
> > > > > since the user in question has always been root, but this must
> > > > > be changed before allowing unprivileged users to mount in user
> > > > > namespaces.
> > > > >
> > > > > To fix this, add an argument to lookup_bdev() to specify the
> > > > > required permissions. If the mask of permissions is zero, or
> > > > > if the user has CAP_SYS_ADMIN, the permission check is skipped,
> > > > > otherwise the lookup fails if the user does not have the
> > > > > specified access rights for the inode at the supplied path.
> > > > >
> > > > > Callers associated with mounting are updated to pass permission
> > > > > masks to lookup_bdev() so that these mounts will fail for an
> > > > > unprivileged user who lacks permissions for the block device
> > > > > inode. All other callers pass 0 to maintain their current
> > > > > behaviors.
> > > > >
> > > > > Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
> > > > > ---
> > > > > drivers/md/bcache/super.c | 2 +-
> > > > > drivers/md/dm-table.c | 2 +-
> > > > > drivers/mtd/mtdsuper.c| 6 +-
> > > > > fs/block_dev.c| 18 +++---
> > > > > fs/quota/quota.c | 2 +-
> > > > > include/linux/fs.h| 2 +-
> > > > > 6 files changed, 24 insertions(+), 8 deletions(-)
> > > > >
> > > > > diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> > > > > index e76ed003769e..35bb3ea4cbe2 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)
> > > >
> > > > Given dm_get_device() is passed @mode why not have it do something like
> > > > you did in blkdev_get_by_path()? e.g.:
> > >
> > > I only dealt with code related to mounting in this patch since that's
> > > what I'm working on. I have it on my TODO list to consider converting
> > > other callers of lookup_bdev. But if you're sure doing so makes sense
> > > for dm_get_device and that it won't cause regressions then I could add a
> > > patch for it.
> >
> > OK, dm_get_device() is called in DM device activation path (by tools
> > like lvm2).
> >
> > After lookup_bdev() it goes on to call blkdev_get_by_dev() with this
> > call chain:
> > dm_get_device -> dm_get_table_device -> open_table_device ->
> > blkdev_get_by_dev
> >
> > Not immediately clear to me why we'd need to augment blkdev_get_by_dev()
> > to do this checking also.
> >
> > However, thinking further: In a device stack (e.g. dm/lvm2, md, etc)
> > new virtual block devices are created that layer ontop of the
> > traditional block devices. This level of indirection may cause your
> > lookup_bdev() check to go on to succeed (if access constraints were not
> > established on the upper level dm or md device?). I'm just thinking
> > outloud here: but have you verified your changes work as intended on
> > devices created with either lvm2 or mdadm?
> >
> > What layer establishes access rights to historically root-only
> > priviledged block devices? Is it user namespaces?
>
> I'm going to star
Re: [PATCH 1/5] fs: Verify access of user towards block device file when mounting
On Wed, Sep 30, 2015 at 07:42:15PM -0400, Mike Snitzer wrote:
> On Wed, Sep 30 2015 at 4:15pm -0400,
> Seth Forshee <seth.fors...@canonical.com> wrote:
>
> > When mounting a filesystem on a block device there is currently
> > no verification that the user has appropriate access to the
> > device file passed to mount. This has not been an issue so far
> > since the user in question has always been root, but this must
> > be changed before allowing unprivileged users to mount in user
> > namespaces.
> >
> > To fix this, add an argument to lookup_bdev() to specify the
> > required permissions. If the mask of permissions is zero, or
> > if the user has CAP_SYS_ADMIN, the permission check is skipped,
> > otherwise the lookup fails if the user does not have the
> > specified access rights for the inode at the supplied path.
> >
> > Callers associated with mounting are updated to pass permission
> > masks to lookup_bdev() so that these mounts will fail for an
> > unprivileged user who lacks permissions for the block device
> > inode. All other callers pass 0 to maintain their current
> > behaviors.
> >
> > Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
> > ---
> > drivers/md/bcache/super.c | 2 +-
> > drivers/md/dm-table.c | 2 +-
> > drivers/mtd/mtdsuper.c| 6 +-
> > fs/block_dev.c| 18 +++---
> > fs/quota/quota.c | 2 +-
> > include/linux/fs.h| 2 +-
> > 6 files changed, 24 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> > index e76ed003769e..35bb3ea4cbe2 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)
>
> Given dm_get_device() is passed @mode why not have it do something like
> you did in blkdev_get_by_path()? e.g.:
I only dealt with code related to mounting in this patch since that's
what I'm working on. I have it on my TODO list to consider converting
other callers of lookup_bdev. But if you're sure doing so makes sense
for dm_get_device and that it won't cause regressions then I could add a
patch for it.
Thanks,
Seth
--
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/5] fs: Verify access of user towards block device file when mounting
On Thu, Oct 01, 2015 at 10:40:08AM -0500, Eric W. Biederman wrote: > Seth Forshee <seth.fors...@canonical.com> writes: > > > When mounting a filesystem on a block device there is currently > > no verification that the user has appropriate access to the > > device file passed to mount. This has not been an issue so far > > since the user in question has always been root, but this must > > be changed before allowing unprivileged users to mount in user > > namespaces. > > > > To fix this, add an argument to lookup_bdev() to specify the > > required permissions. If the mask of permissions is zero, or > > if the user has CAP_SYS_ADMIN, the permission check is skipped, > > otherwise the lookup fails if the user does not have the > > specified access rights for the inode at the supplied path. > > > > Callers associated with mounting are updated to pass permission > > masks to lookup_bdev() so that these mounts will fail for an > > unprivileged user who lacks permissions for the block device > > inode. All other callers pass 0 to maintain their current > > behaviors. > > > > Seth can you split this patch? > > One patch to add an argument to lookup_bdev, > and then for each kind of callsite a follow-on patch (if we are ready > for that). > > That will separate the logical changes and make things easier to track > via bisect and more importantly easier to review things. Sure, I'll do that. Seth -- 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 1/5] fs: Verify access of user towards block device file when mounting
When mounting a filesystem on a block device there is currently
no verification that the user has appropriate access to the
device file passed to mount. This has not been an issue so far
since the user in question has always been root, but this must
be changed before allowing unprivileged users to mount in user
namespaces.
To fix this, add an argument to lookup_bdev() to specify the
required permissions. If the mask of permissions is zero, or
if the user has CAP_SYS_ADMIN, the permission check is skipped,
otherwise the lookup fails if the user does not have the
specified access rights for the inode at the supplied path.
Callers associated with mounting are updated to pass permission
masks to lookup_bdev() so that these mounts will fail for an
unprivileged user who lacks permissions for the block device
inode. All other callers pass 0 to maintain their current
behaviors.
Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
---
drivers/md/bcache/super.c | 2 +-
drivers/md/dm-table.c | 2 +-
drivers/mtd/mtdsuper.c| 6 +-
fs/block_dev.c| 18 +++---
fs/quota/quota.c | 2 +-
include/linux/fs.h| 2 +-
6 files changed, 24 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 e76ed003769e..35bb3ea4cbe2 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..5d7e7705fed8 100644
--- a/drivers/mtd/mtdsuper.c
+++ b/drivers/mtd/mtdsuper.c
@@ -125,6 +125,7 @@ struct dentry *mount_mtd(struct file_system_type *fs_type,
int flags,
#ifdef CONFIG_BLOCK
struct block_device *bdev;
int ret, major;
+ int perm;
#endif
int mtdnr;
@@ -176,7 +177,10 @@ 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);
+ perm = MAY_READ;
+ if (!(flags & MS_RDONLY))
+ perm |= MAY_WRITE;
+ bdev = lookup_bdev(dev_name, perm);
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 26cee058dc02..54d94cd64577 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1394,9 +1394,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);
+ 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;
@@ -1706,12 +1711,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;
@@ -1726,6 +1733,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)
+
[PATCH 2/5] fs: Treat foreign mounts as nosuid
From: Andy Lutomirski <l...@amacapital.net>
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 <l...@amacapital.net>
Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
---
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 e4369d86e588..de05207eb665 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
[PATCH 3/5] selinux: Add support for unprivileged mounts from user namespaces
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 <seth.fors...@canonical.com>
---
security/selinux/hooks.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index de05207eb665..09be1dc21e58 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 4/5] userns: Replace in_userns with current_in_userns
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 <seth.fors...@canonical.com>
---
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 0/5] User namespace mount updates
Hi Eric, Here's a batch of updates for the unprivileged user namespace mount patches based on your feedback. I think everything you mentioned should be addressed here. These are now based on your for-testing branch. Updates include: - Fix for incorrect use of flags argument in mount_mtd. - Eliminate lookup_bdev_perm and instead add an access mode argument to lookup_bdev. - Use __inode_permission instead of inode_permission when checking for rights towards a block device inode. - Add a patch replacing in_user_ns with current_in_user_ns. - Add a patch to handle Smack security labels consistently. Thanks, Seth Andy Lutomirski (1): fs: Treat foreign mounts as nosuid Seth Forshee (4): fs: Verify access of user towards block device file 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 drivers/md/bcache/super.c | 2 +- drivers/md/dm-table.c | 2 +- drivers/mtd/mtdsuper.c | 6 +- fs/block_dev.c | 18 +++--- fs/exec.c | 2 +- fs/namespace.c | 13 + fs/quota/quota.c | 2 +- include/linux/fs.h | 2 +- include/linux/mount.h | 1 + include/linux/user_namespace.h | 6 ++ kernel/user_namespace.c| 6 +++--- security/commoncap.c | 4 ++-- security/selinux/hooks.c | 25 - security/smack/smack_lsm.c | 28 ++-- 14 files changed, 88 insertions(+), 29 deletions(-) -- 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 5/5] Smack: Handle labels consistently in untrusted mounts
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 <seth.fors...@canonical.com>
---
security/smack/smack_lsm.c | 28 ++--
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 621200f86b56..bee0b2652bf4 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -891,6 +891,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)
@@ -900,6 +901,10 @@ 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;
@@ -1703,6 +1708,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;
@@ -1714,6 +1720,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();
@@ -3492,16 +3502,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
