Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <7081a5b9c7d2e8085c49cec2fa72fcbb0b25e0d7.1600778472.git.rgb@redhat.com>
In-Reply-To: <7081a5b9c7d2e8085c49cec2fa72fcbb0b25e0d7.1600778472.git.rgb@redhat.com>
From:   Paul Moore <paul@paul-moore.com>
Date:   Wed, 23 Sep 2020 10:29:41 -0400
Message-ID: <CAHC9VhSPk2RiMsnjTKw02_+_1Kagm06m+-r=ooNruT+fuuixYQ@mail.gmail.com>
Subject: Re: [PATCH ghak120 V5] audit: trigger accompanying records when no
 rules present
To:     Richard Guy Briggs <rgb@redhat.com>
Cc:     Linux-Audit Mailing List <linux-audit@redhat.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Linux Security Module list 
        <linux-security-module@vger.kernel.org>,
        Eric Paris <eparis@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Sep 22, 2020 at 8:45 AM Richard Guy Briggs <rgb@redhat.com> wrote:
>
> When there are no audit rules registered, mandatory records (config,
> etc.) are missing their accompanying records (syscall, proctitle, etc.).
>
> This is due to audit context dummy set on syscall entry based on absence
> of rules that signals that no other records are to be printed.  Clear the dummy
> bit if any record is generated, open coding this in audit_log_start().
>
> The proctitle context and dummy checks are pointless since the
> proctitle record will not be printed if no syscall records are printed.
>
> The fds array is reset to -1 after the first syscall to indicate it
> isn't valid any more, but was never set to -1 when the context was
> allocated to indicate it wasn't yet valid.
>
> Check ctx->pwd in audit_log_name().
>
> The audit_inode* functions can be called without going through
> getname_flags() or getname_kernel() that sets audit_names and cwd, so
> set the cwd in audit_alloc_name() if it has not already been done so due to
> audit_names being valid and purge all other audit_getcwd() calls.
>
> Revert the LSM dump_common_audit_data() LSM_AUDIT_DATA_* cases from the
> ghak96 patch since they are no longer necessary due to cwd coverage in
> audit_alloc_name().
>
> Thanks to bauen1 <j2468h@googlemail.com> for reporting LSM situations in
> which context->cwd is not valid, inadvertantly fixed by the ghak96 patch.
>
> Please see upstream github issue
> https://github.com/linux-audit/audit-kernel/issues/120
> This is also related to upstream github issue
> https://github.com/linux-audit/audit-kernel/issues/96
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> Chagelog:
> v5:
> - open code audit_clear_dummy() in audit_log_start()
> - fix check for ctx->pwd in audit_log_name()
> - open code _audit_getcwd() contents in audit_alloc_name()
> - ditch all *audit_getcwd() calls
>
> v4:
> - resubmit after revert
>
> v3:
> - initialize fds[0] to -1
> - init cwd for ghak96 LSM_AUDIT_DATA_NET:AF_UNIX case
> - init cwd for audit_inode{,_child}
>
> v2:
> - unconditionally clear dummy
> - create audit_clear_dummy accessor function
> - remove proctitle context and dummy checks
>
>  include/linux/audit.h |  8 --------
>  kernel/audit.c        |  3 +++
>  kernel/auditsc.c      | 27 +++++++--------------------
>  security/lsm_audit.c  |  5 -----
>  4 files changed, 10 insertions(+), 33 deletions(-)

I've gone over this revision a couple of times now and it looks okay,
but past experience is whispering in my ear that perhaps this is
better to wait on this for the next cycle so it gets a full set of
-rcX releases.  Thoughts?

-- 
paul moore
www.paul-moore.com