linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Brauner <christian.brauner@ubuntu.com>
To: Stefan Berger <stefanb@linux.vnet.ibm.com>
Cc: linux-integrity@vger.kernel.org, zohar@linux.ibm.com,
	serge@hallyn.com, containers@lists.linux.dev,
	dmitry.kasatkin@gmail.com, ebiederm@xmission.com,
	krzysztof.struczynski@huawei.com, roberto.sassu@huawei.com,
	mpeters@redhat.com, lhinds@redhat.com, lsturman@redhat.com,
	puiterwi@redhat.com, jejb@linux.ibm.com, jamjoom@us.ibm.com,
	linux-kernel@vger.kernel.org, paul@paul-moore.com,
	rgb@redhat.com, linux-security-module@vger.kernel.org,
	jmorris@namei.org, Stefan Berger <stefanb@linux.ibm.com>,
	James Bottomley <James.Bottomley@HansenPartnership.com>
Subject: Re: [PATCH v7 10/14] securityfs: Extend securityfs with namespacing support
Date: Thu, 16 Dec 2021 14:40:27 +0100	[thread overview]
Message-ID: <20211216134027.33sprdmhol2tbctf@wittgenstein> (raw)
In-Reply-To: <20211216054323.1707384-11-stefanb@linux.vnet.ibm.com>

On Thu, Dec 16, 2021 at 12:43:19AM -0500, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> Extend 'securityfs' for support of IMA namespacing so that each
> IMA (user) namespace can have its own front-end for showing the currently
> active policy, the measurement list, number of violations and so on.
> 
> Drop the addition dentry reference to enable simple cleanup of dentries
> upon umount.
> 
> Prevent mounting of an instance of securityfs in another user namespace
> than it belongs to. Also, prevent accesses to directories when another
> user namespace is active than the one that the instance of securityfs
> belongs to.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
>  security/inode.c | 37 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/security/inode.c b/security/inode.c
> index fee01ff4d831..a0d9f086e3d5 100644
> --- a/security/inode.c
> +++ b/security/inode.c
> @@ -26,6 +26,29 @@
>  static struct vfsmount *init_securityfs_mount;
>  static int init_securityfs_mount_count;
>  
> +static int securityfs_permission(struct user_namespace *mnt_userns,
> +				 struct inode *inode, int mask)
> +{
> +	int err;
> +
> +	err = generic_permission(&init_user_ns, inode, mask);
> +	if (!err) {
> +		if (inode->i_sb->s_user_ns != current_user_ns())
> +			err = -EACCES;

I really think the correct semantics is to grant all callers access
whose user namespace is the same as or an ancestor of the securityfs
userns. It's weird to deny access to callers who are located in an
ancestor userns.

For example, a privileged process on the host should be allowed to setns
to the userns of an unprivileged container and inspect its securityfs
instance.

We're mostly interested to block such as scenarios where two sibling
unprivileged containers are created in the initial userns and an fd
proxy or something funnels a file descriptor from one sibling container
to the another one and the receiving sibling container can use readdir()
or openat() on this fd. (I'm not even convinced that this is actually a
problem but stricter semantics at the beginning can't hurt. We can
always relax this later.)

  reply	other threads:[~2021-12-16 13:40 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-16  5:43 [PATCH v7 00/14] ima: Namespace IMA with audit support in IMA-ns Stefan Berger
2021-12-16  5:43 ` [PATCH v7 01/14] ima: Add IMA namespace support Stefan Berger
2021-12-16 14:08   ` Christian Brauner
2021-12-16 21:52     ` James Bottomley
2021-12-17  9:55       ` Christian Brauner
2021-12-16  5:43 ` [PATCH v7 02/14] ima: Define ns_status for storing namespaced iint data Stefan Berger
2021-12-16  5:43 ` [PATCH v7 03/14] ima: Namespace audit status flags Stefan Berger
2021-12-16  5:43 ` [PATCH v7 04/14] ima: Move policy related variables into ima_namespace Stefan Berger
2021-12-16 14:26   ` kernel test robot
2021-12-16  5:43 ` [PATCH v7 05/14] ima: Move ima_htable " Stefan Berger
2021-12-16  5:43 ` [PATCH v7 06/14] ima: Move measurement list related variables " Stefan Berger
2021-12-16  5:43 ` [PATCH v7 07/14] ima: Only accept AUDIT rules for IMA non-init_ima_ns namespaces for now Stefan Berger
2021-12-16  5:43 ` [PATCH v7 08/14] ima: Implement hierarchical processing of file accesses Stefan Berger
2021-12-16  5:43 ` [PATCH v7 09/14] securityfs: Only use simple_pin_fs/simple_release_fs for init_user_ns Stefan Berger
2021-12-16  5:43 ` [PATCH v7 10/14] securityfs: Extend securityfs with namespacing support Stefan Berger
2021-12-16 13:40   ` Christian Brauner [this message]
2021-12-16 16:28     ` Christian Brauner
2022-01-03 14:09     ` Stefan Berger
2021-12-17 16:21   ` [RFC PATCH] securityfs: securityfs_dir_inode_operations can be static kernel test robot
2021-12-17 16:29   ` [PATCH v7 10/14] securityfs: Extend securityfs with namespacing support kernel test robot
2021-12-16  5:43 ` [PATCH v7 11/14] ima: Move some IMA policy and filesystem related variables into ima_namespace Stefan Berger
2021-12-16  5:43 ` [PATCH v7 12/14] ima: Use mac_admin_ns_capable() to check corresponding capability Stefan Berger
2021-12-16  5:43 ` [PATCH v7 13/14] ima: Move dentry into ima_namespace and others onto stack Stefan Berger
2021-12-16  5:43 ` [PATCH v7 14/14] ima: Setup securityfs for IMA namespace Stefan Berger
2021-12-16 10:59   ` kernel test robot
2021-12-16 12:02   ` kernel test robot
2021-12-16 13:51   ` Christian Brauner
2021-12-16 21:38     ` Stefan Berger
2021-12-16 12:50 ` [PATCH v7 00/14] ima: Namespace IMA with audit support in IMA-ns Christian Brauner
2021-12-16 13:31   ` Christian Brauner
2021-12-16 21:27     ` Stefan Berger
2021-12-17 10:25       ` Christian Brauner
2021-12-18  2:38     ` Stefan Berger
2021-12-16 21:00   ` Stefan Berger
2021-12-17 10:06     ` Christian Brauner
2021-12-27 17:29       ` Stefan Berger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211216134027.33sprdmhol2tbctf@wittgenstein \
    --to=christian.brauner@ubuntu.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=containers@lists.linux.dev \
    --cc=dmitry.kasatkin@gmail.com \
    --cc=ebiederm@xmission.com \
    --cc=jamjoom@us.ibm.com \
    --cc=jejb@linux.ibm.com \
    --cc=jmorris@namei.org \
    --cc=krzysztof.struczynski@huawei.com \
    --cc=lhinds@redhat.com \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=lsturman@redhat.com \
    --cc=mpeters@redhat.com \
    --cc=paul@paul-moore.com \
    --cc=puiterwi@redhat.com \
    --cc=rgb@redhat.com \
    --cc=roberto.sassu@huawei.com \
    --cc=serge@hallyn.com \
    --cc=stefanb@linux.ibm.com \
    --cc=stefanb@linux.vnet.ibm.com \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).