Re: [PATCH v10 27/27] ima: Enable IMA namespaces

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2/23/22 12:58, Serge E. Hallyn wrote:
On Tue, Feb 01, 2022 at 03:37:35PM -0500, Stefan Berger wrote:
Introduce the IMA_NS in Kconfig for IMA namespace enablement.

Enable the lazy initialization of an IMA namespace when a user mounts
SecurityFS and writes '1' into IMA's 'active' securityfs file. A
user_namespace will now get a pointer to an ima_namespace and therefore
implement get_current_ns() for the namespacing case that returns this
pointer. Use get_current_ns() in those places that require access to the
current IMA namespace. In some places, primarily those related to
IMA-appraisal and changes to file attributes, keep the pointer to
init_ima_ns, since there flags related to file measurements may be
affected, which are not supported in IMA namespaces, yet.

Before using the ima_namespace pointer test it with ns_is_active()
to check whether it is NULL and whether the ima_namespace is active.
If it's not active, it cannot be used, yet. Therefore, return early
from those functions that may now get either get a NULL pointer from
this call or where ns->active is still 0. The init_ima_ns is always
set to be active, thus passing the check.

Implement ima_ns_from_file() for SecurityFS-related files where we can
now get the IMA namespace via the user namespace pointer associated
with the superblock of the SecurityFS filesystem instance.

Return -EACCES to IMA's securityfs files, except for the 'active' file,
until the IMA namespace has been set to active.

Switch access to userns->ima_ns to use acquire/release semantics to ensure
that a newly created ima_namespace structure is fully visible upon access.

Only emit the kernel log message 'policy update completed' for the
init_ima_ns.

When parsing an IMA policy rule use the user namespace of the opener
to translate uid and gid values to kernel values rather than the user
namespace of the writer.

Gate access to ima_appraise variable to init_ima_ns in ima_load_data()
and ima_write_policy().

Gate access to temp_ima_appraise variable to init_ima_ns in
ima_delete_rules().

Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>

---
v10:
  - dropped ima_ns_to_user_ns(); using current_user_ns() instead
  - Pass user_namespace of file opener into ima_parse_rule and propagate
    this parameter back all the way to the initial caller in the chain
  - Gate access to ima_appraise to init_ima_ns in ima_write_policy()

v9:
  - ima_post_key_create_or_update: Only handle key if in init_ima_ns
  - Removed ns == NULL checks where user_namespace is now passed
  - Defer setting of user_ns->ima_ns until end of ima_fs_ns_init();
    required new ima_free_imans() and new user_ns_set_ima_ns()
  - Only emit log message 'policy update completed' for init_ima_ns
  - Introduce get_current_ns() only in this patch
  - Check for ns == &init_ima_ns in ima_load_data()
---
  include/linux/ima.h                          |  1 +
  init/Kconfig                                 | 13 +++
  kernel/user_namespace.c                      |  2 +
  security/integrity/ima/ima.h                 | 55 +++++++++++--
  security/integrity/ima/ima_appraise.c        |  3 +
  security/integrity/ima/ima_asymmetric_keys.c |  6 +-
  security/integrity/ima/ima_fs.c              | 87 ++++++++++++++++----
  security/integrity/ima/ima_init.c            |  2 +-
  security/integrity/ima/ima_init_ima_ns.c     |  2 +
  security/integrity/ima/ima_main.c            | 34 +++++---
  security/integrity/ima/ima_ns.c              | 15 +++-
  security/integrity/ima/ima_policy.c          | 43 ++++++----
  12 files changed, 202 insertions(+), 61 deletions(-)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index c584527c0f47..a8cb2c269f61 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -11,6 +11,7 @@
  #include <linux/fs.h>
  #include <linux/security.h>
  #include <linux/kexec.h>
+#include <linux/user_namespace.h>
  #include <crypto/hash_info.h>
  struct linux_binprm;
diff --git a/init/Kconfig b/init/Kconfig
index 4b7bac10c72d..e27155e0ddba 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1247,6 +1247,19 @@ config NET_NS
  	  Allow user space to create what appear to be multiple instances
  	  of the network stack.
+config IMA_NS
+	bool "IMA namespace"
+	depends on USER_NS
+	depends on IMA
+	default n
+	help
+	  Allow the creation of an IMA namespace for each user namespace.
+	  Namespaced IMA enables having IMA features work separately
+	  in each IMA 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
config CHECKPOINT_RESTORE
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 6b2e3ca7ee99..653f8fa83b69 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -20,6 +20,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);
@@ -196,6 +197,7 @@ static void free_user_ns(struct work_struct *work)
  			kfree(ns->projid_map.forward);
  			kfree(ns->projid_map.reverse);
  		}
+		free_ima_ns(ns);
  		retire_userns_sysctls(ns);
  		key_free_user_ns(ns);
  		ns_free_inum(&ns->ns);
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 05e2de7697da..73df1d8a2ece 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -337,10 +337,10 @@ int ima_match_policy(struct ima_namespace *ns,
  		     int mask, int flags, int *pcr,
  		     struct ima_template_desc **template_desc,
  		     const char *func_data, unsigned int *allowed_algos);
-void ima_init_policy(struct ima_namespace *ns);
+void ima_init_policy(struct user_namespace *user_ns);
  void ima_update_policy(struct ima_namespace *ns);
  void ima_update_policy_flags(struct ima_namespace *ns);
-ssize_t ima_parse_add_rule(struct ima_namespace *ns, char *rule);
+ssize_t ima_parse_add_rule(struct user_namespace *user_ns, char *rule);
  void ima_delete_rules(struct ima_namespace *ns);
  int ima_check_policy(struct ima_namespace *ns);
  void ima_free_policy_rules(struct ima_namespace *ns);
@@ -538,32 +538,70 @@ struct user_namespace *ima_user_ns_from_file(const struct file *filp)
  	return file_inode(filp)->i_sb->s_user_ns;
  }
+#ifdef CONFIG_IMA_NS
+
  static inline struct ima_namespace
  *ima_ns_from_user_ns(struct user_namespace *user_ns)
  {
-	if (user_ns == &init_user_ns)
-		return &init_ima_ns;
-	return NULL;
+	/* Pairs with smp_store_releases() in user_ns_set_ima_ns(). */
+	return smp_load_acquire(&user_ns->ima_ns);
  }
-#ifdef CONFIG_IMA_NS
+static inline void user_ns_set_ima_ns(struct user_namespace *user_ns,
+				      struct ima_namespace *ns)
+{
+	/* Pairs with smp_load_acquire() in ima_ns_from_user_ns() */
+	smp_store_release(&user_ns->ima_ns, ns);
+}
+
+static inline struct ima_namespace *get_current_ns(void)
+{
+	return ima_ns_from_user_ns(current_user_ns());
+}
struct ima_namespace *create_ima_ns(void); +void ima_free_ima_ns(struct ima_namespace *ns);
+
  struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
  				    struct inode *inode,
  				    struct integrity_iint_cache *iint);
void ima_free_ns_status_tree(struct ima_namespace *ns); +static inline struct ima_namespace *ima_ns_from_file(const struct file *filp)
+{
Why is it ok here to dereference userns->ima_ns without
going through ima_ns_from_user_ns() to do the smp_load_acquire()?

Good catch! Fixed.

    Stefan






[Index of Archives]     [Cgroups]     [Netdev]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux