[PATCH v5 0/2] audit: improve NETFILTER_PKT records

[PATCH v5 0/2] audit: improve NETFILTER_PKT records

[Ricardo Robaina]

Currently, NETFILTER_PKT records lack source and destination
port information, which is often valuable for troubleshooting.
This patch series adds ports numbers, to NETFILTER_PKT records.

The first patch refactors netfilter-related code, by moving
duplicated code to audit.c, creating two helper functions:
'audit_log_packet_ip4' and 'audit_log_packet_ip6'. 
The second one, improves the NETFILTER_PKT records, by 
including source and destination ports for protocols of
interest.

Ricardo Robaina (2):
  audit: add audit_log_packet_ip4 and audit_log_packet_ip6 helper
    functions
  audit: include source and destination ports to NETFILTER_PKT

 include/linux/audit.h    |  12 +++++
 kernel/audit.c           | 114 +++++++++++++++++++++++++++++++++++++++
 net/netfilter/nft_log.c  |  43 ++-------------
 net/netfilter/xt_AUDIT.c |  43 ++-------------
 4 files changed, 134 insertions(+), 78 deletions(-)

-- 
2.51.1

[PATCH v5 1/2] audit: add audit_log_packet_ip4 and audit_log_packet_ip6 helper functions

[Ricardo Robaina]

Netfilter code (net/netfilter/nft_log.c and net/netfilter/xt_AUDIT.c)
have to be kept in sync. Both source files had duplicated versions of
audit_ip4() and audit_ip6() functions, which can result in lack of
consistency and/or duplicated work.

This patch adds two helper functions in audit.c that can be called by
netfilter code commonly, aiming to improve maintainability and
consistency.

Signed-off-by: Ricardo Robaina <rrobaina@redhat.com>
---
 include/linux/audit.h    | 12 +++++++++++
 kernel/audit.c           | 39 ++++++++++++++++++++++++++++++++++++
 net/netfilter/nft_log.c  | 43 ++++------------------------------------
 net/netfilter/xt_AUDIT.c | 43 ++++------------------------------------
 4 files changed, 59 insertions(+), 78 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 536f8ee8da81..0f16ced08fdb 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -195,6 +195,8 @@ extern int audit_log_subj_ctx(struct audit_buffer *ab, struct lsm_prop *prop);
 extern int audit_log_obj_ctx(struct audit_buffer *ab, struct lsm_prop *prop);
 extern int audit_log_task_context(struct audit_buffer *ab);
 extern void audit_log_task_info(struct audit_buffer *ab);
+extern bool audit_log_packet_ip4(struct audit_buffer *ab, struct sk_buff *skb);
+extern bool audit_log_packet_ip6(struct audit_buffer *ab, struct sk_buff *skb);
 
 extern int		    audit_update_lsm_rules(void);
 
@@ -272,6 +274,16 @@ static inline int audit_log_task_context(struct audit_buffer *ab)
 static inline void audit_log_task_info(struct audit_buffer *ab)
 { }
 
+static inline bool audit_log_packet_ip4(struct audit_buffer *ab, struct sk_buff *skb)
+{
+	return false;
+}
+
+static inline bool audit_log_packet_ip6(struct audit_buffer *ab, struct sk_buff *skb)
+{
+	return false;
+}
+
 static inline kuid_t audit_get_loginuid(struct task_struct *tsk)
 {
 	return INVALID_UID;
diff --git a/kernel/audit.c b/kernel/audit.c
index 26a332ffb1b8..09764003db74 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -58,6 +58,8 @@
 #include <linux/freezer.h>
 #include <linux/pid_namespace.h>
 #include <net/netns/generic.h>
+#include <net/ip.h>
+#include <net/ipv6.h>
 
 #include "audit.h"
 
@@ -2538,6 +2540,43 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
 	audit_log_end(ab);
 }
 
+bool audit_log_packet_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)
+		return false;
+
+	audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu",
+			 &ih->saddr, &ih->daddr, ih->protocol);
+
+	return true;
+}
+EXPORT_SYMBOL(audit_log_packet_ip4);
+
+bool audit_log_packet_ip6(struct audit_buffer *ab, struct sk_buff *skb)
+{
+	struct ipv6hdr _ip6h;
+	const struct ipv6hdr *ih;
+	u8 nexthdr;
+	__be16 frag_off;
+
+	ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_ip6h), &_ip6h);
+	if (!ih)
+		return false;
+
+	nexthdr = ih->nexthdr;
+	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);
+
+	return true;
+}
+EXPORT_SYMBOL(audit_log_packet_ip6);
+
 /**
  * audit_set_loginuid - set current task's loginuid
  * @loginuid: loginuid value
diff --git a/net/netfilter/nft_log.c b/net/netfilter/nft_log.c
index e35588137995..f53fb4222134 100644
--- a/net/netfilter/nft_log.c
+++ b/net/netfilter/nft_log.c
@@ -26,41 +26,6 @@ struct nft_log {
 	char			*prefix;
 };
 
-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)
-		return false;
-
-	audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu",
-			 &ih->saddr, &ih->daddr, ih->protocol);
-
-	return true;
-}
-
-static bool audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
-{
-	struct ipv6hdr _ip6h;
-	const struct ipv6hdr *ih;
-	u8 nexthdr;
-	__be16 frag_off;
-
-	ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_ip6h), &_ip6h);
-	if (!ih)
-		return false;
-
-	nexthdr = ih->nexthdr;
-	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);
-
-	return true;
-}
-
 static void nft_log_eval_audit(const struct nft_pktinfo *pkt)
 {
 	struct sk_buff *skb = pkt->skb;
@@ -80,18 +45,18 @@ static void nft_log_eval_audit(const struct nft_pktinfo *pkt)
 	case NFPROTO_BRIDGE:
 		switch (eth_hdr(skb)->h_proto) {
 		case htons(ETH_P_IP):
-			fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
+			fam = audit_log_packet_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
 			break;
 		case htons(ETH_P_IPV6):
-			fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
+			fam = audit_log_packet_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
 			break;
 		}
 		break;
 	case NFPROTO_IPV4:
-		fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
+		fam = audit_log_packet_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
 		break;
 	case NFPROTO_IPV6:
-		fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
+		fam = audit_log_packet_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
 		break;
 	}
 
diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
index b6a015aee0ce..28cdd6435d56 100644
--- a/net/netfilter/xt_AUDIT.c
+++ b/net/netfilter/xt_AUDIT.c
@@ -28,41 +28,6 @@ MODULE_ALIAS("ip6t_AUDIT");
 MODULE_ALIAS("ebt_AUDIT");
 MODULE_ALIAS("arpt_AUDIT");
 
-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)
-		return false;
-
-	audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu",
-			 &ih->saddr, &ih->daddr, ih->protocol);
-
-	return true;
-}
-
-static bool audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
-{
-	struct ipv6hdr _ip6h;
-	const struct ipv6hdr *ih;
-	u8 nexthdr;
-	__be16 frag_off;
-
-	ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_ip6h), &_ip6h);
-	if (!ih)
-		return false;
-
-	nexthdr = ih->nexthdr;
-	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);
-
-	return true;
-}
-
 static unsigned int
 audit_tg(struct sk_buff *skb, const struct xt_action_param *par)
 {
@@ -81,18 +46,18 @@ audit_tg(struct sk_buff *skb, const struct xt_action_param *par)
 	case NFPROTO_BRIDGE:
 		switch (eth_hdr(skb)->h_proto) {
 		case htons(ETH_P_IP):
-			fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
+			fam = audit_log_packet_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
 			break;
 		case htons(ETH_P_IPV6):
-			fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
+			fam = audit_log_packet_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
 			break;
 		}
 		break;
 	case NFPROTO_IPV4:
-		fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
+		fam = audit_log_packet_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
 		break;
 	case NFPROTO_IPV6:
-		fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
+		fam = audit_log_packet_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
 		break;
 	}
 
-- 
2.51.1

[PATCH v5 2/2] audit: include source and destination ports to NETFILTER_PKT

[Ricardo Robaina]

NETFILTER_PKT records show both source and destination
addresses, in addition to the associated networking protocol.
However, it lacks the ports information, which is often
valuable for troubleshooting.

This patch adds both source and destination port numbers,
'sport' and 'dport' respectively, to TCP, UDP, UDP-Lite and
SCTP-related NETFILTER_PKT records.

 $ TESTS="netfilter_pkt" make -e test &> /dev/null
 $ ausearch -i -ts recent |grep NETFILTER_PKT
 type=NETFILTER_PKT ... proto=icmp
 type=NETFILTER_PKT ... proto=ipv6-icmp
 type=NETFILTER_PKT ... proto=udp sport=46333 dport=42424
 type=NETFILTER_PKT ... proto=udp sport=35953 dport=42424
 type=NETFILTER_PKT ... proto=tcp sport=50314 dport=42424
 type=NETFILTER_PKT ... proto=tcp sport=57346 dport=42424

Link: https://github.com/linux-audit/audit-kernel/issues/162

Signed-off-by: Ricardo Robaina <rrobaina@redhat.com>
---
 kernel/audit.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 79 insertions(+), 4 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 09764003db74..71dcadf12c99 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -60,6 +60,7 @@
 #include <net/netns/generic.h>
 #include <net/ip.h>
 #include <net/ipv6.h>
+#include <linux/sctp.h>
 
 #include "audit.h"
 
@@ -2544,13 +2545,50 @@ bool audit_log_packet_ip4(struct audit_buffer *ab, struct sk_buff *skb)
 {
 	struct iphdr _iph;
 	const struct iphdr *ih;
+	struct tcphdr _tcph;
+	const struct tcphdr *th;
+	struct udphdr _udph;
+	const struct udphdr *uh;
+	struct sctphdr _sctph;
+	const struct sctphdr *sh;
 
 	ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_iph), &_iph);
 	if (!ih)
 		return false;
 
-	audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu",
-			 &ih->saddr, &ih->daddr, ih->protocol);
+	switch (ih->protocol) {
+	case IPPROTO_TCP:
+		th = skb_header_pointer(skb, skb_transport_offset(skb), sizeof(_tcph), &_tcph);
+		if (!th)
+			return false;
+
+		audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu sport=%hu dport=%hu",
+				 &ih->saddr, &ih->daddr, ih->protocol,
+				 ntohs(th->source), ntohs(th->dest));
+		break;
+	case IPPROTO_UDP:
+	case IPPROTO_UDPLITE:
+		uh = skb_header_pointer(skb, skb_transport_offset(skb), sizeof(_udph), &_udph);
+		if (!uh)
+			return false;
+
+		audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu sport=%hu dport=%hu",
+				 &ih->saddr, &ih->daddr, ih->protocol,
+				 ntohs(uh->source), ntohs(uh->dest));
+		break;
+	case IPPROTO_SCTP:
+		sh = skb_header_pointer(skb, skb_transport_offset(skb), sizeof(_sctph), &_sctph);
+		if (!sh)
+			return false;
+
+		audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu sport=%hu dport=%hu",
+				 &ih->saddr, &ih->daddr, ih->protocol,
+				 ntohs(sh->source), ntohs(sh->dest));
+		break;
+	default:
+		audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu",
+				 &ih->saddr, &ih->daddr, ih->protocol);
+	}
 
 	return true;
 }
@@ -2562,6 +2600,12 @@ bool audit_log_packet_ip6(struct audit_buffer *ab, struct sk_buff *skb)
 	const struct ipv6hdr *ih;
 	u8 nexthdr;
 	__be16 frag_off;
+	struct tcphdr _tcph;
+	const struct tcphdr *th;
+	struct udphdr _udph;
+	const struct udphdr *uh;
+	struct sctphdr _sctph;
+	const struct sctphdr *sh;
 
 	ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_ip6h), &_ip6h);
 	if (!ih)
@@ -2570,8 +2614,39 @@ bool audit_log_packet_ip6(struct audit_buffer *ab, struct sk_buff *skb)
 	nexthdr = ih->nexthdr;
 	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);
+	switch (nexthdr) {
+	case IPPROTO_TCP:
+		th = skb_header_pointer(skb, skb_transport_offset(skb), sizeof(_tcph), &_tcph);
+		if (!th)
+			return false;
+
+		audit_log_format(ab, " saddr=%pI6c daddr=%pI6c proto=%hhu sport=%hu dport=%hu",
+				 &ih->saddr, &ih->daddr, nexthdr,
+				 ntohs(th->source), ntohs(th->dest));
+		break;
+	case IPPROTO_UDP:
+	case IPPROTO_UDPLITE:
+		uh = skb_header_pointer(skb, skb_transport_offset(skb), sizeof(_udph), &_udph);
+		if (!uh)
+			return false;
+
+		audit_log_format(ab, " saddr=%pI6c daddr=%pI6c proto=%hhu sport=%hu dport=%hu",
+				 &ih->saddr, &ih->daddr, nexthdr,
+				 ntohs(uh->source), ntohs(uh->dest));
+		break;
+	case IPPROTO_SCTP:
+		sh = skb_header_pointer(skb, skb_transport_offset(skb), sizeof(_sctph), &_sctph);
+		if (!sh)
+			return false;
+
+		audit_log_format(ab, " saddr=%pI6c daddr=%pI6c proto=%hhu sport=%hu dport=%hu",
+				 &ih->saddr, &ih->daddr, nexthdr,
+				 ntohs(sh->source), ntohs(sh->dest));
+		break;
+	default:
+		audit_log_format(ab, " saddr=%pI6c daddr=%pI6c proto=%hhu",
+				 &ih->saddr, &ih->daddr, nexthdr);
+	}
 
 	return true;
 }
-- 
2.51.1

Re: [PATCH v5 0/2] audit: improve NETFILTER_PKT records

[Florian Westphal]

Ricardo Robaina <rrobaina@redhat.com> wrote:
> Currently, NETFILTER_PKT records lack source and destination
> port information, which is often valuable for troubleshooting.
> This patch series adds ports numbers, to NETFILTER_PKT records.
> 
> The first patch refactors netfilter-related code, by moving
> duplicated code to audit.c, creating two helper functions:
> 'audit_log_packet_ip4' and 'audit_log_packet_ip6'. 
> The second one, improves the NETFILTER_PKT records, by 
> including source and destination ports for protocols of
> interest.

I'll assume this will go via audit tree, so:

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

Re: [PATCH v5 1/2] audit: add audit_log_packet_ip4 and audit_log_packet_ip6 helper functions

[Paul Moore]

On Nov  6, 2025 Ricardo Robaina <rrobaina@redhat.com> wrote:
> 
> Netfilter code (net/netfilter/nft_log.c and net/netfilter/xt_AUDIT.c)
> have to be kept in sync. Both source files had duplicated versions of
> audit_ip4() and audit_ip6() functions, which can result in lack of
> consistency and/or duplicated work.
> 
> This patch adds two helper functions in audit.c that can be called by
> netfilter code commonly, aiming to improve maintainability and
> consistency.
> 
> Signed-off-by: Ricardo Robaina <rrobaina@redhat.com>
> Acked-by: Florian Westphal <fw@strlen.de>
> ---
>  include/linux/audit.h    | 12 +++++++++++
>  kernel/audit.c           | 39 ++++++++++++++++++++++++++++++++++++
>  net/netfilter/nft_log.c  | 43 ++++------------------------------------
>  net/netfilter/xt_AUDIT.c | 43 ++++------------------------------------
>  4 files changed, 59 insertions(+), 78 deletions(-)

...

> diff --git a/net/netfilter/nft_log.c b/net/netfilter/nft_log.c
> index e35588137995..f53fb4222134 100644
> --- a/net/netfilter/nft_log.c
> +++ b/net/netfilter/nft_log.c
> @@ -26,41 +26,6 @@ struct nft_log {
>  	char			*prefix;
>  };
>  
> -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)
> -		return false;
> -
> -	audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu",
> -			 &ih->saddr, &ih->daddr, ih->protocol);
> -
> -	return true;
> -}
> -
> -static bool audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
> -{
> -	struct ipv6hdr _ip6h;
> -	const struct ipv6hdr *ih;
> -	u8 nexthdr;
> -	__be16 frag_off;
> -
> -	ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_ip6h), &_ip6h);
> -	if (!ih)
> -		return false;
> -
> -	nexthdr = ih->nexthdr;
> -	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);
> -
> -	return true;
> -}
> -
>  static void nft_log_eval_audit(const struct nft_pktinfo *pkt)
>  {
>  	struct sk_buff *skb = pkt->skb;
> @@ -80,18 +45,18 @@ static void nft_log_eval_audit(const struct nft_pktinfo *pkt)
>  	case NFPROTO_BRIDGE:
>  		switch (eth_hdr(skb)->h_proto) {
>  		case htons(ETH_P_IP):
> -			fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
> +			fam = audit_log_packet_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
>  			break;
>  		case htons(ETH_P_IPV6):
> -			fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
> +			fam = audit_log_packet_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
>  			break;
>  		}
>  		break;
>  	case NFPROTO_IPV4:
> -		fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
> +		fam = audit_log_packet_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
>  		break;
>  	case NFPROTO_IPV6:
> -		fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
> +		fam = audit_log_packet_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
>  		break;
>  	}

We can probably take this a step further by moving the case statements
into the audit functions too.  I think this will make some of the other
changes a bit cleaner and should reduce the amount of audit code in the
NFT code.

If we don't want to do that, it might be worthwhile to take the
NFPROTO_BRIDGE protocol family reset shown below in audit_log_nft_skb()
and use that in the nft_log_eval_audit() function so we aren't
duplicating calls into the audit code.

[WARNING: completely untested code, but you should get the basic idea]

diff --git a/kernel/audit.c b/kernel/audit.c
index 26a332ffb1b8..72ba3f51f859 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2538,6 +2538,59 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
        audit_log_end(ab);
 }
 
+int audit_log_nft_skb(struct audit_buffer *ab,
+                     struct sk_buff *skb, u8 nfproto)
+{
+       /* find the IP protocol in the case of NFPROTO_BRIDGE */
+       if (nfproto == NFPROTO_BRIDGE) {
+               switch (eth_hdr(skb)->h_proto) {
+               case htons(ETH_P_IP):
+                       nfproto = NFPROTO_IPV4;
+               case htons(ETH_P_IPV6):
+                       nfproto = NFPROTO_IPV6;
+               default:
+                       goto unknown_proto;
+               }
+       }
+
+       switch (nfproto) {
+       case NFPROTO_IPV4: {
+               struct iphdr iph;
+               const struct iphdr *ih;
+
+               ih = skb_header_pointer(skb, skb_network_offset(skb),
+                                       sizeof(_iph), &_iph);
+               if (!ih)
+                       return -ENOMEM;
+
+               audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu",
+                                &ih->saddr, &ih->daddr, ih->protocol);
+               break;
+       }
+       case NFPROTO_IPV6: {
+               struct ipv6hdr iph;
+               const struct ipv6hdr *ih;
+
+               ih = skb_header_pointer(skb, skb_network_offset(skb),
+                                       sizeof(_iph), &_iph);
+               if (!ih)
+                       return -ENOMEM;
+
+               audit_log_format(ab, " saddr=%pI6 daddr=%pI6 proto=%hhu",
+                                &ih->saddr, &ih->daddr, ih->protocol);
+               break;
+       }
+       default:
+               goto unknown_proto;
+       }
+
+       return 0;
+
+unknown_proto:
+       audit_log_format(ab, " saddr=? daddr=? proto=?");
+       return -EPFNOSUPPORT;
+}
+
 /**
  * audit_set_loginuid - set current task's loginuid
  * @loginuid: loginuid value
diff --git a/net/netfilter/nft_log.c b/net/netfilter/nft_log.c
index e35588137995..6f444e2ad70a 100644
--- a/net/netfilter/nft_log.c
+++ b/net/netfilter/nft_log.c
@@ -75,28 +75,7 @@ static void nft_log_eval_audit(const struct nft_pktinfo *pkt)
                return;
 
        audit_log_format(ab, "mark=%#x", skb->mark);
-
-       switch (nft_pf(pkt)) {
-       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:
-               fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
-               break;
-       case NFPROTO_IPV6:
-               fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
-               break;
-       }
-
-       if (fam == -1)
-               audit_log_format(ab, " saddr=? daddr=? proto=-1");
+       audit_log_nft_skb(ab, skb, nft_pf(pkt));
 
        audit_log_end(ab);
 }

--
paul-moore.com

Re: [PATCH v5 2/2] audit: include source and destination ports to NETFILTER_PKT

[Paul Moore]

On Nov  6, 2025 Ricardo Robaina <rrobaina@redhat.com> wrote:
> 
> NETFILTER_PKT records show both source and destination
> addresses, in addition to the associated networking protocol.
> However, it lacks the ports information, which is often
> valuable for troubleshooting.
> 
> This patch adds both source and destination port numbers,
> 'sport' and 'dport' respectively, to TCP, UDP, UDP-Lite and
> SCTP-related NETFILTER_PKT records.
> 
>  $ TESTS="netfilter_pkt" make -e test &> /dev/null
>  $ ausearch -i -ts recent |grep NETFILTER_PKT
>  type=NETFILTER_PKT ... proto=icmp
>  type=NETFILTER_PKT ... proto=ipv6-icmp
>  type=NETFILTER_PKT ... proto=udp sport=46333 dport=42424
>  type=NETFILTER_PKT ... proto=udp sport=35953 dport=42424
>  type=NETFILTER_PKT ... proto=tcp sport=50314 dport=42424
>  type=NETFILTER_PKT ... proto=tcp sport=57346 dport=42424
> 
> Link: https://github.com/linux-audit/audit-kernel/issues/162
> 
> Signed-off-by: Ricardo Robaina <rrobaina@redhat.com>
> Acked-by: Florian Westphal <fw@strlen.de>
> ---
>  kernel/audit.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 79 insertions(+), 4 deletions(-)

This looks fine to me, although it may change a bit based on the
discussion around patch 1/2.  However, two things I wanted to comment
on in this patch:

- Please try to stick to an 80 char line width for audit code.  There are
obvious exceptions like printf-esque strings, etc. but the
skb_header_pointer() calls in this patch could be easily split into
multiple lines, each under 80 chars.

- This isn't a general comment, but in this particular case it would be
nice to move the protocol header variables into their associated switch
case (see what I did in patch 1/2).

--
paul-moore.com

Re: [PATCH v5 1/2] audit: add audit_log_packet_ip4 and audit_log_packet_ip6 helper functions

[Ricardo Robaina]

On Fri, Nov 7, 2025 at 7:46 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Nov  6, 2025 Ricardo Robaina <rrobaina@redhat.com> wrote:
> >
> > Netfilter code (net/netfilter/nft_log.c and net/netfilter/xt_AUDIT.c)
> > have to be kept in sync. Both source files had duplicated versions of
> > audit_ip4() and audit_ip6() functions, which can result in lack of
> > consistency and/or duplicated work.
> >
> > This patch adds two helper functions in audit.c that can be called by
> > netfilter code commonly, aiming to improve maintainability and
> > consistency.
> >
> > Signed-off-by: Ricardo Robaina <rrobaina@redhat.com>
> > Acked-by: Florian Westphal <fw@strlen.de>
> > ---
> >  include/linux/audit.h    | 12 +++++++++++
> >  kernel/audit.c           | 39 ++++++++++++++++++++++++++++++++++++
> >  net/netfilter/nft_log.c  | 43 ++++------------------------------------
> >  net/netfilter/xt_AUDIT.c | 43 ++++------------------------------------
> >  4 files changed, 59 insertions(+), 78 deletions(-)
>
> ...
>
> > diff --git a/net/netfilter/nft_log.c b/net/netfilter/nft_log.c
> > index e35588137995..f53fb4222134 100644
> > --- a/net/netfilter/nft_log.c
> > +++ b/net/netfilter/nft_log.c
> > @@ -26,41 +26,6 @@ struct nft_log {
> >       char                    *prefix;
> >  };
> >
> > -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)
> > -             return false;
> > -
> > -     audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu",
> > -                      &ih->saddr, &ih->daddr, ih->protocol);
> > -
> > -     return true;
> > -}
> > -
> > -static bool audit_ip6(struct audit_buffer *ab, struct sk_buff *skb)
> > -{
> > -     struct ipv6hdr _ip6h;
> > -     const struct ipv6hdr *ih;
> > -     u8 nexthdr;
> > -     __be16 frag_off;
> > -
> > -     ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_ip6h), &_ip6h);
> > -     if (!ih)
> > -             return false;
> > -
> > -     nexthdr = ih->nexthdr;
> > -     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);
> > -
> > -     return true;
> > -}
> > -
> >  static void nft_log_eval_audit(const struct nft_pktinfo *pkt)
> >  {
> >       struct sk_buff *skb = pkt->skb;
> > @@ -80,18 +45,18 @@ static void nft_log_eval_audit(const struct nft_pktinfo *pkt)
> >       case NFPROTO_BRIDGE:
> >               switch (eth_hdr(skb)->h_proto) {
> >               case htons(ETH_P_IP):
> > -                     fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
> > +                     fam = audit_log_packet_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
> >                       break;
> >               case htons(ETH_P_IPV6):
> > -                     fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
> > +                     fam = audit_log_packet_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
> >                       break;
> >               }
> >               break;
> >       case NFPROTO_IPV4:
> > -             fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
> > +             fam = audit_log_packet_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
> >               break;
> >       case NFPROTO_IPV6:
> > -             fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
> > +             fam = audit_log_packet_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
> >               break;
> >       }
>
> We can probably take this a step further by moving the case statements
> into the audit functions too.  I think this will make some of the other
> changes a bit cleaner and should reduce the amount of audit code in the
> NFT code.
>
> If we don't want to do that, it might be worthwhile to take the
> NFPROTO_BRIDGE protocol family reset shown below in audit_log_nft_skb()
> and use that in the nft_log_eval_audit() function so we aren't
> duplicating calls into the audit code.
>
> [WARNING: completely untested code, but you should get the basic idea]
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 26a332ffb1b8..72ba3f51f859 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2538,6 +2538,59 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
>         audit_log_end(ab);
>  }
>
> +int audit_log_nft_skb(struct audit_buffer *ab,
> +                     struct sk_buff *skb, u8 nfproto)
> +{
> +       /* find the IP protocol in the case of NFPROTO_BRIDGE */
> +       if (nfproto == NFPROTO_BRIDGE) {
> +               switch (eth_hdr(skb)->h_proto) {
> +               case htons(ETH_P_IP):
> +                       nfproto = NFPROTO_IPV4;
> +               case htons(ETH_P_IPV6):
> +                       nfproto = NFPROTO_IPV6;
> +               default:
> +                       goto unknown_proto;
> +               }
> +       }
> +
> +       switch (nfproto) {
> +       case NFPROTO_IPV4: {
> +               struct iphdr iph;
> +               const struct iphdr *ih;
> +
> +               ih = skb_header_pointer(skb, skb_network_offset(skb),
> +                                       sizeof(_iph), &_iph);
> +               if (!ih)
> +                       return -ENOMEM;
> +
> +               audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu",
> +                                &ih->saddr, &ih->daddr, ih->protocol);
> +               break;
> +       }
> +       case NFPROTO_IPV6: {
> +               struct ipv6hdr iph;
> +               const struct ipv6hdr *ih;
> +
> +               ih = skb_header_pointer(skb, skb_network_offset(skb),
> +                                       sizeof(_iph), &_iph);
> +               if (!ih)
> +                       return -ENOMEM;
> +
> +               audit_log_format(ab, " saddr=%pI6 daddr=%pI6 proto=%hhu",
> +                                &ih->saddr, &ih->daddr, ih->protocol);
> +               break;
> +       }
> +       default:
> +               goto unknown_proto;
> +       }
> +
> +       return 0;
> +
> +unknown_proto:
> +       audit_log_format(ab, " saddr=? daddr=? proto=?");
> +       return -EPFNOSUPPORT;
> +}
> +
>  /**
>   * audit_set_loginuid - set current task's loginuid
>   * @loginuid: loginuid value
> diff --git a/net/netfilter/nft_log.c b/net/netfilter/nft_log.c
> index e35588137995..6f444e2ad70a 100644
> --- a/net/netfilter/nft_log.c
> +++ b/net/netfilter/nft_log.c
> @@ -75,28 +75,7 @@ static void nft_log_eval_audit(const struct nft_pktinfo *pkt)
>                 return;
>
>         audit_log_format(ab, "mark=%#x", skb->mark);
> -
> -       switch (nft_pf(pkt)) {
> -       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:
> -               fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1;
> -               break;
> -       case NFPROTO_IPV6:
> -               fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1;
> -               break;
> -       }
> -
> -       if (fam == -1)
> -               audit_log_format(ab, " saddr=? daddr=? proto=-1");
> +       audit_log_nft_skb(ab, skb, nft_pf(pkt));
>
>         audit_log_end(ab);
>  }
>
> --
> paul-moore.com
>

Thanks for reviewing this patch, Paul.

It makes sense to me. I'll work on a newer version addressing your suggestions.

Re: [PATCH v5 2/2] audit: include source and destination ports to NETFILTER_PKT

[Ricardo Robaina]

On Fri, Nov 7, 2025 at 7:46 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Nov  6, 2025 Ricardo Robaina <rrobaina@redhat.com> wrote:
> >
> > NETFILTER_PKT records show both source and destination
> > addresses, in addition to the associated networking protocol.
> > However, it lacks the ports information, which is often
> > valuable for troubleshooting.
> >
> > This patch adds both source and destination port numbers,
> > 'sport' and 'dport' respectively, to TCP, UDP, UDP-Lite and
> > SCTP-related NETFILTER_PKT records.
> >
> >  $ TESTS="netfilter_pkt" make -e test &> /dev/null
> >  $ ausearch -i -ts recent |grep NETFILTER_PKT
> >  type=NETFILTER_PKT ... proto=icmp
> >  type=NETFILTER_PKT ... proto=ipv6-icmp
> >  type=NETFILTER_PKT ... proto=udp sport=46333 dport=42424
> >  type=NETFILTER_PKT ... proto=udp sport=35953 dport=42424
> >  type=NETFILTER_PKT ... proto=tcp sport=50314 dport=42424
> >  type=NETFILTER_PKT ... proto=tcp sport=57346 dport=42424
> >
> > Link: https://github.com/linux-audit/audit-kernel/issues/162
> >
> > Signed-off-by: Ricardo Robaina <rrobaina@redhat.com>
> > Acked-by: Florian Westphal <fw@strlen.de>
> > ---
> >  kernel/audit.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 79 insertions(+), 4 deletions(-)
>
> This looks fine to me, although it may change a bit based on the
> discussion around patch 1/2.  However, two things I wanted to comment
> on in this patch:
>
> - Please try to stick to an 80 char line width for audit code.  There are
> obvious exceptions like printf-esque strings, etc. but the
> skb_header_pointer() calls in this patch could be easily split into
> multiple lines, each under 80 chars.
>

Thanks for the feedback! I'll make sure to follow this guideline from now on.

> - This isn't a general comment, but in this particular case it would be
> nice to move the protocol header variables into their associated switch
> case (see what I did in patch 1/2).
>

Nice, thanks for the tip! I wasn't sure which style to use, so I
decided to use the classic one. However, I do prefer having the
protocol header variables within their associated switch case, too.

> --
> paul-moore.com
>

Re: [PATCH v5 1/2] audit: add audit_log_packet_ip4 and audit_log_packet_ip6 helper functions

[Florian Westphal]

Ricardo Robaina <rrobaina@redhat.com> wrote:
> > +int audit_log_nft_skb(struct audit_buffer *ab,
> > +                     struct sk_buff *skb, u8 nfproto)
> Thanks for reviewing this patch, Paul.
> 
> It makes sense to me. I'll work on a newer version addressing your suggestions.

Nit, but as you need to resend anyway, can you also make this
'const struct sk_buff *' ?

Also, given this isn't nftables specific, I suggest
audit_log_nf_skb, audit_log_netfilter_skb or some such instead.

Thanks.

Re: [PATCH v5 1/2] audit: add audit_log_packet_ip4 and audit_log_packet_ip6 helper functions

[Ricardo Robaina]

On Mon, Nov 10, 2025 at 9:43 AM Florian Westphal <fw@strlen.de> wrote:
>
> Ricardo Robaina <rrobaina@redhat.com> wrote:
> > > +int audit_log_nft_skb(struct audit_buffer *ab,
> > > +                     struct sk_buff *skb, u8 nfproto)
> > Thanks for reviewing this patch, Paul.
> >
> > It makes sense to me. I'll work on a newer version addressing your suggestions.
>
> Nit, but as you need to resend anyway, can you also make this
> 'const struct sk_buff *' ?
>
> Also, given this isn't nftables specific, I suggest
> audit_log_nf_skb, audit_log_netfilter_skb or some such instead.
>
> Thanks.
>

Sure!