* [PATCH ghak124 v3] audit: log nftables configuration change events @ 2020-06-04 13:20 Richard Guy Briggs 2020-06-04 17:03 ` Steve Grubb ` (3 more replies) 0 siblings, 4 replies; 24+ messages in thread From: Richard Guy Briggs @ 2020-06-04 13:20 UTC (permalink / raw) To: Linux-Audit Mailing List, LKML, netfilter-devel Cc: Richard Guy Briggs, fw, twoerner, eparis, tgraf iptables, ip6tables, arptables and ebtables table registration, replacement and unregistration configuration events are logged for the native (legacy) iptables setsockopt api, but not for the nftables netlink api which is used by the nft-variant of iptables in addition to nftables itself. Add calls to log the configuration actions in the nftables netlink api. This uses the same NETFILTER_CFG record format but overloads the table field. type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : table=?:0;?:0 family=unspecified entries=2 op=nft_register_gen pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld ... type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : table=firewalld:1;?:0 family=inet entries=0 op=nft_register_table pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld ... type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;filter_FORWARD:85 family=inet entries=8 op=nft_register_chain pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld ... type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;filter_FORWARD:85 family=inet entries=101 op=nft_register_rule pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld ... type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;__set0:87 family=inet entries=87 op=nft_register_setelem pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld ... type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;__set0:87 family=inet entries=0 op=nft_register_set pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld For further information please see issue https://github.com/linux-audit/audit-kernel/issues/124 Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- Changelog: v3: - inline message type rather than table v2: - differentiate between xtables and nftables - add set, setelem, obj, flowtable, gen - use nentries field as appropriate per type - overload the "tables" field with table handle and chain/set/flowtable include/linux/audit.h | 18 ++++++++ kernel/auditsc.c | 24 ++++++++-- net/netfilter/nf_tables_api.c | 103 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 142 insertions(+), 3 deletions(-) diff --git a/include/linux/audit.h b/include/linux/audit.h index 3fcd9ee49734..604ede630580 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -12,6 +12,7 @@ #include <linux/sched.h> #include <linux/ptrace.h> #include <uapi/linux/audit.h> +#include <uapi/linux/netfilter/nf_tables.h> #define AUDIT_INO_UNSET ((unsigned long)-1) #define AUDIT_DEV_UNSET ((dev_t)-1) @@ -98,6 +99,23 @@ enum audit_nfcfgop { AUDIT_XT_OP_REGISTER, AUDIT_XT_OP_REPLACE, AUDIT_XT_OP_UNREGISTER, + AUDIT_NFT_OP_TABLE_REGISTER, + AUDIT_NFT_OP_TABLE_UNREGISTER, + AUDIT_NFT_OP_CHAIN_REGISTER, + AUDIT_NFT_OP_CHAIN_UNREGISTER, + AUDIT_NFT_OP_RULE_REGISTER, + AUDIT_NFT_OP_RULE_UNREGISTER, + AUDIT_NFT_OP_SET_REGISTER, + AUDIT_NFT_OP_SET_UNREGISTER, + AUDIT_NFT_OP_SETELEM_REGISTER, + AUDIT_NFT_OP_SETELEM_UNREGISTER, + AUDIT_NFT_OP_GEN_REGISTER, + AUDIT_NFT_OP_OBJ_REGISTER, + AUDIT_NFT_OP_OBJ_UNREGISTER, + AUDIT_NFT_OP_OBJ_RESET, + AUDIT_NFT_OP_FLOWTABLE_REGISTER, + AUDIT_NFT_OP_FLOWTABLE_UNREGISTER, + AUDIT_NFT_OP_INVALID, }; extern int is_audit_feature_set(int which); diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 468a23390457..3a9100e95fda 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -75,6 +75,7 @@ #include <linux/uaccess.h> #include <linux/fsnotify_backend.h> #include <uapi/linux/limits.h> +#include <uapi/linux/netfilter/nf_tables.h> #include "audit.h" @@ -136,9 +137,26 @@ struct audit_nfcfgop_tab { }; static const struct audit_nfcfgop_tab audit_nfcfgs[] = { - { AUDIT_XT_OP_REGISTER, "register" }, - { AUDIT_XT_OP_REPLACE, "replace" }, - { AUDIT_XT_OP_UNREGISTER, "unregister" }, + { AUDIT_XT_OP_REGISTER, "xt_register" }, + { AUDIT_XT_OP_REPLACE, "xt_replace" }, + { AUDIT_XT_OP_UNREGISTER, "xt_unregister" }, + { AUDIT_NFT_OP_TABLE_REGISTER, "nft_register_table" }, + { AUDIT_NFT_OP_TABLE_UNREGISTER, "nft_unregister_table" }, + { AUDIT_NFT_OP_CHAIN_REGISTER, "nft_register_chain" }, + { AUDIT_NFT_OP_CHAIN_UNREGISTER, "nft_unregister_chain" }, + { AUDIT_NFT_OP_RULE_REGISTER, "nft_register_rule" }, + { AUDIT_NFT_OP_RULE_UNREGISTER, "nft_unregister_rule" }, + { AUDIT_NFT_OP_SET_REGISTER, "nft_register_set" }, + { AUDIT_NFT_OP_SET_UNREGISTER, "nft_unregister_set" }, + { AUDIT_NFT_OP_SETELEM_REGISTER, "nft_register_setelem" }, + { AUDIT_NFT_OP_SETELEM_UNREGISTER, "nft_unregister_setelem" }, + { AUDIT_NFT_OP_GEN_REGISTER, "nft_register_gen" }, + { AUDIT_NFT_OP_OBJ_REGISTER, "nft_register_obj" }, + { AUDIT_NFT_OP_OBJ_UNREGISTER, "nft_unregister_obj" }, + { AUDIT_NFT_OP_OBJ_RESET, "nft_reset_obj" }, + { AUDIT_NFT_OP_FLOWTABLE_REGISTER, "nft_register_flowtable" }, + { AUDIT_NFT_OP_FLOWTABLE_UNREGISTER, "nft_unregister_flowtable" }, + { AUDIT_NFT_OP_INVALID, "nft_invalid" }, }; static int audit_match_perm(struct audit_context *ctx, int mask) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 3558e76e2733..b9e7440cc87d 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -12,6 +12,7 @@ #include <linux/netlink.h> #include <linux/vmalloc.h> #include <linux/rhashtable.h> +#include <linux/audit.h> #include <linux/netfilter.h> #include <linux/netfilter/nfnetlink.h> #include <linux/netfilter/nf_tables.h> @@ -693,6 +694,16 @@ static void nf_tables_table_notify(const struct nft_ctx *ctx, int event) { struct sk_buff *skb; int err; + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0", + ctx->table->name, ctx->table->handle); + + audit_log_nfcfg(buf, + ctx->family, + ctx->table->use, + event == NFT_MSG_NEWTABLE ? + AUDIT_NFT_OP_TABLE_REGISTER : + AUDIT_NFT_OP_TABLE_UNREGISTER); + kfree(buf); if (!ctx->report && !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES)) @@ -1428,6 +1439,17 @@ static void nf_tables_chain_notify(const struct nft_ctx *ctx, int event) { struct sk_buff *skb; int err; + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;%s:%llu", + ctx->table->name, ctx->table->handle, + ctx->chain->name, ctx->chain->handle); + + audit_log_nfcfg(buf, + ctx->family, + ctx->chain->use, + event == NFT_MSG_NEWCHAIN ? + AUDIT_NFT_OP_CHAIN_REGISTER : + AUDIT_NFT_OP_CHAIN_UNREGISTER); + kfree(buf); if (!ctx->report && !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES)) @@ -2691,6 +2713,17 @@ static void nf_tables_rule_notify(const struct nft_ctx *ctx, { struct sk_buff *skb; int err; + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;%s:%llu", + ctx->table->name, ctx->table->handle, + ctx->chain->name, ctx->chain->handle); + + audit_log_nfcfg(buf, + ctx->family, + rule->handle, + event == NFT_MSG_NEWRULE ? + AUDIT_NFT_OP_RULE_REGISTER : + AUDIT_NFT_OP_RULE_UNREGISTER); + kfree(buf); if (!ctx->report && !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES)) @@ -3693,6 +3726,17 @@ static void nf_tables_set_notify(const struct nft_ctx *ctx, struct sk_buff *skb; u32 portid = ctx->portid; int err; + char *buf = kasprintf(gfp_flags, "%s:%llu;%s:%llu", + ctx->table->name, ctx->table->handle, + set->name, set->handle); + + audit_log_nfcfg(buf, + ctx->family, + set->field_count, + event == NFT_MSG_NEWSET ? + AUDIT_NFT_OP_SET_REGISTER : + AUDIT_NFT_OP_SET_UNREGISTER); + kfree(buf); if (!ctx->report && !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES)) @@ -4809,6 +4853,17 @@ static void nf_tables_setelem_notify(const struct nft_ctx *ctx, u32 portid = ctx->portid; struct sk_buff *skb; int err; + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;%s:%llu", + ctx->table->name, ctx->table->handle, + set->name, set->handle); + + audit_log_nfcfg(buf, + ctx->family, + set->handle, + event == NFT_MSG_NEWSETELEM ? + AUDIT_NFT_OP_SETELEM_REGISTER : + AUDIT_NFT_OP_SETELEM_UNREGISTER); + kfree(buf); if (!ctx->report && !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES)) return; @@ -5890,6 +5945,19 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb) obj->ops->type->type != filter->type) goto cont; + if (reset) { + char *buf = kasprintf(GFP_KERNEL, + "%s:%llu;?:0", + table->name, + table->handle); + + audit_log_nfcfg(buf, + family, + obj->handle, + AUDIT_NFT_OP_OBJ_RESET); + kfree(buf); + } + if (nf_tables_fill_obj_info(skb, net, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq, NFT_MSG_NEWOBJ, @@ -6000,6 +6068,17 @@ static int nf_tables_getobj(struct net *net, struct sock *nlsk, if (NFNL_MSG_TYPE(nlh->nlmsg_type) == NFT_MSG_GETOBJ_RESET) reset = true; + if (reset) { + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0", + table->name, table->handle); + + audit_log_nfcfg(buf, + family, + obj->handle, + AUDIT_NFT_OP_OBJ_RESET); + kfree(buf); + } + err = nf_tables_fill_obj_info(skb2, net, NETLINK_CB(skb).portid, nlh->nlmsg_seq, NFT_MSG_NEWOBJ, 0, family, table, obj, reset); @@ -6075,6 +6154,16 @@ void nft_obj_notify(struct net *net, const struct nft_table *table, { struct sk_buff *skb; int err; + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0", + table->name, table->handle); + + audit_log_nfcfg(buf, + family, + obj->handle, + event == NFT_MSG_NEWOBJ ? + AUDIT_NFT_OP_OBJ_REGISTER : + AUDIT_NFT_OP_OBJ_UNREGISTER); + kfree(buf); if (!report && !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES)) @@ -6701,6 +6790,17 @@ static void nf_tables_flowtable_notify(struct nft_ctx *ctx, { struct sk_buff *skb; int err; + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;%s:%llu", + flowtable->table->name, flowtable->table->handle, + flowtable->name, flowtable->handle); + + audit_log_nfcfg(buf, + ctx->family, + flowtable->hooknum, + event == NFT_MSG_NEWFLOWTABLE ? + AUDIT_NFT_OP_FLOWTABLE_REGISTER : + AUDIT_NFT_OP_FLOWTABLE_UNREGISTER); + kfree(buf); if (ctx->report && !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES)) @@ -6822,6 +6922,9 @@ static void nf_tables_gen_notify(struct net *net, struct sk_buff *skb, struct sk_buff *skb2; int err; + audit_log_nfcfg("?:0;?:0", 0, net->nft.base_seq, + AUDIT_NFT_OP_GEN_REGISTER); + if (nlmsg_report(nlh) && !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES)) return; -- 1.8.3.1 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2020-06-04 13:20 [PATCH ghak124 v3] audit: log nftables configuration change events Richard Guy Briggs @ 2020-06-04 17:03 ` Steve Grubb 2020-06-04 17:57 ` Richard Guy Briggs 2020-06-24 0:34 ` Paul Moore ` (2 subsequent siblings) 3 siblings, 1 reply; 24+ messages in thread From: Steve Grubb @ 2020-06-04 17:03 UTC (permalink / raw) To: Richard Guy Briggs Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, eparis, tgraf On Thursday, June 4, 2020 9:20:49 AM EDT Richard Guy Briggs wrote: > iptables, ip6tables, arptables and ebtables table registration, > replacement and unregistration configuration events are logged for the > native (legacy) iptables setsockopt api, but not for the > nftables netlink api which is used by the nft-variant of iptables in > addition to nftables itself. > > Add calls to log the configuration actions in the nftables netlink api. > > This uses the same NETFILTER_CFG record format but overloads the table > field. > > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : table=?:0;?:0 > family=unspecified entries=2 op=nft_register_gen pid=396 > subj=system_u:system_r:firewalld_t:s0 comm=firewalld ... > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : > table=firewalld:1;?:0 family=inet entries=0 op=nft_register_table pid=396 > subj=system_u:system_r:firewalld_t:s0 comm=firewalld ... > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : > table=firewalld:1;filter_FORWARD:85 family=inet entries=8 > op=nft_register_chain pid=396 subj=system_u:system_r:firewalld_t:s0 > comm=firewalld ... > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : > table=firewalld:1;filter_FORWARD:85 family=inet entries=101 > op=nft_register_rule pid=396 subj=system_u:system_r:firewalld_t:s0 > comm=firewalld ... > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : > table=firewalld:1;__set0:87 family=inet entries=87 op=nft_register_setelem > pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld ... > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : > table=firewalld:1;__set0:87 family=inet entries=0 op=nft_register_set > pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > > For further information please see issue > https://github.com/linux-audit/audit-kernel/issues/124 > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > --- > Changelog: > v3: > - inline message type rather than table > > v2: > - differentiate between xtables and nftables > - add set, setelem, obj, flowtable, gen > - use nentries field as appropriate per type > - overload the "tables" field with table handle and chain/set/flowtable > > include/linux/audit.h | 18 ++++++++ > kernel/auditsc.c | 24 ++++++++-- > net/netfilter/nf_tables_api.c | 103 > ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 142 > insertions(+), 3 deletions(-) > > diff --git a/include/linux/audit.h b/include/linux/audit.h > index 3fcd9ee49734..604ede630580 100644 > --- a/include/linux/audit.h > +++ b/include/linux/audit.h > @@ -12,6 +12,7 @@ > #include <linux/sched.h> > #include <linux/ptrace.h> > #include <uapi/linux/audit.h> > +#include <uapi/linux/netfilter/nf_tables.h> > > #define AUDIT_INO_UNSET ((unsigned long)-1) > #define AUDIT_DEV_UNSET ((dev_t)-1) > @@ -98,6 +99,23 @@ enum audit_nfcfgop { > AUDIT_XT_OP_REGISTER, > AUDIT_XT_OP_REPLACE, > AUDIT_XT_OP_UNREGISTER, > + AUDIT_NFT_OP_TABLE_REGISTER, > + AUDIT_NFT_OP_TABLE_UNREGISTER, > + AUDIT_NFT_OP_CHAIN_REGISTER, > + AUDIT_NFT_OP_CHAIN_UNREGISTER, > + AUDIT_NFT_OP_RULE_REGISTER, > + AUDIT_NFT_OP_RULE_UNREGISTER, > + AUDIT_NFT_OP_SET_REGISTER, > + AUDIT_NFT_OP_SET_UNREGISTER, > + AUDIT_NFT_OP_SETELEM_REGISTER, > + AUDIT_NFT_OP_SETELEM_UNREGISTER, > + AUDIT_NFT_OP_GEN_REGISTER, > + AUDIT_NFT_OP_OBJ_REGISTER, > + AUDIT_NFT_OP_OBJ_UNREGISTER, > + AUDIT_NFT_OP_OBJ_RESET, > + AUDIT_NFT_OP_FLOWTABLE_REGISTER, > + AUDIT_NFT_OP_FLOWTABLE_UNREGISTER, > + AUDIT_NFT_OP_INVALID, > }; > > extern int is_audit_feature_set(int which); > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index 468a23390457..3a9100e95fda 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -75,6 +75,7 @@ > #include <linux/uaccess.h> > #include <linux/fsnotify_backend.h> > #include <uapi/linux/limits.h> > +#include <uapi/linux/netfilter/nf_tables.h> > > #include "audit.h" > > @@ -136,9 +137,26 @@ struct audit_nfcfgop_tab { > }; > > static const struct audit_nfcfgop_tab audit_nfcfgs[] = { > - { AUDIT_XT_OP_REGISTER, "register" }, > - { AUDIT_XT_OP_REPLACE, "replace" }, > - { AUDIT_XT_OP_UNREGISTER, "unregister" }, > + { AUDIT_XT_OP_REGISTER, "xt_register" }, > + { AUDIT_XT_OP_REPLACE, "xt_replace" }, > + { AUDIT_XT_OP_UNREGISTER, "xt_unregister" }, > + { AUDIT_NFT_OP_TABLE_REGISTER, "nft_register_table" }, > + { AUDIT_NFT_OP_TABLE_UNREGISTER, "nft_unregister_table" }, > + { AUDIT_NFT_OP_CHAIN_REGISTER, "nft_register_chain" }, > + { AUDIT_NFT_OP_CHAIN_UNREGISTER, "nft_unregister_chain" }, > + { AUDIT_NFT_OP_RULE_REGISTER, "nft_register_rule" }, > + { AUDIT_NFT_OP_RULE_UNREGISTER, "nft_unregister_rule" }, > + { AUDIT_NFT_OP_SET_REGISTER, "nft_register_set" }, > + { AUDIT_NFT_OP_SET_UNREGISTER, "nft_unregister_set" }, > + { AUDIT_NFT_OP_SETELEM_REGISTER, "nft_register_setelem" }, > + { AUDIT_NFT_OP_SETELEM_UNREGISTER, "nft_unregister_setelem" }, > + { AUDIT_NFT_OP_GEN_REGISTER, "nft_register_gen" }, > + { AUDIT_NFT_OP_OBJ_REGISTER, "nft_register_obj" }, > + { AUDIT_NFT_OP_OBJ_UNREGISTER, "nft_unregister_obj" }, > + { AUDIT_NFT_OP_OBJ_RESET, "nft_reset_obj" }, > + { AUDIT_NFT_OP_FLOWTABLE_REGISTER, "nft_register_flowtable" }, > + { AUDIT_NFT_OP_FLOWTABLE_UNREGISTER, "nft_unregister_flowtable" }, > + { AUDIT_NFT_OP_INVALID, "nft_invalid" }, > }; I still don't like the event format because it doesn't give complete subject information. However, I thought I'd comment on this string table. Usually it's sufficient to log the number and then have the string table in user space which looks it up during interpretation. -Steve > static int audit_match_perm(struct audit_context *ctx, int mask) > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > index 3558e76e2733..b9e7440cc87d 100644 > --- a/net/netfilter/nf_tables_api.c > +++ b/net/netfilter/nf_tables_api.c > @@ -12,6 +12,7 @@ > #include <linux/netlink.h> > #include <linux/vmalloc.h> > #include <linux/rhashtable.h> > +#include <linux/audit.h> > #include <linux/netfilter.h> > #include <linux/netfilter/nfnetlink.h> > #include <linux/netfilter/nf_tables.h> > @@ -693,6 +694,16 @@ static void nf_tables_table_notify(const struct > nft_ctx *ctx, int event) { > struct sk_buff *skb; > int err; > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0", > + ctx->table->name, ctx->table->handle); > + > + audit_log_nfcfg(buf, > + ctx->family, > + ctx->table->use, > + event == NFT_MSG_NEWTABLE ? > + AUDIT_NFT_OP_TABLE_REGISTER : > + AUDIT_NFT_OP_TABLE_UNREGISTER); > + kfree(buf); > > if (!ctx->report && > !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES)) > @@ -1428,6 +1439,17 @@ static void nf_tables_chain_notify(const struct > nft_ctx *ctx, int event) { > struct sk_buff *skb; > int err; > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;%s:%llu", > + ctx->table->name, ctx->table->handle, > + ctx->chain->name, ctx->chain->handle); > + > + audit_log_nfcfg(buf, > + ctx->family, > + ctx->chain->use, > + event == NFT_MSG_NEWCHAIN ? > + AUDIT_NFT_OP_CHAIN_REGISTER : > + AUDIT_NFT_OP_CHAIN_UNREGISTER); > + kfree(buf); > > if (!ctx->report && > !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES)) > @@ -2691,6 +2713,17 @@ static void nf_tables_rule_notify(const struct > nft_ctx *ctx, { > struct sk_buff *skb; > int err; > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;%s:%llu", > + ctx->table->name, ctx->table->handle, > + ctx->chain->name, ctx->chain->handle); > + > + audit_log_nfcfg(buf, > + ctx->family, > + rule->handle, > + event == NFT_MSG_NEWRULE ? > + AUDIT_NFT_OP_RULE_REGISTER : > + AUDIT_NFT_OP_RULE_UNREGISTER); > + kfree(buf); > > if (!ctx->report && > !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES)) > @@ -3693,6 +3726,17 @@ static void nf_tables_set_notify(const struct > nft_ctx *ctx, struct sk_buff *skb; > u32 portid = ctx->portid; > int err; > + char *buf = kasprintf(gfp_flags, "%s:%llu;%s:%llu", > + ctx->table->name, ctx->table->handle, > + set->name, set->handle); > + > + audit_log_nfcfg(buf, > + ctx->family, > + set->field_count, > + event == NFT_MSG_NEWSET ? > + AUDIT_NFT_OP_SET_REGISTER : > + AUDIT_NFT_OP_SET_UNREGISTER); > + kfree(buf); > > if (!ctx->report && > !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES)) > @@ -4809,6 +4853,17 @@ static void nf_tables_setelem_notify(const struct > nft_ctx *ctx, u32 portid = ctx->portid; > struct sk_buff *skb; > int err; > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;%s:%llu", > + ctx->table->name, ctx->table->handle, > + set->name, set->handle); > + > + audit_log_nfcfg(buf, > + ctx->family, > + set->handle, > + event == NFT_MSG_NEWSETELEM ? > + AUDIT_NFT_OP_SETELEM_REGISTER : > + AUDIT_NFT_OP_SETELEM_UNREGISTER); > + kfree(buf); > > if (!ctx->report && !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES)) > return; > @@ -5890,6 +5945,19 @@ static int nf_tables_dump_obj(struct sk_buff *skb, > struct netlink_callback *cb) obj->ops->type->type != filter->type) > goto cont; > > + if (reset) { > + char *buf = kasprintf(GFP_KERNEL, > + "%s:%llu;?:0", > + table->name, > + table->handle); > + > + audit_log_nfcfg(buf, > + family, > + obj->handle, > + AUDIT_NFT_OP_OBJ_RESET); > + kfree(buf); > + } > + > if (nf_tables_fill_obj_info(skb, net, NETLINK_CB(cb- >skb).portid, > cb->nlh->nlmsg_seq, > NFT_MSG_NEWOBJ, > @@ -6000,6 +6068,17 @@ static int nf_tables_getobj(struct net *net, struct > sock *nlsk, if (NFNL_MSG_TYPE(nlh->nlmsg_type) == NFT_MSG_GETOBJ_RESET) > reset = true; > > + if (reset) { > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0", > + table->name, table->handle); > + > + audit_log_nfcfg(buf, > + family, > + obj->handle, > + AUDIT_NFT_OP_OBJ_RESET); > + kfree(buf); > + } > + > err = nf_tables_fill_obj_info(skb2, net, NETLINK_CB(skb).portid, > nlh->nlmsg_seq, NFT_MSG_NEWOBJ, 0, > family, table, obj, reset); > @@ -6075,6 +6154,16 @@ void nft_obj_notify(struct net *net, const struct > nft_table *table, { > struct sk_buff *skb; > int err; > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0", > + table->name, table->handle); > + > + audit_log_nfcfg(buf, > + family, > + obj->handle, > + event == NFT_MSG_NEWOBJ ? > + AUDIT_NFT_OP_OBJ_REGISTER : > + AUDIT_NFT_OP_OBJ_UNREGISTER); > + kfree(buf); > > if (!report && > !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES)) > @@ -6701,6 +6790,17 @@ static void nf_tables_flowtable_notify(struct > nft_ctx *ctx, { > struct sk_buff *skb; > int err; > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;%s:%llu", > + flowtable->table->name, flowtable->table->handle, > + flowtable->name, flowtable->handle); > + > + audit_log_nfcfg(buf, > + ctx->family, > + flowtable->hooknum, > + event == NFT_MSG_NEWFLOWTABLE ? > + AUDIT_NFT_OP_FLOWTABLE_REGISTER : > + AUDIT_NFT_OP_FLOWTABLE_UNREGISTER); > + kfree(buf); > > if (ctx->report && > !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES)) > @@ -6822,6 +6922,9 @@ static void nf_tables_gen_notify(struct net *net, > struct sk_buff *skb, struct sk_buff *skb2; > int err; > > + audit_log_nfcfg("?:0;?:0", 0, net->nft.base_seq, > + AUDIT_NFT_OP_GEN_REGISTER); > + > if (nlmsg_report(nlh) && > !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES)) > return; -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2020-06-04 17:03 ` Steve Grubb @ 2020-06-04 17:57 ` Richard Guy Briggs 2020-06-04 18:51 ` Steve Grubb 0 siblings, 1 reply; 24+ messages in thread From: Richard Guy Briggs @ 2020-06-04 17:57 UTC (permalink / raw) To: Steve Grubb Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, eparis, tgraf On 2020-06-04 13:03, Steve Grubb wrote: > On Thursday, June 4, 2020 9:20:49 AM EDT Richard Guy Briggs wrote: > > iptables, ip6tables, arptables and ebtables table registration, > > replacement and unregistration configuration events are logged for the > > native (legacy) iptables setsockopt api, but not for the > > nftables netlink api which is used by the nft-variant of iptables in > > addition to nftables itself. > > > > Add calls to log the configuration actions in the nftables netlink api. > > > > This uses the same NETFILTER_CFG record format but overloads the table > > field. > > > > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : table=?:0;?:0 > > family=unspecified entries=2 op=nft_register_gen pid=396 > > subj=system_u:system_r:firewalld_t:s0 comm=firewalld ... > > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : > > table=firewalld:1;?:0 family=inet entries=0 op=nft_register_table pid=396 > > subj=system_u:system_r:firewalld_t:s0 comm=firewalld ... > > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : > > table=firewalld:1;filter_FORWARD:85 family=inet entries=8 > > op=nft_register_chain pid=396 subj=system_u:system_r:firewalld_t:s0 > > comm=firewalld ... > > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : > > table=firewalld:1;filter_FORWARD:85 family=inet entries=101 > > op=nft_register_rule pid=396 subj=system_u:system_r:firewalld_t:s0 > > comm=firewalld ... > > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : > > table=firewalld:1;__set0:87 family=inet entries=87 op=nft_register_setelem > > pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld ... > > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : > > table=firewalld:1;__set0:87 family=inet entries=0 op=nft_register_set > > pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > > > > For further information please see issue > > https://github.com/linux-audit/audit-kernel/issues/124 > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > --- > > Changelog: > > v3: > > - inline message type rather than table > > > > v2: > > - differentiate between xtables and nftables > > - add set, setelem, obj, flowtable, gen > > - use nentries field as appropriate per type > > - overload the "tables" field with table handle and chain/set/flowtable > > > > include/linux/audit.h | 18 ++++++++ > > kernel/auditsc.c | 24 ++++++++-- > > net/netfilter/nf_tables_api.c | 103 > > ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 142 > > insertions(+), 3 deletions(-) > > > > diff --git a/include/linux/audit.h b/include/linux/audit.h > > index 3fcd9ee49734..604ede630580 100644 > > --- a/include/linux/audit.h > > +++ b/include/linux/audit.h > > @@ -12,6 +12,7 @@ > > #include <linux/sched.h> > > #include <linux/ptrace.h> > > #include <uapi/linux/audit.h> > > +#include <uapi/linux/netfilter/nf_tables.h> > > > > #define AUDIT_INO_UNSET ((unsigned long)-1) > > #define AUDIT_DEV_UNSET ((dev_t)-1) > > @@ -98,6 +99,23 @@ enum audit_nfcfgop { > > AUDIT_XT_OP_REGISTER, > > AUDIT_XT_OP_REPLACE, > > AUDIT_XT_OP_UNREGISTER, > > + AUDIT_NFT_OP_TABLE_REGISTER, > > + AUDIT_NFT_OP_TABLE_UNREGISTER, > > + AUDIT_NFT_OP_CHAIN_REGISTER, > > + AUDIT_NFT_OP_CHAIN_UNREGISTER, > > + AUDIT_NFT_OP_RULE_REGISTER, > > + AUDIT_NFT_OP_RULE_UNREGISTER, > > + AUDIT_NFT_OP_SET_REGISTER, > > + AUDIT_NFT_OP_SET_UNREGISTER, > > + AUDIT_NFT_OP_SETELEM_REGISTER, > > + AUDIT_NFT_OP_SETELEM_UNREGISTER, > > + AUDIT_NFT_OP_GEN_REGISTER, > > + AUDIT_NFT_OP_OBJ_REGISTER, > > + AUDIT_NFT_OP_OBJ_UNREGISTER, > > + AUDIT_NFT_OP_OBJ_RESET, > > + AUDIT_NFT_OP_FLOWTABLE_REGISTER, > > + AUDIT_NFT_OP_FLOWTABLE_UNREGISTER, > > + AUDIT_NFT_OP_INVALID, > > }; > > > > extern int is_audit_feature_set(int which); > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > > index 468a23390457..3a9100e95fda 100644 > > --- a/kernel/auditsc.c > > +++ b/kernel/auditsc.c > > @@ -75,6 +75,7 @@ > > #include <linux/uaccess.h> > > #include <linux/fsnotify_backend.h> > > #include <uapi/linux/limits.h> > > +#include <uapi/linux/netfilter/nf_tables.h> > > > > #include "audit.h" > > > > @@ -136,9 +137,26 @@ struct audit_nfcfgop_tab { > > }; > > > > static const struct audit_nfcfgop_tab audit_nfcfgs[] = { > > - { AUDIT_XT_OP_REGISTER, "register" }, > > - { AUDIT_XT_OP_REPLACE, "replace" }, > > - { AUDIT_XT_OP_UNREGISTER, "unregister" }, > > + { AUDIT_XT_OP_REGISTER, "xt_register" > }, > > + { AUDIT_XT_OP_REPLACE, "xt_replace" }, > > + { AUDIT_XT_OP_UNREGISTER, "xt_unregister" }, > > + { AUDIT_NFT_OP_TABLE_REGISTER, "nft_register_table" > }, > > + { AUDIT_NFT_OP_TABLE_UNREGISTER, "nft_unregister_table" }, > > + { AUDIT_NFT_OP_CHAIN_REGISTER, "nft_register_chain" > }, > > + { AUDIT_NFT_OP_CHAIN_UNREGISTER, "nft_unregister_chain" }, > > + { AUDIT_NFT_OP_RULE_REGISTER, "nft_register_rule" > }, > > + { AUDIT_NFT_OP_RULE_UNREGISTER, "nft_unregister_rule" > }, > > + { AUDIT_NFT_OP_SET_REGISTER, "nft_register_set" > }, > > + { AUDIT_NFT_OP_SET_UNREGISTER, "nft_unregister_set" > }, > > + { AUDIT_NFT_OP_SETELEM_REGISTER, "nft_register_setelem" }, > > + { AUDIT_NFT_OP_SETELEM_UNREGISTER, "nft_unregister_setelem" }, > > + { AUDIT_NFT_OP_GEN_REGISTER, "nft_register_gen" }, > > + { AUDIT_NFT_OP_OBJ_REGISTER, "nft_register_obj" }, > > + { AUDIT_NFT_OP_OBJ_UNREGISTER, "nft_unregister_obj" }, > > + { AUDIT_NFT_OP_OBJ_RESET, "nft_reset_obj" }, > > + { AUDIT_NFT_OP_FLOWTABLE_REGISTER, "nft_register_flowtable" }, > > + { AUDIT_NFT_OP_FLOWTABLE_UNREGISTER, "nft_unregister_flowtable" }, > > + { AUDIT_NFT_OP_INVALID, "nft_invalid" > }, > > }; > > I still don't like the event format because it doesn't give complete subject > information. However, I thought I'd comment on this string table. Usually > it's sufficient to log the number and then have the string table in user space > which looks it up during interpretation. That is a good idea that would help reduce kernel cycles and netlink bandwidth, but the format was set in 2011 so it is a bit late to change that now: fbabf31e4d48 ("netfilter: create audit records for x_tables replaces") > -Steve > > > static int audit_match_perm(struct audit_context *ctx, int mask) > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > > index 3558e76e2733..b9e7440cc87d 100644 > > --- a/net/netfilter/nf_tables_api.c > > +++ b/net/netfilter/nf_tables_api.c > > @@ -12,6 +12,7 @@ > > #include <linux/netlink.h> > > #include <linux/vmalloc.h> > > #include <linux/rhashtable.h> > > +#include <linux/audit.h> > > #include <linux/netfilter.h> > > #include <linux/netfilter/nfnetlink.h> > > #include <linux/netfilter/nf_tables.h> > > @@ -693,6 +694,16 @@ static void nf_tables_table_notify(const struct > > nft_ctx *ctx, int event) { > > struct sk_buff *skb; > > int err; > > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0", > > + ctx->table->name, ctx->table->handle); > > + > > + audit_log_nfcfg(buf, > > + ctx->family, > > + ctx->table->use, > > + event == NFT_MSG_NEWTABLE ? > > + AUDIT_NFT_OP_TABLE_REGISTER : > > + AUDIT_NFT_OP_TABLE_UNREGISTER); > > + kfree(buf); > > > > if (!ctx->report && > > !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES)) > > @@ -1428,6 +1439,17 @@ static void nf_tables_chain_notify(const struct > > nft_ctx *ctx, int event) { > > struct sk_buff *skb; > > int err; > > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;%s:%llu", > > + ctx->table->name, ctx->table->handle, > > + ctx->chain->name, ctx->chain->handle); > > + > > + audit_log_nfcfg(buf, > > + ctx->family, > > + ctx->chain->use, > > + event == NFT_MSG_NEWCHAIN ? > > + AUDIT_NFT_OP_CHAIN_REGISTER : > > + AUDIT_NFT_OP_CHAIN_UNREGISTER); > > + kfree(buf); > > > > if (!ctx->report && > > !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES)) > > @@ -2691,6 +2713,17 @@ static void nf_tables_rule_notify(const struct > > nft_ctx *ctx, { > > struct sk_buff *skb; > > int err; > > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;%s:%llu", > > + ctx->table->name, ctx->table->handle, > > + ctx->chain->name, ctx->chain->handle); > > + > > + audit_log_nfcfg(buf, > > + ctx->family, > > + rule->handle, > > + event == NFT_MSG_NEWRULE ? > > + AUDIT_NFT_OP_RULE_REGISTER : > > + AUDIT_NFT_OP_RULE_UNREGISTER); > > + kfree(buf); > > > > if (!ctx->report && > > !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES)) > > @@ -3693,6 +3726,17 @@ static void nf_tables_set_notify(const struct > > nft_ctx *ctx, struct sk_buff *skb; > > u32 portid = ctx->portid; > > int err; > > + char *buf = kasprintf(gfp_flags, "%s:%llu;%s:%llu", > > + ctx->table->name, ctx->table->handle, > > + set->name, set->handle); > > + > > + audit_log_nfcfg(buf, > > + ctx->family, > > + set->field_count, > > + event == NFT_MSG_NEWSET ? > > + AUDIT_NFT_OP_SET_REGISTER : > > + AUDIT_NFT_OP_SET_UNREGISTER); > > + kfree(buf); > > > > if (!ctx->report && > > !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES)) > > @@ -4809,6 +4853,17 @@ static void nf_tables_setelem_notify(const struct > > nft_ctx *ctx, u32 portid = ctx->portid; > > struct sk_buff *skb; > > int err; > > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;%s:%llu", > > + ctx->table->name, ctx->table->handle, > > + set->name, set->handle); > > + > > + audit_log_nfcfg(buf, > > + ctx->family, > > + set->handle, > > + event == NFT_MSG_NEWSETELEM ? > > + AUDIT_NFT_OP_SETELEM_REGISTER : > > + AUDIT_NFT_OP_SETELEM_UNREGISTER); > > + kfree(buf); > > > > if (!ctx->report && !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES)) > > return; > > @@ -5890,6 +5945,19 @@ static int nf_tables_dump_obj(struct sk_buff *skb, > > struct netlink_callback *cb) obj->ops->type->type != filter->type) > > goto cont; > > > > + if (reset) { > > + char *buf = kasprintf(GFP_KERNEL, > > + "%s:%llu;?:0", > > + table->name, > > + table->handle); > > + > > + audit_log_nfcfg(buf, > > + family, > > + obj->handle, > > + AUDIT_NFT_OP_OBJ_RESET); > > + kfree(buf); > > + } > > + > > if (nf_tables_fill_obj_info(skb, net, NETLINK_CB(cb- > >skb).portid, > > cb->nlh->nlmsg_seq, > > NFT_MSG_NEWOBJ, > > @@ -6000,6 +6068,17 @@ static int nf_tables_getobj(struct net *net, struct > > sock *nlsk, if (NFNL_MSG_TYPE(nlh->nlmsg_type) == NFT_MSG_GETOBJ_RESET) > > reset = true; > > > > + if (reset) { > > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0", > > + table->name, table->handle); > > + > > + audit_log_nfcfg(buf, > > + family, > > + obj->handle, > > + AUDIT_NFT_OP_OBJ_RESET); > > + kfree(buf); > > + } > > + > > err = nf_tables_fill_obj_info(skb2, net, NETLINK_CB(skb).portid, > > nlh->nlmsg_seq, NFT_MSG_NEWOBJ, 0, > > family, table, obj, reset); > > @@ -6075,6 +6154,16 @@ void nft_obj_notify(struct net *net, const struct > > nft_table *table, { > > struct sk_buff *skb; > > int err; > > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0", > > + table->name, table->handle); > > + > > + audit_log_nfcfg(buf, > > + family, > > + obj->handle, > > + event == NFT_MSG_NEWOBJ ? > > + AUDIT_NFT_OP_OBJ_REGISTER : > > + AUDIT_NFT_OP_OBJ_UNREGISTER); > > + kfree(buf); > > > > if (!report && > > !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES)) > > @@ -6701,6 +6790,17 @@ static void nf_tables_flowtable_notify(struct > > nft_ctx *ctx, { > > struct sk_buff *skb; > > int err; > > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;%s:%llu", > > + flowtable->table->name, flowtable->table->handle, > > + flowtable->name, flowtable->handle); > > + > > + audit_log_nfcfg(buf, > > + ctx->family, > > + flowtable->hooknum, > > + event == NFT_MSG_NEWFLOWTABLE ? > > + AUDIT_NFT_OP_FLOWTABLE_REGISTER : > > + AUDIT_NFT_OP_FLOWTABLE_UNREGISTER); > > + kfree(buf); > > > > if (ctx->report && > > !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES)) > > @@ -6822,6 +6922,9 @@ static void nf_tables_gen_notify(struct net *net, > > struct sk_buff *skb, struct sk_buff *skb2; > > int err; > > > > + audit_log_nfcfg("?:0;?:0", 0, net->nft.base_seq, > > + AUDIT_NFT_OP_GEN_REGISTER); > > + > > if (nlmsg_report(nlh) && > > !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES)) > > return; > > > > - RGB -- Richard Guy Briggs <rgb@redhat.com> 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 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2020-06-04 17:57 ` Richard Guy Briggs @ 2020-06-04 18:51 ` Steve Grubb 0 siblings, 0 replies; 24+ messages in thread From: Steve Grubb @ 2020-06-04 18:51 UTC (permalink / raw) To: Richard Guy Briggs Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, eparis, tgraf On Thursday, June 4, 2020 1:57:56 PM EDT Richard Guy Briggs wrote: > > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > > > index 468a23390457..3a9100e95fda 100644 > > > --- a/kernel/auditsc.c > > > +++ b/kernel/auditsc.c > > > @@ -75,6 +75,7 @@ > > > #include <linux/uaccess.h> > > > #include <linux/fsnotify_backend.h> > > > #include <uapi/linux/limits.h> > > > +#include <uapi/linux/netfilter/nf_tables.h> > > > > > > #include "audit.h" > > > > > > @@ -136,9 +137,26 @@ struct audit_nfcfgop_tab { > > > }; > > > > > > static const struct audit_nfcfgop_tab audit_nfcfgs[] = { > > > - { AUDIT_XT_OP_REGISTER, "register" }, > > > - { AUDIT_XT_OP_REPLACE, "replace" }, > > > - { AUDIT_XT_OP_UNREGISTER, "unregister" }, > > > + { AUDIT_XT_OP_REGISTER, "xt_register" > > > > }, > > > > > + { AUDIT_XT_OP_REPLACE, "xt_replace" > > > }, + { AUDIT_XT_OP_UNREGISTER, "xt_unregister" > > > }, + { AUDIT_NFT_OP_TABLE_REGISTER, > > > "nft_register_table"> > > }, > > > > > + { AUDIT_NFT_OP_TABLE_UNREGISTER, "nft_unregister_table" > > > }, + { AUDIT_NFT_OP_CHAIN_REGISTER, > > > "nft_register_chain"> > > }, > > > > > + { AUDIT_NFT_OP_CHAIN_UNREGISTER, "nft_unregister_chain" > > > }, + { AUDIT_NFT_OP_RULE_REGISTER, > > > "nft_register_rule"> > > }, > > > > > + { AUDIT_NFT_OP_RULE_UNREGISTER, "nft_unregister_rule" > > > > }, > > > > > + { AUDIT_NFT_OP_SET_REGISTER, "nft_register_set" > > > > }, > > > > > + { AUDIT_NFT_OP_SET_UNREGISTER, "nft_unregister_set" > > > > }, > > > > > + { AUDIT_NFT_OP_SETELEM_REGISTER, "nft_register_setelem" > > > }, + { AUDIT_NFT_OP_SETELEM_UNREGISTER, > > > "nft_unregister_setelem" }, + { AUDIT_NFT_OP_GEN_REGISTER, > > > "nft_register_gen" }, + { > > > AUDIT_NFT_OP_OBJ_REGISTER, "nft_register_obj" }, + > > > { AUDIT_NFT_OP_OBJ_UNREGISTER, "nft_unregister_obj" > > > }, + { AUDIT_NFT_OP_OBJ_RESET, "nft_reset_obj" > > > }, + { AUDIT_NFT_OP_FLOWTABLE_REGISTER, > > > "nft_register_flowtable" }, + { > > > AUDIT_NFT_OP_FLOWTABLE_UNREGISTER, "nft_unregister_flowtable" }, + > > > { AUDIT_NFT_OP_INVALID, "nft_invalid" > > > > }, > > > > > }; > > > > I still don't like the event format because it doesn't give complete > > subject information. However, I thought I'd comment on this string > > table. Usually it's sufficient to log the number and then have the > > string table in user space which looks it up during interpretation. > > That is a good idea that would help reduce kernel cycles and netlink > bandwidth, but the format was set in 2011 so it is a bit late to change > that now: > fbabf31e4d48 ("netfilter: create audit records for x_tables > replaces") Nothing searches/interprets that field name. So, you can redefine it by renaming it. Or just go with what you have. My preference is push that to user space. But not a showstopper "as is". -Steve -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2020-06-04 13:20 [PATCH ghak124 v3] audit: log nftables configuration change events Richard Guy Briggs 2020-06-04 17:03 ` Steve Grubb @ 2020-06-24 0:34 ` Paul Moore 2020-06-24 10:03 ` Pablo Neira Ayuso [not found] ` <20210211151606.GX3158@orbyte.nwl.cc> 3 siblings, 0 replies; 24+ messages in thread From: Paul Moore @ 2020-06-24 0:34 UTC (permalink / raw) To: Richard Guy Briggs Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, Eric Paris, tgraf On Thu, Jun 4, 2020 at 9:21 AM Richard Guy Briggs <rgb@redhat.com> wrote: > > iptables, ip6tables, arptables and ebtables table registration, > replacement and unregistration configuration events are logged for the > native (legacy) iptables setsockopt api, but not for the > nftables netlink api which is used by the nft-variant of iptables in > addition to nftables itself. > > Add calls to log the configuration actions in the nftables netlink api. > > This uses the same NETFILTER_CFG record format but overloads the table > field. > > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : table=?:0;?:0 family=unspecified entries=2 op=nft_register_gen pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > ... > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : table=firewalld:1;?:0 family=inet entries=0 op=nft_register_table pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > ... > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;filter_FORWARD:85 family=inet entries=8 op=nft_register_chain pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > ... > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;filter_FORWARD:85 family=inet entries=101 op=nft_register_rule pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > ... > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;__set0:87 family=inet entries=87 op=nft_register_setelem pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > ... > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;__set0:87 family=inet entries=0 op=nft_register_set pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > > For further information please see issue > https://github.com/linux-audit/audit-kernel/issues/124 > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > --- > Changelog: > v3: > - inline message type rather than table > > v2: > - differentiate between xtables and nftables > - add set, setelem, obj, flowtable, gen > - use nentries field as appropriate per type > - overload the "tables" field with table handle and chain/set/flowtable > > include/linux/audit.h | 18 ++++++++ > kernel/auditsc.c | 24 ++++++++-- > net/netfilter/nf_tables_api.c | 103 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 142 insertions(+), 3 deletions(-) I'm not seeing any additional comments from the netfilter folks so I've gone ahead and merged this into audit/next. Thanks Richard. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2020-06-04 13:20 [PATCH ghak124 v3] audit: log nftables configuration change events Richard Guy Briggs 2020-06-04 17:03 ` Steve Grubb 2020-06-24 0:34 ` Paul Moore @ 2020-06-24 10:03 ` Pablo Neira Ayuso 2020-06-24 12:34 ` Richard Guy Briggs [not found] ` <20210211151606.GX3158@orbyte.nwl.cc> 3 siblings, 1 reply; 24+ messages in thread From: Pablo Neira Ayuso @ 2020-06-24 10:03 UTC (permalink / raw) To: Richard Guy Briggs Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, eparis, tgraf On Thu, Jun 04, 2020 at 09:20:49AM -0400, Richard Guy Briggs wrote: > iptables, ip6tables, arptables and ebtables table registration, > replacement and unregistration configuration events are logged for the > native (legacy) iptables setsockopt api, but not for the > nftables netlink api which is used by the nft-variant of iptables in > addition to nftables itself. > > Add calls to log the configuration actions in the nftables netlink api. > > This uses the same NETFILTER_CFG record format but overloads the table > field. > > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : table=?:0;?:0 family=unspecified entries=2 op=nft_register_gen pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > ... > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : table=firewalld:1;?:0 family=inet entries=0 op=nft_register_table pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > ... > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;filter_FORWARD:85 family=inet entries=8 op=nft_register_chain pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > ... > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;filter_FORWARD:85 family=inet entries=101 op=nft_register_rule pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > ... > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;__set0:87 family=inet entries=87 op=nft_register_setelem pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > ... > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;__set0:87 family=inet entries=0 op=nft_register_set pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > > For further information please see issue > https://github.com/linux-audit/audit-kernel/issues/124 > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > --- > Changelog: > v3: > - inline message type rather than table > > v2: > - differentiate between xtables and nftables > - add set, setelem, obj, flowtable, gen > - use nentries field as appropriate per type > - overload the "tables" field with table handle and chain/set/flowtable > > include/linux/audit.h | 18 ++++++++ > kernel/auditsc.c | 24 ++++++++-- > net/netfilter/nf_tables_api.c | 103 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 142 insertions(+), 3 deletions(-) > > diff --git a/include/linux/audit.h b/include/linux/audit.h > index 3fcd9ee49734..604ede630580 100644 > --- a/include/linux/audit.h > +++ b/include/linux/audit.h > @@ -12,6 +12,7 @@ > #include <linux/sched.h> > #include <linux/ptrace.h> > #include <uapi/linux/audit.h> > +#include <uapi/linux/netfilter/nf_tables.h> > > #define AUDIT_INO_UNSET ((unsigned long)-1) > #define AUDIT_DEV_UNSET ((dev_t)-1) > @@ -98,6 +99,23 @@ enum audit_nfcfgop { > AUDIT_XT_OP_REGISTER, > AUDIT_XT_OP_REPLACE, > AUDIT_XT_OP_UNREGISTER, > + AUDIT_NFT_OP_TABLE_REGISTER, > + AUDIT_NFT_OP_TABLE_UNREGISTER, > + AUDIT_NFT_OP_CHAIN_REGISTER, > + AUDIT_NFT_OP_CHAIN_UNREGISTER, > + AUDIT_NFT_OP_RULE_REGISTER, > + AUDIT_NFT_OP_RULE_UNREGISTER, > + AUDIT_NFT_OP_SET_REGISTER, > + AUDIT_NFT_OP_SET_UNREGISTER, > + AUDIT_NFT_OP_SETELEM_REGISTER, > + AUDIT_NFT_OP_SETELEM_UNREGISTER, > + AUDIT_NFT_OP_GEN_REGISTER, > + AUDIT_NFT_OP_OBJ_REGISTER, > + AUDIT_NFT_OP_OBJ_UNREGISTER, > + AUDIT_NFT_OP_OBJ_RESET, > + AUDIT_NFT_OP_FLOWTABLE_REGISTER, > + AUDIT_NFT_OP_FLOWTABLE_UNREGISTER, > + AUDIT_NFT_OP_INVALID, > }; > > extern int is_audit_feature_set(int which); > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index 468a23390457..3a9100e95fda 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -75,6 +75,7 @@ > #include <linux/uaccess.h> > #include <linux/fsnotify_backend.h> > #include <uapi/linux/limits.h> > +#include <uapi/linux/netfilter/nf_tables.h> > > #include "audit.h" > > @@ -136,9 +137,26 @@ struct audit_nfcfgop_tab { > }; > > static const struct audit_nfcfgop_tab audit_nfcfgs[] = { > - { AUDIT_XT_OP_REGISTER, "register" }, > - { AUDIT_XT_OP_REPLACE, "replace" }, > - { AUDIT_XT_OP_UNREGISTER, "unregister" }, > + { AUDIT_XT_OP_REGISTER, "xt_register" }, > + { AUDIT_XT_OP_REPLACE, "xt_replace" }, > + { AUDIT_XT_OP_UNREGISTER, "xt_unregister" }, > + { AUDIT_NFT_OP_TABLE_REGISTER, "nft_register_table" }, > + { AUDIT_NFT_OP_TABLE_UNREGISTER, "nft_unregister_table" }, > + { AUDIT_NFT_OP_CHAIN_REGISTER, "nft_register_chain" }, > + { AUDIT_NFT_OP_CHAIN_UNREGISTER, "nft_unregister_chain" }, > + { AUDIT_NFT_OP_RULE_REGISTER, "nft_register_rule" }, > + { AUDIT_NFT_OP_RULE_UNREGISTER, "nft_unregister_rule" }, > + { AUDIT_NFT_OP_SET_REGISTER, "nft_register_set" }, > + { AUDIT_NFT_OP_SET_UNREGISTER, "nft_unregister_set" }, > + { AUDIT_NFT_OP_SETELEM_REGISTER, "nft_register_setelem" }, > + { AUDIT_NFT_OP_SETELEM_UNREGISTER, "nft_unregister_setelem" }, > + { AUDIT_NFT_OP_GEN_REGISTER, "nft_register_gen" }, > + { AUDIT_NFT_OP_OBJ_REGISTER, "nft_register_obj" }, > + { AUDIT_NFT_OP_OBJ_UNREGISTER, "nft_unregister_obj" }, > + { AUDIT_NFT_OP_OBJ_RESET, "nft_reset_obj" }, > + { AUDIT_NFT_OP_FLOWTABLE_REGISTER, "nft_register_flowtable" }, > + { AUDIT_NFT_OP_FLOWTABLE_UNREGISTER, "nft_unregister_flowtable" }, > + { AUDIT_NFT_OP_INVALID, "nft_invalid" }, > }; > > static int audit_match_perm(struct audit_context *ctx, int mask) > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > index 3558e76e2733..b9e7440cc87d 100644 > --- a/net/netfilter/nf_tables_api.c > +++ b/net/netfilter/nf_tables_api.c > @@ -12,6 +12,7 @@ > #include <linux/netlink.h> > #include <linux/vmalloc.h> > #include <linux/rhashtable.h> > +#include <linux/audit.h> > #include <linux/netfilter.h> > #include <linux/netfilter/nfnetlink.h> > #include <linux/netfilter/nf_tables.h> > @@ -693,6 +694,16 @@ static void nf_tables_table_notify(const struct nft_ctx *ctx, int event) > { > struct sk_buff *skb; > int err; > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0", > + ctx->table->name, ctx->table->handle); > + > + audit_log_nfcfg(buf, > + ctx->family, > + ctx->table->use, > + event == NFT_MSG_NEWTABLE ? > + AUDIT_NFT_OP_TABLE_REGISTER : > + AUDIT_NFT_OP_TABLE_UNREGISTER); > + kfree(buf); As a follow up: Would you wrap this code into a function? nft_table_audit() Same thing for other pieces of code below. Thanks. -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2020-06-24 10:03 ` Pablo Neira Ayuso @ 2020-06-24 12:34 ` Richard Guy Briggs 2020-06-24 13:03 ` Pablo Neira Ayuso 0 siblings, 1 reply; 24+ messages in thread From: Richard Guy Briggs @ 2020-06-24 12:34 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, eparis, tgraf On 2020-06-24 12:03, Pablo Neira Ayuso wrote: > On Thu, Jun 04, 2020 at 09:20:49AM -0400, Richard Guy Briggs wrote: > > iptables, ip6tables, arptables and ebtables table registration, > > replacement and unregistration configuration events are logged for the > > native (legacy) iptables setsockopt api, but not for the > > nftables netlink api which is used by the nft-variant of iptables in > > addition to nftables itself. > > > > Add calls to log the configuration actions in the nftables netlink api. > > > > This uses the same NETFILTER_CFG record format but overloads the table > > field. > > > > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : table=?:0;?:0 family=unspecified entries=2 op=nft_register_gen pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > > ... > > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.878:162) : table=firewalld:1;?:0 family=inet entries=0 op=nft_register_table pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > > ... > > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;filter_FORWARD:85 family=inet entries=8 op=nft_register_chain pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > > ... > > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;filter_FORWARD:85 family=inet entries=101 op=nft_register_rule pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > > ... > > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;__set0:87 family=inet entries=87 op=nft_register_setelem pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > > ... > > type=NETFILTER_CFG msg=audit(2020-05-28 17:46:41.911:163) : table=firewalld:1;__set0:87 family=inet entries=0 op=nft_register_set pid=396 subj=system_u:system_r:firewalld_t:s0 comm=firewalld > > > > For further information please see issue > > https://github.com/linux-audit/audit-kernel/issues/124 > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > --- > > Changelog: > > v3: > > - inline message type rather than table > > > > v2: > > - differentiate between xtables and nftables > > - add set, setelem, obj, flowtable, gen > > - use nentries field as appropriate per type > > - overload the "tables" field with table handle and chain/set/flowtable > > > > include/linux/audit.h | 18 ++++++++ > > kernel/auditsc.c | 24 ++++++++-- > > net/netfilter/nf_tables_api.c | 103 ++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 142 insertions(+), 3 deletions(-) > > > > diff --git a/include/linux/audit.h b/include/linux/audit.h > > index 3fcd9ee49734..604ede630580 100644 > > --- a/include/linux/audit.h > > +++ b/include/linux/audit.h > > @@ -12,6 +12,7 @@ > > #include <linux/sched.h> > > #include <linux/ptrace.h> > > #include <uapi/linux/audit.h> > > +#include <uapi/linux/netfilter/nf_tables.h> > > > > #define AUDIT_INO_UNSET ((unsigned long)-1) > > #define AUDIT_DEV_UNSET ((dev_t)-1) > > @@ -98,6 +99,23 @@ enum audit_nfcfgop { > > AUDIT_XT_OP_REGISTER, > > AUDIT_XT_OP_REPLACE, > > AUDIT_XT_OP_UNREGISTER, > > + AUDIT_NFT_OP_TABLE_REGISTER, > > + AUDIT_NFT_OP_TABLE_UNREGISTER, > > + AUDIT_NFT_OP_CHAIN_REGISTER, > > + AUDIT_NFT_OP_CHAIN_UNREGISTER, > > + AUDIT_NFT_OP_RULE_REGISTER, > > + AUDIT_NFT_OP_RULE_UNREGISTER, > > + AUDIT_NFT_OP_SET_REGISTER, > > + AUDIT_NFT_OP_SET_UNREGISTER, > > + AUDIT_NFT_OP_SETELEM_REGISTER, > > + AUDIT_NFT_OP_SETELEM_UNREGISTER, > > + AUDIT_NFT_OP_GEN_REGISTER, > > + AUDIT_NFT_OP_OBJ_REGISTER, > > + AUDIT_NFT_OP_OBJ_UNREGISTER, > > + AUDIT_NFT_OP_OBJ_RESET, > > + AUDIT_NFT_OP_FLOWTABLE_REGISTER, > > + AUDIT_NFT_OP_FLOWTABLE_UNREGISTER, > > + AUDIT_NFT_OP_INVALID, > > }; > > > > extern int is_audit_feature_set(int which); > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > > index 468a23390457..3a9100e95fda 100644 > > --- a/kernel/auditsc.c > > +++ b/kernel/auditsc.c > > @@ -75,6 +75,7 @@ > > #include <linux/uaccess.h> > > #include <linux/fsnotify_backend.h> > > #include <uapi/linux/limits.h> > > +#include <uapi/linux/netfilter/nf_tables.h> > > > > #include "audit.h" > > > > @@ -136,9 +137,26 @@ struct audit_nfcfgop_tab { > > }; > > > > static const struct audit_nfcfgop_tab audit_nfcfgs[] = { > > - { AUDIT_XT_OP_REGISTER, "register" }, > > - { AUDIT_XT_OP_REPLACE, "replace" }, > > - { AUDIT_XT_OP_UNREGISTER, "unregister" }, > > + { AUDIT_XT_OP_REGISTER, "xt_register" }, > > + { AUDIT_XT_OP_REPLACE, "xt_replace" }, > > + { AUDIT_XT_OP_UNREGISTER, "xt_unregister" }, > > + { AUDIT_NFT_OP_TABLE_REGISTER, "nft_register_table" }, > > + { AUDIT_NFT_OP_TABLE_UNREGISTER, "nft_unregister_table" }, > > + { AUDIT_NFT_OP_CHAIN_REGISTER, "nft_register_chain" }, > > + { AUDIT_NFT_OP_CHAIN_UNREGISTER, "nft_unregister_chain" }, > > + { AUDIT_NFT_OP_RULE_REGISTER, "nft_register_rule" }, > > + { AUDIT_NFT_OP_RULE_UNREGISTER, "nft_unregister_rule" }, > > + { AUDIT_NFT_OP_SET_REGISTER, "nft_register_set" }, > > + { AUDIT_NFT_OP_SET_UNREGISTER, "nft_unregister_set" }, > > + { AUDIT_NFT_OP_SETELEM_REGISTER, "nft_register_setelem" }, > > + { AUDIT_NFT_OP_SETELEM_UNREGISTER, "nft_unregister_setelem" }, > > + { AUDIT_NFT_OP_GEN_REGISTER, "nft_register_gen" }, > > + { AUDIT_NFT_OP_OBJ_REGISTER, "nft_register_obj" }, > > + { AUDIT_NFT_OP_OBJ_UNREGISTER, "nft_unregister_obj" }, > > + { AUDIT_NFT_OP_OBJ_RESET, "nft_reset_obj" }, > > + { AUDIT_NFT_OP_FLOWTABLE_REGISTER, "nft_register_flowtable" }, > > + { AUDIT_NFT_OP_FLOWTABLE_UNREGISTER, "nft_unregister_flowtable" }, > > + { AUDIT_NFT_OP_INVALID, "nft_invalid" }, > > }; > > > > static int audit_match_perm(struct audit_context *ctx, int mask) > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > > index 3558e76e2733..b9e7440cc87d 100644 > > --- a/net/netfilter/nf_tables_api.c > > +++ b/net/netfilter/nf_tables_api.c > > @@ -12,6 +12,7 @@ > > #include <linux/netlink.h> > > #include <linux/vmalloc.h> > > #include <linux/rhashtable.h> > > +#include <linux/audit.h> > > #include <linux/netfilter.h> > > #include <linux/netfilter/nfnetlink.h> > > #include <linux/netfilter/nf_tables.h> > > @@ -693,6 +694,16 @@ static void nf_tables_table_notify(const struct nft_ctx *ctx, int event) > > { > > struct sk_buff *skb; > > int err; > > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0", > > + ctx->table->name, ctx->table->handle); > > + > > + audit_log_nfcfg(buf, > > + ctx->family, > > + ctx->table->use, > > + event == NFT_MSG_NEWTABLE ? > > + AUDIT_NFT_OP_TABLE_REGISTER : > > + AUDIT_NFT_OP_TABLE_UNREGISTER); > > + kfree(buf); > > As a follow up: Would you wrap this code into a function? > > nft_table_audit() > > Same thing for other pieces of code below. If I'm guessing right, you are asking for a supplementary follow-up cleanup patch to this one (or are you nacking this patch)? Also, I gather you would like to see the kasprintf and kfree hidden in nft_table_audit(), handing this function at least 8 parameters? This sounds pretty messy given the format of the table field. > Thanks. - RGB -- Richard Guy Briggs <rgb@redhat.com> 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 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2020-06-24 12:34 ` Richard Guy Briggs @ 2020-06-24 13:03 ` Pablo Neira Ayuso 2020-06-24 13:26 ` Richard Guy Briggs 0 siblings, 1 reply; 24+ messages in thread From: Pablo Neira Ayuso @ 2020-06-24 13:03 UTC (permalink / raw) To: Richard Guy Briggs Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, eparis, tgraf On Wed, Jun 24, 2020 at 08:34:23AM -0400, Richard Guy Briggs wrote: > On 2020-06-24 12:03, Pablo Neira Ayuso wrote: > > On Thu, Jun 04, 2020 at 09:20:49AM -0400, Richard Guy Briggs wrote: [...] > > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > > > index 3558e76e2733..b9e7440cc87d 100644 > > > --- a/net/netfilter/nf_tables_api.c > > > +++ b/net/netfilter/nf_tables_api.c > > > @@ -12,6 +12,7 @@ > > > #include <linux/netlink.h> > > > #include <linux/vmalloc.h> > > > #include <linux/rhashtable.h> > > > +#include <linux/audit.h> > > > #include <linux/netfilter.h> > > > #include <linux/netfilter/nfnetlink.h> > > > #include <linux/netfilter/nf_tables.h> > > > @@ -693,6 +694,16 @@ static void nf_tables_table_notify(const struct nft_ctx *ctx, int event) > > > { > > > struct sk_buff *skb; > > > int err; > > > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0", > > > + ctx->table->name, ctx->table->handle); > > > + > > > + audit_log_nfcfg(buf, > > > + ctx->family, > > > + ctx->table->use, > > > + event == NFT_MSG_NEWTABLE ? > > > + AUDIT_NFT_OP_TABLE_REGISTER : > > > + AUDIT_NFT_OP_TABLE_UNREGISTER); > > > + kfree(buf); > > > > As a follow up: Would you wrap this code into a function? > > > > nft_table_audit() > > > > Same thing for other pieces of code below. > > If I'm guessing right, you are asking for a supplementary follow-up > cleanup patch to this one (or are you nacking this patch)? No nack, it's just that I'd prefer to see this wrapped in a function. I think your patch is already in the audit tree. > Also, I gather you would like to see the kasprintf and kfree hidden in > nft_table_audit(), handing this function at least 8 parameters? This > sounds pretty messy given the format of the table field. I think you can pass ctx and the specific object, e.g. table, in most cases? There is also event and the gfp_flags. That counts 4 here, but maybe I'm overlooking something. Thanks. -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2020-06-24 13:03 ` Pablo Neira Ayuso @ 2020-06-24 13:26 ` Richard Guy Briggs 0 siblings, 0 replies; 24+ messages in thread From: Richard Guy Briggs @ 2020-06-24 13:26 UTC (permalink / raw) To: Pablo Neira Ayuso Cc: fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, eparis, tgraf On 2020-06-24 15:03, Pablo Neira Ayuso wrote: > On Wed, Jun 24, 2020 at 08:34:23AM -0400, Richard Guy Briggs wrote: > > On 2020-06-24 12:03, Pablo Neira Ayuso wrote: > > > On Thu, Jun 04, 2020 at 09:20:49AM -0400, Richard Guy Briggs wrote: > [...] > > > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > > > > index 3558e76e2733..b9e7440cc87d 100644 > > > > --- a/net/netfilter/nf_tables_api.c > > > > +++ b/net/netfilter/nf_tables_api.c > > > > @@ -12,6 +12,7 @@ > > > > #include <linux/netlink.h> > > > > #include <linux/vmalloc.h> > > > > #include <linux/rhashtable.h> > > > > +#include <linux/audit.h> > > > > #include <linux/netfilter.h> > > > > #include <linux/netfilter/nfnetlink.h> > > > > #include <linux/netfilter/nf_tables.h> > > > > @@ -693,6 +694,16 @@ static void nf_tables_table_notify(const struct nft_ctx *ctx, int event) > > > > { > > > > struct sk_buff *skb; > > > > int err; > > > > + char *buf = kasprintf(GFP_KERNEL, "%s:%llu;?:0", > > > > + ctx->table->name, ctx->table->handle); > > > > + > > > > + audit_log_nfcfg(buf, > > > > + ctx->family, > > > > + ctx->table->use, > > > > + event == NFT_MSG_NEWTABLE ? > > > > + AUDIT_NFT_OP_TABLE_REGISTER : > > > > + AUDIT_NFT_OP_TABLE_UNREGISTER); > > > > + kfree(buf); > > > > > > As a follow up: Would you wrap this code into a function? > > > > > > nft_table_audit() > > > > > > Same thing for other pieces of code below. > > > > If I'm guessing right, you are asking for a supplementary follow-up > > cleanup patch to this one (or are you nacking this patch)? > > No nack, it's just that I'd prefer to see this wrapped in a function. > I think your patch is already in the audit tree. > > > Also, I gather you would like to see the kasprintf and kfree hidden in > > nft_table_audit(), handing this function at least 8 parameters? This > > sounds pretty messy given the format of the table field. > > I think you can pass ctx and the specific object, e.g. table, in most > cases? There is also event and the gfp_flags. That counts 4 here, but > maybe I'm overlooking something. Since every event is sufficiently different, it isn't as simple as passing ctx, unfortunately, and the table field I've overloaded with 4 bits of information for tracking the chain as well, some of which are ? that would need an in-band representation (such as -1? that might already be valid). So 4 right there, family, nentries, event, gfp for 8. I did try in the first patch to make it just one call keyed on event, but there was enough variety of information available for each message type that it became necessary to break it out. > Thanks. - RGB -- Richard Guy Briggs <rgb@redhat.com> 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 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
[parent not found: <20210211151606.GX3158@orbyte.nwl.cc>]
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events [not found] ` <20210211151606.GX3158@orbyte.nwl.cc> @ 2021-02-11 16:29 ` Paul Moore 2021-02-11 20:26 ` Richard Guy Briggs 2021-02-11 21:02 ` Steve Grubb 0 siblings, 2 replies; 24+ messages in thread From: Paul Moore @ 2021-02-11 16:29 UTC (permalink / raw) To: Phil Sutter, Richard Guy Briggs, Linux-Audit Mailing List, LKML, netfilter-devel, Paul Moore, sgrubb, Ondrej Mosnacek, fw, twoerner, Eric Paris, tgraf On Thu, Feb 11, 2021 at 10:16 AM Phil Sutter <phil@nwl.cc> wrote: > Hi, > > On Thu, Jun 04, 2020 at 09:20:49AM -0400, Richard Guy Briggs wrote: > > iptables, ip6tables, arptables and ebtables table registration, > > replacement and unregistration configuration events are logged for the > > native (legacy) iptables setsockopt api, but not for the > > nftables netlink api which is used by the nft-variant of iptables in > > addition to nftables itself. > > > > Add calls to log the configuration actions in the nftables netlink api. > > As discussed offline already, these audit notifications are pretty hefty > performance-wise. In an internal report, 300% restore time of a ruleset > containing 70k set elements is measured. If you're going to reference offline/off-list discussions in a post to a public list, perhaps the original discussion shouldn't have been off-list ;) If you don't involve us in the discussion, we have to waste a lot of time getting caught up. > If I'm not mistaken, iptables emits a single audit log per table, ipset > doesn't support audit at all. So I wonder how much audit logging is > required at all (for certification or whatever reason). How much > granularity is desired? That's a question for the people who track these certification requirements, which is thankfully not me at the moment. Unless somebody else wants to speak up, Steve Grubb is probably the only person who tracks that sort of stuff and comments here. I believe the netfilter auditing was mostly a nice-to-have bit of functionality to help add to the completeness of the audit logs, but I could very easily be mistaken. Richard put together those patches, he can probably provide the background/motivation for the effort. > I personally would notify once per transaction. This is easy and quick. > Once per table or chain should be acceptable, as well. At the very > least, we should not have to notify once per each element. This is the > last resort of fast ruleset adjustments. If we lose it, people are > better off with ipset IMHO. > > Unlike nft monitor, auditd is not designed to be disabled "at will". So > turning it off for performance-critical workloads is no option. Patches are always welcome, but it might be wise to get to the bottom of the certification requirements first. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2021-02-11 16:29 ` Paul Moore @ 2021-02-11 20:26 ` Richard Guy Briggs 2021-02-12 20:48 ` Richard Guy Briggs [not found] ` <20210211220930.GC2766@breakpoint.cc> 2021-02-11 21:02 ` Steve Grubb 1 sibling, 2 replies; 24+ messages in thread From: Richard Guy Briggs @ 2021-02-11 20:26 UTC (permalink / raw) To: Paul Moore Cc: Phil Sutter, fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, Eric Paris, tgraf On 2021-02-11 11:29, Paul Moore wrote: > On Thu, Feb 11, 2021 at 10:16 AM Phil Sutter <phil@nwl.cc> wrote: > > Hi, > > > > On Thu, Jun 04, 2020 at 09:20:49AM -0400, Richard Guy Briggs wrote: > > > iptables, ip6tables, arptables and ebtables table registration, > > > replacement and unregistration configuration events are logged for the > > > native (legacy) iptables setsockopt api, but not for the > > > nftables netlink api which is used by the nft-variant of iptables in > > > addition to nftables itself. > > > > > > Add calls to log the configuration actions in the nftables netlink api. > > > > As discussed offline already, these audit notifications are pretty hefty > > performance-wise. In an internal report, 300% restore time of a ruleset > > containing 70k set elements is measured. > > If you're going to reference offline/off-list discussions in a post to > a public list, perhaps the original discussion shouldn't have been > off-list ;) If you don't involve us in the discussion, we have to > waste a lot of time getting caught up. Here's part of that discussion: https://bugzilla.redhat.com/show_bug.cgi?id=1918013 > > If I'm not mistaken, iptables emits a single audit log per table, ipset > > doesn't support audit at all. So I wonder how much audit logging is > > required at all (for certification or whatever reason). How much > > granularity is desired? > > That's a question for the people who track these certification > requirements, which is thankfully not me at the moment. Unless > somebody else wants to speak up, Steve Grubb is probably the only > person who tracks that sort of stuff and comments here. > > I believe the netfilter auditing was mostly a nice-to-have bit of > functionality to help add to the completeness of the audit logs, but I > could very easily be mistaken. Richard put together those patches, he > can probably provide the background/motivation for the effort. It was added because an audit test that normally produced records from iptables on one distro stopped producing any records on another. Investigation led to the fact that on the first it was using iptables-legacy API and on the other it was using iptables-nft API. > > I personally would notify once per transaction. This is easy and quick. This was the goal. iptables was atomic. nftables appears to no longer be so. If I have this wrong, please show how that works. > > Once per table or chain should be acceptable, as well. At the very > > least, we should not have to notify once per each element. This is the > > last resort of fast ruleset adjustments. If we lose it, people are > > better off with ipset IMHO. > > > > Unlike nft monitor, auditd is not designed to be disabled "at will". So > > turning it off for performance-critical workloads is no option. If it were to be disabled "at will" it would defeat the purpose of audit. Those records can already be filtered, or audit can be disabled, but let us look at rationalizing the current nftables records first. > Patches are always welcome, but it might be wise to get to the bottom > of the certification requirements first. > > -- > paul moore > www.paul-moore.com > > -- > Linux-audit mailing list > Linux-audit@redhat.com > https://www.redhat.com/mailman/listinfo/linux-audit - RGB -- Richard Guy Briggs <rgb@redhat.com> 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 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2021-02-11 20:26 ` Richard Guy Briggs @ 2021-02-12 20:48 ` Richard Guy Briggs [not found] ` <20210211220930.GC2766@breakpoint.cc> 1 sibling, 0 replies; 24+ messages in thread From: Richard Guy Briggs @ 2021-02-12 20:48 UTC (permalink / raw) To: Paul Moore Cc: Phil Sutter, fw, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, Eric Paris, tgraf On 2021-02-11 15:26, Richard Guy Briggs wrote: > On 2021-02-11 11:29, Paul Moore wrote: > > On Thu, Feb 11, 2021 at 10:16 AM Phil Sutter <phil@nwl.cc> wrote: > > > Hi, > > > > > > On Thu, Jun 04, 2020 at 09:20:49AM -0400, Richard Guy Briggs wrote: > > > > iptables, ip6tables, arptables and ebtables table registration, > > > > replacement and unregistration configuration events are logged for the > > > > native (legacy) iptables setsockopt api, but not for the > > > > nftables netlink api which is used by the nft-variant of iptables in > > > > addition to nftables itself. > > > > > > > > Add calls to log the configuration actions in the nftables netlink api. > > > > > > As discussed offline already, these audit notifications are pretty hefty > > > performance-wise. In an internal report, 300% restore time of a ruleset > > > containing 70k set elements is measured. > > > > If you're going to reference offline/off-list discussions in a post to > > a public list, perhaps the original discussion shouldn't have been > > off-list ;) If you don't involve us in the discussion, we have to > > waste a lot of time getting caught up. > > Here's part of that discussion: > https://bugzilla.redhat.com/show_bug.cgi?id=1918013 Here's the rest: https://bugzilla.redhat.com/show_bug.cgi?id=1921624 > > > If I'm not mistaken, iptables emits a single audit log per table, ipset > > > doesn't support audit at all. So I wonder how much audit logging is > > > required at all (for certification or whatever reason). How much > > > granularity is desired? > > > > That's a question for the people who track these certification > > requirements, which is thankfully not me at the moment. Unless > > somebody else wants to speak up, Steve Grubb is probably the only > > person who tracks that sort of stuff and comments here. > > > > I believe the netfilter auditing was mostly a nice-to-have bit of > > functionality to help add to the completeness of the audit logs, but I > > could very easily be mistaken. Richard put together those patches, he > > can probably provide the background/motivation for the effort. > > It was added because an audit test that normally produced records from > iptables on one distro stopped producing any records on another. > Investigation led to the fact that on the first it was using > iptables-legacy API and on the other it was using iptables-nft API. > > > > I personally would notify once per transaction. This is easy and quick. > > This was the goal. iptables was atomic. nftables appears to no longer > be so. If I have this wrong, please show how that works. > > > > Once per table or chain should be acceptable, as well. At the very > > > least, we should not have to notify once per each element. This is the > > > last resort of fast ruleset adjustments. If we lose it, people are > > > better off with ipset IMHO. > > > > > > Unlike nft monitor, auditd is not designed to be disabled "at will". So > > > turning it off for performance-critical workloads is no option. > > If it were to be disabled "at will" it would defeat the purpose of > audit. Those records can already be filtered, or audit can be disabled, > but let us look at rationalizing the current nftables records first. > > > Patches are always welcome, but it might be wise to get to the bottom > > of the certification requirements first. > > > > paul moore > > - RGB - RGB -- Richard Guy Briggs <rgb@redhat.com> 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 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
[parent not found: <20210211220930.GC2766@breakpoint.cc>]
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events [not found] ` <20210211220930.GC2766@breakpoint.cc> @ 2021-02-17 23:41 ` Richard Guy Briggs 2021-02-18 8:22 ` Florian Westphal 0 siblings, 1 reply; 24+ messages in thread From: Richard Guy Briggs @ 2021-02-17 23:41 UTC (permalink / raw) To: Florian Westphal Cc: Phil Sutter, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, Eric Paris, tgraf On 2021-02-11 23:09, Florian Westphal wrote: > Richard Guy Briggs <rgb@redhat.com> wrote: > > > > I personally would notify once per transaction. This is easy and quick. > > > > This was the goal. iptables was atomic. nftables appears to no longer > > be so. If I have this wrong, please show how that works. > > nftables transactions are atomic, either the entire batch takes effect or not > at all. > > The audit_log_nfcfg() calls got added to the the nft monitor infra which > is designed to allow userspace to follow the entire content of the > transaction log. > > So, if its just a 'something was changed' event that is needed all of > them can be removed. ATM the audit_log_nfcfg() looks like this: > > /* step 3. Start new generation, rules_gen_X now in use. */ > net->nft.gencursor = nft_gencursor_next(net); > > list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) { > switch (trans->msg_type) { > case NFT_MSG_NEWTABLE: > audit_log_nfcfg(); > ... > case NFT_MSG_... > audit_log_nfcfg(); > .. > } > > which gives an audit_log for every single change in the batch. > > So, if just a summary is needed a single audit_log_nfcfg() > after 'step 3' and outside of the list_for_each_entry_safe() is all > that is needed. Ok, so it should not matter if it is before or after that list_for_each_entry_safe(), which could be used to collect that summary. > If a summary is wanted as well one could fe. count the number of > transaction types in the batch, e.g. table adds, chain adds, rule > adds etc. and then log a summary count instead. The current fields are "table", "family", "entries", "op". Could one batch change more than one table? (I think it could?) It appears it can change more than one family. "family" is currently a single integer, so that might need to be changed to a list, or something to indicate multi-family. A batch can certainly change more than one chain, but that was already a bonus. "entries" would be the obvious place for the summary count. Listing all the ops seems a bit onerous. Is there a hierarchy to the ops and if so, are they in that order in a batch or in nf_tables_commit()? It seems I'd need to filter out the NFT_MSG_GET_* ops. - RGB -- Richard Guy Briggs <rgb@redhat.com> 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 -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2021-02-17 23:41 ` Richard Guy Briggs @ 2021-02-18 8:22 ` Florian Westphal 2021-02-18 12:42 ` Richard Guy Briggs 0 siblings, 1 reply; 24+ messages in thread From: Florian Westphal @ 2021-02-18 8:22 UTC (permalink / raw) To: Richard Guy Briggs Cc: Phil Sutter, Florian Westphal, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, Eric Paris, tgraf Richard Guy Briggs <rgb@redhat.com> wrote: > On 2021-02-11 23:09, Florian Westphal wrote: > > So, if just a summary is needed a single audit_log_nfcfg() > > after 'step 3' and outside of the list_for_each_entry_safe() is all > > that is needed. > > Ok, so it should not matter if it is before or after that > list_for_each_entry_safe(), which could be used to collect that summary. Right, it won't matter. > > If a summary is wanted as well one could fe. count the number of > > transaction types in the batch, e.g. table adds, chain adds, rule > > adds etc. and then log a summary count instead. > > The current fields are "table", "family", "entries", "op". > > Could one batch change more than one table? (I think it could?) Yes. > It appears it can change more than one family. > "family" is currently a single integer, so that might need to be changed > to a list, or something to indicate multi-family. Yes, it can also affect different families. > Listing all the ops seems a bit onerous. Is there a hierarchy to the > ops and if so, are they in that order in a batch or in nf_tables_commit()? No. There is a hierarchy, e.g. you can't add a chain without first adding a table, BUT in case the table was already created by an earlier transaction it can also be stand-alone. > It seems I'd need to filter out the NFT_MSG_GET_* ops. No need, the GET ops do not cause changes and will not trigger a generation id change. -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2021-02-18 8:22 ` Florian Westphal @ 2021-02-18 12:42 ` Richard Guy Briggs 2021-02-18 12:52 ` Florian Westphal 0 siblings, 1 reply; 24+ messages in thread From: Richard Guy Briggs @ 2021-02-18 12:42 UTC (permalink / raw) To: Florian Westphal Cc: Phil Sutter, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, Eric Paris, tgraf On 2021-02-18 09:22, Florian Westphal wrote: > Richard Guy Briggs <rgb@redhat.com> wrote: > > On 2021-02-11 23:09, Florian Westphal wrote: > > > So, if just a summary is needed a single audit_log_nfcfg() > > > after 'step 3' and outside of the list_for_each_entry_safe() is all > > > that is needed. > > > > Ok, so it should not matter if it is before or after that > > list_for_each_entry_safe(), which could be used to collect that summary. > > Right, it won't matter. > > > > If a summary is wanted as well one could fe. count the number of > > > transaction types in the batch, e.g. table adds, chain adds, rule > > > adds etc. and then log a summary count instead. > > > > The current fields are "table", "family", "entries", "op". > > > > Could one batch change more than one table? (I think it could?) > > Yes. Ok, listing all tables involved in one commit with deduplication will be a bit of a nuisance. > > It appears it can change more than one family. > > "family" is currently a single integer, so that might need to be changed > > to a list, or something to indicate multi-family. > > Yes, it can also affect different families. > > > Listing all the ops seems a bit onerous. Is there a hierarchy to the > > ops and if so, are they in that order in a batch or in nf_tables_commit()? > > No. There is a hierarchy, e.g. you can't add a chain without first > adding a table, BUT in case the table was already created by an earlier > transaction it can also be stand-alone. Ok, so there could be a stand-alone chain mod with one table, then a table add of a different one with a "higher ranking" op... > > It seems I'd need to filter out the NFT_MSG_GET_* ops. > > No need, the GET ops do not cause changes and will not trigger a > generation id change. Ah, so it could trigger on generation change. Would GET ops be included in any other change, such that it would be desirable to filter them out to reduce noise in that single log line if it is attempted to list all the change ops? It almost sounds like it would be better to do one audit log line for each table for each family, and possibly for each op to avoid the need to change userspace. This would already be a significant improvement picking the highest ranking op. - RGB -- Richard Guy Briggs <rgb@redhat.com> 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 -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2021-02-18 12:42 ` Richard Guy Briggs @ 2021-02-18 12:52 ` Florian Westphal 2021-02-18 13:28 ` Richard Guy Briggs 2021-02-18 21:20 ` Richard Guy Briggs 0 siblings, 2 replies; 24+ messages in thread From: Florian Westphal @ 2021-02-18 12:52 UTC (permalink / raw) To: Richard Guy Briggs Cc: Phil Sutter, Florian Westphal, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, Eric Paris, tgraf Richard Guy Briggs <rgb@redhat.com> wrote: > On 2021-02-18 09:22, Florian Westphal wrote: > > No. There is a hierarchy, e.g. you can't add a chain without first > > adding a table, BUT in case the table was already created by an earlier > > transaction it can also be stand-alone. > > Ok, so there could be a stand-alone chain mod with one table, then a > table add of a different one with a "higher ranking" op... Yes, that can happen. > > > It seems I'd need to filter out the NFT_MSG_GET_* ops. > > > > No need, the GET ops do not cause changes and will not trigger a > > generation id change. > > Ah, so it could trigger on generation change. Would GET ops be included > in any other change No, GET ops are standalone, they cannot be part of a transaction. If you look at static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = { array in nf_tables_api.c, then those ops with a '.call_batch' can appear in transaction (i.e., can cause modification). The other ones (.call_rcu) are read-only. If they appear in a batch tehy will be ignored, if the batch consists of such non-modifying ops only then nf_tables_commit() returns early because the transaction list is empty (nothing to do/change). > such that it would be desirable to filter them out > to reduce noise in that single log line if it is attempted to list all > the change ops? It almost sounds like it would be better to do one > audit log line for each table for each family, and possibly for each op > to avoid the need to change userspace. This would already be a > significant improvement picking the highest ranking op. I think i understand what you'd like to do. Yes, that would reduce the log output a lot. -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2021-02-18 12:52 ` Florian Westphal @ 2021-02-18 13:28 ` Richard Guy Briggs 2021-02-18 13:41 ` Florian Westphal 2021-02-18 21:20 ` Richard Guy Briggs 1 sibling, 1 reply; 24+ messages in thread From: Richard Guy Briggs @ 2021-02-18 13:28 UTC (permalink / raw) To: Florian Westphal Cc: Phil Sutter, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, Eric Paris, tgraf On 2021-02-18 13:52, Florian Westphal wrote: > Richard Guy Briggs <rgb@redhat.com> wrote: > > On 2021-02-18 09:22, Florian Westphal wrote: > > > No. There is a hierarchy, e.g. you can't add a chain without first > > > adding a table, BUT in case the table was already created by an earlier > > > transaction it can also be stand-alone. > > > > Ok, so there could be a stand-alone chain mod with one table, then a > > table add of a different one with a "higher ranking" op... > > Yes, that can happen. Ok, can I get one more clarification on this "hierarchy"? Is it roughly in the order they appear in nf_tables_commit() after step 3? It appears it might be mostly already. If it isn't already, would it be reasonable to re-order them? Would you suggest a different order? (snip GET bits, that's now clear, thank you) > > such that it would be desirable to filter them out > > to reduce noise in that single log line if it is attempted to list all > > the change ops? It almost sounds like it would be better to do one > > audit log line for each table for each family, and possibly for each op > > to avoid the need to change userspace. This would already be a > > significant improvement picking the highest ranking op. > > I think i understand what you'd like to do. Yes, that would reduce > the log output a lot. Would the generation change id be useful outside the kernel? What exactly does it look like? I don't quite understand the genmask purpose. - RGB -- Richard Guy Briggs <rgb@redhat.com> 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 -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2021-02-18 13:28 ` Richard Guy Briggs @ 2021-02-18 13:41 ` Florian Westphal 0 siblings, 0 replies; 24+ messages in thread From: Florian Westphal @ 2021-02-18 13:41 UTC (permalink / raw) To: Richard Guy Briggs Cc: Phil Sutter, Florian Westphal, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, Eric Paris, tgraf Richard Guy Briggs <rgb@redhat.com> wrote: > Ok, can I get one more clarification on this "hierarchy"? Is it roughly > in the order they appear in nf_tables_commit() after step 3? It appears > it might be mostly already. If it isn't already, would it be reasonable > to re-order them? Would you suggest a different order? For audit purposes I think enum nf_tables_msg_types is already in the most relevant order, the lower numbers being more imporant. So e.g. NEWTABLE would be more interesting than DELRULE, if both are in same batch. > > > such that it would be desirable to filter them out > > > to reduce noise in that single log line if it is attempted to list all > > > the change ops? It almost sounds like it would be better to do one > > > audit log line for each table for each family, and possibly for each op > > > to avoid the need to change userspace. This would already be a > > > significant improvement picking the highest ranking op. > > > > I think i understand what you'd like to do. Yes, that would reduce > > the log output a lot. > > Would the generation change id be useful outside the kernel? Yes, we already announce it to interested parties via nfnetlink. > What > exactly does it look like? Its just a u64 counter that gets incremented whenever there is a change. > I don't quite understand the genmask purpose. Thats an implementation detail only. When we process a transaction, changes to the ruleset are being made but they should not have any effect until the entire transaction is processed. So there are two 'generations' at any time: 1. The active ruleset 2. The future ruleset 2) is what is being changed/modified. When the transaction completes, then the future ruleset becomes the active ruleset. If the transaction has to be aborted, the pending changes are reverted and the genid/genmasks are not changed. -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2021-02-18 12:52 ` Florian Westphal 2021-02-18 13:28 ` Richard Guy Briggs @ 2021-02-18 21:20 ` Richard Guy Briggs 2021-02-18 22:42 ` Florian Westphal 1 sibling, 1 reply; 24+ messages in thread From: Richard Guy Briggs @ 2021-02-18 21:20 UTC (permalink / raw) To: Florian Westphal Cc: Phil Sutter, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, Eric Paris, tgraf On 2021-02-18 13:52, Florian Westphal wrote: > Richard Guy Briggs <rgb@redhat.com> wrote: > > On 2021-02-18 09:22, Florian Westphal wrote: > > > > It seems I'd need to filter out the NFT_MSG_GET_* ops. > > > > > > No need, the GET ops do not cause changes and will not trigger a > > > generation id change. > > > > Ah, so it could trigger on generation change. Would GET ops be included > > in any other change > > No, GET ops are standalone, they cannot be part of a transaction. > If you look at > > static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = { > > array in nf_tables_api.c, then those ops with a '.call_batch' can > appear in transaction (i.e., can cause modification). > > The other ones (.call_rcu) are read-only. > > If they appear in a batch tehy will be ignored, if the batch consists of > such non-modifying ops only then nf_tables_commit() returns early > because the transaction list is empty (nothing to do/change). Ok, one little inconvenient question: what about GETOBJ_RESET? That looks like a hybrid that modifies kernel table counters and reports synchronously. That could be a special case call in nf_tables_dump_obj() and nf_tables_getobj(). Will that cause a storm per commit? > > such that it would be desirable to filter them out > > to reduce noise in that single log line if it is attempted to list all > > the change ops? It almost sounds like it would be better to do one > > audit log line for each table for each family, and possibly for each op > > to avoid the need to change userspace. This would already be a > > significant improvement picking the highest ranking op. > > I think i understand what you'd like to do. Yes, that would reduce > the log output a lot. Coded, testing... - RGB -- Richard Guy Briggs <rgb@redhat.com> 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 -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2021-02-18 21:20 ` Richard Guy Briggs @ 2021-02-18 22:42 ` Florian Westphal 2021-02-19 6:26 ` Richard Guy Briggs 0 siblings, 1 reply; 24+ messages in thread From: Florian Westphal @ 2021-02-18 22:42 UTC (permalink / raw) To: Richard Guy Briggs Cc: Phil Sutter, Florian Westphal, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, Eric Paris, tgraf Richard Guy Briggs <rgb@redhat.com> wrote: > > If they appear in a batch tehy will be ignored, if the batch consists of > > such non-modifying ops only then nf_tables_commit() returns early > > because the transaction list is empty (nothing to do/change). > > Ok, one little inconvenient question: what about GETOBJ_RESET? That > looks like a hybrid that modifies kernel table counters and reports > synchronously. That could be a special case call in > nf_tables_dump_obj() and nf_tables_getobj(). Will that cause a storm > per commit? No, since they can't be part of a commit (they don't implement the 'call_batch' function). I'm not sure GETOBJ_RESET should be reported in the first place: RESET only affects expr internal state, and that state changes all the time anyway in response to network traffic. -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2021-02-18 22:42 ` Florian Westphal @ 2021-02-19 6:26 ` Richard Guy Briggs 2021-02-19 19:25 ` Richard Guy Briggs 0 siblings, 1 reply; 24+ messages in thread From: Richard Guy Briggs @ 2021-02-19 6:26 UTC (permalink / raw) To: Florian Westphal Cc: Phil Sutter, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, Eric Paris, tgraf On 2021-02-18 23:42, Florian Westphal wrote: > Richard Guy Briggs <rgb@redhat.com> wrote: > > > If they appear in a batch tehy will be ignored, if the batch consists of > > > such non-modifying ops only then nf_tables_commit() returns early > > > because the transaction list is empty (nothing to do/change). > > > > Ok, one little inconvenient question: what about GETOBJ_RESET? That > > looks like a hybrid that modifies kernel table counters and reports > > synchronously. That could be a special case call in > > nf_tables_dump_obj() and nf_tables_getobj(). Will that cause a storm > > per commit? > > No, since they can't be part of a commit (they don't implement the > 'call_batch' function). Ok, good, so they should be safe (but still needs the gfp param to audit_log_nfcfg() for atomic alloc in that obj reset callback). > I'm not sure GETOBJ_RESET should be reported in the first place: > RESET only affects expr internal state, and that state changes all the time > anyway in response to network traffic. We report audit lost messages reset as a config change since it affects the view that an admin has about a system. An unaccounted for reset could mislead an administrator into thinking things are alright when some messages were lost and there was nothing to show for it. I could see similar situations with network entity counters. - RGB -- Richard Guy Briggs <rgb@redhat.com> 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 -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2021-02-19 6:26 ` Richard Guy Briggs @ 2021-02-19 19:25 ` Richard Guy Briggs 0 siblings, 0 replies; 24+ messages in thread From: Richard Guy Briggs @ 2021-02-19 19:25 UTC (permalink / raw) To: Florian Westphal Cc: Phil Sutter, LKML, Linux-Audit Mailing List, netfilter-devel, twoerner, Eric Paris, tgraf On 2021-02-19 01:26, Richard Guy Briggs wrote: > On 2021-02-18 23:42, Florian Westphal wrote: > > Richard Guy Briggs <rgb@redhat.com> wrote: > > > > If they appear in a batch tehy will be ignored, if the batch consists of > > > > such non-modifying ops only then nf_tables_commit() returns early > > > > because the transaction list is empty (nothing to do/change). > > > > > > Ok, one little inconvenient question: what about GETOBJ_RESET? That > > > looks like a hybrid that modifies kernel table counters and reports > > > synchronously. That could be a special case call in > > > nf_tables_dump_obj() and nf_tables_getobj(). Will that cause a storm > > > per commit? > > > > No, since they can't be part of a commit (they don't implement the > > 'call_batch' function). > > Ok, good, so they should be safe (but still needs the gfp param to > audit_log_nfcfg() for atomic alloc in that obj reset callback). I just noticed that nft_quota_obj_eval() misses logging NFT_MSG_NEWOBJ in nf_tables_commit(), so that looks like it should be added. > - RGB - RGB -- Richard Guy Briggs <rgb@redhat.com> 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 -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events 2021-02-11 16:29 ` Paul Moore 2021-02-11 20:26 ` Richard Guy Briggs @ 2021-02-11 21:02 ` Steve Grubb [not found] ` <20210212121112.GA3158@orbyte.nwl.cc> 1 sibling, 1 reply; 24+ messages in thread From: Steve Grubb @ 2021-02-11 21:02 UTC (permalink / raw) To: Phil Sutter, Richard Guy Briggs, Linux-Audit Mailing List, LKML, netfilter-devel, Paul Moore, Ondrej Mosnacek, fw, twoerner, Eric Paris, tgraf, Paul Moore On Thursday, February 11, 2021 11:29:34 AM EST Paul Moore wrote: > > If I'm not mistaken, iptables emits a single audit log per table, ipset > > doesn't support audit at all. So I wonder how much audit logging is > > required at all (for certification or whatever reason). How much > > granularity is desired? <snip> > I believe the netfilter auditing was mostly a nice-to-have bit of > functionality to help add to the completeness of the audit logs, but I > could very easily be mistaken. Richard put together those patches, he > can probably provide the background/motivation for the effort. There are certifications which levy requirements on information flow control. The firewall can decide if information should flow or be blocked. Information flow decisions need to be auditable - which we have with the audit target. That then swings in requirements on the configuration of the information flow policy. The requirements state a need to audit any management activity - meaning the creation, modification, and/or deletion of a "firewall ruleset". Because it talks constantly about a ruleset and then individual rules, I suspect only 1 summary event is needed to say something happened, who did it, and the outcome. This would be in line with how selinux is treated: we have 1 summary event for loading/modifying/unloading selinux policy. Hope this helps... -Steve -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
[parent not found: <20210212121112.GA3158@orbyte.nwl.cc>]
* Re: [PATCH ghak124 v3] audit: log nftables configuration change events [not found] ` <20210212121112.GA3158@orbyte.nwl.cc> @ 2021-02-12 20:54 ` Richard Guy Briggs 0 siblings, 0 replies; 24+ messages in thread From: Richard Guy Briggs @ 2021-02-12 20:54 UTC (permalink / raw) To: Phil Sutter, Steve Grubb, Linux-Audit Mailing List, LKML, netfilter-devel, Paul Moore, Ondrej Mosnacek, fw, twoerner, Eric Paris, tgraf On 2021-02-12 13:11, Phil Sutter wrote: > Hi, > > On Thu, Feb 11, 2021 at 04:02:55PM -0500, Steve Grubb wrote: > > On Thursday, February 11, 2021 11:29:34 AM EST Paul Moore wrote: > > > > If I'm not mistaken, iptables emits a single audit log per table, ipset > > > > doesn't support audit at all. So I wonder how much audit logging is > > > > required at all (for certification or whatever reason). How much > > > > granularity is desired? > > > > <snip> > > > > > I believe the netfilter auditing was mostly a nice-to-have bit of > > > functionality to help add to the completeness of the audit logs, but I > > > could very easily be mistaken. Richard put together those patches, he > > > can probably provide the background/motivation for the effort. > > > > There are certifications which levy requirements on information flow control. > > The firewall can decide if information should flow or be blocked. Information > > flow decisions need to be auditable - which we have with the audit target. > > In nftables, this is realized via 'log level audit' statement. > Functionality should by all means be identical to that of xtables' AUDIT > target. > > > That then swings in requirements on the configuration of the information flow > > policy. > > > > The requirements state a need to audit any management activity - meaning the > > creation, modification, and/or deletion of a "firewall ruleset". Because it > > talks constantly about a ruleset and then individual rules, I suspect only 1 > > summary event is needed to say something happened, who did it, and the > > outcome. This would be in line with how selinux is treated: we have 1 summary > > event for loading/modifying/unloading selinux policy. > > So the central element are firewall rules for audit purposes and > NETFILTER_CFG notifications merely serve asserting changes to those > rules are noticed by the auditing system. Looking at xtables again, this > seems coherent: Any change causes the whole table blob to be replaced > (while others stay in place). So table replace/create is the most common > place for a change notification. In nftables, the most common one is > generation dump - all tables are treated as elements of the same > ruleset, not individually like in xtables. > > Richard, assuming the above is correct, are you fine with reducing > nftables auditing to a single notification per transaction then? I guess > Florian sufficiently illustrated how this would be implemented. Yes, that should be possible. > > Hope this helps... > > It does, thanks a lot for the information! > > Thanks, Phil - RGB -- Richard Guy Briggs <rgb@redhat.com> 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 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2021-02-19 19:25 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-04 13:20 [PATCH ghak124 v3] audit: log nftables configuration change events Richard Guy Briggs 2020-06-04 17:03 ` Steve Grubb 2020-06-04 17:57 ` Richard Guy Briggs 2020-06-04 18:51 ` Steve Grubb 2020-06-24 0:34 ` Paul Moore 2020-06-24 10:03 ` Pablo Neira Ayuso 2020-06-24 12:34 ` Richard Guy Briggs 2020-06-24 13:03 ` Pablo Neira Ayuso 2020-06-24 13:26 ` Richard Guy Briggs [not found] ` <20210211151606.GX3158@orbyte.nwl.cc> 2021-02-11 16:29 ` Paul Moore 2021-02-11 20:26 ` Richard Guy Briggs 2021-02-12 20:48 ` Richard Guy Briggs [not found] ` <20210211220930.GC2766@breakpoint.cc> 2021-02-17 23:41 ` Richard Guy Briggs 2021-02-18 8:22 ` Florian Westphal 2021-02-18 12:42 ` Richard Guy Briggs 2021-02-18 12:52 ` Florian Westphal 2021-02-18 13:28 ` Richard Guy Briggs 2021-02-18 13:41 ` Florian Westphal 2021-02-18 21:20 ` Richard Guy Briggs 2021-02-18 22:42 ` Florian Westphal 2021-02-19 6:26 ` Richard Guy Briggs 2021-02-19 19:25 ` Richard Guy Briggs 2021-02-11 21:02 ` Steve Grubb [not found] ` <20210212121112.GA3158@orbyte.nwl.cc> 2021-02-12 20:54 ` Richard Guy Briggs
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).