Messages in this thread | | | Date | Wed, 8 Dec 2021 09:50:15 -0500 | Subject | Re: [PATCH v4 01/16] ima: Add IMA namespace support | From | Stefan Berger <> |
| |
On 12/8/21 06:54, Christian Brauner wrote: > On Wed, Dec 08, 2021 at 12:29:18PM +0100, Christian Brauner wrote: >> On Tue, Dec 07, 2021 at 03:21:12PM -0500, Stefan Berger wrote: >>> Implement an IMA namespace data structure that gets created alongside a >>> user namespace with CLONE_NEWUSER. This lays down the foundation for >>> namespacing the different aspects of IMA (eg. IMA-audit, IMA-measurement, >>> IMA-appraisal). >>> >>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >>> Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com> >>> --- >>> include/linux/ima.h | 59 +++++++++++++++++ >>> include/linux/user_namespace.h | 4 ++ >>> init/Kconfig | 10 +++ >>> kernel/user.c | 9 ++- >>> kernel/user_namespace.c | 16 +++++ >>> 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 | 32 +++++++++ >>> security/integrity/ima/ima_ns.c | 82 ++++++++++++++++++++++++ >>> 10 files changed, 221 insertions(+), 2 deletions(-) >>> 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 b6ab66a546ae..86d126b9ff2f 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; >>> >>> @@ -210,6 +211,64 @@ static inline int ima_inode_removexattr(struct dentry *dentry, >>> } >>> #endif /* CONFIG_IMA_APPRAISE */ >>> >>> +struct ima_namespace { >>> + struct kref kref; >>> + struct user_namespace *user_ns; >>> +}; >>> + >>> +extern struct ima_namespace init_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) >>> +{ >>> + if (ns) >>> + kref_get(&ns->kref); >>> + >>> + return ns; >>> +} >>> + >>> +static inline void put_ima_ns(struct ima_namespace *ns) >>> +{ >>> + if (ns) { >>> + pr_debug("DEREF ima_ns: 0x%p ctr: %d\n", ns, kref_read(&ns->kref)); >>> + kref_put(&ns->kref, free_ima_ns); >>> + } >>> +} >>> + >>> +struct ima_namespace *copy_ima_ns(struct ima_namespace *old_ns, >>> + struct user_namespace *user_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) >>> +{ >>> +} >>> + >>> +static inline struct ima_namespace *copy_ima_ns(struct ima_namespace *old_ns, >>> + struct user_namespace *user_ns) >>> +{ >>> + return old_ns; >>> +} >>> + >>> +static inline struct ima_namespace *get_current_ns(void) >>> +{ >>> + return &init_ima_ns; >>> +} >>> +#endif /* CONFIG_IMA_NS */ >>> + >>> #if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) >>> extern bool ima_appraise_signature(enum kernel_read_file_id func); >>> #else >>> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h >>> index 33a4240e6a6f..5249db04d62b 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, >>> @@ -99,6 +100,9 @@ struct user_namespace { >>> #endif >>> struct ucounts *ucounts; >>> long 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 11f8a845f259..27890607e8cb 100644 >>> --- a/init/Kconfig >>> +++ b/init/Kconfig >>> @@ -1242,6 +1242,16 @@ 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 y >>> + help >>> + Allow the creation of IMA namespaces for each user namespace. >>> + Namespaced IMA enables having IMA features work separately >>> + in each IMA namespace. >>> + >>> endif # NAMESPACES >>> >>> config CHECKPOINT_RESTORE >>> diff --git a/kernel/user.c b/kernel/user.c >>> index e2cf8c22b539..b5dc803a033d 100644 >>> --- a/kernel/user.c >>> +++ b/kernel/user.c >>> @@ -20,6 +20,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... ? >>> @@ -55,7 +59,7 @@ struct user_namespace init_user_ns = { >>> }, >>> }, >>> }, >>> - .ns.count = REFCOUNT_INIT(3), >>> + .ns.count = REFCOUNT_INIT(4), >>> .owner = GLOBAL_ROOT_UID, >>> .group = GLOBAL_ROOT_GID, >>> .ns.inum = PROC_USER_INIT_INO, >>> @@ -67,6 +71,9 @@ struct user_namespace init_user_ns = { >>> .keyring_name_list = LIST_HEAD_INIT(init_user_ns.keyring_name_list), >>> .keyring_sem = __RWSEM_INITIALIZER(init_user_ns.keyring_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 6b2e3ca7ee99..c26885343b19 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); >>> @@ -141,8 +142,20 @@ int create_user_ns(struct cred *new) >>> if (!setup_userns_sysctls(ns)) >>> goto fail_keyring; >>> >>> +#if CONFIG_IMA >>> + ns->ima_ns = copy_ima_ns(parent_ns->ima_ns, 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 > If you rewrite copy_ima_ns() put_ima_ns() a little you can remove all > the ifdefs in here and make this patch tiny. > > Make copy_ima_ns() return an int. Afaict, you can remove passing the > parent ima_ns completely but if you have to pass through the > parent_userns. > Then you can initialize new_userns->ima_ns inside of copy_ima_ns() and > the ifdef will only need to be visible inside copy_ima_ns() and not in > create_user_ns(). Similar put_ima_ns() is only called in a single place > namely kernel/user_namespace.c. Make it take a user_namespace argument > and you can remove the ifdef in __put_user_ns() for put_ima_ns() too. > > So something -- COMPLETELY UNTESTED -- like the below should work:
Thanks. I did this now for v5.
Stefan
| |