On Wed, May 22, 2019 at 6:27 PM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
> > Ok. The question is then how should IMA handle missing domains/types.
> > Just dropping IMA policy rules doesn't sound safe, nor does skipping
> > rules in case the domains/types are restored.
>
> You can just do what audit_dupe_lsm_field() does. It effectively
> disables the rule upon the invalidation (which makes sense, since it can
> no longer match anything since nothing can have that domain/type) but
> retains the string value so it can later re-activate the rule if the
> domain/type becomes valid again later.
Finally got a moment to look into this. It looks to me there is
already a notifier? Could something like this work?
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d213e835c498..2203451862d4 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -154,6 +154,8 @@ unsigned long ima_get_binary_runtime_size(void);
int ima_init_template(void);
void ima_init_template_list(void);
int __init ima_init_digests(void);
+int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
+ void *lsm_data);
/*
* used to protect h_table and sha_table
diff --git a/security/integrity/ima/ima_main.c
b/security/integrity/ima/ima_main.c
index 5749ec92516f..449502f5c3dc 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -52,6 +52,10 @@ int ima_hash_algo = HASH_ALGO_SHA1;
static int hash_setup_done;
static struct workqueue_struct *ima_update_wq;
+static struct notifier_block ima_lsm_policy_notifier = {
+ .notifier_call = ima_lsm_policy_change,
+};
+
static int __init hash_setup(char *str)
{
struct ima_template_desc *template_desc = ima_template_desc_current();
@@ -691,6 +695,10 @@ static int __init init_ima(void)
error = ima_init();
}
+ error = register_lsm_notifier(&ima_lsm_policy_notifier);
+ if (error)
+ pr_warn("Couldn't register LSM notifier, error %d\n", error);
+
if (!error)
ima_update_policy_flag();
else
diff --git a/security/integrity/ima/ima_policy.c
b/security/integrity/ima/ima_policy.c
index e0cc323f948f..c3983d24279a 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -252,8 +252,8 @@ __setup("ima_appraise_tcb", default_appraise_policy_setup);
/*
* The LSM policy can be reloaded, leaving the IMA LSM based rules referring
* to the old, stale LSM policy. Update the IMA LSM based rules to reflect
- * the reloaded LSM policy. We assume the rules still exist; and BUG_ON() if
- * they don't.
+ * the reloaded LSM policy. Keep currently invalid fields around in case
+ * they become valid after a policy reload.
*/
static void ima_lsm_update_rules(void)
{
@@ -269,11 +269,23 @@ static void ima_lsm_update_rules(void)
Audit_equal,
entry->lsm[i].args_p,
&entry->lsm[i].rule);
- BUG_ON(!entry->lsm[i].rule);
+ if (result == -EINVAL)
+ pr_warn("ima: rule for LSM \'%d\' is invalid\n",
+ entry->lsm[i].type);
}
}
}
+int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
+ void *lsm_data)
+{
+ if (event != LSM_POLICY_CHANGE)
+ return NOTIFY_DONE;
+
+ ima_lsm_update_rules();
+ return NOTIFY_DONE;
+}
+
/**
* ima_match_rules - determine whether an inode matches the measure rule.
* @rule: a pointer to a rule
@@ -327,11 +339,10 @@ static bool ima_match_rules(struct
ima_rule_entry *rule, struct inode *inode,
for (i = 0; i < MAX_LSM_RULES; i++) {
int rc = 0;
u32 osid;
- int retried = 0;
if (!rule->lsm[i].rule)
continue;
-retry:
+
switch (i) {
case LSM_OBJ_USER:
case LSM_OBJ_ROLE:
@@ -352,11 +363,6 @@ static bool ima_match_rules(struct ima_rule_entry
*rule, struct inode *inode,
default:
break;
}
- if ((rc < 0) && (!retried)) {
- retried = 1;
- ima_lsm_update_rules();
- goto retry;
- }
if (!rc)
return false;
}
--
Janne
