On Fri, May 18, 2018 at 12:06 PM Matthew Garrett <[email protected]> wrote: > +static int aa_secmark_perm(struct aa_profile *profile, u32 request, u32 secid, > + struct common_audit_data *sa, struct sock *sk) > +{ > + int i, ret; > + struct aa_perms perms = { }; > + > + if (profile->secmark_count == 0) > + return 0; > + > + for (i = 0; i < profile->secmark_count; i++) { > + if (!profile->secmark[i].secid) { > + ret = apparmor_secmark_init(&profile->secmark[i]); > + if (ret) > + return ret; > + } > + > + if (profile->secmark[i].secid == secid) { > + if (profile->secmark[i].deny) > + perms.deny = ALL_PERMS_MASK; > + else > + perms.allow = ALL_PERMS_MASK; > + > + if (profile->secmark[i].audit) > + perms.audit = ALL_PERMS_MASK; > + } > + } > + > + aa_apply_modes_to_perms(profile, &perms); > + > + return aa_check_perms(profile, &perms, request, sa, audit_net_cb); > +}
So, specifically, my question is about whether this is doing the right thing. If someone writes a rule that's just: network then secmark_count should be 0 and the rule will grant full network access. If someone has: network label=foo then secmark_count should be > 0, and the secmark policy checking will pass for packets with a label of foo and fail otherwise. For network deny network label=foo then secmark_count should be > 0, and packets with a label of foo will get perms.deny set to ALL_PERMS_MASK. However, something with a label of bar will end up with aa_apply_modes_to_perms being called without perms.allow having been set, and I think that's going to do the wrong thing. The "easy" thing to do would be to default to perms.allow = ALL_PERMS_MASK if nothing matches, but that would mean: network label=foo on its own would allow non-foo packets to pass. So I think what actually needs to happen here is for a bare network statement to imply a wildcard secmark label? If I keep the secmark_count == 0 special case, I think this can then be done in the parser - any parser version that understands the label=foo syntax would then also generate a special wildcard secmark statement in response to a bare network permission. Thoughts? -- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
