Re: [PATCH v2] audit: log nftables configuration change events once per table
On 2021-03-22 23:57, Pablo Neira Ayuso wrote: > On Mon, Mar 22, 2021 at 04:49:04PM -0400, Richard Guy Briggs wrote: > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > > index c1eb5cdb3033..42ba44890523 100644 > > --- a/net/netfilter/nf_tables_api.c > > +++ b/net/netfilter/nf_tables_api.c > [...] > > @@ -8006,12 +7938,47 @@ static void nft_commit_notify(struct net *net, u32 > > portid) > > WARN_ON_ONCE(!list_empty(>nft.notify_list)); > > } > > > > +void nf_tables_commit_audit_collect(struct list_head *adl, > > + struct nft_trans *trans) { > > nitpick: curly brace starts in the line. Duh, whoops! Brain fart. Too much muckin' in other codebases... > > + struct nft_audit_data *adp; > > + > > + list_for_each_entry(adp, adl, list) { > > + if (adp->table == trans->ctx.table) > > + goto found; > > + } > > + adp = kzalloc(sizeof(*adp), GFP_KERNEL); > > if (!adp) > ... This will need a bit more work since by the time this is called, nf_tables_commit() is not prepared to accept any errors, so I'll need to either ignore the error and continue, or allocate the table entries before step 3. > > + adp->table = trans->ctx.table; > > + INIT_LIST_HEAD(>list); > > + list_add(>list, adl); > > +found: > > + adp->entries++; > > + if (!adp->op || adp->op > trans->msg_type) > > + adp->op = trans->msg_type; > > +} > > + > > +#define AUNFTABLENAMELEN (NFT_TABLE_MAXNAMELEN + 22) > > + > > +void nf_tables_commit_audit_log(struct list_head *adl, u32 generation) { > ^ > same thing here > > > + struct nft_audit_data *adp, *adn; > > + char aubuf[AUNFTABLENAMELEN]; > > + > > + list_for_each_entry_safe(adp, adn, adl, list) { > > + snprintf(aubuf, AUNFTABLENAMELEN, "%s:%u", adp->table->name, > > +generation); > > + audit_log_nfcfg(aubuf, adp->table->family, adp->entries, > > + nft2audit_op[adp->op], GFP_KERNEL); > > + list_del(>list); > > + kfree(adp); > > + } > > +} > > + > > static int nf_tables_commit(struct net *net, struct sk_buff *skb) > > { > > struct nft_trans *trans, *next; > > struct nft_trans_elem *te; > > struct nft_chain *chain; > > struct nft_table *table; > > + LIST_HEAD(adl); > > int err; > > > > if (list_empty(>nft.commit_list)) { > > @@ -8206,12 +8173,15 @@ static int nf_tables_commit(struct net *net, struct > > sk_buff *skb) > > } > > break; > > } > > + nf_tables_commit_audit_collect(, trans); > > } > > > > nft_commit_notify(net, NETLINK_CB(skb).portid); > > nf_tables_gen_notify(net, skb, NFT_MSG_NEWGEN); > > nf_tables_commit_release(net); > > > > + nf_tables_commit_audit_log(, net->nft.base_seq); > > + > > This looks more self-contained and nicer, thanks. > > -- > Linux-audit mailing list > linux-au...@redhat.com > https://listman.redhat.com/mailman/listinfo/linux-audit - RGB -- Richard Guy Briggs 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
Re: [PATCH v2] audit: log nftables configuration change events once per table
On Mon, Mar 22, 2021 at 04:49:04PM -0400, Richard Guy Briggs wrote: > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > index c1eb5cdb3033..42ba44890523 100644 > --- a/net/netfilter/nf_tables_api.c > +++ b/net/netfilter/nf_tables_api.c [...] > @@ -8006,12 +7938,47 @@ static void nft_commit_notify(struct net *net, u32 > portid) > WARN_ON_ONCE(!list_empty(>nft.notify_list)); > } > > +void nf_tables_commit_audit_collect(struct list_head *adl, > + struct nft_trans *trans) { nitpick: curly brace starts in the line. > + struct nft_audit_data *adp; > + > + list_for_each_entry(adp, adl, list) { > + if (adp->table == trans->ctx.table) > + goto found; > + } > + adp = kzalloc(sizeof(*adp), GFP_KERNEL); if (!adp) ... > + adp->table = trans->ctx.table; > + INIT_LIST_HEAD(>list); > + list_add(>list, adl); > +found: > + adp->entries++; > + if (!adp->op || adp->op > trans->msg_type) > + adp->op = trans->msg_type; > +} > + > +#define AUNFTABLENAMELEN (NFT_TABLE_MAXNAMELEN + 22) > + > +void nf_tables_commit_audit_log(struct list_head *adl, u32 generation) { ^ same thing here > + struct nft_audit_data *adp, *adn; > + char aubuf[AUNFTABLENAMELEN]; > + > + list_for_each_entry_safe(adp, adn, adl, list) { > + snprintf(aubuf, AUNFTABLENAMELEN, "%s:%u", adp->table->name, > + generation); > + audit_log_nfcfg(aubuf, adp->table->family, adp->entries, > + nft2audit_op[adp->op], GFP_KERNEL); > + list_del(>list); > + kfree(adp); > + } > +} > + > static int nf_tables_commit(struct net *net, struct sk_buff *skb) > { > struct nft_trans *trans, *next; > struct nft_trans_elem *te; > struct nft_chain *chain; > struct nft_table *table; > + LIST_HEAD(adl); > int err; > > if (list_empty(>nft.commit_list)) { > @@ -8206,12 +8173,15 @@ static int nf_tables_commit(struct net *net, struct > sk_buff *skb) > } > break; > } > + nf_tables_commit_audit_collect(, trans); > } > > nft_commit_notify(net, NETLINK_CB(skb).portid); > nf_tables_gen_notify(net, skb, NFT_MSG_NEWGEN); > nf_tables_commit_release(net); > > + nf_tables_commit_audit_log(, net->nft.base_seq); > + This looks more self-contained and nicer, thanks.
[PATCH v2] audit: log nftables configuration change events once per table
Reduce logging of nftables events to a level similar to iptables. Restore the table field to list the table, adding the generation. Indicate the op as the most significant operation in the event. A couple of sample events: type=PROCTITLE msg=audit(2021-03-18 09:30:49.801:143) : proctitle=/usr/bin/python3 -s /usr/sbin/firewalld --nofork --nopid type=SYSCALL msg=audit(2021-03-18 09:30:49.801:143) : arch=x86_64 syscall=sendmsg success=yes exit=172 a0=0x6 a1=0x7ffdcfcbe650 a2=0x0 a3=0x7ffdcfcbd52c items=0 ppid=1 pid=367 auid=unset uid=root gid=root euid=root suid=root fsuid=root egid=roo t sgid=root fsgid=root tty=(none) ses=unset comm=firewalld exe=/usr/bin/python3.9 subj=system_u:system_r:firewalld_t:s0 key=(null) type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2 family=ipv6 entries=1 op=nft_register_table pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2 family=ipv4 entries=1 op=nft_register_table pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.801:143) : table=firewalld:2 family=inet entries=1 op=nft_register_table pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld type=PROCTITLE msg=audit(2021-03-18 09:30:49.839:144) : proctitle=/usr/bin/python3 -s /usr/sbin/firewalld --nofork --nopid type=SYSCALL msg=audit(2021-03-18 09:30:49.839:144) : arch=x86_64 syscall=sendmsg success=yes exit=22792 a0=0x6 a1=0x7ffdcfcbe650 a2=0x0 a3=0x7ffdcfcbd52c items=0 ppid=1 pid=367 auid=unset uid=root gid=root euid=root suid=root fsuid=root egid=r oot sgid=root fsgid=root tty=(none) ses=unset comm=firewalld exe=/usr/bin/python3.9 subj=system_u:system_r:firewalld_t:s0 key=(null) type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3 family=ipv6 entries=30 op=nft_register_chain pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3 family=ipv4 entries=30 op=nft_register_chain pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld type=NETFILTER_CFG msg=audit(2021-03-18 09:30:49.839:144) : table=firewalld:3 family=inet entries=165 op=nft_register_chain pid=367 subj=system_u:system_r:firewalld_t:s0 comm=firewalld The issue was originally documented in https://github.com/linux-audit/audit-kernel/issues/124 Signed-off-by: Richard Guy Briggs --- Changelog v2: - convert NFT ops to array indicies in nft2audit_op[] - use linux lists - use functions for each of collection and logging of audit data --- include/linux/audit.h | 28 +++ net/netfilter/nf_tables_api.c | 136 +- 2 files changed, 81 insertions(+), 83 deletions(-) diff --git a/include/linux/audit.h b/include/linux/audit.h index 82b7c1116a85..5fafcf4c13de 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -118,6 +118,34 @@ enum audit_nfcfgop { AUDIT_NFT_OP_INVALID, }; +static const u8 nft2audit_op[NFT_MSG_MAX] = { // enum nf_tables_msg_types + [NFT_MSG_NEWTABLE] = AUDIT_NFT_OP_TABLE_REGISTER, + [NFT_MSG_GETTABLE] = AUDIT_NFT_OP_INVALID, + [NFT_MSG_DELTABLE] = AUDIT_NFT_OP_TABLE_UNREGISTER, + [NFT_MSG_NEWCHAIN] = AUDIT_NFT_OP_CHAIN_REGISTER, + [NFT_MSG_GETCHAIN] = AUDIT_NFT_OP_INVALID, + [NFT_MSG_DELCHAIN] = AUDIT_NFT_OP_CHAIN_UNREGISTER, + [NFT_MSG_NEWRULE] = AUDIT_NFT_OP_RULE_REGISTER, + [NFT_MSG_GETRULE] = AUDIT_NFT_OP_INVALID, + [NFT_MSG_DELRULE] = AUDIT_NFT_OP_RULE_UNREGISTER, + [NFT_MSG_NEWSET]= AUDIT_NFT_OP_SET_REGISTER, + [NFT_MSG_GETSET]= AUDIT_NFT_OP_INVALID, + [NFT_MSG_DELSET]= AUDIT_NFT_OP_SET_UNREGISTER, + [NFT_MSG_NEWSETELEM]= AUDIT_NFT_OP_SETELEM_REGISTER, + [NFT_MSG_GETSETELEM]= AUDIT_NFT_OP_INVALID, + [NFT_MSG_DELSETELEM]= AUDIT_NFT_OP_SETELEM_UNREGISTER, + [NFT_MSG_NEWGEN]= AUDIT_NFT_OP_GEN_REGISTER, + [NFT_MSG_GETGEN]= AUDIT_NFT_OP_INVALID, + [NFT_MSG_TRACE] = AUDIT_NFT_OP_INVALID, + [NFT_MSG_NEWOBJ]= AUDIT_NFT_OP_OBJ_REGISTER, + [NFT_MSG_GETOBJ]= AUDIT_NFT_OP_INVALID, + [NFT_MSG_DELOBJ]= AUDIT_NFT_OP_OBJ_UNREGISTER, + [NFT_MSG_GETOBJ_RESET] = AUDIT_NFT_OP_OBJ_RESET, + [NFT_MSG_NEWFLOWTABLE] = AUDIT_NFT_OP_FLOWTABLE_REGISTER, + [NFT_MSG_GETFLOWTABLE] = AUDIT_NFT_OP_INVALID, + [NFT_MSG_DELFLOWTABLE] = AUDIT_NFT_OP_FLOWTABLE_UNREGISTER, +}; + extern int is_audit_feature_set(int which); extern int __init audit_register_class(int class, unsigned *list); diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index c1eb5cdb3033..42ba44890523 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -66,6 +66,13 @@ static const struct