| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5f9b38ef-04d7-e97e-be7d-424b39ef06bc@linux.ibm.com>
Date: Wed, 8 Dec 2021 09:50:15 -0500
From: Stefan Berger <stefanb@...ux.ibm.com>
To: Christian Brauner <christian.brauner@...ntu.com>
Cc: linux-integrity@...r.kernel.org, zohar@...ux.ibm.com,
serge@...lyn.com, containers@...ts.linux.dev,
dmitry.kasatkin@...il.com, ebiederm@...ssion.com,
krzysztof.struczynski@...wei.com, roberto.sassu@...wei.com,
mpeters@...hat.com, lhinds@...hat.com, lsturman@...hat.com,
puiterwi@...hat.com, jejb@...ux.ibm.com, jamjoom@...ibm.com,
linux-kernel@...r.kernel.org, paul@...l-moore.com, rgb@...hat.com,
linux-security-module@...r.kernel.org, jmorris@...ei.org,
James Bottomley <James.Bottomley@...senPartnership.com>
Subject: Re: [PATCH v4 01/16] ima: Add IMA namespace support
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@...ux.ibm.com>
>>> Suggested-by: James Bottomley <James.Bottomley@...senPartnership.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
Powered by blists - more mailing lists