Re: sleep in selinux_audit_rule_init

[Janne Karhunen <janne.karhunen@gmail.com>]

On Wed, May 22, 2019 at 6:27 PM Stephen Smalley <sds@tycho.nsa.gov> 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