[PATCH ulogd2 v3 00/11] Fixes for handling and output of IP addresses

[PATCH ulogd2 v3 00/11] Fixes for handling and output of IP addresses

[Jeremy Sowden]

Robert O'Brien reported a bug in the output of the source and target IP
addresses of ARP packets using the GPRINT output plug-in and proposed a
fix for that particular bug:

  https://lore.kernel.org/netfilter-devel/005601d8f532$49cd7080$dd685180$@foxtrot-research.com/

It transpired that there are a number of incorrect assumptions about the
format of IP addresses in the code-base.  In a couple of places there
are endianness mismatches, but more commonly it is assumed that all IP
addresses are IPv4.

In the previous versions of this work, my solution for fixing the
handling of IPv6 addresses was to handle all addresses internally as
IPv6 by converting IPv4 addresses to IPv4-in-IPv6 ("::ffff:a.b.c.d"),
and then convert IPv4-in-IPv6 address back to IPv4 on output.  However,
Florian pointed out that this means that if ulogd2 receives a real
IPv4-in-IPv6 address as input it will be indistinguishable from the
synthetic ones and so converted to IPv4 format on output.

In this version, I have taken a different approach.  Input keys have a
legnth field which is not used for fixed-width data-types.  I have used
this to distinguish 128-bit IPv6 addresses from 32-bit IPv4 ones.

I have also broken up the single patch of previous versions into a
series of smaller, hopefully more easily comprehensible ones, separating
out, in particular, the endianness fixes from the IPv6 ones.

One thing to note is that this changes the expected endianness of IP
address in the OPRINT plug-in.

Jeremy Sowden (11):
  src: record length of integer key values
  printpkt: fix statement punctuator
  printpkt, raw2packet_BASE: keep gateway address in NBO
  raw2packet_BASE: store ARP address values as integers
  ip2hbin: store ipv6 address as integer
  ipfix: skip non-ipv4 addresses
  gprint, oprint: use inet_ntop to format ip addresses
  gprint, oprint: add support for printing ipv6 addresses
  sqlite3: correct binding of ipv4 addresses and 64-bit integers
  sqlite3: insert ipv6 addresses as null rather than garbage
  db: insert ipv6 addresses in the same format as ip2bin

 filter/raw2packet/ulogd_raw2packet_BASE.c | 15 ++++---
 filter/ulogd_filter_IP2BIN.c              | 33 +--------------
 filter/ulogd_filter_IP2HBIN.c             |  9 ++--
 include/ulogd/ulogd.h                     | 50 ++++++++++++++++++++++-
 output/ipfix/ulogd_output_IPFIX.c         |  3 ++
 output/sqlite3/ulogd_output_SQLITE3.c     | 20 ++++++---
 output/ulogd_output_GPRINT.c              | 32 ++++++++++-----
 output/ulogd_output_OPRINT.c              | 41 +++++++++++--------
 util/db.c                                 | 19 +++++++--
 util/printpkt.c                           |  5 ++-
 10 files changed, 146 insertions(+), 81 deletions(-)

-- 
2.40.1

[PATCH ulogd2 v3 01/11] src: record length of integer key values

[Jeremy Sowden]

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 include/ulogd/ulogd.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/ulogd/ulogd.h b/include/ulogd/ulogd.h
index 092d9f521a70..cb0174042235 100644
--- a/include/ulogd/ulogd.h
+++ b/include/ulogd/ulogd.h
@@ -134,36 +134,42 @@ static inline void okey_set_b(struct ulogd_key *key, uint8_t value)
 {
 	key->u.value.b = value;
 	key->flags |= ULOGD_RETF_VALID;
+	key->len = sizeof(key->u.value.b);
 }
 
 static inline void okey_set_u8(struct ulogd_key *key, uint8_t value)
 {
 	key->u.value.ui8 = value;
 	key->flags |= ULOGD_RETF_VALID;
+	key->len = sizeof(key->u.value.ui8);
 }
 
 static inline void okey_set_u16(struct ulogd_key *key, uint16_t value)
 {
 	key->u.value.ui16 = value;
 	key->flags |= ULOGD_RETF_VALID;
+	key->len = sizeof(key->u.value.ui16);
 }
 
 static inline void okey_set_u32(struct ulogd_key *key, uint32_t value)
 {
 	key->u.value.ui32 = value;
 	key->flags |= ULOGD_RETF_VALID;
+	key->len = sizeof(key->u.value.ui32);
 }
 
 static inline void okey_set_u64(struct ulogd_key *key, uint64_t value)
 {
 	key->u.value.ui64 = value;
 	key->flags |= ULOGD_RETF_VALID;
+	key->len = sizeof(key->u.value.ui64);
 }
 
 static inline void okey_set_u128(struct ulogd_key *key, const void *value)
 {
-	memcpy(key->u.value.ui128, value, 16);
+	memcpy(key->u.value.ui128, value, sizeof(key->u.value.ui128));
 	key->flags |= ULOGD_RETF_VALID;
+	key->len = sizeof(key->u.value.ui128);
 }
 
 static inline void okey_set_ptr(struct ulogd_key *key, void *value)
@@ -309,6 +315,7 @@ void __ulogd_log(int level, char *file, int line, const char *message, ...)
 #define SET_NEEDED(x)	(x.flags |= ULOGD_RETF_NEEDED)
 
 #define GET_FLAGS(res, x)	(res[x].u.source->flags)
+#define GET_LENGTH(res, x)	(res[x].u.source->len)
 #define pp_is_valid(res, x)	\
 	(res[x].u.source && (GET_FLAGS(res, x) & ULOGD_RETF_VALID))
 
-- 
2.40.1

[PATCH ulogd2 v3 02/11] printpkt: fix statement punctuator

[Jeremy Sowden]

Replace comma with semicolon.

Fixes: d4cf078cb71a ("add ukey_* function for key assignation")
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 util/printpkt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/printpkt.c b/util/printpkt.c
index b9b47b2f63a0..11126b3c9af7 100644
--- a/util/printpkt.c
+++ b/util/printpkt.c
@@ -264,7 +264,7 @@ static int printpkt_ipv4(struct ulogd_key *res, char *buf)
 					   ikey_get_u32(&res[KEY_ICMP_GATEWAY]) >> 24);
 			break;
 		case ICMP_REDIRECT:
-			paddr = ikey_get_u32(&res[KEY_ICMP_GATEWAY]),
+			paddr = ikey_get_u32(&res[KEY_ICMP_GATEWAY]);
 			buf_cur += sprintf(buf_cur, "GATEWAY=%s ",
 					   inet_ntop(AF_INET,
 						     &paddr,
-- 
2.40.1

[PATCH ulogd2 v3 03/11] printpkt, raw2packet_BASE: keep gateway address in NBO

[Jeremy Sowden]

Everywhere else ipv4 addresses are left in NBO until output.  The only
exception is the IP2HBIN filter, which is explicitly intended to convert
from NBO to HBO.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 filter/raw2packet/ulogd_raw2packet_BASE.c | 2 +-
 util/printpkt.c                           | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/filter/raw2packet/ulogd_raw2packet_BASE.c b/filter/raw2packet/ulogd_raw2packet_BASE.c
index 9117d27da09a..14423486a880 100644
--- a/filter/raw2packet/ulogd_raw2packet_BASE.c
+++ b/filter/raw2packet/ulogd_raw2packet_BASE.c
@@ -645,7 +645,7 @@ static int _interp_icmp(struct ulogd_pluginstance *pi, struct icmphdr *icmph,
 		break;
 	case ICMP_REDIRECT:
 	case ICMP_PARAMETERPROB:
-		okey_set_u32(&ret[KEY_ICMP_GATEWAY], ntohl(icmph->un.gateway));
+		okey_set_u32(&ret[KEY_ICMP_GATEWAY], icmph->un.gateway);
 		break;
 	case ICMP_DEST_UNREACH:
 		if (icmph->code == ICMP_FRAG_NEEDED) {
diff --git a/util/printpkt.c b/util/printpkt.c
index 11126b3c9af7..09a219417f91 100644
--- a/util/printpkt.c
+++ b/util/printpkt.c
@@ -260,8 +260,9 @@ static int printpkt_ipv4(struct ulogd_key *res, char *buf)
 					   ikey_get_u16(&res[KEY_ICMP_ECHOSEQ]));
 			break;
 		case ICMP_PARAMETERPROB:
+			paddr = ikey_get_u32(&res[KEY_ICMP_GATEWAY]);
 			buf_cur += sprintf(buf_cur, "PARAMETER=%u ",
-					   ikey_get_u32(&res[KEY_ICMP_GATEWAY]) >> 24);
+					   *(uint8_t *) &paddr);
 			break;
 		case ICMP_REDIRECT:
 			paddr = ikey_get_u32(&res[KEY_ICMP_GATEWAY]);
-- 
2.40.1

[PATCH ulogd2 v3 04/11] raw2packet_BASE: store ARP address values as integers

[Jeremy Sowden]

Keys of type `ULOGD_RET_IPADDR` may be ipv4 or ipv6.  ARP protocol
addresses are 32-bits (i.e., ipv4).  By using `okey_set_u32` we keep
track of the size and allow downstream plug-ins to handle them
correctly.

Reported-by: Robert O'Brien <robrien@foxtrot-research.com>
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 filter/raw2packet/ulogd_raw2packet_BASE.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/filter/raw2packet/ulogd_raw2packet_BASE.c b/filter/raw2packet/ulogd_raw2packet_BASE.c
index 14423486a880..09e931349acf 100644
--- a/filter/raw2packet/ulogd_raw2packet_BASE.c
+++ b/filter/raw2packet/ulogd_raw2packet_BASE.c
@@ -896,18 +896,23 @@ static int _interp_arp(struct ulogd_pluginstance *pi, uint32_t len)
 	struct ulogd_key *ret = pi->output.keys;
 	const struct ether_arp *arph =
 		ikey_get_ptr(&pi->input.keys[INKEY_RAW_PCKT]);
+	uint32_t arp_spa, arp_tpa;
 
 	if (len < sizeof(struct ether_arp))
 		return ULOGD_IRET_OK;
 
-	okey_set_u16(&ret[KEY_ARP_HTYPE], ntohs(arph->arp_hrd));
-	okey_set_u16(&ret[KEY_ARP_PTYPE], ntohs(arph->arp_pro));
+	okey_set_u16(&ret[KEY_ARP_HTYPE],  ntohs(arph->arp_hrd));
+	okey_set_u16(&ret[KEY_ARP_PTYPE],  ntohs(arph->arp_pro));
 	okey_set_u16(&ret[KEY_ARP_OPCODE], ntohs(arph->arp_op));
 
 	okey_set_ptr(&ret[KEY_ARP_SHA], (void *)&arph->arp_sha);
-	okey_set_ptr(&ret[KEY_ARP_SPA], (void *)&arph->arp_spa);
 	okey_set_ptr(&ret[KEY_ARP_THA], (void *)&arph->arp_tha);
-	okey_set_ptr(&ret[KEY_ARP_TPA], (void *)&arph->arp_tpa);
+
+	memcpy(&arp_spa, arph->arp_spa, sizeof(arp_spa));
+	memcpy(&arp_tpa, arph->arp_tpa, sizeof(arp_tpa));
+
+	okey_set_u32(&ret[KEY_ARP_SPA], arp_spa);
+	okey_set_u32(&ret[KEY_ARP_TPA], arp_tpa);
 
 	return ULOGD_IRET_OK;
 }
-- 
2.40.1

[PATCH ulogd2 v3 05/11] ip2hbin: store ipv6 address as integer

[Jeremy Sowden]

By using `okey_set_u128` we keep track of the address size and
downstream plug-ins can distinguish the address family.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 filter/ulogd_filter_IP2HBIN.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/filter/ulogd_filter_IP2HBIN.c b/filter/ulogd_filter_IP2HBIN.c
index 2711f9c3e12a..081616edbc51 100644
--- a/filter/ulogd_filter_IP2HBIN.c
+++ b/filter/ulogd_filter_IP2HBIN.c
@@ -157,15 +157,14 @@ static int interp_ip2hbin(struct ulogd_pluginstance *pi)
 		if (pp_is_valid(inp, i)) {
 			switch (convfamily) {
 			case AF_INET:
-				okey_set_u32(&ret[i-START_KEY],
-					ntohl(ikey_get_u32(&inp[i])));
+				okey_set_u32(&ret[i - START_KEY],
+					     ntohl(ikey_get_u32(&inp[i])));
 				break;
 			case AF_INET6:
-				okey_set_ptr(&ret[i-START_KEY],
-					(struct in6_addr *)ikey_get_u128(&inp[i]));
+				okey_set_u128(&ret[i - START_KEY],
+					      ikey_get_u128(&inp[i]));
 				break;
 			default:
-				;
 				break;
 			}
 		}
-- 
2.40.1

[PATCH ulogd2 v3 06/11] ipfix: skip non-ipv4 addresses

[Jeremy Sowden]

This plug-in expects ipv4 addresses.  Check the length of the key value
in order to filter out ipv6 addresses.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 output/ipfix/ulogd_output_IPFIX.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/output/ipfix/ulogd_output_IPFIX.c b/output/ipfix/ulogd_output_IPFIX.c
index 4863d008562e..1c0f730b5b7c 100644
--- a/output/ipfix/ulogd_output_IPFIX.c
+++ b/output/ipfix/ulogd_output_IPFIX.c
@@ -426,6 +426,9 @@ static int ipfix_interp(struct ulogd_pluginstance *pi)
 	if (!(GET_FLAGS(pi->input.keys, InIpSaddr) & ULOGD_RETF_VALID))
 		return ULOGD_IRET_OK;
 
+	if (GET_LENGTH(pi->input.keys, InIpSaddr) != sizeof(data->saddr))
+		return ULOGD_IRET_OK;
+
 	oid = oid_ce(pi->config_kset).u.value;
 	mtu = mtu_ce(pi->config_kset).u.value;
 	send_template = send_template_ce(pi->config_kset).u.string;
-- 
2.40.1

[PATCH ulogd2 v3 07/11] gprint, oprint: use inet_ntop to format ip addresses

[Jeremy Sowden]

Replace hand-rolled ipv4-only formatting code in order to be able to
support ipv6 addresses.  This also changes the byte-order expected by
oprint from HBO to NBO.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 output/ulogd_output_GPRINT.c | 20 ++++++++++----------
 output/ulogd_output_OPRINT.c | 30 ++++++++++++++----------------
 2 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/output/ulogd_output_GPRINT.c b/output/ulogd_output_GPRINT.c
index eeeec6ac3eb0..093d3ea2b254 100644
--- a/output/ulogd_output_GPRINT.c
+++ b/output/ulogd_output_GPRINT.c
@@ -27,6 +27,8 @@
 #include <time.h>
 #include <errno.h>
 #include <inttypes.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
 #include <ulogd/ulogd.h>
 #include <ulogd/conffile.h>
 
@@ -69,12 +71,6 @@ static struct config_keyset gprint_kset = {
 	},
 };
 
-#define NIPQUAD(addr) \
-        ((unsigned char *)&addr)[0], \
-        ((unsigned char *)&addr)[1], \
-        ((unsigned char *)&addr)[2], \
-        ((unsigned char *)&addr)[3]
-
 static int gprint_interp(struct ulogd_pluginstance *upi)
 {
 	struct gprint_priv *opi = (struct gprint_priv *) &upi->private;
@@ -158,20 +154,24 @@ static int gprint_interp(struct ulogd_pluginstance *upi)
 			rem -= ret;
 			size += ret;
 			break;
-		case ULOGD_RET_IPADDR:
+		case ULOGD_RET_IPADDR: {
+			struct in_addr ipv4addr;
+
 			ret = snprintf(buf+size, rem, "%s=", key->name);
 			if (ret < 0)
 				break;
 			rem -= ret;
 			size += ret;
 
-			ret = snprintf(buf+size, rem, "%u.%u.%u.%u,",
-				NIPQUAD(key->u.value.ui32));
-			if (ret < 0)
+			ipv4addr.s_addr = key->u.value.ui32;
+			if (!inet_ntop(AF_INET, &ipv4addr, buf + size, rem))
 				break;
+			ret = strlen(buf + size);
+
 			rem -= ret;
 			size += ret;
 			break;
+		}
 		default:
 			/* don't know how to interpret this key. */
 			break;
diff --git a/output/ulogd_output_OPRINT.c b/output/ulogd_output_OPRINT.c
index 0409133e8227..b5586e850aa4 100644
--- a/output/ulogd_output_OPRINT.c
+++ b/output/ulogd_output_OPRINT.c
@@ -24,6 +24,8 @@
 #include <string.h>
 #include <errno.h>
 #include <inttypes.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
 #include <ulogd/ulogd.h>
 #include <ulogd/conffile.h>
 
@@ -31,18 +33,6 @@
 #define ULOGD_OPRINT_DEFAULT	"/var/log/ulogd_oprint.log"
 #endif
 
-#define NIPQUAD(addr) \
-	((unsigned char *)&addr)[0], \
-	((unsigned char *)&addr)[1], \
-        ((unsigned char *)&addr)[2], \
-        ((unsigned char *)&addr)[3]
-
-#define HIPQUAD(addr) \
-        ((unsigned char *)&addr)[3], \
-        ((unsigned char *)&addr)[2], \
-        ((unsigned char *)&addr)[1], \
-        ((unsigned char *)&addr)[0]
-
 struct oprint_priv {
 	FILE *of;
 };
@@ -59,7 +49,7 @@ static int oprint_interp(struct ulogd_pluginstance *upi)
 		if (!ret)
 			ulogd_log(ULOGD_NOTICE, "no result for %s ?!?\n",
 				  upi->input.keys[i].name);
-		
+
 		if (!IS_VALID(*ret))
 			continue;
 
@@ -85,10 +75,18 @@ static int oprint_interp(struct ulogd_pluginstance *upi)
 		case ULOGD_RET_UINT64:
 			fprintf(opi->of, "%" PRIu64 "\n", ret->u.value.ui64);
 			break;
-		case ULOGD_RET_IPADDR:
-			fprintf(opi->of, "%u.%u.%u.%u\n",
-				HIPQUAD(ret->u.value.ui32));
+		case ULOGD_RET_IPADDR: {
+			char addrbuf[INET_ADDRSTRLEN + 1] = "";
+			struct in_addr ipv4addr;
+
+			ipv4addr.s_addr = ret->u.value.ui32;
+			if (!inet_ntop(AF_INET, &ipv4addr, addrbuf,
+				       sizeof(addrbuf)))
+				break;
+
+			fprintf(opi->of, "%s\n", addrbuf);
 			break;
+		}
 		case ULOGD_RET_NONE:
 			fprintf(opi->of, "<none>\n");
 			break;
-- 
2.40.1

[PATCH ulogd2 v3 08/11] gprint, oprint: add support for printing ipv6 addresses

[Jeremy Sowden]

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 output/ulogd_output_GPRINT.c | 16 ++++++++++++++--
 output/ulogd_output_OPRINT.c | 21 ++++++++++++++++-----
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/output/ulogd_output_GPRINT.c b/output/ulogd_output_GPRINT.c
index 093d3ea2b254..37829fa49e9d 100644
--- a/output/ulogd_output_GPRINT.c
+++ b/output/ulogd_output_GPRINT.c
@@ -155,7 +155,10 @@ static int gprint_interp(struct ulogd_pluginstance *upi)
 			size += ret;
 			break;
 		case ULOGD_RET_IPADDR: {
+			struct in6_addr ipv6addr;
 			struct in_addr ipv4addr;
+			int family;
+			void *addr;
 
 			ret = snprintf(buf+size, rem, "%s=", key->name);
 			if (ret < 0)
@@ -163,8 +166,17 @@ static int gprint_interp(struct ulogd_pluginstance *upi)
 			rem -= ret;
 			size += ret;
 
-			ipv4addr.s_addr = key->u.value.ui32;
-			if (!inet_ntop(AF_INET, &ipv4addr, buf + size, rem))
+			if (key->len == sizeof(ipv6addr)) {
+				memcpy(ipv6addr.s6_addr, key->u.value.ui128,
+				       sizeof(ipv6addr.s6_addr));
+				addr = &ipv6addr;
+				family = AF_INET6;
+			} else {
+				ipv4addr.s_addr = key->u.value.ui32;
+				addr = &ipv4addr;
+				family = AF_INET;
+			}
+			if (!inet_ntop(family, addr, buf + size, rem))
 				break;
 			ret = strlen(buf + size);
 
diff --git a/output/ulogd_output_OPRINT.c b/output/ulogd_output_OPRINT.c
index b5586e850aa4..13934ff19efb 100644
--- a/output/ulogd_output_OPRINT.c
+++ b/output/ulogd_output_OPRINT.c
@@ -76,12 +76,23 @@ static int oprint_interp(struct ulogd_pluginstance *upi)
 			fprintf(opi->of, "%" PRIu64 "\n", ret->u.value.ui64);
 			break;
 		case ULOGD_RET_IPADDR: {
-			char addrbuf[INET_ADDRSTRLEN + 1] = "";
+			char addrbuf[INET6_ADDRSTRLEN + 1] = "";
+			struct in6_addr ipv6addr;
 			struct in_addr ipv4addr;
-
-			ipv4addr.s_addr = ret->u.value.ui32;
-			if (!inet_ntop(AF_INET, &ipv4addr, addrbuf,
-				       sizeof(addrbuf)))
+			int family;
+			void *addr;
+
+			if (ret->len == sizeof(ipv6addr)) {
+				memcpy(ipv6addr.s6_addr, ret->u.value.ui128,
+				       sizeof(ipv6addr.s6_addr));
+				addr = &ipv6addr;
+				family = AF_INET6;
+			} else {
+				ipv4addr.s_addr = ret->u.value.ui32;
+				addr = &ipv4addr;
+				family = AF_INET;
+			}
+			if (!inet_ntop(family, addr, addrbuf, sizeof(addrbuf)))
 				break;
 
 			fprintf(opi->of, "%s\n", addrbuf);
-- 
2.40.1

[PATCH ulogd2 v3 09/11] sqlite3: correct binding of ipv4 addresses and 64-bit integers

[Jeremy Sowden]

Hitherto we have bound ipv4 addresses as 64-bit ints and 64-bit ints as
32-bit.

Move a `ULOGD_RET_BOOL` case for consistency and fix some nearby
formatting.

Fix some nearby formatting.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 output/sqlite3/ulogd_output_SQLITE3.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/output/sqlite3/ulogd_output_SQLITE3.c b/output/sqlite3/ulogd_output_SQLITE3.c
index 0a9ad67edcff..3a823892a2dd 100644
--- a/output/sqlite3/ulogd_output_SQLITE3.c
+++ b/output/sqlite3/ulogd_output_SQLITE3.c
@@ -145,6 +145,10 @@ sqlite3_interp(struct ulogd_pluginstance *pi)
 		}
 
 		switch (f->key->type) {
+		case ULOGD_RET_BOOL:
+			ret = sqlite3_bind_int(priv->p_stmt, i, k_ret->u.value.b);
+			break;
+
 		case ULOGD_RET_INT8:
 			ret = sqlite3_bind_int(priv->p_stmt, i, k_ret->u.value.i8);
 			break;
@@ -158,7 +162,7 @@ sqlite3_interp(struct ulogd_pluginstance *pi)
 			break;
 
 		case ULOGD_RET_INT64:
-			ret = sqlite3_bind_int(priv->p_stmt, i, k_ret->u.value.i64);
+			ret = sqlite3_bind_int64(priv->p_stmt, i, k_ret->u.value.i64);
 			break;
 			
 		case ULOGD_RET_UINT8:
@@ -174,17 +178,16 @@ sqlite3_interp(struct ulogd_pluginstance *pi)
 			break;
 
 		case ULOGD_RET_IPADDR:
-		case ULOGD_RET_UINT64:
-			ret = sqlite3_bind_int64(priv->p_stmt, i, k_ret->u.value.ui64);
+			ret = sqlite3_bind_int(priv->p_stmt, i, k_ret->u.value.ui32);
 			break;
 
-		case ULOGD_RET_BOOL:
-			ret = sqlite3_bind_int(priv->p_stmt, i, k_ret->u.value.b);
+		case ULOGD_RET_UINT64:
+			ret = sqlite3_bind_int64(priv->p_stmt, i, k_ret->u.value.ui64);
 			break;
 
 		case ULOGD_RET_STRING:
 			ret = sqlite3_bind_text(priv->p_stmt, i, k_ret->u.value.ptr,
-									strlen(k_ret->u.value.ptr), SQLITE_STATIC);
+						strlen(k_ret->u.value.ptr), SQLITE_STATIC);
 			break;
 
 		default:
-- 
2.40.1

[PATCH ulogd2 v3 10/11] sqlite3: insert ipv6 addresses as null rather than garbage

[Jeremy Sowden]

Currently, the plug-in assumes that all IP addresses are 32-bit ipv4
addresses, so ipv6 addresses get truncated and inserted as garbage.
Insert nulls instead.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 output/sqlite3/ulogd_output_SQLITE3.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/output/sqlite3/ulogd_output_SQLITE3.c b/output/sqlite3/ulogd_output_SQLITE3.c
index 3a823892a2dd..6aeb7a3865e1 100644
--- a/output/sqlite3/ulogd_output_SQLITE3.c
+++ b/output/sqlite3/ulogd_output_SQLITE3.c
@@ -33,6 +33,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <arpa/inet.h>
+#include <netinet/in.h>
 #include <ulogd/ulogd.h>
 #include <ulogd/conffile.h>
 #include <sqlite3.h>
@@ -178,7 +179,11 @@ sqlite3_interp(struct ulogd_pluginstance *pi)
 			break;
 
 		case ULOGD_RET_IPADDR:
-			ret = sqlite3_bind_int(priv->p_stmt, i, k_ret->u.value.ui32);
+			if (k_ret->len == sizeof(struct in_addr))
+				ret = sqlite3_bind_int(priv->p_stmt, i,
+						       k_ret->u.value.ui32);
+			else
+				ret = sqlite3_bind_null(priv->p_stmt, i);
 			break;
 
 		case ULOGD_RET_UINT64:
-- 
2.40.1

[PATCH ulogd2 v3 11/11] db: insert ipv6 addresses in the same format as ip2bin

[Jeremy Sowden]

Move a `ULOGD_RET_BOOL` case for consistency.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 filter/ulogd_filter_IP2BIN.c | 33 +----------------------------
 include/ulogd/ulogd.h        | 41 ++++++++++++++++++++++++++++++++++++
 util/db.c                    | 19 +++++++++++++----
 3 files changed, 57 insertions(+), 36 deletions(-)

diff --git a/filter/ulogd_filter_IP2BIN.c b/filter/ulogd_filter_IP2BIN.c
index 74e46835ae53..7f7bea5071a7 100644
--- a/filter/ulogd_filter_IP2BIN.c
+++ b/filter/ulogd_filter_IP2BIN.c
@@ -116,27 +116,12 @@ static struct ulogd_key ip2bin_keys[] = {
 
 static char ipbin_array[MAX_KEY - START_KEY + 1][IPADDR_LENGTH];
 
-/**
- * Convert IPv4 address (as 32-bit unsigned integer) to IPv6 address:
- * add 96 bits prefix "::ffff:" to get IPv6 address "::ffff:a.b.c.d".
- */
-static inline void uint32_to_ipv6(const uint32_t ipv4, struct in6_addr *ipv6)
-{
-	ipv6->s6_addr32[0] = 0x00000000;
-	ipv6->s6_addr32[1] = 0x00000000;
-	ipv6->s6_addr32[2] = htonl(0xffff);
-	ipv6->s6_addr32[3] = ipv4;
-}
-
 static int ip2bin(struct ulogd_key *inp, int index, int oindex)
 {
 	char family = ikey_get_u8(&inp[KEY_OOB_FAMILY]);
 	char convfamily = family;
-	unsigned char *addr8;
 	struct in6_addr *addr;
 	struct in6_addr ip4_addr;
-	char *buffer;
-	int i, written;
 
 	if (family == AF_BRIDGE) {
 		if (!pp_is_valid(inp, KEY_OOB_PROTOCOL)) {
@@ -176,23 +161,7 @@ static int ip2bin(struct ulogd_key *inp, int index, int oindex)
 			return ULOGD_IRET_ERR;
 	}
 
-	buffer = ipbin_array[oindex];
-	/* format IPv6 to BINARY(16) as "0x..." */
-	buffer[0] = '0';
-	buffer[1] = 'x';
-	buffer += 2;
-	addr8 = &addr->s6_addr[0];
-	for (i = 0; i < 4; i++) {
-		written = sprintf(buffer, "%02x%02x%02x%02x",
-				  addr8[0], addr8[1], addr8[2], addr8[3]);
-		if (written != 2 * 4) {
-			buffer[0] = 0;
-			return ULOGD_IRET_ERR;
-		}
-		buffer += written;
-		addr8 += 4;
-	}
-	buffer[0] = 0;
+	format_ipv6(ipbin_array[oindex], IPADDR_LENGTH, addr);
 
 	return ULOGD_IRET_OK;
 }
diff --git a/include/ulogd/ulogd.h b/include/ulogd/ulogd.h
index cb0174042235..c7cf40277fcb 100644
--- a/include/ulogd/ulogd.h
+++ b/include/ulogd/ulogd.h
@@ -18,6 +18,7 @@
 #include <signal.h>	/* need this because of extension-sighandler */
 #include <sys/types.h>
 #include <inttypes.h>
+#include <netinet/in.h>
 #include <string.h>
 #include <config.h>
 
@@ -208,6 +209,46 @@ static inline void *ikey_get_ptr(struct ulogd_key *key)
 	return key->u.source->u.value.ptr;
 }
 
+/**
+ * Convert IPv4 address (as 32-bit unsigned integer) to IPv6 address: add 96-bit
+ * prefix "::ffff" to get IPv6 address "::ffff:a.b.c.d".
+ */
+static inline struct in6_addr *
+uint32_to_ipv6(uint32_t ipv4, struct in6_addr *ipv6)
+{
+	static const uint8_t IPV4_IN_IPV6_PREFIX[12] = {
+		[10] = 0xff,
+		[11] = 0xff,
+	};
+	uint8_t *p = ipv6->s6_addr;
+
+	memcpy(p, IPV4_IN_IPV6_PREFIX, sizeof(IPV4_IN_IPV6_PREFIX));
+	p += sizeof(IPV4_IN_IPV6_PREFIX);
+	memcpy(p, &ipv4, sizeof(ipv4));
+
+	return ipv6;
+}
+
+static inline void
+format_ipv6(char *buf, size_t size, const struct in6_addr *ipv6)
+{
+	unsigned i = 0;
+
+	if (size > 2 + sizeof (*ipv6) * 2) {
+		buf[i++] = '0';
+		buf[i++] = 'x';
+
+		for (unsigned j = 0; i < sizeof(*ipv6); j += 4, i += 8) {
+			sprintf(buf + i, "%02hhx%02hhx%02hhx%02hhx",
+				ipv6->s6_addr[j + 0],
+				ipv6->s6_addr[j + 1],
+				ipv6->s6_addr[j + 2],
+				ipv6->s6_addr[j + 3]);
+		}
+	}
+	buf[i] = '\0';
+}
+
 struct ulogd_pluginstance_stack;
 struct ulogd_pluginstance;
 
diff --git a/util/db.c b/util/db.c
index ebd9f152ed83..749a45ff5cd5 100644
--- a/util/db.c
+++ b/util/db.c
@@ -344,6 +344,9 @@ static void __format_query_db(struct ulogd_pluginstance *upi, char *start)
 		}
 
 		switch (res->type) {
+		case ULOGD_RET_BOOL:
+			sprintf(stmt_ins, "'%d',", res->u.value.b);
+			break;
 		case ULOGD_RET_INT8:
 			sprintf(stmt_ins, "%d,", res->u.value.i8);
 			break;
@@ -363,16 +366,24 @@ static void __format_query_db(struct ulogd_pluginstance *upi, char *start)
 			sprintf(stmt_ins, "%u,", res->u.value.ui16);
 			break;
 		case ULOGD_RET_IPADDR:
-			/* fallthrough when logging IP as uint32_t */
+			if (res->len == sizeof(struct in_addr))
+				sprintf(stmt_ins, "%u,", res->u.value.ui32);
+			else {
+				struct in6_addr ipv6;
+				char addrbuf[2 + sizeof(ipv6) * 2  + 1];
+
+				memcpy(ipv6.s6_addr, res->u.value.ui128,
+				       sizeof(ipv6.s6_addr));
+				format_ipv6(addrbuf, sizeof(addrbuf), &ipv6);
+				sprintf(stmt_ins, "%s,", addrbuf);
+			}
+			break;
 		case ULOGD_RET_UINT32:
 			sprintf(stmt_ins, "%u,", res->u.value.ui32);
 			break;
 		case ULOGD_RET_UINT64:
 			sprintf(stmt_ins, "%" PRIu64 ",", res->u.value.ui64);
 			break;
-		case ULOGD_RET_BOOL:
-			sprintf(stmt_ins, "'%d',", res->u.value.b);
-			break;
 		case ULOGD_RET_STRING:
 			*(stmt_ins++) = '\'';
 			if (res->u.value.ptr) {
-- 
2.40.1