2021-12-06 17:27:31

by Stefan Berger

Subject: [PATCH v3 00/16] ima: Namespace IMA with audit support in IMA-ns

The goal of this series of patches is to start with the namespacing of
IMA and support auditing within an IMA namespace (IMA-ns) as the first
step.

In this series the IMA namespace is piggy backing on the user namespace
and therefore an IMA namespace gets created when a user namespace is
created. The advantage of this is that the user namespace can provide
the keys infrastructure that IMA appraisal support will need later on.

We chose the goal of supporting auditing within an IMA namespace since it
requires the least changes to IMA. Following this series, auditing within
an IMA namespace can be activated by a user running the following lines
that rely on a statically linked busybox to be installed on the host for
execution within the minimal container environment:

mkdir -p rootfs/{bin,mnt,proc}
cp /sbin/busybox rootfs/bin
PATH=/bin unshare --user --map-root-user --mount-proc --pid --fork \
--root rootfs busybox sh -c \
"busybox mount -t securityfs /mnt /mnt; \
busybox echo 'audit func=BPRM_CHECK mask=MAY_EXEC' > /mnt/ima/policy; \
busybox cat /mnt/ima/policy"

Following the audit log on the host the last line cat'ing the IMA policy
inside the namespace would have been audited. Unfortunately the auditing
line is not distinguishable from one stemming from actions on the host.
The hope here is that Richard Brigg's container id support for auditing
would help resolve the problem.

The following lines added to a suitable IMA policy on the host would
cause the execution of the commands inside the container (by uid 1000)
to be measured and audited as well on the host, thus leading to two
auditing messages for the 'busybox cat' above and log entries in IMA's
system log.

echo -e "measure func=BPRM_CHECK mask=MAY_EXEC uid=1000\n" \
"audit func=BPRM_CHECK mask=MAY_EXEC uid=1000\n" \
> /sys/kernel/security/ima/policy

The goal of supporting measurement and auditing by the host, of actions
occurring within IMA namespaces, is that users, particularly root,
should not be able to evade the host's IMA policy just by spawning
new IMA namespaces, running programs there, and discarding the namespaces
again. This is achieved through 'hierarchical processing' of file
accesses that are evaluated against the policy of the namespace where
the action occurred and against all namespaces' and their policies leading
back to the root IMA namespace (init_ima_ns).

The patch series adds support for a virtualized SecurityFS with a few
new API calls that are used by IMA namespacing. Only the data relevant
to the IMA namespace are shown. The files and directories of other
security subsystems (TPM, evm, Tomoyo, safesetid) are not showing
up when secruityfs is mounted inside a user namespace.

Much of the code leading up to the virtualization of SecurityFS deals
with moving IMA's variables from various files into the IMA namespace
structure called 'ima_namespace'. When it comes to determining the
current IMA namespace I took the approach to get the current IMA
namespace (get_current_ns()) on the top level and pass the pointer all
the way down to those functions that now need access to the ima_namespace
to get to their variables. This later on comes in handy once hierarchical
processing is implemented in this series where we walk the list of
namespaces backwards and again need to pass the pointer into functions.

This patch also introduces usage of CAP_MAC_ADMIN to allow access to the
IMA policy via reduced capabilities. We would again later on use this
capability to allow users to set file extended attributes for IMA appraisal
support.

The basis for this series of patches is Linux v5.15.
My tree with these patches is here:
https://github.com/stefanberger/linux-ima-namespaces/tree/v5.15%2Bimans.v3.public

Regards,
Stefan

v3:
- Further modifications to virtualized SecurityFS following James's posted patch
- Dropping of early teardown for user_namespaces since not needed anymore

v2:
- Folllwed Christian's suggestion to virtualize securitytfs; no more securityfs_ns
- Followed James's advice for late 'population' of securityfs for IMA namespaces
- Squashed 2 patches dealing with capabilities
- Added missing 'depends on USER_NS' to Kconfig
- Added missing 'static' to several functions


*** BLURB HERE ***

Mehmet Kayaalp (2):
ima: Define ns_status for storing namespaced iint data
ima: Namespace audit status flags

Stefan Berger (14):
ima: Add IMA namespace support
ima: Move delayed work queue and variables into ima_namespace
ima: Move IMA's keys queue related variables into ima_namespace
ima: Move policy related variables into ima_namespace
ima: Move ima_htable into ima_namespace
ima: Move measurement list related variables into ima_namespace
ima: Only accept AUDIT rules for IMA non-init_ima_ns namespaces for
now
ima: Implement hierarchical processing of file accesses
securityfs: Move vfsmount into user_namespace
securityfs: Extend securityfs with namespacing support
ima: Move some IMA policy and filesystem related variables into
ima_namespace
ima: Use mac_admin_ns_capable() to check corresponding capability
ima: Move dentries into ima_namespace
ima: Setup securityfs for IMA namespace

include/linux/capability.h | 6 +
include/linux/ima.h | 126 ++++++++++++++
include/linux/security.h | 8 +
include/linux/user_namespace.h | 8 +
init/Kconfig | 13 ++
kernel/user.c | 9 +-
kernel/user_namespace.c | 16 ++
security/inode.c | 83 +++++++--
security/integrity/ima/Makefile | 4 +-
security/integrity/ima/ima.h | 145 ++++++++++------
security/integrity/ima/ima_api.c | 33 ++--
security/integrity/ima/ima_appraise.c | 26 +--
security/integrity/ima/ima_asymmetric_keys.c | 8 +-
security/integrity/ima/ima_fs.c | 170 ++++++++++++-------
security/integrity/ima/ima_init.c | 22 +--
security/integrity/ima/ima_init_ima_ns.c | 73 ++++++++
security/integrity/ima/ima_main.c | 128 +++++++++-----
security/integrity/ima/ima_ns.c | 111 ++++++++++++
security/integrity/ima/ima_ns_status.c | 132 ++++++++++++++
security/integrity/ima/ima_policy.c | 142 +++++++++-------
security/integrity/ima/ima_queue.c | 75 ++++----
security/integrity/ima/ima_queue_keys.c | 73 +++-----
security/integrity/ima/ima_template.c | 4 +-
23 files changed, 1075 insertions(+), 340 deletions(-)
create mode 100644 security/integrity/ima/ima_init_ima_ns.c
create mode 100644 security/integrity/ima/ima_ns.c
create mode 100644 security/integrity/ima/ima_ns_status.c

--
2.31.1

2021-12-06 17:27:35

by Stefan Berger

Subject: [PATCH v3 16/16] ima: Setup securityfs for IMA namespace

Setup securityfs with symlinks, directories, and files for IMA
namespacing support. The same directory structure that IMA uses on the
host is also created for the namespacing case.

The securityfs file and directory ownerships cannot be set when the
IMA namespace is initialized. Therefore, delay the setup of the file
system to a later point when securityfs initializes the fs_context.
Use securityfs_register_ns_notifier() to register a notifier for
populating the filsystem late.

This filesystem can now be mounted as follows:

mount -t securityfs /sys/kernel/security/ /sys/kernel/security/

The following directories, symlinks, and files are then available.

$ ls -l sys/kernel/security/
total 0
lr--r--r--. 1 root root 0 Dec 2 00:18 ima -> integrity/ima
drwxr-xr-x. 3 root root 0 Dec 2 00:18 integrity

$ ls -l sys/kernel/security/ima/
total 0
-r--r-----. 1 root root 0 Dec 2 00:18 ascii_runtime_measurements
-r--r-----. 1 root root 0 Dec 2 00:18 binary_runtime_measurements
-rw-------. 1 root root 0 Dec 2 00:18 policy
-r--r-----. 1 root root 0 Dec 2 00:18 runtime_measurements_count
-r--r-----. 1 root root 0 Dec 2 00:18 violations

Signed-off-by: Stefan Berger <[email protected]>
Signed-off-by: James Bottomley <[email protected]>
---
include/linux/ima.h | 3 ++-
security/integrity/ima/ima_fs.c | 46 ++++++++++++++++++++++++++++++---
2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index bfb978a7f8d5..cab5fc6caeb3 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -221,7 +221,8 @@ struct ima_h_table {
};

enum {
- IMAFS_DENTRY_DIR = 0,
+ IMAFS_DENTRY_INTEGRITY_DIR = 0,
+ IMAFS_DENTRY_DIR,
IMAFS_DENTRY_SYMLINK,
IMAFS_DENTRY_BINARY_RUNTIME_MEASUREMENTS,
IMAFS_DENTRY_ASCII_RUNTIME_MEASUREMENTS,
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index c2a886c00ac2..c17a6b7eeb95 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -456,12 +456,25 @@ static void ima_fs_ns_free_dentries(struct ima_namespace *ns)
memset(ns->dentry, 0, sizeof(ns->dentry));
}

-static int __init ima_fs_ns_init(struct user_namespace *user_ns)
+static int ima_fs_ns_init(struct user_namespace *user_ns)
{
struct ima_namespace *ns = user_ns->ima_ns;
struct dentry *ima_dir;

- ns->dentry[IMAFS_DENTRY_DIR] = securityfs_create_dir("ima", integrity_dir);
+ /* already initialized? */
+ if (ns->dentry[IMAFS_DENTRY_INTEGRITY_DIR])
+ return 0;
+
+ /* FIXME: update when evm and integrity are namespaced */
+ if (user_ns != &init_user_ns)
+ ns->dentry[IMAFS_DENTRY_INTEGRITY_DIR] =
+ securityfs_create_dir("integrity", NULL);
+ else
+ ns->dentry[IMAFS_DENTRY_INTEGRITY_DIR] = integrity_dir;
+
+ ns->dentry[IMAFS_DENTRY_DIR] =
+ securityfs_create_dir("ima",
+ ns->dentry[IMAFS_DENTRY_INTEGRITY_DIR]);
if (IS_ERR(ns->dentry[IMAFS_DENTRY_DIR]))
return -1;
ima_dir = ns->dentry[IMAFS_DENTRY_DIR];
@@ -511,7 +524,34 @@ static int __init ima_fs_ns_init(struct user_namespace *user_ns)
return -1;
}

-int __init ima_fs_init(void)
+static int ima_ns_notify(struct notifier_block *this, unsigned long msg,
+ void *data)
{
+ int rc = 0;
+ struct user_namespace *user_ns = data;
+
+ switch (msg) {
+ case SECURITYFS_NS_ADD:
+ rc = ima_fs_ns_init(user_ns);
+ break;
+ case SECURITYFS_NS_REMOVE:
+ ima_fs_ns_free_dentries(user_ns->ima_ns);
+ break;
+ }
+ return rc;
+}
+
+static struct notifier_block ima_ns_notifier = {
+ .notifier_call = ima_ns_notify,
+};
+
+int ima_fs_init()
+{
+ int rc;
+
+ rc = securityfs_register_ns_notifier(&ima_ns_notifier);
+ if (rc)
+ return rc;
+
return ima_fs_ns_init(&init_user_ns);
}
--
2.31.1

2021-12-06 17:27:38

by Stefan Berger

Subject: [PATCH v3 06/16] ima: Move policy related variables into ima_namespace

Move variables related to the IMA policy into the ima_namespace. This way
the IMA policy of an IMA namespace can be set and displayed using a
front-end like SecurityFS.

Implement ima_free_policy_rules() that frees the policy rules on
ima_namespace deletion.

Signed-off-by: Stefan Berger <[email protected]>
---
include/linux/ima.h | 10 ++
security/integrity/ima/ima.h | 35 ++---
security/integrity/ima/ima_api.c | 8 +-
security/integrity/ima/ima_appraise.c | 26 ++--
security/integrity/ima/ima_asymmetric_keys.c | 3 +-
security/integrity/ima/ima_fs.c | 11 +-
security/integrity/ima/ima_init.c | 10 +-
security/integrity/ima/ima_init_ima_ns.c | 6 +
security/integrity/ima/ima_main.c | 68 +++++-----
security/integrity/ima/ima_ns.c | 1 +
security/integrity/ima/ima_policy.c | 128 ++++++++++---------
security/integrity/ima/ima_queue_keys.c | 2 +-
12 files changed, 180 insertions(+), 128 deletions(-)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 977df9155cde..e13e63a539d8 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -239,9 +239,19 @@ struct ima_namespace {
long ima_key_queue_timeout;
bool timer_expired;
#endif
+
+ struct list_head ima_default_rules;
+ /* ns's policy rules */
+ struct list_head ima_policy_rules;
+ struct list_head ima_temp_rules;
+ /* Pointer to ns's current policy */
+ struct list_head __rcu *ima_rules;
+ /* current content of the policy */
+ int ima_policy_flag;
};

extern struct ima_namespace init_ima_ns;
+extern struct list_head ima_default_rules;

#ifdef CONFIG_IMA_NS

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 97eb03376855..e295141f2478 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -43,9 +43,6 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };

#define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks : 0)

-/* current content of the policy */
-extern int ima_policy_flag;
-
/* bitset of digests algorithms allowed in the setxattr hook */
extern atomic_t ima_setxattr_allowed_hash_algorithms;

@@ -265,7 +262,8 @@ static inline void ima_process_queued_keys(struct ima_namespace *ns) {}
#endif /* CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS */

/* LIM API function definitions */
-int ima_get_action(struct user_namespace *mnt_userns, struct inode *inode,
+int ima_get_action(struct ima_namespace *ns,
+ struct user_namespace *mnt_userns, struct inode *inode,
const struct cred *cred, u32 secid, int mask,
enum ima_hooks func, int *pcr,
struct ima_template_desc **template_desc,
@@ -279,7 +277,8 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
struct evm_ima_xattr_data *xattr_value,
int xattr_len, const struct modsig *modsig, int pcr,
struct ima_template_desc *template_desc);
-int process_buffer_measurement(struct user_namespace *mnt_userns,
+int process_buffer_measurement(struct ima_namespace *ns,
+ struct user_namespace *mnt_userns,
struct inode *inode, const void *buf, int size,
const char *eventname, enum ima_hooks func,
int pcr, const char *func_data,
@@ -297,17 +296,19 @@ void ima_free_template_entry(struct ima_template_entry *entry);
const char *ima_d_path(const struct path *path, char **pathbuf, char *filename);

/* IMA policy related functions */
-int ima_match_policy(struct user_namespace *mnt_userns, struct inode *inode,
+int ima_match_policy(struct ima_namespace *ns,
+ struct user_namespace *mnt_userns, struct inode *inode,
const struct cred *cred, u32 secid, enum ima_hooks func,
int mask, int flags, int *pcr,
struct ima_template_desc **template_desc,
const char *func_data, unsigned int *allowed_algos);
-void ima_init_policy(void);
+void ima_init_policy(struct ima_namespace *ns);
void ima_update_policy(struct ima_namespace *ns);
-void ima_update_policy_flags(void);
-ssize_t ima_parse_add_rule(char *);
-void ima_delete_rules(void);
-int ima_check_policy(void);
+void ima_update_policy_flags(struct ima_namespace *ns);
+ssize_t ima_parse_add_rule(struct ima_namespace *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);
void *ima_policy_start(struct seq_file *m, loff_t *pos);
void *ima_policy_next(struct seq_file *m, void *v, loff_t *pos);
void ima_policy_stop(struct seq_file *m, void *v);
@@ -323,14 +324,16 @@ int ima_policy_show(struct seq_file *m, void *v);
#define IMA_APPRAISE_KEXEC 0x40

#ifdef CONFIG_IMA_APPRAISE
-int ima_check_blacklist(struct integrity_iint_cache *iint,
+int ima_check_blacklist(struct ima_namespace *ns,
+ struct integrity_iint_cache *iint,
const struct modsig *modsig, int pcr);
int ima_appraise_measurement(enum ima_hooks func,
struct integrity_iint_cache *iint,
struct file *file, const unsigned char *filename,
struct evm_ima_xattr_data *xattr_value,
int xattr_len, const struct modsig *modsig);
-int ima_must_appraise(struct user_namespace *mnt_userns, struct inode *inode,
+int ima_must_appraise(struct ima_namespace *ns,
+ struct user_namespace *mnt_userns, struct inode *inode,
int mask, enum ima_hooks func);
void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
@@ -341,7 +344,8 @@ int ima_read_xattr(struct dentry *dentry,
struct evm_ima_xattr_data **xattr_value);

#else
-static inline int ima_check_blacklist(struct integrity_iint_cache *iint,
+static inline int ima_check_blacklist(struct ima_namespace *ns,
+ struct integrity_iint_cache *iint,
const struct modsig *modsig, int pcr)
{
return 0;
@@ -358,7 +362,8 @@ static inline int ima_appraise_measurement(enum ima_hooks func,
return INTEGRITY_UNKNOWN;
}

-static inline int ima_must_appraise(struct user_namespace *mnt_userns,
+static inline int ima_must_appraise(struct ima_namespace *ns,
+ struct user_namespace *mnt_userns,
struct inode *inode, int mask,
enum ima_hooks func)
{
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 8f7bab17b784..808aec56dbb6 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -14,6 +14,7 @@
#include <linux/xattr.h>
#include <linux/evm.h>
#include <linux/iversion.h>
+#include <linux/ima.h>

#include "ima.h"

@@ -185,7 +186,8 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
* Returns IMA_MEASURE, IMA_APPRAISE mask.
*
*/
-int ima_get_action(struct user_namespace *mnt_userns, struct inode *inode,
+int ima_get_action(struct ima_namespace *ns,
+ struct user_namespace *mnt_userns, struct inode *inode,
const struct cred *cred, u32 secid, int mask,
enum ima_hooks func, int *pcr,
struct ima_template_desc **template_desc,
@@ -193,9 +195,9 @@ int ima_get_action(struct user_namespace *mnt_userns, struct inode *inode,
{
int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE | IMA_HASH;

- flags &= ima_policy_flag;
+ flags &= ns->ima_policy_flag;

- return ima_match_policy(mnt_userns, inode, cred, secid, func, mask,
+ return ima_match_policy(ns, mnt_userns, inode, cred, secid, func, mask,
flags, pcr, template_desc, func_data,
allowed_algos);
}
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index dbba51583e7c..b0c1992d8c4b 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -68,7 +68,8 @@ bool is_ima_appraise_enabled(void)
*
* Return 1 to appraise or hash
*/
-int ima_must_appraise(struct user_namespace *mnt_userns, struct inode *inode,
+int ima_must_appraise(struct ima_namespace *ns,
+ struct user_namespace *mnt_userns, struct inode *inode,
int mask, enum ima_hooks func)
{
u32 secid;
@@ -77,7 +78,7 @@ int ima_must_appraise(struct user_namespace *mnt_userns, struct inode *inode,
return 0;

security_task_getsecid_subj(current, &secid);
- return ima_match_policy(mnt_userns, inode, current_cred(), secid,
+ return ima_match_policy(ns, mnt_userns, inode, current_cred(), secid,
func, mask, IMA_APPRAISE | IMA_HASH, NULL,
NULL, NULL, NULL);
}
@@ -341,7 +342,8 @@ static int modsig_verify(enum ima_hooks func, const struct modsig *modsig,
*
* Returns -EPERM if the hash is blacklisted.
*/
-int ima_check_blacklist(struct integrity_iint_cache *iint,
+int ima_check_blacklist(struct ima_namespace *ns,
+ struct integrity_iint_cache *iint,
const struct modsig *modsig, int pcr)
{
enum hash_algo hash_algo;
@@ -357,7 +359,7 @@ int ima_check_blacklist(struct integrity_iint_cache *iint,

rc = is_binary_blacklisted(digest, digestsize);
if ((rc == -EPERM) && (iint->flags & IMA_MEASURE))
- process_buffer_measurement(&init_user_ns, NULL, digest, digestsize,
+ process_buffer_measurement(ns, &init_user_ns, NULL, digest, digestsize,
"blacklisted-hash", NONE,
pcr, NULL, false, NULL, 0);
}
@@ -527,14 +529,15 @@ void ima_inode_post_setattr(struct user_namespace *mnt_userns,
struct dentry *dentry)
{
struct inode *inode = d_backing_inode(dentry);
+ struct ima_namespace *ns = get_current_ns();
struct integrity_iint_cache *iint;
int action;

- if (!(ima_policy_flag & IMA_APPRAISE) || !S_ISREG(inode->i_mode)
+ if (!(ns->ima_policy_flag & IMA_APPRAISE) || !S_ISREG(inode->i_mode)
|| !(inode->i_opflags & IOP_XATTR))
return;

- action = ima_must_appraise(mnt_userns, inode, MAY_ACCESS, POST_SETATTR);
+ action = ima_must_appraise(ns, mnt_userns, inode, MAY_ACCESS, POST_SETATTR);
iint = integrity_iint_find(inode);
if (iint) {
set_bit(IMA_CHANGE_ATTR, &iint->atomic_flags);
@@ -559,11 +562,12 @@ static int ima_protect_xattr(struct dentry *dentry, const char *xattr_name,
return 0;
}

-static void ima_reset_appraise_flags(struct inode *inode, int digsig)
+static void ima_reset_appraise_flags(struct ima_namespace *ns,
+ struct inode *inode, int digsig)
{
struct integrity_iint_cache *iint;

- if (!(ima_policy_flag & IMA_APPRAISE) || !S_ISREG(inode->i_mode))
+ if (!(ns->ima_policy_flag & IMA_APPRAISE) || !S_ISREG(inode->i_mode))
return;

iint = integrity_iint_find(inode);
@@ -641,6 +645,7 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
const void *xattr_value, size_t xattr_value_len)
{
const struct evm_ima_xattr_data *xvalue = xattr_value;
+ struct ima_namespace *ns = get_current_ns();
int digsig = 0;
int result;

@@ -658,18 +663,19 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
if (result)
return result;

- ima_reset_appraise_flags(d_backing_inode(dentry), digsig);
+ ima_reset_appraise_flags(ns, d_backing_inode(dentry), digsig);
}
return result;
}

int ima_inode_removexattr(struct dentry *dentry, const char *xattr_name)
{
+ struct ima_namespace *ns = get_current_ns();
int result;

result = ima_protect_xattr(dentry, xattr_name, NULL, 0);
if (result == 1 || evm_revalidate_status(xattr_name)) {
- ima_reset_appraise_flags(d_backing_inode(dentry), 0);
+ ima_reset_appraise_flags(ns, d_backing_inode(dentry), 0);
if (result == 1)
result = 0;
}
diff --git a/security/integrity/ima/ima_asymmetric_keys.c b/security/integrity/ima/ima_asymmetric_keys.c
index b20e82eda8f4..b5fe4ed62fec 100644
--- a/security/integrity/ima/ima_asymmetric_keys.c
+++ b/security/integrity/ima/ima_asymmetric_keys.c
@@ -61,7 +61,8 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
* if the IMA policy is configured to measure a key linked
* to the given keyring.
*/
- process_buffer_measurement(&init_user_ns, NULL, payload, payload_len,
+ process_buffer_measurement(get_current_ns(), &init_user_ns, NULL,
+ payload, payload_len,
keyring->description, KEY_CHECK, 0,
keyring->description, false, NULL, 0);
}
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index b89cd69df0de..fc0413c8c358 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -297,7 +297,7 @@ static ssize_t ima_read_policy(char *path)
datap = data;
while (size > 0 && (p = strsep(&datap, "\n"))) {
pr_debug("rule: %s\n", p);
- rc = ima_parse_add_rule(p);
+ rc = ima_parse_add_rule(get_current_ns(), p);
if (rc < 0)
break;
size -= rc;
@@ -345,7 +345,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
1, 0);
result = -EACCES;
} else {
- result = ima_parse_add_rule(data);
+ result = ima_parse_add_rule(get_current_ns(), data);
}
mutex_unlock(&ima_write_mutex);
out_free:
@@ -411,11 +411,12 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
static int ima_release_policy(struct inode *inode, struct file *file)
{
const char *cause = valid_policy ? "completed" : "failed";
+ struct ima_namespace *ns = get_current_ns();

if ((file->f_flags & O_ACCMODE) == O_RDONLY)
return seq_release(inode, file);

- if (valid_policy && ima_check_policy() < 0) {
+ if (valid_policy && ima_check_policy(ns) < 0) {
cause = "failed";
valid_policy = 0;
}
@@ -425,13 +426,13 @@ static int ima_release_policy(struct inode *inode, struct file *file)
"policy_update", cause, !valid_policy, 0);

if (!valid_policy) {
- ima_delete_rules();
+ ima_delete_rules(ns);
valid_policy = 1;
clear_bit(IMA_FS_BUSY, &ima_fs_flags);
return 0;
}

- ima_update_policy(get_current_ns());
+ ima_update_policy(ns);
#if !defined(CONFIG_IMA_WRITE_POLICY) && !defined(CONFIG_IMA_READ_POLICY)
securityfs_remove(ima_policy);
ima_policy = NULL;
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 24848373a061..9f7048b94955 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -102,17 +102,17 @@ static int __init ima_add_boot_aggregate(void)
}

#ifdef CONFIG_IMA_LOAD_X509
-void __init ima_load_x509(void)
+void __init ima_load_x509()
{
- int unset_flags = ima_policy_flag & IMA_APPRAISE;
+ int unset_flags = init_ima_ns.ima_policy_flag & IMA_APPRAISE;

- ima_policy_flag &= ~unset_flags;
+ init_ima_ns.ima_policy_flag &= ~unset_flags;
integrity_load_x509(INTEGRITY_KEYRING_IMA, CONFIG_IMA_X509_PATH);

/* load also EVM key to avoid appraisal */
evm_load_x509();

- ima_policy_flag |= unset_flags;
+ init_ima_ns.ima_policy_flag |= unset_flags;
}
#endif

@@ -149,7 +149,7 @@ int __init ima_init(void)
if (rc != 0)
return rc;

- ima_init_policy();
+ ima_init_policy(&init_ima_ns);

rc = ima_fs_init();
if (rc != 0)
diff --git a/security/integrity/ima/ima_init_ima_ns.c b/security/integrity/ima/ima_init_ima_ns.c
index 7b66fe598789..2d644791a795 100644
--- a/security/integrity/ima/ima_init_ima_ns.c
+++ b/security/integrity/ima/ima_init_ima_ns.c
@@ -34,6 +34,12 @@ int ima_init_namespace(struct ima_namespace *ns)
ima_init_key_queue(ns);
#endif

+ INIT_LIST_HEAD(&ns->ima_default_rules);
+ INIT_LIST_HEAD(&ns->ima_policy_rules);
+ INIT_LIST_HEAD(&ns->ima_temp_rules);
+ ns->ima_rules = (struct list_head __rcu *)(&ns->ima_default_rules);
+ ns->ima_policy_flag = 0;
+
return 0;
}

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 4df60dbb56f7..9cf1fd7c70bf 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -188,7 +188,7 @@ void ima_file_free(struct file *file)
struct inode *inode = file_inode(file);
struct integrity_iint_cache *iint;

- if (!ima_policy_flag || !S_ISREG(inode->i_mode))
+ if (!get_current_ns()->ima_policy_flag || !S_ISREG(inode->i_mode))
return;

iint = integrity_iint_find(inode);
@@ -198,7 +198,8 @@ void ima_file_free(struct file *file)
ima_check_last_writer(iint, inode, file);
}

-static int process_measurement(struct file *file, const struct cred *cred,
+static int process_measurement(struct ima_namespace *ns,
+ struct file *file, const struct cred *cred,
u32 secid, char *buf, loff_t size, int mask,
enum ima_hooks func)
{
@@ -219,18 +220,18 @@ static int process_measurement(struct file *file, const struct cred *cred,
unsigned int allowed_algos = 0;
unsigned long flags;

- if (!ima_policy_flag || !S_ISREG(inode->i_mode))
+ if (!ns->ima_policy_flag || !S_ISREG(inode->i_mode))
return 0;

/* Return an IMA_MEASURE, IMA_APPRAISE, IMA_AUDIT action
* bitmask based on the appraise/audit/measurement policy.
* Included is the appraise submask.
*/
- action = ima_get_action(file_mnt_user_ns(file), inode, cred, secid,
+ action = ima_get_action(ns, file_mnt_user_ns(file), inode, cred, secid,
mask, func, &pcr, &template_desc, NULL,
&allowed_algos);
violation_check = ((func == FILE_CHECK || func == MMAP_CHECK) &&
- (ima_policy_flag & IMA_MEASURE));
+ (ns->ima_policy_flag & IMA_MEASURE));
if (!action && !violation_check)
return 0;

@@ -248,7 +249,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
rc = -ENOMEM;

if (!rc && (action & IMA_NS_STATUS_ACTIONS)) {
- status = ima_get_ns_status(get_current_ns(), inode);
+ status = ima_get_ns_status(ns, inode);
if (IS_ERR(status))
rc = PTR_ERR(status);
}
@@ -356,7 +357,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
xattr_value, xattr_len, modsig, pcr,
template_desc);
if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) {
- rc = ima_check_blacklist(iint, modsig, pcr);
+ rc = ima_check_blacklist(ns, iint, modsig, pcr);
if (rc != -EPERM) {
inode_lock(inode);
rc = ima_appraise_measurement(func, iint, file,
@@ -419,7 +420,8 @@ int ima_file_mmap(struct file *file, unsigned long prot)

if (file && (prot & PROT_EXEC)) {
security_task_getsecid_subj(current, &secid);
- return process_measurement(file, current_cred(), secid, NULL,
+ return process_measurement(get_current_ns(),
+ file, current_cred(), secid, NULL,
0, MAY_EXEC, MMAP_CHECK);
}

@@ -440,6 +442,7 @@ int ima_file_mmap(struct file *file, unsigned long prot)
*/
int ima_file_mprotect(struct vm_area_struct *vma, unsigned long prot)
{
+ struct ima_namespace *ns = get_current_ns();
struct ima_template_desc *template = NULL;
struct file *file = vma->vm_file;
char filename[NAME_MAX];
@@ -452,13 +455,13 @@ int ima_file_mprotect(struct vm_area_struct *vma, unsigned long prot)
int pcr;

/* Is mprotect making an mmap'ed file executable? */
- if (!(ima_policy_flag & IMA_APPRAISE) || !vma->vm_file ||
+ if (!(ns->ima_policy_flag & IMA_APPRAISE) || !vma->vm_file ||
!(prot & PROT_EXEC) || (vma->vm_flags & VM_EXEC))
return 0;

security_task_getsecid_subj(current, &secid);
inode = file_inode(vma->vm_file);
- action = ima_get_action(file_mnt_user_ns(vma->vm_file), inode,
+ action = ima_get_action(ns, file_mnt_user_ns(vma->vm_file), inode,
current_cred(), secid, MAY_EXEC, MMAP_CHECK,
&pcr, &template, NULL, NULL);

@@ -498,13 +501,13 @@ int ima_bprm_check(struct linux_binprm *bprm)
u32 secid;

security_task_getsecid_subj(current, &secid);
- ret = process_measurement(bprm->file, current_cred(), secid, NULL, 0,
+ ret = process_measurement(get_current_ns(), bprm->file, current_cred(), secid, NULL, 0,
MAY_EXEC, BPRM_CHECK);
if (ret)
return ret;

security_cred_getsecid(bprm->cred, &secid);
- return process_measurement(bprm->file, bprm->cred, secid, NULL, 0,
+ return process_measurement(get_current_ns(), bprm->file, bprm->cred, secid, NULL, 0,
MAY_EXEC, CREDS_CHECK);
}

@@ -523,18 +526,19 @@ int ima_file_check(struct file *file, int mask)
u32 secid;

security_task_getsecid_subj(current, &secid);
- return process_measurement(file, current_cred(), secid, NULL, 0,
+ return process_measurement(get_current_ns(), file, current_cred(), secid, NULL, 0,
mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
MAY_APPEND), FILE_CHECK);
}
EXPORT_SYMBOL_GPL(ima_file_check);

-static int __ima_inode_hash(struct inode *inode, char *buf, size_t buf_size)
+static int __ima_inode_hash(struct ima_namespace *ns,
+ struct inode *inode, char *buf, size_t buf_size)
{
struct integrity_iint_cache *iint;
int hash_algo;

- if (!ima_policy_flag)
+ if (!ns->ima_policy_flag)
return -EOPNOTSUPP;

iint = integrity_iint_find(inode);
@@ -587,7 +591,7 @@ int ima_file_hash(struct file *file, char *buf, size_t buf_size)
if (!file)
return -EINVAL;

- return __ima_inode_hash(file_inode(file), buf, buf_size);
+ return __ima_inode_hash(get_current_ns(), file_inode(file), buf, buf_size);
}
EXPORT_SYMBOL_GPL(ima_file_hash);

@@ -614,7 +618,7 @@ int ima_inode_hash(struct inode *inode, char *buf, size_t buf_size)
if (!inode)
return -EINVAL;

- return __ima_inode_hash(inode, buf, buf_size);
+ return __ima_inode_hash(get_current_ns(), inode, buf, buf_size);
}
EXPORT_SYMBOL_GPL(ima_inode_hash);

@@ -630,13 +634,14 @@ EXPORT_SYMBOL_GPL(ima_inode_hash);
void ima_post_create_tmpfile(struct user_namespace *mnt_userns,
struct inode *inode)
{
+ struct ima_namespace *ns = get_current_ns();
struct integrity_iint_cache *iint;
int must_appraise;

- if (!ima_policy_flag || !S_ISREG(inode->i_mode))
+ if (!ns->ima_policy_flag || !S_ISREG(inode->i_mode))
return;

- must_appraise = ima_must_appraise(mnt_userns, inode, MAY_ACCESS,
+ must_appraise = ima_must_appraise(ns, mnt_userns, inode, MAY_ACCESS,
FILE_CHECK);
if (!must_appraise)
return;
@@ -662,14 +667,15 @@ void ima_post_create_tmpfile(struct user_namespace *mnt_userns,
void ima_post_path_mknod(struct user_namespace *mnt_userns,
struct dentry *dentry)
{
+ struct ima_namespace *ns = get_current_ns();
struct integrity_iint_cache *iint;
struct inode *inode = dentry->d_inode;
int must_appraise;

- if (!ima_policy_flag || !S_ISREG(inode->i_mode))
+ if (!ns->ima_policy_flag || !S_ISREG(inode->i_mode))
return;

- must_appraise = ima_must_appraise(mnt_userns, inode, MAY_ACCESS,
+ must_appraise = ima_must_appraise(ns, mnt_userns, inode, MAY_ACCESS,
FILE_CHECK);
if (!must_appraise)
return;
@@ -720,7 +726,7 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
/* Read entire file for all partial reads. */
func = read_idmap[read_id] ?: FILE_CHECK;
security_task_getsecid_subj(current, &secid);
- return process_measurement(file, current_cred(), secid, NULL,
+ return process_measurement(get_current_ns(), file, current_cred(), secid, NULL,
0, MAY_READ, func);
}

@@ -763,7 +769,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,

func = read_idmap[read_id] ?: FILE_CHECK;
security_task_getsecid_subj(current, &secid);
- return process_measurement(file, current_cred(), secid, buf, size,
+ return process_measurement(get_current_ns(), file, current_cred(), secid, buf, size,
MAY_READ, func);
}

@@ -869,7 +875,8 @@ int ima_post_load_data(char *buf, loff_t size,
* has been written to the passed location but not added to a measurement entry,
* a negative value otherwise.
*/
-int process_buffer_measurement(struct user_namespace *mnt_userns,
+int process_buffer_measurement(struct ima_namespace *ns,
+ struct user_namespace *mnt_userns,
struct inode *inode, const void *buf, int size,
const char *eventname, enum ima_hooks func,
int pcr, const char *func_data,
@@ -897,7 +904,7 @@ int process_buffer_measurement(struct user_namespace *mnt_userns,
if (digest && digest_len < digest_hash_len)
return -EINVAL;

- if (!ima_policy_flag && !digest)
+ if (!ns->ima_policy_flag && !digest)
return -ENOENT;

template = ima_template_desc_buf();
@@ -916,7 +923,7 @@ int process_buffer_measurement(struct user_namespace *mnt_userns,
*/
if (func) {
security_task_getsecid_subj(current, &secid);
- action = ima_get_action(mnt_userns, inode, current_cred(),
+ action = ima_get_action(ns, mnt_userns, inode, current_cred(),
secid, 0, func, &pcr, &template,
func_data, NULL);
if (!(action & IMA_MEASURE) && !digest)
@@ -953,7 +960,7 @@ int process_buffer_measurement(struct user_namespace *mnt_userns,
if (digest)
memcpy(digest, iint.ima_hash->digest, digest_hash_len);

- if (!ima_policy_flag || (func && !(action & IMA_MEASURE)))
+ if (!ns->ima_policy_flag || (func && !(action & IMA_MEASURE)))
return 1;

ret = ima_alloc_init_template(&event_data, &entry, template);
@@ -996,7 +1003,8 @@ void ima_kexec_cmdline(int kernel_fd, const void *buf, int size)
if (!f.file)
return;

- process_buffer_measurement(file_mnt_user_ns(f.file), file_inode(f.file),
+ process_buffer_measurement(get_current_ns(),
+ file_mnt_user_ns(f.file), file_inode(f.file),
buf, size, "kexec-cmdline", KEXEC_CMDLINE, 0,
NULL, false, NULL, 0);
fdput(f);
@@ -1029,7 +1037,7 @@ int ima_measure_critical_data(const char *event_label,
if (!event_name || !event_label || !buf || !buf_len)
return -ENOPARAM;

- return process_buffer_measurement(&init_user_ns, NULL, buf, buf_len,
+ return process_buffer_measurement(get_current_ns(), &init_user_ns, NULL, buf, buf_len,
event_name, CRITICAL_DATA, 0,
event_label, hash, digest,
digest_len);
@@ -1062,7 +1070,7 @@ static int __init init_ima(void)
pr_warn("Couldn't register LSM notifier, error %d\n", error);

if (!error)
- ima_update_policy_flags();
+ ima_update_policy_flags(&init_ima_ns);

return error;
}
diff --git a/security/integrity/ima/ima_ns.c b/security/integrity/ima/ima_ns.c
index 11e0343f1f55..efbf7087a8ee 100644
--- a/security/integrity/ima/ima_ns.c
+++ b/security/integrity/ima/ima_ns.c
@@ -67,6 +67,7 @@ struct ima_namespace *copy_ima_ns(struct ima_namespace *old_ns,
static void destroy_ima_ns(struct ima_namespace *ns)
{
pr_debug("DESTROY ima_ns: 0x%p\n", ns);
+ ima_free_policy_rules(ns);
free_ns_status_cache(ns);
kmem_cache_free(imans_cachep, ns);
}
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index e5aef287d14d..96e7d63167e8 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -52,7 +52,6 @@
#define INVALID_PCR(a) (((a) < 0) || \
(a) >= (sizeof_field(struct integrity_iint_cache, measured_pcrs) * 8))

-int ima_policy_flag;
static int temp_ima_appraise;
static int build_ima_appraise __ro_after_init;

@@ -233,11 +232,6 @@ static struct ima_rule_entry critical_data_rules[] __ro_after_init = {
/* An array of architecture specific rules */
static struct ima_rule_entry *arch_policy_entry __ro_after_init;

-static LIST_HEAD(ima_default_rules);
-static LIST_HEAD(ima_policy_rules);
-static LIST_HEAD(ima_temp_rules);
-static struct list_head __rcu *ima_rules = (struct list_head __rcu *)(&ima_default_rules);
-
static int ima_policy __initdata;

static int __init default_measure_policy_setup(char *str)
@@ -454,12 +448,12 @@ static bool ima_rule_contains_lsm_cond(struct ima_rule_entry *entry)
* to the old, stale LSM policy. Update the IMA LSM based rules to reflect
* the reloaded LSM policy.
*/
-static void ima_lsm_update_rules(void)
+static void ima_lsm_update_rules(struct ima_namespace *ns)
{
struct ima_rule_entry *entry, *e;
int result;

- list_for_each_entry_safe(entry, e, &ima_policy_rules, list) {
+ list_for_each_entry_safe(entry, e, &ns->ima_policy_rules, list) {
if (!ima_rule_contains_lsm_cond(entry))
continue;

@@ -477,7 +471,7 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
if (event != LSM_POLICY_CHANGE)
return NOTIFY_DONE;

- ima_lsm_update_rules();
+ ima_lsm_update_rules(get_current_ns());
return NOTIFY_OK;
}

@@ -688,7 +682,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
* list when walking it. Reads are many orders of magnitude more numerous
* than writes so ima_match_policy() is classical RCU candidate.
*/
-int ima_match_policy(struct user_namespace *mnt_userns, struct inode *inode,
+int ima_match_policy(struct ima_namespace *ns,
+ struct user_namespace *mnt_userns, struct inode *inode,
const struct cred *cred, u32 secid, enum ima_hooks func,
int mask, int flags, int *pcr,
struct ima_template_desc **template_desc,
@@ -702,7 +697,7 @@ int ima_match_policy(struct user_namespace *mnt_userns, struct inode *inode,
*template_desc = ima_template_desc_current();

rcu_read_lock();
- ima_rules_tmp = rcu_dereference(ima_rules);
+ ima_rules_tmp = rcu_dereference(ns->ima_rules);
list_for_each_entry_rcu(entry, ima_rules_tmp, list) {

if (!(entry->action & actmask))
@@ -760,14 +755,14 @@ int ima_match_policy(struct user_namespace *mnt_userns, struct inode *inode,
*
* Context: called after a policy update and at system initialization.
*/
-void ima_update_policy_flags(void)
+void ima_update_policy_flags(struct ima_namespace *ns)
{
struct ima_rule_entry *entry;
int new_policy_flag = 0;
struct list_head *ima_rules_tmp;

rcu_read_lock();
- ima_rules_tmp = rcu_dereference(ima_rules);
+ ima_rules_tmp = rcu_dereference(ns->ima_rules);
list_for_each_entry_rcu(entry, ima_rules_tmp, list) {
/*
* SETXATTR_CHECK rules do not implement a full policy check
@@ -797,7 +792,7 @@ void ima_update_policy_flags(void)
if (!ima_appraise)
new_policy_flag &= ~IMA_APPRAISE;

- ima_policy_flag = new_policy_flag;
+ ns->ima_policy_flag = new_policy_flag;
}

static int ima_appraise_flag(enum ima_hooks func)
@@ -813,7 +808,8 @@ static int ima_appraise_flag(enum ima_hooks func)
return 0;
}

-static void add_rules(struct ima_rule_entry *entries, int count,
+static void add_rules(struct ima_namespace *ns,
+ struct ima_rule_entry *entries, int count,
enum policy_rule_list policy_rule)
{
int i = 0;
@@ -822,7 +818,7 @@ static void add_rules(struct ima_rule_entry *entries, int count,
struct ima_rule_entry *entry;

if (policy_rule & IMA_DEFAULT_POLICY)
- list_add_tail(&entries[i].list, &ima_default_rules);
+ list_add_tail(&entries[i].list, &ns->ima_default_rules);

if (policy_rule & IMA_CUSTOM_POLICY) {
entry = kmemdup(&entries[i], sizeof(*entry),
@@ -830,7 +826,7 @@ static void add_rules(struct ima_rule_entry *entries, int count,
if (!entry)
continue;

- list_add_tail(&entry->list, &ima_policy_rules);
+ list_add_tail(&entry->list, &ns->ima_policy_rules);
}
if (entries[i].action == APPRAISE) {
if (entries != build_appraise_rules)
@@ -843,9 +839,10 @@ static void add_rules(struct ima_rule_entry *entries, int count,
}
}

-static int ima_parse_rule(char *rule, struct ima_rule_entry *entry);
+static int ima_parse_rule(struct ima_namespace *ns,
+ char *rule, struct ima_rule_entry *entry);

-static int __init ima_init_arch_policy(void)
+static int __init ima_init_arch_policy(struct ima_namespace *ns)
{
const char * const *arch_rules;
const char * const *rules;
@@ -873,7 +870,7 @@ static int __init ima_init_arch_policy(void)
result = strscpy(rule, *rules, sizeof(rule));

INIT_LIST_HEAD(&arch_policy_entry[i].list);
- result = ima_parse_rule(rule, &arch_policy_entry[i]);
+ result = ima_parse_rule(ns, rule, &arch_policy_entry[i]);
if (result) {
pr_warn("Skipping unknown architecture policy rule: %s\n",
rule);
@@ -891,23 +888,23 @@ static int __init ima_init_arch_policy(void)
*
* ima_rules points to either the ima_default_rules or the new ima_policy_rules.
*/
-void __init ima_init_policy(void)
+void __init ima_init_policy(struct ima_namespace *ns)
{
int build_appraise_entries, arch_entries;

/* if !ima_policy, we load NO default rules */
if (ima_policy)
- add_rules(dont_measure_rules, ARRAY_SIZE(dont_measure_rules),
+ add_rules(ns, dont_measure_rules, ARRAY_SIZE(dont_measure_rules),
IMA_DEFAULT_POLICY);

switch (ima_policy) {
case ORIGINAL_TCB:
- add_rules(original_measurement_rules,
+ add_rules(ns, original_measurement_rules,
ARRAY_SIZE(original_measurement_rules),
IMA_DEFAULT_POLICY);
break;
case DEFAULT_TCB:
- add_rules(default_measurement_rules,
+ add_rules(ns, default_measurement_rules,
ARRAY_SIZE(default_measurement_rules),
IMA_DEFAULT_POLICY);
break;
@@ -921,11 +918,11 @@ void __init ima_init_policy(void)
* and custom policies, prior to other appraise rules.
* (Highest priority)
*/
- arch_entries = ima_init_arch_policy();
+ arch_entries = ima_init_arch_policy(ns);
if (!arch_entries)
pr_info("No architecture policies found\n");
else
- add_rules(arch_policy_entry, arch_entries,
+ add_rules(ns, arch_policy_entry, arch_entries,
IMA_DEFAULT_POLICY | IMA_CUSTOM_POLICY);

/*
@@ -933,7 +930,7 @@ void __init ima_init_policy(void)
* signatures, prior to other appraise rules.
*/
if (ima_use_secure_boot)
- add_rules(secure_boot_rules, ARRAY_SIZE(secure_boot_rules),
+ add_rules(ns, secure_boot_rules, ARRAY_SIZE(secure_boot_rules),
IMA_DEFAULT_POLICY);

/*
@@ -945,32 +942,32 @@ void __init ima_init_policy(void)
build_appraise_entries = ARRAY_SIZE(build_appraise_rules);
if (build_appraise_entries) {
if (ima_use_secure_boot)
- add_rules(build_appraise_rules, build_appraise_entries,
+ add_rules(ns, build_appraise_rules, build_appraise_entries,
IMA_CUSTOM_POLICY);
else
- add_rules(build_appraise_rules, build_appraise_entries,
+ add_rules(ns, build_appraise_rules, build_appraise_entries,
IMA_DEFAULT_POLICY | IMA_CUSTOM_POLICY);
}

if (ima_use_appraise_tcb)
- add_rules(default_appraise_rules,
+ add_rules(ns, default_appraise_rules,
ARRAY_SIZE(default_appraise_rules),
IMA_DEFAULT_POLICY);

if (ima_use_critical_data)
- add_rules(critical_data_rules,
+ add_rules(ns, critical_data_rules,
ARRAY_SIZE(critical_data_rules),
IMA_DEFAULT_POLICY);

atomic_set(&ima_setxattr_allowed_hash_algorithms, 0);

- ima_update_policy_flags();
+ ima_update_policy_flags(ns);
}

/* Make sure we have a valid policy, at least containing some rules. */
-int ima_check_policy(void)
+int ima_check_policy(struct ima_namespace *ns)
{
- if (list_empty(&ima_temp_rules))
+ if (list_empty(&ns->ima_temp_rules))
return -EINVAL;
return 0;
}
@@ -988,14 +985,14 @@ int ima_check_policy(void)
*/
void ima_update_policy(struct ima_namespace *ns)
{
- struct list_head *policy = &ima_policy_rules;
+ struct list_head *policy = &ns->ima_policy_rules;

- list_splice_tail_init_rcu(&ima_temp_rules, policy, synchronize_rcu);
+ list_splice_tail_init_rcu(&ns->ima_temp_rules, policy, synchronize_rcu);

- if (ima_rules != (struct list_head __rcu *)policy) {
- ima_policy_flag = 0;
+ if (ns->ima_rules != (struct list_head __rcu *)policy) {
+ ns->ima_policy_flag = 0;

- rcu_assign_pointer(ima_rules, policy);
+ rcu_assign_pointer(ns->ima_rules, policy);
/*
* IMA architecture specific policy rules are specified
* as strings and converted to an array of ima_entry_rules
@@ -1004,7 +1001,7 @@ void ima_update_policy(struct ima_namespace *ns)
*/
kfree(arch_policy_entry);
}
- ima_update_policy_flags();
+ ima_update_policy_flags(ns);

/* Custom IMA policy has been loaded */
ima_process_queued_keys(ns);
@@ -1077,7 +1074,8 @@ static const match_table_t policy_tokens = {
{Opt_err, NULL}
};

-static int ima_lsm_rule_init(struct ima_rule_entry *entry,
+static int ima_lsm_rule_init(struct ima_namespace *ns,
+ struct ima_rule_entry *entry,
substring_t *args, int lsm_rule, int audit_type)
{
int result;
@@ -1097,7 +1095,7 @@ static int ima_lsm_rule_init(struct ima_rule_entry *entry,
pr_warn("rule for LSM \'%s\' is undefined\n",
entry->lsm[lsm_rule].args_p);

- if (ima_rules == (struct list_head __rcu *)(&ima_default_rules)) {
+ if (ns->ima_rules == (struct list_head __rcu *)(&ns->ima_default_rules)) {
kfree(entry->lsm[lsm_rule].args_p);
entry->lsm[lsm_rule].args_p = NULL;
result = -EINVAL;
@@ -1324,7 +1322,8 @@ static unsigned int ima_parse_appraise_algos(char *arg)
return res;
}

-static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
+static int ima_parse_rule(struct ima_namespace *ns,
+ char *rule, struct ima_rule_entry *entry)
{
struct audit_buffer *ab;
char *from;
@@ -1674,37 +1673,37 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
break;
case Opt_obj_user:
ima_log_string(ab, "obj_user", args[0].from);
- result = ima_lsm_rule_init(entry, args,
+ result = ima_lsm_rule_init(ns, entry, args,
LSM_OBJ_USER,
AUDIT_OBJ_USER);
break;
case Opt_obj_role:
ima_log_string(ab, "obj_role", args[0].from);
- result = ima_lsm_rule_init(entry, args,
+ result = ima_lsm_rule_init(ns, entry, args,
LSM_OBJ_ROLE,
AUDIT_OBJ_ROLE);
break;
case Opt_obj_type:
ima_log_string(ab, "obj_type", args[0].from);
- result = ima_lsm_rule_init(entry, args,
+ result = ima_lsm_rule_init(ns, entry, args,
LSM_OBJ_TYPE,
AUDIT_OBJ_TYPE);
break;
case Opt_subj_user:
ima_log_string(ab, "subj_user", args[0].from);
- result = ima_lsm_rule_init(entry, args,
+ result = ima_lsm_rule_init(ns, entry, args,
LSM_SUBJ_USER,
AUDIT_SUBJ_USER);
break;
case Opt_subj_role:
ima_log_string(ab, "subj_role", args[0].from);
- result = ima_lsm_rule_init(entry, args,
+ result = ima_lsm_rule_init(ns, entry, args,
LSM_SUBJ_ROLE,
AUDIT_SUBJ_ROLE);
break;
case Opt_subj_type:
ima_log_string(ab, "subj_type", args[0].from);
- result = ima_lsm_rule_init(entry, args,
+ result = ima_lsm_rule_init(ns, entry, args,
LSM_SUBJ_TYPE,
AUDIT_SUBJ_TYPE);
break;
@@ -1810,7 +1809,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
* Avoid locking by allowing just one writer at a time in ima_write_policy()
* Returns the length of the rule parsed, an error code on failure
*/
-ssize_t ima_parse_add_rule(char *rule)
+ssize_t ima_parse_add_rule(struct ima_namespace *ns, char *rule)
{
static const char op[] = "update_policy";
char *p;
@@ -1834,7 +1833,7 @@ ssize_t ima_parse_add_rule(char *rule)

INIT_LIST_HEAD(&entry->list);

- result = ima_parse_rule(p, entry);
+ result = ima_parse_rule(ns, p, entry);
if (result) {
ima_free_rule(entry);
integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
@@ -1843,7 +1842,7 @@ ssize_t ima_parse_add_rule(char *rule)
return result;
}

- list_add_tail(&entry->list, &ima_temp_rules);
+ list_add_tail(&entry->list, &ns->ima_temp_rules);

return len;
}
@@ -1854,12 +1853,24 @@ ssize_t ima_parse_add_rule(char *rule)
* different from the active one. There is also only one user of
* ima_delete_rules() at a time.
*/
-void ima_delete_rules(void)
+void ima_delete_rules(struct ima_namespace *ns)
{
struct ima_rule_entry *entry, *tmp;

temp_ima_appraise = 0;
- list_for_each_entry_safe(entry, tmp, &ima_temp_rules, list) {
+ list_for_each_entry_safe(entry, tmp, &ns->ima_temp_rules, list) {
+ list_del(&entry->list);
+ ima_free_rule(entry);
+ }
+}
+
+void ima_free_policy_rules(struct ima_namespace *ns)
+{
+ struct ima_rule_entry *entry, *tmp;
+
+ ima_delete_rules(ns);
+
+ list_for_each_entry_safe(entry, tmp, &ns->ima_policy_rules, list) {
list_del(&entry->list);
ima_free_rule(entry);
}
@@ -1890,7 +1901,7 @@ void *ima_policy_start(struct seq_file *m, loff_t *pos)
struct list_head *ima_rules_tmp;

rcu_read_lock();
- ima_rules_tmp = rcu_dereference(ima_rules);
+ ima_rules_tmp = rcu_dereference(get_current_ns()->ima_rules);
list_for_each_entry_rcu(entry, ima_rules_tmp, list) {
if (!l--) {
rcu_read_unlock();
@@ -1904,14 +1915,15 @@ void *ima_policy_start(struct seq_file *m, loff_t *pos)
void *ima_policy_next(struct seq_file *m, void *v, loff_t *pos)
{
struct ima_rule_entry *entry = v;
+ struct ima_namespace *ns = get_current_ns();

rcu_read_lock();
entry = list_entry_rcu(entry->list.next, struct ima_rule_entry, list);
rcu_read_unlock();
(*pos)++;

- return (&entry->list == &ima_default_rules ||
- &entry->list == &ima_policy_rules) ? NULL : entry;
+ return (&entry->list == &ns->ima_default_rules ||
+ &entry->list == &ns->ima_policy_rules) ? NULL : entry;
}

void ima_policy_stop(struct seq_file *m, void *v)
@@ -2177,7 +2189,7 @@ bool ima_appraise_signature(enum kernel_read_file_id id)
func = read_idmap[id] ?: FILE_CHECK;

rcu_read_lock();
- ima_rules_tmp = rcu_dereference(ima_rules);
+ ima_rules_tmp = rcu_dereference(get_current_ns()->ima_rules);
list_for_each_entry_rcu(entry, ima_rules_tmp, list) {
if (entry->action != APPRAISE)
continue;
diff --git a/security/integrity/ima/ima_queue_keys.c b/security/integrity/ima/ima_queue_keys.c
index 9e5e9a178253..14f334272160 100644
--- a/security/integrity/ima/ima_queue_keys.c
+++ b/security/integrity/ima/ima_queue_keys.c
@@ -142,7 +142,7 @@ void ima_process_queued_keys(struct ima_namespace *ns)

list_for_each_entry_safe(entry, tmp, &ns->ima_keys, list) {
if (!ns->timer_expired)
- process_buffer_measurement(&init_user_ns, NULL,
+ process_buffer_measurement(ns, &init_user_ns, NULL,
entry->payload,
entry->payload_len,
entry->keyring_name,
--
2.31.1

2021-12-06 17:27:42

by Stefan Berger

Subject: [PATCH v3 04/16] ima: Move delayed work queue and variables into ima_namespace

Move the delayed work queue and associated variables to the
ima_namespace and initialize them.

Since keys queued up for measurement currently are only relevant in the
init_ima_ns, call ima_init_key_queue() only when the init_ima_ns is
initialized.

Signed-off-by: Stefan Berger <[email protected]>
---
include/linux/ima.h | 11 ++++++++
security/integrity/ima/ima.h | 12 +++++----
security/integrity/ima/ima_fs.c | 3 ++-
security/integrity/ima/ima_init.c | 2 --
security/integrity/ima/ima_init_ima_ns.c | 8 ++++++
security/integrity/ima/ima_policy.c | 4 +--
security/integrity/ima/ima_queue_keys.c | 34 ++++++++++--------------
7 files changed, 44 insertions(+), 30 deletions(-)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index cc0e8c509fa2..4b5dada581e4 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -217,6 +217,17 @@ struct ima_namespace {
struct rb_root ns_status_tree;
rwlock_t ns_status_lock;
struct kmem_cache *ns_status_cache;
+
+#ifdef CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS
+ /*
+ * If custom IMA policy is not loaded then keys queued up
+ * for measurement should be freed. This worker is used
+ * for handling this scenario.
+ */
+ struct delayed_work ima_keys_delayed_work;
+ long ima_key_queue_timeout;
+ bool timer_expired;
+#endif
};

extern struct ima_namespace init_ima_ns;
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index dd06e16c4e1c..9edab9050dc7 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -77,6 +77,8 @@ struct ima_field_data {
u32 len;
};

+struct ima_namespace;
+
/* IMA template field definition */
struct ima_template_field {
const char field_id[IMA_TEMPLATE_FIELD_ID_MAX_LEN];
@@ -247,18 +249,18 @@ struct ima_key_entry {
size_t payload_len;
char *keyring_name;
};
-void ima_init_key_queue(void);
+void ima_init_key_queue(struct ima_namespace *ns);
bool ima_should_queue_key(void);
bool ima_queue_key(struct key *keyring, const void *payload,
size_t payload_len);
-void ima_process_queued_keys(void);
+void ima_process_queued_keys(struct ima_namespace *ns);
+void ima_keys_handler(struct work_struct *work);
#else
-static inline void ima_init_key_queue(void) {}
static inline bool ima_should_queue_key(void) { return false; }
static inline bool ima_queue_key(struct key *keyring,
const void *payload,
size_t payload_len) { return false; }
-static inline void ima_process_queued_keys(void) {}
+static inline void ima_process_queued_keys(struct ima_namespace *ns) {}
#endif /* CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS */

/* LIM API function definitions */
@@ -300,7 +302,7 @@ int ima_match_policy(struct user_namespace *mnt_userns, struct inode *inode,
struct ima_template_desc **template_desc,
const char *func_data, unsigned int *allowed_algos);
void ima_init_policy(void);
-void ima_update_policy(void);
+void ima_update_policy(struct ima_namespace *ns);
void ima_update_policy_flags(void);
ssize_t ima_parse_add_rule(char *);
void ima_delete_rules(void);
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 3d8e9d5db5aa..b89cd69df0de 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -21,6 +21,7 @@
#include <linux/rcupdate.h>
#include <linux/parser.h>
#include <linux/vmalloc.h>
+#include <linux/ima.h>

#include "ima.h"

@@ -430,7 +431,7 @@ static int ima_release_policy(struct inode *inode, struct file *file)
return 0;
}

- ima_update_policy();
+ ima_update_policy(get_current_ns());
#if !defined(CONFIG_IMA_WRITE_POLICY) && !defined(CONFIG_IMA_READ_POLICY)
securityfs_remove(ima_policy);
ima_policy = NULL;
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index f6ae4557a0da..24848373a061 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -155,8 +155,6 @@ int __init ima_init(void)
if (rc != 0)
return rc;

- ima_init_key_queue();
-
ima_measure_critical_data("kernel_info", "kernel_version",
UTS_RELEASE, strlen(UTS_RELEASE), false,
NULL, 0);
diff --git a/security/integrity/ima/ima_init_ima_ns.c b/security/integrity/ima/ima_init_ima_ns.c
index 1a44963e8ba9..3bc2d3611739 100644
--- a/security/integrity/ima/ima_init_ima_ns.c
+++ b/security/integrity/ima/ima_init_ima_ns.c
@@ -26,6 +26,14 @@ int ima_init_namespace(struct ima_namespace *ns)
if (!ns->ns_status_cache)
return -ENOMEM;

+#ifdef CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS
+ INIT_DELAYED_WORK(&ns->ima_keys_delayed_work, ima_keys_handler);
+ ns->ima_key_queue_timeout = 300000;
+ ns->timer_expired = false;
+ if (ns == &init_ima_ns)
+ ima_init_key_queue(ns);
+#endif
+
return 0;
}

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 320ca80aacab..e5aef287d14d 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -986,7 +986,7 @@ int ima_check_policy(void)
* Policy rules are never deleted so ima_policy_flag gets zeroed only once when
* we switch from the default policy to user defined.
*/
-void ima_update_policy(void)
+void ima_update_policy(struct ima_namespace *ns)
{
struct list_head *policy = &ima_policy_rules;

@@ -1007,7 +1007,7 @@ void ima_update_policy(void)
ima_update_policy_flags();

/* Custom IMA policy has been loaded */
- ima_process_queued_keys();
+ ima_process_queued_keys(ns);
}

/* Keep the enumeration in sync with the policy_tokens! */
diff --git a/security/integrity/ima/ima_queue_keys.c b/security/integrity/ima/ima_queue_keys.c
index 93056c03bf5a..fcaa1645dba3 100644
--- a/security/integrity/ima/ima_queue_keys.c
+++ b/security/integrity/ima/ima_queue_keys.c
@@ -10,6 +10,7 @@

#include <linux/user_namespace.h>
#include <linux/workqueue.h>
+#include <linux/ima.h>
#include <keys/asymmetric-type.h>
#include "ima.h"

@@ -25,34 +26,27 @@ static bool ima_process_keys;
static DEFINE_MUTEX(ima_keys_lock);
static LIST_HEAD(ima_keys);

-/*
- * If custom IMA policy is not loaded then keys queued up
- * for measurement should be freed. This worker is used
- * for handling this scenario.
- */
-static long ima_key_queue_timeout = 300000; /* 5 Minutes */
-static void ima_keys_handler(struct work_struct *work);
-static DECLARE_DELAYED_WORK(ima_keys_delayed_work, ima_keys_handler);
-static bool timer_expired;
-
/*
* This worker function frees keys that may still be
* queued up in case custom IMA policy was not loaded.
*/
-static void ima_keys_handler(struct work_struct *work)
+void ima_keys_handler(struct work_struct *work)
{
- timer_expired = true;
- ima_process_queued_keys();
+ struct ima_namespace *ns;
+
+ ns = container_of(work, struct ima_namespace, ima_keys_delayed_work.work);
+ ns->timer_expired = true;
+ ima_process_queued_keys(ns);
}

/*
* This function sets up a worker to free queued keys in case
* custom IMA policy was never loaded.
*/
-void ima_init_key_queue(void)
+void ima_init_key_queue(struct ima_namespace *ns)
{
- schedule_delayed_work(&ima_keys_delayed_work,
- msecs_to_jiffies(ima_key_queue_timeout));
+ schedule_delayed_work(&ns->ima_keys_delayed_work,
+ msecs_to_jiffies(ns->ima_key_queue_timeout));
}

static void ima_free_key_entry(struct ima_key_entry *entry)
@@ -130,7 +124,7 @@ bool ima_queue_key(struct key *keyring, const void *payload,
* This function sets ima_process_keys to true and processes queued keys.
* From here on keys will be processed right away (not queued).
*/
-void ima_process_queued_keys(void)
+void ima_process_queued_keys(struct ima_namespace *ns)
{
struct ima_key_entry *entry, *tmp;
bool process = false;
@@ -154,11 +148,11 @@ void ima_process_queued_keys(void)
if (!process)
return;

- if (!timer_expired)
- cancel_delayed_work_sync(&ima_keys_delayed_work);
+ if (!ns->timer_expired)
+ cancel_delayed_work_sync(&ns->ima_keys_delayed_work);

list_for_each_entry_safe(entry, tmp, &ima_keys, list) {
- if (!timer_expired)
+ if (!ns->timer_expired)
process_buffer_measurement(&init_user_ns, NULL,
entry->payload,
entry->payload_len,
--
2.31.1

2021-12-06 21:15:16

by James Bottomley

Subject: Re: [PATCH v3 00/16] ima: Namespace IMA with audit support in IMA-ns

On Mon, 2021-12-06 at 12:25 -0500, Stefan Berger wrote:
[...]
> v3:
> - Further modifications to virtualized SecurityFS following James's
> posted patch
> - Dropping of early teardown for user_namespaces since not needed
> anymore

This is my incremental to this series that moves the namespaced
securityfs away from using a vfsmount and on to a root dentry instead,
meaning we can call the blocking notifier from fill_super as Christian
requested (and thus can remove the securityfs_notifier_sent indicator
since it's only called once).

James

---

From 07b680d5fd59f5d3cea5580be25a2c9e08a01c3b Mon Sep 17 00:00:00 2001
From: James Bottomley <[email protected]>
Date: Mon, 6 Dec 2021 20:27:00 +0000
Subject: [PATCH] Incremental for d_root

---
include/linux/user_namespace.h | 3 +-
security/inode.c | 55 +++++++++++++---------------------
2 files changed, 22 insertions(+), 36 deletions(-)

diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index 6b8bd060d8c4..03a0879376a0 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -104,8 +104,7 @@ struct user_namespace {
struct ima_namespace *ima_ns;
#endif
#ifdef CONFIG_SECURITYFS
- struct vfsmount *securityfs_mount;
- bool securityfs_notifier_sent;
+ struct dentry *securityfs_root;
#endif
} __randomize_layout;

diff --git a/security/inode.c b/security/inode.c
index 45211845fc31..f8b6cb3dfb87 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -24,6 +24,7 @@
#include <linux/magic.h>
#include <linux/user_namespace.h>

+static struct vfsmount *securityfs_mount;
static int securityfs_mount_count;

static BLOCKING_NOTIFIER_HEAD(securityfs_ns_notifier);
@@ -40,43 +41,24 @@ static const struct super_operations securityfs_super_operations = {
.free_inode = securityfs_free_inode,
};

-static struct file_system_type fs_type;
-
-static void securityfs_free_context(struct fs_context *fc)
-{
- struct user_namespace *ns = fc->user_ns;
-
- if (ns == &init_user_ns ||
- ns->securityfs_notifier_sent)
- return;
-
- ns->securityfs_notifier_sent = true;
-
- ns->securityfs_mount = vfs_kern_mount(&fs_type, SB_KERNMOUNT,
- fs_type.name, NULL);
- if (IS_ERR(ns->securityfs_mount)) {
- printk(KERN_ERR "kern mount on securityfs ERROR: %ld\n",
- PTR_ERR(ns->securityfs_mount));
- ns->securityfs_mount = NULL;
- return;
- }
-
- blocking_notifier_call_chain(&securityfs_ns_notifier,
- SECURITYFS_NS_ADD, fc->user_ns);
- mntput(ns->securityfs_mount);
-}
-
static int securityfs_fill_super(struct super_block *sb, struct fs_context *fc)
{
static const struct tree_descr files[] = {{""}};
int error;
+ struct user_namespace *ns = fc->user_ns;

error = simple_fill_super(sb, SECURITYFS_MAGIC, files);
if (error)
return error;

+ ns->securityfs_root = dget(sb->s_root);
+
sb->s_op = &securityfs_super_operations;

+ if (ns != &init_user_ns)
+ blocking_notifier_call_chain(&securityfs_ns_notifier,
+ SECURITYFS_NS_ADD, ns);
+
return 0;
}

@@ -87,7 +69,6 @@ static int securityfs_get_tree(struct fs_context *fc)

static const struct fs_context_operations securityfs_context_ops = {
.get_tree = securityfs_get_tree,
- .free = securityfs_free_context,
};

static int securityfs_init_fs_context(struct fs_context *fc)
@@ -104,8 +85,10 @@ static void securityfs_kill_super(struct super_block *sb)
blocking_notifier_call_chain(&securityfs_ns_notifier,
SECURITYFS_NS_REMOVE,
sb->s_fs_info);
- ns->securityfs_notifier_sent = false;
- ns->securityfs_mount = NULL;
+
+ dput(ns->securityfs_root);
+ ns->securityfs_root = NULL;
+
kill_litter_super(sb);
}

@@ -174,14 +157,18 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
pr_debug("securityfs: creating file '%s'\n",name);

if (ns == &init_user_ns) {
- error = simple_pin_fs(&fs_type, &ns->securityfs_mount,
+ error = simple_pin_fs(&fs_type, &securityfs_mount,
&securityfs_mount_count);
if (error)
return ERR_PTR(error);
}

- if (!parent)
- parent = ns->securityfs_mount->mnt_root;
+ if (!parent) {
+ if (ns == &init_user_ns)
+ parent = securityfs_mount->mnt_root;
+ else
+ parent = ns->securityfs_root;
+ }

dir = d_inode(parent);

@@ -227,7 +214,7 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
out:
inode_unlock(dir);
if (ns == &init_user_ns)
- simple_release_fs(&ns->securityfs_mount,
+ simple_release_fs(&securityfs_mount,
&securityfs_mount_count);
return dentry;
}
@@ -371,7 +358,7 @@ void securityfs_remove(struct dentry *dentry)
}
inode_unlock(dir);
if (ns == &init_user_ns)
- simple_release_fs(&ns->securityfs_mount,
+ simple_release_fs(&securityfs_mount,
&securityfs_mount_count);
}
EXPORT_SYMBOL_GPL(securityfs_remove);
--
2.33.0

2021-12-06 22:13:34

by Stefan Berger

Subject: Re: [PATCH v3 00/16] ima: Namespace IMA with audit support in IMA-ns

On 12/6/21 16:14, James Bottomley wrote:
> On Mon, 2021-12-06 at 12:25 -0500, Stefan Berger wrote:
> [...]
>> v3:
>> - Further modifications to virtualized SecurityFS following James's
>> posted patch
>> - Dropping of early teardown for user_namespaces since not needed
>> anymore
> This is my incremental to this series that moves the namespaced
> securityfs away from using a vfsmount and on to a root dentry instead,
> meaning we can call the blocking notifier from fill_super as Christian
> requested (and thus can remove the securityfs_notifier_sent indicator
> since it's only called once).

Thanks. I have this now in a branch for v4.


   Stefan

2021-12-07 14:59:17

by Christian Brauner

Subject: Re: [PATCH v3 00/16] ima: Namespace IMA with audit support in IMA-ns

On Mon, Dec 06, 2021 at 04:14:15PM -0500, James Bottomley wrote:
> On Mon, 2021-12-06 at 12:25 -0500, Stefan Berger wrote:
> [...]
> > v3:
> > - Further modifications to virtualized SecurityFS following James's
> > posted patch
> > - Dropping of early teardown for user_namespaces since not needed
> > anymore
>
> This is my incremental to this series that moves the namespaced
> securityfs away from using a vfsmount and on to a root dentry instead,
> meaning we can call the blocking notifier from fill_super as Christian
> requested (and thus can remove the securityfs_notifier_sent indicator
> since it's only called once).

Somehow b4 retrieves your patch out-of-band which makes it weird to
reply to so I'm copy-pasting it here and reply inline:

On Mon, Dec 06, 2021 at 08:27:00PM +0000, James Bottomley wrote:
> ---
> include/linux/user_namespace.h | 3 +-
> security/inode.c | 55 +++++++++++++---------------------
> 2 files changed, 22 insertions(+), 36 deletions(-)
>
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index 6b8bd060d8c4..03a0879376a0 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -104,8 +104,7 @@ struct user_namespace {
> struct ima_namespace *ima_ns;
> #endif
> #ifdef CONFIG_SECURITYFS
> - struct vfsmount *securityfs_mount;
> - bool securityfs_notifier_sent;
> + struct dentry *securityfs_root;
> #endif
> } __randomize_layout;
>
> diff --git a/security/inode.c b/security/inode.c
> index 45211845fc31..f8b6cb3dfb87 100644
> --- a/security/inode.c
> +++ b/security/inode.c
> @@ -24,6 +24,7 @@
> #include <linux/magic.h>
> #include <linux/user_namespace.h>
>
> +static struct vfsmount *securityfs_mount;
> static int securityfs_mount_count;
>
> static BLOCKING_NOTIFIER_HEAD(securityfs_ns_notifier);
> @@ -40,43 +41,24 @@ static const struct super_operations securityfs_super_operations = {
> .free_inode = securityfs_free_inode,
> };
>
> -static struct file_system_type fs_type;
> -
> -static void securityfs_free_context(struct fs_context *fc)
> -{
> - struct user_namespace *ns = fc->user_ns;
> -
> - if (ns == &init_user_ns ||
> - ns->securityfs_notifier_sent)
> - return;
> -
> - ns->securityfs_notifier_sent = true;
> -
> - ns->securityfs_mount = vfs_kern_mount(&fs_type, SB_KERNMOUNT,
> - fs_type.name, NULL);
> - if (IS_ERR(ns->securityfs_mount)) {
> - printk(KERN_ERR "kern mount on securityfs ERROR: %ld\n",
> - PTR_ERR(ns->securityfs_mount));
> - ns->securityfs_mount = NULL;
> - return;
> - }
> -
> - blocking_notifier_call_chain(&securityfs_ns_notifier,
> - SECURITYFS_NS_ADD, fc->user_ns);
> - mntput(ns->securityfs_mount);
> -}
> -
> static int securityfs_fill_super(struct super_block *sb, struct fs_context *fc)
> {
> static const struct tree_descr files[] = {{""}};
> int error;
> + struct user_namespace *ns = fc->user_ns;
>
> error = simple_fill_super(sb, SECURITYFS_MAGIC, files);
> if (error)
> return error;
>
> + ns->securityfs_root = dget(sb->s_root);
> +
> sb->s_op = &securityfs_super_operations;
>
> + if (ns != &init_user_ns)
> + blocking_notifier_call_chain(&securityfs_ns_notifier,
> + SECURITYFS_NS_ADD, ns);

I would propose not to use the notifier logic. While it might be nifty
it's over-engineered in my opinion. The dentry stashing in struct
user_namespace currently serves the purpose to make it retrievable in
ima_fs_ns_init(). That doesn't justify its existence imho.

There is one central place were all users of namespaced securityfs can
create the files that they need to and that is in
securityfs_fill_super(). (If you want to make that more obvious then give
it a subdirectory securityfs and move inode.c in there.)

We simply will expect users to add:

ima_init_securityfs()
mylsm_init_securityfs()

that are to be placed in fill_super

and

ima_kill_securityfs()
mylsm_kill_securityfs()

that get called in kill_super and the root dentry and other relevant
information should be passed explicitly into those functions. Then we
can remove the dentry stashing from struct user_namespace altogether and
the patch gets smaller too.

2021-12-07 15:17:00

by James Bottomley

Subject: Re: [PATCH v3 00/16] ima: Namespace IMA with audit support in IMA-ns

On Tue, 2021-12-07 at 15:59 +0100, Christian Brauner wrote:
> On Mon, Dec 06, 2021 at 04:14:15PM -0500, James Bottomley wrote:
> > On Mon, 2021-12-06 at 12:25 -0500, Stefan Berger wrote:
> > [...]
> > > v3:
> > > - Further modifications to virtualized SecurityFS following
> > > James's posted patch
> > > - Dropping of early teardown for user_namespaces since not
> > > needed anymore
> >
> > This is my incremental to this series that moves the namespaced
> > securityfs away from using a vfsmount and on to a root dentry
> > instead, meaning we can call the blocking notifier from fill_super
> > as Christian requested (and thus can remove the
> > securityfs_notifier_sent indicator since it's only called once).
>
> Somehow b4 retrieves your patch out-of-band which makes it weird to
> reply to so I'm copy-pasting it here and reply inline:
>
> On Mon, Dec 06, 2021 at 08:27:00PM +0000, James Bottomley wrote:
> > ---
> > include/linux/user_namespace.h | 3 +-
> > security/inode.c | 55 +++++++++++++-----------------
> > ----
> > 2 files changed, 22 insertions(+), 36 deletions(-)
> >
> > diff --git a/include/linux/user_namespace.h
> > b/include/linux/user_namespace.h
> > index 6b8bd060d8c4..03a0879376a0 100644
> > --- a/include/linux/user_namespace.h
> > +++ b/include/linux/user_namespace.h
> > @@ -104,8 +104,7 @@ struct user_namespace {
> > struct ima_namespace *ima_ns;
> > #endif
> > #ifdef CONFIG_SECURITYFS
> > - struct vfsmount *securityfs_mount;
> > - bool securityfs_notifier_sent;
> > + struct dentry *securityfs_root;
> > #endif
> > } __randomize_layout;
> >
> > diff --git a/security/inode.c b/security/inode.c
> > index 45211845fc31..f8b6cb3dfb87 100644
> > --- a/security/inode.c
> > +++ b/security/inode.c
> > @@ -24,6 +24,7 @@
> > #include <linux/magic.h>
> > #include <linux/user_namespace.h>
> >
> > +static struct vfsmount *securityfs_mount;
> > static int securityfs_mount_count;
> >
> > static BLOCKING_NOTIFIER_HEAD(securityfs_ns_notifier);
> > @@ -40,43 +41,24 @@ static const struct super_operations
> > securityfs_super_operations = {
> > .free_inode = securityfs_free_inode,
> > };
> >
> > -static struct file_system_type fs_type;
> > -
> > -static void securityfs_free_context(struct fs_context *fc)
> > -{
> > - struct user_namespace *ns = fc->user_ns;
> > -
> > - if (ns == &init_user_ns ||
> > - ns->securityfs_notifier_sent)
> > - return;
> > -
> > - ns->securityfs_notifier_sent = true;
> > -
> > - ns->securityfs_mount = vfs_kern_mount(&fs_type, SB_KERNMOUNT,
> > - fs_type.name, NULL);
> > - if (IS_ERR(ns->securityfs_mount)) {
> > - printk(KERN_ERR "kern mount on securityfs ERROR:
> > %ld\n",
> > - PTR_ERR(ns->securityfs_mount));
> > - ns->securityfs_mount = NULL;
> > - return;
> > - }
> > -
> > - blocking_notifier_call_chain(&securityfs_ns_notifier,
> > - SECURITYFS_NS_ADD, fc->user_ns);
> > - mntput(ns->securityfs_mount);
> > -}
> > -
> > static int securityfs_fill_super(struct super_block *sb, struct
> > fs_context *fc)
> > {
> > static const struct tree_descr files[] = {{""}};
> > int error;
> > + struct user_namespace *ns = fc->user_ns;
> >
> > error = simple_fill_super(sb, SECURITYFS_MAGIC, files);
> > if (error)
> > return error;
> >
> > + ns->securityfs_root = dget(sb->s_root);
> > +
> > sb->s_op = &securityfs_super_operations;
> >
> > + if (ns != &init_user_ns)
> > + blocking_notifier_call_chain(&securityfs_ns_notifier,
> > + SECURITYFS_NS_ADD, ns);
>
> I would propose not to use the notifier logic. While it might be
> nifty it's over-engineered in my opinion.

The reason for a notifier is that this current patch set only
namespaces ima, but we also have integrity and evm to do. Plus, as
Casey said, we might get apparmour and selinux. Since each of those
will also want to add entries in fill_super, the notifier mechanism
seemed fairly tailor made for this. The alternative is to have a load
of

#if CONFIG_securityfeature
callback()
#endif

Inside securityfs_fill_super which is a bit inelegant.

> The dentry stashing in struct user_namespace currently serves the
> purpose to make it retrievable in ima_fs_ns_init(). That doesn't
> justify its existence imho.

I can thread the root as part of the callback. I think I can still use
the standard securityfs calls because the only reason for the dentry in
the namespace is so the callee can pass NULL and have the dentry
created at the top level. We can insist in the namespaced use case
that the callee always pass in the dentry, even for the top level.

> There is one central place were all users of namespaced securityfs
> can create the files that they need to and that is in
> securityfs_fill_super(). (If you want to make that more obvious then
> give it a subdirectory securityfs and move inode.c in there.)

Right, that's what the patch does.

> We simply will expect users to add:
>
> ima_init_securityfs()
> mylsm_init_securityfs()

Yes, plus all the #ifdefs because securityfs can exist independently of
each of the features. We can hide the ifdefs in the header files and
make the functions static do nothing if not defined, but the ifdeffery
has to live somewhere.

> that are to be placed in fill_super
>
> and
>
> ima_kill_securityfs()
> mylsm_kill_securityfs()
>
> that get called in kill_super and the root dentry and other relevant
> information should be passed explicitly into those functions. Then we
> can remove the dentry stashing from struct user_namespace altogether
> and the patch gets smaller too.

Removing dentry stashing can be done independently of removing the
notifier because the dentry can thread through the notifier (or the
callback, of course).

Let me have a look at doing the recoding.

James

2021-12-07 15:17:51

by Christian Brauner

Subject: Re: [PATCH v3 00/16] ima: Namespace IMA with audit support in IMA-ns

On Mon, Dec 06, 2021 at 12:25:44PM -0500, Stefan Berger wrote:
> The goal of this series of patches is to start with the namespacing of
> IMA and support auditing within an IMA namespace (IMA-ns) as the first
> step.
>
> In this series the IMA namespace is piggy backing on the user namespace
> and therefore an IMA namespace gets created when a user namespace is
> created. The advantage of this is that the user namespace can provide
> the keys infrastructure that IMA appraisal support will need later on.
>
> We chose the goal of supporting auditing within an IMA namespace since it
> requires the least changes to IMA. Following this series, auditing within
> an IMA namespace can be activated by a user running the following lines
> that rely on a statically linked busybox to be installed on the host for
> execution within the minimal container environment:
>
> mkdir -p rootfs/{bin,mnt,proc}
> cp /sbin/busybox rootfs/bin
> PATH=/bin unshare --user --map-root-user --mount-proc --pid --fork \
> --root rootfs busybox sh -c \
> "busybox mount -t securityfs /mnt /mnt; \
> busybox echo 'audit func=BPRM_CHECK mask=MAY_EXEC' > /mnt/ima/policy; \
> busybox cat /mnt/ima/policy"
>
> Following the audit log on the host the last line cat'ing the IMA policy
> inside the namespace would have been audited. Unfortunately the auditing
> line is not distinguishable from one stemming from actions on the host.
> The hope here is that Richard Brigg's container id support for auditing
> would help resolve the problem.
>
> The following lines added to a suitable IMA policy on the host would
> cause the execution of the commands inside the container (by uid 1000)
> to be measured and audited as well on the host, thus leading to two
> auditing messages for the 'busybox cat' above and log entries in IMA's
> system log.
>
> echo -e "measure func=BPRM_CHECK mask=MAY_EXEC uid=1000\n" \
> "audit func=BPRM_CHECK mask=MAY_EXEC uid=1000\n" \
> > /sys/kernel/security/ima/policy
>
> The goal of supporting measurement and auditing by the host, of actions
> occurring within IMA namespaces, is that users, particularly root,
> should not be able to evade the host's IMA policy just by spawning
> new IMA namespaces, running programs there, and discarding the namespaces
> again. This is achieved through 'hierarchical processing' of file
> accesses that are evaluated against the policy of the namespace where
> the action occurred and against all namespaces' and their policies leading
> back to the root IMA namespace (init_ima_ns).
>
> The patch series adds support for a virtualized SecurityFS with a few
> new API calls that are used by IMA namespacing. Only the data relevant
> to the IMA namespace are shown. The files and directories of other
> security subsystems (TPM, evm, Tomoyo, safesetid) are not showing
> up when secruityfs is mounted inside a user namespace.
>
> Much of the code leading up to the virtualization of SecurityFS deals
> with moving IMA's variables from various files into the IMA namespace
> structure called 'ima_namespace'. When it comes to determining the
> current IMA namespace I took the approach to get the current IMA
> namespace (get_current_ns()) on the top level and pass the pointer all
> the way down to those functions that now need access to the ima_namespace
> to get to their variables. This later on comes in handy once hierarchical
> processing is implemented in this series where we walk the list of
> namespaces backwards and again need to pass the pointer into functions.
>
> This patch also introduces usage of CAP_MAC_ADMIN to allow access to the
> IMA policy via reduced capabilities. We would again later on use this
> capability to allow users to set file extended attributes for IMA appraisal
> support.
>
> The basis for this series of patches is Linux v5.15.
> My tree with these patches is here:
> https://github.com/stefanberger/linux-ima-namespaces/tree/v5.15%2Bimans.v3.public

I have one small procedural favor to ask. :)

I couldn't apply your patch series directly. It if isn't too
inconvenient for you could you pass --base with a proper upstream tag,
e.g. --base=v5.15.

The branch you posted here doesn't exist afaict and I had to peruse your
github repo and figured the correct branch might be v5.15+imans.v3.posted.

In any case, --base with a proper upstream tag would make this all a bit
easier or - if it really is necessary to pull from your tree it would be
nice if you could post it in a form directly consumable by git and note
url-escaped. So something like

git clone https://github.com/stefanberger/linux-ima-namespaces v5.15+imans.v3.posted

would already help.

Christian

2021-12-07 15:41:27

by James Bottomley

Subject: Re: [PATCH v3 00/16] ima: Namespace IMA with audit support in IMA-ns

On Tue, 2021-12-07 at 10:16 -0500, James Bottomley wrote:
> On Tue, 2021-12-07 at 15:59 +0100, Christian Brauner wrote:
> > On Mon, Dec 06, 2021 at 04:14:15PM -0500, James Bottomley wrote:
[...]
> > > static int securityfs_fill_super(struct super_block *sb, struct
> > > fs_context *fc)
> > > {
> > > static const struct tree_descr files[] = {{""}};
> > > int error;
> > > + struct user_namespace *ns = fc->user_ns;
> > >
> > > error = simple_fill_super(sb, SECURITYFS_MAGIC, files);
> > > if (error)
> > > return error;
> > >
> > > + ns->securityfs_root = dget(sb->s_root);
> > > +
> > > sb->s_op = &securityfs_super_operations;
> > >
> > > + if (ns != &init_user_ns)
> > > + blocking_notifier_call_chain(&securityfs_ns_notifier,
> > > + SECURITYFS_NS_ADD, ns);
> >
> > I would propose not to use the notifier logic. While it might be
> > nifty it's over-engineered in my opinion.
>
> The reason for a notifier is that this current patch set only
> namespaces ima, but we also have integrity and evm to do. Plus, as
> Casey said, we might get apparmour and selinux. Since each of those
> will also want to add entries in fill_super, the notifier mechanism
> seemed fairly tailor made for this. The alternative is to have a
> load of
>
> #if CONFIG_securityfeature
> callback()
> #endif
>
> Inside securityfs_fill_super which is a bit inelegant.
>
> > The dentry stashing in struct user_namespace currently serves the
> > purpose to make it retrievable in ima_fs_ns_init(). That doesn't
> > justify its existence imho.
>
> I can thread the root as part of the callback. I think I can still
> use the standard securityfs calls because the only reason for the
> dentry in the namespace is so the callee can pass NULL and have the
> dentry created at the top level. We can insist in the namespaced use
> case that the callee always pass in the dentry, even for the top
> level.
>
> > There is one central place were all users of namespaced securityfs
> > can create the files that they need to and that is in
> > securityfs_fill_super(). (If you want to make that more obvious
> > then give it a subdirectory securityfs and move inode.c in there.)
>
> Right, that's what the patch does.
>
> > We simply will expect users to add:
> >
> > ima_init_securityfs()
> > mylsm_init_securityfs()
>
> Yes, plus all the #ifdefs because securityfs can exist independently
> of each of the features. We can hide the ifdefs in the header files
> and make the functions static do nothing if not defined, but the
> ifdeffery has to live somewhere.

Actually, I've got a much better reason: securityfs is a bool; all the
other LSMs and IMA are tristates. We can't call module init functions
from core code, it has to be done by something like a notifier.

James

2021-12-07 15:48:26

by Casey Schaufler

Subject: Re: [PATCH v3 00/16] ima: Namespace IMA with audit support in IMA-ns

On 12/7/2021 7:40 AM, James Bottomley wrote:
> On Tue, 2021-12-07 at 10:16 -0500, James Bottomley wrote:
>> On Tue, 2021-12-07 at 15:59 +0100, Christian Brauner wrote:
>>> On Mon, Dec 06, 2021 at 04:14:15PM -0500, James Bottomley wrote:
> [...]
>>>> static int securityfs_fill_super(struct super_block *sb, struct
>>>> fs_context *fc)
>>>> {
>>>> static const struct tree_descr files[] = {{""}};
>>>> int error;
>>>> + struct user_namespace *ns = fc->user_ns;
>>>>
>>>> error = simple_fill_super(sb, SECURITYFS_MAGIC, files);
>>>> if (error)
>>>> return error;
>>>>
>>>> + ns->securityfs_root = dget(sb->s_root);
>>>> +
>>>> sb->s_op = &securityfs_super_operations;
>>>>
>>>> + if (ns != &init_user_ns)
>>>> + blocking_notifier_call_chain(&securityfs_ns_notifier,
>>>> + SECURITYFS_NS_ADD, ns);
>>> I would propose not to use the notifier logic. While it might be
>>> nifty it's over-engineered in my opinion.
>> The reason for a notifier is that this current patch set only
>> namespaces ima, but we also have integrity and evm to do. Plus, as
>> Casey said, we might get apparmour and selinux. Since each of those
>> will also want to add entries in fill_super, the notifier mechanism
>> seemed fairly tailor made for this. The alternative is to have a
>> load of
>>
>> #if CONFIG_securityfeature
>> callback()
>> #endif
>>
>> Inside securityfs_fill_super which is a bit inelegant.
>>
>>> The dentry stashing in struct user_namespace currently serves the
>>> purpose to make it retrievable in ima_fs_ns_init(). That doesn't
>>> justify its existence imho.
>> I can thread the root as part of the callback. I think I can still
>> use the standard securityfs calls because the only reason for the
>> dentry in the namespace is so the callee can pass NULL and have the
>> dentry created at the top level. We can insist in the namespaced use
>> case that the callee always pass in the dentry, even for the top
>> level.
>>
>>> There is one central place were all users of namespaced securityfs
>>> can create the files that they need to and that is in
>>> securityfs_fill_super(). (If you want to make that more obvious
>>> then give it a subdirectory securityfs and move inode.c in there.)
>> Right, that's what the patch does.
>>
>>> We simply will expect users to add:
>>>
>>> ima_init_securityfs()
>>> mylsm_init_securityfs()
>> Yes, plus all the #ifdefs because securityfs can exist independently
>> of each of the features. We can hide the ifdefs in the header files
>> and make the functions static do nothing if not defined, but the
>> ifdeffery has to live somewhere.
> Actually, I've got a much better reason: securityfs is a bool; all the
> other LSMs and IMA are tristates. We can't call module init functions
> from core code, it has to be done by something like a notifier.

Err, no. LSMs are not available as loadable modules.

2021-12-07 15:57:59

by Stefan Berger

Subject: Re: [PATCH v3 00/16] ima: Namespace IMA with audit support in IMA-ns

On 12/7/21 10:17, Christian Brauner wrote:

> On Mon, Dec 06, 2021 at 12:25:44PM -0500, Stefan Berger wrote:
>> The goal of this series of patches is to start with the namespacing of
>> IMA and support auditing within an IMA namespace (IMA-ns) as the first
>> step.
>>
>> In this series the IMA namespace is piggy backing on the user namespace
>> and therefore an IMA namespace gets created when a user namespace is
>> created. The advantage of this is that the user namespace can provide
>> the keys infrastructure that IMA appraisal support will need later on.
>>
>> We chose the goal of supporting auditing within an IMA namespace since it
>> requires the least changes to IMA. Following this series, auditing within
>> an IMA namespace can be activated by a user running the following lines
>> that rely on a statically linked busybox to be installed on the host for
>> execution within the minimal container environment:
>>
>> mkdir -p rootfs/{bin,mnt,proc}
>> cp /sbin/busybox rootfs/bin
>> PATH=/bin unshare --user --map-root-user --mount-proc --pid --fork \
>> --root rootfs busybox sh -c \
>> "busybox mount -t securityfs /mnt /mnt; \
>> busybox echo 'audit func=BPRM_CHECK mask=MAY_EXEC' > /mnt/ima/policy; \
>> busybox cat /mnt/ima/policy"
>>
>> Following the audit log on the host the last line cat'ing the IMA policy
>> inside the namespace would have been audited. Unfortunately the auditing
>> line is not distinguishable from one stemming from actions on the host.
>> The hope here is that Richard Brigg's container id support for auditing
>> would help resolve the problem.
>>
>> The following lines added to a suitable IMA policy on the host would
>> cause the execution of the commands inside the container (by uid 1000)
>> to be measured and audited as well on the host, thus leading to two
>> auditing messages for the 'busybox cat' above and log entries in IMA's
>> system log.
>>
>> echo -e "measure func=BPRM_CHECK mask=MAY_EXEC uid=1000\n" \
>> "audit func=BPRM_CHECK mask=MAY_EXEC uid=1000\n" \
>> > /sys/kernel/security/ima/policy
>>
>> The goal of supporting measurement and auditing by the host, of actions
>> occurring within IMA namespaces, is that users, particularly root,
>> should not be able to evade the host's IMA policy just by spawning
>> new IMA namespaces, running programs there, and discarding the namespaces
>> again. This is achieved through 'hierarchical processing' of file
>> accesses that are evaluated against the policy of the namespace where
>> the action occurred and against all namespaces' and their policies leading
>> back to the root IMA namespace (init_ima_ns).
>>
>> The patch series adds support for a virtualized SecurityFS with a few
>> new API calls that are used by IMA namespacing. Only the data relevant
>> to the IMA namespace are shown. The files and directories of other
>> security subsystems (TPM, evm, Tomoyo, safesetid) are not showing
>> up when secruityfs is mounted inside a user namespace.
>>
>> Much of the code leading up to the virtualization of SecurityFS deals
>> with moving IMA's variables from various files into the IMA namespace
>> structure called 'ima_namespace'. When it comes to determining the
>> current IMA namespace I took the approach to get the current IMA
>> namespace (get_current_ns()) on the top level and pass the pointer all
>> the way down to those functions that now need access to the ima_namespace
>> to get to their variables. This later on comes in handy once hierarchical
>> processing is implemented in this series where we walk the list of
>> namespaces backwards and again need to pass the pointer into functions.
>>
>> This patch also introduces usage of CAP_MAC_ADMIN to allow access to the
>> IMA policy via reduced capabilities. We would again later on use this
>> capability to allow users to set file extended attributes for IMA appraisal
>> support.
>>
>> The basis for this series of patches is Linux v5.15.
>> My tree with these patches is here:
>> https://github.com/stefanberger/linux-ima-namespaces/tree/v5.15%2Bimans.v3.public
> I have one small procedural favor to ask. :)
>
> I couldn't apply your patch series directly. It if isn't too
> inconvenient for you could you pass --base with a proper upstream tag,
> e.g. --base=v5.15.
>
> The branch you posted here doesn't exist afaict and I had to peruse your
> github repo and figured the correct branch might be v5.15+imans.v3.posted.
>
> In any case, --base with a proper upstream tag would make this all a bit
> easier or - if it really is necessary to pull from your tree it would be
> nice if you could post it in a form directly consumable by git and note
> url-escaped. So something like
>
> git clone https://github.com/stefanberger/linux-ima-namespaces v5.15+imans.v3.posted
>
> would already help.

Sure, will do.

  Stefan


>
> Christian

2021-12-07 17:07:14

by James Bottomley

Subject: Re: [PATCH v3 00/16] ima: Namespace IMA with audit support in IMA-ns

On Tue, 2021-12-07 at 07:48 -0800, Casey Schaufler wrote:
> On 12/7/2021 7:40 AM, James Bottomley wrote:
> > On Tue, 2021-12-07 at 10:16 -0500, James Bottomley wrote:
> > > On Tue, 2021-12-07 at 15:59 +0100, Christian Brauner wrote:
> > > > On Mon, Dec 06, 2021 at 04:14:15PM -0500, James Bottomley
> > > > wrote:
> > [...]
> > > > > static int securityfs_fill_super(struct super_block *sb,
> > > > > struct
> > > > > fs_context *fc)
> > > > > {
> > > > > static const struct tree_descr files[] = {{""}};
> > > > > int error;
> > > > > + struct user_namespace *ns = fc->user_ns;
> > > > >
> > > > > error = simple_fill_super(sb, SECURITYFS_MAGIC, files);
> > > > > if (error)
> > > > > return error;
> > > > >
> > > > > + ns->securityfs_root = dget(sb->s_root);
> > > > > +
> > > > > sb->s_op = &securityfs_super_operations;
> > > > >
> > > > > + if (ns != &init_user_ns)
> > > > > + blocking_notifier_call_chain(&securityfs_ns_not
> > > > > ifier,
> > > > > + SECURITYFS_NS_ADD,
> > > > > ns);
> > > >
> > > > I would propose not to use the notifier logic. While it might
> > > > be nifty it's over-engineered in my opinion.
> > >
> > > The reason for a notifier is that this current patch set only
> > > namespaces ima, but we also have integrity and evm to do. Plus,
> > > as Casey said, we might get apparmour and selinux. Since each of
> > > those will also want to add entries in fill_super, the notifier
> > > mechanism seemed fairly tailor made for this. The alternative is
> > > to have a load of
> > >
> > > #if CONFIG_securityfeature
> > > callback()
> > > #endif
> > >
> > > Inside securityfs_fill_super which is a bit inelegant.
> > >
> > > > The dentry stashing in struct user_namespace currently serves
> > > > the purpose to make it retrievable in ima_fs_ns_init(). That
> > > > doesn't justify its existence imho.
> > >
> > > I can thread the root as part of the callback. I think I can
> > > still use the standard securityfs calls because the only reason
> > > for the dentry in the namespace is so the callee can pass NULL
> > > and have the dentry created at the top level. We can insist in
> > > the namespaced use case that the callee always pass in the
> > > dentry, even for the top level.
> > >
> > > > There is one central place were all users of namespaced
> > > > securityfs can create the files that they need to and that is
> > > > in securityfs_fill_super(). (If you want to make that more
> > > > obvious then give it a subdirectory securityfs and move inode.c
> > > > in there.)
> > > >
> > > Right, that's what the patch does.
> > >
> > > > We simply will expect users to add:
> > > >
> > > > ima_init_securityfs()
> > > > mylsm_init_securityfs()
> > >
> > > Yes, plus all the #ifdefs because securityfs can exist
> > > independently of each of the features. We can hide the ifdefs in
> > > the header files and make the functions static do nothing if not
> > > defined, but the ifdeffery has to live somewhere.
> >
> > Actually, I've got a much better reason: securityfs is a bool; all
> > the other LSMs and IMA are tristates. We can't call module init
> > functions from core code, it has to be done by something like a
> > notifier.
>
> Err, no. LSMs are not available as loadable modules.

Well securityfs has EXPORT_MODULE_GPL() across all its dentry creation
functions ... that does mean it expects to be called by a module.

However, it does appear to be it's only TPM that may use it as a module
... this is still going to cause a problem eventually because now we'll
have to require some of the TPM code be built in once we want to attach
vTPMs to containers.

James

2021-12-07 17:13:29

by James Bottomley

Subject: Re: [PATCH v3 00/16] ima: Namespace IMA with audit support in IMA-ns

On Tue, 2021-12-07 at 15:59 +0100, Christian Brauner wrote:
[...]
> I would propose not to use the notifier logic. While it might be
> nifty it's over-engineered in my opinion. The dentry stashing in
> struct user_namespace currently serves the purpose to make it
> retrievable in ima_fs_ns_init(). That doesn't justify its existence
> imho.

This is the incremental to Stefan's set with the notifier removed and
the root dentry threaded.

James

---

From d9322270157531f4772fe862fa1655993a0c387d Mon Sep 17 00:00:00 2001
From: James Bottomley <[email protected]>
Date: Mon, 6 Dec 2021 20:27:00 +0000
Subject: [PATCH] Incremental for root dentry

---
include/linux/ima.h | 2 +
include/linux/security.h | 8 ----
include/linux/user_namespace.h | 4 --
security/inode.c | 71 ++++++++++-----------------------
security/integrity/ima/ima_fs.c | 40 ++++---------------
5 files changed, 29 insertions(+), 96 deletions(-)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index cab5fc6caeb3..a6e93bb5ce8a 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -40,6 +40,8 @@ extern int ima_measure_critical_data(const char *event_label,
const char *event_name,
const void *buf, size_t buf_len,
bool hash, u8 *digest, size_t digest_len);
+extern int ima_fs_ns_init(struct user_namespace *ns, struct dentry *root);
+extern void ima_fs_ns_free_dentries(struct user_namespace *ns);

#ifdef CONFIG_IMA_APPRAISE_BOOTPARAM
extern void ima_appraise_parse_cmdline(void);
diff --git a/include/linux/security.h b/include/linux/security.h
index a34109c5e3ed..bbf44a466832 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1929,14 +1929,6 @@ struct dentry *securityfs_create_symlink(const char *name,
const struct inode_operations *iops);
extern void securityfs_remove(struct dentry *dentry);

-enum {
- SECURITYFS_NS_ADD,
- SECURITYFS_NS_REMOVE,
-};
-
-extern int securityfs_register_ns_notifier(struct notifier_block *nb);
-extern int securityfs_unregister_ns_notifier(struct notifier_block *nb);
-
#else /* CONFIG_SECURITYFS */

static inline struct dentry *securityfs_create_dir(const char *name,
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index 6b8bd060d8c4..5249db04d62b 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -103,10 +103,6 @@ struct user_namespace {
#ifdef CONFIG_IMA
struct ima_namespace *ima_ns;
#endif
-#ifdef CONFIG_SECURITYFS
- struct vfsmount *securityfs_mount;
- bool securityfs_notifier_sent;
-#endif
} __randomize_layout;

struct ucounts {
diff --git a/security/inode.c b/security/inode.c
index 45211845fc31..0b173792fbd3 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -16,18 +16,17 @@
#include <linux/fs_context.h>
#include <linux/mount.h>
#include <linux/pagemap.h>
+#include <linux/ima.h>
#include <linux/init.h>
#include <linux/namei.h>
-#include <linux/notifier.h>
#include <linux/security.h>
#include <linux/lsm_hooks.h>
#include <linux/magic.h>
#include <linux/user_namespace.h>

+static struct vfsmount *securityfs_mount;
static int securityfs_mount_count;

-static BLOCKING_NOTIFIER_HEAD(securityfs_ns_notifier);
-
static void securityfs_free_inode(struct inode *inode)
{
if (S_ISLNK(inode->i_mode))
@@ -40,36 +39,11 @@ static const struct super_operations securityfs_super_operations = {
.free_inode = securityfs_free_inode,
};

-static struct file_system_type fs_type;
-
-static void securityfs_free_context(struct fs_context *fc)
-{
- struct user_namespace *ns = fc->user_ns;
-
- if (ns == &init_user_ns ||
- ns->securityfs_notifier_sent)
- return;
-
- ns->securityfs_notifier_sent = true;
-
- ns->securityfs_mount = vfs_kern_mount(&fs_type, SB_KERNMOUNT,
- fs_type.name, NULL);
- if (IS_ERR(ns->securityfs_mount)) {
- printk(KERN_ERR "kern mount on securityfs ERROR: %ld\n",
- PTR_ERR(ns->securityfs_mount));
- ns->securityfs_mount = NULL;
- return;
- }
-
- blocking_notifier_call_chain(&securityfs_ns_notifier,
- SECURITYFS_NS_ADD, fc->user_ns);
- mntput(ns->securityfs_mount);
-}
-
static int securityfs_fill_super(struct super_block *sb, struct fs_context *fc)
{
static const struct tree_descr files[] = {{""}};
int error;
+ struct user_namespace *ns = fc->user_ns;

error = simple_fill_super(sb, SECURITYFS_MAGIC, files);
if (error)
@@ -77,6 +51,10 @@ static int securityfs_fill_super(struct super_block *sb, struct fs_context *fc)

sb->s_op = &securityfs_super_operations;

+ if (ns != &init_user_ns) {
+ ima_fs_ns_init(ns, sb->s_root);
+ }
+
return 0;
}

@@ -87,7 +65,6 @@ static int securityfs_get_tree(struct fs_context *fc)

static const struct fs_context_operations securityfs_context_ops = {
.get_tree = securityfs_get_tree,
- .free = securityfs_free_context,
};

static int securityfs_init_fs_context(struct fs_context *fc)
@@ -100,12 +77,10 @@ static void securityfs_kill_super(struct super_block *sb)
{
struct user_namespace *ns = sb->s_fs_info;

- if (ns != &init_user_ns)
- blocking_notifier_call_chain(&securityfs_ns_notifier,
- SECURITYFS_NS_REMOVE,
- sb->s_fs_info);
- ns->securityfs_notifier_sent = false;
- ns->securityfs_mount = NULL;
+ if (ns != &init_user_ns) {
+ ima_fs_ns_free_dentries(ns);
+ }
+
kill_litter_super(sb);
}

@@ -117,16 +92,6 @@ static struct file_system_type fs_type = {
.fs_flags = FS_USERNS_MOUNT,
};

-int securityfs_register_ns_notifier(struct notifier_block *nb)
-{
- return blocking_notifier_chain_register(&securityfs_ns_notifier, nb);
-}
-
-int securityfs_unregister_ns_notifier(struct notifier_block *nb)
-{
- return blocking_notifier_chain_unregister(&securityfs_ns_notifier, nb);
-}
-
/**
* securityfs_create_dentry - create a dentry in the securityfs filesystem
*
@@ -174,14 +139,18 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
pr_debug("securityfs: creating file '%s'\n",name);

if (ns == &init_user_ns) {
- error = simple_pin_fs(&fs_type, &ns->securityfs_mount,
+ error = simple_pin_fs(&fs_type, &securityfs_mount,
&securityfs_mount_count);
if (error)
return ERR_PTR(error);
}

- if (!parent)
- parent = ns->securityfs_mount->mnt_root;
+ if (!parent) {
+ if (ns == &init_user_ns)
+ parent = securityfs_mount->mnt_root;
+ else
+ return ERR_PTR(-EINVAL);
+ }

dir = d_inode(parent);

@@ -227,7 +196,7 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
out:
inode_unlock(dir);
if (ns == &init_user_ns)
- simple_release_fs(&ns->securityfs_mount,
+ simple_release_fs(&securityfs_mount,
&securityfs_mount_count);
return dentry;
}
@@ -371,7 +340,7 @@ void securityfs_remove(struct dentry *dentry)
}
inode_unlock(dir);
if (ns == &init_user_ns)
- simple_release_fs(&ns->securityfs_mount,
+ simple_release_fs(&securityfs_mount,
&securityfs_mount_count);
}
EXPORT_SYMBOL_GPL(securityfs_remove);
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index c17a6b7eeb95..fb29cb7b0384 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -446,9 +446,10 @@ static const struct file_operations ima_measure_policy_ops = {
.llseek = generic_file_llseek,
};

-static void ima_fs_ns_free_dentries(struct ima_namespace *ns)
+void ima_fs_ns_free_dentries(struct user_namespace *user_ns)
{
int i;
+ struct ima_namespace *ns = user_ns->ima_ns;

for (i = IMAFS_DENTRY_LAST - 1; i >= 0; i--)
securityfs_remove(ns->dentry[i]);
@@ -456,7 +457,7 @@ static void ima_fs_ns_free_dentries(struct ima_namespace *ns)
memset(ns->dentry, 0, sizeof(ns->dentry));
}

-static int ima_fs_ns_init(struct user_namespace *user_ns)
+int ima_fs_ns_init(struct user_namespace *user_ns, struct dentry *root)
{
struct ima_namespace *ns = user_ns->ima_ns;
struct dentry *ima_dir;
@@ -468,7 +469,7 @@ static int ima_fs_ns_init(struct user_namespace *user_ns)
/* FIXME: update when evm and integrity are namespaced */
if (user_ns != &init_user_ns)
ns->dentry[IMAFS_DENTRY_INTEGRITY_DIR] =
- securityfs_create_dir("integrity", NULL);
+ securityfs_create_dir("integrity", root);
else
ns->dentry[IMAFS_DENTRY_INTEGRITY_DIR] = integrity_dir;

@@ -480,7 +481,7 @@ static int ima_fs_ns_init(struct user_namespace *user_ns)
ima_dir = ns->dentry[IMAFS_DENTRY_DIR];

ns->dentry[IMAFS_DENTRY_SYMLINK] =
- securityfs_create_symlink("ima", NULL, "integrity/ima", NULL);
+ securityfs_create_symlink("ima", root, "integrity/ima", NULL);
if (IS_ERR(ns->dentry[IMAFS_DENTRY_SYMLINK]))
goto out;

@@ -520,38 +521,11 @@ static int ima_fs_ns_init(struct user_namespace *user_ns)

return 0;
out:
- ima_fs_ns_free_dentries(ns);
+ ima_fs_ns_free_dentries(user_ns);
return -1;
}

-static int ima_ns_notify(struct notifier_block *this, unsigned long msg,
- void *data)
-{
- int rc = 0;
- struct user_namespace *user_ns = data;
-
- switch (msg) {
- case SECURITYFS_NS_ADD:
- rc = ima_fs_ns_init(user_ns);
- break;
- case SECURITYFS_NS_REMOVE:
- ima_fs_ns_free_dentries(user_ns->ima_ns);
- break;
- }
- return rc;
-}
-
-static struct notifier_block ima_ns_notifier = {
- .notifier_call = ima_ns_notify,
-};
-
int ima_fs_init()
{
- int rc;
-
- rc = securityfs_register_ns_notifier(&ima_ns_notifier);
- if (rc)
- return rc;
-
- return ima_fs_ns_init(&init_user_ns);
+ return ima_fs_ns_init(&init_user_ns, NULL);
}
--
2.33.0