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 <[email protected]>
---
Changelog:
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 | 52 +++++++++++++++++++++++++
kernel/auditsc.c | 24 ++++++++++--
net/netfilter/nf_tables_api.c | 89 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 162 insertions(+), 3 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 3fcd9ee49734..d79866a38505 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,57 @@ 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,
+};
+
+struct audit_nftcfgop_tab {
+ enum nf_tables_msg_types nftop;
+ enum audit_nfcfgop op;
+};
+
+static const struct audit_nftcfgop_tab audit_nftcfgs[] = {
+ { NFT_MSG_NEWTABLE, AUDIT_NFT_OP_TABLE_REGISTER },
+ { NFT_MSG_GETTABLE, AUDIT_NFT_OP_INVALID },
+ { NFT_MSG_DELTABLE, AUDIT_NFT_OP_TABLE_UNREGISTER },
+ { NFT_MSG_NEWCHAIN, AUDIT_NFT_OP_CHAIN_REGISTER },
+ { NFT_MSG_GETCHAIN, AUDIT_NFT_OP_INVALID },
+ { NFT_MSG_DELCHAIN, AUDIT_NFT_OP_CHAIN_UNREGISTER },
+ { NFT_MSG_NEWRULE, AUDIT_NFT_OP_RULE_REGISTER },
+ { NFT_MSG_GETRULE, AUDIT_NFT_OP_INVALID },
+ { NFT_MSG_DELRULE, AUDIT_NFT_OP_RULE_UNREGISTER },
+ { NFT_MSG_NEWSET, AUDIT_NFT_OP_SET_REGISTER },
+ { NFT_MSG_GETSET, AUDIT_NFT_OP_INVALID },
+ { NFT_MSG_DELSET, AUDIT_NFT_OP_SET_UNREGISTER },
+ { NFT_MSG_NEWSETELEM, AUDIT_NFT_OP_SETELEM_REGISTER },
+ { NFT_MSG_GETSETELEM, AUDIT_NFT_OP_INVALID },
+ { NFT_MSG_DELSETELEM, AUDIT_NFT_OP_SETELEM_UNREGISTER },
+ { NFT_MSG_NEWGEN, AUDIT_NFT_OP_GEN_REGISTER },
+ { NFT_MSG_GETGEN, AUDIT_NFT_OP_INVALID },
+ { NFT_MSG_TRACE, AUDIT_NFT_OP_INVALID },
+ { NFT_MSG_NEWOBJ, AUDIT_NFT_OP_OBJ_REGISTER },
+ { NFT_MSG_GETOBJ, AUDIT_NFT_OP_INVALID },
+ { NFT_MSG_DELOBJ, AUDIT_NFT_OP_OBJ_UNREGISTER },
+ { NFT_MSG_GETOBJ_RESET, AUDIT_NFT_OP_OBJ_RESET },
+ { NFT_MSG_NEWFLOWTABLE, AUDIT_NFT_OP_FLOWTABLE_REGISTER },
+ { NFT_MSG_GETFLOWTABLE, AUDIT_NFT_OP_INVALID },
+ { NFT_MSG_DELFLOWTABLE, AUDIT_NFT_OP_FLOWTABLE_UNREGISTER },
+ { NFT_MSG_MAX, 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 4471393da6d8..7a386eca6e04 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,14 @@ 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,
+ audit_nftcfgs[event].op);
+ kfree(buf);
if (!ctx->report &&
!nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -1428,6 +1437,15 @@ 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,
+ audit_nftcfgs[event].op);
+ kfree(buf);
if (!ctx->report &&
!nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -2691,6 +2709,15 @@ 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,
+ audit_nftcfgs[event].op);
+ kfree(buf);
if (!ctx->report &&
!nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -3692,6 +3719,15 @@ 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,
+ audit_nftcfgs[event].op);
+ kfree(buf);
if (!ctx->report &&
!nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -4789,6 +4825,15 @@ 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,
+ audit_nftcfgs[event].op);
+ kfree(buf);
if (!ctx->report && !nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
return;
@@ -5875,6 +5920,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_nftcfgs[NFT_MSG_GETOBJ_RESET].op);
+ kfree(buf);
+ }
+
if (nf_tables_fill_obj_info(skb, net, NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
NFT_MSG_NEWOBJ,
@@ -5985,6 +6043,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_nftcfgs[NFT_MSG_GETOBJ_RESET].op);
+ 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);
@@ -6060,6 +6129,14 @@ 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,
+ audit_nftcfgs[event].op);
+ kfree(buf);
if (!report &&
!nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
@@ -6686,6 +6763,15 @@ 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,
+ audit_nftcfgs[event].op);
+ kfree(buf);
if (ctx->report &&
!nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
@@ -6807,6 +6893,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_nftcfgs[event].op);
+
if (nlmsg_report(nlh) &&
!nfnetlink_has_listeners(net, NFNLGRP_NFTABLES))
return;
--
1.8.3.1
Hi Richard,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on pcmoore-audit/next]
[also build test ERROR on next-20200529]
[cannot apply to nf/master nf-next/master linus/master linux/master v5.7-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Richard-Guy-Briggs/audit-log-nftables-configuration-change-events/20200531-043244
base: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git next
config: sparc-allyesconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sparc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <[email protected]>
All errors (new ones prefixed by >>, old ones prefixed by <<):
In file included from arch/sparc/kernel/ptrace_64.c:25:
>> include/linux/audit.h:126:40: error: 'audit_nftcfgs' defined but not used [-Werror=unused-const-variable=]
126 | static const struct audit_nftcfgop_tab audit_nftcfgs[] = {
| ^~~~~~~~~~~~~
cc1: all warnings being treated as errors
vim +/audit_nftcfgs +126 include/linux/audit.h
125
> 126 static const struct audit_nftcfgop_tab audit_nftcfgs[] = {
127 { NFT_MSG_NEWTABLE, AUDIT_NFT_OP_TABLE_REGISTER },
128 { NFT_MSG_GETTABLE, AUDIT_NFT_OP_INVALID },
129 { NFT_MSG_DELTABLE, AUDIT_NFT_OP_TABLE_UNREGISTER },
130 { NFT_MSG_NEWCHAIN, AUDIT_NFT_OP_CHAIN_REGISTER },
131 { NFT_MSG_GETCHAIN, AUDIT_NFT_OP_INVALID },
132 { NFT_MSG_DELCHAIN, AUDIT_NFT_OP_CHAIN_UNREGISTER },
133 { NFT_MSG_NEWRULE, AUDIT_NFT_OP_RULE_REGISTER },
134 { NFT_MSG_GETRULE, AUDIT_NFT_OP_INVALID },
135 { NFT_MSG_DELRULE, AUDIT_NFT_OP_RULE_UNREGISTER },
136 { NFT_MSG_NEWSET, AUDIT_NFT_OP_SET_REGISTER },
137 { NFT_MSG_GETSET, AUDIT_NFT_OP_INVALID },
138 { NFT_MSG_DELSET, AUDIT_NFT_OP_SET_UNREGISTER },
139 { NFT_MSG_NEWSETELEM, AUDIT_NFT_OP_SETELEM_REGISTER },
140 { NFT_MSG_GETSETELEM, AUDIT_NFT_OP_INVALID },
141 { NFT_MSG_DELSETELEM, AUDIT_NFT_OP_SETELEM_UNREGISTER },
142 { NFT_MSG_NEWGEN, AUDIT_NFT_OP_GEN_REGISTER },
143 { NFT_MSG_GETGEN, AUDIT_NFT_OP_INVALID },
144 { NFT_MSG_TRACE, AUDIT_NFT_OP_INVALID },
145 { NFT_MSG_NEWOBJ, AUDIT_NFT_OP_OBJ_REGISTER },
146 { NFT_MSG_GETOBJ, AUDIT_NFT_OP_INVALID },
147 { NFT_MSG_DELOBJ, AUDIT_NFT_OP_OBJ_UNREGISTER },
148 { NFT_MSG_GETOBJ_RESET, AUDIT_NFT_OP_OBJ_RESET },
149 { NFT_MSG_NEWFLOWTABLE, AUDIT_NFT_OP_FLOWTABLE_REGISTER },
150 { NFT_MSG_GETFLOWTABLE, AUDIT_NFT_OP_INVALID },
151 { NFT_MSG_DELFLOWTABLE, AUDIT_NFT_OP_FLOWTABLE_UNREGISTER },
152 { NFT_MSG_MAX, AUDIT_NFT_OP_INVALID },
153 };
154
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Thu, May 28, 2020 at 9:44 PM Richard Guy Briggs <[email protected]> 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 <[email protected]>
> ---
> Changelog:
> 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 | 52 +++++++++++++++++++++++++
> kernel/auditsc.c | 24 ++++++++++--
> net/netfilter/nf_tables_api.c | 89 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 162 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 3fcd9ee49734..d79866a38505 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,57 @@ 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,
> +};
> +
> +struct audit_nftcfgop_tab {
> + enum nf_tables_msg_types nftop;
> + enum audit_nfcfgop op;
> +};
> +
> +static const struct audit_nftcfgop_tab audit_nftcfgs[] = {
> + { NFT_MSG_NEWTABLE, AUDIT_NFT_OP_TABLE_REGISTER },
> + { NFT_MSG_GETTABLE, AUDIT_NFT_OP_INVALID },
> + { NFT_MSG_DELTABLE, AUDIT_NFT_OP_TABLE_UNREGISTER },
> + { NFT_MSG_NEWCHAIN, AUDIT_NFT_OP_CHAIN_REGISTER },
> + { NFT_MSG_GETCHAIN, AUDIT_NFT_OP_INVALID },
> + { NFT_MSG_DELCHAIN, AUDIT_NFT_OP_CHAIN_UNREGISTER },
> + { NFT_MSG_NEWRULE, AUDIT_NFT_OP_RULE_REGISTER },
> + { NFT_MSG_GETRULE, AUDIT_NFT_OP_INVALID },
> + { NFT_MSG_DELRULE, AUDIT_NFT_OP_RULE_UNREGISTER },
> + { NFT_MSG_NEWSET, AUDIT_NFT_OP_SET_REGISTER },
> + { NFT_MSG_GETSET, AUDIT_NFT_OP_INVALID },
> + { NFT_MSG_DELSET, AUDIT_NFT_OP_SET_UNREGISTER },
> + { NFT_MSG_NEWSETELEM, AUDIT_NFT_OP_SETELEM_REGISTER },
> + { NFT_MSG_GETSETELEM, AUDIT_NFT_OP_INVALID },
> + { NFT_MSG_DELSETELEM, AUDIT_NFT_OP_SETELEM_UNREGISTER },
> + { NFT_MSG_NEWGEN, AUDIT_NFT_OP_GEN_REGISTER },
> + { NFT_MSG_GETGEN, AUDIT_NFT_OP_INVALID },
> + { NFT_MSG_TRACE, AUDIT_NFT_OP_INVALID },
> + { NFT_MSG_NEWOBJ, AUDIT_NFT_OP_OBJ_REGISTER },
> + { NFT_MSG_GETOBJ, AUDIT_NFT_OP_INVALID },
> + { NFT_MSG_DELOBJ, AUDIT_NFT_OP_OBJ_UNREGISTER },
> + { NFT_MSG_GETOBJ_RESET, AUDIT_NFT_OP_OBJ_RESET },
> + { NFT_MSG_NEWFLOWTABLE, AUDIT_NFT_OP_FLOWTABLE_REGISTER },
> + { NFT_MSG_GETFLOWTABLE, AUDIT_NFT_OP_INVALID },
> + { NFT_MSG_DELFLOWTABLE, AUDIT_NFT_OP_FLOWTABLE_UNREGISTER },
> + { NFT_MSG_MAX, AUDIT_NFT_OP_INVALID },
> };
I didn't check every "op" defined above to match with the changes in
nf_tables_api.c, but is there a reason why we can't simply hardcode
the AUDIT_NFT_OP_* values in the audit_log_nfcfg() calls in
nf_tables_api.c? If we can, let's do that.
If we can't do that, we need to add some build-time protection to
catch if NFT_MSG_MAX increases without this table being updated.
> 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 4471393da6d8..7a386eca6e04 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,14 @@ 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,
> + audit_nftcfgs[event].op);
As an example, the below would work, yes?
audit_log_nfcfg(...,
(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))
--
paul moore
http://www.paul-moore.com
On 2020-06-01 12:10, Paul Moore wrote:
> On Thu, May 28, 2020 at 9:44 PM Richard Guy Briggs <[email protected]> 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 <[email protected]>
> > ---
> > Changelog:
> > 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 | 52 +++++++++++++++++++++++++
> > kernel/auditsc.c | 24 ++++++++++--
> > net/netfilter/nf_tables_api.c | 89 +++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 162 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 3fcd9ee49734..d79866a38505 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,57 @@ 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,
> > +};
> > +
> > +struct audit_nftcfgop_tab {
> > + enum nf_tables_msg_types nftop;
> > + enum audit_nfcfgop op;
> > +};
> > +
> > +static const struct audit_nftcfgop_tab audit_nftcfgs[] = {
> > + { NFT_MSG_NEWTABLE, AUDIT_NFT_OP_TABLE_REGISTER },
> > + { NFT_MSG_GETTABLE, AUDIT_NFT_OP_INVALID },
> > + { NFT_MSG_DELTABLE, AUDIT_NFT_OP_TABLE_UNREGISTER },
> > + { NFT_MSG_NEWCHAIN, AUDIT_NFT_OP_CHAIN_REGISTER },
> > + { NFT_MSG_GETCHAIN, AUDIT_NFT_OP_INVALID },
> > + { NFT_MSG_DELCHAIN, AUDIT_NFT_OP_CHAIN_UNREGISTER },
> > + { NFT_MSG_NEWRULE, AUDIT_NFT_OP_RULE_REGISTER },
> > + { NFT_MSG_GETRULE, AUDIT_NFT_OP_INVALID },
> > + { NFT_MSG_DELRULE, AUDIT_NFT_OP_RULE_UNREGISTER },
> > + { NFT_MSG_NEWSET, AUDIT_NFT_OP_SET_REGISTER },
> > + { NFT_MSG_GETSET, AUDIT_NFT_OP_INVALID },
> > + { NFT_MSG_DELSET, AUDIT_NFT_OP_SET_UNREGISTER },
> > + { NFT_MSG_NEWSETELEM, AUDIT_NFT_OP_SETELEM_REGISTER },
> > + { NFT_MSG_GETSETELEM, AUDIT_NFT_OP_INVALID },
> > + { NFT_MSG_DELSETELEM, AUDIT_NFT_OP_SETELEM_UNREGISTER },
> > + { NFT_MSG_NEWGEN, AUDIT_NFT_OP_GEN_REGISTER },
> > + { NFT_MSG_GETGEN, AUDIT_NFT_OP_INVALID },
> > + { NFT_MSG_TRACE, AUDIT_NFT_OP_INVALID },
> > + { NFT_MSG_NEWOBJ, AUDIT_NFT_OP_OBJ_REGISTER },
> > + { NFT_MSG_GETOBJ, AUDIT_NFT_OP_INVALID },
> > + { NFT_MSG_DELOBJ, AUDIT_NFT_OP_OBJ_UNREGISTER },
> > + { NFT_MSG_GETOBJ_RESET, AUDIT_NFT_OP_OBJ_RESET },
> > + { NFT_MSG_NEWFLOWTABLE, AUDIT_NFT_OP_FLOWTABLE_REGISTER },
> > + { NFT_MSG_GETFLOWTABLE, AUDIT_NFT_OP_INVALID },
> > + { NFT_MSG_DELFLOWTABLE, AUDIT_NFT_OP_FLOWTABLE_UNREGISTER },
> > + { NFT_MSG_MAX, AUDIT_NFT_OP_INVALID },
> > };
>
> I didn't check every "op" defined above to match with the changes in
> nf_tables_api.c, but is there a reason why we can't simply hardcode
> the AUDIT_NFT_OP_* values in the audit_log_nfcfg() calls in
> nf_tables_api.c? If we can, let's do that.
Yes, to distinguish between x/ebtables and nftables. The first three
overlap (as suggested by the first comment in the changelog above).
> If we can't do that, we need to add some build-time protection to
> catch if NFT_MSG_MAX increases without this table being updated.
I would have liked to do that, but it would then have meant making a new
function name, say audit_log_nftcfg vs the original audit_log_nfcfg so
that the overlapping values could be translated appropriately with the
correct prefix.
Well, considering the buildbot error, I had to restructure it so that
the table was moved from include/linux/audit.h to
net/netfilter/nf_tables_api.c where it would be a bit more evident.
At first I thought the bot was complaining about the first field not
being used, but it turned out it was ptrace that was including it and
not using it.
> > 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 4471393da6d8..7a386eca6e04 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,14 @@ 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,
> > + audit_nftcfgs[event].op);
>
> As an example, the below would work, yes?
>
> audit_log_nfcfg(...,
> (event == NFT_MSG_NEWTABLE ?
> AUDIT_NFT_OP_TABLE_REGISTER :
> AUDIT_NFT_OP_TABLE_UNREGISTER)
Ok, I see what you are getting at now... Yes, it could be done this
way, but it seems noisier to me.
> > + kfree(buf);
> >
> > if (!ctx->report &&
> > !nfnetlink_has_listeners(ctx->net, NFNLGRP_NFTABLES))
>
> paul moore
- RGB
--
Richard Guy Briggs <[email protected]>
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
On Mon, Jun 1, 2020 at 6:58 PM Richard Guy Briggs <[email protected]> wrote:
> On 2020-06-01 12:10, Paul Moore wrote:
> > On Thu, May 28, 2020 at 9:44 PM Richard Guy Briggs <[email protected]> wrote:
...
> > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > > index 4471393da6d8..7a386eca6e04 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,14 @@ 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,
> > > + audit_nftcfgs[event].op);
> >
> > As an example, the below would work, yes?
> >
> > audit_log_nfcfg(...,
> > (event == NFT_MSG_NEWTABLE ?
> > AUDIT_NFT_OP_TABLE_REGISTER :
> > AUDIT_NFT_OP_TABLE_UNREGISTER)
>
> Ok, I see what you are getting at now... Yes, it could be done this
> way, but it seems noisier to me.
I'll admit it is not as clean, but it doesn't hide the mapping between
the netfilter operation and the audit operation which hopefully makes
it clear to those modifying the netfilter/nf_tables/etc. code that
there is an audit impact. I'm basically trying to make sure the code
is as robust as possible in the face of subsystem changes beyond the
audit subsystem.
--
paul moore
http://www.paul-moore.com
On 2020-06-01 20:12, Paul Moore wrote:
> On Mon, Jun 1, 2020 at 6:58 PM Richard Guy Briggs <[email protected]> wrote:
> > On 2020-06-01 12:10, Paul Moore wrote:
> > > On Thu, May 28, 2020 at 9:44 PM Richard Guy Briggs <[email protected]> wrote:
>
> ...
>
> > > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > > > index 4471393da6d8..7a386eca6e04 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,14 @@ 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,
> > > > + audit_nftcfgs[event].op);
> > >
> > > As an example, the below would work, yes?
> > >
> > > audit_log_nfcfg(...,
> > > (event == NFT_MSG_NEWTABLE ?
> > > AUDIT_NFT_OP_TABLE_REGISTER :
> > > AUDIT_NFT_OP_TABLE_UNREGISTER)
> >
> > Ok, I see what you are getting at now... Yes, it could be done this
> > way, but it seems noisier to me.
>
> I'll admit it is not as clean, but it doesn't hide the mapping between
> the netfilter operation and the audit operation which hopefully makes
> it clear to those modifying the netfilter/nf_tables/etc. code that
> there is an audit impact. I'm basically trying to make sure the code
> is as robust as possible in the face of subsystem changes beyond the
> audit subsystem.
Yup, I agree, a compile time check to make sure they aren't out of sync.
> paul moore
- RGB
--
Richard Guy Briggs <[email protected]>
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