bugfix: reverse lookup reduction logic in imudp do DNS queries too often

A comparison was done between the current and the former source address.
However, this was done on the full sockaddr_storage structure and not
on the host address only. This has now been changed for IPv4 and IPv6.
The end result of this bug could be a higher UDP message loss rate than
necessary (note that UDP message loss can not totally be avoided due
to the UDP spec)

4 files changed, 57 insertions, 2 deletions

diff --git a/ChangeLog b/ChangeLog
index b84ad4d8..27dc26b3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,13 @@
 ---------------------------------------------------------------------------
+Version 4.4.2  [v4-stable] (rgerhards), 2009-09-??
+- bugfix: reverse lookup reduction logic in imudp do DNS queries too often
+  A comparison was done between the current and the former source address.
+  However, this was done on the full sockaddr_storage structure and not
+  on the host address only. This has now been changed for IPv4 and IPv6.
+  The end result of this bug could be a higher UDP message loss rate than
+  necessary (note that UDP message loss can not totally be avoided due
+  to the UDP spec)
+---------------------------------------------------------------------------
 Version 4.4.1  [v4-stable] (rgerhards), 2009-09-02
 - features requiring Java are automatically disabled if Java is not
   present (thanks to Michael Biebl for his help!)
diff --git a/plugins/imudp/imudp.c b/plugins/imudp/imudp.c
index 57aec9b6..658ceb23 100644
--- a/plugins/imudp/imudp.c
+++ b/plugins/imudp/imudp.c
@@ -180,7 +180,7 @@ processSocket(int fd, struct sockaddr_storage *frominetPrev, int *pbIsPermitted,
 
 		/* if we reach this point, we had a good receive and can process the packet received */
 		/* check if we have a different sender than before, if so, we need to query some new values */
-		if(memcmp(&frominet, frominetPrev, socklen) != 0) {
+		if(net.CmpHost(&frominet, frominetPrev, socklen) != 0) {
 			CHKiRet(net.cvthname(&frominet, fromHost, fromHostFQDN, fromHostIP));
 			memcpy(frominetPrev, &frominet, socklen); /* update cache indicator */
 			/* Here we check if a host is permitted to send us
diff --git a/runtime/net.c b/runtime/net.c
index db2d7e37..5cafe522 100644
--- a/runtime/net.c
+++ b/runtime/net.c
@@ -1496,6 +1496,49 @@ int *create_udp_socket(uchar *hostname, uchar *pszPort, int bIsServer)
 }
 
 
+/* check if two provided socket addresses point to the same host. Note that the
+ * length of the sockets must be provided as third parameter. This is necessary to
+ * compare non IPv4/v6 hosts, in which case we do a simple memory compare of the
+ * address structure (in that case, the same host may not reliably be detected).
+ * Note that we need to do the comparison not on the full structure, because it contains things
+ * like the port, which we do not need to look at when thinking about hostnames. So we look
+ * at the relevant fields, what means a somewhat more complicated processing.
+ * Also note that we use a non-standard calling interface, as this is much more natural and
+ * it looks extremely unlikely that we get an exception of any kind here. What we
+ * return is mimiced after memcmp(), and as such useful for building binary trees 
+ * (the order relation may be a bit arbritrary, but at least it is consistent).
+ * rgerhards, 2009-09-03
+ */
+static int CmpHost(struct sockaddr_storage *s1, struct sockaddr_storage* s2, size_t socklen)
+{
+	int ret;
+
+	if(((struct sockaddr*) s1)->sa_family != ((struct sockaddr*) s2)->sa_family) {
+		ret = memcmp(s1, s2, socklen);
+		goto finalize_it;
+	}
+
+	if(((struct sockaddr*) s1)->sa_family == AF_INET) {
+		if(((struct sockaddr_in *) s1)->sin_addr.s_addr == ((struct sockaddr_in*)s2)->sin_addr.s_addr) {
+			ret = 0;
+		} else if(((struct sockaddr_in *) s1)->sin_addr.s_addr < ((struct sockaddr_in*)s2)->sin_addr.s_addr) {
+			ret = -1;
+		} else {
+			ret = 1;
+		}
+	} else if(((struct sockaddr*) s1)->sa_family == AF_INET6) {
+		/* IPv6 addresses are always 16 octets long */
+		ret = memcmp(((struct sockaddr_in6 *)s1)->sin6_addr.s6_addr, ((struct sockaddr_in6*)s2)->sin6_addr.s6_addr, 16);
+	} else {
+		ret = memcmp(s1, s2, socklen);
+	}
+
+dbgprintf("CmpHost returns %d\n", ret);
+finalize_it:
+	return ret;
+}
+
+
 /* queryInterface function
  * rgerhards, 2008-03-05
  */
@@ -1524,6 +1567,7 @@ CODESTARTobjQueryInterface(net)
 	pIf->AddPermittedPeer = AddPermittedPeer;
 	pIf->DestructPermittedPeers = DestructPermittedPeers;
 	pIf->PermittedPeerWildcardMatch = PermittedPeerWildcardMatch;
+	pIf->CmpHost = CmpHost;
 finalize_it:
 ENDobjQueryInterface(net)
 
diff --git a/runtime/net.h b/runtime/net.h
index 092c3116..ec364b1c 100644
--- a/runtime/net.h
+++ b/runtime/net.h
@@ -146,11 +146,13 @@ BEGINinterface(net) /* name must also be changed in ENDinterface macro! */
 	rsRetVal (*AddPermittedPeer)(permittedPeers_t **ppRootPeer, uchar *pszID);
 	rsRetVal (*DestructPermittedPeers)(permittedPeers_t **ppRootPeer);
 	rsRetVal (*PermittedPeerWildcardMatch)(permittedPeers_t *pPeer, uchar *pszNameToMatch, int *pbIsMatching);
+	/* v5 interface additions */
+	int (*CmpHost)(struct sockaddr_storage *, struct sockaddr_storage*, size_t);
 	/* data members - these should go away over time... TODO */
 	int    *pACLAddHostnameOnFail; /* add hostname to acl when DNS resolving has failed */
 	int    *pACLDontResolve;       /* add hostname to acl instead of resolving it to IP(s) */
 ENDinterface(net)
-#define netCURR_IF_VERSION 4 /* increment whenever you change the interface structure! */
+#define netCURR_IF_VERSION 5 /* increment whenever you change the interface structure! */
 
 /* prototypes */
 PROTOTYPEObj(net);