LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [RFC PATCH v3 0/3] ima: namespacing IMA
@ 2018-03-27 13:57 Stefan Berger
  2018-03-27 13:57 ` [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support Stefan Berger
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Stefan Berger @ 2018-03-27 13:57 UTC (permalink / raw)
  To: linux-integrity
  Cc: containers, linux-kernel, linux-security-module, tycho, serge,
	sunyuqiong1988, david.safford, mkayaalp, James.Bottomley, zohar,
	ebiederm, Stefan Berger

This patch set implements an IMA namespace data structure that gets
created alongside a user namespace with CLONE_NEWUSER, and lays down the
foundation for namespacing the different aspects of IMA (eg. IMA-audit,
IMA-measurement, IMA-appraisal).

The original PoC patches [1] created a new CLONE_NEWIMA flag to explicitly
control when a new IMA namespace should be created. The previously posted
version 2 of this series had it hooked on the mount namespace, which was
regarded as inappropriate. Based on comments, we elected to hang the IMA
namepace off of the existing user namespace now. In this patch series we
are adding a pointer to the user_namespace pointing to the ima_namespace.
Both are now tied together and joining the user_namespace with setns() also
joins the ima_namespace that was created along with it.

The first patch creates the ima_namespace data, while the second patch
puts the iint->flags in the namespace. The third patch uses these flags
for namespacing the IMA-audit messages, enabling the same file to be
audited each time it is accessed in a new namespace.


   Stefan

Mehmet Kayaalp (2):
  ima: Add ns_status for storing namespaced iint data
  ima: mamespace audit status flags

Yuqiong Sun (1):
  ima: extend clone() with IMA namespace support

 include/linux/ima.h                      |  67 +++++++++
 include/linux/user_namespace.h           |   4 +
 init/Kconfig                             |  10 ++
 kernel/user.c                            |   7 +
 kernel/user_namespace.c                  |  18 +++
 security/integrity/ima/Makefile          |   3 +-
 security/integrity/ima/ima.h             |  47 ++++++-
 security/integrity/ima/ima_api.c         |   8 +-
 security/integrity/ima/ima_init.c        |   4 +
 security/integrity/ima/ima_init_ima_ns.c |  43 ++++++
 security/integrity/ima/ima_main.c        |  15 ++-
 security/integrity/ima/ima_ns.c          | 225 +++++++++++++++++++++++++++++++
 12 files changed, 443 insertions(+), 8 deletions(-)
 create mode 100644 security/integrity/ima/ima_init_ima_ns.c
 create mode 100644 security/integrity/ima/ima_ns.c

-- 
2.14.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support
  2018-03-27 13:57 [RFC PATCH v3 0/3] ima: namespacing IMA Stefan Berger
@ 2018-03-27 13:57 ` Stefan Berger
  2018-03-27 23:01   ` Eric W. Biederman
  2018-03-27 13:57 ` [RFC PATCH v3 2/3] ima: Add ns_status for storing namespaced iint data Stefan Berger
  2018-03-27 13:57 ` [RFC PATCH v3 3/3] ima: mamespace audit status flags Stefan Berger
  2 siblings, 1 reply; 19+ messages in thread
From: Stefan Berger @ 2018-03-27 13:57 UTC (permalink / raw)
  To: linux-integrity
  Cc: containers, linux-kernel, linux-security-module, tycho, serge,
	sunyuqiong1988, david.safford, mkayaalp, James.Bottomley, zohar,
	ebiederm, Yuqiong Sun, Mehmet Kayaalp, Stefan Berger

From: Yuqiong Sun <suny@us.ibm.com>

Add new CONFIG_IMA_NS config option.  Let clone() create a new IMA
namespace upon CLONE_NEWUSER flag. Attach the ima_ns data structure
to user_namespace. ima_ns is allocated and freed upon IMA namespace
creation and exit, which is tied to USER namespace creation and exit.
Currently, the ima_ns contains no useful IMA data but only a dummy
interface. This patch creates the framework for namespacing the different
aspects of IMA (eg. IMA-audit, IMA-measurement, IMA-appraisal).

Changelog:
v3:
* Use CLONE_NEWUSER instead of CLONE_NEWNW flag

v2:
* Moved ima_init_ns and related functions into own file that is
  always compiled; init_ima_ns will always be there
* Fixed putting of imans->parent
* Move IMA namespace creation from nsproxy into mount namespace
  code; get rid of procfs operations for IMA namespace

v1:
* Use CLONE_NEWNS instead of a new CLONE_NEWIMA flag
* Use existing ima.h headers
* Move the ima_namespace.c to security/integrity/ima/ima_ns.c
* Fix typo INFO->INO
* Each namespace free's itself, removed recursively free'ing
  until init_ima_ns from free_ima_ns()

Signed-off-by: Yuqiong Sun <suny@us.ibm.com>
Signed-off-by: Mehmet Kayaalp <mkayaalp@linux.vnet.ibm.com>
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
 include/linux/ima.h                      | 64 ++++++++++++++++++++++++
 include/linux/user_namespace.h           |  4 ++
 init/Kconfig                             |  8 +++
 kernel/user.c                            |  7 +++
 kernel/user_namespace.c                  | 18 +++++++
 security/integrity/ima/Makefile          |  3 +-
 security/integrity/ima/ima.h             |  4 ++
 security/integrity/ima/ima_init.c        |  4 ++
 security/integrity/ima/ima_init_ima_ns.c | 37 ++++++++++++++
 security/integrity/ima/ima_ns.c          | 86 ++++++++++++++++++++++++++++++++
 10 files changed, 234 insertions(+), 1 deletion(-)
 create mode 100644 security/integrity/ima/ima_init_ima_ns.c
 create mode 100644 security/integrity/ima/ima_ns.c

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0e4647e0eb60..8bca67df0ad3 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -12,6 +12,7 @@
 
 #include <linux/fs.h>
 #include <linux/kexec.h>
+#include <linux/user_namespace.h>
 struct linux_binprm;
 
 #ifdef CONFIG_IMA
@@ -105,4 +106,67 @@ static inline int ima_inode_removexattr(struct dentry *dentry,
 	return 0;
 }
 #endif /* CONFIG_IMA_APPRAISE */
+
+struct ima_namespace {
+	struct kref kref;
+	struct ima_namespace *parent;
+};
+
+extern struct ima_namespace init_ima_ns;
+
+void imans_install(struct ns_common *new);
+
+static inline struct ima_namespace *to_ima_ns(struct ns_common *ns)
+{
+	return container_of(ns, struct user_namespace, ns)->ima_ns;
+}
+
+#ifdef CONFIG_IMA_NS
+
+void free_ima_ns(struct kref *kref);
+
+static inline struct ima_namespace *get_ima_ns(struct ima_namespace *ns)
+{
+	BUG_ON(!ns);
+	if (ns)
+		kref_get(&ns->kref);
+	return ns;
+}
+
+static inline void put_ima_ns(struct ima_namespace *ns)
+{
+	BUG_ON(!ns);
+	if (ns)
+		kref_put(&ns->kref, free_ima_ns);
+}
+
+struct ima_namespace *copy_ima(struct ima_namespace *old_ns);
+
+static inline struct ima_namespace *get_current_ns(void)
+{
+	return current_user_ns()->ima_ns;
+}
+
+#else
+
+static inline struct ima_namespace *get_ima_ns(struct ima_namespace *ns)
+{
+	return ns;
+}
+
+static inline void put_ima_ns(struct ima_namespace *ns)
+{
+	return;
+}
+
+static inline struct ima_namespace *copy_ima(struct ima_namespace *old_ns)
+{
+	return old_ns;
+}
+
+static inline struct ima_namespace *get_current_ns(void)
+{
+	return NULL;
+}
+#endif /* CONFIG_IMA_NS */
 #endif /* _LINUX_IMA_H */
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index d6b74b91096b..8884b22d991c 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -36,6 +36,7 @@ struct uid_gid_map { /* 64 bytes -- 1 cache line */
 #define USERNS_INIT_FLAGS USERNS_SETGROUPS_ALLOWED
 
 struct ucounts;
+struct ima_namespace;
 
 enum ucount_type {
 	UCOUNT_USER_NAMESPACES,
@@ -76,6 +77,9 @@ struct user_namespace {
 #endif
 	struct ucounts		*ucounts;
 	int ucount_max[UCOUNT_COUNTS];
+#ifdef CONFIG_IMA
+	struct ima_namespace	*ima_ns;
+#endif
 } __randomize_layout;
 
 struct ucounts {
diff --git a/init/Kconfig b/init/Kconfig
index a9a2e2c86671..a1ad5384e081 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -931,6 +931,14 @@ config NET_NS
 	help
 	  Allow user space to create what appear to be multiple instances
 	  of the network stack.
+config IMA_NS
+	bool "IMA namespace"
+	depends on IMA
+	default y
+	help
+	  Allow the creation of IMA namespaces for each mount namespace.
+	  Namespaced IMA data enables having IMA features work separately
+	  for each mount namespace.
 
 endif # NAMESPACES
 
diff --git a/kernel/user.c b/kernel/user.c
index 9a20acce460d..31c946f3adce 100644
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -19,6 +19,10 @@
 #include <linux/user_namespace.h>
 #include <linux/proc_ns.h>
 
+#ifdef CONFIG_IMA
+extern struct ima_namespace init_ima_ns;
+#endif
+
 /*
  * userns count is 1 for root user, 1 for init_uts_ns,
  * and 1 for... ?
@@ -66,6 +70,9 @@ struct user_namespace init_user_ns = {
 	.persistent_keyring_register_sem =
 	__RWSEM_INITIALIZER(init_user_ns.persistent_keyring_register_sem),
 #endif
+#ifdef CONFIG_IMA
+	.ima_ns = &init_ima_ns,
+#endif
 };
 EXPORT_SYMBOL_GPL(init_user_ns);
 
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 246d4d4ce5c7..7d6e7d6e6a34 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -25,6 +25,7 @@
 #include <linux/fs_struct.h>
 #include <linux/bsearch.h>
 #include <linux/sort.h>
+#include <linux/ima.h>
 
 static struct kmem_cache *user_ns_cachep __read_mostly;
 static DEFINE_MUTEX(userns_state_mutex);
@@ -140,8 +141,20 @@ int create_user_ns(struct cred *new)
 	if (!setup_userns_sysctls(ns))
 		goto fail_keyring;
 
+#if CONFIG_IMA
+	ns->ima_ns = copy_ima(parent_ns->ima_ns);
+	if (IS_ERR(ns->ima_ns)) {
+		ret = PTR_ERR(ns->ima_ns);
+		goto fail_userns_sysctls;
+	}
+#endif
+
 	set_cred_user_ns(new, ns);
 	return 0;
+#if CONFIG_IMA
+fail_userns_sysctls:
+	retire_userns_sysctls(ns);
+#endif
 fail_keyring:
 #ifdef CONFIG_PERSISTENT_KEYRINGS
 	key_put(ns->persistent_keyring_register);
@@ -195,6 +208,9 @@ static void free_user_ns(struct work_struct *work)
 			kfree(ns->projid_map.forward);
 			kfree(ns->projid_map.reverse);
 		}
+#ifdef CONFIG_IMA
+		put_ima_ns(ns->ima_ns);
+#endif
 		retire_userns_sysctls(ns);
 #ifdef CONFIG_PERSISTENT_KEYRINGS
 		key_put(ns->persistent_keyring_register);
@@ -1285,6 +1301,8 @@ static int userns_install(struct nsproxy *nsproxy, struct ns_common *ns)
 	put_user_ns(cred->user_ns);
 	set_cred_user_ns(cred, get_user_ns(user_ns));
 
+	imans_install(ns);
+
 	return commit_creds(cred);
 }
 
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index d921dc4f9eb0..cc60f726e651 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -7,7 +7,8 @@
 obj-$(CONFIG_IMA) += ima.o
 
 ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
-	 ima_policy.o ima_template.o ima_template_lib.o
+	 ima_policy.o ima_template.o ima_template_lib.o ima_init_ima_ns.o
 ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
+ima-$(CONFIG_IMA_NS) += ima_ns.o
 ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
 obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d52b487ad259..e98c11c7cf75 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -291,6 +291,10 @@ static inline int ima_read_xattr(struct dentry *dentry,
 
 #endif /* CONFIG_IMA_APPRAISE */
 
+int ima_ns_init(void);
+struct ima_namespace;
+int ima_init_namespace(struct ima_namespace *ns);
+
 /* LSM based policy rules require audit */
 #ifdef CONFIG_IMA_LSM_RULES
 
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 2967d497a665..7f884a71fa1c 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -137,5 +137,9 @@ int __init ima_init(void)
 
 	ima_init_policy();
 
+	rc = ima_ns_init();
+	if (rc != 0)
+		return rc;
+
 	return ima_fs_init();
 }
diff --git a/security/integrity/ima/ima_init_ima_ns.c b/security/integrity/ima/ima_init_ima_ns.c
new file mode 100644
index 000000000000..52cb94b1d392
--- /dev/null
+++ b/security/integrity/ima/ima_init_ima_ns.c
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2016-2018 IBM Corporation
+ * Author:
+ *   Yuqiong Sun <suny@us.ibm.com>
+ *   Stefan Berger <stefanb@linux.vnet.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+
+#include <linux/export.h>
+#include <linux/user_namespace.h>
+#include <linux/ima.h>
+
+int ima_init_namespace(struct ima_namespace *ns)
+{
+	return 0;
+}
+
+int __init ima_ns_init(void)
+{
+	return ima_init_namespace(&init_ima_ns);
+}
+
+struct ima_namespace init_ima_ns = {
+	.kref = KREF_INIT(1),
+	.parent = NULL,
+};
+EXPORT_SYMBOL(init_ima_ns);
+
+void imans_install(struct ns_common *new)
+{
+	struct ima_namespace *ns = to_ima_ns(new);
+
+	get_ima_ns(ns);
+}
diff --git a/security/integrity/ima/ima_ns.c b/security/integrity/ima/ima_ns.c
new file mode 100644
index 000000000000..62148908015a
--- /dev/null
+++ b/security/integrity/ima/ima_ns.c
@@ -0,0 +1,86 @@
+/*
+ * Copyright (C) 2016-2018 IBM Corporation
+ * Author:
+ *  Yuqiong Sun <suny@us.ibm.com>
+ *  Stefan Berger <stefanb@linux.vnet.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 2 of the License.
+ */
+
+#include <linux/kref.h>
+#include <linux/slab.h>
+#include <linux/ima.h>
+#include <linux/mount.h>
+
+#include "ima.h"
+
+static struct ima_namespace *create_ima_ns(void)
+{
+	struct ima_namespace *ima_ns;
+
+	ima_ns = kmalloc(sizeof(*ima_ns), GFP_KERNEL);
+	if (ima_ns)
+		kref_init(&ima_ns->kref);
+
+	return ima_ns;
+}
+
+/**
+ * Clone a new ns copying an original ima namespace, setting refcount to 1
+ *
+ * @old_ns: old ima namespace to clone
+ * Return ERR_PTR(-ENOMEM) on error (failure to kmalloc), new ns otherwise
+ */
+static struct ima_namespace *clone_ima_ns(struct ima_namespace *old_ns)
+{
+	struct ima_namespace *ns;
+
+	ns = create_ima_ns();
+	if (!ns)
+		return ERR_PTR(-ENOMEM);
+
+	get_ima_ns(old_ns);
+	ns->parent = old_ns;
+
+	ima_init_namespace(ns);
+
+	return ns;
+}
+
+/**
+ * Copy task's ima namespace, or clone it if flags specifies CLONE_NEWNS.
+ *
+ * @flags: flags used in the clone syscall
+ * @old_ns: old ima namespace to clone
+ */
+
+struct ima_namespace *copy_ima(struct ima_namespace *old_ns)
+{
+	struct ima_namespace *new_ns;
+
+	BUG_ON(!old_ns);
+	get_ima_ns(old_ns);
+
+	new_ns = clone_ima_ns(old_ns);
+	put_ima_ns(old_ns);
+
+	return new_ns;
+}
+
+static void destroy_ima_ns(struct ima_namespace *ns)
+{
+	put_ima_ns(ns->parent);
+	kfree(ns);
+}
+
+void free_ima_ns(struct kref *kref)
+{
+	struct ima_namespace *ns;
+
+	ns = container_of(kref, struct ima_namespace, kref);
+	BUG_ON(ns == &init_ima_ns);
+
+	destroy_ima_ns(ns);
+}
-- 
2.14.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH v3 2/3] ima: Add ns_status for storing namespaced iint data
  2018-03-27 13:57 [RFC PATCH v3 0/3] ima: namespacing IMA Stefan Berger
  2018-03-27 13:57 ` [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support Stefan Berger
@ 2018-03-27 13:57 ` Stefan Berger
  2018-03-27 13:57 ` [RFC PATCH v3 3/3] ima: mamespace audit status flags Stefan Berger
  2 siblings, 0 replies; 19+ messages in thread
From: Stefan Berger @ 2018-03-27 13:57 UTC (permalink / raw)
  To: linux-integrity
  Cc: containers, linux-kernel, linux-security-module, tycho, serge,
	sunyuqiong1988, david.safford, mkayaalp, James.Bottomley, zohar,
	ebiederm, Mehmet Kayaalp

From: Mehmet Kayaalp <mkayaalp@linux.vnet.ibm.com>

This patch adds an rbtree to the IMA namespace structure that stores a
namespaced version of iint->flags in ns_status struct. Similar to the
integrity_iint_cache, both the iint ns_struct are looked up using the
inode pointer value. The lookup, allocate, and insertion code is also
similar, except ns_struct is not free'd when the inode is free'd.
Instead, the lookup verifies the i_ino and i_generation fields are also a
match. This could be replaced by a lazy clean up of the rbtree.

Signed-off-by: Mehmet Kayaalp <mkayaalp@linux.vnet.ibm.com>

Changelog:
v2:
 * fixed tree traversal in __ima_ns_status_find()
---
 include/linux/ima.h                      |   3 +
 security/integrity/ima/ima.h             |  19 +++++
 security/integrity/ima/ima_init_ima_ns.c |   6 ++
 security/integrity/ima/ima_ns.c          | 117 +++++++++++++++++++++++++++++++
 4 files changed, 145 insertions(+)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 8bca67df0ad3..734934261011 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -110,6 +110,9 @@ static inline int ima_inode_removexattr(struct dentry *dentry,
 struct ima_namespace {
 	struct kref kref;
 	struct ima_namespace *parent;
+	struct rb_root ns_status_tree;
+	rwlock_t ns_status_lock;
+	struct kmem_cache *ns_status_cache;
 };
 
 extern struct ima_namespace init_ima_ns;
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index e98c11c7cf75..e51a39ff75ff 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -128,6 +128,14 @@ static inline void ima_load_kexec_buffer(void) {}
  */
 extern bool ima_canonical_fmt;
 
+struct ns_status {
+	struct rb_node rb_node;
+	struct inode *inode;
+	ino_t i_ino;
+	u32 i_generation;
+	unsigned long flags;
+};
+
 /* Internal IMA function definitions */
 int ima_init(void);
 int ima_fs_init(void);
@@ -295,6 +303,17 @@ int ima_ns_init(void);
 struct ima_namespace;
 int ima_init_namespace(struct ima_namespace *ns);
 
+#ifdef CONFIG_IMA_NS
+struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
+				    struct inode *inode);
+#else
+static inline struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
+						  struct inode *inode)
+{
+	return NULL;
+}
+#endif /* CONFIG_IMA_NS */
+
 /* LSM based policy rules require audit */
 #ifdef CONFIG_IMA_LSM_RULES
 
diff --git a/security/integrity/ima/ima_init_ima_ns.c b/security/integrity/ima/ima_init_ima_ns.c
index 52cb94b1d392..66b681c1c722 100644
--- a/security/integrity/ima/ima_init_ima_ns.c
+++ b/security/integrity/ima/ima_init_ima_ns.c
@@ -12,9 +12,15 @@
 #include <linux/export.h>
 #include <linux/user_namespace.h>
 #include <linux/ima.h>
+#include <linux/slab.h>
+
+#include "ima.h"
 
 int ima_init_namespace(struct ima_namespace *ns)
 {
+	ns->ns_status_tree = RB_ROOT;
+	rwlock_init(&ns->ns_status_lock);
+	ns->ns_status_cache = KMEM_CACHE(ns_status, SLAB_PANIC);
 	return 0;
 }
 
diff --git a/security/integrity/ima/ima_ns.c b/security/integrity/ima/ima_ns.c
index 62148908015a..ef70fd1e25c6 100644
--- a/security/integrity/ima/ima_ns.c
+++ b/security/integrity/ima/ima_ns.c
@@ -69,9 +69,23 @@ struct ima_namespace *copy_ima(struct ima_namespace *old_ns)
 	return new_ns;
 }
 
+static void free_ns_status_cache(struct ima_namespace *ns)
+{
+	struct ns_status *status, *next;
+
+	write_lock(&ns->ns_status_lock);
+	rbtree_postorder_for_each_entry_safe(status, next,
+					     &ns->ns_status_tree, rb_node)
+		kmem_cache_free(ns->ns_status_cache, status);
+	ns->ns_status_tree = RB_ROOT;
+	write_unlock(&ns->ns_status_lock);
+	kmem_cache_destroy(ns->ns_status_cache);
+}
+
 static void destroy_ima_ns(struct ima_namespace *ns)
 {
 	put_ima_ns(ns->parent);
+	free_ns_status_cache(ns);
 	kfree(ns);
 }
 
@@ -84,3 +98,106 @@ void free_ima_ns(struct kref *kref)
 
 	destroy_ima_ns(ns);
 }
+
+/*
+ * __ima_ns_status_find - return the ns_status associated with an inode
+ */
+static struct ns_status *__ima_ns_status_find(struct ima_namespace *ns,
+					      struct inode *inode)
+{
+	struct ns_status *status;
+	struct rb_node *n = ns->ns_status_tree.rb_node;
+
+	while (n) {
+		status = rb_entry(n, struct ns_status, rb_node);
+
+		if (inode < status->inode)
+			n = n->rb_left;
+		else if (inode > status->inode)
+			n = n->rb_right;
+		else
+			break;
+	}
+	if (!n)
+		return NULL;
+
+	return status;
+}
+
+/*
+ * ima_ns_status_find - return the ns_status associated with an inode
+ */
+static struct ns_status *ima_ns_status_find(struct ima_namespace *ns,
+					    struct inode *inode)
+{
+	struct ns_status *status;
+
+	read_lock(&ns->ns_status_lock);
+	status = __ima_ns_status_find(ns, inode);
+	read_unlock(&ns->ns_status_lock);
+
+	return status;
+}
+
+void insert_ns_status(struct ima_namespace *ns, struct inode *inode,
+		      struct ns_status *status)
+{
+	struct rb_node **p;
+	struct rb_node *node, *parent = NULL;
+	struct ns_status *test_status;
+
+	p = &ns->ns_status_tree.rb_node;
+	while (*p) {
+		parent = *p;
+		test_status = rb_entry(parent, struct ns_status, rb_node);
+		if (inode < test_status->inode)
+			p = &(*p)->rb_left;
+		else
+			p = &(*p)->rb_right;
+	}
+	node = &status->rb_node;
+	rb_link_node(node, parent, p);
+	rb_insert_color(node, &ns->ns_status_tree);
+}
+
+struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
+				    struct inode *inode)
+{
+	struct ns_status *status;
+	int skip_insert = 0;
+
+	status = ima_ns_status_find(ns, inode);
+	if (status) {
+		/*
+		 * Unlike integrity_iint_cache we are not free'ing the
+		 * ns_status data when the inode is free'd. So, in addition to
+		 * checking the inode pointer, we need to make sure the
+		 * (i_generation, i_ino) pair matches as well. In the future
+		 * we might want to add support for lazily walking the rbtree
+		 * to clean it up.
+		 */
+		if (inode->i_ino == status->i_ino &&
+		    inode->i_generation == status->i_generation)
+			return status;
+
+		/* Same inode number is reused, overwrite the ns_status */
+		skip_insert = 1;
+	} else {
+		status = kmem_cache_alloc(ns->ns_status_cache, GFP_NOFS);
+		if (!status)
+			return ERR_PTR(-ENOMEM);
+	}
+
+	write_lock(&ns->ns_status_lock);
+
+	if (!skip_insert)
+		insert_ns_status(ns, inode, status);
+
+	status->inode = inode;
+	status->i_ino = inode->i_ino;
+	status->i_generation = inode->i_generation;
+	status->flags = 0UL;
+	write_unlock(&ns->ns_status_lock);
+
+	return status;
+}
-- 
2.14.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH v3 3/3] ima: mamespace audit status flags
  2018-03-27 13:57 [RFC PATCH v3 0/3] ima: namespacing IMA Stefan Berger
  2018-03-27 13:57 ` [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support Stefan Berger
  2018-03-27 13:57 ` [RFC PATCH v3 2/3] ima: Add ns_status for storing namespaced iint data Stefan Berger
@ 2018-03-27 13:57 ` Stefan Berger
  2 siblings, 0 replies; 19+ messages in thread
From: Stefan Berger @ 2018-03-27 13:57 UTC (permalink / raw)
  To: linux-integrity
  Cc: containers, linux-kernel, linux-security-module, tycho, serge,
	sunyuqiong1988, david.safford, mkayaalp, James.Bottomley, zohar,
	ebiederm, Mehmet Kayaalp

From: Mehmet Kayaalp <mkayaalp@linux.vnet.ibm.com>

The iint cache stores whether the file is measured, appraised, audited
etc. This patch moves the IMA_AUDITED flag into the per-namespace
ns_status, enabling IMA audit mechanism to audit the same file each time
it is accessed in a new namespace.

The ns_status is not looked up if the CONFIG_IMA_NS is disabled or if
any of the IMA_NS_STATUS_ACTIONS (currently only IMA_AUDIT) is not
enabled.

Read and write operations on the iint flags is replaced with function
calls. For reading, iint_flags() returns the bitwise AND of iint->flags
and ns_status->flags. The ns_status flags are masked with
IMA_NS_STATUS_FLAGS (currently only IMA_AUDITED). Similarly
set_iint_flags() only writes the masked portion to the ns_status flags,
while the iint flags is set as before. The ns_status parameter added to
ima_audit_measurement() is used with the above functions to query and
set the ns_status flags.

Signed-off-by: Mehmet Kayaalp <mkayaalp@linux.vnet.ibm.com>

Changelog:
v2:
 * fixed flag calculation in iint_flags()
---
 init/Kconfig                      |  4 +++-
 security/integrity/ima/ima.h      | 24 +++++++++++++++++++++++-
 security/integrity/ima/ima_api.c  |  8 +++++---
 security/integrity/ima/ima_main.c | 15 ++++++++++++---
 security/integrity/ima/ima_ns.c   | 22 ++++++++++++++++++++++
 5 files changed, 65 insertions(+), 8 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index a1ad5384e081..f792ae235424 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -938,7 +938,9 @@ config IMA_NS
 	help
 	  Allow the creation of IMA namespaces for each mount namespace.
 	  Namespaced IMA data enables having IMA features work separately
-	  for each mount namespace.
+	  for each mount namespace. Currently, only the audit status flags
+	  are stored in the namespace, which allows the same file to be
+	  audited each time it is accessed in a new namespace.
 
 endif # NAMESPACES
 
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index e51a39ff75ff..86fd3c1c0caf 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -210,7 +210,8 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
 			   struct evm_ima_xattr_data *xattr_value,
 			   int xattr_len, int pcr);
 void ima_audit_measurement(struct integrity_iint_cache *iint,
-			   const unsigned char *filename);
+			   const unsigned char *filename,
+			   struct ns_status *status);
 int ima_alloc_init_template(struct ima_event_data *event_data,
 			    struct ima_template_entry **entry);
 int ima_store_template(struct ima_template_entry *entry, int violation,
@@ -299,6 +300,9 @@ static inline int ima_read_xattr(struct dentry *dentry,
 
 #endif /* CONFIG_IMA_APPRAISE */
 
+#define IMA_NS_STATUS_ACTIONS   IMA_AUDIT
+#define IMA_NS_STATUS_FLAGS     IMA_AUDITED
+
 int ima_ns_init(void);
 struct ima_namespace;
 int ima_init_namespace(struct ima_namespace *ns);
@@ -306,12 +310,30 @@ int ima_init_namespace(struct ima_namespace *ns);
 #ifdef CONFIG_IMA_NS
 struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
 				    struct inode *inode);
+unsigned long iint_flags(struct integrity_iint_cache *iint,
+			 struct ns_status *status);
+unsigned long set_iint_flags(struct integrity_iint_cache *iint,
+			     struct ns_status *status, unsigned long flags);
 #else
 static inline struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
 						  struct inode *inode)
 {
 	return NULL;
 }
+
+static inline unsigned long iint_flags(struct integrity_iint_cache *iint,
+				       struct ns_status *status)
+{
+	return iint->flags;
+}
+
+static inline unsigned long set_iint_flags(struct integrity_iint_cache *iint,
+					   struct ns_status *status,
+					   unsigned long flags)
+{
+	iint->flags = flags;
+	return flags;
+}
 #endif /* CONFIG_IMA_NS */
 
 /* LSM based policy rules require audit */
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c7e8db0ea4c0..ee55dfd6afdb 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -304,15 +304,17 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
 }
 
 void ima_audit_measurement(struct integrity_iint_cache *iint,
-			   const unsigned char *filename)
+			   const unsigned char *filename,
+			   struct ns_status *status)
 {
 	struct audit_buffer *ab;
 	char hash[(iint->ima_hash->length * 2) + 1];
 	const char *algo_name = hash_algo_name[iint->ima_hash->algo];
 	char algo_hash[sizeof(hash) + strlen(algo_name) + 2];
 	int i;
+	unsigned long flags = iint_flags(iint, status);
 
-	if (iint->flags & IMA_AUDITED)
+	if (flags & IMA_AUDITED)
 		return;
 
 	for (i = 0; i < iint->ima_hash->length; i++)
@@ -333,7 +335,7 @@ void ima_audit_measurement(struct integrity_iint_cache *iint,
 	audit_log_task_info(ab, current);
 	audit_log_end(ab);
 
-	iint->flags |= IMA_AUDITED;
+	set_iint_flags(iint, status, flags | IMA_AUDITED);
 }
 
 /*
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 770654694efc..9328e57d3f09 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -164,6 +164,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 {
 	struct inode *inode = file_inode(file);
 	struct integrity_iint_cache *iint = NULL;
+	struct ns_status *status = NULL;
 	struct ima_template_desc *template_desc;
 	char *pathbuf = NULL;
 	char filename[NAME_MAX];
@@ -174,6 +175,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 	int xattr_len = 0;
 	bool violation_check;
 	enum hash_algo hash_algo;
+	unsigned long flags;
 
 	if (!ima_policy_flag || !S_ISREG(inode->i_mode))
 		return 0;
@@ -200,6 +202,12 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 		iint = integrity_inode_get(inode);
 		if (!iint)
 			goto out;
+
+		if (action & IMA_NS_STATUS_ACTIONS) {
+			status = ima_get_ns_status(get_current_ns(), inode);
+			if (IS_ERR(status))
+				goto out;
+		}
 	}
 
 	if (violation_check) {
@@ -215,9 +223,10 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 	 * (IMA_MEASURE, IMA_MEASURED, IMA_XXXX_APPRAISE, IMA_XXXX_APPRAISED,
 	 *  IMA_AUDIT, IMA_AUDITED)
 	 */
-	iint->flags |= action;
+	flags = iint_flags(iint, status);
+	flags = set_iint_flags(iint, status, flags | action);
 	action &= IMA_DO_MASK;
-	action &= ~((iint->flags & (IMA_DONE_MASK ^ IMA_MEASURED)) >> 1);
+	action &= ~((flags & (IMA_DONE_MASK ^ IMA_MEASURED)) >> 1);
 
 	/* If target pcr is already measured, unset IMA_MEASURE action */
 	if ((action & IMA_MEASURE) && (iint->measured_pcrs & (0x1 << pcr)))
@@ -252,7 +261,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 		rc = ima_appraise_measurement(func, iint, file, pathname,
 					      xattr_value, xattr_len, opened);
 	if (action & IMA_AUDIT)
-		ima_audit_measurement(iint, pathname);
+		ima_audit_measurement(iint, pathname, status);
 
 	if ((file->f_flags & O_DIRECT) && (iint->flags & IMA_PERMIT_DIRECTIO))
 		rc = 0;
diff --git a/security/integrity/ima/ima_ns.c b/security/integrity/ima/ima_ns.c
index ef70fd1e25c6..8e2e388ea808 100644
--- a/security/integrity/ima/ima_ns.c
+++ b/security/integrity/ima/ima_ns.c
@@ -201,3 +201,25 @@ struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
 
 	return status;
 }
+
+#define IMA_NS_STATUS_ACTIONS	IMA_AUDIT
+#define IMA_NS_STATUS_FLAGS	IMA_AUDITED
+
+unsigned long iint_flags(struct integrity_iint_cache *iint,
+			 struct ns_status *status)
+{
+	if (!status)
+		return iint->flags;
+
+	return (iint->flags & ~IMA_NS_STATUS_FLAGS) | (status->flags & IMA_NS_STATUS_FLAGS);
+}
+
+unsigned long set_iint_flags(struct integrity_iint_cache *iint,
+			     struct ns_status *status, unsigned long flags)
+{
+	iint->flags = flags;
+	if (status)
+		status->flags = flags & IMA_NS_STATUS_FLAGS;
+
+	return flags;
+}
-- 
2.14.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support
  2018-03-27 13:57 ` [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support Stefan Berger
@ 2018-03-27 23:01   ` Eric W. Biederman
  2018-03-28 11:10     ` Stefan Berger
  2018-04-13 16:25     ` Mimi Zohar
  0 siblings, 2 replies; 19+ messages in thread
From: Eric W. Biederman @ 2018-03-27 23:01 UTC (permalink / raw)
  To: Stefan Berger
  Cc: linux-integrity, containers, linux-kernel, linux-security-module,
	tycho, serge, sunyuqiong1988, david.safford, mkayaalp,
	James.Bottomley, zohar, Yuqiong Sun, Mehmet Kayaalp

Stefan Berger <stefanb@linux.vnet.ibm.com> writes:

> From: Yuqiong Sun <suny@us.ibm.com>
>
> Add new CONFIG_IMA_NS config option.  Let clone() create a new IMA
> namespace upon CLONE_NEWUSER flag. Attach the ima_ns data structure
> to user_namespace. ima_ns is allocated and freed upon IMA namespace
> creation and exit, which is tied to USER namespace creation and exit.
> Currently, the ima_ns contains no useful IMA data but only a dummy
> interface. This patch creates the framework for namespacing the different
> aspects of IMA (eg. IMA-audit, IMA-measurement, IMA-appraisal).

Tying IMA to the user namespace is far better than tying IMA
to the mount namespace.  It may even be the proper answer.


You had asked what it would take to unstick this so you won't have
problems next time you post and I did not get as far as answering.

I had a conversation a while back with Mimi and I believe what was
agreed was that IMA to start doing it's thing early needs a write
to securityfs/imafs.

As such I expect the best way to create the ima namespace is by simply
writing to securityfs/imafs.  Possibly before the user namespace is
even unshared.  That would allow IMA to keep track of things from
before a container is created.

Eric

> Changelog:
> v3:
> * Use CLONE_NEWUSER instead of CLONE_NEWNW flag
>
> v2:
> * Moved ima_init_ns and related functions into own file that is
>   always compiled; init_ima_ns will always be there
> * Fixed putting of imans->parent
> * Move IMA namespace creation from nsproxy into mount namespace
>   code; get rid of procfs operations for IMA namespace
>
> v1:
> * Use CLONE_NEWNS instead of a new CLONE_NEWIMA flag
> * Use existing ima.h headers
> * Move the ima_namespace.c to security/integrity/ima/ima_ns.c
> * Fix typo INFO->INO
> * Each namespace free's itself, removed recursively free'ing
>   until init_ima_ns from free_ima_ns()
>
> Signed-off-by: Yuqiong Sun <suny@us.ibm.com>
> Signed-off-by: Mehmet Kayaalp <mkayaalp@linux.vnet.ibm.com>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> ---
>  include/linux/ima.h                      | 64 ++++++++++++++++++++++++
>  include/linux/user_namespace.h           |  4 ++
>  init/Kconfig                             |  8 +++
>  kernel/user.c                            |  7 +++
>  kernel/user_namespace.c                  | 18 +++++++
>  security/integrity/ima/Makefile          |  3 +-
>  security/integrity/ima/ima.h             |  4 ++
>  security/integrity/ima/ima_init.c        |  4 ++
>  security/integrity/ima/ima_init_ima_ns.c | 37 ++++++++++++++
>  security/integrity/ima/ima_ns.c          | 86 ++++++++++++++++++++++++++++++++
>  10 files changed, 234 insertions(+), 1 deletion(-)
>  create mode 100644 security/integrity/ima/ima_init_ima_ns.c
>  create mode 100644 security/integrity/ima/ima_ns.c
>
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 0e4647e0eb60..8bca67df0ad3 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -12,6 +12,7 @@
>  
>  #include <linux/fs.h>
>  #include <linux/kexec.h>
> +#include <linux/user_namespace.h>
>  struct linux_binprm;
>  
>  #ifdef CONFIG_IMA
> @@ -105,4 +106,67 @@ static inline int ima_inode_removexattr(struct dentry *dentry,
>  	return 0;
>  }
>  #endif /* CONFIG_IMA_APPRAISE */
> +
> +struct ima_namespace {
> +	struct kref kref;
> +	struct ima_namespace *parent;
> +};
> +
> +extern struct ima_namespace init_ima_ns;
> +
> +void imans_install(struct ns_common *new);
> +
> +static inline struct ima_namespace *to_ima_ns(struct ns_common *ns)
> +{
> +	return container_of(ns, struct user_namespace, ns)->ima_ns;
> +}
> +
> +#ifdef CONFIG_IMA_NS
> +
> +void free_ima_ns(struct kref *kref);
> +
> +static inline struct ima_namespace *get_ima_ns(struct ima_namespace *ns)
> +{
> +	BUG_ON(!ns);
> +	if (ns)
> +		kref_get(&ns->kref);
> +	return ns;
> +}
> +
> +static inline void put_ima_ns(struct ima_namespace *ns)
> +{
> +	BUG_ON(!ns);
> +	if (ns)
> +		kref_put(&ns->kref, free_ima_ns);
> +}
> +
> +struct ima_namespace *copy_ima(struct ima_namespace *old_ns);
> +
> +static inline struct ima_namespace *get_current_ns(void)
> +{
> +	return current_user_ns()->ima_ns;
> +}
> +
> +#else
> +
> +static inline struct ima_namespace *get_ima_ns(struct ima_namespace *ns)
> +{
> +	return ns;
> +}
> +
> +static inline void put_ima_ns(struct ima_namespace *ns)
> +{
> +	return;
> +}
> +
> +static inline struct ima_namespace *copy_ima(struct ima_namespace *old_ns)
> +{
> +	return old_ns;
> +}
> +
> +static inline struct ima_namespace *get_current_ns(void)
> +{
> +	return NULL;
> +}
> +#endif /* CONFIG_IMA_NS */
>  #endif /* _LINUX_IMA_H */
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index d6b74b91096b..8884b22d991c 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -36,6 +36,7 @@ struct uid_gid_map { /* 64 bytes -- 1 cache line */
>  #define USERNS_INIT_FLAGS USERNS_SETGROUPS_ALLOWED
>  
>  struct ucounts;
> +struct ima_namespace;
>  
>  enum ucount_type {
>  	UCOUNT_USER_NAMESPACES,
> @@ -76,6 +77,9 @@ struct user_namespace {
>  #endif
>  	struct ucounts		*ucounts;
>  	int ucount_max[UCOUNT_COUNTS];
> +#ifdef CONFIG_IMA
> +	struct ima_namespace	*ima_ns;
> +#endif
>  } __randomize_layout;
>  
>  struct ucounts {
> diff --git a/init/Kconfig b/init/Kconfig
> index a9a2e2c86671..a1ad5384e081 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -931,6 +931,14 @@ config NET_NS
>  	help
>  	  Allow user space to create what appear to be multiple instances
>  	  of the network stack.
> +config IMA_NS
> +	bool "IMA namespace"
> +	depends on IMA
> +	default y
> +	help
> +	  Allow the creation of IMA namespaces for each mount namespace.
> +	  Namespaced IMA data enables having IMA features work separately
> +	  for each mount namespace.
>  
>  endif # NAMESPACES
>  
> diff --git a/kernel/user.c b/kernel/user.c
> index 9a20acce460d..31c946f3adce 100644
> --- a/kernel/user.c
> +++ b/kernel/user.c
> @@ -19,6 +19,10 @@
>  #include <linux/user_namespace.h>
>  #include <linux/proc_ns.h>
>  
> +#ifdef CONFIG_IMA
> +extern struct ima_namespace init_ima_ns;
> +#endif
> +
>  /*
>   * userns count is 1 for root user, 1 for init_uts_ns,
>   * and 1 for... ?
> @@ -66,6 +70,9 @@ struct user_namespace init_user_ns = {
>  	.persistent_keyring_register_sem =
>  	__RWSEM_INITIALIZER(init_user_ns.persistent_keyring_register_sem),
>  #endif
> +#ifdef CONFIG_IMA
> +	.ima_ns = &init_ima_ns,
> +#endif
>  };
>  EXPORT_SYMBOL_GPL(init_user_ns);
>  
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 246d4d4ce5c7..7d6e7d6e6a34 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -25,6 +25,7 @@
>  #include <linux/fs_struct.h>
>  #include <linux/bsearch.h>
>  #include <linux/sort.h>
> +#include <linux/ima.h>
>  
>  static struct kmem_cache *user_ns_cachep __read_mostly;
>  static DEFINE_MUTEX(userns_state_mutex);
> @@ -140,8 +141,20 @@ int create_user_ns(struct cred *new)
>  	if (!setup_userns_sysctls(ns))
>  		goto fail_keyring;

Having the functions be #ifdef'd rather than the code would
be preferabble.
>  
> +#if CONFIG_IMA
> +	ns->ima_ns = copy_ima(parent_ns->ima_ns);
> +	if (IS_ERR(ns->ima_ns)) {
> +		ret = PTR_ERR(ns->ima_ns);
> +		goto fail_userns_sysctls;
> +	}
> +#endif
> +
>  	set_cred_user_ns(new, ns);
>  	return 0;
> +#if CONFIG_IMA
> +fail_userns_sysctls:
> +	retire_userns_sysctls(ns);
> +#endif
>  fail_keyring:
>  #ifdef CONFIG_PERSISTENT_KEYRINGS
>  	key_put(ns->persistent_keyring_register);
> @@ -195,6 +208,9 @@ static void free_user_ns(struct work_struct *work)
>  			kfree(ns->projid_map.forward);
>  			kfree(ns->projid_map.reverse);
>  		}
> +#ifdef CONFIG_IMA
> +		put_ima_ns(ns->ima_ns);
> +#endif
>  		retire_userns_sysctls(ns);
>  #ifdef CONFIG_PERSISTENT_KEYRINGS
>  		key_put(ns->persistent_keyring_register);
> @@ -1285,6 +1301,8 @@ static int userns_install(struct nsproxy *nsproxy, struct ns_common *ns)
>  	put_user_ns(cred->user_ns);
>  	set_cred_user_ns(cred, get_user_ns(user_ns));
>  
> +	imans_install(ns);
> +
>  	return commit_creds(cred);
>  }
>  
> diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
> index d921dc4f9eb0..cc60f726e651 100644
> --- a/security/integrity/ima/Makefile
> +++ b/security/integrity/ima/Makefile
> @@ -7,7 +7,8 @@
>  obj-$(CONFIG_IMA) += ima.o
>  
>  ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
> -	 ima_policy.o ima_template.o ima_template_lib.o
> +	 ima_policy.o ima_template.o ima_template_lib.o ima_init_ima_ns.o
>  ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
> +ima-$(CONFIG_IMA_NS) += ima_ns.o
>  ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
>  obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index d52b487ad259..e98c11c7cf75 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -291,6 +291,10 @@ static inline int ima_read_xattr(struct dentry *dentry,
>  
>  #endif /* CONFIG_IMA_APPRAISE */
>  
> +int ima_ns_init(void);
> +struct ima_namespace;
> +int ima_init_namespace(struct ima_namespace *ns);
> +
>  /* LSM based policy rules require audit */
>  #ifdef CONFIG_IMA_LSM_RULES
>  
> diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> index 2967d497a665..7f884a71fa1c 100644
> --- a/security/integrity/ima/ima_init.c
> +++ b/security/integrity/ima/ima_init.c
> @@ -137,5 +137,9 @@ int __init ima_init(void)
>  
>  	ima_init_policy();
>  
> +	rc = ima_ns_init();
> +	if (rc != 0)
> +		return rc;
> +
>  	return ima_fs_init();
>  }
> diff --git a/security/integrity/ima/ima_init_ima_ns.c b/security/integrity/ima/ima_init_ima_ns.c
> new file mode 100644
> index 000000000000..52cb94b1d392
> --- /dev/null
> +++ b/security/integrity/ima/ima_init_ima_ns.c
> @@ -0,0 +1,37 @@
> +/*
> + * Copyright (C) 2016-2018 IBM Corporation
> + * Author:
> + *   Yuqiong Sun <suny@us.ibm.com>
> + *   Stefan Berger <stefanb@linux.vnet.ibm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, version 2 of the License.
> + */
> +
> +#include <linux/export.h>
> +#include <linux/user_namespace.h>
> +#include <linux/ima.h>
> +
> +int ima_init_namespace(struct ima_namespace *ns)
> +{
> +	return 0;
> +}
> +
> +int __init ima_ns_init(void)
> +{
> +	return ima_init_namespace(&init_ima_ns);
> +}
> +
> +struct ima_namespace init_ima_ns = {
> +	.kref = KREF_INIT(1),
> +	.parent = NULL,
> +};
> +EXPORT_SYMBOL(init_ima_ns);
> +
> +void imans_install(struct ns_common *new)
> +{
> +	struct ima_namespace *ns = to_ima_ns(new);
> +
> +	get_ima_ns(ns);
> +}
> diff --git a/security/integrity/ima/ima_ns.c b/security/integrity/ima/ima_ns.c
> new file mode 100644
> index 000000000000..62148908015a
> --- /dev/null
> +++ b/security/integrity/ima/ima_ns.c
> @@ -0,0 +1,86 @@
> +/*
> + * Copyright (C) 2016-2018 IBM Corporation
> + * Author:
> + *  Yuqiong Sun <suny@us.ibm.com>
> + *  Stefan Berger <stefanb@linux.vnet.ibm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, version 2 of the License.
> + */
> +
> +#include <linux/kref.h>
> +#include <linux/slab.h>
> +#include <linux/ima.h>
> +#include <linux/mount.h>
> +
> +#include "ima.h"
> +
> +static struct ima_namespace *create_ima_ns(void)
> +{
> +	struct ima_namespace *ima_ns;
> +
> +	ima_ns = kmalloc(sizeof(*ima_ns), GFP_KERNEL);
> +	if (ima_ns)
> +		kref_init(&ima_ns->kref);
> +
> +	return ima_ns;
> +}
> +
> +/**
> + * Clone a new ns copying an original ima namespace, setting refcount to 1
> + *
> + * @old_ns: old ima namespace to clone
> + * Return ERR_PTR(-ENOMEM) on error (failure to kmalloc), new ns otherwise
> + */
> +static struct ima_namespace *clone_ima_ns(struct ima_namespace *old_ns)
> +{
> +	struct ima_namespace *ns;
> +
> +	ns = create_ima_ns();
> +	if (!ns)
> +		return ERR_PTR(-ENOMEM);
> +
> +	get_ima_ns(old_ns);
> +	ns->parent = old_ns;
> +
> +	ima_init_namespace(ns);
> +
> +	return ns;
> +}
> +
> +/**
> + * Copy task's ima namespace, or clone it if flags specifies CLONE_NEWNS.
> + *
> + * @flags: flags used in the clone syscall
> + * @old_ns: old ima namespace to clone
> + */
> +
> +struct ima_namespace *copy_ima(struct ima_namespace *old_ns)
> +{
> +	struct ima_namespace *new_ns;
> +
> +	BUG_ON(!old_ns);
> +	get_ima_ns(old_ns);
> +
> +	new_ns = clone_ima_ns(old_ns);
> +	put_ima_ns(old_ns);
> +
> +	return new_ns;
> +}
> +
> +static void destroy_ima_ns(struct ima_namespace *ns)
> +{
> +	put_ima_ns(ns->parent);
> +	kfree(ns);
> +}
> +
> +void free_ima_ns(struct kref *kref)
> +{
> +	struct ima_namespace *ns;
> +
> +	ns = container_of(kref, struct ima_namespace, kref);
> +	BUG_ON(ns == &init_ima_ns);
> +
> +	destroy_ima_ns(ns);
> +}

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support
  2018-03-27 23:01   ` Eric W. Biederman
@ 2018-03-28 11:10     ` Stefan Berger
  2018-03-28 12:14       ` Dr. Greg Wettstein
  2018-04-18 15:59       ` John Johansen
  2018-04-13 16:25     ` Mimi Zohar
  1 sibling, 2 replies; 19+ messages in thread
From: Stefan Berger @ 2018-03-28 11:10 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: linux-integrity, containers, linux-kernel, linux-security-module,
	tycho, serge, sunyuqiong1988, david.safford, mkayaalp,
	James.Bottomley, zohar, Yuqiong Sun, Mehmet Kayaalp

On 03/27/2018 07:01 PM, Eric W. Biederman wrote:
> Stefan Berger <stefanb@linux.vnet.ibm.com> writes:
>
>> From: Yuqiong Sun <suny@us.ibm.com>
>>
>> Add new CONFIG_IMA_NS config option.  Let clone() create a new IMA
>> namespace upon CLONE_NEWUSER flag. Attach the ima_ns data structure
>> to user_namespace. ima_ns is allocated and freed upon IMA namespace
>> creation and exit, which is tied to USER namespace creation and exit.
>> Currently, the ima_ns contains no useful IMA data but only a dummy
>> interface. This patch creates the framework for namespacing the different
>> aspects of IMA (eg. IMA-audit, IMA-measurement, IMA-appraisal).
> Tying IMA to the user namespace is far better than tying IMA
> to the mount namespace.  It may even be the proper answer.
>
>
> You had asked what it would take to unstick this so you won't have
> problems next time you post and I did not get as far as answering.
>
> I had a conversation a while back with Mimi and I believe what was
> agreed was that IMA to start doing it's thing early needs a write
> to securityfs/imafs.

Above you say 'proper answer' for user namespace. Now this sounds like 
making it independent.

>
> As such I expect the best way to create the ima namespace is by simply
> writing to securityfs/imafs.  Possibly before the user namespace is
> even unshared.  That would allow IMA to keep track of things from
> before a container is created.

So you are saying to not tie it to user namespace but make it an 
independent namespace and to not use a clone flag (0x1000) but use the 
filesystem to spawn a new namespace. Should that be an IMA specific file 
or a file that can be shared with other subsystems?

    Stefan

>
> Eric
>
>> Changelog:
>> v3:
>> * Use CLONE_NEWUSER instead of CLONE_NEWNW flag
>>
>> v2:
>> * Moved ima_init_ns and related functions into own file that is
>>    always compiled; init_ima_ns will always be there
>> * Fixed putting of imans->parent
>> * Move IMA namespace creation from nsproxy into mount namespace
>>    code; get rid of procfs operations for IMA namespace
>>
>> v1:
>> * Use CLONE_NEWNS instead of a new CLONE_NEWIMA flag
>> * Use existing ima.h headers
>> * Move the ima_namespace.c to security/integrity/ima/ima_ns.c
>> * Fix typo INFO->INO
>> * Each namespace free's itself, removed recursively free'ing
>>    until init_ima_ns from free_ima_ns()
>>
>> Signed-off-by: Yuqiong Sun <suny@us.ibm.com>
>> Signed-off-by: Mehmet Kayaalp <mkayaalp@linux.vnet.ibm.com>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> ---
>>   include/linux/ima.h                      | 64 ++++++++++++++++++++++++
>>   include/linux/user_namespace.h           |  4 ++
>>   init/Kconfig                             |  8 +++
>>   kernel/user.c                            |  7 +++
>>   kernel/user_namespace.c                  | 18 +++++++
>>   security/integrity/ima/Makefile          |  3 +-
>>   security/integrity/ima/ima.h             |  4 ++
>>   security/integrity/ima/ima_init.c        |  4 ++
>>   security/integrity/ima/ima_init_ima_ns.c | 37 ++++++++++++++
>>   security/integrity/ima/ima_ns.c          | 86 ++++++++++++++++++++++++++++++++
>>   10 files changed, 234 insertions(+), 1 deletion(-)
>>   create mode 100644 security/integrity/ima/ima_init_ima_ns.c
>>   create mode 100644 security/integrity/ima/ima_ns.c
>>
>> diff --git a/include/linux/ima.h b/include/linux/ima.h
>> index 0e4647e0eb60..8bca67df0ad3 100644
>> --- a/include/linux/ima.h
>> +++ b/include/linux/ima.h
>> @@ -12,6 +12,7 @@
>>   
>>   #include <linux/fs.h>
>>   #include <linux/kexec.h>
>> +#include <linux/user_namespace.h>
>>   struct linux_binprm;
>>   
>>   #ifdef CONFIG_IMA
>> @@ -105,4 +106,67 @@ static inline int ima_inode_removexattr(struct dentry *dentry,
>>   	return 0;
>>   }
>>   #endif /* CONFIG_IMA_APPRAISE */
>> +
>> +struct ima_namespace {
>> +	struct kref kref;
>> +	struct ima_namespace *parent;
>> +};
>> +
>> +extern struct ima_namespace init_ima_ns;
>> +
>> +void imans_install(struct ns_common *new);
>> +
>> +static inline struct ima_namespace *to_ima_ns(struct ns_common *ns)
>> +{
>> +	return container_of(ns, struct user_namespace, ns)->ima_ns;
>> +}
>> +
>> +#ifdef CONFIG_IMA_NS
>> +
>> +void free_ima_ns(struct kref *kref);
>> +
>> +static inline struct ima_namespace *get_ima_ns(struct ima_namespace *ns)
>> +{
>> +	BUG_ON(!ns);
>> +	if (ns)
>> +		kref_get(&ns->kref);
>> +	return ns;
>> +}
>> +
>> +static inline void put_ima_ns(struct ima_namespace *ns)
>> +{
>> +	BUG_ON(!ns);
>> +	if (ns)
>> +		kref_put(&ns->kref, free_ima_ns);
>> +}
>> +
>> +struct ima_namespace *copy_ima(struct ima_namespace *old_ns);
>> +
>> +static inline struct ima_namespace *get_current_ns(void)
>> +{
>> +	return current_user_ns()->ima_ns;
>> +}
>> +
>> +#else
>> +
>> +static inline struct ima_namespace *get_ima_ns(struct ima_namespace *ns)
>> +{
>> +	return ns;
>> +}
>> +
>> +static inline void put_ima_ns(struct ima_namespace *ns)
>> +{
>> +	return;
>> +}
>> +
>> +static inline struct ima_namespace *copy_ima(struct ima_namespace *old_ns)
>> +{
>> +	return old_ns;
>> +}
>> +
>> +static inline struct ima_namespace *get_current_ns(void)
>> +{
>> +	return NULL;
>> +}
>> +#endif /* CONFIG_IMA_NS */
>>   #endif /* _LINUX_IMA_H */
>> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
>> index d6b74b91096b..8884b22d991c 100644
>> --- a/include/linux/user_namespace.h
>> +++ b/include/linux/user_namespace.h
>> @@ -36,6 +36,7 @@ struct uid_gid_map { /* 64 bytes -- 1 cache line */
>>   #define USERNS_INIT_FLAGS USERNS_SETGROUPS_ALLOWED
>>   
>>   struct ucounts;
>> +struct ima_namespace;
>>   
>>   enum ucount_type {
>>   	UCOUNT_USER_NAMESPACES,
>> @@ -76,6 +77,9 @@ struct user_namespace {
>>   #endif
>>   	struct ucounts		*ucounts;
>>   	int ucount_max[UCOUNT_COUNTS];
>> +#ifdef CONFIG_IMA
>> +	struct ima_namespace	*ima_ns;
>> +#endif
>>   } __randomize_layout;
>>   
>>   struct ucounts {
>> diff --git a/init/Kconfig b/init/Kconfig
>> index a9a2e2c86671..a1ad5384e081 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -931,6 +931,14 @@ config NET_NS
>>   	help
>>   	  Allow user space to create what appear to be multiple instances
>>   	  of the network stack.
>> +config IMA_NS
>> +	bool "IMA namespace"
>> +	depends on IMA
>> +	default y
>> +	help
>> +	  Allow the creation of IMA namespaces for each mount namespace.
>> +	  Namespaced IMA data enables having IMA features work separately
>> +	  for each mount namespace.
>>   
>>   endif # NAMESPACES
>>   
>> diff --git a/kernel/user.c b/kernel/user.c
>> index 9a20acce460d..31c946f3adce 100644
>> --- a/kernel/user.c
>> +++ b/kernel/user.c
>> @@ -19,6 +19,10 @@
>>   #include <linux/user_namespace.h>
>>   #include <linux/proc_ns.h>
>>   
>> +#ifdef CONFIG_IMA
>> +extern struct ima_namespace init_ima_ns;
>> +#endif
>> +
>>   /*
>>    * userns count is 1 for root user, 1 for init_uts_ns,
>>    * and 1 for... ?
>> @@ -66,6 +70,9 @@ struct user_namespace init_user_ns = {
>>   	.persistent_keyring_register_sem =
>>   	__RWSEM_INITIALIZER(init_user_ns.persistent_keyring_register_sem),
>>   #endif
>> +#ifdef CONFIG_IMA
>> +	.ima_ns = &init_ima_ns,
>> +#endif
>>   };
>>   EXPORT_SYMBOL_GPL(init_user_ns);
>>   
>> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
>> index 246d4d4ce5c7..7d6e7d6e6a34 100644
>> --- a/kernel/user_namespace.c
>> +++ b/kernel/user_namespace.c
>> @@ -25,6 +25,7 @@
>>   #include <linux/fs_struct.h>
>>   #include <linux/bsearch.h>
>>   #include <linux/sort.h>
>> +#include <linux/ima.h>
>>   
>>   static struct kmem_cache *user_ns_cachep __read_mostly;
>>   static DEFINE_MUTEX(userns_state_mutex);
>> @@ -140,8 +141,20 @@ int create_user_ns(struct cred *new)
>>   	if (!setup_userns_sysctls(ns))
>>   		goto fail_keyring;
> Having the functions be #ifdef'd rather than the code would
> be preferabble.
>>   
>> +#if CONFIG_IMA
>> +	ns->ima_ns = copy_ima(parent_ns->ima_ns);
>> +	if (IS_ERR(ns->ima_ns)) {
>> +		ret = PTR_ERR(ns->ima_ns);
>> +		goto fail_userns_sysctls;
>> +	}
>> +#endif
>> +
>>   	set_cred_user_ns(new, ns);
>>   	return 0;
>> +#if CONFIG_IMA
>> +fail_userns_sysctls:
>> +	retire_userns_sysctls(ns);
>> +#endif
>>   fail_keyring:
>>   #ifdef CONFIG_PERSISTENT_KEYRINGS
>>   	key_put(ns->persistent_keyring_register);
>> @@ -195,6 +208,9 @@ static void free_user_ns(struct work_struct *work)
>>   			kfree(ns->projid_map.forward);
>>   			kfree(ns->projid_map.reverse);
>>   		}
>> +#ifdef CONFIG_IMA
>> +		put_ima_ns(ns->ima_ns);
>> +#endif
>>   		retire_userns_sysctls(ns);
>>   #ifdef CONFIG_PERSISTENT_KEYRINGS
>>   		key_put(ns->persistent_keyring_register);
>> @@ -1285,6 +1301,8 @@ static int userns_install(struct nsproxy *nsproxy, struct ns_common *ns)
>>   	put_user_ns(cred->user_ns);
>>   	set_cred_user_ns(cred, get_user_ns(user_ns));
>>   
>> +	imans_install(ns);
>> +
>>   	return commit_creds(cred);
>>   }
>>   
>> diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
>> index d921dc4f9eb0..cc60f726e651 100644
>> --- a/security/integrity/ima/Makefile
>> +++ b/security/integrity/ima/Makefile
>> @@ -7,7 +7,8 @@
>>   obj-$(CONFIG_IMA) += ima.o
>>   
>>   ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
>> -	 ima_policy.o ima_template.o ima_template_lib.o
>> +	 ima_policy.o ima_template.o ima_template_lib.o ima_init_ima_ns.o
>>   ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
>> +ima-$(CONFIG_IMA_NS) += ima_ns.o
>>   ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
>>   obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>> index d52b487ad259..e98c11c7cf75 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -291,6 +291,10 @@ static inline int ima_read_xattr(struct dentry *dentry,
>>   
>>   #endif /* CONFIG_IMA_APPRAISE */
>>   
>> +int ima_ns_init(void);
>> +struct ima_namespace;
>> +int ima_init_namespace(struct ima_namespace *ns);
>> +
>>   /* LSM based policy rules require audit */
>>   #ifdef CONFIG_IMA_LSM_RULES
>>   
>> diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
>> index 2967d497a665..7f884a71fa1c 100644
>> --- a/security/integrity/ima/ima_init.c
>> +++ b/security/integrity/ima/ima_init.c
>> @@ -137,5 +137,9 @@ int __init ima_init(void)
>>   
>>   	ima_init_policy();
>>   
>> +	rc = ima_ns_init();
>> +	if (rc != 0)
>> +		return rc;
>> +
>>   	return ima_fs_init();
>>   }
>> diff --git a/security/integrity/ima/ima_init_ima_ns.c b/security/integrity/ima/ima_init_ima_ns.c
>> new file mode 100644
>> index 000000000000..52cb94b1d392
>> --- /dev/null
>> +++ b/security/integrity/ima/ima_init_ima_ns.c
>> @@ -0,0 +1,37 @@
>> +/*
>> + * Copyright (C) 2016-2018 IBM Corporation
>> + * Author:
>> + *   Yuqiong Sun <suny@us.ibm.com>
>> + *   Stefan Berger <stefanb@linux.vnet.ibm.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation, version 2 of the License.
>> + */
>> +
>> +#include <linux/export.h>
>> +#include <linux/user_namespace.h>
>> +#include <linux/ima.h>
>> +
>> +int ima_init_namespace(struct ima_namespace *ns)
>> +{
>> +	return 0;
>> +}
>> +
>> +int __init ima_ns_init(void)
>> +{
>> +	return ima_init_namespace(&init_ima_ns);
>> +}
>> +
>> +struct ima_namespace init_ima_ns = {
>> +	.kref = KREF_INIT(1),
>> +	.parent = NULL,
>> +};
>> +EXPORT_SYMBOL(init_ima_ns);
>> +
>> +void imans_install(struct ns_common *new)
>> +{
>> +	struct ima_namespace *ns = to_ima_ns(new);
>> +
>> +	get_ima_ns(ns);
>> +}
>> diff --git a/security/integrity/ima/ima_ns.c b/security/integrity/ima/ima_ns.c
>> new file mode 100644
>> index 000000000000..62148908015a
>> --- /dev/null
>> +++ b/security/integrity/ima/ima_ns.c
>> @@ -0,0 +1,86 @@
>> +/*
>> + * Copyright (C) 2016-2018 IBM Corporation
>> + * Author:
>> + *  Yuqiong Sun <suny@us.ibm.com>
>> + *  Stefan Berger <stefanb@linux.vnet.ibm.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation, version 2 of the License.
>> + */
>> +
>> +#include <linux/kref.h>
>> +#include <linux/slab.h>
>> +#include <linux/ima.h>
>> +#include <linux/mount.h>
>> +
>> +#include "ima.h"
>> +
>> +static struct ima_namespace *create_ima_ns(void)
>> +{
>> +	struct ima_namespace *ima_ns;
>> +
>> +	ima_ns = kmalloc(sizeof(*ima_ns), GFP_KERNEL);
>> +	if (ima_ns)
>> +		kref_init(&ima_ns->kref);
>> +
>> +	return ima_ns;
>> +}
>> +
>> +/**
>> + * Clone a new ns copying an original ima namespace, setting refcount to 1
>> + *
>> + * @old_ns: old ima namespace to clone
>> + * Return ERR_PTR(-ENOMEM) on error (failure to kmalloc), new ns otherwise
>> + */
>> +static struct ima_namespace *clone_ima_ns(struct ima_namespace *old_ns)
>> +{
>> +	struct ima_namespace *ns;
>> +
>> +	ns = create_ima_ns();
>> +	if (!ns)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	get_ima_ns(old_ns);
>> +	ns->parent = old_ns;
>> +
>> +	ima_init_namespace(ns);
>> +
>> +	return ns;
>> +}
>> +
>> +/**
>> + * Copy task's ima namespace, or clone it if flags specifies CLONE_NEWNS.
>> + *
>> + * @flags: flags used in the clone syscall
>> + * @old_ns: old ima namespace to clone
>> + */
>> +
>> +struct ima_namespace *copy_ima(struct ima_namespace *old_ns)
>> +{
>> +	struct ima_namespace *new_ns;
>> +
>> +	BUG_ON(!old_ns);
>> +	get_ima_ns(old_ns);
>> +
>> +	new_ns = clone_ima_ns(old_ns);
>> +	put_ima_ns(old_ns);
>> +
>> +	return new_ns;
>> +}
>> +
>> +static void destroy_ima_ns(struct ima_namespace *ns)
>> +{
>> +	put_ima_ns(ns->parent);
>> +	kfree(ns);
>> +}
>> +
>> +void free_ima_ns(struct kref *kref)
>> +{
>> +	struct ima_namespace *ns;
>> +
>> +	ns = container_of(kref, struct ima_namespace, kref);
>> +	BUG_ON(ns == &init_ima_ns);
>> +
>> +	destroy_ima_ns(ns);
>> +}

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support
  2018-03-28 11:10     ` Stefan Berger
@ 2018-03-28 12:14       ` Dr. Greg Wettstein
  2018-03-28 12:44         ` Stefan Berger
  2018-04-18 15:59       ` John Johansen
  1 sibling, 1 reply; 19+ messages in thread
From: Dr. Greg Wettstein @ 2018-03-28 12:14 UTC (permalink / raw)
  To: Stefan Berger
  Cc: Eric W. Biederman, linux-integrity, containers, linux-kernel,
	linux-security-module, tycho, serge, sunyuqiong1988,
	david.safford, mkayaalp, James.Bottomley, zohar, Yuqiong Sun,
	Mehmet Kayaalp

On Wed, Mar 28, 2018 at 07:10:12AM -0400, Stefan Berger wrote:

Good morning, I hope the day is starting out well for everyone.

> On 03/27/2018 07:01 PM, Eric W. Biederman wrote:
> >Stefan Berger <stefanb@linux.vnet.ibm.com> writes:
> >
> >>From: Yuqiong Sun <suny@us.ibm.com>
> >>
> >>Add new CONFIG_IMA_NS config option.  Let clone() create a new IMA
> >>namespace upon CLONE_NEWUSER flag. Attach the ima_ns data structure
> >>to user_namespace. ima_ns is allocated and freed upon IMA namespace
> >>creation and exit, which is tied to USER namespace creation and exit.
> >>Currently, the ima_ns contains no useful IMA data but only a dummy
> >>interface. This patch creates the framework for namespacing the different
> >>aspects of IMA (eg. IMA-audit, IMA-measurement, IMA-appraisal).
> >Tying IMA to the user namespace is far better than tying IMA
> >to the mount namespace.  It may even be the proper answer.
> >
> >You had asked what it would take to unstick this so you won't have
> >problems next time you post and I did not get as far as answering.
> >
> >I had a conversation a while back with Mimi and I believe what was
> >agreed was that IMA to start doing it's thing early needs a write
> >to securityfs/imafs.
> 
> Above you say 'proper answer' for user namespace. Now this sounds like 
> making it independent.
>
> >As such I expect the best way to create the ima namespace is by simply
> >writing to securityfs/imafs.  Possibly before the user namespace is
> >even unshared.  That would allow IMA to keep track of things from
> >before a container is created.
>
> So you are saying to not tie it to user namespace but make it an
> independent namespace and to not use a clone flag (0x1000) but use
> the filesystem to spawn a new namespace. Should that be an IMA
> specific file or a file that can be shared with other subsystems?

We've been platforming solutions for about 18 months now on top of a
namespaced IMA implementation that we developed and carry against the
4.4.x kernel.  Technically its not an IMA namespace, but rather a
behavioral namespace, since we implement information exchange event
modeling, conceptually though its all the same and its origins were
IMA.

In some configurations we run unmodified Docker containers inside the
behavioral/IMA namespace.  So if experience is a useful metric the
'integrity' namespace needs to be a first class entity and not
subordinate or tied to any other resource namespaces.  We would also
recommend, again based on our experiences, the use of a clone flag.

FWIW, at this point we have hoisted a lot of the integrity
functionality out of the kernel and up into userspace so it can be run
in a trusted execution environment.  There are always the issues with
kernel<->userspace communication, particularly of the symmetric
variety, but userspace seems to be a much better place for a lot of
this functionality.  If the ELF module discussion is any indication it
appears as if userspace and the kernel may be destined to become more
symbiotic in the future.

Just our two cents.

>    Stefan

Have a good remainder of the week.

Dr. 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
------------------------------------------------------------------------------
"So you got your butt kicked by an 'old' guy.

 Before you taunted him did it ever cross your mind that the $1200
 Schmoelke aero-bars he was laying on and the $900 Rocket7 cycling
 shoes he was wearing might mean that the $10,000 custom bike frame he
 was riding might be used for more than transportation to the Dairy
 Queen each night for a Dilly Bar?"
                                -- Dr. G.W. Wettstein
                                   Resurrection

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support
  2018-03-28 12:14       ` Dr. Greg Wettstein
@ 2018-03-28 12:44         ` Stefan Berger
  0 siblings, 0 replies; 19+ messages in thread
From: Stefan Berger @ 2018-03-28 12:44 UTC (permalink / raw)
  To: Dr. Greg Wettstein
  Cc: Eric W. Biederman, linux-integrity, containers, linux-kernel,
	linux-security-module, tycho, serge, sunyuqiong1988,
	david.safford, mkayaalp, James.Bottomley, zohar, Yuqiong Sun,
	Mehmet Kayaalp

On 03/28/2018 08:14 AM, Dr. Greg Wettstein wrote:
> On Wed, Mar 28, 2018 at 07:10:12AM -0400, Stefan Berger wrote:
>
> Good morning, I hope the day is starting out well for everyone.
>
>> On 03/27/2018 07:01 PM, Eric W. Biederman wrote:
>>> Stefan Berger <stefanb@linux.vnet.ibm.com> writes:
>>>
>>>> From: Yuqiong Sun <suny@us.ibm.com>
>>>>
>>>> Add new CONFIG_IMA_NS config option.  Let clone() create a new IMA
>>>> namespace upon CLONE_NEWUSER flag. Attach the ima_ns data structure
>>>> to user_namespace. ima_ns is allocated and freed upon IMA namespace
>>>> creation and exit, which is tied to USER namespace creation and exit.
>>>> Currently, the ima_ns contains no useful IMA data but only a dummy
>>>> interface. This patch creates the framework for namespacing the different
>>>> aspects of IMA (eg. IMA-audit, IMA-measurement, IMA-appraisal).
>>> Tying IMA to the user namespace is far better than tying IMA
>>> to the mount namespace.  It may even be the proper answer.
>>>
>>> You had asked what it would take to unstick this so you won't have
>>> problems next time you post and I did not get as far as answering.
>>>
>>> I had a conversation a while back with Mimi and I believe what was
>>> agreed was that IMA to start doing it's thing early needs a write
>>> to securityfs/imafs.
>> Above you say 'proper answer' for user namespace. Now this sounds like
>> making it independent.
>>
>>> As such I expect the best way to create the ima namespace is by simply
>>> writing to securityfs/imafs.  Possibly before the user namespace is
>>> even unshared.  That would allow IMA to keep track of things from
>>> before a container is created.
>> So you are saying to not tie it to user namespace but make it an
>> independent namespace and to not use a clone flag (0x1000) but use
>> the filesystem to spawn a new namespace. Should that be an IMA
>> specific file or a file that can be shared with other subsystems?
> We've been platforming solutions for about 18 months now on top of a
> namespaced IMA implementation that we developed and carry against the
> 4.4.x kernel.  Technically its not an IMA namespace, but rather a
> behavioral namespace, since we implement information exchange event
> modeling, conceptually though its all the same and its origins were
> IMA.

Are you intending to make this publicly available and/or contribute it ?
>
> In some configurations we run unmodified Docker containers inside the
> behavioral/IMA namespace.  So if experience is a useful metric the
> 'integrity' namespace needs to be a first class entity and not
> subordinate or tied to any other resource namespaces.  We would also
> recommend, again based on our experiences, the use of a clone flag.

We have been using a clone flag in the first implementation, the mount 
flag afterwards.We treat containers independent of the host, meaning 
that it has its own policy, independent of the host, and allows for 
signed files inside containers to enable IMA-appraisal. It does require 
modifications to user space applications like Docker that have to pick 
up the file signatures.


>
> FWIW, at this point we have hoisted a lot of the integrity
> functionality out of the kernel and up into userspace so it can be run
> in a trusted execution environment.  There are always the issues with
> kernel<->userspace communication, particularly of the symmetric
> variety, but userspace seems to be a much better place for a lot of
> this functionality.  If the ELF module discussion is any indication it

Like what functionality? Are you supporting IMA-appraisal? Are you doing 
IMA-measurements? What about IMA-audit? Following our intended IMA 
namespacing, all of this would be done in the kernel following an IMA 
policy parsed by the kernel.


> appears as if userspace and the kernel may be destined to become more
> symbiotic in the future.
>
> Just our two cents.
>
>>     Stefan
> Have a good remainder of the week.
>
> Dr. 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
> ------------------------------------------------------------------------------
> "So you got your butt kicked by an 'old' guy.
>
>   Before you taunted him did it ever cross your mind that the $1200
>   Schmoelke aero-bars he was laying on and the $900 Rocket7 cycling
>   shoes he was wearing might mean that the $10,000 custom bike frame he
>   was riding might be used for more than transportation to the Dairy
>   Queen each night for a Dilly Bar?"
>                                  -- Dr. G.W. Wettstein
>                                     Resurrection
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support
  2018-03-27 23:01   ` Eric W. Biederman
  2018-03-28 11:10     ` Stefan Berger
@ 2018-04-13 16:25     ` Mimi Zohar
  2018-04-18 16:09       ` John Johansen
  1 sibling, 1 reply; 19+ messages in thread
From: Mimi Zohar @ 2018-04-13 16:25 UTC (permalink / raw)
  To: Eric W. Biederman, Stefan Berger
  Cc: linux-integrity, containers, linux-kernel, linux-security-module,
	tycho, serge, sunyuqiong1988, david.safford, mkayaalp,
	James.Bottomley, Yuqiong Sun, Mehmet Kayaalp, John Johansen

[Cc'ing John Johansen]

On Tue, 2018-03-27 at 18:01 -0500, Eric W. Biederman wrote:
[...]
> As such I expect the best way to create the ima namespace is by simply
> writing to securityfs/imafs.  Possibly before the user namespace is
> even unshared.  That would allow IMA to keep track of things from
> before a container is created.

My initial thought was to stage IMA namespacing with just IMA-audit
first, followed by either IMA-measurement or IMA-appraisal.  This
would allow us to get the basic IMA namespacing framework working and
defer dealing with the securityfs related namespacing of the IMA
policy and measurement list issues to later.

By tying IMA namespacing to a securityfs ima/unshare file, we would
need to address the securityfs issues first.

Mimi

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support
  2018-03-28 11:10     ` Stefan Berger
  2018-03-28 12:14       ` Dr. Greg Wettstein
@ 2018-04-18 15:59       ` John Johansen
  1 sibling, 0 replies; 19+ messages in thread
From: John Johansen @ 2018-04-18 15:59 UTC (permalink / raw)
  To: Stefan Berger, Eric W. Biederman
  Cc: linux-integrity, containers, linux-kernel, linux-security-module,
	tycho, serge, sunyuqiong1988, david.safford, mkayaalp,
	James.Bottomley, zohar, Yuqiong Sun, Mehmet Kayaalp

On 03/28/2018 04:10 AM, Stefan Berger wrote:
> On 03/27/2018 07:01 PM, Eric W. Biederman wrote:
>> Stefan Berger <stefanb@linux.vnet.ibm.com> writes:
>>
>>> From: Yuqiong Sun <suny@us.ibm.com>
>>>
>>> Add new CONFIG_IMA_NS config option.  Let clone() create a new IMA
>>> namespace upon CLONE_NEWUSER flag. Attach the ima_ns data structure
>>> to user_namespace. ima_ns is allocated and freed upon IMA namespace
>>> creation and exit, which is tied to USER namespace creation and exit.
>>> Currently, the ima_ns contains no useful IMA data but only a dummy
>>> interface. This patch creates the framework for namespacing the different
>>> aspects of IMA (eg. IMA-audit, IMA-measurement, IMA-appraisal).
>> Tying IMA to the user namespace is far better than tying IMA
>> to the mount namespace.  It may even be the proper answer.
>>
>>
>> You had asked what it would take to unstick this so you won't have
>> problems next time you post and I did not get as far as answering.
>>
>> I had a conversation a while back with Mimi and I believe what was
>> agreed was that IMA to start doing it's thing early needs a write
>> to securityfs/imafs.
> 
> Above you say 'proper answer' for user namespace. Now this sounds like making it independent.
> 
Agreed, and if we want to broaden out to LSMs implementing namespacing keeping them independent is the correct solution

>>
>> As such I expect the best way to create the ima namespace is by simply
>> writing to securityfs/imafs.  Possibly before the user namespace is
>> even unshared.  That would allow IMA to keep track of things from
>> before a container is created.
> 
> So you are saying to not tie it to user namespace but make it an independent namespace and to not use a clone flag (0x1000) but use the filesystem to spawn a new namespace. Should that be an IMA specific file or a file that can be shared with other subsystems?
> 

A clone flag could work for IMA, but I don't see it working for all the LSMs looking at or doing namespacing, especially if the stacking work ever lands.

As for using an IMA specific file vs shared with the other subsystems, I think subsystem specific makes sense, that or we are going to have to design something that can be shared if LSM stacking ever lands.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support
  2018-04-13 16:25     ` Mimi Zohar
@ 2018-04-18 16:09       ` John Johansen
  2018-04-18 19:57         ` Mimi Zohar
  0 siblings, 1 reply; 19+ messages in thread
From: John Johansen @ 2018-04-18 16:09 UTC (permalink / raw)
  To: Mimi Zohar, Eric W. Biederman, Stefan Berger
  Cc: linux-integrity, containers, linux-kernel, linux-security-module,
	tycho, serge, sunyuqiong1988, david.safford, mkayaalp,
	James.Bottomley, Yuqiong Sun, Mehmet Kayaalp

On 04/13/2018 09:25 AM, Mimi Zohar wrote:
> [Cc'ing John Johansen]
> 
> On Tue, 2018-03-27 at 18:01 -0500, Eric W. Biederman wrote:
> [...]
>> As such I expect the best way to create the ima namespace is by simply
>> writing to securityfs/imafs.  Possibly before the user namespace is
>> even unshared.  That would allow IMA to keep track of things from
>> before a container is created.
> 

I do think this is generally the right approach for LSMs when looking
forward to LSM stacking and more LSMs.


> My initial thought was to stage IMA namespacing with just IMA-audit
> first, followed by either IMA-measurement or IMA-appraisal.  This
> would allow us to get the basic IMA namespacing framework working and
> defer dealing with the securityfs related namespacing of the IMA
> policy and measurement list issues to later.
> 
> By tying IMA namespacing to a securityfs ima/unshare file, we would
> need to address the securityfs issues first.
> 

well it depends on what you want to do. It would be possible to have
a simple file (not a jump link) within securityfs that IMA could use
without having to deal with all the securityfs issues first. However it
does require that securityfs (not necessarily imafs) be visible within
the mount namespace of the task doing the setup.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support
  2018-04-18 16:09       ` John Johansen
@ 2018-04-18 19:57         ` Mimi Zohar
  2018-04-18 20:12           ` Eric W. Biederman
  0 siblings, 1 reply; 19+ messages in thread
From: Mimi Zohar @ 2018-04-18 19:57 UTC (permalink / raw)
  To: John Johansen, Eric W. Biederman, Stefan Berger
  Cc: linux-integrity, containers, linux-kernel, linux-security-module,
	tycho, serge, sunyuqiong1988, david.safford, mkayaalp,
	James.Bottomley, Yuqiong Sun, Mehmet Kayaalp

On Wed, 2018-04-18 at 09:09 -0700, John Johansen wrote:
> On 04/13/2018 09:25 AM, Mimi Zohar wrote:
> > [Cc'ing John Johansen]
> > 
> > On Tue, 2018-03-27 at 18:01 -0500, Eric W. Biederman wrote:
> > [...]
> >> As such I expect the best way to create the ima namespace is by simply
> >> writing to securityfs/imafs.  Possibly before the user namespace is
> >> even unshared.  That would allow IMA to keep track of things from
> >> before a container is created.
> > 
> 
> I do think this is generally the right approach for LSMs when looking
> forward to LSM stacking and more LSMs.
> 
> 
> > My initial thought was to stage IMA namespacing with just IMA-audit
> > first, followed by either IMA-measurement or IMA-appraisal.  This
> > would allow us to get the basic IMA namespacing framework working and
> > defer dealing with the securityfs related namespacing of the IMA
> > policy and measurement list issues to later.
> > 
> > By tying IMA namespacing to a securityfs ima/unshare file, we would
> > need to address the securityfs issues first.
> > 
> 
> well it depends on what you want to do. It would be possible to have
> a simple file (not a jump link) within securityfs that IMA could use
> without having to deal with all the securityfs issues first. However it
> does require that securityfs (not necessarily imafs) be visible within
> the mount namespace of the task doing the setup.

Eric, would you be OK with that?

Mimi

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support
  2018-04-18 19:57         ` Mimi Zohar
@ 2018-04-18 20:12           ` Eric W. Biederman
  2018-04-18 20:27             ` Mimi Zohar
  2018-04-18 21:32             ` John Johansen
  0 siblings, 2 replies; 19+ messages in thread
From: Eric W. Biederman @ 2018-04-18 20:12 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: John Johansen, Stefan Berger, linux-integrity, containers,
	linux-kernel, linux-security-module, tycho, serge,
	sunyuqiong1988, david.safford, mkayaalp, James.Bottomley,
	Yuqiong Sun, Mehmet Kayaalp

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

> On Wed, 2018-04-18 at 09:09 -0700, John Johansen wrote:
>> On 04/13/2018 09:25 AM, Mimi Zohar wrote:
>> > [Cc'ing John Johansen]
>> > 
>> > On Tue, 2018-03-27 at 18:01 -0500, Eric W. Biederman wrote:
>> > [...]
>> >> As such I expect the best way to create the ima namespace is by simply
>> >> writing to securityfs/imafs.  Possibly before the user namespace is
>> >> even unshared.  That would allow IMA to keep track of things from
>> >> before a container is created.
>> > 
>> 
>> I do think this is generally the right approach for LSMs when looking
>> forward to LSM stacking and more LSMs.
>> 
>> 
>> > My initial thought was to stage IMA namespacing with just IMA-audit
>> > first, followed by either IMA-measurement or IMA-appraisal.  This
>> > would allow us to get the basic IMA namespacing framework working and
>> > defer dealing with the securityfs related namespacing of the IMA
>> > policy and measurement list issues to later.
>> > 
>> > By tying IMA namespacing to a securityfs ima/unshare file, we would
>> > need to address the securityfs issues first.
>> > 
>> 
>> well it depends on what you want to do. It would be possible to have
>> a simple file (not a jump link) within securityfs that IMA could use
>> without having to deal with all the securityfs issues first. However it
>> does require that securityfs (not necessarily imafs) be visible within
>> the mount namespace of the task doing the setup.
>
> Eric, would you be OK with that?

Roughly.  My understanding is that you have to have a write to some
filesystem to set the ima policy.

I was expecting having to write an "create ima namespace" value
to the filesystem would not be any special effort.

Now it sounds like providing the "create an ima namespace" is going to
be a special case, and that does not sound correct.

Eric

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support
  2018-04-18 20:12           ` Eric W. Biederman
@ 2018-04-18 20:27             ` Mimi Zohar
  2018-04-18 21:32             ` John Johansen
  1 sibling, 0 replies; 19+ messages in thread
From: Mimi Zohar @ 2018-04-18 20:27 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: John Johansen, Stefan Berger, linux-integrity, containers,
	linux-kernel, linux-security-module, serge, sunyuqiong1988,
	david.safford, mkayaalp, James Bottomley, Mehmet Kayaalp

On Wed, 2018-04-18 at 15:12 -0500, Eric W. Biederman wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
> > On Wed, 2018-04-18 at 09:09 -0700, John Johansen wrote:
> >> On 04/13/2018 09:25 AM, Mimi Zohar wrote:
> >> > [Cc'ing John Johansen]
> >> > 
> >> > On Tue, 2018-03-27 at 18:01 -0500, Eric W. Biederman wrote:
> >> > [...]
> >> >> As such I expect the best way to create the ima namespace is by simply
> >> >> writing to securityfs/imafs.  Possibly before the user namespace is
> >> >> even unshared.  That would allow IMA to keep track of things from
> >> >> before a container is created.
> >> > 
> >> 
> >> I do think this is generally the right approach for LSMs when looking
> >> forward to LSM stacking and more LSMs.
> >> 
> >> 
> >> > My initial thought was to stage IMA namespacing with just IMA-audit
> >> > first, followed by either IMA-measurement or IMA-appraisal.  This
> >> > would allow us to get the basic IMA namespacing framework working and
> >> > defer dealing with the securityfs related namespacing of the IMA
> >> > policy and measurement list issues to later.
> >> > 
> >> > By tying IMA namespacing to a securityfs ima/unshare file, we would
> >> > need to address the securityfs issues first.
> >> > 
> >> 
> >> well it depends on what you want to do. It would be possible to have
> >> a simple file (not a jump link) within securityfs that IMA could use
> >> without having to deal with all the securityfs issues first. However it
> >> does require that securityfs (not necessarily imafs) be visible within
> >> the mount namespace of the task doing the setup.
> >
> > Eric, would you be OK with that?
> 
> Roughly.  My understanding is that you have to have a write to some
> filesystem to set the ima policy.
> 
> I was expecting having to write an "create ima namespace" value
> to the filesystem would not be any special effort.
> 
> Now it sounds like providing the "create an ima namespace" is going to
> be a special case, and that does not sound correct.

This is not any different than any of the other security/ima/ files
(eg. policy, ascii_runtime_measurements, ...).  The next IMA
namespacing stage would add support for these files.

Mimi

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support
  2018-04-18 20:12           ` Eric W. Biederman
  2018-04-18 20:27             ` Mimi Zohar
@ 2018-04-18 21:32             ` John Johansen
  2018-04-19 11:03               ` Stefan Berger
  1 sibling, 1 reply; 19+ messages in thread
From: John Johansen @ 2018-04-18 21:32 UTC (permalink / raw)
  To: Eric W. Biederman, Mimi Zohar
  Cc: Stefan Berger, linux-integrity, containers, linux-kernel,
	linux-security-module, tycho, serge, sunyuqiong1988,
	david.safford, mkayaalp, James.Bottomley, Yuqiong Sun,
	Mehmet Kayaalp

On 04/18/2018 01:12 PM, Eric W. Biederman wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
>> On Wed, 2018-04-18 at 09:09 -0700, John Johansen wrote:
>>> On 04/13/2018 09:25 AM, Mimi Zohar wrote:
>>>> [Cc'ing John Johansen]
>>>>
>>>> On Tue, 2018-03-27 at 18:01 -0500, Eric W. Biederman wrote:
>>>> [...]
>>>>> As such I expect the best way to create the ima namespace is by simply
>>>>> writing to securityfs/imafs.  Possibly before the user namespace is
>>>>> even unshared.  That would allow IMA to keep track of things from
>>>>> before a container is created.
>>>>
>>>
>>> I do think this is generally the right approach for LSMs when looking
>>> forward to LSM stacking and more LSMs.
>>>
>>>
>>>> My initial thought was to stage IMA namespacing with just IMA-audit
>>>> first, followed by either IMA-measurement or IMA-appraisal.  This
>>>> would allow us to get the basic IMA namespacing framework working and
>>>> defer dealing with the securityfs related namespacing of the IMA
>>>> policy and measurement list issues to later.
>>>>
>>>> By tying IMA namespacing to a securityfs ima/unshare file, we would
>>>> need to address the securityfs issues first.
>>>>
>>>
>>> well it depends on what you want to do. It would be possible to have
>>> a simple file (not a jump link) within securityfs that IMA could use
>>> without having to deal with all the securityfs issues first. However it
>>> does require that securityfs (not necessarily imafs) be visible within
>>> the mount namespace of the task doing the setup.
>>
>> Eric, would you be OK with that?
> 
> Roughly.  My understanding is that you have to have a write to some
> filesystem to set the ima policy.
> 
> I was expecting having to write an "create ima namespace" value
> to the filesystem would not be any special effort.
> 
> Now it sounds like providing the "create an ima namespace" is going to
> be a special case, and that does not sound correct.
> 
not necessarily special case, but they do need to settle on an interface
that will work for them, and will work with the order they want to land
things. I was just trying to point out that there are fs solutions that
can work without having deal with the full securityfs/imafs namespacing
solution landing first.

While create a file directly in securityfs that lives along side the imafs
dir.
ima_create_ns
ima/
does feel like a special case. It could work

what I was thinking of when I proposed a simple is to do it within the ima
dir, say
ima/create_ns

For now its just a single file but once imafs becomes "virtualized" to a
namespace view, each dir that imafs jumplinks to contains a instance of the
file.


Or they could avoid securityfs/imafs entirely and leverage a file in
procfs

/proc/<pid>/attr/{current,exec,...}

currently those are claimed by the LSMs but new file could be added or
perhaps even better we could lift the some code from the LSM stacking work
to provide an ima specific dir

  /proc/<pid>/attr/ima/{current,exec,...}

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support
  2018-04-18 21:32             ` John Johansen
@ 2018-04-19 11:03               ` Stefan Berger
  2018-04-19 15:35                 ` John Johansen
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Berger @ 2018-04-19 11:03 UTC (permalink / raw)
  To: John Johansen, Eric W. Biederman, Mimi Zohar
  Cc: linux-integrity, containers, linux-kernel, linux-security-module,
	tycho, serge, sunyuqiong1988, david.safford, mkayaalp,
	James.Bottomley, Yuqiong Sun, Mehmet Kayaalp

On 04/18/2018 05:32 PM, John Johansen wrote:
> On 04/18/2018 01:12 PM, Eric W. Biederman wrote:
>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
>>
>>> On Wed, 2018-04-18 at 09:09 -0700, John Johansen wrote:
>>>> On 04/13/2018 09:25 AM, Mimi Zohar wrote:
>>>>> [Cc'ing John Johansen]
>>>>>
>>>>> On Tue, 2018-03-27 at 18:01 -0500, Eric W. Biederman wrote:
>>>>> [...]
>>>>>> As such I expect the best way to create the ima namespace is by simply
>>>>>> writing to securityfs/imafs.  Possibly before the user namespace is
>>>>>> even unshared.  That would allow IMA to keep track of things from
>>>>>> before a container is created.
>>>> I do think this is generally the right approach for LSMs when looking
>>>> forward to LSM stacking and more LSMs.
>>>>
>>>>
>>>>> My initial thought was to stage IMA namespacing with just IMA-audit
>>>>> first, followed by either IMA-measurement or IMA-appraisal.  This
>>>>> would allow us to get the basic IMA namespacing framework working and
>>>>> defer dealing with the securityfs related namespacing of the IMA
>>>>> policy and measurement list issues to later.
>>>>>
>>>>> By tying IMA namespacing to a securityfs ima/unshare file, we would
>>>>> need to address the securityfs issues first.
>>>>>
>>>> well it depends on what you want to do. It would be possible to have
>>>> a simple file (not a jump link) within securityfs that IMA could use
>>>> without having to deal with all the securityfs issues first. However it
>>>> does require that securityfs (not necessarily imafs) be visible within
>>>> the mount namespace of the task doing the setup.
>>> Eric, would you be OK with that?
>> Roughly.  My understanding is that you have to have a write to some
>> filesystem to set the ima policy.
>>
>> I was expecting having to write an "create ima namespace" value
>> to the filesystem would not be any special effort.
>>
>> Now it sounds like providing the "create an ima namespace" is going to
>> be a special case, and that does not sound correct.
>>
> not necessarily special case, but they do need to settle on an interface
> that will work for them, and will work with the order they want to land
> things. I was just trying to point out that there are fs solutions that
> can work without having deal with the full securityfs/imafs namespacing
> solution landing first.
>
> While create a file directly in securityfs that lives along side the imafs
> dir.
> ima_create_ns
> ima/
> does feel like a special case. It could work
>
> what I was thinking of when I proposed a simple is to do it within the ima
> dir, say
> ima/create_ns

Having looked at SELinux and how Steve does it, I chose 'unshare' as the 
filename and put it into the neighborhood of  existing IMA securityfs 
files: /sys/kernel/security/ima/unshare. Write a '1' to it and you'll 
have an IMA namespace upon the next fork()/clone().

>
> For now its just a single file but once imafs becomes "virtualized" to a
> namespace view, each dir that imafs jumplinks to contains a instance of the
> file.

Right. We need to virtualize our IMA securityfs entries pretty soon 
afterwards so that we can start setting policies in an IMA namespace. At 
the beginning we would not be able to create nested IMA namespaces if a 
user namespace is involved. My current patches that attempt to do this 
basically implement it by getting out of securityfs for namespace 
support and hooking it onto sysfs. On the host we would still use 
securityfs.

>
>
> Or they could avoid securityfs/imafs entirely and leverage a file in
> procfs

If we want to it that way for all other subsystems that do not use a 
clone() flag, we should maybe decide on that now...

>
> /proc/<pid>/attr/{current,exec,...}
>
> currently those are claimed by the LSMs but new file could be added or
> perhaps even better we could lift the some code from the LSM stacking work
> to provide an ima specific dir
>
>    /proc/<pid>/attr/ima/{current,exec,...}
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support
  2018-04-19 11:03               ` Stefan Berger
@ 2018-04-19 15:35                 ` John Johansen
  2018-04-26 21:18                   ` Stefan Berger
  0 siblings, 1 reply; 19+ messages in thread
From: John Johansen @ 2018-04-19 15:35 UTC (permalink / raw)
  To: Stefan Berger, Eric W. Biederman, Mimi Zohar
  Cc: linux-integrity, containers, linux-kernel, linux-security-module,
	tycho, serge, sunyuqiong1988, david.safford, mkayaalp,
	James.Bottomley, Yuqiong Sun, Mehmet Kayaalp

On 04/19/2018 04:03 AM, Stefan Berger wrote:
> On 04/18/2018 05:32 PM, John Johansen wrote:
>> On 04/18/2018 01:12 PM, Eric W. Biederman wrote:
>>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
>>>
>>>> On Wed, 2018-04-18 at 09:09 -0700, John Johansen wrote:
>>>>> On 04/13/2018 09:25 AM, Mimi Zohar wrote:
>>>>>> [Cc'ing John Johansen]
>>>>>>
>>>>>> On Tue, 2018-03-27 at 18:01 -0500, Eric W. Biederman wrote:
>>>>>> [...]
>>>>>>> As such I expect the best way to create the ima namespace is by simply
>>>>>>> writing to securityfs/imafs.  Possibly before the user namespace is
>>>>>>> even unshared.  That would allow IMA to keep track of things from
>>>>>>> before a container is created.
>>>>> I do think this is generally the right approach for LSMs when looking
>>>>> forward to LSM stacking and more LSMs.
>>>>>
>>>>>
>>>>>> My initial thought was to stage IMA namespacing with just IMA-audit
>>>>>> first, followed by either IMA-measurement or IMA-appraisal.  This
>>>>>> would allow us to get the basic IMA namespacing framework working and
>>>>>> defer dealing with the securityfs related namespacing of the IMA
>>>>>> policy and measurement list issues to later.
>>>>>>
>>>>>> By tying IMA namespacing to a securityfs ima/unshare file, we would
>>>>>> need to address the securityfs issues first.
>>>>>>
>>>>> well it depends on what you want to do. It would be possible to have
>>>>> a simple file (not a jump link) within securityfs that IMA could use
>>>>> without having to deal with all the securityfs issues first. However it
>>>>> does require that securityfs (not necessarily imafs) be visible within
>>>>> the mount namespace of the task doing the setup.
>>>> Eric, would you be OK with that?
>>> Roughly.  My understanding is that you have to have a write to some
>>> filesystem to set the ima policy.
>>>
>>> I was expecting having to write an "create ima namespace" value
>>> to the filesystem would not be any special effort.
>>>
>>> Now it sounds like providing the "create an ima namespace" is going to
>>> be a special case, and that does not sound correct.
>>>
>> not necessarily special case, but they do need to settle on an interface
>> that will work for them, and will work with the order they want to land
>> things. I was just trying to point out that there are fs solutions that
>> can work without having deal with the full securityfs/imafs namespacing
>> solution landing first.
>>
>> While create a file directly in securityfs that lives along side the imafs
>> dir.
>> ima_create_ns
>> ima/
>> does feel like a special case. It could work
>>
>> what I was thinking of when I proposed a simple is to do it within the ima
>> dir, say
>> ima/create_ns
> 
> Having looked at SELinux and how Steve does it, I chose 'unshare' as the filename and put it into the neighborhood of  existing IMA securityfs files: /sys/kernel/security/ima/unshare. Write a '1' to it and you'll have an IMA namespace upon the next fork()/clone().
> 
>>
>> For now its just a single file but once imafs becomes "virtualized" to a
>> namespace view, each dir that imafs jumplinks to contains a instance of the
>> file.
> 
> Right. We need to virtualize our IMA securityfs entries pretty soon afterwards so that we can start setting policies in an IMA namespace. At the beginning we would not be able to create nested IMA namespaces if a user namespace is involved. My current patches that attempt to do this basically implement it by getting out of securityfs for namespace support and hooking it onto sysfs. On the host we would still use securityfs.
> 
>>
>>
>> Or they could avoid securityfs/imafs entirely and leverage a file in
>> procfs
> 
> If we want to it that way for all other subsystems that do not use a clone() flag, we should maybe decide on that now...
> 

It sounds like its already decided, with ima and selinux going with an unshare file within their own fs.

AppArmor went a different route already, splitting namespace creation (mkdir in the apparmorfs policy/namespace dir) and the task entering the namespace with a write apparmor's equiv of setexeccon.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support
  2018-04-19 15:35                 ` John Johansen
@ 2018-04-26 21:18                   ` Stefan Berger
  2018-04-27  0:49                     ` Eric W. Biederman
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Berger @ 2018-04-26 21:18 UTC (permalink / raw)
  To: John Johansen, Eric W. Biederman, Mimi Zohar
  Cc: linux-integrity, containers, linux-kernel, linux-security-module,
	tycho, serge, sunyuqiong1988, david.safford, mkayaalp,
	James.Bottomley, Yuqiong Sun, Mehmet Kayaalp

On 04/19/2018 11:35 AM, John Johansen wrote:
> On 04/19/2018 04:03 AM, Stefan Berger wrote:
>> On 04/18/2018 05:32 PM, John Johansen wrote:
>>> On 04/18/2018 01:12 PM, Eric W. Biederman wrote:
>>>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
>>>>
>>>>> On Wed, 2018-04-18 at 09:09 -0700, John Johansen wrote:
>>>>>> On 04/13/2018 09:25 AM, Mimi Zohar wrote:
>>>>>>> [Cc'ing John Johansen]
>>>>>>>
>>>>>>> On Tue, 2018-03-27 at 18:01 -0500, Eric W. Biederman wrote:
>>>>>>> [...]
>>>>>>>> As such I expect the best way to create the ima namespace is by simply
>>>>>>>> writing to securityfs/imafs.  Possibly before the user namespace is
>>>>>>>> even unshared.  That would allow IMA to keep track of things from
>>>>>>>> before a container is created.
>>>>>> I do think this is generally the right approach for LSMs when looking
>>>>>> forward to LSM stacking and more LSMs.
>>>>>>
>>>>>>
>>>>>>> My initial thought was to stage IMA namespacing with just IMA-audit
>>>>>>> first, followed by either IMA-measurement or IMA-appraisal.  This
>>>>>>> would allow us to get the basic IMA namespacing framework working and
>>>>>>> defer dealing with the securityfs related namespacing of the IMA
>>>>>>> policy and measurement list issues to later.
>>>>>>>
>>>>>>> By tying IMA namespacing to a securityfs ima/unshare file, we would
>>>>>>> need to address the securityfs issues first.
>>>>>>>
>>>>>> well it depends on what you want to do. It would be possible to have
>>>>>> a simple file (not a jump link) within securityfs that IMA could use
>>>>>> without having to deal with all the securityfs issues first. However it
>>>>>> does require that securityfs (not necessarily imafs) be visible within
>>>>>> the mount namespace of the task doing the setup.
>>>>> Eric, would you be OK with that?
>>>> Roughly.  My understanding is that you have to have a write to some
>>>> filesystem to set the ima policy.
>>>>
>>>> I was expecting having to write an "create ima namespace" value
>>>> to the filesystem would not be any special effort.
>>>>
>>>> Now it sounds like providing the "create an ima namespace" is going to
>>>> be a special case, and that does not sound correct.
>>>>
>>> not necessarily special case, but they do need to settle on an interface
>>> that will work for them, and will work with the order they want to land
>>> things. I was just trying to point out that there are fs solutions that
>>> can work without having deal with the full securityfs/imafs namespacing
>>> solution landing first.
>>>
>>> While create a file directly in securityfs that lives along side the imafs
>>> dir.
>>> ima_create_ns
>>> ima/
>>> does feel like a special case. It could work
>>>
>>> what I was thinking of when I proposed a simple is to do it within the ima
>>> dir, say
>>> ima/create_ns
>> Having looked at SELinux and how Steve does it, I chose 'unshare' as the filename and put it into the neighborhood of  existing IMA securityfs files: /sys/kernel/security/ima/unshare. Write a '1' to it and you'll have an IMA namespace upon the next fork()/clone().
>>
>>> For now its just a single file but once imafs becomes "virtualized" to a
>>> namespace view, each dir that imafs jumplinks to contains a instance of the
>>> file.
>> Right. We need to virtualize our IMA securityfs entries pretty soon afterwards so that we can start setting policies in an IMA namespace. At the beginning we would not be able to create nested IMA namespaces if a user namespace is involved. My current patches that attempt to do this basically implement it by getting out of securityfs for namespace support and hooking it onto sysfs. On the host we would still use securityfs.
>>
>>>
>>> Or they could avoid securityfs/imafs entirely and leverage a file in
>>> procfs
>> If we want to it that way for all other subsystems that do not use a clone() flag, we should maybe decide on that now...
>>
> It sounds like its already decided, with ima and selinux going with an unshare file within their own fs.
>
> AppArmor went a different route already, splitting namespace creation (mkdir in the apparmorfs policy/namespace dir) and the task entering the namespace with a write apparmor's equiv of setexeccon.
>
I am supporting procfs entries for the IMA namespace spawned by writing 
a boolean '1' into IMA's securityfs 'unshare' file. It would allow to 
use setns(fd, 0), obviously with the 0 parameter. I think this is an 
important function to support considering entering a set of namespace. I 
am just wondering about the 0 parameter. We don't have a CLONE flag for 
it, so there's not other way to support it then. Does it matter ?

    Stefan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support
  2018-04-26 21:18                   ` Stefan Berger
@ 2018-04-27  0:49                     ` Eric W. Biederman
  0 siblings, 0 replies; 19+ messages in thread
From: Eric W. Biederman @ 2018-04-27  0:49 UTC (permalink / raw)
  To: Stefan Berger
  Cc: John Johansen, Mimi Zohar, linux-integrity, containers,
	linux-kernel, linux-security-module, tycho, serge,
	sunyuqiong1988, david.safford, mkayaalp, James.Bottomley,
	Yuqiong Sun, Mehmet Kayaalp

Stefan Berger <stefanb@linux.vnet.ibm.com> writes:

> On 04/19/2018 11:35 AM, John Johansen wrote:

>> It sounds like its already decided, with ima and selinux going with an unshare file within their own fs.
>>
>> AppArmor went a different route already, splitting namespace creation (mkdir in the apparmorfs policy/namespace dir) and the task entering the namespace with a write apparmor's equiv of setexeccon.
>>
> I am supporting procfs entries for the IMA namespace spawned by writing a
> boolean '1' into IMA's securityfs 'unshare' file. It would allow to use
> setns(fd, 0), obviously with the 0 parameter. I think this is an important
> function to support considering entering a set of namespace. I am just wondering
> about the 0 parameter. We don't have a CLONE flag for it, so there's not other
> way to support it then. Does it matter ?

That should be fine.  We can pick a flag for setns at some point for
IMA.  The setns function uses the flag field as an enumeration so any of
the low 8 bits or a combination with overlapping bit is valid to setns.

Eric

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2018-04-27  0:50 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-27 13:57 [RFC PATCH v3 0/3] ima: namespacing IMA Stefan Berger
2018-03-27 13:57 ` [RFC PATCH v3 1/3] ima: extend clone() with IMA namespace support Stefan Berger
2018-03-27 23:01   ` Eric W. Biederman
2018-03-28 11:10     ` Stefan Berger
2018-03-28 12:14       ` Dr. Greg Wettstein
2018-03-28 12:44         ` Stefan Berger
2018-04-18 15:59       ` John Johansen
2018-04-13 16:25     ` Mimi Zohar
2018-04-18 16:09       ` John Johansen
2018-04-18 19:57         ` Mimi Zohar
2018-04-18 20:12           ` Eric W. Biederman
2018-04-18 20:27             ` Mimi Zohar
2018-04-18 21:32             ` John Johansen
2018-04-19 11:03               ` Stefan Berger
2018-04-19 15:35                 ` John Johansen
2018-04-26 21:18                   ` Stefan Berger
2018-04-27  0:49                     ` Eric W. Biederman
2018-03-27 13:57 ` [RFC PATCH v3 2/3] ima: Add ns_status for storing namespaced iint data Stefan Berger
2018-03-27 13:57 ` [RFC PATCH v3 3/3] ima: mamespace audit status flags Stefan Berger

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