Message ID | 1522159038-14175-2-git-send-email-stefanb@linux.vnet.ibm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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); > +}
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); >> +}
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
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 >
[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
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.
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.
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
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
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
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,...}
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,...} >
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.
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
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
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); +}