On 2/17/22 15:30, Mimi Zohar wrote:
On Tue, 2022-02-01 at 15:37 -0500, Stefan Berger wrote:
The builtin IMA policy rules are broad and may be constrained by
loading a custom policy, which could be defined in terms of LSM labels.
When an LSM policy is loaded, existing LSM labels might be affected or
even removed. In either case, IMA policy rules based on LSM
labels, need to reflect these changes. If an LSM label is removed,
instead of deleting the IMA policy rule based on the LSM label, the IMA
policy rule is made inactive.
Ok, so I take this paragraph for the patch description.
Move the ima_lsm_policy_notifier into the ima_namespace. Each IMA
namespace can now register its own LSM policy change notifier callback.
The policy change notifier for the init_ima_ns still remains in init_ima()
and therefore handle the registration of the callback for all other
namespaces in init_ima_namespace().
Suppress the kernel warning 'rule for LSM <label> is undefined` for all
other IMA namespaces than the init_ima_ns.
Instead of ignoring the warnings totally, perhaps use either the
"ratelimited" or "once" function options for non init_ima_ns. It would
be nice if these functions could be namespace aware, so that each
affected IMA namespace would contain at least one warning.
The problem is that any user can now repeatedly create user namespaces
and with that IMA namespaces and cause the kernel log to fill up with
these messages and also flood the audit log -- I guess one could
describe it as an unwanted side-effect. I am afraid that for as long as
the kernel log is not namespaced it's probably best to just turn them
off for non-init_ima_ns.
Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
Reviewed-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
Thanks.
---
v10:
- Only call pr_warn('rule for LSM <label> is undefined`) for init_ima_ns
---
security/integrity/ima/ima.h | 2 ++
security/integrity/ima/ima_init_ima_ns.c | 14 ++++++++++++++
security/integrity/ima/ima_main.c | 6 +-----
security/integrity/ima/ima_policy.c | 16 ++++++++++------
4 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 94c6e3a4d666..fb6bd054d899 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -144,6 +144,8 @@ struct ima_namespace {
int valid_policy;
struct dentry *ima_policy;
+
+ struct notifier_block ima_lsm_policy_notifier;
} __randomize_layout;
extern struct ima_namespace init_ima_ns;
diff --git a/security/integrity/ima/ima_init_ima_ns.c b/security/integrity/ima/ima_init_ima_ns.c
index 425eed1c6838..1cba545ae477 100644
--- a/security/integrity/ima/ima_init_ima_ns.c
+++ b/security/integrity/ima/ima_init_ima_ns.c
@@ -10,6 +10,8 @@
static int ima_init_namespace(struct ima_namespace *ns)
{
+ int rc;
+
There's no right or wrong, but the newer IMA code uses 'ret'.
I don't mind to change it. I will take your Reviewed-by, though :-)
INIT_LIST_HEAD(&ns->ima_default_rules);
INIT_LIST_HEAD(&ns->ima_policy_rules);
INIT_LIST_HEAD(&ns->ima_temp_rules);
