All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Serge E. Hallyn" <serge@hallyn.com>
To: Stefan Berger <stefanb@linux.ibm.com>
Cc: linux-integrity@vger.kernel.org, zohar@linux.ibm.com,
	serge@hallyn.com, christian.brauner@ubuntu.com,
	containers@lists.linux.dev, dmitry.kasatkin@gmail.com,
	ebiederm@xmission.com, krzysztof.struczynski@huawei.com,
	roberto.sassu@huawei.com, mpeters@redhat.com, lhinds@redhat.com,
	lsturman@redhat.com, puiterwi@redhat.com, jejb@linux.ibm.com,
	jamjoom@us.ibm.com, linux-kernel@vger.kernel.org,
	paul@paul-moore.com, rgb@redhat.com,
	linux-security-module@vger.kernel.org, jmorris@namei.org
Subject: Re: [PATCH v10 27/27] ima: Enable IMA namespaces
Date: Wed, 23 Feb 2022 11:58:54 -0600	[thread overview]
Message-ID: <20220223175854.GB10272@mail.hallyn.com> (raw)
In-Reply-To: <20220201203735.164593-28-stefanb@linux.ibm.com>

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@linux.ibm.com>
> 
> ---
> 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()?

> +	return ima_user_ns_from_file(filp)->ima_ns;
> +}
> +
>  #else

-serge

  reply	other threads:[~2022-02-23 17:58 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-01 20:37 [PATCH v10 00/27] ima: Namespace IMA with audit support in IMA-ns Stefan Berger
2022-02-01 20:37 ` [PATCH v10 01/27] ima: Remove ima_policy file before directory Stefan Berger
2022-02-10 12:02   ` Mimi Zohar
2022-02-01 20:37 ` [PATCH v10 02/27] ima: Do not print policy rule with inactive LSM labels Stefan Berger
2022-02-02 14:17   ` Christian Brauner
2022-02-01 20:37 ` [PATCH v10 03/27] ima: Return error code obtained from securityfs functions Stefan Berger
2022-02-10 12:02   ` Mimi Zohar
2022-02-15 18:09     ` Mimi Zohar
2022-02-01 20:37 ` [PATCH v10 04/27] securityfs: rework dentry creation Stefan Berger
2022-02-10 12:03   ` Mimi Zohar
2022-02-01 20:37 ` [PATCH v10 05/27] ima: Define ima_namespace struct and start moving variables into it Stefan Berger
2022-02-16 14:41   ` Mimi Zohar
2022-02-16 20:25     ` Stefan Berger
2022-02-01 20:37 ` [PATCH v10 06/27] ima: Move arch_policy_entry into ima_namespace Stefan Berger
2022-02-16 16:39   ` Mimi Zohar
2022-02-16 20:48     ` Stefan Berger
2022-02-16 20:56       ` Mimi Zohar
2022-02-16 21:19         ` Stefan Berger
2022-02-01 20:37 ` [PATCH v10 07/27] ima: Move ima_htable " Stefan Berger
2022-02-16 14:41   ` Mimi Zohar
2022-02-01 20:37 ` [PATCH v10 08/27] ima: Move measurement list related variables " Stefan Berger
2022-02-17 14:46   ` Mimi Zohar
2022-02-01 20:37 ` [PATCH v10 09/27] ima: Move some IMA policy and filesystem " Stefan Berger
2022-02-17 14:44   ` Mimi Zohar
2022-02-01 20:37 ` [PATCH v10 10/27] ima: Move IMA securityfs files into ima_namespace or onto stack Stefan Berger
2022-02-17 14:44   ` Mimi Zohar
2022-02-01 20:37 ` [PATCH v10 11/27] ima: Move ima_lsm_policy_notifier into ima_namespace Stefan Berger
2022-02-17 20:30   ` Mimi Zohar
2022-02-17 20:59     ` Stefan Berger
2022-02-17 21:24       ` Mimi Zohar
2022-02-01 20:37 ` [PATCH v10 12/27] ima: Define mac_admin_ns_capable() as a wrapper for ns_capable() Stefan Berger
2022-02-05  5:58   ` Serge E. Hallyn
2022-02-06 17:20     ` Stefan Berger
2022-02-07 18:43       ` Stefan Berger
2022-02-23 17:51         ` Serge E. Hallyn
2022-02-01 20:37 ` [PATCH v10 13/27] ima: Only accept AUDIT rules for non-init_ima_ns namespaces for now Stefan Berger
2022-02-17 21:32   ` Mimi Zohar
2022-02-01 20:37 ` [PATCH v10 14/27] userns: Add pointer to ima_namespace to user_namespace Stefan Berger
2022-02-18 16:26   ` Mimi Zohar
2022-02-01 20:37 ` [PATCH v10 15/27] ima: Implement hierarchical processing of file accesses Stefan Berger
2022-02-02 13:19   ` [PATCH] ima: fix semicolon.cocci warnings kernel test robot
2022-02-02 13:19     ` kernel test robot
2022-02-02 13:19   ` [PATCH v10 15/27] ima: Implement hierarchical processing of file accesses kernel test robot
2022-02-02 13:19     ` kernel test robot
2022-02-18 16:27   ` Mimi Zohar
2022-02-01 20:37 ` [PATCH v10 16/27] ima: Implement ima_free_policy_rules() for freeing of an ima_namespace Stefan Berger
2022-02-18 17:09   ` Mimi Zohar
2022-02-18 19:38     ` Stefan Berger
2022-02-18 20:04       ` Mimi Zohar
2022-02-01 20:37 ` [PATCH v10 17/27] ima: Add functions for creating and " Stefan Berger
2022-02-18 19:49   ` Mimi Zohar
2022-02-01 20:37 ` [PATCH v10 18/27] integrity/ima: Define ns_status for storing namespaced iint data Stefan Berger
2022-02-23 16:12   ` Mimi Zohar
2022-02-24  2:21     ` Stefan Berger
2022-02-24  2:49       ` Stefan Berger
2022-02-01 20:37 ` [PATCH v10 19/27] integrity: Add optional callback function to integrity_inode_free() Stefan Berger
2022-02-01 20:37 ` [PATCH v10 20/27] ima: Namespace audit status flags Stefan Berger
2022-02-01 20:37 ` [PATCH v10 21/27] ima: Remove unused iints from the integrity_iint_cache Stefan Berger
2022-02-01 20:37 ` [PATCH v10 22/27] securityfs: Extend securityfs with namespacing support Stefan Berger
2022-02-23  1:48   ` Mimi Zohar
2022-02-23  8:14     ` Christian Brauner
2022-02-23 15:44       ` Stefan Berger
2022-02-01 20:37 ` [PATCH v10 23/27] ima: Setup securityfs for IMA namespace Stefan Berger
2022-02-23 11:45   ` Mimi Zohar
2022-02-01 20:37 ` [PATCH v10 24/27] ima: Introduce securityfs file to activate an " Stefan Berger
2022-02-23 13:54   ` Mimi Zohar
2022-02-23 17:08     ` Stefan Berger
2022-02-23 17:12       ` Mimi Zohar
2022-02-23 22:30         ` Stefan Berger
2022-02-01 20:37 ` [PATCH v10 25/27] ima: Show owning user namespace's uid and gid when displaying policy Stefan Berger
2022-02-23 14:06   ` Mimi Zohar
2022-02-01 20:37 ` [PATCH v10 26/27] ima: Limit number of policy rules in non-init_ima_ns Stefan Berger
2022-02-23 15:38   ` Mimi Zohar
2022-02-23 16:37     ` Stefan Berger
2022-02-23 17:04       ` Mimi Zohar
2022-02-23 20:45         ` Stefan Berger
2022-02-23 20:59           ` Mimi Zohar
2022-02-23 21:06             ` Stefan Berger
2022-02-01 20:37 ` [PATCH v10 27/27] ima: Enable IMA namespaces Stefan Berger
2022-02-23 17:58   ` Serge E. Hallyn [this message]
2022-02-23 20:53     ` Stefan Berger
2022-02-02 14:13 ` [PATCH v10 00/27] ima: Namespace IMA with audit support in IMA-ns Christian Brauner
2022-02-02 14:40   ` Stefan Berger
2022-02-02 16:04     ` Mimi Zohar
2022-02-02 18:18       ` Stefan Berger
2022-02-02 21:27         ` Stefan Berger

Reply instructions:

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

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

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

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

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

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.