diff mbox

[RFC,04/11] ima: add support to namespace securityfs file

Message ID 1494511203-8397-5-git-send-email-guilherme.magalhaes@hpe.com (mailing list archive)
State New, archived
Headers show

Commit Message

Guilherme Magalhaes May 11, 2017, 1:59 p.m. UTC
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>
---
 security/integrity/ima/ima.h    |   4 +
 security/integrity/ima/ima_fs.c | 183 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 187 insertions(+)

Comments

Tycho Andersen May 18, 2017, 9:39 p.m. UTC | #1
Hi Guilherme,

On Thu, May 11, 2017 at 10:59:56AM -0300, Guilherme Magalhaes wrote:
> +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;

It probably makes sense to do something like:

if (!(ima_appraise & IMA_APPRAISE_NAMESPACE))
	return -EINVAL;

here.

I'll keep playing around with this patchset and see if I have any
other feedback.

Cheers,

Tycho
Mimi Zohar May 24, 2017, 8:12 p.m. UTC | #2
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;
>  }
John Johansen May 25, 2017, 7:36 a.m. UTC | #3
On 05/24/2017 01:12 PM, Mimi Zohar wrote:
> 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.
> 
I haven't looked at this enough yet on IMAs end, but another possible solution
is using a symlink and a magic jump_link similar to what nsfs is doing.

The patch series I posted out a couple of weeks ago
  [RFC][Patch 0/3] securityfs: add the ability to support symlinks

adds symlink support to securityfs, and then patch 3/3 cribs from nsfs
updating apparmorfs to use jump_link to "virtualize" the apparmor policy
directory. This avoids needing to have the bind mount.

I'll break the patch out more and repost so its easier to see if this
approach might work for IMA.
Mimi Zohar May 25, 2017, 11:46 a.m. UTC | #4
Hi John,

On Thu, 2017-05-25 at 00:36 -0700, John Johansen wrote:
> On 05/24/2017 01:12 PM, Mimi Zohar wrote:
> > 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.
> > 
> I haven't looked at this enough yet on IMAs end, but another possible solution
> is using a symlink and a magic jump_link similar to what nsfs is doing.
> 
> The patch series I posted out a couple of weeks ago
>   [RFC][Patch 0/3] securityfs: add the ability to support symlinks
> 
> adds symlink support to securityfs, and then patch 3/3 cribs from nsfs
> updating apparmorfs to use jump_link to "virtualize" the apparmor policy
> directory. This avoids needing to have the bind mount.
> 
> I'll break the patch out more and repost so its easier to see if this
> approach might work for IMA.

Sorry, I've been meaning to take a look at your patches, but just
haven't gotten to it yet.  This approach sounds really promising.

thanks,

Mimi
Guilherme Magalhaes May 25, 2017, 7:04 p.m. UTC | #5
Mimi,
With the securityfs symlink we would address the case of setting policy inside containers, but we still would need a way to set the IMA policy per namespace outside containers. So, the current proposed interface would address the latter case.
As an alternative to symlinks, taking this patch set as base, and still considering setting policy inside containers (or inside namespaces in general), it is possible to bind mount the securityfs files into the containers, but it would be needed to prevent read/write access to the namespaced IMA policy files for processes not running on the same namespace.

These mechanisms would not require a change in the proposed design. Do you think these mechanisms are enough for the flexibility you asked?

Thanks.
--
Guilherme

-----Original Message-----
From: Mimi Zohar [mailto:zohar@linux.vnet.ibm.com] 

Sent: quinta-feira, 25 de maio de 2017 08:46
To: John Johansen <john.johansen@canonical.com>; Magalhaes, Guilherme (Brazil R&D-CL) <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; Souza, Joaquim (Brazil R&D-ECL) <joaquims@hpe.com>; Edwards, Nigel <nigel.edwards@hpe.com>
Subject: Re: [RFC 04/11] ima: add support to namespace securityfs file

Hi John,

On Thu, 2017-05-25 at 00:36 -0700, John Johansen wrote:
> On 05/24/2017 01:12 PM, Mimi Zohar wrote:

> > 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.

> > 

> I haven't looked at this enough yet on IMAs end, but another possible 

> solution is using a symlink and a magic jump_link similar to what nsfs is doing.

> 

> The patch series I posted out a couple of weeks ago

>   [RFC][Patch 0/3] securityfs: add the ability to support symlinks

> 

> adds symlink support to securityfs, and then patch 3/3 cribs from nsfs 

> updating apparmorfs to use jump_link to "virtualize" the apparmor 

> policy directory. This avoids needing to have the bind mount.

> 

> I'll break the patch out more and repost so its easier to see if this 

> approach might work for IMA.


Sorry, I've been meaning to take a look at your patches, but just haven't gotten to it yet.  This approach sounds really promising.

thanks,

Mimi
Mimi Zohar May 29, 2017, 5:32 p.m. UTC | #6
Hi Guilherme,

(Wow, you should did Cc a lot of people.)

On Thu, 2017-05-25 at 19:04 +0000, Magalhaes, Guilherme (Brazil R&D-
CL) wrote:
> Mimi,
> With the securityfs symlink we would address the case of setting
> policy inside containers, but we still would need a way to set the
> IMA policy per namespace outside containers. So, the current
> proposed interface would address the latter case.
> As an alternative to symlinks, taking this patch set as base, and
> still considering setting policy inside containers (or inside
> namespaces in general), it is possible to bind mount the securityfs
> files into the containers, but it would be needed to prevent
> read/write access to the namespaced IMA policy files for processes
> not running on the same namespace.
> 
> These mechanisms would not require a change in the proposed design.
> Do you think these mechanisms are enough for the flexibility you
> asked?

I'm really sorry Guileherme, but as I previously explained, IMA has
many aspects to it - the original file measurements (IMA-measurement), 
file hash/signature appraisal (IMA-appraisal), and file audit messages
(IMA-audit) used for security analytics/forensics, not the file system
auditing.  To namespace IMA properly requires addressing some
underlying problems - securityfs, root privilege required for writing
security xattrs, per namespace IMA keyring - to name a few.

I understand wanting to namespace the appraisal aspect first, but when
you asked me if anyone else is working on namespacing IMA at the
moment, I suggested starting with the IMA-audit aspect for a
reason.  By beginning with the IMA-audit aspect, we could ignore, at
least for the time being, some of these underlying problems, but
define the basic namespacing architecture.  By jumping to namespacing
the appraisal aspect, you've simply avoided addressing the IMA
namespacing architecture issues, which need to be addressed.

Here are some, not by any means all, of the issues with namespacing
IMA.


- IMA-measurements:

The namespacing design will need to support different types of
environments. For example, depending on the environment, namespaces
can come and go frequently. In such an environment do we really want
all the namespace measurements to be included in the system
measurement list, or should each namespace have its own measurement
list?  Should the namespace measurement policy be the same as the
system measurement policy?  Should the namespace (container) owner be
allowed to modify their own policy?  If a file matches a rule in both
the namespace and system policies, should the file measurement be in
both the namespace and the system measurement lists?


- IMA-appraisal

Assuming the IMA appraisal policy requires file signatures, signatures
are verified based on the keys on the IMA keyring.  When discussing
namespacing IMA-appraisal, we might want different sets of keys in
different namespaces. This requires separate keyrings for each
namespace.  In some use cases, we might want to include the keys on
the system IMA keyring, while in other use cases we might not.

Even if real root initially installs the files with their file
signatures, stored as extended attributes, how will software be
updated, as writing file signatures requires root
privilege?Capabilities has recently added support to permit root in
the namespace to write security.capability.  Similarly when
namespacing IMA, root in the namespace needs the ability to write
security.ima.


- IMA-audit:

The IMA-audit messages can augment other file system security
information used in security analytics/forensics.  This information
should be on a per namespace basis, meaning that each time a new file
is accessed/executed, there needs to be a separate audit message, even
if a message already exists in another namespace.  Maintaining and
cleaning up this per namespace cache information, allows development
of the IMA namespace architecture independently of other issues.

My original suggestion stands.  Start with namespacing IMA-
audit.  Afterwards resolve the securityfs issues needed for
namespacing IMA-measurement, and subsequently resolve the keyring and
xattr issues for namespacing IMA-appraisal. Although other subsystems
have already addressed some of the issues listed here, the advice to
start with IMA-audit is still valid.

Small incremental change does not imply without an overall design, but
an overall design which is broken up in such a way (to ease review)
that builds upon the previous change.

As both Apparmor and IMA use securityfs for policy, it would be nice
if their method for loading namespace policies would be similar too.

Mimi
Dr. Greg May 31, 2017, 9:49 a.m. UTC | #7
On Mon, May 29, 2017 at 01:32:38PM -0400, Mimi Zohar wrote:

> Hi Guilherme,
> 
> (Wow, you should did Cc a lot of people.)

Indeed.

We have namespaced a significant amount of the IMA code so we will
continue the broadcast, under the assumption that this is of general
interest to the community.

Comments below.

> On Thu, 2017-05-25 at 19:04 +0000, Magalhaes, Guilherme (Brazil R&D-
> CL) wrote:
> > Mimi,
> > With the securityfs symlink we would address the case of setting
> > policy inside containers, but we still would need a way to set the
> > IMA policy per namespace outside containers. So, the current
> > proposed interface would address the latter case.
> > As an alternative to symlinks, taking this patch set as base, and
> > still considering setting policy inside containers (or inside
> > namespaces in general), it is possible to bind mount the securityfs
> > files into the containers, but it would be needed to prevent
> > read/write access to the namespaced IMA policy files for processes
> > not running on the same namespace.
> > 
> > These mechanisms would not require a change in the proposed design.
> > Do you think these mechanisms are enough for the flexibility you
> > asked?
> 
> I'm really sorry Guileherme, but as I previously explained, IMA has
> many aspects to it - the original file measurements (IMA-measurement), 
> file hash/signature appraisal (IMA-appraisal), and file audit messages
> (IMA-audit) used for security analytics/forensics, not the file system
> auditing.????To namespace IMA properly requires addressing some
> underlying problems - securityfs, root privilege required for writing
> security xattrs, per namespace IMA keyring - to name a few.
> 
> I understand wanting to namespace the appraisal aspect first, but when
> you asked me if anyone else is working on namespacing IMA at the
> moment, I suggested starting with the IMA-audit aspect for a
> reason.????By beginning with the IMA-audit aspect, we could ignore, at
> least for the time being, some of these underlying problems, but
> define the basic namespacing architecture.????By jumping to namespacing
> the appraisal aspect, you've simply avoided addressing the IMA
> namespacing architecture issues, which need to be addressed.
> 
> Here are some, not by any means all, of the issues with namespacing
> IMA.
>
> 
> - IMA-measurements:
> 
> The namespacing design will need to support different types of
> environments. For example, depending on the environment, namespaces
> can come and go frequently. In such an environment do we really want
> all the namespace measurements to be included in the system
> measurement list, or should each namespace have its own measurement
> list?????Should the namespace measurement policy be the same as the
> system measurement policy?????Should the namespace (container) owner be
> allowed to modify their own policy?????If a file matches a rule in both
> the namespace and system policies, should the file measurement be in
> both the namespace and the system measurement lists?
> 
> 
> - IMA-appraisal
> 
> Assuming the IMA appraisal policy requires file signatures, signatures
> are verified based on the keys on the IMA keyring.????When discussing
> namespacing IMA-appraisal, we might want different sets of keys in
> different namespaces. This requires separate keyrings for each
> namespace. ??In some use cases, we might want to include the keys on
> the system IMA keyring, while in other use cases we might not.
> 
> Even if real root initially installs the files with their file
> signatures, stored as extended attributes, how will software be
> updated, as writing file signatures requires root
> privilege?Capabilities has recently added support to permit root in
> the namespace to write security.capability.????Similarly when
> namespacing IMA, root in the namespace needs the ability to write
> security.ima.
> 
> 
> - IMA-audit:
> 
> The IMA-audit messages can augment other file system security
> information used in security analytics/forensics.????This information
> should be on a per namespace basis, meaning that each time a new file
> is accessed/executed, there needs to be a separate audit message, even
> if a message already exists in another namespace.????Maintaining and
> cleaning up this per namespace cache information, allows development
> of the IMA namespace architecture independently of other issues.
> 
> My original suggestion stands. ??Start with namespacing IMA-
> audit.????Afterwards resolve the securityfs issues needed for
> namespacing IMA-measurement, and subsequently resolve the keyring and
> xattr issues for namespacing IMA-appraisal. Although other subsystems
> have already addressed some of the issues listed here, the advice to
> start with IMA-audit is still valid.
> 
> Small incremental change does not imply without an overall design, but
> an overall design which is broken up in such a way (to ease review)
> that builds upon the previous change.
> 
> As both Apparmor and IMA use securityfs for policy, it would be nice
> if their method for loading namespace policies would be similar too.

Our deterministic platform modeling code is built on top of the IMA
infrastructure.  Our modeling implementation has namespace support so
we ended up OS virtualizing a significant amount of the IMA
infrastructure to implement container specific behavior modeling.  So
the following is under the moniker of been there, done that to some
extent.

As an aside, we refer to containers running in a modeled environment
as 'canisters' since they are containers with a label.

Based on our experiences, the community will want each namespace
implementation to be as independent as possible.  A primary benefit of
namespaced implementations of integrity/behavior modeling is to enable
more tractable deployments since the system can be run from a very
small TCB whose behavior is rooted to a hardware integrity guarantee.
System upgrades are way more tractable in this type of architecture.

Each canister in our model has its own behavioral trajectory path
which is the equivalent of a superset of the IMA measurement list.
After the canister behavior is sealed, each canister reports forensics
violations as new entries on the trajectory path.  So forensic
evaluations are enabled at the canister level, which is extremely
valuable.

Implementing all of this was fairly straight forward on top of the
current IMA infrastructure.  Mimi's recommendation to start with the
audit layer is well taken, that is a solid pathway for determining how
to generically address the relevant infrastructure issues.

From an implementation perspective we don't use bind mounts or the
like.  The securityfs pseudo-files all sense which namespace they are
operating in which resulted in a much cleaner implementation, at least
in our experience.

We implemented a model where the behavioral eigenvectors (events) are
surfaced through a new pseudo-file created for each canister.  This
pseudo-file is rooted in the /sys/fs/iso-identity pseudo-directory and
named by the namespace inode number which was allocated for the
canister namespace when the behavioral domain (namespace) is created.

This could have been done as well in the securityfs filesystem but we
were too {cramped for time,lazy} to implement polling and sysfs files
gives you that for free.

This seems to be a pretty flexible model for orchestration.  We
propagate the behavioral eigenvectors into individual SGX enclaves
which monitor the behavior of each canister.  The model would support
propagating events into a vTPM as well.

From an orchestration perspective we plumbed all of this, including
SGX support, into the most recent release of runc and it has all
worked surprisingly well with respect to running existing containers.
All you need to do is specify in the container config.json that the
container is to run in an independent behavioral namespace.  We do
lament that runc was written in GO rather then C... :-)

There are some rough edges which need polishing though.  A more
expressive and namespace specific policy engine needs to be addressed.
In addition to the namespace implementation we implemented superblock
independent content digests which closes what we believe is a rather
significant measurement gap.  Addressing network inode measurement is
an entirely new and different can of worms as well.

We have also spent a fair amount of time reflecting on Viro's rants
with respect to IMA and all of its inherent ugliness... :-)

Our major concern is what does the community want to do with respect
to defining the CLONE_* flag which is used to signal that the
integrity/behavior namespace should be unshared.  We currently build
on top of 4.4 LTS and we stole the last bit which was available.  What
are the thoughts with respect to expanding this resource as it won't
be the last OS virtualization which is undertaken?

> Mimi

Hopefully the above reflections and experiences are useful datapoints.

Have a good week.

Greg

As always,
Dr. G.W. Wettstein, Ph.D.   Enjellic Systems Development, LLC.
4206 N. 19th Ave.           Specializing in information infra-structure
Fargo, ND  58102            development.
PH: 701-281-1686
FAX: 701-281-3949           EMAIL: greg@enjellic.com
------------------------------------------------------------------------------
"We can't solve today's problems by using the same thinking we used in
 creating them."
                                -- Einstein
diff mbox

Patch

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;
 }