On Wed, Mar 18, 2020 at 6:00 PM Richard Guy Briggs <rgb@xxxxxxxxxx> wrote: > 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. 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
