[RFC PATCH] audit: normalize NETFILTER_PKT

[RFC PATCH] audit: normalize NETFILTER_PKT

[Richard Guy Briggs]

Eliminate flipping in and out of message fields.

https://github.com/linux-audit/audit-kernel/issues/11

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 net/netfilter/xt_AUDIT.c |   92 +++++++++++++++++++++++++++++++++-------------
 1 files changed, 66 insertions(+), 26 deletions(-)

diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
index 4973cbd..8089ec2 100644
--- a/net/netfilter/xt_AUDIT.c
+++ b/net/netfilter/xt_AUDIT.c
@@ -31,24 +31,41 @@ MODULE_ALIAS("ip6t_AUDIT");
 MODULE_ALIAS("ebt_AUDIT");
 MODULE_ALIAS("arpt_AUDIT");
 
+struct nfpkt_par {
+	int ipv;
+	int iptrunc;
+	const void *saddr;
+	const void *daddr;
+	u16 ipid;
+	u8 proto;
+	u8 frag;
+	int ptrunc;
+	u16 sport;
+	u16 dport;
+	u8 icmpt;
+	u8 icmpc;
+};
+
 static void audit_proto(struct audit_buffer *ab, struct sk_buff *skb,
-			unsigned int proto, unsigned int offset)
+			unsigned int proto, unsigned int offset, struct nfpkt_par *apar)
 {
 	switch (proto) {
 	case IPPROTO_TCP:
 	case IPPROTO_UDP:
-	case IPPROTO_UDPLITE: {
+	case IPPROTO_UDPLITE:
+	case IPPROTO_DCCP:
+	case IPPROTO_SCTP: {
 		const __be16 *pptr;
 		__be16 _ports[2];
 
 		pptr = skb_header_pointer(skb, offset, sizeof(_ports), _ports);
 		if (pptr == NULL) {
-			audit_log_format(ab, " truncated=1");
+			apar->ptrunc = 1;
 			return;
 		}
+		apar->sport = ntohs(pptr[0]);
+		apar->dport = ntohs(pptr[1]);
 
-		audit_log_format(ab, " sport=%hu dport=%hu",
-				 ntohs(pptr[0]), ntohs(pptr[1]));
 		}
 		break;
 
@@ -59,41 +76,43 @@ static void audit_proto(struct audit_buffer *ab, struct sk_buff *skb,
 
 		iptr = skb_header_pointer(skb, offset, sizeof(_ih), &_ih);
 		if (iptr == NULL) {
-			audit_log_format(ab, " truncated=1");
+			apar->ptrunc = 1;
 			return;
 		}
-
-		audit_log_format(ab, " icmptype=%hhu icmpcode=%hhu",
-				 iptr[0], iptr[1]);
+		apar->icmpt = iptr[0];
+		apar->icmpc = iptr[1];
 
 		}
 		break;
 	}
 }
 
-static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb)
+static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb, struct nfpkt_par *apar)
 {
 	struct iphdr _iph;
 	const struct iphdr *ih;
 
+	apar->ipv = 4;
 	ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph);
 	if (!ih) {
-		audit_log_format(ab, " truncated=1");
+		apar->iptrunc = 1;
 		return;
 	}
 
-	audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu proto=%hhu",
-		&ih->saddr, &ih->daddr, ntohs(ih->id), ih->protocol);
+	apar->saddr = &ih->saddr;
+	apar->daddr = &ih->daddr;
+	apar->ipid = ntohs(ih->id);
+	apar->proto = ih->protocol;
 
 	if (ntohs(ih->frag_off) & IP_OFFSET) {
-		audit_log_format(ab, " frag=1");
+		apar->frag = 1;
 		return;
 	}
 
-	audit_proto(ab, skb, ih->protocol, ih->ihl * 4);
+	audit_proto(ab, skb, ih->protocol, ih->ihl * 4, apar);
 }
 
-static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
+static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb, struct nfpkt_par *apar)
 {
 	struct ipv6hdr _ip6h;
 	const struct ipv6hdr *ih;
@@ -101,9 +120,10 @@ static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
 	__be16 frag_off;
 	int offset;
 
+	apar->ipv = 6;
 	ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_ip6h), &_ip6h);
 	if (!ih) {
-		audit_log_format(ab, " truncated=1");
+		apar->iptrunc = 1;
 		return;
 	}
 
@@ -111,11 +131,12 @@ static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
 	offset = ipv6_skip_exthdr(skb, skb_network_offset(skb) + sizeof(_ip6h),
 				  &nexthdr, &frag_off);
 
-	audit_log_format(ab, " saddr=%pI6c daddr=%pI6c proto=%hhu",
-			 &ih->saddr, &ih->daddr, nexthdr);
+	apar->saddr = &ih->saddr;
+	apar->daddr = &ih->daddr;
+	apar->proto = nexthdr;
 
 	if (offset)
-		audit_proto(ab, skb, nexthdr, offset);
+		audit_proto(ab, skb, nexthdr, offset, apar);
 }
 
 static unsigned int
@@ -123,6 +144,9 @@ audit_tg(struct sk_buff *skb, const struct xt_action_param *par)
 {
 	const struct xt_audit_info *info = par->targinfo;
 	struct audit_buffer *ab;
+	struct nfpkt_par apar = {
+		-1, -1, NULL, NULL, -1, -1, -1, -1, -1, -1, -1, -1
+	};
 
 	if (audit_enabled == 0)
 		goto errout;
@@ -136,8 +160,7 @@ audit_tg(struct sk_buff *skb, const struct xt_action_param *par)
 			 par->in ? par->in->name : "?",
 			 par->out ? par->out->name : "?");
 
-	if (skb->mark)
-		audit_log_format(ab, " mark=%#x", skb->mark);
+	audit_log_format(ab, " mark=%#x", skb->mark ?: -1);
 
 	if (skb->dev && skb->dev->type == ARPHRD_ETHER) {
 		audit_log_format(ab, " smac=%pM dmac=%pM macproto=0x%04x",
@@ -147,25 +170,42 @@ audit_tg(struct sk_buff *skb, const struct xt_action_param *par)
 		if (par->family == NFPROTO_BRIDGE) {
 			switch (eth_hdr(skb)->h_proto) {
 			case htons(ETH_P_IP):
-				audit_ip4(ab, skb);
+				audit_ip4(ab, skb, &apar);
 				break;
 
 			case htons(ETH_P_IPV6):
-				audit_ip6(ab, skb);
+				audit_ip6(ab, skb, &apar);
 				break;
 			}
 		}
+	} else {
+		audit_log_format(ab, " smac=? dmac=? macproto=0xffff");
 	}
 
 	switch (par->family) {
 	case NFPROTO_IPV4:
-		audit_ip4(ab, skb);
+		audit_ip4(ab, skb, &apar);
 		break;
 
 	case NFPROTO_IPV6:
-		audit_ip6(ab, skb);
+		audit_ip6(ab, skb, &apar);
+		break;
+	}
+
+	switch (apar.ipv) {
+	case 4:
+		audit_log_format(ab, " trunc=%d saddr=%pI4 daddr=%pI4 ipid=%hu proto=%hhu frag=%d",
+			 apar.iptrunc, apar.saddr, apar.daddr, apar.ipid, apar.proto, apar.frag);
+		break;
+	case 6:
+		audit_log_format(ab, " trunc=%d saddr=%pI6c daddr=%pI6c ipid=-1 proto=%hhu frag=-1",
+			 apar.iptrunc, apar.saddr, apar.daddr, apar.proto);
 		break;
+	default:
+		audit_log_format(ab, " trunc=-1 saddr=? daddr=? ipid=-1 proto=-1 frag=-1");
 	}
+	audit_log_format(ab, " trunc=%d sport=%hu dport=%hu icmptype=%hhu icmpcode=%hhu",
+		apar.ptrunc, apar.sport, apar.dport, apar.icmpt, apar.icmpc);
 
 #ifdef CONFIG_NETWORK_SECMARK
 	if (skb->secmark)
-- 
1.7.1

Re: [RFC PATCH] audit: normalize NETFILTER_PKT

[Steve Grubb]

On Fri, 27 Jan 2017 08:11:06 -0500
Richard Guy Briggs <rgb@redhat.com> wrote:
> Eliminate flipping in and out of message fields.
> 
> https://github.com/linux-audit/audit-kernel/issues/11

Do you have sample events that shows how this changes the record
format? I like to review how the event looks when a patch changes or
adds a record.

Thanks,
-Steve


> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  net/netfilter/xt_AUDIT.c |   92
> +++++++++++++++++++++++++++++++++------------- 1 files changed, 66
> insertions(+), 26 deletions(-)
> 
> diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
> index 4973cbd..8089ec2 100644
> --- a/net/netfilter/xt_AUDIT.c
> +++ b/net/netfilter/xt_AUDIT.c
> @@ -31,24 +31,41 @@ MODULE_ALIAS("ip6t_AUDIT");
>  MODULE_ALIAS("ebt_AUDIT");
>  MODULE_ALIAS("arpt_AUDIT");
>  
> +struct nfpkt_par {
> +	int ipv;
> +	int iptrunc;
> +	const void *saddr;
> +	const void *daddr;
> +	u16 ipid;
> +	u8 proto;
> +	u8 frag;
> +	int ptrunc;
> +	u16 sport;
> +	u16 dport;
> +	u8 icmpt;
> +	u8 icmpc;
> +};
> +
>  static void audit_proto(struct audit_buffer *ab, struct sk_buff *skb,
> -			unsigned int proto, unsigned int offset)
> +			unsigned int proto, unsigned int offset,
> struct nfpkt_par *apar) {
>  	switch (proto) {
>  	case IPPROTO_TCP:
>  	case IPPROTO_UDP:
> -	case IPPROTO_UDPLITE: {
> +	case IPPROTO_UDPLITE:
> +	case IPPROTO_DCCP:
> +	case IPPROTO_SCTP: {
>  		const __be16 *pptr;
>  		__be16 _ports[2];
>  
>  		pptr = skb_header_pointer(skb, offset,
> sizeof(_ports), _ports); if (pptr == NULL) {
> -			audit_log_format(ab, " truncated=1");
> +			apar->ptrunc = 1;
>  			return;
>  		}
> +		apar->sport = ntohs(pptr[0]);
> +		apar->dport = ntohs(pptr[1]);
>  
> -		audit_log_format(ab, " sport=%hu dport=%hu",
> -				 ntohs(pptr[0]), ntohs(pptr[1]));
>  		}
>  		break;
>  
> @@ -59,41 +76,43 @@ static void audit_proto(struct audit_buffer *ab,
> struct sk_buff *skb, 
>  		iptr = skb_header_pointer(skb, offset, sizeof(_ih),
> &_ih); if (iptr == NULL) {
> -			audit_log_format(ab, " truncated=1");
> +			apar->ptrunc = 1;
>  			return;
>  		}
> -
> -		audit_log_format(ab, " icmptype=%hhu icmpcode=%hhu",
> -				 iptr[0], iptr[1]);
> +		apar->icmpt = iptr[0];
> +		apar->icmpc = iptr[1];
>  
>  		}
>  		break;
>  	}
>  }
>  
> -static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb)
> +static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb,
> struct nfpkt_par *apar) {
>  	struct iphdr _iph;
>  	const struct iphdr *ih;
>  
> +	apar->ipv = 4;
>  	ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph);
>  	if (!ih) {
> -		audit_log_format(ab, " truncated=1");
> +		apar->iptrunc = 1;
>  		return;
>  	}
>  
> -	audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu
> proto=%hhu",
> -		&ih->saddr, &ih->daddr, ntohs(ih->id), ih->protocol);
> +	apar->saddr = &ih->saddr;
> +	apar->daddr = &ih->daddr;
> +	apar->ipid = ntohs(ih->id);
> +	apar->proto = ih->protocol;
>  
>  	if (ntohs(ih->frag_off) & IP_OFFSET) {
> -		audit_log_format(ab, " frag=1");
> +		apar->frag = 1;
>  		return;
>  	}
>  
> -	audit_proto(ab, skb, ih->protocol, ih->ihl * 4);
> +	audit_proto(ab, skb, ih->protocol, ih->ihl * 4, apar);
>  }
>  
> -static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
> +static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb,
> struct nfpkt_par *apar) {
>  	struct ipv6hdr _ip6h;
>  	const struct ipv6hdr *ih;
> @@ -101,9 +120,10 @@ static void audit_ip6(struct audit_buffer *ab,
> struct sk_buff *skb) __be16 frag_off;
>  	int offset;
>  
> +	apar->ipv = 6;
>  	ih = skb_header_pointer(skb, skb_network_offset(skb),
> sizeof(_ip6h), &_ip6h); if (!ih) {
> -		audit_log_format(ab, " truncated=1");
> +		apar->iptrunc = 1;
>  		return;
>  	}
>  
> @@ -111,11 +131,12 @@ static void audit_ip6(struct audit_buffer *ab,
> struct sk_buff *skb) offset = ipv6_skip_exthdr(skb,
> skb_network_offset(skb) + sizeof(_ip6h), &nexthdr, &frag_off);
>  
> -	audit_log_format(ab, " saddr=%pI6c daddr=%pI6c proto=%hhu",
> -			 &ih->saddr, &ih->daddr, nexthdr);
> +	apar->saddr = &ih->saddr;
> +	apar->daddr = &ih->daddr;
> +	apar->proto = nexthdr;
>  
>  	if (offset)
> -		audit_proto(ab, skb, nexthdr, offset);
> +		audit_proto(ab, skb, nexthdr, offset, apar);
>  }
>  
>  static unsigned int
> @@ -123,6 +144,9 @@ audit_tg(struct sk_buff *skb, const struct
> xt_action_param *par) {
>  	const struct xt_audit_info *info = par->targinfo;
>  	struct audit_buffer *ab;
> +	struct nfpkt_par apar = {
> +		-1, -1, NULL, NULL, -1, -1, -1, -1, -1, -1, -1, -1
> +	};
>  
>  	if (audit_enabled == 0)
>  		goto errout;
> @@ -136,8 +160,7 @@ audit_tg(struct sk_buff *skb, const struct
> xt_action_param *par) par->in ? par->in->name : "?",
>  			 par->out ? par->out->name : "?");
>  
> -	if (skb->mark)
> -		audit_log_format(ab, " mark=%#x", skb->mark);
> +	audit_log_format(ab, " mark=%#x", skb->mark ?: -1);
>  
>  	if (skb->dev && skb->dev->type == ARPHRD_ETHER) {
>  		audit_log_format(ab, " smac=%pM dmac=%pM
> macproto=0x%04x", @@ -147,25 +170,42 @@ audit_tg(struct sk_buff *skb,
> const struct xt_action_param *par) if (par->family == NFPROTO_BRIDGE)
> { switch (eth_hdr(skb)->h_proto) {
>  			case htons(ETH_P_IP):
> -				audit_ip4(ab, skb);
> +				audit_ip4(ab, skb, &apar);
>  				break;
>  
>  			case htons(ETH_P_IPV6):
> -				audit_ip6(ab, skb);
> +				audit_ip6(ab, skb, &apar);
>  				break;
>  			}
>  		}
> +	} else {
> +		audit_log_format(ab, " smac=? dmac=?
> macproto=0xffff"); }
>  
>  	switch (par->family) {
>  	case NFPROTO_IPV4:
> -		audit_ip4(ab, skb);
> +		audit_ip4(ab, skb, &apar);
>  		break;
>  
>  	case NFPROTO_IPV6:
> -		audit_ip6(ab, skb);
> +		audit_ip6(ab, skb, &apar);
> +		break;
> +	}
> +
> +	switch (apar.ipv) {
> +	case 4:
> +		audit_log_format(ab, " trunc=%d saddr=%pI4
> daddr=%pI4 ipid=%hu proto=%hhu frag=%d",
> +			 apar.iptrunc, apar.saddr, apar.daddr,
> apar.ipid, apar.proto, apar.frag);
> +		break;
> +	case 6:
> +		audit_log_format(ab, " trunc=%d saddr=%pI6c
> daddr=%pI6c ipid=-1 proto=%hhu frag=-1",
> +			 apar.iptrunc, apar.saddr, apar.daddr,
> apar.proto); break;
> +	default:
> +		audit_log_format(ab, " trunc=-1 saddr=? daddr=?
> ipid=-1 proto=-1 frag=-1"); }
> +	audit_log_format(ab, " trunc=%d sport=%hu dport=%hu
> icmptype=%hhu icmpcode=%hhu",
> +		apar.ptrunc, apar.sport, apar.dport, apar.icmpt,
> apar.icmpc); 
>  #ifdef CONFIG_NETWORK_SECMARK
>  	if (skb->secmark)

Re: [RFC PATCH] audit: normalize NETFILTER_PKT

[Richard Guy Briggs]

On 2017-01-30 15:53, Steve Grubb wrote:
> On Fri, 27 Jan 2017 08:11:06 -0500
> Richard Guy Briggs <rgb@redhat.com> wrote:
> > Eliminate flipping in and out of message fields.
> > 
> > https://github.com/linux-audit/audit-kernel/issues/11
> 
> Do you have sample events that shows how this changes the record
> format? I like to review how the event looks when a patch changes or
> adds a record.

I used the format that was proposed.  Here are several samples from
running this RFC patch through the RFC test script:

	ausearch --start 01/27/2017 -i -m netfilter_pkt
----
type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.649:183) : action=ACCEPT hook=OUTPUT len=84 inif=? outif=lo mark=0xa044a1d4 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=icmp frag=255 trunc=-1 sport=65535 dport=65535 icmptype=echo icmpcode=0 
----
type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.649:184) : action=ACCEPT hook=INPUT len=84 inif=lo outif=? mark=0xbadeac28 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=icmp frag=255 trunc=-1 sport=65535 dport=65535 icmptype=echo icmpcode=0 
----
type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.652:185) : action=ACCEPT hook=INPUT len=104 inif=lo outif=? mark=0xb404724 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=ipv6-icmp frag=-1 trunc=-1 sport=65535 dport=65535 icmptype=unknown icmp type (128) icmpcode=0 
----
type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.655:186) : action=ACCEPT hook=INPUT len=60 inif=lo outif=? mark=0xe2bd8098 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=29381 proto=tcp frag=255 trunc=-1 sport=51064 dport=42424 icmptype=unknown icmp type (255) icmpcode=255 
----
type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.657:187) : action=ACCEPT hook=INPUT len=80 inif=lo outif=? mark=0xf80a9dd7 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=tcp frag=-1 trunc=-1 sport=38188 dport=42424 icmptype=unknown icmp type (255) icmpcode=255 
----
type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.659:188) : action=ACCEPT hook=INPUT len=31 inif=lo outif=? mark=0xa6d8d4ac smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=46304 proto=udp frag=255 trunc=-1 sport=60095 dport=42424 icmptype=unknown icmp type (255) icmpcode=255 
----
type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.661:189) : action=ACCEPT hook=INPUT len=51 inif=lo outif=? mark=0x3f0d6054 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=udp frag=-1 trunc=-1 sport=43818 dport=42424 icmptype=unknown icmp type (255) icmpcode=255 
----

> -Steve
> 
> 
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> >  net/netfilter/xt_AUDIT.c |   92
> > +++++++++++++++++++++++++++++++++------------- 1 files changed, 66
> > insertions(+), 26 deletions(-)
> > 
> > diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
> > index 4973cbd..8089ec2 100644
> > --- a/net/netfilter/xt_AUDIT.c
> > +++ b/net/netfilter/xt_AUDIT.c
> > @@ -31,24 +31,41 @@ MODULE_ALIAS("ip6t_AUDIT");
> >  MODULE_ALIAS("ebt_AUDIT");
> >  MODULE_ALIAS("arpt_AUDIT");
> >  
> > +struct nfpkt_par {
> > +	int ipv;
> > +	int iptrunc;
> > +	const void *saddr;
> > +	const void *daddr;
> > +	u16 ipid;
> > +	u8 proto;
> > +	u8 frag;
> > +	int ptrunc;
> > +	u16 sport;
> > +	u16 dport;
> > +	u8 icmpt;
> > +	u8 icmpc;
> > +};
> > +
> >  static void audit_proto(struct audit_buffer *ab, struct sk_buff *skb,
> > -			unsigned int proto, unsigned int offset)
> > +			unsigned int proto, unsigned int offset,
> > struct nfpkt_par *apar) {
> >  	switch (proto) {
> >  	case IPPROTO_TCP:
> >  	case IPPROTO_UDP:
> > -	case IPPROTO_UDPLITE: {
> > +	case IPPROTO_UDPLITE:
> > +	case IPPROTO_DCCP:
> > +	case IPPROTO_SCTP: {
> >  		const __be16 *pptr;
> >  		__be16 _ports[2];
> >  
> >  		pptr = skb_header_pointer(skb, offset,
> > sizeof(_ports), _ports); if (pptr == NULL) {
> > -			audit_log_format(ab, " truncated=1");
> > +			apar->ptrunc = 1;
> >  			return;
> >  		}
> > +		apar->sport = ntohs(pptr[0]);
> > +		apar->dport = ntohs(pptr[1]);
> >  
> > -		audit_log_format(ab, " sport=%hu dport=%hu",
> > -				 ntohs(pptr[0]), ntohs(pptr[1]));
> >  		}
> >  		break;
> >  
> > @@ -59,41 +76,43 @@ static void audit_proto(struct audit_buffer *ab,
> > struct sk_buff *skb, 
> >  		iptr = skb_header_pointer(skb, offset, sizeof(_ih),
> > &_ih); if (iptr == NULL) {
> > -			audit_log_format(ab, " truncated=1");
> > +			apar->ptrunc = 1;
> >  			return;
> >  		}
> > -
> > -		audit_log_format(ab, " icmptype=%hhu icmpcode=%hhu",
> > -				 iptr[0], iptr[1]);
> > +		apar->icmpt = iptr[0];
> > +		apar->icmpc = iptr[1];
> >  
> >  		}
> >  		break;
> >  	}
> >  }
> >  
> > -static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb)
> > +static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb,
> > struct nfpkt_par *apar) {
> >  	struct iphdr _iph;
> >  	const struct iphdr *ih;
> >  
> > +	apar->ipv = 4;
> >  	ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph);
> >  	if (!ih) {
> > -		audit_log_format(ab, " truncated=1");
> > +		apar->iptrunc = 1;
> >  		return;
> >  	}
> >  
> > -	audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu
> > proto=%hhu",
> > -		&ih->saddr, &ih->daddr, ntohs(ih->id), ih->protocol);
> > +	apar->saddr = &ih->saddr;
> > +	apar->daddr = &ih->daddr;
> > +	apar->ipid = ntohs(ih->id);
> > +	apar->proto = ih->protocol;
> >  
> >  	if (ntohs(ih->frag_off) & IP_OFFSET) {
> > -		audit_log_format(ab, " frag=1");
> > +		apar->frag = 1;
> >  		return;
> >  	}
> >  
> > -	audit_proto(ab, skb, ih->protocol, ih->ihl * 4);
> > +	audit_proto(ab, skb, ih->protocol, ih->ihl * 4, apar);
> >  }
> >  
> > -static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
> > +static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb,
> > struct nfpkt_par *apar) {
> >  	struct ipv6hdr _ip6h;
> >  	const struct ipv6hdr *ih;
> > @@ -101,9 +120,10 @@ static void audit_ip6(struct audit_buffer *ab,
> > struct sk_buff *skb) __be16 frag_off;
> >  	int offset;
> >  
> > +	apar->ipv = 6;
> >  	ih = skb_header_pointer(skb, skb_network_offset(skb),
> > sizeof(_ip6h), &_ip6h); if (!ih) {
> > -		audit_log_format(ab, " truncated=1");
> > +		apar->iptrunc = 1;
> >  		return;
> >  	}
> >  
> > @@ -111,11 +131,12 @@ static void audit_ip6(struct audit_buffer *ab,
> > struct sk_buff *skb) offset = ipv6_skip_exthdr(skb,
> > skb_network_offset(skb) + sizeof(_ip6h), &nexthdr, &frag_off);
> >  
> > -	audit_log_format(ab, " saddr=%pI6c daddr=%pI6c proto=%hhu",
> > -			 &ih->saddr, &ih->daddr, nexthdr);
> > +	apar->saddr = &ih->saddr;
> > +	apar->daddr = &ih->daddr;
> > +	apar->proto = nexthdr;
> >  
> >  	if (offset)
> > -		audit_proto(ab, skb, nexthdr, offset);
> > +		audit_proto(ab, skb, nexthdr, offset, apar);
> >  }
> >  
> >  static unsigned int
> > @@ -123,6 +144,9 @@ audit_tg(struct sk_buff *skb, const struct
> > xt_action_param *par) {
> >  	const struct xt_audit_info *info = par->targinfo;
> >  	struct audit_buffer *ab;
> > +	struct nfpkt_par apar = {
> > +		-1, -1, NULL, NULL, -1, -1, -1, -1, -1, -1, -1, -1
> > +	};
> >  
> >  	if (audit_enabled == 0)
> >  		goto errout;
> > @@ -136,8 +160,7 @@ audit_tg(struct sk_buff *skb, const struct
> > xt_action_param *par) par->in ? par->in->name : "?",
> >  			 par->out ? par->out->name : "?");
> >  
> > -	if (skb->mark)
> > -		audit_log_format(ab, " mark=%#x", skb->mark);
> > +	audit_log_format(ab, " mark=%#x", skb->mark ?: -1);
> >  
> >  	if (skb->dev && skb->dev->type == ARPHRD_ETHER) {
> >  		audit_log_format(ab, " smac=%pM dmac=%pM
> > macproto=0x%04x", @@ -147,25 +170,42 @@ audit_tg(struct sk_buff *skb,
> > const struct xt_action_param *par) if (par->family == NFPROTO_BRIDGE)
> > { switch (eth_hdr(skb)->h_proto) {
> >  			case htons(ETH_P_IP):
> > -				audit_ip4(ab, skb);
> > +				audit_ip4(ab, skb, &apar);
> >  				break;
> >  
> >  			case htons(ETH_P_IPV6):
> > -				audit_ip6(ab, skb);
> > +				audit_ip6(ab, skb, &apar);
> >  				break;
> >  			}
> >  		}
> > +	} else {
> > +		audit_log_format(ab, " smac=? dmac=?
> > macproto=0xffff"); }
> >  
> >  	switch (par->family) {
> >  	case NFPROTO_IPV4:
> > -		audit_ip4(ab, skb);
> > +		audit_ip4(ab, skb, &apar);
> >  		break;
> >  
> >  	case NFPROTO_IPV6:
> > -		audit_ip6(ab, skb);
> > +		audit_ip6(ab, skb, &apar);
> > +		break;
> > +	}
> > +
> > +	switch (apar.ipv) {
> > +	case 4:
> > +		audit_log_format(ab, " trunc=%d saddr=%pI4
> > daddr=%pI4 ipid=%hu proto=%hhu frag=%d",
> > +			 apar.iptrunc, apar.saddr, apar.daddr,
> > apar.ipid, apar.proto, apar.frag);
> > +		break;
> > +	case 6:
> > +		audit_log_format(ab, " trunc=%d saddr=%pI6c
> > daddr=%pI6c ipid=-1 proto=%hhu frag=-1",
> > +			 apar.iptrunc, apar.saddr, apar.daddr,
> > apar.proto); break;
> > +	default:
> > +		audit_log_format(ab, " trunc=-1 saddr=? daddr=?
> > ipid=-1 proto=-1 frag=-1"); }
> > +	audit_log_format(ab, " trunc=%d sport=%hu dport=%hu
> > icmptype=%hhu icmpcode=%hhu",
> > +		apar.ptrunc, apar.sport, apar.dport, apar.icmpt,
> > apar.icmpc); 
> >  #ifdef CONFIG_NETWORK_SECMARK
> >  	if (skb->secmark)
> 

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

Re: [RFC PATCH] audit: normalize NETFILTER_PKT

[Richard Guy Briggs]

On 2017-01-30 10:13, Richard Guy Briggs wrote:
> On 2017-01-30 15:53, Steve Grubb wrote:
> > On Fri, 27 Jan 2017 08:11:06 -0500
> > Richard Guy Briggs <rgb@redhat.com> wrote:
> > > Eliminate flipping in and out of message fields.
> > > 
> > > https://github.com/linux-audit/audit-kernel/issues/11
> > 
> > Do you have sample events that shows how this changes the record
> > format? I like to review how the event looks when a patch changes or
> > adds a record.
> 
> I used the format that was proposed.  Here are several samples from
> running this RFC patch through the RFC test script:
> 
> 	ausearch --start 01/27/2017 -i -m netfilter_pkt
> ----
> type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.649:183) : action=ACCEPT hook=OUTPUT len=84 inif=? outif=lo mark=0xa044a1d4 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=icmp frag=255 trunc=-1 sport=65535 dport=65535 icmptype=echo icmpcode=0 
> ----
> type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.649:184) : action=ACCEPT hook=INPUT len=84 inif=lo outif=? mark=0xbadeac28 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=icmp frag=255 trunc=-1 sport=65535 dport=65535 icmptype=echo icmpcode=0 
> ----
> type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.652:185) : action=ACCEPT hook=INPUT len=104 inif=lo outif=? mark=0xb404724 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=ipv6-icmp frag=-1 trunc=-1 sport=65535 dport=65535 icmptype=unknown icmp type (128) icmpcode=0 
> ----
> type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.655:186) : action=ACCEPT hook=INPUT len=60 inif=lo outif=? mark=0xe2bd8098 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=29381 proto=tcp frag=255 trunc=-1 sport=51064 dport=42424 icmptype=unknown icmp type (255) icmpcode=255 
> ----
> type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.657:187) : action=ACCEPT hook=INPUT len=80 inif=lo outif=? mark=0xf80a9dd7 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=tcp frag=-1 trunc=-1 sport=38188 dport=42424 icmptype=unknown icmp type (255) icmpcode=255 
> ----
> type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.659:188) : action=ACCEPT hook=INPUT len=31 inif=lo outif=? mark=0xa6d8d4ac smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=46304 proto=udp frag=255 trunc=-1 sport=60095 dport=42424 icmptype=unknown icmp type (255) icmpcode=255 
> ----
> type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.661:189) : action=ACCEPT hook=INPUT len=51 inif=lo outif=? mark=0x3f0d6054 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=udp frag=-1 trunc=-1 sport=43818 dport=42424 icmptype=unknown icmp type (255) icmpcode=255 
> ----

Here are the raw messages:

----
time->Fri Jan 27 08:03:35 2017
type=NETFILTER_PKT msg=audit(1485522215.649:183): action=0 hook=3 len=84 inif=? outif=lo mark=0xa044a1d4 smac=? dmac=? macproto=0xffff trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=1 frag=255 trunc=-1 sport=65535 dport=65535 icmptype=8 icmpcode=0
----
time->Fri Jan 27 08:03:35 2017
type=NETFILTER_PKT msg=audit(1485522215.649:184): action=0 hook=1 len=84 inif=lo outif=? mark=0xbadeac28 smac=? dmac=? macproto=0xffff trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=1 frag=255 trunc=-1 sport=65535 dport=65535 icmptype=8 icmpcode=0
----
time->Fri Jan 27 08:03:35 2017
type=NETFILTER_PKT msg=audit(1485522215.652:185): action=0 hook=1 len=104 inif=lo outif=? mark=0xb404724 smac=? dmac=? macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=58 frag=-1 trunc=-1 sport=65535 dport=65535 icmptype=128 icmpcode=0
----
time->Fri Jan 27 08:03:35 2017
type=NETFILTER_PKT msg=audit(1485522215.655:186): action=0 hook=1 len=60 inif=lo outif=? mark=0xe2bd8098 smac=? dmac=? macproto=0xffff trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=29381 proto=6 frag=255 trunc=-1 sport=51064 dport=42424 icmptype=255 icmpcode=255
----
time->Fri Jan 27 08:03:35 2017
type=NETFILTER_PKT msg=audit(1485522215.657:187): action=0 hook=1 len=80 inif=lo outif=? mark=0xf80a9dd7 smac=? dmac=? macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=6 frag=-1 trunc=-1 sport=38188 dport=42424 icmptype=255 icmpcode=255
----
time->Fri Jan 27 08:03:35 2017
type=NETFILTER_PKT msg=audit(1485522215.659:188): action=0 hook=1 len=31 inif=lo outif=? mark=0xa6d8d4ac smac=? dmac=? macproto=0xffff trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=46304 proto=17 frag=255 trunc=-1 sport=60095 dport=42424 icmptype=255 icmpcode=255
----
time->Fri Jan 27 08:03:35 2017
type=NETFILTER_PKT msg=audit(1485522215.661:189): action=0 hook=1 len=51 inif=lo outif=? mark=0x3f0d6054 smac=? dmac=? macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=17 frag=-1 trunc=-1 sport=43818 dport=42424 icmptype=255 icmpcode=255
----

> > -Steve
> > 
> > 
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > ---
> > >  net/netfilter/xt_AUDIT.c |   92
> > > +++++++++++++++++++++++++++++++++------------- 1 files changed, 66
> > > insertions(+), 26 deletions(-)
> > > 
> > > diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
> > > index 4973cbd..8089ec2 100644
> > > --- a/net/netfilter/xt_AUDIT.c
> > > +++ b/net/netfilter/xt_AUDIT.c
> > > @@ -31,24 +31,41 @@ MODULE_ALIAS("ip6t_AUDIT");
> > >  MODULE_ALIAS("ebt_AUDIT");
> > >  MODULE_ALIAS("arpt_AUDIT");
> > >  
> > > +struct nfpkt_par {
> > > +	int ipv;
> > > +	int iptrunc;
> > > +	const void *saddr;
> > > +	const void *daddr;
> > > +	u16 ipid;
> > > +	u8 proto;
> > > +	u8 frag;
> > > +	int ptrunc;
> > > +	u16 sport;
> > > +	u16 dport;
> > > +	u8 icmpt;
> > > +	u8 icmpc;
> > > +};
> > > +
> > >  static void audit_proto(struct audit_buffer *ab, struct sk_buff *skb,
> > > -			unsigned int proto, unsigned int offset)
> > > +			unsigned int proto, unsigned int offset,
> > > struct nfpkt_par *apar) {
> > >  	switch (proto) {
> > >  	case IPPROTO_TCP:
> > >  	case IPPROTO_UDP:
> > > -	case IPPROTO_UDPLITE: {
> > > +	case IPPROTO_UDPLITE:
> > > +	case IPPROTO_DCCP:
> > > +	case IPPROTO_SCTP: {
> > >  		const __be16 *pptr;
> > >  		__be16 _ports[2];
> > >  
> > >  		pptr = skb_header_pointer(skb, offset,
> > > sizeof(_ports), _ports); if (pptr == NULL) {
> > > -			audit_log_format(ab, " truncated=1");
> > > +			apar->ptrunc = 1;
> > >  			return;
> > >  		}
> > > +		apar->sport = ntohs(pptr[0]);
> > > +		apar->dport = ntohs(pptr[1]);
> > >  
> > > -		audit_log_format(ab, " sport=%hu dport=%hu",
> > > -				 ntohs(pptr[0]), ntohs(pptr[1]));
> > >  		}
> > >  		break;
> > >  
> > > @@ -59,41 +76,43 @@ static void audit_proto(struct audit_buffer *ab,
> > > struct sk_buff *skb, 
> > >  		iptr = skb_header_pointer(skb, offset, sizeof(_ih),
> > > &_ih); if (iptr == NULL) {
> > > -			audit_log_format(ab, " truncated=1");
> > > +			apar->ptrunc = 1;
> > >  			return;
> > >  		}
> > > -
> > > -		audit_log_format(ab, " icmptype=%hhu icmpcode=%hhu",
> > > -				 iptr[0], iptr[1]);
> > > +		apar->icmpt = iptr[0];
> > > +		apar->icmpc = iptr[1];
> > >  
> > >  		}
> > >  		break;
> > >  	}
> > >  }
> > >  
> > > -static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb)
> > > +static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb,
> > > struct nfpkt_par *apar) {
> > >  	struct iphdr _iph;
> > >  	const struct iphdr *ih;
> > >  
> > > +	apar->ipv = 4;
> > >  	ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph);
> > >  	if (!ih) {
> > > -		audit_log_format(ab, " truncated=1");
> > > +		apar->iptrunc = 1;
> > >  		return;
> > >  	}
> > >  
> > > -	audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu
> > > proto=%hhu",
> > > -		&ih->saddr, &ih->daddr, ntohs(ih->id), ih->protocol);
> > > +	apar->saddr = &ih->saddr;
> > > +	apar->daddr = &ih->daddr;
> > > +	apar->ipid = ntohs(ih->id);
> > > +	apar->proto = ih->protocol;
> > >  
> > >  	if (ntohs(ih->frag_off) & IP_OFFSET) {
> > > -		audit_log_format(ab, " frag=1");
> > > +		apar->frag = 1;
> > >  		return;
> > >  	}
> > >  
> > > -	audit_proto(ab, skb, ih->protocol, ih->ihl * 4);
> > > +	audit_proto(ab, skb, ih->protocol, ih->ihl * 4, apar);
> > >  }
> > >  
> > > -static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
> > > +static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb,
> > > struct nfpkt_par *apar) {
> > >  	struct ipv6hdr _ip6h;
> > >  	const struct ipv6hdr *ih;
> > > @@ -101,9 +120,10 @@ static void audit_ip6(struct audit_buffer *ab,
> > > struct sk_buff *skb) __be16 frag_off;
> > >  	int offset;
> > >  
> > > +	apar->ipv = 6;
> > >  	ih = skb_header_pointer(skb, skb_network_offset(skb),
> > > sizeof(_ip6h), &_ip6h); if (!ih) {
> > > -		audit_log_format(ab, " truncated=1");
> > > +		apar->iptrunc = 1;
> > >  		return;
> > >  	}
> > >  
> > > @@ -111,11 +131,12 @@ static void audit_ip6(struct audit_buffer *ab,
> > > struct sk_buff *skb) offset = ipv6_skip_exthdr(skb,
> > > skb_network_offset(skb) + sizeof(_ip6h), &nexthdr, &frag_off);
> > >  
> > > -	audit_log_format(ab, " saddr=%pI6c daddr=%pI6c proto=%hhu",
> > > -			 &ih->saddr, &ih->daddr, nexthdr);
> > > +	apar->saddr = &ih->saddr;
> > > +	apar->daddr = &ih->daddr;
> > > +	apar->proto = nexthdr;
> > >  
> > >  	if (offset)
> > > -		audit_proto(ab, skb, nexthdr, offset);
> > > +		audit_proto(ab, skb, nexthdr, offset, apar);
> > >  }
> > >  
> > >  static unsigned int
> > > @@ -123,6 +144,9 @@ audit_tg(struct sk_buff *skb, const struct
> > > xt_action_param *par) {
> > >  	const struct xt_audit_info *info = par->targinfo;
> > >  	struct audit_buffer *ab;
> > > +	struct nfpkt_par apar = {
> > > +		-1, -1, NULL, NULL, -1, -1, -1, -1, -1, -1, -1, -1
> > > +	};
> > >  
> > >  	if (audit_enabled == 0)
> > >  		goto errout;
> > > @@ -136,8 +160,7 @@ audit_tg(struct sk_buff *skb, const struct
> > > xt_action_param *par) par->in ? par->in->name : "?",
> > >  			 par->out ? par->out->name : "?");
> > >  
> > > -	if (skb->mark)
> > > -		audit_log_format(ab, " mark=%#x", skb->mark);
> > > +	audit_log_format(ab, " mark=%#x", skb->mark ?: -1);
> > >  
> > >  	if (skb->dev && skb->dev->type == ARPHRD_ETHER) {
> > >  		audit_log_format(ab, " smac=%pM dmac=%pM
> > > macproto=0x%04x", @@ -147,25 +170,42 @@ audit_tg(struct sk_buff *skb,
> > > const struct xt_action_param *par) if (par->family == NFPROTO_BRIDGE)
> > > { switch (eth_hdr(skb)->h_proto) {
> > >  			case htons(ETH_P_IP):
> > > -				audit_ip4(ab, skb);
> > > +				audit_ip4(ab, skb, &apar);
> > >  				break;
> > >  
> > >  			case htons(ETH_P_IPV6):
> > > -				audit_ip6(ab, skb);
> > > +				audit_ip6(ab, skb, &apar);
> > >  				break;
> > >  			}
> > >  		}
> > > +	} else {
> > > +		audit_log_format(ab, " smac=? dmac=?
> > > macproto=0xffff"); }
> > >  
> > >  	switch (par->family) {
> > >  	case NFPROTO_IPV4:
> > > -		audit_ip4(ab, skb);
> > > +		audit_ip4(ab, skb, &apar);
> > >  		break;
> > >  
> > >  	case NFPROTO_IPV6:
> > > -		audit_ip6(ab, skb);
> > > +		audit_ip6(ab, skb, &apar);
> > > +		break;
> > > +	}
> > > +
> > > +	switch (apar.ipv) {
> > > +	case 4:
> > > +		audit_log_format(ab, " trunc=%d saddr=%pI4
> > > daddr=%pI4 ipid=%hu proto=%hhu frag=%d",
> > > +			 apar.iptrunc, apar.saddr, apar.daddr,
> > > apar.ipid, apar.proto, apar.frag);
> > > +		break;
> > > +	case 6:
> > > +		audit_log_format(ab, " trunc=%d saddr=%pI6c
> > > daddr=%pI6c ipid=-1 proto=%hhu frag=-1",
> > > +			 apar.iptrunc, apar.saddr, apar.daddr,
> > > apar.proto); break;
> > > +	default:
> > > +		audit_log_format(ab, " trunc=-1 saddr=? daddr=?
> > > ipid=-1 proto=-1 frag=-1"); }
> > > +	audit_log_format(ab, " trunc=%d sport=%hu dport=%hu
> > > icmptype=%hhu icmpcode=%hhu",
> > > +		apar.ptrunc, apar.sport, apar.dport, apar.icmpt,
> > > apar.icmpc); 
> > >  #ifdef CONFIG_NETWORK_SECMARK
> > >  	if (skb->secmark)
> > 
> 
> - RGB
> 
> --
> Richard Guy Briggs <rgb@redhat.com>
> Kernel Security Engineering, Base Operating Systems, Red Hat
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635, Internal: (81) 32635

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

Re: [RFC PATCH] audit: normalize NETFILTER_PKT

[Steve Grubb]

On Tue, 31 Jan 2017 07:57:23 -0500
Richard Guy Briggs <rgb@redhat.com> wrote:

> On 2017-01-30 10:13, Richard Guy Briggs wrote:
> > On 2017-01-30 15:53, Steve Grubb wrote:  
> > > On Fri, 27 Jan 2017 08:11:06 -0500
> > > Richard Guy Briggs <rgb@redhat.com> wrote:  
> > > > Eliminate flipping in and out of message fields.
> > > > 
> > > > https://github.com/linux-audit/audit-kernel/issues/11  
> > > 
> > > Do you have sample events that shows how this changes the record
> > > format? I like to review how the event looks when a patch changes
> > > or adds a record.  
> > 
> > I used the format that was proposed.  Here are several samples from
> > running this RFC patch through the RFC test script:
> > 
> > 	ausearch --start 01/27/2017 -i -m netfilter_pkt
> > ----
> > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.649:183) :
> > action=ACCEPT hook=OUTPUT len=84 inif=? outif=lo mark=0xa044a1d4
> > smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1
> > daddr=127.0.0.1 ipid=26581 proto=icmp frag=255 trunc=-1 sport=65535
> > dport=65535 icmptype=echo icmpcode=0 ---- type=NETFILTER_PKT
> > msg=audit(01/27/2017 08:03:35.649:184) : action=ACCEPT hook=INPUT
> > len=84 inif=lo outif=? mark=0xbadeac28 smac=? dmac=?
> > macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1
> > ipid=26581 proto=icmp frag=255 trunc=-1 sport=65535 dport=65535
> > icmptype=echo icmpcode=0 ---- type=NETFILTER_PKT
> > msg=audit(01/27/2017 08:03:35.652:185) : action=ACCEPT hook=INPUT
> > len=104 inif=lo outif=? mark=0xb404724 smac=? dmac=?
> > macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1
> > proto=ipv6-icmp frag=-1 trunc=-1 sport=65535 dport=65535
> > icmptype=unknown icmp type (128) icmpcode=0 ---- type=NETFILTER_PKT
> > msg=audit(01/27/2017 08:03:35.655:186) : action=ACCEPT hook=INPUT
> > len=60 inif=lo outif=? mark=0xe2bd8098 smac=? dmac=?
> > macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1
> > ipid=29381 proto=tcp frag=255 trunc=-1 sport=51064 dport=42424
> > icmptype=unknown icmp type (255) icmpcode=255 ----
> > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.657:187) :
> > action=ACCEPT hook=INPUT len=80 inif=lo outif=? mark=0xf80a9dd7
> > smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1
> > proto=tcp frag=-1 trunc=-1 sport=38188 dport=42424 icmptype=unknown
> > icmp type (255) icmpcode=255 ---- type=NETFILTER_PKT
> > msg=audit(01/27/2017 08:03:35.659:188) : action=ACCEPT hook=INPUT
> > len=31 inif=lo outif=? mark=0xa6d8d4ac smac=? dmac=?
> > macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1
> > ipid=46304 proto=udp frag=255 trunc=-1 sport=60095 dport=42424
> > icmptype=unknown icmp type (255) icmpcode=255 ----
> > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.661:189) :
> > action=ACCEPT hook=INPUT len=51 inif=lo outif=? mark=0x3f0d6054
> > smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1
> > proto=udp frag=-1 trunc=-1 sport=43818 dport=42424 icmptype=unknown
> > icmp type (255) icmpcode=255 ----  
> 
> Here are the raw messages:
> 
> ----
> time->Fri Jan 27 08:03:35 2017
> type=NETFILTER_PKT msg=audit(1485522215.649:183): action=0 hook=3
> len=84 inif=? outif=lo mark=0xa044a1d4 smac=? dmac=? macproto=0xffff
> trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=1 frag=255
> trunc=-1 sport=65535 dport=65535 icmptype=8 icmpcode=0 ---- time->Fri
> Jan 27 08:03:35 2017 type=NETFILTER_PKT
> msg=audit(1485522215.649:184): action=0 hook=1 len=84 inif=lo outif=?
> mark=0xbadeac28 smac=? dmac=? macproto=0xffff trunc=-1
> saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=1 frag=255 trunc=-1
> sport=65535 dport=65535 icmptype=8 icmpcode=0 ---- time->Fri Jan 27
> 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.652:185):
> action=0 hook=1 len=104 inif=lo outif=? mark=0xb404724 smac=? dmac=?
> macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=58 frag=-1
> trunc=-1 sport=65535 dport=65535 icmptype=128 icmpcode=0 ----
> time->Fri Jan 27 08:03:35 2017 type=NETFILTER_PKT
> msg=audit(1485522215.655:186): action=0 hook=1 len=60 inif=lo outif=?
> mark=0xe2bd8098 smac=? dmac=? macproto=0xffff trunc=-1
> saddr=127.0.0.1 daddr=127.0.0.1 ipid=29381 proto=6 frag=255 trunc=-1
> sport=51064 dport=42424 icmptype=255 icmpcode=255 ---- time->Fri Jan
> 27 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.657:187):
> action=0 hook=1 len=80 inif=lo outif=? mark=0xf80a9dd7 smac=? dmac=?
> macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=6 frag=-1
> trunc=-1 sport=38188 dport=42424 icmptype=255 icmpcode=255 ----
> time->Fri Jan 27 08:03:35 2017 type=NETFILTER_PKT
> msg=audit(1485522215.659:188): action=0 hook=1 len=31 inif=lo outif=?
> mark=0xa6d8d4ac smac=? dmac=? macproto=0xffff trunc=-1
> saddr=127.0.0.1 daddr=127.0.0.1 ipid=46304 proto=17 frag=255 trunc=-1
> sport=60095 dport=42424 icmptype=255 icmpcode=255 ---- time->Fri Jan
> 27 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.661:189):
> action=0 hook=1 len=51 inif=lo outif=? mark=0x3f0d6054 smac=? dmac=?
> macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=17 frag=-1
> trunc=-1 sport=43818 dport=42424 icmptype=255 icmpcode=255 ----

I was curious about something. Auparse is trying to interpret the
icmptype field for every event. This is not good. Which fields are
valid for each kind of packet? Are there fields valid for all packets?
Is the icmptype/code the only ones that don't apply in all cases?

Thanks,
-Steve


> > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > > ---
> > > >  net/netfilter/xt_AUDIT.c |   92
> > > > +++++++++++++++++++++++++++++++++------------- 1 files changed,
> > > > 66 insertions(+), 26 deletions(-)
> > > > 
> > > > diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
> > > > index 4973cbd..8089ec2 100644
> > > > --- a/net/netfilter/xt_AUDIT.c
> > > > +++ b/net/netfilter/xt_AUDIT.c
> > > > @@ -31,24 +31,41 @@ MODULE_ALIAS("ip6t_AUDIT");
> > > >  MODULE_ALIAS("ebt_AUDIT");
> > > >  MODULE_ALIAS("arpt_AUDIT");
> > > >  
> > > > +struct nfpkt_par {
> > > > +	int ipv;
> > > > +	int iptrunc;
> > > > +	const void *saddr;
> > > > +	const void *daddr;
> > > > +	u16 ipid;
> > > > +	u8 proto;
> > > > +	u8 frag;
> > > > +	int ptrunc;
> > > > +	u16 sport;
> > > > +	u16 dport;
> > > > +	u8 icmpt;
> > > > +	u8 icmpc;
> > > > +};
> > > > +
> > > >  static void audit_proto(struct audit_buffer *ab, struct
> > > > sk_buff *skb,
> > > > -			unsigned int proto, unsigned int
> > > > offset)
> > > > +			unsigned int proto, unsigned int
> > > > offset, struct nfpkt_par *apar) {
> > > >  	switch (proto) {
> > > >  	case IPPROTO_TCP:
> > > >  	case IPPROTO_UDP:
> > > > -	case IPPROTO_UDPLITE: {
> > > > +	case IPPROTO_UDPLITE:
> > > > +	case IPPROTO_DCCP:
> > > > +	case IPPROTO_SCTP: {
> > > >  		const __be16 *pptr;
> > > >  		__be16 _ports[2];
> > > >  
> > > >  		pptr = skb_header_pointer(skb, offset,
> > > > sizeof(_ports), _ports); if (pptr == NULL) {
> > > > -			audit_log_format(ab, " truncated=1");
> > > > +			apar->ptrunc = 1;
> > > >  			return;
> > > >  		}
> > > > +		apar->sport = ntohs(pptr[0]);
> > > > +		apar->dport = ntohs(pptr[1]);
> > > >  
> > > > -		audit_log_format(ab, " sport=%hu dport=%hu",
> > > > -				 ntohs(pptr[0]),
> > > > ntohs(pptr[1])); }
> > > >  		break;
> > > >  
> > > > @@ -59,41 +76,43 @@ static void audit_proto(struct audit_buffer
> > > > *ab, struct sk_buff *skb, 
> > > >  		iptr = skb_header_pointer(skb, offset,
> > > > sizeof(_ih), &_ih); if (iptr == NULL) {
> > > > -			audit_log_format(ab, " truncated=1");
> > > > +			apar->ptrunc = 1;
> > > >  			return;
> > > >  		}
> > > > -
> > > > -		audit_log_format(ab, " icmptype=%hhu
> > > > icmpcode=%hhu",
> > > > -				 iptr[0], iptr[1]);
> > > > +		apar->icmpt = iptr[0];
> > > > +		apar->icmpc = iptr[1];
> > > >  
> > > >  		}
> > > >  		break;
> > > >  	}
> > > >  }
> > > >  
> > > > -static void audit_ip4(struct audit_buffer *ab, struct sk_buff
> > > > *skb) +static void audit_ip4(struct audit_buffer *ab, struct
> > > > sk_buff *skb, struct nfpkt_par *apar) {
> > > >  	struct iphdr _iph;
> > > >  	const struct iphdr *ih;
> > > >  
> > > > +	apar->ipv = 4;
> > > >  	ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph);
> > > >  	if (!ih) {
> > > > -		audit_log_format(ab, " truncated=1");
> > > > +		apar->iptrunc = 1;
> > > >  		return;
> > > >  	}
> > > >  
> > > > -	audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu
> > > > proto=%hhu",
> > > > -		&ih->saddr, &ih->daddr, ntohs(ih->id),
> > > > ih->protocol);
> > > > +	apar->saddr = &ih->saddr;
> > > > +	apar->daddr = &ih->daddr;
> > > > +	apar->ipid = ntohs(ih->id);
> > > > +	apar->proto = ih->protocol;
> > > >  
> > > >  	if (ntohs(ih->frag_off) & IP_OFFSET) {
> > > > -		audit_log_format(ab, " frag=1");
> > > > +		apar->frag = 1;
> > > >  		return;
> > > >  	}
> > > >  
> > > > -	audit_proto(ab, skb, ih->protocol, ih->ihl * 4);
> > > > +	audit_proto(ab, skb, ih->protocol, ih->ihl * 4, apar);
> > > >  }
> > > >  
> > > > -static void audit_ip6(struct audit_buffer *ab, struct sk_buff
> > > > *skb) +static void audit_ip6(struct audit_buffer *ab, struct
> > > > sk_buff *skb, struct nfpkt_par *apar) {
> > > >  	struct ipv6hdr _ip6h;
> > > >  	const struct ipv6hdr *ih;
> > > > @@ -101,9 +120,10 @@ static void audit_ip6(struct audit_buffer
> > > > *ab, struct sk_buff *skb) __be16 frag_off;
> > > >  	int offset;
> > > >  
> > > > +	apar->ipv = 6;
> > > >  	ih = skb_header_pointer(skb, skb_network_offset(skb),
> > > > sizeof(_ip6h), &_ip6h); if (!ih) {
> > > > -		audit_log_format(ab, " truncated=1");
> > > > +		apar->iptrunc = 1;
> > > >  		return;
> > > >  	}
> > > >  
> > > > @@ -111,11 +131,12 @@ static void audit_ip6(struct audit_buffer
> > > > *ab, struct sk_buff *skb) offset = ipv6_skip_exthdr(skb,
> > > > skb_network_offset(skb) + sizeof(_ip6h), &nexthdr, &frag_off);
> > > >  
> > > > -	audit_log_format(ab, " saddr=%pI6c daddr=%pI6c
> > > > proto=%hhu",
> > > > -			 &ih->saddr, &ih->daddr, nexthdr);
> > > > +	apar->saddr = &ih->saddr;
> > > > +	apar->daddr = &ih->daddr;
> > > > +	apar->proto = nexthdr;
> > > >  
> > > >  	if (offset)
> > > > -		audit_proto(ab, skb, nexthdr, offset);
> > > > +		audit_proto(ab, skb, nexthdr, offset, apar);
> > > >  }
> > > >  
> > > >  static unsigned int
> > > > @@ -123,6 +144,9 @@ audit_tg(struct sk_buff *skb, const struct
> > > > xt_action_param *par) {
> > > >  	const struct xt_audit_info *info = par->targinfo;
> > > >  	struct audit_buffer *ab;
> > > > +	struct nfpkt_par apar = {
> > > > +		-1, -1, NULL, NULL, -1, -1, -1, -1, -1, -1,
> > > > -1, -1
> > > > +	};
> > > >  
> > > >  	if (audit_enabled == 0)
> > > >  		goto errout;
> > > > @@ -136,8 +160,7 @@ audit_tg(struct sk_buff *skb, const struct
> > > > xt_action_param *par) par->in ? par->in->name : "?",
> > > >  			 par->out ? par->out->name : "?");
> > > >  
> > > > -	if (skb->mark)
> > > > -		audit_log_format(ab, " mark=%#x", skb->mark);
> > > > +	audit_log_format(ab, " mark=%#x", skb->mark ?: -1);
> > > >  
> > > >  	if (skb->dev && skb->dev->type == ARPHRD_ETHER) {
> > > >  		audit_log_format(ab, " smac=%pM dmac=%pM
> > > > macproto=0x%04x", @@ -147,25 +170,42 @@ audit_tg(struct sk_buff
> > > > *skb, const struct xt_action_param *par) if (par->family ==
> > > > NFPROTO_BRIDGE) { switch (eth_hdr(skb)->h_proto) {
> > > >  			case htons(ETH_P_IP):
> > > > -				audit_ip4(ab, skb);
> > > > +				audit_ip4(ab, skb, &apar);
> > > >  				break;
> > > >  
> > > >  			case htons(ETH_P_IPV6):
> > > > -				audit_ip6(ab, skb);
> > > > +				audit_ip6(ab, skb, &apar);
> > > >  				break;
> > > >  			}
> > > >  		}
> > > > +	} else {
> > > > +		audit_log_format(ab, " smac=? dmac=?
> > > > macproto=0xffff"); }
> > > >  
> > > >  	switch (par->family) {
> > > >  	case NFPROTO_IPV4:
> > > > -		audit_ip4(ab, skb);
> > > > +		audit_ip4(ab, skb, &apar);
> > > >  		break;
> > > >  
> > > >  	case NFPROTO_IPV6:
> > > > -		audit_ip6(ab, skb);
> > > > +		audit_ip6(ab, skb, &apar);
> > > > +		break;
> > > > +	}
> > > > +
> > > > +	switch (apar.ipv) {
> > > > +	case 4:
> > > > +		audit_log_format(ab, " trunc=%d saddr=%pI4
> > > > daddr=%pI4 ipid=%hu proto=%hhu frag=%d",
> > > > +			 apar.iptrunc, apar.saddr, apar.daddr,
> > > > apar.ipid, apar.proto, apar.frag);
> > > > +		break;
> > > > +	case 6:
> > > > +		audit_log_format(ab, " trunc=%d saddr=%pI6c
> > > > daddr=%pI6c ipid=-1 proto=%hhu frag=-1",
> > > > +			 apar.iptrunc, apar.saddr, apar.daddr,
> > > > apar.proto); break;
> > > > +	default:
> > > > +		audit_log_format(ab, " trunc=-1 saddr=? daddr=?
> > > > ipid=-1 proto=-1 frag=-1"); }
> > > > +	audit_log_format(ab, " trunc=%d sport=%hu dport=%hu
> > > > icmptype=%hhu icmpcode=%hhu",
> > > > +		apar.ptrunc, apar.sport, apar.dport,
> > > > apar.icmpt, apar.icmpc); 
> > > >  #ifdef CONFIG_NETWORK_SECMARK
> > > >  	if (skb->secmark)  
> > >   
> > 
> > - RGB
> > 
> > --
> > Richard Guy Briggs <rgb@redhat.com>
> > Kernel Security Engineering, Base Operating Systems, Red Hat
> > Remote, Ottawa, Canada
> > Voice: +1.647.777.2635, Internal: (81) 32635  
> 
> - RGB
> 
> --
> Richard Guy Briggs <rgb@redhat.com>
> Kernel Security Engineering, Base Operating Systems, Red Hat
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635, Internal: (81) 32635

Re: [RFC PATCH] audit: normalize NETFILTER_PKT

[Richard Guy Briggs]

On 2017-01-31 17:13, Steve Grubb wrote:
> On Tue, 31 Jan 2017 07:57:23 -0500
> Richard Guy Briggs <rgb@redhat.com> wrote:
> 
> > On 2017-01-30 10:13, Richard Guy Briggs wrote:
> > > On 2017-01-30 15:53, Steve Grubb wrote:  
> > > > On Fri, 27 Jan 2017 08:11:06 -0500
> > > > Richard Guy Briggs <rgb@redhat.com> wrote:  
> > > > > Eliminate flipping in and out of message fields.
> > > > > 
> > > > > https://github.com/linux-audit/audit-kernel/issues/11  
> > > > 
> > > > Do you have sample events that shows how this changes the record
> > > > format? I like to review how the event looks when a patch changes
> > > > or adds a record.  
> > > 
> > > I used the format that was proposed.  Here are several samples from
> > > running this RFC patch through the RFC test script:
> > > 
> > > 	ausearch --start 01/27/2017 -i -m netfilter_pkt
> > > ----
> > > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.649:183) :
> > > action=ACCEPT hook=OUTPUT len=84 inif=? outif=lo mark=0xa044a1d4
> > > smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1
> > > daddr=127.0.0.1 ipid=26581 proto=icmp frag=255 trunc=-1 sport=65535
> > > dport=65535 icmptype=echo icmpcode=0 ---- type=NETFILTER_PKT
> > > msg=audit(01/27/2017 08:03:35.649:184) : action=ACCEPT hook=INPUT
> > > len=84 inif=lo outif=? mark=0xbadeac28 smac=? dmac=?
> > > macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1
> > > ipid=26581 proto=icmp frag=255 trunc=-1 sport=65535 dport=65535
> > > icmptype=echo icmpcode=0 ---- type=NETFILTER_PKT
> > > msg=audit(01/27/2017 08:03:35.652:185) : action=ACCEPT hook=INPUT
> > > len=104 inif=lo outif=? mark=0xb404724 smac=? dmac=?
> > > macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1
> > > proto=ipv6-icmp frag=-1 trunc=-1 sport=65535 dport=65535
> > > icmptype=unknown icmp type (128) icmpcode=0 ---- type=NETFILTER_PKT
> > > msg=audit(01/27/2017 08:03:35.655:186) : action=ACCEPT hook=INPUT
> > > len=60 inif=lo outif=? mark=0xe2bd8098 smac=? dmac=?
> > > macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1
> > > ipid=29381 proto=tcp frag=255 trunc=-1 sport=51064 dport=42424
> > > icmptype=unknown icmp type (255) icmpcode=255 ----
> > > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.657:187) :
> > > action=ACCEPT hook=INPUT len=80 inif=lo outif=? mark=0xf80a9dd7
> > > smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1
> > > proto=tcp frag=-1 trunc=-1 sport=38188 dport=42424 icmptype=unknown
> > > icmp type (255) icmpcode=255 ---- type=NETFILTER_PKT
> > > msg=audit(01/27/2017 08:03:35.659:188) : action=ACCEPT hook=INPUT
> > > len=31 inif=lo outif=? mark=0xa6d8d4ac smac=? dmac=?
> > > macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1
> > > ipid=46304 proto=udp frag=255 trunc=-1 sport=60095 dport=42424
> > > icmptype=unknown icmp type (255) icmpcode=255 ----
> > > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.661:189) :
> > > action=ACCEPT hook=INPUT len=51 inif=lo outif=? mark=0x3f0d6054
> > > smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1
> > > proto=udp frag=-1 trunc=-1 sport=43818 dport=42424 icmptype=unknown
> > > icmp type (255) icmpcode=255 ----  
> > 
> > Here are the raw messages:
> > 
> > ----
> > time->Fri Jan 27 08:03:35 2017
> > type=NETFILTER_PKT msg=audit(1485522215.649:183): action=0 hook=3
> > len=84 inif=? outif=lo mark=0xa044a1d4 smac=? dmac=? macproto=0xffff
> > trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=1 frag=255
> > trunc=-1 sport=65535 dport=65535 icmptype=8 icmpcode=0 ---- time->Fri
> > Jan 27 08:03:35 2017 type=NETFILTER_PKT
> > msg=audit(1485522215.649:184): action=0 hook=1 len=84 inif=lo outif=?
> > mark=0xbadeac28 smac=? dmac=? macproto=0xffff trunc=-1
> > saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=1 frag=255 trunc=-1
> > sport=65535 dport=65535 icmptype=8 icmpcode=0 ---- time->Fri Jan 27
> > 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.652:185):
> > action=0 hook=1 len=104 inif=lo outif=? mark=0xb404724 smac=? dmac=?
> > macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=58 frag=-1
> > trunc=-1 sport=65535 dport=65535 icmptype=128 icmpcode=0 ----
> > time->Fri Jan 27 08:03:35 2017 type=NETFILTER_PKT
> > msg=audit(1485522215.655:186): action=0 hook=1 len=60 inif=lo outif=?
> > mark=0xe2bd8098 smac=? dmac=? macproto=0xffff trunc=-1
> > saddr=127.0.0.1 daddr=127.0.0.1 ipid=29381 proto=6 frag=255 trunc=-1
> > sport=51064 dport=42424 icmptype=255 icmpcode=255 ---- time->Fri Jan
> > 27 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.657:187):
> > action=0 hook=1 len=80 inif=lo outif=? mark=0xf80a9dd7 smac=? dmac=?
> > macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=6 frag=-1
> > trunc=-1 sport=38188 dport=42424 icmptype=255 icmpcode=255 ----
> > time->Fri Jan 27 08:03:35 2017 type=NETFILTER_PKT
> > msg=audit(1485522215.659:188): action=0 hook=1 len=31 inif=lo outif=?
> > mark=0xa6d8d4ac smac=? dmac=? macproto=0xffff trunc=-1
> > saddr=127.0.0.1 daddr=127.0.0.1 ipid=46304 proto=17 frag=255 trunc=-1
> > sport=60095 dport=42424 icmptype=255 icmpcode=255 ---- time->Fri Jan
> > 27 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.661:189):
> > action=0 hook=1 len=51 inif=lo outif=? mark=0x3f0d6054 smac=? dmac=?
> > macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=17 frag=-1
> > trunc=-1 sport=43818 dport=42424 icmptype=255 icmpcode=255 ----
> 
> I was curious about something. Auparse is trying to interpret the
> icmptype field for every event. This is not good. Which fields are
> valid for each kind of packet? Are there fields valid for all packets?
> Is the icmptype/code the only ones that don't apply in all cases?

Ok, this is important to know...  You sound surprised.  So if that field
isn't valid for all cases of that event, then the event should be split
or the "unset" value should be used as a hint to ignore it.

This was the point of my earlier posting:
	https://www.redhat.com/archives/linux-audit/2017-January/msg00074.html
There are still a number of questions from that thread that had no
reply.  Answering those questions would help inform this discussion, so
if you could answer some of those questions in that first thread, I'd
have a better chance of understanding what are the limitations of the
parser and design/work around them.

There is no packet for which all fields are valid.  This is why using
"unset" values in those fields was suggested, seemed to be accepted in
discussion, and implemented.

ICMP codes are obviously not valid on TCP or UDP packets, so maybe there
should be a new message type(s?) such as NETFILTER_PKT_DATA for
TCP/UDP/DCCP/SCTP and/or NETFILTER_PKT_CTRL for ICMP.

For IPv4 vs IPv6 packets, IPv4 adds ipid= and frag= fields which are not
necessarily valid for IPv6.  Do you want
NETFILTER_PKT_IPV{4,6}_{DATA,CTRL} message types?

There's an optional ethernet bridge.  Should messages with those fields
use a new message type so that we have:
NETFILTER_PKT{,_MAC}_IPV{4,6}_{DATA,CTRL} giving us 8 message types.

There are other combinations too that could explode that message type
count including non-TCP/UDP/DCCP/SCTP/ICMP transport layer protocols and
even network layer protocols.

Swinging fields in and out makes it very handy to use one message type
for all of them and can save precious disk bandwidth, but the point was
to normalize these messages.  Is that still realistic and necessary?  If
so, we're trying to find a balance between message type explosion and
disk bandwidth.

We either need to make this fine-grained, ignore fields that aren't
valid for that type, or swing fields in and out.  Or maybe I have missed
something fundamental, such as the presence of subsequent fields depends
on the values of previous fields?

> -Steve
> 
> > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > > > ---
> > > > >  net/netfilter/xt_AUDIT.c |   92
> > > > > +++++++++++++++++++++++++++++++++------------- 1 files changed,
> > > > > 66 insertions(+), 26 deletions(-)
> > > > > 
> > > > > diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
> > > > > index 4973cbd..8089ec2 100644
> > > > > --- a/net/netfilter/xt_AUDIT.c
> > > > > +++ b/net/netfilter/xt_AUDIT.c
> > > > > @@ -31,24 +31,41 @@ MODULE_ALIAS("ip6t_AUDIT");
> > > > >  MODULE_ALIAS("ebt_AUDIT");
> > > > >  MODULE_ALIAS("arpt_AUDIT");
> > > > >  
> > > > > +struct nfpkt_par {
> > > > > +	int ipv;
> > > > > +	int iptrunc;
> > > > > +	const void *saddr;
> > > > > +	const void *daddr;
> > > > > +	u16 ipid;
> > > > > +	u8 proto;
> > > > > +	u8 frag;
> > > > > +	int ptrunc;
> > > > > +	u16 sport;
> > > > > +	u16 dport;
> > > > > +	u8 icmpt;
> > > > > +	u8 icmpc;
> > > > > +};
> > > > > +
> > > > >  static void audit_proto(struct audit_buffer *ab, struct
> > > > > sk_buff *skb,
> > > > > -			unsigned int proto, unsigned int
> > > > > offset)
> > > > > +			unsigned int proto, unsigned int
> > > > > offset, struct nfpkt_par *apar) {
> > > > >  	switch (proto) {
> > > > >  	case IPPROTO_TCP:
> > > > >  	case IPPROTO_UDP:
> > > > > -	case IPPROTO_UDPLITE: {
> > > > > +	case IPPROTO_UDPLITE:
> > > > > +	case IPPROTO_DCCP:
> > > > > +	case IPPROTO_SCTP: {
> > > > >  		const __be16 *pptr;
> > > > >  		__be16 _ports[2];
> > > > >  
> > > > >  		pptr = skb_header_pointer(skb, offset,
> > > > > sizeof(_ports), _ports); if (pptr == NULL) {
> > > > > -			audit_log_format(ab, " truncated=1");
> > > > > +			apar->ptrunc = 1;
> > > > >  			return;
> > > > >  		}
> > > > > +		apar->sport = ntohs(pptr[0]);
> > > > > +		apar->dport = ntohs(pptr[1]);
> > > > >  
> > > > > -		audit_log_format(ab, " sport=%hu dport=%hu",
> > > > > -				 ntohs(pptr[0]),
> > > > > ntohs(pptr[1])); }
> > > > >  		break;
> > > > >  
> > > > > @@ -59,41 +76,43 @@ static void audit_proto(struct audit_buffer
> > > > > *ab, struct sk_buff *skb, 
> > > > >  		iptr = skb_header_pointer(skb, offset,
> > > > > sizeof(_ih), &_ih); if (iptr == NULL) {
> > > > > -			audit_log_format(ab, " truncated=1");
> > > > > +			apar->ptrunc = 1;
> > > > >  			return;
> > > > >  		}
> > > > > -
> > > > > -		audit_log_format(ab, " icmptype=%hhu
> > > > > icmpcode=%hhu",
> > > > > -				 iptr[0], iptr[1]);
> > > > > +		apar->icmpt = iptr[0];
> > > > > +		apar->icmpc = iptr[1];
> > > > >  
> > > > >  		}
> > > > >  		break;
> > > > >  	}
> > > > >  }
> > > > >  
> > > > > -static void audit_ip4(struct audit_buffer *ab, struct sk_buff
> > > > > *skb) +static void audit_ip4(struct audit_buffer *ab, struct
> > > > > sk_buff *skb, struct nfpkt_par *apar) {
> > > > >  	struct iphdr _iph;
> > > > >  	const struct iphdr *ih;
> > > > >  
> > > > > +	apar->ipv = 4;
> > > > >  	ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph);
> > > > >  	if (!ih) {
> > > > > -		audit_log_format(ab, " truncated=1");
> > > > > +		apar->iptrunc = 1;
> > > > >  		return;
> > > > >  	}
> > > > >  
> > > > > -	audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu
> > > > > proto=%hhu",
> > > > > -		&ih->saddr, &ih->daddr, ntohs(ih->id),
> > > > > ih->protocol);
> > > > > +	apar->saddr = &ih->saddr;
> > > > > +	apar->daddr = &ih->daddr;
> > > > > +	apar->ipid = ntohs(ih->id);
> > > > > +	apar->proto = ih->protocol;
> > > > >  
> > > > >  	if (ntohs(ih->frag_off) & IP_OFFSET) {
> > > > > -		audit_log_format(ab, " frag=1");
> > > > > +		apar->frag = 1;
> > > > >  		return;
> > > > >  	}
> > > > >  
> > > > > -	audit_proto(ab, skb, ih->protocol, ih->ihl * 4);
> > > > > +	audit_proto(ab, skb, ih->protocol, ih->ihl * 4, apar);
> > > > >  }
> > > > >  
> > > > > -static void audit_ip6(struct audit_buffer *ab, struct sk_buff
> > > > > *skb) +static void audit_ip6(struct audit_buffer *ab, struct
> > > > > sk_buff *skb, struct nfpkt_par *apar) {
> > > > >  	struct ipv6hdr _ip6h;
> > > > >  	const struct ipv6hdr *ih;
> > > > > @@ -101,9 +120,10 @@ static void audit_ip6(struct audit_buffer
> > > > > *ab, struct sk_buff *skb) __be16 frag_off;
> > > > >  	int offset;
> > > > >  
> > > > > +	apar->ipv = 6;
> > > > >  	ih = skb_header_pointer(skb, skb_network_offset(skb),
> > > > > sizeof(_ip6h), &_ip6h); if (!ih) {
> > > > > -		audit_log_format(ab, " truncated=1");
> > > > > +		apar->iptrunc = 1;
> > > > >  		return;
> > > > >  	}
> > > > >  
> > > > > @@ -111,11 +131,12 @@ static void audit_ip6(struct audit_buffer
> > > > > *ab, struct sk_buff *skb) offset = ipv6_skip_exthdr(skb,
> > > > > skb_network_offset(skb) + sizeof(_ip6h), &nexthdr, &frag_off);
> > > > >  
> > > > > -	audit_log_format(ab, " saddr=%pI6c daddr=%pI6c
> > > > > proto=%hhu",
> > > > > -			 &ih->saddr, &ih->daddr, nexthdr);
> > > > > +	apar->saddr = &ih->saddr;
> > > > > +	apar->daddr = &ih->daddr;
> > > > > +	apar->proto = nexthdr;
> > > > >  
> > > > >  	if (offset)
> > > > > -		audit_proto(ab, skb, nexthdr, offset);
> > > > > +		audit_proto(ab, skb, nexthdr, offset, apar);
> > > > >  }
> > > > >  
> > > > >  static unsigned int
> > > > > @@ -123,6 +144,9 @@ audit_tg(struct sk_buff *skb, const struct
> > > > > xt_action_param *par) {
> > > > >  	const struct xt_audit_info *info = par->targinfo;
> > > > >  	struct audit_buffer *ab;
> > > > > +	struct nfpkt_par apar = {
> > > > > +		-1, -1, NULL, NULL, -1, -1, -1, -1, -1, -1,
> > > > > -1, -1
> > > > > +	};
> > > > >  
> > > > >  	if (audit_enabled == 0)
> > > > >  		goto errout;
> > > > > @@ -136,8 +160,7 @@ audit_tg(struct sk_buff *skb, const struct
> > > > > xt_action_param *par) par->in ? par->in->name : "?",
> > > > >  			 par->out ? par->out->name : "?");
> > > > >  
> > > > > -	if (skb->mark)
> > > > > -		audit_log_format(ab, " mark=%#x", skb->mark);
> > > > > +	audit_log_format(ab, " mark=%#x", skb->mark ?: -1);
> > > > >  
> > > > >  	if (skb->dev && skb->dev->type == ARPHRD_ETHER) {
> > > > >  		audit_log_format(ab, " smac=%pM dmac=%pM
> > > > > macproto=0x%04x", @@ -147,25 +170,42 @@ audit_tg(struct sk_buff
> > > > > *skb, const struct xt_action_param *par) if (par->family ==
> > > > > NFPROTO_BRIDGE) { switch (eth_hdr(skb)->h_proto) {
> > > > >  			case htons(ETH_P_IP):
> > > > > -				audit_ip4(ab, skb);
> > > > > +				audit_ip4(ab, skb, &apar);
> > > > >  				break;
> > > > >  
> > > > >  			case htons(ETH_P_IPV6):
> > > > > -				audit_ip6(ab, skb);
> > > > > +				audit_ip6(ab, skb, &apar);
> > > > >  				break;
> > > > >  			}
> > > > >  		}
> > > > > +	} else {
> > > > > +		audit_log_format(ab, " smac=? dmac=?
> > > > > macproto=0xffff"); }
> > > > >  
> > > > >  	switch (par->family) {
> > > > >  	case NFPROTO_IPV4:
> > > > > -		audit_ip4(ab, skb);
> > > > > +		audit_ip4(ab, skb, &apar);
> > > > >  		break;
> > > > >  
> > > > >  	case NFPROTO_IPV6:
> > > > > -		audit_ip6(ab, skb);
> > > > > +		audit_ip6(ab, skb, &apar);
> > > > > +		break;
> > > > > +	}
> > > > > +
> > > > > +	switch (apar.ipv) {
> > > > > +	case 4:
> > > > > +		audit_log_format(ab, " trunc=%d saddr=%pI4
> > > > > daddr=%pI4 ipid=%hu proto=%hhu frag=%d",
> > > > > +			 apar.iptrunc, apar.saddr, apar.daddr,
> > > > > apar.ipid, apar.proto, apar.frag);
> > > > > +		break;
> > > > > +	case 6:
> > > > > +		audit_log_format(ab, " trunc=%d saddr=%pI6c
> > > > > daddr=%pI6c ipid=-1 proto=%hhu frag=-1",
> > > > > +			 apar.iptrunc, apar.saddr, apar.daddr,
> > > > > apar.proto); break;
> > > > > +	default:
> > > > > +		audit_log_format(ab, " trunc=-1 saddr=? daddr=?
> > > > > ipid=-1 proto=-1 frag=-1"); }
> > > > > +	audit_log_format(ab, " trunc=%d sport=%hu dport=%hu
> > > > > icmptype=%hhu icmpcode=%hhu",
> > > > > +		apar.ptrunc, apar.sport, apar.dport,
> > > > > apar.icmpt, apar.icmpc); 
> > > > >  #ifdef CONFIG_NETWORK_SECMARK
> > > > >  	if (skb->secmark)  
> > > >   
> > > 
> > > - RGB
> > > 
> > > --
> > > Richard Guy Briggs <rgb@redhat.com>
> > > Kernel Security Engineering, Base Operating Systems, Red Hat
> > > Remote, Ottawa, Canada
> > > Voice: +1.647.777.2635, Internal: (81) 32635  
> > 
> > - RGB
> > 
> > --
> > Richard Guy Briggs <rgb@redhat.com>
> > Kernel Security Engineering, Base Operating Systems, Red Hat
> > Remote, Ottawa, Canada
> > Voice: +1.647.777.2635, Internal: (81) 32635
> 

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

Re: [RFC PATCH] audit: normalize NETFILTER_PKT

[Paul Moore]

On Tue, Jan 31, 2017 at 2:44 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2017-01-31 17:13, Steve Grubb wrote:

...

>> I was curious about something. Auparse is trying to interpret the
>> icmptype field for every event. This is not good. Which fields are
>> valid for each kind of packet? Are there fields valid for all packets?
>> Is the icmptype/code the only ones that don't apply in all cases?
>
> Ok, this is important to know...  You sound surprised.  So if that field
> isn't valid for all cases of that event, then the event should be split
> or the "unset" value should be used as a hint to ignore it.
>
> This was the point of my earlier posting:
>         https://www.redhat.com/archives/linux-audit/2017-January/msg00074.html
> There are still a number of questions from that thread that had no
> reply.  Answering those questions would help inform this discussion, so
> if you could answer some of those questions in that first thread, I'd
> have a better chance of understanding what are the limitations of the
> parser and design/work around them.
>
> There is no packet for which all fields are valid.  This is why using
> "unset" values in those fields was suggested, seemed to be accepted in
> discussion, and implemented.

...

> Swinging fields in and out makes it very handy to use one message type
> for all of them and can save precious disk bandwidth, but the point was
> to normalize these messages.  Is that still realistic and necessary?  If
> so, we're trying to find a balance between message type explosion and
> disk bandwidth.
>
> We either need to make this fine-grained, ignore fields that aren't
> valid for that type, or swing fields in and out.  Or maybe I have missed
> something fundamental, such as the presence of subsequent fields depends
> on the values of previous fields?

I'm still trying to understand what purpose this record actually
serves, and what requirements may exist.  In an earlier thread
somewhere Steve mentioned some broad requirements around data
import/export, and I really wonder if the NETFILTER_PKT record
provides anything useful here when it really isn't connecting the
traffic to the sender/receiver without a lot of additional logging and
post-processing smarts.  If you were interested in data import/export
I think auditing the socket syscalls would provide a much more useful
set of records in the audit log.

Considering that one of the primary motivations for the audit
subsystem is to enable compliance with various security
specifications, let's get the ones we know about listed in this thread
and then figure out how best to meet those requirements.

-- 
paul moore
www.paul-moore.com

Re: [RFC PATCH] audit: normalize NETFILTER_PKT

[Steve Grubb]

On Friday, February 3, 2017 6:44:16 PM EST Paul Moore wrote:
> On Tue, Jan 31, 2017 at 2:44 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2017-01-31 17:13, Steve Grubb wrote:
> ...
> 
> >> I was curious about something. Auparse is trying to interpret the
> >> icmptype field for every event. This is not good. Which fields are
> >> valid for each kind of packet? Are there fields valid for all packets?
> >> Is the icmptype/code the only ones that don't apply in all cases?
> > 
> > Ok, this is important to know...  You sound surprised.  So if that field
> > isn't valid for all cases of that event, then the event should be split
> > or the "unset" value should be used as a hint to ignore it.
> > 
> > This was the point of my earlier posting:
> >         https://www.redhat.com/archives/linux-audit/2017-January/msg00074.
> >         html
> > 
> > There are still a number of questions from that thread that had no
> > reply.  Answering those questions would help inform this discussion, so
> > if you could answer some of those questions in that first thread, I'd
> > have a better chance of understanding what are the limitations of the
> > parser and design/work around them.
> > 
> > There is no packet for which all fields are valid.  This is why using
> > "unset" values in those fields was suggested, seemed to be accepted in
> > discussion, and implemented.
> 
> ...
> 
> > Swinging fields in and out makes it very handy to use one message type
> > for all of them and can save precious disk bandwidth, but the point was
> > to normalize these messages.  Is that still realistic and necessary?  If
> > so, we're trying to find a balance between message type explosion and
> > disk bandwidth.
> > 
> > We either need to make this fine-grained, ignore fields that aren't
> > valid for that type, or swing fields in and out.  Or maybe I have missed
> > something fundamental, such as the presence of subsequent fields depends
> > on the values of previous fields?
> 
> I'm still trying to understand what purpose this record actually
> serves, and what requirements may exist.  In an earlier thread
> somewhere Steve mentioned some broad requirements around data
> import/export, and I really wonder if the NETFILTER_PKT record
> provides anything useful here when it really isn't connecting the
> traffic to the sender/receiver without a lot of additional logging and
> post-processing smarts.  If you were interested in data import/export
> I think auditing the socket syscalls would provide a much more useful
> set of records in the audit log.

The problem here is we cannot be selective enough through the syscall 
interface to get exactly what we want. For example, any auditing of connect 
and accept will also get af_unix traffic which is likely to be uid/gid lookups 
through sssd or glibc. Typically we want the IPv4/6 traffic. The netfilter rules 
are better suited to describing which packets are of interest.

> Considering that one of the primary motivations for the audit
> subsystem is to enable compliance with various security
> specifications, let's get the ones we know about listed in this thread
> and then figure out how best to meet those requirements.

Common Criteria calls out for the ability to detect any attempt at information 
flow. Everything else leverages the CC requirements.

-Steve

Re: [RFC PATCH] audit: normalize NETFILTER_PKT

[Paul Moore]

On Sat, Feb 4, 2017 at 8:25 AM, Steve Grubb <sgrubb@redhat.com> wrote:
> On Friday, February 3, 2017 6:44:16 PM EST Paul Moore wrote:
>> I'm still trying to understand what purpose this record actually
>> serves, and what requirements may exist.  In an earlier thread
>> somewhere Steve mentioned some broad requirements around data
>> import/export, and I really wonder if the NETFILTER_PKT record
>> provides anything useful here when it really isn't connecting the
>> traffic to the sender/receiver without a lot of additional logging and
>> post-processing smarts.  If you were interested in data import/export
>> I think auditing the socket syscalls would provide a much more useful
>> set of records in the audit log.
>
> The problem here is we cannot be selective enough through the syscall
> interface to get exactly what we want. For example, any auditing of connect
> and accept will also get af_unix traffic which is likely to be uid/gid lookups
> through sssd or glibc. Typically we want the IPv4/6 traffic. The netfilter rules
> are better suited to describing which packets are of interest.

Okay, but how useful are these NETFILTER_PKT records, really?  The
only linkage you have back to the process on the local machine is via
the addr/proto/port tuple and that seems far from ideal.

>> Considering that one of the primary motivations for the audit
>> subsystem is to enable compliance with various security
>> specifications, let's get the ones we know about listed in this thread
>> and then figure out how best to meet those requirements.
>
> Common Criteria calls out for the ability to detect any attempt at information
> flow. Everything else leverages the CC requirements.

Yes, you've mentioned this previously.  This is good, but we need to
make these requirements a bit more concrete; we need something we can
use to arrive at a working implementation that satisfies these
requirements.

If this is purely about information flowing from A to B, would the
source and destination addr/proto/port for TCP and UDP suffice?  Do we
need anything else?

-- 
paul moore
www.paul-moore.com

Re: [RFC PATCH] audit: normalize NETFILTER_PKT

[Richard Guy Briggs]

On 2017-02-06 14:41, Paul Moore wrote:
> On Sat, Feb 4, 2017 at 8:25 AM, Steve Grubb <sgrubb@redhat.com> wrote:
> > On Friday, February 3, 2017 6:44:16 PM EST Paul Moore wrote:
> >> I'm still trying to understand what purpose this record actually
> >> serves, and what requirements may exist.  In an earlier thread
> >> somewhere Steve mentioned some broad requirements around data
> >> import/export, and I really wonder if the NETFILTER_PKT record
> >> provides anything useful here when it really isn't connecting the
> >> traffic to the sender/receiver without a lot of additional logging and
> >> post-processing smarts.  If you were interested in data import/export
> >> I think auditing the socket syscalls would provide a much more useful
> >> set of records in the audit log.
> >
> > The problem here is we cannot be selective enough through the syscall
> > interface to get exactly what we want. For example, any auditing of connect
> > and accept will also get af_unix traffic which is likely to be uid/gid lookups
> > through sssd or glibc. Typically we want the IPv4/6 traffic. The netfilter rules
> > are better suited to describing which packets are of interest.
> 
> Okay, but how useful are these NETFILTER_PKT records, really?  The
> only linkage you have back to the process on the local machine is via
> the addr/proto/port tuple and that seems far from ideal.

And even that could be spoofed easily and gathering more corroborating
information would seem useful.

Would the presence of the SOCKADDR record in any SYSCALL record be
useful for somehow tagging a class of fd as being of interest?

> >> Considering that one of the primary motivations for the audit
> >> subsystem is to enable compliance with various security
> >> specifications, let's get the ones we know about listed in this thread
> >> and then figure out how best to meet those requirements.
> >
> > Common Criteria calls out for the ability to detect any attempt at information
> > flow. Everything else leverages the CC requirements.
> 
> Yes, you've mentioned this previously.  This is good, but we need to
> make these requirements a bit more concrete; we need something we can
> use to arrive at a working implementation that satisfies these
> requirements.
> 
> If this is purely about information flowing from A to B, would the
> source and destination addr/proto/port for TCP and UDP suffice?  Do we
> need anything else?
> 
> -- 
> paul moore
> www.paul-moore.com

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

Re: [RFC PATCH] audit: normalize NETFILTER_PKT

[Paul Moore]

On Tue, Feb 7, 2017 at 4:22 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2017-02-06 14:41, Paul Moore wrote:
>> On Sat, Feb 4, 2017 at 8:25 AM, Steve Grubb <sgrubb@redhat.com> wrote:
>> > On Friday, February 3, 2017 6:44:16 PM EST Paul Moore wrote:
>> >> I'm still trying to understand what purpose this record actually
>> >> serves, and what requirements may exist.  In an earlier thread
>> >> somewhere Steve mentioned some broad requirements around data
>> >> import/export, and I really wonder if the NETFILTER_PKT record
>> >> provides anything useful here when it really isn't connecting the
>> >> traffic to the sender/receiver without a lot of additional logging and
>> >> post-processing smarts.  If you were interested in data import/export
>> >> I think auditing the socket syscalls would provide a much more useful
>> >> set of records in the audit log.
>> >
>> > The problem here is we cannot be selective enough through the syscall
>> > interface to get exactly what we want. For example, any auditing of connect
>> > and accept will also get af_unix traffic which is likely to be uid/gid lookups
>> > through sssd or glibc. Typically we want the IPv4/6 traffic. The netfilter rules
>> > are better suited to describing which packets are of interest.
>>
>> Okay, but how useful are these NETFILTER_PKT records, really?  The
>> only linkage you have back to the process on the local machine is via
>> the addr/proto/port tuple and that seems far from ideal.
>
> And even that could be spoofed easily and gathering more corroborating
> information would seem useful.
>
> Would the presence of the SOCKADDR record in any SYSCALL record be
> useful for somehow tagging a class of fd as being of interest?

I don't think we want to create a SOCKADDR record for every syscall,
but it seems reasonable that we may want to include it for targeted
syscalls.  Right now it looks like we create a SOCKADDR record
whenever we copy a sockaddr struct across the kernel/userspace
boundary, that should be sufficient, yes?

-- 
paul moore
security @ redhat

Re: [RFC PATCH] audit: normalize NETFILTER_PKT

[Richard Guy Briggs]

On 2017-02-07 23:02, Paul Moore wrote:
> On Tue, Feb 7, 2017 at 4:22 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2017-02-06 14:41, Paul Moore wrote:
> >> On Sat, Feb 4, 2017 at 8:25 AM, Steve Grubb <sgrubb@redhat.com> wrote:
> >> > On Friday, February 3, 2017 6:44:16 PM EST Paul Moore wrote:
> >> >> I'm still trying to understand what purpose this record actually
> >> >> serves, and what requirements may exist.  In an earlier thread
> >> >> somewhere Steve mentioned some broad requirements around data
> >> >> import/export, and I really wonder if the NETFILTER_PKT record
> >> >> provides anything useful here when it really isn't connecting the
> >> >> traffic to the sender/receiver without a lot of additional logging and
> >> >> post-processing smarts.  If you were interested in data import/export
> >> >> I think auditing the socket syscalls would provide a much more useful
> >> >> set of records in the audit log.
> >> >
> >> > The problem here is we cannot be selective enough through the syscall
> >> > interface to get exactly what we want. For example, any auditing of connect
> >> > and accept will also get af_unix traffic which is likely to be uid/gid lookups
> >> > through sssd or glibc. Typically we want the IPv4/6 traffic. The netfilter rules
> >> > are better suited to describing which packets are of interest.
> >>
> >> Okay, but how useful are these NETFILTER_PKT records, really?  The
> >> only linkage you have back to the process on the local machine is via
> >> the addr/proto/port tuple and that seems far from ideal.
> >
> > And even that could be spoofed easily and gathering more corroborating
> > information would seem useful.
> >
> > Would the presence of the SOCKADDR record in any SYSCALL record be
> > useful for somehow tagging a class of fd as being of interest?
> 
> I don't think we want to create a SOCKADDR record for every syscall,
> but it seems reasonable that we may want to include it for targeted
> syscalls.  Right now it looks like we create a SOCKADDR record
> whenever we copy a sockaddr struct across the kernel/userspace
> boundary, that should be sufficient, yes?

Yes, we certainly don't need it for every syscall.  Since the sockaddr
record is only created if it is available we could further flag or check
the protocol to further process only the network-based sockaddrs and
ignore the unix sockaddrs for this purpose.  I'm picturing adding a flag
to the fd, but that is making me a bit nervous about overstepping our
usual code area.

> paul moore

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

Re: [RFC PATCH] audit: normalize NETFILTER_PKT

[Paul Moore]

On Wed, Feb 8, 2017 at 7:32 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2017-02-07 23:02, Paul Moore wrote:
>> On Tue, Feb 7, 2017 at 4:22 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> > On 2017-02-06 14:41, Paul Moore wrote:
>> >> On Sat, Feb 4, 2017 at 8:25 AM, Steve Grubb <sgrubb@redhat.com> wrote:
>> >> > On Friday, February 3, 2017 6:44:16 PM EST Paul Moore wrote:
>> >> >> I'm still trying to understand what purpose this record actually
>> >> >> serves, and what requirements may exist.  In an earlier thread
>> >> >> somewhere Steve mentioned some broad requirements around data
>> >> >> import/export, and I really wonder if the NETFILTER_PKT record
>> >> >> provides anything useful here when it really isn't connecting the
>> >> >> traffic to the sender/receiver without a lot of additional logging and
>> >> >> post-processing smarts.  If you were interested in data import/export
>> >> >> I think auditing the socket syscalls would provide a much more useful
>> >> >> set of records in the audit log.
>> >> >
>> >> > The problem here is we cannot be selective enough through the syscall
>> >> > interface to get exactly what we want. For example, any auditing of connect
>> >> > and accept will also get af_unix traffic which is likely to be uid/gid lookups
>> >> > through sssd or glibc. Typically we want the IPv4/6 traffic. The netfilter rules
>> >> > are better suited to describing which packets are of interest.
>> >>
>> >> Okay, but how useful are these NETFILTER_PKT records, really?  The
>> >> only linkage you have back to the process on the local machine is via
>> >> the addr/proto/port tuple and that seems far from ideal.
>> >
>> > And even that could be spoofed easily and gathering more corroborating
>> > information would seem useful.
>> >
>> > Would the presence of the SOCKADDR record in any SYSCALL record be
>> > useful for somehow tagging a class of fd as being of interest?
>>
>> I don't think we want to create a SOCKADDR record for every syscall,
>> but it seems reasonable that we may want to include it for targeted
>> syscalls.  Right now it looks like we create a SOCKADDR record
>> whenever we copy a sockaddr struct across the kernel/userspace
>> boundary, that should be sufficient, yes?
>
> Yes, we certainly don't need it for every syscall.  Since the sockaddr
> record is only created if it is available we could further flag or check
> the protocol to further process only the network-based sockaddrs and
> ignore the unix sockaddrs for this purpose.  I'm picturing adding a flag
> to the fd, but that is making me a bit nervous about overstepping our
> usual code area.

Let's keep it as-is, I would think there are other cases where having
the address info for AF_UNIX (and others) might be helpful.

-- 
paul moore
www.paul-moore.com

Re: [RFC PATCH] audit: normalize NETFILTER_PKT

[Richard Guy Briggs]

On 2017-02-08 18:11, Paul Moore wrote:
> On Wed, Feb 8, 2017 at 7:32 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2017-02-07 23:02, Paul Moore wrote:
> >> On Tue, Feb 7, 2017 at 4:22 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> >> > On 2017-02-06 14:41, Paul Moore wrote:
> >> >> On Sat, Feb 4, 2017 at 8:25 AM, Steve Grubb <sgrubb@redhat.com> wrote:
> >> >> > On Friday, February 3, 2017 6:44:16 PM EST Paul Moore wrote:
> >> >> >> I'm still trying to understand what purpose this record actually
> >> >> >> serves, and what requirements may exist.  In an earlier thread
> >> >> >> somewhere Steve mentioned some broad requirements around data
> >> >> >> import/export, and I really wonder if the NETFILTER_PKT record
> >> >> >> provides anything useful here when it really isn't connecting the
> >> >> >> traffic to the sender/receiver without a lot of additional logging and
> >> >> >> post-processing smarts.  If you were interested in data import/export
> >> >> >> I think auditing the socket syscalls would provide a much more useful
> >> >> >> set of records in the audit log.
> >> >> >
> >> >> > The problem here is we cannot be selective enough through the syscall
> >> >> > interface to get exactly what we want. For example, any auditing of connect
> >> >> > and accept will also get af_unix traffic which is likely to be uid/gid lookups
> >> >> > through sssd or glibc. Typically we want the IPv4/6 traffic. The netfilter rules
> >> >> > are better suited to describing which packets are of interest.
> >> >>
> >> >> Okay, but how useful are these NETFILTER_PKT records, really?  The
> >> >> only linkage you have back to the process on the local machine is via
> >> >> the addr/proto/port tuple and that seems far from ideal.
> >> >
> >> > And even that could be spoofed easily and gathering more corroborating
> >> > information would seem useful.
> >> >
> >> > Would the presence of the SOCKADDR record in any SYSCALL record be
> >> > useful for somehow tagging a class of fd as being of interest?
> >>
> >> I don't think we want to create a SOCKADDR record for every syscall,
> >> but it seems reasonable that we may want to include it for targeted
> >> syscalls.  Right now it looks like we create a SOCKADDR record
> >> whenever we copy a sockaddr struct across the kernel/userspace
> >> boundary, that should be sufficient, yes?
> >
> > Yes, we certainly don't need it for every syscall.  Since the sockaddr
> > record is only created if it is available we could further flag or check
> > the protocol to further process only the network-based sockaddrs and
> > ignore the unix sockaddrs for this purpose.  I'm picturing adding a flag
> > to the fd, but that is making me a bit nervous about overstepping our
> > usual code area.
> 
> Let's keep it as-is, I would think there are other cases where having
> the address info for AF_UNIX (and others) might be helpful.

I wasn't suggesting removing the existing AUDIT_SOCKADDR support for
AF_UNIX or other types of sockets, but rather when they are encountered
by the audit subsystem flag the fd (if it isn't already identified as
a network socket) as having more interesting information for network
auditing.

> paul moore

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635