geoip doesn't work with ipv6

geoip doesn't work with ipv6

[Thomas B. Clark]

I have tried exhaustively to get xt_geoip to match ipv6 addresses, and 
it doesn't.

I believe my configuration is correct, and that the geoip databases are 
set up correctly.  Matching correctly works on ipv4 with an equivalent 
iptables.

Re: geoip doesn't work with ipv6

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 466 bytes --]

On 2019-08-11, at 05:52:16 -0700, Thomas B. Clark wrote:
> I have tried exhaustively to get xt_geoip to match ipv6 addresses, and
> it doesn't.
>
> I believe my configuration is correct, and that the geoip databases
> are set up correctly.  Matching correctly works on ipv4 with an
> equivalent iptables.

There is an endianness mismatch between the ranges in the GeoIP DB and
the addresses when they are compared by xt_geoip.  I will submit a fix.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH xtables-addons 0/3] xt_geoip: ipv6 fixes

[Jeremy Sowden]

Thomas Clark reported that geoip matching didn't work for ipv6.  This
series fixes that and a couple of other minor issues.

Jeremy Sowden (3):
  configure: update max. supported kernel version.
  xt_geoip: white-space fixes.
  xt_geoip: fix in6_addr little-endian byte-swapping.

 configure.ac             |  2 +-
 extensions/libxt_geoip.c | 28 ++++++++--------------------
 extensions/xt_geoip.c    |  6 +++---
 3 files changed, 12 insertions(+), 24 deletions(-)

-- 
2.24.0

[PATCH xtables-addons 1/3] configure: Fix max. supported kernel version.

[Jeremy Sowden]

The maximum supported version is reported as 5.3.  Bump to 5.4.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index eccb09bf44b4..103cf07d6f9a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -57,7 +57,7 @@ if test -n "$kbuilddir"; then
 		echo "WARNING: Version detection did not succeed. Continue at own luck.";
 	else
 		echo "$kmajor.$kminor.$kmicro.$kstable in $kbuilddir";
-		if test "$kmajor" -gt 5 -o "$kmajor" -eq 5 -a "$kminor" -gt 3; then
+		if test "$kmajor" -gt 5 -o "$kmajor" -eq 5 -a "$kminor" -gt 4; then
 			echo "WARNING: That kernel version is not officially supported yet. Continue at own luck.";
 		elif test "$kmajor" -eq 5 -a "$kminor" -ge 0; then
 			:
-- 
2.24.0

[PATCH xtables-addons 1/3] configure: update max. supported kernel version.

[Jeremy Sowden]

The maximum supported version is reported as 5.3.  Bump to 5.4.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index eccb09bf44b4..103cf07d6f9a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -57,7 +57,7 @@ if test -n "$kbuilddir"; then
 		echo "WARNING: Version detection did not succeed. Continue at own luck.";
 	else
 		echo "$kmajor.$kminor.$kmicro.$kstable in $kbuilddir";
-		if test "$kmajor" -gt 5 -o "$kmajor" -eq 5 -a "$kminor" -gt 3; then
+		if test "$kmajor" -gt 5 -o "$kmajor" -eq 5 -a "$kminor" -gt 4; then
 			echo "WARNING: That kernel version is not officially supported yet. Continue at own luck.";
 		elif test "$kmajor" -eq 5 -a "$kminor" -ge 0; then
 			:
-- 
2.24.0

[PATCH xtables-addons 2/3] xt_geoip: white-space fixes.

[Jeremy Sowden]

Fix the indentation of some xt_geoip module function parameters.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/xt_geoip.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/extensions/xt_geoip.c b/extensions/xt_geoip.c
index 27e60a4643b7..d64d951f19a9 100644
--- a/extensions/xt_geoip.c
+++ b/extensions/xt_geoip.c
@@ -140,7 +140,7 @@ static void geoip_try_remove_node(struct geoip_country_kernel *p)
 }
 
 static struct geoip_country_kernel *find_node(unsigned short cc,
-    enum geoip_proto proto)
+					      enum geoip_proto proto)
 {
 	struct geoip_country_kernel *p;
 	spin_lock(&geoip_lock);
@@ -172,7 +172,7 @@ ipv6_cmp(const struct in6_addr *p, const struct in6_addr *q)
 }
 
 static bool geoip_bsearch6(const struct geoip_subnet6 *range,
-    const struct in6_addr *addr, int lo, int hi)
+			   const struct in6_addr *addr, int lo, int hi)
 {
 	int mid;
 
@@ -227,7 +227,7 @@ xt_geoip_mt6(const struct sk_buff *skb, struct xt_action_param *par)
 }
 
 static bool geoip_bsearch4(const struct geoip_subnet4 *range,
-    uint32_t addr, int lo, int hi)
+			   uint32_t addr, int lo, int hi)
 {
 	int mid;
 
-- 
2.24.0

[PATCH xtables-addons 3/3] xt_geoip: fix in6_addr little-endian byte-swapping.

[Jeremy Sowden]

libxt_geoip attempts to byte-swap IPv6 addresses on little-endian
systems but it doesn't get it quite right.  Rather than doing ntohl on
each 32-bit segment, it does ntohs on each 16-bit segment.

This means that:

  1234::cdef

becomes:

  2143::dcfe

instead of:

  4321::fedc

Fixes: b91dbd03c717 ("geoip: store database in network byte order")
Reported-by: "Thomas B. Clark" <kernel@clark.bz>
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/libxt_geoip.c | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/extensions/libxt_geoip.c b/extensions/libxt_geoip.c
index 116f5f86eb01..5b8697dc6161 100644
--- a/extensions/libxt_geoip.c
+++ b/extensions/libxt_geoip.c
@@ -50,26 +50,6 @@ static struct option geoip_opts[] = {
 };
 
 #if __BYTE_ORDER == __LITTLE_ENDIAN
-static void geoip_swap_le16(uint16_t *buf)
-{
-	unsigned char *p = (void *)buf;
-	uint16_t n= p[0] + (p[1] << 8);
-	p[0] = (n >> 8) & 0xff;
-	p[1] = n & 0xff;
-}
-
-static void geoip_swap_in6(struct in6_addr *in6)
-{
-	geoip_swap_le16(&in6->s6_addr16[0]);
-	geoip_swap_le16(&in6->s6_addr16[1]);
-	geoip_swap_le16(&in6->s6_addr16[2]);
-	geoip_swap_le16(&in6->s6_addr16[3]);
-	geoip_swap_le16(&in6->s6_addr16[4]);
-	geoip_swap_le16(&in6->s6_addr16[5]);
-	geoip_swap_le16(&in6->s6_addr16[6]);
-	geoip_swap_le16(&in6->s6_addr16[7]);
-}
-
 static void geoip_swap_le32(uint32_t *buf)
 {
 	unsigned char *p = (void *)buf;
@@ -79,6 +59,14 @@ static void geoip_swap_le32(uint32_t *buf)
 	p[2] = (n >> 8) & 0xff;
 	p[3] = n & 0xff;
 }
+
+static void geoip_swap_in6(struct in6_addr *in6)
+{
+	geoip_swap_le32(&in6->s6_addr32[0]);
+	geoip_swap_le32(&in6->s6_addr32[1]);
+	geoip_swap_le32(&in6->s6_addr32[2]);
+	geoip_swap_le32(&in6->s6_addr32[3]);
+}
 #endif
 
 static void *
-- 
2.24.0

Re: [PATCH xtables-addons 1/3] configure: Fix max. supported kernel version.

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 265 bytes --]

On 2019-11-30, at 17:02:16 +0000, Jeremy Sowden wrote:
> The maximum supported version is reported as 5.3.  Bump to 5.4.
>
> Signed-off-by: Jeremy Sowden <jeremy@azazel.net>

This is an old version of the first patch that I sent out by accident.
Please ignore.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH xtables-addons 3/3] xt_geoip: fix in6_addr little-endian byte-swapping.

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 430 bytes --]

On 2019-11-30, at 17:02:19 +0000, Jeremy Sowden wrote:
> libxt_geoip attempts to byte-swap IPv6 addresses on little-endian
> systems but it doesn't get it quite right.  Rather than doing ntohl on
> each 32-bit segment, it does ntohs on each 16-bit segment.
>
> This means that:
>
>   1234::cdef
>
> becomes:
>
>   2143::dcfe
>
> instead of:
>
>   4321::fedc

I'm an idiot.  This commit message is wrong.  Will fix and resend.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH xtables-addons v2 0/3] xt_geoip: ipv6 fixes

[Jeremy Sowden]

Thomas Clark reported that geoip matching didn't work for ipv6.  This
series fixes that and a couple of other minor issues.

Since v1:

  * fixed the commit-message in patch 03.

Jeremy Sowden (3):
  configure: update max. supported kernel version.
  xt_geoip: white-space fixes.
  xt_geoip: fix in6_addr little-endian byte-swapping.

 configure.ac             |  2 +-
 extensions/libxt_geoip.c | 28 ++++++++--------------------
 extensions/xt_geoip.c    |  6 +++---
 3 files changed, 12 insertions(+), 24 deletions(-)

-- 
2.24.0

[PATCH xtables-addons v2 1/3] configure: update max. supported kernel version.

[Jeremy Sowden]

The maximum supported version is reported as 5.3.  Bump to 5.4.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index eccb09bf44b4..103cf07d6f9a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -57,7 +57,7 @@ if test -n "$kbuilddir"; then
 		echo "WARNING: Version detection did not succeed. Continue at own luck.";
 	else
 		echo "$kmajor.$kminor.$kmicro.$kstable in $kbuilddir";
-		if test "$kmajor" -gt 5 -o "$kmajor" -eq 5 -a "$kminor" -gt 3; then
+		if test "$kmajor" -gt 5 -o "$kmajor" -eq 5 -a "$kminor" -gt 4; then
 			echo "WARNING: That kernel version is not officially supported yet. Continue at own luck.";
 		elif test "$kmajor" -eq 5 -a "$kminor" -ge 0; then
 			:
-- 
2.24.0

[PATCH xtables-addons v2 2/3] xt_geoip: white-space fixes.

[Jeremy Sowden]

Fix the indentation of some xt_geoip module function parameters.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/xt_geoip.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/extensions/xt_geoip.c b/extensions/xt_geoip.c
index 27e60a4643b7..d64d951f19a9 100644
--- a/extensions/xt_geoip.c
+++ b/extensions/xt_geoip.c
@@ -140,7 +140,7 @@ static void geoip_try_remove_node(struct geoip_country_kernel *p)
 }
 
 static struct geoip_country_kernel *find_node(unsigned short cc,
-    enum geoip_proto proto)
+					      enum geoip_proto proto)
 {
 	struct geoip_country_kernel *p;
 	spin_lock(&geoip_lock);
@@ -172,7 +172,7 @@ ipv6_cmp(const struct in6_addr *p, const struct in6_addr *q)
 }
 
 static bool geoip_bsearch6(const struct geoip_subnet6 *range,
-    const struct in6_addr *addr, int lo, int hi)
+			   const struct in6_addr *addr, int lo, int hi)
 {
 	int mid;
 
@@ -227,7 +227,7 @@ xt_geoip_mt6(const struct sk_buff *skb, struct xt_action_param *par)
 }
 
 static bool geoip_bsearch4(const struct geoip_subnet4 *range,
-    uint32_t addr, int lo, int hi)
+			   uint32_t addr, int lo, int hi)
 {
 	int mid;
 
-- 
2.24.0

[PATCH xtables-addons v2 3/3] xt_geoip: fix in6_addr little-endian byte-swapping.

[Jeremy Sowden]

The Perl script that builds the GeoIP DB's uses inet_pton(3) to convert
the addresses to network byte-order.  This swaps 32-bit segments and
converts:

  1234:5678::90ab:cdef

to:

  8765:4321::fedc:ba09

The kernel module compares the addresses in packets with the ranges from
the DB in host byte-order using binary search.  It uses 32-bit swaps
when converting the addresses.

libxt_geoip, however, which the module uses to load the ranges from the
DB and convert them from NBO to HBO, uses 16-bit swaps to do so, and
this means that:

  1234:5678::90ab:cdef

becomes:

  4321:8765::ba09:fedc

Obviously, this is inconsistent with the kernel-module and DB build-
script and breaks the binary search.

Fixes: b91dbd03c717 ("geoip: store database in network byte order")
Reported-by: "Thomas B. Clark" <kernel@clark.bz>
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/libxt_geoip.c | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/extensions/libxt_geoip.c b/extensions/libxt_geoip.c
index 116f5f86eb01..5b8697dc6161 100644
--- a/extensions/libxt_geoip.c
+++ b/extensions/libxt_geoip.c
@@ -50,26 +50,6 @@ static struct option geoip_opts[] = {
 };
 
 #if __BYTE_ORDER == __LITTLE_ENDIAN
-static void geoip_swap_le16(uint16_t *buf)
-{
-	unsigned char *p = (void *)buf;
-	uint16_t n= p[0] + (p[1] << 8);
-	p[0] = (n >> 8) & 0xff;
-	p[1] = n & 0xff;
-}
-
-static void geoip_swap_in6(struct in6_addr *in6)
-{
-	geoip_swap_le16(&in6->s6_addr16[0]);
-	geoip_swap_le16(&in6->s6_addr16[1]);
-	geoip_swap_le16(&in6->s6_addr16[2]);
-	geoip_swap_le16(&in6->s6_addr16[3]);
-	geoip_swap_le16(&in6->s6_addr16[4]);
-	geoip_swap_le16(&in6->s6_addr16[5]);
-	geoip_swap_le16(&in6->s6_addr16[6]);
-	geoip_swap_le16(&in6->s6_addr16[7]);
-}
-
 static void geoip_swap_le32(uint32_t *buf)
 {
 	unsigned char *p = (void *)buf;
@@ -79,6 +59,14 @@ static void geoip_swap_le32(uint32_t *buf)
 	p[2] = (n >> 8) & 0xff;
 	p[3] = n & 0xff;
 }
+
+static void geoip_swap_in6(struct in6_addr *in6)
+{
+	geoip_swap_le32(&in6->s6_addr32[0]);
+	geoip_swap_le32(&in6->s6_addr32[1]);
+	geoip_swap_le32(&in6->s6_addr32[2]);
+	geoip_swap_le32(&in6->s6_addr32[3]);
+}
 #endif
 
 static void *
-- 
2.24.0

Re: [PATCH xtables-addons v2 0/3] xt_geoip: ipv6 fixes

[Jan Engelhardt]

On Saturday 2019-11-30 18:58, Jeremy Sowden wrote:

>Thomas Clark reported that geoip matching didn't work for ipv6.  This
>series fixes that and a couple of other minor issues.

Applied 1, 3, with little fixes/additions to the messages.

Re: [PATCH xtables-addons v2 0/3] xt_geoip: ipv6 fixes

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 317 bytes --]

On 2019-12-01, at 11:34:50 +0100, Jan Engelhardt wrote:
> On Saturday 2019-11-30 18:58, Jeremy Sowden wrote:
> > Thomas Clark reported that geoip matching didn't work for ipv6.  This
> > series fixes that and a couple of other minor issues.
>
> Applied 1, 3, with little fixes/additions to the messages.

Thanks.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]