On 04/16/2018 11:23 AM, Matthew Garrett wrote:
> This patch adds support to Apparmor for integrating with audit rule
> filtering. Right now it only handles SUBJ_ROLE, interpreting it as a
> single component of a label. This is sufficient to get Apparmor working
> with IMA's appraisal rules without any modifications on the IMA side.
> 
> Signed-off-by: Matthew Garrett <mj...@google.com>

Sorry getting to this took so long. There is an issue and I would like
some clarification on the semantic you desire.

1. The comparison could potential fail if apparmor policy namespaces
   are in use.

   With the current state of audit rules I think we need to make the
   assumption they are associated to the root policy namespace. But I'd
   like to start thinking about what to do in the face of namespacing.

2. Do you ever want to be able to setup an audit rule based off of
   more than a single profile in a stack. That is trigger based
   on A//&B and not just A or just B

3. Is testing that the audit rule profile exist as an element in the
   secid stack what we want for a subject equality test?

   We could do a true equality test, a namespace equality test (the
   part of the stack in the same namespace as the audit rule),
   a subset test (as currently done) or maybe an ns subset test
   which currently should really just be a special case of the
   subset test (it could be potential different in the future if
   policy namespaces are allowed to share profiles, but currently
   they can't).

I have included a patch that uses label_parse. It fixes 1, assuming
audit rules are associated to the root policy ns.

Allows for 2, where a rule could have A//&B

and keeps the current subset semantic. So say the secid resolves to
the label stack A//&B//&C

Then an audit rule specifying a label of

  A would match
  B would match
  C would match
  D would not
  A//&B would match as a subset
  A//&C would match as a subset
  B//&C would match as a subset
  A//&B//&C would match

  A//&D would not match, because while A does D is also specified and does not


diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c
index 7ac7c8190cc4..575f3e9c8c80 100644
--- a/security/apparmor/audit.c
+++ b/security/apparmor/audit.c
@@ -165,7 +165,7 @@ int aa_audit(int type, struct aa_profile *profile, struct 
common_audit_data *sa,
 }
 
 struct aa_audit_rule {
-       char *profile;
+       struct aa_label *label;
 };
 
 void aa_audit_rule_free(void *vrule)
@@ -173,7 +173,8 @@ void aa_audit_rule_free(void *vrule)
        struct aa_audit_rule *rule = vrule;
 
        if (rule) {
-               kfree(rule->profile);
+               if (!IS_ERR(rule->label))
+                       aa_put_label(rule->label);
                kfree(rule);
        }
 }
@@ -196,13 +197,11 @@ int aa_audit_rule_init(u32 field, u32 op, char *rulestr, 
void **vrule)
        if (!rule)
                return -ENOMEM;
 
-       rule->profile = kstrdup(rulestr, GFP_KERNEL);
-
-       if (!rule->profile) {
-               kfree(rule);
-               return -ENOMEM;
-       }
-
+       /* Currently rules are treated as coming from the root ns */
+       rule->label = aa_label_parse(&root_ns->unconfined->label, rulestr,
+                                    GFP_KERNEL, true, false);
+       if (IS_ERR(rule->label))
+               return PTR_ERR(rule->label);
        *vrule = rule;
 
        return 0;
@@ -229,8 +228,6 @@ int aa_audit_rule_match(u32 sid, u32 field, u32 op, void 
*vrule,
 {
        struct aa_audit_rule *rule = vrule;
        struct aa_label *label;
-       struct label_it i;
-       struct aa_profile *profile;
        int found = 0;
 
        label = aa_secid_to_label(sid);
@@ -238,12 +235,8 @@ int aa_audit_rule_match(u32 sid, u32 field, u32 op, void 
*vrule,
        if (!label)
                return -ENOENT;
 
-       label_for_each(i, label, profile) {
-               if (strcmp(rule->profile, profile->base.hname) == 0) {
-                       found = 1;
-                       break;
-               }
-       }
+       if (aa_label_is_subset(label, rule->label))
+               found = 1;
 
        switch (field) {
        case AUDIT_SUBJ_ROLE:

-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to