Re: [PATCH] audit: restore AUDIT_LOGINUID unset ABI
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.
> * 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.
--
paul moore
security and virtualization @ redhat