[RFC][PATCH] audit: add feature audit_lost reset

[Paul Moore]

On Wed, Dec 7, 2016 at 6:30 PM, Steve Grubb <sgrubb at redhat.com> wrote:
> On Wednesday, December 7, 2016 6:10:49 PM EST Paul Moore wrote:
>> On Wed, Dec 7, 2016 at 10:58 AM, Richard Guy Briggs <rgb at redhat.com> wrote:
>> > On 2016-12-07 10:53, Steve Grubb wrote:
>> >> On Wednesday, December 7, 2016 10:05:30 AM EST Paul Moore wrote:
>> >> > On Tue, Dec 6, 2016 at 10:32 PM, Richard Guy Briggs <rgb at redhat.com>
> wrote:
>> >> > > On 2016-12-06 19:17, Paul Moore wrote:
>> >> > >> On Tue, Dec 6, 2016 at 12:13 AM, Richard Guy Briggs <rgb at redhat.com>
>> >> > >> Okay, back up ... this whole mess about atomic_xchg() was always
>> >> > >> unrelated to my original suggestion, let's focus on my original
>> >> > >> comment ... don't reset the counter on a AUDIT_GET, reset it on a
>> >> > >> AUDIT_SET with an AUDIT_STATUS_LOST, does that make sense?
>> >> > >
>> >> > > I understood that.  It sounds like a nice simple and straightforward
>> >> > > method to do it but for the question of accuracy.  Please rewind to
>> >> > > my
>> >> > > fundamental point: How do we get an accurate reading of the last
>> >> > > value
>> >> > > of audit_lost before resetting it?
>> >> >
>> >> > Okay, I thought you were worried about a different race, which is why
>> >> > this discussion wasn't making much sense to me.  I understand your
>> >> > point, but I really dislike the API; although that's not your fault,
>> >> > it's really the only way to do it via AUDIT_GET.
>> >> >
>> >> > I'd much prefer we go with the cleaner AUDIT_SET approach and just not
>> >> > worry about the small race window.  It would only be an issue if you
>> >> > reset the count under heavy audit load, and why would you reset the
>> >> > lost value if you were under a heavy audit load?  That just doesn't
>> >> > make sense.
>> >> >
>> >> > I suppose we should hear from Steve on this since he was the one who
>> >> > has been asking for this feature, although I'm pretty sure I know what
>> >> > he is going to say.
>> >>
>> >> To start with, this request comes from users of the audit system. I just
>> >> passed along the request. The issue is that when you do auditctl -s, you
>> >> get the number of records lost. If you do it the next day, you have to
>> >> do math to see what the one day delta is. So, to make reporting easy,
>> >> they want it to be reset whenever they do audictl -s.
>> >>
>> >> You could also make a AUDIT_GET_RESET that gets the status and resets the
>> >> number atomically. Then I can add another commandline option to auditctl
>> >> that allows an admin to say also reset the counters. If that command
>> >> line option is passed, I call AUDIT_GET_RESET otherwise I call
>> >> AUDIT_GET. Thought?>
>> > This would be slightly simpler in kernel implementation than the method
>> > I proposed and would work fine, off the top of my head.
>>
>> I'd prefer not to introduce another command message type for something
>> small like this.
>>
>> Steve, do you have any objection to the AUDIT_SET based approach?
>
> Either way, we'd need a feature flag so that I can tell if the kernel supports
> this or not.

I think we are okay without a specific feature flag as sending a
AUDIT_SET/reset on an old kernel will be harmless; it won't do
anything, but it shouldn't return an error either.

> Also, it should accept "0" as the only valid value.

Of course.

> We can do this and I can make auditctl do the two back to back before displaying the
> results to minimize the window of risk.
>
>> Based on what you've said above, it would seem like the potential race
>> condition with AUDIT_SET wouldn't be a significant issue.
>
> All a matter of perspective. What I think is a reasonable risk someone else
> may disagree. Does anyone else on the list object? If not I'd say go with it.

Judgement calls are always a matter of perspective, since you are the
only one who has asked for this (even if it is by proxy) I was asking
for your perspective.  Anyway, it looks like we're probably okay here;
if we don't hear anything in the next day or two lets go ahead with
the AUDIT_SET approach.  If it proves to be a problem we can always
introduce one of the other approaches later.

-- 
paul moore
www.paul-moore.com