linux-kernel - Re: [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1703251.0Aix3r8zCV@x2>
Date:   Mon, 19 Mar 2018 13:15:22 -0400
From:   Steve Grubb <sgrubb@...hat.com>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     Jiri Kosina <jikos@...nel.org>, Paul Moore <paul@...l-moore.com>,
        linux-audit@...hat.com, Andrew Morton <akpm@...ux-foundation.org>,
        Michal Hocko <mhocko@...e.com>,
        Oleg Nesterov <oleg@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] audit: set TIF_AUDIT_SYSCALL only if audit filter has been populated

On Tuesday, March 13, 2018 8:35:44 PM EDT Andy Lutomirski wrote:
> On Wed, Mar 14, 2018 at 12:28 AM, Jiri Kosina <jikos@...nel.org> wrote:
> > On Wed, 14 Mar 2018, Andy Lutomirski wrote:
> >> > Yes...I wished I was in on the beginning of this discussion. Here's
> >> > the
> >> > problem. We need all tasks auditable unless specifically dismissed as
> >> > uninteresting. This would be a task,never rule.
> >> > 
> >> > The way we look at it, is if it boots with audit=1, then we know
> >> > auditd
> >> > is expected to run at some point. So, we need all tasks to stay
> >> > auditable. If they weren't and auditd enabled auditing, then we'd need
> >> > to walk the whole proctable and stab TIF_AUDIT_SYSCALL into every
> >> > process in the system. It was decided that this is too ugly.
> >> 
> >> When was that decided?  That's what this patch does.
> > 
> > I'd like to see some more justification as well.
> > 
> > Namely, if I compare "setting TIF_AUDIT_SYSCALL for every process on a
> > need-to-be-so basis" to "we always go through the slow path and
> > pessimistically assume that audit is enabled and has reasonable ruleset
> > loaded", I have my own (different) opinion of what is too ugly.
> 
> Me too.
> 
> That being said, on re-review of my old code, I think that
> audit_dec_n_rules() may be the wrong approach.  It may be better to
> leave TIF_AUDIT_SYSCALL set but to make the audit code itself clear
> the flag the next time through.  That way we don't end up with a
> partially filled in syscall audit record that never gets consumed if
> we clear TIF_AUDIT_SYSCALL in the middle of a syscall.

So what happened when auditd is being restarted due to system update? Its 
possible to have no auditd, no rules, and audit_enabled == 0. But its getting 
ready to start a new auditd, enable audit, and load rules. In this case, we 
expect tasks already dismissed by task filter to stay dismissed because the 
task filter only runs at fork time.

For example, suppose httpd is not desired to be audited. The admin sets up a 
task rule that sets it to never for httpd. The rule triggers at fork and 
marks it inauditable. Would this patch cause httpd to suddenly become 
auditable again during an auditd restart?

Rather than play run time games which is ultimately racey and prone to 
missing something, what about the approach of controlling this from a boot 
variable and letting user space see an error should auditing try to do its 
normal thing when its not wanted? This way auditd can exit and there aren't 
any races.

-Steve