[nf-next PATCH 1/2] netfilter: nf_log_syslog: Merge MAC header dumpers

[nf-next PATCH 1/2] netfilter: nf_log_syslog: Merge MAC header dumpers

[Phil Sutter]

The functions for IPv4 and IPv6 were almost identical apart from extra
SIT tunnel device handling in the latter.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 net/netfilter/nf_log_syslog.c | 91 ++++++++++-------------------------
 1 file changed, 25 insertions(+), 66 deletions(-)

diff --git a/net/netfilter/nf_log_syslog.c b/net/netfilter/nf_log_syslog.c
index 13234641cdb34..d1dcf36545d79 100644
--- a/net/netfilter/nf_log_syslog.c
+++ b/net/netfilter/nf_log_syslog.c
@@ -766,9 +766,9 @@ dump_ipv6_packet(struct net *net, struct nf_log_buf *m,
 		nf_log_buf_add(m, "MARK=0x%x ", skb->mark);
 }
 
-static void dump_ipv4_mac_header(struct nf_log_buf *m,
-				 const struct nf_loginfo *info,
-				 const struct sk_buff *skb)
+static void dump_mac_header(struct nf_log_buf *m,
+			    const struct nf_loginfo *info,
+			    const struct sk_buff *skb)
 {
 	struct net_device *dev = skb->dev;
 	unsigned int logflags = 0;
@@ -798,9 +798,26 @@ static void dump_ipv4_mac_header(struct nf_log_buf *m,
 		const unsigned char *p = skb_mac_header(skb);
 		unsigned int i;
 
-		nf_log_buf_add(m, "%02x", *p++);
-		for (i = 1; i < dev->hard_header_len; i++, p++)
-			nf_log_buf_add(m, ":%02x", *p);
+		if (dev->type == ARPHRD_SIT) {
+			p -= ETH_HLEN;
+
+			if (p < skb->head)
+				p = NULL;
+		}
+
+		if (p) {
+			nf_log_buf_add(m, "%02x", *p++);
+			for (i = 1; i < dev->hard_header_len; i++)
+				nf_log_buf_add(m, ":%02x", *p++);
+		}
+
+		if (dev->type == ARPHRD_SIT) {
+			const struct iphdr *iph =
+				(struct iphdr *)skb_mac_header(skb);
+
+			nf_log_buf_add(m, " TUNNEL=%pI4->%pI4", &iph->saddr,
+				       &iph->daddr);
+		}
 	}
 	nf_log_buf_add(m, " ");
 }
@@ -827,7 +844,7 @@ static void nf_log_ip_packet(struct net *net, u_int8_t pf,
 				  out, loginfo, prefix);
 
 	if (in)
-		dump_ipv4_mac_header(m, loginfo, skb);
+		dump_mac_header(m, loginfo, skb);
 
 	dump_ipv4_packet(net, m, loginfo, skb, 0);
 
@@ -841,64 +858,6 @@ static struct nf_logger nf_ip_logger __read_mostly = {
 	.me		= THIS_MODULE,
 };
 
-static void dump_ipv6_mac_header(struct nf_log_buf *m,
-				 const struct nf_loginfo *info,
-				 const struct sk_buff *skb)
-{
-	struct net_device *dev = skb->dev;
-	unsigned int logflags = 0;
-
-	if (info->type == NF_LOG_TYPE_LOG)
-		logflags = info->u.log.logflags;
-
-	if (!(logflags & NF_LOG_MACDECODE))
-		goto fallback;
-
-	switch (dev->type) {
-	case ARPHRD_ETHER:
-		nf_log_buf_add(m, "MACSRC=%pM MACDST=%pM ",
-			       eth_hdr(skb)->h_source, eth_hdr(skb)->h_dest);
-		nf_log_dump_vlan(m, skb);
-		nf_log_buf_add(m, "MACPROTO=%04x ",
-			       ntohs(eth_hdr(skb)->h_proto));
-		return;
-	default:
-		break;
-	}
-
-fallback:
-	nf_log_buf_add(m, "MAC=");
-	if (dev->hard_header_len &&
-	    skb->mac_header != skb->network_header) {
-		const unsigned char *p = skb_mac_header(skb);
-		unsigned int len = dev->hard_header_len;
-		unsigned int i;
-
-		if (dev->type == ARPHRD_SIT) {
-			p -= ETH_HLEN;
-
-			if (p < skb->head)
-				p = NULL;
-		}
-
-		if (p) {
-			nf_log_buf_add(m, "%02x", *p++);
-			for (i = 1; i < len; i++)
-				nf_log_buf_add(m, ":%02x", *p++);
-		}
-		nf_log_buf_add(m, " ");
-
-		if (dev->type == ARPHRD_SIT) {
-			const struct iphdr *iph =
-				(struct iphdr *)skb_mac_header(skb);
-			nf_log_buf_add(m, "TUNNEL=%pI4->%pI4 ", &iph->saddr,
-				       &iph->daddr);
-		}
-	} else {
-		nf_log_buf_add(m, " ");
-	}
-}
-
 static void nf_log_ip6_packet(struct net *net, u_int8_t pf,
 			      unsigned int hooknum, const struct sk_buff *skb,
 			      const struct net_device *in,
@@ -921,7 +880,7 @@ static void nf_log_ip6_packet(struct net *net, u_int8_t pf,
 				  loginfo, prefix);
 
 	if (in)
-		dump_ipv6_mac_header(m, loginfo, skb);
+		dump_mac_header(m, loginfo, skb);
 
 	dump_ipv6_packet(net, m, loginfo, skb, skb_network_offset(skb), 1);
 
-- 
2.34.1

[nf-next PATCH 2/2] netfilter: nf_log_syslog: Don't ignore unknown protocols

[Phil Sutter]

With netdev and bridge nfprotos, loggers may see arbitrary ethernet
frames. Print at least basic info like interfaces and MAC header data.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 net/netfilter/nf_log_syslog.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/net/netfilter/nf_log_syslog.c b/net/netfilter/nf_log_syslog.c
index d1dcf36545d79..a7ff6fdbafc94 100644
--- a/net/netfilter/nf_log_syslog.c
+++ b/net/netfilter/nf_log_syslog.c
@@ -894,6 +894,33 @@ static struct nf_logger nf_ip6_logger __read_mostly = {
 	.me		= THIS_MODULE,
 };
 
+static void nf_log_unknown_packet(struct net *net, u_int8_t pf,
+				  unsigned int hooknum,
+				  const struct sk_buff *skb,
+				  const struct net_device *in,
+				  const struct net_device *out,
+				  const struct nf_loginfo *loginfo,
+				  const char *prefix)
+{
+	struct nf_log_buf *m;
+
+	/* FIXME: Disabled from containers until syslog ns is supported */
+	if (!net_eq(net, &init_net) && !sysctl_nf_log_all_netns)
+		return;
+
+	m = nf_log_buf_open();
+
+	if (!loginfo)
+		loginfo = &default_loginfo;
+
+	nf_log_dump_packet_common(m, pf, hooknum, skb, in, out, loginfo,
+				  prefix);
+
+	dump_mac_header(m, loginfo, skb);
+
+	nf_log_buf_close(m);
+}
+
 static void nf_log_netdev_packet(struct net *net, u_int8_t pf,
 				 unsigned int hooknum,
 				 const struct sk_buff *skb,
@@ -913,6 +940,10 @@ static void nf_log_netdev_packet(struct net *net, u_int8_t pf,
 	case htons(ETH_P_RARP):
 		nf_log_arp_packet(net, pf, hooknum, skb, in, out, loginfo, prefix);
 		break;
+	default:
+		nf_log_unknown_packet(net, pf, hooknum, skb,
+				      in, out, loginfo, prefix);
+		break;
 	}
 }
 
-- 
2.34.1

Re: [nf-next PATCH 1/2] netfilter: nf_log_syslog: Merge MAC header dumpers

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> The functions for IPv4 and IPv6 were almost identical apart from extra
> SIT tunnel device handling in the latter.

LGTM, thanks.
Acked-by: Florian Westphal <fw@strlen.de>

Re: [nf-next PATCH 2/2] netfilter: nf_log_syslog: Don't ignore unknown protocols

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> With netdev and bridge nfprotos, loggers may see arbitrary ethernet
> frames. Print at least basic info like interfaces and MAC header data.

Makes sense to me.

> +	/* FIXME: Disabled from containers until syslog ns is supported */
> +	if (!net_eq(net, &init_net) && !sysctl_nf_log_all_netns)
> +		return;

Hmm, this is now the 3rd incarnation of this comment + check,
perhaps create another patch that adds a helper?

E.g.

if (!nf_log_allowed(net))
	return;

or similar.  Or just remove the FIXME line?  (Its not really a netfilter
todo/FIXME).

Re: [nf-next PATCH 2/2] netfilter: nf_log_syslog: Don't ignore unknown protocols

[Phil Sutter]

On Thu, Mar 24, 2022 at 03:16:14PM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > With netdev and bridge nfprotos, loggers may see arbitrary ethernet
> > frames. Print at least basic info like interfaces and MAC header data.
> 
> Makes sense to me.
> 
> > +	/* FIXME: Disabled from containers until syslog ns is supported */
> > +	if (!net_eq(net, &init_net) && !sysctl_nf_log_all_netns)
> > +		return;
> 
> Hmm, this is now the 3rd incarnation of this comment + check,
> perhaps create another patch that adds a helper?

Yes, I thought about that already but found it not worth it. OTOH,
copying outdated comments is a sin so I'll submit a follow-up.

Thanks, Phil

Re: [nf-next PATCH 1/2] netfilter: nf_log_syslog: Merge MAC header dumpers

[Pablo Neira Ayuso]

On Thu, Mar 24, 2022 at 03:03:40PM +0100, Phil Sutter wrote:
> The functions for IPv4 and IPv6 were almost identical apart from extra
> SIT tunnel device handling in the latter.

Applied, thanks

Re: [nf-next PATCH 2/2] netfilter: nf_log_syslog: Don't ignore unknown protocols

[Pablo Neira Ayuso]

On Thu, Mar 24, 2022 at 03:03:41PM +0100, Phil Sutter wrote:
> With netdev and bridge nfprotos, loggers may see arbitrary ethernet
> frames. Print at least basic info like interfaces and MAC header data.

Applied, thanks