linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mimi Zohar <zohar@linux.vnet.ibm.com>
To: Guilherme Magalhaes <guilherme.magalhaes@hpe.com>,
	dmitry.kasatkin@gmail.com
Cc: viro@zeniv.linux.org.uk, james.l.morris@oracle.com,
	serge@hallyn.com, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-ima-devel@lists.sourceforge.net,
	linux-ima-user@lists.sourceforge.net,
	linux-security-module@vger.kernel.org, tycho@docker.com,
	joaquims@hpe.com, nigel.edwards@hpe.com
Subject: Re: [RFC 04/11] ima: add support to namespace securityfs file
Date: Wed, 24 May 2017 16:12:54 -0400	[thread overview]
Message-ID: <1495656774.3841.72.camel@linux.vnet.ibm.com> (raw)
In-Reply-To: <1494511203-8397-5-git-send-email-guilherme.magalhaes@hpe.com>

On Thu, 2017-05-11 at 10:59 -0300, Guilherme Magalhaes wrote:
> Creating the namespace securityfs file under ima folder. When a mount
> namespace id is written to the namespace file, a new folder is created and
> with a policy file for that specified namespace. Then, user defined policy
> for namespaces may be set by writing rules to this namespace policy file.
> With this interface, there is no need to give visibility for the securityfs
> inside mount namespaces or containers in userspace.
> 
> Signed-off-by: Guilherme Magalhaes <guilherme.magalhaes@hpe.com>

The design needs to be flexible enough for different types of
containers, not just for when the orchestration layer provides the
policy.  With this design, the container owner has no control over the
policy.

One option is that we bind mount the securityfs/policy, so that root
in the container will be allowed to read/write the policy.  At some
point, we might connect a vTPM to the container so that the container
owner would be able to get a quote.  For now even without a vTPM, the
same mechanism would allow root within the container to read the
measurement list.

Mimi


> ---
>  security/integrity/ima/ima.h    |   4 +
>  security/integrity/ima/ima_fs.c | 183 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 187 insertions(+)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 42fb91ba..6e8ca8e 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -326,4 +326,8 @@ static inline int security_filter_rule_match(u32 secid, u32 field, u32 op,
>  #define	POLICY_FILE_FLAGS	S_IWUSR
>  #endif /* CONFIG_IMA_WRITE_POLICY */
> 
> +#ifdef CONFIG_IMA_PER_NAMESPACE
> +#define NAMESPACES_FILE_FLAGS  S_IWUSR
> +#endif
> +
>  #endif /* __LINUX_IMA_H */
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index ca303e5..6456407 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -23,6 +23,8 @@
>  #include <linux/rcupdate.h>
>  #include <linux/parser.h>
>  #include <linux/vmalloc.h>
> +#include <linux/proc_ns.h>
> +#include <linux/radix-tree.h>
> 
>  #include "ima.h"
> 
> @@ -272,6 +274,40 @@ static const struct file_operations ima_ascii_measurements_ops = {
>  	.release = seq_release,
>  };
> 
> +#ifdef CONFIG_IMA_PER_NAMESPACE
> +/*
> + * check_mntns: check a mount namespace is valid
> + *
> + * @ns_id: namespace id to be checked
> + * Returns 0 if the namespace is valid.
> + *
> + * Note: a better way to implement this check is needed. There are
> + * cases where the namespace id is valid but not in use by any process
> + * and then this implementation misses this case. Could we use an
> + * interface similar to what setns implements?
> + */
> +static int check_mntns(unsigned int ns_id)
> +{
> +	struct task_struct *p;
> +	int result = 1;
> +	struct ns_common *ns;
> +
> +	rcu_read_lock();
> +	for_each_process(p) {
> +		ns = mntns_operations.get(p);
> +		if (ns->inum == ns_id) {
> +			result = 0;
> +			mntns_operations.put(ns);
> +			break;
> +		}
> +		mntns_operations.put(ns);
> +	}
> +	rcu_read_unlock();
> +
> +	return result;
> +}
> +#endif
> +
>  static ssize_t ima_read_policy(char *path)
>  {
>  	void *data;
> @@ -366,6 +402,9 @@ static struct dentry *ascii_runtime_measurements;
>  static struct dentry *runtime_measurements_count;
>  static struct dentry *violations;
>  static struct dentry *ima_policy;
> +#ifdef CONFIG_IMA_PER_NAMESPACE
> +static struct dentry *ima_namespaces;
> +#endif
> 
>  enum ima_fs_flags {
>  	IMA_FS_BUSY,
> @@ -451,6 +490,139 @@ static const struct file_operations ima_measure_policy_ops = {
>  	.llseek = generic_file_llseek,
>  };
> 
> +#ifdef CONFIG_IMA_PER_NAMESPACE
> +/*
> + * Assumes namespace id is in use by some process and this mapping
> + * does not exist in the map table.
> + */
> +static int create_mnt_ns_directory(unsigned int ns_id)
> +{
> +	int result;
> +	struct dentry *ns_dir, *ns_policy;
> +	char dir_name[64];
> +
> +	snprintf(dir_name, sizeof(dir_name), "%u", ns_id);
> +
> +	ns_dir = securityfs_create_dir(dir_name, ima_dir);
> +	if (IS_ERR(ns_dir)) {
> +		result = PTR_ERR(ns_dir);
> +		goto out;
> +	}
> +
> +	ns_policy = securityfs_create_file("policy", POLICY_FILE_FLAGS,
> +		                                ns_dir, NULL,
> +		                                &ima_measure_policy_ops);
> +	if (IS_ERR(ns_policy)) {
> +		result = PTR_ERR(ns_policy);
> +		securityfs_remove(ns_dir);
> +		goto out;
> +	}
> +
> +	result = 0;
> +
> +out:
> +	return result;
> +}
> +
> +static ssize_t handle_new_namespace_policy(const char *data, size_t datalen)
> +{
> +	unsigned int ns_id;
> +	ssize_t result;
> +
> +	result = -EINVAL;
> +
> +	if (sscanf(data, "%u", &ns_id) != 1) {
> +		pr_err("IMA: invalid namespace id: %s\n", data);
> +		goto out;
> +	}
> +
> +	if (check_mntns(ns_id)) {
> +		result = -ENOENT;
> +		pr_err("IMA: unused namespace id %u\n", ns_id);
> +		goto out;
> +	}
> +
> +	result = create_mnt_ns_directory(ns_id);
> +	if (result != 0) {
> +		pr_err("IMA: namespace id %u directory creation failed\n", ns_id);
> +		goto out;
> +	}
> +
> +	result = datalen;
> +	pr_info("IMA: directory created for namespace id %u\n", ns_id);
> +
> +out:
> +	return result;
> +}
> +
> +static ssize_t ima_write_namespaces(struct file *file, const char __user *buf,
> +		                            size_t datalen, loff_t *ppos)
> +{
> +	char *data;
> +	ssize_t result;
> +
> +	if (datalen >= PAGE_SIZE)
> +		datalen = PAGE_SIZE - 1;
> +
> +	/* No partial writes. */
> +	result = -EINVAL;
> +	if (*ppos != 0)
> +		goto out;
> +
> +	result = -ENOMEM;
> +	data = kmalloc(datalen + 1, GFP_KERNEL);
> +	if (!data)
> +		goto out;
> +
> +	*(data + datalen) = '\0';
> +
> +	result = -EFAULT;
> +	if (copy_from_user(data, buf, datalen))
> +		goto out_free;
> +
> +	result = mutex_lock_interruptible(&ima_write_mutex);
> +	if (result < 0)
> +		goto out_free;
> +
> +	result = handle_new_namespace_policy(data, datalen);
> +
> +	mutex_unlock(&ima_write_mutex);
> +
> +out_free:
> +	kfree(data);
> +out:
> +	return result;
> +}
> +
> +static int ima_open_namespaces(struct inode *inode, struct file *filp)
> +{
> +	if (!(filp->f_flags & O_WRONLY))
> +		return -EACCES;
> +
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EPERM;
> +
> +	if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
> +		return -EBUSY;
> +	return 0;
> +}
> +
> +static int ima_release_namespaces(struct inode *inode, struct file *file)
> +{
> +	clear_bit(IMA_FS_BUSY, &ima_fs_flags);
> +
> +	return 0;
> +}
> +
> +static const struct file_operations ima_namespaces_ops = {
> +		.open = ima_open_namespaces,
> +		.write = ima_write_namespaces,
> +		.read = seq_read,
> +		.release = ima_release_namespaces,
> +		.llseek = generic_file_llseek,
> +};
> +#endif
> +
>  int __init ima_fs_init(void)
>  {
>  	ima_dir = securityfs_create_dir("ima", NULL);
> @@ -490,6 +662,14 @@ int __init ima_fs_init(void)
>  	if (IS_ERR(ima_policy))
>  		goto out;
> 
> +#ifdef CONFIG_IMA_PER_NAMESPACE
> +	ima_namespaces = securityfs_create_file("namespaces", NAMESPACES_FILE_FLAGS,
> +						ima_dir, NULL,
> +						&ima_namespaces_ops);
> +	if (IS_ERR(ima_namespaces))
> +		goto out;
> +#endif
> +
>  	return 0;
>  out:
>  	securityfs_remove(violations);
> @@ -498,5 +678,8 @@ int __init ima_fs_init(void)
>  	securityfs_remove(binary_runtime_measurements);
>  	securityfs_remove(ima_dir);
>  	securityfs_remove(ima_policy);
> +#ifdef CONFIG_IMA_PER_NAMESPACE
> +	securityfs_remove(ima_namespaces);
> +#endif
>  	return -1;
>  }

  parent reply	other threads:[~2017-05-24 20:12 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-11 13:59 [RFC 00/11] ima: namespace support for IMA policy Guilherme Magalhaes
2017-05-11 13:59 ` [RFC 01/11] ima: qualify pathname in audit info record Guilherme Magalhaes
2017-05-11 13:59 ` [RFC 02/11] ima: qualify pathname in audit measurement record Guilherme Magalhaes
2017-05-11 13:59 ` [RFC 03/11] ima: qualify pathname in measurement file Guilherme Magalhaes
2017-05-11 13:59 ` [RFC 04/11] ima: add support to namespace securityfs file Guilherme Magalhaes
2017-05-18 21:39   ` Tycho Andersen
2017-05-24 20:12   ` Mimi Zohar [this message]
2017-05-25  7:36     ` John Johansen
2017-05-25 11:46       ` Mimi Zohar
2017-05-25 19:04         ` Magalhaes, Guilherme (Brazil R&D-CL)
2017-05-29 17:32           ` Mimi Zohar
2017-05-31  9:49             ` Dr. Greg Wettstein
2017-05-11 13:59 ` [RFC 05/11] ima: store new namespace policy structure in a radix tree Guilherme Magalhaes
2017-05-11 13:59 ` [RFC 06/11] ima, fs: release namespace policy resources Guilherme Magalhaes
2017-05-11 13:59 ` [RFC 07/11] ima: new namespace policy structure to track initial namespace policy data Guilherme Magalhaes
2017-05-11 14:00 ` [RFC 08/11] ima: block initial namespace id on the namespace policy interface Guilherme Magalhaes
2017-05-11 14:00 ` [RFC 09/11] ima: delete namespace policy securityfs file in write-once mode Guilherme Magalhaes
2017-05-11 14:00 ` [RFC 10/11] ima: handling all policy flags per namespace using ima_ns_policy structure Guilherme Magalhaes
2017-05-11 14:53 ` [RFC 00/11] ima: namespace support for IMA policy Magalhaes, Guilherme (Brazil R&D-CL)

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=1495656774.3841.72.camel@linux.vnet.ibm.com \
    --to=zohar@linux.vnet.ibm.com \
    --cc=dmitry.kasatkin@gmail.com \
    --cc=guilherme.magalhaes@hpe.com \
    --cc=james.l.morris@oracle.com \
    --cc=joaquims@hpe.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-ima-devel@lists.sourceforge.net \
    --cc=linux-ima-user@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=nigel.edwards@hpe.com \
    --cc=serge@hallyn.com \
    --cc=tycho@docker.com \
    --cc=viro@zeniv.linux.org.uk \
    /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).