On Sep 25, 2025 Ricardo Robaina <rroba...@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.
>
> type=NETFILTER_PKT ... saddr=127.0.0.1 daddr=127.0.0.1 proto=icmp
> type=NETFILTER_PKT ... saddr=::1 daddr=::1 proto=ipv6-icmp
> type=NETFILTER_PKT ... daddr=127.0.0.1 proto=udp sport=38173 dport=42424
> type=NETFILTER_PKT ... daddr=::1 proto=udp sport=56852 dport=42424
> type=NETFILTER_PKT ... daddr=127.0.0.1 proto=tcp sport=57022 dport=42424
> type=NETFILTER_PKT ... daddr=::1 proto=tcp sport=50810 dport=42424
> type=NETFILTER_PKT ... daddr=127.0.0.1 proto=sctp sport=54944 dport=42424
> type=NETFILTER_PKT ... daddr=::1 proto=sctp sport=57963 dport=42424
>
> Link: https://github.com/linux-audit/audit-kernel/issues/162
> Signed-off-by: Ricardo Robaina <rroba...@redhat.com>
> ---
> net/netfilter/xt_AUDIT.c | 42 +++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
> index b6a015aee0ce..9fc8a5429fa9 100644
> --- a/net/netfilter/xt_AUDIT.c
> +++ b/net/netfilter/xt_AUDIT.c
> @@ -19,6 +19,7 @@
> #include <linux/netfilter_bridge/ebtables.h>
> #include <net/ipv6.h>
> #include <net/ip.h>
> +#include <linux/sctp.h>
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Thomas Graf <tg...@redhat.com>");
> @@ -32,6 +33,7 @@ static bool audit_ip4(struct audit_buffer *ab, struct
> sk_buff *skb)
> {
> struct iphdr _iph;
> const struct iphdr *ih;
> + __be16 dport, sport;
>
> ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_iph),
> &_iph);
> if (!ih)
> @@ -40,6 +42,25 @@ static bool audit_ip4(struct audit_buffer *ab, struct
> sk_buff *skb)
> audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu",
> &ih->saddr, &ih->daddr, ih->protocol);
>
> + switch (ih->protocol) {
> + case IPPROTO_TCP:
> + sport = tcp_hdr(skb)->source;
> + dport = tcp_hdr(skb)->dest;
> + break;
> + case IPPROTO_UDP:
> + case IPPROTO_UDPLITE:
> + sport = udp_hdr(skb)->source;
> + dport = udp_hdr(skb)->dest;
> + break;
> + case IPPROTO_SCTP:
> + sport = sctp_hdr(skb)->source;
> + dport = sctp_hdr(skb)->dest;
> + }
> +
> + if (ih->protocol == IPPROTO_TCP || ih->protocol == IPPROTO_UDP ||
> + ih->protocol == IPPROTO_UDPLITE || ih->protocol == IPPROTO_SCTP)
> + audit_log_format(ab, " sport=%hu dport=%hu", ntohs(sport),
> ntohs(dport));
> return true;
> }
Instead of having the switch statement and then doing an additional if
statement, why not fold it all into the switch statement? Yes, you
would have multiple audit_log_format() calls, but they are trivial to
cut-n-paste, and it saves the extra per-packet checking at runtime.
switch (ih->protocol) {
case IPPROTO_TCP:
audit_log_format(ab, " sport=...",
tcp_hdr(skb)->source,
tcp_hdr(skb)->dest);
break;
...
}
... considering how expensive multiple audit_log_format() calls can be,
it might even be worth considering consolidating the two calls into one:
switch (ih->protocol) {
case IPPROTO_TCP:
audit_log_format(ab, " saddr=...",
ih->saddr,
...
tcp_hdr(skb)->source,
tcp_hdr(skb)->dest);
break;
...
default:
audit_log_format(ab, " saddr=...",
ih->saddr,
...);
}
--
paul-moore.com
