linux-kernel - Re: [PATCH ghak25 v3 1/3] audit: tidy and extend netfilter_cfg x

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHC9VhTFSNhedSmJA38zdq3gXira9FQ6D-bFUFxnwWKcJOfpUQ@mail.gmail.com>
Date:   Wed, 18 Mar 2020 18:09:36 -0400
From:   Paul Moore <paul@...l-moore.com>
To:     Richard Guy Briggs <rgb@...hat.com>
Cc:     Linux-Audit Mailing List <linux-audit@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        netfilter-devel@...r.kernel.org, sgrubb@...hat.com,
        omosnace@...hat.com, fw@...len.de, twoerner@...hat.com,
        Eric Paris <eparis@...isplace.org>, ebiederm@...ssion.com,
        tgraf@...radead.org
Subject: Re: [PATCH ghak25 v3 1/3] audit: tidy and extend netfilter_cfg
 x_tables and ebtables logging

On Wed, Mar 18, 2020 at 6:00 PM Richard Guy Briggs <rgb@...hat.com> wrote:
> On 2020-03-18 17:54, Paul Moore wrote:
> > On Tue, Mar 17, 2020 at 5:31 PM Richard Guy Briggs <rgb@...hat.com> wrote:
> > >
> > > NETFILTER_CFG record generation was inconsistent for x_tables and
> > > ebtables configuration changes.  The call was needlessly messy and there
> > > were supporting records missing at times while they were produced when
> > > not requested.  Simplify the logging call into a new audit_log_nfcfg
> > > call.  Honour the audit_enabled setting while more consistently
> > > recording information including supporting records by tidying up dummy
> > > checks.
> > >
> > > Add an op= field that indicates the operation being performed (register
> > > or replace).
> > >
> > > Here is the enhanced sample record:
> > >   type=NETFILTER_CFG msg=audit(1580905834.919:82970): table=filter family=2 entries=83 op=replace
> > >
> > > Generate audit NETFILTER_CFG records on ebtables table registration.
> > > Previously this was being done for x_tables registration and replacement
> > > operations and ebtables table replacement only.
> > >
> > > See: https://github.com/linux-audit/audit-kernel/issues/25
> > > See: https://github.com/linux-audit/audit-kernel/issues/35
> > > See: https://github.com/linux-audit/audit-kernel/issues/43
> > >
> > > Signed-off-by: Richard Guy Briggs <rgb@...hat.com>
> > > ---
> > >  include/linux/audit.h           | 19 +++++++++++++++++++
> > >  kernel/auditsc.c                | 24 ++++++++++++++++++++++++
> > >  net/bridge/netfilter/ebtables.c | 12 ++++--------
> > >  net/netfilter/x_tables.c        | 12 +++---------
> > >  4 files changed, 50 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > > index f9ceae57ca8d..f4aed2b9be8d 100644
> > > --- a/include/linux/audit.h
> > > +++ b/include/linux/audit.h
> > > @@ -94,6 +94,11 @@ struct audit_ntp_data {
> > >  struct audit_ntp_data {};
> > >  #endif
> > >
> > > +enum audit_nfcfgop {
> > > +       AUDIT_XT_OP_REGISTER,
> > > +       AUDIT_XT_OP_REPLACE,
> > > +};
> > > +
> > >  extern int is_audit_feature_set(int which);
> > >
> > >  extern int __init audit_register_class(int class, unsigned *list);
> > > @@ -379,6 +384,8 @@ extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm,
> > >  extern void __audit_fanotify(unsigned int response);
> > >  extern void __audit_tk_injoffset(struct timespec64 offset);
> > >  extern void __audit_ntp_log(const struct audit_ntp_data *ad);
> > > +extern void __audit_log_nfcfg(const char *name, u8 af, unsigned int nentries,
> > > +                             enum audit_nfcfgop op);
> > >
> > >  static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
> > >  {
> > > @@ -514,6 +521,13 @@ static inline void audit_ntp_log(const struct audit_ntp_data *ad)
> > >                 __audit_ntp_log(ad);
> > >  }
> > >
> > > +static inline void audit_log_nfcfg(const char *name, u8 af, unsigned int nentries,
> > > +                                  enum audit_nfcfgop op)
> > > +{
> > > +       if (audit_enabled)
> > > +               __audit_log_nfcfg(name, af, nentries, op);
> >
> > Do we want a dummy check here too?  Or do we always want to generate
> > this record (assuming audit is enabled) because it is a configuration
> > related record?
>
> This is an audit configuration change, so it is mandatory unless there
> is a rule that excludes it.  I talked about this in the cover letter,
> but perhaps my wording wasn't as clear as it could have been.

Yes, it wasn't clear to me what your goals were.

In general I think this patchset looks okay, but it's -rc6 so this
should wait for the next cycle; it will also give the netdev/netfilter
folks a chance to comment on this latest revision.

-- 
paul moore
www.paul-moore.com