Re: [PATCH ghak25 v3 1/3] audit: tidy and extend netfilter_cfg x_tables and ebtables logging

From: Richard Guy Briggs
Date: Wed Mar 18 2020 - 18:00:48 EST


On 2020-03-18 17:54, Paul Moore wrote:
> On Tue, Mar 17, 2020 at 5:31 PM Richard Guy Briggs <rgb@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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.

audit_dummy_context was deliberately removed to make this record
delivered by default.

> paul moore

- RGB

--
Richard Guy Briggs <rgb@xxxxxxxxxx>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635