On 9/16/2015 1:02 PM, Seth Forshee wrote:
> Security labels from unprivileged mounts cannot be trusted.
> Ideally for these mounts we would assign the objects in the
> filesystem the same label as the inode for the backing device
> passed to mount. Unfortunately it's currently impossible to
> determine which inode this is from the LSM mount hooks, so we
> settle for the label of the process doing the mount.
>
> This label is assigned to s_root, and also to smk_default to
> ensure that new inodes receive this label. The transmute property
> is also set on s_root to make this behavior more explicit, even
> though it is technically not necessary.
>
> If a filesystem has existing security labels, access to inodes is
> permitted if the label is the same as smk_root, otherwise access
> is denied. The SMACK64EXEC xattr is completely ignored.
>
> Explicit setting of security labels continues to require
> CAP_MAC_ADMIN in init_user_ns.
>
> Altogether, this ensures that filesystem objects are not
> accessible to subjects which cannot already access the backing
> store, that MAC is not violated for any objects in the fileystem
> which are already labeled, and that a user cannot use an
> unprivileged mount to gain elevated MAC privileges.
>
> sysfs, tmpfs, and ramfs are already mountable from user
> namespaces and support security labels. We can't rule out the
> possibility that these filesystems may already be used in mounts
> from user namespaces with security lables set from the init
> namespace, so failing to trust lables in these filesystems may
> introduce regressions. It is safe to trust labels from these
> filesystems, since the unprivileged user does not control the
> backing store and thus cannot supply security labels, so an
> explicit exception is made to trust labels from these
> filesystems.
>
> Signed-off-by: Seth Forshee <seth.fors...@canonical.com>
One coding comment below, otherwise looking good.
> ---
> security/smack/smack.h | 6 ++++++
> security/smack/smack_lsm.c | 35 +++++++++++++++++++++++++++--------
> 2 files changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index fff0c612bbb7..070223960a2c 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -91,8 +91,14 @@ struct superblock_smack {
> struct smack_known *smk_hat;
> struct smack_known *smk_default;
> int smk_initialized;
> + int smk_flags;
How about deleting smk_initialized and using a bit
in smk_flags. A whole int for each seems excessive.
The smk_initialized field is only used in two places,
both in smack_set_mnt_opts.
> };
>
> +/*
> + * Superblock flags
> + */
> +#define SMK_SB_UNTRUSTED 0x01
+ #define SMK_SB_INITIALIZED 0x02
> +
> struct socket_smack {
> struct smack_known *smk_out; /* outbound label */
> struct smack_known *smk_in; /* inbound label */
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 996c88956438..cdfd67b61534 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -793,6 +793,17 @@ static int smack_set_mnt_opts(struct super_block *sb,
> skp = smk_of_current();
> sp->smk_root = skp;
> sp->smk_default = skp;
> + /*
> + * For a handful of fs types with no user-controlled
> + * backing store it's okay to trust security labels
> + * in the filesystem. The rest are untrusted.
> + */
> + if (sb->s_user_ns != &init_user_ns &&
> + sb->s_magic != SYSFS_MAGIC && sb->s_magic != TMPFS_MAGIC &&
> + sb->s_magic != RAMFS_MAGIC) {
> + transmute = 1;
> + sp->smk_flags |= SMK_SB_UNTRUSTED;
> + }
> }
>
> /*
> @@ -1175,6 +1186,7 @@ static int smack_inode_rename(struct inode *old_inode,
> */
> static int smack_inode_permission(struct inode *inode, int mask)
> {
> + struct superblock_smack *sbsp = inode->i_sb->s_security;
> struct smk_audit_info ad;
> int no_block = mask & MAY_NOT_BLOCK;
> int rc;
> @@ -1186,6 +1198,11 @@ static int smack_inode_permission(struct inode *inode,
> int mask)
> if (mask == 0)
> return 0;
>
> + if (sbsp->smk_flags & SMK_SB_UNTRUSTED) {
> + if (smk_of_inode(inode) != sbsp->smk_root)
> + return -EACCES;
> + }
> +
> /* May be droppable after audit */
> if (no_block)
> return -ECHILD;
> @@ -3475,14 +3492,16 @@ static void smack_d_instantiate(struct dentry
> *opt_dentry, struct inode *inode)
> if (rc >= 0)
> transflag = SMK_INODE_TRANSMUTE;
> }
> - /*
> - * Don't let the exec or mmap label be "*" or "@".
> - */
> - skp = smk_fetch(XATTR_NAME_SMACKEXEC, inode, dp);
> - if (IS_ERR(skp) || skp == &smack_known_star ||
> - skp == &smack_known_web)
> - skp = NULL;
> - isp->smk_task = skp;
> + 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 == &smack_known_star ||
> + skp == &smack_known_web)
> + skp = NULL;
> + isp->smk_task = skp;
> + }
>
> skp = smk_fetch(XATTR_NAME_SMACKMMAP, inode, dp);
> if (IS_ERR(skp) || skp == &smack_known_star ||
--
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/
