[PATCH libnetfilter_conntrack 1/2] conntrack: fix BPF code for filtering on big-endian architectures

[PATCH libnetfilter_conntrack 1/2] conntrack: fix BPF code for filtering on big-endian architectures

[Jeremy Sowden]

The BPF for checking the subsystem ID looks for it in the righthand byte of
`nlh->nlmsg_type`.  However, it will only be there on little-endian archi-
tectures.  The result is that on big-endian architectures the subsystem ID
doesn't match, all packets are immediately accepted, and all filters are
ignored.

Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=896716
Fixes: b245e4092c5a ("src: allow to use nfct handler for conntrack and expectations at the same time")
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/conntrack/bsf.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/conntrack/bsf.c b/src/conntrack/bsf.c
index 1549815eedcc..589bfd8e5d18 100644
--- a/src/conntrack/bsf.c
+++ b/src/conntrack/bsf.c
@@ -9,6 +9,7 @@
 
 #include "internal/internal.h"
 #include "internal/stack.h"
+#include <endian.h>
 #include <linux/filter.h>
 #include <stddef.h>		/* offsetof */
 
@@ -301,10 +302,14 @@ bsf_cmp_subsys(struct sock_filter *this, int pos, uint8_t subsys)
 		[1] = {
 			/* A = skb->data[X+k:B] (subsys_id) */
 			.code	= BPF_LD|BPF_B|BPF_IND,
+#if BYTE_ORDER == BIG_ENDIAN
+			.k	= 0,
+#else
 			.k	= sizeof(uint8_t),
+#endif
 		},
 		[2] = {
-			/* A == subsys ? jump +1 : accept */
+			/* A == subsys ? jump + 1 : accept */
 			.code	= BPF_JMP|BPF_JEQ|BPF_K,
 			.k	= subsys,
 			.jt	= 1,
-- 
2.35.1

[PATCH libnetfilter_conntrack 2/2] conntrack: simplify calculation of `struct sock_fprog` length

[Jeremy Sowden]

When assigning the length to the `struct sock_fprog` object, we
calculate it by multiplying the number of `struct sock_filter` objects,
`j`, by `sizeof(struct sock_filter)` and then dividing by
`sizeof(bsf[0])`, which, since `bsf[0]` is a `struct sock_filter`, is
equal to `sizeof(struct sock_filter)`.

Remove the `sizeof` expressions and just assign `j`.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/conntrack/bsf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/conntrack/bsf.c b/src/conntrack/bsf.c
index 589bfd8e5d18..35cc8b7690c0 100644
--- a/src/conntrack/bsf.c
+++ b/src/conntrack/bsf.c
@@ -783,7 +783,7 @@ int __setup_netlink_socket_filter(int fd, struct nfct_filter *f)
 	show_filter(bsf, from, j, "---- final verdict ----");
 	from = j;
 
-	sf.len = (sizeof(struct sock_filter) * j) / sizeof(bsf[0]);
+	sf.len = j;
 	sf.filter = bsf;
 
 	return setsockopt(fd, SOL_SOCKET, SO_ATTACH_FILTER, &sf, sizeof(sf));
-- 
2.35.1

Re: [PATCH libnetfilter_conntrack 2/2] conntrack: simplify calculation of `struct sock_fprog` length

[Pablo Neira Ayuso]

On Fri, Dec 23, 2022 at 04:24:41PM +0000, Jeremy Sowden wrote:
> When assigning the length to the `struct sock_fprog` object, we
> calculate it by multiplying the number of `struct sock_filter` objects,
> `j`, by `sizeof(struct sock_filter)` and then dividing by
> `sizeof(bsf[0])`, which, since `bsf[0]` is a `struct sock_filter`, is
> equal to `sizeof(struct sock_filter)`.
> 
> Remove the `sizeof` expressions and just assign `j`.

Also applied, thanks

Re: [PATCH libnetfilter_conntrack 1/2] conntrack: fix BPF code for filtering on big-endian architectures

[Pablo Neira Ayuso]

On Fri, Dec 23, 2022 at 04:24:40PM +0000, Jeremy Sowden wrote:
> The BPF for checking the subsystem ID looks for it in the righthand byte of
> `nlh->nlmsg_type`.  However, it will only be there on little-endian archi-
> tectures.  The result is that on big-endian architectures the subsystem ID
> doesn't match, all packets are immediately accepted, and all filters are
> ignored.

Applied, thanks