netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V4 1/2] netfilter: xt_AUDIT: use consistent ipv4 network offset
@ 2017-03-22  7:05 Richard Guy Briggs
  2017-03-22  7:05 ` [PATCH V4 2/2] audit: normalize NETFILTER_PKT Richard Guy Briggs
  2017-03-22 11:11 ` [PATCH V4 1/2] netfilter: xt_AUDIT: use consistent ipv4 network offset Pablo Neira Ayuso
  0 siblings, 2 replies; 7+ messages in thread
From: Richard Guy Briggs @ 2017-03-22  7:05 UTC (permalink / raw)
  To: Netfilter Developer Mailing List, linux-audit
  Cc: Richard Guy Briggs, Florian Westphal, Thomas Woerner, Thomas Graf,
	Eric Paris, Paul Moore, Steve Grubb

Even though the skb->data pointer has been moved from the link layer
header to the network layer header, use the same method to calculate the
offset in ipv4 and ipv6 routines.

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

diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
index 4973cbd..cdb7cee 100644
--- a/net/netfilter/xt_AUDIT.c
+++ b/net/netfilter/xt_AUDIT.c
@@ -76,7 +76,7 @@ static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb)
 	struct iphdr _iph;
 	const struct iphdr *ih;
 
-	ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph);
+	ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_iph), &_iph);
 	if (!ih) {
 		audit_log_format(ab, " truncated=1");
 		return;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH V4 2/2] audit: normalize NETFILTER_PKT
  2017-03-22  7:05 [PATCH V4 1/2] netfilter: xt_AUDIT: use consistent ipv4 network offset Richard Guy Briggs
@ 2017-03-22  7:05 ` Richard Guy Briggs
  2017-03-23 18:44   ` Paul Moore
  2017-03-22 11:11 ` [PATCH V4 1/2] netfilter: xt_AUDIT: use consistent ipv4 network offset Pablo Neira Ayuso
  1 sibling, 1 reply; 7+ messages in thread
From: Richard Guy Briggs @ 2017-03-22  7:05 UTC (permalink / raw)
  To: Netfilter Developer Mailing List, linux-audit
  Cc: Richard Guy Briggs, Florian Westphal, Thomas Woerner, Thomas Graf,
	Eric Paris, Paul Moore, Steve Grubb

Eliminate flipping in and out of message fields, dropping fields in the
process.

Sample raw message format IPv4 UDP:
type=NETFILTER_PKT msg=audit(1487874761.386:228):  mark=0xae8a2732 saddr=127.0.0.1 daddr=127.0.0.1 proto=17^]
Sample raw message format IPv6 ICMP6:
type=NETFILTER_PKT msg=audit(1487874761.381:227):  mark=0x223894b7 saddr=::1 daddr=::1 proto=58^]

Issue: https://github.com/linux-audit/audit-kernel/issues/11
Test case: https://github.com/linux-audit/audit-testsuite/issues/43

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
v4:
	Write out nfmark unmodified rather than trying to indicate "unset".
	Collapse/simplify switch/case statements.
v3:
	Don't store interim values, but print immediately.
v2:
	Trim down to 4 fields.  Add raw samples.

 net/netfilter/xt_AUDIT.c |  124 ++++++++++------------------------------------
 1 files changed, 27 insertions(+), 97 deletions(-)

diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
index cdb7cee..582ee54 100644
--- a/net/netfilter/xt_AUDIT.c
+++ b/net/netfilter/xt_AUDIT.c
@@ -31,146 +31,76 @@ MODULE_ALIAS("ip6t_AUDIT");
 MODULE_ALIAS("ebt_AUDIT");
 MODULE_ALIAS("arpt_AUDIT");
 
-static void audit_proto(struct audit_buffer *ab, struct sk_buff *skb,
-			unsigned int proto, unsigned int offset)
-{
-	switch (proto) {
-	case IPPROTO_TCP:
-	case IPPROTO_UDP:
-	case IPPROTO_UDPLITE: {
-		const __be16 *pptr;
-		__be16 _ports[2];
-
-		pptr = skb_header_pointer(skb, offset, sizeof(_ports), _ports);
-		if (pptr == NULL) {
-			audit_log_format(ab, " truncated=1");
-			return;
-		}
-
-		audit_log_format(ab, " sport=%hu dport=%hu",
-				 ntohs(pptr[0]), ntohs(pptr[1]));
-		}
-		break;
-
-	case IPPROTO_ICMP:
-	case IPPROTO_ICMPV6: {
-		const u8 *iptr;
-		u8 _ih[2];
-
-		iptr = skb_header_pointer(skb, offset, sizeof(_ih), &_ih);
-		if (iptr == NULL) {
-			audit_log_format(ab, " truncated=1");
-			return;
-		}
-
-		audit_log_format(ab, " icmptype=%hhu icmpcode=%hhu",
-				 iptr[0], iptr[1]);
-
-		}
-		break;
-	}
-}
-
-static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb)
+static bool audit_ip4(struct audit_buffer *ab, struct sk_buff *skb)
 {
 	struct iphdr _iph;
 	const struct iphdr *ih;
 
 	ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_iph), &_iph);
-	if (!ih) {
-		audit_log_format(ab, " truncated=1");
-		return;
-	}
+	if (!ih)
+		return false;
 
-	audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu proto=%hhu",
-		&ih->saddr, &ih->daddr, ntohs(ih->id), ih->protocol);
+	audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu",
+			 &ih->saddr, &ih->daddr, ih->protocol);
 
-	if (ntohs(ih->frag_off) & IP_OFFSET) {
-		audit_log_format(ab, " frag=1");
-		return;
-	}
-
-	audit_proto(ab, skb, ih->protocol, ih->ihl * 4);
+	return true;
 }
 
-static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
+static bool audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
 {
 	struct ipv6hdr _ip6h;
 	const struct ipv6hdr *ih;
 	u8 nexthdr;
 	__be16 frag_off;
-	int offset;
 
 	ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_ip6h), &_ip6h);
-	if (!ih) {
-		audit_log_format(ab, " truncated=1");
-		return;
-	}
+	if (!ih)
+		return false;
 
 	nexthdr = ih->nexthdr;
-	offset = ipv6_skip_exthdr(skb, skb_network_offset(skb) + sizeof(_ip6h),
-				  &nexthdr, &frag_off);
+	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);
 
-	if (offset)
-		audit_proto(ab, skb, nexthdr, offset);
+	return true;
 }
 
 static unsigned int
 audit_tg(struct sk_buff *skb, const struct xt_action_param *par)
 {
-	const struct xt_audit_info *info = par->targinfo;
 	struct audit_buffer *ab;
+	int fam = -1;
 
 	if (audit_enabled == 0)
 		goto errout;
-
 	ab = audit_log_start(NULL, GFP_ATOMIC, AUDIT_NETFILTER_PKT);
 	if (ab == NULL)
 		goto errout;
 
-	audit_log_format(ab, "action=%hhu hook=%u len=%u inif=%s outif=%s",
-			 info->type, par->hooknum, skb->len,
-			 par->in ? par->in->name : "?",
-			 par->out ? par->out->name : "?");
-
-	if (skb->mark)
-		audit_log_format(ab, " mark=%#x", skb->mark);
-
-	if (skb->dev && skb->dev->type == ARPHRD_ETHER) {
-		audit_log_format(ab, " smac=%pM dmac=%pM macproto=0x%04x",
-				 eth_hdr(skb)->h_source, eth_hdr(skb)->h_dest,
-				 ntohs(eth_hdr(skb)->h_proto));
-
-		if (par->family == NFPROTO_BRIDGE) {
-			switch (eth_hdr(skb)->h_proto) {
-			case htons(ETH_P_IP):
-				audit_ip4(ab, skb);
-				break;
-
-			case htons(ETH_P_IPV6):
-				audit_ip6(ab, skb);
-				break;
-			}
-		}
-	}
+	audit_log_format(ab, "mark=%#x", skb->mark);
 
 	switch (par->family) {
+	case NFPROTO_BRIDGE:
+		switch (eth_hdr(skb)->h_proto) {
+		case htons(ETH_P_IP):
+			fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
+			break;
+		case htons(ETH_P_IPV6):
+			fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
+			break;
+		}
+		break;
 	case NFPROTO_IPV4:
-		audit_ip4(ab, skb);
+		fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
 		break;
-
 	case NFPROTO_IPV6:
-		audit_ip6(ab, skb);
+		fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
 		break;
 	}
 
-#ifdef CONFIG_NETWORK_SECMARK
-	if (skb->secmark)
-		audit_log_secctx(ab, skb->secmark);
-#endif
+	if (fam == -1)
+		audit_log_format(ab, " saddr=? daddr=? proto=-1");
 
 	audit_log_end(ab);
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH V4 1/2] netfilter: xt_AUDIT: use consistent ipv4 network offset
  2017-03-22  7:05 [PATCH V4 1/2] netfilter: xt_AUDIT: use consistent ipv4 network offset Richard Guy Briggs
  2017-03-22  7:05 ` [PATCH V4 2/2] audit: normalize NETFILTER_PKT Richard Guy Briggs
@ 2017-03-22 11:11 ` Pablo Neira Ayuso
  2017-03-22 11:43   ` Richard Guy Briggs
  1 sibling, 1 reply; 7+ messages in thread
From: Pablo Neira Ayuso @ 2017-03-22 11:11 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Netfilter Developer Mailing List, linux-audit, Florian Westphal,
	Thomas Woerner, Thomas Graf, Eric Paris, Paul Moore, Steve Grubb

On Wed, Mar 22, 2017 at 03:05:36AM -0400, Richard Guy Briggs wrote:
> Even though the skb->data pointer has been moved from the link layer
> header to the network layer header, use the same method to calculate the
> offset in ipv4 and ipv6 routines.
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  net/netfilter/xt_AUDIT.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
> index 4973cbd..cdb7cee 100644
> --- a/net/netfilter/xt_AUDIT.c
> +++ b/net/netfilter/xt_AUDIT.c
> @@ -76,7 +76,7 @@ static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb)
>  	struct iphdr _iph;
>  	const struct iphdr *ih;
>  
> -	ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph);
> +	ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_iph), &_iph);

This update is completely pointless.

If you want I can place it in nf-next, your call.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V4 1/2] netfilter: xt_AUDIT: use consistent ipv4 network offset
  2017-03-22 11:11 ` [PATCH V4 1/2] netfilter: xt_AUDIT: use consistent ipv4 network offset Pablo Neira Ayuso
@ 2017-03-22 11:43   ` Richard Guy Briggs
  2017-03-22 12:56     ` Pablo Neira Ayuso
  2017-03-23 18:44     ` Paul Moore
  0 siblings, 2 replies; 7+ messages in thread
From: Richard Guy Briggs @ 2017-03-22 11:43 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Florian Westphal, linux-audit, Netfilter Developer Mailing List,
	Thomas Woerner, Thomas Graf

On 2017-03-22 12:11, Pablo Neira Ayuso wrote:
> On Wed, Mar 22, 2017 at 03:05:36AM -0400, Richard Guy Briggs wrote:
> > Even though the skb->data pointer has been moved from the link layer
> > header to the network layer header, use the same method to calculate the
> > offset in ipv4 and ipv6 routines.
> > 
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> >  net/netfilter/xt_AUDIT.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
> > index 4973cbd..cdb7cee 100644
> > --- a/net/netfilter/xt_AUDIT.c
> > +++ b/net/netfilter/xt_AUDIT.c
> > @@ -76,7 +76,7 @@ static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb)
> >  	struct iphdr _iph;
> >  	const struct iphdr *ih;
> >  
> > -	ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph);
> > +	ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_iph), &_iph);
> 
> This update is completely pointless.

Its point is to be consistent with audit_ip6() and to prevent further
time consumed by confusion and head-scratching.  I know it is slightly
slower with an identical result.

> If you want I can place it in nf-next, your call.

I'd prefer to bring it through the audit-next tree to avoid the merge
conflict.

- 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V4 1/2] netfilter: xt_AUDIT: use consistent ipv4 network offset
  2017-03-22 11:43   ` Richard Guy Briggs
@ 2017-03-22 12:56     ` Pablo Neira Ayuso
  2017-03-23 18:44     ` Paul Moore
  1 sibling, 0 replies; 7+ messages in thread
From: Pablo Neira Ayuso @ 2017-03-22 12:56 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Netfilter Developer Mailing List, linux-audit, Florian Westphal,
	Thomas Woerner, Thomas Graf, Eric Paris, Paul Moore, Steve Grubb

On Wed, Mar 22, 2017 at 07:43:18AM -0400, Richard Guy Briggs wrote:
> On 2017-03-22 12:11, Pablo Neira Ayuso wrote:
> > On Wed, Mar 22, 2017 at 03:05:36AM -0400, Richard Guy Briggs wrote:
> > > Even though the skb->data pointer has been moved from the link layer
> > > header to the network layer header, use the same method to calculate the
> > > offset in ipv4 and ipv6 routines.
> > > 
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > ---
> > >  net/netfilter/xt_AUDIT.c |    2 +-
> > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
> > > index 4973cbd..cdb7cee 100644
> > > --- a/net/netfilter/xt_AUDIT.c
> > > +++ b/net/netfilter/xt_AUDIT.c
> > > @@ -76,7 +76,7 @@ static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb)
> > >  	struct iphdr _iph;
> > >  	const struct iphdr *ih;
> > >  
> > > -	ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph);
> > > +	ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_iph), &_iph);
> > 
> > This update is completely pointless.
> 
> Its point is to be consistent with audit_ip6() and to prevent further
> time consumed by confusion and head-scratching.  I know it is slightly
> slower with an identical result.
> 
> > If you want I can place it in nf-next, your call.
> 
> I'd prefer to bring it through the audit-next tree to avoid the merge
> conflict.

No problem. I remove this patchset from my patchwork then.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V4 1/2] netfilter: xt_AUDIT: use consistent ipv4 network offset
  2017-03-22 11:43   ` Richard Guy Briggs
  2017-03-22 12:56     ` Pablo Neira Ayuso
@ 2017-03-23 18:44     ` Paul Moore
  1 sibling, 0 replies; 7+ messages in thread
From: Paul Moore @ 2017-03-23 18:44 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Pablo Neira Ayuso, Florian Westphal, linux-audit,
	Netfilter Developer Mailing List, Thomas Woerner, Thomas Graf

On Wed, Mar 22, 2017 at 7:43 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2017-03-22 12:11, Pablo Neira Ayuso wrote:
>> On Wed, Mar 22, 2017 at 03:05:36AM -0400, Richard Guy Briggs wrote:
>> > Even though the skb->data pointer has been moved from the link layer
>> > header to the network layer header, use the same method to calculate the
>> > offset in ipv4 and ipv6 routines.
>> >
>> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
>> > ---
>> >  net/netfilter/xt_AUDIT.c |    2 +-
>> >  1 files changed, 1 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
>> > index 4973cbd..cdb7cee 100644
>> > --- a/net/netfilter/xt_AUDIT.c
>> > +++ b/net/netfilter/xt_AUDIT.c
>> > @@ -76,7 +76,7 @@ static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb)
>> >     struct iphdr _iph;
>> >     const struct iphdr *ih;
>> >
>> > -   ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph);
>> > +   ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_iph), &_iph);
>>
>> This update is completely pointless.
>
> Its point is to be consistent with audit_ip6() and to prevent further
> time consumed by confusion and head-scratching.  I know it is slightly
> slower with an identical result.

Agree with Richard, merged to audit/next.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V4 2/2] audit: normalize NETFILTER_PKT
  2017-03-22  7:05 ` [PATCH V4 2/2] audit: normalize NETFILTER_PKT Richard Guy Briggs
@ 2017-03-23 18:44   ` Paul Moore
  0 siblings, 0 replies; 7+ messages in thread
From: Paul Moore @ 2017-03-23 18:44 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Netfilter Developer Mailing List, linux-audit, Florian Westphal,
	Thomas Woerner, Thomas Graf

On Wed, Mar 22, 2017 at 3:05 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> Eliminate flipping in and out of message fields, dropping fields in the
> process.
>
> Sample raw message format IPv4 UDP:
> type=NETFILTER_PKT msg=audit(1487874761.386:228):  mark=0xae8a2732 saddr=127.0.0.1 daddr=127.0.0.1 proto=17^]
> Sample raw message format IPv6 ICMP6:
> type=NETFILTER_PKT msg=audit(1487874761.381:227):  mark=0x223894b7 saddr=::1 daddr=::1 proto=58^]
>
> Issue: https://github.com/linux-audit/audit-kernel/issues/11
> Test case: https://github.com/linux-audit/audit-testsuite/issues/43
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> v4:
>         Write out nfmark unmodified rather than trying to indicate "unset".
>         Collapse/simplify switch/case statements.
> v3:
>         Don't store interim values, but print immediately.
> v2:
>         Trim down to 4 fields.  Add raw samples.
>
>  net/netfilter/xt_AUDIT.c |  124 ++++++++++------------------------------------
>  1 files changed, 27 insertions(+), 97 deletions(-)

Looks good, merged to audit/next.

> diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
> index cdb7cee..582ee54 100644
> --- a/net/netfilter/xt_AUDIT.c
> +++ b/net/netfilter/xt_AUDIT.c
> @@ -31,146 +31,76 @@ MODULE_ALIAS("ip6t_AUDIT");
>  MODULE_ALIAS("ebt_AUDIT");
>  MODULE_ALIAS("arpt_AUDIT");
>
> -static void audit_proto(struct audit_buffer *ab, struct sk_buff *skb,
> -                       unsigned int proto, unsigned int offset)
> -{
> -       switch (proto) {
> -       case IPPROTO_TCP:
> -       case IPPROTO_UDP:
> -       case IPPROTO_UDPLITE: {
> -               const __be16 *pptr;
> -               __be16 _ports[2];
> -
> -               pptr = skb_header_pointer(skb, offset, sizeof(_ports), _ports);
> -               if (pptr == NULL) {
> -                       audit_log_format(ab, " truncated=1");
> -                       return;
> -               }
> -
> -               audit_log_format(ab, " sport=%hu dport=%hu",
> -                                ntohs(pptr[0]), ntohs(pptr[1]));
> -               }
> -               break;
> -
> -       case IPPROTO_ICMP:
> -       case IPPROTO_ICMPV6: {
> -               const u8 *iptr;
> -               u8 _ih[2];
> -
> -               iptr = skb_header_pointer(skb, offset, sizeof(_ih), &_ih);
> -               if (iptr == NULL) {
> -                       audit_log_format(ab, " truncated=1");
> -                       return;
> -               }
> -
> -               audit_log_format(ab, " icmptype=%hhu icmpcode=%hhu",
> -                                iptr[0], iptr[1]);
> -
> -               }
> -               break;
> -       }
> -}
> -
> -static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb)
> +static bool audit_ip4(struct audit_buffer *ab, struct sk_buff *skb)
>  {
>         struct iphdr _iph;
>         const struct iphdr *ih;
>
>         ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_iph), &_iph);
> -       if (!ih) {
> -               audit_log_format(ab, " truncated=1");
> -               return;
> -       }
> +       if (!ih)
> +               return false;
>
> -       audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu proto=%hhu",
> -               &ih->saddr, &ih->daddr, ntohs(ih->id), ih->protocol);
> +       audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu",
> +                        &ih->saddr, &ih->daddr, ih->protocol);
>
> -       if (ntohs(ih->frag_off) & IP_OFFSET) {
> -               audit_log_format(ab, " frag=1");
> -               return;
> -       }
> -
> -       audit_proto(ab, skb, ih->protocol, ih->ihl * 4);
> +       return true;
>  }
>
> -static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
> +static bool audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
>  {
>         struct ipv6hdr _ip6h;
>         const struct ipv6hdr *ih;
>         u8 nexthdr;
>         __be16 frag_off;
> -       int offset;
>
>         ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_ip6h), &_ip6h);
> -       if (!ih) {
> -               audit_log_format(ab, " truncated=1");
> -               return;
> -       }
> +       if (!ih)
> +               return false;
>
>         nexthdr = ih->nexthdr;
> -       offset = ipv6_skip_exthdr(skb, skb_network_offset(skb) + sizeof(_ip6h),
> -                                 &nexthdr, &frag_off);
> +       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);
>
> -       if (offset)
> -               audit_proto(ab, skb, nexthdr, offset);
> +       return true;
>  }
>
>  static unsigned int
>  audit_tg(struct sk_buff *skb, const struct xt_action_param *par)
>  {
> -       const struct xt_audit_info *info = par->targinfo;
>         struct audit_buffer *ab;
> +       int fam = -1;
>
>         if (audit_enabled == 0)
>                 goto errout;
> -
>         ab = audit_log_start(NULL, GFP_ATOMIC, AUDIT_NETFILTER_PKT);
>         if (ab == NULL)
>                 goto errout;
>
> -       audit_log_format(ab, "action=%hhu hook=%u len=%u inif=%s outif=%s",
> -                        info->type, par->hooknum, skb->len,
> -                        par->in ? par->in->name : "?",
> -                        par->out ? par->out->name : "?");
> -
> -       if (skb->mark)
> -               audit_log_format(ab, " mark=%#x", skb->mark);
> -
> -       if (skb->dev && skb->dev->type == ARPHRD_ETHER) {
> -               audit_log_format(ab, " smac=%pM dmac=%pM macproto=0x%04x",
> -                                eth_hdr(skb)->h_source, eth_hdr(skb)->h_dest,
> -                                ntohs(eth_hdr(skb)->h_proto));
> -
> -               if (par->family == NFPROTO_BRIDGE) {
> -                       switch (eth_hdr(skb)->h_proto) {
> -                       case htons(ETH_P_IP):
> -                               audit_ip4(ab, skb);
> -                               break;
> -
> -                       case htons(ETH_P_IPV6):
> -                               audit_ip6(ab, skb);
> -                               break;
> -                       }
> -               }
> -       }
> +       audit_log_format(ab, "mark=%#x", skb->mark);
>
>         switch (par->family) {
> +       case NFPROTO_BRIDGE:
> +               switch (eth_hdr(skb)->h_proto) {
> +               case htons(ETH_P_IP):
> +                       fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
> +                       break;
> +               case htons(ETH_P_IPV6):
> +                       fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
> +                       break;
> +               }
> +               break;
>         case NFPROTO_IPV4:
> -               audit_ip4(ab, skb);
> +               fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
>                 break;
> -
>         case NFPROTO_IPV6:
> -               audit_ip6(ab, skb);
> +               fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
>                 break;
>         }
>
> -#ifdef CONFIG_NETWORK_SECMARK
> -       if (skb->secmark)
> -               audit_log_secctx(ab, skb->secmark);
> -#endif
> +       if (fam == -1)
> +               audit_log_format(ab, " saddr=? daddr=? proto=-1");
>
>         audit_log_end(ab);
>
> --
> 1.7.1
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit



-- 
paul moore
www.paul-moore.com

On Wed, Mar 22, 2017 at 3:05 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> Eliminate flipping in and out of message fields, dropping fields in the
> process.
>
> Sample raw message format IPv4 UDP:
> type=NETFILTER_PKT msg=audit(1487874761.386:228):  mark=0xae8a2732 saddr=127.0.0.1 daddr=127.0.0.1 proto=17^]
> Sample raw message format IPv6 ICMP6:
> type=NETFILTER_PKT msg=audit(1487874761.381:227):  mark=0x223894b7 saddr=::1 daddr=::1 proto=58^]
>
> Issue: https://github.com/linux-audit/audit-kernel/issues/11
> Test case: https://github.com/linux-audit/audit-testsuite/issues/43
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> v4:
>         Write out nfmark unmodified rather than trying to indicate "unset".
>         Collapse/simplify switch/case statements.
> v3:
>         Don't store interim values, but print immediately.
> v2:
>         Trim down to 4 fields.  Add raw samples.
>
>  net/netfilter/xt_AUDIT.c |  124 ++++++++++------------------------------------
>  1 files changed, 27 insertions(+), 97 deletions(-)
>
> diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
> index cdb7cee..582ee54 100644
> --- a/net/netfilter/xt_AUDIT.c
> +++ b/net/netfilter/xt_AUDIT.c
> @@ -31,146 +31,76 @@ MODULE_ALIAS("ip6t_AUDIT");
>  MODULE_ALIAS("ebt_AUDIT");
>  MODULE_ALIAS("arpt_AUDIT");
>
> -static void audit_proto(struct audit_buffer *ab, struct sk_buff *skb,
> -                       unsigned int proto, unsigned int offset)
> -{
> -       switch (proto) {
> -       case IPPROTO_TCP:
> -       case IPPROTO_UDP:
> -       case IPPROTO_UDPLITE: {
> -               const __be16 *pptr;
> -               __be16 _ports[2];
> -
> -               pptr = skb_header_pointer(skb, offset, sizeof(_ports), _ports);
> -               if (pptr == NULL) {
> -                       audit_log_format(ab, " truncated=1");
> -                       return;
> -               }
> -
> -               audit_log_format(ab, " sport=%hu dport=%hu",
> -                                ntohs(pptr[0]), ntohs(pptr[1]));
> -               }
> -               break;
> -
> -       case IPPROTO_ICMP:
> -       case IPPROTO_ICMPV6: {
> -               const u8 *iptr;
> -               u8 _ih[2];
> -
> -               iptr = skb_header_pointer(skb, offset, sizeof(_ih), &_ih);
> -               if (iptr == NULL) {
> -                       audit_log_format(ab, " truncated=1");
> -                       return;
> -               }
> -
> -               audit_log_format(ab, " icmptype=%hhu icmpcode=%hhu",
> -                                iptr[0], iptr[1]);
> -
> -               }
> -               break;
> -       }
> -}
> -
> -static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb)
> +static bool audit_ip4(struct audit_buffer *ab, struct sk_buff *skb)
>  {
>         struct iphdr _iph;
>         const struct iphdr *ih;
>
>         ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_iph), &_iph);
> -       if (!ih) {
> -               audit_log_format(ab, " truncated=1");
> -               return;
> -       }
> +       if (!ih)
> +               return false;
>
> -       audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu proto=%hhu",
> -               &ih->saddr, &ih->daddr, ntohs(ih->id), ih->protocol);
> +       audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu",
> +                        &ih->saddr, &ih->daddr, ih->protocol);
>
> -       if (ntohs(ih->frag_off) & IP_OFFSET) {
> -               audit_log_format(ab, " frag=1");
> -               return;
> -       }
> -
> -       audit_proto(ab, skb, ih->protocol, ih->ihl * 4);
> +       return true;
>  }
>
> -static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
> +static bool audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
>  {
>         struct ipv6hdr _ip6h;
>         const struct ipv6hdr *ih;
>         u8 nexthdr;
>         __be16 frag_off;
> -       int offset;
>
>         ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_ip6h), &_ip6h);
> -       if (!ih) {
> -               audit_log_format(ab, " truncated=1");
> -               return;
> -       }
> +       if (!ih)
> +               return false;
>
>         nexthdr = ih->nexthdr;
> -       offset = ipv6_skip_exthdr(skb, skb_network_offset(skb) + sizeof(_ip6h),
> -                                 &nexthdr, &frag_off);
> +       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);
>
> -       if (offset)
> -               audit_proto(ab, skb, nexthdr, offset);
> +       return true;
>  }
>
>  static unsigned int
>  audit_tg(struct sk_buff *skb, const struct xt_action_param *par)
>  {
> -       const struct xt_audit_info *info = par->targinfo;
>         struct audit_buffer *ab;
> +       int fam = -1;
>
>         if (audit_enabled == 0)
>                 goto errout;
> -
>         ab = audit_log_start(NULL, GFP_ATOMIC, AUDIT_NETFILTER_PKT);
>         if (ab == NULL)
>                 goto errout;
>
> -       audit_log_format(ab, "action=%hhu hook=%u len=%u inif=%s outif=%s",
> -                        info->type, par->hooknum, skb->len,
> -                        par->in ? par->in->name : "?",
> -                        par->out ? par->out->name : "?");
> -
> -       if (skb->mark)
> -               audit_log_format(ab, " mark=%#x", skb->mark);
> -
> -       if (skb->dev && skb->dev->type == ARPHRD_ETHER) {
> -               audit_log_format(ab, " smac=%pM dmac=%pM macproto=0x%04x",
> -                                eth_hdr(skb)->h_source, eth_hdr(skb)->h_dest,
> -                                ntohs(eth_hdr(skb)->h_proto));
> -
> -               if (par->family == NFPROTO_BRIDGE) {
> -                       switch (eth_hdr(skb)->h_proto) {
> -                       case htons(ETH_P_IP):
> -                               audit_ip4(ab, skb);
> -                               break;
> -
> -                       case htons(ETH_P_IPV6):
> -                               audit_ip6(ab, skb);
> -                               break;
> -                       }
> -               }
> -       }
> +       audit_log_format(ab, "mark=%#x", skb->mark);
>
>         switch (par->family) {
> +       case NFPROTO_BRIDGE:
> +               switch (eth_hdr(skb)->h_proto) {
> +               case htons(ETH_P_IP):
> +                       fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
> +                       break;
> +               case htons(ETH_P_IPV6):
> +                       fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
> +                       break;
> +               }
> +               break;
>         case NFPROTO_IPV4:
> -               audit_ip4(ab, skb);
> +               fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
>                 break;
> -
>         case NFPROTO_IPV6:
> -               audit_ip6(ab, skb);
> +               fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
>                 break;
>         }
>
> -#ifdef CONFIG_NETWORK_SECMARK
> -       if (skb->secmark)
> -               audit_log_secctx(ab, skb->secmark);
> -#endif
> +       if (fam == -1)
> +               audit_log_format(ab, " saddr=? daddr=? proto=-1");
>
>         audit_log_end(ab);
>
> --
> 1.7.1
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit



-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-03-23 18:44 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-22  7:05 [PATCH V4 1/2] netfilter: xt_AUDIT: use consistent ipv4 network offset Richard Guy Briggs
2017-03-22  7:05 ` [PATCH V4 2/2] audit: normalize NETFILTER_PKT Richard Guy Briggs
2017-03-23 18:44   ` Paul Moore
2017-03-22 11:11 ` [PATCH V4 1/2] netfilter: xt_AUDIT: use consistent ipv4 network offset Pablo Neira Ayuso
2017-03-22 11:43   ` Richard Guy Briggs
2017-03-22 12:56     ` Pablo Neira Ayuso
2017-03-23 18:44     ` Paul Moore

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).