Fix sockaddr comparison (#575)

This may fix #565

It is not recommended to compare two structs via memcmp although they have been
memset to 0.
Added two helper functions to compare sockaddr_storage, one for address only and
the other one takes port into account.

These two functions are based on Unbound version in util/net_help.c, and
comform needs of shadowsocks-libev.

Signed-off-by: Syrone Wong <wong.syrone@gmail.com>

src/netutils.c

@@ -46,6 +46,8 @@
 #define SO_REUSEPORT 15
 #endif
 
+extern int verbose;
+
 int set_reuseport(int socket)
 {
     int opt = 1;
@@ -132,3 +134,71 @@ ssize_t get_sockaddr(char *host, char *port, struct sockaddr_storage *storage, i
 
     return -1;
 }
+
+int sockaddr_cmp(struct sockaddr_storage* addr1,
+    struct sockaddr_storage* addr2, socklen_t len)
+{
+    struct sockaddr_in* p1_in = (struct sockaddr_in*)addr1;
+    struct sockaddr_in* p2_in = (struct sockaddr_in*)addr2;
+    struct sockaddr_in6* p1_in6 = (struct sockaddr_in6*)addr1;
+    struct sockaddr_in6* p2_in6 = (struct sockaddr_in6*)addr2;
+    if( p1_in->sin_family < p2_in->sin_family)
+        return -1;
+    if( p1_in->sin_family > p2_in->sin_family)
+        return 1;
+    if(verbose) {
+        LOGI("sockaddr_cmp: sin_family equal? %d", p1_in->sin_family == p2_in->sin_family );
+    }
+    /* compare ip4 */
+    if( p1_in->sin_family == AF_INET ) {
+        /* just order it, ntohs not required */
+        if(p1_in->sin_port < p2_in->sin_port)
+            return -1;
+        if(p1_in->sin_port > p2_in->sin_port)
+            return 1;
+        if(verbose) {
+            LOGI("sockaddr_cmp: sin_port equal? %d", p1_in->sin_port == p2_in->sin_port);
+        }
+        return memcmp(&p1_in->sin_addr, &p2_in->sin_addr, INET_SIZE);
+    } else if (p1_in6->sin6_family == AF_INET6) {
+        /* just order it, ntohs not required */
+        if(p1_in6->sin6_port < p2_in6->sin6_port)
+            return -1;
+        if(p1_in6->sin6_port > p2_in6->sin6_port)
+            return 1;
+        if(verbose) {
+            LOGI("sockaddr_cmp: sin6_port equal? %d", p1_in6->sin6_port == p2_in6->sin6_port);
+        }
+        return memcmp(&p1_in6->sin6_addr, &p2_in6->sin6_addr,
+            INET6_SIZE);
+    } else {
+        /* eek unknown type, perform this comparison for sanity. */
+        return memcmp(addr1, addr2, len);
+    }
+}
+
+int sockaddr_cmp_addr(struct sockaddr_storage* addr1,
+    struct sockaddr_storage* addr2, socklen_t len)
+{
+    struct sockaddr_in* p1_in = (struct sockaddr_in*)addr1;
+    struct sockaddr_in* p2_in = (struct sockaddr_in*)addr2;
+    struct sockaddr_in6* p1_in6 = (struct sockaddr_in6*)addr1;
+    struct sockaddr_in6* p2_in6 = (struct sockaddr_in6*)addr2;
+    if( p1_in->sin_family < p2_in->sin_family)
+        return -1;
+    if( p1_in->sin_family > p2_in->sin_family)
+        return 1;
+    if(verbose) {
+        LOGI("sockaddr_cmp_addr: sin_family equal? %d", p1_in->sin_family == p2_in->sin_family );
+    }
+    /* compare ip4 */
+    if( p1_in->sin_family == AF_INET ) {
+        return memcmp(&p1_in->sin_addr, &p2_in->sin_addr, INET_SIZE);
+    } else if (p1_in6->sin6_family == AF_INET6) {
+        return memcmp(&p1_in6->sin6_addr, &p2_in6->sin6_addr,
+            INET6_SIZE);
+    } else {
+        /* eek unknown type, perform this comparison for sanity. */
+        return memcmp(addr1, addr2, len);
+    }
+}

src/netutils.h

@@ -23,8 +23,34 @@
 #ifndef _NETUTILS_H
 #define _NETUTILS_H
 
+/** byte size of ip4 address */
+#define INET_SIZE 4
+/** byte size of ip6 address */
+#define INET6_SIZE 16
+
 size_t get_sockaddr_len(struct sockaddr *addr);
 ssize_t get_sockaddr(char *host, char *port, struct sockaddr_storage *storage, int block);
 int set_reuseport(int socket);
 
+/**
+ * Compare two sockaddrs. Imposes an ordering on the addresses.
+ * Compares address and port.
+ * @param addr1: address 1.
+ * @param addr2: address 2.
+ * @param len: lengths of addr.
+ * @return: 0 if addr1 == addr2. -1 if addr1 is smaller, +1 if larger.
+ */
+int sockaddr_cmp(struct sockaddr_storage* addr1,
+    struct sockaddr_storage* addr2, socklen_t len);
+
+/**
+ * Compare two sockaddrs. Compares address, not the port.
+ * @param addr1: address 1.
+ * @param addr2: address 2.
+ * @param len: lengths of addr.
+ * @return: 0 if addr1 == addr2. -1 if addr1 is smaller, +1 if larger.
+ */
+int sockaddr_cmp_addr(struct sockaddr_storage* addr1,
+    struct sockaddr_storage* addr2, socklen_t len);
+
 #endif

src/udprelay.c

@@ -1022,7 +1022,7 @@ static void server_recv_cb(EV_P_ ev_io *w, int revents)
     cache_lookup(conn_cache, key, HASH_KEY_LEN, (void *)&remote_ctx);
 
     if (remote_ctx != NULL) {
-        if (memcmp(&src_addr, &remote_ctx->src_addr, sizeof(src_addr))) {
+        if (sockaddr_cmp_addr(&src_addr, &remote_ctx->src_addr, sizeof(src_addr))) {
             remote_ctx = NULL;
         }
     }