Re: [PATCH ghak124 v3] audit: log nftables configuration change events
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
> > ---
> > 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
> > #include
> > #include
> > +#include
> >
> > #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
> > #include
> > #include
> > +#include
> >
> > #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_rul
Re: [PATCH ghak124 v3] audit: log nftables configuration change events
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
> ---
> 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
> #include
> #include
> +#include
>
> #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
> #include
> #include
> +#include
>
> #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_unregi
Re: [PATCH ghak124 v3] audit: log nftables configuration change events
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
> > > > #include
> > > > #include
> > > > +#include
> > > > #include
> > > > #include
> > > > #include
> > > > @@ -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
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
Re: [PATCH ghak124 v3] audit: log nftables configuration change events
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
> > > #include
> > > #include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > @@ -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
Re: [PATCH v3 2/2] IMA: Add audit log for failure conditions
On 6/23/20 12:58 PM, Mimi Zohar wrote: Hi Steve\Paul, Sample audit messages: [6.303048] audit: type=1804 audit(1592506281.627:2): pid=1 uid=0 auid=4294967295 ses=4294967295 subj=kernel op=measuring_key cause=ENOMEM comm="swapper/0" name=".builtin_trusted_keys" res=0 errno=-12 My only concern is that auditing -ENOMEM will put additional memory pressure on the system. I'm not sure if this is a concern and, if so, how it should be handled. Do you have any concerns with respect to adding audit messages in low memory conditions? Reviewed-by: Mimi Zohar Thanks Mimi -lakshmi -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH] audit: optionally print warning after waiting to enqueue record
On Tue, Jun 23, 2020 at 08:15:59PM -0400, Paul Moore wrote: > On Thu, Jun 18, 2020 at 8:30 PM Richard Guy Briggs wrote: > > On 2020-06-18 23:48, Max Englander wrote: > > > In case you’re any more receptive to the idea, I thought I’d mention > > > that the need this patch addresses would be just as well fulfilled if > > > wait times were reported in the audit status response along with other > > > currently reported metrics like backlog length and lost events. Wait > > > times could be reported as a cumulative sum, a moving average, or in > > > some other way, and would help directly implicate or rule out backlog > > > waiting as the cause in the event that an admin is faced with debugging > > > degraded kernel performance. It would eliminate the need for a new flag, > > > and fit well with the userspace tooling approach you suggested above. > > > > Such as is captured in this upstream issue from 3 years ago: > > > > https://github.com/linux-audit/audit-kernel/issues/63 > > "RFE: add kernel audit queue statistics" > > I would be more open to the idea of reporting queue statistics as part > of the audit status information, or similar. > > -- > paul moore > www.paul-moore.com Excellent, I'll send a v2 patch. -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
