[PATCH] audit: restore AUDIT_LOGINUID unset ABI
Richard Guy Briggs
rgb at redhat.com
Fri Dec 12 05:19:44 UTC 2014
On 14/12/09, Paul Moore wrote:
> On Tuesday, December 09, 2014 11:30:14 AM Richard Guy Briggs wrote:
> > On 14/12/08, Paul Moore wrote:
> > > As I understand it, when old userspace would set a filter with
> > > AUDIT_LOGINUID but when it listed the audit rules in the kernel it would
> > > see AUDIT_LOGINUID_SET, yes? This patch attempts to fix this by marking a
> > > legacy userspace with the AUDIT_LOGINUID_LEGACY bitmask on the internal
> > > kernel representation so that when the rules are dumped to userspace the
> > > AUDIT_LOGINUID_SET rule can be rewritten as AUDIT_LOGINUID, yes?
> >
> > Correct.
> >
> > > However, there are some things that are not immediately obvious to me:
> > >
> > > * Why are we using a bit in audit_field->type to indicate the legacy
> > > nature of userspace?
> >
> > Convenience. Adding a new member to audit_field or audit_krule seemed
> > unnecessary memory overhead (however, it then complicates other code...).
> >
> > > * Why are we reusing the AUDIT_NEGATE bit in the type field to indicate a
> > > legacy userspace?
> >
> > It wasn't reaped when commit 18900909 went through... (first introduced
> > with original audit in b7b0074c, 2004-04-11). It would have been more
> > clear if I had sent a first patch to remove AUDIT_NEGATE altogether and
> > re-introduce it with a new name in this patch.
>
> The problem is that AUDIT_NEGATE lives in the userspace visible header file
> which means it needs to live there for pretty much forever. While I would
> like to see us remote it for clarity's sake, I think we're stuck with it.
Ok, fair enough, the same goes for AUDIT_{LIST,ADD,DEL} since they are
no longer used, but still remain in the API. I can use another value.
> > > * Why are we not using something in audit_krule? Without looking to in
> > > depth it would appear that there are multiple fields which might be
> > > useful, e.g. "vers_ops", "flags"?
> >
> > audit_krule applies to the set of all fields for this rule. I wanted
> > something that localized it very unambiguously to this one field.
>
> You can only add or delete rules, right? Not modify? If you can only add or
> delete a rule, then if one of the fields in that rule is sent from legacy
> userspace I think it is safe to set an indicator in one of the audit_krule
> fields. I understand your point, but I'm not sure it is something to worry
> too much about; I'd rather see the legacy indicator here than in the
> audit_field->type field where we might have to contend with userspace usage at
> some point.
>
> I'd like to explore the idea of not using audit_field->type; I picked
> "vers_ops" and "flags" since they seemed like reasonable places to start. The
> "vers_ops" field in particular appears to be almost unused in the current code
> and it seems like a good way to track userspace versions perhaps, e.g. 1 =
> legacy, 2 = now current, etc.? I'm curious if this sounds reasonable to you.
vers_ops appears to be assigned, copied and never otherwise read. And
in fact *that* should have been removed with commit 18900909. I'll just
prepare a patch to rip it out.
flags looks like a better choice... and I'll have to do some similar
filtering that I did for type and as has been already done for
AUDIT_FILTER_PREPEND... In fact, the inverse of AUDIT_FILTER_PREPEND.
And this patch is simpler than the one you are critiquing. Bonus.
> paul moore
- RGB
--
Richard Guy Briggs <rbriggs at redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
More information about the Linux-audit
mailing list